Fix GeolocationDbUpdater test

CHANGELOG.md

@@ -15,13 +15,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
     This option effectively replaces the old `REDIRECT_APPEND_EXTRA_PATH` option, which is now deprecated and will be removed in Shlink 5.0.0
 
 ### Changed
-* * [#2281](https://github.com/shlinkio/shlink/issues/2281) Update docker image to PHP 8.4
+* [#2281](https://github.com/shlinkio/shlink/issues/2281) Update docker image to PHP 8.4
+* [#2124](https://github.com/shlinkio/shlink/issues/2124) Improve how Shlink decides if a GeoLite db file needs to be downloaded, and reduces the chances for API limits to be reached.
+
+    Now Shlink tracks all download attempts, and knows which of them failed and succeeded. This lets it know when was the last error or success, how many consecutive errors have happened, etc.
+
+    It also tracks now the reason for a download to be attempted, and the error that happened when one fails.
 
 ### Deprecated
 * *Nothing*
 
 ### Removed
-* * [#2247](https://github.com/shlinkio/shlink/issues/2247) Drop support for PHP 8.2
+* [#2247](https://github.com/shlinkio/shlink/issues/2247) Drop support for PHP 8.2
 
 ### Fixed
 * *Nothing*

module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php

@@ -6,47 +6,50 @@
 
 use Cake\Chronos\Chronos;
 use Shlinkio\Shlink\Common\Entity\AbstractEntity;
+use Shlinkio\Shlink\Core\Exception\RuntimeException;
 
 use function stat;
 
 class GeolocationDbUpdate extends AbstractEntity
 {
     private function __construct(
+        public readonly string $reason,
         private readonly string $filesystemId,
-        private readonly string $reason,
         private GeolocationDbUpdateStatus $status = GeolocationDbUpdateStatus::IN_PROGRESS,
         private readonly Chronos $dateCreated = new Chronos(),
         private Chronos $dateUpdated = new Chronos(),
         private string|null $error = null,
     ) {
     }
 
-    public static function withReason(string $reason, string|null $filesystemId = null): self
+    public static function withReason(string $reason): self
     {
-        return new self($reason, $filesystemId ?? self::currentFilesystemId());
+        return new self($reason, self::currentFilesystemId());
     }
 
     public static function currentFilesystemId(): string
     {
         $system = stat(__FILE__);
         if (! $system) {
-            // TODO Throw error
+            throw new RuntimeException('It was not possible to resolve filesystem ID via stat function');
         }
 
         return (string) $system['dev'];
     }
 
-    public function finishSuccessfully(): void
+    public function finishSuccessfully(): self
     {
         $this->dateUpdated = Chronos::now();
         $this->status = GeolocationDbUpdateStatus::SUCCESS;
+        return $this;
     }
 
-    public function finishWithError(string $error): void
+    public function finishWithError(string $error): self
     {
         $this->error = $error;
         $this->dateUpdated = Chronos::now();
         $this->status = GeolocationDbUpdateStatus::ERROR;
+        return $this;
     }
 
     /**

module/Core/src/Geolocation/GeolocationDbUpdater.php

@@ -95,9 +95,9 @@ private function downloadIfNeeded(
         // - Most recent attempt is older than 30 days (and implicitly, successful)
         $reasonMatch = match (true) {
             $mostRecentDownload === null => [false, 'No download attempts tracked for this instance'],
-            $this->dbUpdater->databaseFileExists() => [false, 'Geolocation db file does not exist'],
+            ! $this->dbUpdater->databaseFileExists() => [false, 'Geolocation db file does not exist'],
             $lastAttemptIsError => [true, 'Max consecutive errors not reached'],
-            $mostRecentDownload->isOlderThan(days: 30) => [true, 'Last successful attempt'],
+            $mostRecentDownload->isOlderThan(days: 30) => [true, 'Last successful attempt is old enough'],
             default => null,
         };
         if ($reasonMatch !== null) {