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'));