Merge pull request #5383 from greg0ire/deprecate-dummy-transac-behavior

Deprecate transaction nesting without savepoints

UPGRADE.md

@@ -8,6 +8,18 @@ awareness about deprecated code.
 
 # Upgrade to 3.4
 
+## Deprecated transaction nesting without savepoints
+
+Starting a transaction inside another transaction with
+`Doctrine\DBAL\Connection::beginTransaction()` without enabling transaction
+nesting with savepoints beforehand is deprecated.
+
+Transaction nesting with savepoints can be enabled with
+`$connection->setNestTransactionsWithSavepoints(true);`
+
+In case your platform does not support savepoints, you will have to rework your
+application logic so as to avoid nested transaction blocks.
+
 ## Added runtime deprecations for the default string column length.
 
 In addition to the formal deprecation introduced in DBAL 3.2, the library will now emit a deprecation message at runtime

docs/en/reference/transactions.rst

@@ -77,8 +77,15 @@ 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

src/Connection.php

@@ -1251,6 +1251,18 @@ public function transactional(Closure $func)
      */
     public function setNestTransactionsWithSavepoints($nestTransactionsWithSavepoints)
     {
+        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 ConnectionException::mayNotAlterNestedTransactionWithSavepointsInTransaction();
         }
@@ -1314,6 +1326,16 @@ public function beginTransaction()
             if ($logger !== null) {
                 $logger->stopQuery();
             }
+        } 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->getEventManager()->dispatchEvent(Events::onTransactionBegin, new TransactionBeginEventArgs($this));

tests/ConnectionTest.php

@@ -22,6 +22,7 @@
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Result;
 use Doctrine\DBAL\VersionAwarePlatformDriver;
+use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Psr\Cache\CacheItemInterface;
@@ -33,6 +34,8 @@
  */
 class ConnectionTest extends TestCase
 {
+    use VerifyDeprecations;
+
     /** @var Connection */
     private $connection;
 

tests/Functional/ConnectionTest.php

@@ -19,6 +19,7 @@
 use Doctrine\DBAL\Tests\FunctionalTestCase;
 use Doctrine\DBAL\Tests\TestUtil;
 use Doctrine\DBAL\Types\Types;
+use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
 use Error;
 use PDO;
 use Throwable;
@@ -28,6 +29,8 @@
 
 class ConnectionTest extends FunctionalTestCase
 {
+    use VerifyDeprecations;
+
     private const TABLE = 'connection_test';
 
     protected function tearDown(): void
@@ -53,6 +56,16 @@ public function testCommitWithRollbackOnlyThrowsException(): void
         $this->connection->commit();
     }
 
+    public function testNestingTransactionsWithoutSavepointsIsDeprecated(): void
+    {
+        if (! $this->connection->getDatabasePlatform()->supportsSavepoints()) {
+            self::markTestSkipped('This test is only supported on platforms that support savepoints.');
+        }
+
+        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/5383');
+        $this->connection->setNestTransactionsWithSavepoints(false);
+    }
+
     public function testTransactionNestingBehavior(): void
     {
         $this->createTestTable();
@@ -62,6 +75,7 @@ public function testTransactionNestingBehavior(): void
             self::assertSame(1, $this->connection->getTransactionNestingLevel());
 
             try {
+                $this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/5383');
                 $this->connection->beginTransaction();
                 self::assertSame(2, $this->connection->getTransactionNestingLevel());