From f3ff059d486380f0c3b731da54ae5172aad35778 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 17 Nov 2025 12:33:08 +0100 Subject: [PATCH 1/7] Improve RoleResolver coverage --- module/CLI/test/ApiKey/RoleResolverTest.php | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/module/CLI/test/ApiKey/RoleResolverTest.php b/module/CLI/test/ApiKey/RoleResolverTest.php index 184780d7..ac47a262 100644 --- a/module/CLI/test/ApiKey/RoleResolverTest.php +++ b/module/CLI/test/ApiKey/RoleResolverTest.php @@ -91,11 +91,17 @@ class RoleResolverTest extends TestCase [RoleDefinition::forAuthoredShortUrls()], 0, ]; - yield 'both roles' => [ - $buildInput( - [Role::DOMAIN_SPECIFIC->paramName() => 'example.com', Role::AUTHORED_SHORT_URLS->paramName() => true], - ), - [RoleDefinition::forAuthoredShortUrls(), RoleDefinition::forDomain($domain)], + yield 'all roles' => [ + $buildInput([ + Role::DOMAIN_SPECIFIC->paramName() => 'example.com', + Role::AUTHORED_SHORT_URLS->paramName() => true, + Role::NO_ORPHAN_VISITS->paramName() => true, + ]), + [ + RoleDefinition::forAuthoredShortUrls(), + RoleDefinition::forDomain($domain), + RoleDefinition::forNoOrphanVisits(), + ], 1, ]; } From 933c54e884ed977b755ee4fed214ccf7db16ae5c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 18 Nov 2025 08:41:42 +0100 Subject: [PATCH 2/7] Improve some console commands coverage --- .../ShortUrl/CreateShortUrlCommandTest.php | 15 +++++++++++++++ .../ShortUrl/GetShortUrlVisitsCommandTest.php | 13 +++++++++++++ .../Command/ShortUrl/ResolveUrlCommandTest.php | 13 +++++++++++++ 3 files changed, 41 insertions(+) diff --git a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php index ff5e3ea6..9452aad5 100644 --- a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php @@ -64,6 +64,21 @@ class CreateShortUrlCommandTest extends TestCase self::assertStringNotContainsString('but the real-time updates cannot', $output); } + #[Test] + public function longUrlIsAskedIfNotProvided(): void + { + $shortUrl = ShortUrl::createFake(); + $this->urlShortener->expects($this->once())->method('shorten')->withAnyParameters()->willReturn( + UrlShorteningResult::withoutErrorOnEventDispatching($shortUrl), + ); + $this->stringifier->expects($this->once())->method('stringify')->with($shortUrl)->willReturn( + 'stringified_short_url', + ); + + $this->commandTester->setInputs([$shortUrl->getLongUrl()]); + $this->commandTester->execute([]); + } + #[Test] public function providingNonUniqueSlugOutputsError(): void { diff --git a/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php index 709974ec..e306b0bc 100644 --- a/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php @@ -50,6 +50,19 @@ class GetShortUrlVisitsCommandTest extends TestCase $this->commandTester->execute(['shortCode' => $shortCode]); } + #[Test] + public function shortCodeIsAskedIfNotProvided(): void + { + $shortCode = 'abc123'; + $this->visitsHelper->expects($this->once())->method('visitsForShortUrl')->with( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), + $this->anything(), + )->willReturn(new Paginator(new ArrayAdapter([]))); + + $this->commandTester->setInputs([$shortCode]); + $this->commandTester->execute([]); + } + #[Test] public function providingDateFlagsTheListGetsFiltered(): void { diff --git a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php index 9c9bbb93..742ae05c 100644 --- a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php @@ -45,6 +45,19 @@ class ResolveUrlCommandTest extends TestCase self::assertEquals('Long URL: ' . $expectedUrl . PHP_EOL, $output); } + #[Test] + public function shortCodeIsAskedIfNotProvided(): void + { + $shortCode = 'abc123'; + $shortUrl = ShortUrl::createFake(); + $this->urlResolver->expects($this->once())->method('resolveShortUrl')->with( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), + )->willReturn($shortUrl); + + $this->commandTester->setInputs([$shortCode]); + $this->commandTester->execute([]); + } + #[Test] public function incorrectShortCodeOutputsErrorMessage(): void { From db1411d3f8163a9c527cd5ad0a0aef250d249e90 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 18 Nov 2025 08:45:31 +0100 Subject: [PATCH 3/7] Remove unused method in ApiKeyNotFoundException --- module/Rest/src/Exception/ApiKeyNotFoundException.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/module/Rest/src/Exception/ApiKeyNotFoundException.php b/module/Rest/src/Exception/ApiKeyNotFoundException.php index 5519b31a..d7b76d33 100644 --- a/module/Rest/src/Exception/ApiKeyNotFoundException.php +++ b/module/Rest/src/Exception/ApiKeyNotFoundException.php @@ -12,10 +12,4 @@ class ApiKeyNotFoundException extends RuntimeException implements ExceptionInter { return new self(sprintf('API key with name "%s" not found', $name)); } - - /** @deprecated */ - public static function forKey(string $key): self - { - return new self(sprintf('API key with key "%s" not found', $key)); - } } From 88e5bb5618facc4979a4c975df25eb06c12b0d71 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 18 Nov 2025 08:56:09 +0100 Subject: [PATCH 4/7] Add test for AbstractRestAction::getRouteDef() --- module/Rest/test/Action/MercureInfoActionTest.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/module/Rest/test/Action/MercureInfoActionTest.php b/module/Rest/test/Action/MercureInfoActionTest.php index 69bfb56a..ddd43338 100644 --- a/module/Rest/test/Action/MercureInfoActionTest.php +++ b/module/Rest/test/Action/MercureInfoActionTest.php @@ -77,4 +77,15 @@ class MercureInfoActionTest extends TestCase yield 'days not defined' => [null]; yield 'days defined' => [10]; } + + #[Test] + public function getRouteDefReturnsExpectedData(): void + { + self::assertEquals([ + 'name' => MercureInfoAction::class, + 'middleware' => [MercureInfoAction::class], + 'path' => '/mercure-info', + 'allowed_methods' => ['GET'], + ], MercureInfoAction::getRouteDef()); + } } From 1e0b6be67da1b024ea978348adf93660d3e8fb88 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 18 Nov 2025 09:06:11 +0100 Subject: [PATCH 5/7] Improved NorFoundRedirectResolver test --- .../Config/NotFoundRedirectResolverTest.php | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/module/Core/test/Config/NotFoundRedirectResolverTest.php b/module/Core/test/Config/NotFoundRedirectResolverTest.php index d2e750db..796cc3bb 100644 --- a/module/Core/test/Config/NotFoundRedirectResolverTest.php +++ b/module/Core/test/Config/NotFoundRedirectResolverTest.php @@ -15,7 +15,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\UriInterface; -use Psr\Log\NullLogger; +use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Config\NotFoundRedirectResolver; use Shlinkio\Shlink\Core\Config\Options\NotFoundRedirectOptions; @@ -28,11 +28,14 @@ class NotFoundRedirectResolverTest extends TestCase { private NotFoundRedirectResolver $resolver; private MockObject & RedirectResponseHelperInterface $helper; + private MockObject & LoggerInterface $logger; protected function setUp(): void { $this->helper = $this->createMock(RedirectResponseHelperInterface::class); - $this->resolver = new NotFoundRedirectResolver($this->helper, new NullLogger()); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->resolver = new NotFoundRedirectResolver($this->helper, $this->logger); } #[Test, DataProvider('provideRedirects')] @@ -123,6 +126,22 @@ class NotFoundRedirectResolverTest extends TestCase self::assertNull($result); } + #[Test] + public function warningMessageIsLoggedIfRedirectUrlIsMalformed(): void + { + $this->logger->expects($this->once())->method('warning')->with( + 'It was not possible to parse "{url}" as a valid URL: {e}', + $this->isArray(), + ); + + $uri = new Uri('/'); + $this->resolver->resolveRedirectResponse( + self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)), + new NotFoundRedirectOptions(baseUrlRedirect: 'http:///example.com'), + $uri, + ); + } + private static function notFoundType(ServerRequestInterface $req): NotFoundType { return NotFoundType::fromRequest($req, ''); From 7812a85b3986d52973bebb40fe31d0bb7c872c6b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 18 Nov 2025 09:20:52 +0100 Subject: [PATCH 6/7] Remove unused AppOptions::__toString method --- module/Core/src/Config/Options/AppOptions.php | 7 ------- 1 file changed, 7 deletions(-) diff --git a/module/Core/src/Config/Options/AppOptions.php b/module/Core/src/Config/Options/AppOptions.php index 71e3f507..45577e98 100644 --- a/module/Core/src/Config/Options/AppOptions.php +++ b/module/Core/src/Config/Options/AppOptions.php @@ -6,8 +6,6 @@ namespace Shlinkio\Shlink\Core\Config\Options; use Shlinkio\Shlink\Core\Config\EnvVars; -use function sprintf; - final class AppOptions { public function __construct(public string $name = 'Shlink', public string $version = '4.0.0') @@ -19,9 +17,4 @@ final class AppOptions $version = EnvVars::isDevEnv() ? 'latest' : '%SHLINK_VERSION%'; return new self(version: $version); } - - public function __toString(): string - { - return sprintf('%s:v%s', $this->name, $this->version); - } } From 9432a5ba78c7efc73b8c5e32174de7c540be6184 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 18 Nov 2025 09:30:30 +0100 Subject: [PATCH 7/7] Add tests for events --- .../EventDispatcher/Event/ShortUrlCreated.php | 4 +-- .../Event/ShortUrlCreatedTest.php | 29 +++++++++++++++ .../EventDispatcher/Event/UrlVisitedTest.php | 35 +++++++++++++++++++ 3 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 module/Core/test/EventDispatcher/Event/ShortUrlCreatedTest.php create mode 100644 module/Core/test/EventDispatcher/Event/UrlVisitedTest.php diff --git a/module/Core/src/EventDispatcher/Event/ShortUrlCreated.php b/module/Core/src/EventDispatcher/Event/ShortUrlCreated.php index 4055935f..0d929ec7 100644 --- a/module/Core/src/EventDispatcher/Event/ShortUrlCreated.php +++ b/module/Core/src/EventDispatcher/Event/ShortUrlCreated.php @@ -15,9 +15,7 @@ final readonly class ShortUrlCreated implements JsonSerializable, JsonUnserializ public function jsonSerialize(): array { - return [ - 'shortUrlId' => $this->shortUrlId, - ]; + return ['shortUrlId' => $this->shortUrlId]; } public static function fromPayload(array $payload): self diff --git a/module/Core/test/EventDispatcher/Event/ShortUrlCreatedTest.php b/module/Core/test/EventDispatcher/Event/ShortUrlCreatedTest.php new file mode 100644 index 00000000..4b59f2e4 --- /dev/null +++ b/module/Core/test/EventDispatcher/Event/ShortUrlCreatedTest.php @@ -0,0 +1,29 @@ + $shortUrlId], new ShortUrlCreated($shortUrlId)->jsonSerialize()); + } + + #[Test] + #[TestWith([['shortUrlId' => '123'], '123'])] + #[TestWith([[], ''])] + public function creationFromPayload(array $payload, string $expectedShortUrlId): void + { + $event = ShortUrlCreated::fromPayload($payload); + self::assertEquals($expectedShortUrlId, $event->shortUrlId); + } +} diff --git a/module/Core/test/EventDispatcher/Event/UrlVisitedTest.php b/module/Core/test/EventDispatcher/Event/UrlVisitedTest.php new file mode 100644 index 00000000..ebcfa265 --- /dev/null +++ b/module/Core/test/EventDispatcher/Event/UrlVisitedTest.php @@ -0,0 +1,35 @@ + $visitId, 'originalIpAddress' => null], + new UrlVisited($visitId)->jsonSerialize(), + ); + } + + #[Test] + #[TestWith([['visitId' => '123', 'originalIpAddress' => '1.2.3.4'], '123', '1.2.3.4'])] + #[TestWith([['visitId' => '123'], '123', null])] + #[TestWith([['originalIpAddress' => '1.2.3.4'], '', '1.2.3.4'])] + #[TestWith([[], '', null])] + public function creationFromPayload(array $payload, string $expectedVisitId, string|null $expectedIpAddress): void + { + $event = UrlVisited::fromPayload($payload); + self::assertEquals($expectedVisitId, $event->visitId); + self::assertEquals($expectedIpAddress, $event->originalIpAddress); + } +}