From a89b53af4f36d4572c78894202ef69b952819d09 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 21 Apr 2024 16:46:24 +0200 Subject: [PATCH 01/41] Link crchived changelogs from main one --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08738f515..ab6c14128 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -824,3 +824,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Fixed * *Nothing* + + +## Older versions +* [2.x.x](docs/changelog-archive/CHANGELOG-2.x.md) +* [1.x.x](docs/changelog-archive/CHANGELOG-1.x.md) From 163244f40f5e3a22a086e2ca1c6ca834b73b91e9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 21 Apr 2024 17:09:20 +0200 Subject: [PATCH 02/41] Add option to allow all URLs to be crawlable via robots.txt --- CHANGELOG.md | 17 +++++++++ composer.json | 2 +- config/autoload/installer.global.php | 1 + config/autoload/robots.global.php | 13 +++++++ module/Core/config/dependencies.config.php | 2 +- module/Core/src/Action/RobotsAction.php | 10 ++++-- module/Core/src/Config/EnvVars.php | 3 +- module/Core/test/Action/RobotsActionTest.php | 38 +++++++++++++++----- 8 files changed, 72 insertions(+), 14 deletions(-) create mode 100644 config/autoload/robots.global.php diff --git a/CHANGELOG.md b/CHANGELOG.md index ab6c14128..d2b93f655 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). +## [Unreleased] +### Added +* [#2018](https://github.com/shlinkio/shlink/issues/2018) Add option to allow all short URLs to be unconditionally crawlable in robots.txt, via `ROBOTS_ALLOW_ALL_SHORT_URLS=true` env var, or config options. + +### Changed +* *Nothing* + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* *Nothing* + + ## [4.1.0] - 2024-04-14 ### Added * [#1330](https://github.com/shlinkio/shlink/issues/1330) All visit-related endpoints now expose the `visitedUrl` prop for any visit. diff --git a/composer.json b/composer.json index 517caf0ca..c00ebee8f 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,7 @@ "shlinkio/shlink-config": "^3.0", "shlinkio/shlink-event-dispatcher": "^4.1", "shlinkio/shlink-importer": "^5.3.2", - "shlinkio/shlink-installer": "^9.1", + "shlinkio/shlink-installer": "dev-develop#11e66d8 as 9.2", "shlinkio/shlink-ip-geolocation": "^4.0", "shlinkio/shlink-json": "^1.1", "spiral/roadrunner": "^2023.3", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 4ebd67164..0fda5d06f 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -45,6 +45,7 @@ Option\UrlShortener\EnableMultiSegmentSlugsConfigOption::class, Option\UrlShortener\EnableTrailingSlashConfigOption::class, Option\UrlShortener\ShortUrlModeConfigOption::class, + Option\UrlShortener\RobotsAllowAllShortUrlsConfigOption::class, Option\Tracking\IpAnonymizationConfigOption::class, Option\Tracking\OrphanVisitsTrackingConfigOption::class, Option\Tracking\DisableTrackParamConfigOption::class, diff --git a/config/autoload/robots.global.php b/config/autoload/robots.global.php new file mode 100644 index 000000000..0ab9c5d2c --- /dev/null +++ b/config/autoload/robots.global.php @@ -0,0 +1,13 @@ + [ + 'allow-all-short-urls' => (bool) Config\EnvVars::ROBOTS_ALLOW_ALL_SHORT_URLS->loadFromEnv(false), + ], + +]; diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 5fcc8e44b..24f323606 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -189,7 +189,7 @@ 'Logger_Shlink', Options\QrCodeOptions::class, ], - Action\RobotsAction::class => [Crawling\CrawlingHelper::class], + Action\RobotsAction::class => [Crawling\CrawlingHelper::class, 'config.robots.allow-all-short-urls'], ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class => [ 'em', diff --git a/module/Core/src/Action/RobotsAction.php b/module/Core/src/Action/RobotsAction.php index 214dc7a09..cb3c99ea8 100644 --- a/module/Core/src/Action/RobotsAction.php +++ b/module/Core/src/Action/RobotsAction.php @@ -15,9 +15,9 @@ use const PHP_EOL; -class RobotsAction implements RequestHandlerInterface, StatusCodeInterface +readonly class RobotsAction implements RequestHandlerInterface, StatusCodeInterface { - public function __construct(private readonly CrawlingHelperInterface $crawlingHelper) + public function __construct(private CrawlingHelperInterface $crawlingHelper, private bool $allowAllShortUrls) { } @@ -37,6 +37,12 @@ private function buildRobots(): iterable ROBOTS; + if ($this->allowAllShortUrls) { + // Disallow rest URLs, but allow all short codes + yield 'Disallow: /rest/'; + return; + } + $shortCodes = $this->crawlingHelper->listCrawlableShortCodes(); foreach ($shortCodes as $shortCode) { yield sprintf('Allow: /%s%s', $shortCode, PHP_EOL); diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index bae68e848..59fafb17b 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -69,8 +69,9 @@ enum EnvVars: string case DEFAULT_DOMAIN = 'DEFAULT_DOMAIN'; case AUTO_RESOLVE_TITLES = 'AUTO_RESOLVE_TITLES'; case REDIRECT_APPEND_EXTRA_PATH = 'REDIRECT_APPEND_EXTRA_PATH'; - case TIMEZONE = 'TIMEZONE'; case MULTI_SEGMENT_SLUGS_ENABLED = 'MULTI_SEGMENT_SLUGS_ENABLED'; + case ROBOTS_ALLOW_ALL_SHORT_URLS = 'ROBOTS_ALLOW_ALL_SHORT_URLS'; + case TIMEZONE = 'TIMEZONE'; case MEMORY_LIMIT = 'MEMORY_LIMIT'; public function loadFromEnv(mixed $default = null): mixed diff --git a/module/Core/test/Action/RobotsActionTest.php b/module/Core/test/Action/RobotsActionTest.php index 6523c2c57..32c1b036e 100644 --- a/module/Core/test/Action/RobotsActionTest.php +++ b/module/Core/test/Action/RobotsActionTest.php @@ -14,24 +14,25 @@ class RobotsActionTest extends TestCase { - private RobotsAction $action; private MockObject & CrawlingHelperInterface $helper; protected function setUp(): void { $this->helper = $this->createMock(CrawlingHelperInterface::class); - $this->action = new RobotsAction($this->helper); } #[Test, DataProvider('provideShortCodes')] - public function buildsRobotsLinesFromCrawlableShortCodes(array $shortCodes, string $expected): void - { + public function buildsRobotsLinesFromCrawlableShortCodes( + array $shortCodes, + bool $allowAllShortUrls, + string $expected, + ): void { $this->helper - ->expects($this->once()) + ->expects($allowAllShortUrls ? $this->never() : $this->once()) ->method('listCrawlableShortCodes') ->willReturn($shortCodes); - $response = $this->action->handle(ServerRequestFactory::fromGlobals()); + $response = $this->action($allowAllShortUrls)->handle(ServerRequestFactory::fromGlobals()); self::assertEquals(200, $response->getStatusCode()); self::assertEquals($expected, $response->getBody()->__toString()); @@ -40,7 +41,7 @@ public function buildsRobotsLinesFromCrawlableShortCodes(array $shortCodes, stri public static function provideShortCodes(): iterable { - yield 'three short codes' => [['foo', 'bar', 'baz'], << [['foo', 'bar', 'baz'], false, << [['foo', 'bar', 'some', 'thing', 'baz'], << [['foo', 'bar', 'some', 'thing', 'baz'], false, << [[], << [[], false, << [['foo', 'bar', 'some'], true, << [[], true, <<helper, allowAllShortUrls: $allowAllShortUrls); } } From 9a76c19615a5bb26c67690b17f1bdca4b363497c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 26 Apr 2024 09:27:21 +0200 Subject: [PATCH 03/41] Migrate to new docker-publish-image reusable workflow --- .github/workflows/publish-docker-image.yml | 2 +- CHANGELOG.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/publish-docker-image.yml b/.github/workflows/publish-docker-image.yml index a57ebe41c..32bbcda7d 100644 --- a/.github/workflows/publish-docker-image.yml +++ b/.github/workflows/publish-docker-image.yml @@ -15,7 +15,7 @@ jobs: - runtime: 'rr' tag-suffix: 'roadrunner' platforms: 'linux/arm64/v8,linux/amd64' - uses: shlinkio/github-actions/.github/workflows/docker-build-and-publish.yml@main + uses: shlinkio/github-actions/.github/workflows/docker-publish-image.yml@main secrets: inherit with: image-name: shlinkio/shlink diff --git a/CHANGELOG.md b/CHANGELOG.md index d2b93f655..0c16da20f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#2018](https://github.com/shlinkio/shlink/issues/2018) Add option to allow all short URLs to be unconditionally crawlable in robots.txt, via `ROBOTS_ALLOW_ALL_SHORT_URLS=true` env var, or config options. ### Changed -* *Nothing* +* Use new reusable workflow to publish docker image ### Deprecated * *Nothing* From c22e3895b566be6402bfcfc31e575c8784e90685 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 29 Apr 2024 08:52:18 +0200 Subject: [PATCH 04/41] Allow more dev hosts in dev mercure --- docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 5bccfd482..026d30706 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -147,7 +147,7 @@ services: SERVER_NAME: ":80" MERCURE_PUBLISHER_JWT_KEY: mercure_jwt_key_long_enough_to_avoid_error MERCURE_SUBSCRIBER_JWT_KEY: mercure_jwt_key_long_enough_to_avoid_error - MERCURE_EXTRA_DIRECTIVES: "cors_origins https://app.shlink.io http://localhost:3000 http://127.0.0.1:3000" + MERCURE_EXTRA_DIRECTIVES: "cors_origins https://app.shlink.io http://localhost:3000 http://127.0.0.1:3000 http://localhost:3002 http://127.0.0.1:3002 http://localhost:3005 http://127.0.0.1:3005" shlink_rabbitmq: container_name: shlink_rabbitmq From 98e4d01feb72acbf25600e6b442e92e54dc17131 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 29 Apr 2024 15:18:54 +0200 Subject: [PATCH 05/41] Fix typo in OAS docs --- CHANGELOG.md | 2 +- .../v3_short-urls_{shortCode}_redirect-rules.json | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c16da20f..563e02e6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * *Nothing* ### Fixed -* *Nothing* +* [#2111](https://github.com/shlinkio/shlink/issues/2111) Fix typo in OAS docs examples where redirect rules with `query-param` condition type were defined as `query`. ## [4.1.0] - 2024-04-14 diff --git a/docs/swagger/paths/v3_short-urls_{shortCode}_redirect-rules.json b/docs/swagger/paths/v3_short-urls_{shortCode}_redirect-rules.json index b87e26cb9..ecb806936 100644 --- a/docs/swagger/paths/v3_short-urls_{shortCode}_redirect-rules.json +++ b/docs/swagger/paths/v3_short-urls_{shortCode}_redirect-rules.json @@ -77,12 +77,12 @@ "priority": 3, "conditions": [ { - "type": "query", + "type": "query-param", "matchKey": "foo", "matchValue": "bar" }, { - "type": "query", + "type": "query-param", "matchKey": "hello", "matchValue": "world" } @@ -209,12 +209,12 @@ "longUrl": "https://example.com/query-foo-bar-hello-world", "conditions": [ { - "type": "query", + "type": "query-param", "matchKey": "foo", "matchValue": "bar" }, { - "type": "query", + "type": "query-param", "matchKey": "hello", "matchValue": "world" } @@ -280,12 +280,12 @@ "priority": 3, "conditions": [ { - "type": "query", + "type": "query-param", "matchKey": "foo", "matchValue": "bar" }, { - "type": "query", + "type": "query-param", "matchKey": "hello", "matchValue": "world" } From cb3a690294b52d5a3bfca5d824c63e7c3f5fb8fb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 6 May 2024 18:50:10 +0200 Subject: [PATCH 06/41] Remove unneeded DISTINCT from list short URLs query --- module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php index 8aac9b73d..75c5c49a3 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php @@ -42,7 +42,7 @@ public function findList(ShortUrlsListFiltering $filtering): array $qb = $this->createListQueryBuilder($filtering); $qb->select( - 'DISTINCT s AS shortUrl', + 's AS shortUrl', '(' . $buildVisitsSubQuery('v', excludingBots: false) . ') AS ' . OrderableField::VISITS->value, '(' . $buildVisitsSubQuery('v2', excludingBots: true) . ') AS ' . OrderableField::NON_BOT_VISITS->value, // This is added only to have a consistent order by title between database engines From 8cd77391ccf34add41fce2883bd3c49da06752e3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 9 May 2024 09:43:55 +0200 Subject: [PATCH 07/41] Revert "Remove unneeded DISTINCT from list short URLs query" --- module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php index 75c5c49a3..8aac9b73d 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php @@ -42,7 +42,7 @@ public function findList(ShortUrlsListFiltering $filtering): array $qb = $this->createListQueryBuilder($filtering); $qb->select( - 's AS shortUrl', + 'DISTINCT s AS shortUrl', '(' . $buildVisitsSubQuery('v', excludingBots: false) . ') AS ' . OrderableField::VISITS->value, '(' . $buildVisitsSubQuery('v2', excludingBots: true) . ') AS ' . OrderableField::NON_BOT_VISITS->value, // This is added only to have a consistent order by title between database engines From 4084d301ca488a4fe6be5eaa7be8db4858407d5f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 12 May 2024 12:49:53 +0200 Subject: [PATCH 08/41] Update to PHPUnit 11 --- CHANGELOG.md | 1 + composer.json | 8 ++--- .../MatomoSendVisitsCommandTest.php | 4 +-- module/CLI/test/Util/CliTestUtils.php | 1 + .../NotFoundRedirectHandlerTest.php | 17 ++++++++--- .../RabbitMq/NotifyVisitToRabbitMqTest.php | 29 +++++++++++-------- 6 files changed, 38 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 563e02e6d..102c9a148 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Changed * Use new reusable workflow to publish docker image +* [#2015](https://github.com/shlinkio/shlink/issues/2015) Update to PHPUnit 11. ### Deprecated * *Nothing* diff --git a/composer.json b/composer.json index c00ebee8f..6b075d2bb 100644 --- a/composer.json +++ b/composer.json @@ -28,7 +28,7 @@ "guzzlehttp/guzzle": "^7.5", "jaybizzle/crawler-detect": "^1.2.116", "laminas/laminas-config": "^3.8", - "laminas/laminas-config-aggregator": "^1.13", + "laminas/laminas-config-aggregator": "^1.15", "laminas/laminas-diactoros": "^3.3", "laminas/laminas-inputfilter": "^2.27", "laminas/laminas-servicemanager": "^3.21", @@ -67,9 +67,9 @@ "phpstan/phpstan-doctrine": "^1.3", "phpstan/phpstan-phpunit": "^1.3", "phpstan/phpstan-symfony": "^1.3", - "phpunit/php-code-coverage": "^10.1", - "phpunit/phpcov": "^9.0", - "phpunit/phpunit": "^10.4", + "phpunit/php-code-coverage": "^11.0", + "phpunit/phpcov": "^10.0", + "phpunit/phpunit": "^11.1", "roave/security-advisories": "dev-master", "shlinkio/php-coding-standard": "~2.3.0", "shlinkio/shlink-test-utils": "^4.1", diff --git a/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php b/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php index e3a527332..78d2f8285 100644 --- a/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php +++ b/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php @@ -34,8 +34,8 @@ public function warningDisplayedIfIntegrationIsNotEnabled(): void } #[Test] - #[TestWith([true])] - #[TestWith([false])] + #[TestWith([true], 'interactive')] + #[TestWith([false], 'not interactive')] public function warningIsOnlyDisplayedInInteractiveMode(bool $interactive): void { $this->visitSender->method('sendVisitsInDateRange')->willReturn(new SendVisitsResult()); diff --git a/module/CLI/test/Util/CliTestUtils.php b/module/CLI/test/Util/CliTestUtils.php index 9c92f882c..c18186ba1 100644 --- a/module/CLI/test/Util/CliTestUtils.php +++ b/module/CLI/test/Util/CliTestUtils.php @@ -25,6 +25,7 @@ public static function createCommandMock(string $name): MockObject & Command $command = $generator->testDouble( Command::class, mockObject: true, + markAsMockObject: true, callOriginalConstructor: false, callOriginalClone: false, cloneArguments: false, diff --git a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php index d8a0390d1..53642f3a2 100644 --- a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php +++ b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php @@ -10,6 +10,7 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\MockObject\Rule\InvokedCount as InvokedCountMatcher; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\UriInterface; @@ -60,14 +61,19 @@ public function nextIsCalledWhenNoRedirectIsResolved(callable $setUp): void public static function provideNonRedirectScenarios(): iterable { + $exactly = static fn (int $expectedCount) => new InvokedCountMatcher($expectedCount); + $once = static fn () => $exactly(1); + yield 'no domain' => [function ( MockObject&DomainServiceInterface $domainService, MockObject&NotFoundRedirectResolverInterface $resolver, + ) use ( + $once, ): void { - $domainService->expects(self::once())->method('findByAuthority')->withAnyParameters()->willReturn( + $domainService->expects($once())->method('findByAuthority')->withAnyParameters()->willReturn( null, ); - $resolver->expects(self::once())->method('resolveRedirectResponse')->with( + $resolver->expects($once())->method('resolveRedirectResponse')->with( self::isInstanceOf(NotFoundType::class), self::isInstanceOf(NotFoundRedirectOptions::class), self::isInstanceOf(UriInterface::class), @@ -76,12 +82,15 @@ public static function provideNonRedirectScenarios(): iterable yield 'non-redirecting domain' => [function ( MockObject&DomainServiceInterface $domainService, MockObject&NotFoundRedirectResolverInterface $resolver, + ) use ( + $once, + $exactly, ): void { - $domainService->expects(self::once())->method('findByAuthority')->withAnyParameters()->willReturn( + $domainService->expects($once())->method('findByAuthority')->withAnyParameters()->willReturn( Domain::withAuthority(''), ); $callCount = 0; - $resolver->expects(self::exactly(2))->method('resolveRedirectResponse')->willReturnCallback( + $resolver->expects($exactly(2))->method('resolveRedirectResponse')->willReturnCallback( function (mixed $arg1, mixed $arg2, mixed $arg3) use (&$callCount) { Assert::assertInstanceOf(NotFoundType::class, $arg1); Assert::assertInstanceOf($callCount === 0 ? Domain::class : NotFoundRedirectOptions::class, $arg2); diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php index 7386169ff..2251d3011 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php @@ -10,6 +10,7 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\MockObject\Rule\InvokedCount as InvokedCountMatcher; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; use RuntimeException; @@ -146,30 +147,34 @@ public function expectedPayloadIsPublishedDependingOnConfig( public static function providePayloads(): iterable { + $exactly = static fn (int $expectedCount) => new InvokedCountMatcher($expectedCount); + $once = static fn () => $exactly(1); + $never = static fn () => $exactly(0); + yield 'non-orphan visit' => [ Visit::forValidShortUrl(ShortUrl::withLongUrl('https://longUrl'), Visitor::emptyInstance()), - function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator): void { + function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator) use ($once, $never): void { $update = Update::forTopicAndPayload('', []); - $updatesGenerator->expects(self::never())->method('newOrphanVisitUpdate'); - $updatesGenerator->expects(self::once())->method('newVisitUpdate')->withAnyParameters()->willReturn( + $updatesGenerator->expects($never())->method('newOrphanVisitUpdate'); + $updatesGenerator->expects($once())->method('newVisitUpdate')->withAnyParameters()->willReturn( $update, ); - $updatesGenerator->expects(self::once())->method('newShortUrlVisitUpdate')->willReturn($update); + $updatesGenerator->expects($once())->method('newShortUrlVisitUpdate')->willReturn($update); }, - function (MockObject & PublishingHelperInterface $helper): void { - $helper->expects(self::exactly(2))->method('publishUpdate')->with(self::isInstanceOf(Update::class)); + function (MockObject & PublishingHelperInterface $helper) use ($exactly): void { + $helper->expects($exactly(2))->method('publishUpdate')->with(self::isInstanceOf(Update::class)); }, ]; yield 'orphan visit' => [ Visit::forBasePath(Visitor::emptyInstance()), - function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator): void { + function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator) use ($once, $never): void { $update = Update::forTopicAndPayload('', []); - $updatesGenerator->expects(self::once())->method('newOrphanVisitUpdate')->willReturn($update); - $updatesGenerator->expects(self::never())->method('newVisitUpdate'); - $updatesGenerator->expects(self::never())->method('newShortUrlVisitUpdate'); + $updatesGenerator->expects($once())->method('newOrphanVisitUpdate')->willReturn($update); + $updatesGenerator->expects($never())->method('newVisitUpdate'); + $updatesGenerator->expects($never())->method('newShortUrlVisitUpdate'); }, - function (MockObject & PublishingHelperInterface $helper): void { - $helper->expects(self::once())->method('publishUpdate')->with(self::isInstanceOf(Update::class)); + function (MockObject & PublishingHelperInterface $helper) use ($once): void { + $helper->expects($once())->method('publishUpdate')->with(self::isInstanceOf(Update::class)); }, ]; } From 9b16d7acc06394061d94d3edd63e9623def83dfd Mon Sep 17 00:00:00 2001 From: Marijn Vandevoorde Date: Thu, 16 May 2024 14:00:39 +0200 Subject: [PATCH 09/41] Replaces short-id by nano-id --- composer.json | 2 +- module/Core/functions/functions.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index 6b075d2bb..6cd3e92dd 100644 --- a/composer.json +++ b/composer.json @@ -26,6 +26,7 @@ "friendsofphp/proxy-manager-lts": "^1.0", "geoip2/geoip2": "^3.0", "guzzlehttp/guzzle": "^7.5", + "hidehalo/nanoid-php": "^1.1", "jaybizzle/crawler-detect": "^1.2.116", "laminas/laminas-config": "^3.8", "laminas/laminas-config-aggregator": "^1.15", @@ -40,7 +41,6 @@ "mlocati/ip-lib": "^1.18", "mobiledetect/mobiledetectlib": "^4.8", "pagerfanta/core": "^3.8", - "pugx/shortid-php": "^1.1", "ramsey/uuid": "^4.7", "shlinkio/doctrine-specification": "^2.1.1", "shlinkio/shlink-common": "^6.1", diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 2e238e997..8a11209d8 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -9,11 +9,11 @@ use DateTimeInterface; use Doctrine\ORM\Mapping\Builder\FieldBuilder; use GuzzleHttp\Psr7\Query; +use Hidehalo\Nanoid\Client; use Jaybizzle\CrawlerDetect\CrawlerDetect; use Laminas\Filter\Word\CamelCaseToSeparator; use Laminas\Filter\Word\CamelCaseToUnderscore; use Laminas\InputFilter\InputFilter; -use PUGX\Shortid\Factory as ShortIdFactory; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; @@ -39,13 +39,13 @@ function generateRandomShortCode(int $length, ShortUrlMode $mode = ShortUrlMode: { static $shortIdFactory; if ($shortIdFactory === null) { - $shortIdFactory = new ShortIdFactory(); + $shortIdFactory = new Client(); } $alphabet = $mode === ShortUrlMode::STRICT ? '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ' : '0123456789abcdefghijklmnopqrstuvwxyz'; - return $shortIdFactory->generate($length, $alphabet)->serialize(); + return $shortIdFactory->formattedId($alphabet, $length); } function parseDateFromQuery(array $query, string $dateName): ?Chronos From 89b73a9cfa13cc0c70065e86aa5529219db074ab Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 21 May 2024 18:09:45 +0200 Subject: [PATCH 10/41] Update to latest phpstan --- CHANGELOG.md | 1 + composer.json | 8 ++++---- docker-compose.yml | 2 +- module/Core/functions/functions.php | 10 +++++----- module/Core/test/Matomo/MatomoTrackerBuilderTest.php | 4 ++-- phpstan.neon | 4 ++-- 6 files changed, 15 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 102c9a148..8ab0a09a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Changed * Use new reusable workflow to publish docker image * [#2015](https://github.com/shlinkio/shlink/issues/2015) Update to PHPUnit 11. +* [#2130](https://github.com/shlinkio/shlink/pull/2130) Replace deprecated `pugx/shortid-php` package with `hidehalo/nanoid-php`. ### Deprecated * *Nothing* diff --git a/composer.json b/composer.json index 6cd3e92dd..82a4a93a0 100644 --- a/composer.json +++ b/composer.json @@ -63,10 +63,10 @@ "require-dev": { "devizzent/cebe-php-openapi": "^1.0.1", "devster/ubench": "^2.1", - "phpstan/phpstan": "^1.10", - "phpstan/phpstan-doctrine": "^1.3", - "phpstan/phpstan-phpunit": "^1.3", - "phpstan/phpstan-symfony": "^1.3", + "phpstan/phpstan": "^1.11", + "phpstan/phpstan-doctrine": "^1.4", + "phpstan/phpstan-phpunit": "^1.4", + "phpstan/phpstan-symfony": "^1.4", "phpunit/php-code-coverage": "^11.0", "phpunit/phpcov": "^10.0", "phpunit/phpunit": "^11.1", diff --git a/docker-compose.yml b/docker-compose.yml index 026d30706..918be7cab 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -105,7 +105,7 @@ services: shlink_db_ms: container_name: shlink_db_ms - image: mcr.microsoft.com/mssql/server:2019-latest + image: mcr.microsoft.com/mssql/server:2022-latest ports: - "1433:1433" environment: diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 8a11209d8..d9fc2b5ee 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -9,7 +9,7 @@ use DateTimeInterface; use Doctrine\ORM\Mapping\Builder\FieldBuilder; use GuzzleHttp\Psr7\Query; -use Hidehalo\Nanoid\Client; +use Hidehalo\Nanoid\Client as NanoidClient; use Jaybizzle\CrawlerDetect\CrawlerDetect; use Laminas\Filter\Word\CamelCaseToSeparator; use Laminas\Filter\Word\CamelCaseToUnderscore; @@ -37,15 +37,15 @@ function generateRandomShortCode(int $length, ShortUrlMode $mode = ShortUrlMode::STRICT): string { - static $shortIdFactory; - if ($shortIdFactory === null) { - $shortIdFactory = new Client(); + static $nanoIdClient; + if ($nanoIdClient === null) { + $nanoIdClient = new NanoidClient(); } $alphabet = $mode === ShortUrlMode::STRICT ? '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ' : '0123456789abcdefghijklmnopqrstuvwxyz'; - return $shortIdFactory->formattedId($alphabet, $length); + return $nanoIdClient->formattedId($alphabet, $length); } function parseDateFromQuery(array $query, string $dateName): ?Chronos diff --git a/module/Core/test/Matomo/MatomoTrackerBuilderTest.php b/module/Core/test/Matomo/MatomoTrackerBuilderTest.php index 5a4e6ab0f..1b55405ee 100644 --- a/module/Core/test/Matomo/MatomoTrackerBuilderTest.php +++ b/module/Core/test/Matomo/MatomoTrackerBuilderTest.php @@ -37,8 +37,8 @@ public function trackerIsCreated(): void { $tracker = $this->builder()->buildMatomoTracker(); - self::assertEquals('api_token', $tracker->token_auth); // @phpstan-ignore-line - self::assertEquals(5, $tracker->idSite); // @phpstan-ignore-line + self::assertEquals('api_token', $tracker->token_auth); + self::assertEquals(5, $tracker->idSite); self::assertEquals(MatomoTrackerBuilder::MATOMO_DEFAULT_TIMEOUT, $tracker->getRequestTimeout()); self::assertEquals(MatomoTrackerBuilder::MATOMO_DEFAULT_TIMEOUT, $tracker->getRequestConnectTimeout()); } diff --git a/phpstan.neon b/phpstan.neon index eee4853b3..9ebaec6e0 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4,8 +4,6 @@ includes: - vendor/phpstan/phpstan-phpunit/extension.neon - vendor/phpstan/phpstan-phpunit/rules.neon parameters: - checkMissingIterableValueType: false - checkGenericClassInNonGenericObjectType: false symfony: console_application_loader: 'config/cli-app.php' doctrine: @@ -14,3 +12,5 @@ parameters: ignoreErrors: - '#should return int<0, max> but returns int#' - '#expects -1|int<1, max>, int given#' + - identifier: missingType.generics + - identifier: missingType.iterableValue From 80e9c2452b34555537a1ac7c66469a9dc01d71b2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 22 May 2024 18:09:40 +0200 Subject: [PATCH 11/41] Convert encoding of resolved titles based on page encoding --- CHANGELOG.md | 1 + composer.json | 1 + config/constants.php | 1 - .../Helper/ShortUrlTitleResolutionHelper.php | 29 +++++++++++++------ .../ShortUrlTitleResolutionHelperTest.php | 11 ++++--- 5 files changed, 29 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ab0a09a9..1257e3d20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Fixed * [#2111](https://github.com/shlinkio/shlink/issues/2111) Fix typo in OAS docs examples where redirect rules with `query-param` condition type were defined as `query`. +* [#2129](https://github.com/shlinkio/shlink/issues/2129) Fix error when resolving title for sites not using UTF-8 charset (detected with Japanese charsets). ## [4.1.0] - 2024-04-14 diff --git a/composer.json b/composer.json index 82a4a93a0..c94cbdfa1 100644 --- a/composer.json +++ b/composer.json @@ -16,6 +16,7 @@ "ext-curl": "*", "ext-gd": "*", "ext-json": "*", + "ext-mbstring": "*", "ext-pdo": "*", "akrabat/ip-address-middleware": "^2.1", "cakephp/chronos": "^3.0.2", diff --git a/config/constants.php b/config/constants.php index 51ee04762..20c64f192 100644 --- a/config/constants.php +++ b/config/constants.php @@ -12,7 +12,6 @@ const DEFAULT_REDIRECT_STATUS_CODE = RedirectStatus::STATUS_302; const DEFAULT_REDIRECT_CACHE_LIFETIME = 30; const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory'; -const TITLE_TAG_VALUE = '/]*>(.*?)<\/title>/i'; // Matches the value inside a html title tag const LOOSE_URI_MATCHER = '/(.+)\:(.+)/i'; // Matches anything starting with a schema. const DEFAULT_QR_CODE_SIZE = 300; const DEFAULT_QR_CODE_MARGIN = 0; diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php index e91b1ff11..3f9b62256 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php @@ -12,20 +12,24 @@ use Throwable; use function html_entity_decode; +use function mb_convert_encoding; use function preg_match; use function str_contains; use function str_starts_with; use function strtolower; use function trim; -use const Shlinkio\Shlink\TITLE_TAG_VALUE; - readonly class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionHelperInterface { public const MAX_REDIRECTS = 15; public const CHROME_USER_AGENT = 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) ' . 'Chrome/121.0.0.0 Safari/537.36'; + // Matches the value inside a html title tag + private const TITLE_TAG_VALUE = '/]*>(.*?)<\/title>/i'; + // Matches the charset inside a Content-Type header + private const CHARSET_VALUE = '/charset=([^;]+)/i'; + public function __construct( private ClientInterface $httpClient, private UrlShortenerOptions $options, @@ -53,7 +57,7 @@ public function processTitle(TitleResolutionModelInterface $data): TitleResoluti return $data; } - $title = $this->tryToResolveTitle($response); + $title = $this->tryToResolveTitle($response, $contentType); return $title !== null ? $data->withResolvedTitle($title) : $data; } @@ -76,7 +80,7 @@ private function fetchUrl(string $url): ?ResponseInterface } } - private function tryToResolveTitle(ResponseInterface $response): ?string + private function tryToResolveTitle(ResponseInterface $response, string $contentType): ?string { $collectedBody = ''; $body = $response->getBody(); @@ -84,12 +88,19 @@ private function tryToResolveTitle(ResponseInterface $response): ?string while (! str_contains($collectedBody, '') && ! $body->eof()) { $collectedBody .= $body->read(1024); } - preg_match(TITLE_TAG_VALUE, $collectedBody, $matches); - return isset($matches[1]) ? $this->normalizeTitle($matches[1]) : null; - } - private function normalizeTitle(string $title): string - { + // Try to match the title from the tag + preg_match(self::TITLE_TAG_VALUE, $collectedBody, $titleMatches); + if (! isset($titleMatches[1])) { + return null; + } + + // Get the page's charset from Content-Type header + preg_match(self::CHARSET_VALUE, $contentType, $charsetMatches); + + $title = isset($charsetMatches[1]) + ? mb_convert_encoding($titleMatches[1], 'utf8', $charsetMatches[1]) + : $titleMatches[1]; return html_entity_decode(trim($title)); } } diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php index 06b47f8c3..92fac8eb2 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php @@ -12,6 +12,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\MockObject; use PHPUnit\Framework\TestCase; @@ -89,10 +90,12 @@ public function dataIsReturnedAsIsWhenTitleCannotBeResolvedFromResponse(): void } #[Test] - public function titleIsUpdatedWhenItCanBeResolvedFromResponse(): void + #[TestWith(['TEXT/html; charset=utf-8'], name: 'charset')] + #[TestWith(['TEXT/html'], name: 'no charset')] + public function titleIsUpdatedWhenItCanBeResolvedFromResponse(string $contentType): void { $data = ShortUrlCreation::fromRawData(['longUrl' => self::LONG_URL]); - $this->expectRequestToBeCalled()->willReturn($this->respWithTitle()); + $this->expectRequestToBeCalled()->willReturn($this->respWithTitle($contentType)); $result = $this->helper(autoResolveTitles: true)->processTitle($data); @@ -122,10 +125,10 @@ private function respWithoutTitle(): Response return new Response($body, 200, ['Content-Type' => 'text/html']); } - private function respWithTitle(): Response + private function respWithTitle(string $contentType): Response { $body = $this->createStreamWithContent('<title data-foo="bar"> Resolved "title" '); - return new Response($body, 200, ['Content-Type' => 'TEXT/html; charset=utf-8']); + return new Response($body, 200, ['Content-Type' => $contentType]); } private function createStreamWithContent(string $content): Stream From 80bdeb280afdea9453541d7b51c719c8668fc875 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 3 Jul 2024 10:09:00 +0200 Subject: [PATCH 12/41] Update to RoadRunner 2024 --- CHANGELOG.md | 2 +- composer.json | 6 +++--- docker-compose.ci.yml | 2 -- docker-compose.override.yml.dist | 2 -- docker-compose.yml | 2 -- 5 files changed, 4 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70d3930af..c70ff1022 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#2018](https://github.com/shlinkio/shlink/issues/2018) Add option to allow all short URLs to be unconditionally crawlable in robots.txt, via `ROBOTS_ALLOW_ALL_SHORT_URLS=true` env var, or config options. ### Changed -* *Nothing* +* [#2096](https://github.com/shlinkio/shlink/issues/2096) Update to RoadRunner 2024. ### Deprecated * *Nothing* diff --git a/composer.json b/composer.json index c94cbdfa1..87b1c1d22 100644 --- a/composer.json +++ b/composer.json @@ -51,10 +51,10 @@ "shlinkio/shlink-installer": "dev-develop#11e66d8 as 9.2", "shlinkio/shlink-ip-geolocation": "^4.0", "shlinkio/shlink-json": "^1.1", - "spiral/roadrunner": "^2023.3", + "spiral/roadrunner": "^2024.1", "spiral/roadrunner-cli": "^2.6", - "spiral/roadrunner-http": "^3.3", - "spiral/roadrunner-jobs": "^4.3", + "spiral/roadrunner-http": "^3.5", + "spiral/roadrunner-jobs": "^4.5", "symfony/console": "^7.0", "symfony/filesystem": "^7.0", "symfony/lock": "^7.0", diff --git a/docker-compose.ci.yml b/docker-compose.ci.yml index f4235dcbf..6bc053c33 100644 --- a/docker-compose.ci.yml +++ b/docker-compose.ci.yml @@ -1,5 +1,3 @@ -version: '3' - services: shlink_db_mysql: environment: diff --git a/docker-compose.override.yml.dist b/docker-compose.override.yml.dist index a3af35464..85e58554a 100644 --- a/docker-compose.override.yml.dist +++ b/docker-compose.override.yml.dist @@ -1,5 +1,3 @@ -version: '3' - services: shlink_php: user: 1000:1000 diff --git a/docker-compose.yml b/docker-compose.yml index 918be7cab..d1673b517 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,5 +1,3 @@ -version: '3' - services: shlink_nginx: container_name: shlink_nginx From c4f8da5f023fa2367afbe8e4cfbe1e962b33bb08 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 3 Jul 2024 19:53:26 +0200 Subject: [PATCH 13/41] Fix phpstan error definition --- phpstan.neon | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan.neon b/phpstan.neon index 9ebaec6e0..691b951a3 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -11,6 +11,6 @@ parameters: objectManagerLoader: 'config/entity-manager.php' ignoreErrors: - '#should return int<0, max> but returns int#' - - '#expects -1|int<1, max>, int given#' + - '#expects -1\|int<1, max>, int given#' - identifier: missingType.generics - identifier: missingType.iterableValue From 4b52c92e97b327a72f16edcdd448c8d75b10c787 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 5 Jul 2024 08:52:41 +0200 Subject: [PATCH 14/41] Add option to customize user agents in robots.txt --- CHANGELOG.md | 3 +- config/autoload/robots.global.php | 1 + config/autoload/tracking.global.php | 49 +++++++++----------- module/Core/config/dependencies.config.php | 3 +- module/Core/functions/functions.php | 13 ++++++ module/Core/src/Action/RobotsAction.php | 11 +++-- module/Core/src/Config/EnvVars.php | 1 + module/Core/src/Options/RobotsOptions.php | 22 +++++++++ module/Core/test/Action/RobotsActionTest.php | 37 ++++++++++----- 9 files changed, 96 insertions(+), 44 deletions(-) create mode 100644 module/Core/src/Options/RobotsOptions.php diff --git a/CHANGELOG.md b/CHANGELOG.md index c70ff1022..5ff88fcf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added -* [#2018](https://github.com/shlinkio/shlink/issues/2018) Add option to allow all short URLs to be unconditionally crawlable in robots.txt, via `ROBOTS_ALLOW_ALL_SHORT_URLS=true` env var, or config options. +* [#2018](https://github.com/shlinkio/shlink/issues/2018) Add option to allow all short URLs to be unconditionally crawlable in robots.txt, via `ROBOTS_ALLOW_ALL_SHORT_URLS=true` env var, or config option. +* [#2109](https://github.com/shlinkio/shlink/issues/2109) Add option to customize user agents robots.txt, via `ROBOTS_USER_AGENTS=foo,bar,baz` env var, or config option. ### Changed * [#2096](https://github.com/shlinkio/shlink/issues/2096) Update to RoadRunner 2024. diff --git a/config/autoload/robots.global.php b/config/autoload/robots.global.php index 0ab9c5d2c..8954dc532 100644 --- a/config/autoload/robots.global.php +++ b/config/autoload/robots.global.php @@ -8,6 +8,7 @@ 'robots' => [ 'allow-all-short-urls' => (bool) Config\EnvVars::ROBOTS_ALLOW_ALL_SHORT_URLS->loadFromEnv(false), + 'user-agents' => splitByComma(Config\EnvVars::ROBOTS_USER_AGENTS->loadFromEnv()), ], ]; diff --git a/config/autoload/tracking.global.php b/config/autoload/tracking.global.php index 4d7a6e9ab..267bb76d3 100644 --- a/config/autoload/tracking.global.php +++ b/config/autoload/tracking.global.php @@ -4,40 +4,35 @@ use Shlinkio\Shlink\Core\Config\EnvVars; -return (static function (): array { - /** @var string|null $disableTrackingFrom */ - $disableTrackingFrom = EnvVars::DISABLE_TRACKING_FROM->loadFromEnv(); +use function Shlinkio\Shlink\Core\splitByComma; - return [ +return [ - 'tracking' => [ - // Tells if IP addresses should be anonymized before persisting, to fulfil data protection regulations - // This applies only if IP address tracking is enabled - 'anonymize_remote_addr' => (bool) EnvVars::ANONYMIZE_REMOTE_ADDR->loadFromEnv(true), + 'tracking' => [ + // Tells if IP addresses should be anonymized before persisting, to fulfil data protection regulations + // This applies only if IP address tracking is enabled + 'anonymize_remote_addr' => (bool) EnvVars::ANONYMIZE_REMOTE_ADDR->loadFromEnv(true), - // Tells if visits to not-found URLs should be tracked. The disable_tracking option takes precedence - 'track_orphan_visits' => (bool) EnvVars::TRACK_ORPHAN_VISITS->loadFromEnv(true), + // Tells if visits to not-found URLs should be tracked. The disable_tracking option takes precedence + 'track_orphan_visits' => (bool) EnvVars::TRACK_ORPHAN_VISITS->loadFromEnv(true), - // A query param that, if provided, will disable tracking of one particular visit. Always takes precedence - 'disable_track_param' => EnvVars::DISABLE_TRACK_PARAM->loadFromEnv(), + // A query param that, if provided, will disable tracking of one particular visit. Always takes precedence + 'disable_track_param' => EnvVars::DISABLE_TRACK_PARAM->loadFromEnv(), - // If true, visits will not be tracked at all - 'disable_tracking' => (bool) EnvVars::DISABLE_TRACKING->loadFromEnv(false), + // If true, visits will not be tracked at all + 'disable_tracking' => (bool) EnvVars::DISABLE_TRACKING->loadFromEnv(false), - // If true, visits will be tracked, but neither the IP address, nor the location will be resolved - 'disable_ip_tracking' => (bool) EnvVars::DISABLE_IP_TRACKING->loadFromEnv(false), + // If true, visits will be tracked, but neither the IP address, nor the location will be resolved + 'disable_ip_tracking' => (bool) EnvVars::DISABLE_IP_TRACKING->loadFromEnv(false), - // If true, the referrer will not be tracked - 'disable_referrer_tracking' => (bool) EnvVars::DISABLE_REFERRER_TRACKING->loadFromEnv(false), + // If true, the referrer will not be tracked + 'disable_referrer_tracking' => (bool) EnvVars::DISABLE_REFERRER_TRACKING->loadFromEnv(false), - // If true, the user agent will not be tracked - 'disable_ua_tracking' => (bool) EnvVars::DISABLE_UA_TRACKING->loadFromEnv(false), + // If true, the user agent will not be tracked + 'disable_ua_tracking' => (bool) EnvVars::DISABLE_UA_TRACKING->loadFromEnv(false), - // A list of IP addresses, patterns or CIDR blocks from which tracking is disabled by default - 'disable_tracking_from' => $disableTrackingFrom === null - ? [] - : array_map(trim(...), explode(',', $disableTrackingFrom)), - ], + // A list of IP addresses, patterns or CIDR blocks from which tracking is disabled by default + 'disable_tracking_from' => splitByComma(EnvVars::DISABLE_TRACKING_FROM->loadFromEnv()), + ], - ]; -})(); +]; diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 24f323606..8f8b609ba 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -32,6 +32,7 @@ Options\TrackingOptions::class => [ValinorConfigFactory::class, 'config.tracking'], Options\QrCodeOptions::class => [ValinorConfigFactory::class, 'config.qr_codes'], Options\RabbitMqOptions::class => [ValinorConfigFactory::class, 'config.rabbitmq'], + Options\RobotsOptions::class => [ValinorConfigFactory::class, 'config.robots'], RedirectRule\ShortUrlRedirectRuleService::class => ConfigAbstractFactory::class, RedirectRule\ShortUrlRedirectionResolver::class => ConfigAbstractFactory::class, @@ -189,7 +190,7 @@ 'Logger_Shlink', Options\QrCodeOptions::class, ], - Action\RobotsAction::class => [Crawling\CrawlingHelper::class, 'config.robots.allow-all-short-urls'], + Action\RobotsAction::class => [Crawling\CrawlingHelper::class, Options\RobotsOptions::class], ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class => [ 'em', diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index d9fc2b5ee..5900e8e1d 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -260,3 +260,16 @@ function enumToString(string $enum): string { return sprintf('["%s"]', implode('", "', enumValues($enum))); } + +/** + * Split provided string by comma and return a list of the results. + * An empty array is returned if provided value is empty + */ +function splitByComma(?string $value): array +{ + if ($value === null || trim($value) === '') { + return []; + } + + return array_map(trim(...), explode(',', $value)); +} diff --git a/module/Core/src/Action/RobotsAction.php b/module/Core/src/Action/RobotsAction.php index cb3c99ea8..29c2c8d28 100644 --- a/module/Core/src/Action/RobotsAction.php +++ b/module/Core/src/Action/RobotsAction.php @@ -10,6 +10,7 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Crawling\CrawlingHelperInterface; +use Shlinkio\Shlink\Core\Options\RobotsOptions; use function sprintf; @@ -17,7 +18,7 @@ readonly class RobotsAction implements RequestHandlerInterface, StatusCodeInterface { - public function __construct(private CrawlingHelperInterface $crawlingHelper, private bool $allowAllShortUrls) + public function __construct(private CrawlingHelperInterface $crawlingHelper, private RobotsOptions $robotsOptions) { } @@ -33,11 +34,15 @@ private function buildRobots(): iterable # For more information about the robots.txt standard, see: # https://www.robotstxt.org/orig.html - User-agent: * ROBOTS; - if ($this->allowAllShortUrls) { + $userAgents = $this->robotsOptions->hasUserAgents() ? $this->robotsOptions->userAgents : ['*']; + foreach ($userAgents as $userAgent) { + yield sprintf('User-agent: %s%s', $userAgent, PHP_EOL); + } + + if ($this->robotsOptions->allowAllShortUrls) { // Disallow rest URLs, but allow all short codes yield 'Disallow: /rest/'; return; diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 59fafb17b..014369675 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -71,6 +71,7 @@ enum EnvVars: string case REDIRECT_APPEND_EXTRA_PATH = 'REDIRECT_APPEND_EXTRA_PATH'; case MULTI_SEGMENT_SLUGS_ENABLED = 'MULTI_SEGMENT_SLUGS_ENABLED'; case ROBOTS_ALLOW_ALL_SHORT_URLS = 'ROBOTS_ALLOW_ALL_SHORT_URLS'; + case ROBOTS_USER_AGENTS = 'ROBOTS_USER_AGENTS'; case TIMEZONE = 'TIMEZONE'; case MEMORY_LIMIT = 'MEMORY_LIMIT'; diff --git a/module/Core/src/Options/RobotsOptions.php b/module/Core/src/Options/RobotsOptions.php new file mode 100644 index 000000000..860c16031 --- /dev/null +++ b/module/Core/src/Options/RobotsOptions.php @@ -0,0 +1,22 @@ +userAgents) > 0; + } +} diff --git a/module/Core/test/Action/RobotsActionTest.php b/module/Core/test/Action/RobotsActionTest.php index 32c1b036e..1d83fb8cc 100644 --- a/module/Core/test/Action/RobotsActionTest.php +++ b/module/Core/test/Action/RobotsActionTest.php @@ -11,6 +11,7 @@ use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Action\RobotsAction; use Shlinkio\Shlink\Core\Crawling\CrawlingHelperInterface; +use Shlinkio\Shlink\Core\Options\RobotsOptions; class RobotsActionTest extends TestCase { @@ -24,15 +25,15 @@ protected function setUp(): void #[Test, DataProvider('provideShortCodes')] public function buildsRobotsLinesFromCrawlableShortCodes( array $shortCodes, - bool $allowAllShortUrls, + RobotsOptions $options, string $expected, ): void { $this->helper - ->expects($allowAllShortUrls ? $this->never() : $this->once()) + ->expects($options->allowAllShortUrls ? $this->never() : $this->once()) ->method('listCrawlableShortCodes') ->willReturn($shortCodes); - $response = $this->action($allowAllShortUrls)->handle(ServerRequestFactory::fromGlobals()); + $response = $this->action($options)->handle(ServerRequestFactory::fromGlobals()); self::assertEquals(200, $response->getStatusCode()); self::assertEquals($expected, $response->getBody()->__toString()); @@ -41,7 +42,7 @@ public function buildsRobotsLinesFromCrawlableShortCodes( public static function provideShortCodes(): iterable { - yield 'three short codes' => [['foo', 'bar', 'baz'], false, << [['foo', 'bar', 'baz'], new RobotsOptions(), << [['foo', 'bar', 'some', 'thing', 'baz'], false, << [['foo', 'bar', 'some', 'thing', 'baz'], new RobotsOptions(), << [[], false, << [[], new RobotsOptions(), << [['foo', 'bar', 'some'], true, << [ + ['foo', 'bar', 'some'], + new RobotsOptions(allowAllShortUrls: true), + << [[], new RobotsOptions(allowAllShortUrls: true), << [[], true, << [[], new RobotsOptions(userAgents: ['foo', 'bar']), <<helper, allowAllShortUrls: $allowAllShortUrls); + return new RobotsAction($this->helper, $options); } } From e4f66b7ce60cd729e2e62b5146c83c1f7b06de3f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 5 Jul 2024 09:41:26 +0200 Subject: [PATCH 15/41] Update installer --- composer.json | 2 +- config/autoload/installer.global.php | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 87b1c1d22..f792a468d 100644 --- a/composer.json +++ b/composer.json @@ -48,7 +48,7 @@ "shlinkio/shlink-config": "^3.0", "shlinkio/shlink-event-dispatcher": "^4.1", "shlinkio/shlink-importer": "^5.3.2", - "shlinkio/shlink-installer": "dev-develop#11e66d8 as 9.2", + "shlinkio/shlink-installer": "dev-develop#ccda72e as 9.2", "shlinkio/shlink-ip-geolocation": "^4.0", "shlinkio/shlink-json": "^1.1", "spiral/roadrunner": "^2024.1", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 0fda5d06f..898e5ef54 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -45,7 +45,8 @@ Option\UrlShortener\EnableMultiSegmentSlugsConfigOption::class, Option\UrlShortener\EnableTrailingSlashConfigOption::class, Option\UrlShortener\ShortUrlModeConfigOption::class, - Option\UrlShortener\RobotsAllowAllShortUrlsConfigOption::class, + Option\Robots\RobotsAllowAllShortUrlsConfigOption::class, + Option\Robots\RobotsUserAgentsConfigOption::class, Option\Tracking\IpAnonymizationConfigOption::class, Option\Tracking\OrphanVisitsTrackingConfigOption::class, Option\Tracking\DisableTrackParamConfigOption::class, From 8d90661d0a7e0b5d7bb91e1b48a477500151ca53 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Jul 2024 10:12:05 +0200 Subject: [PATCH 16/41] Extract logic to match IP address against list of groups --- .../Exception/InvalidIpFormatException.php | 15 +++++ module/Core/src/Util/IpAddressUtils.php | 65 +++++++++++++++++++ module/Core/src/Visit/RequestTracker.php | 51 +++------------ module/Core/test/Visit/RequestTrackerTest.php | 15 +++++ 4 files changed, 103 insertions(+), 43 deletions(-) create mode 100644 module/Core/src/Exception/InvalidIpFormatException.php create mode 100644 module/Core/src/Util/IpAddressUtils.php diff --git a/module/Core/src/Exception/InvalidIpFormatException.php b/module/Core/src/Exception/InvalidIpFormatException.php new file mode 100644 index 000000000..df9d207b1 --- /dev/null +++ b/module/Core/src/Exception/InvalidIpFormatException.php @@ -0,0 +1,15 @@ + strict equality with provided IP address. + * * CIDR block -> provided IP address is part of that block. + * * Wildcard -> static parts match the corresponding ones in provided IP address. + * + * @param string[] $groups + * @throws InvalidIpFormatException + */ + public static function ipAddressMatchesGroups(string $ipAddress, array $groups): bool + { + $ip = IPv4::parseString($ipAddress); + if ($ip === null) { + throw InvalidIpFormatException::fromInvalidIp($ipAddress); + } + + $ipAddressParts = explode('.', $ipAddress); + + return some($groups, function (string $value) use ($ip, $ipAddressParts): bool { + $range = str_contains($value, '*') + ? self::parseValueWithWildcards($value, $ipAddressParts) + : Factory::parseRangeString($value); + + return $range !== null && $ip->matches($range); + }); + } + + private static function parseValueWithWildcards(string $value, array $ipAddressParts): ?RangeInterface + { + $octets = explode('.', $value); + $keys = array_keys($octets); + + // Replace wildcard parts with the corresponding ones from the remote address + return Factory::parseRangeString( + implode('.', array_map( + fn (string $part, int $index) => $part === '*' ? $ipAddressParts[$index] : $part, + $octets, + $keys, + )), + ); + } +} diff --git a/module/Core/src/Visit/RequestTracker.php b/module/Core/src/Visit/RequestTracker.php index 1a6b04f99..ecc3d94f0 100644 --- a/module/Core/src/Visit/RequestTracker.php +++ b/module/Core/src/Visit/RequestTracker.php @@ -5,30 +5,20 @@ namespace Shlinkio\Shlink\Core\Visit; use Fig\Http\Message\RequestMethodInterface; -use IPLib\Address\IPv4; -use IPLib\Factory; -use IPLib\Range\RangeInterface; use Mezzio\Router\Middleware\ImplicitHeadMiddleware; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; +use Shlinkio\Shlink\Core\Exception\InvalidIpFormatException; use Shlinkio\Shlink\Core\Options\TrackingOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Util\IpAddressUtils; use Shlinkio\Shlink\Core\Visit\Model\Visitor; -use function array_keys; -use function array_map; -use function explode; -use function implode; -use function Shlinkio\Shlink\Core\ArrayUtils\some; -use function str_contains; - -class RequestTracker implements RequestTrackerInterface, RequestMethodInterface +readonly class RequestTracker implements RequestTrackerInterface, RequestMethodInterface { - public function __construct( - private readonly VisitsTrackerInterface $visitsTracker, - private readonly TrackingOptions $trackingOptions, - ) { + public function __construct(private VisitsTrackerInterface $visitsTracker, private TrackingOptions $trackingOptions) + { } public function trackIfApplicable(ShortUrl $shortUrl, ServerRequestInterface $request): void @@ -78,35 +68,10 @@ private function shouldDisableTrackingFromAddress(?string $remoteAddr): bool return false; } - $ip = IPv4::parseString($remoteAddr); - if ($ip === null) { + try { + return IpAddressUtils::ipAddressMatchesGroups($remoteAddr, $this->trackingOptions->disableTrackingFrom); + } catch (InvalidIpFormatException) { return false; } - - $remoteAddrParts = explode('.', $remoteAddr); - $disableTrackingFrom = $this->trackingOptions->disableTrackingFrom; - - return some($disableTrackingFrom, function (string $value) use ($ip, $remoteAddrParts): bool { - $range = str_contains($value, '*') - ? $this->parseValueWithWildcards($value, $remoteAddrParts) - : Factory::parseRangeString($value); - - return $range !== null && $ip->matches($range); - }); - } - - private function parseValueWithWildcards(string $value, array $remoteAddrParts): ?RangeInterface - { - $octets = explode('.', $value); - $keys = array_keys($octets); - - // Replace wildcard parts with the corresponding ones from the remote address - return Factory::parseRangeString( - implode('.', array_map( - fn (string $part, int $index) => $part === '*' ? $remoteAddrParts[$index] : $part, - $octets, - $keys, - )), - ); } } diff --git a/module/Core/test/Visit/RequestTrackerTest.php b/module/Core/test/Visit/RequestTrackerTest.php index fdf7e4933..c8746f91c 100644 --- a/module/Core/test/Visit/RequestTrackerTest.php +++ b/module/Core/test/Visit/RequestTrackerTest.php @@ -92,6 +92,21 @@ public function trackingHappensOverShortUrlsWhenRequestMeetsConditions(): void $this->requestTracker->trackIfApplicable($shortUrl, $this->request); } + #[Test] + public function trackingHappensOverShortUrlsWhenRemoteAddressIsInvalid(): void + { + $shortUrl = ShortUrl::withLongUrl(self::LONG_URL); + $this->visitsTracker->expects($this->once())->method('track')->with( + $shortUrl, + $this->isInstanceOf(Visitor::class), + ); + + $this->requestTracker->trackIfApplicable($shortUrl, ServerRequestFactory::fromGlobals()->withAttribute( + IpAddressMiddlewareFactory::REQUEST_ATTR, + 'invalid', + )); + } + #[Test] public function baseUrlErrorIsTracked(): void { From 1312ea61f4ecc260e48a0fb8b3f0cc4a2df3901c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Jul 2024 10:35:33 +0200 Subject: [PATCH 17/41] Add new IP address redirect condition --- .../RedirectRule/Entity/RedirectCondition.php | 19 +++++++++++++++++++ .../Model/RedirectConditionType.php | 1 + 2 files changed, 20 insertions(+) diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index 5e6df4946..3032eb1af 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -5,10 +5,12 @@ use JsonSerializable; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Entity\AbstractEntity; +use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; use Shlinkio\Shlink\Core\RedirectRule\Model\Validation\RedirectRulesInputFilter; +use Shlinkio\Shlink\Core\Util\IpAddressUtils; use function Shlinkio\Shlink\Core\acceptLanguageToLocales; use function Shlinkio\Shlink\Core\ArrayUtils\some; use function Shlinkio\Shlink\Core\normalizeLocale; @@ -41,6 +43,15 @@ public static function forDevice(DeviceType $device): self return new self(RedirectConditionType::DEVICE, $device->value); } + /** + * @param string $ipAddressPattern - A static IP address (100.200.80.40), CIDR block (192.168.10.0/24) or wildcard + * pattern (11.22.*.*) + */ + public static function forIpAddress(string $ipAddressPattern): self + { + return new self(RedirectConditionType::IP_ADDRESS, $ipAddressPattern); + } + public static function fromRawData(array $rawData): self { $type = RedirectConditionType::from($rawData[RedirectRulesInputFilter::CONDITION_TYPE]); @@ -59,6 +70,7 @@ public function matchesRequest(ServerRequestInterface $request): bool RedirectConditionType::QUERY_PARAM => $this->matchesQueryParam($request), RedirectConditionType::LANGUAGE => $this->matchesLanguage($request), RedirectConditionType::DEVICE => $this->matchesDevice($request), + RedirectConditionType::IP_ADDRESS => $this->matchesRemoteIpAddress($request), }; } @@ -100,6 +112,12 @@ private function matchesDevice(ServerRequestInterface $request): bool return $device !== null && $device->value === strtolower($this->matchValue); } + private function matchesRemoteIpAddress(ServerRequestInterface $request): bool + { + $remoteAddress = $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR); + return $remoteAddress !== null && IpAddressUtils::ipAddressMatchesGroups($remoteAddress, [$this->matchValue]); + } + public function jsonSerialize(): array { return [ @@ -119,6 +137,7 @@ public function toHumanFriendly(): string $this->matchKey, $this->matchValue, ), + RedirectConditionType::IP_ADDRESS => sprintf('IP address matches %s', $this->matchValue), }; } } diff --git a/module/Core/src/RedirectRule/Model/RedirectConditionType.php b/module/Core/src/RedirectRule/Model/RedirectConditionType.php index c00cca7f6..891a8ccc1 100644 --- a/module/Core/src/RedirectRule/Model/RedirectConditionType.php +++ b/module/Core/src/RedirectRule/Model/RedirectConditionType.php @@ -7,4 +7,5 @@ enum RedirectConditionType: string case DEVICE = 'device'; case LANGUAGE = 'language'; case QUERY_PARAM = 'query-param'; + case IP_ADDRESS = 'ip-address'; } From f49d98f2ea7bde1b3020d137222d65b82c911d8b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 17 Jul 2024 19:51:13 +0200 Subject: [PATCH 18/41] Add logic for IP-based dynamic redirects --- .../src/RedirectRule/RedirectRuleHandler.php | 3 ++ module/Core/functions/functions.php | 7 +++ .../RedirectRule/Entity/RedirectCondition.php | 6 +-- module/Core/src/Util/IpAddressUtils.php | 3 +- module/Core/src/Visit/Model/Visitor.php | 4 +- module/Core/src/Visit/RequestTracker.php | 5 +- .../Entity/RedirectConditionTest.php | 47 ++++++++++++++----- .../Entity/ShortUrlRedirectRuleTest.php | 26 ++++++++-- .../ShortUrlRedirectionResolverTest.php | 26 ++++++++++ 9 files changed, 102 insertions(+), 25 deletions(-) diff --git a/module/CLI/src/RedirectRule/RedirectRuleHandler.php b/module/CLI/src/RedirectRule/RedirectRuleHandler.php index 068cdc745..cb1d3faf2 100644 --- a/module/CLI/src/RedirectRule/RedirectRuleHandler.php +++ b/module/CLI/src/RedirectRule/RedirectRuleHandler.php @@ -108,6 +108,9 @@ private function addRule(ShortUrl $shortUrl, StyleInterface $io, array $currentR $this->askMandatory('Query param name?', $io), $this->askOptional('Query param value?', $io), ), + RedirectConditionType::IP_ADDRESS => RedirectCondition::forIpAddress( + $this->askMandatory('IP address, CIDR block or wildcard-pattern (1.2.*.*)', $io), + ), }; $continue = $io->confirm('Do you want to add another condition?'); diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 5900e8e1d..720f27c1c 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -14,6 +14,8 @@ use Laminas\Filter\Word\CamelCaseToSeparator; use Laminas\Filter\Word\CamelCaseToUnderscore; use Laminas\InputFilter\InputFilter; +use Psr\Http\Message\ServerRequestInterface; +use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; @@ -273,3 +275,8 @@ function splitByComma(?string $value): array return array_map(trim(...), explode(',', $value)); } + +function ipAddressFromRequest(ServerRequestInterface $request): ?string +{ + return $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR); +} diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index 3032eb1af..59c2798b8 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -5,14 +5,14 @@ use JsonSerializable; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Entity\AbstractEntity; -use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; use Shlinkio\Shlink\Core\RedirectRule\Model\Validation\RedirectRulesInputFilter; - use Shlinkio\Shlink\Core\Util\IpAddressUtils; + use function Shlinkio\Shlink\Core\acceptLanguageToLocales; use function Shlinkio\Shlink\Core\ArrayUtils\some; +use function Shlinkio\Shlink\Core\ipAddressFromRequest; use function Shlinkio\Shlink\Core\normalizeLocale; use function Shlinkio\Shlink\Core\splitLocale; use function sprintf; @@ -114,7 +114,7 @@ private function matchesDevice(ServerRequestInterface $request): bool private function matchesRemoteIpAddress(ServerRequestInterface $request): bool { - $remoteAddress = $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR); + $remoteAddress = ipAddressFromRequest($request); return $remoteAddress !== null && IpAddressUtils::ipAddressMatchesGroups($remoteAddress, [$this->matchValue]); } diff --git a/module/Core/src/Util/IpAddressUtils.php b/module/Core/src/Util/IpAddressUtils.php index 6f8f566b4..79e7f8399 100644 --- a/module/Core/src/Util/IpAddressUtils.php +++ b/module/Core/src/Util/IpAddressUtils.php @@ -14,8 +14,9 @@ use function explode; use function implode; use function Shlinkio\Shlink\Core\ArrayUtils\some; +use function str_contains; -class IpAddressUtils +final class IpAddressUtils { /** * Checks if an IP address matches any of provided groups. diff --git a/module/Core/src/Visit/Model/Visitor.php b/module/Core/src/Visit/Model/Visitor.php index 8e3b7f0cc..9cf524bcd 100644 --- a/module/Core/src/Visit/Model/Visitor.php +++ b/module/Core/src/Visit/Model/Visitor.php @@ -5,9 +5,9 @@ namespace Shlinkio\Shlink\Core\Visit\Model; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\Options\TrackingOptions; +use function Shlinkio\Shlink\Core\ipAddressFromRequest; use function Shlinkio\Shlink\Core\isCrawler; use function substr; @@ -46,7 +46,7 @@ public static function fromRequest(ServerRequestInterface $request): self return new self( $request->getHeaderLine('User-Agent'), $request->getHeaderLine('Referer'), - $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR), + ipAddressFromRequest($request), $request->getUri()->__toString(), ); } diff --git a/module/Core/src/Visit/RequestTracker.php b/module/Core/src/Visit/RequestTracker.php index ecc3d94f0..824e7c244 100644 --- a/module/Core/src/Visit/RequestTracker.php +++ b/module/Core/src/Visit/RequestTracker.php @@ -7,7 +7,6 @@ use Fig\Http\Message\RequestMethodInterface; use Mezzio\Router\Middleware\ImplicitHeadMiddleware; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Exception\InvalidIpFormatException; use Shlinkio\Shlink\Core\Options\TrackingOptions; @@ -15,6 +14,8 @@ use Shlinkio\Shlink\Core\Util\IpAddressUtils; use Shlinkio\Shlink\Core\Visit\Model\Visitor; +use function Shlinkio\Shlink\Core\ipAddressFromRequest; + readonly class RequestTracker implements RequestTrackerInterface, RequestMethodInterface { public function __construct(private VisitsTrackerInterface $visitsTracker, private TrackingOptions $trackingOptions) @@ -53,7 +54,7 @@ private function shouldTrackRequest(ServerRequestInterface $request): bool return false; } - $remoteAddr = $request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR); + $remoteAddr = ipAddressFromRequest($request); if ($this->shouldDisableTrackingFromAddress($remoteAddr)) { return false; } diff --git a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php index a8ab2a167..3cd44ef0c 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Entity\RedirectCondition; @@ -28,19 +29,19 @@ public function matchesQueryParams(string $param, string $value, bool $expectedR } #[Test] - #[TestWith([null, '', false])] // no accept language - #[TestWith(['', '', false])] // empty accept language - #[TestWith(['*', '', false])] // wildcard accept language - #[TestWith(['en', 'en', true])] // single language match - #[TestWith(['es, en,fr', 'en', true])] // multiple languages match - #[TestWith(['es, en-US,fr', 'EN', true])] // multiple locales match - #[TestWith(['es_ES', 'es-ES', true])] // single locale match - #[TestWith(['en-US,es-ES;q=0.6', 'es-ES', false])] // too low quality - #[TestWith(['en-US,es-ES;q=0.9', 'es-ES', true])] // quality high enough - #[TestWith(['en-UK', 'en-uk', true])] // different casing match - #[TestWith(['en-UK', 'en', true])] // only lang - #[TestWith(['es-AR', 'en', false])] // different only lang - #[TestWith(['fr', 'fr-FR', false])] // less restrictive matching locale + #[TestWith([null, '', false], 'no accept language')] + #[TestWith(['', '', false], 'empty accept language')] + #[TestWith(['*', '', false], 'wildcard accept language')] + #[TestWith(['en', 'en', true], 'single language match')] + #[TestWith(['es, en,fr', 'en', true], 'multiple languages match')] + #[TestWith(['es, en-US,fr', 'EN', true], 'multiple locales match')] + #[TestWith(['es_ES', 'es-ES', true], 'single locale match')] + #[TestWith(['en-US,es-ES;q=0.6', 'es-ES', false], 'too low quality')] + #[TestWith(['en-US,es-ES;q=0.9', 'es-ES', true], 'quality high enough')] + #[TestWith(['en-UK', 'en-uk', true], 'different casing match')] + #[TestWith(['en-UK', 'en', true], 'only lang')] + #[TestWith(['es-AR', 'en', false], 'different only lang')] + #[TestWith(['fr', 'fr-FR', false], 'less restrictive matching locale')] public function matchesLanguage(?string $acceptLanguage, string $value, bool $expected): void { $request = ServerRequestFactory::fromGlobals(); @@ -72,4 +73,24 @@ public function matchesDevice(?string $userAgent, DeviceType $value, bool $expec self::assertEquals($expected, $result); } + + #[Test] + #[TestWith([null, '1.2.3.4', false], 'no remote IP address')] + #[TestWith(['1.2.3.4', '1.2.3.4', true], 'static IP address match')] + #[TestWith(['4.3.2.1', '1.2.3.4', false], 'no static IP address match')] + #[TestWith(['192.168.1.35', '192.168.1.0/24', true], 'CIDR block match')] + #[TestWith(['1.2.3.4', '192.168.1.0/24', false], 'no CIDR block match')] + #[TestWith(['192.168.1.35', '192.168.1.*', true], 'wildcard pattern match')] + #[TestWith(['1.2.3.4', '192.168.1.*', false], 'no wildcard pattern match')] + public function matchesRemoteIpAddress(?string $remoteIp, string $ipToMatch, bool $expected): void + { + $request = ServerRequestFactory::fromGlobals(); + if ($remoteIp !== null) { + $request = $request->withAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR, $remoteIp); + } + + $result = RedirectCondition::forIpAddress($ipToMatch)->matchesRequest($request); + + self::assertEquals($expected, $result); + } } diff --git a/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php b/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php index d61bc6fa1..dc19dcd39 100644 --- a/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php +++ b/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php @@ -1,17 +1,20 @@ createRule($conditions); $result = $rule->mapConditions($callback); @@ -78,10 +84,22 @@ public static function provideConditionMappingCallbacks(): iterable 'matchKey' => 'foo', 'matchValue' => 'bar', ], + [ + 'type' => RedirectConditionType::DEVICE->value, + 'matchKey' => null, + 'matchValue' => DeviceType::ANDROID->value, + ], + [ + 'type' => RedirectConditionType::IP_ADDRESS->value, + 'matchKey' => null, + 'matchValue' => '1.2.3.*', + ], ]]; yield 'human-friendly conditions' => [fn (RedirectCondition $cond) => $cond->toHumanFriendly(), [ 'en-UK language is accepted', 'query string contains foo=bar', + sprintf('device is %s', DeviceType::ANDROID->value), + 'IP address matches 1.2.3.*', ]]; } diff --git a/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php b/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php index 5bf435b20..3bf23863d 100644 --- a/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php +++ b/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php @@ -9,6 +9,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; +use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Entity\RedirectCondition; use Shlinkio\Shlink\Core\RedirectRule\Entity\ShortUrlRedirectRule; @@ -88,5 +89,30 @@ public static function provideData(): iterable RedirectCondition::forQueryParam('foo', 'bar'), 'https://example.com/from-rule', ]; + yield 'matching static IP address' => [ + $request()->withAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR, '1.2.3.4'), + RedirectCondition::forIpAddress('1.2.3.4'), + 'https://example.com/from-rule', + ]; + yield 'matching CIDR block' => [ + $request()->withAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR, '192.168.1.35'), + RedirectCondition::forIpAddress('192.168.1.0/24'), + 'https://example.com/from-rule', + ]; + yield 'matching wildcard IP address' => [ + $request()->withAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR, '1.2.5.5'), + RedirectCondition::forIpAddress('1.2.*.*'), + 'https://example.com/from-rule', + ]; + yield 'non-matching IP address' => [ + $request()->withAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR, '4.3.2.1'), + RedirectCondition::forIpAddress('1.2.3.4'), + 'https://example.com/foo/bar', + ]; + yield 'missing remote IP address' => [ + $request(), + RedirectCondition::forIpAddress('1.2.3.4'), + 'https://example.com/foo/bar', + ]; } } From bab6a3951ef91bb63f23a05e822b030ddae33ea9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 17 Jul 2024 19:56:53 +0200 Subject: [PATCH 19/41] Add missing unit test --- module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php | 2 ++ module/Core/src/Util/IpAddressUtils.php | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php index 0c0b7d123..edd1eae34 100644 --- a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php +++ b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php @@ -116,6 +116,7 @@ public function newRulesCanBeAdded( 'Language to match?' => 'en-US', 'Query param name?' => 'foo', 'Query param value?' => 'bar', + 'IP address, CIDR block or wildcard-pattern (1.2.*.*)' => '1.2.3.4', default => '', }, ); @@ -163,6 +164,7 @@ public static function provideDeviceConditions(): iterable [RedirectCondition::forQueryParam('foo', 'bar'), RedirectCondition::forQueryParam('foo', 'bar')], true, ]; + yield 'IP address' => [RedirectConditionType::IP_ADDRESS, [RedirectCondition::forIpAddress('1.2.3.4')]]; } #[Test] diff --git a/module/Core/src/Util/IpAddressUtils.php b/module/Core/src/Util/IpAddressUtils.php index 79e7f8399..f5e1b0523 100644 --- a/module/Core/src/Util/IpAddressUtils.php +++ b/module/Core/src/Util/IpAddressUtils.php @@ -26,7 +26,7 @@ final class IpAddressUtils * Matching will happen as follows: * * Static IP address -> strict equality with provided IP address. * * CIDR block -> provided IP address is part of that block. - * * Wildcard -> static parts match the corresponding ones in provided IP address. + * * Wildcard pattern -> static parts match the corresponding ones in provided IP address. * * @param string[] $groups * @throws InvalidIpFormatException From f4a7712ded59cb08b0bb04d6b01777a4ad74ee9b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 17 Jul 2024 19:59:13 +0200 Subject: [PATCH 20/41] Add InvalidIpFormatExceptionTest --- .../InvalidIpFormatExceptionTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 module/Core/test/Exception/InvalidIpFormatExceptionTest.php diff --git a/module/Core/test/Exception/InvalidIpFormatExceptionTest.php b/module/Core/test/Exception/InvalidIpFormatExceptionTest.php new file mode 100644 index 000000000..6f0c38049 --- /dev/null +++ b/module/Core/test/Exception/InvalidIpFormatExceptionTest.php @@ -0,0 +1,19 @@ +getMessage()); + } +} From 626caa4afa997750d83015e0f2c62bea50f12dfb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 17 Jul 2024 20:12:56 +0200 Subject: [PATCH 21/41] Add API test for dynamic IP-based redirects --- module/Core/test-api/Action/RedirectTest.php | 12 ++++++++++++ .../Rest/test-api/Action/ListRedirectRulesTest.php | 11 +++++++++++ .../Fixtures/ShortUrlRedirectRulesFixture.php | 8 ++++++++ 3 files changed, 31 insertions(+) diff --git a/module/Core/test-api/Action/RedirectTest.php b/module/Core/test-api/Action/RedirectTest.php index 2c54e8a7e..dc6ca1745 100644 --- a/module/Core/test-api/Action/RedirectTest.php +++ b/module/Core/test-api/Action/RedirectTest.php @@ -10,6 +10,8 @@ use PHPUnit\Framework\Attributes\TestWith; 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; @@ -86,6 +88,16 @@ public static function provideRequestOptions(): iterable ], 'https://blog.alejandrocelaya.com/2017/12/09/acmailer-7-0-the-most-important-release-in-a-long-time/', ]; + + $clientDetection = require __DIR__ . '/../../../../config/autoload/client-detection.global.php'; + foreach ($clientDetection['ip_address_resolution']['headers_to_inspect'] as $header) { + yield sprintf('rule: IP address in "%s" header', $header) => [ + [ + RequestOptions::HEADERS => [$header => '1.2.3.4'], + ], + 'https://example.com/static-ip-address', + ]; + } } /** diff --git a/module/Rest/test-api/Action/ListRedirectRulesTest.php b/module/Rest/test-api/Action/ListRedirectRulesTest.php index 8f4efddae..494a6564f 100644 --- a/module/Rest/test-api/Action/ListRedirectRulesTest.php +++ b/module/Rest/test-api/Action/ListRedirectRulesTest.php @@ -87,6 +87,17 @@ public function errorIsReturnedWhenInvalidUrlIsFetched(): void ], ], ], + [ + 'longUrl' => 'https://example.com/static-ip-address', + 'priority' => 6, + 'conditions' => [ + [ + 'type' => 'ip-address', + 'matchKey' => null, + 'matchValue' => '1.2.3.4', + ], + ], + ], ]])] 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 b21f86e14..3aa813154 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php @@ -70,6 +70,14 @@ public function load(ObjectManager $manager): void ); $manager->persist($iosRule); + $ipAddressRule = new ShortUrlRedirectRule( + shortUrl: $defShortUrl, + priority: 6, + longUrl: 'https://example.com/static-ip-address', + conditions: new ArrayCollection([RedirectCondition::forIpAddress('1.2.3.4')]), + ); + $manager->persist($ipAddressRule); + $manager->flush(); } } From ce2ed237c75d4f0b293d92db69458cc9d2b83597 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 17 Jul 2024 20:23:58 +0200 Subject: [PATCH 22/41] Add ip-address condition type to redirect rules API spec docs --- docs/swagger/definitions/SetShortUrlRedirectRule.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/swagger/definitions/SetShortUrlRedirectRule.json b/docs/swagger/definitions/SetShortUrlRedirectRule.json index d17bedb37..00074acfd 100644 --- a/docs/swagger/definitions/SetShortUrlRedirectRule.json +++ b/docs/swagger/definitions/SetShortUrlRedirectRule.json @@ -15,8 +15,8 @@ "properties": { "type": { "type": "string", - "enum": ["device", "language", "query-param"], - "description": "The type of the condition, which will condition the logic used to match it" + "enum": ["device", "language", "query-param", "ip-address"], + "description": "The type of the condition, which will determine the logic used to match it" }, "matchKey": { "type": ["string", "null"] From 7e2f755dfd26e6cc115a9763a915e877496cdd9f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 Jul 2024 21:23:48 +0200 Subject: [PATCH 23/41] Validate IP address patterns when creating ip-address redirect conditions --- .../Validation/RedirectRulesInputFilter.php | 16 +++-- module/Core/src/Util/IpAddressUtils.php | 31 +++++++-- .../Model/RedirectRulesDataTest.php | 67 +++++++++++++++++++ module/Core/test/Util/IpAddressUtilsTest.php | 26 +++++++ .../test-api/Action/SetRedirectRulesTest.php | 65 ++++++++++++++++-- 5 files changed, 185 insertions(+), 20 deletions(-) create mode 100644 module/Core/test/Util/IpAddressUtilsTest.php diff --git a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php index 5decaf4ca..892b93e49 100644 --- a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php +++ b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php @@ -12,6 +12,7 @@ use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; +use Shlinkio\Shlink\Core\Util\IpAddressUtils; use function Shlinkio\Shlink\Core\ArrayUtils\contains; use function Shlinkio\Shlink\Core\enumValues; @@ -71,13 +72,14 @@ private static function createRedirectConditionInputFilter(): InputFilter $redirectConditionInputFilter->add($type); $value = InputFactory::basic(self::CONDITION_MATCH_VALUE, required: true); - $value->getValidatorChain()->attach(new Callback(function (string $value, array $context) { - if ($context[self::CONDITION_TYPE] === RedirectConditionType::DEVICE->value) { - return contains($value, enumValues(DeviceType::class)); - } - - return true; - })); + $value->getValidatorChain()->attach(new Callback( + fn (string $value, array $context) => match ($context[self::CONDITION_TYPE]) { + RedirectConditionType::DEVICE->value => contains($value, enumValues(DeviceType::class)), + RedirectConditionType::IP_ADDRESS->value => IpAddressUtils::isStaticIpCidrOrWildcard($value), + // RedirectConditionType::LANGUAGE->value => TODO, + default => true, + }, + )); $redirectConditionInputFilter->add($value); $redirectConditionInputFilter->add( diff --git a/module/Core/src/Util/IpAddressUtils.php b/module/Core/src/Util/IpAddressUtils.php index f5e1b0523..66354c373 100644 --- a/module/Core/src/Util/IpAddressUtils.php +++ b/module/Core/src/Util/IpAddressUtils.php @@ -18,6 +18,11 @@ final class IpAddressUtils { + public static function isStaticIpCidrOrWildcard(string $candidate): bool + { + return self::candidateToRange($candidate, ['0', '0', '0', '0']) !== null; + } + /** * Checks if an IP address matches any of provided groups. * Every group can be a static IP address (100.200.80.40), a CIDR block (192.168.10.0/24) or a wildcard pattern @@ -40,15 +45,29 @@ public static function ipAddressMatchesGroups(string $ipAddress, array $groups): $ipAddressParts = explode('.', $ipAddress); - return some($groups, function (string $value) use ($ip, $ipAddressParts): bool { - $range = str_contains($value, '*') - ? self::parseValueWithWildcards($value, $ipAddressParts) - : Factory::parseRangeString($value); - - return $range !== null && $ip->matches($range); + return some($groups, function (string $group) use ($ip, $ipAddressParts): bool { + $range = self::candidateToRange($group, $ipAddressParts); + return $range !== null && $range->contains($ip); }); } + /** + * Convert a static IP, CIDR block or wildcard pattern into a Range object + * + * @param string[] $ipAddressParts + */ + private static function candidateToRange(string $candidate, array $ipAddressParts): ?RangeInterface + { + return str_contains($candidate, '*') + ? self::parseValueWithWildcards($candidate, $ipAddressParts) + : Factory::parseRangeString($candidate); + } + + /** + * Try to generate an IP range from a wildcard pattern. + * Factory::parseRangeString can usually do this automatically, but only if wildcards are at the end. This also + * covers cases where wildcards are in between. + */ private static function parseValueWithWildcards(string $value, array $ipAddressParts): ?RangeInterface { $octets = explode('.', $value); diff --git a/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php b/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php index f0ded32ba..e71140cb9 100644 --- a/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php +++ b/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php @@ -51,9 +51,76 @@ class RedirectRulesDataTest extends TestCase ], ], ]]])] + #[TestWith([['redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'ip-address', + 'matchKey' => null, + 'matchValue' => 'not an IP address', + ], + ], + ], + ]]])] public function throwsWhenProvidedDataIsInvalid(array $invalidData): void { $this->expectException(ValidationException::class); RedirectRulesData::fromRawData($invalidData); } + + #[Test] + #[TestWith([['redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'ip-address', + 'matchKey' => null, + 'matchValue' => '1.2.3.4', + ], + ], + ], + ]]], 'static IP')] + #[TestWith([['redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'ip-address', + 'matchKey' => null, + 'matchValue' => '1.2.3.0/24', + ], + ], + ], + ]]], 'CIDR block')] + #[TestWith([['redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'ip-address', + 'matchKey' => null, + 'matchValue' => '1.2.3.*', + ], + ], + ], + ]]], 'IP wildcard pattern')] + #[TestWith([['redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'ip-address', + 'matchKey' => null, + 'matchValue' => '1.2.*.4', + ], + ], + ], + ]]], 'in-between IP wildcard pattern')] + public function allowsValidDataToBeSet(array $validData): void + { + $result = RedirectRulesData::fromRawData($validData); + self::assertEquals($result->rules, $validData['redirectRules']); + } } diff --git a/module/Core/test/Util/IpAddressUtilsTest.php b/module/Core/test/Util/IpAddressUtilsTest.php new file mode 100644 index 000000000..c1045e686 --- /dev/null +++ b/module/Core/test/Util/IpAddressUtilsTest.php @@ -0,0 +1,26 @@ +callApiWithKey(self::METHOD_POST, '/short-urls/invalid/redirect-rules'); $payload = $this->getJsonResponsePayload($response); @@ -39,16 +39,67 @@ public function errorIsReturnedWhenInvalidUrlProvided(): void } #[Test] - public function errorIsReturnedWhenInvalidDataProvided(): void - { - $response = $this->callApiWithKey(self::METHOD_POST, '/short-urls/abc123/redirect-rules', [ - RequestOptions::JSON => [ - 'redirectRules' => [ + #[TestWith([[ + 'redirectRules' => [ + [ + 'longUrl' => 'invalid', + ], + ], + ]], 'invalid long URL')] + #[TestWith([[ + 'redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => 'foo', + ], + ], + ]], 'non-array conditions')] + #[TestWith([[ + 'redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'invalid', + 'matchKey' => null, + 'matchValue' => 'foo', + ], + ], + ], + ], + ]], 'invalid condition type')] + #[TestWith([[ + 'redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'device', + 'matchValue' => 'invalid-device', + 'matchKey' => null, + ], + ], + ], + ], + ]], 'invalid device type')] + #[TestWith([[ + 'redirectRules' => [ + [ + 'longUrl' => 'https://example.com', + 'conditions' => [ [ - 'longUrl' => 'invalid', + 'type' => 'ip-address', + 'matchKey' => null, + 'matchValue' => 'not an IP address', ], ], ], + ], + ]], 'invalid IP address')] + public function errorIsReturnedWhenInvalidDataIsProvided(array $bodyPayload): void + { + $response = $this->callApiWithKey(self::METHOD_POST, '/short-urls/abc123/redirect-rules', [ + RequestOptions::JSON => $bodyPayload, ]); $payload = $this->getJsonResponsePayload($response); From 9e6cdcb838e6bda4b15f35ed898f8f566f2d64cd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 Jul 2024 21:26:28 +0200 Subject: [PATCH 24/41] Update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ff88fcf9..ec85e5ccb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added +* [#2120](https://github.com/shlinkio/shlink/issues/2120) Add new IP address condition for the dynamic rules redirections system. + + The conditions allow you to define IP addresses to match as static IP (1.2.3.4), CIDR block (192.168.1.0/24) or wildcard pattern (1.2.\*.\*). + * [#2018](https://github.com/shlinkio/shlink/issues/2018) Add option to allow all short URLs to be unconditionally crawlable in robots.txt, via `ROBOTS_ALLOW_ALL_SHORT_URLS=true` env var, or config option. * [#2109](https://github.com/shlinkio/shlink/issues/2109) Add option to customize user agents robots.txt, via `ROBOTS_USER_AGENTS=foo,bar,baz` env var, or config option. From ae0ff5f23c44d26ccbf7e49ab974cda01299ba6e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 23 Jul 2024 20:02:49 +0200 Subject: [PATCH 25/41] Add PHP 8.4 to CI --- .github/actions/ci-setup/action.yml | 2 +- .github/workflows/ci-db-tests.yml | 7 ++++--- .github/workflows/ci-docker-image-build.yml | 2 +- .github/workflows/ci-tests.yml | 7 ++++--- .github/workflows/ci.yml | 10 +++++----- .github/workflows/publish-release.yml | 8 ++++---- .github/workflows/publish-swagger-spec.yml | 2 +- 7 files changed, 20 insertions(+), 18 deletions(-) diff --git a/.github/actions/ci-setup/action.yml b/.github/actions/ci-setup/action.yml index 227578f5b..f800cf097 100644 --- a/.github/actions/ci-setup/action.yml +++ b/.github/actions/ci-setup/action.yml @@ -44,5 +44,5 @@ runs: ini-values: pcov.directory=module - 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.4' && '--ignore-platform-req=php' || '' }} shell: bash diff --git a/.github/workflows/ci-db-tests.yml b/.github/workflows/ci-db-tests.yml index 8cea11f78..ba45e9f40 100644 --- a/.github/workflows/ci-db-tests.yml +++ b/.github/workflows/ci-db-tests.yml @@ -10,10 +10,11 @@ on: jobs: db-tests: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 strategy: matrix: - php-version: ['8.2', '8.3'] + php-version: ['8.2', '8.3', '8.4'] + continue-on-error: ${{ matrix.php-version == '8.4' }} env: LC_ALL: C steps: @@ -36,7 +37,7 @@ jobs: run: composer test:db:${{ inputs.platform }} - name: Upload code coverage uses: actions/upload-artifact@v4 - if: ${{ matrix.php-version == '8.2' && inputs.platform == 'sqlite:ci' }} + if: ${{ matrix.php-version == '8.3' && inputs.platform == 'sqlite:ci' }} with: name: coverage-db path: | diff --git a/.github/workflows/ci-docker-image-build.yml b/.github/workflows/ci-docker-image-build.yml index 43812fad5..ab9681c50 100644 --- a/.github/workflows/ci-docker-image-build.yml +++ b/.github/workflows/ci-docker-image-build.yml @@ -7,7 +7,7 @@ on: jobs: build-docker-image: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: - name: Checkout code uses: actions/checkout@v4 diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index d2cf4d9ae..70fe80494 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -10,10 +10,11 @@ on: jobs: tests: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 strategy: matrix: - php-version: ['8.2', '8.3'] + php-version: ['8.2', '8.3', '8.4'] + continue-on-error: ${{ matrix.php-version == '8.4' }} env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # rr get-binary picks this env automatically steps: @@ -33,7 +34,7 @@ jobs: run: ./vendor/bin/rr get --no-interaction --no-config --location bin/ && chmod +x bin/rr - run: composer test:${{ inputs.test-group }}:ci - uses: actions/upload-artifact@v4 - if: ${{ matrix.php-version == '8.2' }} + if: ${{ matrix.php-version == '8.3' }} with: name: coverage-${{ inputs.test-group }} path: | diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 933d71b68..a93559cb3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,10 +24,10 @@ on: jobs: static-analysis: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 strategy: matrix: - php-version: ['8.2'] + php-version: ['8.3'] command: ['cs', 'stan', 'swagger:validate'] steps: - uses: actions/checkout@v4 @@ -66,10 +66,10 @@ jobs: - api-tests - cli-tests - db-tests - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 strategy: matrix: - php-version: ['8.2'] + php-version: ['8.3'] steps: - name: Checkout code uses: actions/checkout@v4 @@ -94,7 +94,7 @@ jobs: delete-artifacts: needs: - upload-coverage - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: - uses: geekyeggo/delete-artifact@v2 with: diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index 7875c07b2..a81d51fb4 100644 --- a/.github/workflows/publish-release.yml +++ b/.github/workflows/publish-release.yml @@ -7,10 +7,10 @@ on: jobs: build: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 strategy: matrix: - php-version: ['8.2', '8.3'] + php-version: ['8.2', '8.3'] # TODO 8.4 steps: - uses: actions/checkout@v4 - uses: './.github/actions/ci-setup' @@ -26,7 +26,7 @@ jobs: publish: needs: ['build'] - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 - uses: actions/download-artifact@v4 @@ -43,7 +43,7 @@ jobs: delete-artifacts: needs: ['publish'] - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: - uses: geekyeggo/delete-artifact@v2 with: diff --git a/.github/workflows/publish-swagger-spec.yml b/.github/workflows/publish-swagger-spec.yml index beebf57f0..9607206ae 100644 --- a/.github/workflows/publish-swagger-spec.yml +++ b/.github/workflows/publish-swagger-spec.yml @@ -7,7 +7,7 @@ on: jobs: build: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 strategy: matrix: php-version: ['8.2'] From fabc7523984dc6ddc9b378badad7284c636e40eb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 25 Jul 2024 23:44:06 +0200 Subject: [PATCH 26/41] Extract reading and parsing of arguments for short URLs data in commands --- .../ShortUrl/CreateShortUrlCommand.php | 82 ++++---------- module/CLI/src/Input/ShortUrlDataInput.php | 106 ++++++++++++++++++ .../Action/ShortUrl/EditShortUrlAction.php | 4 +- 3 files changed, 127 insertions(+), 65 deletions(-) create mode 100644 module/CLI/src/Input/ShortUrlDataInput.php diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index 4b6a088de..fa7a2bea2 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -4,6 +4,7 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl; +use Shlinkio\Shlink\CLI\Input\ShortUrlDataInput; use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; @@ -12,16 +13,11 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; use Shlinkio\Shlink\Core\ShortUrl\UrlShortenerInterface; 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 array_map; -use function array_unique; -use function explode; -use function Shlinkio\Shlink\Core\ArrayUtils\flatten; use function sprintf; class CreateShortUrlCommand extends Command @@ -29,6 +25,7 @@ class CreateShortUrlCommand extends Command public const NAME = 'short-url:create'; private ?SymfonyStyle $io; + private readonly ShortUrlDataInput $shortUrlDataInput; public function __construct( private readonly UrlShortenerInterface $urlShortener, @@ -36,6 +33,7 @@ public function __construct( private readonly UrlShortenerOptions $options, ) { parent::__construct(); + $this->shortUrlDataInput = new ShortUrlDataInput($this); } protected function configure(): void @@ -43,26 +41,11 @@ protected function configure(): void $this ->setName(self::NAME) ->setDescription('Generates a short URL for provided long URL and returns it') - ->addArgument('longUrl', InputArgument::REQUIRED, 'The long URL to parse') ->addOption( - 'tags', - 't', - InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED, - 'Tags to apply to the new short URL', - ) - ->addOption( - 'valid-since', - 's', - InputOption::VALUE_REQUIRED, - 'The date from which this short URL will be valid. ' - . 'If someone tries to access it before this date, it will not be found.', - ) - ->addOption( - 'valid-until', - 'u', + 'domain', + 'd', InputOption::VALUE_REQUIRED, - 'The date until which this short URL will be valid. ' - . 'If someone tries to access it after this date, it will not be found.', + 'The domain to which this short URL will be attached.', ) ->addOption( 'custom-slug', @@ -71,46 +54,22 @@ protected function configure(): void 'If provided, this slug will be used instead of generating a short code', ) ->addOption( - 'path-prefix', - 'p', + 'short-code-length', + 'l', InputOption::VALUE_REQUIRED, - 'Prefix to prepend before the generated short code or provided custom slug', + 'The length for generated short code (it will be ignored if --custom-slug was provided).', ) ->addOption( - 'max-visits', - 'm', + 'path-prefix', + 'p', InputOption::VALUE_REQUIRED, - 'This will limit the number of visits for this short URL.', + 'Prefix to prepend before the generated short code or provided custom slug', ) ->addOption( 'find-if-exists', 'f', InputOption::VALUE_NONE, 'This will force existing matching URL to be returned if found, instead of creating a new one.', - ) - ->addOption( - 'domain', - 'd', - InputOption::VALUE_REQUIRED, - 'The domain to which this short URL will be attached.', - ) - ->addOption( - 'short-code-length', - 'l', - InputOption::VALUE_REQUIRED, - 'The length for generated short code (it will be ignored if --custom-slug was provided).', - ) - ->addOption( - 'crawlable', - 'r', - InputOption::VALUE_NONE, - 'Tells if this URL will be included as "Allow" in Shlink\'s robots.txt.', - ) - ->addOption( - 'no-forward-query', - 'w', - InputOption::VALUE_NONE, - 'Disables the forwarding of the query string to the long URL, when the new short URL is visited.', ); } @@ -136,31 +95,28 @@ private function verifyLongUrlArgument(InputInterface $input, OutputInterface $o protected function execute(InputInterface $input, OutputInterface $output): int { $io = $this->getIO($input, $output); - $longUrl = $input->getArgument('longUrl'); + $longUrl = $this->shortUrlDataInput->longUrl($input); if (empty($longUrl)) { $io->error('A URL was not provided!'); return ExitCode::EXIT_FAILURE; } - $explodeWithComma = static fn (string $tag) => explode(',', $tag); - $tags = array_unique(flatten(array_map($explodeWithComma, $input->getOption('tags')))); - $maxVisits = $input->getOption('max-visits'); $shortCodeLength = $input->getOption('short-code-length') ?? $this->options->defaultShortCodesLength; try { $result = $this->urlShortener->shorten(ShortUrlCreation::fromRawData([ ShortUrlInputFilter::LONG_URL => $longUrl, - ShortUrlInputFilter::VALID_SINCE => $input->getOption('valid-since'), - ShortUrlInputFilter::VALID_UNTIL => $input->getOption('valid-until'), - ShortUrlInputFilter::MAX_VISITS => $maxVisits !== null ? (int) $maxVisits : null, + ShortUrlInputFilter::VALID_SINCE => $this->shortUrlDataInput->validSince($input), + ShortUrlInputFilter::VALID_UNTIL => $this->shortUrlDataInput->validUntil($input), + ShortUrlInputFilter::MAX_VISITS => $this->shortUrlDataInput->maxVisits($input), ShortUrlInputFilter::CUSTOM_SLUG => $input->getOption('custom-slug'), ShortUrlInputFilter::PATH_PREFIX => $input->getOption('path-prefix'), ShortUrlInputFilter::FIND_IF_EXISTS => $input->getOption('find-if-exists'), ShortUrlInputFilter::DOMAIN => $input->getOption('domain'), ShortUrlInputFilter::SHORT_CODE_LENGTH => $shortCodeLength, - ShortUrlInputFilter::TAGS => $tags, - ShortUrlInputFilter::CRAWLABLE => $input->getOption('crawlable'), - ShortUrlInputFilter::FORWARD_QUERY => !$input->getOption('no-forward-query'), + ShortUrlInputFilter::TAGS => $this->shortUrlDataInput->tags($input), + ShortUrlInputFilter::CRAWLABLE => $this->shortUrlDataInput->crawlable($input), + ShortUrlInputFilter::FORWARD_QUERY => !$this->shortUrlDataInput->noForwardQuery($input), ], $this->options)); $result->onEventDispatchingError(static fn () => $io->isVerbose() && $io->warning( diff --git a/module/CLI/src/Input/ShortUrlDataInput.php b/module/CLI/src/Input/ShortUrlDataInput.php new file mode 100644 index 000000000..5ba3126fd --- /dev/null +++ b/module/CLI/src/Input/ShortUrlDataInput.php @@ -0,0 +1,106 @@ +addOption('long-url', 'l', InputOption::VALUE_REQUIRED, 'The long URL to set'); + } else { + $command->addArgument('longUrl', InputArgument::REQUIRED, 'The long URL to set'); + } + + $command + ->addOption( + 'tags', + 't', + InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED, + 'Tags to apply to the short URL', + ) + ->addOption( + 'valid-since', + 's', + InputOption::VALUE_REQUIRED, + 'The date from which this short URL will be valid. ' + . 'If someone tries to access it before this date, it will not be found.', + ) + ->addOption( + 'valid-until', + 'u', + InputOption::VALUE_REQUIRED, + 'The date until which this short URL will be valid. ' + . 'If someone tries to access it after this date, it will not be found.', + ) + ->addOption( + 'max-visits', + 'm', + InputOption::VALUE_REQUIRED, + 'This will limit the number of visits for this short URL.', + ) + ->addOption( + 'crawlable', + 'r', + InputOption::VALUE_NONE, + 'Tells if this short URL will be included as "Allow" in Shlink\'s robots.txt.', + ) + ->addOption( + 'no-forward-query', + 'w', + InputOption::VALUE_NONE, + 'Disables the forwarding of the query string to the long URL, when the short URL is visited.', + ); + } + + public function longUrl(InputInterface $input): ?string + { + return $this->longUrlAsOption ? $input->getOption('long-url') : $input->getArgument('longUrl'); + } + + /** + * @return string[] + */ + public function tags(InputInterface $input): array + { + return array_unique(flatten(array_map(splitByComma(...), $input->getOption('tags')))); + } + + public function validSince(InputInterface $input): ?string + { + return $input->getOption('valid-since'); + } + + public function validUntil(InputInterface $input): ?string + { + return $input->getOption('valid-until'); + } + + public function maxVisits(InputInterface $input): ?int + { + $maxVisits = $input->getOption('max-visits'); + return $maxVisits !== null ? (int) $maxVisits : null; + } + + public function crawlable(InputInterface $input): bool + { + return $input->getOption('crawlable'); + } + + public function noForwardQuery(InputInterface $input): bool + { + return $input->getOption('no-forward-query'); + } +} diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php index 61c1a70c6..f0f8c0683 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php @@ -20,8 +20,8 @@ class EditShortUrlAction extends AbstractRestAction protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PATCH]; public function __construct( - private ShortUrlServiceInterface $shortUrlService, - private DataTransformerInterface $transformer, + private readonly ShortUrlServiceInterface $shortUrlService, + private readonly DataTransformerInterface $transformer, ) { } From 8917ed5c2e732def6e9f20bba801f762d04bb71f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 26 Jul 2024 00:01:40 +0200 Subject: [PATCH 27/41] Create command to edit existing short URLs --- module/CLI/config/cli.config.php | 1 + module/CLI/config/dependencies.config.php | 2 + .../ShortUrl/CreateShortUrlCommand.php | 2 +- .../Command/ShortUrl/EditShortUrlCommand.php | 60 +++++++++++++++++++ module/CLI/src/Input/ShortUrlDataInput.php | 19 ++++++ 5 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 2ee33a1d7..e60bb2e1f 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -9,6 +9,7 @@ 'cli' => [ 'commands' => [ Command\ShortUrl\CreateShortUrlCommand::NAME => Command\ShortUrl\CreateShortUrlCommand::class, + Command\ShortUrl\EditShortUrlCommand::NAME => Command\ShortUrl\EditShortUrlCommand::class, Command\ShortUrl\ResolveUrlCommand::NAME => Command\ShortUrl\ResolveUrlCommand::class, Command\ShortUrl\ListShortUrlsCommand::NAME => Command\ShortUrl\ListShortUrlsCommand::class, Command\ShortUrl\GetShortUrlVisitsCommand::NAME => Command\ShortUrl\GetShortUrlVisitsCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index f9b90daca..f9bb96545 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -41,6 +41,7 @@ ApiKey\RoleResolver::class => ConfigAbstractFactory::class, Command\ShortUrl\CreateShortUrlCommand::class => ConfigAbstractFactory::class, + Command\ShortUrl\EditShortUrlCommand::class => ConfigAbstractFactory::class, Command\ShortUrl\ResolveUrlCommand::class => ConfigAbstractFactory::class, Command\ShortUrl\ListShortUrlsCommand::class => ConfigAbstractFactory::class, Command\ShortUrl\GetShortUrlVisitsCommand::class => ConfigAbstractFactory::class, @@ -92,6 +93,7 @@ ShortUrlStringifier::class, UrlShortenerOptions::class, ], + Command\ShortUrl\EditShortUrlCommand::class => [ShortUrl\ShortUrlService::class], Command\ShortUrl\ResolveUrlCommand::class => [ShortUrl\ShortUrlResolver::class], Command\ShortUrl\ListShortUrlsCommand::class => [ ShortUrl\ShortUrlListService::class, diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index fa7a2bea2..d47c30b9a 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -137,6 +137,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int private function getIO(InputInterface $input, OutputInterface $output): SymfonyStyle { - return $this->io ?? ($this->io = new SymfonyStyle($input, $output)); + return $this->io ??= new SymfonyStyle($input, $output); } } diff --git a/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php new file mode 100644 index 000000000..b3fd0bd45 --- /dev/null +++ b/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php @@ -0,0 +1,60 @@ +shortUrlDataInput = new ShortUrlDataInput($this, longUrlAsOption: true); + $this->shortUrlIdentifierInput = new ShortUrlIdentifierInput( + $this, + shortCodeDesc: 'The short code to edit', + domainDesc: 'The domain to which the short URL is attached.', + ); + } + + protected function configure(): void + { + $this + ->setName(self::NAME) + ->setDescription('Edit an existing short URL'); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $io = new SymfonyStyle($input, $output); + + try { + $shortUrl = $this->shortUrlService->updateShortUrl( + $this->shortUrlIdentifierInput->toShortUrlIdentifier($input), + $this->shortUrlDataInput->toShortUrlEdition($input), + ); + + // TODO Print success + return ExitCode::EXIT_SUCCESS; + } catch (ShortUrlNotFoundException) { + // TODO Print error + return ExitCode::EXIT_FAILURE; + } + } +} diff --git a/module/CLI/src/Input/ShortUrlDataInput.php b/module/CLI/src/Input/ShortUrlDataInput.php index 5ba3126fd..46c5b1cec 100644 --- a/module/CLI/src/Input/ShortUrlDataInput.php +++ b/module/CLI/src/Input/ShortUrlDataInput.php @@ -4,6 +4,8 @@ namespace Shlinkio\Shlink\CLI\Input; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlEdition; +use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -103,4 +105,21 @@ public function noForwardQuery(InputInterface $input): bool { return $input->getOption('no-forward-query'); } + + public function toShortUrlEdition(InputInterface $input): ShortUrlEdition + { + return ShortUrlEdition::fromRawData([ + ShortUrlInputFilter::LONG_URL => $this->longUrl($input), + ShortUrlInputFilter::VALID_SINCE => $this->validSince($input), + ShortUrlInputFilter::VALID_UNTIL => $this->validUntil($input), + ShortUrlInputFilter::MAX_VISITS => $this->maxVisits($input), + ShortUrlInputFilter::TAGS => $this->tags($input), + ShortUrlInputFilter::CRAWLABLE => $this->crawlable($input), + ShortUrlInputFilter::FORWARD_QUERY => !$this->noForwardQuery($input), +// ShortUrlInputFilter::TITLE => TODO, + ]); + } + + // TODO + // public function toShortUrlCreation(InputInterface $input) } From 5bccdded8ab40f2a5a65a5e352b8ab076e33bd40 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 26 Jul 2024 09:20:03 +0200 Subject: [PATCH 28/41] Create command to edit existing short URLs --- module/CLI/config/dependencies.config.php | 2 +- .../ShortUrl/CreateShortUrlCommand.php | 34 +++----- .../Command/ShortUrl/EditShortUrlCommand.php | 23 +++-- module/CLI/src/Input/ShortUrlDataInput.php | 86 +++++++++---------- 4 files changed, 68 insertions(+), 77 deletions(-) diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index f9bb96545..3853fd1d6 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -93,7 +93,7 @@ ShortUrlStringifier::class, UrlShortenerOptions::class, ], - Command\ShortUrl\EditShortUrlCommand::class => [ShortUrl\ShortUrlService::class], + Command\ShortUrl\EditShortUrlCommand::class => [ShortUrl\ShortUrlService::class, ShortUrlStringifier::class], Command\ShortUrl\ResolveUrlCommand::class => [ShortUrl\ShortUrlResolver::class], Command\ShortUrl\ListShortUrlsCommand::class => [ ShortUrl\ShortUrlListService::class, diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index d47c30b9a..0273da71f 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -9,8 +9,6 @@ use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; -use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; -use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; use Shlinkio\Shlink\Core\ShortUrl\UrlShortenerInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -95,29 +93,17 @@ private function verifyLongUrlArgument(InputInterface $input, OutputInterface $o protected function execute(InputInterface $input, OutputInterface $output): int { $io = $this->getIO($input, $output); - $longUrl = $this->shortUrlDataInput->longUrl($input); - if (empty($longUrl)) { - $io->error('A URL was not provided!'); - return ExitCode::EXIT_FAILURE; - } - - $shortCodeLength = $input->getOption('short-code-length') ?? $this->options->defaultShortCodesLength; try { - $result = $this->urlShortener->shorten(ShortUrlCreation::fromRawData([ - ShortUrlInputFilter::LONG_URL => $longUrl, - ShortUrlInputFilter::VALID_SINCE => $this->shortUrlDataInput->validSince($input), - ShortUrlInputFilter::VALID_UNTIL => $this->shortUrlDataInput->validUntil($input), - ShortUrlInputFilter::MAX_VISITS => $this->shortUrlDataInput->maxVisits($input), - ShortUrlInputFilter::CUSTOM_SLUG => $input->getOption('custom-slug'), - ShortUrlInputFilter::PATH_PREFIX => $input->getOption('path-prefix'), - ShortUrlInputFilter::FIND_IF_EXISTS => $input->getOption('find-if-exists'), - ShortUrlInputFilter::DOMAIN => $input->getOption('domain'), - ShortUrlInputFilter::SHORT_CODE_LENGTH => $shortCodeLength, - ShortUrlInputFilter::TAGS => $this->shortUrlDataInput->tags($input), - ShortUrlInputFilter::CRAWLABLE => $this->shortUrlDataInput->crawlable($input), - ShortUrlInputFilter::FORWARD_QUERY => !$this->shortUrlDataInput->noForwardQuery($input), - ], $this->options)); + $result = $this->urlShortener->shorten($this->shortUrlDataInput->toShortUrlCreation( + $input, + $this->options, + customSlugField: 'custom-slug', + shortCodeLengthField: 'short-code-length', + pathPrefixField: 'path-prefix', + findIfExistsField: 'find-if-exists', + domainField: 'domain', + )); $result->onEventDispatchingError(static fn () => $io->isVerbose() && $io->warning( 'Short URL properly created, but the real-time updates cannot be notified when generating the ' @@ -125,7 +111,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int )); $io->writeln([ - sprintf('Processed long URL: %s', $longUrl), + sprintf('Processed long URL: %s', $result->shortUrl->getLongUrl()), sprintf('Generated short URL: %s', $this->stringifier->stringify($result->shortUrl)), ]); return ExitCode::EXIT_SUCCESS; diff --git a/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php index b3fd0bd45..048b3934b 100644 --- a/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/EditShortUrlCommand.php @@ -8,12 +8,15 @@ 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; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; +use function sprintf; + class EditShortUrlCommand extends Command { public const NAME = 'short-url:edit'; @@ -21,8 +24,10 @@ class EditShortUrlCommand extends Command private readonly ShortUrlDataInput $shortUrlDataInput; private readonly ShortUrlIdentifierInput $shortUrlIdentifierInput; - public function __construct(private readonly ShortUrlServiceInterface $shortUrlService) - { + public function __construct( + private readonly ShortUrlServiceInterface $shortUrlService, + private readonly ShortUrlStringifierInterface $stringifier, + ) { parent::__construct(); $this->shortUrlDataInput = new ShortUrlDataInput($this, longUrlAsOption: true); @@ -43,17 +48,23 @@ protected function configure(): void protected function execute(InputInterface $input, OutputInterface $output): int { $io = new SymfonyStyle($input, $output); + $identifier = $this->shortUrlIdentifierInput->toShortUrlIdentifier($input); try { $shortUrl = $this->shortUrlService->updateShortUrl( - $this->shortUrlIdentifierInput->toShortUrlIdentifier($input), + $identifier, $this->shortUrlDataInput->toShortUrlEdition($input), ); - // TODO Print success + $io->success(sprintf('Short URL "%s" properly edited', $this->stringifier->stringify($shortUrl))); return ExitCode::EXIT_SUCCESS; - } catch (ShortUrlNotFoundException) { - // TODO Print error + } catch (ShortUrlNotFoundException $e) { + $io->error(sprintf('Short URL not found for "%s"', $identifier->__toString())); + + if ($io->isVerbose()) { + $this->getApplication()?->renderThrowable($e, $io); + } + return ExitCode::EXIT_FAILURE; } } diff --git a/module/CLI/src/Input/ShortUrlDataInput.php b/module/CLI/src/Input/ShortUrlDataInput.php index 46c5b1cec..301215473 100644 --- a/module/CLI/src/Input/ShortUrlDataInput.php +++ b/module/CLI/src/Input/ShortUrlDataInput.php @@ -4,6 +4,8 @@ namespace Shlinkio\Shlink\CLI\Input; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlEdition; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; use Symfony\Component\Console\Command\Command; @@ -53,6 +55,11 @@ public function __construct(Command $command, private bool $longUrlAsOption = fa InputOption::VALUE_REQUIRED, 'This will limit the number of visits for this short URL.', ) + ->addOption( + 'title', + mode: InputOption::VALUE_REQUIRED, + description: 'A descriptive title for the short URL.', + ) ->addOption( 'crawlable', 'r', @@ -67,59 +74,46 @@ public function __construct(Command $command, private bool $longUrlAsOption = fa ); } - public function longUrl(InputInterface $input): ?string - { - return $this->longUrlAsOption ? $input->getOption('long-url') : $input->getArgument('longUrl'); - } - - /** - * @return string[] - */ - public function tags(InputInterface $input): array - { - return array_unique(flatten(array_map(splitByComma(...), $input->getOption('tags')))); - } - - public function validSince(InputInterface $input): ?string + public function toShortUrlEdition(InputInterface $input): ShortUrlEdition { - return $input->getOption('valid-since'); + return ShortUrlEdition::fromRawData($this->getCommonData($input)); } - public function validUntil(InputInterface $input): ?string - { - return $input->getOption('valid-until'); + public function toShortUrlCreation( + InputInterface $input, + UrlShortenerOptions $options, + string $customSlugField, + string $shortCodeLengthField, + string $pathPrefixField, + string $findIfExistsField, + string $domainField, + ): ShortUrlCreation { + $shortCodeLength = $input->getOption($shortCodeLengthField) ?? $options->defaultShortCodesLength; + return ShortUrlCreation::fromRawData([ + ...$this->getCommonData($input), + ShortUrlInputFilter::CUSTOM_SLUG => $input->getOption($customSlugField), + ShortUrlInputFilter::SHORT_CODE_LENGTH => $shortCodeLength, + ShortUrlInputFilter::PATH_PREFIX => $input->getOption($pathPrefixField), + ShortUrlInputFilter::FIND_IF_EXISTS => $input->getOption($findIfExistsField), + ShortUrlInputFilter::DOMAIN => $input->getOption($domainField), + ], $options); } - public function maxVisits(InputInterface $input): ?int + private function getCommonData(InputInterface $input): array { + $longUrl = $this->longUrlAsOption ? $input->getOption('long-url') : $input->getArgument('longUrl'); + $tags = array_unique(flatten(array_map(splitByComma(...), $input->getOption('tags')))); $maxVisits = $input->getOption('max-visits'); - return $maxVisits !== null ? (int) $maxVisits : null; - } - - public function crawlable(InputInterface $input): bool - { - return $input->getOption('crawlable'); - } - - public function noForwardQuery(InputInterface $input): bool - { - return $input->getOption('no-forward-query'); - } - public function toShortUrlEdition(InputInterface $input): ShortUrlEdition - { - return ShortUrlEdition::fromRawData([ - ShortUrlInputFilter::LONG_URL => $this->longUrl($input), - ShortUrlInputFilter::VALID_SINCE => $this->validSince($input), - ShortUrlInputFilter::VALID_UNTIL => $this->validUntil($input), - ShortUrlInputFilter::MAX_VISITS => $this->maxVisits($input), - ShortUrlInputFilter::TAGS => $this->tags($input), - ShortUrlInputFilter::CRAWLABLE => $this->crawlable($input), - ShortUrlInputFilter::FORWARD_QUERY => !$this->noForwardQuery($input), -// ShortUrlInputFilter::TITLE => TODO, - ]); + return [ + ShortUrlInputFilter::LONG_URL => $longUrl, + ShortUrlInputFilter::VALID_SINCE => $input->getOption('valid-since'), + ShortUrlInputFilter::VALID_UNTIL => $input->getOption('valid-until'), + ShortUrlInputFilter::MAX_VISITS => $maxVisits !== null ? (int) $maxVisits : null, + ShortUrlInputFilter::TAGS => $tags, + ShortUrlInputFilter::TITLE => $input->getOption('title'), + ShortUrlInputFilter::CRAWLABLE => $input->getOption('crawlable'), + ShortUrlInputFilter::FORWARD_QUERY => !$input->getOption('no-forward-query'), + ]; } - - // TODO - // public function toShortUrlCreation(InputInterface $input) } From 65ea1e00a6bc4c0b7a13528f920ec3015150ba52 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 26 Jul 2024 19:26:48 +0200 Subject: [PATCH 29/41] Prevent resetting of non-providen params in EditShortUrlCommand --- module/CLI/src/Input/ShortUrlDataInput.php | 40 +++++++++++++++------- 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/module/CLI/src/Input/ShortUrlDataInput.php b/module/CLI/src/Input/ShortUrlDataInput.php index 301215473..a77de92e5 100644 --- a/module/CLI/src/Input/ShortUrlDataInput.php +++ b/module/CLI/src/Input/ShortUrlDataInput.php @@ -102,18 +102,34 @@ public function toShortUrlCreation( private function getCommonData(InputInterface $input): array { $longUrl = $this->longUrlAsOption ? $input->getOption('long-url') : $input->getArgument('longUrl'); - $tags = array_unique(flatten(array_map(splitByComma(...), $input->getOption('tags')))); - $maxVisits = $input->getOption('max-visits'); + $data = [ShortUrlInputFilter::LONG_URL => $longUrl]; - return [ - ShortUrlInputFilter::LONG_URL => $longUrl, - ShortUrlInputFilter::VALID_SINCE => $input->getOption('valid-since'), - ShortUrlInputFilter::VALID_UNTIL => $input->getOption('valid-until'), - ShortUrlInputFilter::MAX_VISITS => $maxVisits !== null ? (int) $maxVisits : null, - ShortUrlInputFilter::TAGS => $tags, - ShortUrlInputFilter::TITLE => $input->getOption('title'), - ShortUrlInputFilter::CRAWLABLE => $input->getOption('crawlable'), - ShortUrlInputFilter::FORWARD_QUERY => !$input->getOption('no-forward-query'), - ]; + // Avoid setting arguments that were not explicitly provided. + // This is important when editing short URLs and should not make a difference when creating. + if ($input->hasParameterOption(['--valid-since', '-s'])) { + $data[ShortUrlInputFilter::VALID_SINCE] = $input->getOption('valid-since'); + } + if ($input->hasParameterOption(['--valid-until', '-v'])) { + $data[ShortUrlInputFilter::VALID_UNTIL] = $input->getOption('valid-until'); + } + if ($input->hasParameterOption(['--max-visits', '-m'])) { + $maxVisits = $input->getOption('max-visits'); + $data[ShortUrlInputFilter::MAX_VISITS] = $maxVisits !== null ? (int) $maxVisits : null; + } + if ($input->hasParameterOption(['--tags', '-t'])) { + $tags = array_unique(flatten(array_map(splitByComma(...), $input->getOption('tags')))); + $data[ShortUrlInputFilter::TAGS] = $tags; + } + if ($input->hasParameterOption('--title')) { + $data[ShortUrlInputFilter::TITLE] = $input->getOption('title'); + } + if ($input->hasParameterOption(['--crawlable', '-r'])) { + $data[ShortUrlInputFilter::CRAWLABLE] = $input->getOption('crawlable'); + } + if ($input->hasParameterOption(['--no-forward-query', '-w'])) { + $data[ShortUrlInputFilter::FORWARD_QUERY] = !$input->getOption('no-forward-query'); + } + + return $data; } } From df94c68e2ebf15f1b5c91cf5852d9cc8569db034 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 26 Jul 2024 19:52:09 +0200 Subject: [PATCH 30/41] Add unit test for EditShortUrlCommand --- CHANGELOG.md | 3 + .../ShortUrl/EditShortUrlCommandTest.php | 74 +++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 module/CLI/test/Command/ShortUrl/EditShortUrlCommandTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index ec85e5ccb..307b510d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#2018](https://github.com/shlinkio/shlink/issues/2018) Add option to allow all short URLs to be unconditionally crawlable in robots.txt, via `ROBOTS_ALLOW_ALL_SHORT_URLS=true` env var, or config option. * [#2109](https://github.com/shlinkio/shlink/issues/2109) Add option to customize user agents robots.txt, via `ROBOTS_USER_AGENTS=foo,bar,baz` env var, or config option. +* [#2163](https://github.com/shlinkio/shlink/issues/2163) Add `short-urls:edit` command to edit existing short URLs. + + This brings CLI and API interfaces capabilities closer, and solves an overlook since the feature was implemented years ago. ### Changed * [#2096](https://github.com/shlinkio/shlink/issues/2096) Update to RoadRunner 2024. diff --git a/module/CLI/test/Command/ShortUrl/EditShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/EditShortUrlCommandTest.php new file mode 100644 index 000000000..f540b5dcb --- /dev/null +++ b/module/CLI/test/Command/ShortUrl/EditShortUrlCommandTest.php @@ -0,0 +1,74 @@ +shortUrlService = $this->createMock(ShortUrlServiceInterface::class); + $this->stringifier = $this->createMock(ShortUrlStringifierInterface::class); + + $command = new EditShortUrlCommand($this->shortUrlService, $this->stringifier); + $this->commandTester = CliTestUtils::testerForCommand($command); + } + + #[Test] + public function successMessageIsPrintedIfNoErrorOccurs(): void + { + $this->shortUrlService->expects($this->once())->method('updateShortUrl')->willReturn( + ShortUrl::createFake(), + ); + $this->stringifier->expects($this->once())->method('stringify')->willReturn('https://s.test/foo'); + + $this->commandTester->execute(['shortCode' => 'foobar']); + $output = $this->commandTester->getDisplay(); + $exitCode = $this->commandTester->getStatusCode(); + + self::assertStringContainsString('Short URL "https://s.test/foo" properly edited', $output); + self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + } + + #[Test] + #[TestWith([OutputInterface::VERBOSITY_NORMAL])] + #[TestWith([OutputInterface::VERBOSITY_VERBOSE])] + #[TestWith([OutputInterface::VERBOSITY_VERY_VERBOSE])] + #[TestWith([OutputInterface::VERBOSITY_DEBUG])] + public function errorIsPrintedInCaseOfFailure(int $verbosity): void + { + $e = ShortUrlNotFoundException::fromNotFound(ShortUrlIdentifier::fromShortCodeAndDomain('foo')); + $this->shortUrlService->expects($this->once())->method('updateShortUrl')->willThrowException($e); + $this->stringifier->expects($this->never())->method('stringify'); + + $this->commandTester->execute(['shortCode' => 'foo'], ['verbosity' => $verbosity]); + $output = $this->commandTester->getDisplay(); + $exitCode = $this->commandTester->getStatusCode(); + + self::assertStringContainsString('Short URL not found for "foo"', $output); + if ($verbosity >= OutputInterface::VERBOSITY_VERBOSE) { + self::assertStringContainsString('Exception trace:', $output); + } else { + self::assertStringNotContainsString('Exception trace:', $output); + } + self::assertEquals(ExitCode::EXIT_FAILURE, $exitCode); + } +} From a1afc90150e718b65bccaf332b66daf5b124a1aa Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 26 Jul 2024 20:04:13 +0200 Subject: [PATCH 31/41] Fix sqlcmd path --- .github/workflows/ci-db-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-db-tests.yml b/.github/workflows/ci-db-tests.yml index ba45e9f40..33bf8f887 100644 --- a/.github/workflows/ci-db-tests.yml +++ b/.github/workflows/ci-db-tests.yml @@ -32,7 +32,7 @@ jobs: extensions-cache-key: db-tests-extensions-${{ matrix.php-version }}-${{ inputs.platform }} - name: Create test database if: ${{ inputs.platform == 'ms' }} - run: docker compose exec -T shlink_db_ms /opt/mssql-tools/bin/sqlcmd -S localhost -U sa -P 'Passw0rd!' -Q "CREATE DATABASE shlink_test;" + run: docker compose exec -T shlink_db_ms /opt/mssql-tools18/bin/sqlcmd -C -S localhost -U sa -P 'Passw0rd!' -Q "CREATE DATABASE shlink_test;" - name: Run tests run: composer test:db:${{ inputs.platform }} - name: Upload code coverage From b9ba1246d4dddec1220d03e55c9825d4e7ab53a2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 27 Jul 2024 09:12:54 +0200 Subject: [PATCH 32/41] Reduce hardcoded options in ShortUrlDataInput --- module/CLI/src/Input/ShortUrlDataInput.php | 45 +++++++++++---------- module/CLI/src/Input/ShortUrlDataOption.php | 41 +++++++++++++++++++ 2 files changed, 64 insertions(+), 22 deletions(-) create mode 100644 module/CLI/src/Input/ShortUrlDataOption.php diff --git a/module/CLI/src/Input/ShortUrlDataInput.php b/module/CLI/src/Input/ShortUrlDataInput.php index a77de92e5..10c2da7c0 100644 --- a/module/CLI/src/Input/ShortUrlDataInput.php +++ b/module/CLI/src/Input/ShortUrlDataInput.php @@ -30,45 +30,46 @@ public function __construct(Command $command, private bool $longUrlAsOption = fa $command ->addOption( - 'tags', - 't', + ShortUrlDataOption::TAGS->value, + ShortUrlDataOption::TAGS->shortcut(), InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED, 'Tags to apply to the short URL', ) ->addOption( - 'valid-since', - 's', + ShortUrlDataOption::VALID_SINCE->value, + ShortUrlDataOption::VALID_SINCE->shortcut(), InputOption::VALUE_REQUIRED, 'The date from which this short URL will be valid. ' . 'If someone tries to access it before this date, it will not be found.', ) ->addOption( - 'valid-until', - 'u', + ShortUrlDataOption::VALID_UNTIL->value, + ShortUrlDataOption::VALID_UNTIL->shortcut(), InputOption::VALUE_REQUIRED, 'The date until which this short URL will be valid. ' . 'If someone tries to access it after this date, it will not be found.', ) ->addOption( - 'max-visits', - 'm', + ShortUrlDataOption::MAX_VISITS->value, + ShortUrlDataOption::MAX_VISITS->shortcut(), InputOption::VALUE_REQUIRED, 'This will limit the number of visits for this short URL.', ) ->addOption( - 'title', - mode: InputOption::VALUE_REQUIRED, - description: 'A descriptive title for the short URL.', + ShortUrlDataOption::TITLE->value, + ShortUrlDataOption::TITLE->shortcut(), + InputOption::VALUE_REQUIRED, + 'A descriptive title for the short URL.', ) ->addOption( - 'crawlable', - 'r', + ShortUrlDataOption::CRAWLABLE->value, + ShortUrlDataOption::CRAWLABLE->shortcut(), InputOption::VALUE_NONE, 'Tells if this short URL will be included as "Allow" in Shlink\'s robots.txt.', ) ->addOption( - 'no-forward-query', - 'w', + ShortUrlDataOption::NO_FORWARD_QUERY->value, + ShortUrlDataOption::NO_FORWARD_QUERY->shortcut(), InputOption::VALUE_NONE, 'Disables the forwarding of the query string to the long URL, when the short URL is visited.', ); @@ -106,27 +107,27 @@ private function getCommonData(InputInterface $input): array // Avoid setting arguments that were not explicitly provided. // This is important when editing short URLs and should not make a difference when creating. - if ($input->hasParameterOption(['--valid-since', '-s'])) { + if (ShortUrlDataOption::VALID_SINCE->wasProvided($input)) { $data[ShortUrlInputFilter::VALID_SINCE] = $input->getOption('valid-since'); } - if ($input->hasParameterOption(['--valid-until', '-v'])) { + if (ShortUrlDataOption::VALID_UNTIL->wasProvided($input)) { $data[ShortUrlInputFilter::VALID_UNTIL] = $input->getOption('valid-until'); } - if ($input->hasParameterOption(['--max-visits', '-m'])) { + if (ShortUrlDataOption::MAX_VISITS->wasProvided($input)) { $maxVisits = $input->getOption('max-visits'); $data[ShortUrlInputFilter::MAX_VISITS] = $maxVisits !== null ? (int) $maxVisits : null; } - if ($input->hasParameterOption(['--tags', '-t'])) { + if (ShortUrlDataOption::TAGS->wasProvided($input)) { $tags = array_unique(flatten(array_map(splitByComma(...), $input->getOption('tags')))); $data[ShortUrlInputFilter::TAGS] = $tags; } - if ($input->hasParameterOption('--title')) { + if (ShortUrlDataOption::TITLE->wasProvided($input)) { $data[ShortUrlInputFilter::TITLE] = $input->getOption('title'); } - if ($input->hasParameterOption(['--crawlable', '-r'])) { + if (ShortUrlDataOption::CRAWLABLE->wasProvided($input)) { $data[ShortUrlInputFilter::CRAWLABLE] = $input->getOption('crawlable'); } - if ($input->hasParameterOption(['--no-forward-query', '-w'])) { + if (ShortUrlDataOption::NO_FORWARD_QUERY->wasProvided($input)) { $data[ShortUrlInputFilter::FORWARD_QUERY] = !$input->getOption('no-forward-query'); } diff --git a/module/CLI/src/Input/ShortUrlDataOption.php b/module/CLI/src/Input/ShortUrlDataOption.php new file mode 100644 index 000000000..9774d8cbb --- /dev/null +++ b/module/CLI/src/Input/ShortUrlDataOption.php @@ -0,0 +1,41 @@ + 't', + self::VALID_SINCE => 's', + self::VALID_UNTIL => 'u', + self::MAX_VISITS => 'm', + self::TITLE => null, + self::CRAWLABLE => 'r', + self::NO_FORWARD_QUERY => 'w', + }; + } + + public function wasProvided(InputInterface $input): bool + { + $option = sprintf('--%s', $this->value); + $shortcut = $this->shortcut(); + + return $input->hasParameterOption($shortcut === null ? $option : [$option, sprintf('-%s', $shortcut)]); + } +} From 6b0b52853c6d4b7711167b6eedb472ac91442959 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 28 Jul 2024 10:49:24 +0200 Subject: [PATCH 33/41] Improve repro steps description in bug issue template --- .github/ISSUE_TEMPLATE/Bug.yml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/Bug.yml b/.github/ISSUE_TEMPLATE/Bug.yml index f9671a9f5..57b99bbb2 100644 --- a/.github/ISSUE_TEMPLATE/Bug.yml +++ b/.github/ISSUE_TEMPLATE/Bug.yml @@ -61,7 +61,11 @@ body: label: Minimum steps to reproduce value: | From b52ceaff9a95e9a33dd2aca776581875441ff82c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 29 Jul 2024 19:39:31 +0200 Subject: [PATCH 34/41] Update to latest shlink-common and remove deprecation references --- composer.json | 2 +- docker-compose.yml | 2 +- .../src/Command/ShortUrl/ListShortUrlsCommand.php | 10 ++++------ .../EventDispatcher/PublishingUpdatesGenerator.php | 4 ++-- .../Transformer/ShortUrlDataTransformer.php | 11 ++--------- .../ShortUrlDataTransformerInterface.php | 13 +++++++++++++ .../ShortUrl/AbstractCreateShortUrlAction.php | 4 ++-- .../src/Action/ShortUrl/EditShortUrlAction.php | 4 ++-- .../src/Action/ShortUrl/ListShortUrlsAction.php | 12 ++++++------ .../src/Action/ShortUrl/ResolveShortUrlAction.php | 4 ++-- module/Rest/src/Action/Tag/ListTagsAction.php | 8 +++----- module/Rest/src/Action/Tag/TagsStatsAction.php | 6 ++---- .../src/Action/Visit/DeleteOrphanVisitsAction.php | 3 --- .../Rest/src/Action/Visit/DomainVisitsAction.php | 14 ++++++-------- .../src/Action/Visit/NonOrphanVisitsAction.php | 10 +++------- .../Rest/src/Action/Visit/OrphanVisitsAction.php | 8 ++------ .../Rest/src/Action/Visit/ShortUrlVisitsAction.php | 10 +++------- module/Rest/src/Action/Visit/TagVisitsAction.php | 10 +++------- .../Action/ShortUrl/CreateShortUrlActionTest.php | 6 +++--- .../SingleStepCreateShortUrlActionTest.php | 4 ++-- 20 files changed, 62 insertions(+), 83 deletions(-) create mode 100644 module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformerInterface.php diff --git a/composer.json b/composer.json index f792a468d..f695b063f 100644 --- a/composer.json +++ b/composer.json @@ -44,7 +44,7 @@ "pagerfanta/core": "^3.8", "ramsey/uuid": "^4.7", "shlinkio/doctrine-specification": "^2.1.1", - "shlinkio/shlink-common": "^6.1", + "shlinkio/shlink-common": "dev-main#144e5c1 as 6.2", "shlinkio/shlink-config": "^3.0", "shlinkio/shlink-event-dispatcher": "^4.1", "shlinkio/shlink-importer": "^5.3.2", diff --git a/docker-compose.yml b/docker-compose.yml index d1673b517..41f54cba3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -77,7 +77,7 @@ services: shlink_db_postgres: container_name: shlink_db_postgres - image: postgres:12.2-alpine + image: postgres:16.3-alpine ports: - "5434:5432" volumes: diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 4e3a7706c..1aa956311 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -9,14 +9,14 @@ use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Paginator; -use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; -use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; +use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtils; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary; use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlsParamsInputFilter; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlListServiceInterface; +use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformerInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -32,8 +32,6 @@ class ListShortUrlsCommand extends Command { - use PagerfantaUtilsTrait; - public const NAME = 'short-url:list'; private readonly StartDateOption $startDateOption; @@ -41,7 +39,7 @@ class ListShortUrlsCommand extends Command public function __construct( private readonly ShortUrlListServiceInterface $shortUrlService, - private readonly DataTransformerInterface $transformer, + private readonly ShortUrlDataTransformerInterface $transformer, ) { parent::__construct(); $this->startDateOption = new StartDateOption($this, 'short URLs'); @@ -196,7 +194,7 @@ private function renderPage( ShlinkTable::default($output)->render( array_keys($columnsMap), $rows, - $all ? null : $this->formatCurrentPageMessage($shortUrls, 'Page %s of %s'), + $all ? null : PagerfantaUtils::formatCurrentPageMessage($shortUrls, 'Page %s of %s'), ); return $shortUrls; diff --git a/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php b/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php index 82ada6e17..211768b39 100644 --- a/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php +++ b/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php @@ -4,14 +4,14 @@ namespace Shlinkio\Shlink\Core\EventDispatcher; -use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Common\UpdatePublishing\Update; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; +use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformerInterface; use Shlinkio\Shlink\Core\Visit\Entity\Visit; final readonly class PublishingUpdatesGenerator implements PublishingUpdatesGeneratorInterface { - public function __construct(private DataTransformerInterface $shortUrlTransformer) + public function __construct(private ShortUrlDataTransformerInterface $shortUrlTransformer) { } diff --git a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php index 413f5a690..d2bdb73a9 100644 --- a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php +++ b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php @@ -4,24 +4,17 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Transformer; -use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary; -/** - * @fixme Do not implement DataTransformerInterface, but a separate interface - */ -readonly class ShortUrlDataTransformer implements DataTransformerInterface +readonly class ShortUrlDataTransformer implements ShortUrlDataTransformerInterface { public function __construct(private ShortUrlStringifierInterface $stringifier) { } - /** - * @param ShortUrlWithVisitsSummary|ShortUrl $data - */ - public function transform($data): array // phpcs:ignore + public function transform(ShortUrlWithVisitsSummary|ShortUrl $data): array { $shortUrl = $data instanceof ShortUrlWithVisitsSummary ? $data->shortUrl : $data; return [ diff --git a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformerInterface.php b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformerInterface.php new file mode 100644 index 000000000..e1101f701 --- /dev/null +++ b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformerInterface.php @@ -0,0 +1,13 @@ +getQueryParams()), AuthenticationMiddleware::apiKeyFromRequest($request), ); - return new JsonResponse(['shortUrls' => $this->serializePaginator($shortUrls, $this->transformer)]); + return new JsonResponse([ + 'shortUrls' => PagerfantaUtils::serializePaginator($shortUrls, $this->transformer->transform(...)), + ]); } } diff --git a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php index 0fb2cc10f..39a9d7f22 100644 --- a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php @@ -7,9 +7,9 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; -use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; +use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformerInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; @@ -20,7 +20,7 @@ class ResolveShortUrlAction extends AbstractRestAction public function __construct( private readonly ShortUrlResolverInterface $urlResolver, - private readonly DataTransformerInterface $transformer, + private readonly ShortUrlDataTransformerInterface $transformer, ) { } diff --git a/module/Rest/src/Action/Tag/ListTagsAction.php b/module/Rest/src/Action/Tag/ListTagsAction.php index 13898584e..426f94e75 100644 --- a/module/Rest/src/Action/Tag/ListTagsAction.php +++ b/module/Rest/src/Action/Tag/ListTagsAction.php @@ -7,7 +7,7 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; +use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtils; use Shlinkio\Shlink\Core\Tag\Model\TagsParams; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -15,12 +15,10 @@ class ListTagsAction extends AbstractRestAction { - use PagerfantaUtilsTrait; - protected const ROUTE_PATH = '/tags'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - public function __construct(private TagServiceInterface $tagService) + public function __construct(private readonly TagServiceInterface $tagService) { } @@ -30,7 +28,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); return new JsonResponse([ - 'tags' => $this->serializePaginator($this->tagService->listTags($params, $apiKey)), + 'tags' => PagerfantaUtils::serializePaginator($this->tagService->listTags($params, $apiKey)), ]); } } diff --git a/module/Rest/src/Action/Tag/TagsStatsAction.php b/module/Rest/src/Action/Tag/TagsStatsAction.php index 6db3c62ab..1ae470e07 100644 --- a/module/Rest/src/Action/Tag/TagsStatsAction.php +++ b/module/Rest/src/Action/Tag/TagsStatsAction.php @@ -7,7 +7,7 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; +use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtils; use Shlinkio\Shlink\Core\Tag\Model\TagsParams; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -15,8 +15,6 @@ class TagsStatsAction extends AbstractRestAction { - use PagerfantaUtilsTrait; - protected const ROUTE_PATH = '/tags/stats'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; @@ -30,6 +28,6 @@ public function handle(ServerRequestInterface $request): ResponseInterface $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); $tagsInfo = $this->tagService->tagsInfo($params, $apiKey); - return new JsonResponse(['tags' => $this->serializePaginator($tagsInfo)]); + return new JsonResponse(['tags' => PagerfantaUtils::serializePaginator($tagsInfo)]); } } diff --git a/module/Rest/src/Action/Visit/DeleteOrphanVisitsAction.php b/module/Rest/src/Action/Visit/DeleteOrphanVisitsAction.php index d1d2bc846..3a0164518 100644 --- a/module/Rest/src/Action/Visit/DeleteOrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/DeleteOrphanVisitsAction.php @@ -7,15 +7,12 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; use Shlinkio\Shlink\Core\Visit\VisitsDeleterInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; class DeleteOrphanVisitsAction extends AbstractRestAction { - use PagerfantaUtilsTrait; - protected const ROUTE_PATH = '/visits/orphan'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_DELETE]; diff --git a/module/Rest/src/Action/Visit/DomainVisitsAction.php b/module/Rest/src/Action/Visit/DomainVisitsAction.php index 7dce57fbb..91f071b5f 100644 --- a/module/Rest/src/Action/Visit/DomainVisitsAction.php +++ b/module/Rest/src/Action/Visit/DomainVisitsAction.php @@ -7,7 +7,7 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; -use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; +use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtils; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -15,13 +15,13 @@ class DomainVisitsAction extends AbstractRestAction { - use PagerfantaUtilsTrait; - protected const ROUTE_PATH = '/domains/{domain}/visits'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - public function __construct(private VisitsStatsHelperInterface $visitsHelper, private string $defaultDomain) - { + public function __construct( + private readonly VisitsStatsHelperInterface $visitsHelper, + private readonly string $defaultDomain, + ) { } public function handle(Request $request): Response @@ -31,9 +31,7 @@ public function handle(Request $request): Response $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); $visits = $this->visitsHelper->visitsForDomain($domain, $params, $apiKey); - return new JsonResponse([ - 'visits' => $this->serializePaginator($visits), - ]); + return new JsonResponse(['visits' => PagerfantaUtils::serializePaginator($visits)]); } private function resolveDomainParam(Request $request): string diff --git a/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php b/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php index fe0f0e0c6..1fffdb8b5 100644 --- a/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php @@ -7,7 +7,7 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; +use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtils; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -15,12 +15,10 @@ class NonOrphanVisitsAction extends AbstractRestAction { - use PagerfantaUtilsTrait; - protected const ROUTE_PATH = '/visits/non-orphan'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - public function __construct(private VisitsStatsHelperInterface $visitsHelper) + public function __construct(private readonly VisitsStatsHelperInterface $visitsHelper) { } @@ -30,8 +28,6 @@ public function handle(ServerRequestInterface $request): ResponseInterface $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); $visits = $this->visitsHelper->nonOrphanVisits($params, $apiKey); - return new JsonResponse([ - 'visits' => $this->serializePaginator($visits), - ]); + return new JsonResponse(['visits' => PagerfantaUtils::serializePaginator($visits)]); } } diff --git a/module/Rest/src/Action/Visit/OrphanVisitsAction.php b/module/Rest/src/Action/Visit/OrphanVisitsAction.php index 0224022d4..7906fdaec 100644 --- a/module/Rest/src/Action/Visit/OrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/OrphanVisitsAction.php @@ -7,7 +7,7 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; +use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtils; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -15,8 +15,6 @@ class OrphanVisitsAction extends AbstractRestAction { - use PagerfantaUtilsTrait; - protected const ROUTE_PATH = '/visits/orphan'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; @@ -30,8 +28,6 @@ public function handle(ServerRequestInterface $request): ResponseInterface $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); $visits = $this->visitsHelper->orphanVisits($params, $apiKey); - return new JsonResponse([ - 'visits' => $this->serializePaginator($visits), - ]); + return new JsonResponse(['visits' => PagerfantaUtils::serializePaginator($visits)]); } } diff --git a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php index 52f098250..fe5099a2e 100644 --- a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php +++ b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php @@ -7,7 +7,7 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; -use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; +use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtils; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; @@ -16,12 +16,10 @@ class ShortUrlVisitsAction extends AbstractRestAction { - use PagerfantaUtilsTrait; - protected const ROUTE_PATH = '/short-urls/{shortCode}/visits'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - public function __construct(private VisitsStatsHelperInterface $visitsHelper) + public function __construct(private readonly VisitsStatsHelperInterface $visitsHelper) { } @@ -32,8 +30,6 @@ public function handle(Request $request): Response $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); $visits = $this->visitsHelper->visitsForShortUrl($identifier, $params, $apiKey); - return new JsonResponse([ - 'visits' => $this->serializePaginator($visits), - ]); + return new JsonResponse(['visits' => PagerfantaUtils::serializePaginator($visits)]); } } diff --git a/module/Rest/src/Action/Visit/TagVisitsAction.php b/module/Rest/src/Action/Visit/TagVisitsAction.php index 8b88a2cd7..1739264fc 100644 --- a/module/Rest/src/Action/Visit/TagVisitsAction.php +++ b/module/Rest/src/Action/Visit/TagVisitsAction.php @@ -7,7 +7,7 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; -use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; +use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtils; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -15,12 +15,10 @@ class TagVisitsAction extends AbstractRestAction { - use PagerfantaUtilsTrait; - protected const ROUTE_PATH = '/tags/{tag}/visits'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - public function __construct(private VisitsStatsHelperInterface $visitsHelper) + public function __construct(private readonly VisitsStatsHelperInterface $visitsHelper) { } @@ -31,8 +29,6 @@ public function handle(Request $request): Response $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); $visits = $this->visitsHelper->visitsForTag($tag, $params, $apiKey); - return new JsonResponse([ - 'visits' => $this->serializePaginator($visits), - ]); + return new JsonResponse(['visits' => PagerfantaUtils::serializePaginator($visits)]); } } diff --git a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php index c79eab86a..7361df5c2 100644 --- a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php @@ -12,12 +12,12 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\UrlShorteningResult; +use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformerInterface; use Shlinkio\Shlink\Core\ShortUrl\UrlShortener; use Shlinkio\Shlink\Rest\Action\ShortUrl\CreateShortUrlAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -26,12 +26,12 @@ class CreateShortUrlActionTest extends TestCase { private CreateShortUrlAction $action; private MockObject & UrlShortener $urlShortener; - private MockObject & DataTransformerInterface $transformer; + private MockObject & ShortUrlDataTransformerInterface $transformer; protected function setUp(): void { $this->urlShortener = $this->createMock(UrlShortener::class); - $this->transformer = $this->createMock(DataTransformerInterface::class); + $this->transformer = $this->createMock(ShortUrlDataTransformerInterface::class); $this->action = new CreateShortUrlAction($this->urlShortener, $this->transformer, new UrlShortenerOptions()); } diff --git a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php index c1ac65445..69914f0e7 100644 --- a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php @@ -8,11 +8,11 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\UrlShorteningResult; +use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformerInterface; use Shlinkio\Shlink\Core\ShortUrl\UrlShortenerInterface; use Shlinkio\Shlink\Rest\Action\ShortUrl\SingleStepCreateShortUrlAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -25,7 +25,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase protected function setUp(): void { $this->urlShortener = $this->createMock(UrlShortenerInterface::class); - $transformer = $this->createMock(DataTransformerInterface::class); + $transformer = $this->createMock(ShortUrlDataTransformerInterface::class); $transformer->method('transform')->willReturn([]); $this->action = new SingleStepCreateShortUrlAction( From 1d24750f43f1d87f8908f33c20adca4e313975dd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 29 Jul 2024 19:59:46 +0200 Subject: [PATCH 35/41] Fix phpstan checks --- .../src/EventDispatcher/PublishingUpdatesGenerator.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php b/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php index 211768b39..b762af7e8 100644 --- a/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php +++ b/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php @@ -18,7 +18,7 @@ public function __construct(private ShortUrlDataTransformerInterface $shortUrlTr public function newVisitUpdate(Visit $visit): Update { return Update::forTopicAndPayload(Topic::NEW_VISIT->value, [ - 'shortUrl' => $this->shortUrlTransformer->transform($visit->shortUrl), + 'shortUrl' => $this->transformShortUrl($visit->shortUrl), 'visit' => $visit->jsonSerialize(), ]); } @@ -36,7 +36,7 @@ public function newShortUrlVisitUpdate(Visit $visit): Update $topic = Topic::newShortUrlVisit($shortUrl?->getShortCode()); return Update::forTopicAndPayload($topic, [ - 'shortUrl' => $this->shortUrlTransformer->transform($shortUrl), + 'shortUrl' => $this->transformShortUrl($shortUrl), 'visit' => $visit->jsonSerialize(), ]); } @@ -47,4 +47,9 @@ public function newShortUrlUpdate(ShortUrl $shortUrl): Update 'shortUrl' => $this->shortUrlTransformer->transform($shortUrl), ]); } + + private function transformShortUrl(?ShortUrl $shortUrl): array + { + return $shortUrl === null ? [] : $this->shortUrlTransformer->transform($shortUrl); + } } From 037cd8a389bc34e19ea94b2199c9572b05cf3dc8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 29 Jul 2024 20:43:52 +0200 Subject: [PATCH 36/41] Add missing generic tyoes annotations --- .../Command/Domain/GetDomainVisitsCommand.php | 3 +++ .../ShortUrl/GetShortUrlVisitsCommand.php | 3 +++ .../Command/ShortUrl/ListShortUrlsCommand.php | 1 + .../src/Command/Tag/GetTagVisitsCommand.php | 3 +++ .../Visit/AbstractVisitsListCommand.php | 6 ++++++ .../Visit/GetNonOrphanVisitsCommand.php | 3 +++ .../Command/Visit/GetOrphanVisitsCommand.php | 3 +++ .../Command/Db/CreateDatabaseCommandTest.php | 2 ++ .../Domain/Repository/DomainRepository.php | 1 + .../Repository/DomainRepositoryInterface.php | 1 + .../Validation/DomainRedirectsInputFilter.php | 1 + .../src/Exception/ValidationException.php | 3 +++ ...AbstractCacheableCountPaginatorAdapter.php | 4 ++++ .../Validation/RedirectRulesInputFilter.php | 7 +++++++ module/Core/src/ShortUrl/Entity/ShortUrl.php | 6 +++--- .../Model/Validation/ShortUrlInputFilter.php | 1 + .../Validation/ShortUrlsParamsInputFilter.php | 1 + .../Adapter/ShortUrlRepositoryAdapter.php | 2 ++ .../Repository/CrawlableShortCodesQuery.php | 1 + .../Repository/ExpiredShortUrlsRepository.php | 1 + .../Repository/ShortUrlListRepository.php | 1 + .../Repository/ShortUrlRepository.php | 1 + .../ShortUrlRepositoryInterface.php | 1 + .../Core/src/ShortUrl/ShortUrlListService.php | 3 +-- .../ShortUrl/ShortUrlListServiceInterface.php | 2 +- module/Core/src/Tag/Entity/Tag.php | 2 ++ .../Adapter/AbstractTagsPaginatorAdapter.php | 4 ++++ .../Adapter/TagsInfoPaginatorAdapter.php | 2 ++ .../Adapter/TagsPaginatorAdapter.php | 2 ++ .../Core/src/Tag/Repository/TagRepository.php | 1 + .../Tag/Repository/TagRepositoryInterface.php | 2 ++ module/Core/src/Tag/TagService.php | 21 ++++++++++--------- module/Core/src/Tag/TagServiceInterface.php | 4 ++-- .../Adapter/DomainVisitsPaginatorAdapter.php | 12 +++++++---- .../NonOrphanVisitsPaginatorAdapter.php | 8 ++++--- .../Adapter/OrphanVisitsPaginatorAdapter.php | 2 ++ .../ShortUrlVisitsPaginatorAdapter.php | 10 +++++---- .../Adapter/TagVisitsPaginatorAdapter.php | 10 +++++---- .../OrphanVisitsCountRepository.php | 1 + .../ShortUrlVisitsCountRepository.php | 2 ++ .../Repository/VisitDeleterRepository.php | 1 + .../Repository/VisitIterationRepository.php | 1 + .../src/Visit/Repository/VisitRepository.php | 1 + .../Repository/VisitRepositoryInterface.php | 3 +++ module/Core/src/Visit/VisitsStatsHelper.php | 15 ++++++------- .../src/Visit/VisitsStatsHelperInterface.php | 10 ++++----- .../Listener/OrphanVisitsCountTrackerTest.php | 4 ++-- .../ShortUrlVisitsCountTrackerTest.php | 4 ++-- .../Entity/ShortUrlRedirectRuleTest.php | 2 +- .../ShortUrlTitleResolutionHelperTest.php | 3 +++ .../ApiKey/Repository/ApiKeyRepository.php | 3 +++ .../Repository/ApiKeyRepositoryInterface.php | 3 +++ phpstan.neon | 1 - 53 files changed, 144 insertions(+), 51 deletions(-) diff --git a/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php b/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php index dd3797f87..6477539eb 100644 --- a/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php +++ b/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php @@ -33,6 +33,9 @@ protected function configure(): void ->addArgument('domain', InputArgument::REQUIRED, 'The domain which visits we want to get.'); } + /** + * @return Paginator + */ protected function getVisitsPaginator(InputInterface $input, DateRange $dateRange): Paginator { $domain = $input->getArgument('domain'); diff --git a/module/CLI/src/Command/ShortUrl/GetShortUrlVisitsCommand.php b/module/CLI/src/Command/ShortUrl/GetShortUrlVisitsCommand.php index 8a6622091..8583a242e 100644 --- a/module/CLI/src/Command/ShortUrl/GetShortUrlVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/GetShortUrlVisitsCommand.php @@ -46,6 +46,9 @@ protected function interact(InputInterface $input, OutputInterface $output): voi } } + /** + * @return Paginator + */ protected function getVisitsPaginator(InputInterface $input, DateRange $dateRange): Paginator { $identifier = $this->shortUrlIdentifierInput->toShortUrlIdentifier($input); diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 1aa956311..6d38ff0ff 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -177,6 +177,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int /** * @param array $columnsMap + * @return Paginator */ private function renderPage( OutputInterface $output, diff --git a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php index 1dfd0ba9b..18da41fbf 100644 --- a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php +++ b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php @@ -33,6 +33,9 @@ protected function configure(): void ->addArgument('tag', InputArgument::REQUIRED, 'The tag which visits we want to get.'); } + /** + * @return Paginator + */ protected function getVisitsPaginator(InputInterface $input, DateRange $dateRange): Paginator { $tag = $input->getArgument('tag'); diff --git a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php index d3a49c53e..dea28e920 100644 --- a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php +++ b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php @@ -46,6 +46,9 @@ final protected function execute(InputInterface $input, OutputInterface $output) return ExitCode::EXIT_SUCCESS; } + /** + * @param Paginator $paginator + */ private function resolveRowsAndHeaders(Paginator $paginator): array { $extraKeys = []; @@ -74,6 +77,9 @@ private function resolveRowsAndHeaders(Paginator $paginator): array ]; } + /** + * @return Paginator + */ abstract protected function getVisitsPaginator(InputInterface $input, DateRange $dateRange): Paginator; /** diff --git a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php index 1462620da..580355095 100644 --- a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php @@ -30,6 +30,9 @@ protected function configure(): void ->setDescription('Returns the list of non-orphan visits.'); } + /** + * @return Paginator + */ protected function getVisitsPaginator(InputInterface $input, DateRange $dateRange): Paginator { return $this->visitsHelper->nonOrphanVisits(new VisitsParams($dateRange)); diff --git a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php index d495db77c..ea5c0fe2d 100644 --- a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php @@ -30,6 +30,9 @@ enumToString(OrphanVisitType::class), )); } + /** + * @return Paginator + */ protected function getVisitsPaginator(InputInterface $input, DateRange $dateRange): Paginator { $rawType = $input->getOption('type'); diff --git a/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php b/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php index b4a5c8405..9563ef27a 100644 --- a/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php +++ b/module/CLI/test/Command/Db/CreateDatabaseCommandTest.php @@ -7,6 +7,7 @@ use Doctrine\DBAL\Connection; use Doctrine\DBAL\Driver; use Doctrine\DBAL\Platforms\AbstractPlatform; +use Doctrine\DBAL\Platforms\SQLitePlatform; use Doctrine\DBAL\Schema\AbstractSchemaManager; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Mapping\ClassMetadata; @@ -31,6 +32,7 @@ class CreateDatabaseCommandTest extends TestCase private MockObject & ProcessRunnerInterface $processHelper; private MockObject & Connection $regularConn; private MockObject & ClassMetadataFactory $metadataFactory; + /** @var MockObject&AbstractSchemaManager */ private MockObject & AbstractSchemaManager $schemaManager; private MockObject & Driver $driver; diff --git a/module/Core/src/Domain/Repository/DomainRepository.php b/module/Core/src/Domain/Repository/DomainRepository.php index aae44aed1..fedf4f540 100644 --- a/module/Core/src/Domain/Repository/DomainRepository.php +++ b/module/Core/src/Domain/Repository/DomainRepository.php @@ -14,6 +14,7 @@ use Shlinkio\Shlink\Rest\ApiKey\Role; use Shlinkio\Shlink\Rest\Entity\ApiKey; +/** @extends EntitySpecificationRepository */ class DomainRepository extends EntitySpecificationRepository implements DomainRepositoryInterface { /** diff --git a/module/Core/src/Domain/Repository/DomainRepositoryInterface.php b/module/Core/src/Domain/Repository/DomainRepositoryInterface.php index d703542b6..d215e475c 100644 --- a/module/Core/src/Domain/Repository/DomainRepositoryInterface.php +++ b/module/Core/src/Domain/Repository/DomainRepositoryInterface.php @@ -9,6 +9,7 @@ use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Rest\Entity\ApiKey; +/** @extends ObjectRepository */ interface DomainRepositoryInterface extends ObjectRepository, EntitySpecificationRepositoryInterface { /** diff --git a/module/Core/src/Domain/Validation/DomainRedirectsInputFilter.php b/module/Core/src/Domain/Validation/DomainRedirectsInputFilter.php index 48035c6c7..d448b8198 100644 --- a/module/Core/src/Domain/Validation/DomainRedirectsInputFilter.php +++ b/module/Core/src/Domain/Validation/DomainRedirectsInputFilter.php @@ -8,6 +8,7 @@ use Shlinkio\Shlink\Common\Validation\HostAndPortValidator; use Shlinkio\Shlink\Common\Validation\InputFactory; +/** @extends InputFilter */ class DomainRedirectsInputFilter extends InputFilter { public const DOMAIN = 'domain'; diff --git a/module/Core/src/Exception/ValidationException.php b/module/Core/src/Exception/ValidationException.php index dcb11fa49..95da6d5ef 100644 --- a/module/Core/src/Exception/ValidationException.php +++ b/module/Core/src/Exception/ValidationException.php @@ -26,6 +26,9 @@ class ValidationException extends InvalidArgumentException implements ProblemDet private array $invalidElements; + /** + * @param InputFilterInterface $inputFilter + */ public static function fromInputFilter(InputFilterInterface $inputFilter, ?Throwable $prev = null): self { return static::fromArray($inputFilter->getMessages(), $prev); diff --git a/module/Core/src/Paginator/Adapter/AbstractCacheableCountPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/AbstractCacheableCountPaginatorAdapter.php index 217d5effd..890c8845b 100644 --- a/module/Core/src/Paginator/Adapter/AbstractCacheableCountPaginatorAdapter.php +++ b/module/Core/src/Paginator/Adapter/AbstractCacheableCountPaginatorAdapter.php @@ -6,6 +6,10 @@ use Pagerfanta\Adapter\AdapterInterface; +/** + * @template T + * @implements AdapterInterface + */ abstract class AbstractCacheableCountPaginatorAdapter implements AdapterInterface { private ?int $count = null; diff --git a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php index 892b93e49..c2fee661b 100644 --- a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php +++ b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php @@ -17,6 +17,7 @@ use function Shlinkio\Shlink\Core\ArrayUtils\contains; use function Shlinkio\Shlink\Core\enumValues; +/** @extends InputFilter */ class RedirectRulesInputFilter extends InputFilter { public const REDIRECT_RULES = 'redirectRules'; @@ -44,6 +45,9 @@ public static function initialize(array $rawData): self return $instance; } + /** + * @return InputFilter + */ private static function createRedirectRuleInputFilter(): InputFilter { $redirectRuleInputFilter = new InputFilter(); @@ -60,6 +64,9 @@ private static function createRedirectRuleInputFilter(): InputFilter return $redirectRuleInputFilter; } + /** + * @return InputFilter + */ private static function createRedirectConditionInputFilter(): InputFilter { $redirectConditionInputFilter = new InputFilter(); diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index 084acabda..e394fb5ac 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -37,8 +37,8 @@ class ShortUrl extends AbstractEntity { /** * @param Collection $tags - * @param Collection & Selectable $visits - * @param Collection & Selectable $visitsCounts + * @param Collection & Selectable $visits + * @param Collection & Selectable $visitsCounts */ private function __construct( private string $longUrl, @@ -213,7 +213,7 @@ public function mostRecentImportedVisitDate(): ?Chronos } /** - * @param Collection & Selectable $visits + * @param Collection & Selectable $visits * @internal */ public function setVisits(Collection & Selectable $visits): self diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php index e8d35284d..f4a35969d 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php @@ -20,6 +20,7 @@ use const Shlinkio\Shlink\LOOSE_URI_MATCHER; use const Shlinkio\Shlink\MIN_SHORT_CODES_LENGTH; +/** @extends InputFilter */ class ShortUrlInputFilter extends InputFilter { // Fields for creation only diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php index f4f7c3382..0a0d45ed6 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php @@ -13,6 +13,7 @@ use function Shlinkio\Shlink\Core\enumValues; +/** @extends InputFilter */ class ShortUrlsParamsInputFilter extends InputFilter { public const PAGE = 'page'; diff --git a/module/Core/src/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapter.php b/module/Core/src/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapter.php index 56f8e5a50..ac3379df1 100644 --- a/module/Core/src/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapter.php +++ b/module/Core/src/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapter.php @@ -6,11 +6,13 @@ use Pagerfanta\Adapter\AdapterInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlListRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; +/** @implements AdapterInterface */ readonly class ShortUrlRepositoryAdapter implements AdapterInterface { public function __construct( diff --git a/module/Core/src/ShortUrl/Repository/CrawlableShortCodesQuery.php b/module/Core/src/ShortUrl/Repository/CrawlableShortCodesQuery.php index 89aef69e9..2831269dd 100644 --- a/module/Core/src/ShortUrl/Repository/CrawlableShortCodesQuery.php +++ b/module/Core/src/ShortUrl/Repository/CrawlableShortCodesQuery.php @@ -7,6 +7,7 @@ use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; +/** @extends EntitySpecificationRepository */ class CrawlableShortCodesQuery extends EntitySpecificationRepository implements CrawlableShortCodesQueryInterface { /** diff --git a/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepository.php b/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepository.php index 0b796971e..05944f29a 100644 --- a/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepository.php +++ b/module/Core/src/ShortUrl/Repository/ExpiredShortUrlsRepository.php @@ -13,6 +13,7 @@ use function sprintf; +/** @extends EntitySpecificationRepository */ class ExpiredShortUrlsRepository extends EntitySpecificationRepository implements ExpiredShortUrlsRepositoryInterface { /** diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php index 8aac9b73d..e8fd4ac64 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php @@ -20,6 +20,7 @@ use function Shlinkio\Shlink\Core\ArrayUtils\map; use function sprintf; +/** @extends EntitySpecificationRepository */ class ShortUrlListRepository extends EntitySpecificationRepository implements ShortUrlListRepositoryInterface { /** diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php index e151a6c7b..015c8eac8 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php @@ -20,6 +20,7 @@ use function count; use function strtolower; +/** @extends EntitySpecificationRepository */ class ShortUrlRepository extends EntitySpecificationRepository implements ShortUrlRepositoryInterface { public function findOneWithDomainFallback(ShortUrlIdentifier $identifier, ShortUrlMode $shortUrlMode): ?ShortUrl diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php index 8af53cb98..d0934197a 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php @@ -13,6 +13,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; +/** @extends ObjectRepository */ interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificationRepositoryInterface { public function findOneWithDomainFallback(ShortUrlIdentifier $identifier, ShortUrlMode $shortUrlMode): ?ShortUrl; diff --git a/module/Core/src/ShortUrl/ShortUrlListService.php b/module/Core/src/ShortUrl/ShortUrlListService.php index d86c4988f..f84414547 100644 --- a/module/Core/src/ShortUrl/ShortUrlListService.php +++ b/module/Core/src/ShortUrl/ShortUrlListService.php @@ -7,7 +7,6 @@ use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams; -use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary; use Shlinkio\Shlink\Core\ShortUrl\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlListRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -21,7 +20,7 @@ public function __construct( } /** - * @return ShortUrlWithVisitsSummary[]|Paginator + * @inheritDoc */ public function listShortUrls(ShortUrlsParams $params, ?ApiKey $apiKey = null): Paginator { diff --git a/module/Core/src/ShortUrl/ShortUrlListServiceInterface.php b/module/Core/src/ShortUrl/ShortUrlListServiceInterface.php index ffbb1374f..b83abd4c9 100644 --- a/module/Core/src/ShortUrl/ShortUrlListServiceInterface.php +++ b/module/Core/src/ShortUrl/ShortUrlListServiceInterface.php @@ -12,7 +12,7 @@ interface ShortUrlListServiceInterface { /** - * @return ShortUrlWithVisitsSummary[]|Paginator + * @return Paginator */ public function listShortUrls(ShortUrlsParams $params, ?ApiKey $apiKey = null): Paginator; } diff --git a/module/Core/src/Tag/Entity/Tag.php b/module/Core/src/Tag/Entity/Tag.php index 5ad272ea3..9224b38cf 100644 --- a/module/Core/src/Tag/Entity/Tag.php +++ b/module/Core/src/Tag/Entity/Tag.php @@ -7,9 +7,11 @@ use Doctrine\Common\Collections; use JsonSerializable; use Shlinkio\Shlink\Common\Entity\AbstractEntity; +use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; class Tag extends AbstractEntity implements JsonSerializable { + /** @var Collections\Collection */ private Collections\Collection $shortUrls; public function __construct(private string $name) diff --git a/module/Core/src/Tag/Paginator/Adapter/AbstractTagsPaginatorAdapter.php b/module/Core/src/Tag/Paginator/Adapter/AbstractTagsPaginatorAdapter.php index 49f9c7216..98126e278 100644 --- a/module/Core/src/Tag/Paginator/Adapter/AbstractTagsPaginatorAdapter.php +++ b/module/Core/src/Tag/Paginator/Adapter/AbstractTagsPaginatorAdapter.php @@ -12,6 +12,10 @@ use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin; use Shlinkio\Shlink\Rest\Entity\ApiKey; +/** + * @template T + * @implements AdapterInterface + */ abstract class AbstractTagsPaginatorAdapter implements AdapterInterface { public function __construct( diff --git a/module/Core/src/Tag/Paginator/Adapter/TagsInfoPaginatorAdapter.php b/module/Core/src/Tag/Paginator/Adapter/TagsInfoPaginatorAdapter.php index c29172007..2c1a481a0 100644 --- a/module/Core/src/Tag/Paginator/Adapter/TagsInfoPaginatorAdapter.php +++ b/module/Core/src/Tag/Paginator/Adapter/TagsInfoPaginatorAdapter.php @@ -4,8 +4,10 @@ namespace Shlinkio\Shlink\Core\Tag\Paginator\Adapter; +use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\Model\TagsListFiltering; +/** @extends AbstractTagsPaginatorAdapter */ class TagsInfoPaginatorAdapter extends AbstractTagsPaginatorAdapter { public function getSlice(int $offset, int $length): iterable diff --git a/module/Core/src/Tag/Paginator/Adapter/TagsPaginatorAdapter.php b/module/Core/src/Tag/Paginator/Adapter/TagsPaginatorAdapter.php index 7d54940ee..e6cb400c0 100644 --- a/module/Core/src/Tag/Paginator/Adapter/TagsPaginatorAdapter.php +++ b/module/Core/src/Tag/Paginator/Adapter/TagsPaginatorAdapter.php @@ -5,8 +5,10 @@ namespace Shlinkio\Shlink\Core\Tag\Paginator\Adapter; use Happyr\DoctrineSpecification\Spec; +use Shlinkio\Shlink\Core\Tag\Entity\Tag; use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin; +/** @extends AbstractTagsPaginatorAdapter */ class TagsPaginatorAdapter extends AbstractTagsPaginatorAdapter { public function getSlice(int $offset, int $length): iterable diff --git a/module/Core/src/Tag/Repository/TagRepository.php b/module/Core/src/Tag/Repository/TagRepository.php index c63d461d8..a2820e7b8 100644 --- a/module/Core/src/Tag/Repository/TagRepository.php +++ b/module/Core/src/Tag/Repository/TagRepository.php @@ -23,6 +23,7 @@ use const PHP_INT_MAX; +/** @extends EntitySpecificationRepository */ class TagRepository extends EntitySpecificationRepository implements TagRepositoryInterface { public function deleteByName(array $names): int diff --git a/module/Core/src/Tag/Repository/TagRepositoryInterface.php b/module/Core/src/Tag/Repository/TagRepositoryInterface.php index 44ecfbf10..ccb33de04 100644 --- a/module/Core/src/Tag/Repository/TagRepositoryInterface.php +++ b/module/Core/src/Tag/Repository/TagRepositoryInterface.php @@ -6,10 +6,12 @@ use Doctrine\Persistence\ObjectRepository; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepositoryInterface; +use Shlinkio\Shlink\Core\Tag\Entity\Tag; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\Model\TagsListFiltering; use Shlinkio\Shlink\Rest\Entity\ApiKey; +/** @extends ObjectRepository */ interface TagRepositoryInterface extends ObjectRepository, EntitySpecificationRepositoryInterface { public function deleteByName(array $names): int; diff --git a/module/Core/src/Tag/TagService.php b/module/Core/src/Tag/TagService.php index 36fd0514a..de16fada1 100644 --- a/module/Core/src/Tag/TagService.php +++ b/module/Core/src/Tag/TagService.php @@ -11,7 +11,6 @@ use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Tag\Entity\Tag; -use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; use Shlinkio\Shlink\Core\Tag\Model\TagsParams; use Shlinkio\Shlink\Core\Tag\Paginator\Adapter\TagsInfoPaginatorAdapter; @@ -20,14 +19,14 @@ use Shlinkio\Shlink\Core\Tag\Repository\TagRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; -class TagService implements TagServiceInterface +readonly class TagService implements TagServiceInterface { - public function __construct(private readonly ORM\EntityManagerInterface $em) + public function __construct(private ORM\EntityManagerInterface $em) { } /** - * @return Tag[]|Paginator + * @inheritDoc */ public function listTags(TagsParams $params, ?ApiKey $apiKey = null): Paginator { @@ -37,7 +36,7 @@ public function listTags(TagsParams $params, ?ApiKey $apiKey = null): Paginator } /** - * @return TagInfo[]|Paginator + * @inheritDoc */ public function tagsInfo(TagsParams $params, ?ApiKey $apiKey = null): Paginator { @@ -46,6 +45,11 @@ public function tagsInfo(TagsParams $params, ?ApiKey $apiKey = null): Paginator return $this->createPaginator(new TagsInfoPaginatorAdapter($repo, $params, $apiKey), $params); } + /** + * @template T + * @param AdapterInterface $adapter + * @return Paginator + */ private function createPaginator(AdapterInterface $adapter, TagsParams $params): Paginator { return (new Paginator($adapter)) @@ -54,8 +58,7 @@ private function createPaginator(AdapterInterface $adapter, TagsParams $params): } /** - * @param string[] $tagNames - * @throws ForbiddenTagOperationException + * @inheritDoc */ public function deleteTags(array $tagNames, ?ApiKey $apiKey = null): void { @@ -69,9 +72,7 @@ public function deleteTags(array $tagNames, ?ApiKey $apiKey = null): void } /** - * @throws TagNotFoundException - * @throws TagConflictException - * @throws ForbiddenTagOperationException + * @inheritDoc */ public function renameTag(TagRenaming $renaming, ?ApiKey $apiKey = null): Tag { diff --git a/module/Core/src/Tag/TagServiceInterface.php b/module/Core/src/Tag/TagServiceInterface.php index c4f8bd3c1..60aeb7c73 100644 --- a/module/Core/src/Tag/TagServiceInterface.php +++ b/module/Core/src/Tag/TagServiceInterface.php @@ -17,12 +17,12 @@ interface TagServiceInterface { /** - * @return Tag[]|Paginator + * @return Paginator */ public function listTags(TagsParams $params, ?ApiKey $apiKey = null): Paginator; /** - * @return TagInfo[]|Paginator + * @return Paginator */ public function tagsInfo(TagsParams $params, ?ApiKey $apiKey = null): Paginator; diff --git a/module/Core/src/Visit/Paginator/Adapter/DomainVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/DomainVisitsPaginatorAdapter.php index 6d766073d..330d86920 100644 --- a/module/Core/src/Visit/Paginator/Adapter/DomainVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/DomainVisitsPaginatorAdapter.php @@ -5,19 +5,23 @@ namespace Shlinkio\Shlink\Core\Visit\Paginator\Adapter; use Shlinkio\Shlink\Core\Paginator\Adapter\AbstractCacheableCountPaginatorAdapter; +use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; +/** + * @extends AbstractCacheableCountPaginatorAdapter + */ class DomainVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { public function __construct( - private VisitRepositoryInterface $visitRepository, - private string $domain, - private VisitsParams $params, - private ?ApiKey $apiKey, + private readonly VisitRepositoryInterface $visitRepository, + private readonly string $domain, + private readonly VisitsParams $params, + private readonly ?ApiKey $apiKey, ) { } diff --git a/module/Core/src/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapter.php index 0d8fecb97..7929bcfd1 100644 --- a/module/Core/src/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapter.php @@ -5,18 +5,20 @@ namespace Shlinkio\Shlink\Core\Visit\Paginator\Adapter; use Shlinkio\Shlink\Core\Paginator\Adapter\AbstractCacheableCountPaginatorAdapter; +use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; +/** @extends AbstractCacheableCountPaginatorAdapter */ class NonOrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { public function __construct( - private VisitRepositoryInterface $repo, - private VisitsParams $params, - private ?ApiKey $apiKey, + private readonly VisitRepositoryInterface $repo, + private readonly VisitsParams $params, + private readonly ?ApiKey $apiKey, ) { } diff --git a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php index 863460a16..9deedf9a9 100644 --- a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php @@ -5,12 +5,14 @@ namespace Shlinkio\Shlink\Core\Visit\Paginator\Adapter; use Shlinkio\Shlink\Core\Paginator\Adapter\AbstractCacheableCountPaginatorAdapter; +use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; +/** @extends AbstractCacheableCountPaginatorAdapter */ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { public function __construct( diff --git a/module/Core/src/Visit/Paginator/Adapter/ShortUrlVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/ShortUrlVisitsPaginatorAdapter.php index 1268d7b33..43bc02fff 100644 --- a/module/Core/src/Visit/Paginator/Adapter/ShortUrlVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/ShortUrlVisitsPaginatorAdapter.php @@ -6,19 +6,21 @@ use Shlinkio\Shlink\Core\Paginator\Adapter\AbstractCacheableCountPaginatorAdapter; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; +use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; +/** @extends AbstractCacheableCountPaginatorAdapter */ class ShortUrlVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { public function __construct( - private VisitRepositoryInterface $visitRepository, - private ShortUrlIdentifier $identifier, - private VisitsParams $params, - private ?ApiKey $apiKey, + private readonly VisitRepositoryInterface $visitRepository, + private readonly ShortUrlIdentifier $identifier, + private readonly VisitsParams $params, + private readonly ?ApiKey $apiKey, ) { } diff --git a/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php index 30324419b..93a182bd6 100644 --- a/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php @@ -5,19 +5,21 @@ namespace Shlinkio\Shlink\Core\Visit\Paginator\Adapter; use Shlinkio\Shlink\Core\Paginator\Adapter\AbstractCacheableCountPaginatorAdapter; +use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; +/** @extends AbstractCacheableCountPaginatorAdapter */ class TagVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { public function __construct( - private VisitRepositoryInterface $visitRepository, - private string $tag, - private VisitsParams $params, - private ?ApiKey $apiKey, + private readonly VisitRepositoryInterface $visitRepository, + private readonly string $tag, + private readonly VisitsParams $params, + private readonly ?ApiKey $apiKey, ) { } diff --git a/module/Core/src/Visit/Repository/OrphanVisitsCountRepository.php b/module/Core/src/Visit/Repository/OrphanVisitsCountRepository.php index d5e7670cf..628ed2ba1 100644 --- a/module/Core/src/Visit/Repository/OrphanVisitsCountRepository.php +++ b/module/Core/src/Visit/Repository/OrphanVisitsCountRepository.php @@ -9,6 +9,7 @@ use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Rest\ApiKey\Role; +/** @extends EntitySpecificationRepository */ class OrphanVisitsCountRepository extends EntitySpecificationRepository implements OrphanVisitsCountRepositoryInterface { public function countOrphanVisits(VisitsCountFiltering $filtering): int diff --git a/module/Core/src/Visit/Repository/ShortUrlVisitsCountRepository.php b/module/Core/src/Visit/Repository/ShortUrlVisitsCountRepository.php index f07aab337..45ddb1f7f 100644 --- a/module/Core/src/Visit/Repository/ShortUrlVisitsCountRepository.php +++ b/module/Core/src/Visit/Repository/ShortUrlVisitsCountRepository.php @@ -5,9 +5,11 @@ namespace Shlinkio\Shlink\Core\Visit\Repository; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; +use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\Visit\Entity\ShortUrlVisitsCount; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; +/** @extends EntitySpecificationRepository */ class ShortUrlVisitsCountRepository extends EntitySpecificationRepository implements ShortUrlVisitsCountRepositoryInterface { diff --git a/module/Core/src/Visit/Repository/VisitDeleterRepository.php b/module/Core/src/Visit/Repository/VisitDeleterRepository.php index 19f79dc70..94c570910 100644 --- a/module/Core/src/Visit/Repository/VisitDeleterRepository.php +++ b/module/Core/src/Visit/Repository/VisitDeleterRepository.php @@ -8,6 +8,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\Visit\Entity\Visit; +/** @extends EntitySpecificationRepository */ class VisitDeleterRepository extends EntitySpecificationRepository implements VisitDeleterRepositoryInterface { public function deleteShortUrlVisits(ShortUrl $shortUrl): int diff --git a/module/Core/src/Visit/Repository/VisitIterationRepository.php b/module/Core/src/Visit/Repository/VisitIterationRepository.php index cf3426112..71590d7ea 100644 --- a/module/Core/src/Visit/Repository/VisitIterationRepository.php +++ b/module/Core/src/Visit/Repository/VisitIterationRepository.php @@ -12,6 +12,7 @@ /** * Allows iterating large amounts of visits in a memory-efficient way, to use in batch processes + * @extends EntitySpecificationRepository */ class VisitIterationRepository extends EntitySpecificationRepository implements VisitIterationRepositoryInterface { diff --git a/module/Core/src/Visit/Repository/VisitRepository.php b/module/Core/src/Visit/Repository/VisitRepository.php index b86f1463e..0708a4e18 100644 --- a/module/Core/src/Visit/Repository/VisitRepository.php +++ b/module/Core/src/Visit/Repository/VisitRepository.php @@ -23,6 +23,7 @@ use const PHP_INT_MAX; +/** @extends EntitySpecificationRepository */ class VisitRepository extends EntitySpecificationRepository implements VisitRepositoryInterface { /** diff --git a/module/Core/src/Visit/Repository/VisitRepositoryInterface.php b/module/Core/src/Visit/Repository/VisitRepositoryInterface.php index 9904181ba..774e1677f 100644 --- a/module/Core/src/Visit/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Visit/Repository/VisitRepositoryInterface.php @@ -13,6 +13,9 @@ use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +/** + * @extends ObjectRepository + */ interface VisitRepositoryInterface extends ObjectRepository, EntitySpecificationRepositoryInterface { /** diff --git a/module/Core/src/Visit/VisitsStatsHelper.php b/module/Core/src/Visit/VisitsStatsHelper.php index 65d9a1c80..7f3e22822 100644 --- a/module/Core/src/Visit/VisitsStatsHelper.php +++ b/module/Core/src/Visit/VisitsStatsHelper.php @@ -63,8 +63,7 @@ public function getVisitsStats(?ApiKey $apiKey = null): VisitsStats } /** - * @return Visit[]|Paginator - * @throws ShortUrlNotFoundException + * @inheritDoc */ public function visitsForShortUrl( ShortUrlIdentifier $identifier, @@ -87,8 +86,7 @@ public function visitsForShortUrl( } /** - * @return Visit[]|Paginator - * @throws TagNotFoundException + * @inheritDoc */ public function visitsForTag(string $tag, VisitsParams $params, ?ApiKey $apiKey = null): Paginator { @@ -105,8 +103,7 @@ public function visitsForTag(string $tag, VisitsParams $params, ?ApiKey $apiKey } /** - * @return Visit[]|Paginator - * @throws DomainNotFoundException + * @inheritDoc */ public function visitsForDomain(string $domain, VisitsParams $params, ?ApiKey $apiKey = null): Paginator { @@ -123,7 +120,7 @@ public function visitsForDomain(string $domain, VisitsParams $params, ?ApiKey $a } /** - * @return Visit[]|Paginator + * @inheritDoc */ public function orphanVisits(OrphanVisitsParams $params, ?ApiKey $apiKey = null): Paginator { @@ -141,6 +138,10 @@ public function nonOrphanVisits(VisitsParams $params, ?ApiKey $apiKey = null): P return $this->createPaginator(new NonOrphanVisitsPaginatorAdapter($repo, $params, $apiKey), $params); } + /** + * @param AdapterInterface $adapter + * @return Paginator + */ private function createPaginator(AdapterInterface $adapter, VisitsParams $params): Paginator { $paginator = new Paginator($adapter); diff --git a/module/Core/src/Visit/VisitsStatsHelperInterface.php b/module/Core/src/Visit/VisitsStatsHelperInterface.php index 265174ed1..87e0980b4 100644 --- a/module/Core/src/Visit/VisitsStatsHelperInterface.php +++ b/module/Core/src/Visit/VisitsStatsHelperInterface.php @@ -20,7 +20,7 @@ interface VisitsStatsHelperInterface public function getVisitsStats(?ApiKey $apiKey = null): VisitsStats; /** - * @return Visit[]|Paginator + * @return Paginator * @throws ShortUrlNotFoundException */ public function visitsForShortUrl( @@ -30,24 +30,24 @@ public function visitsForShortUrl( ): Paginator; /** - * @return Visit[]|Paginator + * @return Paginator * @throws TagNotFoundException */ public function visitsForTag(string $tag, VisitsParams $params, ?ApiKey $apiKey = null): Paginator; /** - * @return Visit[]|Paginator + * @return Paginator * @throws DomainNotFoundException */ public function visitsForDomain(string $domain, VisitsParams $params, ?ApiKey $apiKey = null): Paginator; /** - * @return Visit[]|Paginator + * @return Paginator */ public function orphanVisits(OrphanVisitsParams $params, ?ApiKey $apiKey = null): Paginator; /** - * @return Visit[]|Paginator + * @return Paginator */ public function nonOrphanVisits(VisitsParams $params, ?ApiKey $apiKey = null): Paginator; } diff --git a/module/Core/test-db/Visit/Listener/OrphanVisitsCountTrackerTest.php b/module/Core/test-db/Visit/Listener/OrphanVisitsCountTrackerTest.php index 20302c755..ad8edcd28 100644 --- a/module/Core/test-db/Visit/Listener/OrphanVisitsCountTrackerTest.php +++ b/module/Core/test-db/Visit/Listener/OrphanVisitsCountTrackerTest.php @@ -4,11 +4,11 @@ namespace ShlinkioDbTest\Shlink\Core\Visit\Listener; -use Doctrine\ORM\EntityRepository; use PHPUnit\Framework\Attributes\Test; use Shlinkio\Shlink\Core\Visit\Entity\OrphanVisitsCount; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\Visitor; +use Shlinkio\Shlink\Core\Visit\Repository\OrphanVisitsCountRepository; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; use function array_filter; @@ -16,7 +16,7 @@ class OrphanVisitsCountTrackerTest extends DatabaseTestCase { - private EntityRepository $repo; + private OrphanVisitsCountRepository $repo; protected function setUp(): void { diff --git a/module/Core/test-db/Visit/Listener/ShortUrlVisitsCountTrackerTest.php b/module/Core/test-db/Visit/Listener/ShortUrlVisitsCountTrackerTest.php index 29f5d5d1f..7a2a6c29e 100644 --- a/module/Core/test-db/Visit/Listener/ShortUrlVisitsCountTrackerTest.php +++ b/module/Core/test-db/Visit/Listener/ShortUrlVisitsCountTrackerTest.php @@ -4,12 +4,12 @@ namespace ShlinkioDbTest\Shlink\Core\Visit\Listener; -use Doctrine\ORM\EntityRepository; use PHPUnit\Framework\Attributes\Test; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\Visit\Entity\ShortUrlVisitsCount; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\Visitor; +use Shlinkio\Shlink\Core\Visit\Repository\ShortUrlVisitsCountRepository; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; use function array_filter; @@ -17,7 +17,7 @@ class ShortUrlVisitsCountTrackerTest extends DatabaseTestCase { - private EntityRepository $repo; + private ShortUrlVisitsCountRepository $repo; protected function setUp(): void { diff --git a/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php b/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php index dc19dcd39..d55f13acd 100644 --- a/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php +++ b/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php @@ -104,7 +104,7 @@ public static function provideConditionMappingCallbacks(): iterable } /** - * @param ArrayCollection $conditions + * @param ArrayCollection $conditions */ private function createRule(ArrayCollection $conditions): ShortUrlRedirectRule { diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php index 92fac8eb2..4e3056b3f 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php @@ -103,6 +103,9 @@ public function titleIsUpdatedWhenItCanBeResolvedFromResponse(string $contentTyp self::assertEquals('Resolved "title"', $result->title); } + /** + * @return InvocationMocker + */ private function expectRequestToBeCalled(): InvocationMocker { return $this->httpClient->expects($this->once())->method('request')->with( diff --git a/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php b/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php index ad09b22db..2b7aa0a2b 100644 --- a/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php +++ b/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php @@ -9,6 +9,9 @@ use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\Entity\ApiKey; +/** + * @extends EntitySpecificationRepository + */ class ApiKeyRepository extends EntitySpecificationRepository implements ApiKeyRepositoryInterface { /** diff --git a/module/Rest/src/ApiKey/Repository/ApiKeyRepositoryInterface.php b/module/Rest/src/ApiKey/Repository/ApiKeyRepositoryInterface.php index a557e603f..57c2a7f62 100644 --- a/module/Rest/src/ApiKey/Repository/ApiKeyRepositoryInterface.php +++ b/module/Rest/src/ApiKey/Repository/ApiKeyRepositoryInterface.php @@ -8,6 +8,9 @@ use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; +/** + * @extends ObjectRepository + */ interface ApiKeyRepositoryInterface extends ObjectRepository, EntitySpecificationRepositoryInterface { /** diff --git a/phpstan.neon b/phpstan.neon index 691b951a3..7e21a038e 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -12,5 +12,4 @@ parameters: ignoreErrors: - '#should return int<0, max> but returns int#' - '#expects -1\|int<1, max>, int given#' - - identifier: missingType.generics - identifier: missingType.iterableValue From 133efff2cde05cdf4b1290a4c1c5268df78ab42b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 31 Jul 2024 19:53:05 +0200 Subject: [PATCH 37/41] Improve PHPStan config --- composer.json | 2 +- module/Core/functions/functions.php | 14 ++++++++++++-- phpstan.neon | 5 +++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index f695b063f..cd0addb28 100644 --- a/composer.json +++ b/composer.json @@ -114,7 +114,7 @@ ], "cs": "phpcs -s", "cs:fix": "phpcbf", - "stan": "APP_ENV=test php vendor/bin/phpstan analyse module/*/src module/*/test* module/*/config module/*/migrations config docker/config --level=8", + "stan": "APP_ENV=test php vendor/bin/phpstan analyse", "test": [ "@parallel test:unit test:db", "@parallel test:api test:cli" diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 720f27c1c..160ba35e4 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -109,7 +109,7 @@ function normalizeLocale(string $locale): string * minimum quality * * @param non-empty-string $acceptLanguage - * @param float<0, 1> $minQuality + * @param float $minQuality * @return iterable; */ function acceptLanguageToLocales(string $acceptLanguage, float $minQuality = 0): iterable @@ -142,21 +142,31 @@ function acceptLanguageToLocales(string $acceptLanguage, float $minQuality = 0): */ function splitLocale(string $locale): array { - return array_pad(explode('-', $locale), length: 2, value: null); + [$lang, $countryCode] = array_pad(explode('-', $locale), length: 2, value: null); + return [$lang, $countryCode]; } +/** + * @param InputFilter $inputFilter + */ function getOptionalIntFromInputFilter(InputFilter $inputFilter, string $fieldName): ?int { $value = $inputFilter->getValue($fieldName); return $value !== null ? (int) $value : null; } +/** + * @param InputFilter $inputFilter + */ function getOptionalBoolFromInputFilter(InputFilter $inputFilter, string $fieldName): ?bool { $value = $inputFilter->getValue($fieldName); return $value !== null ? (bool) $value : null; } +/** + * @param InputFilter $inputFilter + */ function getNonEmptyOptionalValueFromInputFilter(InputFilter $inputFilter, string $fieldName): mixed { $value = $inputFilter->getValue($fieldName); diff --git a/phpstan.neon b/phpstan.neon index 7e21a038e..72c5ea6d1 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4,6 +4,11 @@ includes: - vendor/phpstan/phpstan-phpunit/extension.neon - vendor/phpstan/phpstan-phpunit/rules.neon parameters: + level: 8 + paths: + - module + - config + - docker/config symfony: console_application_loader: 'config/cli-app.php' doctrine: From d76c96ad41c1d34c297d7b7dedadc489a5fe82a3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 1 Aug 2024 08:38:49 +0200 Subject: [PATCH 38/41] Fix coding standard --- module/Core/functions/functions.php | 1 - 1 file changed, 1 deletion(-) diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 160ba35e4..f7bd0cdf1 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -109,7 +109,6 @@ function normalizeLocale(string $locale): string * minimum quality * * @param non-empty-string $acceptLanguage - * @param float $minQuality * @return iterable; */ function acceptLanguageToLocales(string $acceptLanguage, float $minQuality = 0): iterable From 69dcab96f80f3b122072377d8c3a7e58d7c171b4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 4 Aug 2024 13:13:03 +0200 Subject: [PATCH 39/41] Add --testdox-summary flag to phpunit executions --- bin/test/run-api-tests.sh | 2 +- composer.json | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bin/test/run-api-tests.sh b/bin/test/run-api-tests.sh index e39d564de..b96cf6719 100755 --- a/bin/test/run-api-tests.sh +++ b/bin/test/run-api-tests.sh @@ -22,7 +22,7 @@ echo 'Starting server...' -o=logs.channels.server.output="${PWD}/${OUTPUT_LOGS}" & sleep 2 # Let's give the server a couple of seconds to start -vendor/bin/phpunit --order-by=random -c phpunit-api.xml --testdox --colors=always $* +vendor/bin/phpunit --order-by=random -c phpunit-api.xml --testdox --testdox-summary $* TESTS_EXIT_CODE=$? [ "$TEST_RUNTIME" = 'rr' ] && bin/rr stop -w . diff --git a/composer.json b/composer.json index cd0addb28..098717742 100644 --- a/composer.json +++ b/composer.json @@ -70,7 +70,7 @@ "phpstan/phpstan-symfony": "^1.4", "phpunit/php-code-coverage": "^11.0", "phpunit/phpcov": "^10.0", - "phpunit/phpunit": "^11.1", + "phpunit/phpunit": "^11.3", "roave/security-advisories": "dev-master", "shlinkio/php-coding-standard": "~2.3.0", "shlinkio/shlink-test-utils": "^4.1", @@ -119,11 +119,11 @@ "@parallel test:unit test:db", "@parallel test:api test:cli" ], - "test:unit": "COLUMNS=120 vendor/bin/phpunit --order-by=random --colors=always --testdox", + "test:unit": "COLUMNS=120 vendor/bin/phpunit --order-by=random --testdox --testdox-summary", "test:unit:ci": "@test:unit --coverage-php=build/coverage-unit.cov", "test:unit:pretty": "@test:unit --coverage-html build/coverage-unit/coverage-html", "test:db": "@parallel test:db:sqlite:ci test:db:mysql test:db:maria test:db:postgres test:db:ms", - "test:db:sqlite": "APP_ENV=test php vendor/bin/phpunit --order-by=random --colors=always --testdox -c phpunit-db.xml", + "test:db:sqlite": "APP_ENV=test php vendor/bin/phpunit --order-by=random --testdox --testdox-summary -c phpunit-db.xml", "test:db:sqlite:ci": "@test:db:sqlite --coverage-php build/coverage-db.cov", "test:db:mysql": "DB_DRIVER=mysql composer test:db:sqlite -- $*", "test:db:maria": "DB_DRIVER=maria composer test:db:sqlite -- $*", @@ -136,7 +136,7 @@ "test:api:mssql": "DB_DRIVER=mssql composer test:api -- $*", "test:api:ci": "GENERATE_COVERAGE=yes composer test:api && vendor/bin/phpcov merge build/coverage-api --php build/coverage-api.cov && rm build/coverage-api/*.cov", "test:api:pretty": "GENERATE_COVERAGE=yes composer test:api && vendor/bin/phpcov merge build/coverage-api --html build/coverage-api/coverage-html && rm build/coverage-api/*.cov", - "test:cli": "APP_ENV=test DB_DRIVER=maria TEST_ENV=cli php vendor/bin/phpunit --order-by=random --colors=always --testdox -c phpunit-cli.xml", + "test:cli": "APP_ENV=test DB_DRIVER=maria TEST_ENV=cli php vendor/bin/phpunit --order-by=random --testdox --testdox-summary -c phpunit-cli.xml", "test:cli:ci": "GENERATE_COVERAGE=yes composer test:cli && vendor/bin/phpcov merge build/coverage-cli --php build/coverage-cli.cov && rm build/coverage-cli/*.cov", "test:cli:pretty": "GENERATE_COVERAGE=yes composer test:cli && vendor/bin/phpcov merge build/coverage-cli --html build/coverage-cli/coverage-html && rm build/coverage-cli/*.cov", "swagger:validate": "php-openapi validate docs/swagger/swagger.json", From 613b1d304523bd565f7961c5769801b1d67be9b9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 6 Aug 2024 10:13:55 +0200 Subject: [PATCH 40/41] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 307b510d8..91a5141d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this This brings CLI and API interfaces capabilities closer, and solves an overlook since the feature was implemented years ago. +* [#2164](https://github.com/shlinkio/shlink/pull/2164) Add missing `--title` option to `short-url:create` and `short-url:edit` commands. + ### Changed * [#2096](https://github.com/shlinkio/shlink/issues/2096) Update to RoadRunner 2024. From f9658c8da1dc4135961fd3a20e6c1a2692702949 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Aug 2024 18:30:06 +0200 Subject: [PATCH 41/41] Add v4.2.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 91a5141d7..c953b99ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## [4.2.0] - 2024-08-11 ### Added * [#2120](https://github.com/shlinkio/shlink/issues/2120) Add new IP address condition for the dynamic rules redirections system. diff --git a/composer.json b/composer.json index 098717742..08c945576 100644 --- a/composer.json +++ b/composer.json @@ -44,11 +44,11 @@ "pagerfanta/core": "^3.8", "ramsey/uuid": "^4.7", "shlinkio/doctrine-specification": "^2.1.1", - "shlinkio/shlink-common": "dev-main#144e5c1 as 6.2", + "shlinkio/shlink-common": "^6.2", "shlinkio/shlink-config": "^3.0", "shlinkio/shlink-event-dispatcher": "^4.1", "shlinkio/shlink-importer": "^5.3.2", - "shlinkio/shlink-installer": "dev-develop#ccda72e as 9.2", + "shlinkio/shlink-installer": "^9.2", "shlinkio/shlink-ip-geolocation": "^4.0", "shlinkio/shlink-json": "^1.1", "spiral/roadrunner": "^2024.1",