[10.x] Fix "after commit" callbacks not running on nested transactions using RefreshDatabase or DatabaseMigrations

composer.json

@@ -104,7 +104,7 @@
         "league/flysystem-read-only": "^3.3",
         "league/flysystem-sftp-v3": "^3.0",
         "mockery/mockery": "^1.5.1",
-        "orchestra/testbench-core": "^8.10",
+        "orchestra/testbench-core": "^8.12",
         "pda/pheanstalk": "^4.0",
         "phpstan/phpstan": "^1.4.7",
         "phpunit/phpunit": "^10.0.7",

src/Illuminate/Database/Concerns/ManagesTransactions.php

@@ -47,8 +47,6 @@ public function transaction(Closure $callback, $attempts = 1)
                     $this->getPdo()->commit();
                 }
 
-                $this->transactions = max(0, $this->transactions - 1);
-
                 if ($this->afterCommitCallbacksShouldBeExecuted()) {
                     $this->transactionsManager?->commit($this->getName());
                 }
@@ -58,6 +56,8 @@ public function transaction(Closure $callback, $attempts = 1)
                 );
 
                 continue;
+            } finally {
+                $this->transactions = max(0, $this->transactions - 1);
             }
 
             $this->fireConnectionEvent('committed');
@@ -194,12 +194,12 @@ public function commit()
             $this->getPdo()->commit();
         }
 
-        $this->transactions = max(0, $this->transactions - 1);
-
         if ($this->afterCommitCallbacksShouldBeExecuted()) {
             $this->transactionsManager?->commit($this->getName());
         }
 
+        $this->transactions = max(0, $this->transactions - 1);
+
         $this->fireConnectionEvent('committed');
     }
 
@@ -211,8 +211,7 @@ public function commit()
     protected function afterCommitCallbacksShouldBeExecuted()
     {
         return $this->transactions == 0 ||
-            ($this->transactionsManager &&
-             $this->transactionsManager->callbackApplicableTransactions()->count() === 1);
+            $this->transactionsManager?->afterCommitCallbacksShouldBeExecuted($this->transactions);
     }
 
     /**

src/Illuminate/Database/DatabaseTransactionsManager.php

@@ -7,14 +7,14 @@ class DatabaseTransactionsManager
     /**
      * All of the recorded transactions.
      *
-     * @var \Illuminate\Support\Collection
+     * @var \Illuminate\Support\Collection<int, \Illuminate\Database\DatabaseTransactionRecord>
      */
     protected $transactions;
 
     /**
      * The database transaction that should be ignored by callbacks.
      *
-     * @var \Illuminate\Database\DatabaseTransactionRecord
+     * @var \Illuminate\Database\DatabaseTransactionRecord|null
      */
     protected $callbacksShouldIgnore;
 
@@ -112,13 +112,22 @@ public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction)
     /**
      * Get the transactions that are applicable to callbacks.
      *
-     * @return \Illuminate\Support\Collection
+     * @return \Illuminate\Support\Collection<int, \Illuminate\Database\DatabaseTransactionRecord>
      */
     public function callbackApplicableTransactions()
     {
-        return $this->transactions->reject(function ($transaction) {
-            return $transaction === $this->callbacksShouldIgnore;
-        })->values();
+        return $this->transactions->values();
+    }
+
+    /**
+     * Determine if after commit callbacks should be executed.
+     *
+     * @param  int  $level
+     * @return bool
+     */
+    public function afterCommitCallbacksShouldBeExecuted($level)
+    {
+        return $level === 1;
     }
 
     /**

src/Illuminate/Foundation/Testing/DatabaseTransactions.php

@@ -13,19 +13,16 @@ public function beginDatabaseTransaction()
     {
         $database = $this->app->make('db');
 
+        $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager());
+
         foreach ($this->connectionsToTransact() as $name) {
             $connection = $database->connection($name);
+            $connection->setTransactionManager($transactionsManager);
             $dispatcher = $connection->getEventDispatcher();
 
             $connection->unsetEventDispatcher();
             $connection->beginTransaction();
             $connection->setEventDispatcher($dispatcher);
-
-            if ($this->app->resolved('db.transactions')) {
-                $this->app->make('db.transactions')->callbacksShouldIgnore(
-                    $this->app->make('db.transactions')->getTransactions()->first()
-                );
-            }
         }
 
         $this->beforeApplicationDestroyed(function () use ($database) {

src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php

@@ -0,0 +1,47 @@
+<?php
+
+namespace Illuminate\Foundation\Testing;
+
+class DatabaseTransactionsManager extends \Illuminate\Database\DatabaseTransactionsManager
+{
+    /**
+     * Register a transaction callback.
+     *
+     * @param  callable  $callback
+     * @return void
+     */
+    public function addCallback($callback)
+    {
+        // If there are no transactions, we'll run the callbacks right away. Also, we'll run it
+        // right away when we're in test mode and we only have the wrapping transaction. For
+        // every other case, we'll queue up the callback to run after the commit happens.
+        if ($this->callbackApplicableTransactions()->count() === 0) {
+            return $callback();
+        }
+
+        $this->transactions->last()->addCallback($callback);
+    }
+
+    /**
+     * Get the transactions that are applicable to callbacks.
+     *
+     * @return \Illuminate\Support\Collection<int, \Illuminate\Database\DatabaseTransactionRecord>
+     */
+    public function callbackApplicableTransactions()
+    {
+        return $this->transactions->reject(
+            fn ($transaction) => $transaction === $this->transactions->first()
+        )->values();
+    }
+
+    /**
+     * Determine if after commit callbacks should be executed.
+     *
+     * @param  int  $level
+     * @return bool
+     */
+    public function afterCommitCallbacksShouldBeExecuted($level)
+    {
+        return $level === 2;
+    }
+}

src/Illuminate/Foundation/Testing/RefreshDatabase.php

@@ -89,19 +89,16 @@ public function beginDatabaseTransaction()
     {
         $database = $this->app->make('db');
 
+        $this->app->instance('db.transactions', $transactionsManager = new DatabaseTransactionsManager());
+
         foreach ($this->connectionsToTransact() as $name) {
             $connection = $database->connection($name);
+            $connection->setTransactionManager($transactionsManager);
             $dispatcher = $connection->getEventDispatcher();
 
             $connection->unsetEventDispatcher();
             $connection->beginTransaction();
             $connection->setEventDispatcher($dispatcher);
-
-            if ($this->app->resolved('db.transactions')) {
-                $this->app->make('db.transactions')->callbacksShouldIgnore(
-                    $this->app->make('db.transactions')->getTransactions()->first()
-                );
-            }
         }
 
         $this->beforeApplicationDestroyed(function () use ($database) {

tests/Integration/Database/EloquentTransactionWithAfterCommitTest.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Illuminate\Tests\Integration\Database;
+
+class EloquentTransactionWithAfterCommitTest extends DatabaseTestCase
+{
+    use EloquentTransactionWithAfterCommitTests;
+}

tests/Integration/Database/EloquentTransactionWithAfterCommitTests.php

@@ -0,0 +1,104 @@
+<?php
+
+namespace Illuminate\Tests\Integration\Database;
+
+use Illuminate\Foundation\Auth\User;
+use Illuminate\Support\Facades\DB;
+use Orchestra\Testbench\Concerns\WithLaravelMigrations;
+use Orchestra\Testbench\Factories\UserFactory;
+
+trait EloquentTransactionWithAfterCommitTests
+{
+    use WithLaravelMigrations;
+
+    protected function setUpEloquentTransactionWithAfterCommitTests(): void
+    {
+        User::unguard();
+    }
+
+    protected function tearDownEloquentTransactionWithAfterCommitTests(): void
+    {
+        User::reguard();
+    }
+
+    public function testObserverIsCalledOnTestsWithAfterCommit()
+    {
+        User::observe($observer = EloquentTransactionWithAfterCommitTestsUserObserver::resetting());
+
+        $user1 = User::create(UserFactory::new()->raw());
+
+        $this->assertTrue($user1->exists);
+        $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.');
+    }
+
+    public function testObserverCalledWithAfterCommitWhenInsideTransaction()
+    {
+        User::observe($observer = EloquentTransactionWithAfterCommitTestsUserObserver::resetting());
+
+        $user1 = DB::transaction(fn () => User::create(UserFactory::new()->raw()));
+
+        $this->assertTrue($user1->exists);
+        $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.');
+    }
+
+    public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepoint()
+    {
+        User::observe($observer = EloquentTransactionWithAfterCommitTestsUserObserver::resetting());
+
+        $user1 = User::createOrFirst(UserFactory::new()->raw());
+
+        $this->assertTrue($user1->exists);
+        $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.');
+    }
+
+    public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepointAndInsideTransaction()
+    {
+        User::observe($observer = EloquentTransactionWithAfterCommitTestsUserObserver::resetting());
+
+        $user1 = DB::transaction(fn () => User::createOrFirst(UserFactory::new()->raw()));
+
+        $this->assertTrue($user1->exists);
+        $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.');
+    }
+
+    public function testObserverIsCalledEvenWhenDeeplyNestingTransactions()
+    {
+        User::observe($observer = EloquentTransactionWithAfterCommitTestsUserObserver::resetting());
+
+        $user1 = DB::transaction(function () use ($observer) {
+            return tap(DB::transaction(function () use ($observer) {
+                return tap(DB::transaction(function () use ($observer) {
+                    return tap(User::createOrFirst(UserFactory::new()->raw()), function () use ($observer) {
+                        $this->assertEquals(0, $observer::$calledTimes, 'Should not have been called');
+                    });
+                }), function () use ($observer) {
+                    $this->assertEquals(0, $observer::$calledTimes, 'Should not have been called');
+                });
+            }), function () use ($observer) {
+                $this->assertEquals(0, $observer::$calledTimes, 'Should not have been called');
+            });
+        });
+
+        $this->assertTrue($user1->exists);
+        $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.');
+    }
+}
+
+class EloquentTransactionWithAfterCommitTestsUserObserver
+{
+    public static $calledTimes = 0;
+
+    public $afterCommit = true;
+
+    public static function resetting()
+    {
+        static::$calledTimes = 0;
+
+        return new static();
+    }
+
+    public function created($user)
+    {
+        static::$calledTimes++;
+    }
+}

tests/Integration/Database/EloquentTransactionWithAfterCommitUsingDatabaseMigrationsTest.php

@@ -0,0 +1,11 @@
+<?php
+
+namespace Illuminate\Tests\Integration\Database;
+
+use Illuminate\Foundation\Testing\DatabaseMigrations;
+
+class EloquentTransactionWithAfterCommitUsingDatabaseMigrationsTest extends DatabaseTestCase
+{
+    use EloquentTransactionWithAfterCommitTests;
+    use DatabaseMigrations;
+}

tests/Integration/Database/EloquentTransactionWithAfterCommitUsingDatabaseTransactionsTest.php

@@ -0,0 +1,41 @@
+<?php
+
+namespace Illuminate\Tests\Integration\Database;
+
+use Illuminate\Foundation\Testing\DatabaseTransactions;
+use Orchestra\Testbench\TestCase;
+
+class EloquentTransactionWithAfterCommitUsingDatabaseTransactionsTest extends TestCase
+{
+    use EloquentTransactionWithAfterCommitTests;
+    use DatabaseTransactions;
+
+    /**
+     * The current database driver.
+     *
+     * @return string
+     */
+    protected $driver;
+
+    protected function setUp(): void
+    {
+        $this->beforeApplicationDestroyed(function () {
+            foreach (array_keys($this->app['db']->getConnections()) as $name) {
+                $this->app['db']->purge($name);
+            }
+        });
+
+        parent::setUp();
+
+        if ($this->usesSqliteInMemoryDatabaseConnection()) {
+            $this->markTestSkipped('Test cannot be used with in-memory SQLite connection.');
+        }
+    }
+
+    protected function getEnvironmentSetUp($app)
+    {
+        $connection = $app->make('config')->get('database.default');
+
+        $this->driver = $app['config']->get("database.connections.$connection.driver");
+    }
+}

tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace Illuminate\Tests\Integration\Database;
+
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Orchestra\Testbench\TestCase;
+
+class EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest extends TestCase
+{
+    use EloquentTransactionWithAfterCommitTests;
+    use RefreshDatabase;
+
+    /**
+     * The current database driver.
+     *
+     * @return string
+     */
+    protected $driver;
+
+    protected function setUp(): void
+    {
+        $this->beforeApplicationDestroyed(function () {
+            foreach (array_keys($this->app['db']->getConnections()) as $name) {
+                $this->app['db']->purge($name);
+            }
+        });
+
+        parent::setUp();
+    }
+
+    protected function getEnvironmentSetUp($app)
+    {
+        $connection = $app['config']->get('database.default');
+
+        $this->driver = $app['config']->get("database.connections.$connection.driver");
+    }
+}