Merge pull request #1161 from grachevko/all-or-nothing

Bugfix: all-or-nothing from configuration must not be ignored
doctrine · Jun 10, 2021 · 8d0c586 · 8d0c586
2 parents c925cd5 + 6cf7769
commit 8d0c586
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 9 deletions.
diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml
@@ -50,6 +50,8 @@ jobs:
           coverage: "pcov"
           extensions: "pdo_sqlite"
           ini-values: "zend.assertions=1"
+          # Remove this line when a fix for is available https://github.com/box-project/box/issues/555
+          tools: "composer:v2.0.14"
 
       - name: "Download box"
         run: "./download-box.sh"

diff --git a/lib/Doctrine/Migrations/Provider/OrmSchemaProvider.php b/lib/Doctrine/Migrations/Provider/OrmSchemaProvider.php
@@ -35,7 +35,7 @@ public function __construct(EntityManagerInterface $em)
      */
     public function createSchema(): Schema
     {
-        /** @var array<int, ClassMetadata> $metadata */
+        /** @var array<int, ClassMetadata<object>> $metadata */
         $metadata = $this->entityManager->getMetadataFactory()->getAllMetadata();
 
         if (count($metadata) === 0) {

diff --git a/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php b/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php
@@ -73,8 +73,7 @@ protected function configure(): void
                 'all-or-nothing',
                 null,
                 InputOption::VALUE_OPTIONAL,
-                'Wrap the entire migration in a transaction.',
-                false
+                'Wrap the entire migration in a transaction.'
             )
             ->setHelp(<<<EOT
 The <info>%command.name%</info> command executes a migration to a specified version or the latest available version:

diff --git a/lib/Doctrine/Migrations/Tools/Console/ConsoleInputMigratorConfigurationFactory.php b/lib/Doctrine/Migrations/Tools/Console/ConsoleInputMigratorConfigurationFactory.php
@@ -22,7 +22,8 @@ public function getMigratorConfiguration(InputInterface $input): MigratorConfigu
     {
         $timeAllQueries = $input->hasOption('query-time') ? (bool) $input->getOption('query-time') : false;
         $dryRun         = $input->hasOption('dry-run') ? (bool) $input->getOption('dry-run') : false;
-        $allOrNothing   = $input->hasOption('all-or-nothing') ? (bool) $input->getOption('all-or-nothing') : $this->configuration->isAllOrNothing();
+        $allOrNothing   = $input->hasOption('all-or-nothing') ? $input->getOption('all-or-nothing') : null;
+        $allOrNothing   = (bool) ($allOrNothing ?? $this->configuration->isAllOrNothing());
 
         return (new MigratorConfiguration())
             ->setDryRun($dryRun)

diff --git a/tests/Doctrine/Migrations/Tests/Provider/ClassMetadataFactory.php b/tests/Doctrine/Migrations/Tests/Provider/ClassMetadataFactory.php
@@ -12,7 +12,7 @@
 class ClassMetadataFactory extends BaseMetadataFactoryAlias
 {
     /**
-     * @return ClassMetadata[]
+     * @psalm-return list<ClassMetadata<object>>
      */
     public function getAllMetadata(): array
     {

diff --git a/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php b/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php
@@ -31,6 +31,7 @@
 use Doctrine\Migrations\Version\ExecutionResult;
 use Doctrine\Migrations\Version\MigrationFactory;
 use Doctrine\Migrations\Version\Version;
+use Generator;
 use PHPUnit\Framework\MockObject\MockObject;
 use Symfony\Component\Console\Helper\HelperSet;
 use Symfony\Component\Console\Helper\QuestionHelper;
@@ -371,28 +372,46 @@ public function testExecuteMigrateDown(): void
         self::assertStringContainsString('[notice] Migrating down to A', trim($this->migrateCommandTester->getDisplay(true)));
     }
 
-    public function testExecuteMigrateAllOrNothing(): void
+    /**
+     * @dataProvider allOrNothing
+     * @psalm-param array<string, bool> $input
+     */
+    public function testExecuteMigrateAllOrNothing(bool $default, array $input, bool $expected): void
     {
         $migrator = $this->createMock(DbalMigrator::class);
         $this->dependencyFactory->setService(Migrator::class, $migrator);
+        $this->configuration->setAllOrNothing($default);
 
         $migrator->expects(self::once())
             ->method('migrate')
-            ->willReturnCallback(static function (MigrationPlanList $planList, MigratorConfiguration $configuration): array {
-                self::assertTrue($configuration->isAllOrNothing());
+            ->willReturnCallback(static function (MigrationPlanList $planList, MigratorConfiguration $configuration) use ($expected): array {
+                self::assertSame($expected, $configuration->isAllOrNothing());
                 self::assertCount(1, $planList);
 
                 return ['A'];
             });
 
         $this->migrateCommandTester->execute(
-            ['--all-or-nothing' => true],
+            $input,
             ['interactive' => false]
         );
 
         self::assertSame(0, $this->migrateCommandTester->getStatusCode());
     }
 
+    /**
+     * @psalm-return Generator<array{bool, array<string, bool>, bool}>
+     */
+    public function allOrNothing(): Generator
+    {
+        yield [false, ['--all-or-nothing' => false], false];
+        yield [false, ['--all-or-nothing' => true], true];
+        yield [true, ['--all-or-nothing' => false], false];
+
+        yield [true, [], true];
+        yield [false, [], false];
+    }
+
     public function testExecuteMigrateCancelExecutedUnavailableMigrations(): void
     {
         $result = new ExecutionResult(new Version('345'));