diff --git a/CHANGELOG.md b/CHANGELOG.md index 353a4abfb..c91273e5c 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 236005305..8da6c7533 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -6,6 +6,7 @@ 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 @@ protected function execute(InputInterface $input, OutputInterface $output): ?int $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 @@ protected function execute(InputInterface $input, OutputInterface $output): ?int $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 @@ protected function execute(InputInterface $input, OutputInterface $output): ?int return ExitCode::EXIT_SUCCESS; } catch (GeolocationDbUpdateFailedException $e) { - $olderDbExists = $e->olderDbExists(); + return $this->processGeoLiteUpdateError($e, $io); + } + } - 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.'); - } + private function processGeoLiteUpdateError(GeolocationDbUpdateFailedException $e, SymfonyStyle $io): int + { + $olderDbExists = $e->olderDbExists(); - if ($io->isVerbose()) { - $this->getApplication()?->renderThrowable($e, $io); - } + 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.'); + } - return $olderDbExists ? ExitCode::EXIT_WARNING : ExitCode::EXIT_FAILURE; + 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 e8f93b194..b377c14b0 100644 --- a/module/CLI/src/GeoLite/GeolocationDbUpdater.php +++ b/module/CLI/src/GeoLite/GeolocationDbUpdater.php @@ -108,8 +108,7 @@ private function downloadNewDb( $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 7b245943b..859768864 100644 --- a/module/CLI/src/GeoLite/GeolocationResult.php +++ b/module/CLI/src/GeoLite/GeolocationResult.php @@ -5,6 +5,7 @@ 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 78e14fa95..4d2754f8f 100644 --- a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php +++ b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php @@ -72,6 +72,21 @@ public static function provideFailureParams(): iterable ]; } + #[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 2d47d79c7..9d32ca79e 100644 --- a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php +++ b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php @@ -16,6 +16,7 @@ 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 @@ protected function setUp(): void $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 {