From 310032e3035dc90c664955a22e1058863eb38d46 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 24 Nov 2019 23:56:02 +0100 Subject: [PATCH] Converted DeleteShortUrlException into a problem details exception --- .../src/Exception/DeleteShortUrlException.php | 20 +++++++++++-- .../Action/ShortUrl/DeleteShortUrlAction.php | 24 ++------------- module/Rest/src/Util/RestUtils.php | 4 +-- .../Action/DeleteShortUrlActionTest.php | 4 +-- .../ShortUrl/DeleteShortUrlActionTest.php | 30 ------------------- 5 files changed, 24 insertions(+), 58 deletions(-) diff --git a/module/Core/src/Exception/DeleteShortUrlException.php b/module/Core/src/Exception/DeleteShortUrlException.php index c1fd6dd7..037fe942 100644 --- a/module/Core/src/Exception/DeleteShortUrlException.php +++ b/module/Core/src/Exception/DeleteShortUrlException.php @@ -4,12 +4,20 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Exception; +use Fig\Http\Message\StatusCodeInterface; use Throwable; +use Zend\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; +use Zend\ProblemDetails\Exception\ProblemDetailsExceptionInterface; use function sprintf; -class DeleteShortUrlException extends RuntimeException +class DeleteShortUrlException extends DomainException implements ProblemDetailsExceptionInterface { + use CommonProblemDetailsExceptionTrait; + + private const TITLE = 'Cannot delete short URL'; + public const TYPE = 'INVALID_SHORTCODE_DELETION'; // FIXME Should be INVALID_SHORT_URL_DELETION + /** @var int */ private $visitsThreshold; @@ -21,11 +29,19 @@ class DeleteShortUrlException extends RuntimeException public static function fromVisitsThreshold(int $threshold, string $shortCode): self { - return new self($threshold, sprintf( + $e = new self($threshold, sprintf( 'Impossible to delete short URL with short code "%s" since it has more than "%s" visits.', $shortCode, $threshold )); + + $e->detail = $e->getMessage(); + $e->title = self::TITLE; + $e->type = self::TYPE; + $e->status = StatusCodeInterface::STATUS_UNPROCESSABLE_ENTITY; + $e->additional = ['threshold' => $threshold]; + + return $e; } public function getVisitsThreshold(): int diff --git a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php index f6873025..ba39ec82 100644 --- a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php @@ -7,14 +7,9 @@ namespace Shlinkio\Shlink\Rest\Action\ShortUrl; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\Exception; use Shlinkio\Shlink\Core\Service\ShortUrl\DeleteShortUrlServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\EmptyResponse; -use Zend\Diactoros\Response\JsonResponse; - -use function sprintf; class DeleteShortUrlAction extends AbstractRestAction { @@ -30,25 +25,10 @@ class DeleteShortUrlAction extends AbstractRestAction $this->deleteShortUrlService = $deleteShortUrlService; } - /** - * Handle the request and return a response. - */ public function handle(ServerRequestInterface $request): ResponseInterface { $shortCode = $request->getAttribute('shortCode', ''); - - try { - $this->deleteShortUrlService->deleteByShortCode($shortCode); - return new EmptyResponse(); - } catch (Exception\DeleteShortUrlException $e) { - $this->logger->warning('Provided data is invalid. {e}', ['e' => $e]); - $messagePlaceholder = - 'It is not possible to delete URL with short code "%s" because it has reached more than "%s" visits.'; - - return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf($messagePlaceholder, $shortCode, $e->getVisitsThreshold()), - ], self::STATUS_BAD_REQUEST); - } + $this->deleteShortUrlService->deleteByShortCode($shortCode); + return new EmptyResponse(); } } diff --git a/module/Rest/src/Util/RestUtils.php b/module/Rest/src/Util/RestUtils.php index 096db51a..97ca0d0b 100644 --- a/module/Rest/src/Util/RestUtils.php +++ b/module/Rest/src/Util/RestUtils.php @@ -13,8 +13,8 @@ class RestUtils { /** @deprecated */ public const INVALID_SHORTCODE_ERROR = Core\ShortUrlNotFoundException::TYPE; - // FIXME Should be INVALID_SHORT_URL_DELETION - public const INVALID_SHORTCODE_DELETION_ERROR = 'INVALID_SHORTCODE_DELETION'; + /** @deprecated */ + public const INVALID_SHORTCODE_DELETION_ERROR = Core\DeleteShortUrlException::TYPE; /** @deprecated */ public const INVALID_URL_ERROR = Core\InvalidUrlException::TYPE; /** @deprecated */ diff --git a/module/Rest/test-api/Action/DeleteShortUrlActionTest.php b/module/Rest/test-api/Action/DeleteShortUrlActionTest.php index c90b7818..042b8e90 100644 --- a/module/Rest/test-api/Action/DeleteShortUrlActionTest.php +++ b/module/Rest/test-api/Action/DeleteShortUrlActionTest.php @@ -20,7 +20,7 @@ class DeleteShortUrlActionTest extends ApiTestCase } /** @test */ - public function badRequestIsReturnedWhenTryingToDeleteUrlWithTooManyVisits(): void + public function unprocessableEntityIsReturnedWhenTryingToDeleteUrlWithTooManyVisits(): void { // Generate visits first for ($i = 0; $i < 20; $i++) { @@ -30,7 +30,7 @@ class DeleteShortUrlActionTest extends ApiTestCase $resp = $this->callApiWithKey(self::METHOD_DELETE, '/short-urls/abc123'); ['error' => $error] = $this->getJsonResponsePayload($resp); - $this->assertEquals(self::STATUS_BAD_REQUEST, $resp->getStatusCode()); + $this->assertEquals(self::STATUS_UNPROCESSABLE_ENTITY, $resp->getStatusCode()); $this->assertEquals(RestUtils::INVALID_SHORTCODE_DELETION_ERROR, $error); } } diff --git a/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php index 2e77d10a..782181ce 100644 --- a/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php @@ -7,12 +7,8 @@ namespace ShlinkioTest\Shlink\Rest\Action\ShortUrl; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Core\Exception; use Shlinkio\Shlink\Core\Service\ShortUrl\DeleteShortUrlServiceInterface; use Shlinkio\Shlink\Rest\Action\ShortUrl\DeleteShortUrlAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; -use Throwable; -use Zend\Diactoros\Response\JsonResponse; use Zend\Diactoros\ServerRequest; class DeleteShortUrlActionTest extends TestCase @@ -39,30 +35,4 @@ class DeleteShortUrlActionTest extends TestCase $this->assertEquals(204, $resp->getStatusCode()); $deleteByShortCode->shouldHaveBeenCalledOnce(); } - - /** - * @test - * @dataProvider provideExceptions - */ - public function returnsErrorResponseInCaseOfException(Throwable $e, string $error, int $statusCode): void - { - $deleteByShortCode = $this->service->deleteByShortCode(Argument::any())->willThrow($e); - - /** @var JsonResponse $resp */ - $resp = $this->action->handle(new ServerRequest()); - $payload = $resp->getPayload(); - - $this->assertEquals($statusCode, $resp->getStatusCode()); - $this->assertEquals($error, $payload['error']); - $deleteByShortCode->shouldHaveBeenCalledOnce(); - } - - public function provideExceptions(): iterable - { - yield 'bad request' => [ - new Exception\DeleteShortUrlException(5), - RestUtils::INVALID_SHORTCODE_DELETION_ERROR, - 400, - ]; - } }