Make #increment! and #decrement! methods concurency safe

2 files changed, 21 insertions, 35 deletions

diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb
index 260a0c6a2d..41698c5360 100644
--- a/activerecord/lib/active_record/associations/belongs_to_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_association.rb
@@ -10,7 +10,7 @@ module ActiveRecord
       def replace(record)
         if record
           raise_on_type_mismatch!(record)
-          update_counters(record)
+          update_counters_on_replace(record)
           replace_keys(record)
           set_inverse_instance(record)
           @updated = true
@@ -32,45 +32,37 @@ module ActiveRecord
       end
 
       def decrement_counters # :nodoc:
-        with_cache_name { |name| decrement_counter name }
+        update_counters(-1)
       end
 
       def increment_counters # :nodoc:
-        with_cache_name { |name| increment_counter name }
+        update_counters(1)
       end
 
       private
 
-        def find_target?
-          !loaded? && foreign_key_present? && klass
-        end
-
-        def with_cache_name
-          counter_cache_name = reflection.counter_cache_column
-          return unless counter_cache_name && owner.persisted?
-          yield counter_cache_name
+        def update_counters(by)
+          if require_counter_update? && foreign_key_present?
+            if target && !stale_target?
+              target.increment!(reflection.counter_cache_column, by)
+            else
+              klass.update_counters(target_id, reflection.counter_cache_column => by)
+            end
+          end
         end
 
-        def update_counters(record)
-          with_cache_name do |name|
-            return unless different_target? record
-            record.class.increment_counter(name, record.id)
-            decrement_counter name
-          end
+        def find_target?
+          !loaded? && foreign_key_present? && klass
         end
 
-        def decrement_counter(counter_cache_name)
-          if foreign_key_present?
-            klass.decrement_counter(counter_cache_name, target_id)
-          end
+        def require_counter_update?
+          reflection.counter_cache_column && owner.persisted?
         end
 
-        def increment_counter(counter_cache_name)
-          if foreign_key_present?
-            klass.increment_counter(counter_cache_name, target_id)
-            if target && !stale_target?
-              target.increment(counter_cache_name)
-            end
+        def update_counters_on_replace(record)
+          if require_counter_update? && different_target?(record)
+            record.increment!(reflection.counter_cache_column)
+            decrement_counters
           end
         end
 
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index 7da20d8eea..a9f6aaafef 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -88,21 +88,15 @@ module ActiveRecord
         end
 
         def update_counter(difference, reflection = reflection())
-          update_counter_in_database(difference, reflection)
-          update_counter_in_memory(difference, reflection)
-        end
-
-        def update_counter_in_database(difference, reflection = reflection())
           if reflection.has_cached_counter?
-            owner.class.update_counters(owner.id, reflection.counter_cache_column => difference)
+            owner.increment!(reflection.counter_cache_column, difference)
           end
         end
 
         def update_counter_in_memory(difference, reflection = reflection())
           if reflection.counter_must_be_updated_by_has_many?
             counter = reflection.counter_cache_column
-            owner[counter] ||= 0
-            owner[counter] += difference
+            owner.increment(counter, difference)
             owner.send(:clear_attribute_change, counter) # eww
           end
         end