From eab9347522e77b4646f4f8c762132675f2258bbc Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 18 Sep 2022 10:31:14 +0200 Subject: [PATCH] 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());