From 5fba042da6492eea5d19510eaf55f1130be3a958 Mon Sep 17 00:00:00 2001 From: Yohei Yoshimuta Date: Wed, 7 Aug 2024 12:45:38 +0900 Subject: [PATCH 1/2] Do not use sqlite3 v2.0.0 or later to fix CI --- Gemfile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index fe5f736b..7b7c5852 100644 --- a/Gemfile +++ b/Gemfile @@ -5,7 +5,10 @@ sqlite = ENV['SQLITE_VERSION'] if sqlite gem 'sqlite3', sqlite, platforms: [:ruby] else - gem 'sqlite3', platforms: [:ruby] + # Do not use sqlite3 v2.0.0 or later because it is not compatible with the current Rails version. + # We can remove this constraint when Rails 7.2 is released. + # See https://github.com/rails/rails/pull/51592 and https://github.com/rails/rails/blob/v7.2.0.rc1/activerecord/CHANGELOG.md#rails-720beta1-may-29-2024 + gem 'sqlite3', "~> 1.4", platforms: [:ruby] end platforms :jruby do From b918e39983666a696e11b095d147942879aa0bc7 Mon Sep 17 00:00:00 2001 From: Yohei Yoshimuta Date: Wed, 7 Aug 2024 19:17:52 +0900 Subject: [PATCH 2/2] Implement after_restore_commit feature --- README.md | 13 ++++ lib/paranoia.rb | 43 ++++++++++++- test/paranoia_test.rb | 139 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 193 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index cf898b8f..2b8f5efd 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 b69b4402..f1f882fc 100644 --- a/lib/paranoia.rb +++ b/lib/paranoia.rb @@ -88,7 +88,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 @@ -115,6 +124,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) @@ -128,6 +141,10 @@ def restore!(opts = {}) end self + ensure + if paranoia_after_restore_commit + @_trigger_restore_callback = false + end end alias :restore :restore! @@ -262,6 +279,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 + ActiveSupport.on_load(:active_record) do class ActiveRecord::Base def self.acts_as_paranoid(options={}) @@ -278,10 +312,11 @@ def self.acts_as_paranoid(options={}) alias_method :destroy_without_paranoia, :destroy include Paranoia - class_attribute :paranoia_column, :paranoia_sentinel_value + class_attribute :paranoia_column, :paranoia_sentinel_value, :paranoia_after_restore_commit 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 @@ -297,6 +332,10 @@ class << self; alias_method :without_deleted, :paranoia_scope end after_restore { 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 end # Please do not use this method in production. diff --git a/test/paranoia_test.rb b/test/paranoia_test.rb index 14be4d0e..e155a809 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 @@ -1329,6 +1427,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