Ensure HABTM#create and HABTM#build do not load entire association. [Pratik]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@9229 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

5 files changed, 48 insertions, 34 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 6296d9c4f9..4cfd554796 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Ensure HABTM#create and HABTM#build do not load entire association. [Pratik]
+
 * Improve documentation. [Xavier Noria, Jack Danger Canty, leethal]
 
 * Tweak ActiveRecord::Base#to_json to include a root value in the returned hash: {"post": {"title": ...}} [rick]
diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 141fdcab3d..ce1c8a262d 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -166,6 +166,25 @@ module ActiveRecord
       end
 
       protected
+        def load_target
+          if !@owner.new_record? || foreign_key_present
+            begin
+              if !loaded?
+                if @target.is_a?(Array) && @target.any?
+                  @target = find_target + @target.find_all {|t| t.new_record? }
+                else
+                  @target = find_target
+                end
+              end
+            rescue ActiveRecord::RecordNotFound
+              reset
+            end
+          end
+
+          loaded if target
+          target
+        end
+        
         def method_missing(method, *args)
           if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method))
             if block_given?
diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
index b06b618d5a..0edb2397ee 100644
--- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -7,7 +7,6 @@ module ActiveRecord
       end
 
       def build(attributes = {})
-        load_target
         build_record(attributes)
       end
 
@@ -154,7 +153,6 @@ module ActiveRecord
           if attributes.is_a?(Array)
             attributes.collect { |attr| create(attr) }
           else
-            load_target
             build_record(attributes, &block)
           end
         end
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index eedffa052b..de6b843098 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -68,25 +68,6 @@ module ActiveRecord
       end
 
       protected
-        def load_target
-          if !@owner.new_record? || foreign_key_present
-            begin
-              if !loaded?
-                if @target.is_a?(Array) && @target.any?
-                  @target = (find_target + @target).uniq
-                else
-                  @target = find_target
-                end
-              end
-            rescue ActiveRecord::RecordNotFound
-              reset
-            end
-          end
-
-          loaded if target
-          target
-        end
-
         def count_records
           count = if has_cached_counter?
             @owner.send(:read_attribute, cached_counter_attribute_name)
diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb
index 3e2b104a13..c302eda131 100755
--- a/activerecord/test/cases/associations_test.rb
+++ b/activerecord/test/cases/associations_test.rb
@@ -872,21 +872,25 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_build
-    new_client = companies(:first_firm).clients_of_firm.build("name" => "Another Client")
+    company = companies(:first_firm)
+    new_client = assert_no_queries { company.clients_of_firm.build("name" => "Another Client") }
+    assert !company.clients_of_firm.loaded?
+    
     assert_equal "Another Client", new_client.name
     assert new_client.new_record?
-    assert_equal new_client, companies(:first_firm).clients_of_firm.last
-    assert companies(:first_firm).save
+    assert_equal new_client, company.clients_of_firm.last
+    assert_queries(2) { assert company.save }
     assert !new_client.new_record?
-    assert_equal 2, companies(:first_firm).clients_of_firm(true).size
+    assert_equal 2, company.clients_of_firm(true).size
   end
 
   def test_build_many
-    new_clients = companies(:first_firm).clients_of_firm.build([{"name" => "Another Client"}, {"name" => "Another Client II"}])
+    company = companies(:first_firm)
+    new_clients = assert_no_queries { company.clients_of_firm.build([{"name" => "Another Client"}, {"name" => "Another Client II"}]) }
+    
     assert_equal 2, new_clients.size
-
-    assert companies(:first_firm).save
-    assert_equal 3, companies(:first_firm).clients_of_firm(true).size
+    assert_queries(3) { assert company.save }
+    assert_equal 3, company.clients_of_firm(true).size
   end
 
   def test_build_followed_by_save_does_not_load_target
@@ -1908,8 +1912,12 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
 
   def test_build
     devel = Developer.find(1)
-    proj = devel.projects.build("name" => "Projekt")
+    proj = assert_no_queries { devel.projects.build("name" => "Projekt") }
+    assert !devel.projects.loaded?
+    
     assert_equal devel.projects.last, proj
+    assert devel.projects.loaded?
+    
     assert proj.new_record?
     devel.save
     assert !proj.new_record?
@@ -1933,7 +1941,11 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
   def test_create
     devel = Developer.find(1)
     proj = devel.projects.create("name" => "Projekt")
+    assert !devel.projects.loaded?
+    
     assert_equal devel.projects.last, proj
+    assert devel.projects.loaded?
+    
     assert !proj.new_record?
     assert_equal Developer.find(1).projects.sort_by(&:id).last, proj  # prove join table is updated
   end
@@ -1964,10 +1976,12 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_uniq_after_the_fact
-    developers(:jamis).projects << projects(:active_record)
-    developers(:jamis).projects << projects(:active_record)
-    assert_equal 3, developers(:jamis).projects.size
-    assert_equal 1, developers(:jamis).projects.uniq.size
+    dev = developers(:jamis)
+    dev.projects << projects(:active_record)
+    dev.projects << projects(:active_record)
+    
+    assert_equal 3, dev.projects.size
+    assert_equal 1, dev.projects.uniq.size
   end
 
   def test_uniq_before_the_fact