From 7b96793a72f111eef474ca20fd88dd3d536d6633 Mon Sep 17 00:00:00 2001 From: yohei yoshimuta Date: Wed, 2 Oct 2024 15:13:17 +0900 Subject: [PATCH] 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 <99191+mathieujobin@users.noreply.github.com> --- README.md | 13 ++++ lib/paranoia.rb | 44 ++++++++++++- test/paranoia_test.rb | 139 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 194 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index a55fc9ca..3d98b90c 100644 --- 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 index 1b15bf79..93bf9730 100644 --- 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 index 13069c2a..e11dc416 100644 --- 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