Refactored and fixed unit tests

module/Core/src/Exception/DeleteShortUrlException.php

@@ -16,7 +16,7 @@ class DeleteShortUrlException extends DomainException implements ProblemDetailsE
     use CommonProblemDetailsExceptionTrait;
 
     private const TITLE = 'Cannot delete short URL';
-    public const TYPE = 'INVALID_SHORTCODE_DELETION'; // FIXME Should be INVALID_SHORT_URL_DELETION
+    private const TYPE = 'INVALID_SHORTCODE_DELETION'; // FIXME Should be INVALID_SHORT_URL_DELETION
 
     /** @var int */
     private $visitsThreshold;

module/Core/src/Exception/InvalidUrlException.php

@@ -16,7 +16,7 @@ class InvalidUrlException extends DomainException implements ProblemDetailsExcep
     use CommonProblemDetailsExceptionTrait;
 
     private const TITLE = 'Invalid URL';
-    public const TYPE = 'INVALID_URL';
+    private const TYPE = 'INVALID_URL';
 
     public static function fromUrl(string $url, ?Throwable $previous = null): self
     {

module/Core/src/Exception/NonUniqueSlugException.php

@@ -15,7 +15,7 @@ class NonUniqueSlugException extends InvalidArgumentException implements Problem
     use CommonProblemDetailsExceptionTrait;
 
     private const TITLE = 'Invalid custom slug';
-    public const TYPE = 'INVALID_SLUG';
+    private const TYPE = 'INVALID_SLUG';
 
     public static function fromSlug(string $slug, ?string $domain): self
     {

module/Core/src/Exception/ShortUrlNotFoundException.php

@@ -15,7 +15,7 @@ class ShortUrlNotFoundException extends DomainException implements ProblemDetail
     use CommonProblemDetailsExceptionTrait;
 
     private const TITLE = 'Short URL not found';
-    public const TYPE = 'INVALID_SHORTCODE';
+    private const TYPE = 'INVALID_SHORTCODE';
 
     public static function fromNotFoundShortCode(string $shortCode, ?string $domain = null): self
     {

module/Core/src/Exception/TagNotFoundException.php

@@ -15,7 +15,7 @@ class TagNotFoundException extends DomainException implements ProblemDetailsExce
     use CommonProblemDetailsExceptionTrait;
 
     private const TITLE = 'Tag not found';
-    public const TYPE = 'TAG_NOT_FOUND';
+    private const TYPE = 'TAG_NOT_FOUND';
 
     public static function fromTag(string $tag): self
     {

module/Core/src/Exception/ValidationException.php

@@ -22,7 +22,7 @@ class ValidationException extends InvalidArgumentException implements ProblemDet
     use CommonProblemDetailsExceptionTrait;
 
     private const TITLE = 'Invalid data';
-    public const TYPE = 'INVALID_ARGUMENT';
+    private const TYPE = 'INVALID_ARGUMENT';
 
     /** @var array */
     private $invalidElements;

module/Rest/src/Action/AuthenticateAction.php

@@ -10,7 +10,6 @@ use Psr\Log\LoggerInterface;
 use Shlinkio\Shlink\Rest\Authentication\JWTServiceInterface;
 use Shlinkio\Shlink\Rest\Service\ApiKeyService;
 use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
-use Shlinkio\Shlink\Rest\Util\RestUtils;
 use Zend\Diactoros\Response\JsonResponse;
 
 /** @deprecated */

module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php

@@ -9,7 +9,6 @@ use Psr\Http\Message\ServerRequestInterface as Request;
 use Psr\Log\LoggerInterface;
 use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface;
 use Shlinkio\Shlink\Rest\Action\AbstractRestAction;
-use Shlinkio\Shlink\Rest\Util\RestUtils;
 use Zend\Diactoros\Response\JsonResponse;
 
 class EditShortUrlTagsAction extends AbstractRestAction

module/Rest/src/Exception/MissingAuthenticationException.php

@@ -16,7 +16,7 @@ class MissingAuthenticationException extends RuntimeException implements Problem
     use CommonProblemDetailsExceptionTrait;
 
     private const TITLE = 'Invalid authorization';
-    public const TYPE = 'INVALID_AUTHORIZATION';
+    private const TYPE = 'INVALID_AUTHORIZATION';
 
     public static function fromExpectedTypes(array $expectedTypes): self
     {

module/Rest/src/Exception/VerifyAuthenticationException.php

@@ -14,17 +14,10 @@ class VerifyAuthenticationException extends RuntimeException implements ProblemD
 {
     use CommonProblemDetailsExceptionTrait;
 
-    /** @var string */
-    private $errorCode;
-    /** @var string */
-    private $publicMessage;
-
     public static function forInvalidApiKey(): self
     {
         $e = new self('Provided API key does not exist or is invalid.');
 
-        $e->publicMessage = $e->getMessage();
-        $e->errorCode = 'INVALID_API_KEY';
         $e->detail = $e->getMessage();
         $e->title = 'Invalid API key';
         $e->type = 'INVALID_API_KEY';
@@ -41,8 +34,6 @@ class VerifyAuthenticationException extends RuntimeException implements ProblemD
             . 'token on every new request on the Authorization header'
         );
 
-        $e->publicMessage = $e->getMessage();
-        $e->errorCode = 'INVALID_AUTH_TOKEN';
         $e->detail = $e->getMessage();
         $e->title = 'Invalid auth token';
         $e->type = 'INVALID_AUTH_TOKEN';
@@ -56,8 +47,6 @@ class VerifyAuthenticationException extends RuntimeException implements ProblemD
     {
         $e = new self('You need to provide the Bearer type in the Authorization header.');
 
-        $e->publicMessage = $e->getMessage();
-        $e->errorCode = 'INVALID_AUTHORIZATION';
         $e->detail = $e->getMessage();
         $e->title = 'Invalid authorization';
         $e->type = 'INVALID_AUTHORIZATION';
@@ -71,8 +60,6 @@ class VerifyAuthenticationException extends RuntimeException implements ProblemD
     {
         $e = new self(sprintf('Provided authorization type %s is not supported. Use Bearer instead.', $providedType));
 
-        $e->publicMessage = $e->getMessage();
-        $e->errorCode = 'INVALID_AUTHORIZATION';
         $e->detail = $e->getMessage();
         $e->title = 'Invalid authorization';
         $e->type = 'INVALID_AUTHORIZATION';
@@ -80,14 +67,4 @@ class VerifyAuthenticationException extends RuntimeException implements ProblemD
 
         return $e;
     }
-
-    public function getErrorCode(): string
-    {
-        return $this->errorCode;
-    }
-
-    public function getPublicMessage(): string
-    {
-        return $this->publicMessage;
-    }
 }

module/Rest/src/Util/RestUtils.php

@@ -1,13 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Shlinkio\Shlink\Rest\Util;
-
-use Shlinkio\Shlink\Rest\Exception as Rest;
-
-/** @deprecated */
-class RestUtils
-{
-    public const INVALID_AUTHORIZATION_ERROR = Rest\MissingAuthenticationException::TYPE;
-}

module/Rest/test/Authentication/RequestToAuthPluginTest.php

@@ -37,7 +37,7 @@ class RequestToAuthPluginTest extends TestCase
 
         $this->expectException(MissingAuthenticationException::class);
         $this->expectExceptionMessage(sprintf(
-            'None of the valid authentication mechanisms where provided. Expected one of ["%s"]',
+            'Expected one of the following authentication headers, but none were provided, ["%s"]',
             implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS)
         ));
 

module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php

@@ -4,62 +4,16 @@ declare(strict_types=1);
 
 namespace ShlinkioTest\Shlink\Rest\Exception;
 
-use Exception;
 use PHPUnit\Framework\TestCase;
-use Shlinkio\Shlink\Common\Util\StringUtilsTrait;
 use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException;
-use Throwable;
-
-use function array_map;
-use function random_int;
-use function range;
 
 class VerifyAuthenticationExceptionTest extends TestCase
 {
-    use StringUtilsTrait;
-
-    /**
-     * @test
-     * @dataProvider provideConstructorData
-     */
-    public function constructCreatesExpectedException(
-        string $errorCode,
-        string $publicMessage,
-        string $message,
-        int $code,
-        ?Throwable $prev
-    ): void {
-        $e = new VerifyAuthenticationException($errorCode, $publicMessage, $message, $code, $prev);
-
-        $this->assertEquals($code, $e->getCode());
-        $this->assertEquals($message, $e->getMessage());
-        $this->assertEquals($errorCode, $e->getErrorCode());
-        $this->assertEquals($publicMessage, $e->getPublicMessage());
-        $this->assertEquals($prev, $e->getPrevious());
-    }
-
-    public function provideConstructorData(): iterable
-    {
-        return array_map(function (int $i) {
-            return [
-                $this->generateRandomString(),
-                $this->generateRandomString(30),
-                $this->generateRandomString(50),
-                $i,
-                random_int(0, 1) === 1 ? new Exception('Prev') : null,
-            ];
-        }, range(10, 20));
-    }
-
     /** @test */
-    public function defaultConstructorValuesAreKept(): void
+    public function createsExpectedExceptionForInvalidApiKey(): void
     {
-        $e = new VerifyAuthenticationException('foo', 'bar');
+        $e = VerifyAuthenticationException::forInvalidApiKey();
 
-        $this->assertEquals(0, $e->getCode());
+        $this->assertEquals('Provided API key does not exist or is invalid.', $e->getMessage());
-        $this->assertEquals('', $e->getMessage());
-        $this->assertEquals('foo', $e->getErrorCode());
-        $this->assertEquals('bar', $e->getPublicMessage());
-        $this->assertNull($e->getPrevious());
     }
 }

module/Rest/test/Middleware/AuthenticationMiddlewareTest.php

@@ -4,12 +4,10 @@ declare(strict_types=1);
 
 namespace ShlinkioTest\Shlink\Rest\Middleware;
 
-use Exception;
 use Fig\Http\Message\RequestMethodInterface;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
-use Psr\Container\ContainerExceptionInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\MiddlewareInterface;
@@ -17,20 +15,13 @@ use Psr\Http\Server\RequestHandlerInterface;
 use Psr\Log\LoggerInterface;
 use Shlinkio\Shlink\Rest\Action\AuthenticateAction;
 use Shlinkio\Shlink\Rest\Authentication\Plugin\AuthenticationPluginInterface;
-use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPlugin;
 use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPluginInterface;
-use Shlinkio\Shlink\Rest\Exception\MissingAuthenticationException;
-use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException;
 use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware;
-use Shlinkio\Shlink\Rest\Util\RestUtils;
-use Throwable;
 use Zend\Diactoros\Response;
 use Zend\Diactoros\ServerRequest;
 use Zend\Expressive\Router\Route;
 use Zend\Expressive\Router\RouteResult;
 
-use function implode;
-use function sprintf;
 use function Zend\Stratigility\middleware;
 
 class AuthenticationMiddlewareTest extends TestCase
@@ -93,72 +84,6 @@ class AuthenticationMiddlewareTest extends TestCase
         )->withMethod(RequestMethodInterface::METHOD_OPTIONS)];
     }
 
-    /**
-     * @test
-     * @dataProvider provideExceptions
-     */
-    public function errorIsReturnedWhenNoValidAuthIsProvided(Throwable $e): void
-    {
-        $request = (new ServerRequest())->withAttribute(
-            RouteResult::class,
-            RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), [])
-        );
-        $fromRequest = $this->requestToPlugin->fromRequest(Argument::any())->willThrow($e);
-        $logWarning = $this->logger->warning('Invalid or no authentication provided. {e}', ['e' => $e])->will(
-            function () {
-            }
-        );
-
-        /** @var Response\JsonResponse $response */
-        $response = $this->middleware->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal());
-        $payload = $response->getPayload();
-
-        $this->assertEquals(RestUtils::INVALID_AUTHORIZATION_ERROR, $payload['error']);
-        $this->assertEquals(sprintf(
-            'Expected one of the following authentication headers, but none were provided, ["%s"]',
-            implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS)
-        ), $payload['message']);
-        $fromRequest->shouldHaveBeenCalledOnce();
-        $logWarning->shouldHaveBeenCalledOnce();
-    }
-
-    public function provideExceptions(): iterable
-    {
-        $containerException = new class extends Exception implements ContainerExceptionInterface {
-        };
-
-        yield 'container exception' => [$containerException];
-        yield 'authentication exception' => [MissingAuthenticationException::fromExpectedTypes([])];
-    }
-
-    /** @test */
-    public function errorIsReturnedWhenVerificationFails(): void
-    {
-        $request = (new ServerRequest())->withAttribute(
-            RouteResult::class,
-            RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), [])
-        );
-        $e = VerifyAuthenticationException::forInvalidApiKey();
-        $plugin = $this->prophesize(AuthenticationPluginInterface::class);
-
-        $verify = $plugin->verify($request)->willThrow($e);
-        $fromRequest = $this->requestToPlugin->fromRequest(Argument::any())->willReturn($plugin->reveal());
-        $logWarning = $this->logger->warning('Authentication verification failed. {e}', ['e' => $e])->will(
-            function () {
-            }
-        );
-
-        /** @var Response\JsonResponse $response */
-        $response = $this->middleware->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal());
-        $payload = $response->getPayload();
-
-        $this->assertEquals('the_error', $payload['error']);
-        $this->assertEquals('the_message', $payload['message']);
-        $verify->shouldHaveBeenCalledOnce();
-        $fromRequest->shouldHaveBeenCalledOnce();
-        $logWarning->shouldHaveBeenCalledOnce();
-    }
-
     /** @test */
     public function updatedResponseIsReturnedWhenVerificationPasses(): void
     {

phpstan.neon

@@ -1,6 +1,5 @@
 parameters:
 	ignoreErrors:
-		- '#is not subtype of Throwable#'
 		- '#Undefined variable: \$metadata#'
 		- '#AbstractQuery::setParameters()#'
 		- '#mustRun()#'