Remove support for transaction nesting without savepoints

UPGRADE.md

@@ -8,6 +8,25 @@ awareness about deprecated code.
 
 # Upgrade to 4.0
 
+## BC BREAK: remove support for transaction nesting without savepoints
+
+Starting a transaction inside another transaction with
+`Doctrine\DBAL\Connection::beginTransaction()` now always results in
+savepoints being used.
+
+In case your platform does not support savepoints, you will have to
+rework your application logic so as to avoid nested transaction blocks.
+
+## Deprecated: configuration methods related to transaction nesting
+
+Since it is no longer possible to configure whether transaction nesting is
+emulated with savepoints or not, configuring that behavior has no effect and it
+is deprecated to attempt to change it or to know how it is configured. As a
+result, the following methods are deprecated:
+
+- `Connection::setNestTransactionsWithSavepoints()`
+- `Connection::getNestTransactionsWithSavepoints()`
+
 ## BC BREAK: Auto-increment columns on PostgreSQL are implemented as `IDENTITY`, not `SERIAL`.
 
 Instead of using `SERIAL*` column types for `autoincrement` columns, the DBAL will now use

docs/en/reference/transactions.rst

@@ -67,48 +67,12 @@ Transaction Nesting
 -------------------
 
 Calling ``beginTransaction()`` while already in a transaction will
-result in two very different behaviors depending on whether transaction
-nesting with savepoints is enabled or not. In both cases though, there
-won't be an actual transaction inside a transaction, even if your RDBMS
-supports it. There is always only a single, real database transaction.
+not result in an actual transaction inside a transaction, even if your
+RDBMS supports it. Instead, transaction nesting is emulated by resorting
+to SQL savepoints. There is always only a single, real database
+transaction.
 
-By default, transaction nesting at the SQL level with savepoints is
-disabled. The value for that setting can be set on a per-connection
-basis, with
-``Doctrine\DBAL\Connection#setNestTransactionsWithSavepoints()``.
-
-Nesting transactions without savepoints is deprecated, but is the
-default behavior for backward compatibility reasons.
-
-Dummy mode
-~~~~~~~~~~
-.. warning::
-
-    This behavior is deprecated, avoid it with
-    ``Doctrine\DBAL\Connection#setNestTransactionsWithSavepoints(true)``.
-
-When transaction nesting with savepoints is disabled, what happens is
-not so much transaction nesting as propagating transaction control up
-the call stack. For that purpose, the ``Connection`` class keeps an
-internal counter that represents the nesting level and is
-increased/decreased as ``beginTransaction()``, ``commit()`` and
-``rollBack()`` are invoked. ``beginTransaction()`` increases the nesting
-level whilst ``commit()`` and ``rollBack()`` decrease the nesting level.
-The nesting level starts at 0.
-Whenever the nesting level transitions from 0 to 1,
-``beginTransaction()`` is invoked on the underlying driver connection
-and whenever the nesting level transitions from 1 to 0, ``commit()`` or
-``rollBack()`` is invoked on the underlying driver, depending on whether
-the transition was caused by ``Connection#commit()`` or
-``Connection#rollBack()``.
-
-What this means is that transaction control is basically passed to
-code higher up in the call stack and the inner transaction block does
-not actually result in an SQL transaction. It is not ignored either
-though.
-
-To visualize what this means in practice, consider the following
-example:
+Let's examine what happens with an example
 
 ::
 
@@ -121,13 +85,13 @@ example:
 
         // nested transaction block, this might be in some other API/library code that is
         // unaware of the outer transaction.
-        $conn->beginTransaction(); // 1 => 2
+        $conn->beginTransaction(); // 1 => 2, savepoint created
         try {
             ...
 
             $conn->commit(); // 2 => 1
         } catch (\Exception $e) {
-            $conn->rollBack(); // 2 => 1, transaction marked for rollback only
+            $conn->rollBack(); // 2 => 1, rollback to savepoint
             throw $e;
         }
 
@@ -139,33 +103,9 @@ example:
         throw $e;
     }
 
-However, **a rollback in a nested transaction block will always mark the
-current transaction so that the only possible outcome of the transaction
-is to be rolled back**.
-That means in the above example, the rollback in the inner
-transaction block marks the whole transaction for rollback only.
-Even if the nested transaction block would not rethrow the
-exception, the transaction is marked for rollback only and the
-commit of the outer transaction would trigger an exception, leading
-to the final rollback. This also means that you cannot
-successfully commit some changes in an outer transaction if an
-inner transaction block fails and issues a rollback, even if this
-would be the desired behavior (i.e. because the nested operation is
-"optional" for the purpose of the outer transaction block). To
-achieve that, you need to resort to transaction nesting with savepoint.
-
-All that is guaranteed to the inner transaction is that it still
-happens atomically, all or nothing, the transaction just gets a
-wider scope and the control is handed to the outer scope.
-
-.. note::
-
-    The transaction nesting described here is a debated
-    feature that has its critics. Form your own opinion. We recommend
-    avoiding nesting transaction blocks when possible, and most of the
-    time, it is possible. Transaction control should mostly be left to
-    a service layer and not be handled in data access objects or
-    similar.
+Everything is handled at the SQL level: the main transaction is not
+marked for rollback only, but the inner emulated transaction is rolled
+back to the savepoint.
 
 .. warning::
 
@@ -177,45 +117,6 @@ wider scope and the control is handed to the outer scope.
     nesting level, causing errors with broken transaction boundaries
     that may be hard to debug.
 
-Emulated Transaction Nesting with Savepoints
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-Let's now examine what happens when transaction nesting with savepoints
-is enabled, with the same example as above
-
-::
-
-    <?php
-    // $conn instanceof Doctrine\DBAL\Connection
-    $conn->beginTransaction(); // 0 => 1, "real" transaction started
-    try {
-
-        ...
-
-        // nested transaction block, this might be in some other API/library code that is
-        // unaware of the outer transaction.
-        $conn->beginTransaction(); // 1 => 2, savepoint created
-        try {
-            ...
-
-            $conn->commit(); // 2 => 1
-        } catch (\Exception $e) {
-            $conn->rollBack(); // 2 => 1, rollback to savepoint
-            throw $e;
-        }
-
-        ...
-
-        $conn->commit(); // 1 => 0, "real" transaction committed
-    } catch (\Exception $e) {
-        $conn->rollBack(); // 1 => 0, "real" transaction rollback
-        throw $e;
-    }
-
-This time, everything is handled at the SQL level: the main transaction
-is not marked for rollback only, but the inner emulated transaction is
-rolled back to the savepoint.
-
 Auto-commit mode
 ----------------
 

src/Connection.php

@@ -22,7 +22,6 @@
 use Doctrine\DBAL\Exception\DriverException;
 use Doctrine\DBAL\Exception\EmptyCriteriaNotAllowed;
 use Doctrine\DBAL\Exception\InvalidPlatformType;
-use Doctrine\DBAL\Exception\MayNotAlterNestedTransactionWithSavepointsInTransaction;
 use Doctrine\DBAL\Exception\NoActiveTransaction;
 use Doctrine\DBAL\Exception\SavepointsNotSupported;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
@@ -97,11 +96,6 @@ class Connection implements ServerVersionProvider
      */
     private ?int $transactionIsolationLevel = null;
 
-    /**
-     * If nested transactions should use savepoints.
-     */
-    private bool $nestTransactionsWithSavepoints = false;
-
     /**
      * The parameters used during creation of the Connection instance.
      *
@@ -1045,39 +1039,37 @@ public function transactional(Closure $func): mixed
     /**
      * Sets if nested transactions should use savepoints.
      *
+     * @deprecated No replacement planned
+     *
      * @throws Exception
      */
     public function setNestTransactionsWithSavepoints(bool $nestTransactionsWithSavepoints): void
     {
-        if (! $nestTransactionsWithSavepoints) {
-            Deprecation::trigger(
-                'doctrine/dbal',
-                'https://github.com/doctrine/dbal/pull/5383',
-                <<<'DEPRECATION'
-                Nesting transactions without enabling savepoints is deprecated.
-                Call %s::setNestTransactionsWithSavepoints(true) to enable savepoints.
-                DEPRECATION,
-                self::class
-            );
-        }
-
-        if ($this->transactionNestingLevel > 0) {
-            throw MayNotAlterNestedTransactionWithSavepointsInTransaction::new();
-        }
-
-        if (! $this->getDatabasePlatform()->supportsSavepoints()) {
-            throw SavepointsNotSupported::new();
-        }
-
-        $this->nestTransactionsWithSavepoints = $nestTransactionsWithSavepoints;
+        Deprecation::trigger(
+            'doctrine/dbal',
+            'https://github.com/doctrine/dbal/pull/5383',
+            '%s is deprecated and will be removed in 5.0',
+            __METHOD__
+        );
     }
 
     /**
      * Gets if nested transactions should use savepoints.
+     *
+     * @deprecated No replacement planned
+     *
+     * @return true
      */
     public function getNestTransactionsWithSavepoints(): bool
     {
-        return $this->nestTransactionsWithSavepoints;
+        Deprecation::trigger(
+            'doctrine/dbal',
+            'https://github.com/doctrine/dbal/pull/5383',
+            '%s is deprecated and will be removed in 5.0',
+            __METHOD__
+        );
+
+        return true;
     }
 
     /**
@@ -1099,18 +1091,8 @@ public function beginTransaction(): void
 
         if ($this->transactionNestingLevel === 1) {
             $connection->beginTransaction();
-        } elseif ($this->nestTransactionsWithSavepoints) {
-            $this->createSavepoint($this->_getNestedTransactionSavePointName());
         } else {
-            Deprecation::trigger(
-                'doctrine/dbal',
-                'https://github.com/doctrine/dbal/pull/5383',
-                <<<'DEPRECATION'
-                Nesting transactions without enabling savepoints is deprecated.
-                Call %s::setNestTransactionsWithSavepoints(true) to enable savepoints.
-                DEPRECATION,
-                self::class
-            );
+            $this->createSavepoint($this->_getNestedTransactionSavePointName());
         }
 
         $this->getEventManager()->dispatchEvent(Events::onTransactionBegin, new TransactionBeginEventArgs($this));
@@ -1137,7 +1119,7 @@ public function commit(): void
             } catch (Driver\Exception $e) {
                 throw $this->convertException($e);
             }
-        } elseif ($this->nestTransactionsWithSavepoints) {
+        } else {
             $this->releaseSavepoint($this->_getNestedTransactionSavePointName());
         }
 
@@ -1197,11 +1179,8 @@ public function rollBack(): void
                     $this->beginTransaction();
                 }
             }
-        } elseif ($this->nestTransactionsWithSavepoints) {
-            $this->rollbackSavepoint($this->_getNestedTransactionSavePointName());
-            --$this->transactionNestingLevel;
         } else {
-            $this->isRollbackOnly = true;
+            $this->rollbackSavepoint($this->_getNestedTransactionSavePointName());
             --$this->transactionNestingLevel;
         }
 

tests/ConnectionTest.php

@@ -167,11 +167,20 @@ static function (TransactionCommitEventArgs $eventArgs) use ($conn): bool {
         $conn->commit();
     }
 
-    public function testTransactionCommitEventNotCalledAfterRollBack(): void
+    public function testTransactionCommitEventCalledAfterRollBack(): void
     {
         $eventManager = new EventManager();
-        $driverMock   = $this->createMock(Driver::class);
-        $conn         = new Connection([], $driverMock, new Configuration(), $eventManager);
+        $platform     = $this->createStub(AbstractPlatform::class);
+        $platform
+            ->method('supportsSavepoints')
+            ->willReturn(true);
+
+        $driverMock = $this->createMock(Driver::class);
+        $driverMock
+            ->method('getDatabasePlatform')
+            ->willReturn($platform);
+
+        $conn = new Connection([], $driverMock, new Configuration(), $eventManager);
 
         $rollBackListenerMock = $this->createMock(TransactionRollBackDispatchEventListener::class);
         $rollBackListenerMock
@@ -187,7 +196,7 @@ static function (TransactionRollBackEventArgs $eventArgs) use ($conn): bool {
         $eventManager->addEventListener([Events::onTransactionRollBack], $rollBackListenerMock);
 
         $commitListenerMock = $this->createMock(TransactionCommitDispatchEventListener::class);
-        $commitListenerMock->expects(self::never())->method('onTransactionCommit');
+        $commitListenerMock->expects(self::exactly(1))->method('onTransactionCommit');
         $eventManager->addEventListener([Events::onTransactionCommit], $commitListenerMock);
 
         $conn->beginTransaction();
@@ -345,7 +354,15 @@ public function testRollBackStartsTransactionInNoAutoCommitMode(): void
 
     public function testSwitchingAutoCommitModeCommitsAllCurrentTransactions(): void
     {
+        $platform = $this->createStub(AbstractPlatform::class);
+        $platform
+            ->method('supportsSavepoints')
+            ->willReturn(true);
+
         $driverMock = $this->createMock(Driver::class);
+        $driverMock
+            ->method('getDatabasePlatform')
+            ->willReturn($platform);
 
         $conn = new Connection([], $driverMock);