Issues with callbacks (infinite loops, record status...)

.gitignore

@@ -7,3 +7,4 @@ gemfiles/*.lock
 .ruby-version
 coverage/
 log/
+test/tmp

Appraisals

@@ -1,7 +1,9 @@
 # frozen_string_literal: true
 
 appraise "active_record_61" do
+  gem "activejob", "~> 6.1.0", require: "active_job"
   gem "activerecord", "~> 6.1.0", require: "active_record"
+  gem "activestorage", "~> 6.1.0"
   gem "activesupport", "~> 6.1.0", require: "active_support"
 
   group :development do
@@ -10,7 +12,9 @@ appraise "active_record_61" do
 end
 
 appraise "active_record_70" do
+  gem "activejob", "~> 7.0.0", require: "active_job"
   gem "activerecord", "~> 7.0.0", require: "active_record"
+  gem "activestorage", "~> 7.0.0"
   gem "activesupport", "~> 7.0.0", require: "active_support"
 
   group :development do
@@ -19,7 +23,9 @@ appraise "active_record_70" do
 end
 
 appraise "active_record_71" do
+  gem "activejob", "~> 7.1.0", require: "active_job"
   gem "activerecord", "~> 7.1.0", require: "active_record"
+  gem "activestorage", "~> 7.1.0"
   gem "activesupport", "~> 7.1.0", require: "active_support"
 
   group :development do

gemfiles/active_record_61.gemfile

@@ -4,7 +4,9 @@
 
 source "https://rubygems.org"
 
+gem "activejob", "~> 6.1.0", require: "active_job"
 gem "activerecord", "~> 6.1.0", require: "active_record"
+gem "activestorage", "~> 6.1.0"
 gem "activesupport", "~> 6.1.0", require: "active_support"
 
 group :development do

gemfiles/active_record_70.gemfile

@@ -4,7 +4,9 @@
 
 source "https://rubygems.org"
 
+gem "activejob", "~> 7.0.0", require: "active_job"
 gem "activerecord", "~> 7.0.0", require: "active_record"
+gem "activestorage", "~> 7.0.0"
 gem "activesupport", "~> 7.0.0", require: "active_support"
 
 group :development do

gemfiles/active_record_71.gemfile

@@ -4,7 +4,9 @@
 
 source "https://rubygems.org"
 
+gem "activejob", "~> 7.1.0", require: "active_job"
 gem "activerecord", "~> 7.1.0", require: "active_record"
+gem "activestorage", "~> 7.1.0"
 gem "activesupport", "~> 7.1.0", require: "active_support"
 
 group :development do

lib/acts_as_paranoid/core.rb

@@ -158,6 +158,8 @@ def destroy_fully!
             decrement_counters_on_associations
           end
 
+          @_trigger_destroy_callback = true
+
           @destroyed = true
           freeze
         end

test/fixtures/hello.txt

@@ -0,0 +1 @@
+Hello, world!

test/test_active_storage.rb

@@ -0,0 +1,130 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+# load ActiveStorage
+require "global_id"
+ActiveRecord::Base.include(GlobalID::Identification)
+GlobalID.app = "ActsAsParanoid"
+
+require "active_job"
+ActiveJob::Base.queue_adapter = :test
+ActiveJob::Base.logger = Logger.new(File::NULL)
+
+require "active_support/cache"
+
+require "active_storage"
+require "active_storage/attached"
+require "active_storage/service/disk_service"
+require "active_storage/reflection"
+ActiveRecord::Base.include(ActiveStorage::Reflection::ActiveRecordExtensions)
+ActiveRecord::Reflection.singleton_class.prepend(ActiveStorage::Reflection::ReflectionExtension)
+ActiveRecord::Base.include(ActiveStorage::Attached::Model)
+
+require "#{Gem.loaded_specs['activestorage'].full_gem_path}/app/models/active_storage/record"
+module ActiveStorage
+  class Blob < Record
+  end
+end
+Dir.glob("#{Gem.loaded_specs['activestorage'].full_gem_path}/app/models/active_storage/blob/*").sort.each do |f|
+  require f
+end
+$LOAD_PATH << "#{Gem.loaded_specs['activestorage'].full_gem_path}/app/models/"
+Dir.glob("#{Gem.loaded_specs['activestorage'].full_gem_path}/app/models/active_storage/*").sort.each do |f|
+  require f
+end
+if ActiveStorage::Blob.respond_to?(:services=)
+  require "active_storage/service/disk_service"
+  ActiveStorage::Blob.services = {
+    "test" => ActiveStorage::Service::DiskService.build(name: "test", configurator: nil, root: "test/tmp")
+  }
+end
+if File.exist?("#{Gem.loaded_specs['activestorage'].full_gem_path}/app/jobs/active_storage/base_job.rb")
+  require "#{Gem.loaded_specs['activestorage'].full_gem_path}/app/jobs/active_storage/base_job"
+end
+Dir.glob("#{Gem.loaded_specs['activestorage'].full_gem_path}/app/jobs/active_storage/*").sort.each do |f|
+  require f
+end
+ActiveStorage::Blob.service = ActiveStorage::Service::DiskService.new(root: "test/tmp")
+
+class ParanoidActiveStorageTest < ActiveSupport::TestCase
+  self.file_fixture_path = "test/fixtures"
+
+  class ParanoidActiveStorage < ActiveRecord::Base
+    acts_as_paranoid
+
+    has_one_attached :main_file
+    has_many_attached :files
+    has_one_attached :undependent_main_file, dependent: false
+    has_many_attached :undependent_files, dependent: false
+  end
+
+  def setup_db
+    ActiveRecord::Schema.define(version: 1) do
+      create_table :active_storage_blobs do |t|
+        t.string :key, null: false
+        t.string :filename, null: false
+        t.string :content_type
+        t.text :metadata
+        t.string :service_name
+        t.bigint :byte_size, null: false
+        t.string :checksum, null: false
+        t.datetime :created_at, null: false
+        t.index [:key], name: "index_active_storage_blobs_on_key", unique: true
+      end
+
+      create_table :active_storage_attachments do |t|
+        t.string :name, null: false
+        t.references :record, null: false, polymorphic: true, index: false
+        t.references :blob, null: false
+        t.datetime :created_at, null: false
+        t.index [:record_type, :record_id, :name, :blob_id], name: "index_active_storage_attachments_uniqueness", unique: true
+      end
+
+      create_table :active_storage_variant_records do |t|
+        t.belongs_to :blob, null: false, index: false
+        t.string :variation_digest, null: false
+        t.index [:blob_id, :variation_digest], name: "index_active_storage_variant_records_uniqueness", unique: true
+      end
+
+      create_table :paranoid_active_storages do |t|
+        t.datetime :deleted_at
+        timestamps t
+      end
+    end
+  end
+
+  def clean_active_storage_attachments
+    Dir.glob("test/tmp/*").each do |f|
+      FileUtils.rm_r(f)
+    end
+  end
+
+  def create_file_blob(filename: "hello.txt", content_type: "text/plain", metadata: nil)
+    args = { io: file_fixture(filename).open, filename: filename, content_type: content_type, metadata: metadata, service_name: "test" }
+    if ActiveStorage::Blob.respond_to?(:create_and_upload!)
+      ActiveStorage::Blob.create_and_upload!(**args)
+    else
+      ActiveStorage::Blob.create_after_upload!(**args)
+    end
+  end
+
+  def setup
+    setup_db
+  end
+
+  def teardown
+    super
+    clean_active_storage_attachments
+  end
+
+  def test_paranoid_active_storage
+    pt = ParanoidActiveStorage.create({
+      main_file: create_file_blob,
+      files: [create_file_blob, create_file_blob],
+      undependent_main_file: create_file_blob,
+      undependent_files: [create_file_blob, create_file_blob]
+    })
+    pt.destroy
+  end
+end

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
 
@@ -779,21 +780,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
@@ -802,22 +814,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