From 87d5f9bc750160690a7c8c847220b41f25931550 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 24 Mar 2025 19:33:52 +0100 Subject: [PATCH 01/40] Fix compatibility with PHPUnit 12.0.9 and phpstan-phpunit --- composer.json | 9 ++++----- .../RabbitMq/NotifyVisitToRabbitMqTest.php | 3 +++ .../Middleware/IpGeolocationMiddlewareTest.php | 3 +++ module/Core/test/Matomo/MatomoVisitSenderTest.php | 10 +++++++--- .../Helper/ShortUrlTitleResolutionHelperTest.php | 5 ++--- .../Core/test/Visit/Geolocation/VisitLocatorTest.php | 6 ++++++ 6 files changed, 25 insertions(+), 11 deletions(-) diff --git a/composer.json b/composer.json index fa8cc972..1123084d 100644 --- a/composer.json +++ b/composer.json @@ -65,20 +65,19 @@ "devster/ubench": "^2.1", "phpstan/phpstan": "^2.1", "phpstan/phpstan-doctrine": "^2.0", - "phpstan/phpstan-phpunit": "^2.0", + "phpstan/phpstan-phpunit": "^2.0.5", "phpstan/phpstan-symfony": "^2.0", "phpunit/php-code-coverage": "^12.0", "phpunit/phpcov": "^11.0", - "phpunit/phpunit": "^12.0", + "phpunit/phpunit": "^12.0.10", "roave/security-advisories": "dev-master", - "shlinkio/php-coding-standard": "~2.4.0", + "shlinkio/php-coding-standard": "~2.4.2", "shlinkio/shlink-test-utils": "^4.3.1", "symfony/var-dumper": "^7.2", "veewee/composer-run-parallel": "^1.4" }, "conflict": { - "symfony/var-exporter": ">=6.3.9,<=6.4.0", - "phpunit/phpunit": "12.0.9" + "symfony/var-exporter": ">=6.3.9,<=6.4.0" }, "autoload": { "psr-4": { diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php index a3cebaa7..8af719b3 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php @@ -70,6 +70,9 @@ class NotifyVisitToRabbitMqTest extends TestCase ($this->listener())(new UrlVisited($visitId)); } + /** + * @param non-empty-string[] $expectedChannels + */ #[Test, DataProvider('provideVisits')] public function expectedChannelsAreNotifiedBasedOnTheVisitType(Visit $visit, array $expectedChannels): void { diff --git a/module/Core/test/Geolocation/Middleware/IpGeolocationMiddlewareTest.php b/module/Core/test/Geolocation/Middleware/IpGeolocationMiddlewareTest.php index 210fb46f..96eb158f 100644 --- a/module/Core/test/Geolocation/Middleware/IpGeolocationMiddlewareTest.php +++ b/module/Core/test/Geolocation/Middleware/IpGeolocationMiddlewareTest.php @@ -118,6 +118,9 @@ class IpGeolocationMiddlewareTest extends TestCase $this->middleware()->process($request, $this->handler); } + /** + * @param non-empty-string $loggerMethod + */ #[Test] #[TestWith([ new WrongIpException(), diff --git a/module/Core/test/Matomo/MatomoVisitSenderTest.php b/module/Core/test/Matomo/MatomoVisitSenderTest.php index 7d66d868..1b1b303c 100644 --- a/module/Core/test/Matomo/MatomoVisitSenderTest.php +++ b/module/Core/test/Matomo/MatomoVisitSenderTest.php @@ -24,6 +24,8 @@ use Shlinkio\Shlink\Core\Visit\Model\Visitor; use Shlinkio\Shlink\Core\Visit\Repository\VisitIterationRepositoryInterface; use Shlinkio\Shlink\IpGeolocation\Model\Location; +use function array_values; + class MatomoVisitSenderTest extends TestCase { private MockObject & MatomoTrackerBuilderInterface $trackerBuilder; @@ -42,10 +44,10 @@ class MatomoVisitSenderTest extends TestCase ); } - #[Test, DataProvider('provideTrackerMethods')] /** - * @param array $invokedMethods + * @param array $invokedMethods */ + #[Test, DataProvider('provideTrackerMethods')] public function visitIsSentToMatomo(Visit $visit, string|null $originalIpAddress, array $invokedMethods): void { $tracker = $this->createMock(MatomoTracker::class); @@ -70,7 +72,9 @@ class MatomoVisitSenderTest extends TestCase } foreach ($invokedMethods as $invokedMethod => $args) { - $tracker->expects($this->once())->method($invokedMethod)->with(...$args)->willReturn($tracker); + $tracker->expects($this->once())->method($invokedMethod)->with(...array_values($args))->willReturn( + $tracker, + ); } $this->trackerBuilder->expects($this->once())->method('buildMatomoTracker')->willReturn($tracker); diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php index 787ca294..1c55bc0d 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php @@ -13,7 +13,7 @@ use Laminas\Diactoros\Response\JsonResponse; use Laminas\Diactoros\Stream; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\TestWith; -use PHPUnit\Framework\MockObject\Builder\InvocationMocker; +use PHPUnit\Framework\MockObject\InvocationStubber; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; @@ -152,9 +152,8 @@ class ShortUrlTitleResolutionHelperTest extends TestCase } /** - * @return InvocationMocker */ - private function expectRequestToBeCalled(): InvocationMocker + private function expectRequestToBeCalled(): InvocationStubber { return $this->httpClient->expects($this->once())->method('request')->with( RequestMethodInterface::METHOD_GET, diff --git a/module/Core/test/Visit/Geolocation/VisitLocatorTest.php b/module/Core/test/Visit/Geolocation/VisitLocatorTest.php index cd6f12da..68c746d7 100644 --- a/module/Core/test/Visit/Geolocation/VisitLocatorTest.php +++ b/module/Core/test/Visit/Geolocation/VisitLocatorTest.php @@ -40,6 +40,9 @@ class VisitLocatorTest extends TestCase $this->visitService = new VisitLocator($this->em, $this->repo); } + /** + * @param non-empty-string $expectedRepoMethodName + */ #[Test, DataProvider('provideMethodNames')] public function locateVisitsIteratesAndLocatesExpectedVisits( string $serviceMethodName, @@ -80,6 +83,9 @@ class VisitLocatorTest extends TestCase yield 'locateAllVisits' => ['locateAllVisits', 'findAllVisits']; } + /** + * @param non-empty-string $expectedRepoMethodName + */ #[Test, DataProvider('provideIsNonLocatableAddress')] public function visitsWhichCannotBeLocatedAreIgnoredOrLocatedAsEmpty( string $serviceMethodName, From b15e832cf4d388b8642cfe186a3f165654224863 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 22 Apr 2025 08:41:58 +0200 Subject: [PATCH 02/40] Deprecate QR code generation endpoint --- CHANGELOG.md | 18 +++++++++++++ composer.json | 2 +- config/constants.php | 25 +++++++++++------ docs/swagger/paths/{shortCode}_qr-code.json | 5 ++-- module/Core/src/Action/Model/QrCodeParams.php | 1 + module/Core/src/Action/QrCodeAction.php | 1 + module/Core/src/Config/EnvVars.php | 27 ++++++++++++------- .../Core/src/Config/Options/QrCodeOptions.php | 1 + module/Core/test-api/Action/QrCodeTest.php | 1 + module/Core/test/Action/QrCodeActionTest.php | 25 ----------------- 10 files changed, 61 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 818a25f1..2807ddc0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,24 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). + +## [Unreleased] +### Added +* *Nothing* + +### Changed +* *Nothing* + +### Deprecated +* [#2408](https://github.com/shlinkio/shlink/issues/2408) Generating QR codes via `/{short-code}/qr-code` is now deprecated and will be removed in Shlink 5.0. Use the equivalent capability from web clients instead. + +### Removed +* *Nothing* + +### Fixed +* *Nothing* + + ## [4.4.6] - 2025-03-20 ### Added * *Nothing* diff --git a/composer.json b/composer.json index 1123084d..e3548394 100644 --- a/composer.json +++ b/composer.json @@ -43,7 +43,7 @@ "pagerfanta/core": "^3.8", "ramsey/uuid": "^4.7", "shlinkio/doctrine-specification": "^2.2", - "shlinkio/shlink-common": "^7.0", + "shlinkio/shlink-common": "dev-main#e601317 as 7.1", "shlinkio/shlink-config": "^4.0", "shlinkio/shlink-event-dispatcher": "^4.2", "shlinkio/shlink-importer": "^5.6", diff --git a/config/constants.php b/config/constants.php index 09df0e60..f5dff4d5 100644 --- a/config/constants.php +++ b/config/constants.php @@ -13,13 +13,22 @@ const DEFAULT_REDIRECT_STATUS_CODE = RedirectStatus::STATUS_302; const DEFAULT_REDIRECT_CACHE_LIFETIME = 30; const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory'; const LOOSE_URI_MATCHER = '/(.+)\:(.+)/i'; // Matches anything starting with a schema. -const DEFAULT_QR_CODE_SIZE = 300; -const DEFAULT_QR_CODE_MARGIN = 0; -const DEFAULT_QR_CODE_FORMAT = 'png'; -const DEFAULT_QR_CODE_ERROR_CORRECTION = 'l'; -const DEFAULT_QR_CODE_ROUND_BLOCK_SIZE = true; -const DEFAULT_QR_CODE_ENABLED_FOR_DISABLED_SHORT_URLS = true; -const DEFAULT_QR_CODE_COLOR = '#000000'; // Black -const DEFAULT_QR_CODE_BG_COLOR = '#ffffff'; // White const IP_ADDRESS_REQUEST_ATTRIBUTE = 'remote_address'; const REDIRECT_URL_REQUEST_ATTRIBUTE = 'redirect_url'; + +/** @deprecated */ +const DEFAULT_QR_CODE_SIZE = 300; +/** @deprecated */ +const DEFAULT_QR_CODE_MARGIN = 0; +/** @deprecated */ +const DEFAULT_QR_CODE_FORMAT = 'png'; +/** @deprecated */ +const DEFAULT_QR_CODE_ERROR_CORRECTION = 'l'; +/** @deprecated */ +const DEFAULT_QR_CODE_ROUND_BLOCK_SIZE = true; +/** @deprecated */ +const DEFAULT_QR_CODE_ENABLED_FOR_DISABLED_SHORT_URLS = true; +/** @deprecated */ +const DEFAULT_QR_CODE_COLOR = '#000000'; // Black +/** @deprecated */ +const DEFAULT_QR_CODE_BG_COLOR = '#ffffff'; // White diff --git a/docs/swagger/paths/{shortCode}_qr-code.json b/docs/swagger/paths/{shortCode}_qr-code.json index bb95f7ef..dc0f2d81 100644 --- a/docs/swagger/paths/{shortCode}_qr-code.json +++ b/docs/swagger/paths/{shortCode}_qr-code.json @@ -1,11 +1,12 @@ { "get": { + "deprecated": true, "operationId": "shortUrlQrCode", "tags": [ "URL Shortener" ], - "summary": "Short URL QR code", - "description": "Generates a QR code image pointing to a short URL.
Since this is not an API endpoint but an image one, when an invalid value is provided for any of the query params, they will fall to their default values instead of throwing an error.", + "summary": "[Deprecated] Short URL QR code", + "description": "**[Deprecated]** Use an external mechanism to generate QR codes. Shlink dashboard and shlink-web-client provide their own.", "parameters": [ { "$ref": "../parameters/shortCode.json" diff --git a/module/Core/src/Action/Model/QrCodeParams.php b/module/Core/src/Action/Model/QrCodeParams.php index 3b25b611..9bdaf87e 100644 --- a/module/Core/src/Action/Model/QrCodeParams.php +++ b/module/Core/src/Action/Model/QrCodeParams.php @@ -28,6 +28,7 @@ use function trim; use const Shlinkio\Shlink\DEFAULT_QR_CODE_BG_COLOR; use const Shlinkio\Shlink\DEFAULT_QR_CODE_COLOR; +/** @deprecated */ final readonly class QrCodeParams { private const int MIN_SIZE = 50; diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index 889d1d04..0ab126bc 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -19,6 +19,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; +/** @deprecated */ readonly class QrCodeAction implements MiddlewareInterface { public function __construct( diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index f8042feb..2a835932 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -56,15 +56,6 @@ enum EnvVars: string case MATOMO_BASE_URL = 'MATOMO_BASE_URL'; case MATOMO_SITE_ID = 'MATOMO_SITE_ID'; case MATOMO_API_TOKEN = 'MATOMO_API_TOKEN'; - case DEFAULT_QR_CODE_SIZE = 'DEFAULT_QR_CODE_SIZE'; - case DEFAULT_QR_CODE_MARGIN = 'DEFAULT_QR_CODE_MARGIN'; - case DEFAULT_QR_CODE_FORMAT = 'DEFAULT_QR_CODE_FORMAT'; - case DEFAULT_QR_CODE_ERROR_CORRECTION = 'DEFAULT_QR_CODE_ERROR_CORRECTION'; - case DEFAULT_QR_CODE_ROUND_BLOCK_SIZE = 'DEFAULT_QR_CODE_ROUND_BLOCK_SIZE'; - case QR_CODE_FOR_DISABLED_SHORT_URLS = 'QR_CODE_FOR_DISABLED_SHORT_URLS'; - case DEFAULT_QR_CODE_COLOR = 'DEFAULT_QR_CODE_COLOR'; - case DEFAULT_QR_CODE_BG_COLOR = 'DEFAULT_QR_CODE_BG_COLOR'; - case DEFAULT_QR_CODE_LOGO_URL = 'DEFAULT_QR_CODE_LOGO_URL'; case DEFAULT_INVALID_SHORT_URL_REDIRECT = 'DEFAULT_INVALID_SHORT_URL_REDIRECT'; case DEFAULT_REGULAR_404_REDIRECT = 'DEFAULT_REGULAR_404_REDIRECT'; case DEFAULT_BASE_URL_REDIRECT = 'DEFAULT_BASE_URL_REDIRECT'; @@ -95,6 +86,24 @@ enum EnvVars: string case SKIP_INITIAL_GEOLITE_DOWNLOAD = 'SKIP_INITIAL_GEOLITE_DOWNLOAD'; /** @deprecated Use REDIRECT_EXTRA_PATH */ case REDIRECT_APPEND_EXTRA_PATH = 'REDIRECT_APPEND_EXTRA_PATH'; + /** @deprecated */ + case DEFAULT_QR_CODE_SIZE = 'DEFAULT_QR_CODE_SIZE'; + /** @deprecated */ + case DEFAULT_QR_CODE_MARGIN = 'DEFAULT_QR_CODE_MARGIN'; + /** @deprecated */ + case DEFAULT_QR_CODE_FORMAT = 'DEFAULT_QR_CODE_FORMAT'; + /** @deprecated */ + case DEFAULT_QR_CODE_ERROR_CORRECTION = 'DEFAULT_QR_CODE_ERROR_CORRECTION'; + /** @deprecated */ + case DEFAULT_QR_CODE_ROUND_BLOCK_SIZE = 'DEFAULT_QR_CODE_ROUND_BLOCK_SIZE'; + /** @deprecated */ + case QR_CODE_FOR_DISABLED_SHORT_URLS = 'QR_CODE_FOR_DISABLED_SHORT_URLS'; + /** @deprecated */ + case DEFAULT_QR_CODE_COLOR = 'DEFAULT_QR_CODE_COLOR'; + /** @deprecated */ + case DEFAULT_QR_CODE_BG_COLOR = 'DEFAULT_QR_CODE_BG_COLOR'; + /** @deprecated */ + case DEFAULT_QR_CODE_LOGO_URL = 'DEFAULT_QR_CODE_LOGO_URL'; public function loadFromEnv(): mixed { diff --git a/module/Core/src/Config/Options/QrCodeOptions.php b/module/Core/src/Config/Options/QrCodeOptions.php index ac864851..e36a4285 100644 --- a/module/Core/src/Config/Options/QrCodeOptions.php +++ b/module/Core/src/Config/Options/QrCodeOptions.php @@ -15,6 +15,7 @@ use const Shlinkio\Shlink\DEFAULT_QR_CODE_MARGIN; use const Shlinkio\Shlink\DEFAULT_QR_CODE_ROUND_BLOCK_SIZE; use const Shlinkio\Shlink\DEFAULT_QR_CODE_SIZE; +/** @deprecated */ final readonly class QrCodeOptions { public function __construct( diff --git a/module/Core/test-api/Action/QrCodeTest.php b/module/Core/test-api/Action/QrCodeTest.php index 21fd5147..c8285198 100644 --- a/module/Core/test-api/Action/QrCodeTest.php +++ b/module/Core/test-api/Action/QrCodeTest.php @@ -7,6 +7,7 @@ namespace ShlinkioApiTest\Shlink\Core\Action; use PHPUnit\Framework\Attributes\Test; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; +/** @deprecated */ class QrCodeTest extends ApiTestCase { #[Test] diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 4e987277..5ea4cc47 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -9,7 +9,6 @@ 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; @@ -286,30 +285,6 @@ class QrCodeActionTest extends TestCase ); } - #[Test] - #[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's logo - $code = 'abc123'; - $req = ServerRequestFactory::fromGlobals() - ->withAttribute('shortCode', $code) - ->withQueryParams($query); - - $this->urlResolver->method('resolveEnabledShortUrl')->willReturn(ShortUrl::withLongUrl('https://shlink.io')); - $handler = $this->createMock(RequestHandlerInterface::class); - - $resp = $this->action(new QrCodeOptions(size: 250, logoUrl: $logoUrl))->process($req, $handler); - $image = imagecreatefromstring($resp->getBody()->__toString()); - self::assertNotFalse($image); - - // At around 100x100 px we can already find the logo, if present - $resultingColor = imagecolorat($image, 100, 100); - self::assertEquals(hexdec($expectedColor), $resultingColor); - } - public static function provideEnabled(): iterable { yield 'always enabled' => [true]; From 71a3b993b199a05c67a6308ff9e0f6b8425536f5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 22 Apr 2025 09:09:52 +0200 Subject: [PATCH 03/40] Remove references to bootstrap from error templates --- CHANGELOG.md | 2 +- module/Core/templates/404.html | 12 ++++++++---- module/Core/templates/invalid-short-code.html | 12 ++++++++---- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2807ddc0..6f791119 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * *Nothing* ### Changed -* *Nothing* +* [#2406](https://github.com/shlinkio/shlink/issues/2406) Remove references to bootstrap from error templates, and instead inline the very minimum required styles. ### Deprecated * [#2408](https://github.com/shlinkio/shlink/issues/2408) Generating QR codes via `/{short-code}/qr-code` is now deprecated and will be removed in Shlink 5.0. Use the equivalent capability from web clients instead. diff --git a/module/Core/templates/404.html b/module/Core/templates/404.html index 93e6fb64..d9d972bb 100644 --- a/module/Core/templates/404.html +++ b/module/Core/templates/404.html @@ -5,13 +5,17 @@ - diff --git a/module/Core/templates/invalid-short-code.html b/module/Core/templates/invalid-short-code.html index 61c5a804..094b0db5 100644 --- a/module/Core/templates/invalid-short-code.html +++ b/module/Core/templates/invalid-short-code.html @@ -5,13 +5,17 @@ - From cea8a982e2974bd70c86b289f92cdc00eb9d78c6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 22 Apr 2025 12:07:41 +0200 Subject: [PATCH 04/40] Replace ExitCode with standard symfony Command constants --- module/CLI/src/Command/Api/DisableKeyCommand.php | 7 +++---- module/CLI/src/Command/Api/GenerateKeyCommand.php | 3 +-- .../CLI/src/Command/Api/InitialApiKeyCommand.php | 3 +-- module/CLI/src/Command/Api/ListKeysCommand.php | 3 +-- .../CLI/src/Command/Api/RenameApiKeyCommand.php | 3 +-- .../CLI/src/Command/Config/ReadEnvVarCommand.php | 3 +-- .../CLI/src/Command/Db/CreateDatabaseCommand.php | 5 ++--- .../CLI/src/Command/Db/MigrateDatabaseCommand.php | 3 +-- .../src/Command/Domain/DomainRedirectsCommand.php | 3 +-- .../CLI/src/Command/Domain/ListDomainsCommand.php | 3 +-- .../Integration/MatomoSendVisitsCommand.php | 7 +++---- .../RedirectRule/ManageRedirectRulesCommand.php | 5 ++--- .../Command/ShortUrl/CreateShortUrlCommand.php | 5 ++--- .../ShortUrl/DeleteExpiredShortUrlsCommand.php | 7 +++---- .../Command/ShortUrl/DeleteShortUrlCommand.php | 7 +++---- .../ShortUrl/DeleteShortUrlVisitsCommand.php | 5 ++--- .../src/Command/ShortUrl/EditShortUrlCommand.php | 5 ++--- .../src/Command/ShortUrl/ListShortUrlsCommand.php | 3 +-- .../src/Command/ShortUrl/ResolveUrlCommand.php | 5 ++--- module/CLI/src/Command/Tag/DeleteTagsCommand.php | 5 ++--- module/CLI/src/Command/Tag/ListTagsCommand.php | 3 +-- module/CLI/src/Command/Tag/RenameTagCommand.php | 5 ++--- .../src/Command/Util/AbstractLockedCommand.php | 3 +-- .../Command/Visit/AbstractDeleteVisitsCommand.php | 3 +-- .../Command/Visit/AbstractVisitsListCommand.php | 3 +-- .../Command/Visit/DeleteOrphanVisitsCommand.php | 3 +-- .../Command/Visit/DownloadGeoLiteDbCommand.php | 11 +++++------ .../CLI/src/Command/Visit/LocateVisitsCommand.php | 7 +++---- module/CLI/src/Util/ExitCode.php | 12 ------------ .../CLI/test-cli/Command/CreateShortUrlTest.php | 4 ++-- .../CLI/test-cli/Command/GenerateApiKeyTest.php | 4 ++-- module/CLI/test-cli/Command/ListApiKeysTest.php | 4 ++-- .../test/Command/Api/DisableKeyCommandTest.php | 14 +++++++------- .../test/Command/Api/GenerateKeyCommandTest.php | 4 ++-- .../Command/Domain/ListDomainsCommandTest.php | 4 ++-- .../Integration/MatomoSendVisitsCommandTest.php | 6 +++--- .../ManageRedirectRulesCommandTest.php | 8 ++++---- .../ShortUrl/CreateShortUrlCommandTest.php | 10 +++++----- .../DeleteExpiredShortUrlsCommandTest.php | 8 ++++---- .../ShortUrl/DeleteShortUrlVisitsCommandTest.php | 8 ++++---- .../Command/ShortUrl/EditShortUrlCommandTest.php | 6 +++--- .../Visit/DeleteOrphanVisitsCommandTest.php | 4 ++-- .../Visit/DownloadGeoLiteDbCommandTest.php | 10 +++++----- .../Command/Visit/LocateVisitsCommandTest.php | 15 +++++++-------- 44 files changed, 104 insertions(+), 145 deletions(-) delete mode 100644 module/CLI/src/Util/ExitCode.php diff --git a/module/CLI/src/Command/Api/DisableKeyCommand.php b/module/CLI/src/Command/Api/DisableKeyCommand.php index 93178bb5..52169b26 100644 --- a/module/CLI/src/Command/Api/DisableKeyCommand.php +++ b/module/CLI/src/Command/Api/DisableKeyCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Api; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; @@ -89,7 +88,7 @@ class DisableKeyCommand extends Command if (! $keyOrName) { $io->warning('An API key name was not provided.'); - return ExitCode::EXIT_WARNING; + return Command::INVALID; } try { @@ -99,10 +98,10 @@ class DisableKeyCommand extends Command $this->apiKeyService->disableByKey($keyOrName); } $io->success(sprintf('API key "%s" properly disabled', $keyOrName)); - return ExitCode::EXIT_SUCCESS; + return Command::SUCCESS; } catch (InvalidArgumentException $e) { $io->error($e->getMessage()); - return ExitCode::EXIT_FAILURE; + return Command::FAILURE; } } } diff --git a/module/CLI/src/Command/Api/GenerateKeyCommand.php b/module/CLI/src/Command/Api/GenerateKeyCommand.php index 2790fa8c..0c4afcd0 100644 --- a/module/CLI/src/Command/Api/GenerateKeyCommand.php +++ b/module/CLI/src/Command/Api/GenerateKeyCommand.php @@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\CLI\Command\Api; use Cake\Chronos\Chronos; use Shlinkio\Shlink\CLI\ApiKey\RoleResolverInterface; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Role; @@ -123,6 +122,6 @@ class GenerateKeyCommand extends Command ); } - return ExitCode::EXIT_SUCCESS; + return Command::SUCCESS; } } diff --git a/module/CLI/src/Command/Api/InitialApiKeyCommand.php b/module/CLI/src/Command/Api/InitialApiKeyCommand.php index e6515bc3..66968eb3 100644 --- a/module/CLI/src/Command/Api/InitialApiKeyCommand.php +++ b/module/CLI/src/Command/Api/InitialApiKeyCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Api; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -38,6 +37,6 @@ class InitialApiKeyCommand extends Command $output->writeln('Other API keys already exist. Initial API key creation skipped.'); } - return ExitCode::EXIT_SUCCESS; + return Command::SUCCESS; } } diff --git a/module/CLI/src/Command/Api/ListKeysCommand.php b/module/CLI/src/Command/Api/ListKeysCommand.php index 69870a9b..7d63b6a4 100644 --- a/module/CLI/src/Command/Api/ListKeysCommand.php +++ b/module/CLI/src/Command/Api/ListKeysCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Api; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Rest\ApiKey\Role; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -73,7 +72,7 @@ class ListKeysCommand extends Command 'Roles', ]), $rows); - return ExitCode::EXIT_SUCCESS; + return Command::SUCCESS; } private function determineMessagePattern(ApiKey $apiKey): string diff --git a/module/CLI/src/Command/Api/RenameApiKeyCommand.php b/module/CLI/src/Command/Api/RenameApiKeyCommand.php index eb662539..f21d125b 100644 --- a/module/CLI/src/Command/Api/RenameApiKeyCommand.php +++ b/module/CLI/src/Command/Api/RenameApiKeyCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Api; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\InvalidArgumentException; use Shlinkio\Shlink\Core\Model\Renaming; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -72,6 +71,6 @@ class RenameApiKeyCommand extends Command $this->apiKeyService->renameApiKey(Renaming::fromNames($oldName, $newName)); $io->success('API key properly renamed'); - return ExitCode::EXIT_SUCCESS; + return Command::SUCCESS; } } diff --git a/module/CLI/src/Command/Config/ReadEnvVarCommand.php b/module/CLI/src/Command/Config/ReadEnvVarCommand.php index 72e07f97..e1cef3fd 100644 --- a/module/CLI/src/Command/Config/ReadEnvVarCommand.php +++ b/module/CLI/src/Command/Config/ReadEnvVarCommand.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Config; use Closure; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Config\EnvVars; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Exception\InvalidArgumentException; @@ -63,6 +62,6 @@ class ReadEnvVarCommand extends Command $envVar = $input->getArgument('envVar'); $output->writeln(formatEnvVarValue(($this->loadEnvVar)($envVar))); - return ExitCode::EXIT_SUCCESS; + return Command::SUCCESS; } } diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php index b2a2fa18..6dc11dfc 100644 --- a/module/CLI/src/Command/Db/CreateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/CreateDatabaseCommand.php @@ -7,7 +7,6 @@ namespace Shlinkio\Shlink\CLI\Command\Db; use Doctrine\DBAL\Connection; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Mapping\ClassMetadata; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ProcessRunnerInterface; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -55,7 +54,7 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand if ($this->databaseTablesExist()) { $io->success('Database already exists. Run "db:migrate" command to make sure it is up to date.'); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } // Create database @@ -63,7 +62,7 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand $this->runPhpCommand($output, [self::DOCTRINE_SCRIPT, self::DOCTRINE_CREATE_SCHEMA_COMMAND]); $io->success('Database properly created!'); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } private function databaseTablesExist(): bool diff --git a/module/CLI/src/Command/Db/MigrateDatabaseCommand.php b/module/CLI/src/Command/Db/MigrateDatabaseCommand.php index 2e280a06..669b190b 100644 --- a/module/CLI/src/Command/Db/MigrateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/MigrateDatabaseCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Db; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; @@ -31,6 +30,6 @@ class MigrateDatabaseCommand extends AbstractDatabaseCommand $this->runPhpCommand($output, [self::DOCTRINE_MIGRATIONS_SCRIPT, self::DOCTRINE_MIGRATE_COMMAND]); $io->success('Database properly migrated!'); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } } diff --git a/module/CLI/src/Command/Domain/DomainRedirectsCommand.php b/module/CLI/src/Command/Domain/DomainRedirectsCommand.php index 105c10e3..4c2e4350 100644 --- a/module/CLI/src/Command/Domain/DomainRedirectsCommand.php +++ b/module/CLI/src/Command/Domain/DomainRedirectsCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Domain; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Config\NotFoundRedirects; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; use Shlinkio\Shlink\Core\Domain\Model\DomainItem; @@ -109,6 +108,6 @@ class DomainRedirectsCommand extends Command $io->success(sprintf('"Not found" redirects properly set for "%s"', $domainAuthority)); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } } diff --git a/module/CLI/src/Command/Domain/ListDomainsCommand.php b/module/CLI/src/Command/Domain/ListDomainsCommand.php index 79c181f7..935d272e 100644 --- a/module/CLI/src/Command/Domain/ListDomainsCommand.php +++ b/module/CLI/src/Command/Domain/ListDomainsCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Domain; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Core\Config\NotFoundRedirectConfigInterface; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; @@ -59,7 +58,7 @@ class ListDomainsCommand extends Command }, $domains), ); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } private function notFoundRedirectsToString(NotFoundRedirectConfigInterface $config): string diff --git a/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php b/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php index 9a41cc05..c1c22075 100644 --- a/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php +++ b/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Integration; use Cake\Chronos\Chronos; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Matomo\MatomoOptions; use Shlinkio\Shlink\Core\Matomo\MatomoVisitSenderInterface; use Shlinkio\Shlink\Core\Matomo\VisitSendingProgressTrackerInterface; @@ -84,7 +83,7 @@ class MatomoSendVisitsCommand extends Command implements VisitSendingProgressTra if (! $this->matomoEnabled) { $this->io->warning('Matomo integration is not enabled in this Shlink instance'); - return ExitCode::EXIT_WARNING; + return self::INVALID; } // TODO Validate provided date formats @@ -103,7 +102,7 @@ class MatomoSendVisitsCommand extends Command implements VisitSendingProgressTra . 'you have verified only visits in the right date range are going to be sent.', ]); if (! $this->io->confirm('Continue?', default: false)) { - return ExitCode::EXIT_WARNING; + return self::INVALID; } } @@ -122,7 +121,7 @@ class MatomoSendVisitsCommand extends Command implements VisitSendingProgressTra default => $this->io->info('There was no visits matching provided date range.'), }; - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } public function success(int $index): void diff --git a/module/CLI/src/Command/RedirectRule/ManageRedirectRulesCommand.php b/module/CLI/src/Command/RedirectRule/ManageRedirectRulesCommand.php index 646b9d77..9a129e9d 100644 --- a/module/CLI/src/Command/RedirectRule/ManageRedirectRulesCommand.php +++ b/module/CLI/src/Command/RedirectRule/ManageRedirectRulesCommand.php @@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\CLI\Command\RedirectRule; use Shlinkio\Shlink\CLI\Input\ShortUrlIdentifierInput; use Shlinkio\Shlink\CLI\RedirectRule\RedirectRuleHandlerInterface; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; @@ -52,7 +51,7 @@ class ManageRedirectRulesCommand extends Command $shortUrl = $this->shortUrlResolver->resolveShortUrl($identifier); } catch (ShortUrlNotFoundException) { $io->error(sprintf('Short URL for %s not found', $identifier->__toString())); - return ExitCode::EXIT_FAILURE; + return self::FAILURE; } $rulesToSave = $this->ruleHandler->manageRules($io, $shortUrl, $this->ruleService->rulesForShortUrl($shortUrl)); @@ -61,6 +60,6 @@ class ManageRedirectRulesCommand extends Command $io->success('Rules properly saved'); } - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } } diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index df341c96..2e52571b 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Input\ShortUrlDataInput; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; @@ -114,10 +113,10 @@ class CreateShortUrlCommand extends Command sprintf('Processed long URL: %s', $result->shortUrl->getLongUrl()), sprintf('Generated short URL: %s', $this->stringifier->stringify($result->shortUrl)), ]); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } catch (NonUniqueSlugException $e) { $io->error($e->getMessage()); - return ExitCode::EXIT_FAILURE; + return self::FAILURE; } } diff --git a/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php index 1fc9dc38..2b2abd01 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\ShortUrl\DeleteShortUrlServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ExpiredShortUrlsConditions; use Symfony\Component\Console\Command\Command; @@ -58,18 +57,18 @@ class DeleteExpiredShortUrlsCommand extends Command 'This action cannot be undone. Proceed at your own risk', ]); if (! $io->confirm('Continue?', default: false)) { - return ExitCode::EXIT_WARNING; + return self::INVALID; } } if ($dryRun) { $result = $this->deleteShortUrlService->countExpiredShortUrls($conditions); $io->success(sprintf('There are %s expired short URLs matching provided conditions', $result)); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } $result = $this->deleteShortUrlService->deleteExpiredShortUrls($conditions); $io->success(sprintf('%s expired short URLs have been deleted', $result)); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } } diff --git a/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php index edda1b29..e6a11ea1 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Input\ShortUrlIdentifierInput; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception; use Shlinkio\Shlink\Core\ShortUrl\DeleteShortUrlServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; @@ -55,10 +54,10 @@ class DeleteShortUrlCommand extends Command try { $this->runDelete($io, $identifier, $ignoreThreshold); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } catch (Exception\ShortUrlNotFoundException $e) { $io->error($e->getMessage()); - return ExitCode::EXIT_FAILURE; + return self::FAILURE; } catch (Exception\DeleteShortUrlException $e) { return $this->retry($io, $identifier, $e->getMessage()); } @@ -75,7 +74,7 @@ class DeleteShortUrlCommand extends Command $io->warning('Short URL was not deleted.'); } - return $forceDelete ? ExitCode::EXIT_SUCCESS : ExitCode::EXIT_WARNING; + return $forceDelete ? self::SUCCESS : self::INVALID; } private function runDelete(SymfonyStyle $io, ShortUrlIdentifier $identifier, bool $ignoreThreshold): void diff --git a/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php b/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php index a9a709a1..4c238f31 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php @@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Command\Visit\AbstractDeleteVisitsCommand; use Shlinkio\Shlink\CLI\Input\ShortUrlIdentifierInput; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlVisitsDeleterInterface; use Symfony\Component\Console\Input\InputInterface; @@ -44,10 +43,10 @@ class DeleteShortUrlVisitsCommand extends AbstractDeleteVisitsCommand $result = $this->deleter->deleteShortUrlVisits($identifier); $io->success(sprintf('Successfully deleted %s visits', $result->affectedItems)); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } catch (ShortUrlNotFoundException) { $io->warning(sprintf('Short URL not found for "%s"', $identifier->__toString())); - return ExitCode::EXIT_WARNING; + return self::INVALID; } } diff --git a/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php index ad9aaf70..16fe9458 100644 --- a/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php @@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Input\ShortUrlDataInput; use Shlinkio\Shlink\CLI\Input\ShortUrlIdentifierInput; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlServiceInterface; @@ -57,7 +56,7 @@ class EditShortUrlCommand extends Command ); $io->success(sprintf('Short URL "%s" properly edited', $this->stringifier->stringify($shortUrl))); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } catch (ShortUrlNotFoundException $e) { $io->error(sprintf('Short URL not found for "%s"', $identifier->__toString())); @@ -65,7 +64,7 @@ class EditShortUrlCommand extends Command $this->getApplication()?->renderThrowable($e, $io); } - return ExitCode::EXIT_FAILURE; + return self::FAILURE; } } } diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 72222a08..5bdc4c81 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Input\EndDateOption; use Shlinkio\Shlink\CLI\Input\StartDateOption; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtils; @@ -176,7 +175,7 @@ class ListShortUrlsCommand extends Command $io->newLine(); $io->success('Short URLs properly listed'); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } /** diff --git a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php index 7935df6d..b6bf71f7 100644 --- a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Input\ShortUrlIdentifierInput; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Symfony\Component\Console\Command\Command; @@ -59,10 +58,10 @@ class ResolveUrlCommand extends Command try { $url = $this->urlResolver->resolveShortUrl($this->shortUrlIdentifierInput->toShortUrlIdentifier($input)); $output->writeln(sprintf('Long URL: %s', $url->getLongUrl())); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } catch (ShortUrlNotFoundException $e) { $io->error($e->getMessage()); - return ExitCode::EXIT_FAILURE; + return self::FAILURE; } } } diff --git a/module/CLI/src/Command/Tag/DeleteTagsCommand.php b/module/CLI/src/Command/Tag/DeleteTagsCommand.php index 2f13e775..2022a9dc 100644 --- a/module/CLI/src/Command/Tag/DeleteTagsCommand.php +++ b/module/CLI/src/Command/Tag/DeleteTagsCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Tag; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -41,11 +40,11 @@ class DeleteTagsCommand extends Command if (empty($tagNames)) { $io->warning('You have to provide at least one tag name'); - return ExitCode::EXIT_WARNING; + return self::INVALID; } $this->tagService->deleteTags($tagNames); $io->success('Tags properly deleted'); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } } diff --git a/module/CLI/src/Command/Tag/ListTagsCommand.php b/module/CLI/src/Command/Tag/ListTagsCommand.php index 8333b82e..abd9a0dd 100644 --- a/module/CLI/src/Command/Tag/ListTagsCommand.php +++ b/module/CLI/src/Command/Tag/ListTagsCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Tag; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\Model\TagsParams; @@ -34,7 +33,7 @@ class ListTagsCommand extends Command protected function execute(InputInterface $input, OutputInterface $output): int { ShlinkTable::default($output)->render(['Name', 'URLs amount', 'Visits amount'], $this->getTagsRows()); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } private function getTagsRows(): array diff --git a/module/CLI/src/Command/Tag/RenameTagCommand.php b/module/CLI/src/Command/Tag/RenameTagCommand.php index 42092d04..2ae0159c 100644 --- a/module/CLI/src/Command/Tag/RenameTagCommand.php +++ b/module/CLI/src/Command/Tag/RenameTagCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Tag; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Model\Renaming; @@ -42,10 +41,10 @@ class RenameTagCommand extends Command try { $this->tagService->renameTag(Renaming::fromNames($oldName, $newName)); $io->success('Tag properly renamed.'); - return ExitCode::EXIT_SUCCESS; + return Command::SUCCESS; } catch (TagNotFoundException | TagConflictException $e) { $io->error($e->getMessage()); - return ExitCode::EXIT_FAILURE; + return Command::FAILURE; } } } diff --git a/module/CLI/src/Command/Util/AbstractLockedCommand.php b/module/CLI/src/Command/Util/AbstractLockedCommand.php index 8bd728cd..a4c3ef5d 100644 --- a/module/CLI/src/Command/Util/AbstractLockedCommand.php +++ b/module/CLI/src/Command/Util/AbstractLockedCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Util; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -28,7 +27,7 @@ abstract class AbstractLockedCommand extends Command $output->writeln( sprintf('Command "%s" is already in progress. Skipping.', $lockConfig->lockName), ); - return ExitCode::EXIT_WARNING; + return self::INVALID; } try { diff --git a/module/CLI/src/Command/Visit/AbstractDeleteVisitsCommand.php b/module/CLI/src/Command/Visit/AbstractDeleteVisitsCommand.php index 7cb32698..d8ef98e3 100644 --- a/module/CLI/src/Command/Visit/AbstractDeleteVisitsCommand.php +++ b/module/CLI/src/Command/Visit/AbstractDeleteVisitsCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -17,7 +16,7 @@ abstract class AbstractDeleteVisitsCommand extends Command $io = new SymfonyStyle($input, $output); if (! $this->confirm($io)) { $io->info('Operation aborted'); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } return $this->doExecute($input, $io); diff --git a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php index b95c6845..5916fc52 100644 --- a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php +++ b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php @@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\CLI\Command\Visit; use Shlinkio\Shlink\CLI\Input\EndDateOption; use Shlinkio\Shlink\CLI\Input\StartDateOption; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Util\DateRange; @@ -43,7 +42,7 @@ abstract class AbstractVisitsListCommand extends Command ShlinkTable::default($output)->render($headers, $rows); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } /** diff --git a/module/CLI/src/Command/Visit/DeleteOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/DeleteOrphanVisitsCommand.php index 056a9c60..77fefaaa 100644 --- a/module/CLI/src/Command/Visit/DeleteOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/DeleteOrphanVisitsCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Visit\VisitsDeleterInterface; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Style\SymfonyStyle; @@ -32,7 +31,7 @@ class DeleteOrphanVisitsCommand extends AbstractDeleteVisitsCommand $result = $this->deleter->deleteOrphanVisits(); $io->success(sprintf('Successfully deleted %s visits', $result->affectedItems)); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } protected function getWarningMessage(): string diff --git a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php index cd7c4ffb..4d58a7d3 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface; use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface; @@ -48,17 +47,17 @@ class DownloadGeoLiteDbCommand extends Command implements GeolocationDownloadPro if ($result === GeolocationResult::LICENSE_MISSING) { $this->io->warning('It was not possible to download GeoLite2 db, because a license was not provided.'); - return ExitCode::EXIT_WARNING; + return self::INVALID; } if ($result === GeolocationResult::MAX_ERRORS_REACHED) { $this->io->warning('Max consecutive errors reached. Cannot retry for a couple of days.'); - return ExitCode::EXIT_WARNING; + return self::INVALID; } if ($result === GeolocationResult::UPDATE_IN_PROGRESS) { $this->io->warning('A geolocation db is already being downloaded by another process.'); - return ExitCode::EXIT_WARNING; + return self::INVALID; } if ($this->progressBar === null) { @@ -68,7 +67,7 @@ class DownloadGeoLiteDbCommand extends Command implements GeolocationDownloadPro $this->io->success('GeoLite2 db file properly downloaded.'); } - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } catch (GeolocationDbUpdateFailedException $e) { return $this->processGeoLiteUpdateError($e, $this->io); } @@ -90,7 +89,7 @@ class DownloadGeoLiteDbCommand extends Command implements GeolocationDownloadPro $this->getApplication()?->renderThrowable($e, $io); } - return $olderDbExists ? ExitCode::EXIT_WARNING : ExitCode::EXIT_FAILURE; + return $olderDbExists ? self::INVALID : self::FAILURE; } public function beforeDownload(bool $olderDbExists): void diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php index 66e33a78..3ed2edf9 100644 --- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\CLI\Command\Visit; use Shlinkio\Shlink\CLI\Command\Util\AbstractLockedCommand; use Shlinkio\Shlink\CLI\Command\Util\LockedCommandConfig; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException; use Shlinkio\Shlink\Core\Visit\Entity\Visit; @@ -116,14 +115,14 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat } $this->io->success('Finished locating visits'); - return ExitCode::EXIT_SUCCESS; + return self::SUCCESS; } catch (Throwable $e) { $this->io->error($e->getMessage()); if ($this->io->isVerbose()) { $this->getApplication()?->renderThrowable($e, $this->io); } - return ExitCode::EXIT_FAILURE; + return self::FAILURE; } } @@ -171,7 +170,7 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat $downloadDbCommand = $cliApp->find(DownloadGeoLiteDbCommand::NAME); $exitCode = $downloadDbCommand->run(new ArrayInput([]), $this->io); - if ($exitCode === ExitCode::EXIT_FAILURE) { + if ($exitCode === self::FAILURE) { throw new RuntimeException('It is not possible to locate visits without a GeoLite2 db file.'); } } diff --git a/module/CLI/src/Util/ExitCode.php b/module/CLI/src/Util/ExitCode.php deleted file mode 100644 index f2ffa16b..00000000 --- a/module/CLI/src/Util/ExitCode.php +++ /dev/null @@ -1,12 +0,0 @@ -exec([ListShortUrlsCommand::NAME, '--show-domain', '--search-term', $slug]); diff --git a/module/CLI/test-cli/Command/GenerateApiKeyTest.php b/module/CLI/test-cli/Command/GenerateApiKeyTest.php index 7d90c336..a8479ecf 100644 --- a/module/CLI/test-cli/Command/GenerateApiKeyTest.php +++ b/module/CLI/test-cli/Command/GenerateApiKeyTest.php @@ -6,8 +6,8 @@ namespace ShlinkioCliTest\Shlink\CLI\Command; use PHPUnit\Framework\Attributes\Test; use Shlinkio\Shlink\CLI\Command\Api\GenerateKeyCommand; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\TestUtils\CliTest\CliTestCase; +use Symfony\Component\Console\Command\Command; class GenerateApiKeyTest extends CliTestCase { @@ -17,6 +17,6 @@ class GenerateApiKeyTest extends CliTestCase [$output, $exitCode] = $this->exec([GenerateKeyCommand::NAME]); self::assertStringContainsString('[OK] Generated API key', $output); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertEquals(Command::SUCCESS, $exitCode); } } diff --git a/module/CLI/test-cli/Command/ListApiKeysTest.php b/module/CLI/test-cli/Command/ListApiKeysTest.php index 9e0ce90d..0e43f24a 100644 --- a/module/CLI/test-cli/Command/ListApiKeysTest.php +++ b/module/CLI/test-cli/Command/ListApiKeysTest.php @@ -8,8 +8,8 @@ use Cake\Chronos\Chronos; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use Shlinkio\Shlink\CLI\Command\Api\ListKeysCommand; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\TestUtils\CliTest\CliTestCase; +use Symfony\Component\Console\Command\Command; class ListApiKeysTest extends CliTestCase { @@ -19,7 +19,7 @@ class ListApiKeysTest extends CliTestCase [$output, $exitCode] = $this->exec([ListKeysCommand::NAME, ...$flags]); self::assertEquals($expectedOutput, $output); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertEquals(Command::SUCCESS, $exitCode); } public static function provideFlags(): iterable diff --git a/module/CLI/test/Command/Api/DisableKeyCommandTest.php b/module/CLI/test/Command/Api/DisableKeyCommandTest.php index a617539d..e8a08336 100644 --- a/module/CLI/test/Command/Api/DisableKeyCommandTest.php +++ b/module/CLI/test/Command/Api/DisableKeyCommandTest.php @@ -8,12 +8,12 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\Api\DisableKeyCommand; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use ShlinkioTest\Shlink\CLI\Util\CliTestUtils; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; class DisableKeyCommandTest extends TestCase @@ -40,7 +40,7 @@ class DisableKeyCommandTest extends TestCase $output = $this->commandTester->getDisplay(); self::assertStringContainsString('API key "abcd1234" properly disabled', $output); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertEquals(Command::SUCCESS, $exitCode); } #[Test] @@ -57,7 +57,7 @@ class DisableKeyCommandTest extends TestCase $output = $this->commandTester->getDisplay(); self::assertStringContainsString('API key "the key to delete" properly disabled', $output); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertEquals(Command::SUCCESS, $exitCode); } #[Test] @@ -76,7 +76,7 @@ class DisableKeyCommandTest extends TestCase $output = $this->commandTester->getDisplay(); self::assertStringContainsString($expectedMessage, $output); - self::assertEquals(ExitCode::EXIT_FAILURE, $exitCode); + self::assertEquals(Command::FAILURE, $exitCode); } #[Test] @@ -96,7 +96,7 @@ class DisableKeyCommandTest extends TestCase $output = $this->commandTester->getDisplay(); self::assertStringContainsString($expectedMessage, $output); - self::assertEquals(ExitCode::EXIT_FAILURE, $exitCode); + self::assertEquals(Command::FAILURE, $exitCode); } #[Test] @@ -108,7 +108,7 @@ class DisableKeyCommandTest extends TestCase $exitCode = $this->commandTester->execute([], ['interactive' => false]); - self::assertEquals(ExitCode::EXIT_WARNING, $exitCode); + self::assertEquals(Command::INVALID, $exitCode); } #[Test] @@ -128,6 +128,6 @@ class DisableKeyCommandTest extends TestCase $output = $this->commandTester->getDisplay(); self::assertStringContainsString('API key "the key to delete" properly disabled', $output); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertEquals(Command::SUCCESS, $exitCode); } } diff --git a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php index 4fa4498c..849ea0cf 100644 --- a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php +++ b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php @@ -10,11 +10,11 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\ApiKey\RoleResolverInterface; use Shlinkio\Shlink\CLI\Command\Api\GenerateKeyCommand; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use ShlinkioTest\Shlink\CLI\Util\CliTestUtils; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; class GenerateKeyCommandTest extends TestCase @@ -68,6 +68,6 @@ class GenerateKeyCommandTest extends TestCase '--name' => 'Alice', ]); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertEquals(Command::SUCCESS, $exitCode); } } diff --git a/module/CLI/test/Command/Domain/ListDomainsCommandTest.php b/module/CLI/test/Command/Domain/ListDomainsCommandTest.php index 2621f940..159fded4 100644 --- a/module/CLI/test/Command/Domain/ListDomainsCommandTest.php +++ b/module/CLI/test/Command/Domain/ListDomainsCommandTest.php @@ -9,13 +9,13 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\Domain\ListDomainsCommand; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Config\NotFoundRedirects; use Shlinkio\Shlink\Core\Config\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Domain\Model\DomainItem; use ShlinkioTest\Shlink\CLI\Util\CliTestUtils; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; class ListDomainsCommandTest extends TestCase @@ -51,7 +51,7 @@ class ListDomainsCommandTest extends TestCase $this->commandTester->execute($input); self::assertEquals($expectedOutput, $this->commandTester->getDisplay()); - self::assertEquals(ExitCode::EXIT_SUCCESS, $this->commandTester->getStatusCode()); + self::assertEquals(Command::SUCCESS, $this->commandTester->getStatusCode()); } public static function provideInputsAndOutputs(): iterable diff --git a/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php b/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php index 78d2f828..f4eebdf5 100644 --- a/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php +++ b/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php @@ -8,12 +8,12 @@ use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\Integration\MatomoSendVisitsCommand; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Matomo\MatomoOptions; use Shlinkio\Shlink\Core\Matomo\MatomoVisitSenderInterface; use Shlinkio\Shlink\Core\Matomo\Model\SendVisitsResult; use ShlinkioTest\Shlink\CLI\Util\CliTestUtils; +use Symfony\Component\Console\Command\Command; class MatomoSendVisitsCommandTest extends TestCase { @@ -30,7 +30,7 @@ class MatomoSendVisitsCommandTest extends TestCase [$output, $exitCode] = $this->executeCommand(matomoEnabled: false); self::assertStringContainsString('Matomo integration is not enabled in this Shlink instance', $output); - self::assertEquals(ExitCode::EXIT_WARNING, $exitCode); + self::assertEquals(Command::INVALID, $exitCode); } #[Test] @@ -74,7 +74,7 @@ class MatomoSendVisitsCommandTest extends TestCase [$output, $exitCode] = $this->executeCommand(['y']); self::assertStringContainsString($expectedResultMessage, $output); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertEquals(Command::SUCCESS, $exitCode); } #[Test] diff --git a/module/CLI/test/Command/RedirectRule/ManageRedirectRulesCommandTest.php b/module/CLI/test/Command/RedirectRule/ManageRedirectRulesCommandTest.php index 79859d23..5cb45a4b 100644 --- a/module/CLI/test/Command/RedirectRule/ManageRedirectRulesCommandTest.php +++ b/module/CLI/test/Command/RedirectRule/ManageRedirectRulesCommandTest.php @@ -9,13 +9,13 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\RedirectRule\ManageRedirectRulesCommand; use Shlinkio\Shlink\CLI\RedirectRule\RedirectRuleHandlerInterface; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use ShlinkioTest\Shlink\CLI\Util\CliTestUtils; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; class ManageRedirectRulesCommandTest extends TestCase @@ -51,7 +51,7 @@ class ManageRedirectRulesCommandTest extends TestCase $exitCode = $this->commandTester->execute(['shortCode' => 'foo']); $output = $this->commandTester->getDisplay(); - self::assertEquals(ExitCode::EXIT_FAILURE, $exitCode); + self::assertEquals(Command::FAILURE, $exitCode); self::assertStringContainsString('Short URL for foo not found', $output); } @@ -70,7 +70,7 @@ class ManageRedirectRulesCommandTest extends TestCase $exitCode = $this->commandTester->execute(['shortCode' => 'foo']); $output = $this->commandTester->getDisplay(); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertEquals(Command::SUCCESS, $exitCode); self::assertStringNotContainsString('Rules properly saved', $output); } @@ -89,7 +89,7 @@ class ManageRedirectRulesCommandTest extends TestCase $exitCode = $this->commandTester->execute(['shortCode' => 'foo']); $output = $this->commandTester->getDisplay(); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertEquals(Command::SUCCESS, $exitCode); self::assertStringContainsString('Rules properly saved', $output); } } diff --git a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php index a57a2870..728f8f22 100644 --- a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php @@ -11,7 +11,6 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\ShortUrl\CreateShortUrlCommand; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; @@ -20,6 +19,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\UrlShorteningResult; use Shlinkio\Shlink\Core\ShortUrl\UrlShortenerInterface; use ShlinkioTest\Shlink\CLI\Util\CliTestUtils; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Tester\CommandTester; @@ -59,7 +59,7 @@ class CreateShortUrlCommandTest extends TestCase ], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); $output = $this->commandTester->getDisplay(); - self::assertEquals(ExitCode::EXIT_SUCCESS, $this->commandTester->getStatusCode()); + self::assertEquals(Command::SUCCESS, $this->commandTester->getStatusCode()); self::assertStringContainsString('stringified_short_url', $output); self::assertStringNotContainsString('but the real-time updates cannot', $output); } @@ -75,7 +75,7 @@ class CreateShortUrlCommandTest extends TestCase $this->commandTester->execute(['longUrl' => 'http://domain.com/invalid', '--custom-slug' => 'my-slug']); $output = $this->commandTester->getDisplay(); - self::assertEquals(ExitCode::EXIT_FAILURE, $this->commandTester->getStatusCode()); + self::assertEquals(Command::FAILURE, $this->commandTester->getStatusCode()); self::assertStringContainsString('Provided slug "my-slug" is already in use', $output); } @@ -99,7 +99,7 @@ class CreateShortUrlCommandTest extends TestCase ]); $output = $this->commandTester->getDisplay(); - self::assertEquals(ExitCode::EXIT_SUCCESS, $this->commandTester->getStatusCode()); + self::assertEquals(Command::SUCCESS, $this->commandTester->getStatusCode()); self::assertStringContainsString('stringified_short_url', $output); } @@ -117,7 +117,7 @@ class CreateShortUrlCommandTest extends TestCase $input['longUrl'] = 'http://domain.com/foo/bar'; $this->commandTester->execute($input); - self::assertEquals(ExitCode::EXIT_SUCCESS, $this->commandTester->getStatusCode()); + self::assertEquals(Command::SUCCESS, $this->commandTester->getStatusCode()); } public static function provideDomains(): iterable diff --git a/module/CLI/test/Command/ShortUrl/DeleteExpiredShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/DeleteExpiredShortUrlsCommandTest.php index ea580064..2a4b15f9 100644 --- a/module/CLI/test/Command/ShortUrl/DeleteExpiredShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/DeleteExpiredShortUrlsCommandTest.php @@ -9,10 +9,10 @@ use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\ShortUrl\DeleteExpiredShortUrlsCommand; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\ShortUrl\DeleteShortUrlServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ExpiredShortUrlsConditions; use ShlinkioTest\Shlink\CLI\Util\CliTestUtils; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; class DeleteExpiredShortUrlsCommandTest extends TestCase @@ -38,7 +38,7 @@ class DeleteExpiredShortUrlsCommandTest extends TestCase $status = $this->commandTester->getStatusCode(); self::assertStringContainsString('Careful!', $output); - self::assertEquals(ExitCode::EXIT_WARNING, $status); + self::assertEquals(Command::INVALID, $status); } #[Test] @@ -62,7 +62,7 @@ class DeleteExpiredShortUrlsCommandTest extends TestCase self::assertStringNotContainsString('Careful!', $output); } self::assertStringContainsString('5 expired short URLs have been deleted', $output); - self::assertEquals(ExitCode::EXIT_SUCCESS, $status); + self::assertEquals(Command::SUCCESS, $status); } #[Test] @@ -77,7 +77,7 @@ class DeleteExpiredShortUrlsCommandTest extends TestCase self::assertStringNotContainsString('Careful!', $output); self::assertStringContainsString('There are 38 expired short URLs matching provided conditions', $output); - self::assertEquals(ExitCode::EXIT_SUCCESS, $status); + self::assertEquals(Command::SUCCESS, $status); } #[Test] diff --git a/module/CLI/test/Command/ShortUrl/DeleteShortUrlVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/DeleteShortUrlVisitsCommandTest.php index 2a281a8a..de038aef 100644 --- a/module/CLI/test/Command/ShortUrl/DeleteShortUrlVisitsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/DeleteShortUrlVisitsCommandTest.php @@ -9,11 +9,11 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\ShortUrl\DeleteShortUrlVisitsCommand; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\BulkDeleteResult; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlVisitsDeleterInterface; use ShlinkioTest\Shlink\CLI\Util\CliTestUtils; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; class DeleteShortUrlVisitsCommandTest extends TestCase @@ -36,7 +36,7 @@ class DeleteShortUrlVisitsCommandTest extends TestCase $exitCode = $this->commandTester->execute(['shortCode' => 'foo']); $output = $this->commandTester->getDisplay(); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertEquals(Command::SUCCESS, $exitCode); self::assertStringContainsString('Operation aborted', $output); } @@ -58,7 +58,7 @@ class DeleteShortUrlVisitsCommandTest extends TestCase $exitCode = $this->commandTester->execute($args); $output = $this->commandTester->getDisplay(); - self::assertEquals(ExitCode::EXIT_WARNING, $exitCode); + self::assertEquals(Command::INVALID, $exitCode); self::assertStringContainsString($expectedError, $output); } @@ -77,7 +77,7 @@ class DeleteShortUrlVisitsCommandTest extends TestCase $exitCode = $this->commandTester->execute(['shortCode' => 'foo']); $output = $this->commandTester->getDisplay(); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertEquals(Command::SUCCESS, $exitCode); self::assertStringContainsString('Successfully deleted 5 visits', $output); } } diff --git a/module/CLI/test/Command/ShortUrl/EditShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/EditShortUrlCommandTest.php index f540b5dc..0fd9a860 100644 --- a/module/CLI/test/Command/ShortUrl/EditShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/EditShortUrlCommandTest.php @@ -7,13 +7,13 @@ use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\ShortUrl\EditShortUrlCommand; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlServiceInterface; use ShlinkioTest\Shlink\CLI\Util\CliTestUtils; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Tester\CommandTester; @@ -45,7 +45,7 @@ class EditShortUrlCommandTest extends TestCase $exitCode = $this->commandTester->getStatusCode(); self::assertStringContainsString('Short URL "https://s.test/foo" properly edited', $output); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertEquals(Command::SUCCESS, $exitCode); } #[Test] @@ -69,6 +69,6 @@ class EditShortUrlCommandTest extends TestCase } else { self::assertStringNotContainsString('Exception trace:', $output); } - self::assertEquals(ExitCode::EXIT_FAILURE, $exitCode); + self::assertEquals(Command::FAILURE, $exitCode); } } diff --git a/module/CLI/test/Command/Visit/DeleteOrphanVisitsCommandTest.php b/module/CLI/test/Command/Visit/DeleteOrphanVisitsCommandTest.php index cd39c63a..68eedcee 100644 --- a/module/CLI/test/Command/Visit/DeleteOrphanVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/DeleteOrphanVisitsCommandTest.php @@ -8,10 +8,10 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\Visit\DeleteOrphanVisitsCommand; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Model\BulkDeleteResult; use Shlinkio\Shlink\Core\Visit\VisitsDeleterInterface; use ShlinkioTest\Shlink\CLI\Util\CliTestUtils; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; class DeleteOrphanVisitsCommandTest extends TestCase @@ -34,7 +34,7 @@ class DeleteOrphanVisitsCommandTest extends TestCase $exitCode = $this->commandTester->execute([]); $output = $this->commandTester->getDisplay(); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertEquals(Command::SUCCESS, $exitCode); self::assertStringContainsString('You are about to delete all orphan visits.', $output); self::assertStringContainsString('Successfully deleted 5 visits', $output); } diff --git a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php index 01322f0b..3247450e 100644 --- a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php +++ b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php @@ -10,12 +10,12 @@ 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\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\Command\Command; use Symfony\Component\Console\Tester\CommandTester; use function sprintf; @@ -65,12 +65,12 @@ class DownloadGeoLiteDbCommandTest extends TestCase yield 'existing db' => [ true, '[WARNING] GeoLite2 db file update failed. Visits will continue to be located', - ExitCode::EXIT_WARNING, + Command::INVALID, ]; yield 'not existing db' => [ false, '[ERROR] GeoLite2 db file download failed. It will not be possible to locate', - ExitCode::EXIT_FAILURE, + Command::FAILURE, ]; } @@ -87,7 +87,7 @@ class DownloadGeoLiteDbCommandTest extends TestCase $exitCode = $this->commandTester->getStatusCode(); self::assertStringContainsString('[WARNING] ' . $expectedWarningMessage, $output); - self::assertSame(ExitCode::EXIT_WARNING, $exitCode); + self::assertSame(Command::INVALID, $exitCode); } #[Test, DataProvider('provideSuccessParams')] @@ -102,7 +102,7 @@ class DownloadGeoLiteDbCommandTest extends TestCase $exitCode = $this->commandTester->getStatusCode(); self::assertStringContainsString($expectedMessage, $output); - self::assertSame(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertSame(Command::SUCCESS, $exitCode); } public static function provideSuccessParams(): iterable diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php index aa1f5e25..1f34baef 100644 --- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php @@ -10,7 +10,6 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\Visit\DownloadGeoLiteDbCommand; use Shlinkio\Shlink\CLI\Command\Visit\LocateVisitsCommand; -use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\Visit\Entity\Visit; @@ -83,7 +82,7 @@ class LocateVisitsCommandTest extends TestCase $this->visitToLocation->expects( $this->exactly($expectedUnlocatedCalls + $expectedEmptyCalls + $expectedAllCalls), )->method('resolveVisitLocation')->withAnyParameters()->willReturn(Location::emptyInstance()); - $this->downloadDbCommand->method('run')->willReturn(ExitCode::EXIT_SUCCESS); + $this->downloadDbCommand->method('run')->willReturn(Command::SUCCESS); $this->commandTester->setInputs(['y']); $this->commandTester->execute($args); @@ -116,7 +115,7 @@ class LocateVisitsCommandTest extends TestCase ->withAnyParameters() ->willReturnCallback($this->invokeHelperMethods($visit, $location)); $this->visitToLocation->expects($this->once())->method('resolveVisitLocation')->willThrowException($e); - $this->downloadDbCommand->method('run')->willReturn(ExitCode::EXIT_SUCCESS); + $this->downloadDbCommand->method('run')->willReturn(Command::SUCCESS); $this->commandTester->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); @@ -145,7 +144,7 @@ class LocateVisitsCommandTest extends TestCase $this->visitToLocation->expects($this->once())->method('resolveVisitLocation')->willThrowException( IpCannotBeLocatedException::forError(WrongIpException::fromIpAddress('1.2.3.4')), ); - $this->downloadDbCommand->method('run')->willReturn(ExitCode::EXIT_SUCCESS); + $this->downloadDbCommand->method('run')->willReturn(Command::SUCCESS); $this->commandTester->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); @@ -169,7 +168,7 @@ class LocateVisitsCommandTest extends TestCase $this->visitService->expects($this->never())->method('locateUnlocatedVisits'); $this->visitToLocation->expects($this->never())->method('resolveVisitLocation'); - $this->downloadDbCommand->method('run')->willReturn(ExitCode::EXIT_SUCCESS); + $this->downloadDbCommand->method('run')->willReturn(Command::SUCCESS); $this->commandTester->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); $output = $this->commandTester->getDisplay(); @@ -184,7 +183,7 @@ class LocateVisitsCommandTest extends TestCase public function showsProperMessageWhenGeoLiteUpdateFails(): void { $this->lock->method('acquire')->willReturn(true); - $this->downloadDbCommand->method('run')->willReturn(ExitCode::EXIT_FAILURE); + $this->downloadDbCommand->method('run')->willReturn(Command::FAILURE); $this->visitService->expects($this->never())->method('locateUnlocatedVisits'); $this->commandTester->execute([]); @@ -197,7 +196,7 @@ class LocateVisitsCommandTest extends TestCase public function providingAllFlagOnItsOwnDisplaysNotice(): void { $this->lock->method('acquire')->willReturn(true); - $this->downloadDbCommand->method('run')->willReturn(ExitCode::EXIT_SUCCESS); + $this->downloadDbCommand->method('run')->willReturn(Command::SUCCESS); $this->commandTester->execute(['--all' => true]); $output = $this->commandTester->getDisplay(); @@ -208,7 +207,7 @@ class LocateVisitsCommandTest extends TestCase #[Test, DataProvider('provideAbortInputs')] public function processingAllCancelsCommandIfUserDoesNotActivelyAgreeToConfirmation(array $inputs): void { - $this->downloadDbCommand->method('run')->willReturn(ExitCode::EXIT_SUCCESS); + $this->downloadDbCommand->method('run')->willReturn(Command::SUCCESS); $this->expectException(RuntimeException::class); $this->expectExceptionMessage('Execution aborted'); From 1a7a745f2e6ecc38c59882a66b9c1e0b8d49bef1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 4 May 2025 15:56:44 +0200 Subject: [PATCH 05/40] Update Dockerfile marking image-related extensions as delegated --- Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Dockerfile b/Dockerfile index eea707f5..9b12e7eb 100644 --- a/Dockerfile +++ b/Dockerfile @@ -16,6 +16,7 @@ WORKDIR /etc/shlink # Install required PHP extensions RUN \ # Temp install dev dependencies needed to compile the extensions + # FIXME Delegated image-related extensions. They can be removed with QR-code support apk add --no-cache --virtual .dev-deps sqlite-dev postgresql-dev icu-dev libzip-dev zlib-dev libpng-dev linux-headers && \ docker-php-ext-install -j"$(nproc)" pdo_mysql pdo_pgsql intl calendar sockets bcmath zip gd && \ apk add --no-cache sqlite-libs && \ From b2dbc4cf52ff13b7756b9ba5c46a13605e271101 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 4 May 2025 15:57:29 +0200 Subject: [PATCH 06/40] Fix typo in Dockerfile --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 9b12e7eb..9edfd06a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -16,7 +16,7 @@ WORKDIR /etc/shlink # Install required PHP extensions RUN \ # Temp install dev dependencies needed to compile the extensions - # FIXME Delegated image-related extensions. They can be removed with QR-code support + # FIXME Deprecated image-related extensions. They can be removed with QR-code support apk add --no-cache --virtual .dev-deps sqlite-dev postgresql-dev icu-dev libzip-dev zlib-dev libpng-dev linux-headers && \ docker-php-ext-install -j"$(nproc)" pdo_mysql pdo_pgsql intl calendar sockets bcmath zip gd && \ apk add --no-cache sqlite-libs && \ From 27d24a4f15c9696224267bab137a8d71b5b2bc7a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 6 May 2025 11:56:49 +0200 Subject: [PATCH 07/40] Update syntax used for env vars in Dockerfiles --- Dockerfile | 14 +++++++------- data/infra/php.Dockerfile | 8 ++++---- data/infra/roadrunner.Dockerfile | 6 +++--- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/Dockerfile b/Dockerfile index 9edfd06a..1b69a441 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,15 +1,15 @@ FROM php:8.4-alpine3.21 AS base ARG SHLINK_VERSION=latest -ENV SHLINK_VERSION ${SHLINK_VERSION} +ENV SHLINK_VERSION=${SHLINK_VERSION} ARG SHLINK_RUNTIME=rr -ENV SHLINK_RUNTIME ${SHLINK_RUNTIME} +ENV SHLINK_RUNTIME=${SHLINK_RUNTIME} -ENV USER_ID '1001' -ENV PDO_SQLSRV_VERSION 5.12.0 -ENV MS_ODBC_DOWNLOAD '7/6/d/76de322a-d860-4894-9945-f0cc5d6a45f8' -ENV MS_ODBC_SQL_VERSION 18_18.4.1.1 -ENV LC_ALL 'C' +ENV USER_ID='1001' +ENV PDO_SQLSRV_VERSION='5.12.0' +ENV MS_ODBC_DOWNLOAD='7/6/d/76de322a-d860-4894-9945-f0cc5d6a45f8' +ENV MS_ODBC_SQL_VERSION='18_18.4.1.1' +ENV LC_ALL='C' WORKDIR /etc/shlink diff --git a/data/infra/php.Dockerfile b/data/infra/php.Dockerfile index 0c08124f..bc06e876 100644 --- a/data/infra/php.Dockerfile +++ b/data/infra/php.Dockerfile @@ -1,10 +1,10 @@ FROM php:8.4-fpm-alpine3.21 MAINTAINER Alejandro Celaya -ENV APCU_VERSION 5.1.24 -ENV PDO_SQLSRV_VERSION 5.12.0 -ENV MS_ODBC_DOWNLOAD '7/6/d/76de322a-d860-4894-9945-f0cc5d6a45f8' -ENV MS_ODBC_SQL_VERSION 18_18.4.1.1 +ENV APCU_VERSION='5.1.24' +ENV PDO_SQLSRV_VERSION='5.12.0' +ENV MS_ODBC_DOWNLOAD='7/6/d/76de322a-d860-4894-9945-f0cc5d6a45f8' +ENV MS_ODBC_SQL_VERSION='18_18.4.1.1' RUN apk update diff --git a/data/infra/roadrunner.Dockerfile b/data/infra/roadrunner.Dockerfile index 3619aba3..6f5981bf 100644 --- a/data/infra/roadrunner.Dockerfile +++ b/data/infra/roadrunner.Dockerfile @@ -1,9 +1,9 @@ FROM php:8.4-alpine3.21 MAINTAINER Alejandro Celaya -ENV PDO_SQLSRV_VERSION 5.12.0 -ENV MS_ODBC_DOWNLOAD '7/6/d/76de322a-d860-4894-9945-f0cc5d6a45f8' -ENV MS_ODBC_SQL_VERSION 18_18.4.1.1 +ENV PDO_SQLSRV_VERSION='5.12.0' +ENV MS_ODBC_DOWNLOAD='7/6/d/76de322a-d860-4894-9945-f0cc5d6a45f8' +ENV MS_ODBC_SQL_VERSION='18_18.4.1.1' RUN apk update From 9c1db35d812443a15ae3698a3e31bf6c563a7b85 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 22 May 2025 09:20:50 +0200 Subject: [PATCH 08/40] Add new MERCURE_ENABLED env var --- CHANGELOG.md | 4 +++- composer.json | 2 +- config/autoload/mercure.global.php | 1 + config/params/shlink_dev_env.php.dist | 1 + module/Core/src/Config/EnvVars.php | 3 +++ .../src/EventDispatcher/Helper/EnabledListenerChecker.php | 2 +- .../src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php | 2 +- .../EventDispatcher/Helper/EnabledListenerCheckerTest.php | 2 +- 8 files changed, 12 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f791119..078a9835 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added -* *Nothing* +* [#2438](https://github.com/shlinkio/shlink/issues/2438) Add `MERCURE_ENABLED` env var and corresponding config option, to more easily allow the mercure integration to be toggled. + + For BC, if this env vars is not present, we'll still consider the integration enabled if the `MERCURE_PUBLIC_HUB_URL` env var has a value. This is considered deprecated though, and next major version will rely only on `MERCURE_ENABLED`, so if you are using Mercure, make sure to set `MERCURE_ENABLED=true` to be ready. ### Changed * [#2406](https://github.com/shlinkio/shlink/issues/2406) Remove references to bootstrap from error templates, and instead inline the very minimum required styles. diff --git a/composer.json b/composer.json index e3548394..af4b04fd 100644 --- a/composer.json +++ b/composer.json @@ -43,7 +43,7 @@ "pagerfanta/core": "^3.8", "ramsey/uuid": "^4.7", "shlinkio/doctrine-specification": "^2.2", - "shlinkio/shlink-common": "dev-main#e601317 as 7.1", + "shlinkio/shlink-common": "dev-main#7469270 as 7.1", "shlinkio/shlink-config": "^4.0", "shlinkio/shlink-event-dispatcher": "^4.2", "shlinkio/shlink-importer": "^5.6", diff --git a/config/autoload/mercure.global.php b/config/autoload/mercure.global.php index 83abed1a..55e8c5f8 100644 --- a/config/autoload/mercure.global.php +++ b/config/autoload/mercure.global.php @@ -12,6 +12,7 @@ return [ // This config is used by shlink-common. Do not delete 'mercure' => [ + 'enabled' => EnvVars::MERCURE_ENABLED->loadFromEnv(), 'public_hub_url' => EnvVars::MERCURE_PUBLIC_HUB_URL->loadFromEnv(), 'internal_hub_url' => EnvVars::MERCURE_INTERNAL_HUB_URL->loadFromEnv(), 'jwt_secret' => EnvVars::MERCURE_JWT_SECRET->loadFromEnv(), diff --git a/config/params/shlink_dev_env.php.dist b/config/params/shlink_dev_env.php.dist index 2a3905dc..d9de7022 100644 --- a/config/params/shlink_dev_env.php.dist +++ b/config/params/shlink_dev_env.php.dist @@ -58,6 +58,7 @@ return [ // EnvVars::MATOMO_API_TOKEN->value => , // Mercure + EnvVars::MERCURE_ENABLED->value => true, EnvVars::MERCURE_PUBLIC_HUB_URL->value => 'http://localhost:8002', EnvVars::MERCURE_INTERNAL_HUB_URL->value => 'http://shlink_mercure_proxy', EnvVars::MERCURE_JWT_SECRET->value => 'mercure_jwt_key_long_enough_to_avoid_error', diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 2a835932..1f6f2c03 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -42,6 +42,7 @@ enum EnvVars: string case REDIS_SERVERS = 'REDIS_SERVERS'; case REDIS_SENTINEL_SERVICE = 'REDIS_SENTINEL_SERVICE'; case REDIS_PUB_SUB_ENABLED = 'REDIS_PUB_SUB_ENABLED'; + case MERCURE_ENABLED = 'MERCURE_ENABLED'; case MERCURE_PUBLIC_HUB_URL = 'MERCURE_PUBLIC_HUB_URL'; case MERCURE_INTERNAL_HUB_URL = 'MERCURE_INTERNAL_HUB_URL'; case MERCURE_JWT_SECRET = 'MERCURE_JWT_SECRET'; @@ -84,6 +85,7 @@ 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'; /** @deprecated */ @@ -159,6 +161,7 @@ enum EnvVars: string }, self::DB_USE_ENCRYPTION => false, + self::MERCURE_ENABLED => self::MERCURE_PUBLIC_HUB_URL->existsInEnv(), self::MERCURE_INTERNAL_HUB_URL => self::MERCURE_PUBLIC_HUB_URL->loadFromEnv(), self::DEFAULT_QR_CODE_SIZE, => DEFAULT_QR_CODE_SIZE, diff --git a/module/Core/src/EventDispatcher/Helper/EnabledListenerChecker.php b/module/Core/src/EventDispatcher/Helper/EnabledListenerChecker.php index c348cbbd..0b1e58f5 100644 --- a/module/Core/src/EventDispatcher/Helper/EnabledListenerChecker.php +++ b/module/Core/src/EventDispatcher/Helper/EnabledListenerChecker.php @@ -34,7 +34,7 @@ readonly class EnabledListenerChecker implements EnabledListenerCheckerInterface EventDispatcher\RedisPubSub\NotifyVisitToRedis::class, EventDispatcher\RedisPubSub\NotifyNewShortUrlToRedis::class => $this->redisPubSubEnabled, EventDispatcher\Mercure\NotifyVisitToMercure::class, - EventDispatcher\Mercure\NotifyNewShortUrlToMercure::class => $this->mercureOptions->isEnabled(), + EventDispatcher\Mercure\NotifyNewShortUrlToMercure::class => $this->mercureOptions->enabled, EventDispatcher\Matomo\SendVisitToMatomo::class => $this->matomoOptions->enabled, EventDispatcher\UpdateGeoLiteDb::class => $this->geoLiteOptions->hasLicenseKey(), default => false, // Any unknown async listener should not be enabled by default diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php index 366e18e2..f870bb87 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php @@ -123,7 +123,7 @@ readonly class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionH private function encodeToUtf8WithMbString(string $titleInOriginalEncoding, string $pageCharset): string|null { try { - return mb_convert_encoding($titleInOriginalEncoding, 'utf-8', $pageCharset); + return mb_convert_encoding($titleInOriginalEncoding, 'utf-8', $pageCharset) ?: null; } catch (Throwable $e) { $this->logger->warning('It was impossible to encode page title in UTF-8 with mb_convert_encoding. {e}', [ 'e' => $e, diff --git a/module/Core/test/EventDispatcher/Helper/EnabledListenerCheckerTest.php b/module/Core/test/EventDispatcher/Helper/EnabledListenerCheckerTest.php index 180ab48d..9102e482 100644 --- a/module/Core/test/EventDispatcher/Helper/EnabledListenerCheckerTest.php +++ b/module/Core/test/EventDispatcher/Helper/EnabledListenerCheckerTest.php @@ -148,7 +148,7 @@ class EnabledListenerCheckerTest extends TestCase return new EnabledListenerChecker( new RabbitMqOptions(enabled: $rabbitMqEnabled), $redisPubSubEnabled, - new MercureOptions(publicHubUrl: $mercureEnabled ? 'the-url' : null), + new MercureOptions(enabled: $mercureEnabled), new GeoLite2Options(licenseKey: $geoLiteEnabled ? 'the-key' : null), new MatomoOptions(enabled: $matomoEnabled), ); From 2cad5dd435d47dfa42a044ab149f7961a49b2c35 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 27 May 2025 14:23:49 +0200 Subject: [PATCH 09/40] Update to roadrunner 2025.1 --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index af4b04fd..49a7913a 100644 --- a/composer.json +++ b/composer.json @@ -50,8 +50,8 @@ "shlinkio/shlink-installer": "^9.5", "shlinkio/shlink-ip-geolocation": "^4.3", "shlinkio/shlink-json": "^1.2", - "spiral/roadrunner": "^2024.3", - "spiral/roadrunner-cli": "^2.6", + "spiral/roadrunner": "^2025.1", + "spiral/roadrunner-cli": "^2.7", "spiral/roadrunner-http": "^3.5", "spiral/roadrunner-jobs": "^4.6", "symfony/console": "^7.2", From 497429e6859f774671995cd3b80beba6a765b10d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 23 Jun 2025 10:14:18 +0200 Subject: [PATCH 10/40] Forward questions to the global discussions repo --- .github/DISCUSSION_TEMPLATE/help-wanted.yml | 49 --------------------- .github/ISSUE_TEMPLATE.md | 7 --- .github/ISSUE_TEMPLATE/config.yml | 2 +- 3 files changed, 1 insertion(+), 57 deletions(-) delete mode 100644 .github/DISCUSSION_TEMPLATE/help-wanted.yml delete mode 100644 .github/ISSUE_TEMPLATE.md diff --git a/.github/DISCUSSION_TEMPLATE/help-wanted.yml b/.github/DISCUSSION_TEMPLATE/help-wanted.yml deleted file mode 100644 index 08444522..00000000 --- a/.github/DISCUSSION_TEMPLATE/help-wanted.yml +++ /dev/null @@ -1,49 +0,0 @@ -title: 'Help wanted' -body: - - type: input - validations: - required: true - attributes: - label: Shlink version - placeholder: x.y.z - - type: input - validations: - required: true - attributes: - label: PHP version - placeholder: x.y.z - - type: dropdown - validations: - required: true - attributes: - label: How do you serve Shlink - options: - - Self-hosted Apache - - Self-hosted nginx - - Self-hosted RoadRunner - - Docker image - - Other (explain in summary) - - type: dropdown - validations: - required: true - attributes: - label: Database engine - options: - - MySQL - - MariaDB - - PostgreSQL - - MicrosoftSQL - - SQLite - - type: input - validations: - required: true - attributes: - label: Database version - placeholder: x.y.z - - type: textarea - validations: - required: true - attributes: - label: Summary - value: '' - diff --git a/.github/ISSUE_TEMPLATE.md b/.github/ISSUE_TEMPLATE.md deleted file mode 100644 index 9a7f121f..00000000 --- a/.github/ISSUE_TEMPLATE.md +++ /dev/null @@ -1,7 +0,0 @@ - diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index 53fca8ef..70d678ca 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -2,4 +2,4 @@ blank_issues_enabled: true contact_links: - name: Question - Support about: Do you need help setting up or using Shlink? - url: https://github.com/shlinkio/shlink/discussions/new?category=help-wanted + url: https://github.com/orgs/shlinkio/discussions/new?category=help-wanted From f1157aa17735f64a8e37fadffed4285814102906 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 24 Jun 2025 19:47:18 +0200 Subject: [PATCH 11/40] Adjust tests to fix warnings --- .../CLI/test/RedirectRule/RedirectRuleHandlerTest.php | 1 + .../Adapter/ShortUrlRepositoryAdapterTest.php | 10 +++++----- .../Rest/src/Action/ShortUrl/DeleteShortUrlAction.php | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php index a7811060..aa6e0ac7 100644 --- a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php +++ b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php @@ -71,6 +71,7 @@ class RedirectRuleHandlerTest extends TestCase #[Test, DataProvider('provideExitActions')] public function rulesAreDisplayedWhenRulesListIsEmpty( RedirectRuleHandlerAction $action, + array|null $_, ): void { $comment = fn (string $value) => sprintf('%s', $value); diff --git a/module/Core/test/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php b/module/Core/test/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php index 473c2320..37f6a461 100644 --- a/module/Core/test/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php +++ b/module/Core/test/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php @@ -81,11 +81,11 @@ class ShortUrlRepositoryAdapterTest extends TestCase yield ['search']; yield ['search', []]; yield ['search', ['foo', 'bar']]; - yield ['search', ['foo', 'bar'], null, null, 'longUrl']; - yield ['search', ['foo', 'bar'], Chronos::now()->toAtomString(), null, 'longUrl']; - yield ['search', ['foo', 'bar'], null, Chronos::now()->toAtomString(), 'longUrl']; - yield ['search', ['foo', 'bar'], Chronos::now()->toAtomString(), Chronos::now()->toAtomString(), 'longUrl']; - yield [null, ['foo', 'bar'], Chronos::now()->toAtomString(), null, 'longUrl']; + yield ['search', ['foo', 'bar'], null, null]; + yield ['search', ['foo', 'bar'], Chronos::now()->toAtomString(), null]; + yield ['search', ['foo', 'bar'], null, Chronos::now()->toAtomString()]; + yield ['search', ['foo', 'bar'], Chronos::now()->toAtomString(), Chronos::now()->toAtomString()]; + yield [null, ['foo', 'bar'], Chronos::now()->toAtomString(), null]; yield [null, ['foo', 'bar'], Chronos::now()->toAtomString()]; yield [null, ['foo', 'bar'], Chronos::now()->toAtomString(), Chronos::now()->toAtomString()]; } diff --git a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php index b2570d36..9c7becb6 100644 --- a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php @@ -17,7 +17,7 @@ class DeleteShortUrlAction extends AbstractRestAction protected const string ROUTE_PATH = '/short-urls/{shortCode}'; protected const array ROUTE_ALLOWED_METHODS = [self::METHOD_DELETE]; - public function __construct(private DeleteShortUrlServiceInterface $deleteShortUrlService) + public function __construct(private readonly DeleteShortUrlServiceInterface $deleteShortUrlService) { } @@ -26,7 +26,7 @@ class DeleteShortUrlAction extends AbstractRestAction $identifier = ShortUrlIdentifier::fromApiRequest($request); $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); - $this->deleteShortUrlService->deleteByShortCode($identifier, false, $apiKey); + $this->deleteShortUrlService->deleteByShortCode($identifier, apiKey: $apiKey); return new EmptyResponse(); } From 850e8574e92d6c5d58de3c333b443bf7f4b94ec9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 26 Jun 2025 08:32:45 +0200 Subject: [PATCH 12/40] Use invokable commands approach on some API console commands --- composer.json | 10 +-- .../CLI/src/Command/Api/DisableKeyCommand.php | 87 ++++++++----------- .../CLI/src/Command/Api/ListKeysCommand.php | 37 ++++---- .../src/Command/Api/RenameApiKeyCommand.php | 35 ++++---- .../Command/Api/DisableKeyCommandTest.php | 8 +- .../Command/Api/RenameApiKeyCommandTest.php | 8 +- 6 files changed, 82 insertions(+), 103 deletions(-) diff --git a/composer.json b/composer.json index 49a7913a..26669e2e 100644 --- a/composer.json +++ b/composer.json @@ -54,11 +54,11 @@ "spiral/roadrunner-cli": "^2.7", "spiral/roadrunner-http": "^3.5", "spiral/roadrunner-jobs": "^4.6", - "symfony/console": "^7.2", - "symfony/filesystem": "^7.2", + "symfony/console": "^7.3", + "symfony/filesystem": "^7.3", "symfony/lock": "7.1.6", - "symfony/process": "^7.2", - "symfony/string": "^7.2" + "symfony/process": "^7.3", + "symfony/string": "^7.3" }, "require-dev": { "devizzent/cebe-php-openapi": "^1.1.2", @@ -73,7 +73,7 @@ "roave/security-advisories": "dev-master", "shlinkio/php-coding-standard": "~2.4.2", "shlinkio/shlink-test-utils": "^4.3.1", - "symfony/var-dumper": "^7.2", + "symfony/var-dumper": "^7.3", "veewee/composer-run-parallel": "^1.4" }, "conflict": { diff --git a/module/CLI/src/Command/Api/DisableKeyCommand.php b/module/CLI/src/Command/Api/DisableKeyCommand.php index 52169b26..308c432f 100644 --- a/module/CLI/src/Command/Api/DisableKeyCommand.php +++ b/module/CLI/src/Command/Api/DisableKeyCommand.php @@ -7,16 +7,40 @@ namespace Shlinkio\Shlink\CLI\Command\Api; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; +use Symfony\Component\Console\Attribute\Argument; +use Symfony\Component\Console\Attribute\AsCommand; +use Symfony\Component\Console\Attribute\Option; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; use function Shlinkio\Shlink\Core\ArrayUtils\map; use function sprintf; +#[AsCommand( + name: DisableKeyCommand::NAME, + description: 'Disables an API key by name or plain-text key (providing a plain-text key is DEPRECATED)', + help: <<%command.name% command allows you to disable an existing API key, via its name or the + plain-text key. + + If no arguments are provided, you will be prompted to select one of the existing non-disabled API keys. + + %command.full_name% + + You can optionally pass the API key name to be disabled. In that case --by-name is also + required, to indicate the first argument is the API key name and not the plain-text key: + + %command.full_name% the_key_name --by-name + + You can pass the plain-text key to be disabled, but that is DEPRECATED. In next major version, + the argument will always be assumed to be the name: + + %command.full_name% d6b6c60e-edcd-4e43-96ad-fa6b7014c143 + + HELP, +)] class DisableKeyCommand extends Command { public const string NAME = 'api-key:disable'; @@ -26,47 +50,9 @@ class DisableKeyCommand extends Command parent::__construct(); } - protected function configure(): void - { - $help = <<%command.name% command allows you to disable an existing API key, via its name or the - plain-text key. - - If no arguments are provided, you will be prompted to select one of the existing non-disabled API keys. - - %command.full_name% - - You can optionally pass the API key name to be disabled. In that case --by-name is also - required, to indicate the first argument is the API key name and not the plain-text key: - - %command.full_name% the_key_name --by-name - - You can pass the plain-text key to be disabled, but that is DEPRECATED. In next major version, - the argument will always be assumed to be the name: - - %command.full_name% d6b6c60e-edcd-4e43-96ad-fa6b7014c143 - - HELP; - - $this - ->setName(self::NAME) - ->setDescription('Disables an API key by name or plain-text key (providing a plain-text key is DEPRECATED)') - ->addArgument( - 'keyOrName', - InputArgument::OPTIONAL, - 'The API key to disable. Pass `--by-name` to indicate this value is the name and not the key.', - ) - ->addOption( - 'by-name', - mode: InputOption::VALUE_NONE, - description: 'Indicates the first argument is the API key name, not the plain-text key.', - ) - ->setHelp($help); - } - protected function interact(InputInterface $input, OutputInterface $output): void { - $keyOrName = $input->getArgument('keyOrName'); + $keyOrName = $input->getArgument('key-or-name'); if ($keyOrName === null) { $apiKeys = $this->apiKeyService->listKeys(enabledOnly: true); @@ -75,18 +61,21 @@ class DisableKeyCommand extends Command map($apiKeys, static fn (ApiKey $apiKey) => $apiKey->name), ); - $input->setArgument('keyOrName', $name); + $input->setArgument('key-or-name', $name); $input->setOption('by-name', true); } } - protected function execute(InputInterface $input, OutputInterface $output): int - { - $keyOrName = $input->getArgument('keyOrName'); - $byName = $input->getOption('by-name'); - $io = new SymfonyStyle($input, $output); - - if (! $keyOrName) { + public function __invoke( + SymfonyStyle $io, + #[Argument( + description: 'The API key to disable. Pass `--by-name` to indicate this value is the name and not the key.', + )] + string|null $keyOrName = null, + #[Option(description: 'Indicates the first argument is the API key name, not the plain-text key.')] + bool $byName = false, + ): int { + if ($keyOrName === null) { $io->warning('An API key name was not provided.'); return Command::INVALID; } diff --git a/module/CLI/src/Command/Api/ListKeysCommand.php b/module/CLI/src/Command/Api/ListKeysCommand.php index 7d63b6a4..a7b0a4be 100644 --- a/module/CLI/src/Command/Api/ListKeysCommand.php +++ b/module/CLI/src/Command/Api/ListKeysCommand.php @@ -8,16 +8,20 @@ use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Rest\ApiKey\Role; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; +use Symfony\Component\Console\Attribute\AsCommand; +use Symfony\Component\Console\Attribute\Option; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Input\InputOption; -use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Style\SymfonyStyle; use function array_filter; use function array_map; use function implode; use function sprintf; +#[AsCommand( + name: ListKeysCommand::NAME, + description: 'Lists all the available API keys.', +)] class ListKeysCommand extends Command { private const string ERROR_STRING_PATTERN = '%s'; @@ -31,23 +35,14 @@ class ListKeysCommand extends Command parent::__construct(); } - protected function configure(): void - { - $this - ->setName(self::NAME) - ->setDescription('Lists all the available API keys.') - ->addOption( - 'enabled-only', - 'e', - InputOption::VALUE_NONE, - 'Tells if only enabled API keys should be returned.', - ); - } - - protected function execute(InputInterface $input, OutputInterface $output): int - { - $enabledOnly = $input->getOption('enabled-only'); - + public function __invoke( + SymfonyStyle $io, + #[Option( + description: 'Tells if only enabled API keys should be returned.', + shortcut: 'e', + )] + bool $enabledOnly = false, + ): int { $rows = array_map(function (ApiKey $apiKey) use ($enabledOnly) { $expiration = $apiKey->expirationDate; $messagePattern = $this->determineMessagePattern($apiKey); @@ -65,7 +60,7 @@ class ListKeysCommand extends Command return $rowData; }, $this->apiKeyService->listKeys($enabledOnly)); - ShlinkTable::withRowSeparators($output)->render(array_filter([ + ShlinkTable::withRowSeparators($io)->render(array_filter([ 'Name', ! $enabledOnly ? 'Is enabled' : null, 'Expiration date', diff --git a/module/CLI/src/Command/Api/RenameApiKeyCommand.php b/module/CLI/src/Command/Api/RenameApiKeyCommand.php index f21d125b..fcbca1ce 100644 --- a/module/CLI/src/Command/Api/RenameApiKeyCommand.php +++ b/module/CLI/src/Command/Api/RenameApiKeyCommand.php @@ -8,14 +8,19 @@ use Shlinkio\Shlink\Core\Exception\InvalidArgumentException; use Shlinkio\Shlink\Core\Model\Renaming; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; +use Symfony\Component\Console\Attribute\Argument; +use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; use function Shlinkio\Shlink\Core\ArrayUtils\map; +#[AsCommand( + name: RenameApiKeyCommand::NAME, + description: 'Renames an API key by name', +)] class RenameApiKeyCommand extends Command { public const string NAME = 'api-key:rename'; @@ -25,20 +30,11 @@ class RenameApiKeyCommand extends Command parent::__construct(); } - protected function configure(): void - { - $this - ->setName(self::NAME) - ->setDescription('Renames an API key by name') - ->addArgument('oldName', InputArgument::REQUIRED, 'Current name of the API key to rename') - ->addArgument('newName', InputArgument::REQUIRED, 'New name to set to the API key'); - } - protected function interact(InputInterface $input, OutputInterface $output): void { $io = new SymfonyStyle($input, $output); - $oldName = $input->getArgument('oldName'); - $newName = $input->getArgument('newName'); + $oldName = $input->getArgument('old-name'); + $newName = $input->getArgument('new-name'); if ($oldName === null) { $apiKeys = $this->apiKeyService->listKeys(); @@ -47,7 +43,7 @@ class RenameApiKeyCommand extends Command map($apiKeys, static fn (ApiKey $apiKey) => $apiKey->name), ); - $input->setArgument('oldName', $requestedOldName); + $input->setArgument('old-name', $requestedOldName); } if ($newName === null) { @@ -58,16 +54,15 @@ class RenameApiKeyCommand extends Command : throw new InvalidArgumentException('The new name cannot be empty'), ); - $input->setArgument('newName', $requestedNewName); + $input->setArgument('new-name', $requestedNewName); } } - protected function execute(InputInterface $input, OutputInterface $output): int - { - $io = new SymfonyStyle($input, $output); - $oldName = $input->getArgument('oldName'); - $newName = $input->getArgument('newName'); - + public function __invoke( + SymfonyStyle $io, + #[Argument(description: 'Current name of the API key to rename')] string $oldName, + #[Argument(description: 'New name to set to the API key')] string $newName, + ): int { $this->apiKeyService->renameApiKey(Renaming::fromNames($oldName, $newName)); $io->success('API key properly renamed'); diff --git a/module/CLI/test/Command/Api/DisableKeyCommandTest.php b/module/CLI/test/Command/Api/DisableKeyCommandTest.php index e8a08336..85918305 100644 --- a/module/CLI/test/Command/Api/DisableKeyCommandTest.php +++ b/module/CLI/test/Command/Api/DisableKeyCommandTest.php @@ -35,7 +35,7 @@ class DisableKeyCommandTest extends TestCase $this->apiKeyService->expects($this->never())->method('disableByName'); $exitCode = $this->commandTester->execute([ - 'keyOrName' => $apiKey, + 'key-or-name' => $apiKey, ]); $output = $this->commandTester->getDisplay(); @@ -51,7 +51,7 @@ class DisableKeyCommandTest extends TestCase $this->apiKeyService->expects($this->never())->method('disableByKey'); $exitCode = $this->commandTester->execute([ - 'keyOrName' => $name, + 'key-or-name' => $name, '--by-name' => true, ]); $output = $this->commandTester->getDisplay(); @@ -71,7 +71,7 @@ class DisableKeyCommandTest extends TestCase $this->apiKeyService->expects($this->never())->method('disableByName'); $exitCode = $this->commandTester->execute([ - 'keyOrName' => $apiKey, + 'key-or-name' => $apiKey, ]); $output = $this->commandTester->getDisplay(); @@ -90,7 +90,7 @@ class DisableKeyCommandTest extends TestCase $this->apiKeyService->expects($this->never())->method('disableByKey'); $exitCode = $this->commandTester->execute([ - 'keyOrName' => $name, + 'key-or-name' => $name, '--by-name' => true, ]); $output = $this->commandTester->getDisplay(); diff --git a/module/CLI/test/Command/Api/RenameApiKeyCommandTest.php b/module/CLI/test/Command/Api/RenameApiKeyCommandTest.php index 41e5689f..d8c5f07f 100644 --- a/module/CLI/test/Command/Api/RenameApiKeyCommandTest.php +++ b/module/CLI/test/Command/Api/RenameApiKeyCommandTest.php @@ -43,7 +43,7 @@ class RenameApiKeyCommandTest extends TestCase $this->commandTester->setInputs([$oldName]); $this->commandTester->execute([ - 'newName' => $newName, + 'new-name' => $newName, ]); } @@ -60,7 +60,7 @@ class RenameApiKeyCommandTest extends TestCase $this->commandTester->setInputs([$newName]); $this->commandTester->execute([ - 'oldName' => $oldName, + 'old-name' => $oldName, ]); } @@ -76,8 +76,8 @@ class RenameApiKeyCommandTest extends TestCase ); $this->commandTester->execute([ - 'oldName' => $oldName, - 'newName' => $newName, + 'old-name' => $oldName, + 'new-name' => $newName, ]); } } From fb995f2bea7f8cc34f4b0de2f36c6c8bf5a029bb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 3 Jul 2025 10:10:06 +0200 Subject: [PATCH 13/40] Allow individual real-time updates topics to be enabled --- module/Core/config/dependencies.config.php | 1 + .../Core/config/event_dispatcher.config.php | 6 +++ module/Core/functions/functions.php | 16 ++++++++ module/Core/src/Config/EnvVars.php | 1 + .../Config/Options/RealTimeUpdatesOptions.php | 39 +++++++++++++++++++ .../AbstractNotifyNewShortUrlListener.php | 7 ++++ .../Async/AbstractNotifyVisitListener.php | 20 +++++++--- .../RabbitMq/NotifyNewShortUrlToRabbitMq.php | 4 +- .../RabbitMq/NotifyVisitToRabbitMq.php | 4 +- .../RedisPubSub/NotifyNewShortUrlToRedis.php | 4 +- .../RedisPubSub/NotifyVisitToRedis.php | 4 +- module/Core/src/EventDispatcher/Topic.php | 9 ++++- .../NotifyNewShortUrlToMercureTest.php | 2 + .../Mercure/NotifyVisitToMercureTest.php | 9 ++++- .../NotifyNewShortUrlToRabbitMqTest.php | 2 + .../RabbitMq/NotifyVisitToRabbitMqTest.php | 2 + .../NotifyNewShortUrlToRedisTest.php | 10 ++++- .../RedisPubSub/NotifyVisitToRedisTest.php | 10 ++++- 18 files changed, 137 insertions(+), 13 deletions(-) create mode 100644 module/Core/src/Config/Options/RealTimeUpdatesOptions.php diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 1cd1e961..75c70594 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -36,6 +36,7 @@ return [ Config\Options\QrCodeOptions::class => [Config\Options\QrCodeOptions::class, 'fromEnv'], Config\Options\RabbitMqOptions::class => [Config\Options\RabbitMqOptions::class, 'fromEnv'], Config\Options\RobotsOptions::class => [Config\Options\RobotsOptions::class, 'fromEnv'], + Config\Options\RealTimeUpdatesOptions::class => [Config\Options\RealTimeUpdatesOptions::class, 'fromEnv'], RedirectRule\ShortUrlRedirectRuleService::class => ConfigAbstractFactory::class, RedirectRule\ShortUrlRedirectionResolver::class => ConfigAbstractFactory::class, diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index d146a516..b5db4b85 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -110,18 +110,21 @@ return (static function (): array { EventDispatcher\PublishingUpdatesGenerator::class, 'em', 'Logger_Shlink', + Config\Options\RealTimeUpdatesOptions::class, ], EventDispatcher\Mercure\NotifyNewShortUrlToMercure::class => [ MercureHubPublishingHelper::class, EventDispatcher\PublishingUpdatesGenerator::class, 'em', 'Logger_Shlink', + Config\Options\RealTimeUpdatesOptions::class, ], EventDispatcher\RabbitMq\NotifyVisitToRabbitMq::class => [ RabbitMqPublishingHelper::class, EventDispatcher\PublishingUpdatesGenerator::class, 'em', 'Logger_Shlink', + Config\Options\RealTimeUpdatesOptions::class, Config\Options\RabbitMqOptions::class, ], EventDispatcher\RabbitMq\NotifyNewShortUrlToRabbitMq::class => [ @@ -129,6 +132,7 @@ return (static function (): array { EventDispatcher\PublishingUpdatesGenerator::class, 'em', 'Logger_Shlink', + Config\Options\RealTimeUpdatesOptions::class, Config\Options\RabbitMqOptions::class, ], EventDispatcher\RedisPubSub\NotifyVisitToRedis::class => [ @@ -136,6 +140,7 @@ return (static function (): array { EventDispatcher\PublishingUpdatesGenerator::class, 'em', 'Logger_Shlink', + Config\Options\RealTimeUpdatesOptions::class, 'config.redis.pub_sub_enabled', ], EventDispatcher\RedisPubSub\NotifyNewShortUrlToRedis::class => [ @@ -143,6 +148,7 @@ return (static function (): array { EventDispatcher\PublishingUpdatesGenerator::class, 'em', 'Logger_Shlink', + Config\Options\RealTimeUpdatesOptions::class, 'config.redis.pub_sub_enabled', ], diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 513e885d..a4126a13 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -266,6 +266,22 @@ function enumValues(string $enum): array ); } +/** + * @param class-string $enum + * @return string[] + */ +function enumNames(string $enum): array +{ + static $cache; + if ($cache === null) { + $cache = []; + } + + return $cache[$enum] ?? ( + $cache[$enum] = array_map(static fn (BackedEnum $type) => (string) $type->name, $enum::cases()) + ); +} + /** * @param class-string $enum */ diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 1f6f2c03..7fa1a95b 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -85,6 +85,7 @@ enum EnvVars: string case MEMORY_LIMIT = 'MEMORY_LIMIT'; case INITIAL_API_KEY = 'INITIAL_API_KEY'; case SKIP_INITIAL_GEOLITE_DOWNLOAD = 'SKIP_INITIAL_GEOLITE_DOWNLOAD'; + case REAL_TIME_UPDATES_TOPICS = 'REAL_TIME_UPDATES_TOPICS'; /** @deprecated Use REDIRECT_EXTRA_PATH */ case REDIRECT_APPEND_EXTRA_PATH = 'REDIRECT_APPEND_EXTRA_PATH'; diff --git a/module/Core/src/Config/Options/RealTimeUpdatesOptions.php b/module/Core/src/Config/Options/RealTimeUpdatesOptions.php new file mode 100644 index 00000000..35717593 --- /dev/null +++ b/module/Core/src/Config/Options/RealTimeUpdatesOptions.php @@ -0,0 +1,39 @@ +enabledTopics = $enabledTopics ?? Topic::allTopicNames(); + } + + public static function fromEnv(): self + { + $enabledTopics = splitByComma(EnvVars::REAL_TIME_UPDATES_TOPICS->loadFromEnv()); + + return new self( + enabledTopics: count($enabledTopics) === 0 + ? Topic::allTopicNames() + // TODO Validate provided topics are in fact Topic names + : splitByComma(EnvVars::REAL_TIME_UPDATES_TOPICS->loadFromEnv()), + ); + } + + public function isTopicEnabled(Topic $topic): bool + { + return contains($topic->name, $this->enabledTopics); + } +} diff --git a/module/Core/src/EventDispatcher/Async/AbstractNotifyNewShortUrlListener.php b/module/Core/src/EventDispatcher/Async/AbstractNotifyNewShortUrlListener.php index 1aa2235d..aced04f0 100644 --- a/module/Core/src/EventDispatcher/Async/AbstractNotifyNewShortUrlListener.php +++ b/module/Core/src/EventDispatcher/Async/AbstractNotifyNewShortUrlListener.php @@ -7,8 +7,10 @@ namespace Shlinkio\Shlink\Core\EventDispatcher\Async; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; +use Shlinkio\Shlink\Core\Config\Options\RealTimeUpdatesOptions; use Shlinkio\Shlink\Core\EventDispatcher\Event\ShortUrlCreated; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; +use Shlinkio\Shlink\Core\EventDispatcher\Topic; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Throwable; @@ -19,6 +21,7 @@ abstract class AbstractNotifyNewShortUrlListener extends AbstractAsyncListener private readonly PublishingUpdatesGeneratorInterface $updatesGenerator, private readonly EntityManagerInterface $em, private readonly LoggerInterface $logger, + private readonly RealTimeUpdatesOptions $realTimeUpdatesOptions, ) { } @@ -40,6 +43,10 @@ abstract class AbstractNotifyNewShortUrlListener extends AbstractAsyncListener return; } + if (! $this->realTimeUpdatesOptions->isTopicEnabled(Topic::NEW_SHORT_URL)) { + return; + } + try { $this->publishingHelper->publishUpdate($this->updatesGenerator->newShortUrlUpdate($shortUrl)); } catch (Throwable $e) { diff --git a/module/Core/src/EventDispatcher/Async/AbstractNotifyVisitListener.php b/module/Core/src/EventDispatcher/Async/AbstractNotifyVisitListener.php index e871588f..affe4f32 100644 --- a/module/Core/src/EventDispatcher/Async/AbstractNotifyVisitListener.php +++ b/module/Core/src/EventDispatcher/Async/AbstractNotifyVisitListener.php @@ -8,8 +8,10 @@ use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; use Shlinkio\Shlink\Common\UpdatePublishing\Update; +use Shlinkio\Shlink\Core\Config\Options\RealTimeUpdatesOptions; use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; +use Shlinkio\Shlink\Core\EventDispatcher\Topic; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Throwable; @@ -22,6 +24,7 @@ abstract class AbstractNotifyVisitListener extends AbstractAsyncListener private readonly PublishingUpdatesGeneratorInterface $updatesGenerator, private readonly EntityManagerInterface $em, private readonly LoggerInterface $logger, + private readonly RealTimeUpdatesOptions $realTimeUpdatesOptions, ) { } @@ -61,12 +64,19 @@ abstract class AbstractNotifyVisitListener extends AbstractAsyncListener protected function determineUpdatesForVisit(Visit $visit): array { if ($visit->isOrphan()) { - return [$this->updatesGenerator->newOrphanVisitUpdate($visit)]; + return $this->realTimeUpdatesOptions->isTopicEnabled(Topic::NEW_ORPHAN_VISIT) + ? [$this->updatesGenerator->newOrphanVisitUpdate($visit)] + : []; } - return [ - $this->updatesGenerator->newShortUrlVisitUpdate($visit), - $this->updatesGenerator->newVisitUpdate($visit), - ]; + $topics = []; + if ($this->realTimeUpdatesOptions->isTopicEnabled(Topic::NEW_SHORT_URL_VISIT)) { + $topics[] = $this->updatesGenerator->newShortUrlVisitUpdate($visit); + } + if ($this->realTimeUpdatesOptions->isTopicEnabled(Topic::NEW_VISIT)) { + $topics[] = $this->updatesGenerator->newVisitUpdate($visit); + } + + return $topics; } } diff --git a/module/Core/src/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMq.php b/module/Core/src/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMq.php index b8c18fb6..ef63b1e4 100644 --- a/module/Core/src/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMq.php +++ b/module/Core/src/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMq.php @@ -8,6 +8,7 @@ use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; use Shlinkio\Shlink\Core\Config\Options\RabbitMqOptions; +use Shlinkio\Shlink\Core\Config\Options\RealTimeUpdatesOptions; use Shlinkio\Shlink\Core\EventDispatcher\Async\AbstractNotifyNewShortUrlListener; use Shlinkio\Shlink\Core\EventDispatcher\Async\RemoteSystem; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; @@ -19,9 +20,10 @@ class NotifyNewShortUrlToRabbitMq extends AbstractNotifyNewShortUrlListener PublishingUpdatesGeneratorInterface $updatesGenerator, EntityManagerInterface $em, LoggerInterface $logger, + RealTimeUpdatesOptions $realTimeUpdatesOptions, private readonly RabbitMqOptions $options, ) { - parent::__construct($rabbitMqHelper, $updatesGenerator, $em, $logger); + parent::__construct($rabbitMqHelper, $updatesGenerator, $em, $logger, $realTimeUpdatesOptions); } protected function isEnabled(): bool diff --git a/module/Core/src/EventDispatcher/RabbitMq/NotifyVisitToRabbitMq.php b/module/Core/src/EventDispatcher/RabbitMq/NotifyVisitToRabbitMq.php index bb55f169..0bab0739 100644 --- a/module/Core/src/EventDispatcher/RabbitMq/NotifyVisitToRabbitMq.php +++ b/module/Core/src/EventDispatcher/RabbitMq/NotifyVisitToRabbitMq.php @@ -8,6 +8,7 @@ use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; use Shlinkio\Shlink\Core\Config\Options\RabbitMqOptions; +use Shlinkio\Shlink\Core\Config\Options\RealTimeUpdatesOptions; use Shlinkio\Shlink\Core\EventDispatcher\Async\AbstractNotifyVisitListener; use Shlinkio\Shlink\Core\EventDispatcher\Async\RemoteSystem; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; @@ -19,9 +20,10 @@ class NotifyVisitToRabbitMq extends AbstractNotifyVisitListener PublishingUpdatesGeneratorInterface $updatesGenerator, EntityManagerInterface $em, LoggerInterface $logger, + RealTimeUpdatesOptions $realTimeUpdatesOptions, private readonly RabbitMqOptions $options, ) { - parent::__construct($rabbitMqHelper, $updatesGenerator, $em, $logger); + parent::__construct($rabbitMqHelper, $updatesGenerator, $em, $logger, $realTimeUpdatesOptions); } protected function isEnabled(): bool diff --git a/module/Core/src/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedis.php b/module/Core/src/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedis.php index 5cee9d5e..62b4e291 100644 --- a/module/Core/src/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedis.php +++ b/module/Core/src/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedis.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\EventDispatcher\RedisPubSub; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; +use Shlinkio\Shlink\Core\Config\Options\RealTimeUpdatesOptions; use Shlinkio\Shlink\Core\EventDispatcher\Async\AbstractNotifyNewShortUrlListener; use Shlinkio\Shlink\Core\EventDispatcher\Async\RemoteSystem; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; @@ -18,9 +19,10 @@ class NotifyNewShortUrlToRedis extends AbstractNotifyNewShortUrlListener PublishingUpdatesGeneratorInterface $updatesGenerator, EntityManagerInterface $em, LoggerInterface $logger, + RealTimeUpdatesOptions $realTimeUpdatesOptions, private readonly bool $enabled, ) { - parent::__construct($redisHelper, $updatesGenerator, $em, $logger); + parent::__construct($redisHelper, $updatesGenerator, $em, $logger, $realTimeUpdatesOptions); } protected function isEnabled(): bool diff --git a/module/Core/src/EventDispatcher/RedisPubSub/NotifyVisitToRedis.php b/module/Core/src/EventDispatcher/RedisPubSub/NotifyVisitToRedis.php index ae349495..1d4dc5be 100644 --- a/module/Core/src/EventDispatcher/RedisPubSub/NotifyVisitToRedis.php +++ b/module/Core/src/EventDispatcher/RedisPubSub/NotifyVisitToRedis.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\EventDispatcher\RedisPubSub; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; +use Shlinkio\Shlink\Core\Config\Options\RealTimeUpdatesOptions; use Shlinkio\Shlink\Core\EventDispatcher\Async\AbstractNotifyVisitListener; use Shlinkio\Shlink\Core\EventDispatcher\Async\RemoteSystem; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; @@ -18,9 +19,10 @@ class NotifyVisitToRedis extends AbstractNotifyVisitListener PublishingUpdatesGeneratorInterface $updatesGenerator, EntityManagerInterface $em, LoggerInterface $logger, + RealTimeUpdatesOptions $realTimeUpdatesOptions, private readonly bool $enabled, ) { - parent::__construct($redisHelper, $updatesGenerator, $em, $logger); + parent::__construct($redisHelper, $updatesGenerator, $em, $logger, $realTimeUpdatesOptions); } protected function isEnabled(): bool diff --git a/module/Core/src/EventDispatcher/Topic.php b/module/Core/src/EventDispatcher/Topic.php index 8c7a7d45..9f018bb3 100644 --- a/module/Core/src/EventDispatcher/Topic.php +++ b/module/Core/src/EventDispatcher/Topic.php @@ -4,16 +4,23 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\EventDispatcher; +use function Shlinkio\Shlink\Core\enumNames; use function sprintf; enum Topic: string { case NEW_VISIT = 'https://shlink.io/new-visit'; + case NEW_SHORT_URL_VISIT = 'https://shlink.io/new-visit/%s'; case NEW_ORPHAN_VISIT = 'https://shlink.io/new-orphan-visit'; case NEW_SHORT_URL = 'https://shlink.io/new-short-url'; public static function newShortUrlVisit(string|null $shortCode): string { - return sprintf('%s/%s', self::NEW_VISIT->value, $shortCode ?? ''); + return sprintf(self::NEW_SHORT_URL_VISIT->value, $shortCode ?? ''); + } + + public static function allTopicNames(): array + { + return enumNames(self::class); } } diff --git a/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php b/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php index 20d6830d..c6b67727 100644 --- a/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php +++ b/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php @@ -12,6 +12,7 @@ use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; use Shlinkio\Shlink\Common\UpdatePublishing\Update; +use Shlinkio\Shlink\Core\Config\Options\RealTimeUpdatesOptions; use Shlinkio\Shlink\Core\EventDispatcher\Event\ShortUrlCreated; use Shlinkio\Shlink\Core\EventDispatcher\Mercure\NotifyNewShortUrlToMercure; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; @@ -37,6 +38,7 @@ class NotifyNewShortUrlToMercureTest extends TestCase $this->updatesGenerator, $this->em, $this->logger, + new RealTimeUpdatesOptions(), ); } diff --git a/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php b/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php index 91569c9b..b362c1a5 100644 --- a/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php +++ b/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php @@ -13,6 +13,7 @@ use Psr\Log\LoggerInterface; use RuntimeException; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; use Shlinkio\Shlink\Common\UpdatePublishing\Update; +use Shlinkio\Shlink\Core\Config\Options\RealTimeUpdatesOptions; use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited; use Shlinkio\Shlink\Core\EventDispatcher\Mercure\NotifyVisitToMercure; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; @@ -36,7 +37,13 @@ class NotifyVisitToMercureTest extends TestCase $this->em = $this->createMock(EntityManagerInterface::class); $this->logger = $this->createMock(LoggerInterface::class); - $this->listener = new NotifyVisitToMercure($this->helper, $this->updatesGenerator, $this->em, $this->logger); + $this->listener = new NotifyVisitToMercure( + $this->helper, + $this->updatesGenerator, + $this->em, + $this->logger, + new RealTimeUpdatesOptions(), + ); } #[Test] diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php index b0c5a0e0..e4351c36 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php @@ -16,6 +16,7 @@ use RuntimeException; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; use Shlinkio\Shlink\Common\UpdatePublishing\Update; use Shlinkio\Shlink\Core\Config\Options\RabbitMqOptions; +use Shlinkio\Shlink\Core\Config\Options\RealTimeUpdatesOptions; use Shlinkio\Shlink\Core\EventDispatcher\Event\ShortUrlCreated; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; use Shlinkio\Shlink\Core\EventDispatcher\RabbitMq\NotifyNewShortUrlToRabbitMq; @@ -115,6 +116,7 @@ class NotifyNewShortUrlToRabbitMqTest extends TestCase $this->updatesGenerator, $this->em, $this->logger, + new RealTimeUpdatesOptions(), new RabbitMqOptions($enabled), ); } diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php index 8af719b3..78abb5c7 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php @@ -17,6 +17,7 @@ use RuntimeException; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; use Shlinkio\Shlink\Common\UpdatePublishing\Update; use Shlinkio\Shlink\Core\Config\Options\RabbitMqOptions; +use Shlinkio\Shlink\Core\Config\Options\RealTimeUpdatesOptions; use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; use Shlinkio\Shlink\Core\EventDispatcher\RabbitMq\NotifyVisitToRabbitMq; @@ -189,6 +190,7 @@ class NotifyVisitToRabbitMqTest extends TestCase $this->updatesGenerator, $this->em, $this->logger, + new RealTimeUpdatesOptions(), $options ?? new RabbitMqOptions(enabled: true), ); } diff --git a/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php b/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php index abbe23b9..412f603e 100644 --- a/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php +++ b/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php @@ -15,6 +15,7 @@ use Psr\Log\LoggerInterface; use RuntimeException; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; use Shlinkio\Shlink\Common\UpdatePublishing\Update; +use Shlinkio\Shlink\Core\Config\Options\RealTimeUpdatesOptions; use Shlinkio\Shlink\Core\EventDispatcher\Event\ShortUrlCreated; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; use Shlinkio\Shlink\Core\EventDispatcher\RedisPubSub\NotifyNewShortUrlToRedis; @@ -77,6 +78,13 @@ class NotifyNewShortUrlToRedisTest extends TestCase private function createListener(bool $enabled = true): NotifyNewShortUrlToRedis { - return new NotifyNewShortUrlToRedis($this->helper, $this->updatesGenerator, $this->em, $this->logger, $enabled); + return new NotifyNewShortUrlToRedis( + $this->helper, + $this->updatesGenerator, + $this->em, + $this->logger, + new RealTimeUpdatesOptions(), + $enabled, + ); } } diff --git a/module/Core/test/EventDispatcher/RedisPubSub/NotifyVisitToRedisTest.php b/module/Core/test/EventDispatcher/RedisPubSub/NotifyVisitToRedisTest.php index 7cbf68b6..d14ab631 100644 --- a/module/Core/test/EventDispatcher/RedisPubSub/NotifyVisitToRedisTest.php +++ b/module/Core/test/EventDispatcher/RedisPubSub/NotifyVisitToRedisTest.php @@ -15,6 +15,7 @@ use Psr\Log\LoggerInterface; use RuntimeException; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; use Shlinkio\Shlink\Common\UpdatePublishing\Update; +use Shlinkio\Shlink\Core\Config\Options\RealTimeUpdatesOptions; use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; use Shlinkio\Shlink\Core\EventDispatcher\RedisPubSub\NotifyVisitToRedis; @@ -76,6 +77,13 @@ class NotifyVisitToRedisTest extends TestCase private function createListener(bool $enabled = true): NotifyVisitToRedis { - return new NotifyVisitToRedis($this->helper, $this->updatesGenerator, $this->em, $this->logger, $enabled); + return new NotifyVisitToRedis( + $this->helper, + $this->updatesGenerator, + $this->em, + $this->logger, + new RealTimeUpdatesOptions(), + $enabled, + ); } } From 240d9df1770de7f26a610bbe405be89cb5d7cd18 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 3 Jul 2025 14:34:27 +0200 Subject: [PATCH 14/40] Validate topic names in RealTimeUpdateOptions --- .../Config/Options/RealTimeUpdatesOptions.php | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/module/Core/src/Config/Options/RealTimeUpdatesOptions.php b/module/Core/src/Config/Options/RealTimeUpdatesOptions.php index 35717593..7e41f679 100644 --- a/module/Core/src/Config/Options/RealTimeUpdatesOptions.php +++ b/module/Core/src/Config/Options/RealTimeUpdatesOptions.php @@ -6,13 +6,18 @@ namespace Shlinkio\Shlink\Core\Config\Options; use Shlinkio\Shlink\Core\Config\EnvVars; use Shlinkio\Shlink\Core\EventDispatcher\Topic; +use Shlinkio\Shlink\Core\Exception\ValidationException; use function count; +use function implode; use function Shlinkio\Shlink\Core\ArrayUtils\contains; +use function Shlinkio\Shlink\Core\ArrayUtils\map; use function Shlinkio\Shlink\Core\splitByComma; +use function sprintf; final readonly class RealTimeUpdatesOptions { + /** @var string[] */ public array $enabledTopics; public function __construct(array|null $enabledTopics = null) @@ -23,15 +28,35 @@ final readonly class RealTimeUpdatesOptions public static function fromEnv(): self { $enabledTopics = splitByComma(EnvVars::REAL_TIME_UPDATES_TOPICS->loadFromEnv()); + $validTopics = Topic::allTopicNames(); return new self( - enabledTopics: count($enabledTopics) === 0 - ? Topic::allTopicNames() - // TODO Validate provided topics are in fact Topic names - : splitByComma(EnvVars::REAL_TIME_UPDATES_TOPICS->loadFromEnv()), + enabledTopics: count($enabledTopics) === 0 ? $validTopics : self::topicsFromEnv($validTopics), ); } + /** + * @param string[] $validTopics + * @return string[] + */ + private static function topicsFromEnv(array $validTopics): array + { + $topics = splitByComma(EnvVars::REAL_TIME_UPDATES_TOPICS->loadFromEnv()); + return map($topics, function (string $topic) use ($validTopics): string { + if (contains($topic, $validTopics)) { + return $topic; + } + + throw ValidationException::fromArray([ + 'topic' => sprintf( + 'Real-time updates topic "%s" is not valid. Expected one of ["%s"].', + $topic, + implode('", "', $validTopics), + ), + ]); + }); + } + public function isTopicEnabled(Topic $topic): bool { return contains($topic->name, $this->enabledTopics); From 314a99862dae8e1c218e9c59be11883b0fd4ae91 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 3 Jul 2025 18:35:14 +0200 Subject: [PATCH 15/40] Update to latest shlink-installer with real-time updates support --- composer.json | 2 +- config/autoload/installer.global.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 26669e2e..f391842b 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,7 @@ "shlinkio/shlink-config": "^4.0", "shlinkio/shlink-event-dispatcher": "^4.2", "shlinkio/shlink-importer": "^5.6", - "shlinkio/shlink-installer": "^9.5", + "shlinkio/shlink-installer": "dev-develop#c5a8523 as 9.6", "shlinkio/shlink-ip-geolocation": "^4.3", "shlinkio/shlink-json": "^1.2", "spiral/roadrunner": "^2025.1", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 24b9263e..063ff7d4 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -76,6 +76,7 @@ return [ Option\Matomo\MatomoBaseUrlConfigOption::class, Option\Matomo\MatomoSiteIdConfigOption::class, Option\Matomo\MatomoApiTokenConfigOption::class, + Option\RealTimeUpdates\RealTimeUpdatesTopicsConfigOption::class, ], 'installation_commands' => [ From f4aaf02d555998b725815271f06acfebca3512f7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 4 Jul 2025 09:52:35 +0200 Subject: [PATCH 16/40] Reduce duplicated code between enumValues and enumNames --- module/Core/functions/functions.php | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index a4126a13..72406e6a 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -256,14 +256,7 @@ function toProblemDetailsType(string $errorCode): string */ function enumValues(string $enum): array { - static $cache; - if ($cache === null) { - $cache = []; - } - - return $cache[$enum] ?? ( - $cache[$enum] = array_map(static fn (BackedEnum $type) => (string) $type->value, $enum::cases()) - ); + return enumSide($enum, 'value'); } /** @@ -271,14 +264,27 @@ function enumValues(string $enum): array * @return string[] */ function enumNames(string $enum): array +{ + return enumSide($enum, 'name'); +} + +/** + * @param class-string $enum + * @param 'name'|'value' $type + * @return string[] + */ +function enumSide(string $enum, string $type): array { static $cache; if ($cache === null) { $cache = []; } - return $cache[$enum] ?? ( - $cache[$enum] = array_map(static fn (BackedEnum $type) => (string) $type->name, $enum::cases()) + return $cache[$type][$enum] ?? ( + $cache[$type][$enum] = array_map( + static fn (BackedEnum $entry) => (string) ($type === 'name' ? $entry->name : $entry->value), + $enum::cases(), + ) ); } From 26fef87f3bcc4150e900a8001ae8d703a545bf27 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 4 Jul 2025 10:07:40 +0200 Subject: [PATCH 17/40] Add RealTimeUpdatesOptions test --- .../Config/Options/RealTimeUpdatesOptions.php | 17 ++++---- .../Options/RealTimeUpdatesOptionsTest.php | 41 +++++++++++++++++++ 2 files changed, 49 insertions(+), 9 deletions(-) create mode 100644 module/Core/test/Config/Options/RealTimeUpdatesOptionsTest.php diff --git a/module/Core/src/Config/Options/RealTimeUpdatesOptions.php b/module/Core/src/Config/Options/RealTimeUpdatesOptions.php index 7e41f679..1800b92f 100644 --- a/module/Core/src/Config/Options/RealTimeUpdatesOptions.php +++ b/module/Core/src/Config/Options/RealTimeUpdatesOptions.php @@ -22,27 +22,26 @@ final readonly class RealTimeUpdatesOptions public function __construct(array|null $enabledTopics = null) { - $this->enabledTopics = $enabledTopics ?? Topic::allTopicNames(); + $validTopics = Topic::allTopicNames(); + $this->enabledTopics = $enabledTopics === null ? $validTopics : self::validateTopics( + $enabledTopics, + $validTopics, + ); } public static function fromEnv(): self { $enabledTopics = splitByComma(EnvVars::REAL_TIME_UPDATES_TOPICS->loadFromEnv()); - $validTopics = Topic::allTopicNames(); - - return new self( - enabledTopics: count($enabledTopics) === 0 ? $validTopics : self::topicsFromEnv($validTopics), - ); + return new self(enabledTopics: count($enabledTopics) === 0 ? null : $enabledTopics); } /** * @param string[] $validTopics * @return string[] */ - private static function topicsFromEnv(array $validTopics): array + private static function validateTopics(array $providedTopics, array $validTopics): array { - $topics = splitByComma(EnvVars::REAL_TIME_UPDATES_TOPICS->loadFromEnv()); - return map($topics, function (string $topic) use ($validTopics): string { + return map($providedTopics, function (string $topic) use ($validTopics): string { if (contains($topic, $validTopics)) { return $topic; } diff --git a/module/Core/test/Config/Options/RealTimeUpdatesOptionsTest.php b/module/Core/test/Config/Options/RealTimeUpdatesOptionsTest.php new file mode 100644 index 00000000..0e1634f1 --- /dev/null +++ b/module/Core/test/Config/Options/RealTimeUpdatesOptionsTest.php @@ -0,0 +1,41 @@ +enabledTopics); + } + + #[Test] + public function exceptionIsThrownIfAnyProvidedTopicIsInvalid(): void + { + $this->expectException(ValidationException::class); + new RealTimeUpdatesOptions(['NEW_SHORT_URL_VISIT', 'invalid']); + } + + #[Test] + public function checkingIfTopicIsEnabledWorks(): void + { + $options = new RealTimeUpdatesOptions(['NEW_ORPHAN_VISIT', 'NEW_SHORT_URL']); + + self::assertTrue($options->isTopicEnabled(Topic::NEW_ORPHAN_VISIT)); + self::assertTrue($options->isTopicEnabled(Topic::NEW_SHORT_URL)); + self::assertFalse($options->isTopicEnabled(Topic::NEW_VISIT)); + self::assertFalse($options->isTopicEnabled(Topic::NEW_SHORT_URL_VISIT)); + } +} From 733b2e56475d5349d373488324fc3d38132aced0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 4 Jul 2025 18:04:27 +0200 Subject: [PATCH 18/40] Add test to cover when short URL updates topic is disabled --- .../NotifyNewShortUrlToMercureTest.php | 39 +++++++++++++------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php b/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php index c6b67727..3294e929 100644 --- a/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php +++ b/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php @@ -20,7 +20,6 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; class NotifyNewShortUrlToMercureTest extends TestCase { - private NotifyNewShortUrlToMercure $listener; private MockObject & PublishingHelperInterface $helper; private MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator; private MockObject & EntityManagerInterface $em; @@ -32,14 +31,6 @@ class NotifyNewShortUrlToMercureTest extends TestCase $this->updatesGenerator = $this->createMock(PublishingUpdatesGeneratorInterface::class); $this->em = $this->createMock(EntityManagerInterface::class); $this->logger = $this->createMock(LoggerInterface::class); - - $this->listener = new NotifyNewShortUrlToMercure( - $this->helper, - $this->updatesGenerator, - $this->em, - $this->logger, - new RealTimeUpdatesOptions(), - ); } #[Test] @@ -54,7 +45,7 @@ class NotifyNewShortUrlToMercureTest extends TestCase ); $this->logger->expects($this->never())->method('debug'); - ($this->listener)(new ShortUrlCreated('123')); + $this->listener()(new ShortUrlCreated('123')); } #[Test] @@ -71,7 +62,7 @@ class NotifyNewShortUrlToMercureTest extends TestCase $this->logger->expects($this->never())->method('warning'); $this->logger->expects($this->never())->method('debug'); - ($this->listener)(new ShortUrlCreated('123')); + $this->listener()(new ShortUrlCreated('123')); } #[Test] @@ -95,6 +86,30 @@ class NotifyNewShortUrlToMercureTest extends TestCase ['e' => $e, 'name' => 'Mercure'], ); - ($this->listener)(new ShortUrlCreated('123')); + $this->listener()(new ShortUrlCreated('123')); + } + + #[Test] + public function publishingIsSkippedIfNewShortUrlTopicIsNotEnabled(): void + { + $shortUrl = ShortUrl::withLongUrl('https://longUrl'); + $update = Update::forTopicAndPayload('', []); + + $this->em->expects($this->once())->method('find')->with(ShortUrl::class, '123')->willReturn($shortUrl); + $this->updatesGenerator->expects($this->never())->method('newShortUrlUpdate'); + $this->helper->expects($this->never())->method('publishUpdate'); + + $this->listener(enableShortUrlTopic: false)(new ShortUrlCreated('123')); + } + + private function listener(bool $enableShortUrlTopic = true): NotifyNewShortUrlToMercure + { + return new NotifyNewShortUrlToMercure( + $this->helper, + $this->updatesGenerator, + $this->em, + $this->logger, + new RealTimeUpdatesOptions(enabledTopics: $enableShortUrlTopic ? null : []), + ); } } From 9e93e34e12506b1d53e5d8b9dd7377617893fe1b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 4 Jul 2025 18:25:45 +0200 Subject: [PATCH 19/40] Add test to cover when visit updates topics are disabled --- .../Mercure/NotifyVisitToMercureTest.php | 63 ++++++++++--------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php b/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php index b362c1a5..b86a2896 100644 --- a/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php +++ b/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php @@ -7,6 +7,7 @@ namespace ShlinkioTest\Shlink\Core\EventDispatcher\Mercure; use Doctrine\ORM\EntityManagerInterface; 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\Log\LoggerInterface; @@ -24,7 +25,6 @@ use Shlinkio\Shlink\Core\Visit\Model\VisitType; class NotifyVisitToMercureTest extends TestCase { - private NotifyVisitToMercure $listener; private MockObject & PublishingHelperInterface $helper; private MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator; private MockObject & EntityManagerInterface $em; @@ -36,14 +36,6 @@ class NotifyVisitToMercureTest extends TestCase $this->updatesGenerator = $this->createMock(PublishingUpdatesGeneratorInterface::class); $this->em = $this->createMock(EntityManagerInterface::class); $this->logger = $this->createMock(LoggerInterface::class); - - $this->listener = new NotifyVisitToMercure( - $this->helper, - $this->updatesGenerator, - $this->em, - $this->logger, - new RealTimeUpdatesOptions(), - ); } #[Test] @@ -61,11 +53,15 @@ class NotifyVisitToMercureTest extends TestCase $this->updatesGenerator->expects($this->never())->method('newVisitUpdate'); $this->helper->expects($this->never())->method('publishUpdate'); - ($this->listener)(new UrlVisited($visitId)); + $this->listener()(new UrlVisited($visitId)); } #[Test] - public function notificationsAreSentWhenVisitIsFound(): void + #[TestWith([2, ['NEW_SHORT_URL_VISIT', 'NEW_VISIT']])] + #[TestWith([1, ['NEW_VISIT']])] + #[TestWith([1, ['NEW_SHORT_URL_VISIT']])] + #[TestWith([0, []])] + public function notificationsAreSentWhenVisitIsFound(int $publishUpdateCalls, array $enabledTopics): void { $visitId = '123'; $visit = Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::empty()); @@ -74,14 +70,12 @@ class NotifyVisitToMercureTest extends TestCase $this->em->expects($this->once())->method('find')->with(Visit::class, $visitId)->willReturn($visit); $this->logger->expects($this->never())->method('warning'); $this->logger->expects($this->never())->method('debug'); - $this->updatesGenerator->expects($this->once())->method('newShortUrlVisitUpdate')->with($visit)->willReturn( - $update, - ); + $this->updatesGenerator->method('newShortUrlVisitUpdate')->willReturn($update); $this->updatesGenerator->expects($this->never())->method('newOrphanVisitUpdate'); - $this->updatesGenerator->expects($this->once())->method('newVisitUpdate')->with($visit)->willReturn($update); - $this->helper->expects($this->exactly(2))->method('publishUpdate')->with($update); + $this->updatesGenerator->method('newVisitUpdate')->willReturn($update); + $this->helper->expects($this->exactly($publishUpdateCalls))->method('publishUpdate')->with($update); - ($this->listener)(new UrlVisited($visitId)); + $this->listener(enabledTopics: $enabledTopics)(new UrlVisited($visitId)); } #[Test] @@ -105,12 +99,15 @@ class NotifyVisitToMercureTest extends TestCase $this->updatesGenerator->expects($this->once())->method('newVisitUpdate')->with($visit)->willReturn($update); $this->helper->expects($this->once())->method('publishUpdate')->with($update)->willThrowException($e); - ($this->listener)(new UrlVisited($visitId)); + $this->listener()(new UrlVisited($visitId)); } #[Test, DataProvider('provideOrphanVisits')] - public function notificationsAreSentForOrphanVisits(Visit $visit): void - { + public function notificationsAreSentForOrphanVisits( + Visit $visit, + array $enabledTopics, + int $publishUpdateCalls, + ): void { $visitId = '123'; $update = Update::forTopicAndPayload('', []); @@ -118,21 +115,31 @@ class NotifyVisitToMercureTest extends TestCase $this->logger->expects($this->never())->method('warning'); $this->logger->expects($this->never())->method('debug'); $this->updatesGenerator->expects($this->never())->method('newShortUrlVisitUpdate'); - $this->updatesGenerator->expects($this->once())->method('newOrphanVisitUpdate')->with($visit)->willReturn( - $update, - ); + $this->updatesGenerator->method('newOrphanVisitUpdate')->willReturn($update); $this->updatesGenerator->expects($this->never())->method('newVisitUpdate'); - $this->helper->expects($this->once())->method('publishUpdate')->with($update); + $this->helper->expects($this->exactly($publishUpdateCalls))->method('publishUpdate')->with($update); - ($this->listener)(new UrlVisited($visitId)); + $this->listener(enabledTopics: $enabledTopics)(new UrlVisited($visitId)); } public static function provideOrphanVisits(): iterable { $visitor = Visitor::empty(); - yield VisitType::REGULAR_404->value => [Visit::forRegularNotFound($visitor)]; - yield VisitType::INVALID_SHORT_URL->value => [Visit::forInvalidShortUrl($visitor)]; - yield VisitType::BASE_URL->value => [Visit::forBasePath($visitor)]; + yield VisitType::REGULAR_404->value => [Visit::forRegularNotFound($visitor), ['NEW_ORPHAN_VISIT'], 1]; + yield VisitType::INVALID_SHORT_URL->value => [Visit::forInvalidShortUrl($visitor), ['NEW_ORPHAN_VISIT'], 1]; + yield VisitType::BASE_URL->value => [Visit::forBasePath($visitor), ['NEW_ORPHAN_VISIT'], 1]; + yield VisitType::BASE_URL->value . ' disabled' => [Visit::forBasePath($visitor), [], 0]; + } + + private function listener(array|null $enabledTopics = null): NotifyVisitToMercure + { + return new NotifyVisitToMercure( + $this->helper, + $this->updatesGenerator, + $this->em, + $this->logger, + new RealTimeUpdatesOptions(enabledTopics: $enabledTopics), + ); } } From c8e3b3df0a086c98e3914dc6a250de53f8c0d3e6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 4 Jul 2025 18:31:20 +0200 Subject: [PATCH 20/40] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 078a9835..a7b4c4ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this For BC, if this env vars is not present, we'll still consider the integration enabled if the `MERCURE_PUBLIC_HUB_URL` env var has a value. This is considered deprecated though, and next major version will rely only on `MERCURE_ENABLED`, so if you are using Mercure, make sure to set `MERCURE_ENABLED=true` to be ready. +* [#2387](https://github.com/shlinkio/shlink/issues/2387) Add `REAL_TIME_UPDATES_TOPICS` env var and corresponding config option, to granularly decide which real-time updates topics should be enabled. + ### Changed * [#2406](https://github.com/shlinkio/shlink/issues/2406) Remove references to bootstrap from error templates, and instead inline the very minimum required styles. From 92d7a44cee7b664d83a3b93f0ea58bf875af9197 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 5 Jul 2025 10:34:50 +0200 Subject: [PATCH 21/40] Add new CORS configuration options --- config/autoload/cors.global.php | 11 ---- module/Core/config/dependencies.config.php | 1 + module/Core/src/Config/EnvVars.php | 7 +++ .../Core/src/Config/Options/CorsOptions.php | 58 +++++++++++++++++++ module/Rest/config/dependencies.config.php | 2 +- .../src/Middleware/CrossDomainMiddleware.php | 9 +-- .../Middleware/CrossDomainMiddlewareTest.php | 3 +- 7 files changed, 74 insertions(+), 17 deletions(-) delete mode 100644 config/autoload/cors.global.php create mode 100644 module/Core/src/Config/Options/CorsOptions.php diff --git a/config/autoload/cors.global.php b/config/autoload/cors.global.php deleted file mode 100644 index 58ad9428..00000000 --- a/config/autoload/cors.global.php +++ /dev/null @@ -1,11 +0,0 @@ - [ - 'max_age' => 3600, - ], - -]; diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 75c70594..0ad943e9 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -37,6 +37,7 @@ return [ Config\Options\RabbitMqOptions::class => [Config\Options\RabbitMqOptions::class, 'fromEnv'], Config\Options\RobotsOptions::class => [Config\Options\RobotsOptions::class, 'fromEnv'], Config\Options\RealTimeUpdatesOptions::class => [Config\Options\RealTimeUpdatesOptions::class, 'fromEnv'], + Config\Options\CorsOptions::class => [Config\Options\CorsOptions::class, 'fromEnv'], RedirectRule\ShortUrlRedirectRuleService::class => ConfigAbstractFactory::class, RedirectRule\ShortUrlRedirectionResolver::class => ConfigAbstractFactory::class, diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 7fa1a95b..4f57a721 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -86,6 +86,9 @@ enum EnvVars: string case INITIAL_API_KEY = 'INITIAL_API_KEY'; case SKIP_INITIAL_GEOLITE_DOWNLOAD = 'SKIP_INITIAL_GEOLITE_DOWNLOAD'; case REAL_TIME_UPDATES_TOPICS = 'REAL_TIME_UPDATES_TOPICS'; + case CORS_ALLOW_ORIGIN = 'CORS_ALLOW_ORIGIN'; + case CORS_ALLOW_CREDENTIALS = 'CORS_ALLOW_CREDENTIALS'; + case CORS_MAX_AGE = 'CORS_MAX_AGE'; /** @deprecated Use REDIRECT_EXTRA_PATH */ case REDIRECT_APPEND_EXTRA_PATH = 'REDIRECT_APPEND_EXTRA_PATH'; @@ -187,6 +190,10 @@ enum EnvVars: string self::DISABLE_REFERRER_TRACKING, self::DISABLE_UA_TRACKING => false, + self::CORS_ALLOW_ORIGIN => '*', + self::CORS_ALLOW_CREDENTIALS => false, + self::CORS_MAX_AGE => 3600, + default => null, }; } diff --git a/module/Core/src/Config/Options/CorsOptions.php b/module/Core/src/Config/Options/CorsOptions.php new file mode 100644 index 00000000..041503b6 --- /dev/null +++ b/module/Core/src/Config/Options/CorsOptions.php @@ -0,0 +1,58 @@ +'; + + /** @var string[]|'*'|'' */ + public string|array $allowOrigins; + + public function __construct( + string $allowOrigins = '*', + public bool $allowCredentials = false, + public int $maxAge = 3600, + ) { + $this->allowOrigins = $allowOrigins !== '*' && $allowOrigins !== self::ORIGIN_PATTERN + ? splitByComma($allowOrigins) + : $allowOrigins; + } + + public static function fromEnv(): self + { + return new self( + allowOrigins: EnvVars::CORS_ALLOW_ORIGIN->loadFromEnv(), + allowCredentials: EnvVars::CORS_ALLOW_CREDENTIALS->loadFromEnv(), + maxAge: EnvVars::CORS_MAX_AGE->loadFromEnv(), + ); + } + + public function responseWithAllowOrigin(RequestInterface $request, ResponseInterface $response): ResponseInterface + { + if ($this->allowOrigins === '*') { + return $response->withHeader('Access-Control-Allow-Origin', '*'); + } + + $requestOrigin = $request->getHeader('Origin'); + if ( + // The special value means we should allow requests from the origin set in the request's Origin + // header + $this->allowOrigins === self::ORIGIN_PATTERN + // If an array of allowed hosts was provided, set Access-Control-Allow-Origin header only if request's + // Origin header matches one of them + || contains($requestOrigin, $this->allowOrigins) + ) { + return $response->withHeader('Access-Control-Allow-Origin', $requestOrigin); + } + + return $response; + } +} diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index df482a46..058a860d 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -115,7 +115,7 @@ return [ RedirectRule\ShortUrlRedirectRuleService::class, ], - Middleware\CrossDomainMiddleware::class => ['config.cors'], + Middleware\CrossDomainMiddleware::class => [Config\Options\CorsOptions::class], Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class => [ Config\Options\UrlShortenerOptions::class, ], diff --git a/module/Rest/src/Middleware/CrossDomainMiddleware.php b/module/Rest/src/Middleware/CrossDomainMiddleware.php index d6a51a0c..4e3409d2 100644 --- a/module/Rest/src/Middleware/CrossDomainMiddleware.php +++ b/module/Rest/src/Middleware/CrossDomainMiddleware.php @@ -10,12 +10,13 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Shlinkio\Shlink\Core\Config\Options\CorsOptions; use function implode; -class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterface +readonly class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterface { - public function __construct(private array $config) + public function __construct(private CorsOptions $options) { } @@ -27,7 +28,7 @@ class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterfa } // Add Allow-Origin header - $response = $response->withHeader('Access-Control-Allow-Origin', '*'); + $response = $this->options->responseWithAllowOrigin($request, $response); if ($request->getMethod() !== self::METHOD_OPTIONS) { return $response; } @@ -40,7 +41,7 @@ class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterfa $corsHeaders = [ 'Access-Control-Allow-Methods' => $this->resolveCorsAllowedMethods($response), 'Access-Control-Allow-Headers' => $request->getHeaderLine('Access-Control-Request-Headers'), - 'Access-Control-Max-Age' => $this->config['max_age'], + 'Access-Control-Max-Age' => $this->options->maxAge, ]; // Options requests should always be empty and have a 204 status code diff --git a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php index b74a435f..615b0132 100644 --- a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php +++ b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php @@ -11,6 +11,7 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Server\RequestHandlerInterface; +use Shlinkio\Shlink\Core\Config\Options\CorsOptions; use Shlinkio\Shlink\Rest\Middleware\CrossDomainMiddleware; class CrossDomainMiddlewareTest extends TestCase @@ -20,7 +21,7 @@ class CrossDomainMiddlewareTest extends TestCase protected function setUp(): void { - $this->middleware = new CrossDomainMiddleware(['max_age' => 1000]); + $this->middleware = new CrossDomainMiddleware(new CorsOptions(maxAge: 1000)); $this->handler = $this->createMock(RequestHandlerInterface::class); } From 834bc4ae20a2803dc503bbfc248398cede9748e4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 8 Jul 2025 10:36:12 +0200 Subject: [PATCH 22/40] Allow credentials to be enabled in CORS --- module/Core/src/Config/Options/CorsOptions.php | 8 +++++--- module/Rest/src/Middleware/CrossDomainMiddleware.php | 4 ++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/module/Core/src/Config/Options/CorsOptions.php b/module/Core/src/Config/Options/CorsOptions.php index 041503b6..efc6ae1c 100644 --- a/module/Core/src/Config/Options/CorsOptions.php +++ b/module/Core/src/Config/Options/CorsOptions.php @@ -8,6 +8,7 @@ use Shlinkio\Shlink\Core\Config\EnvVars; use function Shlinkio\Shlink\Core\ArrayUtils\contains; use function Shlinkio\Shlink\Core\splitByComma; +use function strtolower; final readonly class CorsOptions { @@ -21,9 +22,10 @@ final readonly class CorsOptions public bool $allowCredentials = false, public int $maxAge = 3600, ) { - $this->allowOrigins = $allowOrigins !== '*' && $allowOrigins !== self::ORIGIN_PATTERN - ? splitByComma($allowOrigins) - : $allowOrigins; + $lowerCaseAllowOrigins = strtolower($allowOrigins); + $this->allowOrigins = contains($lowerCaseAllowOrigins, ['*', self::ORIGIN_PATTERN]) + ? $lowerCaseAllowOrigins + : splitByComma($lowerCaseAllowOrigins); } public static function fromEnv(): self diff --git a/module/Rest/src/Middleware/CrossDomainMiddleware.php b/module/Rest/src/Middleware/CrossDomainMiddleware.php index 4e3409d2..37360e2e 100644 --- a/module/Rest/src/Middleware/CrossDomainMiddleware.php +++ b/module/Rest/src/Middleware/CrossDomainMiddleware.php @@ -44,6 +44,10 @@ readonly class CrossDomainMiddleware implements MiddlewareInterface, RequestMeth 'Access-Control-Max-Age' => $this->options->maxAge, ]; + if ($this->options->allowCredentials) { + $corsHeaders['Access-Control-Allow-Credentials'] = 'true'; + } + // Options requests should always be empty and have a 204 status code return EmptyResponse::withHeaders([...$response->getHeaders(), ...$corsHeaders]); } From cd4fcc9b0a8a04f5e34d753bd9775c3c37d7ad40 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 8 Jul 2025 13:06:16 +0200 Subject: [PATCH 23/40] Update shlink-installer --- composer.json | 2 +- config/autoload/installer.global.php | 3 +++ module/Core/src/Config/Options/CorsOptions.php | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index f391842b..26031b7c 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,7 @@ "shlinkio/shlink-config": "^4.0", "shlinkio/shlink-event-dispatcher": "^4.2", "shlinkio/shlink-importer": "^5.6", - "shlinkio/shlink-installer": "dev-develop#c5a8523 as 9.6", + "shlinkio/shlink-installer": "dev-develop#9005232 as 9.6", "shlinkio/shlink-ip-geolocation": "^4.3", "shlinkio/shlink-json": "^1.2", "spiral/roadrunner": "^2025.1", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 063ff7d4..e9270eb6 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -77,6 +77,9 @@ return [ Option\Matomo\MatomoSiteIdConfigOption::class, Option\Matomo\MatomoApiTokenConfigOption::class, Option\RealTimeUpdates\RealTimeUpdatesTopicsConfigOption::class, + Option\Cors\CorsAllowOriginConfigOption::class, + Option\Cors\CorsAllowCredentialsConfigOption::class, + Option\Cors\CorsMaxAgeConfigOption::class, ], 'installation_commands' => [ diff --git a/module/Core/src/Config/Options/CorsOptions.php b/module/Core/src/Config/Options/CorsOptions.php index efc6ae1c..fbf191ee 100644 --- a/module/Core/src/Config/Options/CorsOptions.php +++ b/module/Core/src/Config/Options/CorsOptions.php @@ -23,7 +23,7 @@ final readonly class CorsOptions public int $maxAge = 3600, ) { $lowerCaseAllowOrigins = strtolower($allowOrigins); - $this->allowOrigins = contains($lowerCaseAllowOrigins, ['*', self::ORIGIN_PATTERN]) + $this->allowOrigins = $lowerCaseAllowOrigins === '*' || $lowerCaseAllowOrigins === self::ORIGIN_PATTERN ? $lowerCaseAllowOrigins : splitByComma($lowerCaseAllowOrigins); } From 1d96cc0279f9bcbe0013bff92f520da326ae9540 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 8 Jul 2025 13:17:46 +0200 Subject: [PATCH 24/40] Update CrossDomainMiddleware test --- .../Middleware/CrossDomainMiddlewareTest.php | 39 +++++++++++++++---- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php index 615b0132..5893ca8c 100644 --- a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php +++ b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php @@ -8,6 +8,7 @@ use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequest; 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\Server\RequestHandlerInterface; @@ -16,12 +17,10 @@ use Shlinkio\Shlink\Rest\Middleware\CrossDomainMiddleware; class CrossDomainMiddlewareTest extends TestCase { - private CrossDomainMiddleware $middleware; private MockObject & RequestHandlerInterface $handler; protected function setUp(): void { - $this->middleware = new CrossDomainMiddleware(new CorsOptions(maxAge: 1000)); $this->handler = $this->createMock(RequestHandlerInterface::class); } @@ -31,7 +30,7 @@ class CrossDomainMiddlewareTest extends TestCase $originalResponse = (new Response())->withStatus(404); $this->handler->expects($this->once())->method('handle')->willReturn($originalResponse); - $response = $this->middleware->process(new ServerRequest(), $this->handler); + $response = $this->middleware()->process(new ServerRequest(), $this->handler); $headers = $response->getHeaders(); self::assertSame($originalResponse, $response); @@ -48,7 +47,7 @@ class CrossDomainMiddlewareTest extends TestCase $originalResponse = new Response(); $this->handler->expects($this->once())->method('handle')->willReturn($originalResponse); - $response = $this->middleware->process((new ServerRequest())->withHeader('Origin', 'local'), $this->handler); + $response = $this->middleware()->process((new ServerRequest())->withHeader('Origin', 'local'), $this->handler); self::assertNotSame($originalResponse, $response); $headers = $response->getHeaders(); @@ -69,7 +68,7 @@ class CrossDomainMiddlewareTest extends TestCase ->withHeader('Access-Control-Request-Headers', 'foo, bar, baz'); $this->handler->expects($this->once())->method('handle')->willReturn($originalResponse); - $response = $this->middleware->process($request, $this->handler); + $response = $this->middleware()->process($request, $this->handler); self::assertNotSame($originalResponse, $response); $headers = $response->getHeaders(); @@ -94,7 +93,7 @@ class CrossDomainMiddlewareTest extends TestCase ->withMethod('OPTIONS'); $this->handler->expects($this->once())->method('handle')->willReturn($originalResponse); - $response = $this->middleware->process($request, $this->handler); + $response = $this->middleware()->process($request, $this->handler); self::assertEquals($response->getHeaderLine('Access-Control-Allow-Methods'), $expectedAllowedMethods); self::assertEquals(204, $response->getStatusCode()); @@ -118,7 +117,7 @@ class CrossDomainMiddlewareTest extends TestCase ->withHeader('Origin', 'local'); $this->handler->expects($this->once())->method('handle')->willReturn($originalResponse); - $response = $this->middleware->process($request, $this->handler); + $response = $this->middleware()->process($request, $this->handler); self::assertEquals($expectedStatus, $response->getStatusCode()); } @@ -141,4 +140,30 @@ class CrossDomainMiddlewareTest extends TestCase yield 'OPTIONS 400' => ['OPTIONS', 400, 204]; yield 'OPTIONS 500' => ['OPTIONS', 500, 204]; } + + #[Test] + #[TestWith([true])] + #[TestWith([false])] + public function credentialsAreAllowedIfConfiguredSo(bool $allowCredentials): void + { + $originalResponse = new Response(); + $request = (new ServerRequest()) + ->withMethod('OPTIONS') + ->withHeader('Origin', 'local'); + $this->handler->method('handle')->willReturn($originalResponse); + + $response = $this->middleware(allowCredentials: $allowCredentials)->process($request, $this->handler); + $headers = $response->getHeaders(); + + if ($allowCredentials) { + self::assertArrayHasKey('Access-Control-Allow-Credentials', $headers); + } else { + self::assertArrayNotHasKey('Access-Control-Allow-Credentials', $headers); + } + } + + private function middleware(bool $allowCredentials = false): CrossDomainMiddleware + { + return new CrossDomainMiddleware(new CorsOptions(allowCredentials: $allowCredentials, maxAge: 1000)); + } } From 3369afe22cdaf7983e41b91e90572a4ec1b90aef Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 16 Jul 2025 08:29:57 +0200 Subject: [PATCH 25/40] Add CorsOptions test --- module/Core/functions/array-utils.php | 5 +++ .../Core/src/Config/Options/CorsOptions.php | 2 +- .../test/Config/Options/CorsOptionsTest.php | 37 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 module/Core/test/Config/Options/CorsOptionsTest.php diff --git a/module/Core/functions/array-utils.php b/module/Core/functions/array-utils.php index d68851d8..3e0010a2 100644 --- a/module/Core/functions/array-utils.php +++ b/module/Core/functions/array-utils.php @@ -10,6 +10,11 @@ use function in_array; use const ARRAY_FILTER_USE_KEY; +/** + * @template T + * @param T $value + * @param T[] $array + */ function contains(mixed $value, array $array): bool { return in_array($value, $array, strict: true); diff --git a/module/Core/src/Config/Options/CorsOptions.php b/module/Core/src/Config/Options/CorsOptions.php index fbf191ee..f4a23139 100644 --- a/module/Core/src/Config/Options/CorsOptions.php +++ b/module/Core/src/Config/Options/CorsOptions.php @@ -43,7 +43,7 @@ final readonly class CorsOptions return $response->withHeader('Access-Control-Allow-Origin', '*'); } - $requestOrigin = $request->getHeader('Origin'); + $requestOrigin = $request->getHeaderLine('Origin'); if ( // The special value means we should allow requests from the origin set in the request's Origin // header diff --git a/module/Core/test/Config/Options/CorsOptionsTest.php b/module/Core/test/Config/Options/CorsOptionsTest.php new file mode 100644 index 00000000..cfed36d7 --- /dev/null +++ b/module/Core/test/Config/Options/CorsOptionsTest.php @@ -0,0 +1,37 @@ +', '', 'https://example.com'])] + #[TestWith(['foo,bar, baz ', ['foo', 'bar', 'baz'], ''])] + #[TestWith(['foo,bar,https://example.com', ['foo', 'bar', 'https://example.com'], 'https://example.com'])] + public function expectedAccessControlAllowOriginIsSet( + string $allowOrigins, + string|array $expectedAllowOrigins, + string $expectedAllowOriginsHeader, + ): void { + $options = new CorsOptions($allowOrigins); + + self::assertEquals($expectedAllowOrigins, $options->allowOrigins); + self::assertEquals( + $expectedAllowOriginsHeader, + $options->responseWithAllowOrigin( + ServerRequestFactory::fromGlobals()->withHeader('Origin', 'https://example.com'), + new Response() + )->getHeaderLine('Access-Control-Allow-Origin'), + ); + } +} From f5c6bc8204c9e2a9fcd20bd984cf713f6798b7c0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 16 Jul 2025 08:37:38 +0200 Subject: [PATCH 26/40] Update changelog --- CHANGELOG.md | 1 + module/Core/test/Config/Options/CorsOptionsTest.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7b4c4ba..a22b5be9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this For BC, if this env vars is not present, we'll still consider the integration enabled if the `MERCURE_PUBLIC_HUB_URL` env var has a value. This is considered deprecated though, and next major version will rely only on `MERCURE_ENABLED`, so if you are using Mercure, make sure to set `MERCURE_ENABLED=true` to be ready. * [#2387](https://github.com/shlinkio/shlink/issues/2387) Add `REAL_TIME_UPDATES_TOPICS` env var and corresponding config option, to granularly decide which real-time updates topics should be enabled. +* [#2418](https://github.com/shlinkio/shlink/issues/2418) Add more granular control over how Shlink handles CORS. It is now possible to customize the `Access-Control-Allow-Origin`, `Access-Control-Max-Age` and `Access-Control-Allow-Credentials` headers via env vars or config options. ### Changed * [#2406](https://github.com/shlinkio/shlink/issues/2406) Remove references to bootstrap from error templates, and instead inline the very minimum required styles. diff --git a/module/Core/test/Config/Options/CorsOptionsTest.php b/module/Core/test/Config/Options/CorsOptionsTest.php index cfed36d7..b02799b0 100644 --- a/module/Core/test/Config/Options/CorsOptionsTest.php +++ b/module/Core/test/Config/Options/CorsOptionsTest.php @@ -30,7 +30,7 @@ class CorsOptionsTest extends TestCase $expectedAllowOriginsHeader, $options->responseWithAllowOrigin( ServerRequestFactory::fromGlobals()->withHeader('Origin', 'https://example.com'), - new Response() + new Response(), )->getHeaderLine('Access-Control-Allow-Origin'), ); } From 18c4c39fee48fa6b8b8e06a48648323b99860c69 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 17 Jul 2025 08:26:52 +0200 Subject: [PATCH 27/40] Add support for any-value and valueless query param redirect rules --- CHANGELOG.md | 5 +++ .../definitions/SetShortUrlRedirectRule.json | 2 ++ .../src/RedirectRule/RedirectRuleHandler.php | 6 ++++ .../RedirectRule/Entity/RedirectCondition.php | 35 +++++++++++++++++++ .../Model/RedirectConditionType.php | 5 +++ 5 files changed, 53 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a22b5be9..ac74a67b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#2387](https://github.com/shlinkio/shlink/issues/2387) Add `REAL_TIME_UPDATES_TOPICS` env var and corresponding config option, to granularly decide which real-time updates topics should be enabled. * [#2418](https://github.com/shlinkio/shlink/issues/2418) Add more granular control over how Shlink handles CORS. It is now possible to customize the `Access-Control-Allow-Origin`, `Access-Control-Max-Age` and `Access-Control-Allow-Credentials` headers via env vars or config options. +* [#2386](https://github.com/shlinkio/shlink/issues/2386) Add new `any-value-query-param` and `valueless-query-param` redirect rule conditions. + + These new rules expand the existing `query-param`, which requires both a specific non-empty value in order to match the condition. + + The new conditions match as soon as a query param exists with any or no value (in the case of `any-value-query-param`), or if a query param exists with no value at all (in the case of `valueless-query-param`). ### Changed * [#2406](https://github.com/shlinkio/shlink/issues/2406) Remove references to bootstrap from error templates, and instead inline the very minimum required styles. diff --git a/docs/swagger/definitions/SetShortUrlRedirectRule.json b/docs/swagger/definitions/SetShortUrlRedirectRule.json index 00f0a27b..b6c66f5f 100644 --- a/docs/swagger/definitions/SetShortUrlRedirectRule.json +++ b/docs/swagger/definitions/SetShortUrlRedirectRule.json @@ -19,6 +19,8 @@ "device", "language", "query-param", + "any-value-query-param", + "valueless-query-param", "ip-address", "geolocation-country-code", "geolocation-city-name" diff --git a/module/CLI/src/RedirectRule/RedirectRuleHandler.php b/module/CLI/src/RedirectRule/RedirectRuleHandler.php index 89f93833..baab9c9e 100644 --- a/module/CLI/src/RedirectRule/RedirectRuleHandler.php +++ b/module/CLI/src/RedirectRule/RedirectRuleHandler.php @@ -108,6 +108,12 @@ class RedirectRuleHandler implements RedirectRuleHandlerInterface $this->askMandatory('Query param name?', $io), $this->askOptional('Query param value?', $io), ), + RedirectConditionType::ANY_VALUE_QUERY_PARAM => RedirectCondition::forAnyValueQueryParam( + $this->askMandatory('Query param name?', $io), + ), + RedirectConditionType::VALUELESS_QUERY_PARAM => RedirectCondition::forValuelessQueryParam( + $this->askMandatory('Query param name?', $io), + ), RedirectConditionType::IP_ADDRESS => RedirectCondition::forIpAddress( $this->askMandatory('IP address, CIDR block or wildcard-pattern (1.2.*.*)', $io), ), diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index 255cb19e..dd24ca6d 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -11,6 +11,7 @@ use Shlinkio\Shlink\Core\RedirectRule\Model\Validation\RedirectRulesInputFilter; use Shlinkio\Shlink\Core\Util\IpAddressUtils; use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectCondition; +use function array_key_exists; use function Shlinkio\Shlink\Core\acceptLanguageToLocales; use function Shlinkio\Shlink\Core\ArrayUtils\some; use function Shlinkio\Shlink\Core\geolocationFromRequest; @@ -35,6 +36,16 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable return new self(RedirectConditionType::QUERY_PARAM, $value, $param); } + public static function forAnyValueQueryParam(string $param): self + { + return new self(RedirectConditionType::ANY_VALUE_QUERY_PARAM, $param); + } + + public static function forValuelessQueryParam(string $param): self + { + return new self(RedirectConditionType::VALUELESS_QUERY_PARAM, $param); + } + public static function forLanguage(string $language): self { return new self(RedirectConditionType::LANGUAGE, $language); @@ -82,6 +93,8 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable return match ($type) { RedirectConditionType::QUERY_PARAM => self::forQueryParam($cond->matchKey ?? '', $cond->matchValue), + RedirectConditionType::ANY_VALUE_QUERY_PARAM => self::forAnyValueQueryParam($cond->matchValue), + RedirectConditionType::VALUELESS_QUERY_PARAM => self::forValuelessQueryParam($cond->matchValue), RedirectConditionType::LANGUAGE => self::forLanguage($cond->matchValue), RedirectConditionType::DEVICE => self::forDevice(DeviceType::from($cond->matchValue)), RedirectConditionType::IP_ADDRESS => self::forIpAddress($cond->matchValue), @@ -97,6 +110,8 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable { return match ($this->type) { RedirectConditionType::QUERY_PARAM => $this->matchesQueryParam($request), + RedirectConditionType::ANY_VALUE_QUERY_PARAM => $this->matchesAnyValueQueryParam($request), + RedirectConditionType::VALUELESS_QUERY_PARAM => $this->matchesValuelessQueryParam($request), RedirectConditionType::LANGUAGE => $this->matchesLanguage($request), RedirectConditionType::DEVICE => $this->matchesDevice($request), RedirectConditionType::IP_ADDRESS => $this->matchesRemoteIpAddress($request), @@ -113,6 +128,18 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable return $queryValue === $this->matchValue; } + private function matchesValuelessQueryParam(ServerRequestInterface $request): bool + { + $query = $request->getQueryParams(); + return array_key_exists($this->matchValue, $query) && empty($query[$this->matchValue]); + } + + private function matchesAnyValueQueryParam(ServerRequestInterface $request): bool + { + $query = $request->getQueryParams(); + return array_key_exists($this->matchValue, $query); + } + private function matchesLanguage(ServerRequestInterface $request): bool { $acceptLanguage = trim($request->getHeaderLine('Accept-Language')); @@ -188,6 +215,14 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable $this->matchKey, $this->matchValue, ), + RedirectConditionType::ANY_VALUE_QUERY_PARAM => sprintf( + 'query string contains %s param', + $this->matchValue, + ), + RedirectConditionType::VALUELESS_QUERY_PARAM => sprintf( + 'query string contains %s param without a value (https://example.com?foo)', + $this->matchValue, + ), RedirectConditionType::IP_ADDRESS => sprintf('IP address matches %s', $this->matchValue), RedirectConditionType::GEOLOCATION_COUNTRY_CODE => sprintf('country code is %s', $this->matchValue), RedirectConditionType::GEOLOCATION_CITY_NAME => sprintf('city name is %s', $this->matchValue), diff --git a/module/Core/src/RedirectRule/Model/RedirectConditionType.php b/module/Core/src/RedirectRule/Model/RedirectConditionType.php index efc314f9..bcd482e7 100644 --- a/module/Core/src/RedirectRule/Model/RedirectConditionType.php +++ b/module/Core/src/RedirectRule/Model/RedirectConditionType.php @@ -13,6 +13,8 @@ enum RedirectConditionType: string case DEVICE = 'device'; case LANGUAGE = 'language'; case QUERY_PARAM = 'query-param'; + case ANY_VALUE_QUERY_PARAM = 'any-value-query-param'; + case VALUELESS_QUERY_PARAM = 'valueless-query-param'; case IP_ADDRESS = 'ip-address'; case GEOLOCATION_COUNTRY_CODE = 'geolocation-country-code'; case GEOLOCATION_CITY_NAME = 'geolocation-city-name'; @@ -45,6 +47,9 @@ enum RedirectConditionType: string 'TO', 'TT', 'TN', 'TR', 'TM', 'TC', 'TV', 'UG', 'UA', 'AE', 'GB', 'US', 'UM', 'UY', 'UZ', 'VU', 'VE', 'VN', 'VG', 'VI', 'WF', 'EH', 'YE', 'ZM', 'ZW', ]), + RedirectConditionType::ANY_VALUE_QUERY_PARAM, RedirectConditionType::VALUELESS_QUERY_PARAM => $value !== '', + // FIXME We should at least validate the value is not empty + // default => $value !== '', default => true, }; } From 47293be85c1ca67179d41b3930d1e4347a6b2da7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 17 Jul 2025 08:37:03 +0200 Subject: [PATCH 28/40] Enhance RedirectConditionTest with new query-param-related conditions --- CHANGELOG.md | 2 +- .../Entity/RedirectConditionTest.php | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ac74a67b..383f89a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Added * [#2438](https://github.com/shlinkio/shlink/issues/2438) Add `MERCURE_ENABLED` env var and corresponding config option, to more easily allow the mercure integration to be toggled. - For BC, if this env vars is not present, we'll still consider the integration enabled if the `MERCURE_PUBLIC_HUB_URL` env var has a value. This is considered deprecated though, and next major version will rely only on `MERCURE_ENABLED`, so if you are using Mercure, make sure to set `MERCURE_ENABLED=true` to be ready. + For BC, if this env var is not present, we'll still consider the integration enabled if the `MERCURE_PUBLIC_HUB_URL` env var has a value. This is considered deprecated though, and next major version will rely only on `MERCURE_ENABLED`, so if you are using Mercure, make sure to set `MERCURE_ENABLED=true` to be ready. * [#2387](https://github.com/shlinkio/shlink/issues/2387) Add `REAL_TIME_UPDATES_TOPICS` env var and corresponding config option, to granularly decide which real-time updates topics should be enabled. * [#2418](https://github.com/shlinkio/shlink/issues/2418) Add more granular control over how Shlink handles CORS. It is now possible to customize the `Access-Control-Allow-Origin`, `Access-Control-Max-Age` and `Access-Control-Allow-Credentials` headers via env vars or config options. diff --git a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php index bec6d263..28e3f6b1 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -32,6 +32,33 @@ class RedirectConditionTest extends TestCase self::assertEquals($expectedResult, $result); } + #[Test] + #[TestWith(['nop', '', false])] // param not present + #[TestWith(['foo', '', true])] + #[TestWith(['foo', 'something', true])] + #[TestWith(['foo', 'something else', true])] + public function matchesAnyValueQueryParams(string $param, string $value, bool $expectedResult): void + { + $request = ServerRequestFactory::fromGlobals()->withQueryParams(['foo' => $value]); + $result = RedirectCondition::forAnyValueQueryParam($param)->matchesRequest($request); + + self::assertEquals($expectedResult, $result); + } + + #[Test] + #[TestWith(['nop', '', false])] // param not present + #[TestWith(['foo', '', true])] + #[TestWith(['foo', null, true])] + #[TestWith(['foo', 'something', false])] + #[TestWith(['foo', 'something else', false])] + public function matchesValuelessQueryParams(string $param, string|null $value, bool $expectedResult): void + { + $request = ServerRequestFactory::fromGlobals()->withQueryParams(['foo' => $value]); + $result = RedirectCondition::forValuelessQueryParam($param)->matchesRequest($request); + + self::assertEquals($expectedResult, $result); + } + #[Test] #[TestWith([null, '', false], 'no accept language')] #[TestWith(['', '', false], 'empty accept language')] @@ -141,6 +168,8 @@ class RedirectConditionTest extends TestCase #[TestWith([RedirectConditionType::DEVICE->value, RedirectConditionType::DEVICE])] #[TestWith([RedirectConditionType::LANGUAGE->value, RedirectConditionType::LANGUAGE])] #[TestWith([RedirectConditionType::QUERY_PARAM->value, RedirectConditionType::QUERY_PARAM])] + #[TestWith([RedirectConditionType::ANY_VALUE_QUERY_PARAM->value, RedirectConditionType::ANY_VALUE_QUERY_PARAM])] + #[TestWith([RedirectConditionType::VALUELESS_QUERY_PARAM->value, RedirectConditionType::VALUELESS_QUERY_PARAM])] #[TestWith([RedirectConditionType::IP_ADDRESS->value, RedirectConditionType::IP_ADDRESS])] #[TestWith( [RedirectConditionType::GEOLOCATION_COUNTRY_CODE->value, RedirectConditionType::GEOLOCATION_COUNTRY_CODE], From 223901324f74fb1dcd8189279df55515791f4f76 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 17 Jul 2025 08:44:19 +0200 Subject: [PATCH 29/40] Enhance RedirectRuleHandlerTest with new query-param-related conditions --- module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php index aa6e0ac7..aed8f3c5 100644 --- a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php +++ b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php @@ -162,6 +162,14 @@ class RedirectRuleHandlerTest extends TestCase yield 'device' => [RedirectConditionType::DEVICE, [RedirectCondition::forDevice(DeviceType::ANDROID)]]; yield 'language' => [RedirectConditionType::LANGUAGE, [RedirectCondition::forLanguage('en-US')]]; yield 'query param' => [RedirectConditionType::QUERY_PARAM, [RedirectCondition::forQueryParam('foo', 'bar')]]; + yield 'any value query param' => [ + RedirectConditionType::ANY_VALUE_QUERY_PARAM, + [RedirectCondition::forAnyValueQueryParam('foo')], + ]; + yield 'valueless query param' => [ + RedirectConditionType::VALUELESS_QUERY_PARAM, + [RedirectCondition::forValuelessQueryParam('foo')], + ]; yield 'multiple query params' => [ RedirectConditionType::QUERY_PARAM, [RedirectCondition::forQueryParam('foo', 'bar'), RedirectCondition::forQueryParam('foo', 'bar')], From c3d3cc62880a75602598bf30a6095dda89f1f993 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 17 Jul 2025 08:51:59 +0200 Subject: [PATCH 30/40] Test RedirectConditionType::isValid() in isolation --- config/constants.php | 22 +++++++++++++++ .../Model/RedirectConditionType.php | 26 ++++------------- .../Model/RedirectConditionTypeTest.php | 28 +++++++++++++++++++ 3 files changed, 56 insertions(+), 20 deletions(-) create mode 100644 module/Core/test/RedirectRule/Model/RedirectConditionTypeTest.php diff --git a/config/constants.php b/config/constants.php index f5dff4d5..28440653 100644 --- a/config/constants.php +++ b/config/constants.php @@ -16,6 +16,28 @@ const LOOSE_URI_MATCHER = '/(.+)\:(.+)/i'; // Matches anything starting with a s const IP_ADDRESS_REQUEST_ATTRIBUTE = 'remote_address'; const REDIRECT_URL_REQUEST_ATTRIBUTE = 'redirect_url'; +/** + * List of ISO 3166-1 alpha-2 two-letter country codes https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2 + */ +const ISO_COUNTRY_CODES = [ + 'AF', 'AX', 'AL', 'DZ', 'AS', 'AD', 'AO', 'AI', 'AQ', 'AG', 'AR', 'AM', 'AW', 'AU', 'AT', 'AZ', + 'BS', 'BH', 'BD', 'BB', 'BY', 'BE', 'BZ', 'BJ', 'BM', 'BT', 'BO', 'BQ', 'BA', 'BW', 'BV', 'BR', + 'IO', 'BN', 'BG', 'BF', 'BI', 'CV', 'KH', 'CM', 'CA', 'KY', 'CF', 'TD', 'CL', 'CN', 'CX', 'CC', + 'CO', 'KM', 'CG', 'CD', 'CK', 'CR', 'CI', 'HR', 'CU', 'CW', 'CY', 'CZ', 'DK', 'DJ', 'DM', 'DO', + 'EC', 'EG', 'SV', 'GQ', 'ER', 'EE', 'SZ', 'ET', 'FK', 'FO', 'FJ', 'FI', 'FR', 'GF', 'PF', 'TF', + 'GA', 'GM', 'GE', 'DE', 'GH', 'GI', 'GR', 'GL', 'GD', 'GP', 'GU', 'GT', 'GG', 'GN', 'GW', 'GY', + 'HT', 'HM', 'VA', 'HN', 'HK', 'HU', 'IS', 'IN', 'ID', 'IR', 'IQ', 'IE', 'IM', 'IL', 'IT', 'JM', + 'JP', 'JE', 'JO', 'KZ', 'KE', 'KI', 'KP', 'KR', 'KW', 'KG', 'LA', 'LV', 'LB', 'LS', 'LR', 'LY', + 'LI', 'LT', 'LU', 'MO', 'MG', 'MW', 'MY', 'MV', 'ML', 'MT', 'MH', 'MQ', 'MR', 'MU', 'YT', 'MX', + 'FM', 'MD', 'MC', 'MN', 'ME', 'MS', 'MA', 'MZ', 'MM', 'NA', 'NR', 'NP', 'NL', 'NC', 'NZ', 'NI', + 'NE', 'NG', 'NU', 'NF', 'MK', 'MP', 'NO', 'OM', 'PK', 'PW', 'PS', 'PA', 'PG', 'PY', 'PE', 'PH', + 'PN', 'PL', 'PT', 'PR', 'QA', 'RE', 'RO', 'RU', 'RW', 'BL', 'SH', 'KN', 'LC', 'MF', 'PM', 'VC', + 'WS', 'SM', 'ST', 'SA', 'SN', 'RS', 'SC', 'SL', 'SG', 'SX', 'SK', 'SI', 'SB', 'SO', 'ZA', 'GS', + 'SS', 'ES', 'LK', 'SD', 'SR', 'SJ', 'SE', 'CH', 'SY', 'TW', 'TJ', 'TZ', 'TH', 'TL', 'TG', 'TK', + 'TO', 'TT', 'TN', 'TR', 'TM', 'TC', 'TV', 'UG', 'UA', 'AE', 'GB', 'US', 'UM', 'UY', 'UZ', 'VU', + 'VE', 'VN', 'VG', 'VI', 'WF', 'EH', 'YE', 'ZM', 'ZW', +]; + /** @deprecated */ const DEFAULT_QR_CODE_SIZE = 300; /** @deprecated */ diff --git a/module/Core/src/RedirectRule/Model/RedirectConditionType.php b/module/Core/src/RedirectRule/Model/RedirectConditionType.php index bcd482e7..8f7657e3 100644 --- a/module/Core/src/RedirectRule/Model/RedirectConditionType.php +++ b/module/Core/src/RedirectRule/Model/RedirectConditionType.php @@ -8,6 +8,8 @@ use Shlinkio\Shlink\Core\Util\IpAddressUtils; use function Shlinkio\Shlink\Core\ArrayUtils\contains; use function Shlinkio\Shlink\Core\enumValues; +use const Shlinkio\Shlink\ISO_COUNTRY_CODES; + enum RedirectConditionType: string { case DEVICE = 'device'; @@ -28,26 +30,10 @@ enum RedirectConditionType: string RedirectConditionType::DEVICE => contains($value, enumValues(DeviceType::class)), // RedirectConditionType::LANGUAGE => TODO Validate at least format, RedirectConditionType::IP_ADDRESS => IpAddressUtils::isStaticIpCidrOrWildcard($value), - RedirectConditionType::GEOLOCATION_COUNTRY_CODE => contains($value, [ - // List of ISO 3166-1 alpha-2 two-letter country codes https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2 - 'AF', 'AX', 'AL', 'DZ', 'AS', 'AD', 'AO', 'AI', 'AQ', 'AG', 'AR', 'AM', 'AW', 'AU', 'AT', 'AZ', - 'BS', 'BH', 'BD', 'BB', 'BY', 'BE', 'BZ', 'BJ', 'BM', 'BT', 'BO', 'BQ', 'BA', 'BW', 'BV', 'BR', - 'IO', 'BN', 'BG', 'BF', 'BI', 'CV', 'KH', 'CM', 'CA', 'KY', 'CF', 'TD', 'CL', 'CN', 'CX', 'CC', - 'CO', 'KM', 'CG', 'CD', 'CK', 'CR', 'CI', 'HR', 'CU', 'CW', 'CY', 'CZ', 'DK', 'DJ', 'DM', 'DO', - 'EC', 'EG', 'SV', 'GQ', 'ER', 'EE', 'SZ', 'ET', 'FK', 'FO', 'FJ', 'FI', 'FR', 'GF', 'PF', 'TF', - 'GA', 'GM', 'GE', 'DE', 'GH', 'GI', 'GR', 'GL', 'GD', 'GP', 'GU', 'GT', 'GG', 'GN', 'GW', 'GY', - 'HT', 'HM', 'VA', 'HN', 'HK', 'HU', 'IS', 'IN', 'ID', 'IR', 'IQ', 'IE', 'IM', 'IL', 'IT', 'JM', - 'JP', 'JE', 'JO', 'KZ', 'KE', 'KI', 'KP', 'KR', 'KW', 'KG', 'LA', 'LV', 'LB', 'LS', 'LR', 'LY', - 'LI', 'LT', 'LU', 'MO', 'MG', 'MW', 'MY', 'MV', 'ML', 'MT', 'MH', 'MQ', 'MR', 'MU', 'YT', 'MX', - 'FM', 'MD', 'MC', 'MN', 'ME', 'MS', 'MA', 'MZ', 'MM', 'NA', 'NR', 'NP', 'NL', 'NC', 'NZ', 'NI', - 'NE', 'NG', 'NU', 'NF', 'MK', 'MP', 'NO', 'OM', 'PK', 'PW', 'PS', 'PA', 'PG', 'PY', 'PE', 'PH', - 'PN', 'PL', 'PT', 'PR', 'QA', 'RE', 'RO', 'RU', 'RW', 'BL', 'SH', 'KN', 'LC', 'MF', 'PM', 'VC', - 'WS', 'SM', 'ST', 'SA', 'SN', 'RS', 'SC', 'SL', 'SG', 'SX', 'SK', 'SI', 'SB', 'SO', 'ZA', 'GS', - 'SS', 'ES', 'LK', 'SD', 'SR', 'SJ', 'SE', 'CH', 'SY', 'TW', 'TJ', 'TZ', 'TH', 'TL', 'TG', 'TK', - 'TO', 'TT', 'TN', 'TR', 'TM', 'TC', 'TV', 'UG', 'UA', 'AE', 'GB', 'US', 'UM', 'UY', 'UZ', 'VU', - 'VE', 'VN', 'VG', 'VI', 'WF', 'EH', 'YE', 'ZM', 'ZW', - ]), - RedirectConditionType::ANY_VALUE_QUERY_PARAM, RedirectConditionType::VALUELESS_QUERY_PARAM => $value !== '', + RedirectConditionType::GEOLOCATION_COUNTRY_CODE => contains($value, ISO_COUNTRY_CODES), + RedirectConditionType::QUERY_PARAM, + RedirectConditionType::ANY_VALUE_QUERY_PARAM, + RedirectConditionType::VALUELESS_QUERY_PARAM => $value !== '', // FIXME We should at least validate the value is not empty // default => $value !== '', default => true, diff --git a/module/Core/test/RedirectRule/Model/RedirectConditionTypeTest.php b/module/Core/test/RedirectRule/Model/RedirectConditionTypeTest.php new file mode 100644 index 00000000..1c68a0f4 --- /dev/null +++ b/module/Core/test/RedirectRule/Model/RedirectConditionTypeTest.php @@ -0,0 +1,28 @@ +isValid($value)); + } +} From 650fafb7c46ac5186c292af5841e79eca04d981d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 17 Jul 2025 09:47:02 +0200 Subject: [PATCH 31/40] Register ReverseForwardedAddressesMiddlewareDecorator via ServiceManager delegator --- config/autoload/ip-address.global.php | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/config/autoload/ip-address.global.php b/config/autoload/ip-address.global.php index 78f5bc6d..81a9f02f 100644 --- a/config/autoload/ip-address.global.php +++ b/config/autoload/ip-address.global.php @@ -2,7 +2,7 @@ declare(strict_types=1); -use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory; +use Psr\Container\ContainerInterface; use RKA\Middleware\IpAddress; use RKA\Middleware\Mezzio\IpAddressFactory; use Shlinkio\Shlink\Core\Middleware\ReverseForwardedAddressesMiddlewareDecorator; @@ -32,19 +32,21 @@ return [ 'dependencies' => [ 'factories' => [ -// IpAddress::class => IpAddressFactory::class, - 'actual_ip_address_middleware' => IpAddressFactory::class, - ReverseForwardedAddressesMiddlewareDecorator::class => ConfigAbstractFactory::class, + IpAddress::class => IpAddressFactory::class, ], - 'aliases' => [ - // Make sure the decorated middleware is resolved when getting IpAddress::class, to make this decoration - // transparent for other parts of the code - IpAddress::class => ReverseForwardedAddressesMiddlewareDecorator::class, + 'delegators' => [ + // Make middleware decoration transparent to other parts of the code + IpAddress::class => [ + function ( + ContainerInterface $container, + string $name, + callable $callback + ): ReverseForwardedAddressesMiddlewareDecorator { + return new ReverseForwardedAddressesMiddlewareDecorator($callback()); + }, + ], ], - ], - ConfigAbstractFactory::class => [ - ReverseForwardedAddressesMiddlewareDecorator::class => ['actual_ip_address_middleware'], ], ]; From 1f825797f698540f1c2a3c593397c64ec25bbd49 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 17 Jul 2025 09:57:34 +0200 Subject: [PATCH 32/40] Allow trusted proxies to be provided via TRUSTED_PROXIES env var --- config/autoload/ip-address.global.php | 77 ++++++++++--------- module/Core/src/Config/EnvVars.php | 1 + ...eForwardedAddressesMiddlewareDecorator.php | 2 + 3 files changed, 44 insertions(+), 36 deletions(-) diff --git a/config/autoload/ip-address.global.php b/config/autoload/ip-address.global.php index 81a9f02f..1bac74c2 100644 --- a/config/autoload/ip-address.global.php +++ b/config/autoload/ip-address.global.php @@ -2,51 +2,56 @@ declare(strict_types=1); -use Psr\Container\ContainerInterface; use RKA\Middleware\IpAddress; use RKA\Middleware\Mezzio\IpAddressFactory; +use Shlinkio\Shlink\Core\Config\EnvVars; use Shlinkio\Shlink\Core\Middleware\ReverseForwardedAddressesMiddlewareDecorator; +use function Shlinkio\Shlink\Core\splitByComma; + use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE; -return [ +return (static function (): array { + $trustedProxies = EnvVars::TRUSTED_PROXIES->loadFromEnv(); - // Configuration for RKA\Middleware\IpAddress - 'rka' => [ - 'ip_address' => [ - 'attribute_name' => IP_ADDRESS_REQUEST_ATTRIBUTE, - 'check_proxy_headers' => true, - 'trusted_proxies' => [], - 'headers_to_inspect' => [ - 'CF-Connecting-IP', - 'X-Forwarded-For', - 'X-Forwarded', - 'Forwarded', - 'True-Client-IP', - 'X-Real-IP', - 'X-Cluster-Client-Ip', - 'Client-Ip', - ], - ], - ], + return [ - 'dependencies' => [ - 'factories' => [ - IpAddress::class => IpAddressFactory::class, - ], - 'delegators' => [ - // Make middleware decoration transparent to other parts of the code - IpAddress::class => [ - function ( - ContainerInterface $container, - string $name, - callable $callback - ): ReverseForwardedAddressesMiddlewareDecorator { - return new ReverseForwardedAddressesMiddlewareDecorator($callback()); - }, + // Configuration for RKA\Middleware\IpAddress + 'rka' => [ + 'ip_address' => [ + 'attribute_name' => IP_ADDRESS_REQUEST_ATTRIBUTE, + 'check_proxy_headers' => true, + 'trusted_proxies' => splitByComma($trustedProxies), + 'headers_to_inspect' => [ + 'CF-Connecting-IP', + 'X-Forwarded-For', + 'X-Forwarded', + 'Forwarded', + 'True-Client-IP', + 'X-Real-IP', + 'X-Cluster-Client-Ip', + 'Client-Ip', + ], ], ], - ], + 'dependencies' => [ + 'factories' => [ + IpAddress::class => IpAddressFactory::class, + ], + 'delegators' => [ + // Make middleware decoration transparent to other parts of the code + IpAddress::class => [ + fn ($c, $n, callable $callback) => + // If trusted proxies have been provided, use original middleware verbatim, otherwise decorate + // with workaround + $trustedProxies !== null + ? $callback() + : new ReverseForwardedAddressesMiddlewareDecorator($callback()), + ], + ], -]; + ], + + ]; +})(); diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 4f57a721..072ee8db 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -89,6 +89,7 @@ enum EnvVars: string case CORS_ALLOW_ORIGIN = 'CORS_ALLOW_ORIGIN'; case CORS_ALLOW_CREDENTIALS = 'CORS_ALLOW_CREDENTIALS'; case CORS_MAX_AGE = 'CORS_MAX_AGE'; + case TRUSTED_PROXIES = 'TRUSTED_PROXIES'; /** @deprecated Use REDIRECT_EXTRA_PATH */ case REDIRECT_APPEND_EXTRA_PATH = 'REDIRECT_APPEND_EXTRA_PATH'; diff --git a/module/Core/src/Middleware/ReverseForwardedAddressesMiddlewareDecorator.php b/module/Core/src/Middleware/ReverseForwardedAddressesMiddlewareDecorator.php index 71d7545c..3a86b129 100644 --- a/module/Core/src/Middleware/ReverseForwardedAddressesMiddlewareDecorator.php +++ b/module/Core/src/Middleware/ReverseForwardedAddressesMiddlewareDecorator.php @@ -26,6 +26,8 @@ use function implode; * if trusted proxies are not set. * * @see https://github.com/akrabat/ip-address-middleware/pull/51 + * @deprecated Remove in future major version, and enforce users with multiple reverse proxies to provide the list via + * TRUSTED_PROXIES */ readonly class ReverseForwardedAddressesMiddlewareDecorator implements MiddlewareInterface { From 3318987d637a4abd46fe91d75976e3f66d592835 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 18 Jul 2025 08:22:07 +0200 Subject: [PATCH 33/40] Allow providing hop count via TRUSTED_PROXIES --- CHANGELOG.md | 4 ++++ composer.json | 2 +- config/autoload/installer.global.php | 1 + config/autoload/ip-address.global.php | 6 +++++- 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 383f89a8..a5d1339d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The new conditions match as soon as a query param exists with any or no value (in the case of `any-value-query-param`), or if a query param exists with no value at all (in the case of `valueless-query-param`). +* [#2387](https://github.com/shlinkio/shlink/issues/2387) Add `TRUSTED_PROXIES` env var and corresponding config option, to configure a comma-separated list of all the proxies in front of Shlink, or simply the amount of trusted proxies in front of Shlink. + + This is important to properly detect visitor's IP addresses instead of incorrectly matching one of the proxy's IP address, and if provided, it disables a workaround introduced in https://github.com/shlinkio/shlink/pull/2359. + ### Changed * [#2406](https://github.com/shlinkio/shlink/issues/2406) Remove references to bootstrap from error templates, and instead inline the very minimum required styles. diff --git a/composer.json b/composer.json index 26031b7c..720a7089 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,7 @@ "shlinkio/shlink-config": "^4.0", "shlinkio/shlink-event-dispatcher": "^4.2", "shlinkio/shlink-importer": "^5.6", - "shlinkio/shlink-installer": "dev-develop#9005232 as 9.6", + "shlinkio/shlink-installer": "dev-develop#eef3749 as 9.6", "shlinkio/shlink-ip-geolocation": "^4.3", "shlinkio/shlink-json": "^1.2", "spiral/roadrunner": "^2025.1", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index e9270eb6..9a7bff04 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -80,6 +80,7 @@ return [ Option\Cors\CorsAllowOriginConfigOption::class, Option\Cors\CorsAllowCredentialsConfigOption::class, Option\Cors\CorsMaxAgeConfigOption::class, + Option\TrustedProxiesConfigOption::class, ], 'installation_commands' => [ diff --git a/config/autoload/ip-address.global.php b/config/autoload/ip-address.global.php index 1bac74c2..11902091 100644 --- a/config/autoload/ip-address.global.php +++ b/config/autoload/ip-address.global.php @@ -13,6 +13,7 @@ use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE; return (static function (): array { $trustedProxies = EnvVars::TRUSTED_PROXIES->loadFromEnv(); + $proxiesIsHopCount = is_numeric($trustedProxies); return [ @@ -21,7 +22,10 @@ return (static function (): array { 'ip_address' => [ 'attribute_name' => IP_ADDRESS_REQUEST_ATTRIBUTE, 'check_proxy_headers' => true, - 'trusted_proxies' => splitByComma($trustedProxies), + // List of trusted proxies + 'trusted_proxies' => $proxiesIsHopCount ? [] : splitByComma($trustedProxies), + // Amount of addresses to skip from the right, before finding the visitor IP address + 'hop_count' => $proxiesIsHopCount ? (int) $trustedProxies : 0, 'headers_to_inspect' => [ 'CF-Connecting-IP', 'X-Forwarded-For', From a68300f19af4fe827cc1e47cf296c8dab39f2674 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 18 Jul 2025 08:29:16 +0200 Subject: [PATCH 34/40] Fix phpstan report --- module/Core/functions/functions.php | 1 + 1 file changed, 1 insertion(+) diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 72406e6a..dd73758f 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -143,6 +143,7 @@ function acceptLanguageToLocales(string $acceptLanguage, float $minQuality = 0): */ function splitLocale(string $locale): array { + /** @var string $lang */ [$lang, $countryCode] = array_pad(explode('-', $locale), length: 2, value: null); return [$lang, $countryCode]; } From 2b330953922df7f87888525a5fca255800fe5c18 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 20 Jul 2025 11:56:33 +0200 Subject: [PATCH 35/40] Add support for more device types in device-specific redirects --- CHANGELOG.md | 10 ++++++++- config/test/constants.php | 10 +++++++-- module/Core/src/Model/DeviceType.php | 22 ++++++++++++++----- .../RedirectRule/Entity/RedirectCondition.php | 4 ++-- module/Core/test-api/Action/RedirectTest.php | 11 ++++++++-- .../Entity/RedirectConditionTest.php | 19 +++++++++++++--- .../ShortUrlRedirectionResolverTest.php | 4 ++-- .../test-api/Action/ListRedirectRulesTest.php | 11 ++++++++++ .../Fixtures/ShortUrlRedirectRulesFixture.php | 8 +++++++ 9 files changed, 82 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5d1339d..35cda1ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,10 +19,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The new conditions match as soon as a query param exists with any or no value (in the case of `any-value-query-param`), or if a query param exists with no value at all (in the case of `valueless-query-param`). -* [#2387](https://github.com/shlinkio/shlink/issues/2387) Add `TRUSTED_PROXIES` env var and corresponding config option, to configure a comma-separated list of all the proxies in front of Shlink, or simply the amount of trusted proxies in front of Shlink. +* [#2360](https://github.com/shlinkio/shlink/issues/2360) Add `TRUSTED_PROXIES` env var and corresponding config option, to configure a comma-separated list of all the proxies in front of Shlink, or simply the amount of trusted proxies in front of Shlink. This is important to properly detect visitor's IP addresses instead of incorrectly matching one of the proxy's IP address, and if provided, it disables a workaround introduced in https://github.com/shlinkio/shlink/pull/2359. +* [#2274](https://github.com/shlinkio/shlink/issues/2274) Add more supported device types for the `device` redirect condition: + + * `linux`: Will match desktop devices with Linux. + * `windows`: Will match desktop devices with Windows. + * `macos`: Will match desktop devices with MacOS. + * `chromeos`: Will match desktop devices with ChromeOS. + * `mobile`: Will match any mobile devices with either Android or iOS. + ### Changed * [#2406](https://github.com/shlinkio/shlink/issues/2406) Remove references to bootstrap from error templates, and instead inline the very minimum required styles. diff --git a/config/test/constants.php b/config/test/constants.php index bce232f3..f2ad124d 100644 --- a/config/test/constants.php +++ b/config/test/constants.php @@ -11,5 +11,11 @@ const ANDROID_USER_AGENT = 'Mozilla/5.0 (Linux; Android 13) AppleWebKit/537.36 ( . 'Chrome/109.0.5414.86 Mobile Safari/537.36'; const IOS_USER_AGENT = 'Mozilla/5.0 (iPhone; CPU iPhone OS 16_2 like Mac OS X) AppleWebKit/605.1.15 ' . '(KHTML, like Gecko) FxiOS/109.0 Mobile/15E148 Safari/605.1.15'; -const DESKTOP_USER_AGENT = 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like ' - . 'Gecko) Chrome/109.0.0.0 Safari/537.36 Edg/109.0.1518.61'; +const WINDOWS_USER_AGENT = 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) ' + . 'Chrome/138.0.0.0 Safari/537.36 Edg/138.0.3351.95'; +const LINUX_USER_AGENT = 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) ' + . 'HeadlessChrome/81.0.4044.113 Safari/537.36'; +const MACOS_USER_AGENT = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 15_5) AppleWebKit/605.1.15 (KHTML, like Gecko) ' + . 'Version/18.4 Safari/605.1.15'; +const CHROMEOS_USER_AGENT = 'Mozilla/5.0 (X11; CrOS x86_64 16181.61.0) AppleWebKit/537.36 (KHTML, like Gecko) ' + . 'Chrome/134.0.6998.198 Safari/537.36'; diff --git a/module/Core/src/Model/DeviceType.php b/module/Core/src/Model/DeviceType.php index c63b2dff..11b57e02 100644 --- a/module/Core/src/Model/DeviceType.php +++ b/module/Core/src/Model/DeviceType.php @@ -9,18 +9,30 @@ enum DeviceType: string { case ANDROID = 'android'; case IOS = 'ios'; + case MOBILE = 'mobile'; + case WINDOWS = 'windows'; + case MACOS = 'macos'; + case LINUX = 'linux'; + case CHROMEOS = 'chromeos'; case DESKTOP = 'desktop'; - public static function matchFromUserAgent(string $userAgent): self|null + /** + * Determines which device types provided user agent matches. It could be more than one + * @return self[] + */ + public static function matchFromUserAgent(string $userAgent): array { static $uaParser = new UserAgentParser(); $ua = $uaParser->parse($userAgent); 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, + Platforms::IPHONE, Platforms::IPAD => [self::IOS, self::MOBILE], // iPhone and iPad (except iPadOS 13+) + Platforms::ANDROID => [self::ANDROID, self::MOBILE], // android phones and android tablets + Platforms::LINUX => [self::LINUX, self::DESKTOP], + Platforms::WINDOWS => [self::WINDOWS, self::DESKTOP], + Platforms::MACINTOSH => [self::MACOS, self::DESKTOP], + Platforms::CHROME_OS => [self::CHROMEOS, self::DESKTOP], + default => [], }; } } diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index dd24ca6d..45e29839 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -166,8 +166,8 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable private function matchesDevice(ServerRequestInterface $request): bool { - $device = DeviceType::matchFromUserAgent($request->getHeaderLine('User-Agent')); - return $device !== null && $device->value === $this->matchValue; + $devices = DeviceType::matchFromUserAgent($request->getHeaderLine('User-Agent')); + return some($devices, fn (DeviceType $device) => $device->value === $this->matchValue); } private function matchesRemoteIpAddress(ServerRequestInterface $request): bool diff --git a/module/Core/test-api/Action/RedirectTest.php b/module/Core/test-api/Action/RedirectTest.php index 799851c7..bc2d0c70 100644 --- a/module/Core/test-api/Action/RedirectTest.php +++ b/module/Core/test-api/Action/RedirectTest.php @@ -13,8 +13,9 @@ use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; use function sprintf; use const ShlinkioTest\Shlink\ANDROID_USER_AGENT; -use const ShlinkioTest\Shlink\DESKTOP_USER_AGENT; use const ShlinkioTest\Shlink\IOS_USER_AGENT; +use const ShlinkioTest\Shlink\LINUX_USER_AGENT; +use const ShlinkioTest\Shlink\WINDOWS_USER_AGENT; class RedirectTest extends ApiTestCase { @@ -41,9 +42,15 @@ class RedirectTest extends ApiTestCase ], 'fb://profile/33138223345', ]; + yield 'linux' => [ + [ + RequestOptions::HEADERS => ['User-Agent' => LINUX_USER_AGENT], + ], + 'https://example.com/linux', + ]; yield 'desktop' => [ [ - RequestOptions::HEADERS => ['User-Agent' => DESKTOP_USER_AGENT], + RequestOptions::HEADERS => ['User-Agent' => WINDOWS_USER_AGENT], ], 'https://blog.alejandrocelaya.com/2017/12/09/acmailer-7-0-the-most-important-release-in-a-long-time/', ]; diff --git a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php index 28e3f6b1..cc544353 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -15,8 +15,11 @@ use Shlinkio\Shlink\IpGeolocation\Model\Location; use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE; use const ShlinkioTest\Shlink\ANDROID_USER_AGENT; -use const ShlinkioTest\Shlink\DESKTOP_USER_AGENT; +use const ShlinkioTest\Shlink\CHROMEOS_USER_AGENT; use const ShlinkioTest\Shlink\IOS_USER_AGENT; +use const ShlinkioTest\Shlink\LINUX_USER_AGENT; +use const ShlinkioTest\Shlink\MACOS_USER_AGENT; +use const ShlinkioTest\Shlink\WINDOWS_USER_AGENT; class RedirectConditionTest extends TestCase { @@ -89,10 +92,20 @@ class RedirectConditionTest extends TestCase #[TestWith([null, DeviceType::ANDROID, false])] #[TestWith(['unknown', DeviceType::ANDROID, false])] #[TestWith([ANDROID_USER_AGENT, DeviceType::ANDROID, true])] - #[TestWith([DESKTOP_USER_AGENT, DeviceType::DESKTOP, true])] + #[TestWith([WINDOWS_USER_AGENT, DeviceType::DESKTOP, true])] + #[TestWith([LINUX_USER_AGENT, DeviceType::DESKTOP, true])] + #[TestWith([MACOS_USER_AGENT, DeviceType::DESKTOP, true])] + #[TestWith([CHROMEOS_USER_AGENT, DeviceType::DESKTOP, true])] + #[TestWith([WINDOWS_USER_AGENT, DeviceType::WINDOWS, true])] + #[TestWith([LINUX_USER_AGENT, DeviceType::LINUX, true])] + #[TestWith([MACOS_USER_AGENT, DeviceType::MACOS, true])] + #[TestWith([CHROMEOS_USER_AGENT, DeviceType::CHROMEOS, true])] #[TestWith([IOS_USER_AGENT, DeviceType::IOS, true])] + #[TestWith([IOS_USER_AGENT, DeviceType::MOBILE, true])] + #[TestWith([ANDROID_USER_AGENT, DeviceType::MOBILE, true])] #[TestWith([IOS_USER_AGENT, DeviceType::ANDROID, false])] - #[TestWith([DESKTOP_USER_AGENT, DeviceType::IOS, false])] + #[TestWith([WINDOWS_USER_AGENT, DeviceType::IOS, false])] + #[TestWith([LINUX_USER_AGENT, DeviceType::WINDOWS, false])] public function matchesDevice(string|null $userAgent, DeviceType $value, bool $expected): void { $request = ServerRequestFactory::fromGlobals(); diff --git a/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php b/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php index 470ff95e..d7948be2 100644 --- a/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php +++ b/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php @@ -19,8 +19,8 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE; use const ShlinkioTest\Shlink\ANDROID_USER_AGENT; -use const ShlinkioTest\Shlink\DESKTOP_USER_AGENT; use const ShlinkioTest\Shlink\IOS_USER_AGENT; +use const ShlinkioTest\Shlink\WINDOWS_USER_AGENT; class ShortUrlRedirectionResolverTest extends TestCase { @@ -68,7 +68,7 @@ class ShortUrlRedirectionResolverTest extends TestCase RedirectCondition::forLanguage('es-ES'), // This condition won't match 'https://example.com/foo/bar', ]; - yield 'desktop user agent' => [$request(DESKTOP_USER_AGENT), null, 'https://example.com/foo/bar']; + yield 'desktop user agent' => [$request(WINDOWS_USER_AGENT), null, 'https://example.com/foo/bar']; yield 'matching android device' => [ $request(ANDROID_USER_AGENT), RedirectCondition::forDevice(DeviceType::ANDROID), diff --git a/module/Rest/test-api/Action/ListRedirectRulesTest.php b/module/Rest/test-api/Action/ListRedirectRulesTest.php index 7909dcfd..b24caa7a 100644 --- a/module/Rest/test-api/Action/ListRedirectRulesTest.php +++ b/module/Rest/test-api/Action/ListRedirectRulesTest.php @@ -98,6 +98,17 @@ class ListRedirectRulesTest extends ApiTestCase ], ], ], + [ + 'longUrl' => 'https://example.com/linux', + 'priority' => 7, + 'conditions' => [ + [ + 'type' => 'device', + 'matchKey' => null, + 'matchValue' => 'linux', + ], + ], + ], ]])] public function returnsListOfRulesForShortUrl(string $shortCode, array $expectedRules): void { diff --git a/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php b/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php index 3aa81315..50b1dae5 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php @@ -78,6 +78,14 @@ class ShortUrlRedirectRulesFixture extends AbstractFixture implements DependentF ); $manager->persist($ipAddressRule); + $linuxRule = new ShortUrlRedirectRule( + shortUrl: $defShortUrl, + priority: 7, + longUrl: 'https://example.com/linux', + conditions: new ArrayCollection([RedirectCondition::forDevice(DeviceType::LINUX)]), + ); + $manager->persist($linuxRule); + $manager->flush(); } } From 8b259b364d7a1d5db7f08e0d098e005ca909ccae Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Jul 2025 10:02:36 +0200 Subject: [PATCH 36/40] Allow redirect cache visibility to be configured --- CHANGELOG.md | 2 + composer.json | 2 +- config/autoload/installer.global.php | 1 + config/constants.php | 1 + module/Core/src/Config/EnvVars.php | 1 + .../src/Config/Options/RedirectOptions.php | 11 ++++- .../Core/src/Util/RedirectResponseHelper.php | 6 ++- .../test/Util/RedirectResponseHelperTest.php | 45 +++++++++++++------ 8 files changed, 51 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35cda1ca..b9671fa9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * `chromeos`: Will match desktop devices with ChromeOS. * `mobile`: Will match any mobile devices with either Android or iOS. +* [#2093](https://github.com/shlinkio/shlink/issues/2093) Add `REDIRECT_CACHE_LIFETIME` env var and corresponding config option, so that it is possible to set the `Cache-Control` visibility directive (`public` or `private`) when the `REDIRECT_STATUS_CODE` has been set to `301` or `308`. + ### Changed * [#2406](https://github.com/shlinkio/shlink/issues/2406) Remove references to bootstrap from error templates, and instead inline the very minimum required styles. diff --git a/composer.json b/composer.json index 720a7089..85ad757b 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,7 @@ "shlinkio/shlink-config": "^4.0", "shlinkio/shlink-event-dispatcher": "^4.2", "shlinkio/shlink-importer": "^5.6", - "shlinkio/shlink-installer": "dev-develop#eef3749 as 9.6", + "shlinkio/shlink-installer": "dev-develop#7f9147b as 9.6", "shlinkio/shlink-ip-geolocation": "^4.3", "shlinkio/shlink-json": "^1.2", "spiral/roadrunner": "^2025.1", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 9a7bff04..21d16bb3 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -41,6 +41,7 @@ return [ Option\UrlShortener\GeoLiteLicenseKeyConfigOption::class, Option\UrlShortener\RedirectStatusCodeConfigOption::class, Option\UrlShortener\RedirectCacheLifeTimeConfigOption::class, + Option\UrlShortener\RedirectCacheVisibilityConfigOption::class, Option\UrlShortener\AutoResolveTitlesConfigOption::class, Option\UrlShortener\ExtraPathModeConfigOption::class, Option\UrlShortener\EnableMultiSegmentSlugsConfigOption::class, diff --git a/config/constants.php b/config/constants.php index 28440653..664c8c9f 100644 --- a/config/constants.php +++ b/config/constants.php @@ -11,6 +11,7 @@ const DEFAULT_SHORT_CODES_LENGTH = 5; const MIN_SHORT_CODES_LENGTH = 4; const DEFAULT_REDIRECT_STATUS_CODE = RedirectStatus::STATUS_302; const DEFAULT_REDIRECT_CACHE_LIFETIME = 30; +const DEFAULT_REDIRECT_CACHE_VISIBILITY = 'private'; const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory'; const LOOSE_URI_MATCHER = '/(.+)\:(.+)/i'; // Matches anything starting with a schema. const IP_ADDRESS_REQUEST_ATTRIBUTE = 'remote_address'; diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 072ee8db..c99635da 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -62,6 +62,7 @@ enum EnvVars: string case DEFAULT_BASE_URL_REDIRECT = 'DEFAULT_BASE_URL_REDIRECT'; case REDIRECT_STATUS_CODE = 'REDIRECT_STATUS_CODE'; case REDIRECT_CACHE_LIFETIME = 'REDIRECT_CACHE_LIFETIME'; + case REDIRECT_CACHE_VISIBILITY = 'REDIRECT_CACHE_VISIBILITY'; case BASE_PATH = 'BASE_PATH'; case SHORT_URL_TRAILING_SLASH = 'SHORT_URL_TRAILING_SLASH'; case SHORT_URL_MODE = 'SHORT_URL_MODE'; diff --git a/module/Core/src/Config/Options/RedirectOptions.php b/module/Core/src/Config/Options/RedirectOptions.php index 75faf097..ffb6dd36 100644 --- a/module/Core/src/Config/Options/RedirectOptions.php +++ b/module/Core/src/Config/Options/RedirectOptions.php @@ -4,26 +4,32 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Config\Options; -use Fig\Http\Message\StatusCodeInterface; use Shlinkio\Shlink\Core\Config\EnvVars; use Shlinkio\Shlink\Core\Util\RedirectStatus; use const Shlinkio\Shlink\DEFAULT_REDIRECT_CACHE_LIFETIME; +use const Shlinkio\Shlink\DEFAULT_REDIRECT_CACHE_VISIBILITY; use const Shlinkio\Shlink\DEFAULT_REDIRECT_STATUS_CODE; final readonly class RedirectOptions { public RedirectStatus $redirectStatusCode; public int $redirectCacheLifetime; + /** @var 'public'|'private' */ + public string $redirectCacheVisibility; public function __construct( - int $redirectStatusCode = StatusCodeInterface::STATUS_FOUND, + int $redirectStatusCode = RedirectStatus::STATUS_302->value, int $redirectCacheLifetime = DEFAULT_REDIRECT_CACHE_LIFETIME, + string|null $redirectCacheVisibility = DEFAULT_REDIRECT_CACHE_VISIBILITY, ) { $this->redirectStatusCode = RedirectStatus::tryFrom($redirectStatusCode) ?? DEFAULT_REDIRECT_STATUS_CODE; $this->redirectCacheLifetime = $redirectCacheLifetime > 0 ? $redirectCacheLifetime : DEFAULT_REDIRECT_CACHE_LIFETIME; + $this->redirectCacheVisibility = $redirectCacheVisibility === 'public' || $redirectCacheVisibility === 'private' + ? $redirectCacheVisibility + : DEFAULT_REDIRECT_CACHE_VISIBILITY; } public static function fromEnv(): self @@ -31,6 +37,7 @@ final readonly class RedirectOptions return new self( redirectStatusCode: (int) EnvVars::REDIRECT_STATUS_CODE->loadFromEnv(), redirectCacheLifetime: (int) EnvVars::REDIRECT_CACHE_LIFETIME->loadFromEnv(), + redirectCacheVisibility: EnvVars::REDIRECT_CACHE_VISIBILITY->loadFromEnv(), ); } } diff --git a/module/Core/src/Util/RedirectResponseHelper.php b/module/Core/src/Util/RedirectResponseHelper.php index 4c4fdd21..31b1d016 100644 --- a/module/Core/src/Util/RedirectResponseHelper.php +++ b/module/Core/src/Util/RedirectResponseHelper.php @@ -20,7 +20,11 @@ readonly class RedirectResponseHelper implements RedirectResponseHelperInterface { $statusCode = $this->options->redirectStatusCode; $headers = ! $statusCode->allowsCache() ? [] : [ - 'Cache-Control' => sprintf('private,max-age=%s', $this->options->redirectCacheLifetime), + 'Cache-Control' => sprintf( + '%s,max-age=%s', + $this->options->redirectCacheVisibility, + $this->options->redirectCacheLifetime, + ), ]; return new RedirectResponse($location, $statusCode->value, $headers); diff --git a/module/Core/test/Util/RedirectResponseHelperTest.php b/module/Core/test/Util/RedirectResponseHelperTest.php index b01333d5..f4e731d9 100644 --- a/module/Core/test/Util/RedirectResponseHelperTest.php +++ b/module/Core/test/Util/RedirectResponseHelperTest.php @@ -15,13 +15,10 @@ class RedirectResponseHelperTest extends TestCase { #[Test, DataProvider('provideRedirectConfigs')] public function expectedStatusCodeAndCacheIsReturnedBasedOnConfig( - int $configuredStatus, - int $configuredLifetime, + RedirectOptions $options, int $expectedStatus, string|null $expectedCacheControl, ): void { - $options = new RedirectOptions($configuredStatus, $configuredLifetime); - $response = $this->helper($options)->buildRedirectResponse('destination'); self::assertInstanceOf(RedirectResponse::class, $response); @@ -34,16 +31,36 @@ class RedirectResponseHelperTest extends TestCase public static function provideRedirectConfigs(): iterable { - yield 'status 302' => [302, 20, 302, null]; - yield 'status 307' => [307, 20, 307, null]; - yield 'status over 308' => [400, 20, 302, null]; - yield 'status below 301' => [201, 20, 302, null]; - yield 'status 301 with valid expiration' => [301, 20, 301, 'private,max-age=20']; - yield 'status 301 with zero expiration' => [301, 0, 301, 'private,max-age=30']; - yield 'status 301 with negative expiration' => [301, -20, 301, 'private,max-age=30']; - yield 'status 308 with valid expiration' => [308, 20, 308, 'private,max-age=20']; - yield 'status 308 with zero expiration' => [308, 0, 308, 'private,max-age=30']; - yield 'status 308 with negative expiration' => [308, -20, 308, 'private,max-age=30']; + yield 'status 302' => [new RedirectOptions(302, 20), 302, null]; + yield 'status 307' => [new RedirectOptions(307, 20), 307, null]; + yield 'status over 308' => [new RedirectOptions(400, 20), 302, null]; + yield 'status below 301' => [new RedirectOptions(201, 20), 302, null]; + yield 'status 301 with valid expiration' => [new RedirectOptions(301, 20), 301, 'private,max-age=20']; + yield 'status 301 with zero expiration' => [new RedirectOptions(301, 0), 301, 'private,max-age=30']; + yield 'status 301 with negative expiration' => [new RedirectOptions(301, -20), 301, 'private,max-age=30']; + yield 'status 308 with valid expiration' => [new RedirectOptions(308, 20), 308, 'private,max-age=20']; + yield 'status 308 with zero expiration' => [new RedirectOptions(308, 0), 308, 'private,max-age=30']; + yield 'status 308 with negative expiration' => [new RedirectOptions(308, -20), 308, 'private,max-age=30']; + yield 'status 301 with public cache' => [ + new RedirectOptions(301, redirectCacheVisibility: 'public'), + 301, + 'public,max-age=30', + ]; + yield 'status 308 with public cache' => [ + new RedirectOptions(308, redirectCacheVisibility: 'public'), + 308, + 'public,max-age=30', + ]; + yield 'status 301 with private cache' => [ + new RedirectOptions(301, redirectCacheVisibility: 'private'), + 301, + 'private,max-age=30', + ]; + yield 'status 301 with invalid cache' => [ + new RedirectOptions(301, redirectCacheVisibility: 'something-else'), + 301, + 'private,max-age=30', + ]; } private function helper(RedirectOptions|null $options = null): RedirectResponseHelper From afa509613aebbd79e3fd09fe5b3263089ffba489 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Jul 2025 10:28:47 +0200 Subject: [PATCH 37/40] Run tests under PHP 8.5 in CI --- .github/actions/ci-setup/action.yml | 2 +- .github/workflows/ci-db-tests.yml | 3 ++- .github/workflows/ci-tests.yml | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/actions/ci-setup/action.yml b/.github/actions/ci-setup/action.yml index 3a6a8642..b45fa6c2 100644 --- a/.github/actions/ci-setup/action.yml +++ b/.github/actions/ci-setup/action.yml @@ -43,5 +43,5 @@ runs: coverage: xdebug - name: Install dependencies if: ${{ inputs.install-deps == 'yes' }} - run: composer install --no-interaction --prefer-dist + run: composer install --no-interaction --prefer-dist ${{ inputs.php-version == '8.5' && '--ignore-platform-req=php' || '' }} shell: bash diff --git a/.github/workflows/ci-db-tests.yml b/.github/workflows/ci-db-tests.yml index cb7638be..82b08317 100644 --- a/.github/workflows/ci-db-tests.yml +++ b/.github/workflows/ci-db-tests.yml @@ -13,7 +13,8 @@ jobs: runs-on: ubuntu-24.04 strategy: matrix: - php-version: ['8.3', '8.4'] + php-version: ['8.3', '8.4', '8.5'] + continue-on-error: ${{ inputs.php-version == '8.5' }} env: LC_ALL: C steps: diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index f0b7d847..9220573d 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -13,7 +13,8 @@ jobs: runs-on: ubuntu-24.04 strategy: matrix: - php-version: ['8.3', '8.4'] + php-version: ['8.3', '8.4', '8.5'] + continue-on-error: ${{ inputs.php-version == '8.5' }} env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # rr get-binary picks this env automatically steps: From 5b5d0aae4991d074d0a126f0be5b79ee6b8ea8eb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 22 Jul 2025 08:20:21 +0200 Subject: [PATCH 38/40] Make RedirectCondition->matchValue nullable --- .../definitions/SetShortUrlRedirectRule.json | 2 +- ....RedirectRule.Entity.RedirectCondition.php | 1 + .../Core/migrations/Version20250722060208.php | 26 +++++++++++++++++++ .../RedirectRule/Entity/RedirectCondition.php | 22 +++++++++------- .../Model/RedirectConditionType.php | 6 +---- .../Validation/RedirectRulesInputFilter.php | 2 +- .../Model/RedirectConditionTypeTest.php | 4 +-- .../test-api/Action/ListRedirectRulesTest.php | 17 +++++++----- .../test-api/Action/SetRedirectRulesTest.php | 17 +++++++----- .../Fixtures/ShortUrlRedirectRulesFixture.php | 2 +- 10 files changed, 66 insertions(+), 33 deletions(-) create mode 100644 module/Core/migrations/Version20250722060208.php diff --git a/docs/swagger/definitions/SetShortUrlRedirectRule.json b/docs/swagger/definitions/SetShortUrlRedirectRule.json index b6c66f5f..15380faa 100644 --- a/docs/swagger/definitions/SetShortUrlRedirectRule.json +++ b/docs/swagger/definitions/SetShortUrlRedirectRule.json @@ -31,7 +31,7 @@ "type": ["string", "null"] }, "matchValue": { - "type": "string" + "type": ["string", "null"] } } } diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.RedirectRule.Entity.RedirectCondition.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.RedirectRule.Entity.RedirectCondition.php index 513089fa..516b0640 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.RedirectRule.Entity.RedirectCondition.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.RedirectRule.Entity.RedirectCondition.php @@ -39,5 +39,6 @@ return static function (ClassMetadata $metadata, array $emConfig): void { fieldWithUtf8Charset($builder->createField('matchValue', Types::STRING), $emConfig) ->columnName('match_value') ->length(512) + ->nullable() ->build(); }; diff --git a/module/Core/migrations/Version20250722060208.php b/module/Core/migrations/Version20250722060208.php new file mode 100644 index 00000000..552b3dcc --- /dev/null +++ b/module/Core/migrations/Version20250722060208.php @@ -0,0 +1,26 @@ +getTable('redirect_conditions')->getColumn('match_value')->setNotnull(false); + } + + public function isTransactional(): bool + { + return ! ($this->connection->getDatabasePlatform() instanceof MySQLPlatform); + } +} diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index 45e29839..2413400d 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -26,7 +26,7 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable { private function __construct( public readonly RedirectConditionType $type, - private readonly string $matchValue, + private readonly string|null $matchValue = null, private readonly string|null $matchKey = null, ) { } @@ -38,12 +38,12 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable public static function forAnyValueQueryParam(string $param): self { - return new self(RedirectConditionType::ANY_VALUE_QUERY_PARAM, $param); + return new self(RedirectConditionType::ANY_VALUE_QUERY_PARAM, matchKey: $param); } public static function forValuelessQueryParam(string $param): self { - return new self(RedirectConditionType::VALUELESS_QUERY_PARAM, $param); + return new self(RedirectConditionType::VALUELESS_QUERY_PARAM, matchKey: $param); } public static function forLanguage(string $language): self @@ -131,19 +131,19 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable private function matchesValuelessQueryParam(ServerRequestInterface $request): bool { $query = $request->getQueryParams(); - return array_key_exists($this->matchValue, $query) && empty($query[$this->matchValue]); + return $this->matchKey !== null && array_key_exists($this->matchKey, $query) && empty($query[$this->matchKey]); } private function matchesAnyValueQueryParam(ServerRequestInterface $request): bool { $query = $request->getQueryParams(); - return array_key_exists($this->matchValue, $query); + return $this->matchKey !== null && array_key_exists($this->matchKey, $query); } private function matchesLanguage(ServerRequestInterface $request): bool { $acceptLanguage = trim($request->getHeaderLine('Accept-Language')); - if ($acceptLanguage === '' || $acceptLanguage === '*') { + if ($acceptLanguage === '' || $acceptLanguage === '*' || $this->matchValue === null) { return false; } @@ -173,13 +173,17 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable private function matchesRemoteIpAddress(ServerRequestInterface $request): bool { $remoteAddress = ipAddressFromRequest($request); - return $remoteAddress !== null && IpAddressUtils::ipAddressMatchesGroups($remoteAddress, [$this->matchValue]); + return ( + $this->matchValue !== null + && $remoteAddress !== null + && IpAddressUtils::ipAddressMatchesGroups($remoteAddress, [$this->matchValue]) + ); } private function matchesGeolocationCountryCode(ServerRequestInterface $request): bool { $geolocation = geolocationFromRequest($request); - if ($geolocation === null) { + if ($geolocation === null || $this->matchValue === null) { return false; } @@ -189,7 +193,7 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable private function matchesGeolocationCityName(ServerRequestInterface $request): bool { $geolocation = geolocationFromRequest($request); - if ($geolocation === null) { + if ($geolocation === null || $this->matchValue === null) { return false; } diff --git a/module/Core/src/RedirectRule/Model/RedirectConditionType.php b/module/Core/src/RedirectRule/Model/RedirectConditionType.php index 8f7657e3..0461d968 100644 --- a/module/Core/src/RedirectRule/Model/RedirectConditionType.php +++ b/module/Core/src/RedirectRule/Model/RedirectConditionType.php @@ -31,11 +31,7 @@ enum RedirectConditionType: string // RedirectConditionType::LANGUAGE => TODO Validate at least format, RedirectConditionType::IP_ADDRESS => IpAddressUtils::isStaticIpCidrOrWildcard($value), RedirectConditionType::GEOLOCATION_COUNTRY_CODE => contains($value, ISO_COUNTRY_CODES), - RedirectConditionType::QUERY_PARAM, - RedirectConditionType::ANY_VALUE_QUERY_PARAM, - RedirectConditionType::VALUELESS_QUERY_PARAM => $value !== '', - // FIXME We should at least validate the value is not empty - // default => $value !== '', + RedirectConditionType::QUERY_PARAM => $value !== '', default => true, }; } diff --git a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php index c802d75f..c1fb7fc7 100644 --- a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php +++ b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php @@ -75,7 +75,7 @@ class RedirectRulesInputFilter extends InputFilter ])); $redirectConditionInputFilter->add($type); - $value = InputFactory::basic(self::CONDITION_MATCH_VALUE, required: true); + $value = InputFactory::basic(self::CONDITION_MATCH_VALUE, required: true)->setAllowEmpty(true); $value->getValidatorChain()->attach(new Callback( function (string $value, array $context): bool { $conditionType = RedirectConditionType::tryFrom($context[self::CONDITION_TYPE]); diff --git a/module/Core/test/RedirectRule/Model/RedirectConditionTypeTest.php b/module/Core/test/RedirectRule/Model/RedirectConditionTypeTest.php index 1c68a0f4..84d9f511 100644 --- a/module/Core/test/RedirectRule/Model/RedirectConditionTypeTest.php +++ b/module/Core/test/RedirectRule/Model/RedirectConditionTypeTest.php @@ -14,9 +14,9 @@ class RedirectConditionTypeTest extends TestCase #[Test] #[TestWith([RedirectConditionType::QUERY_PARAM, '', false])] #[TestWith([RedirectConditionType::QUERY_PARAM, 'foo', true])] - #[TestWith([RedirectConditionType::ANY_VALUE_QUERY_PARAM, '', false])] + #[TestWith([RedirectConditionType::ANY_VALUE_QUERY_PARAM, '', true])] #[TestWith([RedirectConditionType::ANY_VALUE_QUERY_PARAM, 'foo', true])] - #[TestWith([RedirectConditionType::VALUELESS_QUERY_PARAM, '', false])] + #[TestWith([RedirectConditionType::VALUELESS_QUERY_PARAM, '', true])] #[TestWith([RedirectConditionType::VALUELESS_QUERY_PARAM, 'foo', true])] public function isValidFailsForEmptyQueryParams( RedirectConditionType $conditionType, diff --git a/module/Rest/test-api/Action/ListRedirectRulesTest.php b/module/Rest/test-api/Action/ListRedirectRulesTest.php index b24caa7a..859e5ef3 100644 --- a/module/Rest/test-api/Action/ListRedirectRulesTest.php +++ b/module/Rest/test-api/Action/ListRedirectRulesTest.php @@ -17,11 +17,6 @@ class ListRedirectRulesTest extends ApiTestCase 'matchKey' => null, 'matchValue' => 'en', ]; - private const array QUERY_FOO_BAR_CONDITION = [ - 'type' => 'query-param', - 'matchKey' => 'foo', - 'matchValue' => 'bar', - ]; #[Test] public function errorIsReturnedWhenInvalidUrlIsFetched(): void @@ -45,7 +40,11 @@ class ListRedirectRulesTest extends ApiTestCase 'priority' => 1, 'conditions' => [ self::LANGUAGE_EN_CONDITION, - self::QUERY_FOO_BAR_CONDITION, + [ + 'type' => 'any-value-query-param', + 'matchKey' => 'foo', + 'matchValue' => null, + ], ], ], [ @@ -57,7 +56,11 @@ class ListRedirectRulesTest extends ApiTestCase 'matchKey' => 'hello', 'matchValue' => 'world', ], - self::QUERY_FOO_BAR_CONDITION, + [ + 'type' => 'query-param', + 'matchKey' => 'foo', + 'matchValue' => 'bar', + ], ], ], [ diff --git a/module/Rest/test-api/Action/SetRedirectRulesTest.php b/module/Rest/test-api/Action/SetRedirectRulesTest.php index 9fc6fa9d..a9ffa206 100644 --- a/module/Rest/test-api/Action/SetRedirectRulesTest.php +++ b/module/Rest/test-api/Action/SetRedirectRulesTest.php @@ -18,11 +18,6 @@ class SetRedirectRulesTest extends ApiTestCase 'matchKey' => null, 'matchValue' => 'en', ]; - private const array QUERY_FOO_BAR_CONDITION = [ - 'type' => 'query-param', - 'matchKey' => 'foo', - 'matchValue' => 'bar', - ]; #[Test] public function errorIsReturnedWhenInvalidUrlIsProvided(): void @@ -132,7 +127,11 @@ class SetRedirectRulesTest extends ApiTestCase 'priority' => 1, 'conditions' => [ self::LANGUAGE_EN_CONDITION, - self::QUERY_FOO_BAR_CONDITION, + [ + 'type' => 'any-value-query-param', + 'matchKey' => 'foo', + 'matchValue' => null, + ], ], ], [ @@ -144,7 +143,11 @@ class SetRedirectRulesTest extends ApiTestCase 'matchKey' => 'hello', 'matchValue' => 'world', ], - self::QUERY_FOO_BAR_CONDITION, + [ + 'type' => 'query-param', + 'matchKey' => 'foo', + 'matchValue' => 'bar', + ], ], ], ]])] diff --git a/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php b/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php index 50b1dae5..2e963034 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php @@ -41,7 +41,7 @@ class ShortUrlRedirectRulesFixture extends AbstractFixture implements DependentF priority: 1, longUrl: 'https://example.com/english-and-foo-query', conditions: new ArrayCollection( - [RedirectCondition::forLanguage('en'), RedirectCondition::forQueryParam('foo', 'bar')], + [RedirectCondition::forLanguage('en'), RedirectCondition::forAnyValueQueryParam('foo')], ), ); $manager->persist($englishAndFooQueryRule); From e40b82618ad4adf7e6b705038f010022cc19888f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 24 Jul 2025 09:57:34 +0200 Subject: [PATCH 39/40] Allow logs format to be configured as console or JSON --- CHANGELOG.md | 1 + composer.json | 4 ++-- config/autoload/installer.global.php | 1 + config/autoload/logger.global.php | 18 +++++++++++------- config/roadrunner/.rr.dev.yml | 4 ++++ config/roadrunner/.rr.test.yml | 5 +++-- config/roadrunner/.rr.yml | 3 +++ module/Core/src/Config/EnvVars.php | 3 +++ 8 files changed, 28 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9671fa9..7673c67e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * `mobile`: Will match any mobile devices with either Android or iOS. * [#2093](https://github.com/shlinkio/shlink/issues/2093) Add `REDIRECT_CACHE_LIFETIME` env var and corresponding config option, so that it is possible to set the `Cache-Control` visibility directive (`public` or `private`) when the `REDIRECT_STATUS_CODE` has been set to `301` or `308`. +* [#2323](https://github.com/shlinkio/shlink/issues/2323) Add `LOGS_FORMAT` env var and corresponding config option, to allow the logs generated by Shlink to be in console or JSON formats. ### Changed * [#2406](https://github.com/shlinkio/shlink/issues/2406) Remove references to bootstrap from error templates, and instead inline the very minimum required styles. diff --git a/composer.json b/composer.json index 85ad757b..b410da3a 100644 --- a/composer.json +++ b/composer.json @@ -43,11 +43,11 @@ "pagerfanta/core": "^3.8", "ramsey/uuid": "^4.7", "shlinkio/doctrine-specification": "^2.2", - "shlinkio/shlink-common": "dev-main#7469270 as 7.1", + "shlinkio/shlink-common": "dev-main#2c9387c as 7.1", "shlinkio/shlink-config": "^4.0", "shlinkio/shlink-event-dispatcher": "^4.2", "shlinkio/shlink-importer": "^5.6", - "shlinkio/shlink-installer": "dev-develop#7f9147b as 9.6", + "shlinkio/shlink-installer": "dev-develop#1c775a9 as 9.6", "shlinkio/shlink-ip-geolocation": "^4.3", "shlinkio/shlink-json": "^1.2", "spiral/roadrunner": "^2025.1", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 21d16bb3..7f9e8900 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -13,6 +13,7 @@ return [ 'enabled_options' => [ Option\Server\RuntimeConfigOption::class, Option\Server\MemoryLimitConfigOption::class, + Option\Server\LogsFormatConfigOption::class, Option\Database\DatabaseDriverConfigOption::class, Option\Database\DatabaseNameConfigOption::class, Option\Database\DatabaseHostConfigOption::class, diff --git a/config/autoload/logger.global.php b/config/autoload/logger.global.php index 56ba42bb..c81fe45d 100644 --- a/config/autoload/logger.global.php +++ b/config/autoload/logger.global.php @@ -23,11 +23,16 @@ use function Shlinkio\Shlink\Config\runningInRoadRunner; return (static function (): array { $isDev = EnvVars::isDevEnv(); - $common = [ + $format = EnvVars::LOGS_FORMAT->loadFromEnv(); + $buildCommonConfig = static fn (bool $addNewLine = false) => [ 'level' => $isDev ? Level::Debug->value : Level::Info->value, 'processors' => [RequestIdMiddleware::class], - 'line_format' => - '[%datetime%] [%extra.' . RequestIdMiddleware::ATTRIBUTE . '%] %channel%.%level_name% - %message%', + 'formatter' => [ + 'type' => $format, + 'add_new_line' => $addNewLine, + 'line_format' => + '[%datetime%] [%extra.' . RequestIdMiddleware::ATTRIBUTE . '%] %channel%.%level_name% - %message%', + ], ]; // In dev env or the docker container, stream Shlink logs to stderr, otherwise send them to a file @@ -39,16 +44,15 @@ return (static function (): array { 'Shlink' => $useStreamForShlinkLogger ? [ 'type' => LoggerType::STREAM->value, 'destination' => 'php://stderr', - ...$common, + ...$buildCommonConfig(), ] : [ 'type' => LoggerType::FILE->value, - ...$common, + ...$buildCommonConfig(), ], 'Access' => [ 'type' => LoggerType::STREAM->value, 'destination' => 'php://stderr', - 'add_new_line' => ! runningInRoadRunner(), - ...$common, + ...$buildCommonConfig(! runningInRoadRunner()), ], ], diff --git a/config/roadrunner/.rr.dev.yml b/config/roadrunner/.rr.dev.yml index 24c3a2fc..594b65e3 100644 --- a/config/roadrunner/.rr.dev.yml +++ b/config/roadrunner/.rr.dev.yml @@ -30,13 +30,17 @@ jobs: prefetch: 10 logs: + encoding: console mode: development channels: http: mode: 'off' # Disable logging as Shlink handles it internally server: + encoding: console level: info metrics: + encoding: console level: debug jobs: + encoding: console level: debug diff --git a/config/roadrunner/.rr.test.yml b/config/roadrunner/.rr.test.yml index f3e8bb78..c97e5414 100644 --- a/config/roadrunner/.rr.test.yml +++ b/config/roadrunner/.rr.test.yml @@ -35,15 +35,16 @@ jobs: prefetch: 10 logs: - encoding: json + encoding: console mode: development channels: http: mode: 'off' # Disable logging as Shlink handles it internally server: - encoding: json + encoding: console level: info metrics: level: panic jobs: + encoding: console level: panic diff --git a/config/roadrunner/.rr.yml b/config/roadrunner/.rr.yml index 901291b2..328a3a60 100644 --- a/config/roadrunner/.rr.yml +++ b/config/roadrunner/.rr.yml @@ -28,11 +28,14 @@ jobs: prefetch: 10 logs: + encoding: ${LOGS_FORMAT:-console} mode: production channels: http: mode: 'off' # Disable logging as Shlink handles it internally server: + encoding: ${LOGS_FORMAT:-console} level: info jobs: + encoding: ${LOGS_FORMAT:-console} level: debug diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index c99635da..bf5acc3e 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -91,6 +91,7 @@ enum EnvVars: string case CORS_ALLOW_CREDENTIALS = 'CORS_ALLOW_CREDENTIALS'; case CORS_MAX_AGE = 'CORS_MAX_AGE'; case TRUSTED_PROXIES = 'TRUSTED_PROXIES'; + case LOGS_FORMAT = 'LOGS_FORMAT'; /** @deprecated Use REDIRECT_EXTRA_PATH */ case REDIRECT_APPEND_EXTRA_PATH = 'REDIRECT_APPEND_EXTRA_PATH'; @@ -196,6 +197,8 @@ enum EnvVars: string self::CORS_ALLOW_CREDENTIALS => false, self::CORS_MAX_AGE => 3600, + self::LOGS_FORMAT => 'console', + default => null, }; } From 2d5734fc8b236f68bee202f3a4df1de37657f52c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 24 Jul 2025 12:07:11 +0200 Subject: [PATCH 40/40] Add v4.5.0 to changelog --- CHANGELOG.md | 2 +- composer.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7673c67e..3b9a73db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## [4.5.0] - 2025-07-24 ### Added * [#2438](https://github.com/shlinkio/shlink/issues/2438) Add `MERCURE_ENABLED` env var and corresponding config option, to more easily allow the mercure integration to be toggled. diff --git a/composer.json b/composer.json index b410da3a..83eb7058 100644 --- a/composer.json +++ b/composer.json @@ -43,11 +43,11 @@ "pagerfanta/core": "^3.8", "ramsey/uuid": "^4.7", "shlinkio/doctrine-specification": "^2.2", - "shlinkio/shlink-common": "dev-main#2c9387c as 7.1", + "shlinkio/shlink-common": "^7.1", "shlinkio/shlink-config": "^4.0", "shlinkio/shlink-event-dispatcher": "^4.2", "shlinkio/shlink-importer": "^5.6", - "shlinkio/shlink-installer": "dev-develop#1c775a9 as 9.6", + "shlinkio/shlink-installer": "^9.6", "shlinkio/shlink-ip-geolocation": "^4.3", "shlinkio/shlink-json": "^1.2", "spiral/roadrunner": "^2025.1",