From 9d08a07c48274686f8bd21e590249bfe3865df89 Mon Sep 17 00:00:00 2001
From: Michael Koziarski <michael@koziarski.com>
Date: Thu, 26 Apr 2007 00:18:38 +0000
Subject: Improve Performance of calling create on has_many :through
 associations by avoiding loading the target collection. Closes #8150 [evan]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6581 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
---
 activerecord/CHANGELOG                                   |  2 ++
 .../associations/has_many_through_association.rb         | 12 +++++-------
 activerecord/test/associations_test.rb                   | 16 ++++++++++++++--
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index da7572149c..e017ade603 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Improve performance of calling .create on has_many :through associations. [evan]
+
 * Improved cloning performance by relying less on exception raising #8159 [Blaine]
 
 * Added ActiveRecord::Base.inspect to return a column-view like #<Post id:integer, title:string, body:text> [DHH]
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index a8ad52bef9..93f1b2ee6a 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -51,8 +51,6 @@ module ActiveRecord
         through = @reflection.through_reflection
         raise ActiveRecord::HasManyThroughCantAssociateNewRecords.new(@owner, through) if @owner.new_record?
 
-        load_target
-
         klass = through.klass
         klass.transaction do
           flatten_deeper(records).each do |associate|
@@ -60,7 +58,7 @@ module ActiveRecord
             raise ActiveRecord::HasManyThroughCantAssociateNewRecords.new(@owner, through) unless associate.respond_to?(:new_record?) && !associate.new_record?
 
             @owner.send(@reflection.through_reflection.name).proxy_target << klass.with_scope(:create => construct_join_attributes(associate)) { klass.create! }
-            @target << associate
+            @target << associate if loaded?
           end
         end
 
@@ -138,11 +136,11 @@ module ActiveRecord
 
         # Construct attributes for :through pointing to owner and associate.
         def construct_join_attributes(associate)
-          returning construct_owner_attributes(@reflection.through_reflection).merge(@reflection.source_reflection.primary_key_name => associate.id) do |join_attributes|
-            if @reflection.options[:source_type]
-              join_attributes.merge!(@reflection.source_reflection.options[:foreign_type] => associate.class.base_class.name.to_s)
-            end
+          join_attributes = construct_owner_attributes(@reflection.through_reflection).merge(@reflection.source_reflection.primary_key_name => associate.id)
+          if @reflection.options[:source_type]
+            join_attributes.merge!(@reflection.source_reflection.options[:foreign_type] => associate.class.base_class.name.to_s)
           end
+          join_attributes
         end
 
         # Associate attributes pointing to owner, quoted.
diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb
index 250fa1027c..aec6a47583 100755
--- a/activerecord/test/associations_test.rb
+++ b/activerecord/test/associations_test.rb
@@ -7,6 +7,7 @@ require 'fixtures/reply'
 require 'fixtures/computer'
 require 'fixtures/customer'
 require 'fixtures/order'
+require 'fixtures/categorization'
 require 'fixtures/category'
 require 'fixtures/post'
 require 'fixtures/author'
@@ -69,7 +70,7 @@ class AssociationsTest < Test::Unit::TestCase
 end
 
 class AssociationProxyTest < Test::Unit::TestCase
-  fixtures :authors, :posts
+  fixtures :authors, :posts, :categorizations, :categories
   
   def test_proxy_accessors
     welcome = posts(:welcome)
@@ -90,6 +91,17 @@ class AssociationProxyTest < Test::Unit::TestCase
     assert_equal  david.posts_with_extension, david.posts_with_extension.testing_proxy_target
   end
 
+  def test_push_does_not_load_target
+    david = authors(:david)
+    not_loaded_string = '<categories not loaded yet>'
+    not_loaded_re = Regexp.new(not_loaded_string)
+    
+    david.categories << categories(:technology)
+    assert_match not_loaded_re, david.inspect
+    assert_equal not_loaded_string, david.categories.inspect
+    assert david.categories.include?(categories(:technology))
+  end
+  
   def test_inspect_does_not_load_target
     david = authors(:david)
     not_loaded_string = '<posts not loaded yet>'
@@ -660,7 +672,7 @@ class HasManyAssociationsTest < Test::Unit::TestCase
     assert_equal 2, new_firm.clients_of_firm.size
     assert_equal 2, new_firm.clients_of_firm(true).size
   end
-
+  
   def test_invalid_adding
     firm = Firm.find(1)
     assert !(firm.clients_of_firm << c = Client.new)
-- 
cgit v1.2.3