Fixed that has_many collections shouldn't load the entire association to do build or create [DHH]

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

activerecord/CHANGELOG

 *SVN*
 
+* Fixed that has_many collections shouldn't load the entire association to do build or create [DHH]
+
 * Added :allow_nil option for aggregations #5091 [ian.w.white@gmail.com]
 
 * Fix Oracle boolean support and tests. Closes #5139. [schoenm@earthlink.net]

activerecord/lib/active_record/associations/association_collection.rb

@@ -95,7 +95,14 @@ def create(attributes = {})
       # 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] then @target.size else count_records end
+        if loaded? && !@reflection.options[:uniq]
+          @target.size
+        elsif !loaded? && !@reflection.options[:uniq] && @target.is_a?(Array)
+          unsaved_records = Array(@target.detect { |r| r.new_record? }).size
+          unsaved_records + count_records
+        else
+          count_records
+        end
       end
 
       # Returns the size of the collection by loading it and calling size on the array. If you want to use this method to check

activerecord/lib/active_record/associations/has_many_association.rb

@@ -10,10 +10,16 @@ def build(attributes = {})
         if attributes.is_a?(Array)
           attributes.collect { |attr| build(attr) }
         else
-          load_target
           record = @reflection.klass.new(attributes)
           set_belongs_to_association_for(record)
-          @target << record
+          
+          if loaded?
+            @target << record
+          else
+            @target ||= []
+            @target << record
+          end
+          
           record
         end
       end
@@ -107,6 +113,25 @@ def method_missing(method, *args, &block)
           end
         end
 
+        def load_target
+          if !@owner.new_record? || foreign_key_present
+            begin
+              if !loaded?
+                if @target.is_a?(Array) && @target.any?
+                  @target = find_target + @target
+                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)

activerecord/test/associations_test.rb

@@ -512,6 +512,35 @@ def test_build_many
     assert_equal 3, companies(:first_firm).clients_of_firm(true).size
   end
 
+  def test_build_without_loading_association
+    first_topic = Topic.find(:first)
+    Reply.column_names
+
+    assert_equal 1, first_topic.replies.length
+    
+    assert_no_queries do
+      first_topic.replies.build(:title => "Not saved", :content => "Superstars")
+      assert_equal 2, first_topic.replies.size
+    end
+
+    assert_equal 2, first_topic.replies.to_ary.size
+  end
+
+  def test_create_without_loading_association
+    first_firm  = companies(:first_firm)
+    Firm.column_names
+    Client.column_names
+
+    assert_equal 1, first_firm.clients_of_firm.size
+    first_firm.clients_of_firm.reset
+    
+    assert_queries(1) do
+      first_firm.clients_of_firm.create(:name => "Superstars")
+    end
+    
+    assert_equal 2, first_firm.clients_of_firm.size
+  end
+
   def test_invalid_build
     new_client = companies(:first_firm).clients_of_firm.build
     assert new_client.new_record?