From 0e3a54a3b92c0f9a7bf95f8cb11ba2db89f6eecc Mon Sep 17 00:00:00 2001 From: Rick Olson Date: Fri, 28 Dec 2007 18:01:22 +0000 Subject: Don't unnecessarily load has_many associations in after_update callbacks. Closes #6822 [stopdropandrew, canadaduane] git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@8504 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- activerecord/CHANGELOG | 2 ++ activerecord/lib/active_record/associations.rb | 6 +++- activerecord/test/associations_test.rb | 40 ++++++++++++++++++++++---- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 450f3821e8..b56a272b24 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Don't unnecessarily load has_many associations in after_update callbacks. Closes #6822 [stopdropandrew, canadaduane] + * Eager belongs_to :include infers the foreign key from the association name rather than the class name. #10517 [Jonathan Viney] * SQLite: fix rename_ and remove_column for columns with unique indexes. #10576 [Brandon Keepers] diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 3b2c6b7a59..deadba7418 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1079,8 +1079,10 @@ module ActiveRecord if association.respond_to?(:loaded?) if new_record? association - else + elsif association.loaded? association.select { |record| record.new_record? } + else + association.target.select { |record| record.new_record? } end.each do |record| errors.add "#{association_name}" unless record.valid? end @@ -1097,6 +1099,8 @@ module ActiveRecord association elsif association.respond_to?(:loaded?) && association.loaded? association.select { |record| record.new_record? } + elsif association.respond_to?(:loaded?) && !association.loaded? + association.target.select { |record| record.new_record? } else [] end diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb index 88b9020753..7773b2bfb0 100755 --- a/activerecord/test/associations_test.rb +++ b/activerecord/test/associations_test.rb @@ -72,23 +72,23 @@ class AssociationsTest < Test::Unit::TestCase end end end - + class AssociationProxyTest < Test::Unit::TestCase fixtures :authors, :posts, :categorizations, :categories, :developers, :projects, :developers_projects - + def test_proxy_accessors welcome = posts(:welcome) assert_equal welcome, welcome.author.proxy_owner assert_equal welcome.class.reflect_on_association(:author), welcome.author.proxy_reflection welcome.author.class # force load target assert_equal welcome.author, welcome.author.proxy_target - + david = authors(:david) assert_equal david, david.posts.proxy_owner assert_equal david.class.reflect_on_association(:posts), david.posts.proxy_reflection david.posts.first # force load target assert_equal david.posts, david.posts.proxy_target - + assert_equal david, david.posts_with_extension.testing_proxy_owner assert_equal david.class.reflect_on_association(:posts_with_extension), david.posts_with_extension.testing_proxy_reflection david.posts_with_extension.first # force load target @@ -98,10 +98,28 @@ class AssociationProxyTest < Test::Unit::TestCase def test_push_does_not_load_target david = authors(:david) + david.posts << (post = Post.new(:title => "New on Edge", :body => "More cool stuff!")) + assert !david.posts.loaded? + assert david.posts.include?(post) + end + + def test_push_has_many_through_does_not_load_target + david = authors(:david) + david.categories << categories(:technology) assert !david.categories.loaded? assert david.categories.include?(categories(:technology)) end + + def test_push_followed_by_save_does_not_load_target + david = authors(:david) + + david.posts << (post = Post.new(:title => "New on Edge", :body => "More cool stuff!")) + assert !david.posts.loaded? + david.save + assert !david.posts.loaded? + assert david.posts.include?(post) + end def test_push_does_not_lose_additions_to_new_record josh = Author.new(:name => "Josh") @@ -772,7 +790,13 @@ class HasManyAssociationsTest < Test::Unit::TestCase assert companies(:first_firm).save assert_equal 3, companies(:first_firm).clients_of_firm(true).size end - + + def test_build_followed_by_save_does_not_load_target + new_client = companies(:first_firm).clients_of_firm.build("name" => "Another Client") + assert companies(:first_firm).save + assert !companies(:first_firm).clients_of_firm.loaded? + end + def test_build_without_loading_association first_topic = topics(:first) Reply.column_names @@ -825,6 +849,12 @@ class HasManyAssociationsTest < Test::Unit::TestCase assert_equal 3, companies(:first_firm).clients_of_firm(true).size end + def test_create_followed_by_save_does_not_load_target + new_client = companies(:first_firm).clients_of_firm.create("name" => "Another Client") + assert companies(:first_firm).save + assert !companies(:first_firm).clients_of_firm.loaded? + end + def test_find_or_initialize the_client = companies(:first_firm).clients.find_or_initialize_by_name("Yet another client") assert_equal companies(:first_firm).id, the_client.firm_id -- cgit v1.2.3