Merge pull request #2304 from acelaya-forks/feature/geolocation-services-refactor

Move GeolocationDbUpdater to Core module

module/CLI/config/dependencies.config.php

@@ -7,9 +7,9 @@ namespace Shlinkio\Shlink\CLI;
 use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory;
 use Laminas\ServiceManager\Factory\InvokableFactory;
 use Shlinkio\Shlink\Common\Doctrine\NoDbNameConnectionFactory;
-use Shlinkio\Shlink\Core\Config\Options\TrackingOptions;
 use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions;
 use Shlinkio\Shlink\Core\Domain\DomainService;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdater;
 use Shlinkio\Shlink\Core\Matomo;
 use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleService;
 use Shlinkio\Shlink\Core\ShortUrl;
@@ -17,15 +17,11 @@ use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier;
 use Shlinkio\Shlink\Core\Tag\TagService;
 use Shlinkio\Shlink\Core\Visit;
 use Shlinkio\Shlink\Installer\Factory\ProcessHelperFactory;
-use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater;
-use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2ReaderFactory;
 use Shlinkio\Shlink\Rest\Service\ApiKeyService;
 use Symfony\Component\Console as SymfonyCli;
 use Symfony\Component\Lock\LockFactory;
 use Symfony\Component\Process\PhpExecutableFinder;
 
-use const Shlinkio\Shlink\LOCAL_LOCK_FACTORY;
-
 return [
 
     'dependencies' => [
@@ -34,7 +30,6 @@ return [
             SymfonyCli\Helper\ProcessHelper::class => ProcessHelperFactory::class,
             PhpExecutableFinder::class => InvokableFactory::class,
 
-            GeoLite\GeolocationDbUpdater::class => ConfigAbstractFactory::class,
             RedirectRule\RedirectRuleHandler::class => InvokableFactory::class,
             Util\ProcessRunner::class => ConfigAbstractFactory::class,
 
@@ -82,12 +77,6 @@ return [
     ],
 
     ConfigAbstractFactory::class => [
-        GeoLite\GeolocationDbUpdater::class => [
-            DbUpdater::class,
-            GeoLite2ReaderFactory::class,
-            LOCAL_LOCK_FACTORY,
-            TrackingOptions::class,
-        ],
         Util\ProcessRunner::class => [SymfonyCli\Helper\ProcessHelper::class],
         ApiKey\RoleResolver::class => [DomainService::class, UrlShortenerOptions::class],
 
@@ -107,7 +96,7 @@ return [
         Command\ShortUrl\DeleteShortUrlVisitsCommand::class => [ShortUrl\ShortUrlVisitsDeleter::class],
         Command\ShortUrl\DeleteExpiredShortUrlsCommand::class => [ShortUrl\DeleteShortUrlService::class],
 
-        Command\Visit\DownloadGeoLiteDbCommand::class => [GeoLite\GeolocationDbUpdater::class],
+        Command\Visit\DownloadGeoLiteDbCommand::class => [GeolocationDbUpdater::class],
         Command\Visit\LocateVisitsCommand::class => [
             Visit\Geolocation\VisitLocator::class,
             Visit\Geolocation\VisitToLocationHelper::class,

module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php

@@ -4,10 +4,11 @@ declare(strict_types=1);
 
 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 Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationResult;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Helper\ProgressBar;
 use Symfony\Component\Console\Input\InputInterface;
@@ -16,13 +17,14 @@ use Symfony\Component\Console\Style\SymfonyStyle;
 
 use function sprintf;
 
-class DownloadGeoLiteDbCommand extends Command
+class DownloadGeoLiteDbCommand extends Command implements GeolocationDownloadProgressHandlerInterface
 {
     public const string NAME = 'visit:download-db';
 
     private ProgressBar|null $progressBar = null;
+    private SymfonyStyle $io;
 
-    public function __construct(private GeolocationDbUpdaterInterface $dbUpdater)
+    public function __construct(private readonly GeolocationDbUpdaterInterface $dbUpdater)
     {
         parent::__construct();
     }
@@ -39,32 +41,26 @@ class DownloadGeoLiteDbCommand extends Command
 
     protected function execute(InputInterface $input, OutputInterface $output): int
     {
-        $io = new SymfonyStyle($input, $output);
+        $this->io = new SymfonyStyle($input, $output);
 
         try {
-            $result = $this->dbUpdater->checkDbUpdate(function (bool $olderDbExists) use ($io): void {
+            $result = $this->dbUpdater->checkDbUpdate($this);
-                $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.');
+                $this->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.');
+                $this->io->info('GeoLite2 db file is up to date.');
             } else {
                 $this->progressBar->finish();
-                $io->success('GeoLite2 db file properly downloaded.');
+                $this->io->success('GeoLite2 db file properly downloaded.');
             }
 
             return ExitCode::EXIT_SUCCESS;
         } catch (GeolocationDbUpdateFailedException $e) {
-            return $this->processGeoLiteUpdateError($e, $io);
+            return $this->processGeoLiteUpdateError($e, $this->io);
         }
     }
 
@@ -86,4 +82,16 @@ class DownloadGeoLiteDbCommand extends Command
 
         return $olderDbExists ? ExitCode::EXIT_WARNING : ExitCode::EXIT_FAILURE;
     }
+
+    public function beforeDownload(bool $olderDbExists): void
+    {
+        $this->io->text(sprintf('<fg=blue>%s GeoLite2 db file...</>', $olderDbExists ? 'Updating' : 'Downloading'));
+        $this->progressBar = new ProgressBar($this->io);
+    }
+
+    public function handleProgress(int $total, int $downloaded, bool $olderDbExists): void
+    {
+        $this->progressBar?->setMaxSteps($total);
+        $this->progressBar?->setProgress($downloaded);
+    }
 }

module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php

@@ -9,10 +9,11 @@ use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Shlinkio\Shlink\CLI\Command\Visit\DownloadGeoLiteDbCommand;
-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 Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationResult;
 use ShlinkioTest\Shlink\CLI\Util\CliTestUtils;
 use Symfony\Component\Console\Tester\CommandTester;
 
@@ -36,9 +37,9 @@ class DownloadGeoLiteDbCommandTest extends TestCase
         int $expectedExitCode,
     ): void {
         $this->dbUpdater->expects($this->once())->method('checkDbUpdate')->withAnyParameters()->willReturnCallback(
-            function (callable $beforeDownload, callable $handleProgress) use ($olderDbExists): void {
+            function (GeolocationDownloadProgressHandlerInterface $handler) use ($olderDbExists): void {
-                $beforeDownload($olderDbExists);
+                $handler->beforeDownload($olderDbExists);
-                $handleProgress(100, 50);
+                $handler->handleProgress(100, 50, $olderDbExists);
 
                 throw $olderDbExists
                     ? GeolocationDbUpdateFailedException::withOlderDb()
@@ -105,8 +106,8 @@ class DownloadGeoLiteDbCommandTest extends TestCase
     public static function provideSuccessParams(): iterable
     {
         yield 'up to date db' => [fn () => GeolocationResult::CHECK_SKIPPED, '[INFO] GeoLite2 db file is up to date.'];
-        yield 'outdated db' => [function (callable $beforeDownload): GeolocationResult {
+        yield 'outdated db' => [function (GeolocationDownloadProgressHandlerInterface $handler): GeolocationResult {
-            $beforeDownload(true);
+            $handler->beforeDownload(true);
             return GeolocationResult::DB_CREATED;
         }, '[OK] GeoLite2 db file properly downloaded.'];
     }

module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php

@@ -9,7 +9,7 @@ use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\TestCase;
 use RuntimeException;
-use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
+use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException;
 use Throwable;
 
 class GeolocationDbUpdateFailedExceptionTest extends TestCase

module/Core/config/dependencies.config.php

@@ -9,12 +9,16 @@ use Laminas\ServiceManager\Factory\InvokableFactory;
 use Psr\EventDispatcher\EventDispatcherInterface;
 use Shlinkio\Shlink\Common\Doctrine\EntityRepositoryFactory;
 use Shlinkio\Shlink\Core\Config\Options\NotFoundRedirectOptions;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdater;
 use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier;
 use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface;
 use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater;
+use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2ReaderFactory;
 use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
 use Symfony\Component\Lock;
 
+use const Shlinkio\Shlink\LOCAL_LOCK_FACTORY;
+
 return [
 
     'dependencies' => [
@@ -103,6 +107,7 @@ return [
 
             EventDispatcher\PublishingUpdatesGenerator::class => ConfigAbstractFactory::class,
 
+            Geolocation\GeolocationDbUpdater::class => ConfigAbstractFactory::class,
             Geolocation\Middleware\IpGeolocationMiddleware::class => ConfigAbstractFactory::class,
 
             Importer\ImportedLinksProcessor::class => ConfigAbstractFactory::class,
@@ -240,6 +245,12 @@ return [
 
         EventDispatcher\PublishingUpdatesGenerator::class => [ShortUrl\Transformer\ShortUrlDataTransformer::class],
 
+        GeolocationDbUpdater::class => [
+            DbUpdater::class,
+            GeoLite2ReaderFactory::class,
+            LOCAL_LOCK_FACTORY,
+            Config\Options\TrackingOptions::class,
+        ],
         Geolocation\Middleware\IpGeolocationMiddleware::class => [
             IpLocationResolverInterface::class,
             DbUpdater::class,

module/Core/config/event_dispatcher.config.php

@@ -6,11 +6,11 @@ namespace Shlinkio\Shlink\Core;
 
 use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory;
 use Psr\EventDispatcher\EventDispatcherInterface;
-use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdater;
 use Shlinkio\Shlink\Common\Cache\RedisPublishingHelper;
 use Shlinkio\Shlink\Common\Mercure\MercureHubPublishingHelper;
 use Shlinkio\Shlink\Common\Mercure\MercureOptions;
 use Shlinkio\Shlink\Common\RabbitMq\RabbitMqPublishingHelper;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdater;
 use Shlinkio\Shlink\Core\Matomo\MatomoOptions;
 use Shlinkio\Shlink\Core\Visit\Geolocation\VisitLocator;
 use Shlinkio\Shlink\Core\Visit\Geolocation\VisitToLocationHelper;

module/Core/src/EventDispatcher/UpdateGeoLiteDb.php

@@ -6,9 +6,10 @@ namespace Shlinkio\Shlink\Core\EventDispatcher;
 
 use Psr\EventDispatcher\EventDispatcherInterface;
 use Psr\Log\LoggerInterface;
-use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface;
-use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult;
 use Shlinkio\Shlink\Core\EventDispatcher\Event\GeoLiteDbCreated;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationResult;
 use Throwable;
 
 use function sprintf;
@@ -24,21 +25,35 @@ readonly class UpdateGeoLiteDb
 
     public function __invoke(): void
     {
-        $beforeDownload = fn (bool $olderDbExists) => $this->logger->notice(
-            sprintf('%s GeoLite2 db file...', $olderDbExists ? 'Updating' : 'Downloading'),
-        );
-        $messageLogged = false;
-        $handleProgress = function (int $total, int $downloaded, bool $olderDbExists) use (&$messageLogged): void {
-            if ($messageLogged || $total > $downloaded) {
-                return;
-            }
-
-            $messageLogged = true;
-            $this->logger->notice(sprintf('Finished %s GeoLite2 db file', $olderDbExists ? 'updating' : 'downloading'));
-        };
-
         try {
-            $result = $this->dbUpdater->checkDbUpdate($beforeDownload, $handleProgress);
+            $result = $this->dbUpdater->checkDbUpdate(
+                new class ($this->logger) implements GeolocationDownloadProgressHandlerInterface {
+                    public function __construct(
+                        private readonly LoggerInterface $logger,
+                        private bool $messageLogged = false,
+                    ) {
+                    }
+
+                    public function beforeDownload(bool $olderDbExists): void
+                    {
+                        $this->logger->notice(
+                            sprintf('%s GeoLite2 db file...', $olderDbExists ? 'Updating' : 'Downloading'),
+                        );
+                    }
+
+                    public function handleProgress(int $total, int $downloaded, bool $olderDbExists): void
+                    {
+                        if ($this->messageLogged || $total > $downloaded) {
+                            return;
+                        }
+
+                        $this->messageLogged = true;
+                        $this->logger->notice(
+                            sprintf('Finished %s GeoLite2 db file', $olderDbExists ? 'updating' : 'downloading'),
+                        );
+                    }
+                },
+            );
             if ($result === GeolocationResult::DB_CREATED) {
                 $this->eventDispatcher->dispatch(new GeoLiteDbCreated());
             }

module/Core/src/Exception/GeolocationDbUpdateFailedException.php

@@ -2,7 +2,7 @@
 
 declare(strict_types=1);
 
-namespace Shlinkio\Shlink\CLI\Exception;
+namespace Shlinkio\Shlink\Core\Exception;
 
 use RuntimeException;
 use Throwable;

module/Core/src/Geolocation/GeolocationDbUpdater.php

@@ -2,17 +2,16 @@
 
 declare(strict_types=1);
 
-namespace Shlinkio\Shlink\CLI\GeoLite;
+namespace Shlinkio\Shlink\Core\Geolocation;
 
 use Cake\Chronos\Chronos;
 use Closure;
 use GeoIp2\Database\Reader;
 use MaxMind\Db\Reader\Metadata;
-use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
 use Shlinkio\Shlink\Core\Config\Options\TrackingOptions;
+use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException;
 use Shlinkio\Shlink\IpGeolocation\Exception\DbUpdateException;
 use Shlinkio\Shlink\IpGeolocation\Exception\MissingLicenseException;
-use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
 use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface;
 use Symfony\Component\Lock\LockFactory;
 
@@ -41,8 +40,7 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
      * @throws GeolocationDbUpdateFailedException
      */
     public function checkDbUpdate(
-        callable|null $beforeDownload = null,
+        GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler = null,
-        callable|null $handleProgress = null,
     ): GeolocationResult {
         if (! $this->trackingOptions->isGeolocationRelevant()) {
             return GeolocationResult::CHECK_SKIPPED;
@@ -52,7 +50,7 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
         $lock->acquire(true); // Block until lock is released
 
         try {
-            return $this->downloadIfNeeded($beforeDownload, $handleProgress);
+            return $this->downloadIfNeeded($downloadProgressHandler);
         } finally {
             $lock->release();
         }
@@ -61,15 +59,16 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
     /**
      * @throws GeolocationDbUpdateFailedException
      */
-    private function downloadIfNeeded(callable|null $beforeDownload, callable|null $handleProgress): GeolocationResult
+    private function downloadIfNeeded(
-    {
+        GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler,
+    ): GeolocationResult {
         if (! $this->dbUpdater->databaseFileExists()) {
-            return $this->downloadNewDb($beforeDownload, $handleProgress, olderDbExists: false);
+            return $this->downloadNewDb($downloadProgressHandler, olderDbExists: false);
         }
 
         $meta = ($this->geoLiteDbReaderFactory)()->metadata();
         if ($this->buildIsTooOld($meta)) {
-            return $this->downloadNewDb($beforeDownload, $handleProgress, olderDbExists: true);
+            return $this->downloadNewDb($downloadProgressHandler, olderDbExists: true);
         }
 
         return GeolocationResult::DB_IS_UP_TO_DATE;
@@ -106,32 +105,23 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
      * @throws GeolocationDbUpdateFailedException
      */
     private function downloadNewDb(
-        callable|null $beforeDownload,
+        GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler,
-        callable|null $handleProgress,
         bool $olderDbExists,
     ): GeolocationResult {
-        if ($beforeDownload !== null) {
+        $downloadProgressHandler?->beforeDownload($olderDbExists);
-            $beforeDownload($olderDbExists);
-        }
 
         try {
-            $this->dbUpdater->downloadFreshCopy($this->wrapHandleProgressCallback($handleProgress, $olderDbExists));
+            $this->dbUpdater->downloadFreshCopy(
+                static fn (int $total, int $downloaded)
+                    => $downloadProgressHandler?->handleProgress($total, $downloaded, $olderDbExists),
+            );
             return $olderDbExists ? GeolocationResult::DB_UPDATED : GeolocationResult::DB_CREATED;
         } catch (MissingLicenseException) {
             return GeolocationResult::LICENSE_MISSING;
-        } catch (DbUpdateException | WrongIpException $e) {
+        } catch (DbUpdateException $e) {
             throw $olderDbExists
                 ? GeolocationDbUpdateFailedException::withOlderDb($e)
                 : GeolocationDbUpdateFailedException::withoutOlderDb($e);
         }
     }
-
-    private function wrapHandleProgressCallback(callable|null $handleProgress, bool $olderDbExists): callable|null
-    {
-        if ($handleProgress === null) {
-            return null;
-        }
-
-        return static fn (int $total, int $downloaded) => $handleProgress($total, $downloaded, $olderDbExists);
-    }
 }

module/Core/src/Geolocation/GeolocationDbUpdaterInterface.php

@@ -2,9 +2,9 @@
 
 declare(strict_types=1);
 
-namespace Shlinkio\Shlink\CLI\GeoLite;
+namespace Shlinkio\Shlink\Core\Geolocation;
 
-use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
+use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException;
 
 interface GeolocationDbUpdaterInterface
 {
@@ -12,7 +12,6 @@ interface GeolocationDbUpdaterInterface
      * @throws GeolocationDbUpdateFailedException
      */
     public function checkDbUpdate(
-        callable|null $beforeDownload = null,
+        GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler = null,
-        callable|null $handleProgress = null,
     ): GeolocationResult;
 }

module/Core/src/Geolocation/GeolocationDownloadProgressHandlerInterface.php

@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Geolocation;
+
+interface GeolocationDownloadProgressHandlerInterface
+{
+    /**
+     * Invoked right before starting to download a geolocation DB file, and only if it needs to be downloaded.
+     * @param $olderDbExists - Indicates if an older DB file already exists when this method is called
+     */
+    public function beforeDownload(bool $olderDbExists): void;
+
+    /**
+     * Invoked every time a new chunk of the new DB file is downloaded, with the total size of the file and how much has
+     * already been downloaded.
+     * @param $olderDbExists - Indicates if an older DB file already exists when this method is called
+     */
+    public function handleProgress(int $total, int $downloaded, bool $olderDbExists): void;
+}

module/Core/src/Geolocation/GeolocationResult.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace Shlinkio\Shlink\CLI\GeoLite;
+namespace Shlinkio\Shlink\Core\Geolocation;
 
 enum GeolocationResult
 {

module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php

@@ -11,10 +11,11 @@ use PHPUnit\Framework\TestCase;
 use Psr\EventDispatcher\EventDispatcherInterface;
 use Psr\Log\LoggerInterface;
 use RuntimeException;
-use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface;
-use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult;
 use Shlinkio\Shlink\Core\EventDispatcher\Event\GeoLiteDbCreated;
 use Shlinkio\Shlink\Core\EventDispatcher\UpdateGeoLiteDb;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationResult;
 
 use function array_map;
 
@@ -51,11 +52,11 @@ class UpdateGeoLiteDbTest extends TestCase
     }
 
     #[Test, DataProvider('provideFlags')]
-    public function noticeMessageIsPrintedWhenFirstCallbackIsInvoked(bool $oldDbExists, string $expectedMessage): void
+    public function noticeMessageIsPrintedWhenDownloadIsStarted(bool $oldDbExists, string $expectedMessage): void
     {
         $this->dbUpdater->expects($this->once())->method('checkDbUpdate')->withAnyParameters()->willReturnCallback(
-            function (callable $firstCallback) use ($oldDbExists): GeolocationResult {
+            function (GeolocationDownloadProgressHandlerInterface $handler) use ($oldDbExists): GeolocationResult {
-                $firstCallback($oldDbExists);
+                $handler->beforeDownload($oldDbExists);
                 return GeolocationResult::DB_IS_UP_TO_DATE;
             },
         );
@@ -73,18 +74,24 @@ class UpdateGeoLiteDbTest extends TestCase
     }
 
     #[Test, DataProvider('provideDownloaded')]
-    public function noticeMessageIsPrintedWhenSecondCallbackIsInvoked(
+    public function noticeMessageIsPrintedWhenDownloadIsFinished(
         int $total,
         int $downloaded,
         bool $oldDbExists,
         string|null $expectedMessage,
     ): void {
         $this->dbUpdater->expects($this->once())->method('checkDbUpdate')->withAnyParameters()->willReturnCallback(
-            function ($_, callable $secondCallback) use ($total, $downloaded, $oldDbExists): GeolocationResult {
+            function (
+                GeolocationDownloadProgressHandlerInterface $handler,
+            ) use (
+                $total,
+                $downloaded,
+                $oldDbExists,
+            ): GeolocationResult {
                 // Invoke several times to ensure the log is printed only once
-                $secondCallback($total, $downloaded, $oldDbExists);
+                $handler->handleProgress($total, $downloaded, $oldDbExists);
-                $secondCallback($total, $downloaded, $oldDbExists);
+                $handler->handleProgress($total, $downloaded, $oldDbExists);
-                $secondCallback($total, $downloaded, $oldDbExists);
+                $handler->handleProgress($total, $downloaded, $oldDbExists);
 
                 return GeolocationResult::DB_UPDATED;
             },

module/Core/test/Geolocation/GeolocationDbUpdaterTest.php

@@ -2,19 +2,21 @@
 
 declare(strict_types=1);
 
-namespace ShlinkioTest\Shlink\CLI\GeoLite;
+namespace ShlinkioTest\Shlink\Core\Geolocation;
 
 use Cake\Chronos\Chronos;
+use Closure;
 use GeoIp2\Database\Reader;
 use MaxMind\Db\Reader\Metadata;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
-use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
-use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdater;
-use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult;
 use Shlinkio\Shlink\Core\Config\Options\TrackingOptions;
+use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdater;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface;
+use Shlinkio\Shlink\Core\Geolocation\GeolocationResult;
 use Shlinkio\Shlink\IpGeolocation\Exception\DbUpdateException;
 use Shlinkio\Shlink\IpGeolocation\Exception\MissingLicenseException;
 use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface;
@@ -29,6 +31,8 @@ class GeolocationDbUpdaterTest extends TestCase
     private MockObject & DbUpdaterInterface $dbUpdater;
     private MockObject & Reader $geoLiteDbReader;
     private MockObject & Lock\LockInterface $lock;
+    /** @var GeolocationDownloadProgressHandlerInterface&object{beforeDownloadCalled: bool, handleProgressCalled: bool} */
+    private GeolocationDownloadProgressHandlerInterface $progressHandler;
 
     protected function setUp(): void
     {
@@ -36,6 +40,23 @@ class GeolocationDbUpdaterTest extends TestCase
         $this->geoLiteDbReader = $this->createMock(Reader::class);
         $this->lock = $this->createMock(Lock\SharedLockInterface::class);
         $this->lock->method('acquire')->with($this->isTrue())->willReturn(true);
+        $this->progressHandler = new class implements GeolocationDownloadProgressHandlerInterface {
+            public function __construct(
+                public bool $beforeDownloadCalled = false,
+                public bool $handleProgressCalled = false,
+            ) {
+            }
+
+            public function beforeDownload(bool $olderDbExists): void
+            {
+                $this->beforeDownloadCalled = true;
+            }
+
+            public function handleProgress(int $total, int $downloaded, bool $olderDbExists): void
+            {
+                $this->handleProgressCalled = true;
+            }
+        };
     }
 
     #[Test]
@@ -47,12 +68,9 @@ class GeolocationDbUpdaterTest extends TestCase
         );
         $this->geoLiteDbReader->expects($this->never())->method('metadata');
 
-        $isCalled = false;
+        $result = $this->geolocationDbUpdater()->checkDbUpdate($this->progressHandler);
-        $result = $this->geolocationDbUpdater()->checkDbUpdate(function () use (&$isCalled): void {
-            $isCalled = true;
-        });
 
-        self::assertTrue($isCalled);
+        self::assertTrue($this->progressHandler->beforeDownloadCalled);
         self::assertEquals(GeolocationResult::LICENSE_MISSING, $result);
     }
 
@@ -63,21 +81,18 @@ class GeolocationDbUpdaterTest extends TestCase
 
         $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(false);
         $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->with(
-            $this->isNull(),
+            $this->isInstanceOf(Closure::class),
         )->willThrowException($prev);
         $this->geoLiteDbReader->expects($this->never())->method('metadata');
 
-        $isCalled = false;
         try {
-            $this->geolocationDbUpdater()->checkDbUpdate(function () use (&$isCalled): void {
+            $this->geolocationDbUpdater()->checkDbUpdate($this->progressHandler);
-                $isCalled = true;
-            });
             self::fail();
         } catch (Throwable $e) {
             self::assertInstanceOf(GeolocationDbUpdateFailedException::class, $e);
             self::assertSame($prev, $e->getPrevious());
             self::assertFalse($e->olderDbExists());
-            self::assertTrue($isCalled);
+            self::assertTrue($this->progressHandler->beforeDownloadCalled);
         }
     }
 
@@ -87,7 +102,7 @@ class GeolocationDbUpdaterTest extends TestCase
         $prev = new DbUpdateException('');
         $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(true);
         $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->with(
-            $this->isNull(),
+            $this->isInstanceOf(Closure::class),
         )->willThrowException($prev);
         $this->geoLiteDbReader->expects($this->once())->method('metadata')->with()->willReturn(
             $this->buildMetaWithBuildEpoch(Chronos::now()->subDays($days)->getTimestamp()),