Skip to content

Commit bcff5ce

Browse files
greg0iregreg0ire
authored
Merge pull request #1457 from doctrine/3.9.x
Merge 3.9.x up into 4.0.x
2 parents 348aaec + 610092c commit bcff5ce

File tree

7 files changed

+100
-16
lines changed

7 files changed

+100
-16
lines changed

‎docs/en/reference/configuration.rst

+9-4
Original file line numberDiff line numberDiff line change
@@ -243,18 +243,23 @@ Setting ``transactional`` to ``false`` will disable that.
243243
From the Command Line
244244
~~~~~~~~~~~~~~~~~~~~~
245245

246-
You can also set this option from the command line with the ``migrate`` command and the ``--all-or-nothing`` option:
246+
To override the configuration and explicitly enable All or Nothing Transaction from the command line,
247+
use the ``--all-or-nothing`` option:
247248

248249
.. code-block:: sh
249250
250251
$ ./vendor/bin/doctrine-migrations migrate --all-or-nothing
251252
252-
If you have it enabled at the configuration level and want to change it for an individual migration you can
253-
pass a value of ``0`` or ``1`` to ``--all-or-nothing``.
253+
.. note::
254+
255+
Passing options to --all-or-nothing is deprecated from 3.7.x, and will not be allowed in 4.x
256+
257+
To override the configuration and explicitly disable All or Nothing Transaction from the command line,
258+
use the ``--no-all-or-nothing`` option:
254259

255260
.. code-block:: sh
256261
257-
$ ./vendor/bin/doctrine-migrations migrate --all-or-nothing=0
262+
$ ./vendor/bin/doctrine-migrations migrate --no-all-or-nothing
258263
259264
Connection Configuration
260265
------------------------

‎src/Generator/Generator.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public function generateMigration(
7575
string|null $down = null,
7676
): string {
7777
$mch = [];
78-
if (preg_match('~(.*)\\\\([^\\\\]+)~', $fqcn, $mch) === 0) {
78+
if (preg_match('~(.*)\\\\([^\\\\]+)~', $fqcn, $mch) !== 1) {
7979
throw new InvalidArgumentException(sprintf('Invalid FQCN'));
8080
}
8181

‎src/Tools/Console/Command/MigrateCommand.php

+6
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,12 @@ protected function configure(): void
7878
'Wrap the entire migration in a transaction.',
7979
ConsoleInputMigratorConfigurationFactory::ABSENT_CONFIG_VALUE,
8080
)
81+
->addOption(
82+
'no-all-or-nothing',
83+
null,
84+
InputOption::VALUE_NONE,
85+
'Disable wrapping the entire migration in a transaction.',
86+
)
8187
->setHelp(<<<'EOT'
8288
The <info>%command.name%</info> command executes a migration to a specified version or the latest available version:
8389

‎src/Tools/Console/ConsoleInputMigratorConfigurationFactory.php

+32-7
Original file line numberDiff line numberDiff line change
@@ -31,30 +31,55 @@ public function getMigratorConfiguration(InputInterface $input): MigratorConfigu
3131

3232
private function determineAllOrNothingValueFrom(InputInterface $input): bool|null
3333
{
34-
$allOrNothingOption = null;
34+
$enableAllOrNothingOption = self::ABSENT_CONFIG_VALUE;
35+
$disableAllOrNothingOption = null;
36+
37+
if ($input->hasOption('no-all-or-nothing')) {
38+
$disableAllOrNothingOption = $input->getOption('no-all-or-nothing');
39+
}
40+
3541
$wasOptionExplicitlyPassed = $input->hasOption('all-or-nothing');
3642

3743
if ($wasOptionExplicitlyPassed) {
38-
$allOrNothingOption = $input->getOption('all-or-nothing');
44+
/**
45+
* Due to this option being able to receive optional values, its behavior is tricky:
46+
* - when `--all-or-nothing` option is not provided, the default is set to self::ABSENT_CONFIG_VALUE
47+
* - when `--all-or-nothing` option is provided without values, this will be `null`
48+
* - when `--all-or-nothing` option is provided with a value, we get the provided value
49+
*/
50+
$enableAllOrNothingOption = $input->getOption('all-or-nothing');
51+
}
52+
53+
$enableAllOrNothingDeprecation = match ($enableAllOrNothingOption) {
54+
self::ABSENT_CONFIG_VALUE, null => false,
55+
default => true,
56+
};
57+
58+
if ($enableAllOrNothingOption !== self::ABSENT_CONFIG_VALUE && $disableAllOrNothingOption === true) {
59+
throw InvalidAllOrNothingConfiguration::new();
60+
}
61+
62+
if ($disableAllOrNothingOption === true) {
63+
return false;
3964
}
4065

41-
if ($wasOptionExplicitlyPassed && ($allOrNothingOption !== null && $allOrNothingOption !== self::ABSENT_CONFIG_VALUE)) {
66+
if ($enableAllOrNothingDeprecation) {
4267
Deprecation::trigger(
4368
'doctrine/migrations',
4469
'https://github.com/doctrine/migrations/issues/1304',
4570
<<<'DEPRECATION'
4671
Context: Passing values to option `--all-or-nothing`
4772
Problem: Passing values is deprecated
48-
Solution: If you need to disable the behavior, omit the option,
73+
Solution: If you need to disable the behavior, use --no-all-or-nothing,
4974
otherwise, pass the option without a value
5075
DEPRECATION,
5176
);
5277
}
5378

54-
return match ($allOrNothingOption) {
79+
return match ($enableAllOrNothingOption) {
5580
self::ABSENT_CONFIG_VALUE => null,
56-
null => false,
57-
default => (bool) $allOrNothingOption,
81+
null => true,
82+
default => (bool) $enableAllOrNothingOption,
5883
};
5984
}
6085
}

‎src/Tools/Console/InvalidAllOrNothingConfiguration.php

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Migrations\Tools\Console;
6+
7+
use Doctrine\Migrations\Configuration\Exception\ConfigurationException;
8+
use LogicException;
9+
10+
final class InvalidAllOrNothingConfiguration extends LogicException implements ConfigurationException
11+
{
12+
public static function new(): self
13+
{
14+
return new self('Providing --all-or-nothing and --no-all-or-nothing simultaneously is forbidden.');
15+
}
16+
}

‎tests/Configuration/ConfigurationTest.php

+14
Original file line numberDiff line numberDiff line change
@@ -93,4 +93,18 @@ public function testMigrationOrganizationCanBeReset(): void
9393
self::assertFalse($config->areMigrationsOrganizedByYearAndMonth());
9494
self::assertFalse($config->areMigrationsOrganizedByYear());
9595
}
96+
97+
public function testTransactionalConfigDefaultOption(): void
98+
{
99+
$config = new Configuration();
100+
101+
self::assertTrue($config->isTransactional());
102+
}
103+
104+
public function testAllOrNothingConfigDefaultOption(): void
105+
{
106+
$config = new Configuration();
107+
108+
self::assertFalse($config->isAllOrNothing());
109+
}
96110
}

‎tests/Tools/Console/Command/MigrateCommandTest.php

+22-4
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use Doctrine\Migrations\Tests\Helper;
2727
use Doctrine\Migrations\Tests\MigrationTestCase;
2828
use Doctrine\Migrations\Tools\Console\Command\MigrateCommand;
29+
use Doctrine\Migrations\Tools\Console\InvalidAllOrNothingConfiguration;
2930
use Doctrine\Migrations\Version\AlphabeticalComparator;
3031
use Doctrine\Migrations\Version\ExecutionResult;
3132
use Doctrine\Migrations\Version\MigrationFactory;
@@ -343,11 +344,13 @@ public function testExecuteMigrateDown(): void
343344
*
344345
* @dataProvider allOrNothing
345346
*/
346-
public function testExecuteMigrateAllOrNothing(bool $default, array $input, bool $expected, bool $expectDeprecation = true): void
347+
public function testExecuteMigrateAllOrNothing(bool|null $default, array $input, bool $expected, bool $expectDeprecation = true): void
347348
{
348349
$migrator = $this->createMock(DbalMigrator::class);
349350
$this->dependencyFactory->setService(Migrator::class, $migrator);
350-
$this->configuration->setAllOrNothing($default);
351+
if ($default !== null) {
352+
$this->configuration->setAllOrNothing($default);
353+
}
351354

352355
$migrator->expects(self::once())
353356
->method('migrate')
@@ -371,7 +374,7 @@ public function testExecuteMigrateAllOrNothing(bool $default, array $input, bool
371374
self::assertSame(0, $this->migrateCommandTester->getStatusCode());
372375
}
373376

374-
/** @psalm-return Generator<array{0: bool, 1: array<string, bool|int|string|null>, 2: bool, 3?: bool}> */
377+
/** @psalm-return Generator<array{0: bool|null, 1: array<string, bool|int|string|null>, 2: bool, 3?: bool}> */
375378
public static function allOrNothing(): Generator
376379
{
377380
yield [false, ['--all-or-nothing' => false], false];
@@ -381,14 +384,29 @@ public static function allOrNothing(): Generator
381384
yield [false, ['--all-or-nothing' => true], true];
382385
yield [false, ['--all-or-nothing' => 1], true];
383386
yield [false, ['--all-or-nothing' => '1'], true];
384-
yield [false, ['--all-or-nothing' => null], false, false];
387+
yield [false, ['--all-or-nothing' => null], true, false];
388+
yield [true, ['--no-all-or-nothing' => null], false, false];
385389

386390
yield [true, ['--all-or-nothing' => false], false];
387391
yield [true, ['--all-or-nothing' => 0], false];
388392
yield [true, ['--all-or-nothing' => '0'], false];
389393

390394
yield [true, [], true, false];
391395
yield [false, [], false, false];
396+
yield [null, [], false, false];
397+
}
398+
399+
public function testThrowsOnContradictoryAllOrNothingOptions(): void
400+
{
401+
$this->expectException(InvalidAllOrNothingConfiguration::class);
402+
403+
$this->migrateCommandTester->execute(
404+
[
405+
'--all-or-nothing' => null,
406+
'--no-all-or-nothing' => null,
407+
],
408+
['interactive' => false],
409+
);
392410
}
393411

394412
public function testExecuteMigrateCancelExecutedUnavailableMigrations(): void

0 commit comments

Comments
 (0)