diff --git a/composer.json b/composer.json index 436269b51f6f..3e3ee6692170 100644 --- a/composer.json +++ b/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", diff --git a/src/Illuminate/Database/Concerns/ManagesTransactions.php b/src/Illuminate/Database/Concerns/ManagesTransactions.php index 14661cc76ebf..80dac1c57b12 100644 --- a/src/Illuminate/Database/Concerns/ManagesTransactions.php +++ b/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); } /** diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index 8d145188f065..a39924db761b 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -7,14 +7,14 @@ class DatabaseTransactionsManager /** * All of the recorded transactions. * - * @var \Illuminate\Support\Collection + * @var \Illuminate\Support\Collection */ 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 */ public function callbackApplicableTransactions() { - return $this->transactions->reject(function ($transaction) { - return $transaction === $this->callbacksShouldIgnore; - })->values(); + return $this->transactions; + } + + /** + * Determine if after commit callbacks should be executed for the given transaction level. + * + * @param int $level + * @return bool + */ + public function afterCommitCallbacksShouldBeExecuted($level) + { + return $level === 1; } /** diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php b/src/Illuminate/Foundation/Testing/DatabaseTransactions.php index d0e0bafc52de..83a686f3558c 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactions.php +++ b/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) { diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php b/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php new file mode 100644 index 000000000000..b2e2de142e99 --- /dev/null +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php @@ -0,0 +1,47 @@ +callbackApplicableTransactions()->count() === 0) { + return $callback(); + } + + $this->transactions->last()->addCallback($callback); + } + + /** + * Get the transactions that are applicable to callbacks. + * + * @return \Illuminate\Support\Collection + */ + public function callbackApplicableTransactions() + { + return $this->transactions->skip(1)->values(); + } + + /** + * Determine if after commit callbacks should be executed for the given transaction level. + * + * @param int $level + * @return bool + */ + public function afterCommitCallbacksShouldBeExecuted($level) + { + return $level === 2; + } +} diff --git a/src/Illuminate/Foundation/Testing/RefreshDatabase.php b/src/Illuminate/Foundation/Testing/RefreshDatabase.php index b486cbb0ac91..0f916ac55b51 100644 --- a/src/Illuminate/Foundation/Testing/RefreshDatabase.php +++ b/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) { diff --git a/tests/Integration/Database/EloquentTransactionWithAfterCommitTest.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitTest.php new file mode 100644 index 000000000000..3035310093cb --- /dev/null +++ b/tests/Integration/Database/EloquentTransactionWithAfterCommitTest.php @@ -0,0 +1,8 @@ +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++; + } +} diff --git a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingDatabaseMigrationsTest.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingDatabaseMigrationsTest.php new file mode 100644 index 000000000000..b54ba759dbb3 --- /dev/null +++ b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingDatabaseMigrationsTest.php @@ -0,0 +1,11 @@ +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"); + } +} diff --git a/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php new file mode 100644 index 000000000000..21bb8291d480 --- /dev/null +++ b/tests/Integration/Database/EloquentTransactionWithAfterCommitUsingRefreshDatabaseTest.php @@ -0,0 +1,37 @@ +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"); + } +}