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/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 81% rename from .github/workflows/publish-swagger-spec.yml rename to .github/workflows/publish-openapi-spec.yml index 9607206a..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: @@ -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 @@ -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/.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/.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/CHANGELOG.md b/CHANGELOG.md index d8ac53d5..309932c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,42 @@ 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). +# [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: + + * `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 + +* [#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. + +* [#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 +* [#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 + +### Fixed +* *Nothing* + + # [4.3.1] - 2024-11-25 ### Added * *Nothing* diff --git a/Dockerfile b/Dockerfile index 4f3d1ca6..eea707f5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM php:8.3-alpine3.20 AS base +FROM php:8.4-alpine3.21 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/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 68f38f18..f9133a5c 100644 --- a/composer.json +++ b/composer.json @@ -12,56 +12,56 @@ } ], "require": { - "php": "^8.2", + "php": "^8.3", "ext-curl": "*", "ext-gd": "*", "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", "doctrine/orm": "^3.3", + "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", "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", + "shlinkio/doctrine-specification": "^2.2", "shlinkio/shlink-common": "^6.6", "shlinkio/shlink-config": "^3.4", "shlinkio/shlink-event-dispatcher": "^4.1", - "shlinkio/shlink-importer": "^5.3.2", - "shlinkio/shlink-installer": "^9.3", + "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", - "symfony/console": "^7.1", - "symfony/filesystem": "^7.1", - "symfony/lock": "^7.1", - "symfony/process": "^7.1", - "symfony/string": "^7.1" + "spiral/roadrunner-jobs": "^4.6", + "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.0.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": { @@ -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-openapi validate docs/swagger/swagger.json", - "swagger:inline": "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/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/config/autoload/installer.global.php b/config/autoload/installer.global.php index 898e5ef5..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, @@ -41,7 +42,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, diff --git a/data/infra/php.Dockerfile b/data/infra/php.Dockerfile index e594664b..0c08124f 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.21 MAINTAINER Alejandro Celaya ENV APCU_VERSION 5.1.24 diff --git a/data/infra/roadrunner.Dockerfile b/data/infra/roadrunner.Dockerfile index 198a6867..3619aba3 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.21 MAINTAINER Alejandro Celaya ENV PDO_SQLSRV_VERSION 5.12.0 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/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/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..cd7c4ffb 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -4,10 +4,11 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; -use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; use Shlinkio\Shlink\CLI\Util\ExitCode; +use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface; +use Shlinkio\Shlink\Core\Geolocation\GeolocationResult; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputInterface; @@ -16,13 +17,14 @@ use Symfony\Component\Console\Style\SymfonyStyle; use function sprintf; -class DownloadGeoLiteDbCommand extends Command +class DownloadGeoLiteDbCommand extends Command implements GeolocationDownloadProgressHandlerInterface { - public const NAME = 'visit:download-db'; + 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,38 +41,42 @@ 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 ($result === GeolocationResult::MAX_ERRORS_REACHED) { + $this->io->warning('Max consecutive errors reached. Cannot retry for a couple of days.'); + 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) { - $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); } } private function processGeoLiteUpdateError(GeolocationDbUpdateFailedException $e, SymfonyStyle $io): int { - $olderDbExists = $e->olderDbExists(); + $olderDbExists = $e->olderDbExists; if ($olderDbExists) { $io->warning( @@ -86,4 +92,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/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/Exception/GeolocationDbUpdateFailedException.php b/module/CLI/src/Exception/GeolocationDbUpdateFailedException.php deleted file mode 100644 index ee31ac82..00000000 --- a/module/CLI/src/Exception/GeolocationDbUpdateFailedException.php +++ /dev/null @@ -1,58 +0,0 @@ -olderDbExists = true; - - return $e; - } - - public static function withoutOlderDb(Throwable|null $prev = null): self - { - $e = new self( - 'An error occurred while updating geolocation database, and an older version could not be found.', - $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/CLI/src/GeoLite/GeolocationDbUpdater.php b/module/CLI/src/GeoLite/GeolocationDbUpdater.php deleted file mode 100644 index 2a0fda3b..00000000 --- a/module/CLI/src/GeoLite/GeolocationDbUpdater.php +++ /dev/null @@ -1,137 +0,0 @@ -geoLiteDbReaderFactory = $geoLiteDbReaderFactory(...); - } - - /** - * @throws GeolocationDbUpdateFailedException - */ - public function checkDbUpdate( - callable|null $beforeDownload = null, - callable|null $handleProgress = null, - ): GeolocationResult { - if (! $this->trackingOptions->isGeolocationRelevant()) { - return GeolocationResult::CHECK_SKIPPED; - } - - $lock = $this->locker->createLock(self::LOCK_NAME); - $lock->acquire(true); // Block until lock is released - - try { - return $this->downloadIfNeeded($beforeDownload, $handleProgress); - } finally { - $lock->release(); - } - } - - /** - * @throws GeolocationDbUpdateFailedException - */ - private function downloadIfNeeded(callable|null $beforeDownload, callable|null $handleProgress): GeolocationResult - { - if (! $this->dbUpdater->databaseFileExists()) { - return $this->downloadNewDb(false, $beforeDownload, $handleProgress); - } - - $meta = ($this->geoLiteDbReaderFactory)()->metadata(); - if ($this->buildIsTooOld($meta)) { - return $this->downloadNewDb(true, $beforeDownload, $handleProgress); - } - - return GeolocationResult::DB_IS_UP_TO_DATE; - } - - private function buildIsTooOld(Metadata $meta): bool - { - $buildTimestamp = $this->resolveBuildTimestamp($meta); - $buildDate = Chronos::createFromTimestamp($buildTimestamp); - - 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; - } - - $intBuildEpoch = (int) $buildEpoch; - if ($buildEpoch === (string) $intBuildEpoch) { - return $intBuildEpoch; - } - - throw GeolocationDbUpdateFailedException::withInvalidEpochInOldDb($buildEpoch); - } - - /** - * @throws GeolocationDbUpdateFailedException - */ - private function downloadNewDb( - bool $olderDbExists, - callable|null $beforeDownload, - callable|null $handleProgress, - ): GeolocationResult { - if ($beforeDownload !== null) { - $beforeDownload($olderDbExists); - } - - try { - $this->dbUpdater->downloadFreshCopy($this->wrapHandleProgressCallback($handleProgress, $olderDbExists)); - return $olderDbExists ? GeolocationResult::DB_UPDATED : GeolocationResult::DB_CREATED; - } catch (MissingLicenseException) { - return GeolocationResult::LICENSE_MISSING; - } catch (DbUpdateException | WrongIpException $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/CLI/src/GeoLite/GeolocationResult.php b/module/CLI/src/GeoLite/GeolocationResult.php deleted file mode 100644 index 85976886..00000000 --- a/module/CLI/src/GeoLite/GeolocationResult.php +++ /dev/null @@ -1,12 +0,0 @@ - %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/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/DownloadGeoLiteDbCommandTest.php b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php index 4d2754f8..01322f0b 100644 --- a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php +++ b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php @@ -6,13 +6,15 @@ 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; -use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; use Shlinkio\Shlink\CLI\Util\ExitCode; +use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface; +use Shlinkio\Shlink\Core\Geolocation\GeolocationResult; use ShlinkioTest\Shlink\CLI\Util\CliTestUtils; use Symfony\Component\Console\Tester\CommandTester; @@ -36,9 +38,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() @@ -73,17 +75,18 @@ 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'])] + #[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( - 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); } @@ -105,8 +108,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.']; } 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/Exception/GeolocationDbUpdateFailedExceptionTest.php b/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php index 519ddf02..86ed9548 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 @@ -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/CLI/test/GeoLite/GeolocationDbUpdaterTest.php b/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php deleted file mode 100644 index 038d570c..00000000 --- a/module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php +++ /dev/null @@ -1,199 +0,0 @@ -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); - } - - #[Test] - public function properResultIsReturnedWhenLicenseIsMissing(): void - { - $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(false); - $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->willThrowException( - new MissingLicenseException(''), - ); - $this->geoLiteDbReader->expects($this->never())->method('metadata'); - - $isCalled = false; - $result = $this->geolocationDbUpdater()->checkDbUpdate(function () use (&$isCalled): void { - $isCalled = true; - }); - - self::assertTrue($isCalled); - self::assertEquals(GeolocationResult::LICENSE_MISSING, $result); - } - - #[Test] - public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(): void - { - $prev = new DbUpdateException(''); - - $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(false); - $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->with( - $this->isNull(), - )->willThrowException($prev); - $this->geoLiteDbReader->expects($this->never())->method('metadata'); - - $isCalled = false; - try { - $this->geolocationDbUpdater()->checkDbUpdate(function () use (&$isCalled): void { - $isCalled = true; - }); - self::fail(); - } catch (Throwable $e) { - self::assertInstanceOf(GeolocationDbUpdateFailedException::class, $e); - self::assertSame($prev, $e->getPrevious()); - self::assertFalse($e->olderDbExists()); - self::assertTrue($isCalled); - } - } - - #[Test, DataProvider('provideBigDays')] - public function exceptionIsThrownWhenOlderDbIsTooOldAndDownloadFails(int $days): void - { - $prev = new DbUpdateException(''); - $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(true); - $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->with( - $this->isNull(), - )->willThrowException($prev); - $this->geoLiteDbReader->expects($this->once())->method('metadata')->with()->willReturn( - $this->buildMetaWithBuildEpoch(Chronos::now()->subDays($days)->getTimestamp()), - ); - - try { - $this->geolocationDbUpdater()->checkDbUpdate(); - self::fail(); - } catch (Throwable $e) { - self::assertInstanceOf(GeolocationDbUpdateFailedException::class, $e); - self::assertSame($prev, $e->getPrevious()); - self::assertTrue($e->olderDbExists()); - } - } - - public static function provideBigDays(): iterable - { - yield [36]; - yield [50]; - yield [75]; - yield [100]; - } - - #[Test, DataProvider('provideSmallDays')] - public function databaseIsNotUpdatedIfItIsNewEnough(string|int $buildEpoch): 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), - ); - - $result = $this->geolocationDbUpdater()->checkDbUpdate(); - - self::assertEquals(GeolocationResult::DB_IS_UP_TO_DATE, $result); - } - - public static function provideSmallDays(): 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)); - } - - #[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'), - ); - - $this->expectException(GeolocationDbUpdateFailedException::class); - $this->expectExceptionMessage( - 'Build epoch with value "invalid" from existing geolocation database, could not be parsed to integer.', - ); - - $this->geolocationDbUpdater()->checkDbUpdate(); - } - - private function buildMetaWithBuildEpoch(string|int $buildEpoch): Metadata - { - 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, - ]); - } - - #[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'); - - self::assertEquals(GeolocationResult::CHECK_SKIPPED, $result); - } - - public static function provideTrackingOptions(): iterable - { - yield 'disableTracking' => [new TrackingOptions(disableTracking: true)]; - yield 'disableIpTracking' => [new TrackingOptions(disableIpTracking: true)]; - yield 'both' => [new TrackingOptions(disableTracking: true, disableIpTracking: true)]; - } - - private function geolocationDbUpdater(TrackingOptions|null $options = null): GeolocationDbUpdater - { - $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(), - ); - } -} 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/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 4844e6d5..eda556e9 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -9,12 +9,15 @@ 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\Resolver\IpLocationResolverInterface; use Symfony\Component\Lock; +use const Shlinkio\Shlink\LOCAL_LOCK_FACTORY; + return [ 'dependencies' => [ @@ -103,6 +106,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 +244,12 @@ return [ EventDispatcher\PublishingUpdatesGenerator::class => [ShortUrl\Transformer\ShortUrlDataTransformer::class], + GeolocationDbUpdater::class => [ + DbUpdater::class, + LOCAL_LOCK_FACTORY, + Config\Options\TrackingOptions::class, + 'em', + ], Geolocation\Middleware\IpGeolocationMiddleware::class => [ IpLocationResolverInterface::class, DbUpdater::class, @@ -252,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/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..7b60abdc --- /dev/null +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Geolocation.Entity.GeolocationDbUpdate.php @@ -0,0 +1,63 @@ +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('error', Types::STRING), $emConfig) + ->columnName('error') + ->length(1024) + ->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) + ->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 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/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/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/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; - } -} 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 adceb7f2..b19176ef 100644 --- a/module/Core/migrations/Version20240220214031.php +++ b/module/Core/migrations/Version20240220214031.php @@ -17,10 +17,13 @@ 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, - 'device_long_urls' => ['long_url'], '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/migrations/Version20241212131058.php b/module/Core/migrations/Version20241212131058.php new file mode 100644 index 00000000..23e61803 --- /dev/null +++ b/module/Core/migrations/Version20241212131058.php @@ -0,0 +1,64 @@ +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('reason', Types::STRING, ['length' => 1024]); + $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 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 + { + $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/Action/Model/QrCodeParams.php b/module/Core/src/Action/Model/QrCodeParams.php index 3be9097e..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 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, - 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/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 6cdf6297..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'; @@ -84,7 +85,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 +93,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 +128,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, @@ -143,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(), 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/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/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/EventDispatcher/UpdateGeoLiteDb.php b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php index 4e4720c5..8d288959 100644 --- a/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php +++ b/module/Core/src/EventDispatcher/UpdateGeoLiteDb.php @@ -6,13 +6,15 @@ namespace Shlinkio\Shlink\Core\EventDispatcher; use Psr\EventDispatcher\EventDispatcherInterface; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; use Shlinkio\Shlink\Core\EventDispatcher\Event\GeoLiteDbCreated; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface; +use Shlinkio\Shlink\Core\Geolocation\GeolocationResult; use Throwable; use function sprintf; +/** @todo Rename to UpdateGeolocationDb */ readonly class UpdateGeoLiteDb { public function __construct( @@ -24,21 +26,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/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/GeolocationDbUpdateFailedException.php b/module/Core/src/Exception/GeolocationDbUpdateFailedException.php new file mode 100644 index 00000000..a818b263 --- /dev/null +++ b/module/Core/src/Exception/GeolocationDbUpdateFailedException.php @@ -0,0 +1,34 @@ +dateUpdated = Chronos::now(); + $this->status = GeolocationDbUpdateStatus::SUCCESS; + return $this; + } + + public function finishWithError(string $error): self + { + $this->error = $error; + $this->dateUpdated = Chronos::now(); + $this->status = GeolocationDbUpdateStatus::ERROR; + return $this; + } + + /** + * @param positive-int $days + */ + public function isOlderThan(int $days): bool + { + return Chronos::now()->greaterThan($this->dateUpdated->addDays($days)); + } + + public function isInProgress(): bool + { + return $this->status === GeolocationDbUpdateStatus::IN_PROGRESS; + } + + public function isError(): bool + { + return $this->status === GeolocationDbUpdateStatus::ERROR; + } + + public function isSuccess(): bool + { + return $this->status === GeolocationDbUpdateStatus::SUCCESS; + } +} diff --git a/module/Core/src/Geolocation/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 @@ +trackingOptions->isGeolocationRelevant()) { + return GeolocationResult::CHECK_SKIPPED; + } + + + $lock = $this->locker->createLock(self::LOCK_NAME); + $lock->acquire(blocking: true); + + try { + return $this->downloadIfNeeded($downloadProgressHandler); + } finally { + $lock->release(); + } + } + + /** + * @throws GeolocationDbUpdateFailedException + */ + private function downloadIfNeeded( + GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler, + ): GeolocationResult { + $recentDownloads = $this->em->getRepository(GeolocationDbUpdate::class)->findBy( + criteria: ['filesystemId' => GeolocationDbUpdate::currentFilesystemId()], + orderBy: ['dateUpdated' => 'DESC'], + limit: $this->maxRecentAttemptsToCheck, + ); + $mostRecentDownload = $recentDownloads[0] ?? null; + + // 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::UPDATE_IN_PROGRESS; + } + + $amountOfErrorsSinceLastSuccess = 0; + foreach ($recentDownloads as $recentDownload) { + // Count attempts until a success is found + if ($recentDownload->isSuccess()) { + break; + } + $amountOfErrorsSinceLastSuccess++; + } + + // If max amount of consecutive errors has been reached and the most recent one is not old enough, skip download + // for 2 days to avoid hitting potential API limits in geolocation services + $lastAttemptIsError = $mostRecentDownload !== null && $mostRecentDownload->isError(); + // FIXME Once max errors are reached there will be one attempt every 2 days, but it should be 15 attempts every + // 2 days. Leaving like this for simplicity for now. + $maxConsecutiveErrorsReached = $amountOfErrorsSinceLastSuccess === $this->maxRecentAttemptsToCheck; + if ($lastAttemptIsError && $maxConsecutiveErrorsReached && ! $mostRecentDownload->isOlderThan(days: 2)) { + return GeolocationResult::MAX_ERRORS_REACHED; + } + + // Try to download if: + // - There are no attempts 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) + $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 is old enough'], + default => null, + }; + if ($reasonMatch !== null) { + [$olderDbExists, $reason] = $reasonMatch; + return $this->downloadAndTrackUpdate($downloadProgressHandler, $olderDbExists, $reason); + } + + return GeolocationResult::DB_IS_UP_TO_DATE; + } + + /** + * @throws GeolocationDbUpdateFailedException + */ + private function downloadAndTrackUpdate( + GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler, + bool $olderDbExists, + string $reason, + ): GeolocationResult { + $dbUpdate = GeolocationDbUpdate::withReason($reason); + $this->em->persist($dbUpdate); + $this->em->flush(); + + 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; + } catch (Throwable $e) { + $dbUpdate->finishWithError(sprintf('Unknown error: %s', $e->getMessage())); + throw $e; + } finally { + $this->em->flush(); + } + } + + /** + * @throws GeolocationDbUpdateFailedException + */ + private function downloadNewDb( + GeolocationDownloadProgressHandlerInterface|null $downloadProgressHandler, + bool $olderDbExists, + ): GeolocationResult { + $downloadProgressHandler?->beforeDownload($olderDbExists); + + try { + $this->dbUpdater->downloadFreshCopy( + static fn (int $total, int $downloaded) + => $downloadProgressHandler?->handleProgress($total, $downloaded, $olderDbExists), + ); + return $olderDbExists ? GeolocationResult::DB_UPDATED : GeolocationResult::DB_CREATED; + } catch (DbUpdateException $e) { + throw $olderDbExists + ? GeolocationDbUpdateFailedException::withOlderDb($e) + : GeolocationDbUpdateFailedException::withoutOlderDb($e); + } + } +} diff --git a/module/CLI/src/GeoLite/GeolocationDbUpdaterInterface.php b/module/Core/src/Geolocation/GeolocationDbUpdaterInterface.php similarity index 50% rename from module/CLI/src/GeoLite/GeolocationDbUpdaterInterface.php rename to module/Core/src/Geolocation/GeolocationDbUpdaterInterface.php index ba0f0e70..d7322e86 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 { @@ -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 @@ +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..aeb24cce 100644 --- a/module/Core/src/Importer/ShortUrlImporting.php +++ b/module/Core/src/Importer/ShortUrlImporting.php @@ -4,11 +4,18 @@ 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 count; +use function Shlinkio\Shlink\Core\ArrayUtils\map; use function Shlinkio\Shlink\Core\normalizeDate; use function sprintf; @@ -20,12 +27,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 +62,42 @@ final readonly class ShortUrlImporting : sprintf('Skipped. Imported %s visits', $importedVisits); } + /** + * @param ImportedShlinkRedirectRule[] $rules + */ + public function importRedirectRules( + array $rules, + EntityManagerInterface $em, + ShortUrlRedirectRuleServiceInterface $redirectRuleService, + ): void { + if ($this->isNew && count($rules) === 0) { + return; + } + + $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/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/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, }; } 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/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index cf1e134b..255cb19e 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; @@ -23,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, ) { @@ -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/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/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/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/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/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..3d3e7792 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.', @@ -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/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/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); } /** 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-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', ]; diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index f8dea217..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; @@ -32,8 +33,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; @@ -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 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/EventDispatcher/UpdateGeoLiteDbTest.php b/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php index 3b20ab0c..5288aa1b 100644 --- a/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php +++ b/module/Core/test/EventDispatcher/UpdateGeoLiteDbTest.php @@ -11,10 +11,11 @@ use PHPUnit\Framework\TestCase; use Psr\EventDispatcher\EventDispatcherInterface; use Psr\Log\LoggerInterface; use RuntimeException; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; -use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; use Shlinkio\Shlink\Core\EventDispatcher\Event\GeoLiteDbCreated; use Shlinkio\Shlink\Core\EventDispatcher\UpdateGeoLiteDb; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface; +use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface; +use Shlinkio\Shlink\Core\Geolocation\GeolocationResult; use function array_map; @@ -51,11 +52,11 @@ class UpdateGeoLiteDbTest extends TestCase } #[Test, DataProvider('provideFlags')] - public function noticeMessageIsPrintedWhenFirstCallbackIsInvoked(bool $oldDbExists, string $expectedMessage): void + public function noticeMessageIsPrintedWhenDownloadIsStarted(bool $oldDbExists, string $expectedMessage): void { $this->dbUpdater->expects($this->once())->method('checkDbUpdate')->withAnyParameters()->willReturnCallback( - function (callable $firstCallback) use ($oldDbExists): GeolocationResult { - $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; }, diff --git a/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php new file mode 100644 index 00000000..8c9f5a1b --- /dev/null +++ b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php @@ -0,0 +1,298 @@ + */ + 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->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, + 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] + 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 + { + $this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(false); + $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->willThrowException( + new MissingLicenseException(''), + ); + $this->repo->expects($this->once())->method('findBy')->willReturn([ + GeolocationDbUpdate::withReason('')->finishSuccessfully(), + ]); + + $result = $this->geolocationDbUpdater()->checkDbUpdate($this->progressHandler); + + self::assertTrue($this->progressHandler->beforeDownloadCalled); + self::assertEquals(GeolocationResult::LICENSE_MISSING, $result); + } + + #[Test, DataProvider('provideDbDoesNotExist')] + public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(Closure $setUp): void + { + $prev = new DbUpdateException(''); + + $expectedReason = $setUp($this); + + $this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->with( + $this->isInstanceOf(Closure::class), + )->willThrowException($prev); + $this->em->expects($this->once())->method('persist')->with($this->callback( + fn (GeolocationDbUpdate $newUpdate): bool => $newUpdate->reason === $expectedReason, + )); + + try { + $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($this->progressHandler->beforeDownloadCalled); + } + } + + 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 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->repo->expects($this->once())->method('findBy')->willReturn([self::createFinishedOldUpdate($days)]); + + try { + $this->geolocationDbUpdater()->checkDbUpdate(); + self::fail(); + } catch (Throwable $e) { + self::assertInstanceOf(GeolocationDbUpdateFailedException::class, $e); + self::assertSame($prev, $e->getPrevious()); + self::assertTrue($e->olderDbExists); + } + } + + public static function provideBigDays(): iterable + { + yield [31]; + yield [50]; + yield [75]; + yield [100]; + } + + #[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->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 provideNotAldEnoughDays(): iterable + { + return array_map(static fn (int $value) => [$value], range(0, 29)); + } + + #[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, + )); + + $result = $this->geolocationDbUpdater()->checkDbUpdate(); + + self::assertEquals($expectedResult, $result); + } + + public static function provideUpdatesThatWillDownload(): iterable + { + 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 + { + $this->dbUpdater->expects($this->never())->method('databaseFileExists'); + $this->em->expects($this->never())->method('getRepository'); + + $result = $this->geolocationDbUpdater($options)->checkDbUpdate(); + + self::assertEquals(GeolocationResult::CHECK_SKIPPED, $result); + } + + public static function provideTrackingOptions(): iterable + { + yield 'disableTracking' => [new TrackingOptions(disableTracking: true)]; + yield 'disableIpTracking' => [new TrackingOptions(disableIpTracking: true)]; + yield 'both' => [new TrackingOptions(disableTracking: true, disableIpTracking: true)]; + } + + private function geolocationDbUpdater(TrackingOptions|null $options = null): GeolocationDbUpdater + { + $locker = $this->createMock(Lock\LockFactory::class); + $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 36265aa3..e8d800a1 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -14,6 +14,10 @@ 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; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository; @@ -23,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; @@ -44,13 +50,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 +67,7 @@ class ImportedLinksProcessorTest extends TestCase new SimpleShortUrlRelationResolver(), $this->shortCodeHelper, $batchHelper, + $this->redirectRuleService, ); $this->io = $this->createMock(StyleInterface::class); @@ -67,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); @@ -82,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()); } @@ -239,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/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/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), ); } } 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 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/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', 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;