diff options
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 |