From a3554eaf74097300699aa9d131ac231012d0c808 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 23 Nov 2023 09:31:02 +0100 Subject: [PATCH] Print a warning when manually running visit:download-db with no license --- CHANGELOG.md | 1 + .../Visit/DownloadGeoLiteDbCommand.php | 43 ++++++++++++------- .../CLI/src/GeoLite/GeolocationDbUpdater.php | 3 +- module/CLI/src/GeoLite/GeolocationResult.php | 1 + .../Visit/DownloadGeoLiteDbCommandTest.php | 15 +++++++ .../test/GeoLite/GeolocationDbUpdaterTest.php | 16 +++++++ 6 files changed, 61 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 353a4abf..c91273e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#1055](https://github.com/shlinkio/shlink/issues/1055) Update OAS definition to v3.1. * [#1885](https://github.com/shlinkio/shlink/issues/1885) Update to chronos 3.0. * [#1896](https://github.com/shlinkio/shlink/issues/1896) Requests to health endpoint are no longer logged. +* [#1877](https://github.com/shlinkio/shlink/issues/1877) Print a warning when manually running `visit:download-db` command and a GeoLite2 license was not provided. ### Deprecated * [#1783](https://github.com/shlinkio/shlink/issues/1783) Deprecated support for openswoole. RoadRunner is the best replacement, with the same capabilities, but much easier and convenient to install and manage. diff --git a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php index 23600530..8da6c753 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\Visit; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; +use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; use Shlinkio\Shlink\CLI\Util\ExitCode; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\ProgressBar; @@ -41,7 +42,7 @@ class DownloadGeoLiteDbCommand extends Command $io = new SymfonyStyle($input, $output); try { - $this->dbUpdater->checkDbUpdate(function (bool $olderDbExists) use ($io): void { + $result = $this->dbUpdater->checkDbUpdate(function (bool $olderDbExists) use ($io): void { $io->text(sprintf('%s GeoLite2 db file...', $olderDbExists ? 'Updating' : 'Downloading')); $this->progressBar = new ProgressBar($io); }, function (int $total, int $downloaded): void { @@ -49,6 +50,11 @@ class DownloadGeoLiteDbCommand extends Command $this->progressBar?->setProgress($downloaded); }); + if ($result === GeolocationResult::LICENSE_MISSING) { + $io->warning('It was not possible to download GeoLite2 db, because a license was not provided.'); + return ExitCode::EXIT_WARNING; + } + if ($this->progressBar === null) { $io->info('GeoLite2 db file is up to date.'); } else { @@ -58,21 +64,26 @@ class DownloadGeoLiteDbCommand extends Command return ExitCode::EXIT_SUCCESS; } catch (GeolocationDbUpdateFailedException $e) { - $olderDbExists = $e->olderDbExists(); - - if ($olderDbExists) { - $io->warning( - 'GeoLite2 db file update failed. Visits will continue to be located with the old version.', - ); - } else { - $io->error('GeoLite2 db file download failed. It will not be possible to locate visits.'); - } - - if ($io->isVerbose()) { - $this->getApplication()?->renderThrowable($e, $io); - } - - return $olderDbExists ? ExitCode::EXIT_WARNING : ExitCode::EXIT_FAILURE; + return $this->processGeoLiteUpdateError($e, $io); } } + + private function processGeoLiteUpdateError(GeolocationDbUpdateFailedException $e, SymfonyStyle $io): int + { + $olderDbExists = $e->olderDbExists(); + + if ($olderDbExists) { + $io->warning( + 'GeoLite2 db file update failed. Visits will continue to be located with the old version.', + ); + } else { + $io->error('GeoLite2 db file download failed. It will not be possible to locate visits.'); + } + + if ($io->isVerbose()) { + $this->getApplication()?->renderThrowable($e, $io); + } + + return $olderDbExists ? ExitCode::EXIT_WARNING : ExitCode::EXIT_FAILURE; + } } diff --git a/module/CLI/src/GeoLite/GeolocationDbUpdater.php b/module/CLI/src/GeoLite/GeolocationDbUpdater.php index e8f93b19..b377c14b 100644 --- a/module/CLI/src/GeoLite/GeolocationDbUpdater.php +++ b/module/CLI/src/GeoLite/GeolocationDbUpdater.php @@ -108,8 +108,7 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface $this->dbUpdater->downloadFreshCopy($this->wrapHandleProgressCallback($handleProgress, $olderDbExists)); return $olderDbExists ? GeolocationResult::DB_UPDATED : GeolocationResult::DB_CREATED; } catch (MissingLicenseException) { - // If there's no license key, just ignore the error - return GeolocationResult::CHECK_SKIPPED; + return GeolocationResult::LICENSE_MISSING; } catch (DbUpdateException | WrongIpException $e) { throw $olderDbExists ? GeolocationDbUpdateFailedException::withOlderDb($e) diff --git a/module/CLI/src/GeoLite/GeolocationResult.php b/module/CLI/src/GeoLite/GeolocationResult.php index 7b245943..85976886 100644 --- a/module/CLI/src/GeoLite/GeolocationResult.php +++ b/module/CLI/src/GeoLite/GeolocationResult.php @@ -5,6 +5,7 @@ namespace Shlinkio\Shlink\CLI\GeoLite; enum GeolocationResult { case CHECK_SKIPPED; + case LICENSE_MISSING; case DB_CREATED; case DB_UPDATED; case DB_IS_UP_TO_DATE; diff --git a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php index 78e14fa9..4d2754f8 100644 --- a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php +++ b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php @@ -72,6 +72,21 @@ class DownloadGeoLiteDbCommandTest extends TestCase ]; } + #[Test] + public function warningIsPrintedWhenLicenseIsMissing(): void + { + $this->dbUpdater->expects($this->once())->method('checkDbUpdate')->withAnyParameters()->willReturn( + GeolocationResult::LICENSE_MISSING, + ); + + $this->commandTester->execute([]); + $output = $this->commandTester->getDisplay(); + $exitCode = $this->commandTester->getStatusCode(); + + self::assertStringContainsString('[WARNING] It was not possible to download GeoLite2 db', $output); + self::assertSame(ExitCode::EXIT_WARNING, $exitCode); + } + #[Test, DataProvider('provideSuccessParams')] public function printsExpectedMessageWhenNoErrorOccurs(callable $checkUpdateBehavior, string $expectedMessage): void { diff --git a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php index 2d47d79c..9d32ca79 100644 --- a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php +++ b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php @@ -16,6 +16,7 @@ use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdater; use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; use Shlinkio\Shlink\Core\Options\TrackingOptions; use Shlinkio\Shlink\IpGeolocation\Exception\DbUpdateException; +use Shlinkio\Shlink\IpGeolocation\Exception\MissingLicenseException; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; use Symfony\Component\Lock; use Throwable; @@ -37,6 +38,21 @@ class GeolocationDbUpdaterTest extends TestCase $this->lock->method('acquire')->with($this->isTrue())->willReturn(true); } + #[Test] + public function properResultIsReturnedWhenLicenseIsMissing(): void + { + $mustBeUpdated = fn () => self::assertTrue(true); + + $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(false); + $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->willThrowException( + new MissingLicenseException(''), + ); + $this->geoLiteDbReader->expects($this->never())->method('metadata'); + + $result = $this->geolocationDbUpdater()->checkDbUpdate($mustBeUpdated); + self::assertEquals(GeolocationResult::LICENSE_MISSING, $result); + } + #[Test] public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(): void {