Merge pull request #1456 from agustingomes/all-or-nothing-option

Simplify handling of `--all-or-nothing` and `--no-all-or-nothing` options
doctrine · Sep 2, 2024 · a3fc5b3 · a3fc5b3
2 parents bcff5ce + 51d59dd
commit a3fc5b3
Show file tree

Hide file tree

Showing 6 changed files with 13 additions and 104 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,3 +1,12 @@
+# Upgrade to 4.0
+
+## Console
+- The `--all-or-nothing` option for `migrations:migrate` does not accept a value anymore, and is now exclusively used 
+  to enable wrapping all migrations in a single transaction. To disable it, you can use the `--no-all-or-nothing` 
+  option instead. Both options override the configuration value.
+
+
+
 # Upgrade to 3.6
 
 ## Console

diff --git a/docs/en/reference/configuration.rst b/docs/en/reference/configuration.rst
@@ -250,10 +250,6 @@ use the ``--all-or-nothing`` option:
 
     $ ./vendor/bin/doctrine-migrations migrate --all-or-nothing
 
-.. note::
-
-    Passing options to --all-or-nothing is deprecated from 3.7.x, and will not be allowed in 4.x
-
 To override the configuration and explicitly disable All or Nothing Transaction from the command line,
 use the ``--no-all-or-nothing`` option:
 

diff --git a/src/Tools/Console/Command/MigrateCommand.php b/src/Tools/Console/Command/MigrateCommand.php
@@ -8,7 +8,6 @@
 use Doctrine\Migrations\Exception\NoMigrationsToExecute;
 use Doctrine\Migrations\Exception\UnknownMigrationVersion;
 use Doctrine\Migrations\Metadata\ExecutedMigrationsList;
-use Doctrine\Migrations\Tools\Console\ConsoleInputMigratorConfigurationFactory;
 use Symfony\Component\Console\Attribute\AsCommand;
 use Symfony\Component\Console\Formatter\OutputFormatter;
 use Symfony\Component\Console\Input\InputArgument;
@@ -74,15 +73,8 @@ protected function configure(): void
             ->addOption(
                 'all-or-nothing',
                 null,
-                InputOption::VALUE_OPTIONAL,
+                InputOption::VALUE_NEGATABLE,
                 'Wrap the entire migration in a transaction.',
-                ConsoleInputMigratorConfigurationFactory::ABSENT_CONFIG_VALUE,
-            )
-            ->addOption(
-                'no-all-or-nothing',
-                null,
-                InputOption::VALUE_NONE,
-                'Disable wrapping 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/src/Tools/Console/ConsoleInputMigratorConfigurationFactory.php b/src/Tools/Console/ConsoleInputMigratorConfigurationFactory.php
@@ -4,7 +4,6 @@
 
 namespace Doctrine\Migrations\Tools\Console;
 
-use Doctrine\Deprecations\Deprecation;
 use Doctrine\Migrations\Configuration\Configuration;
 use Doctrine\Migrations\MigratorConfiguration;
 use Symfony\Component\Console\Input\InputInterface;
@@ -31,55 +30,9 @@ public function getMigratorConfiguration(InputInterface $input): MigratorConfigu
 
     private function determineAllOrNothingValueFrom(InputInterface $input): bool|null
     {
-        $enableAllOrNothingOption  = self::ABSENT_CONFIG_VALUE;
-        $disableAllOrNothingOption = null;
-
-        if ($input->hasOption('no-all-or-nothing')) {
-            $disableAllOrNothingOption = $input->getOption('no-all-or-nothing');
-        }
-
-        $wasOptionExplicitlyPassed = $input->hasOption('all-or-nothing');
-
-        if ($wasOptionExplicitlyPassed) {
-            /**
-             * Due to this option being able to receive optional values, its behavior is tricky:
-             * - when `--all-or-nothing` option is not provided, the default is set to self::ABSENT_CONFIG_VALUE
-             * - when `--all-or-nothing` option is provided without values, this will be `null`
-             * - when `--all-or-nothing` option is provided with a value, we get the provided value
-             */
-            $enableAllOrNothingOption = $input->getOption('all-or-nothing');
-        }
-
-        $enableAllOrNothingDeprecation = match ($enableAllOrNothingOption) {
-            self::ABSENT_CONFIG_VALUE, null => false,
-            default => true,
-        };
-
-        if ($enableAllOrNothingOption !== self::ABSENT_CONFIG_VALUE && $disableAllOrNothingOption === true) {
-            throw InvalidAllOrNothingConfiguration::new();
-        }
-
-        if ($disableAllOrNothingOption === true) {
-            return false;
-        }
-
-        if ($enableAllOrNothingDeprecation) {
-            Deprecation::trigger(
-                'doctrine/migrations',
-                'https://github.com/doctrine/migrations/issues/1304',
-                <<<'DEPRECATION'
-                    Context: Passing values to option `--all-or-nothing`
-                    Problem: Passing values is deprecated
-                    Solution: If you need to disable the behavior, use --no-all-or-nothing,
-                    otherwise, pass the option without a value
-                    DEPRECATION,
-            );
-        }
-
-        return match ($enableAllOrNothingOption) {
-            self::ABSENT_CONFIG_VALUE => null,
-            null => true,
-            default => (bool) $enableAllOrNothingOption,
+        return match ($input->hasOption('all-or-nothing')) {
+            true => $input->getOption('all-or-nothing'),
+            default => null,
         };
     }
 }
diff --git a/src/Tools/Console/InvalidAllOrNothingConfiguration.php b/src/Tools/Console/InvalidAllOrNothingConfiguration.php
diff --git a/tests/Tools/Console/Command/MigrateCommandTest.php b/tests/Tools/Console/Command/MigrateCommandTest.php
@@ -26,7 +26,6 @@
 use Doctrine\Migrations\Tests\Helper;
 use Doctrine\Migrations\Tests\MigrationTestCase;
 use Doctrine\Migrations\Tools\Console\Command\MigrateCommand;
-use Doctrine\Migrations\Tools\Console\InvalidAllOrNothingConfiguration;
 use Doctrine\Migrations\Version\AlphabeticalComparator;
 use Doctrine\Migrations\Version\ExecutionResult;
 use Doctrine\Migrations\Version\MigrationFactory;
@@ -377,38 +376,14 @@ public function testExecuteMigrateAllOrNothing(bool|null $default, array $input,
     /** @psalm-return Generator<array{0: bool|null, 1: array<string, bool|int|string|null>, 2: bool, 3?: bool}> */
     public static 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, false];
         yield [true, ['--no-all-or-nothing' => null], false, false];
 
-        yield [true, ['--all-or-nothing' => false], false];
-        yield [true, ['--all-or-nothing' => 0], false];
-        yield [true, ['--all-or-nothing' => '0'], false];
-
         yield [true, [], true, false];
         yield [false, [], false, false];
         yield [null, [], false, false];
     }
 
-    public function testThrowsOnContradictoryAllOrNothingOptions(): void
-    {
-        $this->expectException(InvalidAllOrNothingConfiguration::class);
-
-        $this->migrateCommandTester->execute(
-            [
-                '--all-or-nothing' => null,
-                '--no-all-or-nothing' => null,
-            ],
-            ['interactive' => false],
-        );
-    }
-
     public function testExecuteMigrateCancelExecutedUnavailableMigrations(): void
     {
         $result = new ExecutionResult(new Version('345'));