laravel · mateusjatenee · Oct 30, 2023 · Oct 30, 2023 · Oct 30, 2023 · Oct 30, 2023
diff --git 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->transactions = max(0, $this->transactions - 1);

                if ($this->afterCommitCallbacksShouldBeExecuted()) {
@@ -194,6 +196,8 @@ public function commit()
            $this->getPdo()->commit();
        }

+        $this->transactionsManager?->stageTransactions();
+
        $this->transactions = max(0, $this->transactions - 1);

        if ($this->afterCommitCallbacksShouldBeExecuted()) {

diff --git a/src/Illuminate/Database/DatabaseTransactionsManager.php b/src/Illuminate/Database/DatabaseTransactionsManager.php
@@ -5,11 +5,18 @@
 class DatabaseTransactionsManager
 {
    /**
-     * All of the recorded transactions.
+     * All of the recorded ready transactions.
     *
     * @var \Illuminate\Support\Collection<int, \Illuminate\Database\DatabaseTransactionRecord>
     */
-    protected $transactions;
+    protected $readyTransactions;
+
+    /**
+     * All of the recorded pending transactions.
+     *
+     * @var \Illuminate\Support\Collection<int, \Illuminate\Database\DatabaseTransactionRecord>
+     */
+    protected $pendingTransactions;

    /**
     * Create a new database transactions manager instance.
@@ -18,7 +25,8 @@ class DatabaseTransactionsManager
     */
    public function __construct()
    {
-        $this->transactions = collect();
+        $this->readyTransactions = collect();
+        $this->pendingTransactions = collect();
    }

    /**
@@ -30,7 +38,7 @@ public function __construct()
     */
    public function begin($connection, $level)
    {
-        $this->transactions->push(
+        $this->pendingTransactions->push(
            new DatabaseTransactionRecord($connection, $level)
        );
    }
@@ -44,7 +52,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 +65,11 @@ public function rollback($connection, $level)
     */
    public function commit($connection)
    {
-        [$forThisConnection, $forOtherConnections] = $this->transactions->partition(
+        [$forThisConnection, $forOtherConnections] = $this->readyTransactions->partition(
            fn ($transaction) => $transaction->connection == $connection
        );

-        $this->transactions = $forOtherConnections->values();
+        $this->readyTransactions = $forOtherConnections->values();

        $forThisConnection->map->executeCallbacks();
    }
@@ -81,14 +89,28 @@ public function addCallback($callback)
        $callback();
    }

+    /**
+     * Move all the pending transactions to a ready state.
+     *
+     * @return void
+     */
+    public function stageTransactions()
+    {
+        $this->readyTransactions = $this->readyTransactions->merge(
+            $this->pendingTransactions
+        );
+
+        $this->pendingTransactions = collect();
+    }
+
    /**
     * Get the transactions that are applicable to callbacks.
     *
     * @return \Illuminate\Support\Collection<int, \Illuminate\Database\DatabaseTransactionRecord>
     */
    public function callbackApplicableTransactions()
    {
-        return $this->transactions;
+        return $this->pendingTransactions;
    }

    /**
@@ -103,12 +125,22 @@ public function afterCommitCallbacksShouldBeExecuted($level)
    }

    /**
-     * Get all the transactions.
+     * Get all the pending transactions.
+     *
+     * @return \Illuminate\Support\Collection
+     */
+    public function getPendingTransactions()
+    {
+        return $this->pendingTransactions;
+    }
+
+    /**
+     * Get all the ready transactions.
     *
     * @return \Illuminate\Support\Collection
     */
-    public function getTransactions()
+    public function getReadyTransactions()
    {
-        return $this->transactions;
+        return $this->readyTransactions;
    }
 }
diff --git 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
@@ -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,14 @@ public function testCommittingTransactions()
        $manager->begin('default', 2);
        $manager->begin('admin', 1);

+        $manager->stageTransactions();
        $manager->commit('default');

-        $this->assertCount(1, $manager->getTransactions());
+        $this->assertCount(0, $manager->getPendingTransactions());
+        $this->assertCount(1, $manager->getReadyTransactions());

-        $this->assertSame('admin', $manager->getTransactions()[0]->connection);
-        $this->assertEquals(1, $manager->getTransactions()[0]->level);
+        $this->assertSame('admin', $manager->getReadyTransactions()[0]->connection);
+        $this->assertEquals(1, $manager->getReadyTransactions()[0]->level);
    }

    public function testCallbacksAreAddedToTheCurrentTransaction()
@@ -93,9 +95,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 +120,7 @@ public function testCommittingTransactionsExecutesCallbacks()

        $manager->begin('admin', 1);

+        $manager->stageTransactions();
        $manager->commit('default');

        $this->assertCount(2, $callbacks);
@@ -144,6 +147,7 @@ public function testCommittingExecutesOnlyCallbacksOfTheConnection()
            $callbacks[] = ['admin', 1];
        });

+        $manager->stageTransactions();
        $manager->commit('default');

        $this->assertCount(1, $callbacks);

diff --git a/tests/Integration/Database/DatabaseTransactionsTest.php b/tests/Integration/Database/DatabaseTransactionsTest.php
@@ -0,0 +1,50 @@
+<?php
+
+namespace Illuminate\Tests\Integration\Database;
+
+use Illuminate\Support\Facades\DB;
+
+class DatabaseTransactionsTest extends DatabaseTestCase
+{
+    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->assertFalse($thirdObject->ran);
+    }
+}
+
+class TestObjectForTransactions
+{
+    public bool $ran = false;
+
+    public function handle()
+    {
+        $this->ran = true;
+    }
+}