diff --git a/CHANGELOG.md b/CHANGELOG.md index 295b28ee..91442b4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this #### Added -* *Nothing* +* [#377](https://github.com/shlinkio/shlink/issues/377) Updated `visit:locate` command (formerly `visit:process`) to automatically update the GeoLite2 database if it is too old or it does not exist. + + This simplifies processing visits in a container-based infrastructure, since a fresh container is capable of getting an updated version of the file by itself. + + It also removes the need of asynchronously and programmatically updating the file, which deprecates the `visit:update-db` command. #### Changed diff --git a/config/autoload/zend-expressive.global.php b/config/autoload/common.global.php similarity index 100% rename from config/autoload/zend-expressive.global.php rename to config/autoload/common.global.php diff --git a/config/autoload/local.php.dist b/config/autoload/common.local.php.dist similarity index 100% rename from config/autoload/local.php.dist rename to config/autoload/common.local.php.dist diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 84969d1a..2edbae99 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -14,7 +14,7 @@ return [ Command\ShortUrl\GeneratePreviewCommand::NAME => Command\ShortUrl\GeneratePreviewCommand::class, Command\ShortUrl\DeleteShortUrlCommand::NAME => Command\ShortUrl\DeleteShortUrlCommand::class, - Command\Visit\ProcessVisitsCommand::NAME => Command\Visit\ProcessVisitsCommand::class, + Command\Visit\LocateVisitsCommand::NAME => Command\Visit\LocateVisitsCommand::class, Command\Visit\UpdateDbCommand::NAME => Command\Visit\UpdateDbCommand::class, Command\Config\GenerateCharsetCommand::NAME => Command\Config\GenerateCharsetCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 7c34f580..b56ff76e 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -3,6 +3,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI; +use GeoIp2\Database\Reader; +use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater; use Shlinkio\Shlink\Common\IpGeolocation\GeoLite2\DbUpdater; use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface; use Shlinkio\Shlink\Common\Service\PreviewGenerator; @@ -19,6 +21,8 @@ return [ 'factories' => [ Application::class => Factory\ApplicationFactory::class, + GeolocationDbUpdater::class => ConfigAbstractFactory::class, + Command\ShortUrl\GenerateShortUrlCommand::class => ConfigAbstractFactory::class, Command\ShortUrl\ResolveUrlCommand::class => ConfigAbstractFactory::class, Command\ShortUrl\ListShortUrlsCommand::class => ConfigAbstractFactory::class, @@ -26,7 +30,7 @@ return [ Command\ShortUrl\GeneratePreviewCommand::class => ConfigAbstractFactory::class, Command\ShortUrl\DeleteShortUrlCommand::class => ConfigAbstractFactory::class, - Command\Visit\ProcessVisitsCommand::class => ConfigAbstractFactory::class, + Command\Visit\LocateVisitsCommand::class => ConfigAbstractFactory::class, Command\Visit\UpdateDbCommand::class => ConfigAbstractFactory::class, Command\Config\GenerateCharsetCommand::class => InvokableFactory::class, @@ -44,6 +48,8 @@ return [ ], ConfigAbstractFactory::class => [ + GeolocationDbUpdater::class => [DbUpdater::class, Reader::class], + Command\ShortUrl\GenerateShortUrlCommand::class => [Service\UrlShortener::class, 'config.url_shortener.domain'], Command\ShortUrl\ResolveUrlCommand::class => [Service\UrlShortener::class], Command\ShortUrl\ListShortUrlsCommand::class => [Service\ShortUrlService::class, 'config.url_shortener.domain'], @@ -51,10 +57,11 @@ return [ Command\ShortUrl\GeneratePreviewCommand::class => [Service\ShortUrlService::class, PreviewGenerator::class], Command\ShortUrl\DeleteShortUrlCommand::class => [Service\ShortUrl\DeleteShortUrlService::class], - Command\Visit\ProcessVisitsCommand::class => [ + Command\Visit\LocateVisitsCommand::class => [ Service\VisitService::class, IpLocationResolverInterface::class, Lock\Factory::class, + GeolocationDbUpdater::class, ], Command\Visit\UpdateDbCommand::class => [DbUpdater::class], diff --git a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php similarity index 54% rename from module/CLI/src/Command/Visit/ProcessVisitsCommand.php rename to module/CLI/src/Command/Visit/LocateVisitsCommand.php index e3db15d0..fa458413 100644 --- a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -3,7 +3,9 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; +use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface; use Shlinkio\Shlink\Common\Exception\WrongIpException; use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface; use Shlinkio\Shlink\Common\IpGeolocation\Model\Location; @@ -13,6 +15,7 @@ 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; @@ -20,9 +23,10 @@ use Symfony\Component\Lock\Factory as Locker; use function sprintf; -class ProcessVisitsCommand extends Command +class LocateVisitsCommand extends Command { - public const NAME = 'visit:process'; + public const NAME = 'visit:locate'; + public const ALIASES = ['visit:process']; /** @var VisitServiceInterface */ private $visitService; @@ -30,39 +34,48 @@ class ProcessVisitsCommand extends Command private $ipLocationResolver; /** @var Locker */ private $locker; - /** @var OutputInterface */ - private $output; + /** @var GeolocationDbUpdaterInterface */ + private $dbUpdater; + + /** @var SymfonyStyle */ + private $io; + /** @var ProgressBar */ + private $progressBar; public function __construct( VisitServiceInterface $visitService, IpLocationResolverInterface $ipLocationResolver, - Locker $locker + Locker $locker, + GeolocationDbUpdaterInterface $dbUpdater ) { parent::__construct(); $this->visitService = $visitService; $this->ipLocationResolver = $ipLocationResolver; $this->locker = $locker; + $this->dbUpdater = $dbUpdater; } protected function configure(): void { $this ->setName(self::NAME) - ->setDescription('Processes visits where location is not set yet'); + ->setAliases(self::ALIASES) + ->setDescription('Resolves visits origin locations.'); } protected function execute(InputInterface $input, OutputInterface $output): ?int { - $this->output = $output; - $io = new SymfonyStyle($input, $output); + $this->io = new SymfonyStyle($input, $output); $lock = $this->locker->createLock(self::NAME); if (! $lock->acquire()) { - $io->warning(sprintf('There is already an instance of the "%s" command in execution', self::NAME)); + $this->io->warning(sprintf('There is already an instance of the "%s" command in execution', self::NAME)); return ExitCodes::EXIT_WARNING; } try { + $this->checkDbUpdate(); + $this->visitService->locateUnlocatedVisits( [$this, 'getGeolocationDataForVisit'], function (VisitLocation $location) use ($output) { @@ -74,7 +87,7 @@ class ProcessVisitsCommand extends Command } ); - $io->success('Finished processing all IPs'); + $this->io->success('Finished processing all IPs'); } finally { $lock->release(); return ExitCodes::EXIT_SUCCESS; @@ -84,7 +97,7 @@ class ProcessVisitsCommand extends Command public function getGeolocationDataForVisit(Visit $visit): Location { if (! $visit->hasRemoteAddr()) { - $this->output->writeln( + $this->io->writeln( 'Ignored visit with no IP address', OutputInterface::VERBOSITY_VERBOSE ); @@ -92,21 +105,51 @@ class ProcessVisitsCommand extends Command } $ipAddr = $visit->getRemoteAddr(); - $this->output->write(sprintf('Processing IP %s', $ipAddr)); + $this->io->write(sprintf('Processing IP %s', $ipAddr)); if ($ipAddr === IpAddress::LOCALHOST) { - $this->output->writeln(' [Ignored localhost address]'); + $this->io->writeln(' [Ignored localhost address]'); throw IpCannotBeLocatedException::forLocalhost(); } try { return $this->ipLocationResolver->resolveIpLocation($ipAddr); } catch (WrongIpException $e) { - $this->output->writeln(' [An error occurred while locating IP. Skipped]'); - if ($this->output->isVerbose()) { - $this->getApplication()->renderException($e, $this->output); + $this->io->writeln(' [An error occurred while locating IP. Skipped]'); + if ($this->io->isVerbose()) { + $this->getApplication()->renderException($e, $this->io); } throw IpCannotBeLocatedException::forError($e); } } + + private function checkDbUpdate(): void + { + try { + $this->dbUpdater->checkDbUpdate(function (bool $olderDbExists) { + $this->io->writeln( + sprintf('%s GeoLite2 database...', $olderDbExists ? 'Updating' : 'Downloading') + ); + $this->progressBar = new ProgressBar($this->io); + }, function (int $total, int $downloaded) { + $this->progressBar->setMaxSteps($total); + $this->progressBar->setProgress($downloaded); + }); + + if ($this->progressBar !== null) { + $this->progressBar->finish(); + $this->io->newLine(); + } + } catch (GeolocationDbUpdateFailedException $e) { + if (! $e->olderDbExists()) { + $this->io->error('GeoLite2 database download failed. It is not possible to locate visits.'); + throw $e; + } + + $this->io->newLine(); + $this->io->writeln( + '[Warning] GeoLite2 database update failed. Proceeding with old version.' + ); + } + } } diff --git a/module/CLI/src/Command/Visit/UpdateDbCommand.php b/module/CLI/src/Command/Visit/UpdateDbCommand.php index af032e96..3882b2c6 100644 --- a/module/CLI/src/Command/Visit/UpdateDbCommand.php +++ b/module/CLI/src/Command/Visit/UpdateDbCommand.php @@ -15,6 +15,7 @@ use Symfony\Component\Console\Style\SymfonyStyle; use function sprintf; +/** @deprecated */ class UpdateDbCommand extends Command { public const NAME = 'visit:update-db'; @@ -32,7 +33,7 @@ class UpdateDbCommand extends Command { $this ->setName(self::NAME) - ->setDescription('Updates the GeoLite2 database file used to geolocate IP addresses') + ->setDescription('[DEPRECATED] Updates the GeoLite2 database file used to geolocate IP addresses') ->setHelp( 'The GeoLite2 database is updated first Tuesday every month, so this command should be ideally run ' . 'every first Wednesday' diff --git a/module/CLI/src/Exception/ExceptionInterface.php b/module/CLI/src/Exception/ExceptionInterface.php new file mode 100644 index 00000000..2ff19593 --- /dev/null +++ b/module/CLI/src/Exception/ExceptionInterface.php @@ -0,0 +1,10 @@ +olderDbExists = $olderDbExists; + parent::__construct($message, $code, $previous); + } + + public static function create(bool $olderDbExists, Throwable $prev = null): self + { + return new self( + $olderDbExists, + 'An error occurred while updating geolocation database, and an older version could not be found', + 0, + $prev + ); + } + + public function olderDbExists(): bool + { + return $this->olderDbExists; + } +} diff --git a/module/CLI/src/Util/GeolocationDbUpdater.php b/module/CLI/src/Util/GeolocationDbUpdater.php new file mode 100644 index 00000000..067a9561 --- /dev/null +++ b/module/CLI/src/Util/GeolocationDbUpdater.php @@ -0,0 +1,67 @@ +dbUpdater = $dbUpdater; + $this->geoLiteDbReader = $geoLiteDbReader; + } + + /** + * @throws GeolocationDbUpdateFailedException + */ + public function checkDbUpdate(callable $mustBeUpdated = null, callable $handleProgress = null): void + { + try { + $meta = $this->geoLiteDbReader->metadata(); + if ($this->buildIsTooOld($meta->__get('buildEpoch'))) { + $this->downloadNewDb(true, $mustBeUpdated, $handleProgress); + } + } catch (InvalidArgumentException $e) { + // This is the exception thrown by the reader when the database file does not exist + $this->downloadNewDb(false, $mustBeUpdated, $handleProgress); + } + } + + private function buildIsTooOld(int $buildTimestamp): bool + { + $buildDate = Chronos::createFromTimestamp($buildTimestamp); + $now = Chronos::now(); + return $now->gt($buildDate->addDays(35)); + } + + /** + * @throws GeolocationDbUpdateFailedException + */ + private function downloadNewDb( + bool $olderDbExists, + callable $mustBeUpdated = null, + callable $handleProgress = null + ): void { + if ($mustBeUpdated !== null) { + $mustBeUpdated($olderDbExists); + } + + try { + $this->dbUpdater->downloadFreshCopy($handleProgress); + } catch (RuntimeException $e) { + throw GeolocationDbUpdateFailedException::create($olderDbExists, $e); + } + } +} diff --git a/module/CLI/src/Util/GeolocationDbUpdaterInterface.php b/module/CLI/src/Util/GeolocationDbUpdaterInterface.php new file mode 100644 index 00000000..1d5bcf48 --- /dev/null +++ b/module/CLI/src/Util/GeolocationDbUpdaterInterface.php @@ -0,0 +1,14 @@ +visitService = $this->prophesize(VisitService::class); $this->ipResolver = $this->prophesize(IpApiLocationResolver::class); + $this->dbUpdater = $this->prophesize(GeolocationDbUpdaterInterface::class); $this->locker = $this->prophesize(Lock\Factory::class); $this->lock = $this->prophesize(Lock\LockInterface::class); @@ -49,10 +54,11 @@ class ProcessVisitsCommandTest extends TestCase }); $this->locker->createLock(Argument::type('string'))->willReturn($this->lock->reveal()); - $command = new ProcessVisitsCommand( + $command = new LocateVisitsCommand( $this->visitService->reveal(), $this->ipResolver->reveal(), - $this->locker->reveal() + $this->locker->reveal(), + $this->dbUpdater->reveal() ); $app = new Application(); $app->add($command); @@ -176,10 +182,47 @@ class ProcessVisitsCommandTest extends TestCase $output = $this->commandTester->getDisplay(); $this->assertStringContainsString( - sprintf('There is already an instance of the "%s" command', ProcessVisitsCommand::NAME), + sprintf('There is already an instance of the "%s" command', LocateVisitsCommand::NAME), $output ); $locateVisits->shouldNotHaveBeenCalled(); $resolveIpLocation->shouldNotHaveBeenCalled(); } + + /** + * @test + * @dataProvider provideParams + */ + public function showsProperMessageWhenGeoLiteUpdateFails(bool $olderDbExists, string $expectedMessage): void + { + $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(function () { + }); + $checkDbUpdate = $this->dbUpdater->checkDbUpdate(Argument::cetera())->will( + function (array $args) use ($olderDbExists) { + [$mustBeUpdated, $handleProgress] = $args; + + $mustBeUpdated($olderDbExists); + $handleProgress(100, 50); + + throw GeolocationDbUpdateFailedException::create($olderDbExists); + } + ); + + $this->commandTester->execute([]); + $output = $this->commandTester->getDisplay(); + + $this->assertStringContainsString( + sprintf('%s GeoLite2 database...', $olderDbExists ? 'Updating' : 'Downloading'), + $output + ); + $this->assertStringContainsString($expectedMessage, $output); + $locateVisits->shouldHaveBeenCalledTimes((int) $olderDbExists); + $checkDbUpdate->shouldHaveBeenCalledOnce(); + } + + public function provideParams(): iterable + { + yield [true, '[Warning] GeoLite2 database update failed. Proceeding with old version.']; + yield [false, 'GeoLite2 database download failed. It is not possible to locate visits.']; + } } diff --git a/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php b/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php new file mode 100644 index 00000000..b8202967 --- /dev/null +++ b/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php @@ -0,0 +1,79 @@ +assertEquals($olderDbExists, $e->olderDbExists()); + $this->assertEquals('', $e->getMessage()); + $this->assertEquals(0, $e->getCode()); + $this->assertNull($e->getPrevious()); + } + + public function provideOlderDbExists(): iterable + { + yield 'with older DB' => [true]; + yield 'without older DB' => [false]; + } + + /** + * @test + * @dataProvider provideConstructorArgs + */ + public function constructCreatesException(bool $olderDbExists, string $message, int $code, ?Throwable $prev): void + { + $e = new GeolocationDbUpdateFailedException($olderDbExists, $message, $code, $prev); + + $this->assertEquals($olderDbExists, $e->olderDbExists()); + $this->assertEquals($message, $e->getMessage()); + $this->assertEquals($code, $e->getCode()); + $this->assertEquals($prev, $e->getPrevious()); + } + + public function provideConstructorArgs(): iterable + { + yield [true, 'This is a nice error message', 99, new Exception('prev')]; + yield [false, 'Another message', 0, new RuntimeException('prev')]; + yield [true, 'An yet another message', -50, null]; + } + + /** + * @test + * @dataProvider provideCreateArgs + */ + public function createBuildsException(bool $olderDbExists, ?Throwable $prev): void + { + $e = GeolocationDbUpdateFailedException::create($olderDbExists, $prev); + + $this->assertEquals($olderDbExists, $e->olderDbExists()); + $this->assertEquals( + 'An error occurred while updating geolocation database, and an older version could not be found', + $e->getMessage() + ); + $this->assertEquals(0, $e->getCode()); + $this->assertEquals($prev, $e->getPrevious()); + } + + public function provideCreateArgs(): iterable + { + yield 'older DB and no prev' => [true, null]; + yield 'older DB and prev' => [true, new RuntimeException('prev')]; + yield 'no older DB and no prev' => [false, null]; + yield 'no older DB and prev' => [false, new Exception('prev')]; + } +} diff --git a/module/CLI/test/Util/GeolocationDbUpdaterTest.php b/module/CLI/test/Util/GeolocationDbUpdaterTest.php new file mode 100644 index 00000000..3ab69aa7 --- /dev/null +++ b/module/CLI/test/Util/GeolocationDbUpdaterTest.php @@ -0,0 +1,139 @@ +dbUpdater = $this->prophesize(DbUpdaterInterface::class); + $this->geoLiteDbReader = $this->prophesize(Reader::class); + + $this->geolocationDbUpdater = new GeolocationDbUpdater( + $this->dbUpdater->reveal(), + $this->geoLiteDbReader->reveal() + ); + } + + /** @test */ + public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(): void + { + $mustBeUpdated = function () { + $this->assertTrue(true); + }; + $getMeta = $this->geoLiteDbReader->metadata()->willThrow(InvalidArgumentException::class); + $prev = new RuntimeException(''); + $download = $this->dbUpdater->downloadFreshCopy(null)->willThrow($prev); + + try { + $this->geolocationDbUpdater->checkDbUpdate($mustBeUpdated); + $this->assertTrue(false); // If this is reached, the test will fail + } catch (Throwable $e) { + /** @var GeolocationDbUpdateFailedException $e */ + $this->assertInstanceOf(GeolocationDbUpdateFailedException::class, $e); + $this->assertSame($prev, $e->getPrevious()); + $this->assertFalse($e->olderDbExists()); + } + + $getMeta->shouldHaveBeenCalledOnce(); + $download->shouldHaveBeenCalledOnce(); + } + + /** + * @test + * @dataProvider provideBigDays + */ + public function exceptionIsThrownWhenOlderDbIsTooOldAndDownloadFails(int $days): void + { + $getMeta = $this->geoLiteDbReader->metadata()->willReturn(new Metadata([ + 'binary_format_major_version' => '', + 'binary_format_minor_version' => '', + 'build_epoch' => Chronos::now()->subDays($days)->getTimestamp(), + 'database_type' => '', + 'languages' => '', + 'description' => '', + 'ip_version' => '', + 'node_count' => 1, + 'record_size' => 4, + ])); + $prev = new RuntimeException(''); + $download = $this->dbUpdater->downloadFreshCopy(null)->willThrow($prev); + + try { + $this->geolocationDbUpdater->checkDbUpdate(); + $this->assertTrue(false); // If this is reached, the test will fail + } catch (Throwable $e) { + /** @var GeolocationDbUpdateFailedException $e */ + $this->assertInstanceOf(GeolocationDbUpdateFailedException::class, $e); + $this->assertSame($prev, $e->getPrevious()); + $this->assertTrue($e->olderDbExists()); + } + + $getMeta->shouldHaveBeenCalledOnce(); + $download->shouldHaveBeenCalledOnce(); + } + + public function provideBigDays(): iterable + { + yield [36]; + yield [50]; + yield [75]; + yield [100]; + } + + /** + * @test + * @dataProvider provideSmallDays + */ + public function databaseIsNotUpdatedIfItIsYoungerThanOneWeek(int $days): void + { + $getMeta = $this->geoLiteDbReader->metadata()->willReturn(new Metadata([ + 'binary_format_major_version' => '', + 'binary_format_minor_version' => '', + 'build_epoch' => Chronos::now()->subDays($days)->getTimestamp(), + 'database_type' => '', + 'languages' => '', + 'description' => '', + 'ip_version' => '', + 'node_count' => 1, + 'record_size' => 4, + ])); + $download = $this->dbUpdater->downloadFreshCopy(null)->will(function () { + }); + + $this->geolocationDbUpdater->checkDbUpdate(); + + $getMeta->shouldHaveBeenCalledOnce(); + $download->shouldNotHaveBeenCalled(); + } + + public function provideSmallDays(): iterable + { + return map(range(0, 34), function (int $days) { + return [$days]; + }); + } +}