Merge pull request #1167 from shadowhand/fix/1166-all-or-nothing-asse…

…rtion

Check for all-or-nothing conflict in migrations

lib/Doctrine/Migrations/DbalMigrator.php

@@ -5,6 +5,7 @@
 namespace Doctrine\Migrations;
 
 use Doctrine\DBAL\Connection;
+use Doctrine\Migrations\Exception\MigrationConfigurationConflict;
 use Doctrine\Migrations\Metadata\MigrationPlanList;
 use Doctrine\Migrations\Query\Query;
 use Doctrine\Migrations\Tools\BytesFormatter;
@@ -65,6 +66,7 @@ private function executeMigrations(
         $allOrNothing = $migratorConfiguration->isAllOrNothing();
 
         if ($allOrNothing) {
+            $this->assertAllMigrationsAreTransactional($migrationsPlan);
             $this->connection->beginTransaction();
         }
 
@@ -89,6 +91,15 @@ private function executeMigrations(
         return $sql;
     }
 
+    private function assertAllMigrationsAreTransactional(MigrationPlanList $migrationsPlan): void
+    {
+        foreach ($migrationsPlan->getItems() as $plan) {
+            if (! $plan->getMigration()->isTransactional()) {
+                throw MigrationConfigurationConflict::migrationIsNotTransactional($plan->getMigration());
+            }
+        }
+    }
+
     /**
      * @return array<string, Query[]>
      */

lib/Doctrine/Migrations/Exception/MigrationConfigurationConflict.php

@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Migrations\Exception;
+
+use Doctrine\Migrations\AbstractMigration;
+use UnexpectedValueException;
+
+use function get_class;
+use function sprintf;
+
+final class MigrationConfigurationConflict extends UnexpectedValueException implements MigrationException
+{
+    public static function migrationIsNotTransactional(AbstractMigration $migration): self
+    {
+        return new self(sprintf(
+            <<<'EXCEPTION'
+Context: attempting to execute migrations with all-or-nothing enabled
+Problem: migration %s is marked as non-transactional
+Solution: disable all-or-nothing in configuration or by command-line option, or enable transactions for all migrations
+EXCEPTION
+                ,
+            get_class($migration)
+        ));
+    }
+}

tests/Doctrine/Migrations/Tests/MigratorTest.php

@@ -9,6 +9,7 @@
 use Doctrine\Migrations\Configuration\Configuration;
 use Doctrine\Migrations\DbalMigrator;
 use Doctrine\Migrations\EventDispatcher;
+use Doctrine\Migrations\Exception\MigrationConfigurationConflict;
 use Doctrine\Migrations\Metadata\MigrationPlan;
 use Doctrine\Migrations\Metadata\MigrationPlanList;
 use Doctrine\Migrations\Metadata\Storage\MetadataStorage;
@@ -17,6 +18,7 @@
 use Doctrine\Migrations\Provider\SchemaDiffProvider;
 use Doctrine\Migrations\Tests\Stub\Functional\MigrateNotTouchingTheSchema;
 use Doctrine\Migrations\Tests\Stub\Functional\MigrationThrowsError;
+use Doctrine\Migrations\Tests\Stub\NonTransactional\MigrationNonTransactional;
 use Doctrine\Migrations\Version\DbalExecutor;
 use Doctrine\Migrations\Version\Direction;
 use Doctrine\Migrations\Version\Executor;
@@ -170,4 +172,22 @@ public function testMigrateAllOrNothingRollback(): void
 
         $migrator->migrate($planList, $this->migratorConfiguration);
     }
+
+    public function testMigrateAllOrNothingNonTransactionalMigration(): void
+    {
+        $this->config->addMigrationsDirectory('DoctrineMigrations\\', __DIR__ . '/Stub/NonTransactional');
+
+        $migrator = $this->createTestMigrator();
+
+        $this->migratorConfiguration->setAllOrNothing(true);
+
+        $migration = new MigrationNonTransactional($this->conn, $this->logger);
+        $plan      = new MigrationPlan(new Version(MigrationNonTransactional::class), $migration, Direction::UP);
+        $planList  = new MigrationPlanList([$plan], Direction::UP);
+
+        self::expectException(MigrationConfigurationConflict::class);
+        self::expectExceptionMessage(MigrationNonTransactional::class);
+
+        $migrator->migrate($planList, $this->migratorConfiguration);
+    }
 }

tests/Doctrine/Migrations/Tests/Stub/NonTransactional/MigrationNonTransactional.php

@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Migrations\Tests\Stub\NonTransactional;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+class MigrationNonTransactional extends AbstractMigration
+{
+    public function up(Schema $schema): void
+    {
+    }
+
+    public function down(Schema $schema): void
+    {
+    }
+
+    public function isTransactional(): bool
+    {
+        return false;
+    }
+}