diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fe5ac60..9d13de92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this #### Changed * [#577](https://github.com/shlinkio/shlink/issues/577) Wrapped params used to customize short URL lists into a DTO with implicit validation. -* [#620](https://github.com/shlinkio/shlink/issues/620) "Controlled" errors (like validation errors and such) will no longer be logged with error level, preventing logs to be polluted. #### Deprecated @@ -25,7 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this #### Fixed -* *Nothing* +* [#620](https://github.com/shlinkio/shlink/issues/620) Ensured "controlled" errors (like validation errors and such) won't be logged with error level, preventing logs to be polluted. ## 2.0.3 - 2020-01-27 diff --git a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php index 22f384db..e6fdef3d 100644 --- a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -54,11 +55,9 @@ class ResolveUrlCommand extends Command protected function execute(InputInterface $input, OutputInterface $output): ?int { $io = new SymfonyStyle($input, $output); - $shortCode = $input->getArgument('shortCode'); - $domain = $input->getOption('domain'); try { - $url = $this->urlResolver->shortCodeToShortUrl($shortCode, $domain); + $url = $this->urlResolver->resolveShortUrl(ShortUrlIdentifier::fromCli($input)); $output->writeln(sprintf('Long URL: %s', $url->getLongUrl())); return ExitCodes::EXIT_SUCCESS; } catch (ShortUrlNotFoundException $e) { diff --git a/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php index 7fd727ca..9d9a11c1 100644 --- a/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php @@ -9,6 +9,7 @@ use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\ShortUrl\DeleteShortUrlCommand; use Shlinkio\Shlink\Core\Exception; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\DeleteShortUrlServiceInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -56,7 +57,7 @@ class DeleteShortUrlCommandTest extends TestCase { $shortCode = 'abc123'; $deleteByShortCode = $this->service->deleteByShortCode($shortCode, false)->willThrow( - Exception\ShortUrlNotFoundException::fromNotFoundShortCode($shortCode), + Exception\ShortUrlNotFoundException::fromNotFound(new ShortUrlIdentifier($shortCode)), ); $this->commandTester->execute(['shortCode' => $shortCode]); diff --git a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php index cb3e658d..7c307252 100644 --- a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php @@ -9,6 +9,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\ShortUrl\ResolveUrlCommand; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -38,8 +39,8 @@ class ResolveUrlCommandTest extends TestCase $shortCode = 'abc123'; $expectedUrl = 'http://domain.com/foo/bar'; $shortUrl = new ShortUrl($expectedUrl); - $this->urlResolver->shortCodeToShortUrl($shortCode, null)->willReturn($shortUrl) - ->shouldBeCalledOnce(); + $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode))->willReturn($shortUrl) + ->shouldBeCalledOnce(); $this->commandTester->execute(['shortCode' => $shortCode]); $output = $this->commandTester->getDisplay(); @@ -49,9 +50,11 @@ class ResolveUrlCommandTest extends TestCase /** @test */ public function incorrectShortCodeOutputsErrorMessage(): void { - $shortCode = 'abc123'; - $this->urlResolver->shortCodeToShortUrl($shortCode, null) - ->willThrow(ShortUrlNotFoundException::fromNotFoundShortCode($shortCode)) + $identifier = new ShortUrlIdentifier('abc123'); + $shortCode = $identifier->shortCode(); + + $this->urlResolver->resolveShortUrl($identifier) + ->willThrow(ShortUrlNotFoundException::fromNotFound($identifier)) ->shouldBeCalledOnce(); $this->commandTester->execute(['shortCode' => $shortCode]); diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 4e45d9cd..3655bd32 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -13,6 +13,7 @@ use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; @@ -45,12 +46,12 @@ abstract class AbstractTrackingAction implements MiddlewareInterface public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { $shortCode = $request->getAttribute('shortCode', ''); - $domain = $request->getUri()->getAuthority(); + $identifier = ShortUrlIdentifier::fromRequest($request); $query = $request->getQueryParams(); $disableTrackParam = $this->appOptions->getDisableTrackParam(); try { - $url = $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, $domain); + $url = $this->urlResolver->resolveEnabledShortUrl($identifier); // Track visit to this short code if ($disableTrackParam === null || ! array_key_exists($disableTrackParam, $query)) { diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index c302a58d..df06edbd 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -14,6 +14,7 @@ use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Shlinkio\Shlink\Common\Response\QrCodeResponse; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; class QrCodeAction implements MiddlewareInterface @@ -38,18 +39,16 @@ class QrCodeAction implements MiddlewareInterface public function process(Request $request, RequestHandlerInterface $handler): Response { - // Make sure the short URL exists for this short code - $shortCode = $request->getAttribute('shortCode'); - $domain = $request->getUri()->getAuthority(); + $identifier = ShortUrlIdentifier::fromRequest($request); try { - $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, $domain); + $this->urlResolver->resolveEnabledShortUrl($identifier); } catch (ShortUrlNotFoundException $e) { $this->logger->warning('An error occurred while creating QR code. {e}', ['e' => $e]); return $handler->handle($request); } - $path = $this->router->generateUri(RedirectAction::class, ['shortCode' => $shortCode]); + $path = $this->router->generateUri(RedirectAction::class, ['shortCode' => $identifier->shortCode()]); $size = $this->getSizeParam($request); $qrCode = new QrCode((string) $request->getUri()->withPath($path)->withQuery('')); diff --git a/module/Core/src/Exception/ShortUrlNotFoundException.php b/module/Core/src/Exception/ShortUrlNotFoundException.php index e68e55ed..0ae29da5 100644 --- a/module/Core/src/Exception/ShortUrlNotFoundException.php +++ b/module/Core/src/Exception/ShortUrlNotFoundException.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Exception; use Fig\Http\Message\StatusCodeInterface; use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use function sprintf; @@ -17,8 +18,10 @@ class ShortUrlNotFoundException extends DomainException implements ProblemDetail private const TITLE = 'Short URL not found'; private const TYPE = 'INVALID_SHORTCODE'; - public static function fromNotFoundShortCode(string $shortCode, ?string $domain = null): self + public static function fromNotFound(ShortUrlIdentifier $identifier): self { + $shortCode = $identifier->shortCode(); + $domain = $identifier->domain(); $suffix = $domain === null ? '' : sprintf(' for domain "%s"', $domain); $e = new self(sprintf('No URL found with short code "%s"%s', $shortCode, $suffix)); diff --git a/module/Core/src/Model/ShortUrlIdentifier.php b/module/Core/src/Model/ShortUrlIdentifier.php new file mode 100644 index 00000000..dae3a15b --- /dev/null +++ b/module/Core/src/Model/ShortUrlIdentifier.php @@ -0,0 +1,46 @@ +shortCode = $shortCode; + $this->domain = $domain; + } + + public static function fromRequest(ServerRequestInterface $request): self + { + $shortCode = $request->getAttribute('shortCode', ''); + $domain = $request->getQueryParams()['domain'] ?? null; + + return new self($shortCode, $domain); + } + + public static function fromCli(InputInterface $input): self + { + $shortCode = $input->getArgument('shortCode'); + $domain = $input->getOption('domain'); + + return new self($shortCode, $domain); + } + + public function shortCode(): string + { + return $this->shortCode; + } + + public function domain(): ?string + { + return $this->domain; + } +} diff --git a/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php index c9624bf3..2b4ad067 100644 --- a/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php +++ b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Service\ShortUrl; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Options\DeleteShortUrlsOptions; class DeleteShortUrlService implements DeleteShortUrlServiceInterface @@ -28,7 +29,7 @@ class DeleteShortUrlService implements DeleteShortUrlServiceInterface */ public function deleteByShortCode(string $shortCode, bool $ignoreThreshold = false): void { - $shortUrl = $this->findByShortCode($this->em, $shortCode); + $shortUrl = $this->findByShortCode($this->em, new ShortUrlIdentifier($shortCode)); if (! $ignoreThreshold && $this->isThresholdReached($shortUrl)) { throw Exception\DeleteShortUrlException::fromVisitsThreshold( $this->deleteShortUrlsOptions->getVisitsThreshold(), diff --git a/module/Core/src/Service/ShortUrl/FindShortCodeTrait.php b/module/Core/src/Service/ShortUrl/FindShortCodeTrait.php index 95009704..654d587f 100644 --- a/module/Core/src/Service/ShortUrl/FindShortCodeTrait.php +++ b/module/Core/src/Service/ShortUrl/FindShortCodeTrait.php @@ -7,20 +7,21 @@ namespace Shlinkio\Shlink\Core\Service\ShortUrl; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; +use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; trait FindShortCodeTrait { /** * @throws ShortUrlNotFoundException */ - private function findByShortCode(EntityManagerInterface $em, string $shortCode): ShortUrl + private function findByShortCode(EntityManagerInterface $em, ShortUrlIdentifier $identifier): ShortUrl { - /** @var ShortUrl|null $shortUrl */ - $shortUrl = $em->getRepository(ShortUrl::class)->findOneBy([ - 'shortCode' => $shortCode, - ]); + /** @var ShortUrlRepositoryInterface $repo */ + $repo = $em->getRepository(ShortUrl::class); + $shortUrl = $repo->findOneByShortCode($identifier->shortCode(), $identifier->domain()); if ($shortUrl === null) { - throw ShortUrlNotFoundException::fromNotFoundShortCode($shortCode); + throw ShortUrlNotFoundException::fromNotFound($identifier); } return $shortUrl; diff --git a/module/Core/src/Service/ShortUrl/ShortUrlResolver.php b/module/Core/src/Service/ShortUrl/ShortUrlResolver.php index 62f30c11..d7a71b1c 100644 --- a/module/Core/src/Service/ShortUrl/ShortUrlResolver.php +++ b/module/Core/src/Service/ShortUrl/ShortUrlResolver.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Service\ShortUrl; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; class ShortUrlResolver implements ShortUrlResolverInterface @@ -21,13 +22,13 @@ class ShortUrlResolver implements ShortUrlResolverInterface /** * @throws ShortUrlNotFoundException */ - public function shortCodeToShortUrl(string $shortCode, ?string $domain = null): ShortUrl + public function resolveShortUrl(ShortUrlIdentifier $identifier): ShortUrl { /** @var ShortUrlRepository $shortUrlRepo */ $shortUrlRepo = $this->em->getRepository(ShortUrl::class); - $shortUrl = $shortUrlRepo->findOneByShortCode($shortCode, $domain); + $shortUrl = $shortUrlRepo->findOneByShortCode($identifier->shortCode(), $identifier->domain()); if ($shortUrl === null) { - throw ShortUrlNotFoundException::fromNotFoundShortCode($shortCode, $domain); + throw ShortUrlNotFoundException::fromNotFound($identifier); } return $shortUrl; @@ -36,11 +37,11 @@ class ShortUrlResolver implements ShortUrlResolverInterface /** * @throws ShortUrlNotFoundException */ - public function shortCodeToEnabledShortUrl(string $shortCode, ?string $domain = null): ShortUrl + public function resolveEnabledShortUrl(ShortUrlIdentifier $identifier): ShortUrl { - $shortUrl = $this->shortCodeToShortUrl($shortCode, $domain); + $shortUrl = $this->resolveShortUrl($identifier); if (! $shortUrl->isEnabled()) { - throw ShortUrlNotFoundException::fromNotFoundShortCode($shortCode, $domain); + throw ShortUrlNotFoundException::fromNotFound($identifier); } return $shortUrl; diff --git a/module/Core/src/Service/ShortUrl/ShortUrlResolverInterface.php b/module/Core/src/Service/ShortUrl/ShortUrlResolverInterface.php index b00beed5..a3a7c115 100644 --- a/module/Core/src/Service/ShortUrl/ShortUrlResolverInterface.php +++ b/module/Core/src/Service/ShortUrl/ShortUrlResolverInterface.php @@ -6,16 +6,17 @@ namespace Shlinkio\Shlink\Core\Service\ShortUrl; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; interface ShortUrlResolverInterface { /** * @throws ShortUrlNotFoundException */ - public function shortCodeToShortUrl(string $shortCode, ?string $domain = null): ShortUrl; + public function resolveShortUrl(ShortUrlIdentifier $identifier): ShortUrl; /** * @throws ShortUrlNotFoundException */ - public function shortCodeToEnabledShortUrl(string $shortCode, ?string $domain = null): ShortUrl; + public function resolveEnabledShortUrl(ShortUrlIdentifier $identifier): ShortUrl; } diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index dbfb1db6..a505c1fb 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -8,6 +8,7 @@ use Doctrine\ORM; use Laminas\Paginator\Paginator; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter; @@ -47,7 +48,7 @@ class ShortUrlService implements ShortUrlServiceInterface */ public function setTagsByShortCode(string $shortCode, array $tags = []): ShortUrl { - $shortUrl = $this->findByShortCode($this->em, $shortCode); + $shortUrl = $this->findByShortCode($this->em, new ShortUrlIdentifier($shortCode)); $shortUrl->setTags($this->tagNamesToEntities($this->em, $tags)); $this->em->flush(); @@ -59,7 +60,7 @@ class ShortUrlService implements ShortUrlServiceInterface */ public function updateMetadataByShortCode(string $shortCode, ShortUrlMeta $shortUrlMeta): ShortUrl { - $shortUrl = $this->findByShortCode($this->em, $shortCode); + $shortUrl = $this->findByShortCode($this->em, new ShortUrlIdentifier($shortCode)); $shortUrl->updateMeta($shortUrlMeta); $this->em->flush(); diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index 91c83a03..6d31243b 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -11,6 +11,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Paginator\Adapter\VisitsPaginatorAdapter; @@ -56,7 +57,7 @@ class VisitsTracker implements VisitsTrackerInterface /** @var ORM\EntityRepository $repo */ $repo = $this->em->getRepository(ShortUrl::class); if ($repo->count(['shortCode' => $shortCode]) < 1) { - throw ShortUrlNotFoundException::fromNotFoundShortCode($shortCode); + throw ShortUrlNotFoundException::fromNotFound(new ShortUrlIdentifier($shortCode)); // FIXME } /** @var VisitRepository $repo */ diff --git a/module/Core/src/Service/VisitsTrackerInterface.php b/module/Core/src/Service/VisitsTrackerInterface.php index 5eddd96d..c8acc09f 100644 --- a/module/Core/src/Service/VisitsTrackerInterface.php +++ b/module/Core/src/Service/VisitsTrackerInterface.php @@ -15,7 +15,7 @@ interface VisitsTrackerInterface /** * Tracks a new visit to provided short code from provided visitor */ - public function track(string $shortCode, Visitor $visitor): void; + public function track(string $shortCode, Visitor $visitor): void; // FIXME /** * Returns the visits on certain short code @@ -23,5 +23,5 @@ interface VisitsTrackerInterface * @return Visit[]|Paginator * @throws ShortUrlNotFoundException */ - public function info(string $shortCode, VisitsParams $params): Paginator; + public function info(string $shortCode, VisitsParams $params): Paginator; // FIXME } diff --git a/module/Core/test/Action/PixelActionTest.php b/module/Core/test/Action/PixelActionTest.php index ed9d0774..f4aed872 100644 --- a/module/Core/test/Action/PixelActionTest.php +++ b/module/Core/test/Action/PixelActionTest.php @@ -12,6 +12,7 @@ use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Common\Response\PixelResponse; use Shlinkio\Shlink\Core\Action\PixelAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Service\VisitsTracker; @@ -38,7 +39,7 @@ class PixelActionTest extends TestCase public function imageIsReturned(): void { $shortCode = 'abc123'; - $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willReturn( + $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, ''))->willReturn( new ShortUrl('http://domain.com/foo/bar'), )->shouldBeCalledOnce(); $this->visitTracker->track(Argument::cetera())->shouldBeCalledOnce(); diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 63df94af..2a4d1a19 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -15,6 +15,7 @@ use Shlinkio\Shlink\Common\Response\QrCodeResponse; use Shlinkio\Shlink\Core\Action\QrCodeAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; class QrCodeActionTest extends TestCase @@ -36,8 +37,9 @@ class QrCodeActionTest extends TestCase public function aNotFoundShortCodeWillDelegateIntoNextMiddleware(): void { $shortCode = 'abc123'; - $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) - ->shouldBeCalledOnce(); + $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, '')) + ->willThrow(ShortUrlNotFoundException::class) + ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); $process = $delegate->handle(Argument::any())->willReturn(new Response()); @@ -50,8 +52,9 @@ class QrCodeActionTest extends TestCase public function anInvalidShortCodeWillReturnNotFoundResponse(): void { $shortCode = 'abc123'; - $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) - ->shouldBeCalledOnce(); + $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, '')) + ->willThrow(ShortUrlNotFoundException::class) + ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); $process = $delegate->handle(Argument::any())->willReturn(new Response()); @@ -64,8 +67,9 @@ class QrCodeActionTest extends TestCase public function aCorrectRequestReturnsTheQrCodeResponse(): void { $shortCode = 'abc123'; - $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willReturn(new ShortUrl('')) - ->shouldBeCalledOnce(); + $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, '')) + ->willReturn(new ShortUrl('')) + ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); $resp = $this->action->process( diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index 25e67f4b..8d28f21c 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -13,6 +13,7 @@ use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Options; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; @@ -45,7 +46,8 @@ class RedirectActionTest extends TestCase { $shortCode = 'abc123'; $shortUrl = new ShortUrl('http://domain.com/foo/bar?some=thing'); - $shortCodeToUrl = $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willReturn($shortUrl); + $shortCodeToUrl = $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, '')) + ->willReturn($shortUrl); $track = $this->visitTracker->track(Argument::cetera())->will(function (): void { }); @@ -74,8 +76,9 @@ class RedirectActionTest extends TestCase public function nextMiddlewareIsInvokedIfLongUrlIsNotFound(): void { $shortCode = 'abc123'; - $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) - ->shouldBeCalledOnce(); + $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, '')) + ->willThrow(ShortUrlNotFoundException::class) + ->shouldBeCalledOnce(); $this->visitTracker->track(Argument::cetera())->shouldNotBeCalled(); $handler = $this->prophesize(RequestHandlerInterface::class); diff --git a/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php b/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php index be02a66c..d0d77fb8 100644 --- a/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php +++ b/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php @@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Core\Exception; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; class ShortUrlNotFoundExceptionTest extends TestCase { @@ -23,7 +24,7 @@ class ShortUrlNotFoundExceptionTest extends TestCase $expectedAdditional['domain'] = $domain; } - $e = ShortUrlNotFoundException::fromNotFoundShortCode($shortCode, $domain); + $e = ShortUrlNotFoundException::fromNotFound(new ShortUrlIdentifier($shortCode, $domain)); $this->assertEquals($expectedMessage, $e->getMessage()); $this->assertEquals($expectedMessage, $e->getDetail()); diff --git a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php index e7aefd9d..49d6f933 100644 --- a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php @@ -28,15 +28,15 @@ class DeleteShortUrlServiceTest extends TestCase public function setUp(): void { - $shortUrl = (new ShortUrl(''))->setVisits( - new ArrayCollection(map(range(0, 10), fn () => new Visit(new ShortUrl(''), Visitor::emptyInstance()))), - ); + $shortUrl = (new ShortUrl(''))->setVisits(new ArrayCollection( + map(range(0, 10), fn () => new Visit(new ShortUrl(''), Visitor::emptyInstance())), + )); $this->shortCode = $shortUrl->getShortCode(); $this->em = $this->prophesize(EntityManagerInterface::class); $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $repo->findOneBy(Argument::type('array'))->willReturn($shortUrl); + $repo->findOneByShortCode(Argument::cetera())->willReturn($shortUrl); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); } diff --git a/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php b/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php index 58f79606..cde37599 100644 --- a/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php +++ b/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php @@ -12,6 +12,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; @@ -41,7 +42,7 @@ class ShortUrlResolverTest extends TestCase $findOneByShortCode = $repo->findOneByShortCode($shortCode, null)->willReturn($shortUrl); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $result = $this->urlResolver->shortCodeToShortUrl($shortCode); + $result = $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode)); $this->assertSame($shortUrl, $result); $findOneByShortCode->shouldHaveBeenCalledOnce(); @@ -61,7 +62,7 @@ class ShortUrlResolverTest extends TestCase $findOneByShortCode->shouldBeCalledOnce(); $getRepo->shouldBeCalledOnce(); - $this->urlResolver->shortCodeToShortUrl($shortCode); + $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode)); } /** @test */ @@ -74,7 +75,7 @@ class ShortUrlResolverTest extends TestCase $findOneByShortCode = $repo->findOneByShortCode($shortCode, null)->willReturn($shortUrl); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $result = $this->urlResolver->shortCodeToEnabledShortUrl($shortCode); + $result = $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode)); $this->assertSame($shortUrl, $result); $findOneByShortCode->shouldHaveBeenCalledOnce(); @@ -97,7 +98,7 @@ class ShortUrlResolverTest extends TestCase $findOneByShortCode->shouldBeCalledOnce(); $getRepo->shouldBeCalledOnce(); - $this->urlResolver->shortCodeToEnabledShortUrl($shortCode); + $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode)); } public function provideDisabledShortUrls(): iterable diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php index 69f0785d..2fb92f53 100644 --- a/module/Core/test/Service/ShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrlServiceTest.php @@ -57,8 +57,8 @@ class ShortUrlServiceTest extends TestCase { $shortCode = 'abc123'; $repo = $this->prophesize(ShortUrlRepository::class); - $repo->findOneBy(['shortCode' => $shortCode])->willReturn(null) - ->shouldBeCalledOnce(); + $repo->findOneByShortCode($shortCode, null)->willReturn(null) + ->shouldBeCalledOnce(); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $this->expectException(ShortUrlNotFoundException::class); @@ -72,8 +72,8 @@ class ShortUrlServiceTest extends TestCase $shortUrl->setTags(Argument::any())->shouldBeCalledOnce(); $shortCode = 'abc123'; $repo = $this->prophesize(ShortUrlRepository::class); - $repo->findOneBy(['shortCode' => $shortCode])->willReturn($shortUrl->reveal()) - ->shouldBeCalledOnce(); + $repo->findOneByShortCode($shortCode, null)->willReturn($shortUrl->reveal()) + ->shouldBeCalledOnce(); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $tagRepo = $this->prophesize(EntityRepository::class); @@ -90,7 +90,7 @@ class ShortUrlServiceTest extends TestCase $shortUrl = new ShortUrl(''); $repo = $this->prophesize(ShortUrlRepository::class); - $findShortUrl = $repo->findOneBy(['shortCode' => 'abc123'])->willReturn($shortUrl); + $findShortUrl = $repo->findOneByShortCode('abc123', null)->willReturn($shortUrl); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $flush = $this->em->flush()->willReturn(null); diff --git a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php index 03458a2c..bf31805d 100644 --- a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php @@ -8,6 +8,7 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -32,11 +33,9 @@ class ResolveShortUrlAction extends AbstractRestAction public function handle(Request $request): Response { - $shortCode = $request->getAttribute('shortCode'); - $domain = $request->getQueryParams()['domain'] ?? null; $transformer = new ShortUrlDataTransformer($this->domainConfig); + $url = $this->urlResolver->resolveShortUrl(ShortUrlIdentifier::fromRequest($request)); - $url = $this->urlResolver->shortCodeToShortUrl($shortCode, $domain); return new JsonResponse($transformer->transform($url)); } } diff --git a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php index 067b4e0e..a62b1f95 100644 --- a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php @@ -8,6 +8,7 @@ use Laminas\Diactoros\ServerRequest; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Rest\Action\ShortUrl\ResolveShortUrlAction; @@ -28,7 +29,7 @@ class ResolveShortUrlActionTest extends TestCase public function correctShortCodeReturnsSuccess(): void { $shortCode = 'abc123'; - $this->urlResolver->shortCodeToShortUrl($shortCode, null)->willReturn( + $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode))->willReturn( new ShortUrl('http://domain.com/foo/bar'), )->shouldBeCalledOnce();