Merge pull request shlinkio#1926 from acelaya-forks/feature/geolite-d…

…ownload-warn Print a warning when manually running visit:download-db with no license
hamidpeywasti · Nov 23, 2023 · 0511c73 · 0511c73
2 parents cb0bac5 + a3554ea
commit 0511c73
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 15 deletions.
diff --git 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
@@ -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,14 +42,19 @@ 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('<fg=blue>%s GeoLite2 db file...</>', $olderDbExists ? 'Updating' : 'Downloading'));
                 $this->progressBar = new ProgressBar($io);
             }, function (int $total, int $downloaded): void {
                 $this->progressBar?->setMaxSteps($total);
                 $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
@@ -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
@@ -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
@@ -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
@@ -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
     {