diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index 6b04d63a..7d71ca7a 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -9,7 +9,6 @@ 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\InvalidShortCodeException; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; @@ -130,7 +129,6 @@ class UrlShortener implements UrlShortenerInterface } /** - * @throws InvalidShortCodeException * @throws EntityDoesNotExistException */ public function shortCodeToUrl(string $shortCode, ?string $domain = null): ShortUrl diff --git a/module/Core/src/Service/UrlShortenerInterface.php b/module/Core/src/Service/UrlShortenerInterface.php index 74d626fc..9d6b291b 100644 --- a/module/Core/src/Service/UrlShortenerInterface.php +++ b/module/Core/src/Service/UrlShortenerInterface.php @@ -7,7 +7,6 @@ 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\InvalidShortCodeException; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Exception\RuntimeException; @@ -24,7 +23,6 @@ interface UrlShortenerInterface public function urlToShortCode(UriInterface $url, array $tags, ShortUrlMeta $meta): ShortUrl; /** - * @throws InvalidShortCodeException * @throws EntityDoesNotExistException */ public function shortCodeToUrl(string $shortCode, ?string $domain = null): ShortUrl; diff --git a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php index 57d3cd62..9599013a 100644 --- a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php @@ -4,12 +4,11 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Action\ShortUrl; -use Exception; +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\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -41,7 +40,7 @@ class ResolveShortUrlAction extends AbstractRestAction /** * @param Request $request * @return Response - * @throws \InvalidArgumentException + * @throws InvalidArgumentException */ public function handle(Request $request): Response { @@ -52,24 +51,12 @@ class ResolveShortUrlAction extends AbstractRestAction try { $url = $this->urlShortener->shortCodeToUrl($shortCode, $domain); return new JsonResponse($transformer->transform($url)); - } catch (InvalidShortCodeException $e) { - $this->logger->warning('Provided short code with invalid format. {e}', ['e' => $e]); - return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf('Provided short code "%s" has an invalid format', $shortCode), - ], self::STATUS_BAD_REQUEST); } catch (EntityDoesNotExistException $e) { $this->logger->warning('Provided short code couldn\'t be found. {e}', ['e' => $e]); return new JsonResponse([ - 'error' => RestUtils::INVALID_ARGUMENT_ERROR, + '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); - } catch (Exception $e) { - $this->logger->error('Unexpected error while resolving the URL behind a short code. {e}', ['e' => $e]); - return new JsonResponse([ - 'error' => RestUtils::UNKNOWN_ERROR, - 'message' => 'Unexpected error occurred', - ], self::STATUS_INTERNAL_SERVER_ERROR); } } } diff --git a/module/Rest/test-api/Action/DeleteShortUrlActionTest.php b/module/Rest/test-api/Action/DeleteShortUrlActionTest.php index 60631565..c90b7818 100644 --- a/module/Rest/test-api/Action/DeleteShortUrlActionTest.php +++ b/module/Rest/test-api/Action/DeleteShortUrlActionTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Action; -use GuzzleHttp\RequestOptions; use Shlinkio\Shlink\Rest\Util\RestUtils; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; diff --git a/module/Rest/test-api/Action/ResolveShortUrlActionTest.php b/module/Rest/test-api/Action/ResolveShortUrlActionTest.php new file mode 100644 index 00000000..117ae5a9 --- /dev/null +++ b/module/Rest/test-api/Action/ResolveShortUrlActionTest.php @@ -0,0 +1,21 @@ +callApiWithKey(self::METHOD_GET, '/short-urls/invalid'); + ['error' => $error] = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); + $this->assertEquals(RestUtils::INVALID_ARGUMENT_ERROR, $error); + } +} diff --git a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php index e0c23d31..aa852de5 100644 --- a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php @@ -4,12 +4,10 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Action\ShortUrl; -use Exception; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Rest\Action\ShortUrl\ResolveShortUrlAction; use Shlinkio\Shlink\Rest\Util\RestUtils; @@ -56,30 +54,4 @@ class ResolveShortUrlActionTest extends TestCase $this->assertEquals(200, $response->getStatusCode()); $this->assertTrue(strpos($response->getBody()->getContents(), 'http://domain.com/foo/bar') > 0); } - - /** @test */ - public function invalidShortCodeExceptionReturnsError(): void - { - $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(InvalidShortCodeException::class) - ->shouldBeCalledOnce(); - - $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); - $response = $this->action->handle($request); - $this->assertEquals(400, $response->getStatusCode()); - $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::INVALID_SHORTCODE_ERROR) > 0); - } - - /** @test */ - public function unexpectedExceptionWillReturnError(): void - { - $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(Exception::class) - ->shouldBeCalledOnce(); - - $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); - $response = $this->action->handle($request); - $this->assertEquals(500, $response->getStatusCode()); - $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::UNKNOWN_ERROR) > 0); - } }