From bb72c96ebb9a8b56530315b678d4bc8c41d55346 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 26 Nov 2024 10:17:28 +0100 Subject: [PATCH 01/30] Delete some old migrations --- .../Core/migrations/Version20220110113313.php | 73 ------------------- .../Core/migrations/Version20230103105343.php | 53 -------------- .../Core/migrations/Version20230130090946.php | 50 ------------- 3 files changed, 176 deletions(-) delete mode 100644 module/Core/migrations/Version20220110113313.php delete mode 100644 module/Core/migrations/Version20230103105343.php delete mode 100644 module/Core/migrations/Version20230130090946.php diff --git a/module/Core/migrations/Version20220110113313.php b/module/Core/migrations/Version20220110113313.php deleted file mode 100644 index 2b2fb4ea4..000000000 --- a/module/Core/migrations/Version20220110113313.php +++ /dev/null @@ -1,73 +0,0 @@ - [ - 'original_url' => 'unicode_ci', - 'short_code' => 'bin', - 'import_original_short_code' => 'unicode_ci', - 'title' => 'unicode_ci', - ], - 'domains' => [ - 'authority' => 'unicode_ci', - 'base_url_redirect' => 'unicode_ci', - 'regular_not_found_redirect' => 'unicode_ci', - 'invalid_short_url_redirect' => 'unicode_ci', - ], - 'tags' => [ - 'name' => 'unicode_ci', - ], - 'visits' => [ - 'referer' => 'unicode_ci', - 'user_agent' => 'unicode_ci', - 'visited_url' => 'unicode_ci', - ], - 'visit_locations' => [ - 'country_code' => 'unicode_ci', - 'country_name' => 'unicode_ci', - 'region_name' => 'unicode_ci', - 'city_name' => 'unicode_ci', - 'timezone' => 'unicode_ci', - ], - ]; - - public function up(Schema $schema): void - { - $this->skipIf(! $this->isMySql(), 'This only sets MySQL-specific database options'); - - foreach (self::COLLATIONS as $tableName => $columns) { - $table = $schema->getTable($tableName); - - foreach ($columns as $columnName => $collation) { - $table->getColumn($columnName) - ->setPlatformOption('charset', self::CHARSET) - ->setPlatformOption('collation', self::CHARSET . '_' . $collation); - } - } - } - - public function down(Schema $schema): void - { - // No down - } - - public function isTransactional(): bool - { - return ! $this->isMySql(); - } - - private function isMySql(): bool - { - return $this->connection->getDatabasePlatform() instanceof MySQLPlatform; - } -} diff --git a/module/Core/migrations/Version20230103105343.php b/module/Core/migrations/Version20230103105343.php deleted file mode 100644 index c61a8a94e..000000000 --- a/module/Core/migrations/Version20230103105343.php +++ /dev/null @@ -1,53 +0,0 @@ -skipIf($schema->hasTable(self::TABLE_NAME)); - - $table = $schema->createTable(self::TABLE_NAME); - $table->addColumn('id', Types::BIGINT, [ - 'unsigned' => true, - 'autoincrement' => true, - 'notnull' => true, - ]); - $table->setPrimaryKey(['id']); - - $table->addColumn('device_type', Types::STRING, ['length' => 255]); - $table->addColumn('long_url', Types::STRING, ['length' => 2048]); - $table->addColumn('short_url_id', Types::BIGINT, [ - 'unsigned' => true, - 'notnull' => true, - ]); - - $table->addForeignKeyConstraint('short_urls', ['short_url_id'], ['id'], [ - 'onDelete' => 'CASCADE', - 'onUpdate' => 'RESTRICT', - ]); - - $table->addUniqueIndex(['device_type', 'short_url_id'], 'UQ_device_type_per_short_url'); - } - - public function down(Schema $schema): void - { - $this->skipIf(! $schema->hasTable(self::TABLE_NAME)); - $schema->dropTable(self::TABLE_NAME); - } - - public function isTransactional(): bool - { - return ! ($this->connection->getDatabasePlatform() instanceof MySQLPlatform); - } -} diff --git a/module/Core/migrations/Version20230130090946.php b/module/Core/migrations/Version20230130090946.php deleted file mode 100644 index 49e6d9bb5..000000000 --- a/module/Core/migrations/Version20230130090946.php +++ /dev/null @@ -1,50 +0,0 @@ -skipIf(! $this->isMsSql(), 'This only sets MsSQL-specific database options'); - - $shortUrls = $schema->getTable('short_urls'); - $shortCode = $shortUrls->getColumn('short_code'); - // Drop the unique index before changing the collation, as the field is part of this index - $shortUrls->dropIndex('unique_short_code_plus_domain'); - $shortCode->setPlatformOption('collation', 'Latin1_General_CS_AS'); - } - - public function postUp(Schema $schema): void - { - if ($this->isMsSql()) { - // The index needs to be re-created in postUp, but here, we can only use statements run against the - // connection directly - $this->connection->executeStatement( - 'CREATE INDEX unique_short_code_plus_domain ON short_urls (domain_id, short_code);', - ); - } - } - - public function down(Schema $schema): void - { - // No down - } - - public function isTransactional(): bool - { - return ! ($this->connection->getDatabasePlatform() instanceof MySQLPlatform); - } - - private function isMsSql(): bool - { - return $this->connection->getDatabasePlatform() instanceof SQLServerPlatform; - } -} From 8499087a3bc8040c1ad91a8bfa91ad8113f3ae68 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 28 Nov 2024 08:53:19 +0100 Subject: [PATCH 02/30] Move DEFAULT_DOMAIN constant to domains module --- .../Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php | 10 +++++++--- module/Core/migrations/Version20240220214031.php | 1 - 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php index 2277b0e5f..6edd89e57 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php @@ -28,10 +28,14 @@ ->length(2048) ->build(); - fieldWithUtf8Charset($builder->createField('shortCode', Types::STRING), $emConfig, 'bin') + $shortCodeField = fieldWithUtf8Charset($builder->createField('shortCode', Types::STRING), $emConfig, 'bin') ->columnName('short_code') - ->length(255) - ->build(); + ->length(255); + if (($emConfig['connection']['driver'] ?? null) === 'pdo_sqlsrv') { + // Make sure a case-sensitive charset is set in short code for Microsoft SQL Server + $shortCodeField->option('collation', 'Latin1_General_CS_AS'); + } + $shortCodeField->build(); $builder->createField('dateCreated', ChronosDateTimeType::CHRONOS_DATETIME) ->columnName('date_created') diff --git a/module/Core/migrations/Version20240220214031.php b/module/Core/migrations/Version20240220214031.php index adceb7f2b..3af587efa 100644 --- a/module/Core/migrations/Version20240220214031.php +++ b/module/Core/migrations/Version20240220214031.php @@ -20,7 +20,6 @@ final class Version20240220214031 extends AbstractMigration private const DOMAINS_COLUMNS = ['base_url_redirect', 'regular_not_found_redirect', 'invalid_short_url_redirect']; private const TEXT_COLUMNS = [ 'domains' => self::DOMAINS_COLUMNS, - 'device_long_urls' => ['long_url'], 'short_urls' => ['original_url'], ]; From 6331fa3ed340f7e1935394001675c8936fbb06ef Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 28 Nov 2024 12:05:10 +0100 Subject: [PATCH 03/30] Migrate from mobiledetectlib to phpuseragentparser --- composer.json | 2 +- module/Core/src/Model/DeviceType.php | 19 ++++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/composer.json b/composer.json index 68f38f189..fa2872719 100644 --- a/composer.json +++ b/composer.json @@ -23,6 +23,7 @@ "doctrine/dbal": "^4.2", "doctrine/migrations": "^3.8", "doctrine/orm": "^3.3", + "donatj/phpuseragentparser": "^1.10", "endroid/qr-code": "^6.0", "friendsofphp/proxy-manager-lts": "^1.0", "geoip2/geoip2": "^3.0", @@ -39,7 +40,6 @@ "mezzio/mezzio-fastroute": "^3.12", "mezzio/mezzio-problem-details": "^1.15", "mlocati/ip-lib": "^1.18.1", - "mobiledetect/mobiledetectlib": "4.8.x-dev#920c549 as 4.9", "pagerfanta/core": "^3.8", "ramsey/uuid": "^4.7", "shlinkio/doctrine-specification": "^2.1.1", diff --git a/module/Core/src/Model/DeviceType.php b/module/Core/src/Model/DeviceType.php index a4a15cdcd..c63b2dff5 100644 --- a/module/Core/src/Model/DeviceType.php +++ b/module/Core/src/Model/DeviceType.php @@ -2,7 +2,8 @@ namespace Shlinkio\Shlink\Core\Model; -use Detection\MobileDetect; +use donatj\UserAgent\Platforms; +use donatj\UserAgent\UserAgentParser; enum DeviceType: string { @@ -12,17 +13,13 @@ enum DeviceType: string public static function matchFromUserAgent(string $userAgent): self|null { - $detect = new MobileDetect(); - $detect->setUserAgent($userAgent); + static $uaParser = new UserAgentParser(); + $ua = $uaParser->parse($userAgent); - return match (true) { -// $detect->is('iOS') && $detect->isTablet() => self::IOS, // TODO To detect iPad only -// $detect->is('iOS') && ! $detect->isTablet() => self::IOS, // TODO To detect iPhone only -// $detect->is('androidOS') && $detect->isTablet() => self::ANDROID, // TODO To detect Android tablets -// $detect->is('androidOS') && ! $detect->isTablet() => self::ANDROID, // TODO To detect Android phones - $detect->is('iOS') => self::IOS, // Detects both iPhone and iPad - $detect->is('androidOS') => self::ANDROID, // Detects both android phones and android tablets - ! $detect->isMobile() && ! $detect->isTablet() => self::DESKTOP, + return match ($ua->platform()) { + Platforms::IPHONE, Platforms::IPAD => self::IOS, // Detects both iPhone and iPad (except iPadOS 13+) + Platforms::ANDROID => self::ANDROID, // Detects both android phones and android tablets + Platforms::LINUX, Platforms::WINDOWS, Platforms::MACINTOSH, Platforms::CHROME_OS => self::DESKTOP, default => null, }; } From ede58efe965102531ea359a9c0590ff33de2bd70 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 30 Nov 2024 13:53:19 +0100 Subject: [PATCH 04/30] Update docker images to PHP 8.4 --- CHANGELOG.md | 17 +++++++++++++++++ Dockerfile | 4 ++-- data/infra/php.Dockerfile | 2 +- data/infra/roadrunner.Dockerfile | 2 +- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8ac53d5a..19320ac38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ 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). +# [Unreleased] +### Added +* *Nothing* + +### Changed +* * [#2281](https://github.com/shlinkio/shlink/issues/2281) Update docker image to PHP 8.4 + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* *Nothing* + + # [4.3.1] - 2024-11-25 ### Added * *Nothing* diff --git a/Dockerfile b/Dockerfile index 4f3d1ca65..73e62b39a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM php:8.3-alpine3.20 AS base +FROM php:8.4-alpine3.20 AS base ARG SHLINK_VERSION=latest ENV SHLINK_VERSION ${SHLINK_VERSION} @@ -36,7 +36,7 @@ RUN apk add --no-cache --virtual .phpize-deps ${PHPIZE_DEPS} unixodbc-dev && \ apk del .phpize-deps # Install shlink -FROM base as builder +FROM base AS builder COPY . . COPY --from=composer:2 /usr/bin/composer ./composer.phar RUN apk add --no-cache git && \ diff --git a/data/infra/php.Dockerfile b/data/infra/php.Dockerfile index e594664b4..3d9ea0add 100644 --- a/data/infra/php.Dockerfile +++ b/data/infra/php.Dockerfile @@ -1,4 +1,4 @@ -FROM php:8.3-fpm-alpine3.20 +FROM php:8.4-fpm-alpine3.20 MAINTAINER Alejandro Celaya ENV APCU_VERSION 5.1.24 diff --git a/data/infra/roadrunner.Dockerfile b/data/infra/roadrunner.Dockerfile index 198a6867f..f01943b01 100644 --- a/data/infra/roadrunner.Dockerfile +++ b/data/infra/roadrunner.Dockerfile @@ -1,4 +1,4 @@ -FROM php:8.3-alpine3.20 +FROM php:8.4-alpine3.20 MAINTAINER Alejandro Celaya ENV PDO_SQLSRV_VERSION 5.12.0 From c65349d265b05de40fe86f9cf83ac7c2a47bc54d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 1 Dec 2024 09:51:00 +0100 Subject: [PATCH 05/30] Allow the extra path to be ignored when redirecting --- CHANGELOG.md | 8 +++++- composer.json | 4 +-- module/Core/src/Config/EnvVars.php | 8 ++++-- .../Core/src/Config/Options/ExtraPathMode.php | 13 +++++++++ .../Config/Options/UrlShortenerOptions.php | 17 +++++++++--- .../ExtraPathRedirectMiddleware.php | 9 +++++-- .../ExtraPathRedirectMiddlewareTest.php | 27 ++++++++++++++----- 7 files changed, 68 insertions(+), 18 deletions(-) create mode 100644 module/Core/src/Config/Options/ExtraPathMode.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 19320ac38..fd63464d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this # [Unreleased] ### Added -* *Nothing* +* [#2265](https://github.com/shlinkio/shlink/issues/2265) Add a new `REDIRECT_EXTRA_PATH_MODE` option that accepts three values: + + * `default`: Short URLs only match if the path matches their short code or custom slug. + * `append`: Short URLs are matched as soon as the path starts with the short code or custom slug, and the extra path is appended to the long URL before redirecting. + * `ignore`: Short URLs are matched as soon as the path starts with the short code or custom slug, and the extra path is ignored. + + 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 diff --git a/composer.json b/composer.json index fa2872719..2138cefb2 100644 --- a/composer.json +++ b/composer.json @@ -154,8 +154,8 @@ "@test:cli", "phpcov merge build/coverage-cli --html build/coverage-cli/coverage-html && rm build/coverage-cli/*.cov" ], - "swagger:validate": "php-openapi validate docs/swagger/swagger.json", - "swagger:inline": "php-openapi inline docs/swagger/swagger.json docs/swagger/swagger-inlined.json", + "swagger:validate": "@php -d error_reporting=\"E_ALL & ~E_DEPRECATED & ~E_USER_DEPRECATED\" vendor/bin/php-openapi validate docs/swagger/swagger.json", + "swagger:inline": "@php -d error_reporting=\"E_ALL & ~E_DEPRECATED & ~E_USER_DEPRECATED\" vendor/bin/php-openapi inline docs/swagger/swagger.json docs/swagger/swagger-inlined.json", "clean:dev": "rm -f data/database.sqlite && rm -f config/params/generated_config.php" }, "scripts-descriptions": { diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 6cdf62974..e2a6d38ad 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -84,7 +84,7 @@ enum EnvVars: string case IS_HTTPS_ENABLED = 'IS_HTTPS_ENABLED'; case DEFAULT_DOMAIN = 'DEFAULT_DOMAIN'; case AUTO_RESOLVE_TITLES = 'AUTO_RESOLVE_TITLES'; - case REDIRECT_APPEND_EXTRA_PATH = 'REDIRECT_APPEND_EXTRA_PATH'; + case REDIRECT_EXTRA_PATH_MODE = 'REDIRECT_EXTRA_PATH_MODE'; case MULTI_SEGMENT_SLUGS_ENABLED = 'MULTI_SEGMENT_SLUGS_ENABLED'; case ROBOTS_ALLOW_ALL_SHORT_URLS = 'ROBOTS_ALLOW_ALL_SHORT_URLS'; case ROBOTS_USER_AGENTS = 'ROBOTS_USER_AGENTS'; @@ -92,6 +92,8 @@ enum EnvVars: string case MEMORY_LIMIT = 'MEMORY_LIMIT'; case INITIAL_API_KEY = 'INITIAL_API_KEY'; case SKIP_INITIAL_GEOLITE_DOWNLOAD = 'SKIP_INITIAL_GEOLITE_DOWNLOAD'; + /** @deprecated Use REDIRECT_EXTRA_PATH */ + case REDIRECT_APPEND_EXTRA_PATH = 'REDIRECT_APPEND_EXTRA_PATH'; public function loadFromEnv(): mixed { @@ -125,11 +127,13 @@ private function defaultValue(): string|int|bool|null self::DEFAULT_SHORT_CODES_LENGTH => DEFAULT_SHORT_CODES_LENGTH, self::SHORT_URL_MODE => ShortUrlMode::STRICT->value, self::IS_HTTPS_ENABLED, self::AUTO_RESOLVE_TITLES => true, - self::REDIRECT_APPEND_EXTRA_PATH, self::MULTI_SEGMENT_SLUGS_ENABLED, self::SHORT_URL_TRAILING_SLASH => false, self::DEFAULT_DOMAIN, self::BASE_PATH => '', self::CACHE_NAMESPACE => 'Shlink', + // Deprecated. In Shlink 5.0.0, add default value for REDIRECT_EXTRA_PATH_MODE + self::REDIRECT_APPEND_EXTRA_PATH => false, + // self::REDIRECT_EXTRA_PATH_MODE => ExtraPathMode::DEFAULT->value, self::REDIS_PUB_SUB_ENABLED, self::MATOMO_ENABLED, diff --git a/module/Core/src/Config/Options/ExtraPathMode.php b/module/Core/src/Config/Options/ExtraPathMode.php new file mode 100644 index 000000000..4904ca45a --- /dev/null +++ b/module/Core/src/Config/Options/ExtraPathMode.php @@ -0,0 +1,13 @@ +loadFromEnv(), MIN_SHORT_CODES_LENGTH, ); - $mode = EnvVars::SHORT_URL_MODE->loadFromEnv(); + + // Deprecated. Initialize extra path from REDIRECT_APPEND_EXTRA_PATH. + $appendExtraPath = EnvVars::REDIRECT_APPEND_EXTRA_PATH->loadFromEnv(); + $extraPathMode = $appendExtraPath ? ExtraPathMode::APPEND : ExtraPathMode::DEFAULT; + + // If REDIRECT_EXTRA_PATH_MODE was explicitly provided, it has precedence + $extraPathModeFromEnv = EnvVars::REDIRECT_EXTRA_PATH_MODE->loadFromEnv(); + if ($extraPathModeFromEnv !== null) { + $extraPathMode = ExtraPathMode::tryFrom($extraPathModeFromEnv) ?? ExtraPathMode::DEFAULT; + } return new self( defaultDomain: EnvVars::DEFAULT_DOMAIN->loadFromEnv(), schema: ((bool) EnvVars::IS_HTTPS_ENABLED->loadFromEnv()) ? 'https' : 'http', defaultShortCodesLength: $shortCodesLength, autoResolveTitles: (bool) EnvVars::AUTO_RESOLVE_TITLES->loadFromEnv(), - appendExtraPath: (bool) EnvVars::REDIRECT_APPEND_EXTRA_PATH->loadFromEnv(), multiSegmentSlugsEnabled: (bool) EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->loadFromEnv(), trailingSlashEnabled: (bool) EnvVars::SHORT_URL_TRAILING_SLASH->loadFromEnv(), - mode: ShortUrlMode::tryFrom($mode) ?? ShortUrlMode::STRICT, + mode: ShortUrlMode::tryFrom(EnvVars::SHORT_URL_MODE->loadFromEnv()) ?? ShortUrlMode::STRICT, + extraPathMode: $extraPathMode, ); } diff --git a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php index 4b013b330..f101774b4 100644 --- a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php @@ -9,6 +9,7 @@ use Psr\Http\Message\UriInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Shlinkio\Shlink\Core\Config\Options\ExtraPathMode; use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; @@ -51,7 +52,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface private function shouldApplyLogic(NotFoundType|null $notFoundType): bool { - if ($notFoundType === null || ! $this->urlShortenerOptions->appendExtraPath) { + if ($notFoundType === null || $this->urlShortenerOptions->extraPathMode === ExtraPathMode::DEFAULT) { return false; } @@ -75,7 +76,11 @@ private function tryToResolveRedirect( try { $shortUrl = $this->resolver->resolveEnabledShortUrl($identifier); - $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request, $extraPath); + $longUrl = $this->redirectionBuilder->buildShortUrlRedirect( + $shortUrl, + $request, + $this->urlShortenerOptions->extraPathMode === ExtraPathMode::APPEND ? $extraPath : null, + ); $this->requestTracker->trackIfApplicable( $shortUrl, $request->withAttribute(REDIRECT_URL_REQUEST_ATTRIBUTE, $longUrl), diff --git a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php index 84ceb790e..0578c1f86 100644 --- a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php +++ b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php @@ -11,11 +11,13 @@ use Mezzio\Router\RouteResult; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; +use Shlinkio\Shlink\Core\Config\Options\ExtraPathMode; use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; @@ -57,8 +59,8 @@ public function handlerIsCalledWhenConfigPreventsRedirectWithExtraPath( ServerRequestInterface $request, ): void { $options = new UrlShortenerOptions( - appendExtraPath: $appendExtraPath, multiSegmentSlugsEnabled: $multiSegmentEnabled, + extraPathMode: $appendExtraPath ? ExtraPathMode::APPEND : ExtraPathMode::DEFAULT, ); $this->resolver->expects($this->never())->method('resolveEnabledShortUrl'); $this->requestTracker->expects($this->never())->method('trackIfApplicable'); @@ -102,12 +104,17 @@ public static function provideNonRedirectingRequests(): iterable ]; } - #[Test, DataProvider('provideResolves')] + #[Test] + #[TestWith(['multiSegmentEnabled' => false, 'expectedResolveCalls' => 1])] + #[TestWith(['multiSegmentEnabled' => true, 'expectedResolveCalls' => 3])] public function handlerIsCalledWhenNoShortUrlIsFoundAfterExpectedAmountOfIterations( bool $multiSegmentEnabled, int $expectedResolveCalls, ): void { - $options = new UrlShortenerOptions(appendExtraPath: true, multiSegmentSlugsEnabled: $multiSegmentEnabled); + $options = new UrlShortenerOptions( + multiSegmentSlugsEnabled: $multiSegmentEnabled, + extraPathMode: ExtraPathMode::APPEND, + ); $type = $this->createMock(NotFoundType::class); $type->method('isRegularNotFound')->willReturn(true); @@ -127,11 +134,15 @@ public function handlerIsCalledWhenNoShortUrlIsFoundAfterExpectedAmountOfIterati #[Test, DataProvider('provideResolves')] public function visitIsTrackedAndRedirectIsReturnedWhenShortUrlIsFoundAfterExpectedAmountOfIterations( + ExtraPathMode $extraPathMode, bool $multiSegmentEnabled, int $expectedResolveCalls, string|null $expectedExtraPath, ): void { - $options = new UrlShortenerOptions(appendExtraPath: true, multiSegmentSlugsEnabled: $multiSegmentEnabled); + $options = new UrlShortenerOptions( + multiSegmentSlugsEnabled: $multiSegmentEnabled, + extraPathMode: $extraPathMode, + ); $type = $this->createMock(NotFoundType::class); $type->method('isRegularNotFound')->willReturn(true); @@ -171,8 +182,10 @@ function () use ($shortUrl, &$currentIteration, $expectedResolveCalls): ShortUrl public static function provideResolves(): iterable { - yield [false, 1, '/bar/baz']; - yield [true, 3, null]; + yield [ExtraPathMode::APPEND, false, 1, '/bar/baz']; + yield [ExtraPathMode::APPEND, true, 3, null]; + yield [ExtraPathMode::IGNORE, false, 1, null]; + yield [ExtraPathMode::IGNORE, true, 3, null]; } private function middleware(UrlShortenerOptions|null $options = null): ExtraPathRedirectMiddleware @@ -182,7 +195,7 @@ private function middleware(UrlShortenerOptions|null $options = null): ExtraPath $this->requestTracker, $this->redirectionBuilder, $this->redirectResponseHelper, - $options ?? new UrlShortenerOptions(appendExtraPath: true), + $options ?? new UrlShortenerOptions(extraPathMode: ExtraPathMode::APPEND), ); } } From d83081f4e9d31bca0fe51903a17e9c3638b282c4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 1 Dec 2024 12:28:29 +0100 Subject: [PATCH 06/30] Update shlink-installer --- composer.json | 2 +- config/autoload/installer.global.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 2138cefb2..f6cfb17bf 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,7 @@ "shlinkio/shlink-config": "^3.4", "shlinkio/shlink-event-dispatcher": "^4.1", "shlinkio/shlink-importer": "^5.3.2", - "shlinkio/shlink-installer": "^9.3", + "shlinkio/shlink-installer": "dev-develop#957db97 as 9.4", "shlinkio/shlink-ip-geolocation": "^4.2", "shlinkio/shlink-json": "^1.1", "spiral/roadrunner": "^2024.1", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 898e5ef54..e239dd3a1 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -41,7 +41,7 @@ Option\UrlShortener\RedirectStatusCodeConfigOption::class, Option\UrlShortener\RedirectCacheLifeTimeConfigOption::class, Option\UrlShortener\AutoResolveTitlesConfigOption::class, - Option\UrlShortener\AppendExtraPathConfigOption::class, + Option\UrlShortener\ExtraPathModeConfigOption::class, Option\UrlShortener\EnableMultiSegmentSlugsConfigOption::class, Option\UrlShortener\EnableTrailingSlashConfigOption::class, Option\UrlShortener\ShortUrlModeConfigOption::class, From 58de9985968940f5bdc6ff3896529a4e201de102 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 2 Dec 2024 09:13:20 +0100 Subject: [PATCH 07/30] Drop support for PHP 8.2 --- .github/workflows/ci-db-tests.yml | 2 +- .github/workflows/ci-tests.yml | 2 +- .github/workflows/publish-release.yml | 2 +- .github/workflows/publish-swagger-spec.yml | 2 +- CHANGELOG.md | 2 +- README.md | 2 +- composer.json | 2 +- .../CLI/src/Command/Api/DisableKeyCommand.php | 2 +- .../src/Command/Api/GenerateKeyCommand.php | 2 +- .../src/Command/Api/InitialApiKeyCommand.php | 2 +- .../CLI/src/Command/Api/ListKeysCommand.php | 8 +++--- .../src/Command/Api/RenameApiKeyCommand.php | 2 +- .../src/Command/Config/ReadEnvVarCommand.php | 2 +- .../src/Command/Db/CreateDatabaseCommand.php | 6 ++-- .../src/Command/Db/MigrateDatabaseCommand.php | 6 ++-- .../Command/Domain/DomainRedirectsCommand.php | 2 +- .../Command/Domain/GetDomainVisitsCommand.php | 2 +- .../src/Command/Domain/ListDomainsCommand.php | 2 +- .../Integration/MatomoSendVisitsCommand.php | 2 +- .../ManageRedirectRulesCommand.php | 2 +- .../ShortUrl/CreateShortUrlCommand.php | 2 +- .../DeleteExpiredShortUrlsCommand.php | 2 +- .../ShortUrl/DeleteShortUrlCommand.php | 2 +- .../ShortUrl/DeleteShortUrlVisitsCommand.php | 2 +- .../Command/ShortUrl/EditShortUrlCommand.php | 2 +- .../ShortUrl/GetShortUrlVisitsCommand.php | 2 +- .../Command/ShortUrl/ListShortUrlsCommand.php | 2 +- .../Command/ShortUrl/ResolveUrlCommand.php | 2 +- .../CLI/src/Command/Tag/DeleteTagsCommand.php | 4 +-- .../src/Command/Tag/GetTagVisitsCommand.php | 2 +- .../CLI/src/Command/Tag/ListTagsCommand.php | 2 +- .../CLI/src/Command/Tag/RenameTagCommand.php | 2 +- .../src/Command/Util/LockedCommandConfig.php | 2 +- .../Visit/DeleteOrphanVisitsCommand.php | 2 +- .../Visit/DownloadGeoLiteDbCommand.php | 2 +- .../Visit/GetNonOrphanVisitsCommand.php | 2 +- .../Command/Visit/GetOrphanVisitsCommand.php | 2 +- .../src/Command/Visit/LocateVisitsCommand.php | 2 +- .../CLI/src/GeoLite/GeolocationDbUpdater.php | 2 +- module/CLI/src/Util/ExitCode.php | 6 ++-- module/CLI/src/Util/ShlinkTable.php | 4 +-- .../Core/migrations/Version20230211171904.php | 2 +- .../Core/migrations/Version20230303164233.php | 2 +- .../Core/migrations/Version20240220214031.php | 8 ++++-- .../Core/migrations/Version20241124112257.php | 2 +- module/Core/src/Action/Model/QrCodeParams.php | 6 ++-- .../src/Config/NotFoundRedirectResolver.php | 4 +-- .../Config/PostProcessor/BasePathPrefixer.php | 2 +- .../MultiSegmentSlugProcessor.php | 4 +-- module/Core/src/Domain/Entity/Domain.php | 2 +- .../Validation/DomainRedirectsInputFilter.php | 8 +++--- .../ErrorHandler/NotFoundTemplateHandler.php | 6 ++-- .../src/Exception/DeleteShortUrlException.php | 4 +-- .../src/Exception/DomainNotFoundException.php | 4 +-- .../ForbiddenTagOperationException.php | 4 +-- .../src/Exception/NonUniqueSlugException.php | 4 +-- .../Exception/ShortUrlNotFoundException.php | 4 +-- .../src/Exception/TagConflictException.php | 4 +-- .../src/Exception/TagNotFoundException.php | 4 +-- .../src/Exception/ValidationException.php | 4 +-- .../Core/src/Matomo/MatomoTrackerBuilder.php | 2 +- module/Core/src/Model/Ordering.php | 6 ++-- .../Validation/RedirectRulesInputFilter.php | 12 ++++---- .../Helper/ShortUrlTitleResolutionHelper.php | 8 +++--- .../TrimTrailingSlashMiddleware.php | 2 +- .../Model/Validation/CustomSlugValidator.php | 4 +-- .../Model/Validation/ShortUrlInputFilter.php | 28 +++++++++---------- .../Validation/ShortUrlsParamsInputFilter.php | 22 +++++++-------- module/Core/src/Visit/Model/Visitor.php | 10 +++---- .../VisitIterationRepositoryInterface.php | 2 +- module/Core/test/Action/QrCodeActionTest.php | 4 +-- .../Core/test/Action/RedirectActionTest.php | 2 +- .../ShortUrlTitleResolutionHelperTest.php | 2 +- module/Core/test/Visit/RequestTrackerTest.php | 2 +- module/Rest/src/Action/AbstractRestAction.php | 4 +-- .../Action/Domain/DomainRedirectsAction.php | 6 ++-- .../src/Action/Domain/ListDomainsAction.php | 10 ++++--- module/Rest/src/Action/HealthAction.php | 12 ++++---- module/Rest/src/Action/MercureInfoAction.php | 10 ++++--- .../RedirectRule/ListRedirectRulesAction.php | 4 +-- .../RedirectRule/SetRedirectRulesAction.php | 4 +-- .../Action/ShortUrl/CreateShortUrlAction.php | 4 +-- .../Action/ShortUrl/DeleteShortUrlAction.php | 4 +-- .../ShortUrl/DeleteShortUrlVisitsAction.php | 4 +-- .../Action/ShortUrl/EditShortUrlAction.php | 4 +-- .../Action/ShortUrl/ListShortUrlsAction.php | 4 +-- .../Action/ShortUrl/ResolveShortUrlAction.php | 4 +-- .../SingleStepCreateShortUrlAction.php | 4 +-- .../Rest/src/Action/Tag/DeleteTagsAction.php | 6 ++-- module/Rest/src/Action/Tag/ListTagsAction.php | 4 +-- .../Rest/src/Action/Tag/TagsStatsAction.php | 4 +-- .../Rest/src/Action/Tag/UpdateTagAction.php | 6 ++-- .../Action/Visit/AbstractListVisitsAction.php | 2 +- .../Action/Visit/DeleteOrphanVisitsAction.php | 4 +-- .../src/Action/Visit/DomainVisitsAction.php | 2 +- .../src/Action/Visit/GlobalVisitsAction.php | 6 ++-- .../Action/Visit/NonOrphanVisitsAction.php | 2 +- .../src/Action/Visit/OrphanVisitsAction.php | 2 +- .../src/Action/Visit/ShortUrlVisitsAction.php | 2 +- .../Rest/src/Action/Visit/TagVisitsAction.php | 2 +- module/Rest/src/ConfigProvider.php | 6 ++-- .../Rest/src/Exception/MercureException.php | 4 +-- .../MissingAuthenticationException.php | 4 +-- .../VerifyAuthenticationException.php | 2 +- .../Middleware/AuthenticationMiddleware.php | 2 +- ...teShortUrlContentNegotiationMiddleware.php | 4 +-- .../test-api/Action/ListRedirectRulesTest.php | 4 +-- .../test-api/Action/ListShortUrlsTest.php | 12 ++++---- .../Rest/test-api/Action/OrphanVisitsTest.php | 6 ++-- .../test-api/Action/SetRedirectRulesTest.php | 4 +-- 110 files changed, 232 insertions(+), 224 deletions(-) diff --git a/.github/workflows/ci-db-tests.yml b/.github/workflows/ci-db-tests.yml index 010c635f8..cb7638bec 100644 --- a/.github/workflows/ci-db-tests.yml +++ b/.github/workflows/ci-db-tests.yml @@ -13,7 +13,7 @@ jobs: runs-on: ubuntu-24.04 strategy: matrix: - php-version: ['8.2', '8.3', '8.4'] + php-version: ['8.3', '8.4'] env: LC_ALL: C steps: diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index c26aaaca4..f0b7d8470 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -13,7 +13,7 @@ jobs: runs-on: ubuntu-24.04 strategy: matrix: - php-version: ['8.2', '8.3', '8.4'] + php-version: ['8.3', '8.4'] env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # rr get-binary picks this env automatically steps: diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index 443d34a9f..a2783f97e 100644 --- a/.github/workflows/publish-release.yml +++ b/.github/workflows/publish-release.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubuntu-24.04 strategy: matrix: - php-version: ['8.2', '8.3', '8.4'] + php-version: ['8.3', '8.4'] steps: - uses: actions/checkout@v4 - uses: './.github/actions/ci-setup' diff --git a/.github/workflows/publish-swagger-spec.yml b/.github/workflows/publish-swagger-spec.yml index 9607206ae..95692294c 100644 --- a/.github/workflows/publish-swagger-spec.yml +++ b/.github/workflows/publish-swagger-spec.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubuntu-24.04 strategy: matrix: - php-version: ['8.2'] + php-version: ['8.3'] steps: - uses: actions/checkout@v4 - name: Determine version diff --git a/CHANGELOG.md b/CHANGELOG.md index fd63464d7..12b5aae40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * *Nothing* ### Removed -* *Nothing* +* * [#2247](https://github.com/shlinkio/shlink/issues/2247) Drop support for PHP 8.2 ### Fixed * *Nothing* diff --git a/README.md b/README.md index 681a9495c..e061467d5 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ The idea is that you can just generate a container using the image and provide t First, make sure the host where you are going to run shlink fulfills these requirements: -* PHP 8.2 or 8.3 +* PHP 8.3 or 8.4 * The next PHP extensions: json, curl, pdo, intl, gd and gmp/bcmath. * apcu extension is recommended if you don't plan to use RoadRunner. * xml extension is required if you want to generate QR codes in svg format. diff --git a/composer.json b/composer.json index f6cfb17bf..bc81820e2 100644 --- a/composer.json +++ b/composer.json @@ -12,7 +12,7 @@ } ], "require": { - "php": "^8.2", + "php": "^8.3", "ext-curl": "*", "ext-gd": "*", "ext-json": "*", diff --git a/module/CLI/src/Command/Api/DisableKeyCommand.php b/module/CLI/src/Command/Api/DisableKeyCommand.php index c2ed41738..93178bb52 100644 --- a/module/CLI/src/Command/Api/DisableKeyCommand.php +++ b/module/CLI/src/Command/Api/DisableKeyCommand.php @@ -20,7 +20,7 @@ class DisableKeyCommand extends Command { - public const NAME = 'api-key:disable'; + public const string NAME = 'api-key:disable'; public function __construct(private readonly ApiKeyServiceInterface $apiKeyService) { diff --git a/module/CLI/src/Command/Api/GenerateKeyCommand.php b/module/CLI/src/Command/Api/GenerateKeyCommand.php index 9fc0bb1d9..2790fa8c6 100644 --- a/module/CLI/src/Command/Api/GenerateKeyCommand.php +++ b/module/CLI/src/Command/Api/GenerateKeyCommand.php @@ -23,7 +23,7 @@ class GenerateKeyCommand extends Command { - public const NAME = 'api-key:generate'; + public const string NAME = 'api-key:generate'; public function __construct( private readonly ApiKeyServiceInterface $apiKeyService, diff --git a/module/CLI/src/Command/Api/InitialApiKeyCommand.php b/module/CLI/src/Command/Api/InitialApiKeyCommand.php index 0f4945a96..e6515bc37 100644 --- a/module/CLI/src/Command/Api/InitialApiKeyCommand.php +++ b/module/CLI/src/Command/Api/InitialApiKeyCommand.php @@ -13,7 +13,7 @@ class InitialApiKeyCommand extends Command { - public const NAME = 'api-key:initial'; + public const string NAME = 'api-key:initial'; public function __construct(private readonly ApiKeyServiceInterface $apiKeyService) { diff --git a/module/CLI/src/Command/Api/ListKeysCommand.php b/module/CLI/src/Command/Api/ListKeysCommand.php index d341389d1..69870a9bf 100644 --- a/module/CLI/src/Command/Api/ListKeysCommand.php +++ b/module/CLI/src/Command/Api/ListKeysCommand.php @@ -21,11 +21,11 @@ class ListKeysCommand extends Command { - private const ERROR_STRING_PATTERN = '%s'; - private const SUCCESS_STRING_PATTERN = '%s'; - private const WARNING_STRING_PATTERN = '%s'; + private const string ERROR_STRING_PATTERN = '%s'; + private const string SUCCESS_STRING_PATTERN = '%s'; + private const string WARNING_STRING_PATTERN = '%s'; - public const NAME = 'api-key:list'; + public const string NAME = 'api-key:list'; public function __construct(private readonly ApiKeyServiceInterface $apiKeyService) { diff --git a/module/CLI/src/Command/Api/RenameApiKeyCommand.php b/module/CLI/src/Command/Api/RenameApiKeyCommand.php index f7e24992b..eb6625390 100644 --- a/module/CLI/src/Command/Api/RenameApiKeyCommand.php +++ b/module/CLI/src/Command/Api/RenameApiKeyCommand.php @@ -19,7 +19,7 @@ class RenameApiKeyCommand extends Command { - public const NAME = 'api-key:rename'; + public const string NAME = 'api-key:rename'; public function __construct(private readonly ApiKeyServiceInterface $apiKeyService) { diff --git a/module/CLI/src/Command/Config/ReadEnvVarCommand.php b/module/CLI/src/Command/Config/ReadEnvVarCommand.php index 76ec36aee..72e07f97f 100644 --- a/module/CLI/src/Command/Config/ReadEnvVarCommand.php +++ b/module/CLI/src/Command/Config/ReadEnvVarCommand.php @@ -21,7 +21,7 @@ class ReadEnvVarCommand extends Command { - public const NAME = 'env-var:read'; + public const string NAME = 'env-var:read'; /** @var Closure(string $envVar): mixed */ private readonly Closure $loadEnvVar; diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php index 53b854d1d..b2a2fa185 100644 --- a/module/CLI/src/Command/Db/CreateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/CreateDatabaseCommand.php @@ -24,9 +24,9 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand { private readonly Connection $regularConn; - public const NAME = 'db:create'; - public const DOCTRINE_SCRIPT = 'bin/doctrine'; - public const DOCTRINE_CREATE_SCHEMA_COMMAND = 'orm:schema-tool:create'; + public const string NAME = 'db:create'; + public const string DOCTRINE_SCRIPT = 'bin/doctrine'; + public const string DOCTRINE_CREATE_SCHEMA_COMMAND = 'orm:schema-tool:create'; public function __construct( LockFactory $locker, diff --git a/module/CLI/src/Command/Db/MigrateDatabaseCommand.php b/module/CLI/src/Command/Db/MigrateDatabaseCommand.php index a912cf242..2e280a062 100644 --- a/module/CLI/src/Command/Db/MigrateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/MigrateDatabaseCommand.php @@ -11,9 +11,9 @@ class MigrateDatabaseCommand extends AbstractDatabaseCommand { - public const NAME = 'db:migrate'; - public const DOCTRINE_MIGRATIONS_SCRIPT = 'vendor/doctrine/migrations/bin/doctrine-migrations.php'; - public const DOCTRINE_MIGRATE_COMMAND = 'migrations:migrate'; + public const string NAME = 'db:migrate'; + public const string DOCTRINE_MIGRATIONS_SCRIPT = 'vendor/doctrine/migrations/bin/doctrine-migrations.php'; + public const string DOCTRINE_MIGRATE_COMMAND = 'migrations:migrate'; protected function configure(): void { diff --git a/module/CLI/src/Command/Domain/DomainRedirectsCommand.php b/module/CLI/src/Command/Domain/DomainRedirectsCommand.php index 61e4a93b8..105c10e30 100644 --- a/module/CLI/src/Command/Domain/DomainRedirectsCommand.php +++ b/module/CLI/src/Command/Domain/DomainRedirectsCommand.php @@ -21,7 +21,7 @@ class DomainRedirectsCommand extends Command { - public const NAME = 'domain:redirects'; + public const string NAME = 'domain:redirects'; public function __construct(private readonly DomainServiceInterface $domainService) { diff --git a/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php b/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php index 6477539eb..2891c44fe 100644 --- a/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php +++ b/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php @@ -16,7 +16,7 @@ class GetDomainVisitsCommand extends AbstractVisitsListCommand { - public const NAME = 'domain:visits'; + public const string NAME = 'domain:visits'; public function __construct( VisitsStatsHelperInterface $visitsHelper, diff --git a/module/CLI/src/Command/Domain/ListDomainsCommand.php b/module/CLI/src/Command/Domain/ListDomainsCommand.php index 7e6b8cc30..79c181f75 100644 --- a/module/CLI/src/Command/Domain/ListDomainsCommand.php +++ b/module/CLI/src/Command/Domain/ListDomainsCommand.php @@ -18,7 +18,7 @@ class ListDomainsCommand extends Command { - public const NAME = 'domain:list'; + public const string NAME = 'domain:list'; public function __construct(private readonly DomainServiceInterface $domainService) { diff --git a/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php b/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php index ba9a794e6..9a41cc05e 100644 --- a/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php +++ b/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php @@ -22,7 +22,7 @@ class MatomoSendVisitsCommand extends Command implements VisitSendingProgressTrackerInterface { - public const NAME = 'integration:matomo:send-visits'; + public const string NAME = 'integration:matomo:send-visits'; private readonly bool $matomoEnabled; private SymfonyStyle $io; diff --git a/module/CLI/src/Command/RedirectRule/ManageRedirectRulesCommand.php b/module/CLI/src/Command/RedirectRule/ManageRedirectRulesCommand.php index 13b6d1cc9..646b9d77b 100644 --- a/module/CLI/src/Command/RedirectRule/ManageRedirectRulesCommand.php +++ b/module/CLI/src/Command/RedirectRule/ManageRedirectRulesCommand.php @@ -19,7 +19,7 @@ class ManageRedirectRulesCommand extends Command { - public const NAME = 'short-url:manage-rules'; + public const string NAME = 'short-url:manage-rules'; private readonly ShortUrlIdentifierInput $shortUrlIdentifierInput; diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index e3a9b180a..df341c965 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -20,7 +20,7 @@ class CreateShortUrlCommand extends Command { - public const NAME = 'short-url:create'; + public const string NAME = 'short-url:create'; private SymfonyStyle $io; private readonly ShortUrlDataInput $shortUrlDataInput; diff --git a/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php index 109beff78..1fc9dc384 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php @@ -17,7 +17,7 @@ class DeleteExpiredShortUrlsCommand extends Command { - public const NAME = 'short-url:delete-expired'; + public const string NAME = 'short-url:delete-expired'; public function __construct(private readonly DeleteShortUrlServiceInterface $deleteShortUrlService) { diff --git a/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php index 63e9dab58..edda1b296 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php @@ -19,7 +19,7 @@ class DeleteShortUrlCommand extends Command { - public const NAME = 'short-url:delete'; + public const string NAME = 'short-url:delete'; private readonly ShortUrlIdentifierInput $shortUrlIdentifierInput; diff --git a/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php b/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php index a720e12dd..a9a709a10 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php @@ -16,7 +16,7 @@ class DeleteShortUrlVisitsCommand extends AbstractDeleteVisitsCommand { - public const NAME = 'short-url:visits-delete'; + public const string NAME = 'short-url:visits-delete'; private readonly ShortUrlIdentifierInput $shortUrlIdentifierInput; diff --git a/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php index 048b3934b..ad9aaf703 100644 --- a/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php @@ -19,7 +19,7 @@ class EditShortUrlCommand extends Command { - public const NAME = 'short-url:edit'; + public const string NAME = 'short-url:edit'; private readonly ShortUrlDataInput $shortUrlDataInput; private readonly ShortUrlIdentifierInput $shortUrlIdentifierInput; diff --git a/module/CLI/src/Command/ShortUrl/GetShortUrlVisitsCommand.php b/module/CLI/src/Command/ShortUrl/GetShortUrlVisitsCommand.php index 8583a242e..8507b9ca1 100644 --- a/module/CLI/src/Command/ShortUrl/GetShortUrlVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/GetShortUrlVisitsCommand.php @@ -16,7 +16,7 @@ class GetShortUrlVisitsCommand extends AbstractVisitsListCommand { - public const NAME = 'short-url:visits'; + public const string NAME = 'short-url:visits'; private ShortUrlIdentifierInput $shortUrlIdentifierInput; diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index fffeb1f61..72222a08f 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -33,7 +33,7 @@ class ListShortUrlsCommand extends Command { - public const NAME = 'short-url:list'; + public const string NAME = 'short-url:list'; private readonly StartDateOption $startDateOption; private readonly EndDateOption $endDateOption; diff --git a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php index 0a207b685..7935df6d1 100644 --- a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php @@ -17,7 +17,7 @@ class ResolveUrlCommand extends Command { - public const NAME = 'short-url:parse'; + public const string NAME = 'short-url:parse'; private readonly ShortUrlIdentifierInput $shortUrlIdentifierInput; diff --git a/module/CLI/src/Command/Tag/DeleteTagsCommand.php b/module/CLI/src/Command/Tag/DeleteTagsCommand.php index cf05f1b5f..2f13e7759 100644 --- a/module/CLI/src/Command/Tag/DeleteTagsCommand.php +++ b/module/CLI/src/Command/Tag/DeleteTagsCommand.php @@ -14,9 +14,9 @@ class DeleteTagsCommand extends Command { - public const NAME = 'tag:delete'; + public const string NAME = 'tag:delete'; - public function __construct(private TagServiceInterface $tagService) + public function __construct(private readonly TagServiceInterface $tagService) { parent::__construct(); } diff --git a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php index 18da41fbf..b3c083bc7 100644 --- a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php +++ b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php @@ -16,7 +16,7 @@ class GetTagVisitsCommand extends AbstractVisitsListCommand { - public const NAME = 'tag:visits'; + public const string NAME = 'tag:visits'; public function __construct( VisitsStatsHelperInterface $visitsHelper, diff --git a/module/CLI/src/Command/Tag/ListTagsCommand.php b/module/CLI/src/Command/Tag/ListTagsCommand.php index 2efeac5cf..8333b82e7 100644 --- a/module/CLI/src/Command/Tag/ListTagsCommand.php +++ b/module/CLI/src/Command/Tag/ListTagsCommand.php @@ -17,7 +17,7 @@ class ListTagsCommand extends Command { - public const NAME = 'tag:list'; + public const string NAME = 'tag:list'; public function __construct(private readonly TagServiceInterface $tagService) { diff --git a/module/CLI/src/Command/Tag/RenameTagCommand.php b/module/CLI/src/Command/Tag/RenameTagCommand.php index 5830858eb..42092d047 100644 --- a/module/CLI/src/Command/Tag/RenameTagCommand.php +++ b/module/CLI/src/Command/Tag/RenameTagCommand.php @@ -17,7 +17,7 @@ class RenameTagCommand extends Command { - public const NAME = 'tag:rename'; + public const string NAME = 'tag:rename'; public function __construct(private readonly TagServiceInterface $tagService) { diff --git a/module/CLI/src/Command/Util/LockedCommandConfig.php b/module/CLI/src/Command/Util/LockedCommandConfig.php index 8e3573294..a8834d92f 100644 --- a/module/CLI/src/Command/Util/LockedCommandConfig.php +++ b/module/CLI/src/Command/Util/LockedCommandConfig.php @@ -6,7 +6,7 @@ final class LockedCommandConfig { - public const DEFAULT_TTL = 600.0; // 10 minutes + public const float DEFAULT_TTL = 600.0; // 10 minutes private function __construct( public readonly string $lockName, diff --git a/module/CLI/src/Command/Visit/DeleteOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/DeleteOrphanVisitsCommand.php index 2b34ae52e..056a9c603 100644 --- a/module/CLI/src/Command/Visit/DeleteOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/DeleteOrphanVisitsCommand.php @@ -13,7 +13,7 @@ class DeleteOrphanVisitsCommand extends AbstractDeleteVisitsCommand { - public const NAME = 'visit:orphan-delete'; + public const string NAME = 'visit:orphan-delete'; public function __construct(private readonly VisitsDeleterInterface $deleter) { diff --git a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php index 41674a79b..0fdd8ae38 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -18,7 +18,7 @@ class DownloadGeoLiteDbCommand extends Command { - public const NAME = 'visit:download-db'; + public const string NAME = 'visit:download-db'; private ProgressBar|null $progressBar = null; diff --git a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php index 580355095..445bd36fa 100644 --- a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php @@ -14,7 +14,7 @@ class GetNonOrphanVisitsCommand extends AbstractVisitsListCommand { - public const NAME = 'visit:non-orphan'; + public const string NAME = 'visit:non-orphan'; public function __construct( VisitsStatsHelperInterface $visitsHelper, diff --git a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php index ea5c0fe2d..d282d3105 100644 --- a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php @@ -17,7 +17,7 @@ class GetOrphanVisitsCommand extends AbstractVisitsListCommand { - public const NAME = 'visit:orphan'; + public const string NAME = 'visit:orphan'; protected function configure(): void { diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php index 596b287e9..66e33a787 100644 --- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -29,7 +29,7 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocationHelperInterface { - public const NAME = 'visit:locate'; + public const string NAME = 'visit:locate'; private SymfonyStyle $io; diff --git a/module/CLI/src/GeoLite/GeolocationDbUpdater.php b/module/CLI/src/GeoLite/GeolocationDbUpdater.php index 2a0fda3bb..7bdf8150e 100644 --- a/module/CLI/src/GeoLite/GeolocationDbUpdater.php +++ b/module/CLI/src/GeoLite/GeolocationDbUpdater.php @@ -20,7 +20,7 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface { - private const LOCK_NAME = 'geolocation-db-update'; + private const string LOCK_NAME = 'geolocation-db-update'; /** @var Closure(): Reader */ private readonly Closure $geoLiteDbReaderFactory; diff --git a/module/CLI/src/Util/ExitCode.php b/module/CLI/src/Util/ExitCode.php index 128b9f529..f2ffa16bd 100644 --- a/module/CLI/src/Util/ExitCode.php +++ b/module/CLI/src/Util/ExitCode.php @@ -6,7 +6,7 @@ final class ExitCode { - public const EXIT_SUCCESS = 0; - public const EXIT_FAILURE = -1; - public const EXIT_WARNING = 1; + public const int EXIT_SUCCESS = 0; + public const int EXIT_FAILURE = -1; + public const int EXIT_WARNING = 1; } diff --git a/module/CLI/src/Util/ShlinkTable.php b/module/CLI/src/Util/ShlinkTable.php index 108237344..ec0f768f8 100644 --- a/module/CLI/src/Util/ShlinkTable.php +++ b/module/CLI/src/Util/ShlinkTable.php @@ -12,8 +12,8 @@ final class ShlinkTable { - private const DEFAULT_STYLE_NAME = 'default'; - private const TABLE_TITLE_STYLE = ' %s '; + private const string DEFAULT_STYLE_NAME = 'default'; + private const string TABLE_TITLE_STYLE = ' %s '; private function __construct(private readonly Table $baseTable, private readonly bool $withRowSeparators = false) { diff --git a/module/Core/migrations/Version20230211171904.php b/module/Core/migrations/Version20230211171904.php index 1d1acbf77..ff275a8ea 100644 --- a/module/Core/migrations/Version20230211171904.php +++ b/module/Core/migrations/Version20230211171904.php @@ -10,7 +10,7 @@ final class Version20230211171904 extends AbstractMigration { - private const INDEX_NAME = 'IDX_visits_potential_bot'; + private const string INDEX_NAME = 'IDX_visits_potential_bot'; public function up(Schema $schema): void { diff --git a/module/Core/migrations/Version20230303164233.php b/module/Core/migrations/Version20230303164233.php index 3e64c03c0..ae9e7839f 100644 --- a/module/Core/migrations/Version20230303164233.php +++ b/module/Core/migrations/Version20230303164233.php @@ -10,7 +10,7 @@ final class Version20230303164233 extends AbstractMigration { - private const INDEX_NAME = 'visits_potential_bot_IDX'; + private const string INDEX_NAME = 'visits_potential_bot_IDX'; public function up(Schema $schema): void { diff --git a/module/Core/migrations/Version20240220214031.php b/module/Core/migrations/Version20240220214031.php index 3af587efa..b19176ef8 100644 --- a/module/Core/migrations/Version20240220214031.php +++ b/module/Core/migrations/Version20240220214031.php @@ -17,8 +17,12 @@ */ final class Version20240220214031 extends AbstractMigration { - private const DOMAINS_COLUMNS = ['base_url_redirect', 'regular_not_found_redirect', 'invalid_short_url_redirect']; - private const TEXT_COLUMNS = [ + private const array DOMAINS_COLUMNS = [ + 'base_url_redirect', + 'regular_not_found_redirect', + 'invalid_short_url_redirect', + ]; + private const array TEXT_COLUMNS = [ 'domains' => self::DOMAINS_COLUMNS, 'short_urls' => ['original_url'], ]; diff --git a/module/Core/migrations/Version20241124112257.php b/module/Core/migrations/Version20241124112257.php index c11cbe2bd..33807d0d7 100644 --- a/module/Core/migrations/Version20241124112257.php +++ b/module/Core/migrations/Version20241124112257.php @@ -11,7 +11,7 @@ final class Version20241124112257 extends AbstractMigration { - private const COLUMN_NAME = 'redirect_url'; + private const string COLUMN_NAME = 'redirect_url'; public function up(Schema $schema): void { diff --git a/module/Core/src/Action/Model/QrCodeParams.php b/module/Core/src/Action/Model/QrCodeParams.php index 3be9097e6..8bc5b317e 100644 --- a/module/Core/src/Action/Model/QrCodeParams.php +++ b/module/Core/src/Action/Model/QrCodeParams.php @@ -30,9 +30,9 @@ final class QrCodeParams { - private const MIN_SIZE = 50; - private const MAX_SIZE = 1000; - private const SUPPORTED_FORMATS = ['png', 'svg']; + private const int MIN_SIZE = 50; + private const int MAX_SIZE = 1000; + private const array SUPPORTED_FORMATS = ['png', 'svg']; private function __construct( public readonly int $size, diff --git a/module/Core/src/Config/NotFoundRedirectResolver.php b/module/Core/src/Config/NotFoundRedirectResolver.php index 657336c1b..dbdf81515 100644 --- a/module/Core/src/Config/NotFoundRedirectResolver.php +++ b/module/Core/src/Config/NotFoundRedirectResolver.php @@ -17,8 +17,8 @@ class NotFoundRedirectResolver implements NotFoundRedirectResolverInterface { - private const DOMAIN_PLACEHOLDER = '{DOMAIN}'; - private const ORIGINAL_PATH_PLACEHOLDER = '{ORIGINAL_PATH}'; + private const string DOMAIN_PLACEHOLDER = '{DOMAIN}'; + private const string ORIGINAL_PATH_PLACEHOLDER = '{ORIGINAL_PATH}'; public function __construct( private readonly RedirectResponseHelperInterface $redirectResponseHelper, diff --git a/module/Core/src/Config/PostProcessor/BasePathPrefixer.php b/module/Core/src/Config/PostProcessor/BasePathPrefixer.php index 616759f18..b8ed510c4 100644 --- a/module/Core/src/Config/PostProcessor/BasePathPrefixer.php +++ b/module/Core/src/Config/PostProcessor/BasePathPrefixer.php @@ -8,7 +8,7 @@ class BasePathPrefixer { - private const ELEMENTS_WITH_PATH = ['routes', 'middleware_pipeline']; + private const array ELEMENTS_WITH_PATH = ['routes', 'middleware_pipeline']; public function __invoke(array $config): array { diff --git a/module/Core/src/Config/PostProcessor/MultiSegmentSlugProcessor.php b/module/Core/src/Config/PostProcessor/MultiSegmentSlugProcessor.php index c4078890b..aa9eac06f 100644 --- a/module/Core/src/Config/PostProcessor/MultiSegmentSlugProcessor.php +++ b/module/Core/src/Config/PostProcessor/MultiSegmentSlugProcessor.php @@ -11,8 +11,8 @@ class MultiSegmentSlugProcessor { - private const SINGLE_SEGMENT_PATTERN = '{shortCode}'; - private const MULTI_SEGMENT_PATTERN = '{shortCode:.+}'; + private const string SINGLE_SEGMENT_PATTERN = '{shortCode}'; + private const string MULTI_SEGMENT_PATTERN = '{shortCode:.+}'; public function __invoke(array $config): array { diff --git a/module/Core/src/Domain/Entity/Domain.php b/module/Core/src/Domain/Entity/Domain.php index 628335cd0..b55a9dee2 100644 --- a/module/Core/src/Domain/Entity/Domain.php +++ b/module/Core/src/Domain/Entity/Domain.php @@ -11,7 +11,7 @@ class Domain extends AbstractEntity implements JsonSerializable, NotFoundRedirectConfigInterface { - public const DEFAULT_AUTHORITY = 'DEFAULT'; + public const string DEFAULT_AUTHORITY = 'DEFAULT'; private function __construct( public readonly string $authority, diff --git a/module/Core/src/Domain/Validation/DomainRedirectsInputFilter.php b/module/Core/src/Domain/Validation/DomainRedirectsInputFilter.php index d448b8198..39d9fa8e1 100644 --- a/module/Core/src/Domain/Validation/DomainRedirectsInputFilter.php +++ b/module/Core/src/Domain/Validation/DomainRedirectsInputFilter.php @@ -11,10 +11,10 @@ /** @extends InputFilter */ class DomainRedirectsInputFilter extends InputFilter { - public const DOMAIN = 'domain'; - public const BASE_URL_REDIRECT = 'baseUrlRedirect'; - public const REGULAR_404_REDIRECT = 'regular404Redirect'; - public const INVALID_SHORT_URL_REDIRECT = 'invalidShortUrlRedirect'; + public const string DOMAIN = 'domain'; + public const string BASE_URL_REDIRECT = 'baseUrlRedirect'; + public const string REGULAR_404_REDIRECT = 'regular404Redirect'; + public const string INVALID_SHORT_URL_REDIRECT = 'invalidShortUrlRedirect'; private function __construct() { diff --git a/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php b/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php index 9b59f886a..6fc3b5003 100644 --- a/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php +++ b/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php @@ -17,9 +17,9 @@ class NotFoundTemplateHandler implements RequestHandlerInterface { - private const TEMPLATES_BASE_DIR = __DIR__ . '/../../templates'; - public const NOT_FOUND_TEMPLATE = '404.html'; - public const INVALID_SHORT_CODE_TEMPLATE = 'invalid-short-code.html'; + private const string TEMPLATES_BASE_DIR = __DIR__ . '/../../templates'; + public const string NOT_FOUND_TEMPLATE = '404.html'; + public const string INVALID_SHORT_CODE_TEMPLATE = 'invalid-short-code.html'; private Closure $readFile; diff --git a/module/Core/src/Exception/DeleteShortUrlException.php b/module/Core/src/Exception/DeleteShortUrlException.php index 42198dc92..69e015592 100644 --- a/module/Core/src/Exception/DeleteShortUrlException.php +++ b/module/Core/src/Exception/DeleteShortUrlException.php @@ -16,8 +16,8 @@ class DeleteShortUrlException extends DomainException implements ProblemDetailsE { use CommonProblemDetailsExceptionTrait; - private const TITLE = 'Cannot delete short URL'; - public const ERROR_CODE = 'invalid-short-url-deletion'; + private const string TITLE = 'Cannot delete short URL'; + public const string ERROR_CODE = 'invalid-short-url-deletion'; public static function fromVisitsThreshold(int $threshold, ShortUrlIdentifier $identifier): self { diff --git a/module/Core/src/Exception/DomainNotFoundException.php b/module/Core/src/Exception/DomainNotFoundException.php index 688a4edc1..a3f6fd4a1 100644 --- a/module/Core/src/Exception/DomainNotFoundException.php +++ b/module/Core/src/Exception/DomainNotFoundException.php @@ -15,8 +15,8 @@ class DomainNotFoundException extends DomainException implements ProblemDetailsE { use CommonProblemDetailsExceptionTrait; - private const TITLE = 'Domain not found'; - public const ERROR_CODE = 'domain-not-found'; + private const string TITLE = 'Domain not found'; + public const string ERROR_CODE = 'domain-not-found'; private function __construct(string $message, array $additional) { diff --git a/module/Core/src/Exception/ForbiddenTagOperationException.php b/module/Core/src/Exception/ForbiddenTagOperationException.php index 64ae156c0..5c4efbc14 100644 --- a/module/Core/src/Exception/ForbiddenTagOperationException.php +++ b/module/Core/src/Exception/ForbiddenTagOperationException.php @@ -14,8 +14,8 @@ class ForbiddenTagOperationException extends DomainException implements ProblemD { use CommonProblemDetailsExceptionTrait; - private const TITLE = 'Forbidden tag operation'; - public const ERROR_CODE = 'forbidden-tag-operation'; + private const string TITLE = 'Forbidden tag operation'; + public const string ERROR_CODE = 'forbidden-tag-operation'; public static function forDeletion(): self { diff --git a/module/Core/src/Exception/NonUniqueSlugException.php b/module/Core/src/Exception/NonUniqueSlugException.php index 8f9508a26..99f1b2921 100644 --- a/module/Core/src/Exception/NonUniqueSlugException.php +++ b/module/Core/src/Exception/NonUniqueSlugException.php @@ -16,8 +16,8 @@ class NonUniqueSlugException extends InvalidArgumentException implements Problem { use CommonProblemDetailsExceptionTrait; - private const TITLE = 'Invalid custom slug'; - public const ERROR_CODE = 'non-unique-slug'; + private const string TITLE = 'Invalid custom slug'; + public const string ERROR_CODE = 'non-unique-slug'; public static function fromSlug(string $slug, string|null $domain = null): self { diff --git a/module/Core/src/Exception/ShortUrlNotFoundException.php b/module/Core/src/Exception/ShortUrlNotFoundException.php index 68087b537..a1a2135a0 100644 --- a/module/Core/src/Exception/ShortUrlNotFoundException.php +++ b/module/Core/src/Exception/ShortUrlNotFoundException.php @@ -16,8 +16,8 @@ class ShortUrlNotFoundException extends DomainException implements ProblemDetail { use CommonProblemDetailsExceptionTrait; - private const TITLE = 'Short URL not found'; - public const ERROR_CODE = 'short-url-not-found'; + private const string TITLE = 'Short URL not found'; + public const string ERROR_CODE = 'short-url-not-found'; public static function fromNotFound(ShortUrlIdentifier $identifier): self { diff --git a/module/Core/src/Exception/TagConflictException.php b/module/Core/src/Exception/TagConflictException.php index e05754c7c..a44a4a0cc 100644 --- a/module/Core/src/Exception/TagConflictException.php +++ b/module/Core/src/Exception/TagConflictException.php @@ -16,8 +16,8 @@ class TagConflictException extends RuntimeException implements ProblemDetailsExc { use CommonProblemDetailsExceptionTrait; - private const TITLE = 'Tag conflict'; - public const ERROR_CODE = 'tag-conflict'; + private const string TITLE = 'Tag conflict'; + public const string ERROR_CODE = 'tag-conflict'; public static function forExistingTag(Renaming $renaming): self { diff --git a/module/Core/src/Exception/TagNotFoundException.php b/module/Core/src/Exception/TagNotFoundException.php index 8fdd395a1..3bb4f9373 100644 --- a/module/Core/src/Exception/TagNotFoundException.php +++ b/module/Core/src/Exception/TagNotFoundException.php @@ -15,8 +15,8 @@ class TagNotFoundException extends DomainException implements ProblemDetailsExce { use CommonProblemDetailsExceptionTrait; - private const TITLE = 'Tag not found'; - public const ERROR_CODE = 'tag-not-found'; + private const string TITLE = 'Tag not found'; + public const string ERROR_CODE = 'tag-not-found'; public static function fromTag(string $tag): self { diff --git a/module/Core/src/Exception/ValidationException.php b/module/Core/src/Exception/ValidationException.php index f81c1d37b..61f01e6fd 100644 --- a/module/Core/src/Exception/ValidationException.php +++ b/module/Core/src/Exception/ValidationException.php @@ -21,8 +21,8 @@ class ValidationException extends InvalidArgumentException implements ProblemDet { use CommonProblemDetailsExceptionTrait; - private const TITLE = 'Invalid data'; - public const ERROR_CODE = 'invalid-data'; + private const string TITLE = 'Invalid data'; + public const string ERROR_CODE = 'invalid-data'; private array $invalidElements; diff --git a/module/Core/src/Matomo/MatomoTrackerBuilder.php b/module/Core/src/Matomo/MatomoTrackerBuilder.php index e006271bc..a65d9ad65 100644 --- a/module/Core/src/Matomo/MatomoTrackerBuilder.php +++ b/module/Core/src/Matomo/MatomoTrackerBuilder.php @@ -9,7 +9,7 @@ readonly class MatomoTrackerBuilder implements MatomoTrackerBuilderInterface { - public const MATOMO_DEFAULT_TIMEOUT = 10; // Time in seconds + public const int MATOMO_DEFAULT_TIMEOUT = 10; // Time in seconds public function __construct(private MatomoOptions $options) { diff --git a/module/Core/src/Model/Ordering.php b/module/Core/src/Model/Ordering.php index 0e0edab70..d06bee898 100644 --- a/module/Core/src/Model/Ordering.php +++ b/module/Core/src/Model/Ordering.php @@ -6,9 +6,9 @@ final readonly class Ordering { - private const DESC_DIR = 'DESC'; - private const ASC_DIR = 'ASC'; - private const DEFAULT_DIR = self::ASC_DIR; + private const string DESC_DIR = 'DESC'; + private const string ASC_DIR = 'ASC'; + private const string DEFAULT_DIR = self::ASC_DIR; public function __construct(public string|null $field = null, public string $direction = self::DEFAULT_DIR) { diff --git a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php index 42520a971..c802d75f0 100644 --- a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php +++ b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php @@ -17,14 +17,14 @@ /** @extends InputFilter */ class RedirectRulesInputFilter extends InputFilter { - public const REDIRECT_RULES = 'redirectRules'; + public const string REDIRECT_RULES = 'redirectRules'; - public const RULE_LONG_URL = 'longUrl'; - public const RULE_CONDITIONS = 'conditions'; + public const string RULE_LONG_URL = 'longUrl'; + public const string RULE_CONDITIONS = 'conditions'; - public const CONDITION_TYPE = 'type'; - public const CONDITION_MATCH_VALUE = 'matchValue'; - public const CONDITION_MATCH_KEY = 'matchKey'; + public const string CONDITION_TYPE = 'type'; + public const string CONDITION_MATCH_VALUE = 'matchValue'; + public const string CONDITION_MATCH_KEY = 'matchKey'; private function __construct() { diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php index df52c92d1..5af783455 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php @@ -21,14 +21,14 @@ readonly class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionHelperInterface { - public const MAX_REDIRECTS = 15; - public const CHROME_USER_AGENT = 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) ' + public const int MAX_REDIRECTS = 15; + public const string CHROME_USER_AGENT = 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) ' . 'Chrome/121.0.0.0 Safari/537.36'; // Matches the value inside a html title tag - private const TITLE_TAG_VALUE = '/]*>(.*?)<\/title>/i'; + private const string TITLE_TAG_VALUE = '/]*>(.*?)<\/title>/i'; // Matches the charset inside a Content-Type header - private const CHARSET_VALUE = '/charset=([^;]+)/i'; + private const string CHARSET_VALUE = '/charset=([^;]+)/i'; public function __construct( private ClientInterface $httpClient, diff --git a/module/Core/src/ShortUrl/Middleware/TrimTrailingSlashMiddleware.php b/module/Core/src/ShortUrl/Middleware/TrimTrailingSlashMiddleware.php index 0b70c3ae0..4088f4503 100644 --- a/module/Core/src/ShortUrl/Middleware/TrimTrailingSlashMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/TrimTrailingSlashMiddleware.php @@ -14,7 +14,7 @@ class TrimTrailingSlashMiddleware implements MiddlewareInterface { - private const SHORT_CODE_ATTR = 'shortCode'; + private const string SHORT_CODE_ATTR = 'shortCode'; public function __construct(private readonly UrlShortenerOptions $options) { diff --git a/module/Core/src/ShortUrl/Model/Validation/CustomSlugValidator.php b/module/Core/src/ShortUrl/Model/Validation/CustomSlugValidator.php index 2bc417b42..f33416983 100644 --- a/module/Core/src/ShortUrl/Model/Validation/CustomSlugValidator.php +++ b/module/Core/src/ShortUrl/Model/Validation/CustomSlugValidator.php @@ -12,8 +12,8 @@ class CustomSlugValidator extends AbstractValidator { - private const NOT_STRING = 'NOT_STRING'; - private const CONTAINS_URL_CHARACTERS = 'CONTAINS_URL_CHARACTERS'; + private const string NOT_STRING = 'NOT_STRING'; + private const string CONTAINS_URL_CHARACTERS = 'CONTAINS_URL_CHARACTERS'; protected array $messageTemplates = [ self::NOT_STRING => 'Provided value is not a string.', diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php index 5cd7fe38d..88b629e87 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php @@ -24,22 +24,22 @@ class ShortUrlInputFilter extends InputFilter { // Fields for creation only - public const SHORT_CODE_LENGTH = 'shortCodeLength'; - public const CUSTOM_SLUG = 'customSlug'; - public const PATH_PREFIX = 'pathPrefix'; - public const FIND_IF_EXISTS = 'findIfExists'; - public const DOMAIN = 'domain'; + public const string SHORT_CODE_LENGTH = 'shortCodeLength'; + public const string CUSTOM_SLUG = 'customSlug'; + public const string PATH_PREFIX = 'pathPrefix'; + public const string FIND_IF_EXISTS = 'findIfExists'; + public const string DOMAIN = 'domain'; // Fields for creation and edition - public const LONG_URL = 'longUrl'; - public const VALID_SINCE = 'validSince'; - public const VALID_UNTIL = 'validUntil'; - public const MAX_VISITS = 'maxVisits'; - public const TITLE = 'title'; - public const TAGS = 'tags'; - public const CRAWLABLE = 'crawlable'; - public const FORWARD_QUERY = 'forwardQuery'; - public const API_KEY = 'apiKey'; + public const string LONG_URL = 'longUrl'; + public const string VALID_SINCE = 'validSince'; + public const string VALID_UNTIL = 'validUntil'; + public const string MAX_VISITS = 'maxVisits'; + public const string TITLE = 'title'; + public const string TAGS = 'tags'; + public const string CRAWLABLE = 'crawlable'; + public const string FORWARD_QUERY = 'forwardQuery'; + public const string API_KEY = 'apiKey'; public static function forCreation(array $data, UrlShortenerOptions $options): self { diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php index 600ebc339..bc9de3379 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php @@ -16,17 +16,17 @@ /** @extends InputFilter */ class ShortUrlsParamsInputFilter extends InputFilter { - public const PAGE = 'page'; - public const SEARCH_TERM = 'searchTerm'; - public const TAGS = 'tags'; - public const START_DATE = 'startDate'; - public const END_DATE = 'endDate'; - public const ITEMS_PER_PAGE = 'itemsPerPage'; - public const TAGS_MODE = 'tagsMode'; - public const ORDER_BY = 'orderBy'; - public const EXCLUDE_MAX_VISITS_REACHED = 'excludeMaxVisitsReached'; - public const EXCLUDE_PAST_VALID_UNTIL = 'excludePastValidUntil'; - public const DOMAIN = 'domain'; + public const string PAGE = 'page'; + public const string SEARCH_TERM = 'searchTerm'; + public const string TAGS = 'tags'; + public const string START_DATE = 'startDate'; + public const string END_DATE = 'endDate'; + public const string ITEMS_PER_PAGE = 'itemsPerPage'; + public const string TAGS_MODE = 'tagsMode'; + public const string ORDER_BY = 'orderBy'; + public const string EXCLUDE_MAX_VISITS_REACHED = 'excludeMaxVisitsReached'; + public const string EXCLUDE_PAST_VALID_UNTIL = 'excludePastValidUntil'; + public const string DOMAIN = 'domain'; public function __construct(array $data) { diff --git a/module/Core/src/Visit/Model/Visitor.php b/module/Core/src/Visit/Model/Visitor.php index cab834e6a..ae02a1a36 100644 --- a/module/Core/src/Visit/Model/Visitor.php +++ b/module/Core/src/Visit/Model/Visitor.php @@ -17,11 +17,11 @@ final readonly class Visitor { - public const USER_AGENT_MAX_LENGTH = 512; - public const REFERER_MAX_LENGTH = 1024; - public const REMOTE_ADDRESS_MAX_LENGTH = 256; - public const VISITED_URL_MAX_LENGTH = 2048; - public const REDIRECT_URL_MAX_LENGTH = 2048; + public const int USER_AGENT_MAX_LENGTH = 512; + public const int REFERER_MAX_LENGTH = 1024; + public const int REMOTE_ADDRESS_MAX_LENGTH = 256; + public const int VISITED_URL_MAX_LENGTH = 2048; + public const int REDIRECT_URL_MAX_LENGTH = 2048; private function __construct( public string $userAgent, diff --git a/module/Core/src/Visit/Repository/VisitIterationRepositoryInterface.php b/module/Core/src/Visit/Repository/VisitIterationRepositoryInterface.php index 2f4163240..ca9d6405f 100644 --- a/module/Core/src/Visit/Repository/VisitIterationRepositoryInterface.php +++ b/module/Core/src/Visit/Repository/VisitIterationRepositoryInterface.php @@ -9,7 +9,7 @@ interface VisitIterationRepositoryInterface { - public const DEFAULT_BLOCK_SIZE = 10000; + public const int DEFAULT_BLOCK_SIZE = 10000; /** * @return iterable diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index f8dea217c..688badee1 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -32,8 +32,8 @@ class QrCodeActionTest extends TestCase { - private const WHITE = 0xFFFFFF; - private const BLACK = 0x0; + private const int WHITE = 0xFFFFFF; + private const int BLACK = 0x0; private MockObject & ShortUrlResolverInterface $urlResolver; diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index fa4a561d0..59e1a5b8c 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -23,7 +23,7 @@ class RedirectActionTest extends TestCase { - private const LONG_URL = 'https://domain.com/foo/bar?some=thing'; + private const string LONG_URL = 'https://domain.com/foo/bar?some=thing'; private RedirectAction $action; private MockObject & ShortUrlResolverInterface $urlResolver; diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php index d73a1a6d8..b11a28a3f 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php @@ -22,7 +22,7 @@ class ShortUrlTitleResolutionHelperTest extends TestCase { - private const LONG_URL = 'http://foobar.com/12345/hello?foo=bar'; + private const string LONG_URL = 'http://foobar.com/12345/hello?foo=bar'; private MockObject & ClientInterface $httpClient; diff --git a/module/Core/test/Visit/RequestTrackerTest.php b/module/Core/test/Visit/RequestTrackerTest.php index f9357c6ac..e7c2ef16d 100644 --- a/module/Core/test/Visit/RequestTrackerTest.php +++ b/module/Core/test/Visit/RequestTrackerTest.php @@ -23,7 +23,7 @@ class RequestTrackerTest extends TestCase { - private const LONG_URL = 'https://domain.com/foo/bar?some=thing'; + private const string LONG_URL = 'https://domain.com/foo/bar?some=thing'; private RequestTracker $requestTracker; private MockObject & VisitsTrackerInterface $visitsTracker; diff --git a/module/Rest/src/Action/AbstractRestAction.php b/module/Rest/src/Action/AbstractRestAction.php index f330bab18..b51c2f466 100644 --- a/module/Rest/src/Action/AbstractRestAction.php +++ b/module/Rest/src/Action/AbstractRestAction.php @@ -10,8 +10,8 @@ abstract class AbstractRestAction implements RequestHandlerInterface, RequestMethodInterface, StatusCodeInterface { - protected const ROUTE_PATH = ''; - protected const ROUTE_ALLOWED_METHODS = []; + protected const string ROUTE_PATH = ''; + protected const array ROUTE_ALLOWED_METHODS = []; public static function getRouteDef(array $prevMiddleware = [], array $postMiddleware = []): array { diff --git a/module/Rest/src/Action/Domain/DomainRedirectsAction.php b/module/Rest/src/Action/Domain/DomainRedirectsAction.php index e98aa339b..bc48d2d04 100644 --- a/module/Rest/src/Action/Domain/DomainRedirectsAction.php +++ b/module/Rest/src/Action/Domain/DomainRedirectsAction.php @@ -14,10 +14,10 @@ class DomainRedirectsAction extends AbstractRestAction { - protected const ROUTE_PATH = '/domains/redirects'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PATCH]; + protected const string ROUTE_PATH = '/domains/redirects'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_PATCH]; - public function __construct(private DomainServiceInterface $domainService) + public function __construct(private readonly DomainServiceInterface $domainService) { } diff --git a/module/Rest/src/Action/Domain/ListDomainsAction.php b/module/Rest/src/Action/Domain/ListDomainsAction.php index d156a7202..4007ab588 100644 --- a/module/Rest/src/Action/Domain/ListDomainsAction.php +++ b/module/Rest/src/Action/Domain/ListDomainsAction.php @@ -15,11 +15,13 @@ class ListDomainsAction extends AbstractRestAction { - protected const ROUTE_PATH = '/domains'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + protected const string ROUTE_PATH = '/domains'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - public function __construct(private DomainServiceInterface $domainService, private NotFoundRedirectOptions $options) - { + public function __construct( + private readonly DomainServiceInterface $domainService, + private readonly NotFoundRedirectOptions $options, + ) { } public function handle(ServerRequestInterface $request): ResponseInterface diff --git a/module/Rest/src/Action/HealthAction.php b/module/Rest/src/Action/HealthAction.php index ce7313309..be47428db 100644 --- a/module/Rest/src/Action/HealthAction.php +++ b/module/Rest/src/Action/HealthAction.php @@ -13,14 +13,14 @@ class HealthAction extends AbstractRestAction { - private const HEALTH_CONTENT_TYPE = 'application/health+json'; - private const STATUS_PASS = 'pass'; - private const STATUS_FAIL = 'fail'; + private const string HEALTH_CONTENT_TYPE = 'application/health+json'; + private const string STATUS_PASS = 'pass'; + private const string STATUS_FAIL = 'fail'; - public const ROUTE_PATH = '/health'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + public const string ROUTE_PATH = '/health'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - public function __construct(private EntityManagerInterface $em, private AppOptions $options) + public function __construct(private readonly EntityManagerInterface $em, private readonly AppOptions $options) { } diff --git a/module/Rest/src/Action/MercureInfoAction.php b/module/Rest/src/Action/MercureInfoAction.php index 1454cbbcb..f468b4a2c 100644 --- a/module/Rest/src/Action/MercureInfoAction.php +++ b/module/Rest/src/Action/MercureInfoAction.php @@ -15,11 +15,13 @@ class MercureInfoAction extends AbstractRestAction { - protected const ROUTE_PATH = '/mercure-info'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + protected const string ROUTE_PATH = '/mercure-info'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - public function __construct(private JwtProviderInterface $jwtProvider, private array $mercureConfig) - { + public function __construct( + private readonly JwtProviderInterface $jwtProvider, + private readonly array $mercureConfig, + ) { } public function handle(ServerRequestInterface $request): ResponseInterface diff --git a/module/Rest/src/Action/RedirectRule/ListRedirectRulesAction.php b/module/Rest/src/Action/RedirectRule/ListRedirectRulesAction.php index c6c12fd98..ab48bd1ef 100644 --- a/module/Rest/src/Action/RedirectRule/ListRedirectRulesAction.php +++ b/module/Rest/src/Action/RedirectRule/ListRedirectRulesAction.php @@ -13,8 +13,8 @@ class ListRedirectRulesAction extends AbstractRestAction { - protected const ROUTE_PATH = '/short-urls/{shortCode}/redirect-rules'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + protected const string ROUTE_PATH = '/short-urls/{shortCode}/redirect-rules'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; public function __construct( private readonly ShortUrlResolverInterface $urlResolver, diff --git a/module/Rest/src/Action/RedirectRule/SetRedirectRulesAction.php b/module/Rest/src/Action/RedirectRule/SetRedirectRulesAction.php index 913a833da..1f2668218 100644 --- a/module/Rest/src/Action/RedirectRule/SetRedirectRulesAction.php +++ b/module/Rest/src/Action/RedirectRule/SetRedirectRulesAction.php @@ -16,8 +16,8 @@ class SetRedirectRulesAction extends AbstractRestAction { - protected const ROUTE_PATH = '/short-urls/{shortCode}/redirect-rules'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_POST, self::METHOD_PATCH]; + protected const string ROUTE_PATH = '/short-urls/{shortCode}/redirect-rules'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_POST, self::METHOD_PATCH]; public function __construct( private readonly ShortUrlResolverInterface $urlResolver, diff --git a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php index 67509f1ba..6f5e291ca 100644 --- a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php @@ -12,8 +12,8 @@ class CreateShortUrlAction extends AbstractCreateShortUrlAction { - protected const ROUTE_PATH = '/short-urls'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_POST]; + protected const string ROUTE_PATH = '/short-urls'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_POST]; /** * @throws ValidationException diff --git a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php index c1511f77a..b2570d363 100644 --- a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php @@ -14,8 +14,8 @@ class DeleteShortUrlAction extends AbstractRestAction { - protected const ROUTE_PATH = '/short-urls/{shortCode}'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_DELETE]; + protected const string ROUTE_PATH = '/short-urls/{shortCode}'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_DELETE]; public function __construct(private DeleteShortUrlServiceInterface $deleteShortUrlService) { diff --git a/module/Rest/src/Action/ShortUrl/DeleteShortUrlVisitsAction.php b/module/Rest/src/Action/ShortUrl/DeleteShortUrlVisitsAction.php index c9eaf9585..8085a6823 100644 --- a/module/Rest/src/Action/ShortUrl/DeleteShortUrlVisitsAction.php +++ b/module/Rest/src/Action/ShortUrl/DeleteShortUrlVisitsAction.php @@ -14,8 +14,8 @@ class DeleteShortUrlVisitsAction extends AbstractRestAction { - protected const ROUTE_PATH = '/short-urls/{shortCode}/visits'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_DELETE]; + protected const string ROUTE_PATH = '/short-urls/{shortCode}/visits'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_DELETE]; public function __construct(private readonly ShortUrlVisitsDeleterInterface $deleter) { diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php index b4b815a4e..acdc2cb44 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php @@ -16,8 +16,8 @@ class EditShortUrlAction extends AbstractRestAction { - protected const ROUTE_PATH = '/short-urls/{shortCode}'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PATCH]; + protected const string ROUTE_PATH = '/short-urls/{shortCode}'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_PATCH]; public function __construct( private readonly ShortUrlServiceInterface $shortUrlService, diff --git a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php index b21f50fde..4b9991b9f 100644 --- a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php +++ b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php @@ -16,8 +16,8 @@ class ListShortUrlsAction extends AbstractRestAction { - protected const ROUTE_PATH = '/short-urls'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + protected const string ROUTE_PATH = '/short-urls'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; public function __construct( private readonly ShortUrlListServiceInterface $shortUrlService, diff --git a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php index 39a9d7f22..3faae433d 100644 --- a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php @@ -15,8 +15,8 @@ class ResolveShortUrlAction extends AbstractRestAction { - protected const ROUTE_PATH = '/short-urls/{shortCode}'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + protected const string ROUTE_PATH = '/short-urls/{shortCode}'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; public function __construct( private readonly ShortUrlResolverInterface $urlResolver, diff --git a/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php index d7f5a3603..039b82d4f 100644 --- a/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php @@ -11,8 +11,8 @@ class SingleStepCreateShortUrlAction extends AbstractCreateShortUrlAction { - protected const ROUTE_PATH = '/short-urls/shorten'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + protected const string ROUTE_PATH = '/short-urls/shorten'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; protected function buildShortUrlData(Request $request): ShortUrlCreation { diff --git a/module/Rest/src/Action/Tag/DeleteTagsAction.php b/module/Rest/src/Action/Tag/DeleteTagsAction.php index 48e7acd96..8c45acf7d 100644 --- a/module/Rest/src/Action/Tag/DeleteTagsAction.php +++ b/module/Rest/src/Action/Tag/DeleteTagsAction.php @@ -13,10 +13,10 @@ class DeleteTagsAction extends AbstractRestAction { - protected const ROUTE_PATH = '/tags'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_DELETE]; + protected const string ROUTE_PATH = '/tags'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_DELETE]; - public function __construct(private TagServiceInterface $tagService) + public function __construct(private readonly TagServiceInterface $tagService) { } diff --git a/module/Rest/src/Action/Tag/ListTagsAction.php b/module/Rest/src/Action/Tag/ListTagsAction.php index 426f94e75..826ed112e 100644 --- a/module/Rest/src/Action/Tag/ListTagsAction.php +++ b/module/Rest/src/Action/Tag/ListTagsAction.php @@ -15,8 +15,8 @@ class ListTagsAction extends AbstractRestAction { - protected const ROUTE_PATH = '/tags'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + protected const string ROUTE_PATH = '/tags'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; public function __construct(private readonly TagServiceInterface $tagService) { diff --git a/module/Rest/src/Action/Tag/TagsStatsAction.php b/module/Rest/src/Action/Tag/TagsStatsAction.php index 1ae470e07..827f62418 100644 --- a/module/Rest/src/Action/Tag/TagsStatsAction.php +++ b/module/Rest/src/Action/Tag/TagsStatsAction.php @@ -15,8 +15,8 @@ class TagsStatsAction extends AbstractRestAction { - protected const ROUTE_PATH = '/tags/stats'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + protected const string ROUTE_PATH = '/tags/stats'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; public function __construct(private readonly TagServiceInterface $tagService) { diff --git a/module/Rest/src/Action/Tag/UpdateTagAction.php b/module/Rest/src/Action/Tag/UpdateTagAction.php index e1dc16116..373c26319 100644 --- a/module/Rest/src/Action/Tag/UpdateTagAction.php +++ b/module/Rest/src/Action/Tag/UpdateTagAction.php @@ -14,10 +14,10 @@ class UpdateTagAction extends AbstractRestAction { - protected const ROUTE_PATH = '/tags'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PUT]; + protected const string ROUTE_PATH = '/tags'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_PUT]; - public function __construct(private TagServiceInterface $tagService) + public function __construct(private readonly TagServiceInterface $tagService) { } diff --git a/module/Rest/src/Action/Visit/AbstractListVisitsAction.php b/module/Rest/src/Action/Visit/AbstractListVisitsAction.php index 090d30787..b63133fa0 100644 --- a/module/Rest/src/Action/Visit/AbstractListVisitsAction.php +++ b/module/Rest/src/Action/Visit/AbstractListVisitsAction.php @@ -18,7 +18,7 @@ abstract class AbstractListVisitsAction extends AbstractRestAction { - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; public function __construct(protected readonly VisitsStatsHelperInterface $visitsHelper) { diff --git a/module/Rest/src/Action/Visit/DeleteOrphanVisitsAction.php b/module/Rest/src/Action/Visit/DeleteOrphanVisitsAction.php index 3a0164518..c08de858b 100644 --- a/module/Rest/src/Action/Visit/DeleteOrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/DeleteOrphanVisitsAction.php @@ -13,8 +13,8 @@ class DeleteOrphanVisitsAction extends AbstractRestAction { - protected const ROUTE_PATH = '/visits/orphan'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_DELETE]; + protected const string ROUTE_PATH = '/visits/orphan'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_DELETE]; public function __construct(private readonly VisitsDeleterInterface $visitsDeleter) { diff --git a/module/Rest/src/Action/Visit/DomainVisitsAction.php b/module/Rest/src/Action/Visit/DomainVisitsAction.php index ee1625e00..0e0279559 100644 --- a/module/Rest/src/Action/Visit/DomainVisitsAction.php +++ b/module/Rest/src/Action/Visit/DomainVisitsAction.php @@ -14,7 +14,7 @@ class DomainVisitsAction extends AbstractListVisitsAction { - protected const ROUTE_PATH = '/domains/{domain}/visits'; + protected const string ROUTE_PATH = '/domains/{domain}/visits'; public function __construct( VisitsStatsHelperInterface $visitsHelper, diff --git a/module/Rest/src/Action/Visit/GlobalVisitsAction.php b/module/Rest/src/Action/Visit/GlobalVisitsAction.php index 1f2e1211e..779fd4a3a 100644 --- a/module/Rest/src/Action/Visit/GlobalVisitsAction.php +++ b/module/Rest/src/Action/Visit/GlobalVisitsAction.php @@ -13,10 +13,10 @@ class GlobalVisitsAction extends AbstractRestAction { - protected const ROUTE_PATH = '/visits'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + protected const string ROUTE_PATH = '/visits'; + protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - public function __construct(private VisitsStatsHelperInterface $statsHelper) + public function __construct(private readonly VisitsStatsHelperInterface $statsHelper) { } diff --git a/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php b/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php index 8406b515b..b2f7471b2 100644 --- a/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php @@ -11,7 +11,7 @@ class NonOrphanVisitsAction extends AbstractListVisitsAction { - protected const ROUTE_PATH = '/visits/non-orphan'; + protected const string ROUTE_PATH = '/visits/non-orphan'; protected function getVisitsPaginator( ServerRequestInterface $request, diff --git a/module/Rest/src/Action/Visit/OrphanVisitsAction.php b/module/Rest/src/Action/Visit/OrphanVisitsAction.php index 341524c35..b3c246ca4 100644 --- a/module/Rest/src/Action/Visit/OrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/OrphanVisitsAction.php @@ -12,7 +12,7 @@ class OrphanVisitsAction extends AbstractListVisitsAction { - protected const ROUTE_PATH = '/visits/orphan'; + protected const string ROUTE_PATH = '/visits/orphan'; protected function getVisitsPaginator( ServerRequestInterface $request, diff --git a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php index 95ac42cc6..d8fc36e9a 100644 --- a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php +++ b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php @@ -12,7 +12,7 @@ class ShortUrlVisitsAction extends AbstractListVisitsAction { - protected const ROUTE_PATH = '/short-urls/{shortCode}/visits'; + protected const string ROUTE_PATH = '/short-urls/{shortCode}/visits'; protected function getVisitsPaginator(Request $request, VisitsParams $params, ApiKey $apiKey): Pagerfanta { diff --git a/module/Rest/src/Action/Visit/TagVisitsAction.php b/module/Rest/src/Action/Visit/TagVisitsAction.php index 08553ec53..07ad7167e 100644 --- a/module/Rest/src/Action/Visit/TagVisitsAction.php +++ b/module/Rest/src/Action/Visit/TagVisitsAction.php @@ -11,7 +11,7 @@ class TagVisitsAction extends AbstractListVisitsAction { - protected const ROUTE_PATH = '/tags/{tag}/visits'; + protected const string ROUTE_PATH = '/tags/{tag}/visits'; protected function getVisitsPaginator(Request $request, VisitsParams $params, ApiKey $apiKey): Pagerfanta { diff --git a/module/Rest/src/ConfigProvider.php b/module/Rest/src/ConfigProvider.php index 1768c7c8b..95a70b0c3 100644 --- a/module/Rest/src/ConfigProvider.php +++ b/module/Rest/src/ConfigProvider.php @@ -12,9 +12,9 @@ class ConfigProvider { - private const ROUTES_PREFIX = '/rest/v{version:1|2|3}'; - private const UNVERSIONED_ROUTES_PREFIX = '/rest'; - public const UNVERSIONED_HEALTH_ENDPOINT_NAME = 'unversioned_health'; + private const string ROUTES_PREFIX = '/rest/v{version:1|2|3}'; + private const string UNVERSIONED_ROUTES_PREFIX = '/rest'; + public const string UNVERSIONED_HEALTH_ENDPOINT_NAME = 'unversioned_health'; public function __invoke(): array { diff --git a/module/Rest/src/Exception/MercureException.php b/module/Rest/src/Exception/MercureException.php index 7e47b519e..c97e701b9 100644 --- a/module/Rest/src/Exception/MercureException.php +++ b/module/Rest/src/Exception/MercureException.php @@ -14,8 +14,8 @@ class MercureException extends RuntimeException implements ProblemDetailsExcepti { use CommonProblemDetailsExceptionTrait; - private const TITLE = 'Mercure integration not configured'; - public const ERROR_CODE = 'mercure-not-configured'; + private const string TITLE = 'Mercure integration not configured'; + public const string ERROR_CODE = 'mercure-not-configured'; public static function mercureNotConfigured(): self { diff --git a/module/Rest/src/Exception/MissingAuthenticationException.php b/module/Rest/src/Exception/MissingAuthenticationException.php index 3fd2e2c68..ff0c60268 100644 --- a/module/Rest/src/Exception/MissingAuthenticationException.php +++ b/module/Rest/src/Exception/MissingAuthenticationException.php @@ -16,8 +16,8 @@ class MissingAuthenticationException extends RuntimeException implements Problem { use CommonProblemDetailsExceptionTrait; - private const TITLE = 'Invalid authorization'; - public const ERROR_CODE = 'missing-authentication'; + private const string TITLE = 'Invalid authorization'; + public const string ERROR_CODE = 'missing-authentication'; public static function forHeaders(array $expectedHeaders): self { diff --git a/module/Rest/src/Exception/VerifyAuthenticationException.php b/module/Rest/src/Exception/VerifyAuthenticationException.php index 25f1b0503..9ed03fe16 100644 --- a/module/Rest/src/Exception/VerifyAuthenticationException.php +++ b/module/Rest/src/Exception/VerifyAuthenticationException.php @@ -14,7 +14,7 @@ class VerifyAuthenticationException extends RuntimeException implements ProblemD { use CommonProblemDetailsExceptionTrait; - public const ERROR_CODE = 'invalid-api-key'; + public const string ERROR_CODE = 'invalid-api-key'; public static function forInvalidApiKey(): self { diff --git a/module/Rest/src/Middleware/AuthenticationMiddleware.php b/module/Rest/src/Middleware/AuthenticationMiddleware.php index cf73ba10c..eaa85c961 100644 --- a/module/Rest/src/Middleware/AuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/AuthenticationMiddleware.php @@ -21,7 +21,7 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterface, RequestMethodInterface { - public const API_KEY_HEADER = 'X-Api-Key'; + public const string API_KEY_HEADER = 'X-Api-Key'; public function __construct( private readonly ApiKeyServiceInterface $apiKeyService, diff --git a/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php b/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php index 085037571..a84b9e8ae 100644 --- a/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php +++ b/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php @@ -18,8 +18,8 @@ class CreateShortUrlContentNegotiationMiddleware implements MiddlewareInterface { - private const PLAIN_TEXT = 'text'; - private const JSON = 'json'; + private const string PLAIN_TEXT = 'text'; + private const string JSON = 'json'; public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { diff --git a/module/Rest/test-api/Action/ListRedirectRulesTest.php b/module/Rest/test-api/Action/ListRedirectRulesTest.php index 494a6564f..7909dcfdf 100644 --- a/module/Rest/test-api/Action/ListRedirectRulesTest.php +++ b/module/Rest/test-api/Action/ListRedirectRulesTest.php @@ -12,12 +12,12 @@ class ListRedirectRulesTest extends ApiTestCase { - private const LANGUAGE_EN_CONDITION = [ + private const array LANGUAGE_EN_CONDITION = [ 'type' => 'language', 'matchKey' => null, 'matchValue' => 'en', ]; - private const QUERY_FOO_BAR_CONDITION = [ + private const array QUERY_FOO_BAR_CONDITION = [ 'type' => 'query-param', 'matchKey' => 'foo', 'matchValue' => 'bar', diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index 1eba6db88..60b493d66 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -15,7 +15,7 @@ class ListShortUrlsTest extends ApiTestCase { - private const SHORT_URL_SHLINK_WITH_TITLE = [ + private const array SHORT_URL_SHLINK_WITH_TITLE = [ 'shortCode' => 'abc123', 'shortUrl' => 'http://s.test/abc123', 'longUrl' => 'https://shlink.io', @@ -37,7 +37,7 @@ class ListShortUrlsTest extends ApiTestCase 'forwardQuery' => true, 'hasRedirectRules' => false, ]; - private const SHORT_URL_DOCS = [ + private const array SHORT_URL_DOCS = [ 'shortCode' => 'ghi789', 'shortUrl' => 'http://s.test/ghi789', 'longUrl' => 'https://shlink.io/documentation/', @@ -59,7 +59,7 @@ class ListShortUrlsTest extends ApiTestCase 'forwardQuery' => true, 'hasRedirectRules' => false, ]; - private const SHORT_URL_CUSTOM_SLUG_AND_DOMAIN = [ + private const array SHORT_URL_CUSTOM_SLUG_AND_DOMAIN = [ 'shortCode' => 'custom-with-domain', 'shortUrl' => 'http://some-domain.com/custom-with-domain', 'longUrl' => 'https://google.com', @@ -81,7 +81,7 @@ class ListShortUrlsTest extends ApiTestCase 'forwardQuery' => true, 'hasRedirectRules' => false, ]; - private const SHORT_URL_META = [ + private const array SHORT_URL_META = [ 'shortCode' => 'def456', 'shortUrl' => 'http://s.test/def456', 'longUrl' => @@ -105,7 +105,7 @@ class ListShortUrlsTest extends ApiTestCase 'forwardQuery' => true, 'hasRedirectRules' => true, ]; - private const SHORT_URL_CUSTOM_SLUG = [ + private const array SHORT_URL_CUSTOM_SLUG = [ 'shortCode' => 'custom', 'shortUrl' => 'http://s.test/custom', 'longUrl' => 'https://shlink.io', @@ -127,7 +127,7 @@ class ListShortUrlsTest extends ApiTestCase 'forwardQuery' => false, 'hasRedirectRules' => false, ]; - private const SHORT_URL_CUSTOM_DOMAIN = [ + private const array SHORT_URL_CUSTOM_DOMAIN = [ 'shortCode' => 'ghi789', 'shortUrl' => 'http://example.com/ghi789', 'longUrl' => diff --git a/module/Rest/test-api/Action/OrphanVisitsTest.php b/module/Rest/test-api/Action/OrphanVisitsTest.php index 3761e113d..f94148997 100644 --- a/module/Rest/test-api/Action/OrphanVisitsTest.php +++ b/module/Rest/test-api/Action/OrphanVisitsTest.php @@ -13,7 +13,7 @@ class OrphanVisitsTest extends ApiTestCase { - private const INVALID_SHORT_URL = [ + private const array INVALID_SHORT_URL = [ 'referer' => 'https://s.test/foo', 'date' => '2020-03-01T00:00:00+00:00', 'userAgent' => 'cf-facebook', @@ -23,7 +23,7 @@ class OrphanVisitsTest extends ApiTestCase 'type' => 'invalid_short_url', 'redirectUrl' => null, ]; - private const REGULAR_NOT_FOUND = [ + private const array REGULAR_NOT_FOUND = [ 'referer' => 'https://s.test/foo/bar', 'date' => '2020-02-01T00:00:00+00:00', 'userAgent' => 'shlink-tests-agent', @@ -33,7 +33,7 @@ class OrphanVisitsTest extends ApiTestCase 'type' => 'regular_404', 'redirectUrl' => null, ]; - private const BASE_URL = [ + private const array BASE_URL = [ 'referer' => 'https://s.test', 'date' => '2020-01-01T00:00:00+00:00', 'userAgent' => 'shlink-tests-agent', diff --git a/module/Rest/test-api/Action/SetRedirectRulesTest.php b/module/Rest/test-api/Action/SetRedirectRulesTest.php index 6501ef13a..9fc6fa9d0 100644 --- a/module/Rest/test-api/Action/SetRedirectRulesTest.php +++ b/module/Rest/test-api/Action/SetRedirectRulesTest.php @@ -13,12 +13,12 @@ class SetRedirectRulesTest extends ApiTestCase { - private const LANGUAGE_EN_CONDITION = [ + private const array LANGUAGE_EN_CONDITION = [ 'type' => 'language', 'matchKey' => null, 'matchValue' => 'en', ]; - private const QUERY_FOO_BAR_CONDITION = [ + private const array QUERY_FOO_BAR_CONDITION = [ 'type' => 'query-param', 'matchKey' => 'foo', 'matchValue' => 'bar', From 85c4c09afac2e91a4d297e81352cd864c85adeea Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 6 Dec 2024 11:36:47 +0100 Subject: [PATCH 08/30] Use the openapi terminology over swagger --- .github/workflows/ci.yml | 2 +- ...gger-spec.yml => publish-openapi-spec.yml} | 8 ++-- .gitignore | 3 +- composer.json | 42 ++++++------------- .../CLI/src/GeoLite/GeolocationDbUpdater.php | 6 +-- 5 files changed, 21 insertions(+), 40 deletions(-) rename .github/workflows/{publish-swagger-spec.yml => publish-openapi-spec.yml} (83%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a93559cb3..f2bc57d2f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,7 +28,7 @@ jobs: strategy: matrix: php-version: ['8.3'] - command: ['cs', 'stan', 'swagger:validate'] + command: ['cs', 'stan', 'openapi:validate'] steps: - uses: actions/checkout@v4 - uses: './.github/actions/ci-setup' diff --git a/.github/workflows/publish-swagger-spec.yml b/.github/workflows/publish-openapi-spec.yml similarity index 83% rename from .github/workflows/publish-swagger-spec.yml rename to .github/workflows/publish-openapi-spec.yml index 95692294c..6195ce90d 100644 --- a/.github/workflows/publish-swagger-spec.yml +++ b/.github/workflows/publish-openapi-spec.yml @@ -1,4 +1,4 @@ -name: Publish swagger spec +name: Publish openapi spec on: push: @@ -20,10 +20,10 @@ jobs: - uses: './.github/actions/ci-setup' with: php-version: ${{ matrix.php-version }} - extensions-cache-key: publish-swagger-spec-extensions-${{ matrix.php-version }} - - run: composer swagger:inline + extensions-cache-key: publish-openapi-spec-extensions-${{ matrix.php-version }} + - run: composer openapi:inline - run: mkdir ${{ steps.determine_version.outputs.version }} - - run: mv docs/swagger/swagger-inlined.json ${{ steps.determine_version.outputs.version }}/open-api-spec.json + - run: mv docs/swagger/openapi-inlined.json ${{ steps.determine_version.outputs.version }}/open-api-spec.json - name: Publish spec uses: JamesIves/github-pages-deploy-action@v4 with: diff --git a/.gitignore b/.gitignore index 9353d40c9..4061960bb 100644 --- a/.gitignore +++ b/.gitignore @@ -10,7 +10,6 @@ data/database.sqlite data/shlink-tests.db data/GeoLite2-City.* data/infra/matomo -docs/swagger-ui* docs/mercure.html .phpunit.result.cache -docs/swagger/swagger-inlined.json +docs/swagger/openapi-inlined.json diff --git a/composer.json b/composer.json index bc81820e2..9fe15aeac 100644 --- a/composer.json +++ b/composer.json @@ -61,7 +61,7 @@ "symfony/string": "^7.1" }, "require-dev": { - "devizzent/cebe-php-openapi": "^1.0.1", + "devizzent/cebe-php-openapi": "^1.1.1", "devster/ubench": "^2.1", "phpstan/phpstan": "^2.0", "phpstan/phpstan-doctrine": "^2.0", @@ -108,7 +108,7 @@ }, "scripts": { "ci": [ - "@parallel cs stan swagger:validate test:unit:ci test:db:sqlite:ci test:db:postgres test:db:mysql test:db:maria test:db:ms", + "@parallel cs stan openapi:validate test:unit:ci test:db:sqlite:ci test:db:postgres test:db:mysql test:db:maria test:db:ms", "@parallel test:api:ci test:cli:ci" ], "cs": "phpcs -s", @@ -154,36 +154,18 @@ "@test:cli", "phpcov merge build/coverage-cli --html build/coverage-cli/coverage-html && rm build/coverage-cli/*.cov" ], - "swagger:validate": "@php -d error_reporting=\"E_ALL & ~E_DEPRECATED & ~E_USER_DEPRECATED\" vendor/bin/php-openapi validate docs/swagger/swagger.json", - "swagger:inline": "@php -d error_reporting=\"E_ALL & ~E_DEPRECATED & ~E_USER_DEPRECATED\" vendor/bin/php-openapi inline docs/swagger/swagger.json docs/swagger/swagger-inlined.json", + "openapi:validate": "@php -d error_reporting=\"E_ALL & ~E_DEPRECATED & ~E_USER_DEPRECATED\" vendor/bin/php-openapi validate docs/swagger/swagger.json", + "openapi:inline": "@php -d error_reporting=\"E_ALL & ~E_DEPRECATED & ~E_USER_DEPRECATED\" vendor/bin/php-openapi inline docs/swagger/swagger.json docs/swagger/openapi-inlined.json", + "swagger:validate": [ + "echo \"This command is deprecated. Use openapi:validate instead\"", + "@openapi:validate" + ], + "swagger:inline": [ + "echo \"This command is deprecated. Use openapi:inline instead\"", + "@openapi:inline" + ], "clean:dev": "rm -f data/database.sqlite && rm -f config/params/generated_config.php" }, - "scripts-descriptions": { - "ci": "Alias for \"cs\", \"stan\", \"swagger:validate\" and \"test:ci\"", - "cs": "Checks coding styles", - "cs:fix": "Fixes coding styles, when possible", - "stan": "Inspects code with phpstan", - "test": "Runs all test suites", - "test:unit": "Runs unit test suites", - "test:unit:ci": "Runs unit test suites, generating all needed reports and logs for CI envs", - "test:unit:pretty": "Runs unit test suites and generates an HTML code coverage report", - "test:db": "Runs database test suites on a SQLite, MySQL, MariaDB, PostgreSQL and MsSQL", - "test:db:sqlite": "Runs database test suites on a SQLite database", - "test:db:sqlite:ci": "Runs database test suites on a SQLite database, generating all needed reports and logs for CI envs", - "test:db:mysql": "Runs database test suites on a MySQL database", - "test:db:maria": "Runs database test suites on a MariaDB database", - "test:db:postgres": "Runs database test suites on a PostgreSQL database", - "test:db:ms": "Runs database test suites on a Microsoft SQL Server database", - "test:api": "Runs API test suites", - "test:api:ci": "Runs API test suites, and generates code coverage for CI", - "test:api:pretty": "Runs API test suites, and generates code coverage in HTML format", - "test:cli": "Runs CLI test suites", - "test:cli:ci": "Runs CLI test suites, and generates code coverage for CI", - "test:cli:pretty": "Runs CLI test suites, and generates code coverage in HTML format", - "swagger:validate": "Validates the swagger docs, making sure they fulfil the spec", - "swagger:inline": "Inlines swagger docs in a single file", - "clean:dev": "Deletes artifacts which are gitignored and could affect dev env" - }, "config": { "sort-packages": true, "platform-check": false, diff --git a/module/CLI/src/GeoLite/GeolocationDbUpdater.php b/module/CLI/src/GeoLite/GeolocationDbUpdater.php index 7bdf8150e..0d3d96db4 100644 --- a/module/CLI/src/GeoLite/GeolocationDbUpdater.php +++ b/module/CLI/src/GeoLite/GeolocationDbUpdater.php @@ -64,12 +64,12 @@ public function checkDbUpdate( private function downloadIfNeeded(callable|null $beforeDownload, callable|null $handleProgress): GeolocationResult { if (! $this->dbUpdater->databaseFileExists()) { - return $this->downloadNewDb(false, $beforeDownload, $handleProgress); + return $this->downloadNewDb($beforeDownload, $handleProgress, olderDbExists: false); } $meta = ($this->geoLiteDbReaderFactory)()->metadata(); if ($this->buildIsTooOld($meta)) { - return $this->downloadNewDb(true, $beforeDownload, $handleProgress); + return $this->downloadNewDb($beforeDownload, $handleProgress, olderDbExists: true); } return GeolocationResult::DB_IS_UP_TO_DATE; @@ -106,9 +106,9 @@ private function resolveBuildTimestamp(Metadata $meta): int * @throws GeolocationDbUpdateFailedException */ private function downloadNewDb( - bool $olderDbExists, callable|null $beforeDownload, callable|null $handleProgress, + bool $olderDbExists, ): GeolocationResult { if ($beforeDownload !== null) { $beforeDownload($olderDbExists); From 06c0a94b31ffed7a19f8ac4d2ad583f34a73047b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 10 Dec 2024 10:58:08 +0100 Subject: [PATCH 09/30] Move GeolocationDbUpdater from CLI to Core module --- module/CLI/config/dependencies.config.php | 15 ++------------- .../Command/Visit/DownloadGeoLiteDbCommand.php | 6 +++--- .../Visit/DownloadGeoLiteDbCommandTest.php | 6 +++--- .../GeolocationDbUpdateFailedExceptionTest.php | 2 +- .../CLI/test/GeoLite/GeolocationDbUpdaterTest.php | 6 +++--- module/Core/config/dependencies.config.php | 11 +++++++++++ module/Core/config/event_dispatcher.config.php | 2 +- .../Core/src/EventDispatcher/UpdateGeoLiteDb.php | 4 ++-- .../GeolocationDbUpdateFailedException.php | 2 +- .../src/Geolocation}/GeolocationDbUpdater.php | 4 ++-- .../GeolocationDbUpdaterInterface.php | 4 ++-- .../src/Geolocation}/GeolocationResult.php | 2 +- .../test/EventDispatcher/UpdateGeoLiteDbTest.php | 4 ++-- 13 files changed, 34 insertions(+), 34 deletions(-) rename module/{CLI => Core}/src/Exception/GeolocationDbUpdateFailedException.php (97%) rename module/{CLI/src/GeoLite => Core/src/Geolocation}/GeolocationDbUpdater.php (97%) rename module/{CLI/src/GeoLite => Core/src/Geolocation}/GeolocationDbUpdaterInterface.php (72%) rename module/{CLI/src/GeoLite => Core/src/Geolocation}/GeolocationResult.php (77%) diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 76e7c4f54..5df74822a 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -7,9 +7,9 @@ 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\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 @@ 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 @@ ], 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 @@ 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, diff --git a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php index 0fdd8ae38..f6110d07a 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -4,10 +4,10 @@ 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\GeolocationResult; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputInterface; diff --git a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php index 4d2754f8f..2b477f031 100644 --- a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php +++ b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php @@ -9,10 +9,10 @@ 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\GeolocationResult; use ShlinkioTest\Shlink\CLI\Util\CliTestUtils; use Symfony\Component\Console\Tester\CommandTester; diff --git a/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php b/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php index 519ddf02e..a1d6db657 100644 --- a/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php +++ b/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php @@ -9,7 +9,7 @@ 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 diff --git a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php index 038d570c9..dc1d614c5 100644 --- a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php +++ b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php @@ -11,10 +11,10 @@ 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\GeolocationResult; use Shlinkio\Shlink\IpGeolocation\Exception\DbUpdateException; use Shlinkio\Shlink\IpGeolocation\Exception\MissingLicenseException; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 4844e6d5c..b16a4c5cc 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -9,12 +9,16 @@ 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 @@ 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 @@ 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, diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 4e130fcf9..39efa3cb7 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -6,11 +6,11 @@ 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; diff --git a/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php index 4e4720c5e..7f14cc24e 100644 --- a/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php +++ b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php @@ -6,9 +6,9 @@ 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\GeolocationResult; use Throwable; use function sprintf; diff --git a/module/CLI/src/Exception/GeolocationDbUpdateFailedException.php b/module/Core/src/Exception/GeolocationDbUpdateFailedException.php similarity index 97% rename from module/CLI/src/Exception/GeolocationDbUpdateFailedException.php rename to module/Core/src/Exception/GeolocationDbUpdateFailedException.php index ee31ac82e..f3c3f65fd 100644 --- a/module/CLI/src/Exception/GeolocationDbUpdateFailedException.php +++ b/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; diff --git a/module/CLI/src/GeoLite/GeolocationDbUpdater.php b/module/Core/src/Geolocation/GeolocationDbUpdater.php similarity index 97% rename from module/CLI/src/GeoLite/GeolocationDbUpdater.php rename to module/Core/src/Geolocation/GeolocationDbUpdater.php index 0d3d96db4..73515a45a 100644 --- a/module/CLI/src/GeoLite/GeolocationDbUpdater.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php @@ -2,14 +2,14 @@ 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; diff --git a/module/CLI/src/GeoLite/GeolocationDbUpdaterInterface.php b/module/Core/src/Geolocation/GeolocationDbUpdaterInterface.php similarity index 72% rename from module/CLI/src/GeoLite/GeolocationDbUpdaterInterface.php rename to module/Core/src/Geolocation/GeolocationDbUpdaterInterface.php index ba0f0e708..1e583e203 100644 --- a/module/CLI/src/GeoLite/GeolocationDbUpdaterInterface.php +++ b/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 { diff --git a/module/CLI/src/GeoLite/GeolocationResult.php b/module/Core/src/Geolocation/GeolocationResult.php similarity index 77% rename from module/CLI/src/GeoLite/GeolocationResult.php rename to module/Core/src/Geolocation/GeolocationResult.php index 859768864..3b472d09c 100644 --- a/module/CLI/src/GeoLite/GeolocationResult.php +++ b/module/Core/src/Geolocation/GeolocationResult.php @@ -1,6 +1,6 @@ Date: Wed, 11 Dec 2024 08:27:50 +0100 Subject: [PATCH 10/30] Add more strict parameter for GeolocationDbUpdater --- .../Visit/DownloadGeoLiteDbCommand.php | 36 +++++++++------- .../test/GeoLite/GeolocationDbUpdaterTest.php | 34 ++++++++++----- .../src/EventDispatcher/UpdateGeoLiteDb.php | 41 +++++++++++++------ .../src/Geolocation/GeolocationDbUpdater.php | 38 +++++++---------- .../GeolocationDbUpdaterInterface.php | 3 +- ...cationDownloadProgressHandlerInterface.php | 21 ++++++++++ 6 files changed, 110 insertions(+), 63 deletions(-) create mode 100644 module/Core/src/Geolocation/GeolocationDownloadProgressHandlerInterface.php diff --git a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php index f6110d07a..b0a22c97b 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -7,6 +7,7 @@ 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; @@ -16,13 +17,14 @@ 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 @@ protected function configure(): void 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 { - $io->text(sprintf('%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); - }); + $result = $this->dbUpdater->checkDbUpdate($this); 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 @@ private function processGeoLiteUpdateError(GeolocationDbUpdateFailedException $e return $olderDbExists ? ExitCode::EXIT_WARNING : ExitCode::EXIT_FAILURE; } + + public function beforeDownload(bool $olderDbExists): void + { + $this->io->text(sprintf('%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); + } } diff --git a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php index dc1d614c5..5a4518010 100644 --- a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php +++ b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php @@ -14,6 +14,7 @@ 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; @@ -29,6 +30,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 +39,23 @@ protected function setUp(): void $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 +67,9 @@ public function properResultIsReturnedWhenLicenseIsMissing(): void ); $this->geoLiteDbReader->expects($this->never())->method('metadata'); - $isCalled = false; - $result = $this->geolocationDbUpdater()->checkDbUpdate(function () use (&$isCalled): void { - $isCalled = true; - }); + $result = $this->geolocationDbUpdater()->checkDbUpdate($this->progressHandler); - self::assertTrue($isCalled); + self::assertTrue($this->progressHandler->beforeDownloadCalled); self::assertEquals(GeolocationResult::LICENSE_MISSING, $result); } @@ -67,17 +84,14 @@ public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(): void )->willThrowException($prev); $this->geoLiteDbReader->expects($this->never())->method('metadata'); - $isCalled = false; try { - $this->geolocationDbUpdater()->checkDbUpdate(function () use (&$isCalled): void { - $isCalled = true; - }); + $this->geolocationDbUpdater()->checkDbUpdate($this->progressHandler); 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); } } diff --git a/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php index 7f14cc24e..871082798 100644 --- a/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php +++ b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php @@ -8,6 +8,7 @@ use Psr\Log\LoggerInterface; 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; @@ -24,21 +25,35 @@ public function __construct( 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; - } + try { + $result = $this->dbUpdater->checkDbUpdate( + new class ($this->logger) implements GeolocationDownloadProgressHandlerInterface { + public function __construct( + private readonly LoggerInterface $logger, + private bool $messageLogged = false, + ) { + } - $messageLogged = true; - $this->logger->notice(sprintf('Finished %s GeoLite2 db file', $olderDbExists ? 'updating' : 'downloading')); - }; + public function beforeDownload(bool $olderDbExists): void + { + $this->logger->notice( + sprintf('%s GeoLite2 db file...', $olderDbExists ? 'Updating' : 'Downloading'), + ); + } - try { - $result = $this->dbUpdater->checkDbUpdate($beforeDownload, $handleProgress); + 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()); } diff --git a/module/Core/src/Geolocation/GeolocationDbUpdater.php b/module/Core/src/Geolocation/GeolocationDbUpdater.php index 73515a45a..b8d77a4b3 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdater.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php @@ -12,7 +12,6 @@ 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 @@ public function __construct( * @throws GeolocationDbUpdateFailedException */ public function checkDbUpdate( - callable|null $beforeDownload = null, - callable|null $handleProgress = null, + GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler = null, ): GeolocationResult { if (! $this->trackingOptions->isGeolocationRelevant()) { return GeolocationResult::CHECK_SKIPPED; @@ -52,7 +50,7 @@ public function checkDbUpdate( $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 @@ public function checkDbUpdate( /** * @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 @@ private function resolveBuildTimestamp(Metadata $meta): int * @throws GeolocationDbUpdateFailedException */ private function downloadNewDb( - callable|null $beforeDownload, - callable|null $handleProgress, + GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler, bool $olderDbExists, ): GeolocationResult { - if ($beforeDownload !== null) { - $beforeDownload($olderDbExists); - } + $downloadProgressHandler?->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); - } } diff --git a/module/Core/src/Geolocation/GeolocationDbUpdaterInterface.php b/module/Core/src/Geolocation/GeolocationDbUpdaterInterface.php index 1e583e203..d7322e863 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdaterInterface.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdaterInterface.php @@ -12,7 +12,6 @@ interface GeolocationDbUpdaterInterface * @throws GeolocationDbUpdateFailedException */ public function checkDbUpdate( - callable|null $beforeDownload = null, - callable|null $handleProgress = null, + GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler = null, ): GeolocationResult; } diff --git a/module/Core/src/Geolocation/GeolocationDownloadProgressHandlerInterface.php b/module/Core/src/Geolocation/GeolocationDownloadProgressHandlerInterface.php new file mode 100644 index 000000000..74cb90ae4 --- /dev/null +++ b/module/Core/src/Geolocation/GeolocationDownloadProgressHandlerInterface.php @@ -0,0 +1,21 @@ + Date: Wed, 11 Dec 2024 08:35:24 +0100 Subject: [PATCH 11/30] Fix UpdateGeoLiteDbTest --- .../EventDispatcher/UpdateGeoLiteDbTest.php | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php b/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php index 55662d5a5..5288aa1bf 100644 --- a/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php +++ b/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php @@ -14,6 +14,7 @@ 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 @@ public function exceptionWhileUpdatingDbLogsError(): void } #[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 { - $firstCallback($oldDbExists); + function (GeolocationDownloadProgressHandlerInterface $handler) use ($oldDbExists): GeolocationResult { + $handler->beforeDownload($oldDbExists); return GeolocationResult::DB_IS_UP_TO_DATE; }, ); @@ -73,18 +74,24 @@ public static function provideFlags(): iterable } #[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); - $secondCallback($total, $downloaded, $oldDbExists); - $secondCallback($total, $downloaded, $oldDbExists); + $handler->handleProgress($total, $downloaded, $oldDbExists); + $handler->handleProgress($total, $downloaded, $oldDbExists); + $handler->handleProgress($total, $downloaded, $oldDbExists); return GeolocationResult::DB_UPDATED; }, From 84d12f681108811c5c1df65fc6df0a85d3b1bd31 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 11 Dec 2024 08:47:13 +0100 Subject: [PATCH 12/30] Move GeolocationDbUpdaterTest to Core module --- .../test/Geolocation}/GeolocationDbUpdaterTest.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) rename module/{CLI/test/GeoLite => Core/test/Geolocation}/GeolocationDbUpdaterTest.php (98%) diff --git a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php similarity index 98% rename from module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php rename to module/Core/test/Geolocation/GeolocationDbUpdaterTest.php index 5a4518010..5c76747b1 100644 --- a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php +++ b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php @@ -2,9 +2,10 @@ 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; @@ -80,7 +81,7 @@ public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(): void $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'); @@ -101,7 +102,7 @@ public function exceptionIsThrownWhenOlderDbIsTooOldAndDownloadFails(int $days): $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()), From 2ede615da8c181b19b2019064cf210661aad359d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 11 Dec 2024 08:50:56 +0100 Subject: [PATCH 13/30] Fix DownloadGeoLiteDbCommandTest --- .../Command/Visit/DownloadGeoLiteDbCommandTest.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php index 2b477f031..7fa46a05a 100644 --- a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php +++ b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php @@ -12,6 +12,7 @@ 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 @@ public function showsProperMessageWhenGeoLiteUpdateFails( int $expectedExitCode, ): void { $this->dbUpdater->expects($this->once())->method('checkDbUpdate')->withAnyParameters()->willReturnCallback( - function (callable $beforeDownload, callable $handleProgress) use ($olderDbExists): void { - $beforeDownload($olderDbExists); - $handleProgress(100, 50); + function (GeolocationDownloadProgressHandlerInterface $handler) use ($olderDbExists): void { + $handler->beforeDownload($olderDbExists); + $handler->handleProgress(100, 50, $olderDbExists); throw $olderDbExists ? GeolocationDbUpdateFailedException::withOlderDb() @@ -105,8 +106,8 @@ public function printsExpectedMessageWhenNoErrorOccurs(callable $checkUpdateBeha 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 { - $beforeDownload(true); + yield 'outdated db' => [function (GeolocationDownloadProgressHandlerInterface $handler): GeolocationResult { + $handler->beforeDownload(true); return GeolocationResult::DB_CREATED; }, '[OK] GeoLite2 db file properly downloaded.']; } From 9e34183901647dde4059aa27f641dd97cf4227e1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 12 Dec 2024 08:51:23 +0100 Subject: [PATCH 14/30] Update docker images to Alpine 3.21 --- Dockerfile | 2 +- data/infra/php.Dockerfile | 2 +- data/infra/roadrunner.Dockerfile | 2 +- module/Core/src/Geolocation/GeolocationDbUpdater.php | 2 +- .../ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Dockerfile b/Dockerfile index 73e62b39a..eea707f5e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM php:8.4-alpine3.20 AS base +FROM php:8.4-alpine3.21 AS base ARG SHLINK_VERSION=latest ENV SHLINK_VERSION ${SHLINK_VERSION} diff --git a/data/infra/php.Dockerfile b/data/infra/php.Dockerfile index 3d9ea0add..0c08124f4 100644 --- a/data/infra/php.Dockerfile +++ b/data/infra/php.Dockerfile @@ -1,4 +1,4 @@ -FROM php:8.4-fpm-alpine3.20 +FROM php:8.4-fpm-alpine3.21 MAINTAINER Alejandro Celaya ENV APCU_VERSION 5.1.24 diff --git a/data/infra/roadrunner.Dockerfile b/data/infra/roadrunner.Dockerfile index f01943b01..3619aba32 100644 --- a/data/infra/roadrunner.Dockerfile +++ b/data/infra/roadrunner.Dockerfile @@ -1,4 +1,4 @@ -FROM php:8.4-alpine3.20 +FROM php:8.4-alpine3.21 MAINTAINER Alejandro Celaya ENV PDO_SQLSRV_VERSION 5.12.0 diff --git a/module/Core/src/Geolocation/GeolocationDbUpdater.php b/module/Core/src/Geolocation/GeolocationDbUpdater.php index b8d77a4b3..9c7d61d3a 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdater.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php @@ -47,7 +47,7 @@ public function checkDbUpdate( } $lock = $this->locker->createLock(self::LOCK_NAME); - $lock->acquire(true); // Block until lock is released + $lock->acquire(blocking: true); try { return $this->downloadIfNeeded($downloadProgressHandler); diff --git a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php index df578387e..7f43d4438 100644 --- a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php @@ -106,8 +106,8 @@ private function lock(array &$locks, string $name): void { // Lock dependency creation for up to 5 seconds. This will prevent errors when trying to create the same one // more than once in parallel. - $locks[$name] = $lock = $this->locker->createLock($name, 5); - $lock->acquire(true); + $locks[$name] = $lock = $this->locker->createLock($name, ttl: 5); + $lock->acquire(blocking: true); } /** From d4d97c3182e8fd0910836114edf33247a3bfca29 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 13 Dec 2024 10:33:53 +0100 Subject: [PATCH 15/30] Create new table to track geolocation updates --- ...Geolocation.Entity.GeolocationDbUpdate.php | 64 +++++++++++++++++ .../Core/migrations/Version20241212131058.php | 68 +++++++++++++++++++ .../Entity/GeolocationDbUpdate.php | 42 ++++++++++++ .../Entity/GeolocationDbUpdateStatus.php | 10 +++ 4 files changed, 184 insertions(+) create mode 100644 module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php create mode 100644 module/Core/migrations/Version20241212131058.php create mode 100644 module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php create mode 100644 module/Core/src/Geolocation/Entity/GeolocationDbUpdateStatus.php diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php new file mode 100644 index 000000000..ecf21fb2a --- /dev/null +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php @@ -0,0 +1,64 @@ +setTable(determineTableName('geolocation_db_updates', $emConfig)); + + $builder->createField('id', Types::BIGINT) + ->columnName('id') + ->makePrimaryKey() + ->generatedValue('IDENTITY') + ->option('unsigned', true) + ->build(); + + $builder->createField('dateCreated', ChronosDateTimeType::CHRONOS_DATETIME) + ->columnName('date_created') + ->build(); + + $builder->createField('dateUpdated', ChronosDateTimeType::CHRONOS_DATETIME) + ->columnName('date_updated') + ->nullable() + ->build(); + + (new FieldBuilder($builder, [ + 'fieldName' => 'status', + 'type' => Types::STRING, + 'enumType' => GeolocationDbUpdateStatus::class, + ]))->columnName('status') + ->length(128) + ->build(); + + fieldWithUtf8Charset($builder->createField('filename', Types::STRING), $emConfig) + ->columnName('filename') + ->length(512) + ->nullable() + ->build(); + + fieldWithUtf8Charset($builder->createField('error', Types::STRING), $emConfig) + ->columnName('error') + ->length(1024) + ->nullable() + ->build(); + + fieldWithUtf8Charset($builder->createField('filesystemId', Types::STRING), $emConfig) + ->columnName('filesystem_id') + ->length(512) + ->build(); + + // Index on date_updated, as we'll usually sort the query by this field + $builder->addIndex(['date_updated'], 'IDX_geolocation_date_updated'); + // Index on status and filesystem_id, as we'll usually filter the query by those fields + $builder->addIndex(['status', 'filesystem_id'], 'IDX_geolocation_status_filesystem'); +}; diff --git a/module/Core/migrations/Version20241212131058.php b/module/Core/migrations/Version20241212131058.php new file mode 100644 index 000000000..1ba4bc02c --- /dev/null +++ b/module/Core/migrations/Version20241212131058.php @@ -0,0 +1,68 @@ +skipIf($schema->hasTable(self::TABLE_NAME)); + + $table = $schema->createTable(self::TABLE_NAME); + $table->addColumn('id', Types::BIGINT, [ + 'unsigned' => true, + 'autoincrement' => true, + 'notnull' => true, + ]); + $table->setPrimaryKey(['id']); + + $table->addColumn('date_created', ChronosDateTimeType::CHRONOS_DATETIME, ['default' => 'CURRENT_TIMESTAMP']); + $table->addColumn('date_updated', ChronosDateTimeType::CHRONOS_DATETIME, ['default' => 'CURRENT_TIMESTAMP']); + + $table->addColumn('status', Types::STRING, [ + 'length' => 128, + 'default' => 'in-progress', // in-progress, success, error + ]); + $table->addColumn('filesystem_id', Types::STRING, ['length' => 512]); + + $table->addColumn('filename', Types::STRING, [ + 'length' => 512, + 'default' => null, + 'notnull' => false, + ]); + $table->addColumn('error', Types::STRING, [ + 'length' => 1024, + 'default' => null, + 'notnull' => false, + ]); + + // Index on date_updated, as we'll usually sort the query by this field + $table->addIndex(['date_updated'], 'IDX_geolocation_date_updated'); + // Index on status and filesystem_id, as we'll usually filter the query by those fields + $table->addIndex(['status', 'filesystem_id'], 'IDX_geolocation_status_filesystem'); + } + + public function down(Schema $schema): void + { + $this->skipIf(! $schema->hasTable(self::TABLE_NAME)); + $schema->dropTable(self::TABLE_NAME); + } + + public function isTransactional(): bool + { + return ! ($this->connection->getDatabasePlatform() instanceof MySQLPlatform); + } +} diff --git a/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php b/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php new file mode 100644 index 000000000..0f376cb94 --- /dev/null +++ b/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php @@ -0,0 +1,42 @@ +dateUpdated = Chronos::now(); + $this->filename = $filename; + $this->status = GeolocationDbUpdateStatus::SUCCESS; + } + + public function finishWithError(string $error): void + { + $this->dateUpdated = Chronos::now(); + $this->error = $error; + $this->status = GeolocationDbUpdateStatus::ERROR; + } +} diff --git a/module/Core/src/Geolocation/Entity/GeolocationDbUpdateStatus.php b/module/Core/src/Geolocation/Entity/GeolocationDbUpdateStatus.php new file mode 100644 index 000000000..8216f2bd3 --- /dev/null +++ b/module/Core/src/Geolocation/Entity/GeolocationDbUpdateStatus.php @@ -0,0 +1,10 @@ + Date: Sun, 15 Dec 2024 10:05:32 +0100 Subject: [PATCH 16/30] Refactor geolocation download logic based on database table --- .../Visit/DownloadGeoLiteDbCommand.php | 2 +- ...GeolocationDbUpdateFailedExceptionTest.php | 16 +-- module/Core/config/dependencies.config.php | 3 +- ...Geolocation.Entity.GeolocationDbUpdate.php | 6 - .../Core/migrations/Version20241212131058.php | 5 - .../src/EventDispatcher/UpdateGeoLiteDb.php | 1 + .../GeolocationDbUpdateFailedException.php | 40 ++----- .../Entity/GeolocationDbUpdate.php | 44 ++++++- .../src/Geolocation/GeolocationDbUpdater.php | 111 ++++++++++-------- .../Geolocation/GeolocationDbUpdaterTest.php | 4 +- 10 files changed, 117 insertions(+), 115 deletions(-) diff --git a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php index b0a22c97b..8e873b1dc 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -66,7 +66,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int private function processGeoLiteUpdateError(GeolocationDbUpdateFailedException $e, SymfonyStyle $io): int { - $olderDbExists = $e->olderDbExists(); + $olderDbExists = $e->olderDbExists; if ($olderDbExists) { $io->warning( diff --git a/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php b/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php index a1d6db657..86ed95486 100644 --- a/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php +++ b/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php @@ -19,7 +19,7 @@ public function withOlderDbBuildsException(Throwable|null $prev): void { $e = GeolocationDbUpdateFailedException::withOlderDb($prev); - self::assertTrue($e->olderDbExists()); + self::assertTrue($e->olderDbExists); self::assertEquals( 'An error occurred while updating geolocation database, but an older DB is already present.', $e->getMessage(), @@ -33,7 +33,7 @@ public function withoutOlderDbBuildsException(Throwable|null $prev): void { $e = GeolocationDbUpdateFailedException::withoutOlderDb($prev); - self::assertFalse($e->olderDbExists()); + self::assertFalse($e->olderDbExists); self::assertEquals( 'An error occurred while updating geolocation database, and an older version could not be found.', $e->getMessage(), @@ -48,16 +48,4 @@ public static function providePrev(): iterable yield 'RuntimeException' => [new RuntimeException('prev')]; yield 'Exception' => [new Exception('prev')]; } - - #[Test] - public function withInvalidEpochInOldDbBuildsException(): void - { - $e = GeolocationDbUpdateFailedException::withInvalidEpochInOldDb('foobar'); - - self::assertTrue($e->olderDbExists()); - self::assertEquals( - 'Build epoch with value "foobar" from existing geolocation database, could not be parsed to integer.', - $e->getMessage(), - ); - } } diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index b16a4c5cc..adc9ae2a3 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -13,7 +13,6 @@ 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; @@ -247,9 +246,9 @@ GeolocationDbUpdater::class => [ DbUpdater::class, - GeoLite2ReaderFactory::class, LOCAL_LOCK_FACTORY, Config\Options\TrackingOptions::class, + 'em', ], Geolocation\Middleware\IpGeolocationMiddleware::class => [ IpLocationResolverInterface::class, diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php index ecf21fb2a..20a67c983 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php @@ -40,12 +40,6 @@ ->length(128) ->build(); - fieldWithUtf8Charset($builder->createField('filename', Types::STRING), $emConfig) - ->columnName('filename') - ->length(512) - ->nullable() - ->build(); - fieldWithUtf8Charset($builder->createField('error', Types::STRING), $emConfig) ->columnName('error') ->length(1024) diff --git a/module/Core/migrations/Version20241212131058.php b/module/Core/migrations/Version20241212131058.php index 1ba4bc02c..592869314 100644 --- a/module/Core/migrations/Version20241212131058.php +++ b/module/Core/migrations/Version20241212131058.php @@ -38,11 +38,6 @@ public function up(Schema $schema): void ]); $table->addColumn('filesystem_id', Types::STRING, ['length' => 512]); - $table->addColumn('filename', Types::STRING, [ - 'length' => 512, - 'default' => null, - 'notnull' => false, - ]); $table->addColumn('error', Types::STRING, [ 'length' => 1024, 'default' => null, diff --git a/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php index 871082798..8d288959a 100644 --- a/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php +++ b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php @@ -14,6 +14,7 @@ use function sprintf; +/** @todo Rename to UpdateGeolocationDb */ readonly class UpdateGeoLiteDb { public function __construct( diff --git a/module/Core/src/Exception/GeolocationDbUpdateFailedException.php b/module/Core/src/Exception/GeolocationDbUpdateFailedException.php index f3c3f65fd..a818b2639 100644 --- a/module/Core/src/Exception/GeolocationDbUpdateFailedException.php +++ b/module/Core/src/Exception/GeolocationDbUpdateFailedException.php @@ -7,52 +7,28 @@ use RuntimeException; use Throwable; -use function sprintf; - class GeolocationDbUpdateFailedException extends RuntimeException implements ExceptionInterface { - private bool $olderDbExists; - - private function __construct(string $message, Throwable|null $previous = null) + private function __construct(string $message, public readonly bool $olderDbExists, Throwable|null $prev = null) { - parent::__construct($message, previous: $previous); + parent::__construct($message, previous: $prev); } public static function withOlderDb(Throwable|null $prev = null): self { - $e = new self( + return new self( 'An error occurred while updating geolocation database, but an older DB is already present.', - $prev, + olderDbExists: true, + prev: $prev, ); - $e->olderDbExists = true; - - return $e; } public static function withoutOlderDb(Throwable|null $prev = null): self { - $e = new self( + return new self( 'An error occurred while updating geolocation database, and an older version could not be found.', - $prev, + olderDbExists: false, + prev: $prev, ); - $e->olderDbExists = false; - - return $e; - } - - public static function withInvalidEpochInOldDb(mixed $buildEpoch): self - { - $e = new self(sprintf( - 'Build epoch with value "%s" from existing geolocation database, could not be parsed to integer.', - $buildEpoch, - )); - $e->olderDbExists = true; - - return $e; - } - - public function olderDbExists(): bool - { - return $this->olderDbExists; } } diff --git a/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php b/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php index 0f376cb94..1e1d544f3 100644 --- a/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php +++ b/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php @@ -16,27 +16,59 @@ private function __construct( private GeolocationDbUpdateStatus $status = GeolocationDbUpdateStatus::IN_PROGRESS, private readonly Chronos $dateCreated = new Chronos(), private Chronos $dateUpdated = new Chronos(), - private string|null $filename = null, private string|null $error = null, ) { } - public static function createForCurrentFilesystem(): self + public static function forFilesystemId(string|null $filesystemId = null): self { - return new self(stat(__FILE__)['dev']); + return new self($filesystemId ?? self::currentFilesystemId()); } - public function finishSuccessfully(string $filename): void + public static function currentFilesystemId(): string + { + $system = stat(__FILE__); + if (! $system) { + // TODO Throw error + } + + return (string) $system['dev']; + } + + public function finishSuccessfully(): void { $this->dateUpdated = Chronos::now(); - $this->filename = $filename; $this->status = GeolocationDbUpdateStatus::SUCCESS; } public function finishWithError(string $error): void { - $this->dateUpdated = Chronos::now(); $this->error = $error; + $this->dateUpdated = Chronos::now(); $this->status = GeolocationDbUpdateStatus::ERROR; } + + /** + * This update would require a new download if: + * - It is successful and older than 30 days + * - It is error and older than 2 days + */ + public function needsUpdate(): bool + { + return match ($this->status) { + GeolocationDbUpdateStatus::SUCCESS => Chronos::now()->greaterThan($this->dateUpdated->addDays(30)), + GeolocationDbUpdateStatus::ERROR => Chronos::now()->greaterThan($this->dateUpdated->addDays(2)), + default => false, + }; + } + + public function isInProgress(): bool + { + return $this->status === GeolocationDbUpdateStatus::IN_PROGRESS; + } + + public function isError(): bool + { + return $this->status === GeolocationDbUpdateStatus::ERROR; + } } diff --git a/module/Core/src/Geolocation/GeolocationDbUpdater.php b/module/Core/src/Geolocation/GeolocationDbUpdater.php index 9c7d61d3a..e5bd0ea19 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdater.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php @@ -4,36 +4,29 @@ namespace Shlinkio\Shlink\Core\Geolocation; -use Cake\Chronos\Chronos; -use Closure; -use GeoIp2\Database\Reader; -use MaxMind\Db\Reader\Metadata; +use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Config\Options\TrackingOptions; use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException; +use Shlinkio\Shlink\Core\Geolocation\Entity\GeolocationDbUpdate; use Shlinkio\Shlink\IpGeolocation\Exception\DbUpdateException; use Shlinkio\Shlink\IpGeolocation\Exception\MissingLicenseException; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; use Symfony\Component\Lock\LockFactory; -use function is_int; +use function count; +use function Shlinkio\Shlink\Core\ArrayUtils\every; +use function sprintf; -class GeolocationDbUpdater implements GeolocationDbUpdaterInterface +readonly class GeolocationDbUpdater implements GeolocationDbUpdaterInterface { private const string LOCK_NAME = 'geolocation-db-update'; - /** @var Closure(): Reader */ - private readonly Closure $geoLiteDbReaderFactory; - - /** - * @param callable(): Reader $geoLiteDbReaderFactory - */ public function __construct( - private readonly DbUpdaterInterface $dbUpdater, - callable $geoLiteDbReaderFactory, - private readonly LockFactory $locker, - private readonly TrackingOptions $trackingOptions, + private DbUpdaterInterface $dbUpdater, + private LockFactory $locker, + private TrackingOptions $trackingOptions, + private EntityManagerInterface $em, ) { - $this->geoLiteDbReaderFactory = $geoLiteDbReaderFactory(...); } /** @@ -46,6 +39,7 @@ public function checkDbUpdate( return GeolocationResult::CHECK_SKIPPED; } + $lock = $this->locker->createLock(self::LOCK_NAME); $lock->acquire(blocking: true); @@ -62,43 +56,68 @@ public function checkDbUpdate( private function downloadIfNeeded( GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler, ): GeolocationResult { - if (! $this->dbUpdater->databaseFileExists()) { - return $this->downloadNewDb($downloadProgressHandler, olderDbExists: false); + $maxRecentAttemptsToCheck = 15; // TODO Make this configurable + + // Get last 15 download attempts + $recentDownloads = $this->em->getRepository(GeolocationDbUpdate::class)->findBy( + criteria: ['filesystemId' => GeolocationDbUpdate::currentFilesystemId()], + orderBy: ['dateUpdated' => 'DESC'], + limit: $maxRecentAttemptsToCheck, + ); + $mostRecentDownload = $recentDownloads[0] ?? null; + $amountOfRecentAttempts = count($recentDownloads); + + // 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; } - $meta = ($this->geoLiteDbReaderFactory)()->metadata(); - if ($this->buildIsTooOld($meta)) { - return $this->downloadNewDb($downloadProgressHandler, olderDbExists: true); + // If all recent attempts are errors, and the most recent one is not old enough, skip download + if ( + $amountOfRecentAttempts === $maxRecentAttemptsToCheck + && every($recentDownloads, static fn (GeolocationDbUpdate $update) => $update->isError()) + && ! $mostRecentDownload->needsUpdate() + ) { + return GeolocationResult::CHECK_SKIPPED; } - return GeolocationResult::DB_IS_UP_TO_DATE; - } - - private function buildIsTooOld(Metadata $meta): bool - { - $buildTimestamp = $this->resolveBuildTimestamp($meta); - $buildDate = Chronos::createFromTimestamp($buildTimestamp); + // Try to download if there are no attempts, the database file does not exist or most recent attempt was + // successful and is old enough + $olderDbExists = $amountOfRecentAttempts > 0 && $this->dbUpdater->databaseFileExists(); + if (! $olderDbExists || $mostRecentDownload->needsUpdate()) { + return $this->downloadAndTrackUpdate($downloadProgressHandler, $olderDbExists); + } - return Chronos::now()->greaterThan($buildDate->addDays(35)); + return GeolocationResult::DB_IS_UP_TO_DATE; } - private function resolveBuildTimestamp(Metadata $meta): int - { - // In theory the buildEpoch should be an int, but it has been reported to come as a string. - // See https://github.com/shlinkio/shlink/issues/1002 for context - - /** @var int|string $buildEpoch */ - $buildEpoch = $meta->buildEpoch; - if (is_int($buildEpoch)) { - return $buildEpoch; - } + /** + * @throws GeolocationDbUpdateFailedException + */ + private function downloadAndTrackUpdate( + GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler, + bool $olderDbExists, + ): GeolocationResult { + $dbUpdate = GeolocationDbUpdate::forFilesystemId(); + $this->em->persist($dbUpdate); + $this->em->flush(); - $intBuildEpoch = (int) $buildEpoch; - if ($buildEpoch === (string) $intBuildEpoch) { - return $intBuildEpoch; + try { + $result = $this->downloadNewDb($downloadProgressHandler, $olderDbExists); + $dbUpdate->finishSuccessfully(); + return $result; + } catch (MissingLicenseException) { + $dbUpdate->finishWithError('Geolocation license key is missing'); + return GeolocationResult::LICENSE_MISSING; + } catch (GeolocationDbUpdateFailedException $e) { + $dbUpdate->finishWithError( + sprintf('%s. Prev: %s', $e->getMessage(), $e->getPrevious()?->getMessage() ?? '-'), + ); + throw $e; + } finally { + $this->em->flush(); } - - throw GeolocationDbUpdateFailedException::withInvalidEpochInOldDb($buildEpoch); } /** @@ -116,8 +135,6 @@ private function downloadNewDb( => $downloadProgressHandler?->handleProgress($total, $downloaded, $olderDbExists), ); return $olderDbExists ? GeolocationResult::DB_UPDATED : GeolocationResult::DB_CREATED; - } catch (MissingLicenseException) { - return GeolocationResult::LICENSE_MISSING; } catch (DbUpdateException $e) { throw $olderDbExists ? GeolocationDbUpdateFailedException::withOlderDb($e) diff --git a/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php index 5c76747b1..d2ec1bfa7 100644 --- a/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php +++ b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php @@ -91,7 +91,7 @@ public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(): void } catch (Throwable $e) { self::assertInstanceOf(GeolocationDbUpdateFailedException::class, $e); self::assertSame($prev, $e->getPrevious()); - self::assertFalse($e->olderDbExists()); + self::assertFalse($e->olderDbExists); self::assertTrue($this->progressHandler->beforeDownloadCalled); } } @@ -114,7 +114,7 @@ public function exceptionIsThrownWhenOlderDbIsTooOldAndDownloadFails(int $days): } catch (Throwable $e) { self::assertInstanceOf(GeolocationDbUpdateFailedException::class, $e); self::assertSame($prev, $e->getPrevious()); - self::assertTrue($e->olderDbExists()); + self::assertTrue($e->olderDbExists); } } From f10a9d3972a295c02e0246eb600a1e4e1518c84b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Dec 2024 10:08:22 +0100 Subject: [PATCH 17/30] Simplify geolocation_db_updates indexes --- ...kio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php | 4 ++-- module/Core/migrations/Version20241212131058.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php index 20a67c983..e7f10ca13 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php @@ -53,6 +53,6 @@ // Index on date_updated, as we'll usually sort the query by this field $builder->addIndex(['date_updated'], 'IDX_geolocation_date_updated'); - // Index on status and filesystem_id, as we'll usually filter the query by those fields - $builder->addIndex(['status', 'filesystem_id'], 'IDX_geolocation_status_filesystem'); + // Index on filesystem_id, as we'll usually filter the query by this field + $builder->addIndex(['filesystem_id'], 'IDX_geolocation_status_filesystem'); }; diff --git a/module/Core/migrations/Version20241212131058.php b/module/Core/migrations/Version20241212131058.php index 592869314..e27958f62 100644 --- a/module/Core/migrations/Version20241212131058.php +++ b/module/Core/migrations/Version20241212131058.php @@ -46,8 +46,8 @@ public function up(Schema $schema): void // Index on date_updated, as we'll usually sort the query by this field $table->addIndex(['date_updated'], 'IDX_geolocation_date_updated'); - // Index on status and filesystem_id, as we'll usually filter the query by those fields - $table->addIndex(['status', 'filesystem_id'], 'IDX_geolocation_status_filesystem'); + // Index on filesystem_id, as we'll usually filter the query by this field + $table->addIndex(['filesystem_id'], 'IDX_geolocation_status_filesystem'); } public function down(Schema $schema): void From 853c50a81974851b68c46f95e457ee0d782a8725 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Dec 2024 11:34:38 +0100 Subject: [PATCH 18/30] Fix some cases of database download in GeolocationDbUpdater --- .../Visit/DownloadGeoLiteDbCommand.php | 5 +++ .../Visit/DownloadGeoLiteDbCommandTest.php | 11 ++--- .../Entity/GeolocationDbUpdate.php | 17 ++++--- .../src/Geolocation/GeolocationDbUpdater.php | 45 +++++++++++-------- .../src/Geolocation/GeolocationResult.php | 1 + 5 files changed, 46 insertions(+), 33 deletions(-) diff --git a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php index 8e873b1dc..e1ea5fce7 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -51,6 +51,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int return ExitCode::EXIT_WARNING; } + if ($result === GeolocationResult::MAX_ERRORS_REACHED) { + $this->io->warning('Max consecutive errors reached. Cannot retry for a couple of days.'); + 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 7fa46a05a..f232bdca0 100644 --- a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php +++ b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\Visit\DownloadGeoLiteDbCommand; @@ -74,17 +75,17 @@ public static function provideFailureParams(): iterable } #[Test] - public function warningIsPrintedWhenLicenseIsMissing(): void + #[TestWith([GeolocationResult::LICENSE_MISSING, 'It was not possible to download GeoLite2 db'])] + #[TestWith([GeolocationResult::MAX_ERRORS_REACHED, 'Max consecutive errors reached'])] + public function warningIsPrintedForSomeResults(GeolocationResult $result, string $expectedWarningMessage): void { - $this->dbUpdater->expects($this->once())->method('checkDbUpdate')->withAnyParameters()->willReturn( - GeolocationResult::LICENSE_MISSING, - ); + $this->dbUpdater->expects($this->once())->method('checkDbUpdate')->withAnyParameters()->willReturn($result); $this->commandTester->execute([]); $output = $this->commandTester->getDisplay(); $exitCode = $this->commandTester->getStatusCode(); - self::assertStringContainsString('[WARNING] It was not possible to download GeoLite2 db', $output); + self::assertStringContainsString('[WARNING] ' . $expectedWarningMessage, $output); self::assertSame(ExitCode::EXIT_WARNING, $exitCode); } diff --git a/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php b/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php index 1e1d544f3..35a9697dc 100644 --- a/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php +++ b/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php @@ -49,17 +49,11 @@ public function finishWithError(string $error): void } /** - * This update would require a new download if: - * - It is successful and older than 30 days - * - It is error and older than 2 days + * @param positive-int $days */ - public function needsUpdate(): bool + public function isOlderThan(int $days): bool { - return match ($this->status) { - GeolocationDbUpdateStatus::SUCCESS => Chronos::now()->greaterThan($this->dateUpdated->addDays(30)), - GeolocationDbUpdateStatus::ERROR => Chronos::now()->greaterThan($this->dateUpdated->addDays(2)), - default => false, - }; + return Chronos::now()->greaterThan($this->dateUpdated->addDays($days)); } public function isInProgress(): bool @@ -71,4 +65,9 @@ public function isError(): bool { return $this->status === GeolocationDbUpdateStatus::ERROR; } + + public function isSuccess(): bool + { + return $this->status === GeolocationDbUpdateStatus::SUCCESS; + } } diff --git a/module/Core/src/Geolocation/GeolocationDbUpdater.php b/module/Core/src/Geolocation/GeolocationDbUpdater.php index e5bd0ea19..f6ca7b493 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdater.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php @@ -13,8 +13,6 @@ use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; use Symfony\Component\Lock\LockFactory; -use function count; -use function Shlinkio\Shlink\Core\ArrayUtils\every; use function sprintf; readonly class GeolocationDbUpdater implements GeolocationDbUpdaterInterface @@ -26,6 +24,7 @@ public function __construct( private LockFactory $locker, private TrackingOptions $trackingOptions, private EntityManagerInterface $em, + private int $maxRecentAttemptsToCheck = 15, // TODO Make this configurable ) { } @@ -56,16 +55,12 @@ public function checkDbUpdate( private function downloadIfNeeded( GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler, ): GeolocationResult { - $maxRecentAttemptsToCheck = 15; // TODO Make this configurable - - // Get last 15 download attempts $recentDownloads = $this->em->getRepository(GeolocationDbUpdate::class)->findBy( criteria: ['filesystemId' => GeolocationDbUpdate::currentFilesystemId()], orderBy: ['dateUpdated' => 'DESC'], - limit: $maxRecentAttemptsToCheck, + limit: $this->maxRecentAttemptsToCheck, ); $mostRecentDownload = $recentDownloads[0] ?? null; - $amountOfRecentAttempts = count($recentDownloads); // 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. @@ -73,19 +68,31 @@ private function downloadIfNeeded( return GeolocationResult::CHECK_SKIPPED; } - // If all recent attempts are errors, and the most recent one is not old enough, skip download - if ( - $amountOfRecentAttempts === $maxRecentAttemptsToCheck - && every($recentDownloads, static fn (GeolocationDbUpdate $update) => $update->isError()) - && ! $mostRecentDownload->needsUpdate() - ) { - return GeolocationResult::CHECK_SKIPPED; + $amountOfErrorsSinceLastSuccess = 0; + foreach ($recentDownloads as $recentDownload) { + // Count attempts until a success is found + if ($recentDownload->isSuccess()) { + break; + } + $amountOfErrorsSinceLastSuccess++; + } + + // If max amount of consecutive errors has been reached and the most recent one is not old enough, skip download + // for 2 days to avoid hitting potential API limits in geolocation services + $lastAttemptIsError = $mostRecentDownload !== null && $mostRecentDownload->isError(); + // FIXME Once max errors are reached there will be one attempt every 2 days, but it should be 15 attempts every + // 2 days. Leaving like this for simplicity for now. + $maxConsecutiveErrorsReached = $amountOfErrorsSinceLastSuccess === $this->maxRecentAttemptsToCheck; + if ($lastAttemptIsError && $maxConsecutiveErrorsReached && ! $mostRecentDownload->isOlderThan(days: 2)) { + return GeolocationResult::MAX_ERRORS_REACHED; } - // Try to download if there are no attempts, the database file does not exist or most recent attempt was - // successful and is old enough - $olderDbExists = $amountOfRecentAttempts > 0 && $this->dbUpdater->databaseFileExists(); - if (! $olderDbExists || $mostRecentDownload->needsUpdate()) { + // Try to download if: + // - There are no attempts or the database file does not exist + // - Last update errored (and implicitly, the max amount of consecutive errors has not been reached) + // - Most recent attempt is older than 30 days (and implicitly, successful) + $olderDbExists = $mostRecentDownload !== null && $this->dbUpdater->databaseFileExists(); + if (! $olderDbExists || $lastAttemptIsError || $mostRecentDownload->isOlderThan(days: 30)) { return $this->downloadAndTrackUpdate($downloadProgressHandler, $olderDbExists); } @@ -112,7 +119,7 @@ private function downloadAndTrackUpdate( return GeolocationResult::LICENSE_MISSING; } catch (GeolocationDbUpdateFailedException $e) { $dbUpdate->finishWithError( - sprintf('%s. Prev: %s', $e->getMessage(), $e->getPrevious()?->getMessage() ?? '-'), + sprintf('%s Prev: %s', $e->getMessage(), $e->getPrevious()?->getMessage() ?? '-'), ); throw $e; } finally { diff --git a/module/Core/src/Geolocation/GeolocationResult.php b/module/Core/src/Geolocation/GeolocationResult.php index 3b472d09c..1eeceddaf 100644 --- a/module/Core/src/Geolocation/GeolocationResult.php +++ b/module/Core/src/Geolocation/GeolocationResult.php @@ -5,6 +5,7 @@ enum GeolocationResult { case CHECK_SKIPPED; + case MAX_ERRORS_REACHED; case LICENSE_MISSING; case DB_CREATED; case DB_UPDATED; From 72a962ec6d00dc348b588c6b9fdac33e7a0c6aa6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 15 Dec 2024 12:03:01 +0100 Subject: [PATCH 19/30] 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 e1ea5fce7..cd7c4ffb9 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -56,6 +56,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int 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 f232bdca0..01322f0b3 100644 --- a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php +++ b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php @@ -77,6 +77,7 @@ public static function provideFailureParams(): iterable #[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 f6ca7b493..662e6b7a7 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdater.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php @@ -12,6 +12,7 @@ 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 @@ private function downloadIfNeeded( // 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 @@ private function downloadAndTrackUpdate( 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 1eeceddaf..eb5b4a5c8 100644 --- a/module/Core/src/Geolocation/GeolocationResult.php +++ b/module/Core/src/Geolocation/GeolocationResult.php @@ -4,10 +4,18 @@ 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; } From e715a0fb6f15a9132d58559ba95ebab1664b9cc6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 16 Dec 2024 09:23:30 +0100 Subject: [PATCH 20/30] Track reason for which a geolocation db download was attempted --- ...Geolocation.Entity.GeolocationDbUpdate.php | 5 +++++ .../Core/migrations/Version20241212131058.php | 1 + .../Entity/GeolocationDbUpdate.php | 5 +++-- .../src/Geolocation/GeolocationDbUpdater.php | 19 ++++++++++++++----- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php index e7f10ca13..7b60abdca 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php @@ -46,6 +46,11 @@ ->nullable() ->build(); + fieldWithUtf8Charset($builder->createField('reason', Types::STRING), $emConfig) + ->columnName('reason') + ->length(1024) + ->build(); + fieldWithUtf8Charset($builder->createField('filesystemId', Types::STRING), $emConfig) ->columnName('filesystem_id') ->length(512) diff --git a/module/Core/migrations/Version20241212131058.php b/module/Core/migrations/Version20241212131058.php index e27958f62..23e618030 100644 --- a/module/Core/migrations/Version20241212131058.php +++ b/module/Core/migrations/Version20241212131058.php @@ -38,6 +38,7 @@ public function up(Schema $schema): void ]); $table->addColumn('filesystem_id', Types::STRING, ['length' => 512]); + $table->addColumn('reason', Types::STRING, ['length' => 1024]); $table->addColumn('error', Types::STRING, [ 'length' => 1024, 'default' => null, diff --git a/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php b/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php index 35a9697dc..42cdfa4b7 100644 --- a/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php +++ b/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php @@ -13,6 +13,7 @@ class GeolocationDbUpdate extends AbstractEntity { private function __construct( 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(), @@ -20,9 +21,9 @@ private function __construct( ) { } - public static function forFilesystemId(string|null $filesystemId = null): self + public static function withReason(string $reason, string|null $filesystemId = null): self { - return new self($filesystemId ?? self::currentFilesystemId()); + return new self($reason, $filesystemId ?? self::currentFilesystemId()); } public static function currentFilesystemId(): string diff --git a/module/Core/src/Geolocation/GeolocationDbUpdater.php b/module/Core/src/Geolocation/GeolocationDbUpdater.php index 662e6b7a7..7be1cd562 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdater.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php @@ -89,12 +89,20 @@ private function downloadIfNeeded( } // Try to download if: - // - There are no attempts or the database file does not exist + // - There are no attempts tracked + // - The database file does not exist // - Last update errored (and implicitly, the max amount of consecutive errors has not been reached) // - Most recent attempt is older than 30 days (and implicitly, successful) - $olderDbExists = $mostRecentDownload !== null && $this->dbUpdater->databaseFileExists(); - if (! $olderDbExists || $lastAttemptIsError || $mostRecentDownload->isOlderThan(days: 30)) { - return $this->downloadAndTrackUpdate($downloadProgressHandler, $olderDbExists); + $reasonMatch = match (true) { + $mostRecentDownload === null => [false, 'No download attempts tracked for this instance'], + $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'], + default => null, + }; + if ($reasonMatch !== null) { + [$olderDbExists, $reason] = $reasonMatch; + return $this->downloadAndTrackUpdate($downloadProgressHandler, $olderDbExists, $reason); } return GeolocationResult::DB_IS_UP_TO_DATE; @@ -106,8 +114,9 @@ private function downloadIfNeeded( private function downloadAndTrackUpdate( GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler, bool $olderDbExists, + string $reason, ): GeolocationResult { - $dbUpdate = GeolocationDbUpdate::forFilesystemId(); + $dbUpdate = GeolocationDbUpdate::withReason($reason); $this->em->persist($dbUpdate); $this->em->flush(); From 509ef668e6f9664f0edd32d60f2cad9af2b96745 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 16 Dec 2024 19:50:06 +0100 Subject: [PATCH 21/30] Fix GeolocationDbUpdater test --- CHANGELOG.md | 9 +- .../Entity/GeolocationDbUpdate.php | 15 +- .../src/Geolocation/GeolocationDbUpdater.php | 4 +- .../Geolocation/GeolocationDbUpdaterTest.php | 202 +++++++++++++----- 4 files changed, 161 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12b5aae40..fb43ec7ee 100644 --- a/CHANGELOG.md +++ b/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* diff --git a/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php b/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php index 42cdfa4b7..f3735a64b 100644 --- a/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php +++ b/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php @@ -6,14 +6,15 @@ 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(), @@ -21,32 +22,34 @@ private function __construct( ) { } - 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; } /** diff --git a/module/Core/src/Geolocation/GeolocationDbUpdater.php b/module/Core/src/Geolocation/GeolocationDbUpdater.php index 7be1cd562..9e63cf5c3 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdater.php +++ b/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) { diff --git a/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php index d2ec1bfa7..c29830307 100644 --- a/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php +++ b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php @@ -6,14 +6,16 @@ use Cake\Chronos\Chronos; use Closure; -use GeoIp2\Database\Reader; -use MaxMind\Db\Reader\Metadata; +use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\EntityRepository; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use RuntimeException; use Shlinkio\Shlink\Core\Config\Options\TrackingOptions; use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException; +use Shlinkio\Shlink\Core\Geolocation\Entity\GeolocationDbUpdate; use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdater; use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface; use Shlinkio\Shlink\Core\Geolocation\GeolocationResult; @@ -29,17 +31,24 @@ class GeolocationDbUpdaterTest extends TestCase { private MockObject & DbUpdaterInterface $dbUpdater; - private MockObject & Reader $geoLiteDbReader; private MockObject & Lock\LockInterface $lock; + private MockObject & EntityManagerInterface $em; + /** @var MockObject&EntityRepository */ + private MockObject & EntityRepository $repo; /** @var GeolocationDownloadProgressHandlerInterface&object{beforeDownloadCalled: bool, handleProgressCalled: bool} */ private GeolocationDownloadProgressHandlerInterface $progressHandler; protected function setUp(): void { $this->dbUpdater = $this->createMock(DbUpdaterInterface::class); - $this->geoLiteDbReader = $this->createMock(Reader::class); + $this->lock = $this->createMock(Lock\SharedLockInterface::class); $this->lock->method('acquire')->with($this->isTrue())->willReturn(true); + + $this->em = $this->createMock(EntityManagerInterface::class); + $this->repo = $this->createMock(EntityRepository::class); + $this->em->method('getRepository')->willReturn($this->repo); + $this->progressHandler = new class implements GeolocationDownloadProgressHandlerInterface { public function __construct( public bool $beforeDownloadCalled = false, @@ -59,6 +68,32 @@ public function handleProgress(int $total, int $downloaded, bool $olderDbExists) }; } + #[Test] + public function properResultIsReturnedIfMostRecentUpdateIsInProgress(): void + { + $this->repo->expects($this->once())->method('findBy')->willReturn([GeolocationDbUpdate::withReason('')]); + $this->dbUpdater->expects($this->never())->method('databaseFileExists'); + + $result = $this->geolocationDbUpdater()->checkDbUpdate(); + + self::assertEquals(GeolocationResult::UPDATE_IN_PROGRESS, $result); + } + + #[Test] + public function properResultIsReturnedIfMaxConsecutiveErrorsAreReached(): void + { + $this->repo->expects($this->once())->method('findBy')->willReturn([ + GeolocationDbUpdate::withReason('')->finishWithError(''), + GeolocationDbUpdate::withReason('')->finishWithError(''), + GeolocationDbUpdate::withReason('')->finishWithError(''), + ]); + $this->dbUpdater->expects($this->never())->method('databaseFileExists'); + + $result = $this->geolocationDbUpdater()->checkDbUpdate(); + + self::assertEquals(GeolocationResult::MAX_ERRORS_REACHED, $result); + } + #[Test] public function properResultIsReturnedWhenLicenseIsMissing(): void { @@ -66,7 +101,9 @@ public function properResultIsReturnedWhenLicenseIsMissing(): void $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->willThrowException( new MissingLicenseException(''), ); - $this->geoLiteDbReader->expects($this->never())->method('metadata'); + $this->repo->expects($this->once())->method('findBy')->willReturn([ + GeolocationDbUpdate::withReason('')->finishSuccessfully(), + ]); $result = $this->geolocationDbUpdater()->checkDbUpdate($this->progressHandler); @@ -74,16 +111,19 @@ public function properResultIsReturnedWhenLicenseIsMissing(): void self::assertEquals(GeolocationResult::LICENSE_MISSING, $result); } - #[Test] - public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(): void + #[Test, DataProvider('provideDbDoesNotExist')] + public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(Closure $setUp): void { $prev = new DbUpdateException(''); - $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(false); + $expectedReason = $setUp($this); + $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->with( $this->isInstanceOf(Closure::class), )->willThrowException($prev); - $this->geoLiteDbReader->expects($this->never())->method('metadata'); + $this->em->expects($this->once())->method('persist')->with($this->callback( + fn (GeolocationDbUpdate $newUpdate): bool => $newUpdate->reason === $expectedReason, + )); try { $this->geolocationDbUpdater()->checkDbUpdate($this->progressHandler); @@ -96,17 +136,31 @@ public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(): void } } + public static function provideDbDoesNotExist(): iterable + { + yield 'file does not exist' => [function (self $test): string { + $test->repo->expects($test->once())->method('findBy')->willReturn([ + GeolocationDbUpdate::withReason('')->finishSuccessfully(), + ]); + $test->dbUpdater->expects($test->once())->method('databaseFileExists')->willReturn(false); + return 'Geolocation db file does not exist'; + }]; + yield 'no attempts' => [function (self $test): string { + $test->repo->expects($test->once())->method('findBy')->willReturn([]); + $test->dbUpdater->expects($test->never())->method('databaseFileExists'); + return 'No download attempts tracked for this instance'; + }]; + } + #[Test, DataProvider('provideBigDays')] - public function exceptionIsThrownWhenOlderDbIsTooOldAndDownloadFails(int $days): void + public function exceptionIsThrownWhenOlderDbIsOldEnoughAndDownloadFails(int $days): void { $prev = new DbUpdateException(''); $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(true); $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->with( $this->isInstanceOf(Closure::class), )->willThrowException($prev); - $this->geoLiteDbReader->expects($this->once())->method('metadata')->with()->willReturn( - $this->buildMetaWithBuildEpoch(Chronos::now()->subDays($days)->getTimestamp()), - ); + $this->repo->expects($this->once())->method('findBy')->willReturn([self::createFinishedOldUpdate($days)]); try { $this->geolocationDbUpdater()->checkDbUpdate(); @@ -120,74 +174,109 @@ public function exceptionIsThrownWhenOlderDbIsTooOldAndDownloadFails(int $days): public static function provideBigDays(): iterable { - yield [36]; + yield [31]; yield [50]; yield [75]; yield [100]; } - #[Test, DataProvider('provideSmallDays')] - public function databaseIsNotUpdatedIfItIsNewEnough(string|int $buildEpoch): void + #[Test] + public function exceptionIsThrownWhenUnknownErrorHappens(): void + { + $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->with( + $this->isInstanceOf(Closure::class), + )->willThrowException(new RuntimeException('An error occurred')); + + $newUpdate = null; + $this->em->expects($this->once())->method('persist')->with($this->callback( + function (GeolocationDbUpdate $u) use (&$newUpdate): bool { + $newUpdate = $u; + return true; + }, + )); + + try { + $this->geolocationDbUpdater()->checkDbUpdate($this->progressHandler); + self::fail(); + } catch (Throwable) { + } + + self::assertTrue($this->progressHandler->beforeDownloadCalled); + self::assertNotNull($newUpdate); + self::assertTrue($newUpdate->isError()); + } + + #[Test, DataProvider('provideNotAldEnoughDays')] + public function databaseIsNotUpdatedIfItIsNewEnough(int $days): void { $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(true); $this->dbUpdater->expects($this->never())->method('downloadFreshCopy'); - $this->geoLiteDbReader->expects($this->once())->method('metadata')->with()->willReturn( - $this->buildMetaWithBuildEpoch($buildEpoch), - ); + $this->repo->expects($this->once())->method('findBy')->willReturn([self::createFinishedOldUpdate($days)]); $result = $this->geolocationDbUpdater()->checkDbUpdate(); self::assertEquals(GeolocationResult::DB_IS_UP_TO_DATE, $result); } - public static function provideSmallDays(): iterable + public static function provideNotAldEnoughDays(): iterable { - $generateParamsWithTimestamp = static function (int $days) { - $timestamp = Chronos::now()->subDays($days)->getTimestamp(); - return [$days % 2 === 0 ? $timestamp : (string) $timestamp]; - }; - - return array_map($generateParamsWithTimestamp, range(0, 34)); + return array_map(static fn (int $value) => [$value], range(0, 29)); } - #[Test] - public function exceptionIsThrownWhenCheckingExistingDatabaseWithInvalidBuildEpoch(): void - { - $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(true); - $this->dbUpdater->expects($this->never())->method('downloadFreshCopy'); - $this->geoLiteDbReader->expects($this->once())->method('metadata')->with()->willReturn( - $this->buildMetaWithBuildEpoch('invalid'), - ); + #[Test, DataProvider('provideUpdatesThatWillDownload')] + public function properResultIsReturnedWhenDownloadSucceeds( + array $updates, + GeolocationResult $expectedResult, + string $expectedReason, + ): void { + $this->repo->expects($this->once())->method('findBy')->willReturn($updates); + $this->dbUpdater->method('databaseFileExists')->willReturn(true); + $this->dbUpdater->expects($this->once())->method('downloadFreshCopy'); + $this->em->expects($this->once())->method('persist')->with($this->callback( + fn (GeolocationDbUpdate $newUpdate): bool => $newUpdate->reason === $expectedReason, + )); - $this->expectException(GeolocationDbUpdateFailedException::class); - $this->expectExceptionMessage( - 'Build epoch with value "invalid" from existing geolocation database, could not be parsed to integer.', - ); + $result = $this->geolocationDbUpdater()->checkDbUpdate(); - $this->geolocationDbUpdater()->checkDbUpdate(); + self::assertEquals($expectedResult, $result); } - private function buildMetaWithBuildEpoch(string|int $buildEpoch): Metadata + public static function provideUpdatesThatWillDownload(): iterable { - return new Metadata([ - 'binary_format_major_version' => '', - 'binary_format_minor_version' => '', - 'build_epoch' => $buildEpoch, - 'database_type' => '', - 'languages' => '', - 'description' => '', - 'ip_version' => '', - 'node_count' => 1, - 'record_size' => 4, - ]); + yield 'no updates' => [[], GeolocationResult::DB_CREATED, 'No download attempts tracked for this instance']; + yield 'old successful update' => [ + [self::createFinishedOldUpdate(days: 31)], + GeolocationResult::DB_UPDATED, + 'Last successful attempt is old enough', + ]; + yield 'not enough errors' => [ + [self::createFinishedOldUpdate(days: 3, successful: false)], + GeolocationResult::DB_UPDATED, + 'Max consecutive errors not reached', + ]; + } + + public static function createFinishedOldUpdate(int $days, bool $successful = true): GeolocationDbUpdate + { + Chronos::setTestNow(Chronos::now()->subDays($days)); + $update = GeolocationDbUpdate::withReason(''); + if ($successful) { + $update->finishSuccessfully(); + } else { + $update->finishWithError(''); + } + Chronos::setTestNow(); + + return $update; } #[Test, DataProvider('provideTrackingOptions')] public function downloadDbIsSkippedIfTrackingIsDisabled(TrackingOptions $options): void { - $result = $this->geolocationDbUpdater($options)->checkDbUpdate(); $this->dbUpdater->expects($this->never())->method('databaseFileExists'); - $this->geoLiteDbReader->expects($this->never())->method('metadata'); + $this->em->expects($this->never())->method('getRepository'); + + $result = $this->geolocationDbUpdater($options)->checkDbUpdate(); self::assertEquals(GeolocationResult::CHECK_SKIPPED, $result); } @@ -204,11 +293,6 @@ private function geolocationDbUpdater(TrackingOptions|null $options = null): Geo $locker = $this->createMock(Lock\LockFactory::class); $locker->method('createLock')->with($this->isType('string'))->willReturn($this->lock); - return new GeolocationDbUpdater( - $this->dbUpdater, - fn () => $this->geoLiteDbReader, - $locker, - $options ?? new TrackingOptions(), - ); + return new GeolocationDbUpdater($this->dbUpdater, $locker, $options ?? new TrackingOptions(), $this->em, 3); } } From e80af78e097f319272f0896dcf15d1ee44dce2dc Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 17 Dec 2024 18:00:02 +0100 Subject: [PATCH 22/30] Be less restrictive on what characters are disallowed in custom slugs --- CHANGELOG.md | 4 ++++ .../src/ShortUrl/Model/Validation/CustomSlugValidator.php | 6 +++--- .../ShortUrl/Model/Validation/CustomSlugValidatorTest.php | 6 ++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb43ec7ee..241b734ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ 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 +* [#2156](https://github.com/shlinkio/shlink/issues/2156) Be less restrictive on what characters are disallowed in custom slugs. + + All [URI-reserved characters](https://datatracker.ietf.org/doc/html/rfc3986#section-2.2) were disallowed up until now, but from now on, only the gen-delimiters are. + ### Changed * [#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. diff --git a/module/Core/src/ShortUrl/Model/Validation/CustomSlugValidator.php b/module/Core/src/ShortUrl/Model/Validation/CustomSlugValidator.php index f33416983..3d3e7792f 100644 --- a/module/Core/src/ShortUrl/Model/Validation/CustomSlugValidator.php +++ b/module/Core/src/ShortUrl/Model/Validation/CustomSlugValidator.php @@ -46,10 +46,10 @@ public function isValid(mixed $value): bool return false; } - // URL reserved characters: https://datatracker.ietf.org/doc/html/rfc3986#section-2.2 - $reservedChars = "!*'();:@&=+$,?%#[]"; + // URL gen-delimiter reserved characters, except `/`: https://datatracker.ietf.org/doc/html/rfc3986#section-2.2 + $reservedChars = ':?#[]@'; if (! $this->options->multiSegmentSlugsEnabled) { - // Slashes should be allowed for multi-segment slugs + // Slashes should only be allowed if multi-segment slugs are enabled $reservedChars .= '/'; } diff --git a/module/Core/test/ShortUrl/Model/Validation/CustomSlugValidatorTest.php b/module/Core/test/ShortUrl/Model/Validation/CustomSlugValidatorTest.php index 86f695c7d..f763b44e2 100644 --- a/module/Core/test/ShortUrl/Model/Validation/CustomSlugValidatorTest.php +++ b/module/Core/test/ShortUrl/Model/Validation/CustomSlugValidatorTest.php @@ -59,13 +59,11 @@ public function valuesWithReservedCharsAreInvalid(string $value): void public static function provideInvalidValues(): iterable { + yield ['port:8080']; yield ['foo?bar=baz']; yield ['some-thing#foo']; - yield ['call()']; - yield ['array[]']; + yield ['brackets[]']; yield ['email@example.com']; - yield ['wildcard*']; - yield ['$500']; } public function createValidator(bool $multiSegmentSlugsEnabled = false): CustomSlugValidator From 6ad8b03850b9b7149d856ddd40065a1f7345f067 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 18 Dec 2024 08:53:28 +0100 Subject: [PATCH 23/30] Allow QR code logo to be individually disabled --- CHANGELOG.md | 2 ++ docs/swagger/paths/{shortCode}_qr-code.json | 10 ++++++++++ module/Core/src/Action/Model/QrCodeParams.php | 18 ++++++++++-------- module/Core/src/Action/QrCodeAction.php | 2 +- module/Core/test/Action/QrCodeActionTest.php | 16 +++++++++++----- 5 files changed, 34 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 241b734ed..25c508a8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this All [URI-reserved characters](https://datatracker.ietf.org/doc/html/rfc3986#section-2.2) were disallowed up until now, but from now on, only the gen-delimiters are. +* [#2229](https://github.com/shlinkio/shlink/issues/2229) Add `logo=disabled` query param to dynamically disable the default logo on QR codes. + ### Changed * [#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. diff --git a/docs/swagger/paths/{shortCode}_qr-code.json b/docs/swagger/paths/{shortCode}_qr-code.json index 39be2dc9e..bb95f7ef1 100644 --- a/docs/swagger/paths/{shortCode}_qr-code.json +++ b/docs/swagger/paths/{shortCode}_qr-code.json @@ -85,6 +85,16 @@ "type": "string", "default": "#ffffff" } + }, + { + "name": "logo", + "in": "query", + "description": "Currently used to disable the logo that was set via configuration options. It may be used in future to dynamically choose from multiple logos.", + "required": false, + "schema": { + "type": "string", + "enum": ["disable"] + } } ], "responses": { diff --git a/module/Core/src/Action/Model/QrCodeParams.php b/module/Core/src/Action/Model/QrCodeParams.php index 8bc5b317e..459c99b7d 100644 --- a/module/Core/src/Action/Model/QrCodeParams.php +++ b/module/Core/src/Action/Model/QrCodeParams.php @@ -28,20 +28,21 @@ use const Shlinkio\Shlink\DEFAULT_QR_CODE_BG_COLOR; use const Shlinkio\Shlink\DEFAULT_QR_CODE_COLOR; -final class QrCodeParams +final readonly class QrCodeParams { private const int MIN_SIZE = 50; private const int MAX_SIZE = 1000; private const array SUPPORTED_FORMATS = ['png', 'svg']; private function __construct( - public readonly int $size, - public readonly int $margin, - public readonly WriterInterface $writer, - public readonly ErrorCorrectionLevel $errorCorrectionLevel, - public readonly RoundBlockSizeMode $roundBlockSizeMode, - public readonly ColorInterface $color, - public readonly ColorInterface $bgColor, + public int $size, + public int $margin, + public WriterInterface $writer, + public ErrorCorrectionLevel $errorCorrectionLevel, + public RoundBlockSizeMode $roundBlockSizeMode, + public ColorInterface $color, + public ColorInterface $bgColor, + public bool $disableLogo, ) { } @@ -57,6 +58,7 @@ public static function fromRequest(ServerRequestInterface $request, QrCodeOption roundBlockSizeMode: self::resolveRoundBlockSize($query, $defaults), color: self::resolveColor($query, $defaults), bgColor: self::resolveBackgroundColor($query, $defaults), + disableLogo: isset($query['logo']) && $query['logo'] === 'disable', ); } diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index 607fab50b..fbc83c218 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -60,7 +60,7 @@ public function process(Request $request, RequestHandlerInterface $handler): Res private function buildQrCode(Builder $qrCodeBuilder, QrCodeParams $params): ResultInterface { $logoUrl = $this->options->logoUrl; - if ($logoUrl === null) { + if ($logoUrl === null || $params->disableLogo) { return $qrCodeBuilder->build(); } diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 688badee1..9db42752a 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -9,6 +9,7 @@ use Laminas\Diactoros\ServerRequestFactory; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; @@ -294,11 +295,16 @@ public function qrCodeIsResolvedBasedOnOptions(bool $enabledForDisabledShortUrls } #[Test] - public function logoIsAddedToQrCodeIfOptionIsDefined(): void + #[TestWith([[], '4696E5'])] // The logo has Shlink's brand color + #[TestWith([['logo' => 'invalid'], '4696E5'])] // Invalid `logo` values are ignored. Default logo is still rendered + #[TestWith([['logo' => 'disable'], '000000'])] // No logo will be added if explicitly disabled + public function logoIsAddedToQrCodeIfOptionIsDefined(array $query, string $expectedColor): void { - $logoUrl = 'https://avatars.githubusercontent.com/u/20341790?v=4'; // Shlink logo + $logoUrl = 'https://avatars.githubusercontent.com/u/20341790?v=4'; // Shlink's logo $code = 'abc123'; - $req = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $code); + $req = ServerRequestFactory::fromGlobals() + ->withAttribute('shortCode', $code) + ->withQueryParams($query); $this->urlResolver->method('resolveEnabledShortUrl')->with( ShortUrlIdentifier::fromShortCodeAndDomain($code), @@ -309,9 +315,9 @@ public function logoIsAddedToQrCodeIfOptionIsDefined(): void $image = imagecreatefromstring($resp->getBody()->__toString()); self::assertNotFalse($image); - // At around 100x100 px we can already find the logo, which has Shlink's brand color + // At around 100x100 px we can already find the logo, if present $resultingColor = imagecolorat($image, 100, 100); - self::assertEquals(hexdec('4696E5'), $resultingColor); + self::assertEquals(hexdec($expectedColor), $resultingColor); } public static function provideEnabled(): iterable From 4e7d09035a07e8b2768dbb936c5200803d4cbbe1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 18 Dec 2024 09:47:21 +0100 Subject: [PATCH 24/30] Support encrypted connections to MySQL/Maria and Postgres --- config/autoload/entity-manager.global.php | 30 +++++++++++++++-------- module/Core/src/Config/EnvVars.php | 2 ++ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/config/autoload/entity-manager.global.php b/config/autoload/entity-manager.global.php index 4338a8b67..1bd3db444 100644 --- a/config/autoload/entity-manager.global.php +++ b/config/autoload/entity-manager.global.php @@ -12,9 +12,10 @@ return (static function (): array { $driver = EnvVars::DB_DRIVER->loadFromEnv(); + $useEncryption = (bool) EnvVars::DB_USE_ENCRYPTION->loadFromEnv(); $isMysqlCompatible = contains($driver, ['maria', 'mysql']); - $resolveDriver = static fn () => match ($driver) { + $doctrineDriver = match ($driver) { 'postgres' => 'pdo_pgsql', 'mssql' => 'pdo_sqlsrv', default => 'pdo_mysql', @@ -23,31 +24,40 @@ $value = $envVar->loadFromEnv(); return $value === null ? null : (string) $value; }; - $resolveCharset = static fn () => match ($driver) { + $charset = match ($driver) { // This does not determine charsets or collations in tables or columns, but the charset used in the data // flowing in the connection, so it has to match what has been set in the database. 'maria', 'mysql' => 'utf8mb4', 'postgres' => 'utf8', default => null, }; - - $resolveConnection = static fn () => match ($driver) { + $driverOptions = match ($driver) { + 'mssql' => ['TrustServerCertificate' => 'true'], + 'maria', 'mysql' => ! $useEncryption ? [] : [ + 1007 => true, // PDO::MYSQL_ATTR_SSL_KEY: Require using SSL + 1014 => false, // PDO::MYSQL_ATTR_SSL_VERIFY_SERVER_CERT: Trust any certificate + ], + 'postgres' => ! $useEncryption ? [] : [ + 'sslmode' => 'require', // Require connections to be encrypted + 'sslrootcert' => '', // Allow any certificate + ], + default => [], + }; + $connection = match ($driver) { null, 'sqlite' => [ 'driver' => 'pdo_sqlite', 'path' => 'data/database.sqlite', ], default => [ - 'driver' => $resolveDriver(), + 'driver' => $doctrineDriver, 'dbname' => EnvVars::DB_NAME->loadFromEnv(), 'user' => $readCredentialAsString(EnvVars::DB_USER), 'password' => $readCredentialAsString(EnvVars::DB_PASSWORD), 'host' => EnvVars::DB_HOST->loadFromEnv(), 'port' => EnvVars::DB_PORT->loadFromEnv(), 'unix_socket' => $isMysqlCompatible ? EnvVars::DB_UNIX_SOCKET->loadFromEnv() : null, - 'charset' => $resolveCharset(), - 'driverOptions' => $driver !== 'mssql' ? [] : [ - 'TrustServerCertificate' => 'true', - ], + 'charset' => $charset, + 'driverOptions' => $driverOptions, ], }; @@ -63,7 +73,7 @@ Events::postFlush => [ShortUrlVisitsCountTracker::class, OrphanVisitsCountTracker::class], ], ], - 'connection' => $resolveConnection(), + 'connection' => $connection, ], ]; diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index e2a6d38ad..f8042febd 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -36,6 +36,7 @@ enum EnvVars: string case DB_HOST = 'DB_HOST'; case DB_UNIX_SOCKET = 'DB_UNIX_SOCKET'; case DB_PORT = 'DB_PORT'; + case DB_USE_ENCRYPTION = 'DB_USE_ENCRYPTION'; case GEOLITE_LICENSE_KEY = 'GEOLITE_LICENSE_KEY'; case CACHE_NAMESPACE = 'CACHE_NAMESPACE'; case REDIS_SERVERS = 'REDIS_SERVERS'; @@ -147,6 +148,7 @@ private function defaultValue(): string|int|bool|null 'mssql' => '1433', default => '3306', }, + self::DB_USE_ENCRYPTION => false, self::MERCURE_INTERNAL_HUB_URL => self::MERCURE_PUBLIC_HUB_URL->loadFromEnv(), From c34bfac6b112b146ee88ddedd8ec42a18c62c3c4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 20 Dec 2024 09:29:28 +0100 Subject: [PATCH 25/30] Update installer with support for DB_USE_ENCRYPTION option --- CHANGELOG.md | 1 + composer.json | 2 +- config/autoload/installer.global.php | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25c508a8b..b3cdeab8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this All [URI-reserved characters](https://datatracker.ietf.org/doc/html/rfc3986#section-2.2) were disallowed up until now, but from now on, only the gen-delimiters are. * [#2229](https://github.com/shlinkio/shlink/issues/2229) Add `logo=disabled` query param to dynamically disable the default logo on QR codes. +* [#2206](https://github.com/shlinkio/shlink/issues/2206) Add new `DB_USE_ENCRYPTION` config option to enable SSL database connections trusting any server certificate. ### Changed * [#2281](https://github.com/shlinkio/shlink/issues/2281) Update docker image to PHP 8.4 diff --git a/composer.json b/composer.json index 9fe15aeac..aa1a1e526 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,7 @@ "shlinkio/shlink-config": "^3.4", "shlinkio/shlink-event-dispatcher": "^4.1", "shlinkio/shlink-importer": "^5.3.2", - "shlinkio/shlink-installer": "dev-develop#957db97 as 9.4", + "shlinkio/shlink-installer": "dev-develop#3675f6d as 9.4", "shlinkio/shlink-ip-geolocation": "^4.2", "shlinkio/shlink-json": "^1.1", "spiral/roadrunner": "^2024.1", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index e239dd3a1..24b9263e4 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -20,6 +20,7 @@ Option\Database\DatabaseUserConfigOption::class, Option\Database\DatabasePasswordConfigOption::class, Option\Database\DatabaseUnixSocketConfigOption::class, + Option\Database\DatabaseUseEncryptionConfigOption::class, Option\UrlShortener\ShortDomainHostConfigOption::class, Option\UrlShortener\ShortDomainSchemaConfigOption::class, Option\Redirect\BaseUrlRedirectConfigOption::class, From d228c16f8260d8d93535cccae212b640b4fba720 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 20 Dec 2024 09:52:30 +0100 Subject: [PATCH 26/30] Fix test for ip middleware --- composer.json | 2 +- module/Core/test-api/Action/RedirectTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index aa1a1e526..ca6cab077 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,7 @@ "ext-json": "*", "ext-mbstring": "*", "ext-pdo": "*", - "akrabat/ip-address-middleware": "^2.4", + "akrabat/ip-address-middleware": "^2.5", "cakephp/chronos": "^3.1", "doctrine/dbal": "^4.2", "doctrine/migrations": "^3.8", diff --git a/module/Core/test-api/Action/RedirectTest.php b/module/Core/test-api/Action/RedirectTest.php index 79a13fbf4..2cfe9417a 100644 --- a/module/Core/test-api/Action/RedirectTest.php +++ b/module/Core/test-api/Action/RedirectTest.php @@ -93,7 +93,7 @@ public static function provideRequestOptions(): iterable foreach ($ipAddressConfig['rka']['ip_address']['headers_to_inspect'] as $header) { yield sprintf('rule: IP address in "%s" header', $header) => [ [ - RequestOptions::HEADERS => [$header => '1.2.3.4'], + RequestOptions::HEADERS => [$header => $header !== 'Forwarded' ? '1.2.3.4' : 'for=1.2.3.4'], ], 'https://example.com/static-ip-address', ]; From 2f39aff2fe84eaaaa317b5078ce4a4e0d3fca1a5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 Dec 2024 12:42:06 +0100 Subject: [PATCH 27/30] Implement logic to import redirect rules from other Shlink instances --- composer.json | 28 ++++++------- module/Core/config/dependencies.config.php | 1 + .../src/Importer/ImportedLinksProcessor.php | 3 ++ .../Core/src/Importer/ShortUrlImporting.php | 42 ++++++++++++++++++- .../RedirectRule/Entity/RedirectCondition.php | 18 ++++++++ .../ShortUrlRedirectRuleService.php | 8 ++-- .../ShortUrlRedirectRuleServiceInterface.php | 2 + module/Core/src/ShortUrl/Entity/ShortUrl.php | 1 - .../Importer/ImportedLinksProcessorTest.php | 6 ++- 9 files changed, 87 insertions(+), 22 deletions(-) diff --git a/composer.json b/composer.json index ca6cab077..15ba3b70e 100644 --- a/composer.json +++ b/composer.json @@ -26,15 +26,15 @@ "donatj/phpuseragentparser": "^1.10", "endroid/qr-code": "^6.0", "friendsofphp/proxy-manager-lts": "^1.0", - "geoip2/geoip2": "^3.0", + "geoip2/geoip2": "^3.1", "guzzlehttp/guzzle": "^7.9", "hidehalo/nanoid-php": "^2.0", "jaybizzle/crawler-detect": "^1.3", - "laminas/laminas-config-aggregator": "^1.15", + "laminas/laminas-config-aggregator": "^1.17", "laminas/laminas-diactoros": "^3.5", - "laminas/laminas-inputfilter": "^2.30", - "laminas/laminas-servicemanager": "^3.22", - "laminas/laminas-stdlib": "^3.19", + "laminas/laminas-inputfilter": "^2.31", + "laminas/laminas-servicemanager": "^3.23", + "laminas/laminas-stdlib": "^3.20", "matomo/matomo-php-tracker": "^3.3", "mezzio/mezzio": "^3.20", "mezzio/mezzio-fastroute": "^3.12", @@ -46,7 +46,7 @@ "shlinkio/shlink-common": "^6.6", "shlinkio/shlink-config": "^3.4", "shlinkio/shlink-event-dispatcher": "^4.1", - "shlinkio/shlink-importer": "^5.3.2", + "shlinkio/shlink-importer": "dev-main#6c305ee as 5.5", "shlinkio/shlink-installer": "dev-develop#3675f6d as 9.4", "shlinkio/shlink-ip-geolocation": "^4.2", "shlinkio/shlink-json": "^1.1", @@ -54,14 +54,14 @@ "spiral/roadrunner-cli": "^2.6", "spiral/roadrunner-http": "^3.5", "spiral/roadrunner-jobs": "^4.5", - "symfony/console": "^7.1", - "symfony/filesystem": "^7.1", - "symfony/lock": "^7.1", - "symfony/process": "^7.1", - "symfony/string": "^7.1" + "symfony/console": "^7.2", + "symfony/filesystem": "^7.2", + "symfony/lock": "^7.2", + "symfony/process": "^7.2", + "symfony/string": "^7.2" }, "require-dev": { - "devizzent/cebe-php-openapi": "^1.1.1", + "devizzent/cebe-php-openapi": "^1.1.2", "devster/ubench": "^2.1", "phpstan/phpstan": "^2.0", "phpstan/phpstan-doctrine": "^2.0", @@ -69,11 +69,11 @@ "phpstan/phpstan-symfony": "^2.0", "phpunit/php-code-coverage": "^11.0", "phpunit/phpcov": "^10.0", - "phpunit/phpunit": "^11.4", + "phpunit/phpunit": "^11.5", "roave/security-advisories": "dev-master", "shlinkio/php-coding-standard": "~2.4.0", "shlinkio/shlink-test-utils": "^4.2", - "symfony/var-dumper": "^7.1", + "symfony/var-dumper": "^7.2", "veewee/composer-run-parallel": "^1.4" }, "conflict": { diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index adc9ae2a3..eda556e9d 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -262,6 +262,7 @@ ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class, ShortUrl\Helper\ShortCodeUniquenessHelper::class, Util\DoctrineBatchHelper::class, + RedirectRule\ShortUrlRedirectRuleService::class, ], Crawling\CrawlingHelper::class => [ShortUrl\Repository\CrawlableShortCodesQuery::class], diff --git a/module/Core/src/Importer/ImportedLinksProcessor.php b/module/Core/src/Importer/ImportedLinksProcessor.php index e8434d4f6..af4ce9171 100644 --- a/module/Core/src/Importer/ImportedLinksProcessor.php +++ b/module/Core/src/Importer/ImportedLinksProcessor.php @@ -6,6 +6,7 @@ use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; +use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortCodeUniquenessHelperInterface; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository; @@ -32,6 +33,7 @@ public function __construct( private ShortUrlRelationResolverInterface $relationResolver, private ShortCodeUniquenessHelperInterface $shortCodeHelper, private DoctrineBatchHelperInterface $batchHelper, + private ShortUrlRedirectRuleServiceInterface $redirectRuleService, ) { } @@ -80,6 +82,7 @@ private function importShortUrls(StyleInterface $io, iterable $shlinkUrls, Impor continue; } + $shortUrlImporting->importRedirectRules($importedUrl->redirectRules, $this->em, $this->redirectRuleService); $resultMessage = $shortUrlImporting->importVisits( $this->batchHelper->wrapIterable($importedUrl->visits, 100), $this->em, diff --git a/module/Core/src/Importer/ShortUrlImporting.php b/module/Core/src/Importer/ShortUrlImporting.php index ad812e8c8..cc534fc23 100644 --- a/module/Core/src/Importer/ShortUrlImporting.php +++ b/module/Core/src/Importer/ShortUrlImporting.php @@ -4,11 +4,17 @@ namespace Shlinkio\Shlink\Core\Importer; +use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; +use Shlinkio\Shlink\Core\RedirectRule\Entity\RedirectCondition; +use Shlinkio\Shlink\Core\RedirectRule\Entity\ShortUrlRedirectRule; +use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\Visit\Entity\Visit; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectRule; use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisit; +use function Shlinkio\Shlink\Core\ArrayUtils\map; use function Shlinkio\Shlink\Core\normalizeDate; use function sprintf; @@ -20,12 +26,12 @@ private function __construct(private ShortUrl $shortUrl, private bool $isNew) public static function fromExistingShortUrl(ShortUrl $shortUrl): self { - return new self($shortUrl, false); + return new self($shortUrl, isNew: false); } public static function fromNewShortUrl(ShortUrl $shortUrl): self { - return new self($shortUrl, true); + return new self($shortUrl, isNew: true); } /** @@ -55,6 +61,38 @@ public function importVisits(iterable $visits, EntityManagerInterface $em): stri : sprintf('Skipped. Imported %s visits', $importedVisits); } + /** + * @param ImportedShlinkRedirectRule[] $rules + */ + public function importRedirectRules( + array $rules, + EntityManagerInterface $em, + ShortUrlRedirectRuleServiceInterface $redirectRuleService, + ): void { + $shortUrl = $this->resolveShortUrl($em); + $redirectRules = map( + $rules, + function (ImportedShlinkRedirectRule $rule, int|string|float $index) use ($shortUrl): ShortUrlRedirectRule { + $conditions = new ArrayCollection(); + foreach ($rule->conditions as $cond) { + $redirectCondition = RedirectCondition::fromImport($cond); + if ($redirectCondition !== null) { + $conditions->add($redirectCondition); + } + } + + return new ShortUrlRedirectRule( + shortUrl: $shortUrl, + priority: ((int) $index) + 1, + longUrl:$rule->longUrl, + conditions: $conditions, + ); + }, + ); + + $redirectRuleService->saveRulesForShortUrl($shortUrl, $redirectRules); + } + private function resolveShortUrl(EntityManagerInterface $em): ShortUrl { // If wrapped ShortUrl has no ID, avoid trying to query the EM, as it would fail in Postgres. diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index cf1e134b7..602d07b7f 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -9,6 +9,7 @@ use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; use Shlinkio\Shlink\Core\RedirectRule\Model\Validation\RedirectRulesInputFilter; use Shlinkio\Shlink\Core\Util\IpAddressUtils; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectCondition; use function Shlinkio\Shlink\Core\acceptLanguageToLocales; use function Shlinkio\Shlink\Core\ArrayUtils\some; @@ -72,6 +73,23 @@ public static function fromRawData(array $rawData): self return new self($type, $value, $key); } + public static function fromImport(ImportedShlinkRedirectCondition $cond): self|null + { + $type = RedirectConditionType::tryFrom($cond->type); + if ($type === null) { + return null; + } + + return match ($type) { + RedirectConditionType::QUERY_PARAM => self::forQueryParam($cond->matchKey ?? '', $cond->matchValue), + RedirectConditionType::LANGUAGE => self::forLanguage($cond->matchValue), + RedirectConditionType::DEVICE => self::forDevice(DeviceType::from($cond->matchValue)), + RedirectConditionType::IP_ADDRESS => self::forIpAddress($cond->matchValue), + RedirectConditionType::GEOLOCATION_COUNTRY_CODE => self::forGeolocationCountryCode($cond->matchValue), + RedirectConditionType::GEOLOCATION_CITY_NAME => self::forGeolocationCityName($cond->matchValue), + }; + } + /** * Tells if this condition matches provided request */ diff --git a/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php b/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php index 01ba0a8f0..dac0dc618 100644 --- a/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php +++ b/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php @@ -20,7 +20,7 @@ public function __construct(private EntityManagerInterface $em) } /** - * @return ShortUrlRedirectRule[] + * @inheritDoc */ public function rulesForShortUrl(ShortUrl $shortUrl): array { @@ -31,7 +31,7 @@ public function rulesForShortUrl(ShortUrl $shortUrl): array } /** - * @return ShortUrlRedirectRule[] + * @inheritDoc */ public function setRulesForShortUrl(ShortUrl $shortUrl, RedirectRulesData $data): array { @@ -55,7 +55,7 @@ public function setRulesForShortUrl(ShortUrl $shortUrl, RedirectRulesData $data) } /** - * @param ShortUrlRedirectRule[] $rules + * @inheritDoc */ public function saveRulesForShortUrl(ShortUrl $shortUrl, array $rules): void { @@ -74,7 +74,7 @@ public function saveRulesForShortUrl(ShortUrl $shortUrl, array $rules): void /** * @param ShortUrlRedirectRule[] $rules */ - public function doSetRulesForShortUrl(ShortUrl $shortUrl, array $rules): void + private function doSetRulesForShortUrl(ShortUrl $shortUrl, array $rules): void { $this->em->wrapInTransaction(function () use ($shortUrl, $rules): void { // First, delete existing rules for the short URL diff --git a/module/Core/src/RedirectRule/ShortUrlRedirectRuleServiceInterface.php b/module/Core/src/RedirectRule/ShortUrlRedirectRuleServiceInterface.php index 186be87e9..e05c6ab84 100644 --- a/module/Core/src/RedirectRule/ShortUrlRedirectRuleServiceInterface.php +++ b/module/Core/src/RedirectRule/ShortUrlRedirectRuleServiceInterface.php @@ -14,11 +14,13 @@ interface ShortUrlRedirectRuleServiceInterface public function rulesForShortUrl(ShortUrl $shortUrl): array; /** + * Resolve a set of redirect rules and attach them to a short URL, replacing any already existing rules. * @return ShortUrlRedirectRule[] */ public function setRulesForShortUrl(ShortUrl $shortUrl, RedirectRulesData $data): array; /** + * Save provided set of rules for a short URL, replacing any already existing rules. * @param ShortUrlRedirectRule[] $rules */ public function saveRulesForShortUrl(ShortUrl $shortUrl, array $rules): void; diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index b7fb6c567..086a47bbb 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -76,7 +76,6 @@ public static function createFake(): self /** * @param non-empty-string $longUrl - * @internal */ public static function withLongUrl(string $longUrl): self { diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index 36265aa37..8fb63e683 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -14,6 +14,7 @@ use PHPUnit\Framework\TestCase; use RuntimeException; use Shlinkio\Shlink\Core\Importer\ImportedLinksProcessor; +use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortCodeUniquenessHelperInterface; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository; @@ -44,13 +45,15 @@ class ImportedLinksProcessorTest extends TestCase private MockObject & ShortCodeUniquenessHelperInterface $shortCodeHelper; private MockObject & ShortUrlRepository $repo; private MockObject & StyleInterface $io; + private MockObject & ShortUrlRedirectRuleServiceInterface $redirectRuleService; protected function setUp(): void { $this->em = $this->createMock(EntityManagerInterface::class); $this->repo = $this->createMock(ShortUrlRepository::class); - $this->shortCodeHelper = $this->createMock(ShortCodeUniquenessHelperInterface::class); + $this->redirectRuleService = $this->createMock(ShortUrlRedirectRuleServiceInterface::class); + $batchHelper = $this->createMock(DoctrineBatchHelperInterface::class); $batchHelper->method('wrapIterable')->willReturnArgument(0); @@ -59,6 +62,7 @@ protected function setUp(): void new SimpleShortUrlRelationResolver(), $this->shortCodeHelper, $batchHelper, + $this->redirectRuleService, ); $this->io = $this->createMock(StyleInterface::class); From 2807b9ce2fedd0150c0c68c4ca5117d6f10f75c8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 Dec 2024 18:24:56 +0100 Subject: [PATCH 28/30] Fix ImportedLinksProcessorTest --- .../ShortUrl/DeleteShortUrlCommandTest.php | 2 +- .../Command/Visit/LocateVisitsCommandTest.php | 2 +- .../RedirectRule/RedirectRuleHandlerTest.php | 2 +- .../Core/src/Importer/ShortUrlImporting.php | 5 ++ .../RedirectRule/Entity/RedirectCondition.php | 2 +- .../Geolocation/GeolocationDbUpdaterTest.php | 2 +- .../Importer/ImportedLinksProcessorTest.php | 49 +++++++++++++++++-- .../Entity/RedirectConditionTest.php | 20 ++++++++ ...ersistenceShortUrlRelationResolverTest.php | 6 +-- .../Core/test/ShortUrl/UrlShortenerTest.php | 2 +- .../Rest/test/Service/ApiKeyServiceTest.php | 4 +- 11 files changed, 80 insertions(+), 16 deletions(-) diff --git a/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php index 0402dc8c1..6ffec859a 100644 --- a/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php @@ -74,7 +74,7 @@ public function deleteIsRetriedWhenThresholdIsReachedAndQuestionIsAccepted( $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($shortCode); $this->service->expects($this->exactly($expectedDeleteCalls))->method('deleteByShortCode')->with( $identifier, - $this->isType('bool'), + $this->isBool(), )->willReturnCallback(function ($_, bool $ignoreThreshold) use ($shortCode): void { if (!$ignoreThreshold) { throw Exception\DeleteShortUrlException::fromVisitsThreshold( diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php index 0f24a6034..9b35324aa 100644 --- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php @@ -47,7 +47,7 @@ protected function setUp(): void $locker = $this->createMock(Lock\LockFactory::class); $this->lock = $this->createMock(Lock\SharedLockInterface::class); - $locker->method('createLock')->with($this->isType('string'), 600.0, false)->willReturn($this->lock); + $locker->method('createLock')->with($this->isString(), 600.0, false)->willReturn($this->lock); $command = new LocateVisitsCommand($this->visitService, $this->visitToLocation, $locker); diff --git a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php index eb78da613..a78110607 100644 --- a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php +++ b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php @@ -77,7 +77,7 @@ public function rulesAreDisplayedWhenRulesListIsEmpty( $this->io->expects($this->once())->method('choice')->willReturn($action->value); $this->io->expects($this->never())->method('newLine'); $this->io->expects($this->never())->method('text'); - $this->io->expects($this->once())->method('table')->with($this->isType('array'), [ + $this->io->expects($this->once())->method('table')->with($this->isArray(), [ ['1', $comment($this->cond1->toHumanFriendly()), 'https://example.com/one'], [ '2', diff --git a/module/Core/src/Importer/ShortUrlImporting.php b/module/Core/src/Importer/ShortUrlImporting.php index cc534fc23..aeb24ccea 100644 --- a/module/Core/src/Importer/ShortUrlImporting.php +++ b/module/Core/src/Importer/ShortUrlImporting.php @@ -14,6 +14,7 @@ use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectRule; use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisit; +use function count; use function Shlinkio\Shlink\Core\ArrayUtils\map; use function Shlinkio\Shlink\Core\normalizeDate; use function sprintf; @@ -69,6 +70,10 @@ public function importRedirectRules( EntityManagerInterface $em, ShortUrlRedirectRuleServiceInterface $redirectRuleService, ): void { + if ($this->isNew && count($rules) === 0) { + return; + } + $shortUrl = $this->resolveShortUrl($em); $redirectRules = map( $rules, diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index 602d07b7f..255cb19ea 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -24,7 +24,7 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable { private function __construct( - private readonly RedirectConditionType $type, + public readonly RedirectConditionType $type, private readonly string $matchValue, private readonly string|null $matchKey = null, ) { diff --git a/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php index c29830307..8c9f5a1bd 100644 --- a/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php +++ b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php @@ -291,7 +291,7 @@ public static function provideTrackingOptions(): iterable private function geolocationDbUpdater(TrackingOptions|null $options = null): GeolocationDbUpdater { $locker = $this->createMock(Lock\LockFactory::class); - $locker->method('createLock')->with($this->isType('string'))->willReturn($this->lock); + $locker->method('createLock')->with($this->isString())->willReturn($this->lock); return new GeolocationDbUpdater($this->dbUpdater, $locker, $options ?? new TrackingOptions(), $this->em, 3); } diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index 8fb63e683..e8d800a1e 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -14,6 +14,9 @@ use PHPUnit\Framework\TestCase; use RuntimeException; use Shlinkio\Shlink\Core\Importer\ImportedLinksProcessor; +use Shlinkio\Shlink\Core\Model\DeviceType; +use Shlinkio\Shlink\Core\RedirectRule\Entity\ShortUrlRedirectRule; +use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortCodeUniquenessHelperInterface; @@ -24,6 +27,8 @@ use Shlinkio\Shlink\Core\Visit\Model\Visitor; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepository; use Shlinkio\Shlink\Importer\Model\ImportedShlinkOrphanVisit; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectCondition; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectRule; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisit; use Shlinkio\Shlink\Importer\Model\ImportResult; @@ -71,10 +76,31 @@ protected function setUp(): void #[Test] public function newUrlsWithNoErrorsAreAllPersisted(): void { + $now = Chronos::now(); $urls = [ - new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], Chronos::now(), null, 'foo', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'https://bar', [], Chronos::now(), null, 'bar', 'foo'), - new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz', [], Chronos::now(), null, 'baz', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], $now, null, 'foo', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://bar', [], $now, null, 'bar', 'foo'), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz', [], $now, null, 'baz', null, redirectRules: [ + new ImportedShlinkRedirectRule( + longUrl: 'https://example.com/android', + conditions: [ + new ImportedShlinkRedirectCondition( + RedirectConditionType::DEVICE->value, + DeviceType::ANDROID->value, + ), + ], + ), + new ImportedShlinkRedirectRule( + longUrl: 'https://example.com/spain', + conditions: [ + new ImportedShlinkRedirectCondition( + RedirectConditionType::GEOLOCATION_COUNTRY_CODE->value, + 'ES', + ), + new ImportedShlinkRedirectCondition(RedirectConditionType::LANGUAGE->value, 'es-ES'), + ], + ), + ]), ]; $expectedCalls = count($urls); @@ -86,7 +112,19 @@ public function newUrlsWithNoErrorsAreAllPersisted(): void $this->em->expects($this->exactly($expectedCalls))->method('persist')->with( $this->isInstanceOf(ShortUrl::class), ); - $this->io->expects($this->exactly($expectedCalls))->method('text')->with($this->isType('string')); + $this->io->expects($this->exactly($expectedCalls))->method('text')->with($this->isString()); + $this->redirectRuleService->expects($this->once())->method('saveRulesForShortUrl')->with( + $this->isInstanceOf(ShortUrl::class), + $this->callback(function (array $rules): bool { + Assert::assertCount(2, $rules); + Assert::assertInstanceOf(ShortUrlRedirectRule::class, $rules[0]); + Assert::assertInstanceOf(ShortUrlRedirectRule::class, $rules[1]); + Assert::assertCount(1, $rules[0]->mapConditions(fn ($c) => $c)); + Assert::assertCount(2, $rules[1]->mapConditions(fn ($c) => $c)); + + return true; + }), + ); $this->processor->process($this->io, ImportResult::withShortUrls($urls), $this->buildParams()); } @@ -243,7 +281,8 @@ public function visitsArePersistedWithProperShortUrl(ShortUrl $originalShortUrl, if (!$originalShortUrl->getId()) { $this->em->expects($this->never())->method('find'); } else { - $this->em->expects($this->exactly(2))->method('find')->willReturn($foundShortUrl); + // 3 times: Initial short URL checking, before creating redirect rules, before creating visits + $this->em->expects($this->exactly(3))->method('find')->willReturn($foundShortUrl); } $this->em->expects($this->once())->method('persist')->willReturnCallback( static fn (Visit $visit) => Assert::assertSame( diff --git a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php index 5a4a2e2be..bec6d2635 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -9,6 +9,8 @@ use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Entity\RedirectCondition; +use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectCondition; use Shlinkio\Shlink\IpGeolocation\Model\Location; use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE; @@ -133,4 +135,22 @@ public static function provideVisitsWithCity(): iterable yield 'matching location' => [new Location(city: 'Madrid'), 'Madrid', true]; yield 'matching case-insensitive' => [new Location(city: 'Los Angeles'), 'los angeles', true]; } + + #[Test] + #[TestWith(['invalid', null])] + #[TestWith([RedirectConditionType::DEVICE->value, RedirectConditionType::DEVICE])] + #[TestWith([RedirectConditionType::LANGUAGE->value, RedirectConditionType::LANGUAGE])] + #[TestWith([RedirectConditionType::QUERY_PARAM->value, RedirectConditionType::QUERY_PARAM])] + #[TestWith([RedirectConditionType::IP_ADDRESS->value, RedirectConditionType::IP_ADDRESS])] + #[TestWith( + [RedirectConditionType::GEOLOCATION_COUNTRY_CODE->value, RedirectConditionType::GEOLOCATION_COUNTRY_CODE], + )] + #[TestWith([RedirectConditionType::GEOLOCATION_CITY_NAME->value, RedirectConditionType::GEOLOCATION_CITY_NAME])] + public function canBeCreatedFromImport(string $type, RedirectConditionType|null $expectedType): void + { + $condition = RedirectCondition::fromImport( + new ImportedShlinkRedirectCondition($type, DeviceType::ANDROID->value, ''), + ); + self::assertEquals($expectedType, $condition?->type); + } } diff --git a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php index 934d85118..31f42bc76 100644 --- a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php @@ -78,7 +78,7 @@ public function findsAndPersistsTagsWrappedIntoCollection(array $tags, array $ex $tagRepo = $this->createMock(TagRepository::class); $tagRepo->expects($this->exactly($expectedLookedOutTags))->method('findOneBy')->with( - $this->isType('array'), + $this->isArray(), )->willReturnCallback(function (array $criteria): Tag|null { ['name' => $name] = $criteria; return $name === 'foo' ? new Tag($name) : null; @@ -115,7 +115,7 @@ public function returnsEmptyCollectionWhenProvidingEmptyListOfTags(): void public function newDomainsAreMemoizedUntilStateIsCleared(): void { $repo = $this->createMock(DomainRepository::class); - $repo->expects($this->exactly(3))->method('findOneBy')->with($this->isType('array'))->willReturn(null); + $repo->expects($this->exactly(3))->method('findOneBy')->with($this->isArray())->willReturn(null); $this->em->method('getRepository')->with(Domain::class)->willReturn($repo); $authority = 'foo.com'; @@ -134,7 +134,7 @@ public function newDomainsAreMemoizedUntilStateIsCleared(): void public function newTagsAreMemoizedUntilStateIsCleared(): void { $tagRepo = $this->createMock(TagRepository::class); - $tagRepo->expects($this->exactly(6))->method('findOneBy')->with($this->isType('array'))->willReturn(null); + $tagRepo->expects($this->exactly(6))->method('findOneBy')->with($this->isArray())->willReturn(null); $this->em->method('getRepository')->with(Tag::class)->willReturn($tagRepo); $tags = ['foo', 'bar']; diff --git a/module/Core/test/ShortUrl/UrlShortenerTest.php b/module/Core/test/ShortUrl/UrlShortenerTest.php index a6cacc469..fb191e955 100644 --- a/module/Core/test/ShortUrl/UrlShortenerTest.php +++ b/module/Core/test/ShortUrl/UrlShortenerTest.php @@ -38,7 +38,7 @@ protected function setUp(): void // FIXME Should use the interface, but it doe snot define wrapInTransaction explicitly $this->em = $this->createMock(EntityManager::class); $this->em->method('persist')->willReturnCallback(fn (ShortUrl $shortUrl) => $shortUrl->setId('10')); - $this->em->method('wrapInTransaction')->with($this->isType('callable'))->willReturnCallback( + $this->em->method('wrapInTransaction')->with($this->isCallable())->willReturnCallback( fn (callable $callback) => $callback(), ); diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php index 81bec4ea8..a5b6cecbe 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -45,7 +45,7 @@ protected function setUp(): void public function apiKeyIsProperlyCreated(Chronos|null $date, string|null $name, array $roles): void { $this->repo->expects($this->once())->method('nameExists')->with( - ! empty($name) ? $name : $this->isType('string'), + ! empty($name) ? $name : $this->isString(), )->willReturn(false); $this->em->expects($this->once())->method('persist')->with($this->isInstanceOf(ApiKey::class)); @@ -83,7 +83,7 @@ public function autoGeneratedNameIsRegeneratedIfAlreadyExists(): void { $callCount = 0; $this->repo->expects($this->exactly(3))->method('nameExists')->with( - $this->isType('string'), + $this->isString(), )->willReturnCallback(function () use (&$callCount): bool { $callCount++; return $callCount < 3; From 9c251b364618b6a56656b8a5dbb27ea6ed514484 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 Dec 2024 18:41:58 +0100 Subject: [PATCH 29/30] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3cdeab8d..2f906363b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#2229](https://github.com/shlinkio/shlink/issues/2229) Add `logo=disabled` query param to dynamically disable the default logo on QR codes. * [#2206](https://github.com/shlinkio/shlink/issues/2206) Add new `DB_USE_ENCRYPTION` config option to enable SSL database connections trusting any server certificate. +* [#2209](https://github.com/shlinkio/shlink/issues/2209) Redirect rules are now imported when importing short URLs from a Shlink >=4.0 instance. ### Changed * [#2281](https://github.com/shlinkio/shlink/issues/2281) Update docker image to PHP 8.4 From d7e51b388e1916d4938b6767c771d8cb1c86643d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 27 Dec 2024 16:24:25 +0100 Subject: [PATCH 30/30] Add v4.4.0 to changelog and update dependencies --- CHANGELOG.md | 2 +- composer.json | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f906363b..309932c15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ 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). -# [Unreleased] +# [4.4.0] - 2024-12-27 ### Added * [#2265](https://github.com/shlinkio/shlink/issues/2265) Add a new `REDIRECT_EXTRA_PATH_MODE` option that accepts three values: diff --git a/composer.json b/composer.json index 15ba3b70e..f9133a5ce 100644 --- a/composer.json +++ b/composer.json @@ -42,18 +42,18 @@ "mlocati/ip-lib": "^1.18.1", "pagerfanta/core": "^3.8", "ramsey/uuid": "^4.7", - "shlinkio/doctrine-specification": "^2.1.1", + "shlinkio/doctrine-specification": "^2.2", "shlinkio/shlink-common": "^6.6", "shlinkio/shlink-config": "^3.4", "shlinkio/shlink-event-dispatcher": "^4.1", - "shlinkio/shlink-importer": "dev-main#6c305ee as 5.5", - "shlinkio/shlink-installer": "dev-develop#3675f6d as 9.4", + "shlinkio/shlink-importer": "^5.5", + "shlinkio/shlink-installer": "^9.4", "shlinkio/shlink-ip-geolocation": "^4.2", - "shlinkio/shlink-json": "^1.1", - "spiral/roadrunner": "^2024.1", + "shlinkio/shlink-json": "^1.2", + "spiral/roadrunner": "^2024.3", "spiral/roadrunner-cli": "^2.6", "spiral/roadrunner-http": "^3.5", - "spiral/roadrunner-jobs": "^4.5", + "spiral/roadrunner-jobs": "^4.6", "symfony/console": "^7.2", "symfony/filesystem": "^7.2", "symfony/lock": "^7.2",