Changeset 8504

Timestamp:

12/28/07 18:01:22 (1 year ago)

Author:

rick

Message:

Don't unnecessarily load has_many associations in after_update callbacks. Closes #6822 [stopdropandrew, canadaduane]

Files:

• trunk/activerecord/CHANGELOG (modified) (1 diff)
• trunk/activerecord/lib/active_record/associations.rb (modified) (2 diffs)
• trunk/activerecord/test/associations_test.rb (modified) (6 diffs)

trunk/activerecord/CHANGELOG

@@ -1 +1 @@
-*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]

trunk/activerecord/lib/active_record/associations.rb

@@ -1080 +1080 @@
-              if new_record?
-                association
+              elsif association.loaded?
+                association.select { |record| record.new_record? }
-              else
-
+target.
-              end.each do |record|
-                errors.add "#{association_name}" unless record.valid?
@@ -1098 +1100 @@
-            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
-              []

trunk/activerecord/test/associations_test.rb

@@ -73 +73 @@
-  end
-end
-
+
-class AssociationProxyTest < Test::Unit::TestCase
-  fixtures :authors, :posts, :categorizations, :categories, :developers, :projects, :developers_projects
-
+
-  def test_proxy_accessors
-    welcome = posts(:welcome)
@@ -83 +83 @@
-    welcome.author.class  # force load target
-    assert_equal  welcome.author, welcome.author.proxy_target
-
+
-    david = authors(:david)
-    assert_equal  david, david.posts.proxy_owner
@@ -89 +89 @@
-    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
@@ -99 +99 @@
-    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
-
@@ -773 +791 @@
-    assert_equal 3, companies(:first_firm).clients_of_firm(true).size
-  end
-
+
+
+
+
+
+
+
-  def test_build_without_loading_association
-    first_topic = topics(:first)
@@ -824 +848 @@
-    companies(:first_firm).clients_of_firm.create([{"name" => "Another Client"}, {"name" => "Another Client II"}])
-    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
-