From 3cb3932d85caed3d0016208abe1f2440f9f650d5 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Fri, 15 May 2020 07:29:17 +0200 Subject: [PATCH 1/3] Change alias resolution providing more descriptive error messages and info on the current version --- .../Tools/Console/Command/MigrateCommand.php | 48 ++++++------- .../Version/DefaultAliasResolver.php | 6 +- .../Console/Command/MigrateCommandTest.php | 69 ++++++++++--------- 3 files changed, 63 insertions(+), 60 deletions(-) diff --git a/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php b/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php index e9342aa4ae..9f453f22fa 100644 --- a/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php +++ b/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php @@ -5,6 +5,7 @@ namespace Doctrine\Migrations\Tools\Console\Command; use Doctrine\Migrations\Exception\NoMigrationsFoundWithCriteria; +use Doctrine\Migrations\Exception\NoMigrationsToExecute; use Doctrine\Migrations\Exception\UnknownMigrationVersion; use Doctrine\Migrations\Metadata\ExecutedMigrationsList; use Symfony\Component\Console\Formatter\OutputFormatter; @@ -14,10 +15,11 @@ use Symfony\Component\Console\Output\OutputInterface; use function count; use function getcwd; +use function in_array; use function is_string; use function is_writable; use function sprintf; -use function substr; +use function strpos; /** * The MigrateCommand class is responsible for executing a migration from the current version to another @@ -134,10 +136,8 @@ protected function execute(InputInterface $input, OutputInterface $output) : int try { $version = $this->getDependencyFactory()->getVersionAliasResolver()->resolveVersionAlias($versionAlias); - } catch (UnknownMigrationVersion|NoMigrationsFoundWithCriteria $e) { - $this->getVersionNameFromAlias($versionAlias); - - return 1; + } catch (UnknownMigrationVersion|NoMigrationsToExecute|NoMigrationsFoundWithCriteria $e) { + return $this->errorForAlias($versionAlias, $allowNoMigration); } $planCalculator = $this->getDependencyFactory()->getMigrationPlanCalculator(); @@ -150,16 +150,8 @@ protected function execute(InputInterface $input, OutputInterface $output) : int $plan = $planCalculator->getPlanUntilVersion($version); - if (count($plan) === 0 && ! $allowNoMigration) { - $this->io->warning('Could not find any migrations to execute.'); - - return 1; - } - if (count($plan) === 0) { - $this->getVersionNameFromAlias($versionAlias); - - return 0; + return $this->errorForAlias($versionAlias, $allowNoMigration); } $migrator = $this->getDependencyFactory()->getMigrator(); @@ -227,29 +219,33 @@ private function checkExecutedUnavailableMigrations( return true; } - private function getVersionNameFromAlias(string $versionAlias) : void + private function errorForAlias(string $versionAlias, bool $allowNoMigration) : int { - if ($versionAlias === 'first') { - $this->io->error('Already at first version.'); + if (in_array($versionAlias, ['first', 'next', 'latest'], true) || strpos($versionAlias, 'current') === 0) { + $version = $this->getDependencyFactory()->getVersionAliasResolver()->resolveVersionAlias('current'); - return; - } + $message = sprintf( + 'The version "%s" couldn\'t be reached, you are at version "%s"', + $versionAlias, + (string) $version + ); - if ($versionAlias === 'next' || $versionAlias === 'latest') { - $this->io->error('Already at latest version.'); + if ($allowNoMigration) { + $this->io->warning($message); - return; - } + return 0; + } - if (substr($versionAlias, 0, 7) === 'current') { - $this->io->error('The delta couldn\'t be reached.'); + $this->io->error($message); - return; + return 1; } $this->io->error(sprintf( 'Unknown version: %s', OutputFormatter::escape($versionAlias) )); + + return 1; } } diff --git a/lib/Doctrine/Migrations/Version/DefaultAliasResolver.php b/lib/Doctrine/Migrations/Version/DefaultAliasResolver.php index 0c76691ff9..b02a97ac1e 100644 --- a/lib/Doctrine/Migrations/Version/DefaultAliasResolver.php +++ b/lib/Doctrine/Migrations/Version/DefaultAliasResolver.php @@ -54,6 +54,10 @@ public function __construct( * - latest: The latest available version. * * If an existing version number is specified, it is returned verbatimly. + * + * @throws NoMigrationsToExecute + * @throws UnknownMigrationVersion + * @throws NoMigrationsFoundWithCriteria */ public function resolveVersionAlias(string $alias) : Version { @@ -93,7 +97,7 @@ public function resolveVersionAlias(string $alias) : Version try { return $availableMigrations->getLast()->getVersion(); } catch (NoMigrationsFoundWithCriteria $e) { - throw NoMigrationsToExecute::new($e); + return $this->resolveVersionAlias(self::ALIAS_CURRENT); } // no break because of return diff --git a/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php b/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php index 04903f96e0..ec772c6c26 100644 --- a/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php +++ b/tests/Doctrine/Migrations/Tests/Tools/Console/Command/MigrateCommandTest.php @@ -36,6 +36,7 @@ use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Tester\CommandTester; use function getcwd; +use function sprintf; use function strpos; use function trim; @@ -71,7 +72,7 @@ class MigrateCommandTest extends MigrationTestCase /** @var TableMetadataStorageConfiguration */ private $metadataConfiguration; - public function testExecuteEmptyMigrationPlanCausesException() : void + public function testTargetUnknownVersion() : void { $result = new ExecutionResult(new Version('A')); $this->storage->complete($result); @@ -81,53 +82,55 @@ public function testExecuteEmptyMigrationPlanCausesException() : void ['interactive' => false] ); - self::assertTrue(strpos($this->migrateCommandTester->getDisplay(true), 'Could not find any migrations to execute') !== false); + self::assertStringContainsString('[ERROR] Unknown version: A', $this->migrateCommandTester->getDisplay(true)); self::assertSame(1, $this->migrateCommandTester->getStatusCode()); } - public function testExecuteAlreadyAtFirstVersion() : void + /** + * @return array> + */ + public function getTargetAliases() : array { - $this->migrateCommandTester->execute( - [ - 'version' => 'first', - '--allow-no-migration' => true, - ], - ['interactive' => false] - ); - - self::assertTrue(strpos($this->migrateCommandTester->getDisplay(true), 'Already at first version.') !== false); - self::assertSame(0, $this->migrateCommandTester->getStatusCode()); + return [ + ['latest', true, 'A'], + ['latest', false, 'A'], + ['first', true, null], + ['first', false, null], + ['next', true, 'A'], + ['next', false, 'A'], + ['current+1', false, 'A'], + ['current+1', true, 'A'], + ]; } - public function testExecuteAlreadyAtLatestVersion() : void + /** + * @dataProvider getTargetAliases + */ + public function testExecuteAtVersion(string $targetAlias, bool $allowNoMigration, ?string $executedMigration) : void { - $result = new ExecutionResult(new Version('A')); - $this->storage->complete($result); + if ($executedMigration !== null) { + $result = new ExecutionResult(new Version($executedMigration)); + $this->storage->complete($result); + } $this->migrateCommandTester->execute( [ - 'version' => 'latest', - '--allow-no-migration' => true, + 'version' => $targetAlias, + '--allow-no-migration' => $allowNoMigration, ], ['interactive' => false] ); - self::assertTrue(strpos($this->migrateCommandTester->getDisplay(true), 'Already at latest version.') !== false); - self::assertSame(0, $this->migrateCommandTester->getStatusCode()); - } - - public function testExecuteTheDeltaCouldNotBeReached() : void - { - $result = new ExecutionResult(new Version('A')); - $this->storage->complete($result); - - $this->migrateCommandTester->execute( - ['version' => 'current+1'], - ['interactive' => false] + self::assertStringContainsString( + trim($this->migrateCommandTester->getDisplay(true)), + sprintf( + '[%s] The version "%s" couldn\'t be reached, you are at version "%s"', + ($allowNoMigration ? 'WARNING' : 'ERROR'), + $targetAlias, + ($executedMigration ?? '0') + ) ); - - self::assertTrue(strpos($this->migrateCommandTester->getDisplay(true), 'The delta couldn\'t be reached.') !== false); - self::assertSame(1, $this->migrateCommandTester->getStatusCode()); + self::assertSame($allowNoMigration ? 0 : 1, $this->migrateCommandTester->getStatusCode()); } public function testExecuteUnknownVersion() : void From 6005969ed0c65962827778439c4209e1a9987e91 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Fri, 22 May 2020 09:53:18 +0200 Subject: [PATCH 2/3] Display latest version on the status command --- .../Tools/Console/Helper/MigrationStatusInfosHelper.php | 4 ---- .../Tests/Tools/Console/Command/StatusCommandTest.php | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/Doctrine/Migrations/Tools/Console/Helper/MigrationStatusInfosHelper.php b/lib/Doctrine/Migrations/Tools/Console/Helper/MigrationStatusInfosHelper.php index 534f8be3aa..6e4ca14393 100644 --- a/lib/Doctrine/Migrations/Tools/Console/Helper/MigrationStatusInfosHelper.php +++ b/lib/Doctrine/Migrations/Tools/Console/Helper/MigrationStatusInfosHelper.php @@ -216,10 +216,6 @@ private function getFormattedVersionAlias(string $alias, ExecutedMigrationsList } } - if ($alias === 'latest' && $version!== null && $executedMigrations->hasMigration($version)) { - return 'Already at latest version'; - } - // Before first version "virtual" version number if ((string) $version === '0') { return '0'; diff --git a/tests/Doctrine/Migrations/Tests/Tools/Console/Command/StatusCommandTest.php b/tests/Doctrine/Migrations/Tests/Tools/Console/Command/StatusCommandTest.php index 66dccd99da..cf424d0dc8 100644 --- a/tests/Doctrine/Migrations/Tests/Tools/Console/Command/StatusCommandTest.php +++ b/tests/Doctrine/Migrations/Tests/Tools/Console/Command/StatusCommandTest.php @@ -87,7 +87,7 @@ public function testExecute() : void '| Versions | Previous | 1230 |', '| | Current | 1233 |', '| | Next | Already at latest version |', - '| | Latest | |', + '| | Latest | 1233 |', '|----------------------------------------------------------------------------------------------------------------------|', '| Migrations | Executed | 2 |', '| | Executed Unavailable | 2 |', From 5712f4c3032d740010a0b40f84b742c3c5e4cfb1 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Sat, 23 May 2020 07:25:43 +0200 Subject: [PATCH 3/3] alias resolver is able to resolve the migration '0' --- lib/Doctrine/Migrations/Version/DefaultAliasResolver.php | 1 + tests/Doctrine/Migrations/Tests/Version/AliasResolverTest.php | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/Doctrine/Migrations/Version/DefaultAliasResolver.php b/lib/Doctrine/Migrations/Version/DefaultAliasResolver.php index b02a97ac1e..820f7a9014 100644 --- a/lib/Doctrine/Migrations/Version/DefaultAliasResolver.php +++ b/lib/Doctrine/Migrations/Version/DefaultAliasResolver.php @@ -66,6 +66,7 @@ public function resolveVersionAlias(string $alias) : Version switch ($alias) { case self::ALIAS_FIRST: + case '0': return new Version('0'); case self::ALIAS_CURRENT: try { diff --git a/tests/Doctrine/Migrations/Tests/Version/AliasResolverTest.php b/tests/Doctrine/Migrations/Tests/Version/AliasResolverTest.php index 67699c66e0..9c2d43019f 100644 --- a/tests/Doctrine/Migrations/Tests/Version/AliasResolverTest.php +++ b/tests/Doctrine/Migrations/Tests/Version/AliasResolverTest.php @@ -132,6 +132,7 @@ public function getAliases() : array ['current-1', 'A'], ['current+1', 'C'], ['B', 'B'], + ['0', '0'], ['X', null, UnknownMigrationVersion::class], ]; }