From cd4fe4362bcb325e665dfbc537c1c9eb8ea42b0d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 13 Aug 2022 16:50:19 +0200 Subject: [PATCH 01/10] Created middleware to keep backwards compatibility on errors when using v1 and 2 of the API --- .../autoload/middleware-pipeline.global.php | 1 + .../src/Exception/ValidationException.php | 2 +- module/Rest/config/dependencies.config.php | 1 + ...wardsCompatibleProblemDetailsException.php | 74 ++++++++++++++++++ ...ckwardsCompatibleProblemDetailsHandler.php | 30 ++++++++ ...rdsCompatibleProblemDetailsHandlerTest.php | 76 +++++++++++++++++++ 6 files changed, 183 insertions(+), 1 deletion(-) create mode 100644 module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php create mode 100644 module/Rest/src/Middleware/ErrorHandler/BackwardsCompatibleProblemDetailsHandler.php create mode 100644 module/Rest/test/Middleware/ErrorHandler/BackwardsCompatibleProblemDetailsHandlerTest.php diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index c628c4fd..5291db8c 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -26,6 +26,7 @@ return [ 'path' => '/rest', 'middleware' => [ ProblemDetails\ProblemDetailsMiddleware::class, + Rest\Middleware\ErrorHandler\BackwardsCompatibleProblemDetailsHandler::class, ], ], diff --git a/module/Core/src/Exception/ValidationException.php b/module/Core/src/Exception/ValidationException.php index 326eec11..0f625bc5 100644 --- a/module/Core/src/Exception/ValidationException.php +++ b/module/Core/src/Exception/ValidationException.php @@ -21,7 +21,7 @@ class ValidationException extends InvalidArgumentException implements ProblemDet use CommonProblemDetailsExceptionTrait; private const TITLE = 'Invalid data'; - private const TYPE = 'INVALID_ARGUMENT'; + public const TYPE = 'https://shlink.io/api/error/invalid-data'; private array $invalidElements; diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 189180b0..a70cb7f1 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -53,6 +53,7 @@ return [ Middleware\ShortUrl\DefaultShortCodesLengthMiddleware::class => ConfigAbstractFactory::class, Middleware\ShortUrl\OverrideDomainMiddleware::class => ConfigAbstractFactory::class, Middleware\Mercure\NotConfiguredMercureErrorHandler::class => ConfigAbstractFactory::class, + Middleware\ErrorHandler\BackwardsCompatibleProblemDetailsHandler::class => InvokableFactory::class, ], ], diff --git a/module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php b/module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php new file mode 100644 index 00000000..ddc3768f --- /dev/null +++ b/module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php @@ -0,0 +1,74 @@ +getMessage(), $e->getCode(), $e); + } + + public static function fromProblemDetails(ProblemDetailsExceptionInterface $e): self + { + return new self($e); + } + + public function getStatus(): int + { + return $this->e->getStatus(); + } + + public function getType(): string + { + return $this->remapType($this->e->getType()); + } + + public function getTitle(): string + { + return $this->e->getTitle(); + } + + public function getDetail(): string + { + return $this->e->getDetail(); + } + + public function getAdditionalData(): array + { + return $this->e->getAdditionalData(); + } + + public function toArray(): array + { + return $this->remapTypeInArray($this->e->toArray()); + } + + public function jsonSerialize(): array + { + return $this->remapTypeInArray($this->e->jsonSerialize()); + } + + private function remapTypeInArray(array $wrappedArray): array + { + if (! isset($wrappedArray['type'])) { + return $wrappedArray; + } + + return [...$wrappedArray, 'type' => $this->remapType($wrappedArray['type'])]; + } + + private function remapType(string $wrappedType): string + { + return match ($wrappedType) { + ValidationException::TYPE => 'INVALID_ARGUMENT', + default => $wrappedType, + }; + } +} diff --git a/module/Rest/src/Middleware/ErrorHandler/BackwardsCompatibleProblemDetailsHandler.php b/module/Rest/src/Middleware/ErrorHandler/BackwardsCompatibleProblemDetailsHandler.php new file mode 100644 index 00000000..c099ad70 --- /dev/null +++ b/module/Rest/src/Middleware/ErrorHandler/BackwardsCompatibleProblemDetailsHandler.php @@ -0,0 +1,30 @@ +handle($request); + } catch (ProblemDetailsExceptionInterface $e) { + $version = $request->getAttribute('version') ?? '2'; + throw version_compare($version, '3', '>=') + ? $e + : BackwardsCompatibleProblemDetailsException::fromProblemDetails($e); + } + } +} diff --git a/module/Rest/test/Middleware/ErrorHandler/BackwardsCompatibleProblemDetailsHandlerTest.php b/module/Rest/test/Middleware/ErrorHandler/BackwardsCompatibleProblemDetailsHandlerTest.php new file mode 100644 index 00000000..00dddb2f --- /dev/null +++ b/module/Rest/test/Middleware/ErrorHandler/BackwardsCompatibleProblemDetailsHandlerTest.php @@ -0,0 +1,76 @@ +handler = new BackwardsCompatibleProblemDetailsHandler(); + } + + /** + * @test + * @dataProvider provideExceptions + */ + public function expectedExceptionIsThrownBasedOnTheRequestVersion( + ServerRequestInterface $request, + Throwable $thrownException, + string $expectedException, + ): void { + $handler = $this->prophesize(RequestHandlerInterface::class); + $handle = $handler->handle($request)->willThrow($thrownException); + + $this->expectException($expectedException); + $handle->shouldBeCalledOnce(); + + $this->handler->process($request, $handler->reveal()); + } + + public function provideExceptions(): iterable + { + $baseRequest = ServerRequestFactory::fromGlobals(); + + yield 'no version' => [ + $baseRequest, + ValidationException::fromArray([]), + BackwardsCompatibleProblemDetailsException::class, + ]; + yield 'version 1' => [ + $baseRequest->withAttribute('version', '1'), + ValidationException::fromArray([]), + BackwardsCompatibleProblemDetailsException::class, + ]; + yield 'version 2' => [ + $baseRequest->withAttribute('version', '2'), + ValidationException::fromArray([]), + BackwardsCompatibleProblemDetailsException::class, + ]; + yield 'version 3' => [ + $baseRequest->withAttribute('version', '3'), + ValidationException::fromArray([]), + ValidationException::class, + ]; + yield 'version 4' => [ + $baseRequest->withAttribute('version', '3'), + ValidationException::fromArray([]), + ValidationException::class, + ]; + } +} From 905f51fbd0583a587ecf6a63ad18a18b3fc35cf4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 13 Aug 2022 17:15:04 +0200 Subject: [PATCH 02/10] Added logic to properly map all existing errors from v3 to v2 in the API --- composer.json | 2 +- .../src/Exception/DeleteShortUrlException.php | 2 +- .../src/Exception/DomainNotFoundException.php | 2 +- .../ForbiddenTagOperationException.php | 2 +- .../src/Exception/InvalidUrlException.php | 2 +- .../src/Exception/NonUniqueSlugException.php | 2 +- .../Exception/ShortUrlNotFoundException.php | 2 +- .../src/Exception/TagConflictException.php | 2 +- .../src/Exception/TagNotFoundException.php | 2 +- .../Exception/DeleteShortUrlExceptionTest.php | 2 +- .../Exception/DomainNotFoundExceptionTest.php | 4 ++-- .../ForbiddenTagOperationExceptionTest.php | 2 +- .../Exception/InvalidUrlExceptionTest.php | 2 +- .../Exception/NonUniqueSlugExceptionTest.php | 2 +- .../ShortUrlNotFoundExceptionTest.php | 2 +- .../Exception/TagConflictExceptionTest.php | 2 +- .../Exception/TagNotFoundExceptionTest.php | 2 +- module/Rest/src/ConfigProvider.php | 2 +- ...wardsCompatibleProblemDetailsException.php | 19 +++++++++++++++++++ .../Rest/src/Exception/MercureException.php | 2 +- .../MissingAuthenticationException.php | 2 +- .../VerifyAuthenticationException.php | 4 +++- module/Rest/test/ConfigProviderTest.php | 14 +++++++------- .../MissingAuthenticationExceptionTest.php | 2 +- 24 files changed, 51 insertions(+), 30 deletions(-) diff --git a/composer.json b/composer.json index ad11461b..168418a5 100644 --- a/composer.json +++ b/composer.json @@ -69,7 +69,7 @@ "phpunit/phpunit": "^9.5", "roave/security-advisories": "dev-master", "shlinkio/php-coding-standard": "~2.3.0", - "shlinkio/shlink-test-utils": "^3.1.0", + "shlinkio/shlink-test-utils": "^3.2", "symfony/var-dumper": "^6.1", "veewee/composer-run-parallel": "^1.1" }, diff --git a/module/Core/src/Exception/DeleteShortUrlException.php b/module/Core/src/Exception/DeleteShortUrlException.php index 0d331400..f6638221 100644 --- a/module/Core/src/Exception/DeleteShortUrlException.php +++ b/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'; - private const TYPE = 'INVALID_SHORT_URL_DELETION'; + public const TYPE = 'https://shlink.io/api/error/invalid-short-url-deletion'; public static function fromVisitsThreshold(int $threshold, ShortUrlIdentifier $identifier): self { diff --git a/module/Core/src/Exception/DomainNotFoundException.php b/module/Core/src/Exception/DomainNotFoundException.php index cb19608a..aca67813 100644 --- a/module/Core/src/Exception/DomainNotFoundException.php +++ b/module/Core/src/Exception/DomainNotFoundException.php @@ -15,7 +15,7 @@ class DomainNotFoundException extends DomainException implements ProblemDetailsE use CommonProblemDetailsExceptionTrait; private const TITLE = 'Domain not found'; - private const TYPE = 'DOMAIN_NOT_FOUND'; + public const TYPE = 'https://shlink.io/api/error/domain-not-found'; private function __construct(string $message, array $additional) { diff --git a/module/Core/src/Exception/ForbiddenTagOperationException.php b/module/Core/src/Exception/ForbiddenTagOperationException.php index d4200c92..6da1cb61 100644 --- a/module/Core/src/Exception/ForbiddenTagOperationException.php +++ b/module/Core/src/Exception/ForbiddenTagOperationException.php @@ -13,7 +13,7 @@ class ForbiddenTagOperationException extends DomainException implements ProblemD use CommonProblemDetailsExceptionTrait; private const TITLE = 'Forbidden tag operation'; - private const TYPE = 'FORBIDDEN_OPERATION'; + public const TYPE = 'https://shlink.io/api/error/forbidden-tag-operation'; public static function forDeletion(): self { diff --git a/module/Core/src/Exception/InvalidUrlException.php b/module/Core/src/Exception/InvalidUrlException.php index ee4caaf6..6d1f93c6 100644 --- a/module/Core/src/Exception/InvalidUrlException.php +++ b/module/Core/src/Exception/InvalidUrlException.php @@ -16,7 +16,7 @@ class InvalidUrlException extends DomainException implements ProblemDetailsExcep use CommonProblemDetailsExceptionTrait; private const TITLE = 'Invalid URL'; - private const TYPE = 'INVALID_URL'; + public const TYPE = 'https://shlink.io/api/error/invalid-url'; public static function fromUrl(string $url, ?Throwable $previous = null): self { diff --git a/module/Core/src/Exception/NonUniqueSlugException.php b/module/Core/src/Exception/NonUniqueSlugException.php index f61c480f..dc1fcca9 100644 --- a/module/Core/src/Exception/NonUniqueSlugException.php +++ b/module/Core/src/Exception/NonUniqueSlugException.php @@ -16,7 +16,7 @@ class NonUniqueSlugException extends InvalidArgumentException implements Problem use CommonProblemDetailsExceptionTrait; private const TITLE = 'Invalid custom slug'; - private const TYPE = 'INVALID_SLUG'; + public const TYPE = 'https://shlink.io/api/error/non-unique-slug'; public static function fromSlug(string $slug, ?string $domain = null): self { diff --git a/module/Core/src/Exception/ShortUrlNotFoundException.php b/module/Core/src/Exception/ShortUrlNotFoundException.php index c59c20ef..4da6972e 100644 --- a/module/Core/src/Exception/ShortUrlNotFoundException.php +++ b/module/Core/src/Exception/ShortUrlNotFoundException.php @@ -16,7 +16,7 @@ class ShortUrlNotFoundException extends DomainException implements ProblemDetail use CommonProblemDetailsExceptionTrait; private const TITLE = 'Short URL not found'; - private const TYPE = 'INVALID_SHORTCODE'; + public const TYPE = 'https://shlink.io/api/error/short-url-not-found'; public static function fromNotFound(ShortUrlIdentifier $identifier): self { diff --git a/module/Core/src/Exception/TagConflictException.php b/module/Core/src/Exception/TagConflictException.php index d551ec19..09ea7be4 100644 --- a/module/Core/src/Exception/TagConflictException.php +++ b/module/Core/src/Exception/TagConflictException.php @@ -16,7 +16,7 @@ class TagConflictException extends RuntimeException implements ProblemDetailsExc use CommonProblemDetailsExceptionTrait; private const TITLE = 'Tag conflict'; - private const TYPE = 'TAG_CONFLICT'; + public const TYPE = 'https://shlink.io/api/error/tag-conflict'; public static function forExistingTag(TagRenaming $renaming): self { diff --git a/module/Core/src/Exception/TagNotFoundException.php b/module/Core/src/Exception/TagNotFoundException.php index 18c1554c..da2426aa 100644 --- a/module/Core/src/Exception/TagNotFoundException.php +++ b/module/Core/src/Exception/TagNotFoundException.php @@ -15,7 +15,7 @@ class TagNotFoundException extends DomainException implements ProblemDetailsExce use CommonProblemDetailsExceptionTrait; private const TITLE = 'Tag not found'; - private const TYPE = 'TAG_NOT_FOUND'; + public const TYPE = 'https://shlink.io/api/error/tag-not-found'; public static function fromTag(string $tag): self { diff --git a/module/Core/test/Exception/DeleteShortUrlExceptionTest.php b/module/Core/test/Exception/DeleteShortUrlExceptionTest.php index b331bdc2..e658e55d 100644 --- a/module/Core/test/Exception/DeleteShortUrlExceptionTest.php +++ b/module/Core/test/Exception/DeleteShortUrlExceptionTest.php @@ -37,7 +37,7 @@ class DeleteShortUrlExceptionTest extends TestCase 'threshold' => $threshold, ], $e->getAdditionalData()); self::assertEquals('Cannot delete short URL', $e->getTitle()); - self::assertEquals('INVALID_SHORT_URL_DELETION', $e->getType()); + self::assertEquals('https://shlink.io/api/error/invalid-short-url-deletion', $e->getType()); self::assertEquals(422, $e->getStatus()); } diff --git a/module/Core/test/Exception/DomainNotFoundExceptionTest.php b/module/Core/test/Exception/DomainNotFoundExceptionTest.php index 5f2b9889..f2f5daba 100644 --- a/module/Core/test/Exception/DomainNotFoundExceptionTest.php +++ b/module/Core/test/Exception/DomainNotFoundExceptionTest.php @@ -21,7 +21,7 @@ class DomainNotFoundExceptionTest extends TestCase self::assertEquals($expectedMessage, $e->getMessage()); self::assertEquals($expectedMessage, $e->getDetail()); self::assertEquals('Domain not found', $e->getTitle()); - self::assertEquals('DOMAIN_NOT_FOUND', $e->getType()); + self::assertEquals('https://shlink.io/api/error/domain-not-found', $e->getType()); self::assertEquals(['id' => $id], $e->getAdditionalData()); self::assertEquals(404, $e->getStatus()); } @@ -36,7 +36,7 @@ class DomainNotFoundExceptionTest extends TestCase self::assertEquals($expectedMessage, $e->getMessage()); self::assertEquals($expectedMessage, $e->getDetail()); self::assertEquals('Domain not found', $e->getTitle()); - self::assertEquals('DOMAIN_NOT_FOUND', $e->getType()); + self::assertEquals('https://shlink.io/api/error/domain-not-found', $e->getType()); self::assertEquals(['authority' => $authority], $e->getAdditionalData()); self::assertEquals(404, $e->getStatus()); } diff --git a/module/Core/test/Exception/ForbiddenTagOperationExceptionTest.php b/module/Core/test/Exception/ForbiddenTagOperationExceptionTest.php index 40ccd0ee..b064cf91 100644 --- a/module/Core/test/Exception/ForbiddenTagOperationExceptionTest.php +++ b/module/Core/test/Exception/ForbiddenTagOperationExceptionTest.php @@ -25,7 +25,7 @@ class ForbiddenTagOperationExceptionTest extends TestCase self::assertEquals($expectedMessage, $e->getMessage()); self::assertEquals($expectedMessage, $e->getDetail()); self::assertEquals('Forbidden tag operation', $e->getTitle()); - self::assertEquals('FORBIDDEN_OPERATION', $e->getType()); + self::assertEquals('https://shlink.io/api/error/forbidden-tag-operation', $e->getType()); self::assertEquals(403, $e->getStatus()); } diff --git a/module/Core/test/Exception/InvalidUrlExceptionTest.php b/module/Core/test/Exception/InvalidUrlExceptionTest.php index 5351c1b3..e9b0d75a 100644 --- a/module/Core/test/Exception/InvalidUrlExceptionTest.php +++ b/module/Core/test/Exception/InvalidUrlExceptionTest.php @@ -27,7 +27,7 @@ class InvalidUrlExceptionTest extends TestCase self::assertEquals($expectedMessage, $e->getMessage()); self::assertEquals($expectedMessage, $e->getDetail()); self::assertEquals('Invalid URL', $e->getTitle()); - self::assertEquals('INVALID_URL', $e->getType()); + self::assertEquals('https://shlink.io/api/error/invalid-url', $e->getType()); self::assertEquals(['url' => $url], $e->getAdditionalData()); self::assertEquals(StatusCodeInterface::STATUS_BAD_REQUEST, $e->getCode()); self::assertEquals(StatusCodeInterface::STATUS_BAD_REQUEST, $e->getStatus()); diff --git a/module/Core/test/Exception/NonUniqueSlugExceptionTest.php b/module/Core/test/Exception/NonUniqueSlugExceptionTest.php index 6720f0f3..77a71df3 100644 --- a/module/Core/test/Exception/NonUniqueSlugExceptionTest.php +++ b/module/Core/test/Exception/NonUniqueSlugExceptionTest.php @@ -25,7 +25,7 @@ class NonUniqueSlugExceptionTest extends TestCase self::assertEquals($expectedMessage, $e->getMessage()); self::assertEquals($expectedMessage, $e->getDetail()); self::assertEquals('Invalid custom slug', $e->getTitle()); - self::assertEquals('INVALID_SLUG', $e->getType()); + self::assertEquals('https://shlink.io/api/error/non-unique-slug', $e->getType()); self::assertEquals(400, $e->getStatus()); self::assertEquals($expectedAdditional, $e->getAdditionalData()); } diff --git a/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php b/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php index e86a63cb..2818f350 100644 --- a/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php +++ b/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php @@ -29,7 +29,7 @@ class ShortUrlNotFoundExceptionTest extends TestCase self::assertEquals($expectedMessage, $e->getMessage()); self::assertEquals($expectedMessage, $e->getDetail()); self::assertEquals('Short URL not found', $e->getTitle()); - self::assertEquals('INVALID_SHORTCODE', $e->getType()); + self::assertEquals('https://shlink.io/api/error/short-url-not-found', $e->getType()); self::assertEquals(404, $e->getStatus()); self::assertEquals($expectedAdditional, $e->getAdditionalData()); } diff --git a/module/Core/test/Exception/TagConflictExceptionTest.php b/module/Core/test/Exception/TagConflictExceptionTest.php index 4427eb40..ba7dfa1d 100644 --- a/module/Core/test/Exception/TagConflictExceptionTest.php +++ b/module/Core/test/Exception/TagConflictExceptionTest.php @@ -23,7 +23,7 @@ class TagConflictExceptionTest extends TestCase self::assertEquals($expectedMessage, $e->getMessage()); self::assertEquals($expectedMessage, $e->getDetail()); self::assertEquals('Tag conflict', $e->getTitle()); - self::assertEquals('TAG_CONFLICT', $e->getType()); + self::assertEquals('https://shlink.io/api/error/tag-conflict', $e->getType()); self::assertEquals(['oldName' => $oldName, 'newName' => $newName], $e->getAdditionalData()); self::assertEquals(409, $e->getStatus()); } diff --git a/module/Core/test/Exception/TagNotFoundExceptionTest.php b/module/Core/test/Exception/TagNotFoundExceptionTest.php index ccd63788..f22463c2 100644 --- a/module/Core/test/Exception/TagNotFoundExceptionTest.php +++ b/module/Core/test/Exception/TagNotFoundExceptionTest.php @@ -21,7 +21,7 @@ class TagNotFoundExceptionTest extends TestCase self::assertEquals($expectedMessage, $e->getMessage()); self::assertEquals($expectedMessage, $e->getDetail()); self::assertEquals('Tag not found', $e->getTitle()); - self::assertEquals('TAG_NOT_FOUND', $e->getType()); + self::assertEquals('https://shlink.io/api/error/tag-not-found', $e->getType()); self::assertEquals(['tag' => $tag], $e->getAdditionalData()); self::assertEquals(404, $e->getStatus()); } diff --git a/module/Rest/src/ConfigProvider.php b/module/Rest/src/ConfigProvider.php index 6d389038..215a4d6e 100644 --- a/module/Rest/src/ConfigProvider.php +++ b/module/Rest/src/ConfigProvider.php @@ -11,7 +11,7 @@ use function sprintf; class ConfigProvider { - private const ROUTES_PREFIX = '/rest/v{version:1|2}'; + private const ROUTES_PREFIX = '/rest/v{version:1|2|3}'; private const UNVERSIONED_ROUTES_PREFIX = '/rest'; public const UNVERSIONED_HEALTH_ENDPOINT_NAME = 'unversioned_health'; diff --git a/module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php b/module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php index ddc3768f..14a6e934 100644 --- a/module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php +++ b/module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php @@ -5,6 +5,14 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Exception; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; +use Shlinkio\Shlink\Core\Exception\DeleteShortUrlException; +use Shlinkio\Shlink\Core\Exception\DomainNotFoundException; +use Shlinkio\Shlink\Core\Exception\ForbiddenTagOperationException; +use Shlinkio\Shlink\Core\Exception\InvalidUrlException; +use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Exception\TagConflictException; +use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Exception\ValidationException; /** @deprecated */ @@ -68,6 +76,17 @@ class BackwardsCompatibleProblemDetailsException extends RuntimeException implem { return match ($wrappedType) { ValidationException::TYPE => 'INVALID_ARGUMENT', + DeleteShortUrlException::TYPE => 'INVALID_SHORT_URL_DELETION', + DomainNotFoundException::TYPE => 'DOMAIN_NOT_FOUND', + ForbiddenTagOperationException::TYPE => 'FORBIDDEN_OPERATION', + InvalidUrlException::TYPE => 'INVALID_URL', + NonUniqueSlugException::TYPE => 'INVALID_SLUG', + ShortUrlNotFoundException::TYPE => 'INVALID_SHORTCODE', + TagConflictException::TYPE => 'TAG_CONFLICT', + TagNotFoundException::TYPE => 'TAG_NOT_FOUND', + MercureException::TYPE => 'MERCURE_NOT_CONFIGURED', + MissingAuthenticationException::TYPE => 'INVALID_AUTHORIZATION', + VerifyAuthenticationException::TYPE => 'INVALID_API_KEY', default => $wrappedType, }; } diff --git a/module/Rest/src/Exception/MercureException.php b/module/Rest/src/Exception/MercureException.php index 9435cb54..0e9a6edb 100644 --- a/module/Rest/src/Exception/MercureException.php +++ b/module/Rest/src/Exception/MercureException.php @@ -13,7 +13,7 @@ class MercureException extends RuntimeException implements ProblemDetailsExcepti use CommonProblemDetailsExceptionTrait; private const TITLE = 'Mercure integration not configured'; - private const TYPE = 'MERCURE_NOT_CONFIGURED'; + public const TYPE = 'https://shlink.io/api/error/mercure-not-configured'; public static function mercureNotConfigured(): self { diff --git a/module/Rest/src/Exception/MissingAuthenticationException.php b/module/Rest/src/Exception/MissingAuthenticationException.php index 99dbc0df..3c1f5b9f 100644 --- a/module/Rest/src/Exception/MissingAuthenticationException.php +++ b/module/Rest/src/Exception/MissingAuthenticationException.php @@ -16,7 +16,7 @@ class MissingAuthenticationException extends RuntimeException implements Problem use CommonProblemDetailsExceptionTrait; private const TITLE = 'Invalid authorization'; - private const TYPE = 'INVALID_AUTHORIZATION'; + public const TYPE = 'https://shlink.io/api/error/missing-authentication'; public static function forHeaders(array $expectedHeaders): self { diff --git a/module/Rest/src/Exception/VerifyAuthenticationException.php b/module/Rest/src/Exception/VerifyAuthenticationException.php index 702230ff..25541d03 100644 --- a/module/Rest/src/Exception/VerifyAuthenticationException.php +++ b/module/Rest/src/Exception/VerifyAuthenticationException.php @@ -12,13 +12,15 @@ class VerifyAuthenticationException extends RuntimeException implements ProblemD { use CommonProblemDetailsExceptionTrait; + public const TYPE = 'https://shlink.io/api/error/invalid-api-key'; + public static function forInvalidApiKey(): self { $e = new self('Provided API key does not exist or is invalid.'); $e->detail = $e->getMessage(); $e->title = 'Invalid API key'; - $e->type = 'INVALID_API_KEY'; + $e->type = self::TYPE; $e->status = StatusCodeInterface::STATUS_UNAUTHORIZED; return $e; diff --git a/module/Rest/test/ConfigProviderTest.php b/module/Rest/test/ConfigProviderTest.php index a3f7d0c9..d3288151 100644 --- a/module/Rest/test/ConfigProviderTest.php +++ b/module/Rest/test/ConfigProviderTest.php @@ -48,10 +48,10 @@ class ConfigProviderTest extends TestCase ['path' => '/health'], ], [ - ['path' => '/rest/v{version:1|2}/foo'], - ['path' => '/rest/v{version:1|2}/bar'], - ['path' => '/rest/v{version:1|2}/baz/foo'], - ['path' => '/rest/v{version:1|2}/health'], + ['path' => '/rest/v{version:1|2|3}/foo'], + ['path' => '/rest/v{version:1|2|3}/bar'], + ['path' => '/rest/v{version:1|2|3}/baz/foo'], + ['path' => '/rest/v{version:1|2|3}/health'], ['path' => '/rest/health', 'name' => ConfigProvider::UNVERSIONED_HEALTH_ENDPOINT_NAME], ], ]; @@ -62,9 +62,9 @@ class ConfigProviderTest extends TestCase ['path' => '/baz/foo'], ], [ - ['path' => '/rest/v{version:1|2}/foo'], - ['path' => '/rest/v{version:1|2}/bar'], - ['path' => '/rest/v{version:1|2}/baz/foo'], + ['path' => '/rest/v{version:1|2|3}/foo'], + ['path' => '/rest/v{version:1|2|3}/bar'], + ['path' => '/rest/v{version:1|2|3}/baz/foo'], ], ]; } diff --git a/module/Rest/test/Exception/MissingAuthenticationExceptionTest.php b/module/Rest/test/Exception/MissingAuthenticationExceptionTest.php index 5d80ca17..ab79ba2f 100644 --- a/module/Rest/test/Exception/MissingAuthenticationExceptionTest.php +++ b/module/Rest/test/Exception/MissingAuthenticationExceptionTest.php @@ -65,7 +65,7 @@ class MissingAuthenticationExceptionTest extends TestCase private function assertCommonExceptionShape(MissingAuthenticationException $e): void { self::assertEquals('Invalid authorization', $e->getTitle()); - self::assertEquals('INVALID_AUTHORIZATION', $e->getType()); + self::assertEquals('https://shlink.io/api/error/missing-authentication', $e->getType()); self::assertEquals(401, $e->getStatus()); } } From 40bbcb325036887eae08a45e0d1682ef174a7c13 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 13 Aug 2022 17:48:55 +0200 Subject: [PATCH 03/10] Added some API tests for v3 API errors --- config/autoload/error-handler.global.php | 4 +- .../autoload/middleware-pipeline.global.php | 2 +- docs/swagger/parameters/version.json | 1 + docs/swagger/swagger.json | 2 +- .../test-api/Action/CreateShortUrlTest.php | 60 +++++++++++++++---- .../test-api/Action/DeleteShortUrlTest.php | 24 ++++++++ .../Rest/test-api/Action/DeleteTagsTest.php | 13 ++-- .../Rest/test-api/Action/DomainVisitsTest.php | 19 ++++++ 8 files changed, 105 insertions(+), 20 deletions(-) diff --git a/config/autoload/error-handler.global.php b/config/autoload/error-handler.global.php index b4872bfe..2cf28a47 100644 --- a/config/autoload/error-handler.global.php +++ b/config/autoload/error-handler.global.php @@ -10,8 +10,8 @@ return [ 'problem-details' => [ 'default_types_map' => [ - 404 => 'NOT_FOUND', - 500 => 'INTERNAL_SERVER_ERROR', + 404 => 'NOT_FOUND', // TODO Define new values, with backwards compatibility if possible + 500 => 'INTERNAL_SERVER_ERROR', // TODO Define new values, with backwards compatibility if possible ], ], diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index 5291db8c..25db6b7b 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -26,7 +26,6 @@ return [ 'path' => '/rest', 'middleware' => [ ProblemDetails\ProblemDetailsMiddleware::class, - Rest\Middleware\ErrorHandler\BackwardsCompatibleProblemDetailsHandler::class, ], ], @@ -46,6 +45,7 @@ return [ 'rest' => [ 'path' => '/rest', 'middleware' => [ + Rest\Middleware\ErrorHandler\BackwardsCompatibleProblemDetailsHandler::class, Router\Middleware\ImplicitOptionsMiddleware::class, Rest\Middleware\BodyParserMiddleware::class, Rest\Middleware\AuthenticationMiddleware::class, diff --git a/docs/swagger/parameters/version.json b/docs/swagger/parameters/version.json index c2b1cc1a..abb7e0f7 100644 --- a/docs/swagger/parameters/version.json +++ b/docs/swagger/parameters/version.json @@ -6,6 +6,7 @@ "schema": { "type": "string", "enum": [ + "3", "2", "1" ] diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json index 840ac84e..b80ae3b2 100644 --- a/docs/swagger/swagger.json +++ b/docs/swagger/swagger.json @@ -3,7 +3,7 @@ "info": { "title": "Shlink", "description": "Shlink, the self-hosted URL shortener", - "version": "2.0" + "version": "3.0" }, "externalDocs": { diff --git a/module/Rest/test-api/Action/CreateShortUrlTest.php b/module/Rest/test-api/Action/CreateShortUrlTest.php index 2fe529a3..26d271f0 100644 --- a/module/Rest/test-api/Action/CreateShortUrlTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlTest.php @@ -60,6 +60,25 @@ class CreateShortUrlTest extends ApiTestCase } } + /** + * @test + * @dataProvider provideDuplicatedSlugApiVersions + */ + public function expectedTypeIsReturnedForConflictingSlugBasedOnApiVersion( + string $version, + string $expectedType, + ): void { + [, $payload] = $this->createShortUrl(['customSlug' => 'custom'], version: $version); + self::assertEquals($expectedType, $payload['type']); + } + + public function provideDuplicatedSlugApiVersions(): iterable + { + yield ['1', 'INVALID_SLUG']; + yield ['2', 'INVALID_SLUG']; + yield ['3', 'https://shlink.io/api/error/non-unique-slug']; + } + /** * @test * @dataProvider provideTags @@ -226,15 +245,15 @@ class CreateShortUrlTest extends ApiTestCase * @test * @dataProvider provideInvalidUrls */ - public function failsToCreateShortUrlWithInvalidLongUrl(string $url): void + public function failsToCreateShortUrlWithInvalidLongUrl(string $url, string $version, string $expectedType): void { $expectedDetail = sprintf('Provided URL %s is invalid. Try with a different one.', $url); - [$statusCode, $payload] = $this->createShortUrl(['longUrl' => $url, 'validateUrl' => true]); + [$statusCode, $payload] = $this->createShortUrl(['longUrl' => $url, 'validateUrl' => true], version: $version); self::assertEquals(self::STATUS_BAD_REQUEST, $statusCode); self::assertEquals(self::STATUS_BAD_REQUEST, $payload['status']); - self::assertEquals('INVALID_URL', $payload['type']); + self::assertEquals($expectedType, $payload['type']); self::assertEquals($expectedDetail, $payload['detail']); self::assertEquals('Invalid URL', $payload['title']); self::assertEquals($url, $payload['url']); @@ -242,23 +261,37 @@ class CreateShortUrlTest extends ApiTestCase public function provideInvalidUrls(): iterable { - yield 'empty URL' => ['']; - yield 'non-reachable URL' => ['https://this-has-to-be-invalid.com']; + yield 'empty URL' => ['', '2', 'INVALID_URL']; + yield 'non-reachable URL' => ['https://this-has-to-be-invalid.com', '2', 'INVALID_URL']; + yield 'API version 3' => ['', '3', 'https://shlink.io/api/error/invalid-url']; } - /** @test */ - public function failsToCreateShortUrlWithoutLongUrl(): void + /** + * @test + * @dataProvider provideInvalidArgumentApiVersions + */ + public function failsToCreateShortUrlWithoutLongUrl(string $version, string $expectedType): void { - $resp = $this->callApiWithKey(self::METHOD_POST, '/short-urls', [RequestOptions::JSON => []]); + $resp = $this->callApiWithKey( + self::METHOD_POST, + sprintf('/rest/v%s/short-urls', $version), + [RequestOptions::JSON => []], + ); $payload = $this->getJsonResponsePayload($resp); self::assertEquals(self::STATUS_BAD_REQUEST, $resp->getStatusCode()); self::assertEquals(self::STATUS_BAD_REQUEST, $payload['status']); - self::assertEquals('INVALID_ARGUMENT', $payload['type']); + self::assertEquals($expectedType, $payload['type']); self::assertEquals('Provided data is not valid', $payload['detail']); self::assertEquals('Invalid data', $payload['title']); } + public function provideInvalidArgumentApiVersions(): iterable + { + yield ['2', 'INVALID_ARGUMENT']; + yield ['3', 'https://shlink.io/api/error/invalid-data']; + } + /** @test */ public function defaultDomainIsDroppedIfProvided(): void { @@ -332,12 +365,17 @@ class CreateShortUrlTest extends ApiTestCase /** * @return array{int $statusCode, array $payload} */ - private function createShortUrl(array $body = [], string $apiKey = 'valid_api_key'): array + private function createShortUrl(array $body = [], string $apiKey = 'valid_api_key', string $version = '2'): array { if (! isset($body['longUrl'])) { $body['longUrl'] = 'https://app.shlink.io'; } - $resp = $this->callApiWithKey(self::METHOD_POST, '/short-urls', [RequestOptions::JSON => $body], $apiKey); + $resp = $this->callApiWithKey( + self::METHOD_POST, + sprintf('/rest/v%s/short-urls', $version), + [RequestOptions::JSON => $body], + $apiKey, + ); $payload = $this->getJsonResponsePayload($resp); return [$resp->getStatusCode(), $payload]; diff --git a/module/Rest/test-api/Action/DeleteShortUrlTest.php b/module/Rest/test-api/Action/DeleteShortUrlTest.php index 5cac3dbd..f8ba6ef1 100644 --- a/module/Rest/test-api/Action/DeleteShortUrlTest.php +++ b/module/Rest/test-api/Action/DeleteShortUrlTest.php @@ -7,6 +7,8 @@ namespace ShlinkioApiTest\Shlink\Rest\Action; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; use ShlinkioApiTest\Shlink\Rest\Utils\NotFoundUrlHelpersTrait; +use function sprintf; + class DeleteShortUrlTest extends ApiTestCase { use NotFoundUrlHelpersTrait; @@ -33,6 +35,28 @@ class DeleteShortUrlTest extends ApiTestCase self::assertEquals($domain, $payload['domain'] ?? null); } + /** + * @test + * @dataProvider provideApiVersions + */ + public function expectedTypeIsReturnedBasedOnApiVersion(string $version, string $expectedType): void + { + $resp = $this->callApiWithKey( + self::METHOD_DELETE, + sprintf('/rest/v%s/short-urls/invalid-short-code', $version), + ); + $payload = $this->getJsonResponsePayload($resp); + + self::assertEquals($expectedType, $payload['type']); + } + + public function provideApiVersions(): iterable + { + yield ['1', 'INVALID_SHORTCODE']; + yield ['2', 'INVALID_SHORTCODE']; + yield ['3', 'https://shlink.io/api/error/short-url-not-found']; + } + /** @test */ public function properShortUrlIsDeletedWhenDomainIsProvided(): void { diff --git a/module/Rest/test-api/Action/DeleteTagsTest.php b/module/Rest/test-api/Action/DeleteTagsTest.php index ca175b69..c81d7906 100644 --- a/module/Rest/test-api/Action/DeleteTagsTest.php +++ b/module/Rest/test-api/Action/DeleteTagsTest.php @@ -7,29 +7,32 @@ namespace ShlinkioApiTest\Shlink\Rest\Action; use GuzzleHttp\RequestOptions; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; +use function sprintf; + class DeleteTagsTest extends ApiTestCase { /** * @test * @dataProvider provideNonAdminApiKeys */ - public function anErrorIsReturnedWithNonAdminApiKeys(string $apiKey): void + public function anErrorIsReturnedWithNonAdminApiKeys(string $apiKey, string $version, string $expectedType): void { - $resp = $this->callApiWithKey(self::METHOD_DELETE, '/tags', [ + $resp = $this->callApiWithKey(self::METHOD_DELETE, sprintf('/rest/v%s/tags', $version), [ RequestOptions::QUERY => ['tags' => ['foo']], ], $apiKey); $payload = $this->getJsonResponsePayload($resp); self::assertEquals(self::STATUS_FORBIDDEN, $resp->getStatusCode()); self::assertEquals(self::STATUS_FORBIDDEN, $payload['status']); - self::assertEquals('FORBIDDEN_OPERATION', $payload['type']); + self::assertEquals($expectedType, $payload['type']); self::assertEquals('You are not allowed to delete tags', $payload['detail']); self::assertEquals('Forbidden tag operation', $payload['title']); } public function provideNonAdminApiKeys(): iterable { - yield 'author' => ['author_api_key']; - yield 'domain' => ['domain_api_key']; + yield 'author' => ['author_api_key', '2', 'FORBIDDEN_OPERATION']; + yield 'domain' => ['domain_api_key', '2', 'FORBIDDEN_OPERATION']; + yield 'version 3' => ['domain_api_key', '3', 'https://shlink.io/api/error/forbidden-tag-operation']; } } diff --git a/module/Rest/test-api/Action/DomainVisitsTest.php b/module/Rest/test-api/Action/DomainVisitsTest.php index b6e29a12..c6c31ebb 100644 --- a/module/Rest/test-api/Action/DomainVisitsTest.php +++ b/module/Rest/test-api/Action/DomainVisitsTest.php @@ -65,4 +65,23 @@ class DomainVisitsTest extends ApiTestCase yield 'domain API key with not-owned valid domain' => ['domain_api_key', 'this_domain_is_detached.com']; yield 'author API key with valid domain not used in URLs' => ['author_api_key', 'this_domain_is_detached.com']; } + + /** + * @test + * @dataProvider provideApiVersions + */ + public function expectedNotFoundTypeIsReturnedForApiVersion(string $version, string $expectedType): void + { + $resp = $this->callApiWithKey(self::METHOD_GET, sprintf('/rest/v%s/domains/invalid.com/visits', $version)); + $payload = $this->getJsonResponsePayload($resp); + + self::assertEquals($expectedType, $payload['type']); + } + + public function provideApiVersions(): iterable + { + yield ['1', 'DOMAIN_NOT_FOUND']; + yield ['2', 'DOMAIN_NOT_FOUND']; + yield ['3', 'https://shlink.io/api/error/domain-not-found']; + } } From ce4bf62d75199203c4a98079732d98ea382f77bb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 Aug 2022 10:34:27 +0200 Subject: [PATCH 04/10] Added more granular resolution of arguments for infection based on branch --- .github/workflows/ci-mutation-tests.yml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci-mutation-tests.yml b/.github/workflows/ci-mutation-tests.yml index a4ce7432..f152ed15 100644 --- a/.github/workflows/ci-mutation-tests.yml +++ b/.github/workflows/ci-mutation-tests.yml @@ -24,9 +24,19 @@ jobs: - uses: actions/download-artifact@v3 with: path: build + - name: Resolve infection args + id: infection_args + run: | + BRANCH="${GITHUB_REF#refs/heads/}" | + if [[ $BRANCH == 'main' || $BRANCH == 'develop' ]]; then + echo "::set-output name=args::--logger-github=false" + else + echo "::set-output name=args::--logger-github=false --git-diff-lines --git-diff-base=develop" + fi; + shell: bash - if: ${{ inputs.test-group == 'unit' }} - run: composer infect:ci:unit -- --git-diff-lines --logger-github=false + run: composer infect:ci:unit -- ${{ steps.infection_args.outputs.args }} env: INFECTION_BADGE_API_KEY: ${{ secrets.INFECTION_BADGE_API_KEY }} - if: ${{ inputs.test-group != 'unit' }} - run: composer infect:ci:${{ inputs.test-group }} -- --git-diff-lines --logger-github=false + run: composer infect:ci:${{ inputs.test-group }} -- ${{ steps.infection_args.outputs.args }} From 4a122e0209d95deaf6f3f90fbb415dd40c6473cd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 Aug 2022 10:51:12 +0200 Subject: [PATCH 05/10] Added remaining API tests covering error type convertions --- module/Rest/test-api/Action/UpdateTagTest.php | 38 +++++++++++++++---- .../Middleware/AuthenticationTest.php | 38 +++++++++++++------ 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/module/Rest/test-api/Action/UpdateTagTest.php b/module/Rest/test-api/Action/UpdateTagTest.php index 262789d7..414e7670 100644 --- a/module/Rest/test-api/Action/UpdateTagTest.php +++ b/module/Rest/test-api/Action/UpdateTagTest.php @@ -7,6 +7,8 @@ namespace ShlinkioApiTest\Shlink\Rest\Action; use GuzzleHttp\RequestOptions; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; +use function sprintf; + class UpdateTagTest extends ApiTestCase { /** @@ -34,12 +36,15 @@ class UpdateTagTest extends ApiTestCase yield [['newName' => 'foo']]; } - /** @test */ - public function tryingToRenameInvalidTagReturnsNotFound(): void + /** + * @test + * @dataProvider provideTagNotFoundApiVersions + */ + public function tryingToRenameInvalidTagReturnsNotFound(string $version, string $expectedType): void { $expectedDetail = 'Tag with name "invalid_tag" could not be found'; - $resp = $this->callApiWithKey(self::METHOD_PUT, '/tags', [RequestOptions::JSON => [ + $resp = $this->callApiWithKey(self::METHOD_PUT, sprintf('/rest/v%s/tags', $version), [RequestOptions::JSON => [ 'oldName' => 'invalid_tag', 'newName' => 'foo', ]]); @@ -47,17 +52,27 @@ class UpdateTagTest extends ApiTestCase self::assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); self::assertEquals(self::STATUS_NOT_FOUND, $payload['status']); - self::assertEquals('TAG_NOT_FOUND', $payload['type']); + self::assertEquals($expectedType, $payload['type']); self::assertEquals($expectedDetail, $payload['detail']); self::assertEquals('Tag not found', $payload['title']); } - /** @test */ - public function errorIsThrownWhenTryingToRenameTagToAnotherTagName(): void + public function provideTagNotFoundApiVersions(): iterable + { + yield 'version 1' => ['1', 'TAG_NOT_FOUND']; + yield 'version 2' => ['2', 'TAG_NOT_FOUND']; + yield 'version 3' => ['3', 'https://shlink.io/api/error/tag-not-found']; + } + + /** + * @test + * @dataProvider provideTagConflictsApiVersions + */ + public function errorIsThrownWhenTryingToRenameTagToAnotherTagName(string $version, string $expectedType): void { $expectedDetail = 'You cannot rename tag foo to bar, because it already exists'; - $resp = $this->callApiWithKey(self::METHOD_PUT, '/tags', [RequestOptions::JSON => [ + $resp = $this->callApiWithKey(self::METHOD_PUT, sprintf('/rest/v%s/tags', $version), [RequestOptions::JSON => [ 'oldName' => 'foo', 'newName' => 'bar', ]]); @@ -65,11 +80,18 @@ class UpdateTagTest extends ApiTestCase self::assertEquals(self::STATUS_CONFLICT, $resp->getStatusCode()); self::assertEquals(self::STATUS_CONFLICT, $payload['status']); - self::assertEquals('TAG_CONFLICT', $payload['type']); + self::assertEquals($expectedType, $payload['type']); self::assertEquals($expectedDetail, $payload['detail']); self::assertEquals('Tag conflict', $payload['title']); } + public function provideTagConflictsApiVersions(): iterable + { + yield 'version 1' => ['1', 'TAG_CONFLICT']; + yield 'version 2' => ['2', 'TAG_CONFLICT']; + yield 'version 3' => ['3', 'https://shlink.io/api/error/tag-conflict']; + } + /** @test */ public function tagIsProperlyRenamedWhenRenamingToItself(): void { diff --git a/module/Rest/test-api/Middleware/AuthenticationTest.php b/module/Rest/test-api/Middleware/AuthenticationTest.php index 61dbd2c5..51128079 100644 --- a/module/Rest/test-api/Middleware/AuthenticationTest.php +++ b/module/Rest/test-api/Middleware/AuthenticationTest.php @@ -6,32 +6,47 @@ namespace ShlinkioApiTest\Shlink\Rest\Middleware; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; +use function sprintf; + class AuthenticationTest extends ApiTestCase { - /** @test */ - public function authorizationErrorIsReturnedIfNoApiKeyIsSent(): void + /** + * @test + * @dataProvider provideApiVersions + */ + public function authorizationErrorIsReturnedIfNoApiKeyIsSent(string $version, string $expectedType): void { $expectedDetail = 'Expected one of the following authentication headers, ["X-Api-Key"], but none were provided'; - $resp = $this->callApi(self::METHOD_GET, '/short-urls'); + $resp = $this->callApi(self::METHOD_GET, sprintf('/rest/v%s/short-urls', $version)); $payload = $this->getJsonResponsePayload($resp); self::assertEquals(self::STATUS_UNAUTHORIZED, $resp->getStatusCode()); self::assertEquals(self::STATUS_UNAUTHORIZED, $payload['status']); - self::assertEquals('INVALID_AUTHORIZATION', $payload['type']); + self::assertEquals($expectedType, $payload['type']); self::assertEquals($expectedDetail, $payload['detail']); self::assertEquals('Invalid authorization', $payload['title']); } + public function provideApiVersions(): iterable + { + yield 'version 1' => ['1', 'INVALID_AUTHORIZATION']; + yield 'version 2' => ['2', 'INVALID_AUTHORIZATION']; + yield 'version 3' => ['3', 'https://shlink.io/api/error/missing-authentication']; + } + /** * @test * @dataProvider provideInvalidApiKeys */ - public function apiKeyErrorIsReturnedWhenProvidedApiKeyIsInvalid(string $apiKey): void - { + public function apiKeyErrorIsReturnedWhenProvidedApiKeyIsInvalid( + string $apiKey, + string $version, + string $expectedType, + ): void { $expectedDetail = 'Provided API key does not exist or is invalid.'; - $resp = $this->callApi(self::METHOD_GET, '/short-urls', [ + $resp = $this->callApi(self::METHOD_GET, sprintf('/rest/v%s/short-urls', $version), [ 'headers' => [ 'X-Api-Key' => $apiKey, ], @@ -40,15 +55,16 @@ class AuthenticationTest extends ApiTestCase self::assertEquals(self::STATUS_UNAUTHORIZED, $resp->getStatusCode()); self::assertEquals(self::STATUS_UNAUTHORIZED, $payload['status']); - self::assertEquals('INVALID_API_KEY', $payload['type']); + self::assertEquals($expectedType, $payload['type']); self::assertEquals($expectedDetail, $payload['detail']); self::assertEquals('Invalid API key', $payload['title']); } public function provideInvalidApiKeys(): iterable { - yield 'key which does not exist' => ['invalid']; - yield 'key which is expired' => ['expired_api_key']; - yield 'key which is disabled' => ['disabled_api_key']; + yield 'key which does not exist' => ['invalid', '2', 'INVALID_API_KEY']; + yield 'key which is expired' => ['expired_api_key', '2', 'INVALID_API_KEY']; + yield 'key which is disabled' => ['disabled_api_key', '2', 'INVALID_API_KEY']; + yield 'version 3' => ['disabled_api_key', '3', 'https://shlink.io/api/error/invalid-api-key']; } } From 2650cb89b5eb03a1dfa855fff9aafd593e127706 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 Aug 2022 12:39:05 +0200 Subject: [PATCH 06/10] Created BackwardsCompatibleProblemDetailsExceptionTest --- ...sCompatibleProblemDetailsExceptionTest.php | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 module/Rest/test/Exception/BackwardsCompatibleProblemDetailsExceptionTest.php diff --git a/module/Rest/test/Exception/BackwardsCompatibleProblemDetailsExceptionTest.php b/module/Rest/test/Exception/BackwardsCompatibleProblemDetailsExceptionTest.php new file mode 100644 index 00000000..13df168b --- /dev/null +++ b/module/Rest/test/Exception/BackwardsCompatibleProblemDetailsExceptionTest.php @@ -0,0 +1,114 @@ +type; + } + + public function getTitle(): string + { + return 'title'; + } + + public function getDetail(): string + { + return 'detail'; + } + + public function getAdditionalData(): array + { + return []; + } + + public function toArray(): array + { + return ['type' => $this->type]; + } + + public function jsonSerialize(): array + { + return ['type' => $this->type]; + } + }; + $e = BackwardsCompatibleProblemDetailsException::fromProblemDetails($original); + + self::assertEquals($e->getType(), $expectedType); + self::assertEquals($e->toArray(), ['type' => $expectedType]); + self::assertEquals($e->jsonSerialize(), ['type' => $expectedType]); + + self::assertEquals($original->getTitle(), $e->getTitle()); + self::assertEquals($original->getDetail(), $e->getDetail()); + self::assertEquals($original->getAdditionalData(), $e->getAdditionalData()); + + if ($expectSameType) { + self::assertEquals($original->getType(), $e->getType()); + self::assertEquals($original->toArray(), $e->toArray()); + self::assertEquals($original->jsonSerialize(), $e->jsonSerialize()); + } else { + self::assertNotEquals($original->getType(), $e->getType()); + self::assertNotEquals($original->toArray(), $e->toArray()); + self::assertNotEquals($original->jsonSerialize(), $e->jsonSerialize()); + } + } + + public function provideTypes(): iterable + { + yield ['foo', 'foo', true]; + yield ['bar', 'bar', true]; + yield [ValidationException::TYPE, 'INVALID_ARGUMENT']; + yield [DeleteShortUrlException::TYPE, 'INVALID_SHORT_URL_DELETION']; + yield [DomainNotFoundException::TYPE, 'DOMAIN_NOT_FOUND']; + yield [ForbiddenTagOperationException::TYPE, 'FORBIDDEN_OPERATION']; + yield [InvalidUrlException::TYPE, 'INVALID_URL']; + yield [NonUniqueSlugException::TYPE, 'INVALID_SLUG']; + yield [ShortUrlNotFoundException::TYPE, 'INVALID_SHORTCODE']; + yield [TagConflictException::TYPE, 'TAG_CONFLICT']; + yield [TagNotFoundException::TYPE, 'TAG_NOT_FOUND']; + yield [MercureException::TYPE, 'MERCURE_NOT_CONFIGURED']; + yield [MissingAuthenticationException::TYPE, 'INVALID_AUTHORIZATION']; + yield [VerifyAuthenticationException::TYPE, 'INVALID_API_KEY']; + } +} From a41835573b959357724c9d43558f5ee47ca82e1c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 Aug 2022 13:12:10 +0200 Subject: [PATCH 07/10] Centralized prefix for problem detail types --- config/autoload/error-handler.global.php | 6 ++-- module/Core/functions/functions.php | 5 ++++ .../src/Exception/DeleteShortUrlException.php | 5 ++-- .../src/Exception/DomainNotFoundException.php | 5 ++-- .../ForbiddenTagOperationException.php | 6 ++-- .../src/Exception/InvalidUrlException.php | 5 ++-- .../src/Exception/NonUniqueSlugException.php | 5 ++-- .../Exception/ShortUrlNotFoundException.php | 5 ++-- .../src/Exception/TagConflictException.php | 5 ++-- .../src/Exception/TagNotFoundException.php | 5 ++-- .../src/Exception/ValidationException.php | 5 ++-- ...wardsCompatibleProblemDetailsException.php | 30 +++++++++++-------- .../Rest/src/Exception/MercureException.php | 6 ++-- .../MissingAuthenticationException.php | 5 ++-- .../VerifyAuthenticationException.php | 6 ++-- ...sCompatibleProblemDetailsExceptionTest.php | 24 +++++++-------- 16 files changed, 77 insertions(+), 51 deletions(-) diff --git a/config/autoload/error-handler.global.php b/config/autoload/error-handler.global.php index 2cf28a47..65e5b616 100644 --- a/config/autoload/error-handler.global.php +++ b/config/autoload/error-handler.global.php @@ -6,12 +6,14 @@ use Laminas\Stratigility\Middleware\ErrorHandler; use Mezzio\ProblemDetails\ProblemDetailsMiddleware; use Shlinkio\Shlink\Common\Logger; +use function Shlinkio\Shlink\Core\toProblemDetailsType; + return [ 'problem-details' => [ 'default_types_map' => [ - 404 => 'NOT_FOUND', // TODO Define new values, with backwards compatibility if possible - 500 => 'INTERNAL_SERVER_ERROR', // TODO Define new values, with backwards compatibility if possible + 404 => toProblemDetailsType('not-found'), + 500 => toProblemDetailsType('internal-server-error'), ], ], diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index c5186e41..d34175c7 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -127,3 +127,8 @@ function camelCaseToHumanFriendly(string $value): string return ucfirst($filter->filter($value)); } + +function toProblemDetailsType(string $errorCode): string +{ + return sprintf('https://shlink.io/api/error/%s', $errorCode); +} diff --git a/module/Core/src/Exception/DeleteShortUrlException.php b/module/Core/src/Exception/DeleteShortUrlException.php index f6638221..f8a5cfa8 100644 --- a/module/Core/src/Exception/DeleteShortUrlException.php +++ b/module/Core/src/Exception/DeleteShortUrlException.php @@ -9,6 +9,7 @@ use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; +use function Shlinkio\Shlink\Core\toProblemDetailsType; use function sprintf; class DeleteShortUrlException extends DomainException implements ProblemDetailsExceptionInterface @@ -16,7 +17,7 @@ class DeleteShortUrlException extends DomainException implements ProblemDetailsE use CommonProblemDetailsExceptionTrait; private const TITLE = 'Cannot delete short URL'; - public const TYPE = 'https://shlink.io/api/error/invalid-short-url-deletion'; + public const ERROR_CODE = 'invalid-short-url-deletion'; public static function fromVisitsThreshold(int $threshold, ShortUrlIdentifier $identifier): self { @@ -32,7 +33,7 @@ class DeleteShortUrlException extends DomainException implements ProblemDetailsE $e->detail = $e->getMessage(); $e->title = self::TITLE; - $e->type = self::TYPE; + $e->type = toProblemDetailsType(self::ERROR_CODE); $e->status = StatusCodeInterface::STATUS_UNPROCESSABLE_ENTITY; $e->additional = [ 'shortCode' => $shortCode, diff --git a/module/Core/src/Exception/DomainNotFoundException.php b/module/Core/src/Exception/DomainNotFoundException.php index aca67813..688a4edc 100644 --- a/module/Core/src/Exception/DomainNotFoundException.php +++ b/module/Core/src/Exception/DomainNotFoundException.php @@ -8,6 +8,7 @@ use Fig\Http\Message\StatusCodeInterface; use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; +use function Shlinkio\Shlink\Core\toProblemDetailsType; use function sprintf; class DomainNotFoundException extends DomainException implements ProblemDetailsExceptionInterface @@ -15,7 +16,7 @@ class DomainNotFoundException extends DomainException implements ProblemDetailsE use CommonProblemDetailsExceptionTrait; private const TITLE = 'Domain not found'; - public const TYPE = 'https://shlink.io/api/error/domain-not-found'; + public const ERROR_CODE = 'domain-not-found'; private function __construct(string $message, array $additional) { @@ -23,7 +24,7 @@ class DomainNotFoundException extends DomainException implements ProblemDetailsE $this->detail = $message; $this->title = self::TITLE; - $this->type = self::TYPE; + $this->type = toProblemDetailsType(self::ERROR_CODE); $this->status = StatusCodeInterface::STATUS_NOT_FOUND; $this->additional = $additional; } diff --git a/module/Core/src/Exception/ForbiddenTagOperationException.php b/module/Core/src/Exception/ForbiddenTagOperationException.php index 6da1cb61..64ae156c 100644 --- a/module/Core/src/Exception/ForbiddenTagOperationException.php +++ b/module/Core/src/Exception/ForbiddenTagOperationException.php @@ -8,12 +8,14 @@ use Fig\Http\Message\StatusCodeInterface; use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; +use function Shlinkio\Shlink\Core\toProblemDetailsType; + class ForbiddenTagOperationException extends DomainException implements ProblemDetailsExceptionInterface { use CommonProblemDetailsExceptionTrait; private const TITLE = 'Forbidden tag operation'; - public const TYPE = 'https://shlink.io/api/error/forbidden-tag-operation'; + public const ERROR_CODE = 'forbidden-tag-operation'; public static function forDeletion(): self { @@ -31,7 +33,7 @@ class ForbiddenTagOperationException extends DomainException implements ProblemD $e->detail = $message; $e->title = self::TITLE; - $e->type = self::TYPE; + $e->type = toProblemDetailsType(self::ERROR_CODE); $e->status = StatusCodeInterface::STATUS_FORBIDDEN; return $e; diff --git a/module/Core/src/Exception/InvalidUrlException.php b/module/Core/src/Exception/InvalidUrlException.php index 6d1f93c6..200914c2 100644 --- a/module/Core/src/Exception/InvalidUrlException.php +++ b/module/Core/src/Exception/InvalidUrlException.php @@ -9,6 +9,7 @@ use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; use Throwable; +use function Shlinkio\Shlink\Core\toProblemDetailsType; use function sprintf; class InvalidUrlException extends DomainException implements ProblemDetailsExceptionInterface @@ -16,7 +17,7 @@ class InvalidUrlException extends DomainException implements ProblemDetailsExcep use CommonProblemDetailsExceptionTrait; private const TITLE = 'Invalid URL'; - public const TYPE = 'https://shlink.io/api/error/invalid-url'; + public const ERROR_CODE = 'invalid-url'; public static function fromUrl(string $url, ?Throwable $previous = null): self { @@ -25,7 +26,7 @@ class InvalidUrlException extends DomainException implements ProblemDetailsExcep $e->detail = $e->getMessage(); $e->title = self::TITLE; - $e->type = self::TYPE; + $e->type = toProblemDetailsType(self::ERROR_CODE); $e->status = $status; $e->additional = ['url' => $url]; diff --git a/module/Core/src/Exception/NonUniqueSlugException.php b/module/Core/src/Exception/NonUniqueSlugException.php index dc1fcca9..5336786c 100644 --- a/module/Core/src/Exception/NonUniqueSlugException.php +++ b/module/Core/src/Exception/NonUniqueSlugException.php @@ -9,6 +9,7 @@ use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; +use function Shlinkio\Shlink\Core\toProblemDetailsType; use function sprintf; class NonUniqueSlugException extends InvalidArgumentException implements ProblemDetailsExceptionInterface @@ -16,7 +17,7 @@ class NonUniqueSlugException extends InvalidArgumentException implements Problem use CommonProblemDetailsExceptionTrait; private const TITLE = 'Invalid custom slug'; - public const TYPE = 'https://shlink.io/api/error/non-unique-slug'; + public const ERROR_CODE = 'non-unique-slug'; public static function fromSlug(string $slug, ?string $domain = null): self { @@ -25,7 +26,7 @@ class NonUniqueSlugException extends InvalidArgumentException implements Problem $e->detail = $e->getMessage(); $e->title = self::TITLE; - $e->type = self::TYPE; + $e->type = toProblemDetailsType(self::ERROR_CODE); $e->status = StatusCodeInterface::STATUS_BAD_REQUEST; $e->additional = ['customSlug' => $slug]; diff --git a/module/Core/src/Exception/ShortUrlNotFoundException.php b/module/Core/src/Exception/ShortUrlNotFoundException.php index 4da6972e..49b8cc02 100644 --- a/module/Core/src/Exception/ShortUrlNotFoundException.php +++ b/module/Core/src/Exception/ShortUrlNotFoundException.php @@ -9,6 +9,7 @@ use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; +use function Shlinkio\Shlink\Core\toProblemDetailsType; use function sprintf; class ShortUrlNotFoundException extends DomainException implements ProblemDetailsExceptionInterface @@ -16,7 +17,7 @@ class ShortUrlNotFoundException extends DomainException implements ProblemDetail use CommonProblemDetailsExceptionTrait; private const TITLE = 'Short URL not found'; - public const TYPE = 'https://shlink.io/api/error/short-url-not-found'; + public const ERROR_CODE = 'short-url-not-found'; public static function fromNotFound(ShortUrlIdentifier $identifier): self { @@ -27,7 +28,7 @@ class ShortUrlNotFoundException extends DomainException implements ProblemDetail $e->detail = $e->getMessage(); $e->title = self::TITLE; - $e->type = self::TYPE; + $e->type = toProblemDetailsType(self::ERROR_CODE); $e->status = StatusCodeInterface::STATUS_NOT_FOUND; $e->additional = ['shortCode' => $shortCode]; diff --git a/module/Core/src/Exception/TagConflictException.php b/module/Core/src/Exception/TagConflictException.php index 09ea7be4..0fc5c317 100644 --- a/module/Core/src/Exception/TagConflictException.php +++ b/module/Core/src/Exception/TagConflictException.php @@ -9,6 +9,7 @@ use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; +use function Shlinkio\Shlink\Core\toProblemDetailsType; use function sprintf; class TagConflictException extends RuntimeException implements ProblemDetailsExceptionInterface @@ -16,7 +17,7 @@ class TagConflictException extends RuntimeException implements ProblemDetailsExc use CommonProblemDetailsExceptionTrait; private const TITLE = 'Tag conflict'; - public const TYPE = 'https://shlink.io/api/error/tag-conflict'; + public const ERROR_CODE = 'tag-conflict'; public static function forExistingTag(TagRenaming $renaming): self { @@ -24,7 +25,7 @@ class TagConflictException extends RuntimeException implements ProblemDetailsExc $e->detail = $e->getMessage(); $e->title = self::TITLE; - $e->type = self::TYPE; + $e->type = toProblemDetailsType(self::ERROR_CODE); $e->status = StatusCodeInterface::STATUS_CONFLICT; $e->additional = $renaming->toArray(); diff --git a/module/Core/src/Exception/TagNotFoundException.php b/module/Core/src/Exception/TagNotFoundException.php index da2426aa..8fdd395a 100644 --- a/module/Core/src/Exception/TagNotFoundException.php +++ b/module/Core/src/Exception/TagNotFoundException.php @@ -8,6 +8,7 @@ use Fig\Http\Message\StatusCodeInterface; use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; +use function Shlinkio\Shlink\Core\toProblemDetailsType; use function sprintf; class TagNotFoundException extends DomainException implements ProblemDetailsExceptionInterface @@ -15,7 +16,7 @@ class TagNotFoundException extends DomainException implements ProblemDetailsExce use CommonProblemDetailsExceptionTrait; private const TITLE = 'Tag not found'; - public const TYPE = 'https://shlink.io/api/error/tag-not-found'; + public const ERROR_CODE = 'tag-not-found'; public static function fromTag(string $tag): self { @@ -23,7 +24,7 @@ class TagNotFoundException extends DomainException implements ProblemDetailsExce $e->detail = $e->getMessage(); $e->title = self::TITLE; - $e->type = self::TYPE; + $e->type = toProblemDetailsType(self::ERROR_CODE); $e->status = StatusCodeInterface::STATUS_NOT_FOUND; $e->additional = ['tag' => $tag]; diff --git a/module/Core/src/Exception/ValidationException.php b/module/Core/src/Exception/ValidationException.php index 0f625bc5..dcb11fa4 100644 --- a/module/Core/src/Exception/ValidationException.php +++ b/module/Core/src/Exception/ValidationException.php @@ -12,6 +12,7 @@ use Throwable; use function array_keys; use function Shlinkio\Shlink\Core\arrayToString; +use function Shlinkio\Shlink\Core\toProblemDetailsType; use function sprintf; use const PHP_EOL; @@ -21,7 +22,7 @@ class ValidationException extends InvalidArgumentException implements ProblemDet use CommonProblemDetailsExceptionTrait; private const TITLE = 'Invalid data'; - public const TYPE = 'https://shlink.io/api/error/invalid-data'; + public const ERROR_CODE = 'invalid-data'; private array $invalidElements; @@ -37,7 +38,7 @@ class ValidationException extends InvalidArgumentException implements ProblemDet $e->detail = $e->getMessage(); $e->title = self::TITLE; - $e->type = self::TYPE; + $e->type = toProblemDetailsType(self::ERROR_CODE); $e->status = StatusCodeInterface::STATUS_BAD_REQUEST; $e->invalidElements = $invalidData; $e->additional = ['invalidElements' => array_keys($invalidData)]; diff --git a/module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php b/module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php index 14a6e934..685d3795 100644 --- a/module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php +++ b/module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php @@ -15,6 +15,9 @@ use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Exception\ValidationException; +use function explode; +use function Functional\last; + /** @deprecated */ class BackwardsCompatibleProblemDetailsException extends RuntimeException implements ProblemDetailsExceptionInterface { @@ -74,19 +77,20 @@ class BackwardsCompatibleProblemDetailsException extends RuntimeException implem private function remapType(string $wrappedType): string { - return match ($wrappedType) { - ValidationException::TYPE => 'INVALID_ARGUMENT', - DeleteShortUrlException::TYPE => 'INVALID_SHORT_URL_DELETION', - DomainNotFoundException::TYPE => 'DOMAIN_NOT_FOUND', - ForbiddenTagOperationException::TYPE => 'FORBIDDEN_OPERATION', - InvalidUrlException::TYPE => 'INVALID_URL', - NonUniqueSlugException::TYPE => 'INVALID_SLUG', - ShortUrlNotFoundException::TYPE => 'INVALID_SHORTCODE', - TagConflictException::TYPE => 'TAG_CONFLICT', - TagNotFoundException::TYPE => 'TAG_NOT_FOUND', - MercureException::TYPE => 'MERCURE_NOT_CONFIGURED', - MissingAuthenticationException::TYPE => 'INVALID_AUTHORIZATION', - VerifyAuthenticationException::TYPE => 'INVALID_API_KEY', + $lastSegment = last(explode('/', $wrappedType)); + return match ($lastSegment) { + ValidationException::ERROR_CODE => 'INVALID_ARGUMENT', + DeleteShortUrlException::ERROR_CODE => 'INVALID_SHORT_URL_DELETION', + DomainNotFoundException::ERROR_CODE => 'DOMAIN_NOT_FOUND', + ForbiddenTagOperationException::ERROR_CODE => 'FORBIDDEN_OPERATION', + InvalidUrlException::ERROR_CODE => 'INVALID_URL', + NonUniqueSlugException::ERROR_CODE => 'INVALID_SLUG', + ShortUrlNotFoundException::ERROR_CODE => 'INVALID_SHORTCODE', + TagConflictException::ERROR_CODE => 'TAG_CONFLICT', + TagNotFoundException::ERROR_CODE => 'TAG_NOT_FOUND', + MercureException::ERROR_CODE => 'MERCURE_NOT_CONFIGURED', + MissingAuthenticationException::ERROR_CODE => 'INVALID_AUTHORIZATION', + VerifyAuthenticationException::ERROR_CODE => 'INVALID_API_KEY', default => $wrappedType, }; } diff --git a/module/Rest/src/Exception/MercureException.php b/module/Rest/src/Exception/MercureException.php index 0e9a6edb..7e47b519 100644 --- a/module/Rest/src/Exception/MercureException.php +++ b/module/Rest/src/Exception/MercureException.php @@ -8,12 +8,14 @@ use Fig\Http\Message\StatusCodeInterface; use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; +use function Shlinkio\Shlink\Core\toProblemDetailsType; + class MercureException extends RuntimeException implements ProblemDetailsExceptionInterface { use CommonProblemDetailsExceptionTrait; private const TITLE = 'Mercure integration not configured'; - public const TYPE = 'https://shlink.io/api/error/mercure-not-configured'; + public const ERROR_CODE = 'mercure-not-configured'; public static function mercureNotConfigured(): self { @@ -21,7 +23,7 @@ class MercureException extends RuntimeException implements ProblemDetailsExcepti $e->detail = $e->getMessage(); $e->title = self::TITLE; - $e->type = self::TYPE; + $e->type = toProblemDetailsType(self::ERROR_CODE); $e->status = StatusCodeInterface::STATUS_NOT_IMPLEMENTED; return $e; diff --git a/module/Rest/src/Exception/MissingAuthenticationException.php b/module/Rest/src/Exception/MissingAuthenticationException.php index 3c1f5b9f..3fd2e2c6 100644 --- a/module/Rest/src/Exception/MissingAuthenticationException.php +++ b/module/Rest/src/Exception/MissingAuthenticationException.php @@ -9,6 +9,7 @@ use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; use function implode; +use function Shlinkio\Shlink\Core\toProblemDetailsType; use function sprintf; class MissingAuthenticationException extends RuntimeException implements ProblemDetailsExceptionInterface @@ -16,7 +17,7 @@ class MissingAuthenticationException extends RuntimeException implements Problem use CommonProblemDetailsExceptionTrait; private const TITLE = 'Invalid authorization'; - public const TYPE = 'https://shlink.io/api/error/missing-authentication'; + public const ERROR_CODE = 'missing-authentication'; public static function forHeaders(array $expectedHeaders): self { @@ -43,7 +44,7 @@ class MissingAuthenticationException extends RuntimeException implements Problem $e->detail = $message; $e->title = self::TITLE; - $e->type = self::TYPE; + $e->type = toProblemDetailsType(self::ERROR_CODE); $e->status = StatusCodeInterface::STATUS_UNAUTHORIZED; return $e; diff --git a/module/Rest/src/Exception/VerifyAuthenticationException.php b/module/Rest/src/Exception/VerifyAuthenticationException.php index 25541d03..25f1b050 100644 --- a/module/Rest/src/Exception/VerifyAuthenticationException.php +++ b/module/Rest/src/Exception/VerifyAuthenticationException.php @@ -8,11 +8,13 @@ use Fig\Http\Message\StatusCodeInterface; use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; +use function Shlinkio\Shlink\Core\toProblemDetailsType; + class VerifyAuthenticationException extends RuntimeException implements ProblemDetailsExceptionInterface { use CommonProblemDetailsExceptionTrait; - public const TYPE = 'https://shlink.io/api/error/invalid-api-key'; + public const ERROR_CODE = 'invalid-api-key'; public static function forInvalidApiKey(): self { @@ -20,7 +22,7 @@ class VerifyAuthenticationException extends RuntimeException implements ProblemD $e->detail = $e->getMessage(); $e->title = 'Invalid API key'; - $e->type = self::TYPE; + $e->type = toProblemDetailsType(self::ERROR_CODE); $e->status = StatusCodeInterface::STATUS_UNAUTHORIZED; return $e; diff --git a/module/Rest/test/Exception/BackwardsCompatibleProblemDetailsExceptionTest.php b/module/Rest/test/Exception/BackwardsCompatibleProblemDetailsExceptionTest.php index 13df168b..c63cee71 100644 --- a/module/Rest/test/Exception/BackwardsCompatibleProblemDetailsExceptionTest.php +++ b/module/Rest/test/Exception/BackwardsCompatibleProblemDetailsExceptionTest.php @@ -98,17 +98,17 @@ class BackwardsCompatibleProblemDetailsExceptionTest extends TestCase { yield ['foo', 'foo', true]; yield ['bar', 'bar', true]; - yield [ValidationException::TYPE, 'INVALID_ARGUMENT']; - yield [DeleteShortUrlException::TYPE, 'INVALID_SHORT_URL_DELETION']; - yield [DomainNotFoundException::TYPE, 'DOMAIN_NOT_FOUND']; - yield [ForbiddenTagOperationException::TYPE, 'FORBIDDEN_OPERATION']; - yield [InvalidUrlException::TYPE, 'INVALID_URL']; - yield [NonUniqueSlugException::TYPE, 'INVALID_SLUG']; - yield [ShortUrlNotFoundException::TYPE, 'INVALID_SHORTCODE']; - yield [TagConflictException::TYPE, 'TAG_CONFLICT']; - yield [TagNotFoundException::TYPE, 'TAG_NOT_FOUND']; - yield [MercureException::TYPE, 'MERCURE_NOT_CONFIGURED']; - yield [MissingAuthenticationException::TYPE, 'INVALID_AUTHORIZATION']; - yield [VerifyAuthenticationException::TYPE, 'INVALID_API_KEY']; + yield [ValidationException::ERROR_CODE, 'INVALID_ARGUMENT']; + yield [DeleteShortUrlException::ERROR_CODE, 'INVALID_SHORT_URL_DELETION']; + yield [DomainNotFoundException::ERROR_CODE, 'DOMAIN_NOT_FOUND']; + yield [ForbiddenTagOperationException::ERROR_CODE, 'FORBIDDEN_OPERATION']; + yield [InvalidUrlException::ERROR_CODE, 'INVALID_URL']; + yield [NonUniqueSlugException::ERROR_CODE, 'INVALID_SLUG']; + yield [ShortUrlNotFoundException::ERROR_CODE, 'INVALID_SHORTCODE']; + yield [TagConflictException::ERROR_CODE, 'TAG_CONFLICT']; + yield [TagNotFoundException::ERROR_CODE, 'TAG_NOT_FOUND']; + yield [MercureException::ERROR_CODE, 'MERCURE_NOT_CONFIGURED']; + yield [MissingAuthenticationException::ERROR_CODE, 'INVALID_AUTHORIZATION']; + yield [VerifyAuthenticationException::ERROR_CODE, 'INVALID_API_KEY']; } } From 750a546faf94b25d878a912f106d65b1d5ae5c28 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 Aug 2022 13:18:29 +0200 Subject: [PATCH 08/10] Disabled mutation tests filtering until it properly works --- .github/workflows/ci-mutation-tests.yml | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci-mutation-tests.yml b/.github/workflows/ci-mutation-tests.yml index f152ed15..337894e2 100644 --- a/.github/workflows/ci-mutation-tests.yml +++ b/.github/workflows/ci-mutation-tests.yml @@ -26,13 +26,15 @@ jobs: path: build - name: Resolve infection args id: infection_args - run: | - BRANCH="${GITHUB_REF#refs/heads/}" | - if [[ $BRANCH == 'main' || $BRANCH == 'develop' ]]; then - echo "::set-output name=args::--logger-github=false" - else - echo "::set-output name=args::--logger-github=false --git-diff-lines --git-diff-base=develop" - fi; + run: echo "::set-output name=args::--logger-github=false" +# TODO Try to filter mutation tests to improve execution times. Investigate why --git-diff-lines --git-diff-base=develop does not work +# run: | +# BRANCH="${GITHUB_REF#refs/heads/}" | +# if [[ $BRANCH == 'main' || $BRANCH == 'develop' ]]; then +# echo "::set-output name=args::--logger-github=false" +# else +# echo "::set-output name=args::--logger-github=false --git-diff-lines --git-diff-base=develop" +# fi; shell: bash - if: ${{ inputs.test-group == 'unit' }} run: composer infect:ci:unit -- ${{ steps.infection_args.outputs.args }} From 672b7283790d1b80e0673ca210f59f0e6dc42f01 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 Aug 2022 13:55:43 +0200 Subject: [PATCH 09/10] Updated swagger docs, with new API v3 error types --- ...gs.json => short-url-invalid-args-v2.json} | 0 .../examples/short-url-invalid-args-v3.json | 9 ++ ...found.json => short-url-not-found-v2.json} | 0 .../examples/short-url-not-found-v3.json | 9 ++ ...g-not-found.json => tag-not-found-v2.json} | 0 docs/swagger/examples/tag-not-found-v3.json | 9 ++ docs/swagger/paths/v1_short-urls.json | 33 +++++- docs/swagger/paths/v1_short-urls_shorten.json | 34 ++++-- .../paths/v1_short-urls_{shortCode}.json | 56 +++++++--- .../v1_short-urls_{shortCode}_visits.json | 7 +- docs/swagger/paths/v1_tags.json | 104 +++++++++++++----- docs/swagger/paths/v2_domains_redirects.json | 25 ++++- .../paths/v2_domains_{domain}_visits.json | 25 ++++- docs/swagger/paths/v2_mercure-info.json | 22 +++- docs/swagger/paths/v2_tags_{tag}_visits.json | 8 +- 15 files changed, 267 insertions(+), 74 deletions(-) rename docs/swagger/examples/{short-url-invalid-args.json => short-url-invalid-args-v2.json} (100%) create mode 100644 docs/swagger/examples/short-url-invalid-args-v3.json rename docs/swagger/examples/{short-url-not-found.json => short-url-not-found-v2.json} (100%) create mode 100644 docs/swagger/examples/short-url-not-found-v3.json rename docs/swagger/examples/{tag-not-found.json => tag-not-found-v2.json} (100%) create mode 100644 docs/swagger/examples/tag-not-found-v3.json diff --git a/docs/swagger/examples/short-url-invalid-args.json b/docs/swagger/examples/short-url-invalid-args-v2.json similarity index 100% rename from docs/swagger/examples/short-url-invalid-args.json rename to docs/swagger/examples/short-url-invalid-args-v2.json diff --git a/docs/swagger/examples/short-url-invalid-args-v3.json b/docs/swagger/examples/short-url-invalid-args-v3.json new file mode 100644 index 00000000..3e9171c6 --- /dev/null +++ b/docs/swagger/examples/short-url-invalid-args-v3.json @@ -0,0 +1,9 @@ +{ + "value": { + "title": "Invalid data", + "type": "https://shlink.io/api/error/invalid-data", + "detail": "Provided data is not valid", + "status": 400, + "invalidElements": ["maxVisits", "validSince"] + } +} diff --git a/docs/swagger/examples/short-url-not-found.json b/docs/swagger/examples/short-url-not-found-v2.json similarity index 100% rename from docs/swagger/examples/short-url-not-found.json rename to docs/swagger/examples/short-url-not-found-v2.json diff --git a/docs/swagger/examples/short-url-not-found-v3.json b/docs/swagger/examples/short-url-not-found-v3.json new file mode 100644 index 00000000..82f3469c --- /dev/null +++ b/docs/swagger/examples/short-url-not-found-v3.json @@ -0,0 +1,9 @@ +{ + "value": { + "detail": "No URL found with short code \"abc123\"", + "title": "Short URL not found", + "type": "https://shlink.io/api/error/short-url-not-found", + "status": 404, + "shortCode": "abc123" + } +} diff --git a/docs/swagger/examples/tag-not-found.json b/docs/swagger/examples/tag-not-found-v2.json similarity index 100% rename from docs/swagger/examples/tag-not-found.json rename to docs/swagger/examples/tag-not-found-v2.json diff --git a/docs/swagger/examples/tag-not-found-v3.json b/docs/swagger/examples/tag-not-found-v3.json new file mode 100644 index 00000000..62beb42c --- /dev/null +++ b/docs/swagger/examples/tag-not-found-v3.json @@ -0,0 +1,9 @@ +{ + "value": { + "detail": "Tag with name \"foo\" could not be found", + "title": "Tag not found", + "type": "https://shlink.io/api/error/tag-not-found", + "status": 404, + "tag": "foo" + } +} diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index 6e8bb015..2675ab61 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -327,11 +327,11 @@ }, "url": { "type": "string", - "description": "A URL that could not be verified, if the error type is INVALID_URL" + "description": "A URL that could not be verified, if the error type is https://shlink.io/api/error/invalid-url" }, "customSlug": { "type": "string", - "description": "Provided custom slug when the error type is INVALID_SLUG" + "description": "Provided custom slug when the error type is https://shlink.io/api/error/non-unique-slug" }, "domain": { "type": "string", @@ -342,10 +342,31 @@ ] }, "examples": { - "Invalid arguments": { - "$ref": "../examples/short-url-invalid-args.json" + "Invalid arguments with API v3 and newer": { + "$ref": "../examples/short-url-invalid-args-v3.json" }, - "Invalid long URL": { + "Invalid long URL with API v3 and newer": { + "value": { + "title": "Invalid URL", + "type": "https://shlink.io/api/error/invalid-url", + "detail": "Provided URL foo is invalid. Try with a different one.", + "status": 400, + "url": "https://invalid-url.com" + } + }, + "Non-unique slug with API v3 and newer": { + "value": { + "title": "Invalid custom slug", + "type": "https://shlink.io/api/error/non-unique-slug", + "detail": "Provided slug \"my-slug\" is already in use.", + "status": 400, + "customSlug": "my-slug" + } + }, + "Invalid arguments previous to API v3": { + "$ref": "../examples/short-url-invalid-args-v2.json" + }, + "Invalid long URL previous to API v3": { "value": { "title": "Invalid URL", "type": "INVALID_URL", @@ -354,7 +375,7 @@ "url": "https://invalid-url.com" } }, - "Non-unique slug": { + "Non-unique slug previous to API v3": { "value": { "title": "Invalid custom slug", "type": "INVALID_SLUG", diff --git a/docs/swagger/paths/v1_short-urls_shorten.json b/docs/swagger/paths/v1_short-urls_shorten.json index 722476bb..aa26fa1b 100644 --- a/docs/swagger/paths/v1_short-urls_shorten.json +++ b/docs/swagger/paths/v1_short-urls_shorten.json @@ -85,19 +85,39 @@ "schema": { "$ref": "../definitions/Error.json" }, - "example": { - "title": "Invalid URL", - "type": "INVALID_URL", - "detail": "Provided URL foo is invalid. Try with a different one.", - "status": 400, - "url": "https://invalid-url.com" + "examples": { + "API v3 and newer": { + "value": { + "title": "Invalid URL", + "type": "https://shlink.io/api/error/invalid-url", + "detail": "Provided URL foo is invalid. Try with a different one.", + "status": 400, + "url": "https://invalid-url.com" + } + }, + "Previous to API v3": { + "value": { + "title": "Invalid URL", + "type": "INVALID_URL", + "detail": "Provided URL foo is invalid. Try with a different one.", + "status": 400, + "url": "https://invalid-url.com" + } + } } }, "text/plain": { "schema": { "type": "string" }, - "example": "INVALID_URL" + "examples": { + "API v3 and newer": { + "value": "https://shlink.io/api/error/invalid-url" + }, + "Previous to API v3": { + "value": "INVALID_URL" + } + } } } }, diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}.json b/docs/swagger/paths/v1_short-urls_{shortCode}.json index 9065ff89..1b001cc9 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}.json @@ -83,8 +83,11 @@ ] }, "examples": { - "Not found": { - "$ref": "../examples/short-url-not-found.json" + "API v3 and newer": { + "$ref": "../examples/short-url-not-found-v3.json" + }, + "Previous to API v3": { + "$ref": "../examples/short-url-not-found-v2.json" } } } @@ -203,8 +206,11 @@ ] }, "examples": { - "Invalid arguments": { - "$ref": "../examples/short-url-invalid-args.json" + "API v3 and newer": { + "$ref": "../examples/short-url-invalid-args-v3.json" + }, + "Previous to API v3": { + "$ref": "../examples/short-url-invalid-args-v2.json" } } } @@ -236,8 +242,11 @@ ] }, "examples": { - "Not found": { - "$ref": "../examples/short-url-not-found.json" + "API v3 and newer": { + "$ref": "../examples/short-url-not-found-v3.json" + }, + "Previous to API v3": { + "$ref": "../examples/short-url-not-found-v2.json" } } } @@ -318,13 +327,27 @@ } ] }, - "example": { - "title": "Cannot delete short URL", - "type": "INVALID_SHORT_URL_DELETION", - "detail": "Impossible to delete short URL with short code \"abc123\", since it has more than \"15\" visits.", - "status": 422, - "shortCode": "abc123", - "threshold": 15 + "examples": { + "API v3 and newer": { + "value": { + "title": "Cannot delete short URL", + "type": "https://shlink.io/api/error/invalid-short-url-deletion", + "detail": "Impossible to delete short URL with short code \"abc123\", since it has more than \"15\" visits.", + "status": 422, + "shortCode": "abc123", + "threshold": 15 + } + }, + "Previous to API v3": { + "value": { + "title": "Cannot delete short URL", + "type": "INVALID_SHORT_URL_DELETION", + "detail": "Impossible to delete short URL with short code \"abc123\", since it has more than \"15\" visits.", + "status": 422, + "shortCode": "abc123", + "threshold": 15 + } + } } } } @@ -355,8 +378,11 @@ ] }, "examples": { - "Not found": { - "$ref": "../examples/short-url-not-found.json" + "API v3 and newer": { + "$ref": "../examples/short-url-not-found-v3.json" + }, + "Previous to API v3": { + "$ref": "../examples/short-url-not-found-v2.json" } } } diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json b/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json index 08a93b68..e86bb698 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json @@ -151,8 +151,11 @@ "$ref": "../definitions/Error.json" }, "examples": { - "Short URL not found": { - "$ref": "../examples/short-url-not-found.json" + "Short URL not found with API v3 and newer": { + "$ref": "../examples/short-url-not-found-v3.json" + }, + "Short URL not found previous to API v3": { + "$ref": "../examples/short-url-not-found-v2.json" } } } diff --git a/docs/swagger/paths/v1_tags.json b/docs/swagger/paths/v1_tags.json index a8219bf1..0e77cf3c 100644 --- a/docs/swagger/paths/v1_tags.json +++ b/docs/swagger/paths/v1_tags.json @@ -188,12 +188,25 @@ "schema": { "$ref": "../definitions/Error.json" }, - "example": { - "title": "Invalid data", - "type": "INVALID_ARGUMENT", - "detail": "Provided data is not valid", - "status": 400, - "invalidElements": ["oldName", "newName"] + "examples": { + "API v3 and newer": { + "value": { + "title": "Invalid data", + "type": "https://shlink.io/api/error/invalid-data", + "detail": "Provided data is not valid", + "status": 400, + "invalidElements": ["oldName", "newName"] + } + }, + "Previous to API v3": { + "value": { + "title": "Invalid data", + "type": "INVALID_ARGUMENT", + "detail": "Provided data is not valid", + "status": 400, + "invalidElements": ["oldName", "newName"] + } + } } } } @@ -205,11 +218,23 @@ "schema": { "$ref": "../definitions/Error.json" }, - "example": { - "detail": "You are not allowed to rename tags", - "title": "Forbidden tag operation", - "type": "FORBIDDEN_OPERATION", - "status": 403 + "examples": { + "API v3 and newer": { + "value": { + "detail": "You are not allowed to rename tags", + "title": "Forbidden tag operation", + "type": "https://shlink.io/api/error/forbidden-tag-operation", + "status": 403 + } + }, + "Previous to API v3": { + "value": { + "detail": "You are not allowed to rename tags", + "title": "Forbidden tag operation", + "type": "FORBIDDEN_OPERATION", + "status": 403 + } + } } } } @@ -222,8 +247,11 @@ "$ref": "../definitions/Error.json" }, "examples": { - "Tag not found": { - "$ref": "../examples/tag-not-found.json" + "API v3 and newer": { + "$ref": "../examples/tag-not-found-v3.json" + }, + "Previous to API v3": { + "$ref": "../examples/tag-not-found-v2.json" } } } @@ -236,13 +264,27 @@ "schema": { "$ref": "../definitions/Error.json" }, - "example": { - "detail": "You cannot rename tag foo, because it already exists", - "title": "Tag conflict", - "type": "TAG_CONFLICT", - "status": 409, - "oldName": "bar", - "newName": "foo" + "examples": { + "API v3 and newer": { + "value": { + "detail": "You cannot rename tag foo, because it already exists", + "title": "Tag conflict", + "type": "https://shlink.io/api/error/tag-conflict", + "status": 409, + "oldName": "bar", + "newName": "foo" + } + }, + "Previous to API v3": { + "value": { + "detail": "You cannot rename tag foo, because it already exists", + "title": "Tag conflict", + "type": "TAG_CONFLICT", + "status": 409, + "oldName": "bar", + "newName": "foo" + } + } } } } @@ -300,11 +342,23 @@ "schema": { "$ref": "../definitions/Error.json" }, - "example": { - "detail": "You are not allowed to delete tags", - "title": "Forbidden tag operation", - "type": "FORBIDDEN_OPERATION", - "status": 403 + "examples": { + "API v3 and newer": { + "value": { + "detail": "You are not allowed to delete tags", + "title": "Forbidden tag operation", + "type": "https://shlink.io/api/error/forbidden-tag-operation", + "status": 403 + } + }, + "Previous to API v3": { + "value": { + "detail": "You are not allowed to delete tags", + "title": "Forbidden tag operation", + "type": "FORBIDDEN_OPERATION", + "status": 403 + } + } } } } diff --git a/docs/swagger/paths/v2_domains_redirects.json b/docs/swagger/paths/v2_domains_redirects.json index d4d4338c..cc328040 100644 --- a/docs/swagger/paths/v2_domains_redirects.json +++ b/docs/swagger/paths/v2_domains_redirects.json @@ -94,12 +94,25 @@ } ] }, - "example": { - "title": "Invalid data", - "type": "INVALID_ARGUMENT", - "detail": "Provided data is not valid", - "status": 400, - "invalidElements": ["domain", "invalidShortUrlRedirect"] + "examples": { + "API v3 and newer": { + "value": { + "title": "Invalid data", + "type": "https://shlink.io/api/error/invalid-data", + "detail": "Provided data is not valid", + "status": 400, + "invalidElements": ["domain", "invalidShortUrlRedirect"] + } + }, + "Previous to API v3": { + "value": { + "title": "Invalid data", + "type": "INVALID_ARGUMENT", + "detail": "Provided data is not valid", + "status": 400, + "invalidElements": ["domain", "invalidShortUrlRedirect"] + } + } } } } diff --git a/docs/swagger/paths/v2_domains_{domain}_visits.json b/docs/swagger/paths/v2_domains_{domain}_visits.json index 33389f32..d3acf60e 100644 --- a/docs/swagger/paths/v2_domains_{domain}_visits.json +++ b/docs/swagger/paths/v2_domains_{domain}_visits.json @@ -147,12 +147,25 @@ "schema": { "$ref": "../definitions/Error.json" }, - "example": { - "detail": "Domain with authority \"example.com\" could not be found", - "title": "Domain not found", - "type": "DOMAIN_NOT_FOUND", - "status": 404, - "authority": "example.com" + "examples": { + "API v3 and newer": { + "value": { + "detail": "Domain with authority \"example.com\" could not be found", + "title": "Domain not found", + "type": "https://shlink.io/api/error/domain-not-found", + "status": 404, + "authority": "example.com" + } + }, + "Previous to API v3": { + "value": { + "detail": "Domain with authority \"example.com\" could not be found", + "title": "Domain not found", + "type": "DOMAIN_NOT_FOUND", + "status": 404, + "authority": "example.com" + } + } } } } diff --git a/docs/swagger/paths/v2_mercure-info.json b/docs/swagger/paths/v2_mercure-info.json index a341573f..e637ca33 100644 --- a/docs/swagger/paths/v2_mercure-info.json +++ b/docs/swagger/paths/v2_mercure-info.json @@ -39,11 +39,23 @@ "schema": { "$ref": "../definitions/Error.json" }, - "example": { - "title": "Mercure integration not configured", - "type": "MERCURE_NOT_CONFIGURED", - "detail": "This Shlink instance is not integrated with a mercure hub.", - "status": 501 + "examples": { + "API v3 and newer": { + "value": { + "title": "Mercure integration not configured", + "type": "https://shlink.io/api/error/mercure-not-configured", + "detail": "This Shlink instance is not integrated with a mercure hub.", + "status": 501 + } + }, + "Previous to API v3": { + "value": { + "title": "Mercure integration not configured", + "type": "MERCURE_NOT_CONFIGURED", + "detail": "This Shlink instance is not integrated with a mercure hub.", + "status": 501 + } + } } } } diff --git a/docs/swagger/paths/v2_tags_{tag}_visits.json b/docs/swagger/paths/v2_tags_{tag}_visits.json index 109cb1d0..d40b7020 100644 --- a/docs/swagger/paths/v2_tags_{tag}_visits.json +++ b/docs/swagger/paths/v2_tags_{tag}_visits.json @@ -148,8 +148,12 @@ "$ref": "../definitions/Error.json" }, "examples": { - "Tag not found": { - "$ref": "../examples/tag-not-found.json" + + "API v3 and newer": { + "$ref": "../examples/tag-not-found-v3.json" + }, + "Previous to API v3": { + "$ref": "../examples/tag-not-found-v2.json" } } } From 39c71638e65429db1f936babbb9cee7dcbdf7792 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 Aug 2022 14:02:09 +0200 Subject: [PATCH 10/10] Updated changelog --- CHANGELOG.md | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 202e1ba2..b2ec6b82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,26 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added -* *Nothing* +* [#1406](https://github.com/shlinkio/shlink/issues/1406) Added new REST API version 3. + + When making requests to the REST API with `/rest/v3/...` and an error occurs, all error types will be different, with the next correlation: + + * `INVALID_ARGUMENT` -> `https://shlink.io/api/error/invalid-data` + * `INVALID_SHORT_URL_DELETION` -> `https://shlink.io/api/error/invalid-short-url-deletion` + * `DOMAIN_NOT_FOUND` -> `https://shlink.io/api/error/domain-not-found` + * `FORBIDDEN_OPERATION` -> `https://shlink.io/api/error/forbidden-tag-operation` + * `INVALID_URL` -> `https://shlink.io/api/error/invalid-url` + * `INVALID_SLUG` -> `https://shlink.io/api/error/non-unique-slug` + * `INVALID_SHORTCODE` -> `https://shlink.io/api/error/short-url-not-found` + * `TAG_CONFLICT` -> `https://shlink.io/api/error/tag-conflict` + * `TAG_NOT_FOUND` -> `https://shlink.io/api/error/tag-not-found` + * `MERCURE_NOT_CONFIGURED` -> `https://shlink.io/api/error/mercure-not-configured` + * `INVALID_AUTHORIZATION` -> `https://shlink.io/api/error/missing-authentication` + * `INVALID_API_KEY` -> `https://shlink.io/api/error/invalid-api-key` + + If you make a request to the API with v2 or v1, the old error types will be returned, until Shlink 4 is released, when only the new ones will be used. + + Non-error responses are not affected. ### Changed * [#1339](https://github.com/shlinkio/shlink/issues/1339) Added new test suite for CLI E2E tests.