Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #14735 from byroot/idempotent-counter-caches

Idempotent counter caches, fix concurrency issues with counter caches
  • Loading branch information...
commit a1e2db2e9bb4ca2fdf6190aa8f448fe85cf76529 2 parents 3bf8f8b + 655a366
@tenderlove tenderlove authored
View
9 activerecord/lib/active_record/associations/builder/belongs_to.rb
@@ -37,13 +37,14 @@ def belongs_to_counter_cache_after_create(reflection)
end
end
- def belongs_to_counter_cache_before_destroy(reflection)
+ def belongs_to_counter_cache_after_destroy(reflection)
foreign_key = reflection.foreign_key.to_sym
unless destroyed_by_association && destroyed_by_association.foreign_key.to_sym == foreign_key
record = send reflection.name
- if record && !self.destroyed?
+ if record && self.actually_destroyed?
cache_column = reflection.counter_cache_column
record.class.decrement_counter(cache_column, record.id)
+ self.clear_destroy_state
end
end
end
@@ -77,8 +78,8 @@ def self.add_counter_cache_callbacks(model, reflection)
record.belongs_to_counter_cache_after_create(reflection)
}
- model.before_destroy lambda { |record|
- record.belongs_to_counter_cache_before_destroy(reflection)
+ model.after_destroy lambda { |record|
+ record.belongs_to_counter_cache_after_destroy(reflection)
}
model.after_update lambda { |record|
View
21 activerecord/lib/active_record/counter_cache.rb
@@ -118,5 +118,26 @@ def decrement_counter(counter_name, id)
update_counters(id, counter_name => -1)
end
end
+
+ protected
+
+ def actually_destroyed?
+ @_actually_destroyed
+ end
+
+ def clear_destroy_state
+ @_actually_destroyed = nil
+ end
+
+ private
+
+ def destroy_row
+ affected_rows = super
+
+ @_actually_destroyed = affected_rows > 0
+
+ affected_rows
+ end
+
end
end
View
21 activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -502,6 +502,27 @@ def test_counter_cache_double_destroy
assert_equal 4, topic.replies.size
end
+ def test_concurrent_counter_cache_double_destroy
+ topic = Topic.create :title => "Zoom-zoom-zoom"
+
+ 5.times do
+ topic.replies.create(:title => "re: zoom", :content => "speedy quick!")
+ end
+
+ assert_equal 5, topic.reload[:replies_count]
+ assert_equal 5, topic.replies.size
+
+ reply = topic.replies.first
+ reply_clone = Reply.find(reply.id)
+
+ reply.destroy
+ assert_equal 4, topic.reload[:replies_count]
+
+ reply_clone.destroy
+ assert_equal 4, topic.reload[:replies_count]
+ assert_equal 4, topic.replies.size
+ end
+
def test_custom_counter_cache
reply = Reply.create(:title => "re: zoom", :content => "speedy quick!")
assert_equal 0, reply[:replies_count]
Please sign in to comment.
Something went wrong with that request. Please try again.