Merge pull request rails#43448 from composerinteralia/stale-inversed-target

Make inversed association stale after updating id

Author: eileencodes

activerecord/lib/active_record/associations/association.rb

@@ -52,7 +52,6 @@ def reset
         @loaded = false
         @target = nil
         @stale_state = nil
-        @inversed = false
       end
 
       def reset_negative_cache # :nodoc:
@@ -78,7 +77,6 @@ def loaded?
       def loaded!
         @loaded = true
         @stale_state = stale_state
-        @inversed = false
       end
 
       # The target is stale if the target no longer points to the record(s) that the
@@ -88,7 +86,7 @@ def loaded!
       #
       # Note that if the target has not been loaded, it is not considered stale.
       def stale_target?
-        !@inversed && loaded? && @stale_state != stale_state
+        loaded? && @stale_state != stale_state
       end
 
       # Sets the target of this association to <tt>\target</tt>, and the \loaded flag to +true+.
@@ -137,15 +135,11 @@ def remove_inverse_instance(record)
 
       def inversed_from(record)
         self.target = record
-        @inversed = !!record
       end
 
       def inversed_from_queries(record)
         if inversable?(record)
           self.target = record
-          @inversed = true
-        else
-          @inversed = false
         end
       end
 

activerecord/lib/active_record/autosave_association.rb

@@ -404,6 +404,8 @@ def save_collection_association(reflection)
               saved = true
 
               if autosave != false && (new_record_before_save || record.new_record?)
+                association.set_inverse_instance(record)
+
                 if autosave
                   saved = association.insert_record(record, false)
                 elsif !reflection.nested?
@@ -447,9 +449,7 @@ def save_has_one_association(reflection)
             if (autosave && record.changed_for_autosave?) || record_changed?(reflection, record, key)
               unless reflection.through_reflection
                 record[reflection.foreign_key] = key
-                if inverse_reflection = reflection.inverse_of
-                  record.association(inverse_reflection.name).inversed_from(self)
-                end
+                association.set_inverse_instance(record)
               end
 
               saved = record.save(validate: !autosave)

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -1440,7 +1440,7 @@ def test_calling_update_on_id_changes_the_counter_cache
     assert_equal original_count, topic.reload.replies_count
   end
 
-  def test_calling_update_changing_ids_doesnt_change_counter_cache
+  def test_calling_update_changing_ids_changes_the_counter_cache
     topic1 = Topic.find(1)
     topic2 = Topic.find(3)
     original_count1 = topic1.replies.to_a.size
@@ -1458,6 +1458,24 @@ def test_calling_update_changing_ids_doesnt_change_counter_cache
     assert_equal original_count2, topic2.reload.replies_count
   end
 
+  def test_calling_update_changing_ids_of_inversed_association_changes_the_counter_cache
+    assert_predicate Post.reflect_on_association(:comments), :has_inverse?
+
+    post1 = Post.first
+    post2 = Post.second
+
+    original_count1 = post1.comments.count
+    original_count2 = post2.comments.count
+
+    post1.comments.first.update(post_id: post2.id)
+    assert_equal original_count1 - 1, post1.reload.comments_count
+    assert_equal original_count2 + 1, post2.reload.comments_count
+
+    post2.comments.first.update(post_id: post1.id)
+    assert_equal original_count1, post1.reload.comments_count
+    assert_equal original_count2, post2.reload.comments_count
+  end
+
   def test_deleting_a_collection
     force_signal37_to_load_all_clients_of_firm
 

activerecord/test/cases/associations/inverse_associations_test.rb

@@ -664,6 +664,18 @@ def test_inverse_works_when_the_association_self_references_the_same_object
     comment.body = "OMG"
     assert_equal comment.body, comment.children.first.parent.body
   end
+
+  def test_changing_the_association_id_makes_the_inversed_association_target_stale
+    post1 = Post.first
+    post2 = Post.second
+    comment = post1.comments.first
+
+    assert_same post1, comment.post
+
+    comment.update!(post_id: post2.id)
+
+    assert_equal post2, comment.post
+  end
 end
 
 class InverseBelongsToTests < ActiveRecord::TestCase