Converted DeleteShortUrlException into a problem details exception

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

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();
     }
 }

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 */

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);
     }
 }

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,
-        ];
-    }
 }