Skip to content

Commit

Permalink
Fix many callback problems such as callbacks not being called or bein…
Browse files Browse the repository at this point in the history
…g called too many times.
  • Loading branch information
aymeric-ledorze committed Apr 13, 2022
1 parent 4a7e2d6 commit 5ac470b
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 24 deletions.
31 changes: 31 additions & 0 deletions lib/acts_as_paranoid/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -156,6 +162,8 @@ def destroy_fully!
decrement_counters_on_associations
end

@_trigger_destroy_callback = true

@destroyed = true
freeze
end
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
104 changes: 80 additions & 24 deletions test/test_core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 5ac470b

Please sign in to comment.