Deprecate using --all-or-nothing with values

During the iteration of the PR #1296 an idea to deprecate passing values to this option was suggested, given how complicated was to handle using that option without any value, which led to a bug and inconsistency with the documentation. As specified in the deprecation message, using this option from 4.0.x onwards in the CLI will be treated as `true`, and the option itself won't accept values.
doctrine · Oct 10, 2023 · 64f23e0 · 64f23e0
1 parent ae7afcf
commit 64f23e0
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 7 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,3 +1,12 @@
+# Upgrade to 3.6
+
+## Console
+- The `--all-or-nothing` option for `migrations:migrate` does not accept a value anymore, and passing it a 
+  value will generate a deprecation. Specifying `--all-or-nothing` will wrap all the migrations to be 
+  executed into a single transaction, regardless of the specified configuration.
+
+
+
 # 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/Tools/Console/ConsoleInputMigratorConfigurationFactory.php b/lib/Doctrine/Migrations/Tools/Console/ConsoleInputMigratorConfigurationFactory.php
@@ -4,6 +4,7 @@
 
 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,16 +32,30 @@ public function getMigratorConfiguration(InputInterface $input): MigratorConfigu
 
     private function determineAllOrNothingValueFrom(InputInterface $input): ?bool
     {
-        if (! $input->hasOption('all-or-nothing')) {
-            return null;
+        $allOrNothingOption        = null;
+        $wasOptionExplicitlyPassed = $input->hasOption('all-or-nothing');
+
+        if ($wasOptionExplicitlyPassed) {
+            $allOrNothingOption = $input->getOption('all-or-nothing');
         }
 
-        $allOrNothingOption = $input->getOption('all-or-nothing');
+        if ($wasOptionExplicitlyPassed && $allOrNothingOption !== null) {
+            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: From version 4.0.x, `--all-or-nothing` option won't accept any value, 
+                    and the presence of the option will be treated as `true`.
+                    DEPRECATION,
+            );
+        }
 
         if ($allOrNothingOption === 'notprovided') {
             return null;
         }
 
-        return (bool) ($allOrNothingOption ?? true);
+        return (bool) ($allOrNothingOption ?? false);
     }
 }
diff --git a/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php b/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php
@@ -6,6 +6,7 @@
 
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Types\Types;
+use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
 use Doctrine\Migrations\AbstractMigration;
 use Doctrine\Migrations\Configuration\Configuration;
 use Doctrine\Migrations\Configuration\Connection\ExistingConnection;
@@ -43,6 +44,8 @@
 
 class MigrateCommandTest extends MigrationTestCase
 {
+    use VerifyDeprecations;
+
     private DependencyFactory $dependencyFactory;
 
     private Configuration $configuration;
@@ -357,7 +360,7 @@ public function testExecuteMigrateDown(): void
      *
      * @dataProvider allOrNothing
      */
-    public function testExecuteMigrateAllOrNothing(bool $default, array $input, bool $expected): void
+    public function testExecuteMigrateAllOrNothing(bool $default, array $input, bool $expected, bool $expectDeprecation = true): void
     {
         $migrator = $this->createMock(DbalMigrator::class);
         $this->dependencyFactory->setService(Migrator::class, $migrator);
@@ -372,6 +375,12 @@ public function testExecuteMigrateAllOrNothing(bool $default, array $input, bool
                 return ['A'];
             });
 
+        if ($expectDeprecation) {
+            $this->expectDeprecationWithIdentifier(
+                'https://github.com/doctrine/migrations/issues/1304',
+            );
+        }
+
         $this->migrateCommandTester->execute(
             $input,
             ['interactive' => false],
@@ -380,7 +389,7 @@ public function testExecuteMigrateAllOrNothing(bool $default, array $input, bool
         self::assertSame(0, $this->migrateCommandTester->getStatusCode());
     }
 
-    /** @psalm-return Generator<array{bool, array<string, bool|int|string|null>, bool}> */
+    /** @psalm-return Generator<array{0: bool, 1: array<string, bool|int|string|null>, 2: bool, 3?: bool}> */
     public static function allOrNothing(): Generator
     {
         yield [false, ['--all-or-nothing' => false], false];
@@ -390,7 +399,7 @@ public static function allOrNothing(): Generator
         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 [false, ['--all-or-nothing' => null], false, false];
 
         yield [true, ['--all-or-nothing' => false], false];
         yield [true, ['--all-or-nothing' => 0], false];