From 09321eaa93a3fae6f30f01ce1606268e9f4027b8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 23 Nov 2019 13:41:07 +0100 Subject: [PATCH] Updated InvalidShortCodeException to implement ProblemDetails --- .../Exception/InvalidShortCodeException.php | 31 ++++++++++++------- .../InvalidShortCodeExceptionTest.php | 23 +------------- .../Action/ShortUrl/DeleteShortUrlAction.php | 9 ------ module/Rest/src/Util/RestUtils.php | 4 ++- .../ShortUrl/DeleteShortUrlActionTest.php | 1 - 5 files changed, 23 insertions(+), 45 deletions(-) diff --git a/module/Core/src/Exception/InvalidShortCodeException.php b/module/Core/src/Exception/InvalidShortCodeException.php index 37ecfffb..77240aca 100644 --- a/module/Core/src/Exception/InvalidShortCodeException.php +++ b/module/Core/src/Exception/InvalidShortCodeException.php @@ -4,24 +4,31 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Exception; -use Throwable; +use Fig\Http\Message\StatusCodeInterface; +use Zend\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; +use Zend\ProblemDetails\Exception\ProblemDetailsExceptionInterface; use function sprintf; -class InvalidShortCodeException extends RuntimeException +class InvalidShortCodeException extends RuntimeException implements ProblemDetailsExceptionInterface { - public static function fromCharset(string $shortCode, string $charSet, ?Throwable $previous = null): self - { - $code = $previous !== null ? $previous->getCode() : -1; - return new static( - sprintf('Provided short code "%s" does not match the char set "%s"', $shortCode, $charSet), - $code, - $previous - ); - } + use CommonProblemDetailsExceptionTrait; + + public const TITLE = 'Invalid short code'; + public const TYPE = 'INVALID_SHORTCODE'; public static function fromNotFoundShortCode(string $shortCode): self { - return new static(sprintf('Provided short code "%s" does not belong to a short URL', $shortCode)); + $e = new self(sprintf('No URL found for short code "%s"', $shortCode)); + $e->detail = $e->getMessage(); + $e->title = self::TITLE; + $e->type = self::TYPE; + $e->status = StatusCodeInterface::STATUS_NOT_FOUND; + $e->additional = [ + 'error' => $e->type, + 'message' => $e->detail, + ]; + + return $e; } } diff --git a/module/Core/test/Exception/InvalidShortCodeExceptionTest.php b/module/Core/test/Exception/InvalidShortCodeExceptionTest.php index 02a3edf2..4639fda3 100644 --- a/module/Core/test/Exception/InvalidShortCodeExceptionTest.php +++ b/module/Core/test/Exception/InvalidShortCodeExceptionTest.php @@ -4,37 +4,16 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Exception; -use Exception; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; -use Throwable; class InvalidShortCodeExceptionTest extends TestCase { - /** - * @test - * @dataProvider providePrevious - */ - public function properlyCreatesExceptionFromCharset(?Throwable $prev): void - { - $e = InvalidShortCodeException::fromCharset('abc123', 'def456', $prev); - - $this->assertEquals('Provided short code "abc123" does not match the char set "def456"', $e->getMessage()); - $this->assertEquals($prev !== null ? $prev->getCode() : -1, $e->getCode()); - $this->assertEquals($prev, $e->getPrevious()); - } - - public function providePrevious(): iterable - { - yield 'null previous' => [null]; - yield 'instance previous' => [new Exception('Previous error', 10)]; - } - /** @test */ public function properlyCreatesExceptionFromNotFoundShortCode(): void { $e = InvalidShortCodeException::fromNotFoundShortCode('abc123'); - $this->assertEquals('Provided short code "abc123" does not belong to a short URL', $e->getMessage()); + $this->assertEquals('No URL found for short code "abc123"', $e->getMessage()); } } diff --git a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php index 54755baa..f6873025 100644 --- a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php @@ -40,15 +40,6 @@ class DeleteShortUrlAction extends AbstractRestAction try { $this->deleteShortUrlService->deleteByShortCode($shortCode); return new EmptyResponse(); - } catch (Exception\InvalidShortCodeException $e) { - $this->logger->warning( - 'Provided short code {shortCode} does not belong to any URL. {e}', - ['e' => $e, 'shortCode' => $shortCode] - ); - return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf('No URL found for short code "%s"', $shortCode), - ], self::STATUS_NOT_FOUND); } catch (Exception\DeleteShortUrlException $e) { $this->logger->warning('Provided data is invalid. {e}', ['e' => $e]); $messagePlaceholder = diff --git a/module/Rest/src/Util/RestUtils.php b/module/Rest/src/Util/RestUtils.php index 60aade54..71c65e88 100644 --- a/module/Rest/src/Util/RestUtils.php +++ b/module/Rest/src/Util/RestUtils.php @@ -6,12 +6,14 @@ namespace Shlinkio\Shlink\Rest\Util; use Shlinkio\Shlink\Common\Exception as Common; use Shlinkio\Shlink\Core\Exception as Core; +use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Rest\Exception as Rest; use Throwable; class RestUtils { - public const INVALID_SHORTCODE_ERROR = 'INVALID_SHORTCODE'; + /** @deprecated */ + public const INVALID_SHORTCODE_ERROR = InvalidShortCodeException::TYPE; // FIXME Should be INVALID_SHORT_URL_DELETION public const INVALID_SHORTCODE_DELETION_ERROR = 'INVALID_SHORTCODE_DELETION'; public const INVALID_URL_ERROR = 'INVALID_URL'; diff --git a/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php index 35f87dc8..2e77d10a 100644 --- a/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php @@ -59,7 +59,6 @@ class DeleteShortUrlActionTest extends TestCase public function provideExceptions(): iterable { - yield 'not found' => [new Exception\InvalidShortCodeException(), RestUtils::INVALID_SHORTCODE_ERROR, 404]; yield 'bad request' => [ new Exception\DeleteShortUrlException(5), RestUtils::INVALID_SHORTCODE_DELETION_ERROR,