Fix many callback problems such as callbacks not being called are bei…

…ng called too many times.

lib/acts_as_paranoid/core.rb

@@ -2,6 +2,12 @@ module ActsAsParanoid
   module Core
     def self.included(base)
       base.extend ClassMethods
+      if ActiveRecord::VERSION::MAJOR > 5 || (ActiveRecord::VERSION::MAJOR == 5 && ActiveRecord::VERSION::MINOR >= 2)
+        base.alias_method :remember_transaction_record_state_without_paranoid, :remember_transaction_record_state
+        base.alias_method :remember_transaction_record_state, :remember_transaction_record_state_with_paranoid
+      end
+      base.alias_method :force_clear_transaction_record_state_without_paranoid, :force_clear_transaction_record_state
+      base.alias_method :force_clear_transaction_record_state, :force_clear_transaction_record_state_with_paranoid
     end
 
     module ClassMethods
@@ -117,7 +123,12 @@ def destroy_fully!
         run_callbacks :destroy do
           destroy_dependent_associations!
           # Handle composite keys, otherwise we would just use `self.class.primary_key.to_sym => self.id`.
-          self.class.delete_all!(Hash[[Array(self.class.primary_key), Array(self.id)].transpose]) if persisted?
+          @_trigger_destroy_callback = if persisted?
+                                          self.class.delete_all!(Hash[[Array(self.class.primary_key), Array(self.id)].transpose])
+                                        else
+                                          true
+                                        end
+
           stale_paranoid_value
           @destroyed = true
           freeze
@@ -235,5 +246,23 @@ 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 || (ActiveRecord::VERSION::MAJOR == 5 && ActiveRecord::VERSION::MINOR >= 2)
+      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
+        if _committed_already_called && defined?(@_trigger_destroy_callback) && !@destroyed && deleted?
+          @_trigger_destroy_callback = nil
+        end
+      end
+    end
+
+    def force_clear_transaction_record_state_with_paranoid
+      @_trigger_destroy_callback = nil if defined?(@_trigger_destroy_callback) && !@destroyed && deleted?
+      force_clear_transaction_record_state_without_paranoid
+    end
   end
 end

test/test_core.rb

@@ -341,21 +341,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
@@ -364,14 +375,63 @@ 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
+    # whether or not the after_destroy_commit callback should be called if the record is recovered is debatable
+    # for now, ActiveRecord versions >= 5.2.0 will call it and the other versions will not
+    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
+    if ActiveRecord::VERSION::MAJOR < 5 || (ActiveRecord::VERSION::MAJOR == 5 && ActiveRecord::VERSION::MINOR < 1)
+      skip 'ActiveRecord versions < 5.1.0 cannot know in which transaction the record was destroyed'
+    end
+    @paranoid_with_callback = ParanoidWithCallback.first
+    ParanoidWithCallback.transaction do
+      @paranoid_with_callback.destroy
+    end
+    ParanoidWithCallback.transaction do
+      @paranoid_with_callback.update_attributes(: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_delete_by_multiple_id_is_paranoid

test/test_helper.rb

@@ -301,29 +301,26 @@ class ParanoidWithCallback < ActiveRecord::Base
   before_recover :call_me_before_recover
   after_recover :call_me_after_recover
 
-  def initialize(*attrs)
-    @called_before_destroy = @called_after_destroy = @called_after_commit_on_destroy = false
-    super(*attrs)
-  end
+  set_callback :initialize, lambda { @called_before_destroy = @called_after_destroy = @called_after_commit_on_destroy = @called_before_recover = @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