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.
doctrine · Jan 14, 2023 · ca8cb14 · ca8cb14
1 parent 53303eb
commit ca8cb14
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 46 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
     }
 }