From d83d2f82bdf27b512e416bce05677073087af593 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 27 Nov 2019 20:48:35 +0100 Subject: [PATCH] Added more strict checks on API errors tests --- .../Action/CreateShortUrlActionTest.php | 31 +++++++++-- .../Action/DeleteShortUrlActionTest.php | 22 ++++++-- .../Action/EditShortUrlActionTest.php | 23 +++++++-- .../Action/EditShortUrlTagsActionTest.php | 23 +++++++-- .../test-api/Action/GetVisitsActionTest.php | 12 ++++- .../Action/ResolveShortUrlActionTest.php | 12 ++++- .../test-api/Action/UpdateTagActionTest.php | 22 ++++++-- .../Middleware/AuthenticationTest.php | 51 ++++++++++++------- 8 files changed, 156 insertions(+), 40 deletions(-) diff --git a/module/Rest/test-api/Action/CreateShortUrlActionTest.php b/module/Rest/test-api/Action/CreateShortUrlActionTest.php index 2098ea0c..37a2630c 100644 --- a/module/Rest/test-api/Action/CreateShortUrlActionTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlActionTest.php @@ -10,6 +10,7 @@ use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; use function Functional\map; use function range; +use function sprintf; class CreateShortUrlActionTest extends ApiTestCase { @@ -40,10 +41,25 @@ class CreateShortUrlActionTest extends ApiTestCase */ public function failsToCreateShortUrlWithDuplicatedSlug(string $slug, ?string $domain): void { + $suffix = $domain === null ? '' : sprintf(' for domain "%s"', $domain); + $detail = sprintf('Provided slug "%s" is already in use%s.', $slug, $suffix); + [$statusCode, $payload] = $this->createShortUrl(['customSlug' => $slug, 'domain' => $domain]); $this->assertEquals(self::STATUS_BAD_REQUEST, $statusCode); - $this->assertEquals('INVALID_SLUG', $payload['error']); + $this->assertEquals(self::STATUS_BAD_REQUEST, $payload['status']); + $this->assertEquals($detail, $payload['detail']); + $this->assertEquals($detail, $payload['message']); // Deprecated + $this->assertEquals('INVALID_SLUG', $payload['type']); + $this->assertEquals('INVALID_SLUG', $payload['error']); // Deprecated + $this->assertEquals('Invalid custom slug', $payload['title']); + $this->assertEquals($slug, $payload['customSlug']); + + if ($domain !== null) { + $this->assertEquals($domain, $payload['domain']); + } else { + $this->assertArrayNotHasKey('domain', $payload); + } } /** @test */ @@ -203,10 +219,19 @@ class CreateShortUrlActionTest extends ApiTestCase /** @test */ public function failsToCreateShortUrlWithInvalidOriginalUrl(): void { - [$statusCode, $payload] = $this->createShortUrl(['longUrl' => 'https://this-has-to-be-invalid.com']); + $url = 'https://this-has-to-be-invalid.com'; + $expectedDetail = sprintf('Provided URL %s is invalid. Try with a different one.', $url); + + [$statusCode, $payload] = $this->createShortUrl(['longUrl' => $url]); $this->assertEquals(self::STATUS_BAD_REQUEST, $statusCode); - $this->assertEquals('INVALID_URL', $payload['error']); + $this->assertEquals(self::STATUS_BAD_REQUEST, $payload['status']); + $this->assertEquals('INVALID_URL', $payload['type']); + $this->assertEquals('INVALID_URL', $payload['error']); // Deprecated + $this->assertEquals($expectedDetail, $payload['detail']); + $this->assertEquals($expectedDetail, $payload['message']); // Deprecated + $this->assertEquals('Invalid URL', $payload['title']); + $this->assertEquals($url, $payload['url']); } /** diff --git a/module/Rest/test-api/Action/DeleteShortUrlActionTest.php b/module/Rest/test-api/Action/DeleteShortUrlActionTest.php index c6cf443e..4680a6f6 100644 --- a/module/Rest/test-api/Action/DeleteShortUrlActionTest.php +++ b/module/Rest/test-api/Action/DeleteShortUrlActionTest.php @@ -11,11 +11,19 @@ class DeleteShortUrlActionTest extends ApiTestCase /** @test */ public function notFoundErrorIsReturnWhenDeletingInvalidUrl(): void { + $expectedDetail = 'No URL found with short code "invalid"'; + $resp = $this->callApiWithKey(self::METHOD_DELETE, '/short-urls/invalid'); - ['error' => $error] = $this->getJsonResponsePayload($resp); + $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); - $this->assertEquals('INVALID_SHORTCODE', $error); + $this->assertEquals(self::STATUS_NOT_FOUND, $payload['status']); + $this->assertEquals('INVALID_SHORTCODE', $payload['type']); + $this->assertEquals('INVALID_SHORTCODE', $payload['error']); // Deprecated + $this->assertEquals($expectedDetail, $payload['detail']); + $this->assertEquals($expectedDetail, $payload['message']); // Deprecated + $this->assertEquals('Short URL not found', $payload['title']); + $this->assertEquals('invalid', $payload['shortCode']); } /** @test */ @@ -25,11 +33,17 @@ class DeleteShortUrlActionTest extends ApiTestCase for ($i = 0; $i < 20; $i++) { $this->assertEquals(self::STATUS_FOUND, $this->callShortUrl('abc123')->getStatusCode()); } + $expectedDetail = 'Impossible to delete short URL with short code "abc123" since it has more than "15" visits.'; $resp = $this->callApiWithKey(self::METHOD_DELETE, '/short-urls/abc123'); - ['error' => $error] = $this->getJsonResponsePayload($resp); + $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_UNPROCESSABLE_ENTITY, $resp->getStatusCode()); - $this->assertEquals('INVALID_SHORTCODE_DELETION', $error); + $this->assertEquals(self::STATUS_UNPROCESSABLE_ENTITY, $payload['status']); + $this->assertEquals('INVALID_SHORTCODE_DELETION', $payload['type']); + $this->assertEquals('INVALID_SHORTCODE_DELETION', $payload['error']); // Deprecated + $this->assertEquals($expectedDetail, $payload['detail']); + $this->assertEquals($expectedDetail, $payload['message']); // Deprecated + $this->assertEquals('Cannot delete short URL', $payload['title']); } } diff --git a/module/Rest/test-api/Action/EditShortUrlActionTest.php b/module/Rest/test-api/Action/EditShortUrlActionTest.php index 739ec191..bbcb043a 100644 --- a/module/Rest/test-api/Action/EditShortUrlActionTest.php +++ b/module/Rest/test-api/Action/EditShortUrlActionTest.php @@ -12,22 +12,37 @@ class EditShortUrlActionTest extends ApiTestCase /** @test */ public function tryingToEditInvalidUrlReturnsNotFoundError(): void { + $expectedDetail = 'No URL found with short code "invalid"'; + $resp = $this->callApiWithKey(self::METHOD_PATCH, '/short-urls/invalid', [RequestOptions::JSON => []]); - ['error' => $error] = $this->getJsonResponsePayload($resp); + $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); - $this->assertEquals('INVALID_SHORTCODE', $error); + $this->assertEquals(self::STATUS_NOT_FOUND, $payload['status']); + $this->assertEquals('INVALID_SHORTCODE', $payload['type']); + $this->assertEquals('INVALID_SHORTCODE', $payload['error']); // Deprecated + $this->assertEquals($expectedDetail, $payload['detail']); + $this->assertEquals($expectedDetail, $payload['message']); // Deprecated + $this->assertEquals('Short URL not found', $payload['title']); + $this->assertEquals('invalid', $payload['shortCode']); } /** @test */ public function providingInvalidDataReturnsBadRequest(): void { + $expectedDetail = 'Provided data is not valid'; + $resp = $this->callApiWithKey(self::METHOD_PATCH, '/short-urls/invalid', [RequestOptions::JSON => [ 'maxVisits' => 'not_a_number', ]]); - ['error' => $error] = $this->getJsonResponsePayload($resp); + $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_BAD_REQUEST, $resp->getStatusCode()); - $this->assertEquals('INVALID_ARGUMENT', $error); + $this->assertEquals(self::STATUS_BAD_REQUEST, $payload['status']); + $this->assertEquals('INVALID_ARGUMENT', $payload['type']); + $this->assertEquals('INVALID_ARGUMENT', $payload['error']); // Deprecated + $this->assertEquals($expectedDetail, $payload['detail']); + $this->assertEquals($expectedDetail, $payload['message']); // Deprecated + $this->assertEquals('Invalid data', $payload['title']); } } diff --git a/module/Rest/test-api/Action/EditShortUrlTagsActionTest.php b/module/Rest/test-api/Action/EditShortUrlTagsActionTest.php index 5116d1e6..e2922092 100644 --- a/module/Rest/test-api/Action/EditShortUrlTagsActionTest.php +++ b/module/Rest/test-api/Action/EditShortUrlTagsActionTest.php @@ -12,22 +12,37 @@ class EditShortUrlTagsActionTest extends ApiTestCase /** @test */ public function notProvidingTagsReturnsBadRequest(): void { + $expectedDetail = 'Provided data is not valid'; + $resp = $this->callApiWithKey(self::METHOD_PUT, '/short-urls/abc123/tags', [RequestOptions::JSON => []]); - ['error' => $error] = $this->getJsonResponsePayload($resp); + $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_BAD_REQUEST, $resp->getStatusCode()); - $this->assertEquals('INVALID_ARGUMENT', $error); + $this->assertEquals(self::STATUS_BAD_REQUEST, $payload['status']); + $this->assertEquals('INVALID_ARGUMENT', $payload['type']); + $this->assertEquals('INVALID_ARGUMENT', $payload['error']); // Deprecated + $this->assertEquals($expectedDetail, $payload['detail']); + $this->assertEquals($expectedDetail, $payload['message']); // Deprecated + $this->assertEquals('Invalid data', $payload['title']); } /** @test */ public function providingInvalidShortCodeReturnsBadRequest(): void { + $expectedDetail = 'No URL found with short code "invalid"'; + $resp = $this->callApiWithKey(self::METHOD_PUT, '/short-urls/invalid/tags', [RequestOptions::JSON => [ 'tags' => ['foo', 'bar'], ]]); - ['error' => $error] = $this->getJsonResponsePayload($resp); + $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); - $this->assertEquals('INVALID_SHORTCODE', $error); + $this->assertEquals(self::STATUS_NOT_FOUND, $payload['status']); + $this->assertEquals('INVALID_SHORTCODE', $payload['type']); + $this->assertEquals('INVALID_SHORTCODE', $payload['error']); // Deprecated + $this->assertEquals($expectedDetail, $payload['detail']); + $this->assertEquals($expectedDetail, $payload['message']); // Deprecated + $this->assertEquals('Short URL not found', $payload['title']); + $this->assertEquals('invalid', $payload['shortCode']); } } diff --git a/module/Rest/test-api/Action/GetVisitsActionTest.php b/module/Rest/test-api/Action/GetVisitsActionTest.php index f9c6f404..0db06848 100644 --- a/module/Rest/test-api/Action/GetVisitsActionTest.php +++ b/module/Rest/test-api/Action/GetVisitsActionTest.php @@ -11,10 +11,18 @@ class GetVisitsActionTest extends ApiTestCase /** @test */ public function tryingToGetVisitsForInvalidUrlReturnsNotFoundError(): void { + $expectedDetail = 'No URL found with short code "invalid"'; + $resp = $this->callApiWithKey(self::METHOD_GET, '/short-urls/invalid/visits'); - ['error' => $error] = $this->getJsonResponsePayload($resp); + $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); - $this->assertEquals('INVALID_SHORTCODE', $error); + $this->assertEquals(self::STATUS_NOT_FOUND, $payload['status']); + $this->assertEquals('INVALID_SHORTCODE', $payload['type']); + $this->assertEquals('INVALID_SHORTCODE', $payload['error']); // Deprecated + $this->assertEquals($expectedDetail, $payload['detail']); + $this->assertEquals($expectedDetail, $payload['message']); // Deprecated + $this->assertEquals('Short URL not found', $payload['title']); + $this->assertEquals('invalid', $payload['shortCode']); } } diff --git a/module/Rest/test-api/Action/ResolveShortUrlActionTest.php b/module/Rest/test-api/Action/ResolveShortUrlActionTest.php index 46f07d53..e4d45f4a 100644 --- a/module/Rest/test-api/Action/ResolveShortUrlActionTest.php +++ b/module/Rest/test-api/Action/ResolveShortUrlActionTest.php @@ -11,10 +11,18 @@ class ResolveShortUrlActionTest extends ApiTestCase /** @test */ public function tryingToResolveInvalidUrlReturnsNotFoundError(): void { + $expectedDetail = 'No URL found with short code "invalid"'; + $resp = $this->callApiWithKey(self::METHOD_GET, '/short-urls/invalid'); - ['error' => $error] = $this->getJsonResponsePayload($resp); + $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); - $this->assertEquals('INVALID_SHORTCODE', $error); + $this->assertEquals(self::STATUS_NOT_FOUND, $payload['status']); + $this->assertEquals('INVALID_SHORTCODE', $payload['type']); + $this->assertEquals('INVALID_SHORTCODE', $payload['error']); // Deprecated + $this->assertEquals($expectedDetail, $payload['detail']); + $this->assertEquals($expectedDetail, $payload['message']); // Deprecated + $this->assertEquals('Short URL not found', $payload['title']); + $this->assertEquals('invalid', $payload['shortCode']); } } diff --git a/module/Rest/test-api/Action/UpdateTagActionTest.php b/module/Rest/test-api/Action/UpdateTagActionTest.php index 12f37296..8c14774c 100644 --- a/module/Rest/test-api/Action/UpdateTagActionTest.php +++ b/module/Rest/test-api/Action/UpdateTagActionTest.php @@ -15,11 +15,18 @@ class UpdateTagActionTest extends ApiTestCase */ public function notProvidingTagsReturnsBadRequest(array $body): void { + $expectedDetail = 'Provided data is not valid'; + $resp = $this->callApiWithKey(self::METHOD_PUT, '/tags', [RequestOptions::JSON => $body]); - ['error' => $error] = $this->getJsonResponsePayload($resp); + $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_BAD_REQUEST, $resp->getStatusCode()); - $this->assertEquals('INVALID_ARGUMENT', $error); + $this->assertEquals(self::STATUS_BAD_REQUEST, $payload['status']); + $this->assertEquals('INVALID_ARGUMENT', $payload['type']); + $this->assertEquals('INVALID_ARGUMENT', $payload['error']); // Deprecated + $this->assertEquals($expectedDetail, $payload['detail']); + $this->assertEquals($expectedDetail, $payload['message']); // Deprecated + $this->assertEquals('Invalid data', $payload['title']); } public function provideInvalidBody(): iterable @@ -32,13 +39,20 @@ class UpdateTagActionTest extends ApiTestCase /** @test */ public function tryingToRenameInvalidTagReturnsNotFound(): void { + $expectedDetail = 'Tag with name "invalid_tag" could not be found'; + $resp = $this->callApiWithKey(self::METHOD_PUT, '/tags', [RequestOptions::JSON => [ 'oldName' => 'invalid_tag', 'newName' => 'foo', ]]); - ['error' => $error] = $this->getJsonResponsePayload($resp); + $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); - $this->assertEquals('TAG_NOT_FOUND', $error); + $this->assertEquals(self::STATUS_NOT_FOUND, $payload['status']); + $this->assertEquals('TAG_NOT_FOUND', $payload['type']); + $this->assertEquals('TAG_NOT_FOUND', $payload['error']); // Deprecated + $this->assertEquals($expectedDetail, $payload['detail']); + $this->assertEquals($expectedDetail, $payload['message']); // Deprecated + $this->assertEquals('Tag not found', $payload['title']); } } diff --git a/module/Rest/test-api/Middleware/AuthenticationTest.php b/module/Rest/test-api/Middleware/AuthenticationTest.php index b918a369..3526c5f5 100644 --- a/module/Rest/test-api/Middleware/AuthenticationTest.php +++ b/module/Rest/test-api/Middleware/AuthenticationTest.php @@ -16,18 +16,21 @@ class AuthenticationTest extends ApiTestCase /** @test */ public function authorizationErrorIsReturnedIfNoApiKeyIsSent(): void { + $expectedDetail = sprintf( + 'Expected one of the following authentication headers, but none were provided, ["%s"]', + implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS) + ); + $resp = $this->callApi(self::METHOD_GET, '/short-codes'); - ['error' => $error, 'message' => $message] = $this->getJsonResponsePayload($resp); + $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_UNAUTHORIZED, $resp->getStatusCode()); - $this->assertEquals('INVALID_AUTHORIZATION', $error); - $this->assertEquals( - sprintf( - 'Expected one of the following authentication headers, but none were provided, ["%s"]', - implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS) - ), - $message - ); + $this->assertEquals(self::STATUS_UNAUTHORIZED, $payload['status']); + $this->assertEquals('INVALID_AUTHORIZATION', $payload['type']); + $this->assertEquals('INVALID_AUTHORIZATION', $payload['error']); // Deprecated + $this->assertEquals($expectedDetail, $payload['detail']); + $this->assertEquals($expectedDetail, $payload['message']); // Deprecated + $this->assertEquals('Invalid authorization', $payload['title']); } /** @@ -36,16 +39,22 @@ class AuthenticationTest extends ApiTestCase */ public function apiKeyErrorIsReturnedWhenProvidedApiKeyIsInvalid(string $apiKey): void { + $expectedDetail = 'Provided API key does not exist or is invalid.'; + $resp = $this->callApi(self::METHOD_GET, '/short-codes', [ 'headers' => [ Plugin\ApiKeyHeaderPlugin::HEADER_NAME => $apiKey, ], ]); - ['error' => $error, 'message' => $message] = $this->getJsonResponsePayload($resp); + $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_UNAUTHORIZED, $resp->getStatusCode()); - $this->assertEquals('INVALID_API_KEY', $error); - $this->assertEquals('Provided API key does not exist or is invalid.', $message); + $this->assertEquals(self::STATUS_UNAUTHORIZED, $payload['status']); + $this->assertEquals('INVALID_API_KEY', $payload['type']); + $this->assertEquals('INVALID_API_KEY', $payload['error']); // Deprecated + $this->assertEquals($expectedDetail, $payload['detail']); + $this->assertEquals($expectedDetail, $payload['message']); // Deprecated + $this->assertEquals('Invalid API key', $payload['title']); } public function provideInvalidApiKeys(): iterable @@ -61,19 +70,24 @@ class AuthenticationTest extends ApiTestCase */ public function authorizationErrorIsReturnedIfInvalidDataIsProvided( string $authValue, - string $expectedMessage, - string $expectedError + string $expectedDetail, + string $expectedType, + string $expectedTitle ): void { $resp = $this->callApi(self::METHOD_GET, '/short-codes', [ 'headers' => [ Plugin\AuthorizationHeaderPlugin::HEADER_NAME => $authValue, ], ]); - ['error' => $error, 'message' => $message] = $this->getJsonResponsePayload($resp); + $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_UNAUTHORIZED, $resp->getStatusCode()); - $this->assertEquals($expectedError, $error); - $this->assertEquals($expectedMessage, $message); + $this->assertEquals(self::STATUS_UNAUTHORIZED, $payload['status']); + $this->assertEquals($expectedType, $payload['type']); + $this->assertEquals($expectedType, $payload['error']); // Deprecated + $this->assertEquals($expectedDetail, $payload['detail']); + $this->assertEquals($expectedDetail, $payload['message']); // Deprecated + $this->assertEquals($expectedTitle, $payload['title']); } public function provideInvalidAuthorizations(): iterable @@ -82,17 +96,20 @@ class AuthenticationTest extends ApiTestCase 'invalid', 'You need to provide the Bearer type in the Authorization header.', 'INVALID_AUTHORIZATION', + 'Invalid authorization', ]; yield 'invalid type' => [ 'Basic invalid', 'Provided authorization type Basic is not supported. Use Bearer instead.', 'INVALID_AUTHORIZATION', + 'Invalid authorization', ]; yield 'invalid JWT' => [ 'Bearer invalid', 'Missing or invalid auth token provided. Perform a new authentication request and send provided ' . 'token on every new request on the Authorization header', 'INVALID_AUTH_TOKEN', + 'Invalid auth token', ]; } }