Merge pull request #2052 from acelaya-forks/feature/fix-geolite-update-backport

Fix infinite GeoLite2 downloads - v3.x backport

CHANGELOG.md

@@ -4,6 +4,41 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org).
 
+## [3.7.4] - 2024-03-09
+### Added
+* *Nothing*
+
+### Changed
+* *Nothing*
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* [#2021](https://github.com/shlinkio/shlink/issues/2021) Fix infinite GeoLite2 downloads.
+
+
+## [3.7.3] - 2024-01-04
+### Added
+* *Nothing*
+
+### Changed
+* [#1968](https://github.com/shlinkio/shlink/issues/1968) Move migrations from `data` to `module/Core`.
+* *Nothing*
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* [#1967](https://github.com/shlinkio/shlink/issues/1967) Allow an empty dir to be mounted in `data` when using the docker image.
+
+
 ## [3.7.2] - 2023-12-26
 ### Added
 * *Nothing*

CONTRIBUTING.md

@@ -46,17 +46,18 @@ This is a simplified version of the project structure:
 ```
 shlink
 ├── bin
-│   └── cli
+│   ├── cli
+│   └── [...]
 ├── config
 │   ├── autoload
 │   ├── params
 │   ├── config.php
-│   └── container.php
+│   ├── container.php
+│   └── [...]
 ├── data
 │   ├── cache
 │   ├── locks
 │   ├── log
-│   ├── migrations
 │   └── proxies
 ├── docs
 │   ├── adr
@@ -67,6 +68,7 @@ shlink
 │   ├── Core
 │   └── Rest
 ├── public
+│   └── [...]
 ├── composer.json
 └── README.md
 ```
@@ -75,7 +77,7 @@ The purposes of every folder are:
 
 * `bin`: It contains the CLI tools. The `cli` one is the main entry point to run Shlink from the command line.
 * `config`: Contains application-wide configurations, which are later merged with the ones provided by every module.
-* `data`: Common runtime-generated git-ignored assets, like logs, caches, etc.
+* `data`: Common git-ignored assets, like logs, caches, lock files, GeoLite DB files, etc. It's the only location where Shlink may need to write at runtime.
 * `docs`: Any project documentation is stored here, like API spec definitions or architectural decision records.
 * `module`: Contains a sub-folder for every module in the project. Modules contain the source code, tests and configurations for every context in the project.
 * `public`: Few assets (like `favicon.ico` or `robots.txt`) and the web entry point are stored here. This web entry point is not used when serving the app with RoadRunner or openswoole.

composer.json

@@ -50,7 +50,7 @@
         "shlinkio/shlink-event-dispatcher": "^3.1",
         "shlinkio/shlink-importer": "^5.2.1",
         "shlinkio/shlink-installer": "^8.7",
-        "shlinkio/shlink-ip-geolocation": "^3.4",
+        "shlinkio/shlink-ip-geolocation": "^4.0",
         "shlinkio/shlink-json": "^1.1",
         "spiral/roadrunner": "^2023.2",
         "spiral/roadrunner-cli": "^2.5",
@@ -75,7 +75,7 @@
         "phpunit/phpunit": "^10.4",
         "roave/security-advisories": "dev-master",
         "shlinkio/php-coding-standard": "~2.3.0",
-        "shlinkio/shlink-test-utils": "^3.8",
+        "shlinkio/shlink-test-utils": "^3.8.1",
         "symfony/var-dumper": "^6.3",
         "veewee/composer-run-parallel": "^1.3"
     },
@@ -116,7 +116,7 @@
         ],
         "cs": "phpcs -s",
         "cs:fix": "phpcbf",
-        "stan": "APP_ENV=test php vendor/bin/phpstan analyse module/*/src module/*/test* module/*/config config docker/config data/migrations --level=8",
+        "stan": "APP_ENV=test php vendor/bin/phpstan analyse module/*/src module/*/test* module/*/config module/*/migrations config docker/config --level=8",
         "test": [
             "@parallel test:unit test:db",
             "@parallel test:api test:cli"

config/autoload/router.global.php

@@ -11,7 +11,9 @@ return [
         'base_path' => EnvVars::BASE_PATH->loadFromEnv(''),
 
         'fastroute' => [
-            FastRouteRouter::CONFIG_CACHE_ENABLED => true,
+            // Disabling config cache for cli, ensures it's never used for openswoole/RoadRunner, and also that console
+            // commands don't generate a cache file that's then used by php-fpm web executions
+            FastRouteRouter::CONFIG_CACHE_ENABLED => PHP_SAPI !== 'cli',
             FastRouteRouter::CONFIG_CACHE_FILE => 'data/cache/fastroute_cached_routes.php',
         ],
     ],

config/cli-config.php

@@ -11,7 +11,7 @@ use Doctrine\Migrations\DependencyFactory;
 return (static function () {
     $migrationsConfig = [
         'migrations_paths' => [
-            'ShlinkMigrations' => 'data/migrations',
+            'ShlinkMigrations' => 'module/Core/migrations',
         ],
         'table_storage' => [
             'table_name' => 'migrations',

config/test/bootstrap_api_tests.php

@@ -29,10 +29,10 @@ register_shutdown_function(function () use ($httpClient): void {
 });
 
 $testHelper->createTestDb(
-    ['bin/cli', 'db:create'],
-    ['bin/cli', 'db:migrate'],
-    ['bin/doctrine', 'orm:schema-tool:drop'],
-    ['bin/doctrine', 'dbal:run-sql'],
+    createDbCommand: ['bin/cli', 'db:create'],
+    migrateDbCommand: ['bin/cli', 'db:migrate'],
+    dropSchemaCommand: ['bin/doctrine', 'orm:schema-tool:drop'],
+    runSqlCommand: ['bin/doctrine', 'dbal:run-sql'],
 );
 ApiTest\ApiTestCase::setApiClient($httpClient);
 ApiTest\ApiTestCase::setSeedFixturesCallback(fn () => $testHelper->seedFixtures($em, $config['data_fixtures'] ?? []));

config/test/bootstrap_db_tests.php

@@ -9,9 +9,9 @@ use Psr\Container\ContainerInterface;
 /** @var ContainerInterface $container */
 $container = require __DIR__ . '/../container.php';
 $container->get(Helper\TestHelper::class)->createTestDb(
-    ['bin/cli', 'db:create'],
-    ['bin/cli', 'db:migrate'],
-    ['bin/doctrine', 'orm:schema-tool:drop'],
-    ['bin/doctrine', 'dbal:run-sql'],
+    createDbCommand: ['bin/cli', 'db:create'],
+    migrateDbCommand: ['bin/cli', 'db:migrate'],
+    dropSchemaCommand: ['bin/doctrine', 'orm:schema-tool:drop'],
+    runSqlCommand: ['bin/doctrine', 'dbal:run-sql'],
 );
 DbTest\DatabaseTestCase::setEntityManager($container->get('em'));

docker/docker-entrypoint.sh

@@ -3,6 +3,9 @@ set -e
 
 cd /etc/shlink
 
+# Create data directories if they do not exist. This allows data dir to be mounted as an empty dir if needed
+mkdir -p data/cache data/locks data/log data/proxies
+
 flags="--no-interaction --clear-db-cache"
 
 # Skip downloading GeoLite2 db file if the license key env var was not defined or skipping was explicitly set

module/CLI/config/dependencies.config.php

@@ -4,7 +4,6 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\CLI;
 
-use GeoIp2\Database\Reader;
 use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory;
 use Laminas\ServiceManager\Factory\InvokableFactory;
 use Shlinkio\Shlink\Common\Doctrine\NoDbNameConnectionFactory;
@@ -17,6 +16,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\GeoLite2ReaderFactory;
 use Shlinkio\Shlink\Rest\Service\ApiKeyService;
 use Symfony\Component\Console as SymfonyCli;
 use Symfony\Component\Lock\LockFactory;
@@ -72,7 +72,7 @@ return [
     ConfigAbstractFactory::class => [
         GeoLite\GeolocationDbUpdater::class => [
             DbUpdater::class,
-            Reader::class,
+            GeoLite2ReaderFactory::class,
             LOCAL_LOCK_FACTORY,
             TrackingOptions::class,
         ],

module/CLI/src/GeoLite/GeolocationDbUpdater.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\CLI\GeoLite;
 
 use Cake\Chronos\Chronos;
+use Closure;
 use GeoIp2\Database\Reader;
 use MaxMind\Db\Reader\Metadata;
 use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
@@ -21,12 +22,19 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
 {
     private const LOCK_NAME = 'geolocation-db-update';
 
+    /** @var Closure(): Reader */
+    private readonly Closure $geoLiteDbReaderFactory;
+
+    /**
+     * @param callable(): Reader $geoLiteDbReaderFactory
+     */
     public function __construct(
         private readonly DbUpdaterInterface $dbUpdater,
-        private readonly Reader $geoLiteDbReader,
+        callable $geoLiteDbReaderFactory,
         private readonly LockFactory $locker,
         private readonly TrackingOptions $trackingOptions,
     ) {
+        $this->geoLiteDbReaderFactory = $geoLiteDbReaderFactory(...);
     }
 
     /**
@@ -57,7 +65,7 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
             return $this->downloadNewDb(false, $beforeDownload, $handleProgress);
         }
 
-        $meta = $this->geoLiteDbReader->metadata();
+        $meta = ($this->geoLiteDbReaderFactory)()->metadata();
         if ($this->buildIsTooOld($meta)) {
             return $this->downloadNewDb(true, $beforeDownload, $handleProgress);
         }

module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php

@@ -187,7 +187,7 @@ class GeolocationDbUpdaterTest extends TestCase
 
         return new GeolocationDbUpdater(
             $this->dbUpdater,
-            $this->geoLiteDbReader,
+            fn () => $this->geoLiteDbReader,
             $locker,
             $options ?? new TrackingOptions(),
         );

phpcs.xml

@@ -12,7 +12,6 @@
     <!-- Paths to check -->
     <file>bin</file>
     <file>module</file>
-    <file>data/migrations</file>
     <file>config</file>
     <file>docker/config</file>
     <file>public/index.php</file>