From 59bcd627175d5f8e86da0e4be05c8aeb4e598fcf Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 18 Sep 2022 10:01:22 +0200 Subject: [PATCH 01/10] Moved Geolocation services to its own namespace inside CLI module --- module/CLI/config/dependencies.config.php | 6 +++--- module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php | 2 +- module/CLI/src/{Util => GeoLite}/GeolocationDbUpdater.php | 2 +- .../src/{Util => GeoLite}/GeolocationDbUpdaterInterface.php | 2 +- .../CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php | 2 +- .../CLI/test/{Util => GeoLite}/GeolocationDbUpdaterTest.php | 4 ++-- module/Core/config/event_dispatcher.config.php | 2 +- module/Core/src/Action/AbstractTrackingAction.php | 4 ++-- module/Core/src/EventDispatcher/UpdateGeoLiteDb.php | 2 +- module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php | 2 +- 10 files changed, 14 insertions(+), 14 deletions(-) rename module/CLI/src/{Util => GeoLite}/GeolocationDbUpdater.php (98%) rename module/CLI/src/{Util => GeoLite}/GeolocationDbUpdaterInterface.php (89%) rename module/CLI/test/{Util => GeoLite}/GeolocationDbUpdaterTest.php (98%) diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 6920e839..38368746 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -35,7 +35,7 @@ return [ SymfonyCli\Helper\ProcessHelper::class => ProcessHelperFactory::class, PhpExecutableFinder::class => InvokableFactory::class, - Util\GeolocationDbUpdater::class => ConfigAbstractFactory::class, + GeoLite\GeolocationDbUpdater::class => ConfigAbstractFactory::class, Util\ProcessRunner::class => ConfigAbstractFactory::class, ApiKey\RoleResolver::class => ConfigAbstractFactory::class, @@ -70,7 +70,7 @@ return [ ], ConfigAbstractFactory::class => [ - Util\GeolocationDbUpdater::class => [ + GeoLite\GeolocationDbUpdater::class => [ DbUpdater::class, Reader::class, LOCAL_LOCK_FACTORY, @@ -92,7 +92,7 @@ return [ Command\ShortUrl\GetShortUrlVisitsCommand::class => [Visit\VisitsStatsHelper::class], Command\ShortUrl\DeleteShortUrlCommand::class => [Service\ShortUrl\DeleteShortUrlService::class], - Command\Visit\DownloadGeoLiteDbCommand::class => [Util\GeolocationDbUpdater::class], + Command\Visit\DownloadGeoLiteDbCommand::class => [GeoLite\GeolocationDbUpdater::class], Command\Visit\LocateVisitsCommand::class => [ Visit\VisitLocator::class, IpLocationResolverInterface::class, diff --git a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php index 41fb5f8d..c4384d33 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -5,8 +5,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; +use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; use Shlinkio\Shlink\CLI\Util\ExitCodes; -use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputInterface; diff --git a/module/CLI/src/Util/GeolocationDbUpdater.php b/module/CLI/src/GeoLite/GeolocationDbUpdater.php similarity index 98% rename from module/CLI/src/Util/GeolocationDbUpdater.php rename to module/CLI/src/GeoLite/GeolocationDbUpdater.php index 913ad438..8061c1a0 100644 --- a/module/CLI/src/Util/GeolocationDbUpdater.php +++ b/module/CLI/src/GeoLite/GeolocationDbUpdater.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Shlinkio\Shlink\CLI\Util; +namespace Shlinkio\Shlink\CLI\GeoLite; use Cake\Chronos\Chronos; use GeoIp2\Database\Reader; diff --git a/module/CLI/src/Util/GeolocationDbUpdaterInterface.php b/module/CLI/src/GeoLite/GeolocationDbUpdaterInterface.php similarity index 89% rename from module/CLI/src/Util/GeolocationDbUpdaterInterface.php rename to module/CLI/src/GeoLite/GeolocationDbUpdaterInterface.php index 714f6a11..f7d5ecba 100644 --- a/module/CLI/src/Util/GeolocationDbUpdaterInterface.php +++ b/module/CLI/src/GeoLite/GeolocationDbUpdaterInterface.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Shlinkio\Shlink\CLI\Util; +namespace Shlinkio\Shlink\CLI\GeoLite; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; diff --git a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php index 62ea161a..28977ba8 100644 --- a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php +++ b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php @@ -9,8 +9,8 @@ use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Visit\DownloadGeoLiteDbCommand; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; +use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; use Shlinkio\Shlink\CLI\Util\ExitCodes; -use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface; use ShlinkioTest\Shlink\CLI\CliTestUtilsTrait; use Symfony\Component\Console\Tester\CommandTester; diff --git a/module/CLI/test/Util/GeolocationDbUpdaterTest.php b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php similarity index 98% rename from module/CLI/test/Util/GeolocationDbUpdaterTest.php rename to module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php index a884dd7c..9f09ee3b 100644 --- a/module/CLI/test/Util/GeolocationDbUpdaterTest.php +++ b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace ShlinkioTest\Shlink\CLI\Util; +namespace ShlinkioTest\Shlink\CLI\GeoLite; use Cake\Chronos\Chronos; use GeoIp2\Database\Reader; @@ -12,7 +12,7 @@ use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; -use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater; +use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdater; use Shlinkio\Shlink\Core\Options\TrackingOptions; use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 467f63cc..4b6c9048 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core; use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory; use Psr\EventDispatcher\EventDispatcherInterface; -use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater; +use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdater; use Shlinkio\Shlink\Common\Cache\RedisPublishingHelper; use Shlinkio\Shlink\Common\Mercure\MercureHubPublishingHelper; use Shlinkio\Shlink\Common\RabbitMq\RabbitMqPublishingHelper; diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 8e9aaa09..0bf86258 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -19,8 +19,8 @@ use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMethodInterface { public function __construct( - private ShortUrlResolverInterface $urlResolver, - private RequestTrackerInterface $requestTracker, + private readonly ShortUrlResolverInterface $urlResolver, + private readonly RequestTrackerInterface $requestTracker, ) { } diff --git a/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php index 13941f43..b75b8a5d 100644 --- a/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php +++ b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php @@ -5,7 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\EventDispatcher; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface; +use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; use Throwable; use function sprintf; diff --git a/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php b/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php index 178a142f..f9ba94fe 100644 --- a/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php +++ b/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php @@ -10,7 +10,7 @@ use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Psr\Log\LoggerInterface; use RuntimeException; -use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface; +use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; use Shlinkio\Shlink\Core\EventDispatcher\UpdateGeoLiteDb; class UpdateGeoLiteDbTest extends TestCase From eab9347522e77b4646f4f8c762132675f2258bbc Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 18 Sep 2022 10:31:14 +0200 Subject: [PATCH 02/10] Created enum to determine what was the result of updating a geolite DB --- module/CLI/src/GeoLite/GeolocationDbUpdater.php | 13 ++++++++----- .../src/GeoLite/GeolocationDbUpdaterInterface.php | 5 ++++- module/CLI/src/GeoLite/GeolocationResult.php | 11 +++++++++++ .../Command/Visit/DownloadGeoLiteDbCommandTest.php | 8 +++++--- .../CLI/test/GeoLite/GeolocationDbUpdaterTest.php | 9 ++++++--- .../test/EventDispatcher/UpdateGeoLiteDbTest.php | 9 +++++++-- 6 files changed, 41 insertions(+), 14 deletions(-) create mode 100644 module/CLI/src/GeoLite/GeolocationResult.php diff --git a/module/CLI/src/GeoLite/GeolocationDbUpdater.php b/module/CLI/src/GeoLite/GeolocationDbUpdater.php index 8061c1a0..b04ee3f2 100644 --- a/module/CLI/src/GeoLite/GeolocationDbUpdater.php +++ b/module/CLI/src/GeoLite/GeolocationDbUpdater.php @@ -30,17 +30,17 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface /** * @throws GeolocationDbUpdateFailedException */ - public function checkDbUpdate(?callable $beforeDownload = null, ?callable $handleProgress = null): void + public function checkDbUpdate(?callable $beforeDownload = null, ?callable $handleProgress = null): GeolocationResult { if ($this->trackingOptions->disableTracking || $this->trackingOptions->disableIpTracking) { - return; + return GeolocationResult::CHECK_SKIPPED; } $lock = $this->locker->createLock(self::LOCK_NAME); $lock->acquire(true); // Block until lock is released try { - $this->downloadIfNeeded($beforeDownload, $handleProgress); + return $this->downloadIfNeeded($beforeDownload, $handleProgress); } finally { $lock->release(); } @@ -49,17 +49,20 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface /** * @throws GeolocationDbUpdateFailedException */ - private function downloadIfNeeded(?callable $beforeDownload, ?callable $handleProgress): void + private function downloadIfNeeded(?callable $beforeDownload, ?callable $handleProgress): GeolocationResult { if (! $this->dbUpdater->databaseFileExists()) { $this->downloadNewDb(false, $beforeDownload, $handleProgress); - return; + return GeolocationResult::DB_CREATED; } $meta = $this->geoLiteDbReader->metadata(); if ($this->buildIsTooOld($meta)) { $this->downloadNewDb(true, $beforeDownload, $handleProgress); + return GeolocationResult::DB_UPDATED; } + + return GeolocationResult::DB_IS_UP_TO_DATE; } private function buildIsTooOld(Metadata $meta): bool diff --git a/module/CLI/src/GeoLite/GeolocationDbUpdaterInterface.php b/module/CLI/src/GeoLite/GeolocationDbUpdaterInterface.php index f7d5ecba..a143abb8 100644 --- a/module/CLI/src/GeoLite/GeolocationDbUpdaterInterface.php +++ b/module/CLI/src/GeoLite/GeolocationDbUpdaterInterface.php @@ -11,5 +11,8 @@ interface GeolocationDbUpdaterInterface /** * @throws GeolocationDbUpdateFailedException */ - public function checkDbUpdate(?callable $beforeDownload = null, ?callable $handleProgress = null): void; + public function checkDbUpdate( + ?callable $beforeDownload = null, + ?callable $handleProgress = null, + ): GeolocationResult; } diff --git a/module/CLI/src/GeoLite/GeolocationResult.php b/module/CLI/src/GeoLite/GeolocationResult.php new file mode 100644 index 00000000..7b245943 --- /dev/null +++ b/module/CLI/src/GeoLite/GeolocationResult.php @@ -0,0 +1,11 @@ + [function (): void { - }, '[INFO] GeoLite2 db file is up to date.']; - yield 'outdated db' => [function (array $args): void { + yield 'up to date db' => [fn () => GeolocationResult::CHECK_SKIPPED, '[INFO] GeoLite2 db file is up to date.']; + yield 'outdated db' => [function (array $args): GeolocationResult { [$beforeDownload] = $args; $beforeDownload(true); + + return GeolocationResult::DB_CREATED; }, '[OK] GeoLite2 db file properly downloaded.']; } } diff --git a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php index 9f09ee3b..89667f3b 100644 --- a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php +++ b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php @@ -13,6 +13,7 @@ use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdater; +use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; use Shlinkio\Shlink\Core\Options\TrackingOptions; use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; @@ -110,15 +111,16 @@ class GeolocationDbUpdaterTest extends TestCase * @test * @dataProvider provideSmallDays */ - public function databaseIsNotUpdatedIfItIsYoungerThanOneWeek(string|int $buildEpoch): void + public function databaseIsNotUpdatedIfItIsNewEnough(string|int $buildEpoch): void { $fileExists = $this->dbUpdater->databaseFileExists()->willReturn(true); $getMeta = $this->geoLiteDbReader->metadata()->willReturn($this->buildMetaWithBuildEpoch($buildEpoch)); $download = $this->dbUpdater->downloadFreshCopy(null)->will(function (): void { }); - $this->geolocationDbUpdater()->checkDbUpdate(); + $result = $this->geolocationDbUpdater()->checkDbUpdate(); + self::assertEquals(GeolocationResult::DB_IS_UP_TO_DATE, $result); $fileExists->shouldHaveBeenCalledOnce(); $getMeta->shouldHaveBeenCalledOnce(); $download->shouldNotHaveBeenCalled(); @@ -174,8 +176,9 @@ class GeolocationDbUpdaterTest extends TestCase */ public function downloadDbIsSkippedIfTrackingIsDisabled(TrackingOptions $options): void { - $this->geolocationDbUpdater($options)->checkDbUpdate(); + $result = $this->geolocationDbUpdater($options)->checkDbUpdate(); + self::assertEquals(GeolocationResult::CHECK_SKIPPED, $result); $this->dbUpdater->databaseFileExists(Argument::cetera())->shouldNotHaveBeenCalled(); $this->geoLiteDbReader->metadata(Argument::cetera())->shouldNotHaveBeenCalled(); } diff --git a/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php b/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php index f9ba94fe..a811fdc4 100644 --- a/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php +++ b/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php @@ -11,6 +11,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Psr\Log\LoggerInterface; use RuntimeException; use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; +use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; use Shlinkio\Shlink\Core\EventDispatcher\UpdateGeoLiteDb; class UpdateGeoLiteDbTest extends TestCase @@ -51,9 +52,11 @@ class UpdateGeoLiteDbTest extends TestCase public function noticeMessageIsPrintedWhenFirstCallbackIsInvoked(bool $oldDbExists, string $expectedMessage): void { $checkDbUpdate = $this->dbUpdater->checkDbUpdate(Argument::cetera())->will( - function (array $args) use ($oldDbExists): void { + function (array $args) use ($oldDbExists): GeolocationResult { [$firstCallback] = $args; $firstCallback($oldDbExists); + + return GeolocationResult::DB_CREATED; }, ); $logNotice = $this->logger->notice($expectedMessage); @@ -82,13 +85,15 @@ class UpdateGeoLiteDbTest extends TestCase ?string $expectedMessage, ): void { $checkDbUpdate = $this->dbUpdater->checkDbUpdate(Argument::cetera())->will( - function (array $args) use ($total, $downloaded, $oldDbExists): void { + function (array $args) use ($total, $downloaded, $oldDbExists): GeolocationResult { [, $secondCallback] = $args; // Invoke several times to ensure the log is printed only once $secondCallback($total, $downloaded, $oldDbExists); $secondCallback($total, $downloaded, $oldDbExists); $secondCallback($total, $downloaded, $oldDbExists); + + return GeolocationResult::DB_CREATED; }, ); $logNotice = $this->logger->notice($expectedMessage ?? Argument::cetera()); From ef01754ad5f0e7e703b75343e635412ebf64ba05 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 18 Sep 2022 11:17:17 +0200 Subject: [PATCH 03/10] Added event dispatching to UpdateGeoLiteDb dispatcher so that it locates visits when file has just been created --- ...l-geolite2-db-download-in-docker-images.md | 0 docs/adr/README.md | 1 + .../Core/config/event_dispatcher.config.php | 9 +++- .../Event/GeoLiteDbCreated.php | 9 ++++ .../src/EventDispatcher/UpdateGeoLiteDb.php | 15 +++++-- .../EventDispatcher/UpdateGeoLiteDbTest.php | 43 +++++++++++++++++-- 6 files changed, 70 insertions(+), 7 deletions(-) create mode 100644 docs/adr/2022-09-18-allow-delaying-initial-geolite2-db-download-in-docker-images.md create mode 100644 module/Core/src/EventDispatcher/Event/GeoLiteDbCreated.php diff --git a/docs/adr/2022-09-18-allow-delaying-initial-geolite2-db-download-in-docker-images.md b/docs/adr/2022-09-18-allow-delaying-initial-geolite2-db-download-in-docker-images.md new file mode 100644 index 00000000..e69de29b diff --git a/docs/adr/README.md b/docs/adr/README.md index 7cfccdf7..f12d17c1 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -2,6 +2,7 @@ Here listed you will find the different architectural decisions taken in the project, including all the reasoning behind it, options considered, and final outcome. +* [2022-09-18 Allow delaying initial GeoLite2 DB download in docker images](2022-09-18-allow-delaying-initial-geolite2-db-download-in-docker-images.md) * [2022-08-05 Support multi-segment custom slugs](2022-08-05-support-multi-segment-custom-slugs.md) * [2022-01-15 Update env vars behavior to have precedence over installer options](2022-01-15-update-env-vars-behavior-to-have-precedence-over-installer-options.md) * [2021-08-05 Migrate to a new caching library](2021-08-05-migrate-to-a-new-caching-library.md) diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 4b6c9048..3a4d10f5 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -20,6 +20,9 @@ return [ EventDispatcher\Event\UrlVisited::class => [ EventDispatcher\LocateVisit::class, ], + EventDispatcher\Event\GeoLiteDbCreated::class => [ +// EventDispatcher\LocateUnloctedVisits::class, + ], ], 'async' => [ EventDispatcher\Event\VisitLocated::class => [ @@ -132,7 +135,11 @@ return [ 'Logger_Shlink', 'config.redis.pub_sub_enabled', ], - EventDispatcher\UpdateGeoLiteDb::class => [GeolocationDbUpdater::class, 'Logger_Shlink'], + EventDispatcher\UpdateGeoLiteDb::class => [ + GeolocationDbUpdater::class, + 'Logger_Shlink', + EventDispatcherInterface::class, + ], ], ]; diff --git a/module/Core/src/EventDispatcher/Event/GeoLiteDbCreated.php b/module/Core/src/EventDispatcher/Event/GeoLiteDbCreated.php new file mode 100644 index 00000000..3fc86cd7 --- /dev/null +++ b/module/Core/src/EventDispatcher/Event/GeoLiteDbCreated.php @@ -0,0 +1,9 @@ +dbUpdater->checkDbUpdate($beforeDownload, $handleProgress); + $result = $this->dbUpdater->checkDbUpdate($beforeDownload, $handleProgress); + if ($result === GeolocationResult::DB_CREATED) { + $this->eventDispatcher->dispatch(new GeoLiteDbCreated()); + } } catch (Throwable $e) { $this->logger->error('GeoLite2 database download failed. {e}', ['e' => $e]); } diff --git a/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php b/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php index a811fdc4..9ce20801 100644 --- a/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php +++ b/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php @@ -8,12 +8,16 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; +use Psr\EventDispatcher\EventDispatcherInterface; use Psr\Log\LoggerInterface; use RuntimeException; use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; +use Shlinkio\Shlink\Core\EventDispatcher\Event\GeoLiteDbCreated; use Shlinkio\Shlink\Core\EventDispatcher\UpdateGeoLiteDb; +use function Functional\map; + class UpdateGeoLiteDbTest extends TestCase { use ProphecyTrait; @@ -21,13 +25,19 @@ class UpdateGeoLiteDbTest extends TestCase private UpdateGeoLiteDb $listener; private ObjectProphecy $dbUpdater; private ObjectProphecy $logger; + private ObjectProphecy $eventDispatcher; protected function setUp(): void { $this->dbUpdater = $this->prophesize(GeolocationDbUpdaterInterface::class); $this->logger = $this->prophesize(LoggerInterface::class); + $this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class); - $this->listener = new UpdateGeoLiteDb($this->dbUpdater->reveal(), $this->logger->reveal()); + $this->listener = new UpdateGeoLiteDb( + $this->dbUpdater->reveal(), + $this->logger->reveal(), + $this->eventDispatcher->reveal(), + ); } /** @test */ @@ -43,6 +53,7 @@ class UpdateGeoLiteDbTest extends TestCase $checkDbUpdate->shouldHaveBeenCalledOnce(); $logError->shouldHaveBeenCalledOnce(); $this->logger->notice(Argument::cetera())->shouldNotHaveBeenCalled(); + $this->eventDispatcher->dispatch(Argument::cetera())->shouldNotHaveBeenCalled(); } /** @@ -56,7 +67,7 @@ class UpdateGeoLiteDbTest extends TestCase [$firstCallback] = $args; $firstCallback($oldDbExists); - return GeolocationResult::DB_CREATED; + return GeolocationResult::DB_IS_UP_TO_DATE; }, ); $logNotice = $this->logger->notice($expectedMessage); @@ -66,6 +77,7 @@ class UpdateGeoLiteDbTest extends TestCase $checkDbUpdate->shouldHaveBeenCalledOnce(); $logNotice->shouldHaveBeenCalledOnce(); $this->logger->error(Argument::cetera())->shouldNotHaveBeenCalled(); + $this->eventDispatcher->dispatch(Argument::cetera())->shouldNotHaveBeenCalled(); } public function provideFlags(): iterable @@ -93,7 +105,7 @@ class UpdateGeoLiteDbTest extends TestCase $secondCallback($total, $downloaded, $oldDbExists); $secondCallback($total, $downloaded, $oldDbExists); - return GeolocationResult::DB_CREATED; + return GeolocationResult::DB_UPDATED; }, ); $logNotice = $this->logger->notice($expectedMessage ?? Argument::cetera()); @@ -107,6 +119,7 @@ class UpdateGeoLiteDbTest extends TestCase } $checkDbUpdate->shouldHaveBeenCalledOnce(); $this->logger->error(Argument::cetera())->shouldNotHaveBeenCalled(); + $this->eventDispatcher->dispatch(Argument::cetera())->shouldNotHaveBeenCalled(); } public function provideDownloaded(): iterable @@ -120,4 +133,28 @@ class UpdateGeoLiteDbTest extends TestCase yield [100, 101, true, 'Finished updating GeoLite2 db file']; yield [100, 101, false, 'Finished downloading GeoLite2 db file']; } + + /** + * @test + * @dataProvider provideGeolocationResults + */ + public function dispatchesEventOnlyWhenDbFileHasBeenCreatedForTheFirstTime( + GeolocationResult $result, + int $expectedDispatches, + ): void { + $checkDbUpdate = $this->dbUpdater->checkDbUpdate(Argument::cetera())->willReturn($result); + + ($this->listener)(); + + $checkDbUpdate->shouldHaveBeenCalledOnce(); + $this->eventDispatcher->dispatch(new GeoLiteDbCreated())->shouldHaveBeenCalledTimes($expectedDispatches); + } + + public function provideGeolocationResults(): iterable + { + return map(GeolocationResult::cases(), static fn (GeolocationResult $value) => [ + $value, + $value === GeolocationResult::DB_CREATED ? 1 : 0, + ]); + } } From 6f17f70137d1da868a6d3a15be5fa6dd6ba6a9c1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 18 Sep 2022 17:00:03 +0200 Subject: [PATCH 04/10] Allowed to delay GeoLite2 db download on docker images --- composer.json | 2 +- docker/docker-entrypoint.sh | 6 +- module/CLI/config/dependencies.config.php | 2 + .../src/Command/Visit/LocateVisitsCommand.php | 4 +- .../CLI/src/GeoLite/GeolocationDbUpdater.php | 33 ++++++----- .../test/GeoLite/GeolocationDbUpdaterTest.php | 6 +- .../Core/config/event_dispatcher.config.php | 8 ++- .../EventDispatcher/LocateUnlocatedVisits.php | 57 +++++++++++++++++++ 8 files changed, 95 insertions(+), 23 deletions(-) create mode 100644 module/Core/src/EventDispatcher/LocateUnlocatedVisits.php diff --git a/composer.json b/composer.json index d6122753..c99678ed 100644 --- a/composer.json +++ b/composer.json @@ -50,7 +50,7 @@ "shlinkio/shlink-event-dispatcher": "dev-main#48c0137 as 2.6", "shlinkio/shlink-importer": "^4.0", "shlinkio/shlink-installer": "dev-develop#a01bca9 as 8.2", - "shlinkio/shlink-ip-geolocation": "^3.0", + "shlinkio/shlink-ip-geolocation": "^3.1", "spiral/roadrunner": "^2.11", "spiral/roadrunner-jobs": "^2.3", "symfony/console": "^6.1", diff --git a/docker/docker-entrypoint.sh b/docker/docker-entrypoint.sh index f28627d2..fb8b7bf2 100644 --- a/docker/docker-entrypoint.sh +++ b/docker/docker-entrypoint.sh @@ -18,14 +18,14 @@ php bin/doctrine orm:generate-proxies -n ${flags} echo "Clearing entities cache..." php bin/doctrine orm:clear-cache:metadata -n ${flags} -# Try to download GeoLite2 db file only if the license key env var was defined -if [ ! -z "${GEOLITE_LICENSE_KEY}" ]; then +# Try to download GeoLite2 db file only if the license key env var was defined and skipping was not explicitly set +if [ ! -z "${GEOLITE_LICENSE_KEY}" ] && [ "${SKIP_INITIAL_GEOLITE_DOWNLOAD}" != "true" ]; then echo "Downloading GeoLite2 db file..." php bin/cli visit:download-db -n ${flags} fi # Periodically run visit:locate every hour, if ENABLE_PERIODIC_VISIT_LOCATE=true was provided -if [ $ENABLE_PERIODIC_VISIT_LOCATE ]; then +if [ "${ENABLE_PERIODIC_VISIT_LOCATE}" = "true" ]; then echo "Configuring periodic visit location..." echo "0 * * * * php /etc/shlink/bin/cli visit:locate -q" > /etc/crontabs/root /usr/sbin/crond & diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 38368746..a9de1f0c 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -19,6 +19,7 @@ use Shlinkio\Shlink\Core\Tag\TagService; use Shlinkio\Shlink\Core\Visit; use Shlinkio\Shlink\Installer\Factory\ProcessHelperFactory; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater; +use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2Options; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Shlinkio\Shlink\Rest\Service\ApiKeyService; use Symfony\Component\Console as SymfonyCli; @@ -75,6 +76,7 @@ return [ Reader::class, LOCAL_LOCK_FACTORY, TrackingOptions::class, + GeoLite2Options::class, ], Util\ProcessRunner::class => [SymfonyCli\Helper\ProcessHelper::class], ApiKey\RoleResolver::class => [DomainService::class, 'config.url_shortener.domain.hostname'], diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php index fe898dbb..47aa50d1 100644 --- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -34,8 +34,8 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat private SymfonyStyle $io; public function __construct( - private VisitLocatorInterface $visitLocator, - private IpLocationResolverInterface $ipLocationResolver, + private readonly VisitLocatorInterface $visitLocator, + private readonly IpLocationResolverInterface $ipLocationResolver, LockFactory $locker, ) { parent::__construct($locker); diff --git a/module/CLI/src/GeoLite/GeolocationDbUpdater.php b/module/CLI/src/GeoLite/GeolocationDbUpdater.php index b04ee3f2..f33b8796 100644 --- a/module/CLI/src/GeoLite/GeolocationDbUpdater.php +++ b/module/CLI/src/GeoLite/GeolocationDbUpdater.php @@ -9,7 +9,9 @@ use GeoIp2\Database\Reader; use MaxMind\Db\Reader\Metadata; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\Core\Options\TrackingOptions; -use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException; +use Shlinkio\Shlink\IpGeolocation\Exception\DbUpdateException; +use Shlinkio\Shlink\IpGeolocation\Exception\MissingLicenseException; +use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; use Symfony\Component\Lock\LockFactory; @@ -20,10 +22,10 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface private const LOCK_NAME = 'geolocation-db-update'; public function __construct( - private DbUpdaterInterface $dbUpdater, - private Reader $geoLiteDbReader, - private LockFactory $locker, - private TrackingOptions $trackingOptions, + private readonly DbUpdaterInterface $dbUpdater, + private readonly Reader $geoLiteDbReader, + private readonly LockFactory $locker, + private readonly TrackingOptions $trackingOptions, ) { } @@ -52,14 +54,12 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface private function downloadIfNeeded(?callable $beforeDownload, ?callable $handleProgress): GeolocationResult { if (! $this->dbUpdater->databaseFileExists()) { - $this->downloadNewDb(false, $beforeDownload, $handleProgress); - return GeolocationResult::DB_CREATED; + return $this->downloadNewDb(false, $beforeDownload, $handleProgress); } $meta = $this->geoLiteDbReader->metadata(); if ($this->buildIsTooOld($meta)) { - $this->downloadNewDb(true, $beforeDownload, $handleProgress); - return GeolocationResult::DB_UPDATED; + return $this->downloadNewDb(true, $beforeDownload, $handleProgress); } return GeolocationResult::DB_IS_UP_TO_DATE; @@ -95,15 +95,22 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface /** * @throws GeolocationDbUpdateFailedException */ - private function downloadNewDb(bool $olderDbExists, ?callable $beforeDownload, ?callable $handleProgress): void - { + private function downloadNewDb( + bool $olderDbExists, + ?callable $beforeDownload, + ?callable $handleProgress, + ): GeolocationResult { if ($beforeDownload !== null) { $beforeDownload($olderDbExists); } try { $this->dbUpdater->downloadFreshCopy($this->wrapHandleProgressCallback($handleProgress, $olderDbExists)); - } catch (RuntimeException $e) { + return $olderDbExists ? GeolocationResult::DB_UPDATED : GeolocationResult::DB_CREATED; + } catch (MissingLicenseException) { + // If there's no license key, just ignore the error + return GeolocationResult::CHECK_SKIPPED; + } catch (DbUpdateException | WrongIpException $e) { throw $olderDbExists ? GeolocationDbUpdateFailedException::withOlderDb($e) : GeolocationDbUpdateFailedException::withoutOlderDb($e); @@ -116,6 +123,6 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface return null; } - return fn (int $total, int $downloaded) => $handleProgress($total, $downloaded, $olderDbExists); + return static fn (int $total, int $downloaded) => $handleProgress($total, $downloaded, $olderDbExists); } } diff --git a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php index 89667f3b..61056922 100644 --- a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php +++ b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php @@ -15,7 +15,7 @@ use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdater; use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; use Shlinkio\Shlink\Core\Options\TrackingOptions; -use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException; +use Shlinkio\Shlink\IpGeolocation\Exception\DbUpdateException; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; use Symfony\Component\Lock; use Throwable; @@ -48,7 +48,7 @@ class GeolocationDbUpdaterTest extends TestCase public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(): void { $mustBeUpdated = fn () => self::assertTrue(true); - $prev = new RuntimeException(''); + $prev = new DbUpdateException(''); $fileExists = $this->dbUpdater->databaseFileExists()->willReturn(false); $getMeta = $this->geoLiteDbReader->metadata(); @@ -81,7 +81,7 @@ class GeolocationDbUpdaterTest extends TestCase $getMeta = $this->geoLiteDbReader->metadata()->willReturn($this->buildMetaWithBuildEpoch( Chronos::now()->subDays($days)->getTimestamp(), )); - $prev = new RuntimeException(''); + $prev = new DbUpdateException(''); $download = $this->dbUpdater->downloadFreshCopy(null)->willThrow($prev); try { diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 3a4d10f5..f6e198b8 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -10,6 +10,7 @@ use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdater; use Shlinkio\Shlink\Common\Cache\RedisPublishingHelper; use Shlinkio\Shlink\Common\Mercure\MercureHubPublishingHelper; use Shlinkio\Shlink\Common\RabbitMq\RabbitMqPublishingHelper; +use Shlinkio\Shlink\Core\Visit\VisitLocator; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; @@ -21,7 +22,7 @@ return [ EventDispatcher\LocateVisit::class, ], EventDispatcher\Event\GeoLiteDbCreated::class => [ -// EventDispatcher\LocateUnloctedVisits::class, + EventDispatcher\LocateUnlocatedVisits::class, ], ], 'async' => [ @@ -43,6 +44,7 @@ return [ 'dependencies' => [ 'factories' => [ EventDispatcher\LocateVisit::class => ConfigAbstractFactory::class, + EventDispatcher\LocateUnlocatedVisits::class => ConfigAbstractFactory::class, EventDispatcher\NotifyVisitToWebHooks::class => ConfigAbstractFactory::class, EventDispatcher\Mercure\NotifyVisitToMercure::class => ConfigAbstractFactory::class, EventDispatcher\Mercure\NotifyNewShortUrlToMercure::class => ConfigAbstractFactory::class, @@ -72,6 +74,9 @@ return [ EventDispatcher\RedisPubSub\NotifyNewShortUrlToRedis::class => [ EventDispatcher\CloseDbConnectionEventListenerDelegator::class, ], + EventDispatcher\LocateUnlocatedVisits::class => [ + EventDispatcher\CloseDbConnectionEventListenerDelegator::class, + ], EventDispatcher\NotifyVisitToWebHooks::class => [ EventDispatcher\CloseDbConnectionEventListenerDelegator::class, ], @@ -86,6 +91,7 @@ return [ DbUpdater::class, EventDispatcherInterface::class, ], + EventDispatcher\LocateUnlocatedVisits::class => [VisitLocator::class, IpLocationResolverInterface::class], EventDispatcher\NotifyVisitToWebHooks::class => [ 'httpClient', 'em', diff --git a/module/Core/src/EventDispatcher/LocateUnlocatedVisits.php b/module/Core/src/EventDispatcher/LocateUnlocatedVisits.php new file mode 100644 index 00000000..a72e8853 --- /dev/null +++ b/module/Core/src/EventDispatcher/LocateUnlocatedVisits.php @@ -0,0 +1,57 @@ +locator->locateUnlocatedVisits($this); + } + + /** + * @throws IpCannotBeLocatedException + */ + public function geolocateVisit(Visit $visit): Location + { + // TODO This method duplicates code from LocateVisitsCommand. Move to a common place. + if (! $visit->hasRemoteAddr()) { + throw IpCannotBeLocatedException::forEmptyAddress(); + } + + $ipAddr = $visit->getRemoteAddr() ?? ''; + if ($ipAddr === IpAddress::LOCALHOST) { + throw IpCannotBeLocatedException::forLocalhost(); + } + + try { + return $this->ipLocationResolver->resolveIpLocation($ipAddr); + } catch (WrongIpException $e) { + throw IpCannotBeLocatedException::forError($e); + } + } + + public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void + { + // Do nothing + } +} From d76e6647d29714881aa336dd11fa8fdfc7b1d2c9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 18 Sep 2022 17:10:11 +0200 Subject: [PATCH 05/10] Added real version for composer dependencies --- composer.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/composer.json b/composer.json index c99678ed..8be0e37a 100644 --- a/composer.json +++ b/composer.json @@ -45,11 +45,11 @@ "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.0", "ramsey/uuid": "^4.3", - "shlinkio/shlink-common": "dev-main#c9e6474 as 5.1", - "shlinkio/shlink-config": "dev-main#12fb295 as 2.1", - "shlinkio/shlink-event-dispatcher": "dev-main#48c0137 as 2.6", + "shlinkio/shlink-common": "^5.1", + "shlinkio/shlink-config": "^2.1", + "shlinkio/shlink-event-dispatcher": "^2.6", "shlinkio/shlink-importer": "^4.0", - "shlinkio/shlink-installer": "dev-develop#a01bca9 as 8.2", + "shlinkio/shlink-installer": "^8.2", "shlinkio/shlink-ip-geolocation": "^3.1", "spiral/roadrunner": "^2.11", "spiral/roadrunner-jobs": "^2.3", @@ -73,7 +73,7 @@ "phpunit/phpunit": "^9.5", "roave/security-advisories": "dev-master", "shlinkio/php-coding-standard": "~2.3.0", - "shlinkio/shlink-test-utils": "dev-main#404fdf6 as 3.3", + "shlinkio/shlink-test-utils": "^3.3", "symfony/var-dumper": "^6.1", "veewee/composer-run-parallel": "^1.1" }, From fe41e9d573a7c7338148047f4104d72cbbd7a001 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 18 Sep 2022 17:12:21 +0200 Subject: [PATCH 06/10] Updated changelog --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f85af37f..d491db0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## [3.3.0] - 2022-09-18 ### Added * [#1221](https://github.com/shlinkio/shlink/issues/1221) Added experimental support to run Shlink with [RoadRunner](https://roadrunner.dev) instead of openswoole. * [#1531](https://github.com/shlinkio/shlink/issues/1531) and [#1090](https://github.com/shlinkio/shlink/issues/1090) Added support for trailing slashes in short URLs. @@ -34,6 +34,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this Also, the installer tool now allows to generate an initial API key that can be copy-pasted (this tool is run interactively), in case you use php-fpm or you don't want to use env vars. +* [#1528](https://github.com/shlinkio/shlink/issues/1528) Added support to delay when the GeoLite2 DB file is downloaded in docker images, speeding up its startup time. + + In order to do it, pass `SKIP_INITIAL_GEOLITE_DOWNLOAD=true` when creating the container. + ### Changed * [#1339](https://github.com/shlinkio/shlink/issues/1339) Added new test suite for CLI E2E tests. * [#1503](https://github.com/shlinkio/shlink/issues/1503) Drastically improved build time in GitHub Actions, by optimizing parallelization and adding php extensions cache. From 83b7d5a5f1734afcd917fadbee23d7d8882b28d9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 18 Sep 2022 18:44:01 +0200 Subject: [PATCH 07/10] Extracted logic to geolocate a visit, handling possible domain errors --- module/Core/config/dependencies.config.php | 3 + .../Core/config/event_dispatcher.config.php | 3 +- .../EventDispatcher/LocateUnlocatedVisits.php | 23 +------ .../Exception/IpCannotBeLocatedException.php | 23 ++++--- .../src/Visit/Model/UnlocatableIpType.php | 10 +++ .../Core/src/Visit/VisitToLocationHelper.php | 40 +++++++++++ .../Visit/VisitToLocationHelperInterface.php | 17 +++++ .../LocateUnlocatedVisitsTest.php | 54 +++++++++++++++ .../IpCannotBeLocatedExceptionTest.php | 4 ++ module/Core/test/Visit/VisitLocatorTest.php | 2 +- .../test/Visit/VisitToLocationHelperTest.php | 66 +++++++++++++++++++ 11 files changed, 214 insertions(+), 31 deletions(-) create mode 100644 module/Core/src/Visit/Model/UnlocatableIpType.php create mode 100644 module/Core/src/Visit/VisitToLocationHelper.php create mode 100644 module/Core/src/Visit/VisitToLocationHelperInterface.php create mode 100644 module/Core/test/EventDispatcher/LocateUnlocatedVisitsTest.php create mode 100644 module/Core/test/Visit/VisitToLocationHelperTest.php diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 9855e2aa..49b2857a 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -11,6 +11,7 @@ use Shlinkio\Shlink\Config\Factory\ValinorConfigFactory; use Shlinkio\Shlink\Core\ErrorHandler; use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface; +use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; return [ @@ -44,6 +45,7 @@ return [ Visit\VisitsTracker::class => ConfigAbstractFactory::class, Visit\RequestTracker::class => ConfigAbstractFactory::class, Visit\VisitLocator::class => ConfigAbstractFactory::class, + Visit\VisitToLocationHelper::class => ConfigAbstractFactory::class, Visit\VisitsStatsHelper::class => ConfigAbstractFactory::class, Visit\Transformer\OrphanVisitDataTransformer::class => InvokableFactory::class, @@ -108,6 +110,7 @@ return [ ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class, ], Visit\VisitLocator::class => ['em'], + Visit\VisitToLocationHelper::class => [IpLocationResolverInterface::class], Visit\VisitsStatsHelper::class => ['em'], Tag\TagService::class => ['em'], Service\ShortUrl\DeleteShortUrlService::class => [ diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index f6e198b8..3d473010 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -11,6 +11,7 @@ use Shlinkio\Shlink\Common\Cache\RedisPublishingHelper; use Shlinkio\Shlink\Common\Mercure\MercureHubPublishingHelper; use Shlinkio\Shlink\Common\RabbitMq\RabbitMqPublishingHelper; use Shlinkio\Shlink\Core\Visit\VisitLocator; +use Shlinkio\Shlink\Core\Visit\VisitToLocationHelper; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; @@ -91,7 +92,7 @@ return [ DbUpdater::class, EventDispatcherInterface::class, ], - EventDispatcher\LocateUnlocatedVisits::class => [VisitLocator::class, IpLocationResolverInterface::class], + EventDispatcher\LocateUnlocatedVisits::class => [VisitLocator::class, VisitToLocationHelper::class], EventDispatcher\NotifyVisitToWebHooks::class => [ 'httpClient', 'em', diff --git a/module/Core/src/EventDispatcher/LocateUnlocatedVisits.php b/module/Core/src/EventDispatcher/LocateUnlocatedVisits.php index a72e8853..c036c450 100644 --- a/module/Core/src/EventDispatcher/LocateUnlocatedVisits.php +++ b/module/Core/src/EventDispatcher/LocateUnlocatedVisits.php @@ -4,22 +4,20 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\EventDispatcher; -use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; use Shlinkio\Shlink\Core\EventDispatcher\Event\GeoLiteDbCreated; use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException; use Shlinkio\Shlink\Core\Visit\VisitGeolocationHelperInterface; use Shlinkio\Shlink\Core\Visit\VisitLocatorInterface; -use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; +use Shlinkio\Shlink\Core\Visit\VisitToLocationHelperInterface; use Shlinkio\Shlink\IpGeolocation\Model\Location; -use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; class LocateUnlocatedVisits implements VisitGeolocationHelperInterface { public function __construct( private readonly VisitLocatorInterface $locator, - private readonly IpLocationResolverInterface $ipLocationResolver, + private readonly VisitToLocationHelperInterface $visitToLocation, ) { } @@ -33,25 +31,10 @@ class LocateUnlocatedVisits implements VisitGeolocationHelperInterface */ public function geolocateVisit(Visit $visit): Location { - // TODO This method duplicates code from LocateVisitsCommand. Move to a common place. - if (! $visit->hasRemoteAddr()) { - throw IpCannotBeLocatedException::forEmptyAddress(); - } - - $ipAddr = $visit->getRemoteAddr() ?? ''; - if ($ipAddr === IpAddress::LOCALHOST) { - throw IpCannotBeLocatedException::forLocalhost(); - } - - try { - return $this->ipLocationResolver->resolveIpLocation($ipAddr); - } catch (WrongIpException $e) { - throw IpCannotBeLocatedException::forError($e); - } + return $this->visitToLocation->resolveVisitLocation($visit); } public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void { - // Do nothing } } diff --git a/module/Core/src/Exception/IpCannotBeLocatedException.php b/module/Core/src/Exception/IpCannotBeLocatedException.php index b1ba731c..2ebc3e62 100644 --- a/module/Core/src/Exception/IpCannotBeLocatedException.php +++ b/module/Core/src/Exception/IpCannotBeLocatedException.php @@ -4,35 +4,40 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Exception; +use Shlinkio\Shlink\Core\Visit\Model\UnlocatableIpType; use Throwable; class IpCannotBeLocatedException extends RuntimeException { - private bool $isNonLocatableAddress = true; + private function __construct( + string $message, + public readonly UnlocatableIpType $type, + int $code = 0, + ?Throwable $previous = null, + ) { + parent::__construct($message, $code, $previous); + } public static function forEmptyAddress(): self { - return new self('Ignored visit with no IP address'); + return new self('Ignored visit with no IP address', UnlocatableIpType::EMPTY_ADDRESS); } public static function forLocalhost(): self { - return new self('Ignored localhost address'); + return new self('Ignored localhost address', UnlocatableIpType::LOCALHOST); } public static function forError(Throwable $e): self { - $e = new self('An error occurred while locating IP', $e->getCode(), $e); - $e->isNonLocatableAddress = false; - - return $e; + return new self('An error occurred while locating IP', UnlocatableIpType::ERROR, $e->getCode(), $e); } /** - * Tells if this error belongs to an address that will never be possible locate + * Tells if this belongs to an address that will never be possible to locate */ public function isNonLocatableAddress(): bool { - return $this->isNonLocatableAddress; + return $this->type !== UnlocatableIpType::ERROR; } } diff --git a/module/Core/src/Visit/Model/UnlocatableIpType.php b/module/Core/src/Visit/Model/UnlocatableIpType.php new file mode 100644 index 00000000..56490209 --- /dev/null +++ b/module/Core/src/Visit/Model/UnlocatableIpType.php @@ -0,0 +1,10 @@ +hasRemoteAddr()) { + throw IpCannotBeLocatedException::forEmptyAddress(); + } + + $ipAddr = $visit->getRemoteAddr() ?? ''; + if ($ipAddr === IpAddress::LOCALHOST) { + throw IpCannotBeLocatedException::forLocalhost(); + } + + try { + return $this->ipLocationResolver->resolveIpLocation($ipAddr); + } catch (WrongIpException $e) { + throw IpCannotBeLocatedException::forError($e); + } + } +} diff --git a/module/Core/src/Visit/VisitToLocationHelperInterface.php b/module/Core/src/Visit/VisitToLocationHelperInterface.php new file mode 100644 index 00000000..7d553527 --- /dev/null +++ b/module/Core/src/Visit/VisitToLocationHelperInterface.php @@ -0,0 +1,17 @@ +locator = $this->prophesize(VisitLocatorInterface::class); + $this->visitToLocation = $this->prophesize(VisitToLocationHelperInterface::class); + + $this->listener = new LocateUnlocatedVisits($this->locator->reveal(), $this->visitToLocation->reveal()); + } + + /** @test */ + public function locatorIsCalledWhenInvoked(): void + { + ($this->listener)(new GeoLiteDbCreated()); + $this->locator->locateUnlocatedVisits($this->listener)->shouldHaveBeenCalledOnce(); + } + + /** @test */ + public function visitToLocationHelperIsCalledToGeolocateVisits(): void + { + $visit = Visit::forBasePath(Visitor::emptyInstance()); + $location = Location::emptyInstance(); + + $resolve = $this->visitToLocation->resolveVisitLocation($visit)->willReturn($location); + + $result = $this->listener->geolocateVisit($visit); + + self::assertSame($location, $result); + $resolve->shouldHaveBeenCalledOnce(); + } +} diff --git a/module/Core/test/Exception/IpCannotBeLocatedExceptionTest.php b/module/Core/test/Exception/IpCannotBeLocatedExceptionTest.php index b1487b69..2089daba 100644 --- a/module/Core/test/Exception/IpCannotBeLocatedExceptionTest.php +++ b/module/Core/test/Exception/IpCannotBeLocatedExceptionTest.php @@ -9,6 +9,7 @@ use LogicException; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException; use Shlinkio\Shlink\Core\Exception\RuntimeException; +use Shlinkio\Shlink\Core\Visit\Model\UnlocatableIpType; use Throwable; class IpCannotBeLocatedExceptionTest extends TestCase @@ -22,6 +23,7 @@ class IpCannotBeLocatedExceptionTest extends TestCase self::assertEquals('Ignored visit with no IP address', $e->getMessage()); self::assertEquals(0, $e->getCode()); self::assertNull($e->getPrevious()); + self::assertEquals(UnlocatableIpType::EMPTY_ADDRESS, $e->type); } /** @test */ @@ -33,6 +35,7 @@ class IpCannotBeLocatedExceptionTest extends TestCase self::assertEquals('Ignored localhost address', $e->getMessage()); self::assertEquals(0, $e->getCode()); self::assertNull($e->getPrevious()); + self::assertEquals(UnlocatableIpType::LOCALHOST, $e->type); } /** @@ -47,6 +50,7 @@ class IpCannotBeLocatedExceptionTest extends TestCase self::assertEquals('An error occurred while locating IP', $e->getMessage()); self::assertEquals($prev->getCode(), $e->getCode()); self::assertSame($prev, $e->getPrevious()); + self::assertEquals(UnlocatableIpType::ERROR, $e->type); } public function provideErrors(): iterable diff --git a/module/Core/test/Visit/VisitLocatorTest.php b/module/Core/test/Visit/VisitLocatorTest.php index b740d143..21908be8 100644 --- a/module/Core/test/Visit/VisitLocatorTest.php +++ b/module/Core/test/Visit/VisitLocatorTest.php @@ -129,7 +129,7 @@ class VisitLocatorTest extends TestCase public function geolocateVisit(Visit $visit): Location { throw $this->isNonLocatableAddress - ? new IpCannotBeLocatedException('Cannot be located') + ? IpCannotBeLocatedException::forEmptyAddress() : IpCannotBeLocatedException::forError(new Exception('')); } diff --git a/module/Core/test/Visit/VisitToLocationHelperTest.php b/module/Core/test/Visit/VisitToLocationHelperTest.php new file mode 100644 index 00000000..ee22272f --- /dev/null +++ b/module/Core/test/Visit/VisitToLocationHelperTest.php @@ -0,0 +1,66 @@ +ipLocationResolver = $this->prophesize(IpLocationResolverInterface::class); + $this->helper = new VisitToLocationHelper($this->ipLocationResolver->reveal()); + } + + /** + * @test + * @dataProvider provideNonLocatableVisits + */ + public function throwsExpectedErrorForNonLocatableVisit( + Visit $visit, + IpCannotBeLocatedException $expectedException, + ): void { + $this->expectExceptionObject($expectedException); + $this->ipLocationResolver->resolveIpLocation(Argument::cetera())->shouldNotBeCalled(); + + $this->helper->resolveVisitLocation($visit); + } + + public function provideNonLocatableVisits(): iterable + { + yield [Visit::forBasePath(Visitor::emptyInstance()), IpCannotBeLocatedException::forEmptyAddress()]; + yield [ + Visit::forBasePath(new Visitor('foo', 'bar', IpAddress::LOCALHOST, '')), + IpCannotBeLocatedException::forLocalhost(), + ]; + } + + /** @test */ + public function throwsGenericErrorWhenResolvingIpFails(): void + { + $e = new WrongIpException(''); + + $this->expectExceptionObject(IpCannotBeLocatedException::forError($e)); + $this->ipLocationResolver->resolveIpLocation(Argument::cetera())->willThrow($e) + ->shouldBeCalledOnce(); + + $this->helper->resolveVisitLocation(Visit::forBasePath(new Visitor('foo', 'bar', '1.2.3.4', ''))); + } +} From 36680e82aa7433ff4772f1af21e36ac75902adb1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 18 Sep 2022 19:21:59 +0200 Subject: [PATCH 08/10] Reduced duplication in LocateVisitsCommand by reusing VisitToLocationHelper --- module/CLI/config/dependencies.config.php | 3 +- .../src/Command/Visit/LocateVisitsCommand.php | 44 ++++++++----------- .../Command/Visit/LocateVisitsCommandTest.php | 39 +++++++--------- 3 files changed, 37 insertions(+), 49 deletions(-) diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index a9de1f0c..47a47735 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -20,7 +20,6 @@ use Shlinkio\Shlink\Core\Visit; use Shlinkio\Shlink\Installer\Factory\ProcessHelperFactory; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater; use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2Options; -use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Shlinkio\Shlink\Rest\Service\ApiKeyService; use Symfony\Component\Console as SymfonyCli; use Symfony\Component\Lock\LockFactory; @@ -97,7 +96,7 @@ return [ Command\Visit\DownloadGeoLiteDbCommand::class => [GeoLite\GeolocationDbUpdater::class], Command\Visit\LocateVisitsCommand::class => [ Visit\VisitLocator::class, - IpLocationResolverInterface::class, + Visit\VisitToLocationHelper::class, LockFactory::class, ], Command\Visit\GetOrphanVisitsCommand::class => [Visit\VisitsStatsHelper::class], diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php index 47aa50d1..59db9367 100644 --- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -11,11 +11,11 @@ use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException; +use Shlinkio\Shlink\Core\Visit\Model\UnlocatableIpType; use Shlinkio\Shlink\Core\Visit\VisitGeolocationHelperInterface; use Shlinkio\Shlink\Core\Visit\VisitLocatorInterface; -use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; +use Shlinkio\Shlink\Core\Visit\VisitToLocationHelperInterface; use Shlinkio\Shlink\IpGeolocation\Model\Location; -use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Symfony\Component\Console\Exception\RuntimeException; use Symfony\Component\Console\Input\ArrayInput; use Symfony\Component\Console\Input\InputInterface; @@ -35,7 +35,7 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat public function __construct( private readonly VisitLocatorInterface $visitLocator, - private readonly IpLocationResolverInterface $ipLocationResolver, + private readonly VisitToLocationHelperInterface $visitToLocation, LockFactory $locker, ) { parent::__construct($locker); @@ -132,39 +132,33 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat */ public function geolocateVisit(Visit $visit): Location { - if (! $visit->hasRemoteAddr()) { - $this->io->writeln( - 'Ignored visit with no IP address', - OutputInterface::VERBOSITY_VERBOSE, - ); - throw IpCannotBeLocatedException::forEmptyAddress(); - } - - $ipAddr = $visit->getRemoteAddr() ?? ''; + $ipAddr = $visit->getRemoteAddr() ?? '?'; $this->io->write(sprintf('Processing IP %s', $ipAddr)); - if ($ipAddr === IpAddress::LOCALHOST) { - $this->io->writeln(' [Ignored localhost address]'); - throw IpCannotBeLocatedException::forLocalhost(); - } try { - return $this->ipLocationResolver->resolveIpLocation($ipAddr); - } catch (WrongIpException $e) { - $this->io->writeln(' [An error occurred while locating IP. Skipped]'); - if ($this->io->isVerbose()) { + return $this->visitToLocation->resolveVisitLocation($visit); + } catch (IpCannotBeLocatedException $e) { + $this->io->writeln(match ($e->type) { + UnlocatableIpType::EMPTY_ADDRESS => ' [Ignored visit with no IP address]', + UnlocatableIpType::LOCALHOST => ' [Ignored localhost address]', + UnlocatableIpType::ERROR => ' [An error occurred while locating IP. Skipped]', + }); + + if ($e->type === UnlocatableIpType::ERROR && $this->io->isVerbose()) { $this->getApplication()?->renderThrowable($e, $this->io); } - throw IpCannotBeLocatedException::forError($e); + throw $e; } } public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void { - $message = ! $visitLocation->isEmpty() - ? sprintf(' [Address located in "%s"]', $visitLocation->getCountryName()) - : ' [Address not found]'; - $this->io->writeln($message); + if (! $visitLocation->isEmpty()) { + $this->io->writeln(sprintf(' [Address located in "%s"]', $visitLocation->getCountryName())); + } elseif ($visit->hasRemoteAddr() && $visit->getRemoteAddr() !== IpAddress::LOCALHOST) { + $this->io->writeln(' [Could not locate address]'); + } } private function checkDbUpdate(): void diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php index 4111c1dc..63ad3e52 100644 --- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php @@ -10,16 +10,16 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Visit\DownloadGeoLiteDbCommand; use Shlinkio\Shlink\CLI\Command\Visit\LocateVisitsCommand; use Shlinkio\Shlink\CLI\Util\ExitCodes; -use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; +use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Visit\VisitGeolocationHelperInterface; use Shlinkio\Shlink\Core\Visit\VisitLocator; +use Shlinkio\Shlink\Core\Visit\VisitToLocationHelperInterface; use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\Model\Location; -use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use ShlinkioTest\Shlink\CLI\CliTestUtilsTrait; use Symfony\Component\Console\Exception\RuntimeException; use Symfony\Component\Console\Output\OutputInterface; @@ -36,14 +36,14 @@ class LocateVisitsCommandTest extends TestCase private CommandTester $commandTester; private ObjectProphecy $visitService; - private ObjectProphecy $ipResolver; + private ObjectProphecy $visitToLocation; private ObjectProphecy $lock; private ObjectProphecy $downloadDbCommand; protected function setUp(): void { $this->visitService = $this->prophesize(VisitLocator::class); - $this->ipResolver = $this->prophesize(IpLocationResolverInterface::class); + $this->visitToLocation = $this->prophesize(VisitToLocationHelperInterface::class); $locker = $this->prophesize(Lock\LockFactory::class); $this->lock = $this->prophesize(Lock\LockInterface::class); @@ -54,7 +54,7 @@ class LocateVisitsCommandTest extends TestCase $command = new LocateVisitsCommand( $this->visitService->reveal(), - $this->ipResolver->reveal(), + $this->visitToLocation->reveal(), $locker->reveal(), ); @@ -84,7 +84,7 @@ class LocateVisitsCommandTest extends TestCase $mockMethodBehavior, ); $locateAllVisits = $this->visitService->locateAllVisits(Argument::cetera())->will($mockMethodBehavior); - $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn( + $resolveIpLocation = $this->visitToLocation->resolveVisitLocation(Argument::any())->willReturn( Location::emptyInstance(), ); @@ -117,36 +117,29 @@ class LocateVisitsCommandTest extends TestCase * @test * @dataProvider provideIgnoredAddresses */ - public function localhostAndEmptyAddressesAreIgnored(?string $address, string $message): void + public function localhostAndEmptyAddressesAreIgnored(IpCannotBeLocatedException $e, string $message): void { - $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $address, '')); + $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()); $location = VisitLocation::fromGeolocation(Location::emptyInstance()); $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will( $this->invokeHelperMethods($visit, $location), ); - $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn( - Location::emptyInstance(), - ); + $resolveIpLocation = $this->visitToLocation->resolveVisitLocation(Argument::any())->willThrow($e); $this->commandTester->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); $output = $this->commandTester->getDisplay(); + self::assertStringContainsString('Processing IP', $output); self::assertStringContainsString($message, $output); - if (empty($address)) { - self::assertStringNotContainsString('Processing IP', $output); - } else { - self::assertStringContainsString('Processing IP', $output); - } $locateVisits->shouldHaveBeenCalledOnce(); - $resolveIpLocation->shouldNotHaveBeenCalled(); + $resolveIpLocation->shouldHaveBeenCalledOnce(); } public function provideIgnoredAddresses(): iterable { - yield 'with empty address' => ['', 'Ignored visit with no IP address']; - yield 'with null address' => [null, 'Ignored visit with no IP address']; - yield 'with localhost address' => [IpAddress::LOCALHOST, 'Ignored localhost address']; + yield 'empty address' => [IpCannotBeLocatedException::forEmptyAddress(), 'Ignored visit with no IP address']; + yield 'localhost address' => [IpCannotBeLocatedException::forLocalhost(), 'Ignored localhost address']; } /** @test */ @@ -158,7 +151,9 @@ class LocateVisitsCommandTest extends TestCase $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will( $this->invokeHelperMethods($visit, $location), ); - $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willThrow(WrongIpException::class); + $resolveIpLocation = $this->visitToLocation->resolveVisitLocation(Argument::any())->willThrow( + IpCannotBeLocatedException::forError(WrongIpException::fromIpAddress('1.2.3.4')), + ); $this->commandTester->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); @@ -187,7 +182,7 @@ class LocateVisitsCommandTest extends TestCase $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(function (): void { }); - $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]); + $resolveIpLocation = $this->visitToLocation->resolveVisitLocation(Argument::any()); $this->commandTester->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); $output = $this->commandTester->getDisplay(); From 8605b35b576171a9acb1dd863dc6489c5ded66d8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 18 Sep 2022 19:22:57 +0200 Subject: [PATCH 09/10] Removed unneeded injected dependency --- module/CLI/config/dependencies.config.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 47a47735..dffc6010 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -19,7 +19,6 @@ use Shlinkio\Shlink\Core\Tag\TagService; use Shlinkio\Shlink\Core\Visit; use Shlinkio\Shlink\Installer\Factory\ProcessHelperFactory; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater; -use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2Options; use Shlinkio\Shlink\Rest\Service\ApiKeyService; use Symfony\Component\Console as SymfonyCli; use Symfony\Component\Lock\LockFactory; @@ -75,7 +74,6 @@ return [ Reader::class, LOCAL_LOCK_FACTORY, TrackingOptions::class, - GeoLite2Options::class, ], Util\ProcessRunner::class => [SymfonyCli\Helper\ProcessHelper::class], ApiKey\RoleResolver::class => [DomainService::class, 'config.url_shortener.domain.hostname'], From 68e1c61e7fc9757350fa12c33ff735e6c5a33d13 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 18 Sep 2022 19:28:14 +0200 Subject: [PATCH 10/10] Removed unnecessary ADR entry --- ...low-delaying-initial-geolite2-db-download-in-docker-images.md | 0 docs/adr/README.md | 1 - 2 files changed, 1 deletion(-) delete mode 100644 docs/adr/2022-09-18-allow-delaying-initial-geolite2-db-download-in-docker-images.md diff --git a/docs/adr/2022-09-18-allow-delaying-initial-geolite2-db-download-in-docker-images.md b/docs/adr/2022-09-18-allow-delaying-initial-geolite2-db-download-in-docker-images.md deleted file mode 100644 index e69de29b..00000000 diff --git a/docs/adr/README.md b/docs/adr/README.md index f12d17c1..7cfccdf7 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -2,7 +2,6 @@ Here listed you will find the different architectural decisions taken in the project, including all the reasoning behind it, options considered, and final outcome. -* [2022-09-18 Allow delaying initial GeoLite2 DB download in docker images](2022-09-18-allow-delaying-initial-geolite2-db-download-in-docker-images.md) * [2022-08-05 Support multi-segment custom slugs](2022-08-05-support-multi-segment-custom-slugs.md) * [2022-01-15 Update env vars behavior to have precedence over installer options](2022-01-15-update-env-vars-behavior-to-have-precedence-over-installer-options.md) * [2021-08-05 Migrate to a new caching library](2021-08-05-migrate-to-a-new-caching-library.md)