Fix failing to remove files with ActiveRecord 7.1 after_commit order change enabled

Fixes #2713

Author: mshibuya

lib/carrierwave/orm/activerecord.rb

@@ -24,11 +24,16 @@ def mount_base(column, uploader=nil, options={}, &block)
 
       after_save :"store_#{column}!"
       before_save :"write_#{column}_identifier"
+      if ::ActiveRecord.try(:run_after_transaction_callbacks_in_order_defined)
+        after_commit :"remove_previously_stored_#{column}", :on => :update
+        after_commit :"reset_previous_changes_for_#{column}"
+        after_commit :"mark_remove_#{column}_false", :on => :update
+      else
+        after_commit :"mark_remove_#{column}_false", :on => :update
+        after_commit :"reset_previous_changes_for_#{column}"
+        after_commit :"remove_previously_stored_#{column}", :on => :update
+      end
       after_commit :"remove_#{column}!", :on => :destroy
-      after_commit :"mark_remove_#{column}_false", :on => :update
-
-      after_commit :"reset_previous_changes_for_#{column}"
-      after_commit :"remove_previously_stored_#{column}", :on => :update
       after_rollback :"remove_rolled_back_#{column}"
 
       mod = Module.new

spec/orm/activerecord_spec.rb

@@ -775,8 +775,6 @@ def filename
   describe '#mount_uploader removing old files' do
     before do
       reset_class("Event")
-      Event.mount_uploader(:image, @uploader)
-      @event = Event.new
     end
 
     after do
@@ -785,6 +783,8 @@ def filename
 
     describe 'normally' do
       before do
+        Event.mount_uploader(:image, @uploader)
+        @event = Event.new
         @event.image = stub_file('old.jpeg')
 
         expect(@event.save).to be_truthy
@@ -837,6 +837,8 @@ def filename
 
     describe 'with an overridden filename' do
       before do
+        Event.mount_uploader(:image, @uploader)
+        @event = Event.new
         @uploader.class_eval do
           def filename
             model.foo + File.extname(super)
@@ -865,6 +867,48 @@ def filename
         expect(File.exist?(public_path('uploads/test.jpeg'))).to be_falsey
       end
     end
+
+    describe 'with after_commit callbacks invoked by defined order' do
+      before do
+        ActiveRecord.run_after_transaction_callbacks_in_order_defined = true if ActiveRecord.respond_to?(:run_after_transaction_callbacks_in_order_defined=)
+        Event.mount_uploader(:image, @uploader)
+        @event = Event.new
+        @event.image = stub_file('old.jpeg')
+
+        expect(@event.save).to be_truthy
+        expect(File.exist?(public_path('uploads/old.jpeg'))).to be_truthy
+      end
+
+      after do
+        ActiveRecord.run_after_transaction_callbacks_in_order_defined = false if ActiveRecord.respond_to?(:run_after_transaction_callbacks_in_order_defined=)
+      end
+
+      it "should invoke the callbacks in correct order" do
+        @event.image = stub_file('new.jpeg')
+        expect(@event).to receive(:remove_previously_stored_image).ordered.and_call_original
+        expect(@event).to receive(:reset_previous_changes_for_image).ordered.and_call_original
+        expect(@event).to receive(:mark_remove_image_false).ordered.and_call_original
+        @event.save
+      end
+
+      it "should remove old file on replace" do
+        @event.image = stub_file('new.jpeg')
+        expect(@event.save).to be_truthy
+        expect(File.exist?(public_path('uploads/new.jpeg'))).to be_truthy
+        expect(File.exist?(public_path('uploads/old.jpeg'))).to be_falsey
+      end
+
+      it "should remove the file when remove_image is set to true" do
+        @event.remove_image = true
+        expect(@event.save).to be_truthy
+        expect(File.exist?(public_path('uploads/old.jpeg'))).to be_falsey
+      end
+
+      it "should remove old file on destroy" do
+        expect(@event.destroy).to be_truthy
+        expect(File.exist?(public_path('uploads/old.jpeg'))).to be_falsey
+      end
+    end
   end
 
   describe '#mount_uploader removing old files with versions' do