[10.x] Only stage committed transactions

src/Illuminate/Database/Concerns/ManagesTransactions.php

@@ -47,7 +47,7 @@ public function transaction(Closure $callback, $attempts = 1)
                     $this->getPdo()->commit();
                 }
 
-                $this->transactionsManager?->stageTransactions($this->getName());
+                $this->transactionsManager?->stageTransactions($this->getName(), $this->transactions);
 
                 $this->transactions = max(0, $this->transactions - 1);
 
@@ -196,7 +196,7 @@ public function commit()
             $this->getPdo()->commit();
         }
 
-        $this->transactionsManager?->stageTransactions($this->getName());
+        $this->transactionsManager?->stageTransactions($this->getName(), $this->transactions);
 
         $this->transactions = max(0, $this->transactions - 1);
 

src/Illuminate/Database/DatabaseTransactionRecord.php

@@ -18,6 +18,13 @@ class DatabaseTransactionRecord
      */
     public $level;
 
+    /**
+     * The parent instance of this transaction.
+     *
+     * @var \Illuminate\Database\DatabaseTransactionRecord
+     */
+    public $parent;
+
     /**
      * The callbacks that should be executed after committing.
      *
@@ -30,12 +37,14 @@ class DatabaseTransactionRecord
      *
      * @param  string  $connection
      * @param  int  $level
+     * @param  \Illuminate\Database\DatabaseTransactionRecord|null  $parent
      * @return void
      */
-    public function __construct($connection, $level)
+    public function __construct($connection, $level, ?DatabaseTransactionRecord $parent = null)
     {
         $this->connection = $connection;
         $this->level = $level;
+        $this->parent = $parent;
     }
 
     /**

src/Illuminate/Database/DatabaseTransactionsManager.php

@@ -20,6 +20,13 @@ class DatabaseTransactionsManager
      */
     protected $pendingTransactions;
 
+    /**
+     * The current transaction.
+     *
+     * @var array
+     */
+    protected $currentTransaction = [];
+
     /**
      * Create a new database transactions manager instance.
      *
@@ -41,32 +48,57 @@ public function __construct()
     public function begin($connection, $level)
     {
         $this->pendingTransactions->push(
-            new DatabaseTransactionRecord($connection, $level)
+            $newTransaction = new DatabaseTransactionRecord(
+                $connection,
+                $level,
+                $this->currentTransaction[$connection] ?? null
+            )
         );
+
+        $this->currentTransaction[$connection] = $newTransaction;
     }
 
     /**
-     * Rollback the active database transaction.
+     * Move relevant pending transactions to a committed state.
      *
      * @param  string  $connection
-     * @param  int  $level
+     * @param  int  $levelBeingCommitted
      * @return void
      */
-    public function rollback($connection, $level)
+    public function stageTransactions($connection, $levelBeingCommitted)
     {
+        $this->committedTransactions = $this->committedTransactions->merge(
+            $this->pendingTransactions->filter(
+                fn ($transaction) => $transaction->connection === $connection &&
+                                     $transaction->level >= $levelBeingCommitted
+            )
+        );
+
         $this->pendingTransactions = $this->pendingTransactions->reject(
-            fn ($transaction) => $transaction->connection == $connection && $transaction->level > $level
-        )->values();
+            fn ($transaction) => $transaction->connection === $connection &&
+                                 $transaction->level >= $levelBeingCommitted
+        );
+
+        if (isset($this->currentTransaction[$connection])) {
+            $this->currentTransaction[$connection] = $this->currentTransaction[$connection]->parent;
+        }
     }
 
     /**
-     * Commit the active database transaction.
+     * Commit the root database transaction and execute callbacks.
      *
      * @param  string  $connection
      * @return void
      */
     public function commit($connection)
     {
+        // This method is only called when the root database transaction is committed so there
+        // shouldn't be any pending transactions, but going to clear them here anyways just
+        // in case. This method could be refactored to receive a level in the future too.
+        $this->pendingTransactions = $this->pendingTransactions->reject(
+            fn ($transaction) => $transaction->connection === $connection
+        )->values();
+
         [$forThisConnection, $forOtherConnections] = $this->committedTransactions->partition(
             fn ($transaction) => $transaction->connection == $connection
         );
@@ -77,35 +109,88 @@ public function commit($connection)
     }
 
     /**
-     * Register a transaction callback.
+     * Rollback the active database transaction.
      *
-     * @param  callable  $callback
+     * @param  string  $connection
+     * @param  int  $newTransactionLevel
      * @return void
      */
-    public function addCallback($callback)
+    public function rollback($connection, $newTransactionLevel)
     {
-        if ($current = $this->callbackApplicableTransactions()->last()) {
-            return $current->addCallback($callback);
+        if ($newTransactionLevel === 0) {
+            $this->removeAllTransactionsForConnection($connection);
+        } else {
+            $this->pendingTransactions = $this->pendingTransactions->reject(
+                fn ($transaction) => $transaction->connection == $connection &&
+                                     $transaction->level > $newTransactionLevel
+            )->values();
+
+            if ($this->currentTransaction) {
+                do {
+                    $this->removeCommittedTransactionsThatAreChildrenOf($this->currentTransaction[$connection]);
+
+                    $this->currentTransaction[$connection] = $this->currentTransaction[$connection]->parent;
+                } while (
+                    isset($this->currentTransaction[$connection]) &&
+                    $this->currentTransaction[$connection]->level > $newTransactionLevel
+                );
+            }
         }
-
-        $callback();
     }
 
     /**
-     * Move all the pending transactions to a committed state.
+     * Remove all pending, completed, and current transactions for the given connection name.
      *
      * @param  string  $connection
      * @return void
      */
-    public function stageTransactions($connection)
+    protected function removeAllTransactionsForConnection($connection)
     {
-        $this->committedTransactions = $this->committedTransactions->merge(
-            $this->pendingTransactions->filter(fn ($transaction) => $transaction->connection === $connection)
-        );
+        $this->currentTransaction[$connection] = null;
 
         $this->pendingTransactions = $this->pendingTransactions->reject(
-            fn ($transaction) => $transaction->connection === $connection
+            fn ($transaction) => $transaction->connection == $connection
+        )->values();
+
+        $this->committedTransactions = $this->committedTransactions->reject(
+            fn ($transaction) => $transaction->connection == $connection
+        )->values();
+    }
+
+    /**
+     * Remove all transactions that are children of the given transaction.
+     *
+     * @param  \Illuminate\Database\DatabaseTransactionRecord  $transaction
+     * @return void
+     */
+    protected function removeCommittedTransactionsThatAreChildrenOf(DatabaseTransactionRecord $transaction)
+    {
+        [$removedTransactions, $this->committedTransactions] = $this->committedTransactions->partition(
+            fn ($committed) => $committed->connection == $transaction->connection &&
+                               $committed->parent === $transaction
         );
+
+        // There may be multiple deeply nested transactions that have already committed that we
+        // also need to remove. We will recurse down the children of all removed transaction
+        // instances until there are no more deeply nested child transactions for removal.
+        $removedTransactions->each(
+            fn ($transaction) => $this->removeCommittedTransactionsThatAreChildrenOf($transaction)
+        );
+    }
+
+    /**
+     * Register a transaction callback.
+     *
+     * @param  callable  $callback
+     * @return void
+     */
+    public function addCallback($callback)
+    {
+        if ($current = $this->callbackApplicableTransactions()->last()) {
+            return $current->addCallback($callback);
+        }
+
+        $callback();
     }
 
     /**

tests/Database/DatabaseTransactionsManagerTest.php

@@ -67,8 +67,8 @@ public function testCommittingTransactions()
         $manager->begin('default', 2);
         $manager->begin('admin', 1);
 
-        $manager->stageTransactions('default');
-        $manager->stageTransactions('admin');
+        $manager->stageTransactions('default', 1);
+        $manager->stageTransactions('admin', 1);
         $manager->commit('default');
 
         $this->assertCount(0, $manager->getPendingTransactions());
@@ -121,7 +121,7 @@ public function testCommittingTransactionsExecutesCallbacks()
 
         $manager->begin('admin', 1);
 
-        $manager->stageTransactions('default');
+        $manager->stageTransactions('default', 1);
         $manager->commit('default');
 
         $this->assertCount(2, $callbacks);
@@ -148,7 +148,7 @@ public function testCommittingExecutesOnlyCallbacksOfTheConnection()
             $callbacks[] = ['admin', 1];
         });
 
-        $manager->stageTransactions('default');
+        $manager->stageTransactions('default', 1);
         $manager->commit('default');
 
         $this->assertCount(1, $callbacks);
@@ -185,16 +185,29 @@ public function testStageTransactions()
         $this->assertEquals(1, $pendingTransactions[1]->level);
         $this->assertEquals('admin', $pendingTransactions[1]->connection);
 
-        $manager->stageTransactions('default');
+        $manager->stageTransactions('default', 1);
 
         $this->assertCount(1, $manager->getPendingTransactions());
         $this->assertCount(1, $manager->getCommittedTransactions());
         $this->assertEquals('default', $manager->getCommittedTransactions()[0]->connection);
 
-        $manager->stageTransactions('admin');
+        $manager->stageTransactions('admin', 1);
 
         $this->assertCount(0, $manager->getPendingTransactions());
         $this->assertCount(2, $manager->getCommittedTransactions());
         $this->assertEquals('admin', $manager->getCommittedTransactions()[1]->connection);
     }
+
+    public function testStageTransactionsOnlyStagesTheTransactionsAtOrAboveTheGivenLevel()
+    {
+        $manager = (new DatabaseTransactionsManager);
+
+        $manager->begin('default', 1);
+        $manager->begin('default', 2);
+        $manager->begin('default', 3);
+        $manager->stageTransactions('default', 2);
+
+        $this->assertCount(1, $manager->getPendingTransactions());
+        $this->assertCount(2, $manager->getCommittedTransactions());
+    }
 }

tests/Database/DatabaseTransactionsTest.php

@@ -64,7 +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('stageTransactions')->once()->with('default', 1);
         $transactionManager->shouldReceive('commit')->once()->with('default');
 
         $this->connection()->setTransactionManager($transactionManager);
@@ -84,7 +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('stageTransactions')->once()->with('default', 1);
         $transactionManager->shouldReceive('commit')->once()->with('default');
 
         $this->connection()->setTransactionManager($transactionManager);
@@ -105,7 +105,8 @@ 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('stageTransactions')->once()->with('default', 1);
+        $transactionManager->shouldReceive('stageTransactions')->once()->with('default', 2);
         $transactionManager->shouldReceive('commit')->once()->with('default');
 
         $this->connection()->setTransactionManager($transactionManager);
@@ -133,8 +134,9 @@ 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('stageTransactions')->once()->with('default', 1);
+        $transactionManager->shouldReceive('stageTransactions')->once()->with('second_connection', 1);
+        $transactionManager->shouldReceive('stageTransactions')->once()->with('second_connection', 2);
         $transactionManager->shouldReceive('commit')->once()->with('default');
         $transactionManager->shouldReceive('commit')->once()->with('second_connection');
 

tests/Foundation/Testing/DatabaseTransactionsManagerTest.php

@@ -42,7 +42,7 @@ public function testItExecutesCallbacksForTheSecondTransaction()
 
         $this->assertFalse($testObject->ran);
 
-        $manager->stageTransactions('foo');
+        $manager->stageTransactions('foo', 1);
         $manager->commit('foo');
         $this->assertTrue($testObject->ran);
         $this->assertEquals(1, $testObject->runs);