From 0c5eec7e95b7a7d6a817dd73316343f2d7281d47 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 25 Nov 2019 18:54:25 +0100 Subject: [PATCH] Replaced the use of EntityDoesNotExistException by ShorturlNotFoundException where applicable --- .../Command/ShortUrl/ResolveUrlCommand.php | 4 -- .../ShortUrl/ResolveUrlCommandTest.php | 19 ++-------- .../src/Action/AbstractTrackingAction.php | 3 +- module/Core/src/Action/PreviewAction.php | 3 +- module/Core/src/Action/QrCodeAction.php | 3 +- .../src/Exception/NonUniqueSlugException.php | 6 +-- .../Exception/ShortUrlNotFoundException.php | 5 ++- module/Core/src/Service/UrlShortener.php | 9 ++--- .../src/Service/UrlShortenerInterface.php | 6 +-- module/Core/test/Action/PreviewActionTest.php | 14 ------- module/Core/test/Action/QrCodeActionTest.php | 3 +- .../Core/test/Action/RedirectActionTest.php | 4 +- .../InvalidShortCodeExceptionTest.php | 19 ---------- .../ShortUrlNotFoundExceptionTest.php | 38 +++++++++++++++++++ .../Action/ShortUrl/ResolveShortUrlAction.php | 16 +------- .../Action/ResolveShortUrlActionTest.php | 2 +- .../ShortUrl/ResolveShortUrlActionTest.php | 15 -------- 17 files changed, 60 insertions(+), 109 deletions(-) delete mode 100644 module/Core/test/Exception/InvalidShortCodeExceptionTest.php create mode 100644 module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php diff --git a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php index b758ddd4..28564369 100644 --- a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Util\ExitCodes; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Symfony\Component\Console\Command\Command; @@ -66,9 +65,6 @@ class ResolveUrlCommand extends Command $output->writeln(sprintf('Long URL: %s', $url->getLongUrl())); return ExitCodes::EXIT_SUCCESS; } catch (ShortUrlNotFoundException $e) { - $io->error(sprintf('Provided short code "%s" has an invalid format.', $shortCode)); - return ExitCodes::EXIT_FAILURE; - } catch (EntityDoesNotExistException $e) { $io->error(sprintf('Provided short code "%s" could not be found.', $shortCode)); return ExitCodes::EXIT_FAILURE; } diff --git a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php index 9e86bbd1..23b4ec28 100644 --- a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php @@ -8,12 +8,13 @@ use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\ShortUrl\ResolveUrlCommand; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; +use function sprintf; + use const PHP_EOL; class ResolveUrlCommandTest extends TestCase @@ -51,23 +52,11 @@ class ResolveUrlCommandTest extends TestCase public function incorrectShortCodeOutputsErrorMessage(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(EntityDoesNotExistException::class) + $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(ShortUrlNotFoundException::class) ->shouldBeCalledOnce(); $this->commandTester->execute(['shortCode' => $shortCode]); $output = $this->commandTester->getDisplay(); - $this->assertStringContainsString('Provided short code "' . $shortCode . '" could not be found.', $output); - } - - /** @test */ - public function wrongShortCodeFormatOutputsErrorMessage(): void - { - $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(new ShortUrlNotFoundException()) - ->shouldBeCalledOnce(); - - $this->commandTester->execute(['shortCode' => $shortCode]); - $output = $this->commandTester->getDisplay(); - $this->assertStringContainsString('Provided short code "' . $shortCode . '" has an invalid format.', $output); + $this->assertStringContainsString(sprintf('Provided short code "%s" could not be found', $shortCode), $output); } } diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 5bf195f1..ff8d91f2 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -11,7 +11,6 @@ use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Options\AppOptions; @@ -72,7 +71,7 @@ abstract class AbstractTrackingAction implements MiddlewareInterface } return $this->createSuccessResp($this->buildUrlToRedirectTo($url, $query, $disableTrackParam)); - } catch (ShortUrlNotFoundException | EntityDoesNotExistException $e) { + } catch (ShortUrlNotFoundException $e) { $this->logger->warning('An error occurred while tracking short code. {e}', ['e' => $e]); return $this->createErrorResp($request, $handler); } diff --git a/module/Core/src/Action/PreviewAction.php b/module/Core/src/Action/PreviewAction.php index 9ba0eaf5..d243f12c 100644 --- a/module/Core/src/Action/PreviewAction.php +++ b/module/Core/src/Action/PreviewAction.php @@ -11,7 +11,6 @@ use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Shlinkio\Shlink\Common\Response\ResponseUtilsTrait; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\PreviewGenerator\Exception\PreviewGenerationException; @@ -56,7 +55,7 @@ class PreviewAction implements MiddlewareInterface $url = $this->urlShortener->shortCodeToUrl($shortCode); $imagePath = $this->previewGenerator->generatePreview($url->getLongUrl()); return $this->generateImageResponse($imagePath); - } catch (ShortUrlNotFoundException | EntityDoesNotExistException | PreviewGenerationException $e) { + } catch (ShortUrlNotFoundException | PreviewGenerationException $e) { $this->logger->warning('An error occurred while generating preview image. {e}', ['e' => $e]); return $handler->handle($request); } diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index 1c5c08df..3cdee70e 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -12,7 +12,6 @@ use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Shlinkio\Shlink\Common\Response\QrCodeResponse; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Zend\Expressive\Router\Exception\RuntimeException; @@ -60,7 +59,7 @@ class QrCodeAction implements MiddlewareInterface try { $this->urlShortener->shortCodeToUrl($shortCode, $domain); - } catch (ShortUrlNotFoundException | EntityDoesNotExistException $e) { + } catch (ShortUrlNotFoundException $e) { $this->logger->warning('An error occurred while creating QR code. {e}', ['e' => $e]); return $handler->handle($request); } diff --git a/module/Core/src/Exception/NonUniqueSlugException.php b/module/Core/src/Exception/NonUniqueSlugException.php index 9e22d354..74e03f06 100644 --- a/module/Core/src/Exception/NonUniqueSlugException.php +++ b/module/Core/src/Exception/NonUniqueSlugException.php @@ -19,11 +19,7 @@ class NonUniqueSlugException extends InvalidArgumentException implements Problem public static function fromSlug(string $slug, ?string $domain): self { - $suffix = ''; - if ($domain !== null) { - $suffix = sprintf(' for domain "%s"', $domain); - } - + $suffix = $domain === null ? '' : sprintf(' for domain "%s"', $domain); $e = new self(sprintf('Provided slug "%s" is already in use%s.', $slug, $suffix)); $e->detail = $e->getMessage(); diff --git a/module/Core/src/Exception/ShortUrlNotFoundException.php b/module/Core/src/Exception/ShortUrlNotFoundException.php index d8682047..e07624c7 100644 --- a/module/Core/src/Exception/ShortUrlNotFoundException.php +++ b/module/Core/src/Exception/ShortUrlNotFoundException.php @@ -17,9 +17,10 @@ class ShortUrlNotFoundException extends DomainException implements ProblemDetail private const TITLE = 'Short URL not found'; public const TYPE = 'INVALID_SHORTCODE'; - public static function fromNotFoundShortCode(string $shortCode): self + public static function fromNotFoundShortCode(string $shortCode, ?string $domain = null): self { - $e = new self(sprintf('No URL found for short code "%s"', $shortCode)); + $suffix = $domain === null ? '' : sprintf(' for domain "%s"', $domain); + $e = new self(sprintf('No URL found with short code "%s"%s', $shortCode, $suffix)); $e->detail = $e->getMessage(); $e->title = self::TITLE; diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index 7d71ca7a..0e4062ec 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -8,9 +8,9 @@ use Doctrine\ORM\EntityManagerInterface; use Psr\Http\Message\UriInterface; use Shlinkio\Shlink\Core\Domain\Resolver\PersistenceDomainResolver; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; @@ -129,7 +129,7 @@ class UrlShortener implements UrlShortenerInterface } /** - * @throws EntityDoesNotExistException + * @throws ShortUrlNotFoundException */ public function shortCodeToUrl(string $shortCode, ?string $domain = null): ShortUrl { @@ -137,10 +137,7 @@ class UrlShortener implements UrlShortenerInterface $shortUrlRepo = $this->em->getRepository(ShortUrl::class); $shortUrl = $shortUrlRepo->findOneByShortCode($shortCode, $domain); if ($shortUrl === null) { - throw EntityDoesNotExistException::createFromEntityAndConditions(ShortUrl::class, [ - 'shortCode' => $shortCode, - 'domain' => $domain, - ]); + throw ShortUrlNotFoundException::fromNotFoundShortCode($shortCode, $domain); } return $shortUrl; diff --git a/module/Core/src/Service/UrlShortenerInterface.php b/module/Core/src/Service/UrlShortenerInterface.php index 9d6b291b..ee9cc343 100644 --- a/module/Core/src/Service/UrlShortenerInterface.php +++ b/module/Core/src/Service/UrlShortenerInterface.php @@ -6,10 +6,9 @@ namespace Shlinkio\Shlink\Core\Service; use Psr\Http\Message\UriInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; -use Shlinkio\Shlink\Core\Exception\RuntimeException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; interface UrlShortenerInterface @@ -18,12 +17,11 @@ interface UrlShortenerInterface * @param string[] $tags * @throws NonUniqueSlugException * @throws InvalidUrlException - * @throws RuntimeException */ public function urlToShortCode(UriInterface $url, array $tags, ShortUrlMeta $meta): ShortUrl; /** - * @throws EntityDoesNotExistException + * @throws ShortUrlNotFoundException */ public function shortCodeToUrl(string $shortCode, ?string $domain = null): ShortUrl; } diff --git a/module/Core/test/Action/PreviewActionTest.php b/module/Core/test/Action/PreviewActionTest.php index 909d70b7..e2cb089c 100644 --- a/module/Core/test/Action/PreviewActionTest.php +++ b/module/Core/test/Action/PreviewActionTest.php @@ -11,7 +11,6 @@ use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\PreviewAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\PreviewGenerator\Service\PreviewGenerator; @@ -38,19 +37,6 @@ class PreviewActionTest extends TestCase $this->action = new PreviewAction($this->previewGenerator->reveal(), $this->urlShortener->reveal()); } - /** @test */ - public function invalidShortCodeFallsBackToNextMiddleware(): void - { - $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class) - ->shouldBeCalledOnce(); - $delegate = $this->prophesize(RequestHandlerInterface::class); - $delegate->handle(Argument::cetera())->shouldBeCalledOnce() - ->willReturn(new Response()); - - $this->action->process((new ServerRequest())->withAttribute('shortCode', $shortCode), $delegate->reveal()); - } - /** @test */ public function correctShortCodeReturnsImageResponse(): void { diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 24fe8c4b..6327ad69 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -11,7 +11,6 @@ use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Common\Response\QrCodeResponse; use Shlinkio\Shlink\Core\Action\QrCodeAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Zend\Diactoros\Response; @@ -39,7 +38,7 @@ class QrCodeActionTest extends TestCase public function aNotFoundShortCodeWillDelegateIntoNextMiddleware(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(EntityDoesNotExistException::class) + $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); $process = $delegate->handle(Argument::any())->willReturn(new Response()); diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index a65927eb..55342cb5 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -10,7 +10,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Options; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Service\VisitsTracker; @@ -76,7 +76,7 @@ class RedirectActionTest extends TestCase public function nextMiddlewareIsInvokedIfLongUrlIsNotFound(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(EntityDoesNotExistException::class) + $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) ->shouldBeCalledOnce(); $this->visitTracker->track(Argument::cetera())->shouldNotBeCalled(); diff --git a/module/Core/test/Exception/InvalidShortCodeExceptionTest.php b/module/Core/test/Exception/InvalidShortCodeExceptionTest.php deleted file mode 100644 index c1815db4..00000000 --- a/module/Core/test/Exception/InvalidShortCodeExceptionTest.php +++ /dev/null @@ -1,19 +0,0 @@ -assertEquals('No URL found for short code "abc123"', $e->getMessage()); - } -} diff --git a/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php b/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php new file mode 100644 index 00000000..6f0d58f4 --- /dev/null +++ b/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php @@ -0,0 +1,38 @@ +assertEquals($expectedMessage, $e->getMessage()); + } + + public function provideMessages(): iterable + { + yield 'without domain' => [ + 'No URL found with short code "abc123"', + 'abc123', + null, + ]; + yield 'with domain' => [ + 'No URL found with short code "bar" for domain "foo"', + 'bar', + 'foo', + ]; + } +} diff --git a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php index 9599013a..e041f4dc 100644 --- a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php @@ -8,15 +8,11 @@ use InvalidArgumentException; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\JsonResponse; -use function sprintf; - class ResolveShortUrlAction extends AbstractRestAction { protected const ROUTE_PATH = '/short-urls/{shortCode}'; @@ -48,15 +44,7 @@ class ResolveShortUrlAction extends AbstractRestAction $domain = $request->getQueryParams()['domain'] ?? null; $transformer = new ShortUrlDataTransformer($this->domainConfig); - try { - $url = $this->urlShortener->shortCodeToUrl($shortCode, $domain); - return new JsonResponse($transformer->transform($url)); - } catch (EntityDoesNotExistException $e) { - $this->logger->warning('Provided short code couldn\'t be found. {e}', ['e' => $e]); - return new JsonResponse([ - 'error' => RestUtils::INVALID_ARGUMENT_ERROR, // FIXME Not correct. Use correct value on "type" - 'message' => sprintf('No URL found for short code "%s"', $shortCode), - ], self::STATUS_NOT_FOUND); - } + $url = $this->urlShortener->shortCodeToUrl($shortCode, $domain); + return new JsonResponse($transformer->transform($url)); } } diff --git a/module/Rest/test-api/Action/ResolveShortUrlActionTest.php b/module/Rest/test-api/Action/ResolveShortUrlActionTest.php index 117ae5a9..4065517e 100644 --- a/module/Rest/test-api/Action/ResolveShortUrlActionTest.php +++ b/module/Rest/test-api/Action/ResolveShortUrlActionTest.php @@ -16,6 +16,6 @@ class ResolveShortUrlActionTest extends ApiTestCase ['error' => $error] = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_ARGUMENT_ERROR, $error); + $this->assertEquals(RestUtils::INVALID_SHORTCODE_ERROR, $error); } } diff --git a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php index aa852de5..b77a6e45 100644 --- a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php @@ -7,10 +7,8 @@ namespace ShlinkioTest\Shlink\Rest\Action\ShortUrl; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Rest\Action\ShortUrl\ResolveShortUrlAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\ServerRequest; use function strpos; @@ -28,19 +26,6 @@ class ResolveShortUrlActionTest extends TestCase $this->action = new ResolveShortUrlAction($this->urlShortener->reveal(), []); } - /** @test */ - public function incorrectShortCodeReturnsError(): void - { - $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(EntityDoesNotExistException::class) - ->shouldBeCalledOnce(); - - $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); - $response = $this->action->handle($request); - $this->assertEquals(404, $response->getStatusCode()); - $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::INVALID_ARGUMENT_ERROR) > 0); - } - /** @test */ public function correctShortCodeReturnsSuccess(): void {