Merge pull request #39771 from eugeneius/autosave_exactly_once

Autosave collection associations exactly once

activerecord/lib/active_record/autosave_association.rb

@@ -29,7 +29,7 @@ module ActiveRecord
   # == Callbacks
   #
   # Association with autosave option defines several callbacks on your
-  # model (before_save, after_create, after_update). Please note that
+  # model (around_save, before_save, after_create, after_update). Please note that
   # callbacks are executed in the order they were defined in
   # model. You should avoid modifying the association content before
   # autosave callbacks are executed. Placing your callbacks after
@@ -188,8 +188,7 @@ def add_autosave_association_callbacks(reflection)
           save_method = :"autosave_associated_records_for_#{reflection.name}"
 
           if reflection.collection?
-            before_save :before_save_collection_association
-            after_save :after_save_collection_association
+            around_save :around_save_collection_association
 
             define_non_cyclic_method(save_method) { save_collection_association(reflection) }
             # Doesn't use after_save as that would save associations added in after_create/after_update twice
@@ -362,14 +361,15 @@ def normalize_reflection_attribute(indexed_attribute, reflection, index, attribu
         end
       end
 
-      # Is used as a before_save callback to check while saving a collection
+      # Is used as an around_save callback to check while saving a collection
       # association whether or not the parent was a new record before saving.
-      def before_save_collection_association
-        @new_record_before_save ||= new_record?
-      end
+      def around_save_collection_association
+        previously_new_record_before_save = (@new_record_before_save ||= false)
+        @new_record_before_save = !previously_new_record_before_save && new_record?
 
-      def after_save_collection_association
-        @new_record_before_save = false
+        yield
+      ensure
+        @new_record_before_save = previously_new_record_before_save
       end
 
       # Saves any new associated records, or all loaded autosave associations if

activerecord/test/cases/autosave_association_test.rb

@@ -6,6 +6,7 @@
 require "models/bird"
 require "models/post"
 require "models/comment"
+require "models/category"
 require "models/company"
 require "models/contract"
 require "models/customer"
@@ -820,6 +821,15 @@ def test_autosave_new_record_with_after_create_callback
 
     assert_not_nil post.author_id
   end
+
+  def test_autosave_new_record_with_after_create_callback_and_habtm_association
+    post = PostWithAfterCreateCallback.new(title: "Captain Murphy", body: "is back")
+    post.comments.build(body: "foo")
+    post.categories.build(name: "bar")
+    post.save!
+
+    assert_equal 1, post.categories.reload.length
+  end
 end
 
 class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase

activerecord/test/models/post.rb

@@ -291,6 +291,7 @@ class PostWithAfterCreateCallback < ActiveRecord::Base
   self.inheritance_column = :disabled
   self.table_name = "posts"
   has_many :comments, foreign_key: :post_id
+  has_and_belongs_to_many :categories, foreign_key: :post_id
 
   after_create do |post|
     update_attribute(:author_id, comments.first.id)