rubysherpas / paranoia

acts_as_paranoid for Rails 5, 6 and 7
Other
2.89k stars 529 forks source link

Rails 6: after_commit on create callbacks are called after destroy on newly created records #511

Closed nin9 closed 2 years ago

nin9 commented 3 years ago

When a record is created then destroyed, after_commit on create callbacks are called.

Steps to reproduce

Run the following ruby script:

require 'bundler/inline'
gemfile(true) do
  source 'https://rubygems.org'

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem 'activerecord', '6.0.0'
  # gem 'activerecord', '5.2.6' # this version works
  gem 'paranoia'
  gem 'sqlite3'
  gem 'byebug'
end

require 'active_record'
require 'minitest/autorun'
require 'paranoia'

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')

ActiveRecord::Schema.define do
  create_table :users, force: true do |t|
    t.integer  :value, default: 0
    t.datetime :created_at
    t.datetime :updated_at
    t.datetime :deleted_at
  end
end

class User < ActiveRecord::Base
  acts_as_paranoid

  after_commit -> { self.value += 1 }, on: :create
end

class BugTest < Minitest::Test
  def test_freshly_loaded_model
    user = User.create!
    user.destroy

    # This should fail but it succeeds
    assert user.value == 2
  end
end

Note that if you called transaction_include_any_action?([:create]) inside the callback function it will return true which is wrong since the action is destroy not create.

zhengpd commented 3 years ago

Got the same bug after upgraded to rails 6.1 & paranoia 2.4.3. I figured out a workaround by overwriting paranoia_destroy:

# config/initializers/fix_paranoia.rb
module Paranoia
  # override method in paranoia
  def paranoia_destroy
    transaction do
      run_callbacks(:destroy) do
        @_disable_counter_cache = deleted?
        result = paranoia_delete
        next result unless result && ActiveRecord::VERSION::STRING >= '4.2'
        each_counter_cached_associations do |association|
          foreign_key = association.reflection.foreign_key.to_sym
          next if destroyed_by_association && destroyed_by_association.foreign_key.to_sym == foreign_key
          next unless send(association.reflection.name)
          association.decrement_counters
        end
        @_trigger_destroy_callback = true
        @_disable_counter_cache = false

        # Modified line
        # fix the after_commit callback bug
        # https://github.com/rubysherpas/paranoia/issues/511
        @_new_record_before_last_commit = false

        result
      end
    end
  end
  alias_method :destroy, :paranoia_destroy
end
bkDJ commented 2 years ago

@nin9 @zhengpd I also made a PR for this: #513

mathieujobin commented 2 years ago

fix will be released as v2.5.4 later

mathieujobin commented 2 years ago

released as v2.6.0