diff --git a/src/Illuminate/Database/Concerns/ManagesTransactions.php b/src/Illuminate/Database/Concerns/ManagesTransactions.php index 80dac1c57b12..a690f7b5cb46 100644 --- a/src/Illuminate/Database/Concerns/ManagesTransactions.php +++ b/src/Illuminate/Database/Concerns/ManagesTransactions.php @@ -47,6 +47,8 @@ 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()); } @@ -56,8 +58,6 @@ 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'); } @@ -210,8 +210,7 @@ public function commit() */ protected function afterCommitCallbacksShouldBeExecuted() { - return $this->transactions == 0 || - $this->transactionsManager?->afterCommitCallbacksShouldBeExecuted($this->transactions); + return $this->transactionsManager?->afterCommitCallbacksShouldBeExecuted($this->transactions) || $this->transactions == 0; } /** diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index a39924db761b..e198f4f3f6d6 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -11,13 +11,6 @@ class DatabaseTransactionsManager */ protected $transactions; - /** - * The database transaction that should be ignored by callbacks. - * - * @var \Illuminate\Database\DatabaseTransactionRecord|null - */ - protected $callbacksShouldIgnore; - /** * Create a new database transactions manager instance. * @@ -54,10 +47,6 @@ public function rollback($connection, $level) $this->transactions = $this->transactions->reject( fn ($transaction) => $transaction->connection == $connection && $transaction->level > $level )->values(); - - if ($this->transactions->isEmpty()) { - $this->callbacksShouldIgnore = null; - } } /** @@ -75,10 +64,6 @@ public function commit($connection) $this->transactions = $forOtherConnections->values(); $forThisConnection->map->executeCallbacks(); - - if ($this->transactions->isEmpty()) { - $this->callbacksShouldIgnore = null; - } } /** @@ -96,19 +81,6 @@ public function addCallback($callback) $callback(); } - /** - * Specify that callbacks should ignore the given transaction when determining if they should be executed. - * - * @param \Illuminate\Database\DatabaseTransactionRecord $transaction - * @return $this - */ - public function callbacksShouldIgnore(DatabaseTransactionRecord $transaction) - { - $this->callbacksShouldIgnore = $transaction; - - return $this; - } - /** * Get the transactions that are applicable to callbacks. * @@ -127,7 +99,7 @@ public function callbackApplicableTransactions() */ public function afterCommitCallbacksShouldBeExecuted($level) { - return $level === 1; + return $level === 0; } /** diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php b/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php index b2e2de142e99..08c1635443a6 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php @@ -42,6 +42,6 @@ public function callbackApplicableTransactions() */ public function afterCommitCallbacksShouldBeExecuted($level) { - return $level === 2; + return $level === 1; } } diff --git a/tests/Database/DatabaseConnectionTest.php b/tests/Database/DatabaseConnectionTest.php index 0df367eed634..9ad3819a0057 100755 --- a/tests/Database/DatabaseConnectionTest.php +++ b/tests/Database/DatabaseConnectionTest.php @@ -7,6 +7,7 @@ use Exception; use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Database\Connection; +use Illuminate\Database\DatabaseTransactionsManager; use Illuminate\Database\Events\QueryExecuted; use Illuminate\Database\Events\TransactionBeginning; use Illuminate\Database\Events\TransactionCommitted; @@ -289,6 +290,20 @@ public function testCommittingFiresEventsIfSet() $connection->commit(); } + public function testAfterCommitIsExecutedOnFinalCommit() + { + $pdo = $this->getMockBuilder(DatabaseConnectionTestMockPDO::class)->onlyMethods(['beginTransaction', 'commit'])->getMock(); + $transactionsManager = $this->getMockBuilder(DatabaseTransactionsManager::class)->onlyMethods(['afterCommitCallbacksShouldBeExecuted'])->getMock(); + $transactionsManager->expects($this->once())->method('afterCommitCallbacksShouldBeExecuted')->with(0)->willReturn(true); + + $connection = $this->getMockConnection([], $pdo); + $connection->setTransactionManager($transactionsManager); + + $connection->transaction(function () { + // do nothing + }); + } + public function testRollBackedFiresEventsIfSet() { $pdo = $this->createMock(DatabaseConnectionTestMockPDO::class); diff --git a/tests/Integration/Database/EloquentTransactionWithAfterCommitTests.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitTests.php index 41a2eed8a0da..262ef43d9c5d 100644 --- a/tests/Integration/Database/EloquentTransactionWithAfterCommitTests.php +++ b/tests/Integration/Database/EloquentTransactionWithAfterCommitTests.php @@ -2,7 +2,11 @@ namespace Illuminate\Tests\Integration\Database; +use Illuminate\Bus\Queueable; +use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Auth\User; +use Illuminate\Foundation\Bus\Dispatchable; +use Illuminate\Queue\InteractsWithQueue; use Illuminate\Support\Facades\DB; use Orchestra\Testbench\Concerns\WithLaravelMigrations; use Orchestra\Testbench\Factories\UserFactory; @@ -41,6 +45,21 @@ public function testObserverCalledWithAfterCommitWhenInsideTransaction() $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.'); } + public function testObserverCalledWithAfterCommitWhenInsideTransactionWithDispatchSync() + { + User::observe($observer = EloquentTransactionWithAfterCommitTestsUserObserverUsingDispatchSync::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.'); + + $this->assertDatabaseHas('password_reset_tokens', [ + 'email' => $user1->email, + 'token' => sha1($user1->email), + ]); + } + public function testObserverIsCalledOnTestsWithAfterCommitWhenUsingSavepoint() { User::observe($observer = EloquentTransactionWithAfterCommitTestsUserObserver::resetting()); @@ -102,3 +121,32 @@ public function created($user) static::$calledTimes++; } } + +class EloquentTransactionWithAfterCommitTestsUserObserverUsingDispatchSync extends EloquentTransactionWithAfterCommitTestsUserObserver +{ + public function created($user) + { + dispatch_sync(new EloquentTransactionWithAfterCommitTestsJob($user->email)); + + parent::created($user); + } +} + +class EloquentTransactionWithAfterCommitTestsJob implements ShouldQueue +{ + use Dispatchable, InteractsWithQueue, Queueable; + + public function __construct(public string $email) + { + // ... + } + + public function handle(): void + { + DB::transaction(function () { + DB::table('password_reset_tokens')->insert([ + ['email' => $this->email, 'token' => sha1($this->email), 'created_at' => now()], + ]); + }); + } +}