From 7ed85e8916e205e2d12c3f669b5449d4467948f1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 4 Aug 2019 11:16:46 +0200 Subject: [PATCH 01/12] Moved locking logic for CLI commands to a common abstract class --- composer.json | 42 ++++++++--------- .../Command/Util/AbstractLockedCommand.php | 47 +++++++++++++++++++ .../src/Command/Util/LockedCommandConfig.php | 38 +++++++++++++++ .../src/Command/Visit/LocateVisitsCommand.php | 30 +++++------- module/CLI/src/Factory/ApplicationFactory.php | 23 +-------- .../Command/Visit/LocateVisitsCommandTest.php | 10 ++-- .../test/Factory/ApplicationFactoryTest.php | 12 +---- 7 files changed, 128 insertions(+), 74 deletions(-) create mode 100644 module/CLI/src/Command/Util/AbstractLockedCommand.php create mode 100644 module/CLI/src/Command/Util/LockedCommandConfig.php diff --git a/composer.json b/composer.json index 0d9d5c76..e0a12dee 100644 --- a/composer.json +++ b/composer.json @@ -31,41 +31,41 @@ "monolog/monolog": "^1.21", "ocramius/proxy-manager": "^2.0", "phly/phly-event-dispatcher": "^1.0", - "shlinkio/shlink-installer": "^1.1", - "symfony/console": "^4.2", - "symfony/filesystem": "^4.2", - "symfony/lock": "^4.2", - "symfony/process": "^4.2", + "shlinkio/shlink-installer": "^1.2", + "symfony/console": "^4.3", + "symfony/filesystem": "^4.3", + "symfony/lock": "^4.3", + "symfony/process": "^4.3", "theorchard/monolog-cascade": "^0.4", - "zendframework/zend-config": "^3.0", - "zendframework/zend-config-aggregator": "^1.0", - "zendframework/zend-diactoros": "^2.1.1", - "zendframework/zend-expressive": "^3.0", + "zendframework/zend-config": "^3.3", + "zendframework/zend-config-aggregator": "^1.1", + "zendframework/zend-diactoros": "^2.1.3", + "zendframework/zend-expressive": "^3.2", "zendframework/zend-expressive-fastroute": "^3.0", - "zendframework/zend-expressive-helpers": "^5.0", - "zendframework/zend-expressive-platesrenderer": "^2.0", + "zendframework/zend-expressive-helpers": "^5.3", + "zendframework/zend-expressive-platesrenderer": "^2.1", "zendframework/zend-expressive-swoole": "^2.4", - "zendframework/zend-i18n": "^2.7", - "zendframework/zend-inputfilter": "^2.8", - "zendframework/zend-paginator": "^2.6", - "zendframework/zend-servicemanager": "^3.2", - "zendframework/zend-stdlib": "^3.0" + "zendframework/zend-i18n": "^2.9", + "zendframework/zend-inputfilter": "^2.10", + "zendframework/zend-paginator": "^2.8", + "zendframework/zend-servicemanager": "^3.4", + "zendframework/zend-stdlib": "^3.2" }, "require-dev": { "devster/ubench": "^2.0", "doctrine/data-fixtures": "^1.3", "eaglewu/swoole-ide-helper": "dev-master", - "filp/whoops": "^2.0", + "filp/whoops": "^2.4", "infection/infection": "^0.12.2", "phpstan/phpstan": "^0.11.2", "phpunit/phpcov": "^6.0", - "phpunit/phpunit": "^8.0", + "phpunit/phpunit": "^8.3", "roave/security-advisories": "dev-master", "shlinkio/php-coding-standard": "~1.2.2", - "symfony/dotenv": "^4.2", - "symfony/var-dumper": "^4.2", + "symfony/dotenv": "^4.3", + "symfony/var-dumper": "^4.3", "zendframework/zend-component-installer": "^2.1", - "zendframework/zend-expressive-tooling": "^1.0" + "zendframework/zend-expressive-tooling": "^1.2" }, "autoload": { "psr-4": { diff --git a/module/CLI/src/Command/Util/AbstractLockedCommand.php b/module/CLI/src/Command/Util/AbstractLockedCommand.php new file mode 100644 index 00000000..9de3d3e9 --- /dev/null +++ b/module/CLI/src/Command/Util/AbstractLockedCommand.php @@ -0,0 +1,47 @@ +locker = $locker; + } + + final protected function execute(InputInterface $input, OutputInterface $output): ?int + { + $lockConfig = $this->getLockConfig(); + $lock = $this->locker->createLock($lockConfig->lockName(), $lockConfig->ttl(), $lockConfig->isBlocking()); + + if (! $lock->acquire($lockConfig->isBlocking())) { + $output->writeln( + sprintf('Command "%s" is already in progress. Skipping.', $lockConfig->lockName()) + ); + return ExitCodes::EXIT_WARNING; + } + + try { + return $this->lockedExecute($input, $output); + } finally { + $lock->release(); + } + } + + abstract protected function lockedExecute(InputInterface $input, OutputInterface $output): int; + + abstract protected function getLockConfig(): LockedCommandConfig; +} diff --git a/module/CLI/src/Command/Util/LockedCommandConfig.php b/module/CLI/src/Command/Util/LockedCommandConfig.php new file mode 100644 index 00000000..bd74db8b --- /dev/null +++ b/module/CLI/src/Command/Util/LockedCommandConfig.php @@ -0,0 +1,38 @@ +lockName = $lockName; + $this->isBlocking = $isBlocking; + $this->ttl = $ttl; + } + + public function lockName(): string + { + return $this->lockName; + } + + public function isBlocking(): bool + { + return $this->isBlocking; + } + + public function ttl(): float + { + return $this->ttl; + } +} diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php index ef1f0298..857028c3 100644 --- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -4,6 +4,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; use Exception; +use Shlinkio\Shlink\CLI\Command\Util\AbstractLockedCommand; +use Shlinkio\Shlink\CLI\Command\Util\LockedCommandConfig; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface; @@ -15,16 +17,16 @@ use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException; use Shlinkio\Shlink\Core\Service\VisitServiceInterface; -use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; use Symfony\Component\Lock\Factory as Locker; +use Throwable; use function sprintf; -class LocateVisitsCommand extends Command +class LocateVisitsCommand extends AbstractLockedCommand { public const NAME = 'visit:locate'; public const ALIASES = ['visit:process']; @@ -33,8 +35,6 @@ class LocateVisitsCommand extends Command private $visitService; /** @var IpLocationResolverInterface */ private $ipLocationResolver; - /** @var Locker */ - private $locker; /** @var GeolocationDbUpdaterInterface */ private $dbUpdater; @@ -49,10 +49,9 @@ class LocateVisitsCommand extends Command Locker $locker, GeolocationDbUpdaterInterface $dbUpdater ) { - parent::__construct(); + parent::__construct($locker); $this->visitService = $visitService; $this->ipLocationResolver = $ipLocationResolver; - $this->locker = $locker; $this->dbUpdater = $dbUpdater; } @@ -64,16 +63,10 @@ class LocateVisitsCommand extends Command ->setDescription('Resolves visits origin locations.'); } - protected function execute(InputInterface $input, OutputInterface $output): ?int + protected function lockedExecute(InputInterface $input, OutputInterface $output): int { $this->io = new SymfonyStyle($input, $output); - $lock = $this->locker->createLock(self::NAME); - if (! $lock->acquire()) { - $this->io->warning(sprintf('There is already an instance of the "%s" command in execution', self::NAME)); - return ExitCodes::EXIT_WARNING; - } - try { $this->checkDbUpdate(); @@ -90,15 +83,13 @@ class LocateVisitsCommand extends Command $this->io->success('Finished processing all IPs'); return ExitCodes::EXIT_SUCCESS; - } catch (Exception $e) { + } catch (Throwable $e) { $this->io->error($e->getMessage()); - if ($this->io->isVerbose()) { + if ($e instanceof Exception && $this->io->isVerbose()) { $this->getApplication()->renderException($e, $this->io); } return ExitCodes::EXIT_FAILURE; - } finally { - $lock->release(); } } @@ -160,4 +151,9 @@ class LocateVisitsCommand extends Command ); } } + + protected function getLockConfig(): LockedCommandConfig + { + return new LockedCommandConfig($this->getName()); + } } diff --git a/module/CLI/src/Factory/ApplicationFactory.php b/module/CLI/src/Factory/ApplicationFactory.php index 7acfc815..e866c190 100644 --- a/module/CLI/src/Factory/ApplicationFactory.php +++ b/module/CLI/src/Factory/ApplicationFactory.php @@ -4,32 +4,13 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Factory; use Interop\Container\ContainerInterface; -use Interop\Container\Exception\ContainerException; -use Psr\Container\ContainerExceptionInterface; -use Psr\Container\NotFoundExceptionInterface; use Shlinkio\Shlink\Core\Options\AppOptions; use Symfony\Component\Console\Application as CliApp; use Symfony\Component\Console\CommandLoader\ContainerCommandLoader; -use Zend\ServiceManager\Exception\ServiceNotCreatedException; -use Zend\ServiceManager\Exception\ServiceNotFoundException; -use Zend\ServiceManager\Factory\FactoryInterface; -class ApplicationFactory implements FactoryInterface +class ApplicationFactory { - /** - * Create an object - * - * @param ContainerInterface $container - * @param string $requestedName - * @param null|array $options - * @return CliApp - * @throws NotFoundExceptionInterface - * @throws ContainerExceptionInterface - * @throws ServiceNotFoundException if unable to resolve the service. - * @throws ServiceNotCreatedException if an exception is raised when creating a service. - * @throws ContainerException if any other error occurs - */ - public function __invoke(ContainerInterface $container, $requestedName, ?array $options = null): CliApp + public function __invoke(ContainerInterface $container): CliApp { $config = $container->get('config')['cli']; $appOptions = $container->get(AppOptions::class); diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php index a09d90a8..e0bace38 100644 --- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php @@ -49,10 +49,10 @@ class LocateVisitsCommandTest extends TestCase $this->locker = $this->prophesize(Lock\Factory::class); $this->lock = $this->prophesize(Lock\LockInterface::class); - $this->lock->acquire()->willReturn(true); + $this->lock->acquire(false)->willReturn(true); $this->lock->release()->will(function () { }); - $this->locker->createLock(Argument::type('string'))->willReturn($this->lock->reveal()); + $this->locker->createLock(Argument::type('string'), 90.0, false)->willReturn($this->lock->reveal()); $command = new LocateVisitsCommand( $this->visitService->reveal(), @@ -162,9 +162,9 @@ class LocateVisitsCommandTest extends TestCase } /** @test */ - public function noActionIsPerformedIfLockIsAcquired() + public function noActionIsPerformedIfLockIsAcquired(): void { - $this->lock->acquire()->willReturn(false); + $this->lock->acquire(false)->willReturn(false); $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(function () { }); @@ -174,7 +174,7 @@ class LocateVisitsCommandTest extends TestCase $output = $this->commandTester->getDisplay(); $this->assertStringContainsString( - sprintf('There is already an instance of the "%s" command', LocateVisitsCommand::NAME), + sprintf('Command "%s" is already in progress. Skipping.', LocateVisitsCommand::NAME), $output ); $locateVisits->shouldNotHaveBeenCalled(); diff --git a/module/CLI/test/Factory/ApplicationFactoryTest.php b/module/CLI/test/Factory/ApplicationFactoryTest.php index 8e45674c..285d41c6 100644 --- a/module/CLI/test/Factory/ApplicationFactoryTest.php +++ b/module/CLI/test/Factory/ApplicationFactoryTest.php @@ -25,14 +25,7 @@ class ApplicationFactoryTest extends TestCase } /** @test */ - public function serviceIsCreated() - { - $instance = ($this->factory)($this->createServiceManager(), ''); - $this->assertInstanceOf(Application::class, $instance); - } - - /** @test */ - public function allCommandsWhichAreServicesAreAdded() + public function allCommandsWhichAreServicesAreAdded(): void { $sm = $this->createServiceManager([ 'commands' => [ @@ -45,8 +38,7 @@ class ApplicationFactoryTest extends TestCase $sm->setService('bar', $this->createCommandMock('bar')->reveal()); /** @var Application $instance */ - $instance = ($this->factory)($sm, ''); - $this->assertInstanceOf(Application::class, $instance); + $instance = ($this->factory)($sm); $this->assertTrue($instance->has('foo')); $this->assertTrue($instance->has('bar')); From 7fa1f1c63c9fbac053e714da84d55313ecc331b1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 4 Aug 2019 11:30:35 +0200 Subject: [PATCH 02/12] Created empoty locked command to create shlink database --- module/CLI/config/cli.config.php | 2 + module/CLI/config/dependencies.config.php | 21 +++++-- .../src/Command/Db/CreateDatabaseCommand.php | 61 +++++++++++++++++++ .../CLI/src/Factory/ProcessHelperFactory.php | 20 ++++++ .../test/Factory/ProcessHelperFactoryTest.php | 29 +++++++++ 5 files changed, 128 insertions(+), 5 deletions(-) create mode 100644 module/CLI/src/Command/Db/CreateDatabaseCommand.php create mode 100644 module/CLI/src/Factory/ProcessHelperFactory.php create mode 100644 module/CLI/test/Factory/ProcessHelperFactoryTest.php diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 2edbae99..512090c2 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -28,6 +28,8 @@ return [ Command\Tag\CreateTagCommand::NAME => Command\Tag\CreateTagCommand::class, Command\Tag\RenameTagCommand::NAME => Command\Tag\RenameTagCommand::class, Command\Tag\DeleteTagsCommand::NAME => Command\Tag\DeleteTagsCommand::class, + + Command\Db\CreateDatabaseCommand::NAME => Command\Db\CreateDatabaseCommand::class, ], ], diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 0a90c1ff..b71598a9 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -10,8 +10,9 @@ use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface; use Shlinkio\Shlink\Common\Service\PreviewGenerator; use Shlinkio\Shlink\Core\Service; use Shlinkio\Shlink\Rest\Service\ApiKeyService; -use Symfony\Component\Console\Application; -use Symfony\Component\Lock; +use Symfony\Component\Console as SymfonyCli; +use Symfony\Component\Lock\Factory as Locker; +use Symfony\Component\Process\PhpExecutableFinder; use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory; use Zend\ServiceManager\Factory\InvokableFactory; @@ -19,7 +20,9 @@ return [ 'dependencies' => [ 'factories' => [ - Application::class => Factory\ApplicationFactory::class, + SymfonyCli\Application::class => Factory\ApplicationFactory::class, + SymfonyCli\Helper\ProcessHelper::class => Factory\ProcessHelperFactory::class, + PhpExecutableFinder::class => InvokableFactory::class, GeolocationDbUpdater::class => ConfigAbstractFactory::class, @@ -44,11 +47,13 @@ return [ Command\Tag\CreateTagCommand::class => ConfigAbstractFactory::class, Command\Tag\RenameTagCommand::class => ConfigAbstractFactory::class, Command\Tag\DeleteTagsCommand::class => ConfigAbstractFactory::class, + + Command\Db\CreateDatabaseCommand::class => ConfigAbstractFactory::class, ], ], ConfigAbstractFactory::class => [ - GeolocationDbUpdater::class => [DbUpdater::class, Reader::class, Lock\Factory::class], + GeolocationDbUpdater::class => [DbUpdater::class, Reader::class, Locker::class], Command\ShortUrl\GenerateShortUrlCommand::class => [Service\UrlShortener::class, 'config.url_shortener.domain'], Command\ShortUrl\ResolveUrlCommand::class => [Service\UrlShortener::class], @@ -60,7 +65,7 @@ return [ Command\Visit\LocateVisitsCommand::class => [ Service\VisitService::class, IpLocationResolverInterface::class, - Lock\Factory::class, + Locker::class, GeolocationDbUpdater::class, ], Command\Visit\UpdateDbCommand::class => [DbUpdater::class], @@ -73,6 +78,12 @@ return [ Command\Tag\CreateTagCommand::class => [Service\Tag\TagService::class], Command\Tag\RenameTagCommand::class => [Service\Tag\TagService::class], Command\Tag\DeleteTagsCommand::class => [Service\Tag\TagService::class], + + Command\Db\CreateDatabaseCommand::class => [ + Locker::class, + SymfonyCli\Helper\ProcessHelper::class, + PhpExecutableFinder::class, + ], ], ]; diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php new file mode 100644 index 00000000..ab334c40 --- /dev/null +++ b/module/CLI/src/Command/Db/CreateDatabaseCommand.php @@ -0,0 +1,61 @@ +processHelper = $processHelper; + $this->phpBinary = $phpFinder->find(false) ?: 'php'; + } + + protected function configure(): void + { + $this + ->setName(self::NAME) + ->setDescription( + 'Creates the database needed for shlink to work. It will do nothing if the database already exists' + ); + } + + protected function lockedExecute(InputInterface $input, OutputInterface $output): int + { + $this->checkDbExists(); + + $command = [$this->phpBinary, self::DOCTRINE_HELPER_SCRIPT, self::DOCTRINE_HELPER_COMMAND]; + $this->processHelper->run($output, $command); + + return ExitCodes::EXIT_SUCCESS; + } + + private function checkDbExists(): void + { + // TODO + } + + protected function getLockConfig(): LockedCommandConfig + { + return new LockedCommandConfig($this->getName(), true); + } +} diff --git a/module/CLI/src/Factory/ProcessHelperFactory.php b/module/CLI/src/Factory/ProcessHelperFactory.php new file mode 100644 index 00000000..005d513d --- /dev/null +++ b/module/CLI/src/Factory/ProcessHelperFactory.php @@ -0,0 +1,20 @@ +setHelperSet(new Helper\HelperSet([ + new Helper\FormatterHelper(), + new Helper\DebugFormatterHelper(), + ])); + + return $processHelper; + } +} diff --git a/module/CLI/test/Factory/ProcessHelperFactoryTest.php b/module/CLI/test/Factory/ProcessHelperFactoryTest.php new file mode 100644 index 00000000..5042fb73 --- /dev/null +++ b/module/CLI/test/Factory/ProcessHelperFactoryTest.php @@ -0,0 +1,29 @@ +factory = new ProcessHelperFactory(); + } + + /** @test */ + public function createsTheServiceWithTheProperSetOfHelpers(): void + { + $processHelper = ($this->factory)(); + $helperSet = $processHelper->getHelperSet(); + + $this->assertCount(2, $helperSet); + $this->assertTrue($helperSet->has('formatter')); + $this->assertTrue($helperSet->has('debug_formatter')); + } +} From 3916b06e7cf4c5d7fcafee2ef8dd93d0f84fc028 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 4 Aug 2019 21:31:37 +0200 Subject: [PATCH 03/12] Added improvements and new steps to CreateDatabaseCommand --- .../src/Command/Db/CreateDatabaseCommand.php | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php index ab334c40..188e2e68 100644 --- a/module/CLI/src/Command/Db/CreateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/CreateDatabaseCommand.php @@ -9,6 +9,7 @@ use Shlinkio\Shlink\CLI\Util\ExitCodes; use Symfony\Component\Console\Helper\ProcessHelper; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Style\SymfonyStyle; use Symfony\Component\Lock\Factory as Locker; use Symfony\Component\Process\PhpExecutableFinder; @@ -41,17 +42,36 @@ class CreateDatabaseCommand extends AbstractLockedCommand protected function lockedExecute(InputInterface $input, OutputInterface $output): int { - $this->checkDbExists(); + $io = new SymfonyStyle($input, $output); + if ($this->dbExistsAndIsPopulated()) { + $io->success('Database already exists.'); + return ExitCodes::EXIT_SUCCESS; + } + + if (! $this->schemaExists()) { + // TODO Create empty database + } + + // Create database + $io->writeln('Creating database tables...'); $command = [$this->phpBinary, self::DOCTRINE_HELPER_SCRIPT, self::DOCTRINE_HELPER_COMMAND]; $this->processHelper->run($output, $command); + $io->success('Database properly created!'); return ExitCodes::EXIT_SUCCESS; } - private function checkDbExists(): void + private function dbExistsAndIsPopulated(): bool { - // TODO + // TODO Implement + return false; + } + + private function schemaExists(): bool + { + // TODO Implement + return true; } protected function getLockConfig(): LockedCommandConfig From f78fa58cf1889abdf0d992177dc87be952fe49ce Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 5 Aug 2019 10:08:59 +0200 Subject: [PATCH 04/12] Updated CreateDatabaseCommand to create the empty database if it does not exist --- config/autoload/installer.global.php | 9 ++++ module/CLI/config/dependencies.config.php | 2 + .../Command/Db/AbstractDatabaseCommand.php | 33 +++++++++++++ .../src/Command/Db/CreateDatabaseCommand.php | 47 ++++++++++--------- module/Common/config/doctrine.config.php | 2 + .../Common/src/Doctrine/ConnectionFactory.php | 17 +++++++ .../test/Doctrine/ConnectionFactoryTest.php | 44 +++++++++++++++++ 7 files changed, 132 insertions(+), 22 deletions(-) create mode 100644 module/CLI/src/Command/Db/AbstractDatabaseCommand.php create mode 100644 module/Common/src/Doctrine/ConnectionFactory.php create mode 100644 module/Common/test/Doctrine/ConnectionFactoryTest.php diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index b258585e..7739ae4c 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -36,4 +36,13 @@ return [ ], ], + 'installation_commands' => [ + 'db_create_schema' => [ + 'command' => 'bin/cli db:create', + ], +// 'db_migrate' => [ +// 'command' => 'bin/cli db:migrate', +// ], + ], + ]; diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index b71598a9..5df6ea4d 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -3,6 +3,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI; +use Doctrine\DBAL\Connection; use GeoIp2\Database\Reader; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater; use Shlinkio\Shlink\Common\IpGeolocation\GeoLite2\DbUpdater; @@ -83,6 +84,7 @@ return [ Locker::class, SymfonyCli\Helper\ProcessHelper::class, PhpExecutableFinder::class, + Connection::class, ], ], diff --git a/module/CLI/src/Command/Db/AbstractDatabaseCommand.php b/module/CLI/src/Command/Db/AbstractDatabaseCommand.php new file mode 100644 index 00000000..99eb5c36 --- /dev/null +++ b/module/CLI/src/Command/Db/AbstractDatabaseCommand.php @@ -0,0 +1,33 @@ +processHelper = $processHelper; + $this->phpBinary = $phpFinder->find(false) ?: 'php'; + } + + protected function runPhpCommand(OutputInterface $output, array $command): void + { + array_unshift($command, $this->phpBinary); + $this->processHelper->run($output, $command); + } +} diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php index 188e2e68..4b4202a0 100644 --- a/module/CLI/src/Command/Db/CreateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/CreateDatabaseCommand.php @@ -3,7 +3,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Db; -use Shlinkio\Shlink\CLI\Command\Util\AbstractLockedCommand; +use Doctrine\DBAL\Connection; use Shlinkio\Shlink\CLI\Command\Util\LockedCommandConfig; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Symfony\Component\Console\Helper\ProcessHelper; @@ -13,22 +13,25 @@ use Symfony\Component\Console\Style\SymfonyStyle; use Symfony\Component\Lock\Factory as Locker; use Symfony\Component\Process\PhpExecutableFinder; -class CreateDatabaseCommand extends AbstractLockedCommand +use function Functional\contains; + +class CreateDatabaseCommand extends AbstractDatabaseCommand { public const NAME = 'db:create'; private const DOCTRINE_HELPER_SCRIPT = 'vendor/doctrine/orm/bin/doctrine.php'; private const DOCTRINE_HELPER_COMMAND = 'orm:schema-tool:create'; - /** @var ProcessHelper */ - private $processHelper; - /** @var string */ - private $phpBinary; + /** @var Connection */ + private $conn; - public function __construct(Locker $locker, ProcessHelper $processHelper, PhpExecutableFinder $phpFinder) - { - parent::__construct($locker); - $this->processHelper = $processHelper; - $this->phpBinary = $phpFinder->find(false) ?: 'php'; + public function __construct( + Locker $locker, + ProcessHelper $processHelper, + PhpExecutableFinder $phpFinder, + Connection $conn + ) { + parent::__construct($locker, $processHelper, $phpFinder); + $this->conn = $conn; } protected function configure(): void @@ -44,34 +47,34 @@ class CreateDatabaseCommand extends AbstractLockedCommand { $io = new SymfonyStyle($input, $output); - if ($this->dbExistsAndIsPopulated()) { + $this->checkDbExists(); + + if ($this->schemaExists()) { $io->success('Database already exists.'); return ExitCodes::EXIT_SUCCESS; } - if (! $this->schemaExists()) { - // TODO Create empty database - } - // Create database $io->writeln('Creating database tables...'); - $command = [$this->phpBinary, self::DOCTRINE_HELPER_SCRIPT, self::DOCTRINE_HELPER_COMMAND]; - $this->processHelper->run($output, $command); + $this->runPhpCommand($output, [self::DOCTRINE_HELPER_SCRIPT, self::DOCTRINE_HELPER_COMMAND]); $io->success('Database properly created!'); return ExitCodes::EXIT_SUCCESS; } - private function dbExistsAndIsPopulated(): bool + private function checkDbExists(): void { - // TODO Implement - return false; + $schemaManager = $this->conn->getSchemaManager(); + $databases = $schemaManager->listDatabases(); + if (! contains($databases, '')) { + $schemaManager->createDatabase($this->conn->getDatabase()); + } } private function schemaExists(): bool { // TODO Implement - return true; + return false; } protected function getLockConfig(): LockedCommandConfig diff --git a/module/Common/config/doctrine.config.php b/module/Common/config/doctrine.config.php index 66d678af..547724f2 100644 --- a/module/Common/config/doctrine.config.php +++ b/module/Common/config/doctrine.config.php @@ -3,6 +3,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Common; +use Doctrine\DBAL\Connection; use Doctrine\ORM\EntityManager; return [ @@ -18,6 +19,7 @@ return [ 'dependencies' => [ 'factories' => [ EntityManager::class => Doctrine\EntityManagerFactory::class, + Connection::class => Doctrine\ConnectionFactory::class, ], 'aliases' => [ 'em' => EntityManager::class, diff --git a/module/Common/src/Doctrine/ConnectionFactory.php b/module/Common/src/Doctrine/ConnectionFactory.php new file mode 100644 index 00000000..f0c1a561 --- /dev/null +++ b/module/Common/src/Doctrine/ConnectionFactory.php @@ -0,0 +1,17 @@ +get(EntityManager::class); + return $em->getConnection(); + } +} diff --git a/module/Common/test/Doctrine/ConnectionFactoryTest.php b/module/Common/test/Doctrine/ConnectionFactoryTest.php new file mode 100644 index 00000000..a89ef79c --- /dev/null +++ b/module/Common/test/Doctrine/ConnectionFactoryTest.php @@ -0,0 +1,44 @@ +container = $this->prophesize(ContainerInterface::class); + $this->em = $this->prophesize(EntityManagerInterface::class); + $this->container->get(EntityManager::class)->willReturn($this->em->reveal()); + + $this->factory = new ConnectionFactory(); + } + + /** @test */ + public function properServiceFallbackOccursWhenInvoked(): void + { + $connection = $this->prophesize(Connection::class)->reveal(); + $getConnection = $this->em->getConnection()->willReturn($connection); + + $result = ($this->factory)($this->container->reveal()); + + $this->assertSame($connection, $result); + $getConnection->shouldHaveBeenCalledOnce(); + $this->container->get(EntityManager::class)->shouldHaveBeenCalledOnce(); + } +} From b68e262eac6aa59c873ddd17349ea8188a3b87b5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 5 Aug 2019 10:16:58 +0200 Subject: [PATCH 05/12] Implemented how the CreateDatabaseCommand checks if the database tables exist --- module/CLI/src/Command/Db/CreateDatabaseCommand.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php index 4b4202a0..2703aefb 100644 --- a/module/CLI/src/Command/Db/CreateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/CreateDatabaseCommand.php @@ -66,15 +66,19 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand { $schemaManager = $this->conn->getSchemaManager(); $databases = $schemaManager->listDatabases(); - if (! contains($databases, '')) { - $schemaManager->createDatabase($this->conn->getDatabase()); + $shlinkDatabase = $this->conn->getDatabase(); + + if (! contains($databases, $shlinkDatabase)) { + $schemaManager->createDatabase($shlinkDatabase); } } private function schemaExists(): bool { - // TODO Implement - return false; + // 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 + $schemaManager = $this->conn->getSchemaManager(); + return ! empty($schemaManager->listTableNames()); } protected function getLockConfig(): LockedCommandConfig From 1aba77c752528a3a96781c54f8444b132407d010 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 5 Aug 2019 10:27:38 +0200 Subject: [PATCH 06/12] Enforced fixed shlink-installer version --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index e0a12dee..a9507b4d 100644 --- a/composer.json +++ b/composer.json @@ -31,7 +31,7 @@ "monolog/monolog": "^1.21", "ocramius/proxy-manager": "^2.0", "phly/phly-event-dispatcher": "^1.0", - "shlinkio/shlink-installer": "^1.2", + "shlinkio/shlink-installer": "^1.2.1", "symfony/console": "^4.3", "symfony/filesystem": "^4.3", "symfony/lock": "^4.3", From a575f2eced04b14b4a116ce039d0a047bf06c0bc Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 5 Aug 2019 18:48:33 +0200 Subject: [PATCH 07/12] Created new service which is the database connection but without the dbname, and used in in create db command --- module/CLI/config/dependencies.config.php | 2 ++ .../src/Command/Db/CreateDatabaseCommand.php | 14 ++++++++----- module/Common/config/doctrine.config.php | 1 + .../Doctrine/NoDbNameConnectionFactory.php | 21 +++++++++++++++++++ 4 files changed, 33 insertions(+), 5 deletions(-) create mode 100644 module/Common/src/Doctrine/NoDbNameConnectionFactory.php diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 5df6ea4d..69ace4b3 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\CLI; use Doctrine\DBAL\Connection; use GeoIp2\Database\Reader; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater; +use Shlinkio\Shlink\Common\Doctrine\NoDbNameConnectionFactory; use Shlinkio\Shlink\Common\IpGeolocation\GeoLite2\DbUpdater; use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface; use Shlinkio\Shlink\Common\Service\PreviewGenerator; @@ -85,6 +86,7 @@ return [ SymfonyCli\Helper\ProcessHelper::class, PhpExecutableFinder::class, Connection::class, + NoDbNameConnectionFactory::SERVICE_NAME, ], ], diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php index 2703aefb..7b316cc3 100644 --- a/module/CLI/src/Command/Db/CreateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/CreateDatabaseCommand.php @@ -23,15 +23,19 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand /** @var Connection */ private $conn; + /** @var Connection */ + private $noDbNameConn; public function __construct( Locker $locker, ProcessHelper $processHelper, PhpExecutableFinder $phpFinder, - Connection $conn + Connection $conn, + Connection $noDbNameConn ) { parent::__construct($locker, $processHelper, $phpFinder); $this->conn = $conn; + $this->noDbNameConn = $noDbNameConn; } protected function configure(): void @@ -50,12 +54,12 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand $this->checkDbExists(); if ($this->schemaExists()) { - $io->success('Database already exists.'); + $io->success('Database already exists. Run "db:migrate" command to make sure it is up to date.'); return ExitCodes::EXIT_SUCCESS; } // Create database - $io->writeln('Creating database tables...'); + $io->writeln('Creating database tables...'); $this->runPhpCommand($output, [self::DOCTRINE_HELPER_SCRIPT, self::DOCTRINE_HELPER_COMMAND]); $io->success('Database properly created!'); @@ -64,9 +68,9 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand private function checkDbExists(): void { - $schemaManager = $this->conn->getSchemaManager(); - $databases = $schemaManager->listDatabases(); $shlinkDatabase = $this->conn->getDatabase(); + $schemaManager = $this->noDbNameConn->getSchemaManager(); + $databases = $schemaManager->listDatabases(); if (! contains($databases, $shlinkDatabase)) { $schemaManager->createDatabase($shlinkDatabase); diff --git a/module/Common/config/doctrine.config.php b/module/Common/config/doctrine.config.php index 547724f2..0b569043 100644 --- a/module/Common/config/doctrine.config.php +++ b/module/Common/config/doctrine.config.php @@ -20,6 +20,7 @@ return [ 'factories' => [ EntityManager::class => Doctrine\EntityManagerFactory::class, Connection::class => Doctrine\ConnectionFactory::class, + Doctrine\NoDbNameConnectionFactory::SERVICE_NAME => Doctrine\NoDbNameConnectionFactory::class, ], 'aliases' => [ 'em' => EntityManager::class, diff --git a/module/Common/src/Doctrine/NoDbNameConnectionFactory.php b/module/Common/src/Doctrine/NoDbNameConnectionFactory.php new file mode 100644 index 00000000..fdf470e0 --- /dev/null +++ b/module/Common/src/Doctrine/NoDbNameConnectionFactory.php @@ -0,0 +1,21 @@ +get(Connection::class); + $params = $conn->getParams(); + unset($params['dbname']); + + return new Connection($params, $conn->getDriver(), $conn->getConfiguration(), $conn->getEventManager()); + } +} From e79c41d753fa801caf6ffbb9bb3bd8088ab90c1c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 6 Aug 2019 17:30:28 +0200 Subject: [PATCH 08/12] Created NoDbNameConnectionFactoryTest --- .../src/Command/Db/CreateDatabaseCommand.php | 4 +- .../NoDbNameConnectionFactoryTest.php | 56 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 module/Common/test/Doctrine/NoDbNameConnectionFactoryTest.php diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php index 7b316cc3..c507a601 100644 --- a/module/CLI/src/Command/Db/CreateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/CreateDatabaseCommand.php @@ -68,9 +68,11 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand private function checkDbExists(): void { - $shlinkDatabase = $this->conn->getDatabase(); + // In order to create the new database, we have to use a connection where the dbname was not set. + // Otherwise, it will fail to connect and will not be able to create the new database $schemaManager = $this->noDbNameConn->getSchemaManager(); $databases = $schemaManager->listDatabases(); + $shlinkDatabase = $this->conn->getDatabase(); if (! contains($databases, $shlinkDatabase)) { $schemaManager->createDatabase($shlinkDatabase); diff --git a/module/Common/test/Doctrine/NoDbNameConnectionFactoryTest.php b/module/Common/test/Doctrine/NoDbNameConnectionFactoryTest.php new file mode 100644 index 00000000..d8f58f8e --- /dev/null +++ b/module/Common/test/Doctrine/NoDbNameConnectionFactoryTest.php @@ -0,0 +1,56 @@ +container = $this->prophesize(ContainerInterface::class); + $this->originalConn = $this->prophesize(Connection::class); + $this->container->get(Connection::class)->willReturn($this->originalConn->reveal()); + + $this->factory = new NoDbNameConnectionFactory(); + } + + /** @test */ + public function createsNewConnectionRemovingDbNameFromOriginalConnectionParams(): void + { + $params = [ + 'username' => 'foo', + 'password' => 'bar', + 'dbname' => 'something', + ]; + $getOriginalParams = $this->originalConn->getParams()->willReturn($params); + $getOriginalDriver = $this->originalConn->getDriver()->willReturn($this->prophesize(Driver::class)->reveal()); + $getOriginalConfig = $this->originalConn->getConfiguration()->willReturn(null); + $getOriginalEvents = $this->originalConn->getEventManager()->willReturn(null); + + $conn = ($this->factory)($this->container->reveal()); + + $this->assertEquals([ + 'username' => 'foo', + 'password' => 'bar', + ], $conn->getParams()); + $getOriginalParams->shouldHaveBeenCalledOnce(); + $getOriginalDriver->shouldHaveBeenCalledOnce(); + $getOriginalConfig->shouldHaveBeenCalledOnce(); + $getOriginalEvents->shouldHaveBeenCalledOnce(); + $this->container->get(Connection::class)->shouldHaveBeenCalledOnce(); + } +} From 749671c2303d5b5a5a34c4b43b4d090491bb9670 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 6 Aug 2019 18:22:49 +0200 Subject: [PATCH 09/12] Created CreateDatabaseCommandTest --- .../src/Command/Db/CreateDatabaseCommand.php | 12 +- .../Command/Db/CreateDatabaseCommandTest.php | 130 ++++++++++++++++++ 2 files changed, 136 insertions(+), 6 deletions(-) create mode 100644 module/CLI/test/Command/Db/CreateDatabaseCommandTest.php diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php index c507a601..1ac933fa 100644 --- a/module/CLI/src/Command/Db/CreateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/CreateDatabaseCommand.php @@ -18,11 +18,11 @@ use function Functional\contains; class CreateDatabaseCommand extends AbstractDatabaseCommand { public const NAME = 'db:create'; - private const DOCTRINE_HELPER_SCRIPT = 'vendor/doctrine/orm/bin/doctrine.php'; - private const DOCTRINE_HELPER_COMMAND = 'orm:schema-tool:create'; + public const DOCTRINE_HELPER_SCRIPT = 'vendor/doctrine/orm/bin/doctrine.php'; + public const DOCTRINE_HELPER_COMMAND = 'orm:schema-tool:create'; /** @var Connection */ - private $conn; + private $regularConn; /** @var Connection */ private $noDbNameConn; @@ -34,7 +34,7 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand Connection $noDbNameConn ) { parent::__construct($locker, $processHelper, $phpFinder); - $this->conn = $conn; + $this->regularConn = $conn; $this->noDbNameConn = $noDbNameConn; } @@ -72,7 +72,7 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand // Otherwise, it will fail to connect and will not be able to create the new database $schemaManager = $this->noDbNameConn->getSchemaManager(); $databases = $schemaManager->listDatabases(); - $shlinkDatabase = $this->conn->getDatabase(); + $shlinkDatabase = $this->regularConn->getDatabase(); if (! contains($databases, $shlinkDatabase)) { $schemaManager->createDatabase($shlinkDatabase); @@ -83,7 +83,7 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand { // 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 - $schemaManager = $this->conn->getSchemaManager(); + $schemaManager = $this->regularConn->getSchemaManager(); return ! empty($schemaManager->listTableNames()); } diff --git a/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php b/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php new file mode 100644 index 00000000..33e9d94c --- /dev/null +++ b/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php @@ -0,0 +1,130 @@ +prophesize(Locker::class); + $lock = $this->prophesize(LockInterface::class); + $lock->acquire(Argument::any())->willReturn(true); + $lock->release()->will(function () { + }); + $locker->createLock(Argument::cetera())->willReturn($lock->reveal()); + + $phpExecutableFinder = $this->prophesize(PhpExecutableFinder::class); + $phpExecutableFinder->find(false)->willReturn('/usr/local/bin/php'); + + $this->processHelper = $this->prophesize(ProcessHelper::class); + $this->schemaManager = $this->prophesize(AbstractSchemaManager::class); + + $this->regularConn = $this->prophesize(Connection::class); + $this->regularConn->getSchemaManager()->willReturn($this->schemaManager->reveal()); + $this->noDbNameConn = $this->prophesize(Connection::class); + $this->noDbNameConn->getSchemaManager()->willReturn($this->schemaManager->reveal()); + + $command = new CreateDatabaseCommand( + $locker->reveal(), + $this->processHelper->reveal(), + $phpExecutableFinder->reveal(), + $this->regularConn->reveal(), + $this->noDbNameConn->reveal() + ); + $app = new Application(); + $app->add($command); + + $this->commandTester = new CommandTester($command); + } + + /** @test */ + public function successMessageIsPrintedIfDatabaseAlreadyExists(): void + { + $shlinkDatabase = 'shlink_database'; + $getDatabase = $this->regularConn->getDatabase()->willReturn($shlinkDatabase); + $listDatabases = $this->schemaManager->listDatabases()->willReturn(['foo', $shlinkDatabase, 'bar']); + $createDatabase = $this->schemaManager->createDatabase($shlinkDatabase)->will(function () { + }); + $listTables = $this->schemaManager->listTableNames()->willReturn(['foo_table', 'bar_table']); + + $this->commandTester->execute([]); + $output = $this->commandTester->getDisplay(); + + $this->assertStringContainsString('Database already exists. Run "db:migrate" command', $output); + $getDatabase->shouldHaveBeenCalledOnce(); + $listDatabases->shouldHaveBeenCalledOnce(); + $createDatabase->shouldNotHaveBeenCalled(); + $listTables->shouldHaveBeenCalledOnce(); + } + + /** @test */ + public function databaseIsCreatedIfItDoesNotExist(): void + { + $shlinkDatabase = 'shlink_database'; + $getDatabase = $this->regularConn->getDatabase()->willReturn($shlinkDatabase); + $listDatabases = $this->schemaManager->listDatabases()->willReturn(['foo', 'bar']); + $createDatabase = $this->schemaManager->createDatabase($shlinkDatabase)->will(function () { + }); + $listTables = $this->schemaManager->listTableNames()->willReturn(['foo_table', 'bar_table']); + + $this->commandTester->execute([]); + + $getDatabase->shouldHaveBeenCalledOnce(); + $listDatabases->shouldHaveBeenCalledOnce(); + $createDatabase->shouldHaveBeenCalledOnce(); + $listTables->shouldHaveBeenCalledOnce(); + } + + /** @test */ + public function tablesAreCreatedIfDatabaseIsEMpty(): void + { + $shlinkDatabase = 'shlink_database'; + $getDatabase = $this->regularConn->getDatabase()->willReturn($shlinkDatabase); + $listDatabases = $this->schemaManager->listDatabases()->willReturn(['foo', $shlinkDatabase, 'bar']); + $createDatabase = $this->schemaManager->createDatabase($shlinkDatabase)->will(function () { + }); + $listTables = $this->schemaManager->listTableNames()->willReturn([]); + $runCommand = $this->processHelper->run(Argument::type(OutputInterface::class), [ + '/usr/local/bin/php', + CreateDatabaseCommand::DOCTRINE_HELPER_SCRIPT, + CreateDatabaseCommand::DOCTRINE_HELPER_COMMAND, + ]); + + $this->commandTester->execute([]); + $output = $this->commandTester->getDisplay(); + + $this->assertStringContainsString('Creating database tables...', $output); + $this->assertStringContainsString('Database properly created!', $output); + $getDatabase->shouldHaveBeenCalledOnce(); + $listDatabases->shouldHaveBeenCalledOnce(); + $createDatabase->shouldNotHaveBeenCalled(); + $listTables->shouldHaveBeenCalledOnce(); + $runCommand->shouldHaveBeenCalledOnce(); + } +} From 5d5d89afb929f5bc95a95784207cadf9512ee7f2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 6 Aug 2019 18:49:32 +0200 Subject: [PATCH 10/12] Updated changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 509bcee2..651bb6db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * True-Client-IP * X-Real-IP + * [#440](https://github.com/shlinkio/shlink/pull/440) Created `db:create` command, which improves how the shlink database is created, with these benefits: + + * It sets up a lock which prevents the command to be run multiple times. + * It checks of the database does not exist, and creates it in that case. + * It checks if the database tables already exist, exiting gracefully in that case. + #### Changed * [#430](https://github.com/shlinkio/shlink/issues/430) Updated to [shlinkio/php-coding-standard](https://github.com/shlinkio/php-coding-standard) 1.2.2 From e04838eaa2b411e6242100b22f6d81f449a29910 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 6 Aug 2019 18:56:47 +0200 Subject: [PATCH 11/12] Updated readme cli help --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 7afae708..f2c13057 100644 --- a/README.md +++ b/README.md @@ -268,6 +268,8 @@ Available commands: config config:generate-charset [DEPRECATED] Generates a character set sample just by shuffling the default one, "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ". Then it can be set in the SHORTCODE_CHARS environment variable config:generate-secret [DEPRECATED] Generates a random secret string that can be used for JWT token encryption + db + db:create Creates the database needed for shlink to work. It will do nothing if the database already exists short-url short-url:delete [short-code:delete] Deletes a short URL short-url:generate [shortcode:generate|short-code:generate] Generates a short URL for provided long URL and returns it From bc3fc59b1e3e94c7a3bbff701e0acf8eab0292ac Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 6 Aug 2019 20:16:16 +0200 Subject: [PATCH 12/12] Fixed error on new database creation command when database platform is sqlite --- .../src/Command/Db/CreateDatabaseCommand.php | 4 +++ .../Command/Db/CreateDatabaseCommandTest.php | 25 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php index 1ac933fa..da570aeb 100644 --- a/module/CLI/src/Command/Db/CreateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/CreateDatabaseCommand.php @@ -68,6 +68,10 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand private function checkDbExists(): void { + if ($this->regularConn->getDatabasePlatform()->getName() === 'sqlite') { + return; + } + // In order to create the new database, we have to use a connection where the dbname was not set. // Otherwise, it will fail to connect and will not be able to create the new database $schemaManager = $this->noDbNameConn->getSchemaManager(); diff --git a/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php b/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php index 33e9d94c..813a5d69 100644 --- a/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php +++ b/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\CLI\Command\Db; use Doctrine\DBAL\Connection; +use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Schema\AbstractSchemaManager; use PHPUnit\Framework\TestCase; use Prophecy\Argument; @@ -29,6 +30,8 @@ class CreateDatabaseCommandTest extends TestCase private $noDbNameConn; /** @var ObjectProphecy */ private $schemaManager; + /** @var ObjectProphecy */ + private $databasePlatform; public function setUp(): void { @@ -44,9 +47,11 @@ class CreateDatabaseCommandTest extends TestCase $this->processHelper = $this->prophesize(ProcessHelper::class); $this->schemaManager = $this->prophesize(AbstractSchemaManager::class); + $this->databasePlatform = $this->prophesize(AbstractPlatform::class); $this->regularConn = $this->prophesize(Connection::class); $this->regularConn->getSchemaManager()->willReturn($this->schemaManager->reveal()); + $this->regularConn->getDatabasePlatform()->willReturn($this->databasePlatform->reveal()); $this->noDbNameConn = $this->prophesize(Connection::class); $this->noDbNameConn->getSchemaManager()->willReturn($this->schemaManager->reveal()); @@ -127,4 +132,24 @@ class CreateDatabaseCommandTest extends TestCase $listTables->shouldHaveBeenCalledOnce(); $runCommand->shouldHaveBeenCalledOnce(); } + + /** @test */ + public function databaseCheckIsSkippedForSqlite(): void + { + $this->databasePlatform->getName()->willReturn('sqlite'); + + $shlinkDatabase = 'shlink_database'; + $getDatabase = $this->regularConn->getDatabase()->willReturn($shlinkDatabase); + $listDatabases = $this->schemaManager->listDatabases()->willReturn(['foo', 'bar']); + $createDatabase = $this->schemaManager->createDatabase($shlinkDatabase)->will(function () { + }); + $listTables = $this->schemaManager->listTableNames()->willReturn(['foo_table', 'bar_table']); + + $this->commandTester->execute([]); + + $getDatabase->shouldNotHaveBeenCalled(); + $listDatabases->shouldNotHaveBeenCalled(); + $createDatabase->shouldNotHaveBeenCalled(); + $listTables->shouldHaveBeenCalledOnce(); + } }