diff --git a/src/Illuminate/Database/Concerns/ManagesTransactions.php b/src/Illuminate/Database/Concerns/ManagesTransactions.php index a690f7b5cb46..0f6aa0239a71 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->transactionsManager?->stageTransactions($this->getName()); + $this->transactions = max(0, $this->transactions - 1); if ($this->afterCommitCallbacksShouldBeExecuted()) { @@ -194,6 +196,8 @@ public function commit() $this->getPdo()->commit(); } + $this->transactionsManager?->stageTransactions($this->getName()); + $this->transactions = max(0, $this->transactions - 1); if ($this->afterCommitCallbacksShouldBeExecuted()) { diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php index e198f4f3f6d6..2914464858e6 100755 --- a/src/Illuminate/Database/DatabaseTransactionsManager.php +++ b/src/Illuminate/Database/DatabaseTransactionsManager.php @@ -2,14 +2,23 @@ namespace Illuminate\Database; +use Illuminate\Support\Collection; + class DatabaseTransactionsManager { /** - * All of the recorded transactions. + * All of the committed transactions. + * + * @var \Illuminate\Support\Collection + */ + protected $committedTransactions; + + /** + * All of the pending transactions. * * @var \Illuminate\Support\Collection */ - protected $transactions; + protected $pendingTransactions; /** * Create a new database transactions manager instance. @@ -18,7 +27,8 @@ class DatabaseTransactionsManager */ public function __construct() { - $this->transactions = collect(); + $this->committedTransactions = new Collection; + $this->pendingTransactions = new Collection; } /** @@ -30,7 +40,7 @@ public function __construct() */ public function begin($connection, $level) { - $this->transactions->push( + $this->pendingTransactions->push( new DatabaseTransactionRecord($connection, $level) ); } @@ -44,7 +54,7 @@ public function begin($connection, $level) */ public function rollback($connection, $level) { - $this->transactions = $this->transactions->reject( + $this->pendingTransactions = $this->pendingTransactions->reject( fn ($transaction) => $transaction->connection == $connection && $transaction->level > $level )->values(); } @@ -57,11 +67,11 @@ public function rollback($connection, $level) */ public function commit($connection) { - [$forThisConnection, $forOtherConnections] = $this->transactions->partition( + [$forThisConnection, $forOtherConnections] = $this->committedTransactions->partition( fn ($transaction) => $transaction->connection == $connection ); - $this->transactions = $forOtherConnections->values(); + $this->committedTransactions = $forOtherConnections->values(); $forThisConnection->map->executeCallbacks(); } @@ -81,6 +91,23 @@ public function addCallback($callback) $callback(); } + /** + * Move all the pending transactions to a committed state. + * + * @param string $connection + * @return void + */ + public function stageTransactions($connection) + { + $this->committedTransactions = $this->committedTransactions->merge( + $this->pendingTransactions->filter(fn ($transaction) => $transaction->connection === $connection) + ); + + $this->pendingTransactions = $this->pendingTransactions->reject( + fn ($transaction) => $transaction->connection === $connection + ); + } + /** * Get the transactions that are applicable to callbacks. * @@ -88,7 +115,7 @@ public function addCallback($callback) */ public function callbackApplicableTransactions() { - return $this->transactions; + return $this->pendingTransactions; } /** @@ -103,12 +130,22 @@ public function afterCommitCallbacksShouldBeExecuted($level) } /** - * Get all the transactions. + * Get all of the pending transactions. + * + * @return \Illuminate\Support\Collection + */ + public function getPendingTransactions() + { + return $this->pendingTransactions; + } + + /** + * Get all of the committed transactions. * * @return \Illuminate\Support\Collection */ - public function getTransactions() + public function getCommittedTransactions() { - return $this->transactions; + return $this->committedTransactions; } } diff --git a/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php b/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php index 08c1635443a6..bc5450486d48 100644 --- a/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php +++ b/src/Illuminate/Foundation/Testing/DatabaseTransactionsManager.php @@ -21,7 +21,7 @@ public function addCallback($callback) return $callback(); } - $this->transactions->last()->addCallback($callback); + $this->pendingTransactions->last()->addCallback($callback); } /** @@ -31,7 +31,7 @@ public function addCallback($callback) */ public function callbackApplicableTransactions() { - return $this->transactions->skip(1)->values(); + return $this->pendingTransactions->skip(1)->values(); } /** diff --git a/tests/Database/DatabaseTransactionsManagerTest.php b/tests/Database/DatabaseTransactionsManagerTest.php index e8d82e048720..e303b82d89cd 100755 --- a/tests/Database/DatabaseTransactionsManagerTest.php +++ b/tests/Database/DatabaseTransactionsManagerTest.php @@ -15,13 +15,13 @@ public function testBeginningTransactions() $manager->begin('default', 2); $manager->begin('admin', 1); - $this->assertCount(3, $manager->getTransactions()); - $this->assertSame('default', $manager->getTransactions()[0]->connection); - $this->assertEquals(1, $manager->getTransactions()[0]->level); - $this->assertSame('default', $manager->getTransactions()[1]->connection); - $this->assertEquals(2, $manager->getTransactions()[1]->level); - $this->assertSame('admin', $manager->getTransactions()[2]->connection); - $this->assertEquals(1, $manager->getTransactions()[2]->level); + $this->assertCount(3, $manager->getPendingTransactions()); + $this->assertSame('default', $manager->getPendingTransactions()[0]->connection); + $this->assertEquals(1, $manager->getPendingTransactions()[0]->level); + $this->assertSame('default', $manager->getPendingTransactions()[1]->connection); + $this->assertEquals(2, $manager->getPendingTransactions()[1]->level); + $this->assertSame('admin', $manager->getPendingTransactions()[2]->connection); + $this->assertEquals(1, $manager->getPendingTransactions()[2]->level); } public function testRollingBackTransactions() @@ -34,13 +34,13 @@ public function testRollingBackTransactions() $manager->rollback('default', 1); - $this->assertCount(2, $manager->getTransactions()); + $this->assertCount(2, $manager->getPendingTransactions()); - $this->assertSame('default', $manager->getTransactions()[0]->connection); - $this->assertEquals(1, $manager->getTransactions()[0]->level); + $this->assertSame('default', $manager->getPendingTransactions()[0]->connection); + $this->assertEquals(1, $manager->getPendingTransactions()[0]->level); - $this->assertSame('admin', $manager->getTransactions()[1]->connection); - $this->assertEquals(1, $manager->getTransactions()[1]->level); + $this->assertSame('admin', $manager->getPendingTransactions()[1]->connection); + $this->assertEquals(1, $manager->getPendingTransactions()[1]->level); } public function testRollingBackTransactionsAllTheWay() @@ -53,10 +53,10 @@ public function testRollingBackTransactionsAllTheWay() $manager->rollback('default', 0); - $this->assertCount(1, $manager->getTransactions()); + $this->assertCount(1, $manager->getPendingTransactions()); - $this->assertSame('admin', $manager->getTransactions()[0]->connection); - $this->assertEquals(1, $manager->getTransactions()[0]->level); + $this->assertSame('admin', $manager->getPendingTransactions()[0]->connection); + $this->assertEquals(1, $manager->getPendingTransactions()[0]->level); } public function testCommittingTransactions() @@ -67,12 +67,15 @@ public function testCommittingTransactions() $manager->begin('default', 2); $manager->begin('admin', 1); + $manager->stageTransactions('default'); + $manager->stageTransactions('admin'); $manager->commit('default'); - $this->assertCount(1, $manager->getTransactions()); + $this->assertCount(0, $manager->getPendingTransactions()); + $this->assertCount(1, $manager->getCommittedTransactions()); - $this->assertSame('admin', $manager->getTransactions()[0]->connection); - $this->assertEquals(1, $manager->getTransactions()[0]->level); + $this->assertSame('admin', $manager->getCommittedTransactions()[0]->connection); + $this->assertEquals(1, $manager->getCommittedTransactions()[0]->level); } public function testCallbacksAreAddedToTheCurrentTransaction() @@ -93,9 +96,9 @@ public function testCallbacksAreAddedToTheCurrentTransaction() $manager->addCallback(function () use (&$callbacks) { }); - $this->assertCount(1, $manager->getTransactions()[0]->getCallbacks()); - $this->assertCount(0, $manager->getTransactions()[1]->getCallbacks()); - $this->assertCount(1, $manager->getTransactions()[2]->getCallbacks()); + $this->assertCount(1, $manager->getPendingTransactions()[0]->getCallbacks()); + $this->assertCount(0, $manager->getPendingTransactions()[1]->getCallbacks()); + $this->assertCount(1, $manager->getPendingTransactions()[2]->getCallbacks()); } public function testCommittingTransactionsExecutesCallbacks() @@ -118,6 +121,7 @@ public function testCommittingTransactionsExecutesCallbacks() $manager->begin('admin', 1); + $manager->stageTransactions('default'); $manager->commit('default'); $this->assertCount(2, $callbacks); @@ -144,6 +148,7 @@ public function testCommittingExecutesOnlyCallbacksOfTheConnection() $callbacks[] = ['admin', 1]; }); + $manager->stageTransactions('default'); $manager->commit('default'); $this->assertCount(1, $callbacks); @@ -163,4 +168,33 @@ public function testCallbackIsExecutedIfNoTransactions() $this->assertCount(1, $callbacks); $this->assertEquals(['default', 1], $callbacks[0]); } + + public function testStageTransactions() + { + $manager = (new DatabaseTransactionsManager); + + $manager->begin('default', 1); + $manager->begin('admin', 1); + + $this->assertCount(2, $manager->getPendingTransactions()); + + $pendingTransactions = $manager->getPendingTransactions(); + + $this->assertEquals(1, $pendingTransactions[0]->level); + $this->assertEquals('default', $pendingTransactions[0]->connection); + $this->assertEquals(1, $pendingTransactions[1]->level); + $this->assertEquals('admin', $pendingTransactions[1]->connection); + + $manager->stageTransactions('default'); + + $this->assertCount(1, $manager->getPendingTransactions()); + $this->assertCount(1, $manager->getCommittedTransactions()); + $this->assertEquals('default', $manager->getCommittedTransactions()[0]->connection); + + $manager->stageTransactions('admin'); + + $this->assertCount(0, $manager->getPendingTransactions()); + $this->assertCount(2, $manager->getCommittedTransactions()); + $this->assertEquals('admin', $manager->getCommittedTransactions()[1]->connection); + } } diff --git a/tests/Database/DatabaseTransactionsTest.php b/tests/Database/DatabaseTransactionsTest.php index ac9587419eeb..94998f010c01 100644 --- a/tests/Database/DatabaseTransactionsTest.php +++ b/tests/Database/DatabaseTransactionsTest.php @@ -64,6 +64,7 @@ public function testTransactionIsRecordedAndCommitted() { $transactionManager = m::mock(new DatabaseTransactionsManager); $transactionManager->shouldReceive('begin')->once()->with('default', 1); + $transactionManager->shouldReceive('stageTransactions')->once()->with('default'); $transactionManager->shouldReceive('commit')->once()->with('default'); $this->connection()->setTransactionManager($transactionManager); @@ -83,6 +84,7 @@ public function testTransactionIsRecordedAndCommittedUsingTheSeparateMethods() { $transactionManager = m::mock(new DatabaseTransactionsManager); $transactionManager->shouldReceive('begin')->once()->with('default', 1); + $transactionManager->shouldReceive('stageTransactions')->once()->with('default'); $transactionManager->shouldReceive('commit')->once()->with('default'); $this->connection()->setTransactionManager($transactionManager); @@ -103,6 +105,7 @@ public function testNestedTransactionIsRecordedAndCommitted() $transactionManager = m::mock(new DatabaseTransactionsManager); $transactionManager->shouldReceive('begin')->once()->with('default', 1); $transactionManager->shouldReceive('begin')->once()->with('default', 2); + $transactionManager->shouldReceive('stageTransactions')->twice()->with('default'); $transactionManager->shouldReceive('commit')->once()->with('default'); $this->connection()->setTransactionManager($transactionManager); @@ -130,6 +133,8 @@ public function testNestedTransactionIsRecordeForDifferentConnectionsdAndCommitt $transactionManager->shouldReceive('begin')->once()->with('default', 1); $transactionManager->shouldReceive('begin')->once()->with('second_connection', 1); $transactionManager->shouldReceive('begin')->once()->with('second_connection', 2); + $transactionManager->shouldReceive('stageTransactions')->once()->with('default'); + $transactionManager->shouldReceive('stageTransactions')->twice()->with('second_connection'); $transactionManager->shouldReceive('commit')->once()->with('default'); $transactionManager->shouldReceive('commit')->once()->with('second_connection'); diff --git a/tests/Foundation/Testing/DatabaseTransactionsManagerTest.php b/tests/Foundation/Testing/DatabaseTransactionsManagerTest.php new file mode 100644 index 000000000000..b0add2e650eb --- /dev/null +++ b/tests/Foundation/Testing/DatabaseTransactionsManagerTest.php @@ -0,0 +1,71 @@ +begin('foo', 1); + + $manager->addCallback(fn () => $testObject->handle()); + + $this->assertTrue($testObject->ran); + $this->assertEquals(1, $testObject->runs); + } + + public function testItIgnoresTheBaseTransactionForCallbackApplicableTransactions() + { + $manager = new DatabaseTransactionsManager; + + $manager->begin('foo', 1); + $manager->begin('foo', 2); + + $this->assertCount(1, $manager->callbackApplicableTransactions()); + $this->assertEquals(2, $manager->callbackApplicableTransactions()[0]->level); + } + + public function testItExecutesCallbacksForTheSecondTransaction() + { + $testObject = new TestingDatabaseTransactionsManagerTestObject(); + $manager = new DatabaseTransactionsManager; + $manager->begin('foo', 1); + $manager->begin('foo', 2); + + $manager->addCallback(fn () => $testObject->handle()); + + $this->assertFalse($testObject->ran); + + $manager->stageTransactions('foo'); + $manager->commit('foo'); + $this->assertTrue($testObject->ran); + $this->assertEquals(1, $testObject->runs); + } + + public function testItExecutesTransactionCallbacksAtLevelOne() + { + $manager = new DatabaseTransactionsManager; + + $this->assertFalse($manager->afterCommitCallbacksShouldBeExecuted(0)); + $this->assertTrue($manager->afterCommitCallbacksShouldBeExecuted(1)); + $this->assertFalse($manager->afterCommitCallbacksShouldBeExecuted(2)); + } +} + +class TestingDatabaseTransactionsManagerTestObject +{ + public $ran = false; + public $runs = 0; + + public function handle() + { + $this->ran = true; + $this->runs++; + } +} diff --git a/tests/Integration/Database/DatabaseTransactionsTest.php b/tests/Integration/Database/DatabaseTransactionsTest.php new file mode 100644 index 000000000000..58894d01ae5e --- /dev/null +++ b/tests/Integration/Database/DatabaseTransactionsTest.php @@ -0,0 +1,121 @@ +set([ + 'database.connections.second_connection' => [ + 'driver' => 'sqlite', + 'database' => ':memory:', + ], + ]); + } + + public function testTransactionCallbacks() + { + [$firstObject, $secondObject, $thirdObject] = [ + new TestObjectForTransactions(), + new TestObjectForTransactions(), + new TestObjectForTransactions(), + ]; + + DB::transaction(function () use ($secondObject, $firstObject) { + DB::afterCommit(fn () => $firstObject->handle()); + + DB::transaction(function () use ($secondObject) { + DB::afterCommit(fn () => $secondObject->handle()); + }); + }); + + $this->assertTrue($firstObject->ran); + $this->assertTrue($secondObject->ran); + $this->assertEquals(1, $firstObject->runs); + $this->assertEquals(1, $secondObject->runs); + $this->assertFalse($thirdObject->ran); + } + + public function testTransactionCallbacksDoNotInterfereWithOneAnother() + { + [$firstObject, $secondObject, $thirdObject] = [ + new TestObjectForTransactions(), + new TestObjectForTransactions(), + new TestObjectForTransactions(), + ]; + + // The problem here is that we're initiating a base transaction, and then two nested transactions. + // Although these two nested transactions are not the same, they share the same level (2). + // Since they are not the same, the latter one failing should not affect the first one. + DB::transaction(function () use ($thirdObject, $secondObject, $firstObject) { // Adds a transaction @ level 1 + DB::transaction(function () use ($firstObject) { // Adds a transaction @ level 2 + DB::afterCommit(fn () => $firstObject->handle()); // Adds a callback to be executed after transaction level 2 is committed + }); + + DB::afterCommit(fn () => $secondObject->handle()); // Adds a callback to be executed after transaction 1 @ lvl 1 + + try { + DB::transaction(function () use ($thirdObject) { // Adds a transaction 3 @ level 2 + DB::afterCommit(fn () => $thirdObject->handle()); + throw new \Exception(); // This should only affect callback 3, not 1, even though both share the same transaction level. + }); + } catch (\Exception) { + } + }); + + $this->assertTrue($firstObject->ran); + $this->assertTrue($secondObject->ran); + $this->assertEquals(1, $firstObject->runs); + $this->assertEquals(1, $secondObject->runs); + $this->assertFalse($thirdObject->ran); + } + + public function testTransactionsDoNotAffectDifferentConnections() + { + [$firstObject, $secondObject, $thirdObject] = [ + new TestObjectForTransactions(), + new TestObjectForTransactions(), + new TestObjectForTransactions(), + ]; + + DB::transaction(function () use ($secondObject, $firstObject, $thirdObject) { + DB::transaction(function () use ($secondObject) { + DB::afterCommit(fn () => $secondObject->handle()); + }); + + DB::afterCommit(fn () => $firstObject->handle()); + + try { + DB::connection('second_connection')->transaction(function () use ($thirdObject) { + DB::afterCommit(fn () => $thirdObject->handle()); + + throw new \Exception; + }); + } catch (\Exception) { + // + } + }); + + $this->assertTrue($firstObject->ran); + $this->assertTrue($secondObject->ran); + $this->assertFalse($thirdObject->ran); + } +} + +class TestObjectForTransactions +{ + public $ran = false; + + public $runs = 0; + + public function handle() + { + $this->ran = true; + $this->runs++; + } +}