after_commit for inner transaction has different object id

[SheldonNunes]

I have a model that has a queue structure that should be executed on after_commit. The model is saved within a nested transaction and the queue is not executing. I have discovered that the after_commit model has a different object_id from the one in the inner transaction.

Steps to reproduce

I created a reproducible test case:
Note: I have tried with both the inclusion and exclusion of Company.transaction(requires_new: true)

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  # Activate the gem you are reporting the issue against.
  gem "activerecord", "6.0.3"
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :companies, force: true do |t|
    t.string :name, null: false
  end
end

$actual_after_commit_call_objects = []

class Company < ActiveRecord::Base
  after_commit { $actual_after_commit_call_objects << object_id }
end

class BugTest < ActiveSupport::TestCase
  include ActiveRecord::TestFixtures

  self.use_transactional_tests = true

  def test_commit_callbacks_for_identical_ar_model
    expected_after_commit_call_objects = []
    Company.create(name:"S")
    Company.transaction do
      Company.find(1).tap do |company| # object A
        expected_after_commit_call_objects << company.object_id
        company.name = "A"
        company.save!
      end
      Company.transaction do
        Company.find(1).tap do |company| # object A
          expected_after_commit_call_objects << company.object_id
          company.name = "B"
          company.save!
        end
      end
    end
    assert_equal expected_after_commit_call_objects, $actual_after_commit_call_objects
  end
end

Expected behavior

Expected behavior is that the object id of from the inner transaction and after_commit callback should be the same

Actual behavior

The outermost transaction has matching object id (note: they are not in the same position in the expected/actual) but the inner transaction has different object id.

--- expected
+++ actual
@@ -1 +1 @@
-[70320209145860, 70320209176540]
+[70320208918760, 70320209145860]

System configuration

Rails version: 6.0.2.1
Activerecord version: "6.0.3"
Ruby version:
ruby 2.6.3p62

[kamipo]

Related with #39400.

For legacy reason, if saving same id's (but different object_id) objects in a transaction, transactional callbacks are called only for the first enrolled object for now.

[composerinteralia]

Yup, this seems to match #39400. One detail that may make that a bit easier to see:

The first object_id in $actual_after_commit_call_objects is coming from the after_commit of the initial company creation:

Company.create(name:"S")

So we should add that first object_id into expected_after_commit_call_objects as well:

company = Company.create(name:"S")
expected_after_commit_call_objects << company.object_id

Then the test failure will look more like:

--- expected
+++ actual
@@ -1 +1 @@
-[70105658854740, 70105658954200, 70105658970840]
+[70105658854740, 70105658954200]

This makes it more clear that the final after_commit is not happening. This is the case regardless of whether there is a nested transaction. So this version of the test would fail in the same way:

  def test_commit_callbacks_for_identical_ar_model
    expected_after_commit_call_objects = []
    company = Company.create(name: "S")
    expected_after_commit_call_objects << company.object_id

    Company.transaction do
      Company.find(1).tap do |company|
        expected_after_commit_call_objects << company.object_id
        company.save!
      end

      Company.find(1).tap do |company| # object A
        expected_after_commit_call_objects << company.object_id
        company.save!
      end
    end

    assert_equal expected_after_commit_call_objects, $actual_after_commit_call_objects
  end

[tjoyal]

For legacy reason, if saving same id's (but different object_id) objects in a transaction, transactional callbacks are called only for the first enrolled object for now.

Again, big thank for the context.

Could you elaborate beyond "legacy behaviour" if it is a desired effect we want to maintain and to what purpose?

I agree that making behaviour changes is tricky. Base on the information presented I see this as more of a bug than a behaviour we wish to maintain.

Reading how this currently behave, it seems that Rails is saying "you should never refer to the current object in memory during a callback and should fetch a new version from the source as the current object might be stale", making some use cases of after_commit very tricky and performance costly to re-implement.

[kamipo]

I'm not against changing the behavior.

Even if it was not a desired behavior, after_commit has been behaving this way for over 10 years (da840d1), so people may rely on this regardless of intentionally or not (for example, registration mail will be sent only once, etc).

I think we should provide safe migration path for existing users who may rely on the current behavior if we decided to change the behavior.

Deprecation example:

#35280
#36029
#39328
#39358