From f3c7942c91272a8ce3b774182f8c882b42387294 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 23 May 2020 07:05:51 +0200 Subject: [PATCH] Allow to use --write-sql to save the migration execution log to file --- UPGRADE.md | 4 +- .../Tools/Console/Command/ExecuteCommand.php | 38 +++++++++---------- .../Tools/Console/Command/MigrateCommand.php | 34 +++++++---------- ...nsoleInputMigratorConfigurationFactory.php | 3 +- .../Console/Command/ExecuteCommandTest.php | 34 ++++++++++++----- .../Console/Command/MigrateCommandTest.php | 36 +++++++++++++----- 6 files changed, 87 insertions(+), 62 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 68422d2157..68c9ecbc76 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -10,7 +10,9 @@ please refer to the [Code BC breaks](#code-bc-breaks) section. Console output is not covered by the BC promise, so please try not to rely on specific a output. Different levels of verbosity are available now (`-v`, `-vv` and `-vvv` ). - The `--show-versions` option from `migrations:status` command has been removed, - use `migrations:list` instead. + use `migrations:list` instead. +- The `--write-sql` option for `migrations:migrate` and `migrations:execute` does not imply dry-run anymore, +use the `--dry-run` parameter instead. ## Migrations table diff --git a/lib/Doctrine/Migrations/Tools/Console/Command/ExecuteCommand.php b/lib/Doctrine/Migrations/Tools/Console/Command/ExecuteCommand.php index 1aa7135b77..3893f2370e 100644 --- a/lib/Doctrine/Migrations/Tools/Console/Command/ExecuteCommand.php +++ b/lib/Doctrine/Migrations/Tools/Console/Command/ExecuteCommand.php @@ -15,6 +15,7 @@ use function implode; use function is_string; use function is_writable; +use function sprintf; /** * The ExecuteCommand class is responsible for executing migration versions up or down manually. @@ -114,34 +115,22 @@ protected function execute(InputInterface $input, OutputInterface $output) : int $this->getDependencyFactory()->getMetadataStorage()->ensureInitialized(); $versions = $input->getArgument('versions'); - $path = $input->getOption('write-sql'); $direction = $input->getOption('down') !== false ? Direction::DOWN : Direction::UP; - $migrator = $this->getDependencyFactory()->getMigrator(); + $path = $input->getOption('write-sql') ?? getcwd(); + if (is_string($path) && ! is_writable($path)) { + $this->io->error(sprintf('The path "%s" not writeable!', $path)); + + return 1; + } + $planCalculator = $this->getDependencyFactory()->getMigrationPlanCalculator(); $plan = $planCalculator->getPlanForVersions(array_map(static function (string $version) : Version { return new Version($version); }, $versions), $direction); - if ($migratorConfiguration->isDryRun()) { - $sql = $migrator->migrate($plan, $migratorConfiguration); - - $path = is_string($path) ? $path : getcwd(); - - if (! is_string($path) || ! is_writable($path)) { - $this->io->error('Path not writeable!'); - - return 1; - } - - $writer = $this->getDependencyFactory()->getQueryWriter(); - $writer->write($path, $direction, $sql); - - return 0; - } - $this->getDependencyFactory()->getLogger()->notice( 'Executing' . ($migratorConfiguration->isDryRun() ? ' (dry-run)' : '') . ' {versions} {direction}', [ @@ -149,7 +138,16 @@ protected function execute(InputInterface $input, OutputInterface $output) : int 'versions' => implode(', ', $versions), ] ); - $migrator->migrate($plan, $migratorConfiguration); + + $migrator = $this->getDependencyFactory()->getMigrator(); + $sql = $migrator->migrate($plan, $migratorConfiguration); + + if (is_string($path)) { + $writer = $this->getDependencyFactory()->getQueryWriter(); + $writer->write($path, $direction, $sql); + } + + $this->io->newLine(); return 0; } diff --git a/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php b/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php index 9f453f22fa..ce148f2123 100644 --- a/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php +++ b/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php @@ -132,7 +132,13 @@ protected function execute(InputInterface $input, OutputInterface $output) : int $allowNoMigration = $input->getOption('allow-no-migration'); $versionAlias = $input->getArgument('version'); - $path = $input->getOption('write-sql'); + + $path = $input->getOption('write-sql') ?? getcwd(); + if (is_string($path) && ! is_writable($path)) { + $this->io->error(sprintf('The path "%s" not writeable!', $path)); + + return 1; + } try { $version = $this->getDependencyFactory()->getVersionAliasResolver()->resolveVersionAlias($versionAlias); @@ -154,24 +160,6 @@ protected function execute(InputInterface $input, OutputInterface $output) : int return $this->errorForAlias($versionAlias, $allowNoMigration); } - $migrator = $this->getDependencyFactory()->getMigrator(); - if ($migratorConfiguration->isDryRun()) { - $sql = $migrator->migrate($plan, $migratorConfiguration); - - $path = is_string($path) ? $path : getcwd(); - - if (! is_string($path) || ! is_writable($path)) { - $this->io->error('Path not writeable!'); - - return 1; - } - - $writer = $this->getDependencyFactory()->getQueryWriter(); - $writer->write($path, $plan->getDirection(), $sql); - - return 0; - } - $this->getDependencyFactory()->getLogger()->notice( 'Migrating' . ($migratorConfiguration->isDryRun() ? ' (dry-run)' : '') . ' {direction} to {to}', [ @@ -180,7 +168,13 @@ protected function execute(InputInterface $input, OutputInterface $output) : int ] ); - $migrator->migrate($plan, $migratorConfiguration); + $migrator = $this->getDependencyFactory()->getMigrator(); + $sql = $migrator->migrate($plan, $migratorConfiguration); + + if (is_string($path)) { + $writer = $this->getDependencyFactory()->getQueryWriter(); + $writer->write($path, $plan->getDirection(), $sql); + } $this->io->newLine(); diff --git a/lib/Doctrine/Migrations/Tools/Console/ConsoleInputMigratorConfigurationFactory.php b/lib/Doctrine/Migrations/Tools/Console/ConsoleInputMigratorConfigurationFactory.php index 0c962c24ad..b6fd894e9f 100644 --- a/lib/Doctrine/Migrations/Tools/Console/ConsoleInputMigratorConfigurationFactory.php +++ b/lib/Doctrine/Migrations/Tools/Console/ConsoleInputMigratorConfigurationFactory.php @@ -22,11 +22,10 @@ public function getMigratorConfiguration(InputInterface $input) : MigratorConfig { $timeAllQueries = $input->hasOption('query-time') ? (bool) $input->getOption('query-time') : false; $dryRun = $input->hasOption('dry-run') ? (bool) $input->getOption('dry-run') : false; - $writeSql = $input->hasOption('write-sql') ? $input->getOption('write-sql') : false; $allOrNothing = $input->hasOption('all-or-nothing') ? (bool) $input->getOption('all-or-nothing') : $this->configuration->isAllOrNothing(); return (new MigratorConfiguration()) - ->setDryRun($dryRun || $writeSql !== false) + ->setDryRun($dryRun) ->setTimeAllQueries($timeAllQueries) ->setAllOrNothing($allOrNothing); } diff --git a/tests/Doctrine/Migrations/Tests/Tools/Console/Command/ExecuteCommandTest.php b/tests/Doctrine/Migrations/Tests/Tools/Console/Command/ExecuteCommandTest.php index 74798da383..d12b2a56fc 100644 --- a/tests/Doctrine/Migrations/Tests/Tools/Console/Command/ExecuteCommandTest.php +++ b/tests/Doctrine/Migrations/Tests/Tools/Console/Command/ExecuteCommandTest.php @@ -47,30 +47,38 @@ class ExecuteCommandTest extends MigrationTestCase private $planCalculator; /** - * @param mixed $arg + * @param bool|string|null $arg * * @dataProvider getWriteSqlValues */ - public function testWriteSql($arg, string $path) : void + public function testWriteSql(bool $dryRun, $arg, ?string $path) : void { $this->migrator ->expects(self::once()) ->method('migrate') - ->willReturnCallback(static function (MigrationPlanList $planList, MigratorConfiguration $configuration) : array { - self::assertTrue($configuration->isDryRun()); + ->willReturnCallback(static function (MigrationPlanList $planList, MigratorConfiguration $configuration) use ($dryRun) : array { + self::assertSame($dryRun, $configuration->isDryRun()); return ['A']; }); - $this->queryWriter->expects(self::once()) - ->method('write') - ->with($path, 'down', ['A']); + if ($arg === false) { + $this->queryWriter + ->expects(self::never()) + ->method('write'); + } else { + $this->queryWriter + ->expects(self::once()) + ->method('write') + ->with($path, 'down', ['A']); + } $this->executeCommandTester->execute([ 'versions' => ['1'], '--down' => true, '--write-sql' => $arg, - ]); + '--dry-run' => $dryRun, + ], ['interactive' => false]); self::assertSame(0, $this->executeCommandTester->getStatusCode()); } @@ -81,8 +89,14 @@ public function testWriteSql($arg, string $path) : void public function getWriteSqlValues() : array { return [ - [true, getcwd()], - [ __DIR__ . '/_files', __DIR__ . '/_files'], + // dry-run, write-path, path + [true, false, null], + [true, null, getcwd()], + [true, __DIR__ . '/_files', __DIR__ . '/_files'], + + [false, false, null], + [false, null, getcwd()], + [false, __DIR__ . '/_files', __DIR__ . '/_files'], ]; } diff --git a/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php b/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php index ec772c6c26..27caeef11e 100644 --- a/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php +++ b/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php @@ -168,11 +168,11 @@ public function testExecutedUnavailableMigrationsCancel() : void } /** - * @param mixed $arg + * @param bool|string|null $arg * * @dataProvider getWriteSqlValues */ - public function testExecuteWriteSql($arg, string $path) : void + public function testExecuteWriteSql(bool $dryRun, $arg, ?string $path) : void { $migrator = $this->createMock(DbalMigrator::class); @@ -180,16 +180,28 @@ public function testExecuteWriteSql($arg, string $path) : void $migrator->expects(self::once()) ->method('migrate') - ->willReturnCallback(static function (MigrationPlanList $planList, MigratorConfiguration $configuration) : array { + ->willReturnCallback(static function (MigrationPlanList $planList, MigratorConfiguration $configuration) use ($dryRun) : array { + self::assertSame($dryRun, $configuration->isDryRun()); + return ['A']; }); - $this->queryWriter->expects(self::once()) - ->method('write') - ->with($path, 'up', ['A']); + if ($arg === false) { + $this->queryWriter + ->expects(self::never()) + ->method('write'); + } else { + $this->queryWriter + ->expects(self::once()) + ->method('write') + ->with($path, 'up', ['A']); + } $this->migrateCommandTester->execute( - ['--write-sql' => $arg], + [ + '--write-sql' => $arg, + '--dry-run' => $dryRun, + ], ['interactive' => false] ); self::assertSame(0, $this->migrateCommandTester->getStatusCode()); @@ -201,8 +213,14 @@ public function testExecuteWriteSql($arg, string $path) : void public function getWriteSqlValues() : array { return [ - [true, getcwd()], - [ __DIR__ . '/_files', __DIR__ . '/_files'], + // dry-run, write-path, path + [true, false, null], + [true, null, getcwd()], + [true, __DIR__ . '/_files', __DIR__ . '/_files'], + + [false, false, null], + [false, null, getcwd()], + [false, __DIR__ . '/_files', __DIR__ . '/_files'], ]; }