From 6f17f70137d1da868a6d3a15be5fa6dd6ba6a9c1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 18 Sep 2022 17:00:03 +0200 Subject: [PATCH] Allowed to delay GeoLite2 db download on docker images --- composer.json | 2 +- docker/docker-entrypoint.sh | 6 +- module/CLI/config/dependencies.config.php | 2 + .../src/Command/Visit/LocateVisitsCommand.php | 4 +- .../CLI/src/GeoLite/GeolocationDbUpdater.php | 33 ++++++----- .../test/GeoLite/GeolocationDbUpdaterTest.php | 6 +- .../Core/config/event_dispatcher.config.php | 8 ++- .../EventDispatcher/LocateUnlocatedVisits.php | 57 +++++++++++++++++++ 8 files changed, 95 insertions(+), 23 deletions(-) create mode 100644 module/Core/src/EventDispatcher/LocateUnlocatedVisits.php diff --git a/composer.json b/composer.json index d6122753..c99678ed 100644 --- a/composer.json +++ b/composer.json @@ -50,7 +50,7 @@ "shlinkio/shlink-event-dispatcher": "dev-main#48c0137 as 2.6", "shlinkio/shlink-importer": "^4.0", "shlinkio/shlink-installer": "dev-develop#a01bca9 as 8.2", - "shlinkio/shlink-ip-geolocation": "^3.0", + "shlinkio/shlink-ip-geolocation": "^3.1", "spiral/roadrunner": "^2.11", "spiral/roadrunner-jobs": "^2.3", "symfony/console": "^6.1", diff --git a/docker/docker-entrypoint.sh b/docker/docker-entrypoint.sh index f28627d2..fb8b7bf2 100644 --- a/docker/docker-entrypoint.sh +++ b/docker/docker-entrypoint.sh @@ -18,14 +18,14 @@ php bin/doctrine orm:generate-proxies -n ${flags} echo "Clearing entities cache..." php bin/doctrine orm:clear-cache:metadata -n ${flags} -# Try to download GeoLite2 db file only if the license key env var was defined -if [ ! -z "${GEOLITE_LICENSE_KEY}" ]; then +# Try to download GeoLite2 db file only if the license key env var was defined and skipping was not explicitly set +if [ ! -z "${GEOLITE_LICENSE_KEY}" ] && [ "${SKIP_INITIAL_GEOLITE_DOWNLOAD}" != "true" ]; then echo "Downloading GeoLite2 db file..." php bin/cli visit:download-db -n ${flags} fi # Periodically run visit:locate every hour, if ENABLE_PERIODIC_VISIT_LOCATE=true was provided -if [ $ENABLE_PERIODIC_VISIT_LOCATE ]; then +if [ "${ENABLE_PERIODIC_VISIT_LOCATE}" = "true" ]; then echo "Configuring periodic visit location..." echo "0 * * * * php /etc/shlink/bin/cli visit:locate -q" > /etc/crontabs/root /usr/sbin/crond & diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 38368746..a9de1f0c 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -19,6 +19,7 @@ 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\GeoLite2Options; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Shlinkio\Shlink\Rest\Service\ApiKeyService; use Symfony\Component\Console as SymfonyCli; @@ -75,6 +76,7 @@ return [ Reader::class, LOCAL_LOCK_FACTORY, TrackingOptions::class, + GeoLite2Options::class, ], Util\ProcessRunner::class => [SymfonyCli\Helper\ProcessHelper::class], ApiKey\RoleResolver::class => [DomainService::class, 'config.url_shortener.domain.hostname'], diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php index fe898dbb..47aa50d1 100644 --- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -34,8 +34,8 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat private SymfonyStyle $io; public function __construct( - private VisitLocatorInterface $visitLocator, - private IpLocationResolverInterface $ipLocationResolver, + private readonly VisitLocatorInterface $visitLocator, + private readonly IpLocationResolverInterface $ipLocationResolver, LockFactory $locker, ) { parent::__construct($locker); diff --git a/module/CLI/src/GeoLite/GeolocationDbUpdater.php b/module/CLI/src/GeoLite/GeolocationDbUpdater.php index b04ee3f2..f33b8796 100644 --- a/module/CLI/src/GeoLite/GeolocationDbUpdater.php +++ b/module/CLI/src/GeoLite/GeolocationDbUpdater.php @@ -9,7 +9,9 @@ use GeoIp2\Database\Reader; use MaxMind\Db\Reader\Metadata; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\Core\Options\TrackingOptions; -use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException; +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; @@ -20,10 +22,10 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface private const LOCK_NAME = 'geolocation-db-update'; public function __construct( - private DbUpdaterInterface $dbUpdater, - private Reader $geoLiteDbReader, - private LockFactory $locker, - private TrackingOptions $trackingOptions, + private readonly DbUpdaterInterface $dbUpdater, + private readonly Reader $geoLiteDbReader, + private readonly LockFactory $locker, + private readonly TrackingOptions $trackingOptions, ) { } @@ -52,14 +54,12 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface private function downloadIfNeeded(?callable $beforeDownload, ?callable $handleProgress): GeolocationResult { if (! $this->dbUpdater->databaseFileExists()) { - $this->downloadNewDb(false, $beforeDownload, $handleProgress); - return GeolocationResult::DB_CREATED; + return $this->downloadNewDb(false, $beforeDownload, $handleProgress); } $meta = $this->geoLiteDbReader->metadata(); if ($this->buildIsTooOld($meta)) { - $this->downloadNewDb(true, $beforeDownload, $handleProgress); - return GeolocationResult::DB_UPDATED; + return $this->downloadNewDb(true, $beforeDownload, $handleProgress); } return GeolocationResult::DB_IS_UP_TO_DATE; @@ -95,15 +95,22 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface /** * @throws GeolocationDbUpdateFailedException */ - private function downloadNewDb(bool $olderDbExists, ?callable $beforeDownload, ?callable $handleProgress): void - { + private function downloadNewDb( + bool $olderDbExists, + ?callable $beforeDownload, + ?callable $handleProgress, + ): GeolocationResult { if ($beforeDownload !== null) { $beforeDownload($olderDbExists); } try { $this->dbUpdater->downloadFreshCopy($this->wrapHandleProgressCallback($handleProgress, $olderDbExists)); - } catch (RuntimeException $e) { + return $olderDbExists ? GeolocationResult::DB_UPDATED : GeolocationResult::DB_CREATED; + } catch (MissingLicenseException) { + // If there's no license key, just ignore the error + return GeolocationResult::CHECK_SKIPPED; + } catch (DbUpdateException | WrongIpException $e) { throw $olderDbExists ? GeolocationDbUpdateFailedException::withOlderDb($e) : GeolocationDbUpdateFailedException::withoutOlderDb($e); @@ -116,6 +123,6 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface return null; } - return fn (int $total, int $downloaded) => $handleProgress($total, $downloaded, $olderDbExists); + return static fn (int $total, int $downloaded) => $handleProgress($total, $downloaded, $olderDbExists); } } diff --git a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php index 89667f3b..61056922 100644 --- a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php +++ b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php @@ -15,7 +15,7 @@ use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdater; use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; use Shlinkio\Shlink\Core\Options\TrackingOptions; -use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException; +use Shlinkio\Shlink\IpGeolocation\Exception\DbUpdateException; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; use Symfony\Component\Lock; use Throwable; @@ -48,7 +48,7 @@ class GeolocationDbUpdaterTest extends TestCase public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(): void { $mustBeUpdated = fn () => self::assertTrue(true); - $prev = new RuntimeException(''); + $prev = new DbUpdateException(''); $fileExists = $this->dbUpdater->databaseFileExists()->willReturn(false); $getMeta = $this->geoLiteDbReader->metadata(); @@ -81,7 +81,7 @@ class GeolocationDbUpdaterTest extends TestCase $getMeta = $this->geoLiteDbReader->metadata()->willReturn($this->buildMetaWithBuildEpoch( Chronos::now()->subDays($days)->getTimestamp(), )); - $prev = new RuntimeException(''); + $prev = new DbUpdateException(''); $download = $this->dbUpdater->downloadFreshCopy(null)->willThrow($prev); try { diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 3a4d10f5..f6e198b8 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -10,6 +10,7 @@ use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdater; use Shlinkio\Shlink\Common\Cache\RedisPublishingHelper; use Shlinkio\Shlink\Common\Mercure\MercureHubPublishingHelper; use Shlinkio\Shlink\Common\RabbitMq\RabbitMqPublishingHelper; +use Shlinkio\Shlink\Core\Visit\VisitLocator; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; @@ -21,7 +22,7 @@ return [ EventDispatcher\LocateVisit::class, ], EventDispatcher\Event\GeoLiteDbCreated::class => [ -// EventDispatcher\LocateUnloctedVisits::class, + EventDispatcher\LocateUnlocatedVisits::class, ], ], 'async' => [ @@ -43,6 +44,7 @@ return [ 'dependencies' => [ 'factories' => [ EventDispatcher\LocateVisit::class => ConfigAbstractFactory::class, + EventDispatcher\LocateUnlocatedVisits::class => ConfigAbstractFactory::class, EventDispatcher\NotifyVisitToWebHooks::class => ConfigAbstractFactory::class, EventDispatcher\Mercure\NotifyVisitToMercure::class => ConfigAbstractFactory::class, EventDispatcher\Mercure\NotifyNewShortUrlToMercure::class => ConfigAbstractFactory::class, @@ -72,6 +74,9 @@ return [ EventDispatcher\RedisPubSub\NotifyNewShortUrlToRedis::class => [ EventDispatcher\CloseDbConnectionEventListenerDelegator::class, ], + EventDispatcher\LocateUnlocatedVisits::class => [ + EventDispatcher\CloseDbConnectionEventListenerDelegator::class, + ], EventDispatcher\NotifyVisitToWebHooks::class => [ EventDispatcher\CloseDbConnectionEventListenerDelegator::class, ], @@ -86,6 +91,7 @@ return [ DbUpdater::class, EventDispatcherInterface::class, ], + EventDispatcher\LocateUnlocatedVisits::class => [VisitLocator::class, IpLocationResolverInterface::class], EventDispatcher\NotifyVisitToWebHooks::class => [ 'httpClient', 'em', diff --git a/module/Core/src/EventDispatcher/LocateUnlocatedVisits.php b/module/Core/src/EventDispatcher/LocateUnlocatedVisits.php new file mode 100644 index 00000000..a72e8853 --- /dev/null +++ b/module/Core/src/EventDispatcher/LocateUnlocatedVisits.php @@ -0,0 +1,57 @@ +locator->locateUnlocatedVisits($this); + } + + /** + * @throws IpCannotBeLocatedException + */ + public function geolocateVisit(Visit $visit): Location + { + // TODO This method duplicates code from LocateVisitsCommand. Move to a common place. + if (! $visit->hasRemoteAddr()) { + throw IpCannotBeLocatedException::forEmptyAddress(); + } + + $ipAddr = $visit->getRemoteAddr() ?? ''; + if ($ipAddr === IpAddress::LOCALHOST) { + throw IpCannotBeLocatedException::forLocalhost(); + } + + try { + return $this->ipLocationResolver->resolveIpLocation($ipAddr); + } catch (WrongIpException $e) { + throw IpCannotBeLocatedException::forError($e); + } + } + + public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void + { + // Do nothing + } +}