feat: Trigger an after_commit callback when restoring a record (#559)

* Do not use sqlite3 v2.0.0 or later to fix CI * Implement after_restore_commit feature --------- Co-authored-by: Mathieu Jobin <[email protected]>
rubysherpas · Oct 2, 2024 · 7b96793 · 7b96793
1 parent ba6dde7
commit 7b96793
Show file tree

Hide file tree

Showing 3 changed files with 194 additions and 2 deletions.
diff --git a/README.md b/README.md
@@ -206,6 +206,19 @@ Client.restore(id, :recursive => true, :recovery_window => 2.minutes)
 client.restore(:recursive => true, :recovery_window => 2.minutes)
 ```
 
+If you want to trigger an after_commit callback when restoring a record:
+
+``` ruby
+class Client < ActiveRecord::Base
+  acts_as_paranoid after_restore_commit: true
+
+  after_commit          :commit_called, on: :restore
+  # or
+  after_restore_commit  :commit_called
+  ...
+end
+```
+
 Note that by default paranoia will not prevent that a soft destroyed object can't be associated with another object of a different model.
 A Rails validator is provided should you require this functionality:
   ``` ruby

diff --git a/lib/paranoia.rb b/lib/paranoia.rb
@@ -94,7 +94,16 @@ def paranoia_destroy!
   end
 
   def trigger_transactional_callbacks?
-    super || @_trigger_destroy_callback && paranoia_destroyed?
+    super || @_trigger_destroy_callback && paranoia_destroyed? ||
+      @_trigger_restore_callback && !paranoia_destroyed?
+  end
+
+  def transaction_include_any_action?(actions)
+    super || actions.any? do |action|
+      if action == :restore
+        paranoia_after_restore_commit && @_trigger_restore_callback
+      end
+    end
   end
 
   def paranoia_delete
@@ -121,6 +130,10 @@ def restore!(opts = {})
         if within_recovery_window?(recovery_window_range) && ((noop_if_frozen && !@attributes.frozen?) || !noop_if_frozen)
           @_disable_counter_cache = !paranoia_destroyed?
           write_attribute paranoia_column, paranoia_sentinel_value
+          if paranoia_after_restore_commit
+            @_trigger_restore_callback = true
+            add_to_transaction
+          end
           update_columns(paranoia_restore_attributes)
           each_counter_cached_associations do |association|
             if send(association.reflection.name)
@@ -134,6 +147,10 @@ def restore!(opts = {})
     end
 
     self
+  ensure
+    if paranoia_after_restore_commit
+      @_trigger_restore_callback = false
+    end
   end
   alias :restore :restore!
 
@@ -260,6 +277,23 @@ def restore_associated_records(recovery_window_range = nil)
   end
 end
 
+module ActiveRecord
+  module Transactions
+    module RestoreSupport
+      def self.included(base)
+        base::ACTIONS << :restore unless base::ACTIONS.include?(:restore)
+      end
+    end
+
+    module ClassMethods
+      def after_restore_commit(*args, &block)
+        set_options_for_callbacks!(args, on: :restore)
+        set_callback(:commit, :after, *args, &block)
+      end
+    end
+  end
+end
+
 module Paranoia::Relation  
   def paranoia_delete_all
     update_all(klass.paranoia_destroy_attributes)
@@ -285,10 +319,12 @@ def self.acts_as_paranoid(options={})
       class << self; delegate :really_delete_all, to: :all end
 
       include Paranoia
-      class_attribute :paranoia_column, :paranoia_sentinel_value, :delete_all_enabled
+      class_attribute :paranoia_column, :paranoia_sentinel_value, :paranoia_after_restore_commit,
+        :delete_all_enabled
 
       self.paranoia_column = (options[:column] || :deleted_at).to_s
       self.paranoia_sentinel_value = options.fetch(:sentinel_value) { Paranoia.default_sentinel_value }
+      self.paranoia_after_restore_commit = options.fetch(:after_restore_commit) { false }
       def self.paranoia_scope
         where(paranoia_column => paranoia_sentinel_value)
       end
@@ -305,6 +341,10 @@ class << self; alias_method :without_deleted, :paranoia_scope end
         self.class.notify_observers(:after_restore, self) if self.class.respond_to?(:notify_observers)
       }
 
+      if paranoia_after_restore_commit
+        ActiveRecord::Transactions.send(:include, ActiveRecord::Transactions::RestoreSupport)
+      end
+
       self.delete_all_enabled = options[:delete_all_enabled] || Paranoia.delete_all_enabled
 
       if self.delete_all_enabled

diff --git a/test/paranoia_test.rb b/test/paranoia_test.rb
@@ -30,6 +30,10 @@ def setup!
     'featureful_models' => 'deleted_at DATETIME, name VARCHAR(32)',
     'plain_models' => 'deleted_at DATETIME',
     'callback_models' => 'deleted_at DATETIME',
+    'after_commit_on_restore_callback_models' => 'deleted_at DATETIME',
+    'after_restore_commit_callback_models' => 'deleted_at DATETIME',
+    'after_commit_callback_restore_enabled_models' => 'deleted_at DATETIME',
+    'after_other_commit_callback_restore_enabled_models' => 'deleted_at DATETIME',
     'after_commit_callback_models' => 'deleted_at DATETIME',
     'fail_callback_models' => 'deleted_at DATETIME',
     'association_with_abort_models' => 'deleted_at DATETIME',
@@ -626,6 +630,100 @@ def test_restore_behavior_for_callbacks
     model.reload
 
     assert model.instance_variable_get(:@restore_callback_called)
+    assert_nil model.instance_variable_get(:@after_commit_callback_called)
+  end
+
+  def test_after_commit_on_restore
+    model = AfterCommitOnRestoreCallbackModel.new
+    model.save
+    id = model.id
+    model.destroy
+
+    assert model.paranoia_destroyed?
+
+    model = AfterCommitOnRestoreCallbackModel.only_deleted.find(id)
+    model.restore!
+    model.reload
+
+    assert model.instance_variable_get(:@restore_callback_called)
+    assert model.instance_variable_get(:@after_restore_callback_called)
+    assert model.instance_variable_get(:@after_restore_commit_callback_called)
+  end
+
+  def test_after_restore_commit
+    model = AfterRestoreCommitCallbackModel.new
+    model.save
+    id = model.id
+    model.destroy
+
+    assert model.paranoia_destroyed?
+
+    model = AfterRestoreCommitCallbackModel.only_deleted.find(id)
+    model.restore!
+    model.reload
+
+    assert model.instance_variable_get(:@restore_callback_called)
+    assert model.instance_variable_get(:@after_restore_callback_called)
+    assert model.instance_variable_get(:@after_restore_commit_callback_called)
+  end
+
+  def test_after_restore_commit_once
+    model = AfterRestoreCommitCallbackModel.new
+    model.save
+    id = model.id
+    model.destroy
+
+    assert model.paranoia_destroyed?
+    assert model.instance_variable_get(:@after_destroy_commit_callback_called)
+
+    model.remove_called_variables
+    model = AfterRestoreCommitCallbackModel.only_deleted.find(id)
+    model.restore!
+    model.reload
+
+    assert model.instance_variable_get(:@restore_callback_called)
+    assert model.instance_variable_get(:@after_restore_callback_called)
+    assert model.instance_variable_get(:@after_restore_commit_callback_called)
+    assert_nil model.instance_variable_get(:@after_destroy_commit_callback_called)
+
+    model.remove_called_variables
+    model.destroy
+    assert model.instance_variable_get(:@after_destroy_commit_callback_called)
+    assert_nil model.instance_variable_get(:@after_restore_commit_callback_called)
+  end
+
+  def test_after_commit_restore_enabled
+    model = AfterCommitCallbackRestoreEnabledModel.new
+    model.save
+    id = model.id
+    model.destroy
+
+    assert model.paranoia_destroyed?
+
+    model = AfterCommitCallbackRestoreEnabledModel.only_deleted.find(id)
+    model.restore!
+    model.reload
+
+    assert model.instance_variable_get(:@restore_callback_called)
+    assert model.instance_variable_get(:@after_restore_callback_called)
+    assert model.instance_variable_get(:@after_commit_callback_called)
+  end
+
+  def test_not_call_after_other_commit_restore_enabled
+    model = AfterOtherCommitCallbackRestoreEnabledModel.new
+    model.save
+    id = model.id
+    model.destroy
+
+    assert model.paranoia_destroyed?
+
+    model = AfterOtherCommitCallbackRestoreEnabledModel.only_deleted.find(id)
+    model.restore!
+    model.reload
+
+    assert model.instance_variable_get(:@restore_callback_called)
+    assert model.instance_variable_get(:@after_restore_callback_called)
+    assert_nil model.instance_variable_get(:@after_other_commit_callback_called)
   end
 
   def test_really_destroy
@@ -1394,6 +1492,47 @@ def remove_called_variables
   end
 end
 
+class AfterCommitOnRestoreCallbackModel < ActiveRecord::Base
+  acts_as_paranoid after_restore_commit: true
+  before_restore { |model| model.instance_variable_set :@restore_callback_called, true }
+  after_restore  { |model| model.instance_variable_set :@after_restore_callback_called, true }
+  after_commit   :set_after_restore_commit_called, on: :restore
+
+  def set_after_restore_commit_called
+    @after_restore_commit_callback_called = true
+  end
+end
+
+class AfterRestoreCommitCallbackModel < ActiveRecord::Base
+  acts_as_paranoid after_restore_commit: true
+  before_restore        { |model| model.instance_variable_set :@restore_callback_called, true }
+  after_restore         { |model| model.instance_variable_set :@after_restore_callback_called, true }
+  after_restore_commit  { |model| model.instance_variable_set :@after_restore_commit_callback_called, true }
+  after_destroy_commit  { |model| model.instance_variable_set :@after_destroy_commit_callback_called, true }
+
+  def remove_called_variables
+    instance_variables.each {|name| (name.to_s.end_with?('_called')) ? remove_instance_variable(name) : nil}
+  end
+end
+
+class AfterCommitCallbackRestoreEnabledModel < ActiveRecord::Base
+  acts_as_paranoid after_restore_commit: true
+  before_restore { |model| model.instance_variable_set :@restore_callback_called, true }
+  after_restore  { |model| model.instance_variable_set :@after_restore_callback_called, true }
+  after_commit   { |model| model.instance_variable_set :@after_commit_callback_called, true }
+end
+
+class AfterOtherCommitCallbackRestoreEnabledModel < ActiveRecord::Base
+  acts_as_paranoid after_restore_commit: true
+  before_restore { |model| model.instance_variable_set :@restore_callback_called, true }
+  after_restore  { |model| model.instance_variable_set :@after_restore_callback_called, true }
+  after_commit   :set_after_other_commit_called, on: [:create, :destroy, :update]
+
+  def set_after_other_commit_called
+    @after_other_commit_callback_called = true
+  end
+end
+
 class AssociationWithAbortModel < ActiveRecord::Base
   acts_as_paranoid
   has_many :related_models, class_name: 'RelatedModel', foreign_key: :parent_model_id, dependent: :destroy