MONGOID-5852 Fix after_action_commit callbacks (#5968)

lib/mongoid/clients/sessions.rb

@@ -213,8 +213,8 @@ def run_abort_callbacks(session)
 
         # Transforms custom options for after_commit and after_rollback callbacks
         # into options for +set_callback+.
-        def set_options_for_callbacks!(args)
-          options = args.extract_options!
+        def set_options_for_callbacks!(args, enforced_options = {})
+          options = args.extract_options!.merge(enforced_options)
           args << options
 
           if options[:on]

spec/mongoid/clients/transactions_spec.rb

@@ -787,8 +787,12 @@ def capture_exception
         Mongoid::Clients.with_name(:default).database.collections.each(&:drop)
         TransactionsSpecPerson.collection.create
         TransactionsSpecPersonWithOnCreate.collection.create
+        TransactionsSpecPersonWithAfterCreateCommit.collection.create
         TransactionsSpecPersonWithOnUpdate.collection.create
+        TransactionsSpecPersonWithAfterUpdateCommit.collection.create
+        TransactionsSpecPersonWithAfterSaveCommit.collection.create
         TransactionsSpecPersonWithOnDestroy.collection.create
+        TransactionsSpecPersonWithAfterDestroyCommit.collection.create
         TransactionSpecRaisesBeforeSave.collection.create
         TransactionSpecRaisesAfterSave.collection.create
       end
@@ -818,6 +822,18 @@ def capture_exception
 
             it_behaves_like 'commit callbacks are called'
           end
+
+          context 'when callback is after_create_commit' do
+            let!(:subject) do
+              person = nil
+              TransactionsSpecPersonWithAfterCreateCommit.transaction do
+                person = TransactionsSpecPersonWithAfterCreateCommit.create!(name: 'James Bond')
+              end
+              person
+            end
+
+            it_behaves_like 'commit callbacks are called'
+          end
         end
 
         context 'save' do
@@ -886,6 +902,94 @@ def capture_exception
               it_behaves_like 'commit callbacks are called'
             end
           end
+
+          context 'with after_update_commit callback' do
+            let(:subject) do
+              TransactionsSpecPersonWithAfterUpdateCommit.create!(name: 'James Bond').tap do |subject|
+                subject.after_commit_counter.reset
+                subject.after_rollback_counter.reset
+              end
+            end
+
+            context 'when modified once' do
+              before do
+                subject.transaction do
+                  subject.name = 'Austin Powers'
+                  subject.save!
+                end
+              end
+
+              it_behaves_like 'commit callbacks are called'
+            end
+
+            context 'when modified multiple times' do
+              before do
+                subject.transaction do
+                  subject.name = 'Austin Powers'
+                  subject.save!
+                  subject.name = 'Jason Bourne'
+                  subject.save!
+                end
+              end
+
+              it_behaves_like 'commit callbacks are called'
+            end
+          end
+
+          context 'with after_save_commit callback' do
+            let(:subject) do
+              TransactionsSpecPersonWithAfterSaveCommit.create!(name: 'James Bond').tap do |subject|
+                subject.after_commit_counter.reset
+                subject.after_rollback_counter.reset
+              end
+            end
+
+            context 'when modified once' do
+              before do
+                subject.transaction do
+                  subject.name = 'Austin Powers'
+                  subject.save!
+                end
+              end
+
+              it_behaves_like 'commit callbacks are called'
+            end
+
+            context 'when created' do
+              before do
+                TransactionsSpecPersonWithAfterSaveCommit.transaction do
+                  subject
+                end
+              end
+
+              it_behaves_like 'commit callbacks are called'
+            end
+
+            context 'when modified multiple times' do
+              before do
+                subject.transaction do
+                  subject.name = 'Austin Powers'
+                  subject.save!
+                  subject.name = 'Jason Bourne'
+                  subject.save!
+                end
+              end
+
+              it_behaves_like 'commit callbacks are called'
+            end
+
+            context 'when created and modified' do
+              before do
+                TransactionsSpecPersonWithAfterSaveCommit.transaction do
+                  subject
+                  subject.name = 'Jason Bourne'
+                  subject.save!
+                end
+              end
+
+              it_behaves_like 'commit callbacks are called'
+            end
+          end
         end
 
         context 'update_attributes' do
@@ -919,6 +1023,34 @@ def capture_exception
 
             it_behaves_like 'commit callbacks are called'
           end
+
+          context 'when callback is after_update_commit' do
+            let(:subject) do
+              TransactionsSpecPersonWithAfterUpdateCommit.create!(name: 'Jason Bourne')
+            end
+
+            before do
+              TransactionsSpecPersonWithAfterUpdateCommit.transaction do
+                subject.update_attributes!(name: 'Foma Kiniaev')
+              end
+            end
+
+            it_behaves_like 'commit callbacks are called'
+          end
+
+          context 'when callback is after_save_commit' do
+            let(:subject) do
+              TransactionsSpecPersonWithAfterSaveCommit.create!(name: 'Jason Bourne')
+            end
+
+            before do
+              TransactionsSpecPersonWithAfterSaveCommit.transaction do
+                subject.update_attributes!(name: 'Foma Kiniaev')
+              end
+            end
+
+            it_behaves_like 'commit callbacks are called'
+          end
         end
 
         context 'destroy' do
@@ -971,6 +1103,31 @@ def capture_exception
 
             it_behaves_like 'commit callbacks are called'
           end
+
+          context 'with after_destroy_commit' do
+            let(:after_commit_counter) do
+              TransactionsSpecCounter.new
+            end
+
+            let(:after_rollback_counter) do
+              TransactionsSpecCounter.new
+            end
+
+            let(:subject) do
+              TransactionsSpecPersonWithAfterDestroyCommit.create!(name: 'James Bond').tap do |p|
+                p.after_commit_counter = after_commit_counter
+                p.after_rollback_counter = after_rollback_counter
+              end
+            end
+
+            before do
+              subject.transaction do
+                subject.destroy
+              end
+            end
+
+            it_behaves_like 'commit callbacks are called'
+          end
         end
       end
 

spec/mongoid/clients/transactions_spec_models.rb

@@ -33,6 +33,38 @@ def after_rollback_counter
   def after_rollback_counter=(new_counter)
     @after_rollback_counter = new_counter
   end
+
+  def after_save_commit_counter
+    @after_save_commit_counter ||= TransactionsSpecCounter.new
+  end
+
+  def after_save_commit_counter=(new_counter)
+    @after_save_commit_counter = new_counter
+  end
+
+  def after_create_commit_counter
+    @after_create_commit_counter ||= TransactionsSpecCounter.new
+  end
+
+  def after_create_commit_counter=(new_counter)
+    @after_create_commit_counter = new_counter
+  end
+
+  def after_update_commit_counter
+    @after_update_commit_counter ||= TransactionsSpecCounter.new
+  end
+
+  def after_update_commit_counter=(new_counter)
+    @after_update_commit_counter = new_counter
+  end
+
+  def after_destroy_commit_counter
+    @after_destroy_commit_counter ||= TransactionsSpecCounter.new
+  end
+
+  def after_destroy_commit_counter=(new_counter)
+    @after_destroy_commit_counter = new_counter
+  end
 end
 
 class TransactionsSpecPerson
@@ -65,6 +97,21 @@ class TransactionsSpecPersonWithOnCreate
   end
 end
 
+class TransactionsSpecPersonWithAfterCreateCommit
+  include Mongoid::Document
+  include TransactionsSpecCountable
+
+  field :name, type: String
+
+  after_create_commit do
+    after_commit_counter.inc
+  end
+
+  after_rollback on: :create do
+    after_rollback_counter.inc
+  end
+end
+
 class TransactionsSpecPersonWithOnUpdate
   include Mongoid::Document
   include TransactionsSpecCountable
@@ -80,6 +127,36 @@ class TransactionsSpecPersonWithOnUpdate
   end
 end
 
+class TransactionsSpecPersonWithAfterUpdateCommit
+  include Mongoid::Document
+  include TransactionsSpecCountable
+
+  field :name, type: String
+
+  after_update_commit do
+    after_commit_counter.inc
+  end
+
+  after_rollback on: :create do
+    after_rollback_counter.inc
+  end
+end
+
+class TransactionsSpecPersonWithAfterSaveCommit
+  include Mongoid::Document
+  include TransactionsSpecCountable
+
+  field :name, type: String
+
+  after_save_commit do
+    after_commit_counter.inc
+  end
+
+  after_rollback on: :create do
+    after_rollback_counter.inc
+  end
+end
+
 class TransactionsSpecPersonWithOnDestroy
   include Mongoid::Document
   include TransactionsSpecCountable
@@ -94,6 +171,22 @@ class TransactionsSpecPersonWithOnDestroy
     after_rollback_counter.inc
   end
 end
+
+class TransactionsSpecPersonWithAfterDestroyCommit
+  include Mongoid::Document
+  include TransactionsSpecCountable
+
+  field :name, type: String
+
+  after_destroy_commit do
+    after_commit_counter.inc
+  end
+
+  after_rollback on: :create do
+    after_rollback_counter.inc
+  end
+end
+
 class TransactionSpecRaisesBeforeSave
   include Mongoid::Document
   include TransactionsSpecCountable