From c34d5a35e2092782a7ece7cf113682aa69362f1d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 17 Nov 2019 09:52:45 +0100 Subject: [PATCH 1/3] Updated database commands so that internal commands are run with mustRun --- .../Command/Db/AbstractDatabaseCommand.php | 8 ++++- .../src/Command/Db/CreateDatabaseCommand.php | 14 +++----- .../src/Command/Db/MigrateDatabaseCommand.php | 12 ++----- .../Command/Db/CreateDatabaseCommandTest.php | 6 ++-- .../Command/Db/MigrateDatabaseCommandTest.php | 34 +++++-------------- phpstan.neon | 1 + 6 files changed, 27 insertions(+), 48 deletions(-) diff --git a/module/CLI/src/Command/Db/AbstractDatabaseCommand.php b/module/CLI/src/Command/Db/AbstractDatabaseCommand.php index dade4d95..3ab12b3b 100644 --- a/module/CLI/src/Command/Db/AbstractDatabaseCommand.php +++ b/module/CLI/src/Command/Db/AbstractDatabaseCommand.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Db; use Shlinkio\Shlink\CLI\Command\Util\AbstractLockedCommand; +use Shlinkio\Shlink\CLI\Command\Util\LockedCommandConfig; use Symfony\Component\Console\Helper\ProcessHelper; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Lock\Factory as Locker; @@ -29,6 +30,11 @@ abstract class AbstractDatabaseCommand extends AbstractLockedCommand protected function runPhpCommand(OutputInterface $output, array $command): void { array_unshift($command, $this->phpBinary); - $this->processHelper->run($output, $command, null, null, $output->getVerbosity()); + $this->processHelper->mustRun($output, $command); + } + + protected function getLockConfig(): LockedCommandConfig + { + return new LockedCommandConfig($this->getName(), true); } } diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php index 9e349e31..36bb9de4 100644 --- a/module/CLI/src/Command/Db/CreateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/CreateDatabaseCommand.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Db; use Doctrine\DBAL\Connection; -use Shlinkio\Shlink\CLI\Command\Util\LockedCommandConfig; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Symfony\Component\Console\Helper\ProcessHelper; use Symfony\Component\Console\Input\InputInterface; @@ -19,8 +18,8 @@ use function Functional\contains; class CreateDatabaseCommand extends AbstractDatabaseCommand { public const NAME = 'db:create'; - public const DOCTRINE_HELPER_SCRIPT = 'vendor/doctrine/orm/bin/doctrine.php'; - public const DOCTRINE_HELPER_COMMAND = 'orm:schema-tool:create'; + public const DOCTRINE_SCRIPT = 'vendor/doctrine/orm/bin/doctrine.php'; + public const DOCTRINE_CREATE_SCHEMA_COMMAND = 'orm:schema-tool:create'; /** @var Connection */ private $regularConn; @@ -61,7 +60,7 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand // Create database $io->writeln('Creating database tables...'); - $this->runPhpCommand($output, [self::DOCTRINE_HELPER_SCRIPT, self::DOCTRINE_HELPER_COMMAND]); + $this->runPhpCommand($output, [self::DOCTRINE_SCRIPT, self::DOCTRINE_CREATE_SCHEMA_COMMAND]); $io->success('Database properly created!'); return ExitCodes::EXIT_SUCCESS; @@ -87,13 +86,8 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand private function schemaExists(): bool { // If at least one of the shlink tables exist, we will consider the database exists somehow. - // Any inconsistency will be taken care by the migrations + // Any inconsistency should be taken care by the migrations $schemaManager = $this->regularConn->getSchemaManager(); return ! empty($schemaManager->listTableNames()); } - - protected function getLockConfig(): LockedCommandConfig - { - return new LockedCommandConfig($this->getName(), true); - } } diff --git a/module/CLI/src/Command/Db/MigrateDatabaseCommand.php b/module/CLI/src/Command/Db/MigrateDatabaseCommand.php index ca964b3b..23b39fc6 100644 --- a/module/CLI/src/Command/Db/MigrateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/MigrateDatabaseCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Db; -use Shlinkio\Shlink\CLI\Command\Util\LockedCommandConfig; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -13,8 +12,8 @@ use Symfony\Component\Console\Style\SymfonyStyle; class MigrateDatabaseCommand extends AbstractDatabaseCommand { public const NAME = 'db:migrate'; - public const DOCTRINE_HELPER_SCRIPT = 'vendor/doctrine/migrations/bin/doctrine-migrations.php'; - public const DOCTRINE_HELPER_COMMAND = 'migrations:migrate'; + public const DOCTRINE_MIGRATIONS_SCRIPT = 'vendor/doctrine/migrations/bin/doctrine-migrations.php'; + public const DOCTRINE_MIGRATE_COMMAND = 'migrations:migrate'; protected function configure(): void { @@ -28,14 +27,9 @@ class MigrateDatabaseCommand extends AbstractDatabaseCommand $io = new SymfonyStyle($input, $output); $io->writeln('Migrating database...'); - $this->runPhpCommand($output, [self::DOCTRINE_HELPER_SCRIPT, self::DOCTRINE_HELPER_COMMAND]); + $this->runPhpCommand($output, [self::DOCTRINE_MIGRATIONS_SCRIPT, self::DOCTRINE_MIGRATE_COMMAND]); $io->success('Database properly migrated!'); return ExitCodes::EXIT_SUCCESS; } - - protected function getLockConfig(): LockedCommandConfig - { - return new LockedCommandConfig($this->getName(), true); - } } diff --git a/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php b/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php index a61b2ec0..89322544 100644 --- a/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php +++ b/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php @@ -116,10 +116,10 @@ class CreateDatabaseCommandTest extends TestCase $createDatabase = $this->schemaManager->createDatabase($shlinkDatabase)->will(function () { }); $listTables = $this->schemaManager->listTableNames()->willReturn([]); - $runCommand = $this->processHelper->run(Argument::type(OutputInterface::class), [ + $runCommand = $this->processHelper->mustRun(Argument::type(OutputInterface::class), [ '/usr/local/bin/php', - CreateDatabaseCommand::DOCTRINE_HELPER_SCRIPT, - CreateDatabaseCommand::DOCTRINE_HELPER_COMMAND, + CreateDatabaseCommand::DOCTRINE_SCRIPT, + CreateDatabaseCommand::DOCTRINE_CREATE_SCHEMA_COMMAND, ], Argument::cetera()); $this->commandTester->execute([]); diff --git a/module/CLI/test/Command/Db/MigrateDatabaseCommandTest.php b/module/CLI/test/Command/Db/MigrateDatabaseCommandTest.php index 4e099e5e..1e7690ae 100644 --- a/module/CLI/test/Command/Db/MigrateDatabaseCommandTest.php +++ b/module/CLI/test/Command/Db/MigrateDatabaseCommandTest.php @@ -48,36 +48,20 @@ class MigrateDatabaseCommandTest extends TestCase $this->commandTester = new CommandTester($command); } - /** - * @test - * @dataProvider provideVerbosities - */ - public function migrationsCommandIsRunWithProperVerbosity(int $verbosity): void + /** @test */ + public function migrationsCommandIsRunWithProperVerbosity(): void { - $runCommand = $this->processHelper->run(Argument::type(OutputInterface::class), [ + $runCommand = $this->processHelper->mustRun(Argument::type(OutputInterface::class), [ '/usr/local/bin/php', - MigrateDatabaseCommand::DOCTRINE_HELPER_SCRIPT, - MigrateDatabaseCommand::DOCTRINE_HELPER_COMMAND, - ], null, null, $verbosity); + MigrateDatabaseCommand::DOCTRINE_MIGRATIONS_SCRIPT, + MigrateDatabaseCommand::DOCTRINE_MIGRATE_COMMAND, + ], Argument::cetera()); - $this->commandTester->execute([], [ - 'verbosity' => $verbosity, - ]); + $this->commandTester->execute([]); $output = $this->commandTester->getDisplay(); - if ($verbosity >= OutputInterface::VERBOSITY_VERBOSE) { - $this->assertStringContainsString('Migrating database...', $output); - $this->assertStringContainsString('Database properly migrated!', $output); - } + $this->assertStringContainsString('Migrating database...', $output); + $this->assertStringContainsString('Database properly migrated!', $output); $runCommand->shouldHaveBeenCalledOnce(); } - - public function provideVerbosities(): iterable - { - yield 'debug' => [OutputInterface::VERBOSITY_DEBUG]; - yield 'normal' => [OutputInterface::VERBOSITY_NORMAL]; - yield 'quiet' => [OutputInterface::VERBOSITY_QUIET]; - yield 'verbose' => [OutputInterface::VERBOSITY_VERBOSE]; - yield 'very verbose' => [OutputInterface::VERBOSITY_VERY_VERBOSE]; - } } diff --git a/phpstan.neon b/phpstan.neon index ae2723cb..8b5db673 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4,3 +4,4 @@ parameters: - '#ObjectManager::flush()#' - '#Undefined variable: \$metadata#' - '#AbstractQuery::setParameters()#' + - '#mustRun()#' From 4e3b5419d555642afe1e1e47546e37c6940aa23c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 17 Nov 2019 10:00:29 +0100 Subject: [PATCH 2/3] Created small helper composer command --- composer.json | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/composer.json b/composer.json index 4a1654ed..34787bd9 100644 --- a/composer.json +++ b/composer.json @@ -102,11 +102,9 @@ "@test:ci", "@infect:ci" ], - "cs": "phpcs", "cs:fix": "phpcbf", "stan": "phpstan analyse module/*/src/ module/*/config config docker/config --level=5 -c phpstan.neon", - "test": [ "@test:unit", "@test:db", @@ -135,20 +133,19 @@ "test:db:maria": "DB_DRIVER=maria composer test:db:sqlite", "test:db:postgres": "DB_DRIVER=postgres composer test:db:sqlite", "test:api": "bin/test/run-api-tests.sh", - "test:pretty": [ "@test", "phpdbg -qrr vendor/bin/phpcov merge build --html build/html" ], "test:unit:pretty": "phpdbg -qrr vendor/bin/phpunit --order-by=random --colors=always --coverage-html build/coverage", - "infect": "infection --threads=4 --min-msi=75 --log-verbosity=default --only-covered", "infect:ci": "infection --threads=4 --min-msi=75 --log-verbosity=default --only-covered --coverage=build", "infect:show": "infection --threads=4 --min-msi=75 --log-verbosity=default --only-covered --show-mutations", "infect:test": [ "@test:unit:ci", "@infect:ci" - ] + ], + "clean:dev": "rm -f data/database.sqlite && rm -f config/params/generated_config.php" }, "scripts-descriptions": { "ci": "Alias for \"cs\", \"stan\", \"test:ci\" and \"infect:ci\"", @@ -171,7 +168,8 @@ "infect": "Checks unit tests quality applying mutation testing", "infect:ci": "Checks unit tests quality applying mutation testing with existing reports and logs", "infect:show": "Checks unit tests quality applying mutation testing and shows applied mutators", - "infect:test": "Checks unit tests quality applying mutation testing" + "infect:test": "Checks unit tests quality applying mutation testing", + "clean:dev": "Deletes artifacts which are gitignored and could affect dev env" }, "config": { "sort-packages": true From 372b83d92f05ccd2bb2e89f04e5bbdd731c1eaee Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 17 Nov 2019 10:02:03 +0100 Subject: [PATCH 3/3] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d563441e..5ed6fde4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#512](https://github.com/shlinkio/shlink/issues/512) Fixed query params not being properly forwarded from short URL to long one. * [#540](https://github.com/shlinkio/shlink/issues/540) Fixed errors thrown when creating short URLs if the original URL has an internationalized domain name and URL validation is enabled. +* [#528](https://github.com/shlinkio/shlink/issues/528) Ensured `db:create` and `db:migrate` commands do not silently fail when run as part of `install` or `update`. ## 1.20.0 - 2019-11-02