Merge pull request #1296 from agustingomes/patch-1

Handle absence of value for `--all-or-nothing` properly
doctrine · Jan 11, 2023 · 01f89a1 · 01f89a1
2 parents 2c6ee5f + 4da00c5
commit 01f89a1
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 5 deletions.
diff --git a/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php b/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php
@@ -75,7 +75,8 @@ protected function configure(): void
                 'all-or-nothing',
                 null,
                 InputOption::VALUE_OPTIONAL,
-                'Wrap the entire migration in a transaction.'
+                'Wrap the entire migration in a transaction.',
+                'notprovided'
             )
             ->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
@@ -21,12 +21,26 @@ 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') ? $input->getOption('all-or-nothing') : null;
-        $allOrNothing   = (bool) ($allOrNothing ?? $this->configuration->isAllOrNothing());
+        $allOrNothing   = $this->determineAllOrNothingValueFrom($input) ?? $this->configuration->isAllOrNothing();
 
         return (new MigratorConfiguration())
             ->setDryRun($dryRun)
             ->setTimeAllQueries($timeAllQueries)
             ->setAllOrNothing($allOrNothing);
     }
+
+    private function determineAllOrNothingValueFrom(InputInterface $input): ?bool
+    {
+        $allOrNothingOption = null;
+
+        if ($input->hasOption('all-or-nothing')) {
+            $allOrNothingOption = $input->getOption('all-or-nothing');
+        }
+
+        if ($allOrNothingOption === 'notprovided') {
+            return null;
+        }
+
+        return (bool) ($allOrNothingOption ?? true);
+    }
 }
diff --git a/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php b/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php
@@ -362,7 +362,7 @@ public function testExecuteMigrateDown(): void
     }
 
     /**
-     * @psalm-param array<string, bool> $input
+     * @psalm-param array<string, bool|int|string|null> $input
      *
      * @dataProvider allOrNothing
      */
@@ -390,13 +390,22 @@ public function testExecuteMigrateAllOrNothing(bool $default, array $input, bool
     }
 
     /**
-     * @psalm-return Generator<array{bool, array<string, bool>, bool}>
+     * @psalm-return Generator<array{bool, array<string, bool|int|string|null>, bool}>
      */
     public function allOrNothing(): Generator
     {
         yield [false, ['--all-or-nothing' => false], false];
+        yield [false, ['--all-or-nothing' => 0], false];
+        yield [false, ['--all-or-nothing' => '0'], false];
+
         yield [false, ['--all-or-nothing' => true], true];
+        yield [false, ['--all-or-nothing' => 1], true];
+        yield [false, ['--all-or-nothing' => '1'], true];
+        yield [false, ['--all-or-nothing' => null], true];
+
         yield [true, ['--all-or-nothing' => false], false];
+        yield [true, ['--all-or-nothing' => 0], false];
+        yield [true, ['--all-or-nothing' => '0'], false];
 
         yield [true, [], true];
         yield [false, [], false];