From 7fa7aa9be6b21c8c6b8553e9bf6b0e93dc64fc46 Mon Sep 17 00:00:00 2001 From: Yves P Date: Fri, 16 Oct 2015 19:46:56 +0200 Subject: [PATCH 1/3] Use the correct option mode for the commands options Options mode should use a InputOption constant. Flags (bool values) use a `InputOption::VALUE_NONE` mode and options which are expecting a value should use `InputOption;;VALUE_REQUIRED` --- src/Command/Create.php | 6 ++--- src/Command/MarkMigrated.php | 25 +++++++++++---------- src/Command/Migrate.php | 11 ++++----- src/Command/Rollback.php | 11 ++++----- src/Command/Status.php | 9 ++++---- tests/TestCase/Command/MarkMigratedTest.php | 20 ++++++++--------- 6 files changed, 43 insertions(+), 39 deletions(-) diff --git a/src/Command/Create.php b/src/Command/Create.php index c9102d2c..50d349d2 100644 --- a/src/Command/Create.php +++ b/src/Command/Create.php @@ -34,9 +34,9 @@ protected function configure() PHP_EOL, PHP_EOL )); - $this->addOption('plugin', 'p', InputArgument::OPTIONAL, 'The plugin the file should be created for') - ->addOption('connection', 'c', InputArgument::OPTIONAL, 'The datasource connection to use') - ->addOption('source', 's', InputArgument::OPTIONAL, 'The folder where migrations are in') + $this->addOption('plugin', 'p', InputOption::VALUE_REQUIRED, 'The plugin the file should be created for') + ->addOption('connection', 'c', InputOption::VALUE_REQUIRED, 'The datasource connection to use') + ->addOption('source', 's', InputOption::VALUE_REQUIRED, 'The folder where migrations are in') ->addOption('template', 't', InputOption::VALUE_REQUIRED, 'Use an alternative template') ->addOption( 'class', diff --git a/src/Command/MarkMigrated.php b/src/Command/MarkMigrated.php index 246e9565..ee97bba2 100644 --- a/src/Command/MarkMigrated.php +++ b/src/Command/MarkMigrated.php @@ -11,6 +11,7 @@ */ namespace Migrations\Command; +use InvalidArgumentException; use Migrations\ConfigurationTrait; use Phinx\Console\Command\AbstractCommand; use Symfony\Component\Console\Input\InputArgument; @@ -52,32 +53,32 @@ protected function configure() ->addArgument( 'version', InputArgument::OPTIONAL, - 'DEPRECATED: use `bin/cake migrations mark_migrated --target=VERSION` instead' + 'DEPRECATED: use `bin/cake migrations mark_migrated --target=VERSION --only` instead' ) ->setHelp(sprintf( '%sMark migrations as migrated%s', PHP_EOL, PHP_EOL )); - $this->addOption('plugin', 'p', InputArgument::OPTIONAL, 'The plugin the file should be created for') - ->addOption('connection', 'c', InputArgument::OPTIONAL, 'The datasource connection to use') - ->addOption('source', 's', InputArgument::OPTIONAL, 'The folder where migrations are in') + $this->addOption('plugin', 'p', InputOption::VALUE_REQUIRED, 'The plugin the file should be created for') + ->addOption('connection', 'c', InputOption::VALUE_REQUIRED, 'The datasource connection to use') + ->addOption('source', 's', InputOption::VALUE_REQUIRED, 'The folder where migrations are in') ->addOption( 'target', 't', - InputArgument::OPTIONAL, + InputOption::VALUE_REQUIRED, 'It will mark migrations from beginning to the given version' ) ->addOption( 'exclude', 'x', - InputArgument::OPTIONAL, + InputOption::VALUE_NONE, 'If present it will mark migrations from beginning until the given version, excluding it' ) ->addOption( 'only', 'o', - InputArgument::OPTIONAL, + InputOption::VALUE_NONE, 'If present it will only mark the given migration version' ); } @@ -122,7 +123,7 @@ protected function execute(InputInterface $input, OutputInterface $output) try { $versions = $this->getVersionsToMark($input); - } catch (\InvalidArgumentException $e) { + } catch (InvalidArgumentException $e) { $output->writeln(sprintf("%s", $e->getMessage())); return; } @@ -181,7 +182,7 @@ protected function markVersionsAsMigrated($path, $versions) * Decides which versions it should mark as migrated * * @return array Array of versions that should be marked as migrated - * @throws InvalidArgumentException If the `--exclude` or `--only` options are used without `--target` + * @throws \InvalidArgumentException If the `--exclude` or `--only` options are used without `--target` * or version not found */ protected function getVersionsToMark() @@ -197,7 +198,7 @@ protected function getVersionsToMark() if ($this->isOnly()) { if (!in_array($version, $versions)) { - throw new \InvalidArgumentException("Migration `$version` was not found !"); + throw new InvalidArgumentException("Migration `$version` was not found !"); } return [$version]; @@ -267,7 +268,7 @@ protected function isOnly() */ protected function hasExclude() { - return $this->input->getOption('exclude') !== null; + return $this->input->getOption('exclude'); } /** @@ -277,7 +278,7 @@ protected function hasExclude() */ protected function hasOnly() { - return $this->input->getOption('only') !== null; + return $this->input->getOption('only'); } /** diff --git a/src/Command/Migrate.php b/src/Command/Migrate.php index 5303d430..48e3f726 100644 --- a/src/Command/Migrate.php +++ b/src/Command/Migrate.php @@ -16,6 +16,7 @@ use Phinx\Console\Command\Migrate as MigrateCommand; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; class Migrate extends MigrateCommand @@ -34,11 +35,11 @@ protected function configure() $this->setName('migrate') ->setDescription('Migrate the database') ->setHelp('runs all available migrations, optionally up to a specific version') - ->addOption('--target', '-t', InputArgument::OPTIONAL, 'The version number to migrate to') - ->addOption('--date', '-d', InputArgument::OPTIONAL, 'The date to migrate to') - ->addOption('--plugin', '-p', InputArgument::OPTIONAL, 'The plugin containing the migrations') - ->addOption('--connection', '-c', InputArgument::OPTIONAL, 'The datasource connection to use') - ->addOption('--source', '-s', InputArgument::OPTIONAL, 'The folder where migrations are in'); + ->addOption('--target', '-t', InputOption::VALUE_REQUIRED, 'The version number to migrate to') + ->addOption('--date', '-d', InputOption::VALUE_REQUIRED, 'The date to migrate to') + ->addOption('--plugin', '-p', InputOption::VALUE_REQUIRED, 'The plugin containing the migrations') + ->addOption('--connection', '-c', InputOption::VALUE_REQUIRED, 'The datasource connection to use') + ->addOption('--source', '-s', InputOption::VALUE_REQUIRED, 'The folder where migrations are in'); } /** diff --git a/src/Command/Rollback.php b/src/Command/Rollback.php index 3c10415f..2fde4b5f 100644 --- a/src/Command/Rollback.php +++ b/src/Command/Rollback.php @@ -16,6 +16,7 @@ use Phinx\Console\Command\Rollback as RollbackCommand; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; class Rollback extends RollbackCommand @@ -34,11 +35,11 @@ protected function configure() $this->setName('rollback') ->setDescription('Rollback the last or to a specific migration') ->setHelp('reverts the last migration, or optionally up to a specific version') - ->addOption('--target', '-t', InputArgument::OPTIONAL, 'The version number to rollback to') - ->addOption('--date', '-d', InputArgument::OPTIONAL, 'The date to migrate to') - ->addOption('--plugin', '-p', InputArgument::OPTIONAL, 'The plugin containing the migrations') - ->addOption('--connection', '-c', InputArgument::OPTIONAL, 'The datasource connection to use') - ->addOption('--source', '-s', InputArgument::OPTIONAL, 'The folder where migrations are in'); + ->addOption('--target', '-t', InputOption::VALUE_REQUIRED, 'The version number to rollback to') + ->addOption('--date', '-d', InputOption::VALUE_REQUIRED, 'The date to migrate to') + ->addOption('--plugin', '-p', InputOption::VALUE_REQUIRED, 'The plugin containing the migrations') + ->addOption('--connection', '-c', InputOption::VALUE_REQUIRED, 'The datasource connection to use') + ->addOption('--source', '-s', InputOption::VALUE_REQUIRED, 'The folder where migrations are in'); } /** diff --git a/src/Command/Status.php b/src/Command/Status.php index 76d8bf09..19ef5c3b 100644 --- a/src/Command/Status.php +++ b/src/Command/Status.php @@ -15,6 +15,7 @@ use Phinx\Console\Command\Status as StatusCommand; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; class Status extends StatusCommand @@ -29,11 +30,11 @@ protected function configure() { $this->setName('status') ->setDescription('Show migration status') - ->addOption('--format', '-f', InputArgument::OPTIONAL, 'The output format: text or json. Defaults to text.') + ->addOption('--format', '-f', InputOption::VALUE_REQUIRED, 'The output format: text or json. Defaults to text.') ->setHelp('prints a list of all migrations, along with their current status') - ->addOption('--plugin', '-p', InputArgument::OPTIONAL, 'The plugin containing the migrations') - ->addOption('--connection', '-c', InputArgument::OPTIONAL, 'The datasource connection to use') - ->addOption('--source', '-s', InputArgument::OPTIONAL, 'The folder where migrations are in'); + ->addOption('--plugin', '-p', InputOption::VALUE_REQUIRED, 'The plugin containing the migrations') + ->addOption('--connection', '-c', InputOption::VALUE_REQUIRED, 'The datasource connection to use') + ->addOption('--source', '-s', InputOption::VALUE_REQUIRED, 'The folder where migrations are in'); } /** diff --git a/tests/TestCase/Command/MarkMigratedTest.php b/tests/TestCase/Command/MarkMigratedTest.php index ad5eb642..4333d211 100644 --- a/tests/TestCase/Command/MarkMigratedTest.php +++ b/tests/TestCase/Command/MarkMigratedTest.php @@ -293,7 +293,7 @@ public function testExecuteTargetWithExclude() $this->commandTester->execute([ 'command' => $this->command->getName(), '--target' => '20150724233100', - '--exclude' => '', + '--exclude' => true, '--connection' => 'test', '--source' => 'TestsMigrations' ]); @@ -309,7 +309,7 @@ public function testExecuteTargetWithExclude() $this->commandTester->execute([ 'command' => $this->command->getName(), '--target' => '20150826191400', - '--exclude' => '', + '--exclude' => true, '--connection' => 'test', '--source' => 'TestsMigrations' ]); @@ -332,7 +332,7 @@ public function testExecuteTargetWithExclude() $this->commandTester->execute([ 'command' => $this->command->getName(), '--target' => '20150704160610', - '--exclude' => '', + '--exclude' => true, '--connection' => 'test', '--source' => 'TestsMigrations' ]); @@ -348,7 +348,7 @@ public function testExecuteTargetWithOnly() $this->commandTester->execute([ 'command' => $this->command->getName(), '--target' => '20150724233100', - '--only' => '', + '--only' => true, '--connection' => 'test', '--source' => 'TestsMigrations' ]); @@ -364,7 +364,7 @@ public function testExecuteTargetWithOnly() $this->commandTester->execute([ 'command' => $this->command->getName(), '--target' => '20150826191400', - '--only' => '', + '--only' => true, '--connection' => 'test', '--source' => 'TestsMigrations' ]); @@ -383,7 +383,7 @@ public function testExecuteTargetWithOnly() $this->commandTester->execute([ 'command' => $this->command->getName(), '--target' => '20150704160610', - '--only' => '', + '--only' => true, '--connection' => 'test', '--source' => 'TestsMigrations' ]); @@ -422,7 +422,7 @@ public function testExecuteInvalidUseOfOnlyAndExclude() { $this->commandTester->execute([ 'command' => $this->command->getName(), - '--exclude' => '', + '--exclude' => true, '--connection' => 'test', '--source' => 'TestsMigrations' ]); @@ -436,7 +436,7 @@ public function testExecuteInvalidUseOfOnlyAndExclude() $this->commandTester->execute([ 'command' => $this->command->getName(), - '--only' => '', + '--only' => true, '--connection' => 'test', '--source' => 'TestsMigrations' ]); @@ -451,8 +451,8 @@ public function testExecuteInvalidUseOfOnlyAndExclude() $this->commandTester->execute([ 'command' => $this->command->getName(), '--target' => '20150724233100', - '--only' => '', - '--exclude' => '', + '--only' => true, + '--exclude' => true, '--connection' => 'test', '--source' => 'TestsMigrations' ]); From 49e6d2db5f399f5076d95ee040b9de6d153118e1 Mon Sep 17 00:00:00 2001 From: Yves P Date: Fri, 16 Oct 2015 20:24:54 +0200 Subject: [PATCH 2/3] Add the new options to the MigrationsShell and update the dispatch command when a snapshot is baked --- src/Command/MarkMigrated.php | 4 ++-- src/Shell/MigrationsShell.php | 4 +++- src/Shell/Task/MigrationSnapshotTask.php | 2 +- tests/TestCase/Shell/Task/MigrationSnapshotTaskTest.php | 4 ++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Command/MarkMigrated.php b/src/Command/MarkMigrated.php index ee97bba2..3e17f04f 100644 --- a/src/Command/MarkMigrated.php +++ b/src/Command/MarkMigrated.php @@ -59,8 +59,8 @@ protected function configure() '%sMark migrations as migrated%s', PHP_EOL, PHP_EOL - )); - $this->addOption('plugin', 'p', InputOption::VALUE_REQUIRED, 'The plugin the file should be created for') + )) + ->addOption('plugin', 'p', InputOption::VALUE_REQUIRED, 'The plugin the file should be created for') ->addOption('connection', 'c', InputOption::VALUE_REQUIRED, 'The datasource connection to use') ->addOption('source', 's', InputOption::VALUE_REQUIRED, 'The folder where migrations are in') ->addOption( diff --git a/src/Shell/MigrationsShell.php b/src/Shell/MigrationsShell.php index 6e151b4f..295d53ee 100644 --- a/src/Shell/MigrationsShell.php +++ b/src/Shell/MigrationsShell.php @@ -49,7 +49,9 @@ public function getOptionParser() ->addOption('version', ['short' => 'V']) ->addOption('no-interaction', ['short' => 'n']) ->addOption('template', ['short' => 't']) - ->addOption('format', ['short' => 'f']); + ->addOption('format', ['short' => 'f']) + ->addOption('only', ['short' => 'o']) + ->addOption('exclude', ['short' => 'x']); } /** diff --git a/src/Shell/Task/MigrationSnapshotTask.php b/src/Shell/Task/MigrationSnapshotTask.php index 07b34cae..7bbe6251 100644 --- a/src/Shell/Task/MigrationSnapshotTask.php +++ b/src/Shell/Task/MigrationSnapshotTask.php @@ -85,7 +85,7 @@ protected function markSnapshotApplied($path) list($version, ) = explode('_', $fileName, 2); - $dispatchCommand = 'migrations mark_migrated ' . $version; + $dispatchCommand = 'migrations mark_migrated -t ' . $version . ' -o'; if (!empty($this->params['connection'])) { $dispatchCommand .= ' -c ' . $this->params['connection']; } diff --git a/tests/TestCase/Shell/Task/MigrationSnapshotTaskTest.php b/tests/TestCase/Shell/Task/MigrationSnapshotTaskTest.php index 83ef184a..b65c5d0d 100644 --- a/tests/TestCase/Shell/Task/MigrationSnapshotTaskTest.php +++ b/tests/TestCase/Shell/Task/MigrationSnapshotTaskTest.php @@ -81,8 +81,8 @@ public function testNotEmptySnapshot() ->method('dispatchShell') ->with( $this->logicalAnd( - $this->stringContains('migrations mark_migrated'), - $this->stringContains('-c test -p BogusPlugin') + $this->stringContains('migrations mark_migrated -t'), + $this->stringContains('-o -c test -p BogusPlugin') ) ); From befbda17948e133e98bbcdc3dc80d77b8f22b367 Mon Sep 17 00:00:00 2001 From: Yves P Date: Fri, 16 Oct 2015 20:29:01 +0200 Subject: [PATCH 3/3] Fix CS --- src/Command/Status.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Command/Status.php b/src/Command/Status.php index 19ef5c3b..ee1c3346 100644 --- a/src/Command/Status.php +++ b/src/Command/Status.php @@ -30,7 +30,12 @@ protected function configure() { $this->setName('status') ->setDescription('Show migration status') - ->addOption('--format', '-f', InputOption::VALUE_REQUIRED, 'The output format: text or json. Defaults to text.') + ->addOption( + '--format', + '-f', + InputOption::VALUE_REQUIRED, + 'The output format: text or json. Defaults to text.' + ) ->setHelp('prints a list of all migrations, along with their current status') ->addOption('--plugin', '-p', InputOption::VALUE_REQUIRED, 'The plugin containing the migrations') ->addOption('--connection', '-c', InputOption::VALUE_REQUIRED, 'The datasource connection to use')