Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge release 3.8.1 into 3.9.x #1455

Merged
merged 7 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions docs/en/reference/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -243,18 +243,23 @@ Setting ``transactional`` to ``false`` will disable that.
From the Command Line
~~~~~~~~~~~~~~~~~~~~~

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

.. code-block:: sh

$ ./vendor/bin/doctrine-migrations migrate --all-or-nothing

If you have it enabled at the configuration level and want to change it for an individual migration you can
pass a value of ``0`` or ``1`` to ``--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:

.. code-block:: sh

$ ./vendor/bin/doctrine-migrations migrate --all-or-nothing=0
$ ./vendor/bin/doctrine-migrations migrate --no-all-or-nothing

Connection Configuration
------------------------
Expand Down
2 changes: 1 addition & 1 deletion src/Generator/Generator.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public function generateMigration(
string|null $down = null,
): string {
$mch = [];
if (preg_match('~(.*)\\\\([^\\\\]+)~', $fqcn, $mch) === 0) {
if (preg_match('~(.*)\\\\([^\\\\]+)~', $fqcn, $mch) !== 1) {
throw new InvalidArgumentException(sprintf('Invalid FQCN'));
}

Expand Down
6 changes: 6 additions & 0 deletions src/Tools/Console/Command/MigrateCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ protected function configure(): void
'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:

Expand Down
39 changes: 32 additions & 7 deletions src/Tools/Console/ConsoleInputMigratorConfigurationFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,30 +31,55 @@ public function getMigratorConfiguration(InputInterface $input): MigratorConfigu

private function determineAllOrNothingValueFrom(InputInterface $input): bool|null
{
$allOrNothingOption = 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) {
$allOrNothingOption = $input->getOption('all-or-nothing');
/**
* 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 ($wasOptionExplicitlyPassed && ($allOrNothingOption !== null && $allOrNothingOption !== self::ABSENT_CONFIG_VALUE)) {
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, omit the option,
Solution: If you need to disable the behavior, use --no-all-or-nothing,
otherwise, pass the option without a value
DEPRECATION,
);
}

return match ($allOrNothingOption) {
return match ($enableAllOrNothingOption) {
self::ABSENT_CONFIG_VALUE => null,
null => false,
default => (bool) $allOrNothingOption,
null => true,
default => (bool) $enableAllOrNothingOption,
};
}
}
16 changes: 16 additions & 0 deletions src/Tools/Console/InvalidAllOrNothingConfiguration.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types=1);

namespace Doctrine\Migrations\Tools\Console;

use Doctrine\Migrations\Configuration\Exception\ConfigurationException;
use LogicException;

final class InvalidAllOrNothingConfiguration extends LogicException implements ConfigurationException
{
public static function new(): self
{
return new self('Providing --all-or-nothing and --no-all-or-nothing simultaneously is forbidden.');
}
}
14 changes: 14 additions & 0 deletions tests/Configuration/ConfigurationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,18 @@ public function testMigrationOrganizationCanBeReset(): void
self::assertFalse($config->areMigrationsOrganizedByYearAndMonth());
self::assertFalse($config->areMigrationsOrganizedByYear());
}

public function testTransactionalConfigDefaultOption(): void
{
$config = new Configuration();

self::assertTrue($config->isTransactional());
}

public function testAllOrNothingConfigDefaultOption(): void
{
$config = new Configuration();

self::assertFalse($config->isAllOrNothing());
}
}
26 changes: 22 additions & 4 deletions tests/Tools/Console/Command/MigrateCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
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;
Expand Down Expand Up @@ -343,11 +344,13 @@ public function testExecuteMigrateDown(): void
*
* @dataProvider allOrNothing
*/
public function testExecuteMigrateAllOrNothing(bool $default, array $input, bool $expected, bool $expectDeprecation = true): void
public function testExecuteMigrateAllOrNothing(bool|null $default, array $input, bool $expected, bool $expectDeprecation = true): void
{
$migrator = $this->createMock(DbalMigrator::class);
$this->dependencyFactory->setService(Migrator::class, $migrator);
$this->configuration->setAllOrNothing($default);
if ($default !== null) {
$this->configuration->setAllOrNothing($default);
}

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

/** @psalm-return Generator<array{0: bool, 1: array<string, bool|int|string|null>, 2: bool, 3?: bool}> */
/** @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];
Expand All @@ -381,14 +384,29 @@ 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], false, false];
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
Expand Down