Improve consistency of counter caches updating in memory

When we made sure that the counter gets updated in memory, we only did
it on the has many side. The has many side only does the update if the
belongs to cannot. The belongs to side was updated to update the counter
cache (if it is able). This means that we need to check if the
belongs_to is able to update in memory on the has_many side.

We also found an inconsistency where the reflection names were used to
grab the association which should update the counter cache. Since
reflection names are now strings, this means it was using a different
instance than the one which would have the inverse instance set.

Fixes #18689

[Sean Griffin & anthonynavarre]

2 files changed, 18 insertions, 5 deletions

diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb
index c63b42e2a0..265a65c4c1 100644
--- a/activerecord/lib/active_record/associations/belongs_to_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_association.rb
@@ -68,6 +68,9 @@ module ActiveRecord
         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
           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 2a782c06d0..b574185561 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -101,7 +101,7 @@ module ActiveRecord
         end
 
         def update_counter_in_memory(difference, reflection = reflection())
-          if has_cached_counter?(reflection)
+          if counter_must_be_updated_by_has_many?(reflection)
             counter = cached_counter_attribute_name(reflection)
             owner[counter] += difference
             owner.send(:clear_attribute_changes, counter) # eww
@@ -118,18 +118,28 @@ module ActiveRecord
         #     it will be decremented twice.
         #
         # Hence this method.
-        def inverse_updates_counter_cache?(reflection = reflection())
+        def inverse_which_updates_counter_cache(reflection = reflection())
           counter_name = cached_counter_attribute_name(reflection)
-          inverse_updates_counter_named?(counter_name, reflection)
+          inverse_which_updates_counter_named(counter_name, reflection)
         end
+        alias inverse_updates_counter_cache? inverse_which_updates_counter_cache
 
-        def inverse_updates_counter_named?(counter_name, reflection = reflection())
-          reflection.klass._reflections.values.any? { |inverse_reflection|
+        def inverse_which_updates_counter_named(counter_name, reflection)
+          reflection.klass._reflections.values.find { |inverse_reflection|
             inverse_reflection.belongs_to? &&
             inverse_reflection.counter_cache_column == counter_name
           }
         end
 
+        def inverse_updates_counter_in_memory?(reflection)
+          inverse = inverse_which_updates_counter_cache(reflection)
+          inverse && inverse == reflection.inverse_of
+        end
+
+        def counter_must_be_updated_by_has_many?(reflection)
+          !inverse_updates_counter_in_memory?(reflection) && has_cached_counter?(reflection)
+        end
+
         def delete_count(method, scope)
           if method == :delete_all
             scope.delete_all