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']; + } }