From ca8cb14a7d99ec2b304fad382a5ccaae2fa43a91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Wed, 11 Jan 2023 10:04:11 +0100 Subject: [PATCH] Drop support for silencing errors about DDL in transactions I hesitated between this and removing the transaction helper entirely, but I think this solution will result in less support for us. --- UPGRADE.md | 5 +++ lib/Doctrine/Migrations/DbalMigrator.php | 4 +- .../Event/Listeners/AutoCommitListener.php | 2 +- .../Migrations/Tools/TransactionHelper.php | 29 +++++------- .../Migrations/Version/DbalExecutor.php | 4 +- .../Tests/Tools/TransactionHelperTest.php | 44 +++++++++---------- 6 files changed, 42 insertions(+), 46 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 91ead5afbd..f40f960f89 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,3 +1,8 @@ +# Upgrade to 4.0 + +It is no longer possible to relying on silencing to enable transactional +migrations when the platform does not allow DDL inside transactions. + # Upgrade to 3.1 - The "version" is the FQCN of the migration class (existing entries in the migrations table will be automatically updated). diff --git a/lib/Doctrine/Migrations/DbalMigrator.php b/lib/Doctrine/Migrations/DbalMigrator.php index e0edccdfc2..0a53775dc7 100644 --- a/lib/Doctrine/Migrations/DbalMigrator.php +++ b/lib/Doctrine/Migrations/DbalMigrator.php @@ -73,14 +73,14 @@ private function executeMigrations( $this->dispatcher->dispatchMigrationEvent(Events::onMigrationsMigrated, $migrationsPlan, $migratorConfiguration); } catch (Throwable $e) { if ($allOrNothing) { - TransactionHelper::rollbackIfInTransaction($this->connection); + TransactionHelper::rollback($this->connection); } throw $e; } if ($allOrNothing) { - TransactionHelper::commitIfInTransaction($this->connection); + TransactionHelper::commit($this->connection); } return $sql; diff --git a/lib/Doctrine/Migrations/Event/Listeners/AutoCommitListener.php b/lib/Doctrine/Migrations/Event/Listeners/AutoCommitListener.php index 7fb8a97160..4027bb9594 100644 --- a/lib/Doctrine/Migrations/Event/Listeners/AutoCommitListener.php +++ b/lib/Doctrine/Migrations/Event/Listeners/AutoCommitListener.php @@ -26,7 +26,7 @@ public function onMigrationsMigrated(MigrationsEventArgs $args): void return; } - TransactionHelper::commitIfInTransaction($conn); + TransactionHelper::commit($conn); } /** {@inheritdoc} */ diff --git a/lib/Doctrine/Migrations/Tools/TransactionHelper.php b/lib/Doctrine/Migrations/Tools/TransactionHelper.php index 581088d188..b977359871 100644 --- a/lib/Doctrine/Migrations/Tools/TransactionHelper.php +++ b/lib/Doctrine/Migrations/Tools/TransactionHelper.php @@ -5,7 +5,6 @@ namespace Doctrine\Migrations\Tools; use Doctrine\DBAL\Connection; -use Doctrine\Deprecations\Deprecation; use LogicException; use PDO; @@ -16,43 +15,35 @@ */ final class TransactionHelper { - public static function commitIfInTransaction(Connection $connection): void + public static function commit(Connection $connection): void { if (! self::inTransaction($connection)) { - Deprecation::trigger( - 'doctrine/migrations', - 'https://github.com/doctrine/migrations/issues/1169', - <<<'DEPRECATION' + throw new LogicException( + <<<'EXCEPTION' Context: trying to commit a transaction -Problem: the transaction is already committed, relying on silencing is deprecated. +Problem: the transaction is already committed. Solution: override `AbstractMigration::isTransactional()` so that it returns false. Automate that by setting `transactional` to false in the configuration. More details at https://www.doctrine-project.org/projects/doctrine-migrations/en/3.2/explanation/implicit-commits.html -DEPRECATION +EXCEPTION ); - - return; } $connection->commit(); } - public static function rollbackIfInTransaction(Connection $connection): void + public static function rollback(Connection $connection): void { if (! self::inTransaction($connection)) { - Deprecation::trigger( - 'doctrine/migrations', - 'https://github.com/doctrine/migrations/issues/1169', - <<<'DEPRECATION' + throw new LogicException( + <<<'EXCEPTION' Context: trying to rollback a transaction -Problem: the transaction is already rolled back, relying on silencing is deprecated. +Problem: the transaction is already rolled back. Solution: override `AbstractMigration::isTransactional()` so that it returns false. Automate that by setting `transactional` to false in the configuration. More details at https://www.doctrine-project.org/projects/doctrine-migrations/en/3.2/explanation/implicit-commits.html -DEPRECATION +EXCEPTION ); - - return; } $connection->rollBack(); diff --git a/lib/Doctrine/Migrations/Version/DbalExecutor.php b/lib/Doctrine/Migrations/Version/DbalExecutor.php index 73181613ee..8eeed8f6ea 100644 --- a/lib/Doctrine/Migrations/Version/DbalExecutor.php +++ b/lib/Doctrine/Migrations/Version/DbalExecutor.php @@ -211,7 +211,7 @@ private function executeMigration( } if ($migration->isTransactional()) { - TransactionHelper::commitIfInTransaction($this->connection); + TransactionHelper::commit($this->connection); } $plan->markAsExecuted($result); @@ -252,7 +252,7 @@ private function migrationEnd(Throwable $e, MigrationPlan $plan, ExecutionResult $migration = $plan->getMigration(); if ($migration->isTransactional()) { //only rollback transaction if in transactional mode - TransactionHelper::rollbackIfInTransaction($this->connection); + TransactionHelper::rollback($this->connection); } $plan->markAsExecuted($result); diff --git a/tests/Doctrine/Migrations/Tests/Tools/TransactionHelperTest.php b/tests/Doctrine/Migrations/Tests/Tools/TransactionHelperTest.php index 044b14e614..388a31a4d6 100644 --- a/tests/Doctrine/Migrations/Tests/Tools/TransactionHelperTest.php +++ b/tests/Doctrine/Migrations/Tests/Tools/TransactionHelperTest.php @@ -5,16 +5,14 @@ namespace Doctrine\Migrations\Tests\Tools; use Doctrine\DBAL\Connection; -use Doctrine\Deprecations\PHPUnit\VerifyDeprecations; use Doctrine\Migrations\Tools\TransactionHelper; +use LogicException; use PDO; use PHPUnit\Framework\TestCase; final class TransactionHelperTest extends TestCase { - use VerifyDeprecations; - - public function testItTriggersADeprecationWhenUseful(): void + public function testItThrowsAnExceptionWhenAttemptingToCommitWhileNotInsideATransaction(): void { $connection = $this->createStub(Connection::class); $wrappedConnection = $this->createStub(PDO::class); @@ -23,18 +21,27 @@ public function testItTriggersADeprecationWhenUseful(): void $wrappedConnection->method('inTransaction')->willReturn(false); - $this->expectDeprecationWithIdentifier( - 'https://github.com/doctrine/migrations/issues/1169' - ); - TransactionHelper::commitIfInTransaction($connection); + $this->expectException(LogicException::class); + TransactionHelper::commit($connection); + } + + public function testItThrowsAnExceptionWhenAttemptingToRollbackWhileNotInsideATransaction(): void + { + $connection = $this->createStub(Connection::class); + $wrappedConnection = $this->createStub(PDO::class); - $this->expectDeprecationWithIdentifier( - 'https://github.com/doctrine/migrations/issues/1169' - ); - TransactionHelper::rollbackIfInTransaction($connection); + $connection->method('getNativeConnection')->willReturn($wrappedConnection); + + $wrappedConnection->method('inTransaction')->willReturn(false); + + $this->expectException(LogicException::class); + TransactionHelper::rollback($connection); } - public function testItDoesNotTriggerADeprecationWhenUseless(): void + /** + * @doesNotPerformAssertions + */ + public function testItDoesNotThrowAnExceptionWhenUseless(): void { $connection = $this->createStub(Connection::class); $wrappedConnection = $this->createStub(PDO::class); @@ -43,14 +50,7 @@ public function testItDoesNotTriggerADeprecationWhenUseless(): void $wrappedConnection->method('inTransaction')->willReturn(true); - $this->expectNoDeprecationWithIdentifier( - 'https://github.com/doctrine/migrations/issues/1169' - ); - TransactionHelper::commitIfInTransaction($connection); - - $this->expectNoDeprecationWithIdentifier( - 'https://github.com/doctrine/migrations/issues/1169' - ); - TransactionHelper::rollbackIfInTransaction($connection); + TransactionHelper::commit($connection); + TransactionHelper::rollback($connection); } }