From 72a962ec6d00dc348b588c6b9fdac33e7a0c6aa6 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Sun, 15 Dec 2024 12:03:01 +0100
Subject: [PATCH] Handle differently when trying to update geolocation and
 already in progress

---
 module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php | 5 +++++
 .../test/Command/Visit/DownloadGeoLiteDbCommandTest.php   | 1 +
 module/Core/src/Geolocation/GeolocationDbUpdater.php      | 6 +++++-
 module/Core/src/Geolocation/GeolocationResult.php         | 8 ++++++++
 4 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php
index e1ea5fce..cd7c4ffb 100644
--- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php
+++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php
@@ -56,6 +56,11 @@ class DownloadGeoLiteDbCommand extends Command implements GeolocationDownloadPro
                 return ExitCode::EXIT_WARNING;
             }
 
+            if ($result === GeolocationResult::UPDATE_IN_PROGRESS) {
+                $this->io->warning('A geolocation db is already being downloaded by another process.');
+                return ExitCode::EXIT_WARNING;
+            }
+
             if ($this->progressBar === null) {
                 $this->io->info('GeoLite2 db file is up to date.');
             } else {
diff --git a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php
index f232bdca..01322f0b 100644
--- a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php
+++ b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php
@@ -77,6 +77,7 @@ class DownloadGeoLiteDbCommandTest extends TestCase
     #[Test]
     #[TestWith([GeolocationResult::LICENSE_MISSING, 'It was not possible to download GeoLite2 db'])]
     #[TestWith([GeolocationResult::MAX_ERRORS_REACHED, 'Max consecutive errors reached'])]
+    #[TestWith([GeolocationResult::UPDATE_IN_PROGRESS, 'A geolocation db is already being downloaded'])]
     public function warningIsPrintedForSomeResults(GeolocationResult $result, string $expectedWarningMessage): void
     {
         $this->dbUpdater->expects($this->once())->method('checkDbUpdate')->withAnyParameters()->willReturn($result);
diff --git a/module/Core/src/Geolocation/GeolocationDbUpdater.php b/module/Core/src/Geolocation/GeolocationDbUpdater.php
index f6ca7b49..662e6b7a 100644
--- a/module/Core/src/Geolocation/GeolocationDbUpdater.php
+++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php
@@ -12,6 +12,7 @@ use Shlinkio\Shlink\IpGeolocation\Exception\DbUpdateException;
 use Shlinkio\Shlink\IpGeolocation\Exception\MissingLicenseException;
 use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface;
 use Symfony\Component\Lock\LockFactory;
+use Throwable;
 
 use function sprintf;
 
@@ -65,7 +66,7 @@ readonly class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
         // If most recent attempt is in progress, skip check.
         // This is a safety check in case the lock is released before the previous download has finished.
         if ($mostRecentDownload?->isInProgress()) {
-            return GeolocationResult::CHECK_SKIPPED;
+            return GeolocationResult::UPDATE_IN_PROGRESS;
         }
 
         $amountOfErrorsSinceLastSuccess = 0;
@@ -122,6 +123,9 @@ readonly class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
                 sprintf('%s Prev: %s', $e->getMessage(), $e->getPrevious()?->getMessage() ?? '-'),
             );
             throw $e;
+        } catch (Throwable $e) {
+            $dbUpdate->finishWithError(sprintf('Unknown error: %s', $e->getMessage()));
+            throw $e;
         } finally {
             $this->em->flush();
         }
diff --git a/module/Core/src/Geolocation/GeolocationResult.php b/module/Core/src/Geolocation/GeolocationResult.php
index 1eecedda..eb5b4a5c 100644
--- a/module/Core/src/Geolocation/GeolocationResult.php
+++ b/module/Core/src/Geolocation/GeolocationResult.php
@@ -4,10 +4,18 @@ namespace Shlinkio\Shlink\Core\Geolocation;
 
 enum GeolocationResult
 {
+    /** Geolocation is not relevant, so updates are skipped */
     case CHECK_SKIPPED;
+    /** Update is skipped because max amount of consecutive errors was reached */
     case MAX_ERRORS_REACHED;
+    /** Update was skipped because a geolocation license key was not provided */
     case LICENSE_MISSING;
+    /** A geolocation database didn't exist and has been created */
     case DB_CREATED;
+    /** An outdated geolocation database existed and has been updated */
     case DB_UPDATED;
+    /** Geolocation database does not need to be updated yet */
     case DB_IS_UP_TO_DATE;
+    /** Geolocation db update is currently in progress */
+    case UPDATE_IN_PROGRESS;
 }