[10.x] Fix how nested transaction callbacks are handled

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()) {

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,29 @@ public function addCallback($callback)
         $callback();
     }
 
+    /**
+     * Move all the pending transactions to a ready state.
+     *
+     * @param  string  $connection
+     * @return void
+     */
+    public function stageTransactions($connection)
+    {
+        $this->readyTransactions = $this->readyTransactions->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.
      *
      * @return \Illuminate\Support\Collection<int, \Illuminate\Database\DatabaseTransactionRecord>
      */
     public function callbackApplicableTransactions()
     {
-        return $this->transactions;
+        return $this->pendingTransactions;
     }
 
     /**
@@ -103,12 +126,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;
     }
 }

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();
     }
 
     /**

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->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 +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->getReadyTransactions());
+        $this->assertEquals('default', $manager->getReadyTransactions()[0]->connection);
+
+        $manager->stageTransactions('admin');
+
+        $this->assertCount(0, $manager->getPendingTransactions());
+        $this->assertCount(2, $manager->getReadyTransactions());
+        $this->assertEquals('admin', $manager->getReadyTransactions()[1]->connection);
+    }
 }

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');
 

tests/Foundation/Testing/DatabaseTransactionsManagerTest.php

@@ -0,0 +1,71 @@
+<?php
+
+namespace Illuminate\Tests\Foundation\Testing;
+
+use Illuminate\Foundation\Testing\DatabaseTransactionsManager;
+use PHPUnit\Framework\TestCase;
+
+class DatabaseTransactionsManagerTest extends TestCase
+{
+    public function testItExecutesCallbacksImmediatelyIfThereIsOnlyOneTransaction()
+    {
+        $testObject = new TestingDatabaseTransactionsManagerTestObject();
+        $manager = new DatabaseTransactionsManager;
+
+        $manager->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++;
+    }
+}