From 5ac470b9cd3008cf85ee97fa0f68f7ac63b5cd84 Mon Sep 17 00:00:00 2001 From: Aymeric Le Dorze Date: Wed, 1 Aug 2018 13:57:56 +0200 Subject: [PATCH] Fix many callback problems such as callbacks not being called or being called too many times. --- lib/acts_as_paranoid/core.rb | 31 +++++++++++ test/test_core.rb | 104 +++++++++++++++++++++++++++-------- 2 files changed, 111 insertions(+), 24 deletions(-) diff --git a/lib/acts_as_paranoid/core.rb b/lib/acts_as_paranoid/core.rb index eaa0038..e315b29 100644 --- a/lib/acts_as_paranoid/core.rb +++ b/lib/acts_as_paranoid/core.rb @@ -4,6 +4,12 @@ module ActsAsParanoid module Core def self.included(base) base.extend ClassMethods + if ActiveRecord::VERSION::MAJOR == 5 + base.send(:alias_method, :committed_without_paranoid!, :committed!) + base.send(:alias_method, :committed!, :committed_with_paranoid!) + base.send(:alias_method, :remember_transaction_record_state_without_paranoid, :remember_transaction_record_state) + base.send(:alias_method, :remember_transaction_record_state, :remember_transaction_record_state_with_paranoid) + end end module ClassMethods @@ -156,6 +162,8 @@ def destroy_fully! decrement_counters_on_associations end + @_trigger_destroy_callback = true + @destroyed = true freeze end @@ -242,6 +250,16 @@ def deleted_fully? alias destroyed_fully? deleted_fully? + if ActiveRecord::VERSION::MAJOR == 5 + def committed_with_paranoid!(should_run_callbacks: true) + committed_without_paranoid!(should_run_callbacks: should_run_callbacks) + ensure + if defined?(@_trigger_destroy_callback) && !@_trigger_destroy_callback.nil? && !@destroyed && deleted? + @_trigger_destroy_callback = nil + end + end + end + private def recover_dependent_associations(deleted_value, options) @@ -327,5 +345,18 @@ def stale_paranoid_value self.paranoid_value = self.class.delete_now_value clear_attribute_changes([self.class.paranoid_column]) end + + if ActiveRecord::VERSION::MAJOR == 5 + def remember_transaction_record_state_with_paranoid + remember_transaction_record_state_without_paranoid + remember_trigger_destroy_callback_before_last_commit + end + + def remember_trigger_destroy_callback_before_last_commit + return unless _committed_already_called && defined?(@_trigger_destroy_callback) && !@_trigger_destroy_callback.nil? && !@destroyed && deleted? + + @_trigger_destroy_callback = nil + end + end end end diff --git a/test/test_core.rb b/test/test_core.rb index 64cd798..68c6558 100644 --- a/test/test_core.rb +++ b/test/test_core.rb @@ -123,31 +123,32 @@ class ParanoidWithCallback < ActiveRecord::Base before_recover :call_me_before_recover after_recover :call_me_after_recover - def initialize(*attrs) - @called_before_destroy = false - @called_after_destroy = false - @called_after_commit_on_destroy = false - super(*attrs) - end + set_callback :initialize, lambda { + @called_before_destroy = 0 + @called_after_destroy = 0 + @called_after_commit_on_destroy = 0 + @called_before_recover = 0 + @called_after_recover = 0 + } def call_me_before_destroy - @called_before_destroy = true + @called_before_destroy += 1 end def call_me_after_destroy - @called_after_destroy = true + @called_after_destroy += 1 end def call_me_after_commit_on_destroy - @called_after_commit_on_destroy = true + @called_after_commit_on_destroy += 1 end def call_me_before_recover - @called_before_recover = true + @called_before_recover += 1 end def call_me_after_recover - @called_after_recover = true + @called_after_recover += 1 end end @@ -740,21 +741,32 @@ def test_paranoid_destroy_callbacks @paranoid_with_callback.destroy end - assert @paranoid_with_callback.called_before_destroy - assert @paranoid_with_callback.called_after_destroy - assert @paranoid_with_callback.called_after_commit_on_destroy + assert_equal 1, @paranoid_with_callback.called_before_destroy + assert_equal 1, @paranoid_with_callback.called_after_destroy + assert_equal 1, @paranoid_with_callback.called_after_commit_on_destroy + end + + def test_paranoid_destroy_destroy_callbacks + @paranoid_with_callback = ParanoidWithCallback.first + ParanoidWithCallback.transaction do + @paranoid_with_callback.destroy.destroy + end + + assert_equal 2, @paranoid_with_callback.called_before_destroy + assert_equal 2, @paranoid_with_callback.called_after_destroy + assert_equal 1, @paranoid_with_callback.called_after_commit_on_destroy end def test_hard_destroy_callbacks @paranoid_with_callback = ParanoidWithCallback.first ParanoidWithCallback.transaction do - @paranoid_with_callback.destroy! + @paranoid_with_callback.destroy_fully! end - assert @paranoid_with_callback.called_before_destroy - assert @paranoid_with_callback.called_after_destroy - assert @paranoid_with_callback.called_after_commit_on_destroy + assert_equal 1, @paranoid_with_callback.called_before_destroy + assert_equal 1, @paranoid_with_callback.called_after_destroy + assert_equal 1, @paranoid_with_callback.called_after_commit_on_destroy end def test_recovery_callbacks @@ -763,22 +775,66 @@ def test_recovery_callbacks ParanoidWithCallback.transaction do @paranoid_with_callback.destroy - assert_nil @paranoid_with_callback.called_before_recover - assert_nil @paranoid_with_callback.called_after_recover + assert_equal 1, @paranoid_with_callback.called_before_destroy + assert_equal 1, @paranoid_with_callback.called_after_destroy + assert_equal 0, @paranoid_with_callback.called_after_commit_on_destroy + assert_equal 0, @paranoid_with_callback.called_before_recover + assert_equal 0, @paranoid_with_callback.called_after_recover @paranoid_with_callback.recover end - assert @paranoid_with_callback.called_before_recover - assert @paranoid_with_callback.called_after_recover + assert_equal 1, @paranoid_with_callback.called_before_destroy + assert_equal 1, @paranoid_with_callback.called_after_destroy + assert_includes 0..1, @paranoid_with_callback.called_after_commit_on_destroy + assert_equal 1, @paranoid_with_callback.called_before_recover + assert_equal 1, @paranoid_with_callback.called_after_recover + end + + def test_recovery_callbacks_with_2_transactions + @paranoid_with_callback = ParanoidWithCallback.first + + ParanoidWithCallback.transaction do + @paranoid_with_callback.destroy + end + + assert_equal 1, @paranoid_with_callback.called_before_destroy + assert_equal 1, @paranoid_with_callback.called_after_destroy + assert_equal 1, @paranoid_with_callback.called_after_commit_on_destroy + assert_equal 0, @paranoid_with_callback.called_before_recover + assert_equal 0, @paranoid_with_callback.called_after_recover + + ParanoidWithCallback.transaction do + @paranoid_with_callback.recover + end + + assert_equal 1, @paranoid_with_callback.called_before_destroy + assert_equal 1, @paranoid_with_callback.called_after_destroy + assert_equal 1, @paranoid_with_callback.called_after_commit_on_destroy + assert_equal 1, @paranoid_with_callback.called_before_recover + assert_equal 1, @paranoid_with_callback.called_after_recover + end + + def test_paranoid_destroy_with_update_callbacks + @paranoid_with_callback = ParanoidWithCallback.first + ParanoidWithCallback.transaction do + @paranoid_with_callback.destroy + end + ParanoidWithCallback.transaction do + @paranoid_with_callback.update(name: "still paranoid") + end + + assert_equal 1, @paranoid_with_callback.called_before_destroy + assert_equal 1, @paranoid_with_callback.called_after_destroy + assert_equal 1, @paranoid_with_callback.called_after_commit_on_destroy end def test_recovery_callbacks_without_destroy @paranoid_with_callback = ParanoidWithCallback.first @paranoid_with_callback.recover - assert_nil @paranoid_with_callback.called_before_recover - assert_nil @paranoid_with_callback.called_after_recover + assert_equal 0, @paranoid_with_callback.called_before_recover + assert_equal 0, @paranoid_with_callback.called_after_recover end def test_delete_by_multiple_id_is_paranoid