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 2b2fb4ea..00000000 --- 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 c61a8a94..00000000 --- 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 49e6d9bb..00000000 --- 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 2277b0e5..6edd89e5 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 @@ return static function (ClassMetadata $metadata, array $emConfig): void { ->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 adceb7f2..3af587ef 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 68f38f18..fa287271 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 a4a15cdc..c63b2dff 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 d8ac53d5..19320ac3 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 4f3d1ca6..73e62b39 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 e594664b..3d9ea0ad 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 198a6867..f01943b0 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 19320ac3..fd63464d 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 fa287271..2138cefb 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 6cdf6297..e2a6d38a 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 @@ enum EnvVars: string 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 00000000..4904ca45 --- /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 4b013b33..f101774b 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\ServerRequestInterface; 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 @@ readonly class ExtraPathRedirectMiddleware implements MiddlewareInterface 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 @@ readonly class ExtraPathRedirectMiddleware implements MiddlewareInterface 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 84ceb790..0578c1f8 100644 --- a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php +++ b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php @@ -11,11 +11,13 @@ use Mezzio\Router\Route; 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 @@ class ExtraPathRedirectMiddlewareTest extends TestCase 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 @@ class ExtraPathRedirectMiddlewareTest extends TestCase ]; } - #[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 @@ class ExtraPathRedirectMiddlewareTest extends TestCase #[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 @@ class ExtraPathRedirectMiddlewareTest extends TestCase 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 @@ class ExtraPathRedirectMiddlewareTest extends TestCase $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 2138cefb..f6cfb17b 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 898e5ef5..e239dd3a 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -41,7 +41,7 @@ return [ 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 010c635f..cb7638be 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 c26aaaca..f0b7d847 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 443d34a9..a2783f97 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 9607206a..95692294 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 fd63464d..12b5aae4 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 681a9495..e061467d 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 f6cfb17b..bc81820e 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 c2ed4173..93178bb5 100644 --- a/module/CLI/src/Command/Api/DisableKeyCommand.php +++ b/module/CLI/src/Command/Api/DisableKeyCommand.php @@ -20,7 +20,7 @@ use function sprintf; 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 9fc0bb1d..2790fa8c 100644 --- a/module/CLI/src/Command/Api/GenerateKeyCommand.php +++ b/module/CLI/src/Command/Api/GenerateKeyCommand.php @@ -23,7 +23,7 @@ use function sprintf; 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 0f4945a9..e6515bc3 100644 --- a/module/CLI/src/Command/Api/InitialApiKeyCommand.php +++ b/module/CLI/src/Command/Api/InitialApiKeyCommand.php @@ -13,7 +13,7 @@ use Symfony\Component\Console\Output\OutputInterface; 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 d341389d..69870a9b 100644 --- a/module/CLI/src/Command/Api/ListKeysCommand.php +++ b/module/CLI/src/Command/Api/ListKeysCommand.php @@ -21,11 +21,11 @@ use function sprintf; 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 f7e24992..eb662539 100644 --- a/module/CLI/src/Command/Api/RenameApiKeyCommand.php +++ b/module/CLI/src/Command/Api/RenameApiKeyCommand.php @@ -19,7 +19,7 @@ use function Shlinkio\Shlink\Core\ArrayUtils\map; 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 76ec36ae..72e07f97 100644 --- a/module/CLI/src/Command/Config/ReadEnvVarCommand.php +++ b/module/CLI/src/Command/Config/ReadEnvVarCommand.php @@ -21,7 +21,7 @@ use function sprintf; 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 53b854d1..b2a2fa18 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 a912cf24..2e280a06 100644 --- a/module/CLI/src/Command/Db/MigrateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/MigrateDatabaseCommand.php @@ -11,9 +11,9 @@ use Symfony\Component\Console\Style\SymfonyStyle; 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 61e4a93b..105c10e3 100644 --- a/module/CLI/src/Command/Domain/DomainRedirectsCommand.php +++ b/module/CLI/src/Command/Domain/DomainRedirectsCommand.php @@ -21,7 +21,7 @@ use function str_contains; 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 6477539e..2891c44f 100644 --- a/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php +++ b/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php @@ -16,7 +16,7 @@ use Symfony\Component\Console\Input\InputInterface; 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 7e6b8cc3..79c181f7 100644 --- a/module/CLI/src/Command/Domain/ListDomainsCommand.php +++ b/module/CLI/src/Command/Domain/ListDomainsCommand.php @@ -18,7 +18,7 @@ use function array_map; 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 ba9a794e..9a41cc05 100644 --- a/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php +++ b/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php @@ -22,7 +22,7 @@ use function sprintf; 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 13b6d1cc..646b9d77 100644 --- a/module/CLI/src/Command/RedirectRule/ManageRedirectRulesCommand.php +++ b/module/CLI/src/Command/RedirectRule/ManageRedirectRulesCommand.php @@ -19,7 +19,7 @@ use function sprintf; 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 e3a9b180..df341c96 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -20,7 +20,7 @@ use function sprintf; 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 109beff7..1fc9dc38 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php @@ -17,7 +17,7 @@ use function sprintf; 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 63e9dab5..edda1b29 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php @@ -19,7 +19,7 @@ use function sprintf; 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 a720e12d..a9a709a1 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php @@ -16,7 +16,7 @@ use function sprintf; 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 048b3934..ad9aaf70 100644 --- a/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php @@ -19,7 +19,7 @@ use function sprintf; 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 8583a242..8507b9ca 100644 --- a/module/CLI/src/Command/ShortUrl/GetShortUrlVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/GetShortUrlVisitsCommand.php @@ -16,7 +16,7 @@ use Symfony\Component\Console\Style\SymfonyStyle; 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 fffeb1f6..72222a08 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -33,7 +33,7 @@ use function sprintf; 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 0a207b68..7935df6d 100644 --- a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php @@ -17,7 +17,7 @@ use function sprintf; 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 cf05f1b5..2f13e775 100644 --- a/module/CLI/src/Command/Tag/DeleteTagsCommand.php +++ b/module/CLI/src/Command/Tag/DeleteTagsCommand.php @@ -14,9 +14,9 @@ use Symfony\Component\Console\Style\SymfonyStyle; 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 18da41fb..b3c083bc 100644 --- a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php +++ b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php @@ -16,7 +16,7 @@ use Symfony\Component\Console\Input\InputInterface; 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 2efeac5c..8333b82e 100644 --- a/module/CLI/src/Command/Tag/ListTagsCommand.php +++ b/module/CLI/src/Command/Tag/ListTagsCommand.php @@ -17,7 +17,7 @@ use function array_map; 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 5830858e..42092d04 100644 --- a/module/CLI/src/Command/Tag/RenameTagCommand.php +++ b/module/CLI/src/Command/Tag/RenameTagCommand.php @@ -17,7 +17,7 @@ use Symfony\Component\Console\Style\SymfonyStyle; 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 8e357329..a8834d92 100644 --- a/module/CLI/src/Command/Util/LockedCommandConfig.php +++ b/module/CLI/src/Command/Util/LockedCommandConfig.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\Util; 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 2b34ae52..056a9c60 100644 --- a/module/CLI/src/Command/Visit/DeleteOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/DeleteOrphanVisitsCommand.php @@ -13,7 +13,7 @@ use function sprintf; 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 41674a79..0fdd8ae3 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -18,7 +18,7 @@ use function sprintf; 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 58035509..445bd36f 100644 --- a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php @@ -14,7 +14,7 @@ use Symfony\Component\Console\Input\InputInterface; 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 ea5c0fe2..d282d310 100644 --- a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php @@ -17,7 +17,7 @@ use function sprintf; 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 596b287e..66e33a78 100644 --- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -29,7 +29,7 @@ use function sprintf; 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 2a0fda3b..7bdf8150 100644 --- a/module/CLI/src/GeoLite/GeolocationDbUpdater.php +++ b/module/CLI/src/GeoLite/GeolocationDbUpdater.php @@ -20,7 +20,7 @@ use function is_int; 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 128b9f52..f2ffa16b 100644 --- a/module/CLI/src/Util/ExitCode.php +++ b/module/CLI/src/Util/ExitCode.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\CLI\Util; 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 10823734..ec0f768f 100644 --- a/module/CLI/src/Util/ShlinkTable.php +++ b/module/CLI/src/Util/ShlinkTable.php @@ -12,8 +12,8 @@ use function array_pop; 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 1d1acbf7..ff275a8e 100644 --- a/module/Core/migrations/Version20230211171904.php +++ b/module/Core/migrations/Version20230211171904.php @@ -10,7 +10,7 @@ use Doctrine\Migrations\AbstractMigration; 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 3e64c03c..ae9e7839 100644 --- a/module/Core/migrations/Version20230303164233.php +++ b/module/Core/migrations/Version20230303164233.php @@ -10,7 +10,7 @@ use Doctrine\Migrations\AbstractMigration; 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 3af587ef..b19176ef 100644 --- a/module/Core/migrations/Version20240220214031.php +++ b/module/Core/migrations/Version20240220214031.php @@ -17,8 +17,12 @@ use function in_array; */ 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 c11cbe2b..33807d0d 100644 --- a/module/Core/migrations/Version20241124112257.php +++ b/module/Core/migrations/Version20241124112257.php @@ -11,7 +11,7 @@ use Doctrine\Migrations\AbstractMigration; 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 3be9097e..8bc5b317 100644 --- a/module/Core/src/Action/Model/QrCodeParams.php +++ b/module/Core/src/Action/Model/QrCodeParams.php @@ -30,9 +30,9 @@ use const Shlinkio\Shlink\DEFAULT_QR_CODE_COLOR; 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 657336c1..dbdf8151 100644 --- a/module/Core/src/Config/NotFoundRedirectResolver.php +++ b/module/Core/src/Config/NotFoundRedirectResolver.php @@ -17,8 +17,8 @@ use function urlencode; 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 616759f1..b8ed510c 100644 --- a/module/Core/src/Config/PostProcessor/BasePathPrefixer.php +++ b/module/Core/src/Config/PostProcessor/BasePathPrefixer.php @@ -8,7 +8,7 @@ use function array_map; 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 c4078890..aa9eac06 100644 --- a/module/Core/src/Config/PostProcessor/MultiSegmentSlugProcessor.php +++ b/module/Core/src/Config/PostProcessor/MultiSegmentSlugProcessor.php @@ -11,8 +11,8 @@ use function str_replace; 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 628335cd..b55a9dee 100644 --- a/module/Core/src/Domain/Entity/Domain.php +++ b/module/Core/src/Domain/Entity/Domain.php @@ -11,7 +11,7 @@ use Shlinkio\Shlink\Core\Config\NotFoundRedirects; 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 d448b819..39d9fa8e 100644 --- a/module/Core/src/Domain/Validation/DomainRedirectsInputFilter.php +++ b/module/Core/src/Domain/Validation/DomainRedirectsInputFilter.php @@ -11,10 +11,10 @@ use Shlinkio\Shlink\Common\Validation\InputFactory; /** @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 9b59f886..6fc3b500 100644 --- a/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php +++ b/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php @@ -17,9 +17,9 @@ use function sprintf; 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 42198dc9..69e01559 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 688a4edc..a3f6fd4a 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 64ae156c..5c4efbc1 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 8f9508a2..99f1b292 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 68087b53..a1a2135a 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 e05754c7..a44a4a0c 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 8fdd395a..3bb4f937 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 f81c1d37..61f01e6f 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 e006271b..a65d9ad6 100644 --- a/module/Core/src/Matomo/MatomoTrackerBuilder.php +++ b/module/Core/src/Matomo/MatomoTrackerBuilder.php @@ -9,7 +9,7 @@ use Shlinkio\Shlink\Core\Exception\RuntimeException; 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 0e0edab7..d06bee89 100644 --- a/module/Core/src/Model/Ordering.php +++ b/module/Core/src/Model/Ordering.php @@ -6,9 +6,9 @@ namespace Shlinkio\Shlink\Core\Model; 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 42520a97..c802d75f 100644 --- a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php +++ b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php @@ -17,14 +17,14 @@ use function Shlinkio\Shlink\Core\enumValues; /** @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 df52c92d..5af78345 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php @@ -21,14 +21,14 @@ use function trim; 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 0b70c3ae..4088f450 100644 --- a/module/Core/src/ShortUrl/Middleware/TrimTrailingSlashMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/TrimTrailingSlashMiddleware.php @@ -14,7 +14,7 @@ use function rtrim; 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 2bc417b4..f3341698 100644 --- a/module/Core/src/ShortUrl/Model/Validation/CustomSlugValidator.php +++ b/module/Core/src/ShortUrl/Model/Validation/CustomSlugValidator.php @@ -12,8 +12,8 @@ use function strpbrk; 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 5cd7fe38..88b629e8 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php @@ -24,22 +24,22 @@ use const Shlinkio\Shlink\MIN_SHORT_CODES_LENGTH; 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 600ebc33..bc9de337 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php @@ -16,17 +16,17 @@ use function Shlinkio\Shlink\Core\enumValues; /** @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 cab834e6..ae02a1a3 100644 --- a/module/Core/src/Visit/Model/Visitor.php +++ b/module/Core/src/Visit/Model/Visitor.php @@ -17,11 +17,11 @@ use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE; 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 2f416324..ca9d6405 100644 --- a/module/Core/src/Visit/Repository/VisitIterationRepositoryInterface.php +++ b/module/Core/src/Visit/Repository/VisitIterationRepositoryInterface.php @@ -9,7 +9,7 @@ use Shlinkio\Shlink\Core\Visit\Entity\Visit; 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 f8dea217..688badee 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -32,8 +32,8 @@ use const Shlinkio\Shlink\DEFAULT_QR_CODE_COLOR; 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 fa4a561d..59e1a5b8 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -23,7 +23,7 @@ use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE; 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 d73a1a6d..b11a28a3 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php @@ -22,7 +22,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; 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 f9357c6a..e7c2ef16 100644 --- a/module/Core/test/Visit/RequestTrackerTest.php +++ b/module/Core/test/Visit/RequestTrackerTest.php @@ -23,7 +23,7 @@ use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE; 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 f330bab1..b51c2f46 100644 --- a/module/Rest/src/Action/AbstractRestAction.php +++ b/module/Rest/src/Action/AbstractRestAction.php @@ -10,8 +10,8 @@ use Psr\Http\Server\RequestHandlerInterface; 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 e98aa339..bc48d2d0 100644 --- a/module/Rest/src/Action/Domain/DomainRedirectsAction.php +++ b/module/Rest/src/Action/Domain/DomainRedirectsAction.php @@ -14,10 +14,10 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 d156a720..4007ab58 100644 --- a/module/Rest/src/Action/Domain/ListDomainsAction.php +++ b/module/Rest/src/Action/Domain/ListDomainsAction.php @@ -15,11 +15,13 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 ce731330..be47428d 100644 --- a/module/Rest/src/Action/HealthAction.php +++ b/module/Rest/src/Action/HealthAction.php @@ -13,14 +13,14 @@ use Throwable; 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 1454cbbc..f468b4a2 100644 --- a/module/Rest/src/Action/MercureInfoAction.php +++ b/module/Rest/src/Action/MercureInfoAction.php @@ -15,11 +15,13 @@ use function sprintf; 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 c6c12fd9..ab48bd1e 100644 --- a/module/Rest/src/Action/RedirectRule/ListRedirectRulesAction.php +++ b/module/Rest/src/Action/RedirectRule/ListRedirectRulesAction.php @@ -13,8 +13,8 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 913a833d..1f266821 100644 --- a/module/Rest/src/Action/RedirectRule/SetRedirectRulesAction.php +++ b/module/Rest/src/Action/RedirectRule/SetRedirectRulesAction.php @@ -16,8 +16,8 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 67509f1b..6f5e291c 100644 --- a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php @@ -12,8 +12,8 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 c1511f77..b2570d36 100644 --- a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php @@ -14,8 +14,8 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 c9eaf958..8085a682 100644 --- a/module/Rest/src/Action/ShortUrl/DeleteShortUrlVisitsAction.php +++ b/module/Rest/src/Action/ShortUrl/DeleteShortUrlVisitsAction.php @@ -14,8 +14,8 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 b4b815a4..acdc2cb4 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php @@ -16,8 +16,8 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 b21f50fd..4b9991b9 100644 --- a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php +++ b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php @@ -16,8 +16,8 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 39a9d7f2..3faae433 100644 --- a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php @@ -15,8 +15,8 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 d7f5a360..039b82d4 100644 --- a/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php @@ -11,8 +11,8 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 48e7acd9..8c45acf7 100644 --- a/module/Rest/src/Action/Tag/DeleteTagsAction.php +++ b/module/Rest/src/Action/Tag/DeleteTagsAction.php @@ -13,10 +13,10 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 426f94e7..826ed112 100644 --- a/module/Rest/src/Action/Tag/ListTagsAction.php +++ b/module/Rest/src/Action/Tag/ListTagsAction.php @@ -15,8 +15,8 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 1ae470e0..827f6241 100644 --- a/module/Rest/src/Action/Tag/TagsStatsAction.php +++ b/module/Rest/src/Action/Tag/TagsStatsAction.php @@ -15,8 +15,8 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 e1dc1611..373c2631 100644 --- a/module/Rest/src/Action/Tag/UpdateTagAction.php +++ b/module/Rest/src/Action/Tag/UpdateTagAction.php @@ -14,10 +14,10 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 090d3078..b63133fa 100644 --- a/module/Rest/src/Action/Visit/AbstractListVisitsAction.php +++ b/module/Rest/src/Action/Visit/AbstractListVisitsAction.php @@ -18,7 +18,7 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 3a016451..c08de858 100644 --- a/module/Rest/src/Action/Visit/DeleteOrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/DeleteOrphanVisitsAction.php @@ -13,8 +13,8 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 ee1625e0..0e027955 100644 --- a/module/Rest/src/Action/Visit/DomainVisitsAction.php +++ b/module/Rest/src/Action/Visit/DomainVisitsAction.php @@ -14,7 +14,7 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; 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 1f2e1211..779fd4a3 100644 --- a/module/Rest/src/Action/Visit/GlobalVisitsAction.php +++ b/module/Rest/src/Action/Visit/GlobalVisitsAction.php @@ -13,10 +13,10 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; 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 8406b515..b2f7471b 100644 --- a/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php @@ -11,7 +11,7 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; 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 341524c3..b3c246ca 100644 --- a/module/Rest/src/Action/Visit/OrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/OrphanVisitsAction.php @@ -12,7 +12,7 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; 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 95ac42cc..d8fc36e9 100644 --- a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php +++ b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php @@ -12,7 +12,7 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; 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 08553ec5..07ad7167 100644 --- a/module/Rest/src/Action/Visit/TagVisitsAction.php +++ b/module/Rest/src/Action/Visit/TagVisitsAction.php @@ -11,7 +11,7 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; 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 1768c7c8..95a70b0c 100644 --- a/module/Rest/src/ConfigProvider.php +++ b/module/Rest/src/ConfigProvider.php @@ -12,9 +12,9 @@ use function sprintf; 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 7e47b519..c97e701b 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 3fd2e2c6..ff0c6026 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 25f1b050..9ed03fe1 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 cf73ba10..eaa85c96 100644 --- a/module/Rest/src/Middleware/AuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/AuthenticationMiddleware.php @@ -21,7 +21,7 @@ use function Shlinkio\Shlink\Core\ArrayUtils\contains; 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 08503757..a84b9e8a 100644 --- a/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php +++ b/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php @@ -18,8 +18,8 @@ use function strtolower; 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 494a6564..7909dcfd 100644 --- a/module/Rest/test-api/Action/ListRedirectRulesTest.php +++ b/module/Rest/test-api/Action/ListRedirectRulesTest.php @@ -12,12 +12,12 @@ use function sprintf; 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 1eba6db8..60b493d6 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -15,7 +15,7 @@ use function count; 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 3761e113..f9414899 100644 --- a/module/Rest/test-api/Action/OrphanVisitsTest.php +++ b/module/Rest/test-api/Action/OrphanVisitsTest.php @@ -13,7 +13,7 @@ use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; 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 6501ef13..9fc6fa9d 100644 --- a/module/Rest/test-api/Action/SetRedirectRulesTest.php +++ b/module/Rest/test-api/Action/SetRedirectRulesTest.php @@ -13,12 +13,12 @@ use function sprintf; 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 a93559cb..f2bc57d2 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 95692294..6195ce90 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 9353d40c..4061960b 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 bc81820e..9fe15aea 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 7bdf8150..0d3d96db 100644 --- a/module/CLI/src/GeoLite/GeolocationDbUpdater.php +++ b/module/CLI/src/GeoLite/GeolocationDbUpdater.php @@ -64,12 +64,12 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface 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 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface * @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 76e7c4f5..5df74822 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -7,9 +7,9 @@ namespace Shlinkio\Shlink\CLI; use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory; use Laminas\ServiceManager\Factory\InvokableFactory; use Shlinkio\Shlink\Common\Doctrine\NoDbNameConnectionFactory; -use Shlinkio\Shlink\Core\Config\Options\TrackingOptions; use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Domain\DomainService; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdater; use Shlinkio\Shlink\Core\Matomo; use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleService; use Shlinkio\Shlink\Core\ShortUrl; @@ -17,15 +17,11 @@ use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; use Shlinkio\Shlink\Core\Tag\TagService; use Shlinkio\Shlink\Core\Visit; use Shlinkio\Shlink\Installer\Factory\ProcessHelperFactory; -use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater; -use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2ReaderFactory; use Shlinkio\Shlink\Rest\Service\ApiKeyService; use Symfony\Component\Console as SymfonyCli; use Symfony\Component\Lock\LockFactory; use Symfony\Component\Process\PhpExecutableFinder; -use const Shlinkio\Shlink\LOCAL_LOCK_FACTORY; - return [ 'dependencies' => [ @@ -34,7 +30,6 @@ return [ SymfonyCli\Helper\ProcessHelper::class => ProcessHelperFactory::class, PhpExecutableFinder::class => InvokableFactory::class, - GeoLite\GeolocationDbUpdater::class => ConfigAbstractFactory::class, RedirectRule\RedirectRuleHandler::class => InvokableFactory::class, Util\ProcessRunner::class => ConfigAbstractFactory::class, @@ -82,12 +77,6 @@ return [ ], ConfigAbstractFactory::class => [ - GeoLite\GeolocationDbUpdater::class => [ - DbUpdater::class, - GeoLite2ReaderFactory::class, - LOCAL_LOCK_FACTORY, - TrackingOptions::class, - ], Util\ProcessRunner::class => [SymfonyCli\Helper\ProcessHelper::class], ApiKey\RoleResolver::class => [DomainService::class, UrlShortenerOptions::class], @@ -107,7 +96,7 @@ return [ Command\ShortUrl\DeleteShortUrlVisitsCommand::class => [ShortUrl\ShortUrlVisitsDeleter::class], Command\ShortUrl\DeleteExpiredShortUrlsCommand::class => [ShortUrl\DeleteShortUrlService::class], - Command\Visit\DownloadGeoLiteDbCommand::class => [GeoLite\GeolocationDbUpdater::class], + Command\Visit\DownloadGeoLiteDbCommand::class => [GeolocationDbUpdater::class], Command\Visit\LocateVisitsCommand::class => [ Visit\Geolocation\VisitLocator::class, Visit\Geolocation\VisitToLocationHelper::class, diff --git a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php index 0fdd8ae3..f6110d07 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -4,10 +4,10 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; -use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; use Shlinkio\Shlink\CLI\Util\ExitCode; +use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface; +use Shlinkio\Shlink\Core\Geolocation\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 4d2754f8..2b477f03 100644 --- a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php +++ b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php @@ -9,10 +9,10 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\Visit\DownloadGeoLiteDbCommand; -use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; use Shlinkio\Shlink\CLI\Util\ExitCode; +use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface; +use Shlinkio\Shlink\Core\Geolocation\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 519ddf02..a1d6db65 100644 --- a/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php +++ b/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php @@ -9,7 +9,7 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; use RuntimeException; -use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; +use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException; use Throwable; class GeolocationDbUpdateFailedExceptionTest extends TestCase diff --git a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php index 038d570c..dc1d614c 100644 --- a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php +++ b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php @@ -11,10 +11,10 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdater; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; use Shlinkio\Shlink\Core\Config\Options\TrackingOptions; +use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdater; +use Shlinkio\Shlink\Core\Geolocation\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 4844e6d5..b16a4c5c 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -9,12 +9,16 @@ use Laminas\ServiceManager\Factory\InvokableFactory; use Psr\EventDispatcher\EventDispatcherInterface; use Shlinkio\Shlink\Common\Doctrine\EntityRepositoryFactory; use Shlinkio\Shlink\Core\Config\Options\NotFoundRedirectOptions; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdater; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater; +use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2ReaderFactory; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Symfony\Component\Lock; +use const Shlinkio\Shlink\LOCAL_LOCK_FACTORY; + return [ 'dependencies' => [ @@ -103,6 +107,7 @@ return [ EventDispatcher\PublishingUpdatesGenerator::class => ConfigAbstractFactory::class, + Geolocation\GeolocationDbUpdater::class => ConfigAbstractFactory::class, Geolocation\Middleware\IpGeolocationMiddleware::class => ConfigAbstractFactory::class, Importer\ImportedLinksProcessor::class => ConfigAbstractFactory::class, @@ -240,6 +245,12 @@ return [ EventDispatcher\PublishingUpdatesGenerator::class => [ShortUrl\Transformer\ShortUrlDataTransformer::class], + GeolocationDbUpdater::class => [ + DbUpdater::class, + GeoLite2ReaderFactory::class, + LOCAL_LOCK_FACTORY, + Config\Options\TrackingOptions::class, + ], Geolocation\Middleware\IpGeolocationMiddleware::class => [ IpLocationResolverInterface::class, DbUpdater::class, diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 4e130fcf..39efa3cb 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -6,11 +6,11 @@ namespace Shlinkio\Shlink\Core; use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory; use Psr\EventDispatcher\EventDispatcherInterface; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdater; use Shlinkio\Shlink\Common\Cache\RedisPublishingHelper; use Shlinkio\Shlink\Common\Mercure\MercureHubPublishingHelper; use Shlinkio\Shlink\Common\Mercure\MercureOptions; use Shlinkio\Shlink\Common\RabbitMq\RabbitMqPublishingHelper; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdater; use Shlinkio\Shlink\Core\Matomo\MatomoOptions; use Shlinkio\Shlink\Core\Visit\Geolocation\VisitLocator; use Shlinkio\Shlink\Core\Visit\Geolocation\VisitToLocationHelper; diff --git a/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php index 4e4720c5..7f14cc24 100644 --- a/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php +++ b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php @@ -6,9 +6,9 @@ namespace Shlinkio\Shlink\Core\EventDispatcher; use Psr\EventDispatcher\EventDispatcherInterface; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; use Shlinkio\Shlink\Core\EventDispatcher\Event\GeoLiteDbCreated; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface; +use Shlinkio\Shlink\Core\Geolocation\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 ee31ac82..f3c3f65f 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 0d3d96db..73515a45 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 ba0f0e70..1e583e20 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 85976886..3b472d09 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 | 43 +++++++++++++------ .../src/Geolocation/GeolocationDbUpdater.php | 38 ++++++---------- .../GeolocationDbUpdaterInterface.php | 3 +- ...cationDownloadProgressHandlerInterface.php | 21 +++++++++ 6 files changed, 111 insertions(+), 64 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 f6110d07..b0a22c97 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\CLI\Command\Visit; 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 Symfony\Component\Console\Style\SymfonyStyle; use function sprintf; -class DownloadGeoLiteDbCommand extends Command +class DownloadGeoLiteDbCommand extends Command implements GeolocationDownloadProgressHandlerInterface { public const string NAME = 'visit:download-db'; private ProgressBar|null $progressBar = null; + private SymfonyStyle $io; - public function __construct(private GeolocationDbUpdaterInterface $dbUpdater) + public function __construct(private readonly GeolocationDbUpdaterInterface $dbUpdater) { parent::__construct(); } @@ -39,32 +41,26 @@ class DownloadGeoLiteDbCommand extends Command protected function execute(InputInterface $input, OutputInterface $output): int { - $io = new SymfonyStyle($input, $output); + $this->io = new SymfonyStyle($input, $output); try { - $result = $this->dbUpdater->checkDbUpdate(function (bool $olderDbExists) use ($io): void { - $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 @@ class DownloadGeoLiteDbCommand extends Command 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 dc1d614c..5a451801 100644 --- a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php +++ b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php @@ -14,6 +14,7 @@ use PHPUnit\Framework\TestCase; 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 @@ class GeolocationDbUpdaterTest extends TestCase $this->geoLiteDbReader = $this->createMock(Reader::class); $this->lock = $this->createMock(Lock\SharedLockInterface::class); $this->lock->method('acquire')->with($this->isTrue())->willReturn(true); + $this->progressHandler = new class implements GeolocationDownloadProgressHandlerInterface { + public function __construct( + public bool $beforeDownloadCalled = false, + public bool $handleProgressCalled = false, + ) { + } + + public function beforeDownload(bool $olderDbExists): void + { + $this->beforeDownloadCalled = true; + } + + public function handleProgress(int $total, int $downloaded, bool $olderDbExists): void + { + $this->handleProgressCalled = true; + } + }; } #[Test] @@ -47,12 +67,9 @@ class GeolocationDbUpdaterTest extends TestCase ); $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 @@ class GeolocationDbUpdaterTest extends TestCase )->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 7f14cc24..87108279 100644 --- a/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php +++ b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php @@ -8,6 +8,7 @@ use Psr\EventDispatcher\EventDispatcherInterface; 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 @@ readonly class UpdateGeoLiteDb public function __invoke(): void { - $beforeDownload = fn (bool $olderDbExists) => $this->logger->notice( - sprintf('%s GeoLite2 db file...', $olderDbExists ? 'Updating' : 'Downloading'), - ); - $messageLogged = false; - $handleProgress = function (int $total, int $downloaded, bool $olderDbExists) use (&$messageLogged): void { - if ($messageLogged || $total > $downloaded) { - return; - } - - $messageLogged = true; - $this->logger->notice(sprintf('Finished %s GeoLite2 db file', $olderDbExists ? 'updating' : 'downloading')); - }; - try { - $result = $this->dbUpdater->checkDbUpdate($beforeDownload, $handleProgress); + $result = $this->dbUpdater->checkDbUpdate( + new class ($this->logger) implements GeolocationDownloadProgressHandlerInterface { + public function __construct( + private readonly LoggerInterface $logger, + private bool $messageLogged = false, + ) { + } + + public function beforeDownload(bool $olderDbExists): void + { + $this->logger->notice( + sprintf('%s GeoLite2 db file...', $olderDbExists ? 'Updating' : 'Downloading'), + ); + } + + public function handleProgress(int $total, int $downloaded, bool $olderDbExists): void + { + if ($this->messageLogged || $total > $downloaded) { + return; + } + + $this->messageLogged = true; + $this->logger->notice( + sprintf('Finished %s GeoLite2 db file', $olderDbExists ? 'updating' : 'downloading'), + ); + } + }, + ); if ($result === GeolocationResult::DB_CREATED) { $this->eventDispatcher->dispatch(new GeoLiteDbCreated()); } diff --git a/module/Core/src/Geolocation/GeolocationDbUpdater.php b/module/Core/src/Geolocation/GeolocationDbUpdater.php index 73515a45..b8d77a4b 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdater.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php @@ -12,7 +12,6 @@ use Shlinkio\Shlink\Core\Config\Options\TrackingOptions; use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\IpGeolocation\Exception\DbUpdateException; use Shlinkio\Shlink\IpGeolocation\Exception\MissingLicenseException; -use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; use Symfony\Component\Lock\LockFactory; @@ -41,8 +40,7 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface * @throws GeolocationDbUpdateFailedException */ public function checkDbUpdate( - callable|null $beforeDownload = null, - callable|null $handleProgress = null, + GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler = null, ): GeolocationResult { if (! $this->trackingOptions->isGeolocationRelevant()) { return GeolocationResult::CHECK_SKIPPED; @@ -52,7 +50,7 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface $lock->acquire(true); // Block until lock is released try { - return $this->downloadIfNeeded($beforeDownload, $handleProgress); + return $this->downloadIfNeeded($downloadProgressHandler); } finally { $lock->release(); } @@ -61,15 +59,16 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface /** * @throws GeolocationDbUpdateFailedException */ - private function downloadIfNeeded(callable|null $beforeDownload, callable|null $handleProgress): GeolocationResult - { + private function downloadIfNeeded( + GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler, + ): GeolocationResult { if (! $this->dbUpdater->databaseFileExists()) { - return $this->downloadNewDb($beforeDownload, $handleProgress, olderDbExists: false); + return $this->downloadNewDb($downloadProgressHandler, olderDbExists: false); } $meta = ($this->geoLiteDbReaderFactory)()->metadata(); if ($this->buildIsTooOld($meta)) { - return $this->downloadNewDb($beforeDownload, $handleProgress, olderDbExists: true); + return $this->downloadNewDb($downloadProgressHandler, olderDbExists: true); } return GeolocationResult::DB_IS_UP_TO_DATE; @@ -106,32 +105,23 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface * @throws GeolocationDbUpdateFailedException */ private function downloadNewDb( - callable|null $beforeDownload, - 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 1e583e20..d7322e86 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 00000000..74cb90ae --- /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 55662d5a..5288aa1b 100644 --- a/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php +++ b/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php @@ -14,6 +14,7 @@ use RuntimeException; use Shlinkio\Shlink\Core\EventDispatcher\Event\GeoLiteDbCreated; use Shlinkio\Shlink\Core\EventDispatcher\UpdateGeoLiteDb; use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface; use Shlinkio\Shlink\Core\Geolocation\GeolocationResult; use function array_map; @@ -51,11 +52,11 @@ class UpdateGeoLiteDbTest extends TestCase } #[Test, DataProvider('provideFlags')] - public function noticeMessageIsPrintedWhenFirstCallbackIsInvoked(bool $oldDbExists, string $expectedMessage): void + public function noticeMessageIsPrintedWhenDownloadIsStarted(bool $oldDbExists, string $expectedMessage): void { $this->dbUpdater->expects($this->once())->method('checkDbUpdate')->withAnyParameters()->willReturnCallback( - function (callable $firstCallback) use ($oldDbExists): GeolocationResult { - $firstCallback($oldDbExists); + function (GeolocationDownloadProgressHandlerInterface $handler) use ($oldDbExists): GeolocationResult { + $handler->beforeDownload($oldDbExists); return GeolocationResult::DB_IS_UP_TO_DATE; }, ); @@ -73,18 +74,24 @@ class UpdateGeoLiteDbTest extends TestCase } #[Test, DataProvider('provideDownloaded')] - public function noticeMessageIsPrintedWhenSecondCallbackIsInvoked( + public function noticeMessageIsPrintedWhenDownloadIsFinished( int $total, int $downloaded, bool $oldDbExists, string|null $expectedMessage, ): void { $this->dbUpdater->expects($this->once())->method('checkDbUpdate')->withAnyParameters()->willReturnCallback( - function ($_, callable $secondCallback) use ($total, $downloaded, $oldDbExists): GeolocationResult { + function ( + GeolocationDownloadProgressHandlerInterface $handler, + ) use ( + $total, + $downloaded, + $oldDbExists, + ): GeolocationResult { // Invoke several times to ensure the log is printed only once - $secondCallback($total, $downloaded, $oldDbExists); - $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 5a451801..5c76747b 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 @@ class GeolocationDbUpdaterTest extends TestCase $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(false); $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->with( - $this->isNull(), + $this->isInstanceOf(Closure::class), )->willThrowException($prev); $this->geoLiteDbReader->expects($this->never())->method('metadata'); @@ -101,7 +102,7 @@ class GeolocationDbUpdaterTest extends TestCase $prev = new DbUpdateException(''); $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(true); $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->with( - $this->isNull(), + $this->isInstanceOf(Closure::class), )->willThrowException($prev); $this->geoLiteDbReader->expects($this->once())->method('metadata')->with()->willReturn( $this->buildMetaWithBuildEpoch(Chronos::now()->subDays($days)->getTimestamp()), 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 2b477f03..7fa46a05 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\Command\Visit\DownloadGeoLiteDbCommand; use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface; use Shlinkio\Shlink\Core\Geolocation\GeolocationResult; use ShlinkioTest\Shlink\CLI\Util\CliTestUtils; use Symfony\Component\Console\Tester\CommandTester; @@ -36,9 +37,9 @@ class DownloadGeoLiteDbCommandTest extends TestCase int $expectedExitCode, ): void { $this->dbUpdater->expects($this->once())->method('checkDbUpdate')->withAnyParameters()->willReturnCallback( - function (callable $beforeDownload, callable $handleProgress) use ($olderDbExists): void { - $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 @@ class DownloadGeoLiteDbCommandTest extends TestCase public static function provideSuccessParams(): iterable { yield 'up to date db' => [fn () => GeolocationResult::CHECK_SKIPPED, '[INFO] GeoLite2 db file is up to date.']; - yield 'outdated db' => [function (callable $beforeDownload): GeolocationResult { - $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 73e62b39..eea707f5 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 3d9ea0ad..0c08124f 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 f01943b0..3619aba3 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 b8d77a4b..9c7d61d3 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdater.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php @@ -47,7 +47,7 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface } $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 df578387..7f43d443 100644 --- a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php @@ -106,8 +106,8 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt { // 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 00000000..ecf21fb2 --- /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 00000000..1ba4bc02 --- /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 00000000..0f376cb9 --- /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 00000000..8216f2bd --- /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 b0a22c97..8e873b1d 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -66,7 +66,7 @@ class DownloadGeoLiteDbCommand extends Command implements GeolocationDownloadPro 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 a1d6db65..86ed9548 100644 --- a/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php +++ b/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php @@ -19,7 +19,7 @@ class GeolocationDbUpdateFailedExceptionTest extends TestCase { $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 @@ class GeolocationDbUpdateFailedExceptionTest extends TestCase { $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 @@ class GeolocationDbUpdateFailedExceptionTest extends TestCase 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 b16a4c5c..adc9ae2a 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -13,7 +13,6 @@ 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; @@ -247,9 +246,9 @@ return [ 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 ecf21fb2..20a67c98 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 @@ return static function (ClassMetadata $metadata, array $emConfig): void { ->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 1ba4bc02..59286931 100644 --- a/module/Core/migrations/Version20241212131058.php +++ b/module/Core/migrations/Version20241212131058.php @@ -38,11 +38,6 @@ final class Version20241212131058 extends AbstractMigration ]); $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 87108279..8d288959 100644 --- a/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php +++ b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php @@ -14,6 +14,7 @@ use Throwable; 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 f3c3f65f..a818b263 100644 --- a/module/Core/src/Exception/GeolocationDbUpdateFailedException.php +++ b/module/Core/src/Exception/GeolocationDbUpdateFailedException.php @@ -7,52 +7,28 @@ namespace Shlinkio\Shlink\Core\Exception; 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 0f376cb9..1e1d544f 100644 --- a/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php +++ b/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php @@ -16,27 +16,59 @@ class GeolocationDbUpdate extends AbstractEntity 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 9c7d61d3..e5bd0ea1 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdater.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php @@ -4,36 +4,29 @@ declare(strict_types=1); 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 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface return GeolocationResult::CHECK_SKIPPED; } + $lock = $this->locker->createLock(self::LOCK_NAME); $lock->acquire(blocking: true); @@ -62,43 +56,68 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface 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; + } + + // 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 GeolocationResult::DB_IS_UP_TO_DATE; } - private function buildIsTooOld(Metadata $meta): bool - { - $buildTimestamp = $this->resolveBuildTimestamp($meta); - $buildDate = Chronos::createFromTimestamp($buildTimestamp); + /** + * @throws GeolocationDbUpdateFailedException + */ + private function downloadAndTrackUpdate( + GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler, + bool $olderDbExists, + ): GeolocationResult { + $dbUpdate = GeolocationDbUpdate::forFilesystemId(); + $this->em->persist($dbUpdate); + $this->em->flush(); - return Chronos::now()->greaterThan($buildDate->addDays(35)); - } - - 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; + 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(); } - - $intBuildEpoch = (int) $buildEpoch; - if ($buildEpoch === (string) $intBuildEpoch) { - return $intBuildEpoch; - } - - throw GeolocationDbUpdateFailedException::withInvalidEpochInOldDb($buildEpoch); } /** @@ -116,8 +135,6 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface => $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 5c76747b..d2ec1bfa 100644 --- a/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php +++ b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php @@ -91,7 +91,7 @@ class GeolocationDbUpdaterTest extends TestCase } 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 @@ class GeolocationDbUpdaterTest extends TestCase } 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 20a67c98..e7f10ca1 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 @@ return static function (ClassMetadata $metadata, array $emConfig): void { // 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 59286931..e27958f6 100644 --- a/module/Core/migrations/Version20241212131058.php +++ b/module/Core/migrations/Version20241212131058.php @@ -46,8 +46,8 @@ final class Version20241212131058 extends AbstractMigration // 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 8e873b1d..e1ea5fce 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -51,6 +51,11 @@ class DownloadGeoLiteDbCommand extends Command implements GeolocationDownloadPro 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 7fa46a05..f232bdca 100644 --- a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php +++ b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php @@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\CLI\Command\Visit; 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 @@ class DownloadGeoLiteDbCommandTest extends TestCase } #[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 1e1d544f..35a9697d 100644 --- a/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php +++ b/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php @@ -49,17 +49,11 @@ class GeolocationDbUpdate extends AbstractEntity } /** - * 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 @@ class GeolocationDbUpdate extends AbstractEntity { 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 e5bd0ea1..f6ca7b49 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdater.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php @@ -13,8 +13,6 @@ use Shlinkio\Shlink\IpGeolocation\Exception\MissingLicenseException; 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 @@ readonly class GeolocationDbUpdater implements GeolocationDbUpdaterInterface private LockFactory $locker, private TrackingOptions $trackingOptions, private EntityManagerInterface $em, + private int $maxRecentAttemptsToCheck = 15, // TODO Make this configurable ) { } @@ -56,16 +55,12 @@ readonly class GeolocationDbUpdater implements GeolocationDbUpdaterInterface 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 @@ readonly class GeolocationDbUpdater implements GeolocationDbUpdaterInterface 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++; } - // 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()) { + // 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 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 @@ readonly class GeolocationDbUpdater implements GeolocationDbUpdaterInterface 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 3b472d09..1eecedda 100644 --- a/module/Core/src/Geolocation/GeolocationResult.php +++ b/module/Core/src/Geolocation/GeolocationResult.php @@ -5,6 +5,7 @@ namespace Shlinkio\Shlink\Core\Geolocation; 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 e1ea5fce..cd7c4ffb 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -56,6 +56,11 @@ class DownloadGeoLiteDbCommand extends Command implements GeolocationDownloadPro 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 f232bdca..01322f0b 100644 --- a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php +++ b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php @@ -77,6 +77,7 @@ class DownloadGeoLiteDbCommandTest extends TestCase #[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 f6ca7b49..662e6b7a 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdater.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php @@ -12,6 +12,7 @@ use Shlinkio\Shlink\IpGeolocation\Exception\DbUpdateException; 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 @@ readonly class GeolocationDbUpdater implements GeolocationDbUpdaterInterface // 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 @@ readonly class GeolocationDbUpdater implements GeolocationDbUpdaterInterface 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 1eecedda..eb5b4a5c 100644 --- a/module/Core/src/Geolocation/GeolocationResult.php +++ b/module/Core/src/Geolocation/GeolocationResult.php @@ -4,10 +4,18 @@ namespace Shlinkio\Shlink\Core\Geolocation; 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 e7f10ca1..7b60abdc 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 @@ return static function (ClassMetadata $metadata, array $emConfig): void { ->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 e27958f6..23e61803 100644 --- a/module/Core/migrations/Version20241212131058.php +++ b/module/Core/migrations/Version20241212131058.php @@ -38,6 +38,7 @@ final class Version20241212131058 extends AbstractMigration ]); $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 35a9697d..42cdfa4b 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 @@ class GeolocationDbUpdate extends AbstractEntity ) { } - 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 662e6b7a..7be1cd56 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdater.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php @@ -89,12 +89,20 @@ readonly class GeolocationDbUpdater implements GeolocationDbUpdaterInterface } // 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 @@ readonly class GeolocationDbUpdater implements GeolocationDbUpdaterInterface 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 12b5aae4..fb43ec7e 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 42cdfa4b..f3735a64 100644 --- a/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php +++ b/module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php @@ -6,14 +6,15 @@ namespace Shlinkio\Shlink\Core\Geolocation\Entity; 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 @@ class GeolocationDbUpdate extends AbstractEntity ) { } - 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 7be1cd56..9e63cf5c 100644 --- a/module/Core/src/Geolocation/GeolocationDbUpdater.php +++ b/module/Core/src/Geolocation/GeolocationDbUpdater.php @@ -95,9 +95,9 @@ readonly class GeolocationDbUpdater implements GeolocationDbUpdaterInterface // - 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 d2ec1bfa..c2983030 100644 --- a/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php +++ b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php @@ -6,14 +6,16 @@ namespace ShlinkioTest\Shlink\Core\Geolocation; 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 @@ use function range; 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 @@ class GeolocationDbUpdaterTest extends TestCase }; } + #[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 @@ class GeolocationDbUpdaterTest extends TestCase $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 @@ class GeolocationDbUpdaterTest extends TestCase 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 @@ class GeolocationDbUpdaterTest extends TestCase } } + 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 @@ class GeolocationDbUpdaterTest extends TestCase 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 @@ class GeolocationDbUpdaterTest extends TestCase $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 fb43ec7e..241b734e 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 f3341698..3d3e7792 100644 --- a/module/Core/src/ShortUrl/Model/Validation/CustomSlugValidator.php +++ b/module/Core/src/ShortUrl/Model/Validation/CustomSlugValidator.php @@ -46,10 +46,10 @@ class CustomSlugValidator extends AbstractValidator 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 86f695c7..f763b44e 100644 --- a/module/Core/test/ShortUrl/Model/Validation/CustomSlugValidatorTest.php +++ b/module/Core/test/ShortUrl/Model/Validation/CustomSlugValidatorTest.php @@ -59,13 +59,11 @@ class CustomSlugValidatorTest extends TestCase 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 241b734e..25c508a8 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 39be2dc9..bb95f7ef 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 8bc5b317..459c99b7 100644 --- a/module/Core/src/Action/Model/QrCodeParams.php +++ b/module/Core/src/Action/Model/QrCodeParams.php @@ -28,20 +28,21 @@ use function trim; 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 @@ final class QrCodeParams 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 607fab50..fbc83c21 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -60,7 +60,7 @@ readonly class QrCodeAction implements MiddlewareInterface 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 688badee..9db42752 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -9,6 +9,7 @@ use Laminas\Diactoros\ServerRequest; 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 @@ class QrCodeActionTest extends TestCase } #[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 @@ class QrCodeActionTest extends TestCase $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 4338a8b6..1bd3db44 100644 --- a/config/autoload/entity-manager.global.php +++ b/config/autoload/entity-manager.global.php @@ -12,9 +12,10 @@ use function Shlinkio\Shlink\Core\ArrayUtils\contains; 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 @@ return (static function (): array { $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 @@ return (static function (): array { 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 e2a6d38a..f8042feb 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 @@ enum EnvVars: string '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 25c508a8..b3cdeab8 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 9fe15aea..aa1a1e52 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 e239dd3a..24b9263e 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -20,6 +20,7 @@ return [ 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 aa1a1e52..ca6cab07 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 79a13fbf..2cfe9417 100644 --- a/module/Core/test-api/Action/RedirectTest.php +++ b/module/Core/test-api/Action/RedirectTest.php @@ -93,7 +93,7 @@ class RedirectTest extends ApiTestCase 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 ca6cab07..15ba3b70 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 adc9ae2a..eda556e9 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -262,6 +262,7 @@ return [ 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 e8434d4f..af4ce917 100644 --- a/module/Core/src/Importer/ImportedLinksProcessor.php +++ b/module/Core/src/Importer/ImportedLinksProcessor.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Importer; 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 @@ readonly class ImportedLinksProcessor implements ImportedLinksProcessorInterface private ShortUrlRelationResolverInterface $relationResolver, private ShortCodeUniquenessHelperInterface $shortCodeHelper, private DoctrineBatchHelperInterface $batchHelper, + private ShortUrlRedirectRuleServiceInterface $redirectRuleService, ) { } @@ -80,6 +82,7 @@ readonly class ImportedLinksProcessor implements ImportedLinksProcessorInterface 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 ad812e8c..cc534fc2 100644 --- a/module/Core/src/Importer/ShortUrlImporting.php +++ b/module/Core/src/Importer/ShortUrlImporting.php @@ -4,11 +4,17 @@ declare(strict_types=1); 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 @@ final readonly class ShortUrlImporting 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 @@ final readonly class ShortUrlImporting : 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 cf1e134b..602d07b7 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\Model\DeviceType; 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 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable 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 01ba0a8f..dac0dc61 100644 --- a/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php +++ b/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php @@ -20,7 +20,7 @@ readonly class ShortUrlRedirectRuleService implements ShortUrlRedirectRuleServic } /** - * @return ShortUrlRedirectRule[] + * @inheritDoc */ public function rulesForShortUrl(ShortUrl $shortUrl): array { @@ -31,7 +31,7 @@ readonly class ShortUrlRedirectRuleService implements ShortUrlRedirectRuleServic } /** - * @return ShortUrlRedirectRule[] + * @inheritDoc */ public function setRulesForShortUrl(ShortUrl $shortUrl, RedirectRulesData $data): array { @@ -55,7 +55,7 @@ readonly class ShortUrlRedirectRuleService implements ShortUrlRedirectRuleServic } /** - * @param ShortUrlRedirectRule[] $rules + * @inheritDoc */ public function saveRulesForShortUrl(ShortUrl $shortUrl, array $rules): void { @@ -74,7 +74,7 @@ readonly class ShortUrlRedirectRuleService implements ShortUrlRedirectRuleServic /** * @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 186be87e..e05c6ab8 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 b7fb6c56..086a47bb 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -76,7 +76,6 @@ class ShortUrl extends AbstractEntity /** * @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 36265aa3..8fb63e68 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -14,6 +14,7 @@ use PHPUnit\Framework\MockObject\MockObject; 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 @@ class ImportedLinksProcessorTest extends TestCase 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 0402dc8c..6ffec859 100644 --- a/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php @@ -74,7 +74,7 @@ class DeleteShortUrlCommandTest extends TestCase $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 0f24a603..9b35324a 100644 --- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php @@ -47,7 +47,7 @@ class LocateVisitsCommandTest extends TestCase $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 eb78da61..a7811060 100644 --- a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php +++ b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php @@ -77,7 +77,7 @@ class RedirectRuleHandlerTest extends TestCase $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 cc534fc2..aeb24cce 100644 --- a/module/Core/src/Importer/ShortUrlImporting.php +++ b/module/Core/src/Importer/ShortUrlImporting.php @@ -14,6 +14,7 @@ use Shlinkio\Shlink\Core\Visit\Entity\Visit; 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 @@ final readonly class ShortUrlImporting 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 602d07b7..255cb19e 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -24,7 +24,7 @@ use function trim; 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 c2983030..8c9f5a1b 100644 --- a/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php +++ b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php @@ -291,7 +291,7 @@ class GeolocationDbUpdaterTest extends TestCase 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 8fb63e68..e8d800a1 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -14,6 +14,9 @@ use PHPUnit\Framework\MockObject\MockObject; 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\Entity\Visit; 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 @@ class ImportedLinksProcessorTest extends TestCase #[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 @@ class ImportedLinksProcessorTest extends TestCase $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 @@ class ImportedLinksProcessorTest extends TestCase 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 5a4a2e2b..bec6d263 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -9,6 +9,8 @@ use PHPUnit\Framework\Attributes\TestWith; 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 @@ class RedirectConditionTest extends TestCase 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 934d8511..31f42bc7 100644 --- a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php @@ -78,7 +78,7 @@ class PersistenceShortUrlRelationResolverTest extends TestCase $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 @@ class PersistenceShortUrlRelationResolverTest extends TestCase 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 @@ class PersistenceShortUrlRelationResolverTest extends TestCase 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 a6cacc46..fb191e95 100644 --- a/module/Core/test/ShortUrl/UrlShortenerTest.php +++ b/module/Core/test/ShortUrl/UrlShortenerTest.php @@ -38,7 +38,7 @@ class UrlShortenerTest extends TestCase // 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 81bec4ea..a5b6cecb 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -45,7 +45,7 @@ class ApiKeyServiceTest extends TestCase 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 @@ class ApiKeyServiceTest extends TestCase { $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 b3cdeab8..2f906363 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 2f906363..309932c1 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 15ba3b70..f9133a5c 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",