From 74ea5969be76d39fc2a92ac70bf98f40bd12336f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 7 Apr 2021 16:29:29 +0200 Subject: [PATCH] Created new listener to update the GeoLite db after a visit occurs --- .../src/Command/Visit/LocateVisitsCommand.php | 2 +- .../Command/Visit/LocateVisitsCommandTest.php | 2 +- .../Core/config/event_dispatcher.config.php | 5 +- .../src/EventDispatcher/UpdateGeoLiteDb.php | 43 +++++++ .../test/EventDispatcher/LocateVisitTest.php | 63 ---------- .../EventDispatcher/UpdateGeoLiteDbTest.php | 114 ++++++++++++++++++ 6 files changed, 163 insertions(+), 66 deletions(-) create mode 100644 module/Core/src/EventDispatcher/UpdateGeoLiteDb.php create mode 100644 module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php index 67678d4d..a71ee410 100644 --- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -181,7 +181,7 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat try { $this->dbUpdater->checkDbUpdate(function (bool $olderDbExists): void { $this->io->writeln( - sprintf('%s GeoLite2 database...', $olderDbExists ? 'Updating' : 'Downloading'), + sprintf('%s GeoLite2 db file...', $olderDbExists ? 'Updating' : 'Downloading'), ); $this->progressBar = new ProgressBar($this->io); }, function (int $total, int $downloaded): void { diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php index d5ee2982..be6846a8 100644 --- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php @@ -227,7 +227,7 @@ class LocateVisitsCommandTest extends TestCase $output = $this->commandTester->getDisplay(); self::assertStringContainsString( - sprintf('%s GeoLite2 database...', $olderDbExists ? 'Updating' : 'Downloading'), + sprintf('%s GeoLite2 db file...', $olderDbExists ? 'Updating' : 'Downloading'), $output, ); self::assertStringContainsString($expectedMessage, $output); diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 610412fe..bddd59f5 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core; use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory; use Psr\EventDispatcher\EventDispatcherInterface; +use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Symfony\Component\Mercure\Hub; @@ -22,7 +23,7 @@ return [ EventDispatcher\Event\VisitLocated::class => [ EventDispatcher\NotifyVisitToMercure::class, EventDispatcher\NotifyVisitToWebHooks::class, -// EventDispatcher\UpdateGeoLiteDb::class, + EventDispatcher\UpdateGeoLiteDb::class, ], ], ], @@ -32,6 +33,7 @@ return [ EventDispatcher\LocateVisit::class => ConfigAbstractFactory::class, EventDispatcher\NotifyVisitToWebHooks::class => ConfigAbstractFactory::class, EventDispatcher\NotifyVisitToMercure::class => ConfigAbstractFactory::class, + EventDispatcher\UpdateGeoLiteDb::class => ConfigAbstractFactory::class, ], 'delegators' => [ @@ -66,6 +68,7 @@ return [ 'em', 'Logger_Shlink', ], + EventDispatcher\UpdateGeoLiteDb::class => [GeolocationDbUpdater::class, 'Logger_Shlink'], ], ]; diff --git a/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php new file mode 100644 index 00000000..7b985026 --- /dev/null +++ b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php @@ -0,0 +1,43 @@ +dbUpdater = $dbUpdater; + $this->logger = $logger; + } + + public function __invoke(): void + { + $beforeDownload = fn (bool $olderDbExists) => $this->logger->notice( + sprintf('%s GeoLite2 db file...', $olderDbExists ? 'Updating' : 'Downloading'), + ); + $handleProgress = function (int $total, int $downloaded, bool $olderDbExists): void { + if ($total > $downloaded) { + return; + } + + $this->logger->notice(sprintf('Finished %s GeoLite2 db file', $olderDbExists ? 'updating' : 'downloading')); + }; + + try { + $this->dbUpdater->checkDbUpdate($beforeDownload, $handleProgress); + } catch (Throwable $e) { + $this->logger->error('GeoLite2 database download failed. {e}', ['e' => $e]); + } + } +} diff --git a/module/Core/test/EventDispatcher/LocateVisitTest.php b/module/Core/test/EventDispatcher/LocateVisitTest.php index b56cd97f..68c7496f 100644 --- a/module/Core/test/EventDispatcher/LocateVisitTest.php +++ b/module/Core/test/EventDispatcher/LocateVisitTest.php @@ -226,67 +226,4 @@ class LocateVisitTest extends TestCase yield 'invalid short url' => [Visit::forInvalidShortUrl(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4']; yield 'regular not found' => [Visit::forRegularNotFound(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4']; } - -// /** @test */ -// public function errorWhenUpdatingGeoLiteWithExistingCopyLogsWarning(): void -// { -// $e = GeolocationDbUpdateFailedException::withOlderDb(); -// $ipAddr = '1.2.3.0'; -// $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr, '')); -// $location = new Location('', '', '', '', 0.0, 0.0, ''); -// $event = new UrlVisited('123'); -// -// $findVisit = $this->em->find(Visit::class, '123')->willReturn($visit); -// $flush = $this->em->flush()->will(function (): void { -// }); -// $resolveIp = $this->ipLocationResolver->resolveIpLocation($ipAddr)->willReturn($location); -// $checkUpdateDb = $this->dbUpdater->checkDbUpdate(Argument::cetera())->willThrow($e); -// $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function (): void { -// }); -// -// ($this->locateVisit)($event); -// -// self::assertEquals($visit->getVisitLocation(), new VisitLocation($location)); -// $findVisit->shouldHaveBeenCalledOnce(); -// $flush->shouldHaveBeenCalledOnce(); -// $resolveIp->shouldHaveBeenCalledOnce(); -// $checkUpdateDb->shouldHaveBeenCalledOnce(); -// $this->logger->warning( -// 'GeoLite2 database update failed. Proceeding with old version. {e}', -// ['e' => $e], -// )->shouldHaveBeenCalledOnce(); -// $dispatch->shouldHaveBeenCalledOnce(); -// } -// -// /** @test */ -// public function errorWhenDownloadingGeoLiteCancelsLocation(): void -// { -// $e = GeolocationDbUpdateFailedException::withoutOlderDb(); -// $ipAddr = '1.2.3.0'; -// $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr, '')); -// $location = new Location('', '', '', '', 0.0, 0.0, ''); -// $event = new UrlVisited('123'); -// -// $findVisit = $this->em->find(Visit::class, '123')->willReturn($visit); -// $flush = $this->em->flush()->will(function (): void { -// }); -// $resolveIp = $this->ipLocationResolver->resolveIpLocation($ipAddr)->willReturn($location); -// $checkUpdateDb = $this->dbUpdater->checkDbUpdate(Argument::cetera())->willThrow($e); -// $logError = $this->logger->error( -// 'GeoLite2 database download failed. It is not possible to locate visit with id {visitId}. {e}', -// ['e' => $e, 'visitId' => 123], -// ); -// $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function (): void { -// }); -// -// ($this->locateVisit)($event); -// -// self::assertNull($visit->getVisitLocation()); -// $findVisit->shouldHaveBeenCalledOnce(); -// $flush->shouldNotHaveBeenCalled(); -// $resolveIp->shouldNotHaveBeenCalled(); -// $checkUpdateDb->shouldHaveBeenCalledOnce(); -// $logError->shouldHaveBeenCalledOnce(); -// $dispatch->shouldHaveBeenCalledOnce(); -// } } diff --git a/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php b/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php new file mode 100644 index 00000000..a5edcbcb --- /dev/null +++ b/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php @@ -0,0 +1,114 @@ +dbUpdater = $this->prophesize(GeolocationDbUpdaterInterface::class); + $this->logger = $this->prophesize(LoggerInterface::class); + + $this->listener = new UpdateGeoLiteDb($this->dbUpdater->reveal(), $this->logger->reveal()); + } + + /** @test */ + public function exceptionWhileUpdatingDbLogsError(): void + { + $e = new RuntimeException(); + + $checkDbUpdate = $this->dbUpdater->checkDbUpdate(Argument::cetera())->willThrow($e); + $logError = $this->logger->error('GeoLite2 database download failed. {e}', ['e' => $e]); + + ($this->listener)(); + + $checkDbUpdate->shouldHaveBeenCalledOnce(); + $logError->shouldHaveBeenCalledOnce(); + $this->logger->notice(Argument::cetera())->shouldNotHaveBeenCalled(); + } + + /** + * @test + * @dataProvider provideFlags + */ + public function noticeMessageIsPrintedWhenFirstCallbackIsInvoked(bool $oldDbExists, string $expectedMessage): void + { + $checkDbUpdate = $this->dbUpdater->checkDbUpdate(Argument::cetera())->will( + function (array $args) use ($oldDbExists): void { + [$firstCallback] = $args; + $firstCallback($oldDbExists); + }, + ); + $logNotice = $this->logger->notice($expectedMessage); + + ($this->listener)(); + + $checkDbUpdate->shouldHaveBeenCalledOnce(); + $logNotice->shouldHaveBeenCalledOnce(); + $this->logger->error(Argument::cetera())->shouldNotHaveBeenCalled(); + } + + public function provideFlags(): iterable + { + yield 'existing old db' => [true, 'Updating GeoLite2 db file...']; + yield 'not existing old db' => [false, 'Downloading GeoLite2 db file...']; + } + + /** + * @test + * @dataProvider provideDownloaded + */ + public function noticeMessageIsPrintedWhenSecondCallbackIsInvoked( + int $total, + int $downloaded, + bool $oldDbExists, + ?string $expectedMessage + ): void { + $checkDbUpdate = $this->dbUpdater->checkDbUpdate(Argument::cetera())->will( + function (array $args) use ($total, $downloaded, $oldDbExists): void { + [, $secondCallback] = $args; + $secondCallback($total, $downloaded, $oldDbExists); + }, + ); + $logNotice = $this->logger->notice($expectedMessage ?? Argument::cetera()); + + ($this->listener)(); + + if ($expectedMessage !== null) { + $logNotice->shouldHaveBeenCalledOnce(); + } else { + $logNotice->shouldNotHaveBeenCalled(); + } + $checkDbUpdate->shouldHaveBeenCalledOnce(); + $this->logger->error(Argument::cetera())->shouldNotHaveBeenCalled(); + } + + public function provideDownloaded(): iterable + { + yield [100, 0, true, null]; + yield [100, 0, false, null]; + yield [100, 99, true, null]; + yield [100, 99, false, null]; + yield [100, 100, true, 'Finished updating GeoLite2 db file']; + yield [100, 100, false, 'Finished downloading GeoLite2 db file']; + yield [100, 101, true, 'Finished updating GeoLite2 db file']; + yield [100, 101, false, 'Finished downloading GeoLite2 db file']; + } +}