Skip to content

Commit

Permalink
feat: Trigger an after_commit callback when restoring a record (#559)
Browse files Browse the repository at this point in the history
* Do not use sqlite3 v2.0.0 or later to fix CI

* Implement after_restore_commit feature

---------

Co-authored-by: Mathieu Jobin <[email protected]>
  • Loading branch information
yoheimuta and mathieujobin authored Oct 2, 2024
1 parent ba6dde7 commit 7b96793
Show file tree
Hide file tree
Showing 3 changed files with 194 additions and 2 deletions.
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 42 additions & 2 deletions lib/paranoia.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -134,6 +147,10 @@ def restore!(opts = {})
end

self
ensure
if paranoia_after_restore_commit
@_trigger_restore_callback = false
end
end
alias :restore :restore!

Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand Down
139 changes: 139 additions & 0 deletions test/paranoia_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

1 comment on commit 7b96793

@rapito
Copy link

@rapito rapito commented on 7b96793 Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi!

Just a quick question, not sure if this is the right place to put it since the PR was already merged...
Why does the new feature needs to be an optional flag on the model?

acts_as_paranoid after_restore_commit: true

Seems like it's redundant, if you don't use the after_restore_commit or on: commit it simply won't execute. It's the extra flag necessary?

Please sign in to comment.