From b1437af9f8d0c3aa8c14c6c5330059f79a0c7228 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Sat, 7 May 2022 13:59:27 +0200 Subject: [PATCH] Deprecate transaction nesting without savepoints When transaction nesting without savepoints is disabled, the behavior is suprising: there is no inner transaction, and the outer transaction is rolled back. Users relying on a platform that does not support savepoints will have to rework their application logic so as to avoid nested transaction blocks. --- UPGRADE.md | 12 ++++++++++++ docs/en/reference/transactions.rst | 7 +++++++ src/Connection.php | 22 ++++++++++++++++++++++ tests/ConnectionTest.php | 3 +++ tests/Functional/ConnectionTest.php | 14 ++++++++++++++ 5 files changed, 58 insertions(+) diff --git a/UPGRADE.md b/UPGRADE.md index 83f1658789c..af74354f2d4 100644 --- a/UPGRADE.md +++ b/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 diff --git a/docs/en/reference/transactions.rst b/docs/en/reference/transactions.rst index 30bcd1049aa..5dc03048e0d 100644 --- a/docs/en/reference/transactions.rst +++ b/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 diff --git a/src/Connection.php b/src/Connection.php index 4db448ec1ef..f7ca639cba5 100644 --- a/src/Connection.php +++ b/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)); diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php index 58147d40ebe..4a1c53c227a 100644 --- a/tests/ConnectionTest.php +++ b/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; diff --git a/tests/Functional/ConnectionTest.php b/tests/Functional/ConnectionTest.php index 1f593284584..5c2bcaf82b7 100644 --- a/tests/Functional/ConnectionTest.php +++ b/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());