Use database lock to ensure data integrity on counter caches.

Brian Durand · Brian Durand · commit db74fcd7a610 · 2012-10-19T09:53:03.000-07:00
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
@@ -910,4 +910,8 @@
 
     *Aaron Patterson*
 
+*   Use database lock to ensure data integrity on counter caches.
+
+    *Brian Durand*
+
 Please check [3-2-stable](https://github.com/rails/rails/blob/3-2-stable/activerecord/CHANGELOG.md) for previous changes.
diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb
@@ -31,7 +31,9 @@ def belongs_to_counter_cache_after_create_for_#{name}
         def belongs_to_counter_cache_before_destroy_for_#{name}
           unless marked_for_destruction?
             record = #{name}
-            record.class.decrement_counter(:#{cache_column}, record.id) unless record.nil?
+            if record && self.class.obtain_lock(id)
+              record.class.decrement_counter(:#{cache_column}, record.id)
+            end
           end
         end
       CODE
diff --git a/activerecord/lib/active_record/locking/pessimistic.rb b/activerecord/lib/active_record/locking/pessimistic.rb
@@ -54,6 +54,17 @@ module Locking
     #   MySQL: http://dev.mysql.com/doc/refman/5.1/en/innodb-locking-reads.html
     #   PostgreSQL: http://www.postgresql.org/docs/current/interactive/sql-select.html#SQL-FOR-UPDATE-SHARE
     module Pessimistic
+      extend ActiveSupport::Concern
+
+      module ClassMethods
+        # Obtain a row level lock on the row with the specified primary key value (if row locking is supported).
+        # Returns +true+ if the row exists or +false+ if the row does not exist.
+        def obtain_lock(id, lock = true)
+          sql = where(primary_key => id).select(primary_key).lock(lock).to_sql
+          !!connection.select_one(sql)
+        end
+      end
+
       # Obtain a row lock on this record. Reloads the record to obtain the requested
       # lock. Pass an SQL locking clause to append the end of the SELECT statement
       # or pass true for "FOR UPDATE" (the default, an exclusive row lock). Returns
diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -393,6 +393,20 @@ def test_custom_counter_cache
     assert_equal 17, reply.replies.size
   end
 
+  def test_counter_cache_with_destroy
+    topic = Topic.create!(:title => "Zoom-zoom-zoom")
+    topic.replies.create!(:title => "re: zoom", :content => "speedy quick!")
+
+    assert_equal 1, topic.reload[:replies_count]
+
+    reply_1 = Reply.find(topic.replies.first.id)
+    reply_2 = Reply.find(topic.replies.first.id)
+    reply_1.destroy
+    reply_2.destroy
+
+    assert_equal 0, topic.reload[:replies_count]
+  end
+
   def test_association_assignment_sticks
     post = Post.first
 
diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb
@@ -408,6 +408,11 @@ def test_with_lock_rolls_back_transaction
       assert_equal old, person.reload.first_name
     end
 
+    def test_obtain_lock
+      assert Person.obtain_lock(1)
+      assert !Person.obtain_lock(0)
+    end
+
     if current_adapter?(:PostgreSQLAdapter, :OracleAdapter)
       def test_no_locks_no_wait
         first, second = duel { Person.find 1 }
