Permalink
Browse files

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
  • Loading branch information...
dhh committed May 21, 2006
1 parent 59c8c63 commit 66586bc168f358e3385416def6eb30ad1242ffe6
View
@@ -1,5 +1,7 @@
*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]
@@ -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
@@ -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)
@@ -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?

0 comments on commit 66586bc

Please sign in to comment.