counter_cache not decrementing on delete

This is especially annoying on has_many :through associations (given these associations):

class Tag; belongs_to :post, :counter_cache => true; belongs_to :tag_name; end
class TagNames; ...; end
class Post; has_many :tags; has_many :tag_names, :through => :tags; end

When a form submission directly assigns to @post.tag_names the counter get's increased correctly, but never decreases if one removes tags. Or clears all.

Here is the patch to fix this, tests for has many and HMT included.

And postgresql.

diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 1f8b297..d97f6f3 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -553,15 +553,16 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_deleting_updates_counter_cache
-    post = Post.first
+    topic = Topic.first
+    assert_equal topic.replies.to_a.size, topic.replies_count
 
-    post.comments.delete(post.comments.first)
-    post.reload
-    assert_equal post.comments(true).size, post.comments_count
+    topic.replies.delete(topic.replies.first)
+    topic.reload
+    assert_equal topic.replies.to_a.size, topic.replies_count
 
-    post.comments.delete(post.comments.first)
-    post.reload
-    assert_equal 0, post.comments_count
+    topic.replies.delete(topic.replies.first)
+    topic.reload
+    assert_equal topic.replies.to_a.size, topic.replies_count
   end
 
   def test_deleting_before_save
@@ -618,11 +619,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_clearing_updates_counter_cache
-    post = Post.first
+    topic = Topic.first
 
-    post.comments.clear
-    post.reload
-    assert_equal 0, post.comments_count
+    topic.replies.clear
+    topic.reload
+    assert_equal 0, topic.replies_count
   end
 
   def test_clearing_a_dependent_association_collection

Because comments.post_id is not null. This switches it to use topic + replies, but the test still fails. I'm reverting the original commit until this is resolved.

(from [5b290082cf91e130af4da989d6bafc69d8f2607a]) Revert "Fix: counter_cache should decrement on deleting associated records."

[#1196 state:open]

This reverts commit 757e4364dc3f808f0002a6c8cb03531e69e2f356. http://github.com/rails/rails/co...

(from [8fcd9cfb9f4e6856df3349a8235621768a565e17]) Revert "Fix: counter_cache should decrement on deleting associated records."

[#1196 state:open]

This reverts commit c9e176de78067c5941233b12686276d3016fbb38. http://github.com/rails/rails/co...

(from [b30ae1974851b20ef430df9de17e6e79e5b25ad2]) Revert "Fix: counter_cache should decrement on deleting associated records."

[#1196 state:open]

This reverts commit 05f2183747c8e75c9e8bbaadb9573b4bdf41ecfc. http://github.com/rails/rails/co...

Rebased the test cases against 2-3-stable and verified that they do indeed fail.

I've attached a test.

Okay, after integrating bitsweat's test changes it was revealed that the actual problem footprint is much smaller. (The original patch conflated a buggy test with a buggy fix). What I came up with is that counter_caches were broken when the association was deleted by nullifying the foreign key rather than destroying the record. I refactored and stripped down the test that shows this and moved it into the has_many test rather than has_many :through test_deleting_updates_counter_cache_without_dependent_destroy. I left the other two tests in place for posterity though they do not fail before the fix.

The actual fix was a one-liner.

I've attached a patch.

+1 verified, applied cleanly on master and 2-3-stable, all tests pass.

verified

+1 The patch works cleanly. All tests are passing as well.

(from [7e3364ac4634f7017305c4bc725710ab0e7ba4c2]) Fix that counter_cache breaks with has_many :dependent => :nullify.

[#1196 state:committed]

Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/7e3364ac4634f7017305c4bc725710...

Automatic cleanup of spam.

Automatic cleanup of spam.

louis vuitton watches