Pushing a record on an association collection doesn't unnecessarily load all the associated records.

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

4 files changed, 15 insertions, 10 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 357a88a6d4..c5c6925e24 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Pushing a record on an association collection doesn't unnecessarily load all the associated records.  [Obie Fernandez, Jeremy Kemper]
+
 * Oracle: fix connection reset failure.  #6846 [leonlleslie]
 
 * Subclass instantiation doesn't try to explicitly require the corresponding subclass.  #6840 [leei, Jeremy Kemper]
diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index d3b76e2087..bda72637c3 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -7,7 +7,7 @@ module ActiveRecord
         load_target
         @target.to_ary
       end
-  
+
       def reset
         reset_target!
         @loaded = false
@@ -17,7 +17,6 @@ module ActiveRecord
       # Since << flattens its argument list and inserts each record, +push+ and +concat+ behave identically.
       def <<(*records)
         result = true
-        load_target
 
         @owner.transaction do
           flatten_deeper(records).each do |record|
@@ -34,7 +33,7 @@ module ActiveRecord
 
       alias_method :push, :<<
       alias_method :concat, :<<
-                      
+
       # Remove all records from this association
       def delete_all
         load_target
@@ -103,7 +102,7 @@ module ActiveRecord
       # calling collection.size if it has. If it's more likely than not that the collection does have a size larger than zero
       # and you need to fetch that collection afterwards, it'll take one less SELECT query if you use length.
       def size
-        if loaded? && !@reflection.options[:uniq]
+        if @owner.new_record? || (loaded? && !@reflection.options[:uniq])
           @target.size
         elsif !loaded? && !@reflection.options[:uniq] && @target.is_a?(Array)
           unsaved_records = Array(@target.detect { |r| r.new_record? })
diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb
index 511653dfce..c5d89bb216 100755
--- a/activerecord/test/associations_test.rb
+++ b/activerecord/test/associations_test.rb
@@ -637,12 +637,15 @@ class HasManyAssociationsTest < Test::Unit::TestCase
   def test_adding_before_save
     no_of_firms = Firm.count
     no_of_clients = Client.count
+
     new_firm = Firm.new("name" => "A New Firm, Inc")
+    c = Client.new("name" => "Apple")
+
     new_firm.clients_of_firm.push Client.new("name" => "Natural Company")
-    new_firm.clients_of_firm << (c = Client.new("name" => "Apple"))
-    assert new_firm.new_record?
-    assert c.new_record?
+    assert_equal 1, new_firm.clients_of_firm.size
+    new_firm.clients_of_firm << c
     assert_equal 2, new_firm.clients_of_firm.size
+
     assert_equal no_of_firms, Firm.count      # Firm was not saved to database.
     assert_equal no_of_clients, Client.count  # Clients were not saved to database.
     assert new_firm.save
@@ -651,6 +654,7 @@ class HasManyAssociationsTest < Test::Unit::TestCase
     assert_equal new_firm, c.firm
     assert_equal no_of_firms+1, Firm.count      # Firm was saved to database.
     assert_equal no_of_clients+2, Client.count  # Clients were saved to database.
+
     assert_equal 2, new_firm.clients_of_firm.size
     assert_equal 2, new_firm.clients_of_firm(true).size
   end
diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb
index bd9666bc79..3335ab6019 100755
--- a/activerecord/test/base_test.rb
+++ b/activerecord/test/base_test.rb
@@ -1288,12 +1288,12 @@ class BasicsTest < Test::Unit::TestCase
      client_new      = Client.new
      client_new.name = "The Joneses"
      clients         = [ client_stored, client_new ]
-     
+
      firm.clients    << clients
+     assert_equal clients.map(&:name).to_set, firm.clients.map(&:name).to_set
 
      firm.clear_association_cache
-
-     assert_equal    firm.clients.collect{ |x| x.name }.sort, clients.collect{ |x| x.name }.sort
+     assert_equal clients.map(&:name).to_set, firm.clients.map(&:name).to_set
   end
 
   def test_interpolate_sql