Converted EntityDoesNotExistException into a problem details exception renamed as TagNotFoundException

docs/swagger/paths/v1_short-urls_shorten.json

@@ -112,10 +112,10 @@
                 },
                 "examples": {
                     "application/json": {
-                        "error": "UNKNOWN_ERROR",
+                        "error": "INTERNAL_SERVER_ERROR",
                         "message": "Unexpected error occurred"
                     },
-                    "text/plain": "UNKNOWN_ERROR"
+                    "text/plain": "INTERNAL_SERVER_ERROR"
                 }
             }
         }

module/CLI/src/Command/Tag/RenameTagCommand.php

@@ -5,7 +5,7 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\CLI\Command\Tag;
 
 use Shlinkio\Shlink\CLI\Util\ExitCodes;
-use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
+use Shlinkio\Shlink\Core\Exception\TagNotFoundException;
 use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputArgument;
@@ -47,7 +47,7 @@ class RenameTagCommand extends Command
             $this->tagService->renameTag($oldName, $newName);
             $io->success('Tag properly renamed.');
             return ExitCodes::EXIT_SUCCESS;
-        } catch (EntityDoesNotExistException $e) {
+        } catch (TagNotFoundException $e) {
             $io->error(sprintf('A tag with name "%s" was not found', $oldName));
             return ExitCodes::EXIT_FAILURE;
         }

module/CLI/test/Command/Tag/RenameTagCommandTest.php

@@ -8,7 +8,7 @@ use PHPUnit\Framework\TestCase;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\CLI\Command\Tag\RenameTagCommand;
 use Shlinkio\Shlink\Core\Entity\Tag;
-use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
+use Shlinkio\Shlink\Core\Exception\TagNotFoundException;
 use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface;
 use Symfony\Component\Console\Application;
 use Symfony\Component\Console\Tester\CommandTester;
@@ -38,7 +38,7 @@ class RenameTagCommandTest extends TestCase
     {
         $oldName = 'foo';
         $newName = 'bar';
-        $renameTag = $this->tagService->renameTag($oldName, $newName)->willThrow(EntityDoesNotExistException::class);
+        $renameTag = $this->tagService->renameTag($oldName, $newName)->willThrow(TagNotFoundException::class);
 
         $this->commandTester->execute([
             'oldName' => $oldName,

module/Core/src/Exception/EntityDoesNotExistException.php

@@ -1,30 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Shlinkio\Shlink\Core\Exception;
-
-use function implode;
-use function sprintf;
-
-class EntityDoesNotExistException extends RuntimeException
-{
-    public static function createFromEntityAndConditions($entityName, array $conditions)
-    {
-        return new self(sprintf(
-            'Entity of type %s with params [%s] does not exist',
-            $entityName,
-            static::serializeParams($conditions)
-        ));
-    }
-
-    private static function serializeParams(array $params)
-    {
-        $result = [];
-        foreach ($params as $key => $value) {
-            $result[] = sprintf('"%s" => "%s"', $key, $value);
-        }
-
-        return implode(', ', $result);
-    }
-}

module/Core/src/Exception/TagNotFoundException.php

@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Exception;
+
+use Fig\Http\Message\StatusCodeInterface;
+use Zend\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait;
+use Zend\ProblemDetails\Exception\ProblemDetailsExceptionInterface;
+
+use function sprintf;
+
+class TagNotFoundException extends DomainException implements ProblemDetailsExceptionInterface
+{
+    use CommonProblemDetailsExceptionTrait;
+
+    private const TITLE = 'Tag not found';
+    public const TYPE = 'TAG_NOT_FOUND';
+
+    public static function fromTag(string $tag): self
+    {
+        $e = new self(sprintf('Tag with name "%s" could not be found', $tag));
+
+        $e->detail = $e->getMessage();
+        $e->title = self::TITLE;
+        $e->type = self::TYPE;
+        $e->status = StatusCodeInterface::STATUS_NOT_FOUND;
+
+        return $e;
+    }
+}

module/Core/src/Service/Tag/TagService.php

@@ -7,7 +7,7 @@ namespace Shlinkio\Shlink\Core\Service\Tag;
 use Doctrine\Common\Collections\Collection;
 use Doctrine\ORM;
 use Shlinkio\Shlink\Core\Entity\Tag;
-use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
+use Shlinkio\Shlink\Core\Exception\TagNotFoundException;
 use Shlinkio\Shlink\Core\Repository\TagRepository;
 use Shlinkio\Shlink\Core\Util\TagManagerTrait;
 
@@ -35,8 +35,7 @@ class TagService implements TagServiceInterface
     }
 
     /**
-     * @param array $tagNames
+     * @param string[] $tagNames
-     * @return void
      */
     public function deleteTags(array $tagNames): void
     {
@@ -60,23 +59,17 @@ class TagService implements TagServiceInterface
     }
 
     /**
-     * @param string $oldName
+     * @throws TagNotFoundException
-     * @param string $newName
-     * @return Tag
-     * @throws EntityDoesNotExistException
-     * @throws ORM\OptimisticLockException
      */
-    public function renameTag($oldName, $newName): Tag
+    public function renameTag(string $oldName, string $newName): Tag
     {
-        $criteria = ['name' => $oldName];
         /** @var Tag|null $tag */
-        $tag = $this->em->getRepository(Tag::class)->findOneBy($criteria);
+        $tag = $this->em->getRepository(Tag::class)->findOneBy(['name' => $oldName]);
         if ($tag === null) {
-            throw EntityDoesNotExistException::createFromEntityAndConditions(Tag::class, $criteria);
+            throw TagNotFoundException::fromTag($oldName);
         }
 
         $tag->rename($newName);
-
         $this->em->flush();
 
         return $tag;

module/Core/src/Service/Tag/TagServiceInterface.php

@@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core\Service\Tag;
 
 use Doctrine\Common\Collections\Collection;
 use Shlinkio\Shlink\Core\Entity\Tag;
-use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
+use Shlinkio\Shlink\Core\Exception\TagNotFoundException;
 
 interface TagServiceInterface
 {
@@ -17,23 +17,17 @@ interface TagServiceInterface
 
     /**
      * @param string[] $tagNames
-     * @return void
      */
     public function deleteTags(array $tagNames): void;
 
     /**
-     * Provided a list of tag names, creates all that do not exist yet
-     *
      * @param string[] $tagNames
      * @return Collection|Tag[]
      */
     public function createTags(array $tagNames): Collection;
 
     /**
-     * @param string $oldName
+     * @throws TagNotFoundException
-     * @param string $newName
-     * @return Tag
-     * @throws EntityDoesNotExistException
      */
-    public function renameTag($oldName, $newName): Tag;
+    public function renameTag(string $oldName, string $newName): Tag;
 }

module/Core/test/Service/Tag/TagServiceTest.php

@@ -10,7 +10,7 @@ use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Entity\Tag;
-use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
+use Shlinkio\Shlink\Core\Exception\TagNotFoundException;
 use Shlinkio\Shlink\Core\Repository\TagRepository;
 use Shlinkio\Shlink\Core\Service\Tag\TagService;
 
@@ -83,7 +83,7 @@ class TagServiceTest extends TestCase
 
         $find->shouldBeCalled();
         $getRepo->shouldBeCalled();
-        $this->expectException(EntityDoesNotExistException::class);
+        $this->expectException(TagNotFoundException::class);
 
         $this->service->renameTag('foo', 'bar');
     }

module/Rest/src/Action/Tag/UpdateTagAction.php

@@ -7,14 +7,10 @@ namespace Shlinkio\Shlink\Rest\Action\Tag;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Log\LoggerInterface;
-use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
+use Shlinkio\Shlink\Core\Exception\ValidationException;
 use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface;
 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 UpdateTagAction extends AbstractRestAction
 {
@@ -43,21 +39,13 @@ class UpdateTagAction extends AbstractRestAction
     {
         $body = $request->getParsedBody();
         if (! isset($body['oldName'], $body['newName'])) {
-            return new JsonResponse([
+            throw ValidationException::fromArray([
-                'error' => RestUtils::INVALID_ARGUMENT_ERROR,
+                'oldName' => 'oldName is required',
-                'message' =>
+                'newName' => 'newName is required',
-                    'You have to provide both \'oldName\' and \'newName\' params in order to properly rename the tag',
+            ]);
-            ], self::STATUS_BAD_REQUEST);
         }
 
-        try {
+        $this->tagService->renameTag($body['oldName'], $body['newName']);
-            $this->tagService->renameTag($body['oldName'], $body['newName']);
+        return new EmptyResponse();
-            return new EmptyResponse();
-        } catch (EntityDoesNotExistException $e) {
-            return new JsonResponse([
-                'error' => RestUtils::NOT_FOUND_ERROR,
-                'message' => sprintf('It was not possible to find a tag with name %s', $body['oldName']),
-            ], self::STATUS_NOT_FOUND);
-        }
     }
 }

module/Rest/src/Action/Visit/GetVisitsAction.php

@@ -8,15 +8,11 @@ use Psr\Http\Message\ResponseInterface as Response;
 use Psr\Http\Message\ServerRequestInterface as Request;
 use Psr\Log\LoggerInterface;
 use Shlinkio\Shlink\Common\Paginator\Util\PaginatorUtilsTrait;
-use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
 use Shlinkio\Shlink\Core\Model\VisitsParams;
 use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface;
 use Shlinkio\Shlink\Rest\Action\AbstractRestAction;
-use Shlinkio\Shlink\Rest\Util\RestUtils;
 use Zend\Diactoros\Response\JsonResponse;
 
-use function sprintf;
-
 class GetVisitsAction extends AbstractRestAction
 {
     use PaginatorUtilsTrait;
@@ -36,19 +32,10 @@ class GetVisitsAction extends AbstractRestAction
     public function handle(Request $request): Response
     {
         $shortCode = $request->getAttribute('shortCode');
+        $visits = $this->visitsTracker->info($shortCode, VisitsParams::fromRawData($request->getQueryParams()));
 
-        try {
+        return new JsonResponse([
-            $visits = $this->visitsTracker->info($shortCode, VisitsParams::fromRawData($request->getQueryParams()));
+            'visits' => $this->serializePaginator($visits),
-
+        ]);
-            return new JsonResponse([
-                'visits' => $this->serializePaginator($visits),
-            ]);
-        } catch (ShortUrlNotFoundException $e) {
-            $this->logger->warning('Provided nonexistent short code {e}', ['e' => $e]);
-            return new JsonResponse([
-                'error' => RestUtils::INVALID_ARGUMENT_ERROR, // FIXME Wrong code. Use correct one in "type"
-                'message' => sprintf('Provided short code %s does not exist', $shortCode),
-            ], self::STATUS_NOT_FOUND);
-        }
     }
 }

module/Rest/src/Exception/AuthenticationException.php

@@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Rest\Exception;
 
 use Throwable;
 
+/** @deprecated */
 class AuthenticationException extends RuntimeException
 {
     public static function expiredJWT(?Throwable $prev = null): self

module/Rest/src/Util/RestUtils.php

@@ -21,11 +21,14 @@ class RestUtils
     public const INVALID_ARGUMENT_ERROR = Core\ValidationException::TYPE;
     /** @deprecated */
     public const INVALID_SLUG_ERROR = Core\NonUniqueSlugException::TYPE;
+    /** @deprecated */
     public const INVALID_CREDENTIALS_ERROR = 'INVALID_CREDENTIALS';
     public const INVALID_AUTH_TOKEN_ERROR = 'INVALID_AUTH_TOKEN';
     public const INVALID_AUTHORIZATION_ERROR = 'INVALID_AUTHORIZATION';
     public const INVALID_API_KEY_ERROR = 'INVALID_API_KEY';
-    public const NOT_FOUND_ERROR = 'NOT_FOUND';
+    /** @deprecated */
+    public const NOT_FOUND_ERROR = Core\TagNotFoundException::TYPE;
+    /** @deprecated */
     public const UNKNOWN_ERROR = 'UNKNOWN_ERROR';
 
     /** @deprecated */

module/Rest/test-api/Action/GetVisitsActionTest.php

@@ -16,6 +16,6 @@ class GetVisitsActionTest 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);
     }
 }

module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php

@@ -11,8 +11,6 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\ValidationException;
 use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface;
 use Shlinkio\Shlink\Rest\Action\ShortUrl\EditShortUrlAction;
-use Shlinkio\Shlink\Rest\Util\RestUtils;
-use Zend\Diactoros\Response\JsonResponse;
 use Zend\Diactoros\ServerRequest;
 
 class EditShortUrlActionTest extends TestCase

module/Rest/test/Action/Tag/UpdateTagActionTest.php

@@ -7,7 +7,7 @@ namespace ShlinkioTest\Shlink\Rest\Action\Tag;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Entity\Tag;
-use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
+use Shlinkio\Shlink\Core\Exception\ValidationException;
 use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface;
 use Shlinkio\Shlink\Rest\Action\Tag\UpdateTagAction;
 use Zend\Diactoros\ServerRequest;
@@ -32,9 +32,10 @@ class UpdateTagActionTest extends TestCase
     public function whenInvalidParamsAreProvidedAnErrorIsReturned(array $bodyParams): void
     {
         $request = (new ServerRequest())->withParsedBody($bodyParams);
-        $resp = $this->action->handle($request);
 
-        $this->assertEquals(400, $resp->getStatusCode());
+        $this->expectException(ValidationException::class);
+
+        $this->action->handle($request);
     }
 
     public function provideParams(): iterable
@@ -44,21 +45,6 @@ class UpdateTagActionTest extends TestCase
         yield 'no params' => [[]];
     }
 
-    /** @test */
-    public function requestingInvalidTagReturnsError(): void
-    {
-        $request = (new ServerRequest())->withParsedBody([
-            'oldName' => 'foo',
-            'newName' => 'bar',
-        ]);
-        $rename = $this->tagService->renameTag('foo', 'bar')->willThrow(EntityDoesNotExistException::class);
-
-        $resp = $this->action->handle($request);
-
-        $this->assertEquals(404, $resp->getStatusCode());
-        $rename->shouldHaveBeenCalled();
-    }
-
     /** @test */
     public function correctInvocationRenamesTag(): void
     {

module/Rest/test/Action/Visit/GetVisitsActionTest.php

@@ -9,7 +9,6 @@ use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Common\Util\DateRange;
-use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
 use Shlinkio\Shlink\Core\Model\VisitsParams;
 use Shlinkio\Shlink\Core\Service\VisitsTracker;
 use Shlinkio\Shlink\Rest\Action\Visit\GetVisitsAction;
@@ -42,18 +41,6 @@ class GetVisitsActionTest extends TestCase
         $this->assertEquals(200, $response->getStatusCode());
     }
 
-    /** @test */
-    public function providingInvalidShortCodeReturnsError(): void
-    {
-        $shortCode = 'abc123';
-        $this->visitsTracker->info($shortCode, Argument::type(VisitsParams::class))->willThrow(
-            ShortUrlNotFoundException::class
-        )->shouldBeCalledOnce();
-
-        $response = $this->action->handle((new ServerRequest())->withAttribute('shortCode', $shortCode));
-        $this->assertEquals(404, $response->getStatusCode());
-    }
-
     /** @test */
     public function paramsAreReadFromQuery(): void
     {

module/Rest/test/Util/RestUtilsTest.php

@@ -6,8 +6,8 @@ namespace ShlinkioTest\Shlink\Rest\Util;
 
 use PHPUnit\Framework\TestCase;
 use Shlinkio\Shlink\Common\Exception\InvalidArgumentException;
-use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
 use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
+use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
 use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
 use Shlinkio\Shlink\Rest\Exception\AuthenticationException;
 use Shlinkio\Shlink\Rest\Util\RestUtils;