From f63f9856d2e9b11e9746a49c268f75de6750e657 Mon Sep 17 00:00:00 2001 From: Pauline Vos Date: Thu, 4 Sep 2025 16:00:42 +0200 Subject: [PATCH] Use DB tx manager in `ManagesTransactions` trait `DatabaseTransactionsManager` was introduced in Laravel to keep track of things like pending transactions and fire transaction events, so that e.g. dispatches could hook into transaction state. Our trait was not using it yet, resulting in missing `afterCommit` behavior (see PHPLIB-373) --- src/Concerns/ManagesTransactions.php | 78 +++++++- tests/Ticket/GH3328Test.php | 266 +++++++++++++++++++++++++++ 2 files changed, 341 insertions(+), 3 deletions(-) create mode 100644 tests/Ticket/GH3328Test.php diff --git a/src/Concerns/ManagesTransactions.php b/src/Concerns/ManagesTransactions.php index 6403cc45d..534e1548e 100644 --- a/src/Concerns/ManagesTransactions.php +++ b/src/Concerns/ManagesTransactions.php @@ -8,9 +8,12 @@ use MongoDB\Client; use MongoDB\Driver\Exception\RuntimeException; use MongoDB\Driver\Session; +use MongoDB\Laravel\Connection; use Throwable; +use function max; use function MongoDB\with_transaction; +use function property_exists; /** * @internal @@ -55,8 +58,23 @@ private function getSessionOrThrow(): Session */ public function beginTransaction(array $options = []): void { + $this->runCallbacksBeforeTransaction(); + $this->getSessionOrCreate()->startTransaction($options); + + $this->handleInitialTransactionState(); + } + + private function handleInitialTransactionState(): void + { $this->transactions = 1; + + $this->transactionsManager?->begin( + $this->getName(), + $this->transactions, + ); + + $this->fireConnectionEvent('beganTransaction'); } /** @@ -64,8 +82,26 @@ public function beginTransaction(array $options = []): void */ public function commit(): void { + $this->fireConnectionEvent('committing'); $this->getSessionOrThrow()->commitTransaction(); - $this->transactions = 0; + + $this->handleCommitState(); + } + + private function handleCommitState(): void + { + [$levelBeingCommitted, $this->transactions] = [ + $this->transactions, + max(0, $this->transactions - 1), + ]; + + $this->transactionsManager?->commit( + $this->getName(), + $levelBeingCommitted, + $this->transactions, + ); + + $this->fireConnectionEvent('committed'); } /** @@ -73,14 +109,42 @@ public function commit(): void */ public function rollBack($toLevel = null): void { - $this->getSessionOrThrow()->abortTransaction(); + $session = $this->getSessionOrThrow(); + if ($session->isInTransaction()) { + $session->abortTransaction(); + } + + $this->handleRollbackState(); + } + + private function handleRollbackState(): void + { $this->transactions = 0; + + $this->transactionsManager?->rollback( + $this->getName(), + $this->transactions, + ); + + $this->fireConnectionEvent('rollingBack'); + } + + private function runCallbacksBeforeTransaction(): void + { + // ToDo: remove conditional once we stop supporting Laravel 10.x + if (property_exists(Connection::class, 'beforeStartingTransaction')) { + foreach ($this->beforeStartingTransaction as $beforeTransactionCallback) { + $beforeTransactionCallback($this); + } + } } /** * Static transaction function realize the with_transaction functionality provided by MongoDB. * - * @param int $attempts + * @param int $attempts + * + * @throws Throwable */ public function transaction(Closure $callback, $attempts = 1, array $options = []): mixed { @@ -93,15 +157,20 @@ public function transaction(Closure $callback, $attempts = 1, array $options = [ if ($attemptsLeft < 0) { $session->abortTransaction(); + $this->handleRollbackState(); return; } + $this->runCallbacksBeforeTransaction(); + $this->handleInitialTransactionState(); + // Catch, store, and re-throw any exception thrown during execution // of the callable. The last exception is re-thrown if the transaction // was aborted because the number of callback attempts has been exceeded. try { $callbackResult = $callback($this); + $this->fireConnectionEvent('committing'); } catch (Throwable $throwable) { throw $throwable; } @@ -110,9 +179,12 @@ public function transaction(Closure $callback, $attempts = 1, array $options = [ with_transaction($this->getSessionOrCreate(), $callbackFunction, $options); if ($attemptsLeft < 0 && $throwable) { + $this->handleRollbackState(); throw $throwable; } + $this->handleCommitState(); + return $callbackResult; } } diff --git a/tests/Ticket/GH3328Test.php b/tests/Ticket/GH3328Test.php new file mode 100644 index 000000000..6b25426f1 --- /dev/null +++ b/tests/Ticket/GH3328Test.php @@ -0,0 +1,266 @@ +beforeStartingTransactionIsSupported()) { + Event::assertDispatchedTimes(BeforeTransactionEvent::class); + } + + Event::assertDispatchedTimes(RegularEvent::class); + Event::assertDispatchedTimes(AfterCommitEvent::class); + + Event::assertDispatched(TransactionBeginning::class); + Event::assertDispatched(TransactionCommitting::class); + Event::assertDispatched(TransactionCommitted::class); + }; + + $this->assertTransactionCallbackResult($callback, $assert); + } + + public function testAfterCommitOnFailedTransaction(): void + { + $callback = static function (): void { + event(new RegularEvent()); + event(new AfterCommitEvent()); + + // Transaction failed; after commit event should not be dispatched + throw new Fake(); + }; + + $assert = function (): void { + if ($this->beforeStartingTransactionIsSupported()) { + Event::assertDispatchedTimes(BeforeTransactionEvent::class, 3); + } + + Event::assertDispatchedTimes(RegularEvent::class, 3); + + Event::assertDispatchedTimes(TransactionBeginning::class, 3); + Event::assertDispatched(TransactionRolledBack::class); + Event::assertNotDispatched(TransactionCommitting::class); + Event::assertNotDispatched(TransactionCommitted::class); + }; + + $this->assertCallbackResultForConnection( + DB::connection('mongodb'), + $callback, + $assert, + 3, + ); + + if (! interface_exists(ConcurrencyErrorDetector::class)) { + // Earlier versions of Laravel use a trait instead of DI to detect concurrency errors + // That would increase the scope of this comparison dramatically and is probably not worth it. + return; + } + + $this->app->bind(ConcurrencyErrorDetector::class, FakeConcurrencyErrorDetector::class); + + $this->assertCallbackResultForConnection( + DB::connection('sqlite'), + $callback, + $assert, + 3, + ); + } + + public function testAfterCommitOnSuccessfulManualTransaction(): void + { + $callback = function (): void { + event(new RegularEvent()); + event(new AfterCommitEvent()); + }; + + $assert = function (): void { + if ($this->beforeStartingTransactionIsSupported()) { + Event::assertDispatchedTimes(BeforeTransactionEvent::class); + } + + Event::assertDispatchedTimes(RegularEvent::class); + Event::assertDispatchedTimes(AfterCommitEvent::class); + + Event::assertDispatched(TransactionBeginning::class); + Event::assertNotDispatched(TransactionRolledBack::class); + Event::assertDispatched(TransactionCommitting::class); + Event::assertDispatched(TransactionCommitted::class); + }; + + $this->assertTransactionResult($callback, $assert); + } + + public function testAfterCommitOnFailedManualTransaction(): void + { + $callback = function (): void { + event(new RegularEvent()); + event(new AfterCommitEvent()); + + throw new Fake(); + }; + + $assert = function (): void { + if ($this->beforeStartingTransactionIsSupported()) { + Event::assertDispatchedTimes(BeforeTransactionEvent::class); + } + + Event::assertDispatchedTimes(RegularEvent::class); + Event::assertNotDispatched(AfterCommitEvent::class); + + Event::assertDispatched(TransactionBeginning::class); + Event::assertDispatched(TransactionRolledBack::class); + Event::assertNotDispatched(TransactionCommitting::class); + Event::assertNotDispatched(TransactionCommitted::class); + }; + + $this->assertTransactionResult($callback, $assert); + } + + private function assertTransactionCallbackResult(Closure $callback, Closure $assert, ?int $attempts = 1): void + { + $this->assertCallbackResultForConnection( + DB::connection('sqlite'), + $callback, + $assert, + $attempts, + ); + + $this->assertCallbackResultForConnection( + DB::connection('mongodb'), + $callback, + $assert, + $attempts, + ); + } + + /** + * Ensure equal transaction behavior between SQLite (handled by Laravel) and MongoDB + */ + private function assertCallbackResultForConnection(Connection $connection, Closure $callback, Closure $assertions, int $attempts): void + { + $fake = Event::fake(); + $connection->setEventDispatcher($fake); + + if ($this->beforeStartingTransactionIsSupported()) { + $connection->beforeStartingTransaction(function () { + event(new BeforeTransactionEvent()); + }); + } + + try { + $connection->transaction($callback, $attempts); + } catch (Exception) { + } + + $assertions(); + } + + private function assertTransactionResult(Closure $callback, Closure $assert): void + { + $this->assertManualResultForConnection( + DB::connection('sqlite'), + $callback, + $assert, + ); + + $this->assertManualResultForConnection( + DB::connection('mongodb'), + $callback, + $assert, + ); + } + + /** + * Ensure equal transaction behavior between SQLite (handled by Laravel) and MongoDB + */ + private function assertManualResultForConnection(Connection $connection, Closure $callback, Closure $assert): void + { + $fake = Event::fake(); + $connection->setEventDispatcher($fake); + + if ($this->beforeStartingTransactionIsSupported()) { + $connection->beforeStartingTransaction(function () { + event(new BeforeTransactionEvent()); + }); + } + + $connection->beginTransaction(); + + try { + $callback(); + $connection->commit(); + } catch (Exception) { + $connection->rollBack(); + } + + $assert(); + } + + private function beforeStartingTransactionIsSupported(): bool + { + return property_exists(ManagesTransactions::class, 'beforeStartingTransaction'); + } +} + +class AfterCommitEvent implements ShouldDispatchAfterCommit +{ + use Dispatchable; +} + +class BeforeTransactionEvent +{ + use Dispatchable; +} +class RegularEvent +{ + use Dispatchable; +} +class Fake extends RuntimeException +{ + public function __construct() + { + $this->errorLabels = ['TransientTransactionError']; + } +} + +if (interface_exists(ConcurrencyErrorDetector::class)) { + class FakeConcurrencyErrorDetector implements ConcurrencyErrorDetector + { + public function causedByConcurrencyError(Throwable $e): bool + { + return true; + } + } +}