diff --git a/bin/test/run-api-tests.sh b/bin/test/run-api-tests.sh index 75a51387..cbc10a1a 100755 --- a/bin/test/run-api-tests.sh +++ b/bin/test/run-api-tests.sh @@ -9,7 +9,7 @@ echo 'Starting server...' vendor/bin/zend-expressive-swoole start -d sleep 2 -vendor/bin/phpunit --order-by=random -c phpunit-api.xml --testdox --colors=always +vendor/bin/phpunit --order-by=random -c phpunit-api.xml --testdox --colors=always $* testsExitCode=$? vendor/bin/zend-expressive-swoole stop diff --git a/composer.json b/composer.json index 1c3ea632..5d035b60 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,6 @@ "php": "^7.2", "ext-json": "*", "ext-pdo": "*", - "acelaya/ze-content-based-error-handler": "^3.0", "akrabat/ip-address-middleware": "^1.0", "cakephp/chronos": "^1.2", "cocur/slugify": "^3.0", @@ -34,7 +33,7 @@ "phly/phly-event-dispatcher": "^1.0", "predis/predis": "^1.1", "pugx/shortid-php": "^0.5", - "shlinkio/shlink-common": "^2.2.1", + "shlinkio/shlink-common": "^2.3", "shlinkio/shlink-event-dispatcher": "^1.0", "shlinkio/shlink-installer": "^3.1", "shlinkio/shlink-ip-geolocation": "^1.1", @@ -53,20 +52,20 @@ "zendframework/zend-expressive-swoole": "^2.4", "zendframework/zend-inputfilter": "^2.10", "zendframework/zend-paginator": "^2.8", + "zendframework/zend-problem-details": "^1.0", "zendframework/zend-servicemanager": "^3.4", "zendframework/zend-stdlib": "^3.2" }, "require-dev": { "devster/ubench": "^2.0", "eaglewu/swoole-ide-helper": "dev-master", - "filp/whoops": "^2.4", "infection/infection": "^0.14.2", "phpstan/phpstan": "^0.11.16", "phpunit/phpcov": "^6.0", "phpunit/phpunit": "^8.3", "roave/security-advisories": "dev-master", "shlinkio/php-coding-standard": "~2.0.0", - "shlinkio/shlink-test-utils": "^1.0", + "shlinkio/shlink-test-utils": "^1.1", "symfony/dotenv": "^4.3", "symfony/var-dumper": "^4.3", "zendframework/zend-component-installer": "^2.1", diff --git a/config/autoload/error-handler.global.php b/config/autoload/error-handler.global.php new file mode 100644 index 00000000..964b90de --- /dev/null +++ b/config/autoload/error-handler.global.php @@ -0,0 +1,34 @@ + [ + 'default_type_fallbacks' => [ + 404 => 'NOT_FOUND', + 500 => 'INTERNAL_SERVER_ERROR', + ], + 'json_flags' => JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE | JSON_PRESERVE_ZERO_FRACTION, + ], + + 'error_handler' => [ + 'listeners' => [Logger\ErrorLogger::class], + ], + + 'dependencies' => [ + 'delegators' => [ + ErrorHandler::class => [ + Logger\ErrorHandlerListenerAttachingDelegator::class, + ], + ProblemDetailsMiddleware::class => [ + Logger\ErrorHandlerListenerAttachingDelegator::class, + ], + ], + ], + +]; diff --git a/config/autoload/errorhandler.local.php.dist b/config/autoload/errorhandler.local.php.dist deleted file mode 100644 index 6dea98cb..00000000 --- a/config/autoload/errorhandler.local.php.dist +++ /dev/null @@ -1,29 +0,0 @@ - [ - 'invokables' => [ - 'Zend\Expressive\Whoops' => Whoops\Run::class, - 'Zend\Expressive\WhoopsPageHandler' => Whoops\Handler\PrettyPageHandler::class, - ], - ], - - 'whoops' => [ - 'json_exceptions' => [ - 'display' => true, - 'show_trace' => true, - 'ajax_only' => true, - ], - ], - - 'error_handler' => [ - 'plugins' => [ - 'factories' => [ - 'text/html' => WhoopsErrorResponseGeneratorFactory::class, - ], - ], - ], -]; diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index f9af01e3..35a9a16b 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -5,18 +5,31 @@ declare(strict_types=1); namespace Shlinkio\Shlink; use Zend\Expressive; +use Zend\ProblemDetails; use Zend\Stratigility\Middleware\ErrorHandler; return [ 'middleware_pipeline' => [ + 'error-handler' => [ + 'middleware' => [ + Expressive\Helper\ContentLengthMiddleware::class, + ErrorHandler::class, + ], + ], + 'error-handler-rest' => [ + 'path' => '/rest', + 'middleware' => [ + Rest\Middleware\CrossDomainMiddleware::class, + Rest\Middleware\BackwardsCompatibleProblemDetailsMiddleware::class, + ProblemDetails\ProblemDetailsMiddleware::class, + ], + ], + 'pre-routing' => [ 'middleware' => [ - ErrorHandler::class, - Expressive\Helper\ContentLengthMiddleware::class, Common\Middleware\CloseDbConnectionMiddleware::class, ], - 'priority' => 12, ], 'pre-routing-rest' => [ 'path' => '/rest', @@ -24,33 +37,40 @@ return [ Rest\Middleware\PathVersionMiddleware::class, Rest\Middleware\ShortUrl\ShortCodePathMiddleware::class, ], - 'priority' => 11, ], 'routing' => [ 'middleware' => [ Expressive\Router\Middleware\RouteMiddleware::class, ], - 'priority' => 10, ], 'rest' => [ 'path' => '/rest', 'middleware' => [ - Rest\Middleware\CrossDomainMiddleware::class, Expressive\Router\Middleware\ImplicitOptionsMiddleware::class, Rest\Middleware\BodyParserMiddleware::class, Rest\Middleware\AuthenticationMiddleware::class, ], - 'priority' => 5, ], - 'post-routing' => [ + 'dispatch' => [ 'middleware' => [ Expressive\Router\Middleware\DispatchMiddleware::class, - Core\Response\NotFoundHandler::class, ], - 'priority' => 1, + ], + + 'not-found-rest' => [ + 'path' => '/rest', + 'middleware' => [ + ProblemDetails\ProblemDetailsNotFoundHandler::class, + ], + ], + 'not-found' => [ + 'middleware' => [ + Core\ErrorHandler\NotFoundRedirectHandler::class, + Core\ErrorHandler\NotFoundTemplateHandler::class, + ], ], ], ]; diff --git a/config/config.php b/config/config.php index 352a27ca..ad18fd20 100644 --- a/config/config.php +++ b/config/config.php @@ -4,9 +4,9 @@ declare(strict_types=1); namespace Shlinkio\Shlink; -use Acelaya\ExpressiveErrorHandler; use Zend\ConfigAggregator; use Zend\Expressive; +use Zend\ProblemDetails; use function Shlinkio\Shlink\Common\env; @@ -16,7 +16,7 @@ return (new ConfigAggregator\ConfigAggregator([ Expressive\Router\FastRouteRouter\ConfigProvider::class, Expressive\Plates\ConfigProvider::class, Expressive\Swoole\ConfigProvider::class, - ExpressiveErrorHandler\ConfigProvider::class, + ProblemDetails\ConfigProvider::class, Common\ConfigProvider::class, IpGeolocation\ConfigProvider::class, Core\ConfigProvider::class, diff --git a/docs/swagger/definitions/Error.json b/docs/swagger/definitions/Error.json index 3585227d..006fa47a 100644 --- a/docs/swagger/definitions/Error.json +++ b/docs/swagger/definitions/Error.json @@ -1,13 +1,32 @@ { "type": "object", + "required": ["type", "title", "detail", "status"], "properties": { - "code": { + "type": { "type": "string", "description": "A machine unique code" }, + "title": { + "type": "string", + "description": "A unique title" + }, + "detail": { + "type": "string", + "description": "A human-friendly error description" + }, + "status": { + "type": "number", + "description": "HTTP response status code" + }, + "code": { + "type": "string", + "description": "**[Deprecated] Use type instead. Not returned for v2 of the REST API** A machine unique code", + "deprecated": true + }, "message": { "type": "string", - "description": "A human-friendly error message" + "description": "**[Deprecated] Use detail instead. Not returned for v2 of the REST API** A human-friendly error message", + "deprecated": true } } } diff --git a/docs/swagger/parameters/version.json b/docs/swagger/parameters/version.json new file mode 100644 index 00000000..c2b1cc1a --- /dev/null +++ b/docs/swagger/parameters/version.json @@ -0,0 +1,13 @@ +{ + "name": "version", + "description": "The API version to be consumed", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": [ + "2", + "1" + ] + } +} diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index f8fa1ec8..d0aebfec 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -7,6 +7,9 @@ "summary": "List short URLs", "description": "Returns the list of short URLs.

**Important note**: Before shlink v1.13, this endpoint used to use the `/short-codes` path instead of `/short-urls`. Both of them will keep working, while the first one is considered deprecated.", "parameters": [ + { + "$ref": "../parameters/version.json" + }, { "name": "page", "in": "query", @@ -150,7 +153,7 @@ "500": { "description": "Unexpected error.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -175,6 +178,11 @@ "Bearer": [] } ], + "parameters": [ + { + "$ref": "../parameters/version.json" + } + ], "requestBody": { "description": "Request body.", "required": true, @@ -256,11 +264,43 @@ } }, "400": { - "description": "The long URL was not provided or is invalid.", + "description": "Some of provided data is invalid. Check extra fields to know exactly what.", "content": { - "application/json": { + "application/problem+json": { "schema": { - "$ref": "../definitions/Error.json" + "type": "object", + "allOf": [ + { + "$ref": "../definitions/Error.json" + }, + { + "type": "object", + "properties": { + "invalidElements": { + "type": "array", + "items": { + "type": "string", + "enum": [ + "validSince", + "validUntil", + "customSlug", + "maxVisits", + "findIfExists", + "domain" + ] + } + }, + "url": { + "type": "string", + "description": "A URL that could not be verified, if the error type is INVALID_URL" + }, + "customSlug": { + "type": "string", + "description": "Provided custom slug when the error type is INVALID_SLUG" + } + } + } + ] } } } @@ -268,7 +308,7 @@ "500": { "description": "Unexpected error.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } diff --git a/docs/swagger/paths/v1_short-urls_shorten.json b/docs/swagger/paths/v1_short-urls_shorten.json index 803d77d5..49233c49 100644 --- a/docs/swagger/paths/v1_short-urls_shorten.json +++ b/docs/swagger/paths/v1_short-urls_shorten.json @@ -7,6 +7,9 @@ "summary": "Create a short URL", "description": "Creates a short URL in a single API call. Useful for third party integrations.

**Important note**: Before shlink v1.13, this endpoint used to use the `/short-codes` path instead of `/short-urls`. Both of them will keep working, while the first one is considered deprecated.", "parameters": [ + { + "$ref": "../parameters/version.json" + }, { "name": "apiKey", "in": "query", @@ -77,7 +80,7 @@ "400": { "description": "The long URL was not provided or is invalid.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -89,9 +92,12 @@ } }, "examples": { - "application/json": { - "error": "INVALID_URL", - "message": "Provided URL foo is invalid. Try with a different one." + "application/problem+json": { + "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": "INVALID_URL" } @@ -99,7 +105,7 @@ "500": { "description": "Unexpected error.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -111,11 +117,11 @@ } }, "examples": { - "application/json": { - "error": "UNKNOWN_ERROR", + "application/problem+json": { + "error": "INTERNAL_SERVER_ERROR", "message": "Unexpected error occurred" }, - "text/plain": "UNKNOWN_ERROR" + "text/plain": "INTERNAL_SERVER_ERROR" } } } diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}.json b/docs/swagger/paths/v1_short-urls_{shortCode}.json index bbb7145c..95532868 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}.json @@ -7,6 +7,9 @@ "summary": "Parse short code", "description": "Get the long URL behind a short URL's short code.

**Important note**: Before shlink v1.13, this endpoint used to use the `/short-codes` path instead of `/short-urls`. Both of them will keep working, while the first one is considered deprecated.", "parameters": [ + { + "$ref": "../parameters/version.json" + }, { "name": "shortCode", "in": "path", @@ -62,20 +65,10 @@ } } }, - "400": { - "description": "Provided shortCode does not match the character set currently used by the app to generate short codes.", - "content": { - "application/json": { - "schema": { - "$ref": "../definitions/Error.json" - } - } - } - }, "404": { "description": "No URL was found for provided short code.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -85,7 +78,7 @@ "500": { "description": "Unexpected error.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -103,6 +96,9 @@ "summary": "Edit short URL", "description": "Update certain meta arguments from an existing short URL.

**Important note**: Before shlink v1.13, this endpoint used to use the `/short-codes` path instead of `/short-urls`. Both of them will keep working, while the first one is considered deprecated.", "parameters": [ + { + "$ref": "../parameters/version.json" + }, { "name": "shortCode", "in": "path", @@ -153,9 +149,31 @@ "400": { "description": "Provided meta arguments are invalid.", "content": { - "application/json": { + "application/problem+json": { "schema": { - "$ref": "../definitions/Error.json" + "type": "object", + "allOf": [ + { + "$ref": "../definitions/Error.json" + }, + { + "type": "object", + "required": ["invalidElements"], + "properties": { + "invalidElements": { + "type": "array", + "items": { + "type": "string", + "enum": [ + "validSince", + "validUntil", + "maxVisits" + ] + } + } + } + } + ] } } } @@ -163,7 +181,7 @@ "404": { "description": "No short URL was found for provided short code.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -173,7 +191,7 @@ "500": { "description": "Unexpected error.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -192,6 +210,9 @@ "summary": "[DEPRECATED] Edit short URL", "description": "**[DEPRECATED]** Use [editShortUrl](#/Short_URLs/getShortUrl) instead", "parameters": [ + { + "$ref": "../parameters/version.json" + }, { "name": "shortCode", "in": "path", @@ -242,7 +263,7 @@ "400": { "description": "Provided meta arguments are invalid.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -252,7 +273,7 @@ "404": { "description": "No short URL was found for provided short code.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -262,7 +283,7 @@ "500": { "description": "Unexpected error.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -280,6 +301,9 @@ "summary": "Delete short URL", "description": "Deletes the short URL for provided short code.

**Important note**: Before shlink v1.13, this endpoint used to use the `/short-codes` path instead of `/short-urls`. Both of them will keep working, while the first one is considered deprecated.", "parameters": [ + { + "$ref": "../parameters/version.json" + }, { "name": "shortCode", "in": "path", @@ -302,26 +326,28 @@ "204": { "description": "The short URL has been properly deleted." }, - "400": { + "422": { "description": "The visits threshold in shlink does not allow this short URL to be deleted.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } } }, "examples": { - "application/json": { - "error": "INVALID_SHORTCODE_DELETION", - "message": "It is not possible to delete URL with short code \"abc123\" because it has reached more than \"15\" visits." + "application/problem+json": { + "title": "Cannot delete short URL", + "type": "INVALID_SHORTCODE_DELETION", + "detail": "It is not possible to delete URL with short code \"abc123\" because it has reached more than \"15\" visits.", + "status": 422 } } }, "404": { "description": "No short URL was found for provided short code.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -331,7 +357,7 @@ "500": { "description": "Unexpected error.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json b/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json index ab05d230..45142e62 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json @@ -7,6 +7,9 @@ "summary": "Edit tags on short URL", "description": "Edit the tags on URL identified by provided short code.

**Important note**: Before shlink v1.13, this endpoint used to use the `/short-codes` path instead of `/short-urls`. Both of them will keep working, while the first one is considered deprecated.", "parameters": [ + { + "$ref": "../parameters/version.json" + }, { "name": "shortCode", "in": "path", @@ -78,7 +81,7 @@ "400": { "description": "The request body does not contain a \"tags\" param with array type.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json b/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json index a3cf5f10..508449de 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json @@ -7,6 +7,9 @@ "summary": "List visits for short URL", "description": "Get the list of visits on the short URL behind provided short code.

**Important note**: Before shlink v1.13, this endpoint used to use the `/short-codes` path instead of `/short-urls`. Both of them will keep working, while the first one is considered deprecated.", "parameters": [ + { + "$ref": "../parameters/version.json" + }, { "name": "shortCode", "in": "path", @@ -132,7 +135,7 @@ "404": { "description": "The short code does not belong to any short URL.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -142,7 +145,7 @@ "500": { "description": "Unexpected error.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } diff --git a/docs/swagger/paths/v1_tags.json b/docs/swagger/paths/v1_tags.json index 5bf260bb..da0159bd 100644 --- a/docs/swagger/paths/v1_tags.json +++ b/docs/swagger/paths/v1_tags.json @@ -14,6 +14,11 @@ "Bearer": [] } ], + "parameters": [ + { + "$ref": "../parameters/version.json" + } + ], "responses": { "200": { "description": "The list of tags", @@ -53,7 +58,7 @@ "500": { "description": "Unexpected error.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -78,6 +83,11 @@ "Bearer": [] } ], + "parameters": [ + { + "$ref": "../parameters/version.json" + } + ], "requestBody": { "description": "Request body.", "required": true, @@ -140,7 +150,7 @@ "500": { "description": "Unexpected error.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -165,6 +175,11 @@ "Bearer": [] } ], + "parameters": [ + { + "$ref": "../parameters/version.json" + } + ], "requestBody": { "description": "Request body.", "required": true, @@ -197,7 +212,7 @@ "400": { "description": "You have not provided either the oldName or the newName params.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -207,7 +222,7 @@ "404": { "description": "There's no tag found with the name provided in oldName param.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -217,7 +232,7 @@ "500": { "description": "Unexpected error.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } @@ -235,6 +250,9 @@ "summary": "Delete tags", "description": "Deletes provided list of tags", "parameters": [ + { + "$ref": "../parameters/version.json" + }, { "name": "tags[]", "in": "query", @@ -263,7 +281,7 @@ "500": { "description": "Unexpected error.", "content": { - "application/json": { + "application/problem+json": { "schema": { "$ref": "../definitions/Error.json" } diff --git a/docs/swagger/paths/{shortCode}.json b/docs/swagger/paths/{shortCode}.json index eccd5ba1..bbebacbd 100644 --- a/docs/swagger/paths/{shortCode}.json +++ b/docs/swagger/paths/{shortCode}.json @@ -20,16 +20,6 @@ "responses": { "302": { "description": "Visit properly tracked and redirected" - }, - "500": { - "description": "Unexpected error.", - "content": { - "application/json": { - "schema": { - "$ref": "../definitions/Error.json" - } - } - } } } } diff --git a/docs/swagger/paths/{shortCode}_preview.json b/docs/swagger/paths/{shortCode}_preview.json index f6168b4d..98df559c 100644 --- a/docs/swagger/paths/{shortCode}_preview.json +++ b/docs/swagger/paths/{shortCode}_preview.json @@ -29,16 +29,6 @@ } } } - }, - "500": { - "description": "Unexpected error.", - "content": { - "application/json": { - "schema": { - "$ref": "../definitions/Error.json" - } - } - } } } } diff --git a/docs/swagger/paths/{shortCode}_qr-code.json b/docs/swagger/paths/{shortCode}_qr-code.json index 14a8fddc..300a7429 100644 --- a/docs/swagger/paths/{shortCode}_qr-code.json +++ b/docs/swagger/paths/{shortCode}_qr-code.json @@ -40,16 +40,6 @@ } } } - }, - "500": { - "description": "Unexpected error.", - "content": { - "application/json": { - "schema": { - "$ref": "../definitions/Error.json" - } - } - } } } } diff --git a/docs/swagger/paths/{shortCode}_track.json b/docs/swagger/paths/{shortCode}_track.json index b4b62bba..50f6bc5e 100644 --- a/docs/swagger/paths/{shortCode}_track.json +++ b/docs/swagger/paths/{shortCode}_track.json @@ -28,16 +28,6 @@ } } } - }, - "500": { - "description": "Unexpected error.", - "content": { - "application/json": { - "schema": { - "$ref": "../definitions/Error.json" - } - } - } } } } diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json index 6dd3ff8f..d924b8a0 100644 --- a/docs/swagger/swagger.json +++ b/docs/swagger/swagger.json @@ -71,24 +71,24 @@ ], "paths": { - "/rest/v1/short-urls": { + "/rest/v{version}/short-urls": { "$ref": "paths/v1_short-urls.json" }, - "/rest/v1/short-urls/shorten": { + "/rest/v{version}/short-urls/shorten": { "$ref": "paths/v1_short-urls_shorten.json" }, - "/rest/v1/short-urls/{shortCode}": { + "/rest/v{version}/short-urls/{shortCode}": { "$ref": "paths/v1_short-urls_{shortCode}.json" }, - "/rest/v1/short-urls/{shortCode}/tags": { + "/rest/v{version}/short-urls/{shortCode}/tags": { "$ref": "paths/v1_short-urls_{shortCode}_tags.json" }, - "/rest/v1/tags": { + "/rest/v{version}/tags": { "$ref": "paths/v1_tags.json" }, - "/rest/v1/short-urls/{shortCode}/visits": { + "/rest/v{version}/short-urls/{shortCode}/visits": { "$ref": "paths/v1_short-urls_{shortCode}_visits.json" }, diff --git a/module/CLI/src/Command/Api/DisableKeyCommand.php b/module/CLI/src/Command/Api/DisableKeyCommand.php index e0f2b79c..f3eb7f6b 100644 --- a/module/CLI/src/Command/Api/DisableKeyCommand.php +++ b/module/CLI/src/Command/Api/DisableKeyCommand.php @@ -4,8 +4,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Api; -use InvalidArgumentException; use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -45,7 +45,7 @@ class DisableKeyCommand extends Command $io->success(sprintf('API key "%s" properly disabled', $apiKey)); return ExitCodes::EXIT_SUCCESS; } catch (InvalidArgumentException $e) { - $io->error(sprintf('API key "%s" does not exist.', $apiKey)); + $io->error($e->getMessage()); return ExitCodes::EXIT_FAILURE; } } diff --git a/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php index 683fd893..c2e81d0d 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php @@ -55,22 +55,17 @@ class DeleteShortUrlCommand extends Command try { $this->runDelete($io, $shortCode, $ignoreThreshold); return ExitCodes::EXIT_SUCCESS; - } catch (Exception\InvalidShortCodeException $e) { - $io->error(sprintf('Provided short code "%s" could not be found.', $shortCode)); + } catch (Exception\ShortUrlNotFoundException $e) { + $io->error($e->getMessage()); return ExitCodes::EXIT_FAILURE; } catch (Exception\DeleteShortUrlException $e) { - return $this->retry($io, $shortCode, $e); + return $this->retry($io, $shortCode, $e->getMessage()); } } - private function retry(SymfonyStyle $io, string $shortCode, Exception\DeleteShortUrlException $e): int + private function retry(SymfonyStyle $io, string $shortCode, string $warningMsg): int { - $warningMsg = sprintf( - 'It was not possible to delete the short URL with short code "%s" because it has more than %s visits.', - $shortCode, - $e->getVisitsThreshold() - ); - $io->writeln('' . $warningMsg . ''); + $io->writeln(sprintf('%s', $warningMsg)); $forceDelete = $io->confirm('Do you want to delete it anyway?', false); if ($forceDelete) { diff --git a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php index 9d9c464b..5cce7030 100644 --- a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php @@ -141,13 +141,8 @@ class GenerateShortUrlCommand extends Command sprintf('Generated short URL: %s', $shortUrl->toString($this->domainConfig)), ]); return ExitCodes::EXIT_SUCCESS; - } catch (InvalidUrlException $e) { - $io->error(sprintf('Provided URL "%s" is invalid. Try with a different one.', $longUrl)); - return ExitCodes::EXIT_FAILURE; - } catch (NonUniqueSlugException $e) { - $io->error( - sprintf('Provided slug "%s" is already in use by another URL. Try with a different one.', $customSlug) - ); + } catch (InvalidUrlException | NonUniqueSlugException $e) { + $io->error($e->getMessage()); return ExitCodes::EXIT_FAILURE; } } diff --git a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php index bc159a79..e8db28e2 100644 --- a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php @@ -5,8 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Util\ExitCodes; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -65,11 +64,8 @@ class ResolveUrlCommand extends Command $url = $this->urlShortener->shortCodeToUrl($shortCode, $domain); $output->writeln(sprintf('Long URL: %s', $url->getLongUrl())); return ExitCodes::EXIT_SUCCESS; - } catch (InvalidShortCodeException $e) { - $io->error(sprintf('Provided short code "%s" has an invalid format.', $shortCode)); - return ExitCodes::EXIT_FAILURE; - } catch (EntityDoesNotExistException $e) { - $io->error(sprintf('Provided short code "%s" could not be found.', $shortCode)); + } catch (ShortUrlNotFoundException $e) { + $io->error($e->getMessage()); return ExitCodes::EXIT_FAILURE; } } diff --git a/module/CLI/src/Command/Tag/RenameTagCommand.php b/module/CLI/src/Command/Tag/RenameTagCommand.php index 3e9f021b..b3a21a2e 100644 --- a/module/CLI/src/Command/Tag/RenameTagCommand.php +++ b/module/CLI/src/Command/Tag/RenameTagCommand.php @@ -5,7 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Tag; use Shlinkio\Shlink\CLI\Util\ExitCodes; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; +use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -13,8 +13,6 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; -use function sprintf; - class RenameTagCommand extends Command { public const NAME = 'tag:rename'; @@ -47,8 +45,8 @@ class RenameTagCommand extends Command $this->tagService->renameTag($oldName, $newName); $io->success('Tag properly renamed.'); return ExitCodes::EXIT_SUCCESS; - } catch (EntityDoesNotExistException $e) { - $io->error(sprintf('A tag with name "%s" was not found', $oldName)); + } catch (TagNotFoundException $e) { + $io->error($e->getMessage()); return ExitCodes::EXIT_FAILURE; } } diff --git a/module/CLI/src/Exception/GeolocationDbUpdateFailedException.php b/module/CLI/src/Exception/GeolocationDbUpdateFailedException.php index fcb680d5..38bb4c5f 100644 --- a/module/CLI/src/Exception/GeolocationDbUpdateFailedException.php +++ b/module/CLI/src/Exception/GeolocationDbUpdateFailedException.php @@ -12,20 +12,16 @@ class GeolocationDbUpdateFailedException extends RuntimeException implements Exc /** @var bool */ private $olderDbExists; - public function __construct(bool $olderDbExists, string $message = '', int $code = 0, ?Throwable $previous = null) - { - $this->olderDbExists = $olderDbExists; - parent::__construct($message, $code, $previous); - } - public static function create(bool $olderDbExists, ?Throwable $prev = null): self { - return new self( - $olderDbExists, + $e = new self( 'An error occurred while updating geolocation database, and an older version could not be found', 0, $prev ); + $e->olderDbExists = $olderDbExists; + + return $e; } public function olderDbExists(): bool diff --git a/module/CLI/src/Util/GeolocationDbUpdater.php b/module/CLI/src/Util/GeolocationDbUpdater.php index ebff82aa..2e530a34 100644 --- a/module/CLI/src/Util/GeolocationDbUpdater.php +++ b/module/CLI/src/Util/GeolocationDbUpdater.php @@ -10,7 +10,6 @@ use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface; use Symfony\Component\Lock\Factory as Locker; -use Throwable; class GeolocationDbUpdater implements GeolocationDbUpdaterInterface { @@ -40,8 +39,6 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface try { $this->downloadIfNeeded($mustBeUpdated, $handleProgress); - } catch (Throwable $e) { - throw $e; } finally { $lock->release(); } diff --git a/module/CLI/test/Command/Api/DisableKeyCommandTest.php b/module/CLI/test/Command/Api/DisableKeyCommandTest.php index ca17aa7a..37629091 100644 --- a/module/CLI/test/Command/Api/DisableKeyCommandTest.php +++ b/module/CLI/test/Command/Api/DisableKeyCommandTest.php @@ -29,7 +29,7 @@ class DisableKeyCommandTest extends TestCase } /** @test */ - public function providedApiKeyIsDisabled() + public function providedApiKeyIsDisabled(): void { $apiKey = 'abcd1234'; $this->apiKeyService->disable($apiKey)->shouldBeCalledOnce(); @@ -43,17 +43,18 @@ class DisableKeyCommandTest extends TestCase } /** @test */ - public function errorIsReturnedIfServiceThrowsException() + public function errorIsReturnedIfServiceThrowsException(): void { $apiKey = 'abcd1234'; - $disable = $this->apiKeyService->disable($apiKey)->willThrow(InvalidArgumentException::class); + $expectedMessage = 'API key "abcd1234" does not exist.'; + $disable = $this->apiKeyService->disable($apiKey)->willThrow(new InvalidArgumentException($expectedMessage)); $this->commandTester->execute([ 'apiKey' => $apiKey, ]); $output = $this->commandTester->getDisplay(); - $this->assertStringContainsString('API key "abcd1234" does not exist.', $output); + $this->assertStringContainsString($expectedMessage, $output); $disable->shouldHaveBeenCalledOnce(); } } diff --git a/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php index f0fad2cc..85521835 100644 --- a/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php @@ -58,13 +58,13 @@ class DeleteShortUrlCommandTest extends TestCase { $shortCode = 'abc123'; $deleteByShortCode = $this->service->deleteByShortCode($shortCode, false)->willThrow( - Exception\InvalidShortCodeException::class + Exception\ShortUrlNotFoundException::fromNotFoundShortCode($shortCode) ); $this->commandTester->execute(['shortCode' => $shortCode]); $output = $this->commandTester->getDisplay(); - $this->assertStringContainsString(sprintf('Provided short code "%s" could not be found.', $shortCode), $output); + $this->assertStringContainsString(sprintf('No URL found with short code "%s"', $shortCode), $output); $deleteByShortCode->shouldHaveBeenCalledOnce(); } @@ -79,11 +79,11 @@ class DeleteShortUrlCommandTest extends TestCase ): void { $shortCode = 'abc123'; $deleteByShortCode = $this->service->deleteByShortCode($shortCode, Argument::type('bool'))->will( - function (array $args) { + function (array $args) use ($shortCode) { $ignoreThreshold = array_pop($args); if (!$ignoreThreshold) { - throw new Exception\DeleteShortUrlException(10); + throw Exception\DeleteShortUrlException::fromVisitsThreshold(10, $shortCode); } } ); @@ -93,7 +93,7 @@ class DeleteShortUrlCommandTest extends TestCase $output = $this->commandTester->getDisplay(); $this->assertStringContainsString(sprintf( - 'It was not possible to delete the short URL with short code "%s" because it has more than 10 visits.', + 'Impossible to delete short URL with short code "%s" since it has more than "10" visits.', $shortCode ), $output); $this->assertStringContainsString($expectedMessage, $output); @@ -112,7 +112,7 @@ class DeleteShortUrlCommandTest extends TestCase { $shortCode = 'abc123'; $deleteByShortCode = $this->service->deleteByShortCode($shortCode, false)->willThrow( - new Exception\DeleteShortUrlException(10) + Exception\DeleteShortUrlException::fromVisitsThreshold(10, $shortCode) ); $this->commandTester->setInputs(['no']); @@ -120,7 +120,7 @@ class DeleteShortUrlCommandTest extends TestCase $output = $this->commandTester->getDisplay(); $this->assertStringContainsString(sprintf( - 'It was not possible to delete the short URL with short code "%s" because it has more than 10 visits.', + 'Impossible to delete short URL with short code "%s" since it has more than "10" visits.', $shortCode ), $output); $this->assertStringContainsString('Short URL was not deleted.', $output); diff --git a/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php index d83bd042..abae0fe6 100644 --- a/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php @@ -59,21 +59,22 @@ class GenerateShortUrlCommandTest extends TestCase /** @test */ public function exceptionWhileParsingLongUrlOutputsError(): void { - $this->urlShortener->urlToShortCode(Argument::cetera())->willThrow(new InvalidUrlException()) + $url = 'http://domain.com/invalid'; + $this->urlShortener->urlToShortCode(Argument::cetera())->willThrow(InvalidUrlException::fromUrl($url)) ->shouldBeCalledOnce(); - $this->commandTester->execute(['longUrl' => 'http://domain.com/invalid']); + $this->commandTester->execute(['longUrl' => $url]); $output = $this->commandTester->getDisplay(); $this->assertEquals(ExitCodes::EXIT_FAILURE, $this->commandTester->getStatusCode()); - $this->assertStringContainsString('Provided URL "http://domain.com/invalid" is invalid.', $output); + $this->assertStringContainsString('Provided URL http://domain.com/invalid is invalid.', $output); } /** @test */ public function providingNonUniqueSlugOutputsError(): void { $urlToShortCode = $this->urlShortener->urlToShortCode(Argument::cetera())->willThrow( - NonUniqueSlugException::class + NonUniqueSlugException::fromSlug('my-slug') ); $this->commandTester->execute(['longUrl' => 'http://domain.com/invalid', '--customSlug' => 'my-slug']); diff --git a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php index 283b3a4f..11b549e5 100644 --- a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php @@ -8,12 +8,13 @@ use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\ShortUrl\ResolveUrlCommand; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; +use function sprintf; + use const PHP_EOL; class ResolveUrlCommandTest extends TestCase @@ -51,23 +52,12 @@ class ResolveUrlCommandTest extends TestCase public function incorrectShortCodeOutputsErrorMessage(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(EntityDoesNotExistException::class) - ->shouldBeCalledOnce(); + $this->urlShortener->shortCodeToUrl($shortCode, null) + ->willThrow(ShortUrlNotFoundException::fromNotFoundShortCode($shortCode)) + ->shouldBeCalledOnce(); $this->commandTester->execute(['shortCode' => $shortCode]); $output = $this->commandTester->getDisplay(); - $this->assertStringContainsString('Provided short code "' . $shortCode . '" could not be found.', $output); - } - - /** @test */ - public function wrongShortCodeFormatOutputsErrorMessage(): void - { - $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(new InvalidShortCodeException()) - ->shouldBeCalledOnce(); - - $this->commandTester->execute(['shortCode' => $shortCode]); - $output = $this->commandTester->getDisplay(); - $this->assertStringContainsString('Provided short code "' . $shortCode . '" has an invalid format.', $output); + $this->assertStringContainsString(sprintf('No URL found with short code "%s"', $shortCode), $output); } } diff --git a/module/CLI/test/Command/Tag/RenameTagCommandTest.php b/module/CLI/test/Command/Tag/RenameTagCommandTest.php index 228d854d..c626e0c0 100644 --- a/module/CLI/test/Command/Tag/RenameTagCommandTest.php +++ b/module/CLI/test/Command/Tag/RenameTagCommandTest.php @@ -8,7 +8,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Tag\RenameTagCommand; use Shlinkio\Shlink\Core\Entity\Tag; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; +use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -34,11 +34,11 @@ class RenameTagCommandTest extends TestCase } /** @test */ - public function errorIsPrintedIfExceptionIsThrown() + public function errorIsPrintedIfExceptionIsThrown(): void { $oldName = 'foo'; $newName = 'bar'; - $renameTag = $this->tagService->renameTag($oldName, $newName)->willThrow(EntityDoesNotExistException::class); + $renameTag = $this->tagService->renameTag($oldName, $newName)->willThrow(TagNotFoundException::fromTag('foo')); $this->commandTester->execute([ 'oldName' => $oldName, @@ -46,12 +46,12 @@ class RenameTagCommandTest extends TestCase ]); $output = $this->commandTester->getDisplay(); - $this->assertStringContainsString('A tag with name "foo" was not found', $output); + $this->assertStringContainsString('Tag with name "foo" could not be found', $output); $renameTag->shouldHaveBeenCalled(); } /** @test */ - public function successIsPrintedIfNoErrorOccurs() + public function successIsPrintedIfNoErrorOccurs(): void { $oldName = 'foo'; $newName = 'bar'; diff --git a/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php b/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php index 51c87cb3..70a8cc6f 100644 --- a/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php +++ b/module/CLI/test/Exception/GeolocationDbUpdateFailedExceptionTest.php @@ -12,47 +12,6 @@ use Throwable; class GeolocationDbUpdateFailedExceptionTest extends TestCase { - /** - * @test - * @dataProvider provideOlderDbExists - */ - public function constructCreatesExceptionWithDefaultArgs(bool $olderDbExists): void - { - $e = new GeolocationDbUpdateFailedException($olderDbExists); - - $this->assertEquals($olderDbExists, $e->olderDbExists()); - $this->assertEquals('', $e->getMessage()); - $this->assertEquals(0, $e->getCode()); - $this->assertNull($e->getPrevious()); - } - - public function provideOlderDbExists(): iterable - { - yield 'with older DB' => [true]; - yield 'without older DB' => [false]; - } - - /** - * @test - * @dataProvider provideConstructorArgs - */ - public function constructCreatesException(bool $olderDbExists, string $message, int $code, ?Throwable $prev): void - { - $e = new GeolocationDbUpdateFailedException($olderDbExists, $message, $code, $prev); - - $this->assertEquals($olderDbExists, $e->olderDbExists()); - $this->assertEquals($message, $e->getMessage()); - $this->assertEquals($code, $e->getCode()); - $this->assertEquals($prev, $e->getPrevious()); - } - - public function provideConstructorArgs(): iterable - { - yield [true, 'This is a nice error message', 99, new Exception('prev')]; - yield [false, 'Another message', 0, new RuntimeException('prev')]; - yield [true, 'An yet another message', -50, null]; - } - /** * @test * @dataProvider provideCreateArgs diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 426d0969..0ebdae7d 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -6,8 +6,8 @@ namespace Shlinkio\Shlink\Core; use Doctrine\Common\Cache\Cache; use Psr\EventDispatcher\EventDispatcherInterface; +use Shlinkio\Shlink\Core\ErrorHandler; use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; -use Shlinkio\Shlink\Core\Response\NotFoundHandler; use Shlinkio\Shlink\PreviewGenerator\Service\PreviewGenerator; use Zend\Expressive\Router\RouterInterface; use Zend\Expressive\Template\TemplateRendererInterface; @@ -17,7 +17,8 @@ return [ 'dependencies' => [ 'factories' => [ - NotFoundHandler::class => ConfigAbstractFactory::class, + ErrorHandler\NotFoundRedirectHandler::class => ConfigAbstractFactory::class, + ErrorHandler\NotFoundTemplateHandler::class => ConfigAbstractFactory::class, Options\AppOptions::class => ConfigAbstractFactory::class, Options\DeleteShortUrlsOptions::class => ConfigAbstractFactory::class, @@ -43,11 +44,8 @@ return [ ], ConfigAbstractFactory::class => [ - NotFoundHandler::class => [ - TemplateRendererInterface::class, - NotFoundRedirectOptions::class, - 'config.router.base_path', - ], + ErrorHandler\NotFoundRedirectHandler::class => [NotFoundRedirectOptions::class, 'config.router.base_path'], + ErrorHandler\NotFoundTemplateHandler::class => [TemplateRendererInterface::class], Options\AppOptions::class => ['config.app_options'], Options\DeleteShortUrlsOptions::class => ['config.delete_short_urls'], diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 9bacaf39..ff8d91f2 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -11,8 +11,7 @@ use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; @@ -72,7 +71,7 @@ abstract class AbstractTrackingAction implements MiddlewareInterface } return $this->createSuccessResp($this->buildUrlToRedirectTo($url, $query, $disableTrackParam)); - } catch (InvalidShortCodeException | EntityDoesNotExistException $e) { + } catch (ShortUrlNotFoundException $e) { $this->logger->warning('An error occurred while tracking short code. {e}', ['e' => $e]); return $this->createErrorResp($request, $handler); } diff --git a/module/Core/src/Action/PreviewAction.php b/module/Core/src/Action/PreviewAction.php index 4ac2a50c..d243f12c 100644 --- a/module/Core/src/Action/PreviewAction.php +++ b/module/Core/src/Action/PreviewAction.php @@ -11,8 +11,7 @@ use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Shlinkio\Shlink\Common\Response\ResponseUtilsTrait; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\PreviewGenerator\Exception\PreviewGenerationException; use Shlinkio\Shlink\PreviewGenerator\Service\PreviewGeneratorInterface; @@ -56,7 +55,7 @@ class PreviewAction implements MiddlewareInterface $url = $this->urlShortener->shortCodeToUrl($shortCode); $imagePath = $this->previewGenerator->generatePreview($url->getLongUrl()); return $this->generateImageResponse($imagePath); - } catch (InvalidShortCodeException | EntityDoesNotExistException | PreviewGenerationException $e) { + } catch (ShortUrlNotFoundException | PreviewGenerationException $e) { $this->logger->warning('An error occurred while generating preview image. {e}', ['e' => $e]); return $handler->handle($request); } diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index a1fdae5b..3cdee70e 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -12,8 +12,7 @@ use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Shlinkio\Shlink\Common\Response\QrCodeResponse; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Zend\Expressive\Router\Exception\RuntimeException; use Zend\Expressive\Router\RouterInterface; @@ -60,7 +59,7 @@ class QrCodeAction implements MiddlewareInterface try { $this->urlShortener->shortCodeToUrl($shortCode, $domain); - } catch (InvalidShortCodeException | EntityDoesNotExistException $e) { + } catch (ShortUrlNotFoundException $e) { $this->logger->warning('An error occurred while creating QR code. {e}', ['e' => $e]); return $handler->handle($request); } diff --git a/module/Core/src/Response/NotFoundHandler.php b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php similarity index 50% rename from module/Core/src/Response/NotFoundHandler.php rename to module/Core/src/ErrorHandler/NotFoundRedirectHandler.php index 6bf245c1..f6a03395 100644 --- a/module/Core/src/Response/NotFoundHandler.php +++ b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php @@ -2,78 +2,40 @@ declare(strict_types=1); -namespace Shlinkio\Shlink\Core\Response; +namespace Shlinkio\Shlink\Core\ErrorHandler; -use Fig\Http\Message\StatusCodeInterface; -use InvalidArgumentException; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\UriInterface; +use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; use Zend\Diactoros\Response; use Zend\Expressive\Router\RouteResult; -use Zend\Expressive\Template\TemplateRendererInterface; -use function array_shift; -use function explode; -use function Functional\contains; use function rtrim; -class NotFoundHandler implements RequestHandlerInterface +class NotFoundRedirectHandler implements MiddlewareInterface { - public const NOT_FOUND_TEMPLATE = 'ShlinkCore::error/404'; - public const INVALID_SHORT_CODE_TEMPLATE = 'ShlinkCore::invalid-short-code'; - - /** @var TemplateRendererInterface */ - private $renderer; /** @var NotFoundRedirectOptions */ private $redirectOptions; /** @var string */ private $shlinkBasePath; - public function __construct( - TemplateRendererInterface $renderer, - NotFoundRedirectOptions $redirectOptions, - string $shlinkBasePath - ) { - $this->renderer = $renderer; + public function __construct(NotFoundRedirectOptions $redirectOptions, string $shlinkBasePath) + { $this->redirectOptions = $redirectOptions; $this->shlinkBasePath = $shlinkBasePath; } - /** - * Dispatch the next available middleware and return the response. - * - * @param ServerRequestInterface $request - * - * @return ResponseInterface - * @throws InvalidArgumentException - */ - public function handle(ServerRequestInterface $request): ResponseInterface + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { /** @var RouteResult $routeResult */ $routeResult = $request->getAttribute(RouteResult::class, RouteResult::fromRouteFailure(null)); $redirectResponse = $this->createRedirectResponse($routeResult, $request->getUri()); - if ($redirectResponse !== null) { - return $redirectResponse; - } - $accepts = explode(',', $request->getHeaderLine('Accept')); - $accept = array_shift($accepts); - $status = StatusCodeInterface::STATUS_NOT_FOUND; - - // If the first accepted type is json, return a json response - if (contains(['application/json', 'text/json', 'application/x-json'], $accept)) { - return new Response\JsonResponse([ - 'error' => 'NOT_FOUND', - 'message' => 'Not found', - ], $status); - } - - $template = $routeResult->isFailure() ? self::NOT_FOUND_TEMPLATE : self::INVALID_SHORT_CODE_TEMPLATE; - return new Response\HtmlResponse($this->renderer->render($template), $status); + return $redirectResponse ?? $handler->handle($request); } private function createRedirectResponse(RouteResult $routeResult, UriInterface $uri): ?ResponseInterface diff --git a/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php b/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php new file mode 100644 index 00000000..7b84043d --- /dev/null +++ b/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php @@ -0,0 +1,46 @@ +renderer = $renderer; + } + + /** + * Dispatch the next available middleware and return the response. + * + * @param ServerRequestInterface $request + * + * @return ResponseInterface + * @throws InvalidArgumentException + */ + public function handle(ServerRequestInterface $request): ResponseInterface + { + /** @var RouteResult $routeResult */ + $routeResult = $request->getAttribute(RouteResult::class, RouteResult::fromRouteFailure(null)); + $status = StatusCodeInterface::STATUS_NOT_FOUND; + + $template = $routeResult->isFailure() ? self::NOT_FOUND_TEMPLATE : self::INVALID_SHORT_CODE_TEMPLATE; + return new Response\HtmlResponse($this->renderer->render($template), $status); + } +} diff --git a/module/Core/src/Exception/DeleteShortUrlException.php b/module/Core/src/Exception/DeleteShortUrlException.php index c1fd6dd7..ddd77418 100644 --- a/module/Core/src/Exception/DeleteShortUrlException.php +++ b/module/Core/src/Exception/DeleteShortUrlException.php @@ -4,32 +4,41 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Exception; -use Throwable; +use Fig\Http\Message\StatusCodeInterface; +use Zend\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; +use Zend\ProblemDetails\Exception\ProblemDetailsExceptionInterface; use function sprintf; -class DeleteShortUrlException extends RuntimeException +class DeleteShortUrlException extends DomainException implements ProblemDetailsExceptionInterface { - /** @var int */ - private $visitsThreshold; + use CommonProblemDetailsExceptionTrait; - public function __construct(int $visitsThreshold, string $message = '', int $code = 0, ?Throwable $previous = null) - { - $this->visitsThreshold = $visitsThreshold; - parent::__construct($message, $code, $previous); - } + private const TITLE = 'Cannot delete short URL'; + private const TYPE = 'INVALID_SHORTCODE_DELETION'; // FIXME Should be INVALID_SHORT_URL_DELETION public static function fromVisitsThreshold(int $threshold, string $shortCode): self { - return new self($threshold, sprintf( + $e = new self(sprintf( 'Impossible to delete short URL with short code "%s" since it has more than "%s" visits.', $shortCode, $threshold )); + + $e->detail = $e->getMessage(); + $e->title = self::TITLE; + $e->type = self::TYPE; + $e->status = StatusCodeInterface::STATUS_UNPROCESSABLE_ENTITY; + $e->additional = [ + 'shortCode' => $shortCode, + 'threshold' => $threshold, + ]; + + return $e; } public function getVisitsThreshold(): int { - return $this->visitsThreshold; + return $this->additional['threshold']; } } diff --git a/module/Core/src/Exception/DomainException.php b/module/Core/src/Exception/DomainException.php new file mode 100644 index 00000000..63134952 --- /dev/null +++ b/module/Core/src/Exception/DomainException.php @@ -0,0 +1,11 @@ + $value) { - $result[] = sprintf('"%s" => "%s"', $key, $value); - } - - return implode(', ', $result); - } -} diff --git a/module/Core/src/Exception/InvalidShortCodeException.php b/module/Core/src/Exception/InvalidShortCodeException.php deleted file mode 100644 index 37ecfffb..00000000 --- a/module/Core/src/Exception/InvalidShortCodeException.php +++ /dev/null @@ -1,27 +0,0 @@ -getCode() : -1; - return new static( - sprintf('Provided short code "%s" does not match the char set "%s"', $shortCode, $charSet), - $code, - $previous - ); - } - - public static function fromNotFoundShortCode(string $shortCode): self - { - return new static(sprintf('Provided short code "%s" does not belong to a short URL', $shortCode)); - } -} diff --git a/module/Core/src/Exception/InvalidUrlException.php b/module/Core/src/Exception/InvalidUrlException.php index 5cd79af5..ffec94df 100644 --- a/module/Core/src/Exception/InvalidUrlException.php +++ b/module/Core/src/Exception/InvalidUrlException.php @@ -4,15 +4,31 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Exception; +use Fig\Http\Message\StatusCodeInterface; use Throwable; +use Zend\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; +use Zend\ProblemDetails\Exception\ProblemDetailsExceptionInterface; use function sprintf; -class InvalidUrlException extends RuntimeException +class InvalidUrlException extends DomainException implements ProblemDetailsExceptionInterface { + use CommonProblemDetailsExceptionTrait; + + private const TITLE = 'Invalid URL'; + private const TYPE = 'INVALID_URL'; + public static function fromUrl(string $url, ?Throwable $previous = null): self { - $code = $previous !== null ? $previous->getCode() : -1; - return new static(sprintf('Provided URL "%s" is not an existing and valid URL', $url), $code, $previous); + $status = StatusCodeInterface::STATUS_BAD_REQUEST; + $e = new self(sprintf('Provided URL %s is invalid. Try with a different one.', $url), $status, $previous); + + $e->detail = $e->getMessage(); + $e->title = self::TITLE; + $e->type = self::TYPE; + $e->status = $status; + $e->additional = ['url' => $url]; + + return $e; } } diff --git a/module/Core/src/Exception/NonUniqueSlugException.php b/module/Core/src/Exception/NonUniqueSlugException.php index fb7e4503..51beff82 100644 --- a/module/Core/src/Exception/NonUniqueSlugException.php +++ b/module/Core/src/Exception/NonUniqueSlugException.php @@ -4,17 +4,34 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Exception; +use Fig\Http\Message\StatusCodeInterface; +use Zend\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; +use Zend\ProblemDetails\Exception\ProblemDetailsExceptionInterface; + use function sprintf; -class NonUniqueSlugException extends InvalidArgumentException +class NonUniqueSlugException extends InvalidArgumentException implements ProblemDetailsExceptionInterface { - public static function fromSlug(string $slug, ?string $domain): self + use CommonProblemDetailsExceptionTrait; + + private const TITLE = 'Invalid custom slug'; + private const TYPE = 'INVALID_SLUG'; + + public static function fromSlug(string $slug, ?string $domain = null): self { - $suffix = ''; + $suffix = $domain === null ? '' : sprintf(' for domain "%s"', $domain); + $e = new self(sprintf('Provided slug "%s" is already in use%s.', $slug, $suffix)); + + $e->detail = $e->getMessage(); + $e->title = self::TITLE; + $e->type = self::TYPE; + $e->status = StatusCodeInterface::STATUS_BAD_REQUEST; + $e->additional = ['customSlug' => $slug]; + if ($domain !== null) { - $suffix = sprintf(' for domain "%s"', $domain); + $e->additional['domain'] = $domain; } - return new self(sprintf('Provided slug "%s" is not unique%s.', $slug, $suffix)); + return $e; } } diff --git a/module/Core/src/Exception/ShortUrlNotFoundException.php b/module/Core/src/Exception/ShortUrlNotFoundException.php new file mode 100644 index 00000000..aca1bce9 --- /dev/null +++ b/module/Core/src/Exception/ShortUrlNotFoundException.php @@ -0,0 +1,37 @@ +detail = $e->getMessage(); + $e->title = self::TITLE; + $e->type = self::TYPE; + $e->status = StatusCodeInterface::STATUS_NOT_FOUND; + $e->additional = ['shortCode' => $shortCode]; + + if ($domain !== null) { + $e->additional['domain'] = $domain; + } + + return $e; + } +} diff --git a/module/Core/src/Exception/TagNotFoundException.php b/module/Core/src/Exception/TagNotFoundException.php new file mode 100644 index 00000000..1924e89a --- /dev/null +++ b/module/Core/src/Exception/TagNotFoundException.php @@ -0,0 +1,32 @@ +detail = $e->getMessage(); + $e->title = self::TITLE; + $e->type = self::TYPE; + $e->status = StatusCodeInterface::STATUS_NOT_FOUND; + $e->additional = ['tag' => $tag]; + + return $e; + } +} diff --git a/module/Core/src/Exception/ValidationException.php b/module/Core/src/Exception/ValidationException.php index 1b767594..abceec91 100644 --- a/module/Core/src/Exception/ValidationException.php +++ b/module/Core/src/Exception/ValidationException.php @@ -4,9 +4,13 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Exception; +use Fig\Http\Message\StatusCodeInterface; use Throwable; use Zend\InputFilter\InputFilterInterface; +use Zend\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; +use Zend\ProblemDetails\Exception\ProblemDetailsExceptionInterface; +use function array_keys; use function Functional\reduce_left; use function is_array; use function print_r; @@ -14,44 +18,59 @@ use function sprintf; use const PHP_EOL; -class ValidationException extends RuntimeException +class ValidationException extends InvalidArgumentException implements ProblemDetailsExceptionInterface { + use CommonProblemDetailsExceptionTrait; + + private const TITLE = 'Invalid data'; + private const TYPE = 'INVALID_ARGUMENT'; + /** @var array */ private $invalidElements; - public function __construct( - string $message = '', - array $invalidElements = [], - int $code = 0, - ?Throwable $previous = null - ) { - $this->invalidElements = $invalidElements; - parent::__construct($message, $code, $previous); - } - public static function fromInputFilter(InputFilterInterface $inputFilter, ?Throwable $prev = null): self { return static::fromArray($inputFilter->getMessages(), $prev); } - private static function fromArray(array $invalidData, ?Throwable $prev = null): self + public static function fromArray(array $invalidData, ?Throwable $prev = null): self { - return new self( - sprintf( - 'Provided data is not valid. These are the messages:%s%s%s', - PHP_EOL, - self::formMessagesToString($invalidData), - PHP_EOL - ), - $invalidData, - -1, - $prev + $status = StatusCodeInterface::STATUS_BAD_REQUEST; + $e = new self('Provided data is not valid', $status, $prev); + + $e->detail = $e->getMessage(); + $e->title = self::TITLE; + $e->type = self::TYPE; + $e->status = StatusCodeInterface::STATUS_BAD_REQUEST; + $e->invalidElements = $invalidData; + $e->additional = ['invalidElements' => array_keys($invalidData)]; + + return $e; + } + + public function getInvalidElements(): array + { + return $this->invalidElements; + } + + public function __toString(): string + { + return sprintf( + '%s %s in %s:%s%s%sStack trace:%s%s', + __CLASS__, + $this->getMessage(), + $this->getFile(), + $this->getLine(), + $this->invalidElementsToString(), + PHP_EOL, + PHP_EOL, + $this->getTraceAsString() ); } - private static function formMessagesToString(array $messages = []): string + private function invalidElementsToString(): string { - return reduce_left($messages, function ($messageSet, $name, $_, string $acc) { + return reduce_left($this->getInvalidElements(), function ($messageSet, string $name, $_, string $acc) { return $acc . sprintf( "\n '%s' => %s", $name, @@ -59,9 +78,4 @@ class ValidationException extends RuntimeException ); }, ''); } - - public function getInvalidElements(): array - { - return $this->invalidElements; - } } diff --git a/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php index 00837cff..c7cfb9c4 100644 --- a/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php +++ b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php @@ -25,7 +25,7 @@ class DeleteShortUrlService implements DeleteShortUrlServiceInterface } /** - * @throws Exception\InvalidShortCodeException + * @throws Exception\ShortUrlNotFoundException * @throws Exception\DeleteShortUrlException */ public function deleteByShortCode(string $shortCode, bool $ignoreThreshold = false): void diff --git a/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php index cd954ae2..b196375d 100644 --- a/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php @@ -9,7 +9,7 @@ use Shlinkio\Shlink\Core\Exception; interface DeleteShortUrlServiceInterface { /** - * @throws Exception\InvalidShortCodeException + * @throws Exception\ShortUrlNotFoundException * @throws Exception\DeleteShortUrlException */ public function deleteByShortCode(string $shortCode, bool $ignoreThreshold = false): void; diff --git a/module/Core/src/Service/ShortUrl/FindShortCodeTrait.php b/module/Core/src/Service/ShortUrl/FindShortCodeTrait.php index 47253244..81cc0e84 100644 --- a/module/Core/src/Service/ShortUrl/FindShortCodeTrait.php +++ b/module/Core/src/Service/ShortUrl/FindShortCodeTrait.php @@ -6,14 +6,14 @@ namespace Shlinkio\Shlink\Core\Service\ShortUrl; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; trait FindShortCodeTrait { /** * @param string $shortCode * @return ShortUrl - * @throws InvalidShortCodeException + * @throws ShortUrlNotFoundException */ private function findByShortCode(EntityManagerInterface $em, string $shortCode): ShortUrl { @@ -22,7 +22,7 @@ trait FindShortCodeTrait 'shortCode' => $shortCode, ]); if ($shortUrl === null) { - throw InvalidShortCodeException::fromNotFoundShortCode($shortCode); + throw ShortUrlNotFoundException::fromNotFoundShortCode($shortCode); } return $shortUrl; diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index 02977186..7719cafe 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core\Service; use Doctrine\ORM; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; @@ -45,7 +45,7 @@ class ShortUrlService implements ShortUrlServiceInterface /** * @param string[] $tags - * @throws InvalidShortCodeException + * @throws ShortUrlNotFoundException */ public function setTagsByShortCode(string $shortCode, array $tags = []): ShortUrl { @@ -57,7 +57,7 @@ class ShortUrlService implements ShortUrlServiceInterface } /** - * @throws InvalidShortCodeException + * @throws ShortUrlNotFoundException */ public function updateMetadataByShortCode(string $shortCode, ShortUrlMeta $shortUrlMeta): ShortUrl { diff --git a/module/Core/src/Service/ShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrlServiceInterface.php index e519f7c9..cb2c7dca 100644 --- a/module/Core/src/Service/ShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrlServiceInterface.php @@ -5,7 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Zend\Paginator\Paginator; @@ -20,12 +20,12 @@ interface ShortUrlServiceInterface /** * @param string[] $tags - * @throws InvalidShortCodeException + * @throws ShortUrlNotFoundException */ public function setTagsByShortCode(string $shortCode, array $tags = []): ShortUrl; /** - * @throws InvalidShortCodeException + * @throws ShortUrlNotFoundException */ public function updateMetadataByShortCode(string $shortCode, ShortUrlMeta $shortUrlMeta): ShortUrl; } diff --git a/module/Core/src/Service/Tag/TagService.php b/module/Core/src/Service/Tag/TagService.php index d5ac562e..40bdbfcd 100644 --- a/module/Core/src/Service/Tag/TagService.php +++ b/module/Core/src/Service/Tag/TagService.php @@ -7,7 +7,7 @@ namespace Shlinkio\Shlink\Core\Service\Tag; use Doctrine\Common\Collections\Collection; use Doctrine\ORM; use Shlinkio\Shlink\Core\Entity\Tag; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; +use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\Core\Util\TagManagerTrait; @@ -35,8 +35,7 @@ class TagService implements TagServiceInterface } /** - * @param array $tagNames - * @return void + * @param string[] $tagNames */ public function deleteTags(array $tagNames): void { @@ -60,23 +59,17 @@ class TagService implements TagServiceInterface } /** - * @param string $oldName - * @param string $newName - * @return Tag - * @throws EntityDoesNotExistException - * @throws ORM\OptimisticLockException + * @throws TagNotFoundException */ - public function renameTag($oldName, $newName): Tag + public function renameTag(string $oldName, string $newName): Tag { - $criteria = ['name' => $oldName]; /** @var Tag|null $tag */ - $tag = $this->em->getRepository(Tag::class)->findOneBy($criteria); + $tag = $this->em->getRepository(Tag::class)->findOneBy(['name' => $oldName]); if ($tag === null) { - throw EntityDoesNotExistException::createFromEntityAndConditions(Tag::class, $criteria); + throw TagNotFoundException::fromTag($oldName); } $tag->rename($newName); - $this->em->flush(); return $tag; diff --git a/module/Core/src/Service/Tag/TagServiceInterface.php b/module/Core/src/Service/Tag/TagServiceInterface.php index 1eb11112..dec01e31 100644 --- a/module/Core/src/Service/Tag/TagServiceInterface.php +++ b/module/Core/src/Service/Tag/TagServiceInterface.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core\Service\Tag; use Doctrine\Common\Collections\Collection; use Shlinkio\Shlink\Core\Entity\Tag; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; +use Shlinkio\Shlink\Core\Exception\TagNotFoundException; interface TagServiceInterface { @@ -17,23 +17,17 @@ interface TagServiceInterface /** * @param string[] $tagNames - * @return void */ public function deleteTags(array $tagNames): void; /** - * Provided a list of tag names, creates all that do not exist yet - * * @param string[] $tagNames * @return Collection|Tag[] */ public function createTags(array $tagNames): Collection; /** - * @param string $oldName - * @param string $newName - * @return Tag - * @throws EntityDoesNotExistException + * @throws TagNotFoundException */ - public function renameTag($oldName, $newName): Tag; + public function renameTag(string $oldName, string $newName): Tag; } diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index 6b04d63a..70112bd7 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -8,10 +8,9 @@ use Doctrine\ORM\EntityManagerInterface; use Psr\Http\Message\UriInterface; use Shlinkio\Shlink\Core\Domain\Resolver\PersistenceDomainResolver; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; @@ -130,8 +129,8 @@ class UrlShortener implements UrlShortenerInterface } /** - * @throws InvalidShortCodeException - * @throws EntityDoesNotExistException + * @throws ShortUrlNotFoundException + * @fixme Move this method to a different service */ public function shortCodeToUrl(string $shortCode, ?string $domain = null): ShortUrl { @@ -139,10 +138,7 @@ class UrlShortener implements UrlShortenerInterface $shortUrlRepo = $this->em->getRepository(ShortUrl::class); $shortUrl = $shortUrlRepo->findOneByShortCode($shortCode, $domain); if ($shortUrl === null) { - throw EntityDoesNotExistException::createFromEntityAndConditions(ShortUrl::class, [ - 'shortCode' => $shortCode, - 'domain' => $domain, - ]); + throw ShortUrlNotFoundException::fromNotFoundShortCode($shortCode, $domain); } return $shortUrl; diff --git a/module/Core/src/Service/UrlShortenerInterface.php b/module/Core/src/Service/UrlShortenerInterface.php index 74d626fc..ee9cc343 100644 --- a/module/Core/src/Service/UrlShortenerInterface.php +++ b/module/Core/src/Service/UrlShortenerInterface.php @@ -6,11 +6,9 @@ namespace Shlinkio\Shlink\Core\Service; use Psr\Http\Message\UriInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; -use Shlinkio\Shlink\Core\Exception\RuntimeException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; interface UrlShortenerInterface @@ -19,13 +17,11 @@ interface UrlShortenerInterface * @param string[] $tags * @throws NonUniqueSlugException * @throws InvalidUrlException - * @throws RuntimeException */ public function urlToShortCode(UriInterface $url, array $tags, ShortUrlMeta $meta): ShortUrl; /** - * @throws InvalidShortCodeException - * @throws EntityDoesNotExistException + * @throws ShortUrlNotFoundException */ public function shortCodeToUrl(string $shortCode, ?string $domain = null): ShortUrl; } diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index 12af69ce..612ad4ee 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -9,15 +9,13 @@ use Psr\EventDispatcher\EventDispatcherInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited; -use Shlinkio\Shlink\Core\Exception\InvalidArgumentException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Paginator\Adapter\VisitsPaginatorAdapter; use Shlinkio\Shlink\Core\Repository\VisitRepository; use Zend\Paginator\Paginator; -use function sprintf; - class VisitsTracker implements VisitsTrackerInterface { /** @var ORM\EntityManagerInterface */ @@ -53,14 +51,14 @@ class VisitsTracker implements VisitsTrackerInterface * Returns the visits on certain short code * * @return Visit[]|Paginator - * @throws InvalidArgumentException + * @throws ShortUrlNotFoundException */ public function info(string $shortCode, VisitsParams $params): Paginator { /** @var ORM\EntityRepository $repo */ $repo = $this->em->getRepository(ShortUrl::class); if ($repo->count(['shortCode' => $shortCode]) < 1) { - throw new InvalidArgumentException(sprintf('Short code "%s" not found', $shortCode)); + throw ShortUrlNotFoundException::fromNotFoundShortCode($shortCode); } /** @var VisitRepository $repo */ diff --git a/module/Core/src/Service/VisitsTrackerInterface.php b/module/Core/src/Service/VisitsTrackerInterface.php index 03af8299..2786d23b 100644 --- a/module/Core/src/Service/VisitsTrackerInterface.php +++ b/module/Core/src/Service/VisitsTrackerInterface.php @@ -5,7 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service; use Shlinkio\Shlink\Core\Entity\Visit; -use Shlinkio\Shlink\Core\Exception\InvalidArgumentException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\VisitsParams; use Zend\Paginator\Paginator; @@ -21,7 +21,7 @@ interface VisitsTrackerInterface * Returns the visits on certain short code * * @return Visit[]|Paginator - * @throws InvalidArgumentException + * @throws ShortUrlNotFoundException */ public function info(string $shortCode, VisitsParams $params): Paginator; } diff --git a/module/Core/test/Action/PreviewActionTest.php b/module/Core/test/Action/PreviewActionTest.php index bb52ec0e..e2cb089c 100644 --- a/module/Core/test/Action/PreviewActionTest.php +++ b/module/Core/test/Action/PreviewActionTest.php @@ -11,8 +11,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\PreviewAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\PreviewGenerator\Service\PreviewGenerator; use Zend\Diactoros\Response; @@ -38,19 +37,6 @@ class PreviewActionTest extends TestCase $this->action = new PreviewAction($this->previewGenerator->reveal(), $this->urlShortener->reveal()); } - /** @test */ - public function invalidShortCodeFallsBackToNextMiddleware(): void - { - $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class) - ->shouldBeCalledOnce(); - $delegate = $this->prophesize(RequestHandlerInterface::class); - $delegate->handle(Argument::cetera())->shouldBeCalledOnce() - ->willReturn(new Response()); - - $this->action->process((new ServerRequest())->withAttribute('shortCode', $shortCode), $delegate->reveal()); - } - /** @test */ public function correctShortCodeReturnsImageResponse(): void { @@ -74,7 +60,7 @@ class PreviewActionTest extends TestCase public function invalidShortCodeExceptionFallsBackToNextMiddleware(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class) + $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(ShortUrlNotFoundException::class) ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); $process = $delegate->handle(Argument::any())->willReturn(new Response()); diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index ddb062dd..6327ad69 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -11,8 +11,7 @@ use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Common\Response\QrCodeResponse; use Shlinkio\Shlink\Core\Action\QrCodeAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequest; @@ -39,7 +38,7 @@ class QrCodeActionTest extends TestCase public function aNotFoundShortCodeWillDelegateIntoNextMiddleware(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(EntityDoesNotExistException::class) + $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); $process = $delegate->handle(Argument::any())->willReturn(new Response()); @@ -53,7 +52,7 @@ class QrCodeActionTest extends TestCase public function anInvalidShortCodeWillReturnNotFoundResponse(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(InvalidShortCodeException::class) + $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); $process = $delegate->handle(Argument::any())->willReturn(new Response()); diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index a65927eb..55342cb5 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -10,7 +10,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Options; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Service\VisitsTracker; @@ -76,7 +76,7 @@ class RedirectActionTest extends TestCase public function nextMiddlewareIsInvokedIfLongUrlIsNotFound(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(EntityDoesNotExistException::class) + $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) ->shouldBeCalledOnce(); $this->visitTracker->track(Argument::cetera())->shouldNotBeCalled(); diff --git a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php new file mode 100644 index 00000000..b7796e61 --- /dev/null +++ b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php @@ -0,0 +1,101 @@ +redirectOptions = new NotFoundRedirectOptions(); + $this->middleware = new NotFoundRedirectHandler($this->redirectOptions, ''); + } + + /** + * @test + * @dataProvider provideRedirects + */ + public function expectedRedirectionIsReturnedDependingOnTheCase( + ServerRequestInterface $request, + string $expectedRedirectTo + ): void { + $this->redirectOptions->invalidShortUrl = 'invalidShortUrl'; + $this->redirectOptions->regular404 = 'regular404'; + $this->redirectOptions->baseUrl = 'baseUrl'; + + $next = $this->prophesize(RequestHandlerInterface::class); + $handle = $next->handle($request)->willReturn(new Response()); + + $resp = $this->middleware->process($request, $next->reveal()); + + $this->assertInstanceOf(Response\RedirectResponse::class, $resp); + $this->assertEquals($expectedRedirectTo, $resp->getHeaderLine('Location')); + $handle->shouldNotHaveBeenCalled(); + } + + public function provideRedirects(): iterable + { + yield 'base URL with trailing slash' => [ + ServerRequestFactory::fromGlobals()->withUri(new Uri('/')), + 'baseUrl', + ]; + yield 'base URL without trailing slash' => [ + ServerRequestFactory::fromGlobals()->withUri(new Uri('')), + 'baseUrl', + ]; + yield 'regular 404' => [ + ServerRequestFactory::fromGlobals()->withUri(new Uri('/foo/bar')), + 'regular404', + ]; + yield 'invalid short URL' => [ + ServerRequestFactory::fromGlobals() + ->withAttribute( + RouteResult::class, + RouteResult::fromRoute( + new Route( + '', + $this->prophesize(MiddlewareInterface::class)->reveal(), + ['GET'], + RedirectAction::class + ) + ) + ) + ->withUri(new Uri('/abc123')), + 'invalidShortUrl', + ]; + } + + /** @test */ + public function nextMiddlewareIsInvokedWhenNotRedirectNeedsToOccur(): void + { + $req = ServerRequestFactory::fromGlobals(); + $resp = new Response(); + + $next = $this->prophesize(RequestHandlerInterface::class); + $handle = $next->handle($req)->willReturn($resp); + + $result = $this->middleware->process($req, $next->reveal()); + + $this->assertSame($resp, $result); + $handle->shouldHaveBeenCalledOnce(); + } +} diff --git a/module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php new file mode 100644 index 00000000..7d763448 --- /dev/null +++ b/module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php @@ -0,0 +1,59 @@ +renderer = $this->prophesize(TemplateRendererInterface::class); + $this->handler = new NotFoundTemplateHandler($this->renderer->reveal()); + } + + /** + * @test + * @dataProvider provideTemplates + */ + public function properErrorTemplateIsRendered(ServerRequestInterface $request, string $expectedTemplate): void + { + $request = $request->withHeader('Accept', 'text/html'); + $render = $this->renderer->render($expectedTemplate)->willReturn(''); + + $resp = $this->handler->handle($request); + + $this->assertInstanceOf(Response\HtmlResponse::class, $resp); + $render->shouldHaveBeenCalledOnce(); + } + + public function provideTemplates(): iterable + { + $request = ServerRequestFactory::fromGlobals(); + + yield [$request, NotFoundTemplateHandler::NOT_FOUND_TEMPLATE]; + yield [ + $request->withAttribute( + RouteResult::class, + RouteResult::fromRoute(new Route('', $this->prophesize(MiddlewareInterface::class)->reveal())) + ), + NotFoundTemplateHandler::INVALID_SHORT_CODE_TEMPLATE, + ]; + } +} diff --git a/module/Core/test/Exception/DeleteShortUrlExceptionTest.php b/module/Core/test/Exception/DeleteShortUrlExceptionTest.php index 4b132eea..0e2012a2 100644 --- a/module/Core/test/Exception/DeleteShortUrlExceptionTest.php +++ b/module/Core/test/Exception/DeleteShortUrlExceptionTest.php @@ -5,17 +5,15 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Exception; use PHPUnit\Framework\TestCase; -use Shlinkio\Shlink\Common\Util\StringUtilsTrait; use Shlinkio\Shlink\Core\Exception\DeleteShortUrlException; use function Functional\map; use function range; +use function Shlinkio\Shlink\Core\generateRandomShortCode; use function sprintf; class DeleteShortUrlExceptionTest extends TestCase { - use StringUtilsTrait; - /** * @test * @dataProvider provideThresholds @@ -29,29 +27,12 @@ class DeleteShortUrlExceptionTest extends TestCase $this->assertEquals($threshold, $e->getVisitsThreshold()); $this->assertEquals($expectedMessage, $e->getMessage()); - $this->assertEquals(0, $e->getCode()); - $this->assertNull($e->getPrevious()); - } - - /** - * @test - * @dataProvider provideThresholds - */ - public function visitsThresholdIsProperlyReturned(int $threshold): void - { - $e = new DeleteShortUrlException($threshold); - - $this->assertEquals($threshold, $e->getVisitsThreshold()); - $this->assertEquals('', $e->getMessage()); - $this->assertEquals(0, $e->getCode()); - $this->assertNull($e->getPrevious()); } public function provideThresholds(): array { return map(range(5, 50, 5), function (int $number) { - $shortCode = $this->generateRandomString(6); - return [$number, $shortCode, sprintf( + return [$number, $shortCode = generateRandomShortCode(6), sprintf( 'Impossible to delete short URL with short code "%s" since it has more than "%s" visits.', $shortCode, $number diff --git a/module/Core/test/Exception/InvalidShortCodeExceptionTest.php b/module/Core/test/Exception/InvalidShortCodeExceptionTest.php deleted file mode 100644 index 02a3edf2..00000000 --- a/module/Core/test/Exception/InvalidShortCodeExceptionTest.php +++ /dev/null @@ -1,40 +0,0 @@ -assertEquals('Provided short code "abc123" does not match the char set "def456"', $e->getMessage()); - $this->assertEquals($prev !== null ? $prev->getCode() : -1, $e->getCode()); - $this->assertEquals($prev, $e->getPrevious()); - } - - public function providePrevious(): iterable - { - yield 'null previous' => [null]; - yield 'instance previous' => [new Exception('Previous error', 10)]; - } - - /** @test */ - public function properlyCreatesExceptionFromNotFoundShortCode(): void - { - $e = InvalidShortCodeException::fromNotFoundShortCode('abc123'); - - $this->assertEquals('Provided short code "abc123" does not belong to a short URL', $e->getMessage()); - } -} diff --git a/module/Core/test/Exception/InvalidUrlExceptionTest.php b/module/Core/test/Exception/InvalidUrlExceptionTest.php index a5d19341..1b7de449 100644 --- a/module/Core/test/Exception/InvalidUrlExceptionTest.php +++ b/module/Core/test/Exception/InvalidUrlExceptionTest.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Exception; use Exception; +use Fig\Http\Message\StatusCodeInterface; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Throwable; @@ -19,8 +20,8 @@ class InvalidUrlExceptionTest extends TestCase { $e = InvalidUrlException::fromUrl('http://the_url.com', $prev); - $this->assertEquals('Provided URL "http://the_url.com" is not an existing and valid URL', $e->getMessage()); - $this->assertEquals($prev !== null ? $prev->getCode() : -1, $e->getCode()); + $this->assertEquals('Provided URL http://the_url.com is invalid. Try with a different one.', $e->getMessage()); + $this->assertEquals(StatusCodeInterface::STATUS_BAD_REQUEST, $e->getCode()); $this->assertEquals($prev, $e->getPrevious()); } diff --git a/module/Core/test/Exception/NonUniqueSlugExceptionTest.php b/module/Core/test/Exception/NonUniqueSlugExceptionTest.php index 71c4a276..d2008621 100644 --- a/module/Core/test/Exception/NonUniqueSlugExceptionTest.php +++ b/module/Core/test/Exception/NonUniqueSlugExceptionTest.php @@ -22,12 +22,12 @@ class NonUniqueSlugExceptionTest extends TestCase public function provideMessages(): iterable { yield 'without domain' => [ - 'Provided slug "foo" is not unique.', + 'Provided slug "foo" is already in use.', 'foo', null, ]; yield 'with domain' => [ - 'Provided slug "baz" is not unique for domain "bar".', + 'Provided slug "baz" is already in use for domain "bar".', 'baz', 'bar', ]; diff --git a/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php b/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php new file mode 100644 index 00000000..6f0d58f4 --- /dev/null +++ b/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php @@ -0,0 +1,38 @@ +assertEquals($expectedMessage, $e->getMessage()); + } + + public function provideMessages(): iterable + { + yield 'without domain' => [ + 'No URL found with short code "abc123"', + 'abc123', + null, + ]; + yield 'with domain' => [ + 'No URL found with short code "bar" for domain "foo"', + 'bar', + 'foo', + ]; + } +} diff --git a/module/Core/test/Exception/ValidationExceptionTest.php b/module/Core/test/Exception/ValidationExceptionTest.php index 5cab422c..11bb8026 100644 --- a/module/Core/test/Exception/ValidationExceptionTest.php +++ b/module/Core/test/Exception/ValidationExceptionTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Exception; +use Fig\Http\Message\StatusCodeInterface; use LogicException; use PHPUnit\Framework\TestCase; use RuntimeException; @@ -11,39 +12,11 @@ use Shlinkio\Shlink\Core\Exception\ValidationException; use Throwable; use Zend\InputFilter\InputFilterInterface; +use function array_keys; use function print_r; -use function random_int; class ValidationExceptionTest extends TestCase { - /** - * @test - * @dataProvider provideExceptionData - */ - public function createsExceptionWrappingExpectedData( - array $args, - string $expectedMessage, - array $expectedInvalidElements, - int $expectedCode, - ?Throwable $expectedPrev - ): void { - $e = new ValidationException(...$args); - - $this->assertEquals($expectedMessage, $e->getMessage()); - $this->assertEquals($expectedInvalidElements, $e->getInvalidElements()); - $this->assertEquals($expectedCode, $e->getCode()); - $this->assertEquals($expectedPrev, $e->getPrevious()); - } - - public function provideExceptionData(): iterable - { - yield 'empty args' => [[], '', [], 0, null]; - yield 'with message' => [['something'], 'something', [], 0, null]; - yield 'with elements' => [['something_else', [1, 2, 3]], 'something_else', [1, 2, 3], 0, null]; - yield 'with code' => [['foo', [], $foo = random_int(-100, 100)], 'foo', [], $foo, null]; - yield 'with prev' => [['bar', [], 8, $e = new RuntimeException()], 'bar', [], 8, $e]; - } - /** * @test * @dataProvider provideExceptions @@ -55,12 +28,9 @@ class ValidationExceptionTest extends TestCase 'something' => ['baz', 'foo'], ]; $barValue = print_r(['baz', 'foo'], true); - $expectedMessage = << bar 'something' => {$barValue} - EOT; $inputFilter = $this->prophesize(InputFilterInterface::class); @@ -69,9 +39,11 @@ EOT; $e = ValidationException::fromInputFilter($inputFilter->reveal()); $this->assertEquals($invalidData, $e->getInvalidElements()); - $this->assertEquals($expectedMessage, $e->getMessage()); - $this->assertEquals(-1, $e->getCode()); + $this->assertEquals(['invalidElements' => array_keys($invalidData)], $e->getAdditionalData()); + $this->assertEquals('Provided data is not valid', $e->getMessage()); + $this->assertEquals(StatusCodeInterface::STATUS_BAD_REQUEST, $e->getCode()); $this->assertEquals($prev, $e->getPrevious()); + $this->assertStringContainsString($expectedStringRepresentation, (string) $e); $getMessages->shouldHaveBeenCalledOnce(); } diff --git a/module/Core/test/Response/NotFoundHandlerTest.php b/module/Core/test/Response/NotFoundHandlerTest.php deleted file mode 100644 index 8f1d6f51..00000000 --- a/module/Core/test/Response/NotFoundHandlerTest.php +++ /dev/null @@ -1,142 +0,0 @@ -renderer = $this->prophesize(TemplateRendererInterface::class); - $this->redirectOptions = new NotFoundRedirectOptions(); - - $this->delegate = new NotFoundHandler($this->renderer->reveal(), $this->redirectOptions, ''); - } - - /** - * @test - * @dataProvider provideResponses - */ - public function properResponseTypeIsReturned(string $expectedResponse, string $accept, int $renderCalls): void - { - $request = (new ServerRequest())->withHeader('Accept', $accept); - $render = $this->renderer->render(Argument::cetera())->willReturn(''); - - $resp = $this->delegate->handle($request); - - $this->assertInstanceOf($expectedResponse, $resp); - $render->shouldHaveBeenCalledTimes($renderCalls); - } - - public function provideResponses(): iterable - { - yield 'application/json' => [Response\JsonResponse::class, 'application/json', 0]; - yield 'text/json' => [Response\JsonResponse::class, 'text/json', 0]; - yield 'application/x-json' => [Response\JsonResponse::class, 'application/x-json', 0]; - yield 'text/html' => [Response\HtmlResponse::class, 'text/html', 1]; - } - - /** - * @test - * @dataProvider provideRedirects - */ - public function expectedRedirectionIsReturnedDependingOnTheCase( - ServerRequestInterface $request, - string $expectedRedirectTo - ): void { - $this->redirectOptions->invalidShortUrl = 'invalidShortUrl'; - $this->redirectOptions->regular404 = 'regular404'; - $this->redirectOptions->baseUrl = 'baseUrl'; - - $resp = $this->delegate->handle($request); - - $this->assertInstanceOf(Response\RedirectResponse::class, $resp); - $this->assertEquals($expectedRedirectTo, $resp->getHeaderLine('Location')); - $this->renderer->render(Argument::cetera())->shouldNotHaveBeenCalled(); - } - - public function provideRedirects(): iterable - { - yield 'base URL with trailing slash' => [ - ServerRequestFactory::fromGlobals()->withUri(new Uri('/')), - 'baseUrl', - ]; - yield 'base URL without trailing slash' => [ - ServerRequestFactory::fromGlobals()->withUri(new Uri('')), - 'baseUrl', - ]; - yield 'regular 404' => [ - ServerRequestFactory::fromGlobals()->withUri(new Uri('/foo/bar')), - 'regular404', - ]; - yield 'invalid short URL' => [ - ServerRequestFactory::fromGlobals() - ->withAttribute( - RouteResult::class, - RouteResult::fromRoute( - new Route( - '', - $this->prophesize(MiddlewareInterface::class)->reveal(), - ['GET'], - RedirectAction::class - ) - ) - ) - ->withUri(new Uri('/abc123')), - 'invalidShortUrl', - ]; - } - - /** - * @test - * @dataProvider provideTemplates - */ - public function properErrorTemplateIsRendered(ServerRequestInterface $request, string $expectedTemplate): void - { - $request = $request->withHeader('Accept', 'text/html'); - $render = $this->renderer->render($expectedTemplate)->willReturn(''); - - $resp = $this->delegate->handle($request); - - $this->assertInstanceOf(Response\HtmlResponse::class, $resp); - $render->shouldHaveBeenCalledOnce(); - } - - public function provideTemplates(): iterable - { - $request = ServerRequestFactory::fromGlobals(); - - yield [$request, NotFoundHandler::NOT_FOUND_TEMPLATE]; - yield [ - $request->withAttribute( - RouteResult::class, - RouteResult::fromRoute(new Route('', $this->prophesize(MiddlewareInterface::class)->reveal())) - ), - NotFoundHandler::INVALID_SHORT_CODE_TEMPLATE, - ]; - } -} diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php index 07628f7c..3dae00a7 100644 --- a/module/Core/test/Service/ShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrlServiceTest.php @@ -12,7 +12,7 @@ use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Tag; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\Service\ShortUrlService; @@ -62,7 +62,7 @@ class ShortUrlServiceTest extends TestCase ->shouldBeCalledOnce(); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $this->expectException(InvalidShortCodeException::class); + $this->expectException(ShortUrlNotFoundException::class); $this->service->setTagsByShortCode($shortCode); } diff --git a/module/Core/test/Service/Tag/TagServiceTest.php b/module/Core/test/Service/Tag/TagServiceTest.php index 0dd63ff5..0131d9e9 100644 --- a/module/Core/test/Service/Tag/TagServiceTest.php +++ b/module/Core/test/Service/Tag/TagServiceTest.php @@ -10,7 +10,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\Tag; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; +use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\Core\Service\Tag\TagService; @@ -83,7 +83,7 @@ class TagServiceTest extends TestCase $find->shouldBeCalled(); $getRepo->shouldBeCalled(); - $this->expectException(EntityDoesNotExistException::class); + $this->expectException(TagNotFoundException::class); $this->service->renameTag('foo', 'bar'); } diff --git a/module/Rest/config/auth.config.php b/module/Rest/config/auth.config.php index 7acb133e..9d0987e0 100644 --- a/module/Rest/config/auth.config.php +++ b/module/Rest/config/auth.config.php @@ -48,7 +48,6 @@ return [ Middleware\AuthenticationMiddleware::class => [ Authentication\RequestToHttpAuthPlugin::class, 'config.auth.routes_whitelist', - 'Logger_Shlink', ], ], diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 54f380b3..17c30642 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest; +use Doctrine\DBAL\Connection; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Service; @@ -20,15 +21,15 @@ return [ ApiKeyService::class => ConfigAbstractFactory::class, Action\AuthenticateAction::class => ConfigAbstractFactory::class, - Action\HealthAction::class => Action\HealthActionFactory::class, + Action\HealthAction::class => ConfigAbstractFactory::class, Action\ShortUrl\CreateShortUrlAction::class => ConfigAbstractFactory::class, Action\ShortUrl\SingleStepCreateShortUrlAction::class => ConfigAbstractFactory::class, Action\ShortUrl\EditShortUrlAction::class => ConfigAbstractFactory::class, Action\ShortUrl\DeleteShortUrlAction::class => ConfigAbstractFactory::class, Action\ShortUrl\ResolveShortUrlAction::class => ConfigAbstractFactory::class, - Action\Visit\GetVisitsAction::class => ConfigAbstractFactory::class, Action\ShortUrl\ListShortUrlsAction::class => ConfigAbstractFactory::class, Action\ShortUrl\EditShortUrlTagsAction::class => ConfigAbstractFactory::class, + Action\Visit\GetVisitsAction::class => ConfigAbstractFactory::class, Action\Tag\ListTagsAction::class => ConfigAbstractFactory::class, Action\Tag\DeleteTagsAction::class => ConfigAbstractFactory::class, Action\Tag\CreateTagsAction::class => ConfigAbstractFactory::class, @@ -38,6 +39,7 @@ return [ Middleware\BodyParserMiddleware::class => InvokableFactory::class, Middleware\CrossDomainMiddleware::class => InvokableFactory::class, Middleware\PathVersionMiddleware::class => InvokableFactory::class, + Middleware\BackwardsCompatibleProblemDetailsMiddleware::class => ConfigAbstractFactory::class, Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class => InvokableFactory::class, Middleware\ShortUrl\ShortCodePathMiddleware::class => InvokableFactory::class, ], @@ -48,6 +50,7 @@ return [ ApiKeyService::class => ['em'], Action\AuthenticateAction::class => [ApiKeyService::class, Authentication\JWTService::class, 'Logger_Shlink'], + Action\HealthAction::class => [Connection::class, AppOptions::class, 'Logger_Shlink'], Action\ShortUrl\CreateShortUrlAction::class => [ Service\UrlShortener::class, 'config.url_shortener.domain', @@ -73,6 +76,11 @@ return [ Action\Tag\DeleteTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\CreateTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\UpdateTagAction::class => [Service\Tag\TagService::class, LoggerInterface::class], + + Middleware\BackwardsCompatibleProblemDetailsMiddleware::class => [ + 'config.backwards_compatible_problem_details.default_type_fallbacks', + 'config.backwards_compatible_problem_details.json_flags', + ], ], ]; diff --git a/module/Rest/config/error-handler.config.php b/module/Rest/config/error-handler.config.php deleted file mode 100644 index c7503b72..00000000 --- a/module/Rest/config/error-handler.config.php +++ /dev/null @@ -1,21 +0,0 @@ - [ - 'plugins' => [ - 'invokables' => [ - 'application/json' => JsonErrorResponseGenerator::class, - ], - 'aliases' => [ - 'application/x-json' => 'application/json', - 'text/json' => 'application/json', - ], - ], - ], - -]; diff --git a/module/Rest/src/Action/AuthenticateAction.php b/module/Rest/src/Action/AuthenticateAction.php index 1476f6de..b0919aae 100644 --- a/module/Rest/src/Action/AuthenticateAction.php +++ b/module/Rest/src/Action/AuthenticateAction.php @@ -10,7 +10,6 @@ use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Rest\Authentication\JWTServiceInterface; use Shlinkio\Shlink\Rest\Service\ApiKeyService; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\JsonResponse; /** @deprecated */ @@ -44,7 +43,7 @@ class AuthenticateAction extends AbstractRestAction $authData = $request->getParsedBody(); if (! isset($authData['apiKey'])) { return new JsonResponse([ - 'error' => RestUtils::INVALID_ARGUMENT_ERROR, + 'error' => 'INVALID_ARGUMENT', 'message' => 'You have to provide a valid API key under the "apiKey" param name.', ], self::STATUS_BAD_REQUEST); } @@ -53,7 +52,7 @@ class AuthenticateAction extends AbstractRestAction $apiKey = $this->apiKeyService->getByKey($authData['apiKey']); if ($apiKey === null || ! $apiKey->isValid()) { return new JsonResponse([ - 'error' => RestUtils::INVALID_API_KEY_ERROR, + 'error' => 'INVALID_API_KEY', 'message' => 'Provided API key does not exist or is invalid.', ], self::STATUS_UNAUTHORIZED); } diff --git a/module/Rest/src/Action/HealthAction.php b/module/Rest/src/Action/HealthAction.php index 23090165..ac548fde 100644 --- a/module/Rest/src/Action/HealthAction.php +++ b/module/Rest/src/Action/HealthAction.php @@ -15,8 +15,8 @@ use Zend\Diactoros\Response\JsonResponse; class HealthAction extends AbstractRestAction { private const HEALTH_CONTENT_TYPE = 'application/health+json'; - private const PASS_STATUS = 'pass'; - private const FAIL_STATUS = 'fail'; + private const STATUS_PASS = 'pass'; + private const STATUS_FAIL = 'fail'; protected const ROUTE_PATH = '/health'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; @@ -48,7 +48,7 @@ class HealthAction extends AbstractRestAction $statusCode = $connected ? self::STATUS_OK : self::STATUS_SERVICE_UNAVAILABLE; return new JsonResponse([ - 'status' => $connected ? self::PASS_STATUS : self::FAIL_STATUS, + 'status' => $connected ? self::STATUS_PASS : self::STATUS_FAIL, 'version' => $this->options->getVersion(), 'links' => [ 'about' => 'https://shlink.io', diff --git a/module/Rest/src/Action/HealthActionFactory.php b/module/Rest/src/Action/HealthActionFactory.php deleted file mode 100644 index 269a8f4d..00000000 --- a/module/Rest/src/Action/HealthActionFactory.php +++ /dev/null @@ -1,20 +0,0 @@ -get(EntityManager::class); - $options = $container->get(AppOptions::class); - $logger = $container->get('Logger_Shlink'); - return new HealthAction($em->getConnection(), $options, $logger); - } -} diff --git a/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php index e55f564e..af392067 100644 --- a/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php @@ -7,19 +7,13 @@ namespace Shlinkio\Shlink\Rest\Action\ShortUrl; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\Exception\InvalidArgumentException; -use Shlinkio\Shlink\Core\Exception\InvalidUrlException; -use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; +use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Model\CreateShortUrlData; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; -use Throwable; use Zend\Diactoros\Response\JsonResponse; -use function sprintf; - abstract class AbstractCreateShortUrlAction extends AbstractRestAction { /** @var UrlShortenerInterface */ @@ -37,56 +31,23 @@ abstract class AbstractCreateShortUrlAction extends AbstractRestAction $this->domainConfig = $domainConfig; } - /** - * @param Request $request - * @return Response - */ public function handle(Request $request): Response { - try { - $shortUrlData = $this->buildShortUrlData($request); - } catch (InvalidArgumentException $e) { - $this->logger->warning('Provided data is invalid. {e}', ['e' => $e]); - return new JsonResponse([ - 'error' => RestUtils::INVALID_ARGUMENT_ERROR, - 'message' => $e->getMessage(), - ], self::STATUS_BAD_REQUEST); - } - + $shortUrlData = $this->buildShortUrlData($request); $longUrl = $shortUrlData->getLongUrl(); + $tags = $shortUrlData->getTags(); $shortUrlMeta = $shortUrlData->getMeta(); - try { - $shortUrl = $this->urlShortener->urlToShortCode($longUrl, $shortUrlData->getTags(), $shortUrlMeta); - $transformer = new ShortUrlDataTransformer($this->domainConfig); + $shortUrl = $this->urlShortener->urlToShortCode($longUrl, $tags, $shortUrlMeta); + $transformer = new ShortUrlDataTransformer($this->domainConfig); - return new JsonResponse($transformer->transform($shortUrl)); - } catch (InvalidUrlException $e) { - $this->logger->warning('Provided Invalid URL. {e}', ['e' => $e]); - return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf('Provided URL %s is invalid. Try with a different one.', $longUrl), - ], self::STATUS_BAD_REQUEST); - } catch (NonUniqueSlugException $e) { - $customSlug = $shortUrlMeta->getCustomSlug(); - $this->logger->warning('Provided non-unique slug. {e}', ['e' => $e]); - return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf('Provided slug %s is already in use. Try with a different one.', $customSlug), - ], self::STATUS_BAD_REQUEST); - } catch (Throwable $e) { - $this->logger->error('Unexpected error creating short url. {e}', ['e' => $e]); - return new JsonResponse([ - 'error' => RestUtils::UNKNOWN_ERROR, - 'message' => 'Unexpected error occurred', - ], self::STATUS_INTERNAL_SERVER_ERROR); - } + return new JsonResponse($transformer->transform($shortUrl)); } /** * @param Request $request * @return CreateShortUrlData - * @throws InvalidArgumentException + * @throws ValidationException */ abstract protected function buildShortUrlData(Request $request): CreateShortUrlData; } diff --git a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php index 3b0a0b61..9c20de6c 100644 --- a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php @@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\Rest\Action\ShortUrl; use Cake\Chronos\Chronos; use Psr\Http\Message\ServerRequestInterface as Request; -use Shlinkio\Shlink\Core\Exception\InvalidArgumentException; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Model\CreateShortUrlData; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; @@ -20,30 +19,27 @@ class CreateShortUrlAction extends AbstractCreateShortUrlAction /** * @param Request $request * @return CreateShortUrlData - * @throws InvalidArgumentException - * @throws \InvalidArgumentException + * @throws ValidationException */ protected function buildShortUrlData(Request $request): CreateShortUrlData { $postData = (array) $request->getParsedBody(); if (! isset($postData['longUrl'])) { - throw new InvalidArgumentException('A URL was not provided'); + throw ValidationException::fromArray([ + 'longUrl' => 'A URL was not provided', + ]); } - try { - $meta = ShortUrlMeta::createFromParams( - $this->getOptionalDate($postData, 'validSince'), - $this->getOptionalDate($postData, 'validUntil'), - $postData['customSlug'] ?? null, - $postData['maxVisits'] ?? null, - $postData['findIfExists'] ?? null, - $postData['domain'] ?? null - ); + $meta = ShortUrlMeta::createFromParams( + $this->getOptionalDate($postData, 'validSince'), + $this->getOptionalDate($postData, 'validUntil'), + $postData['customSlug'] ?? null, + $postData['maxVisits'] ?? null, + $postData['findIfExists'] ?? null, + $postData['domain'] ?? null + ); - return new CreateShortUrlData(new Uri($postData['longUrl']), (array) ($postData['tags'] ?? []), $meta); - } catch (ValidationException $e) { - throw new InvalidArgumentException('Provided meta data is not valid', -1, $e); - } + return new CreateShortUrlData(new Uri($postData['longUrl']), (array) ($postData['tags'] ?? []), $meta); } private function getOptionalDate(array $postData, string $fieldName): ?Chronos diff --git a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php index 54755baa..ba39ec82 100644 --- a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php @@ -7,14 +7,9 @@ namespace Shlinkio\Shlink\Rest\Action\ShortUrl; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\Exception; use Shlinkio\Shlink\Core\Service\ShortUrl\DeleteShortUrlServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\EmptyResponse; -use Zend\Diactoros\Response\JsonResponse; - -use function sprintf; class DeleteShortUrlAction extends AbstractRestAction { @@ -30,34 +25,10 @@ class DeleteShortUrlAction extends AbstractRestAction $this->deleteShortUrlService = $deleteShortUrlService; } - /** - * Handle the request and return a response. - */ public function handle(ServerRequestInterface $request): ResponseInterface { $shortCode = $request->getAttribute('shortCode', ''); - - try { - $this->deleteShortUrlService->deleteByShortCode($shortCode); - return new EmptyResponse(); - } catch (Exception\InvalidShortCodeException $e) { - $this->logger->warning( - 'Provided short code {shortCode} does not belong to any URL. {e}', - ['e' => $e, 'shortCode' => $shortCode] - ); - return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf('No URL found for short code "%s"', $shortCode), - ], self::STATUS_NOT_FOUND); - } catch (Exception\DeleteShortUrlException $e) { - $this->logger->warning('Provided data is invalid. {e}', ['e' => $e]); - $messagePlaceholder = - 'It is not possible to delete URL with short code "%s" because it has reached more than "%s" visits.'; - - return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf($messagePlaceholder, $shortCode, $e->getVisitsThreshold()), - ], self::STATUS_BAD_REQUEST); - } + $this->deleteShortUrlService->deleteByShortCode($shortCode); + return new EmptyResponse(); } } diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php index fe609e03..33796f7d 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php @@ -7,15 +7,10 @@ namespace Shlinkio\Shlink\Rest\Action\ShortUrl; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\Exception; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\EmptyResponse; -use Zend\Diactoros\Response\JsonResponse; - -use function sprintf; class EditShortUrlAction extends AbstractRestAction { @@ -31,38 +26,12 @@ class EditShortUrlAction extends AbstractRestAction $this->shortUrlService = $shortUrlService; } - /** - * Process an incoming server request and return a response, optionally delegating - * to the next middleware component to create the response. - * - * @param ServerRequestInterface $request - * - * @return ResponseInterface - * @throws \InvalidArgumentException - */ public function handle(ServerRequestInterface $request): ResponseInterface { $postData = (array) $request->getParsedBody(); $shortCode = $request->getAttribute('shortCode', ''); - try { - $this->shortUrlService->updateMetadataByShortCode( - $shortCode, - ShortUrlMeta::createFromRawData($postData) - ); - return new EmptyResponse(); - } catch (Exception\InvalidShortCodeException $e) { - $this->logger->warning('Provided data is invalid. {e}', ['e' => $e]); - return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf('No URL found for short code "%s"', $shortCode), - ], self::STATUS_NOT_FOUND); - } catch (Exception\ValidationException $e) { - $this->logger->warning('Provided data is invalid. {e}', ['e' => $e]); - return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => 'Provided data is invalid.', - ], self::STATUS_BAD_REQUEST); - } + $this->shortUrlService->updateMetadataByShortCode($shortCode, ShortUrlMeta::createFromRawData($postData)); + return new EmptyResponse(); } } diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php index 4daa0fac..208be169 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php @@ -7,14 +7,11 @@ namespace Shlinkio\Shlink\Rest\Action\ShortUrl; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\JsonResponse; -use function sprintf; - class EditShortUrlTagsAction extends AbstractRestAction { protected const ROUTE_PATH = '/short-urls/{shortCode}/tags'; @@ -29,32 +26,19 @@ class EditShortUrlTagsAction extends AbstractRestAction $this->shortUrlService = $shortUrlService; } - /** - * @param Request $request - * @return Response - * @throws \InvalidArgumentException - */ public function handle(Request $request): Response { $shortCode = $request->getAttribute('shortCode'); $bodyParams = $request->getParsedBody(); if (! isset($bodyParams['tags'])) { - return new JsonResponse([ - 'error' => RestUtils::INVALID_ARGUMENT_ERROR, - 'message' => 'A list of tags was not provided', - ], self::STATUS_BAD_REQUEST); + throw ValidationException::fromArray([ + 'tags' => 'List of tags has to be provided', + ]); } $tags = $bodyParams['tags']; - try { - $shortUrl = $this->shortUrlService->setTagsByShortCode($shortCode, $tags); - return new JsonResponse(['tags' => $shortUrl->getTags()->toArray()]); - } catch (InvalidShortCodeException $e) { - return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf('No URL found for short code "%s"', $shortCode), - ], self::STATUS_NOT_FOUND); - } + $shortUrl = $this->shortUrlService->setTagsByShortCode($shortCode, $tags); + return new JsonResponse(['tags' => $shortUrl->getTags()->toArray()]); } } diff --git a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php index 8c4211c8..157fbe06 100644 --- a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php +++ b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Action\ShortUrl; -use Exception; +use InvalidArgumentException; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; @@ -12,7 +12,6 @@ use Shlinkio\Shlink\Common\Paginator\Util\PaginatorUtilsTrait; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\JsonResponse; class ListShortUrlsAction extends AbstractRestAction @@ -40,23 +39,15 @@ class ListShortUrlsAction extends AbstractRestAction /** * @param Request $request * @return Response - * @throws \InvalidArgumentException + * @throws InvalidArgumentException */ public function handle(Request $request): Response { - try { - $params = $this->queryToListParams($request->getQueryParams()); - $shortUrls = $this->shortUrlService->listShortUrls(...$params); - return new JsonResponse(['shortUrls' => $this->serializePaginator($shortUrls, new ShortUrlDataTransformer( - $this->domainConfig - ))]); - } catch (Exception $e) { - $this->logger->error('Unexpected error while listing short URLs. {e}', ['e' => $e]); - return new JsonResponse([ - 'error' => RestUtils::UNKNOWN_ERROR, - 'message' => 'Unexpected error occurred', - ], self::STATUS_INTERNAL_SERVER_ERROR); - } + $params = $this->queryToListParams($request->getQueryParams()); + $shortUrls = $this->shortUrlService->listShortUrls(...$params); + return new JsonResponse(['shortUrls' => $this->serializePaginator($shortUrls, new ShortUrlDataTransformer( + $this->domainConfig + ))]); } /** diff --git a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php index 57d3cd62..e041f4dc 100644 --- a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php @@ -4,20 +4,15 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Action\ShortUrl; -use Exception; +use InvalidArgumentException; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\JsonResponse; -use function sprintf; - class ResolveShortUrlAction extends AbstractRestAction { protected const ROUTE_PATH = '/short-urls/{shortCode}'; @@ -41,7 +36,7 @@ class ResolveShortUrlAction extends AbstractRestAction /** * @param Request $request * @return Response - * @throws \InvalidArgumentException + * @throws InvalidArgumentException */ public function handle(Request $request): Response { @@ -49,27 +44,7 @@ class ResolveShortUrlAction extends AbstractRestAction $domain = $request->getQueryParams()['domain'] ?? null; $transformer = new ShortUrlDataTransformer($this->domainConfig); - try { - $url = $this->urlShortener->shortCodeToUrl($shortCode, $domain); - return new JsonResponse($transformer->transform($url)); - } catch (InvalidShortCodeException $e) { - $this->logger->warning('Provided short code with invalid format. {e}', ['e' => $e]); - return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf('Provided short code "%s" has an invalid format', $shortCode), - ], self::STATUS_BAD_REQUEST); - } catch (EntityDoesNotExistException $e) { - $this->logger->warning('Provided short code couldn\'t be found. {e}', ['e' => $e]); - return new JsonResponse([ - 'error' => RestUtils::INVALID_ARGUMENT_ERROR, - 'message' => sprintf('No URL found for short code "%s"', $shortCode), - ], self::STATUS_NOT_FOUND); - } catch (Exception $e) { - $this->logger->error('Unexpected error while resolving the URL behind a short code. {e}', ['e' => $e]); - return new JsonResponse([ - 'error' => RestUtils::UNKNOWN_ERROR, - 'message' => 'Unexpected error occurred', - ], self::STATUS_INTERNAL_SERVER_ERROR); - } + $url = $this->urlShortener->shortCodeToUrl($shortCode, $domain); + return new JsonResponse($transformer->transform($url)); } } diff --git a/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php index 327df46e..834c6b12 100644 --- a/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Rest\Action\ShortUrl; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\Exception\InvalidArgumentException; +use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Model\CreateShortUrlData; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; @@ -33,19 +33,22 @@ class SingleStepCreateShortUrlAction extends AbstractCreateShortUrlAction /** * @param Request $request * @return CreateShortUrlData - * @throws \InvalidArgumentException - * @throws InvalidArgumentException + * @throws ValidationException */ protected function buildShortUrlData(Request $request): CreateShortUrlData { $query = $request->getQueryParams(); if (! $this->apiKeyService->check($query['apiKey'] ?? '')) { - throw new InvalidArgumentException('No API key was provided or it is not valid'); + throw ValidationException::fromArray([ + 'apiKey' => 'No API key was provided or it is not valid', + ]); } if (! isset($query['longUrl'])) { - throw new InvalidArgumentException('A URL was not provided'); + throw ValidationException::fromArray([ + 'longUrl' => 'A URL was not provided', + ]); } return new CreateShortUrlData(new Uri($query['longUrl'])); diff --git a/module/Rest/src/Action/Tag/UpdateTagAction.php b/module/Rest/src/Action/Tag/UpdateTagAction.php index 64ec7a21..175ed0a0 100644 --- a/module/Rest/src/Action/Tag/UpdateTagAction.php +++ b/module/Rest/src/Action/Tag/UpdateTagAction.php @@ -7,14 +7,10 @@ namespace Shlinkio\Shlink\Rest\Action\Tag; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; +use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\EmptyResponse; -use Zend\Diactoros\Response\JsonResponse; - -use function sprintf; class UpdateTagAction extends AbstractRestAction { @@ -43,21 +39,13 @@ class UpdateTagAction extends AbstractRestAction { $body = $request->getParsedBody(); if (! isset($body['oldName'], $body['newName'])) { - return new JsonResponse([ - 'error' => RestUtils::INVALID_ARGUMENT_ERROR, - 'message' => - 'You have to provide both \'oldName\' and \'newName\' params in order to properly rename the tag', - ], self::STATUS_BAD_REQUEST); + throw ValidationException::fromArray([ + 'oldName' => 'oldName is required', + 'newName' => 'newName is required', + ]); } - try { - $this->tagService->renameTag($body['oldName'], $body['newName']); - return new EmptyResponse(); - } catch (EntityDoesNotExistException $e) { - return new JsonResponse([ - 'error' => RestUtils::NOT_FOUND_ERROR, - 'message' => sprintf('It was not possible to find a tag with name %s', $body['oldName']), - ], self::STATUS_NOT_FOUND); - } + $this->tagService->renameTag($body['oldName'], $body['newName']); + return new EmptyResponse(); } } diff --git a/module/Rest/src/Action/Visit/GetVisitsAction.php b/module/Rest/src/Action/Visit/GetVisitsAction.php index 222a76b8..ca8c2838 100644 --- a/module/Rest/src/Action/Visit/GetVisitsAction.php +++ b/module/Rest/src/Action/Visit/GetVisitsAction.php @@ -7,16 +7,12 @@ namespace Shlinkio\Shlink\Rest\Action\Visit; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Common\Paginator\Util\PaginatorUtilsTrait; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\JsonResponse; -use function sprintf; - class GetVisitsAction extends AbstractRestAction { use PaginatorUtilsTrait; @@ -33,27 +29,13 @@ class GetVisitsAction extends AbstractRestAction $this->visitsTracker = $visitsTracker; } - /** - * @param Request $request - * @return Response - * @throws \InvalidArgumentException - */ public function handle(Request $request): Response { $shortCode = $request->getAttribute('shortCode'); + $visits = $this->visitsTracker->info($shortCode, VisitsParams::fromRawData($request->getQueryParams())); - try { - $visits = $this->visitsTracker->info($shortCode, VisitsParams::fromRawData($request->getQueryParams())); - - return new JsonResponse([ - 'visits' => $this->serializePaginator($visits), - ]); - } catch (InvalidArgumentException $e) { - $this->logger->warning('Provided nonexistent short code {e}', ['e' => $e]); - return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf('Provided short code %s does not exist', $shortCode), - ], self::STATUS_NOT_FOUND); - } + return new JsonResponse([ + 'visits' => $this->serializePaginator($visits), + ]); } } diff --git a/module/Rest/src/Authentication/JWTService.php b/module/Rest/src/Authentication/JWTService.php index 841527e4..77ec6728 100644 --- a/module/Rest/src/Authentication/JWTService.php +++ b/module/Rest/src/Authentication/JWTService.php @@ -12,6 +12,7 @@ use UnexpectedValueException; use function time; +/** @deprecated */ class JWTService implements JWTServiceInterface { /** @var AppOptions */ diff --git a/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php b/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php index d5cd38f9..14735f9a 100644 --- a/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php +++ b/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php @@ -8,7 +8,6 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; -use Shlinkio\Shlink\Rest\Util\RestUtils; class ApiKeyHeaderPlugin implements AuthenticationPluginInterface { @@ -28,14 +27,9 @@ class ApiKeyHeaderPlugin implements AuthenticationPluginInterface public function verify(ServerRequestInterface $request): void { $apiKey = $request->getHeaderLine(self::HEADER_NAME); - if ($this->apiKeyService->check($apiKey)) { - return; + if (! $this->apiKeyService->check($apiKey)) { + throw VerifyAuthenticationException::forInvalidApiKey(); } - - throw VerifyAuthenticationException::withError( - RestUtils::INVALID_API_KEY_ERROR, - 'Provided API key does not exist or is invalid.' - ); } public function update(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface diff --git a/module/Rest/src/Authentication/Plugin/AuthorizationHeaderPlugin.php b/module/Rest/src/Authentication/Plugin/AuthorizationHeaderPlugin.php index e5280ac6..bf75d09b 100644 --- a/module/Rest/src/Authentication/Plugin/AuthorizationHeaderPlugin.php +++ b/module/Rest/src/Authentication/Plugin/AuthorizationHeaderPlugin.php @@ -8,7 +8,6 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Rest\Authentication\JWTServiceInterface; use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Throwable; use function count; @@ -16,6 +15,7 @@ use function explode; use function sprintf; use function strtolower; +/** @deprecated */ class AuthorizationHeaderPlugin implements AuthenticationPluginInterface { public const HEADER_NAME = 'Authorization'; @@ -37,19 +37,13 @@ class AuthorizationHeaderPlugin implements AuthenticationPluginInterface $authToken = $request->getHeaderLine(self::HEADER_NAME); $authTokenParts = explode(' ', $authToken); if (count($authTokenParts) === 1) { - throw VerifyAuthenticationException::withError( - RestUtils::INVALID_AUTHORIZATION_ERROR, - sprintf('You need to provide the Bearer type in the %s header.', self::HEADER_NAME) - ); + throw VerifyAuthenticationException::forMissingAuthType(); } // Make sure the authorization type is Bearer [$authType, $jwt] = $authTokenParts; if (strtolower($authType) !== 'bearer') { - throw VerifyAuthenticationException::withError( - RestUtils::INVALID_AUTHORIZATION_ERROR, - sprintf('Provided authorization type %s is not supported. Use Bearer instead.', $authType) - ); + throw VerifyAuthenticationException::forInvalidAuthType($authType); } try { @@ -57,21 +51,13 @@ class AuthorizationHeaderPlugin implements AuthenticationPluginInterface throw $this->createInvalidTokenError(); } } catch (Throwable $e) { - throw $this->createInvalidTokenError($e); + throw $this->createInvalidTokenError(); } } - private function createInvalidTokenError(?Throwable $prev = null): VerifyAuthenticationException + private function createInvalidTokenError(): VerifyAuthenticationException { - return VerifyAuthenticationException::withError( - RestUtils::INVALID_AUTH_TOKEN_ERROR, - sprintf( - 'Missing or invalid auth token provided. Perform a new authentication request and send provided ' - . 'token on every new request on the %s header', - self::HEADER_NAME - ), - $prev - ); + return VerifyAuthenticationException::forInvalidAuthToken(); } public function update(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface diff --git a/module/Rest/src/Authentication/RequestToHttpAuthPlugin.php b/module/Rest/src/Authentication/RequestToHttpAuthPlugin.php index f0cc0fe4..c10c0321 100644 --- a/module/Rest/src/Authentication/RequestToHttpAuthPlugin.php +++ b/module/Rest/src/Authentication/RequestToHttpAuthPlugin.php @@ -4,9 +4,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Authentication; -use Psr\Container; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException; +use Shlinkio\Shlink\Rest\Exception\MissingAuthenticationException; use function array_filter; use function array_reduce; @@ -30,16 +29,12 @@ class RequestToHttpAuthPlugin implements RequestToHttpAuthPluginInterface } /** - * @throws Container\ContainerExceptionInterface - * @throws NoAuthenticationException + * @throws MissingAuthenticationException */ public function fromRequest(ServerRequestInterface $request): Plugin\AuthenticationPluginInterface { if (! $this->hasAnySupportedHeader($request)) { - throw NoAuthenticationException::fromExpectedTypes([ - Plugin\ApiKeyHeaderPlugin::HEADER_NAME, - Plugin\AuthorizationHeaderPlugin::HEADER_NAME, - ]); + throw MissingAuthenticationException::fromExpectedTypes(self::SUPPORTED_AUTH_HEADERS); } return $this->authPluginManager->get($this->getFirstAvailableHeader($request)); diff --git a/module/Rest/src/Authentication/RequestToHttpAuthPluginInterface.php b/module/Rest/src/Authentication/RequestToHttpAuthPluginInterface.php index 18e68ee2..b8002431 100644 --- a/module/Rest/src/Authentication/RequestToHttpAuthPluginInterface.php +++ b/module/Rest/src/Authentication/RequestToHttpAuthPluginInterface.php @@ -4,15 +4,13 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Authentication; -use Psr\Container; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException; +use Shlinkio\Shlink\Rest\Exception\MissingAuthenticationException; interface RequestToHttpAuthPluginInterface { /** - * @throws Container\ContainerExceptionInterface - * @throws NoAuthenticationException + * @throws MissingAuthenticationException */ public function fromRequest(ServerRequestInterface $request): Plugin\AuthenticationPluginInterface; } diff --git a/module/Rest/src/ConfigProvider.php b/module/Rest/src/ConfigProvider.php index 11e6f811..d942cf51 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}'; + private const ROUTES_PREFIX = '/rest/v{version:1|2}'; public function __invoke() { diff --git a/module/Rest/src/ErrorHandler/JsonErrorResponseGenerator.php b/module/Rest/src/ErrorHandler/JsonErrorResponseGenerator.php deleted file mode 100644 index ca9ed12c..00000000 --- a/module/Rest/src/ErrorHandler/JsonErrorResponseGenerator.php +++ /dev/null @@ -1,44 +0,0 @@ -getStatusCode(); - $responsePhrase = $status < 400 ? 'Internal Server Error' : $response->getReasonPhrase(); - $status = $status < 400 ? self::STATUS_INTERNAL_SERVER_ERROR : $status; - - return new JsonResponse([ - 'error' => $this->responsePhraseToCode($responsePhrase), - 'message' => $responsePhrase, - ], $status); - } - - private function responsePhraseToCode(string $responsePhrase): string - { - return strtoupper(str_replace(' ', '_', $responsePhrase)); - } -} diff --git a/module/Rest/src/Exception/AuthenticationException.php b/module/Rest/src/Exception/AuthenticationException.php index 1613bac5..3e60edb6 100644 --- a/module/Rest/src/Exception/AuthenticationException.php +++ b/module/Rest/src/Exception/AuthenticationException.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Rest\Exception; use Throwable; +/** @deprecated */ class AuthenticationException extends RuntimeException { public static function expiredJWT(?Throwable $prev = null): self diff --git a/module/Rest/src/Exception/MissingAuthenticationException.php b/module/Rest/src/Exception/MissingAuthenticationException.php new file mode 100644 index 00000000..6ed76e2a --- /dev/null +++ b/module/Rest/src/Exception/MissingAuthenticationException.php @@ -0,0 +1,36 @@ +detail = $e->getMessage(); + $e->title = self::TITLE; + $e->type = self::TYPE; + $e->status = StatusCodeInterface::STATUS_UNAUTHORIZED; + $e->additional = ['expectedTypes' => $expectedTypes]; + + return $e; + } +} diff --git a/module/Rest/src/Exception/NoAuthenticationException.php b/module/Rest/src/Exception/NoAuthenticationException.php deleted file mode 100644 index b5e8bfc8..00000000 --- a/module/Rest/src/Exception/NoAuthenticationException.php +++ /dev/null @@ -1,19 +0,0 @@ -errorCode = $errorCode; - $this->publicMessage = $publicMessage; + 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->status = StatusCodeInterface::STATUS_UNAUTHORIZED; + + return $e; } - public static function withError(string $errorCode, string $publicMessage, ?Throwable $prev = null): self + /** @deprecated */ + public static function forInvalidAuthToken(): self { - return new self( - $errorCode, - $publicMessage, - sprintf('Authentication verification failed with the public message "%s"', $publicMessage), - 0, - $prev + $e = new self( + 'Missing or invalid auth token provided. Perform a new authentication request and send provided ' + . 'token on every new request on the Authorization header' ); + + $e->detail = $e->getMessage(); + $e->title = 'Invalid auth token'; + $e->type = 'INVALID_AUTH_TOKEN'; + $e->status = StatusCodeInterface::STATUS_UNAUTHORIZED; + + return $e; } - public function getErrorCode(): string + /** @deprecated */ + public static function forMissingAuthType(): self { - return $this->errorCode; + $e = new self('You need to provide the Bearer type in the Authorization header.'); + + $e->detail = $e->getMessage(); + $e->title = 'Invalid authorization'; + $e->type = 'INVALID_AUTHORIZATION'; + $e->status = StatusCodeInterface::STATUS_UNAUTHORIZED; + + return $e; } - public function getPublicMessage(): string + /** @deprecated */ + public static function forInvalidAuthType(string $providedType): self { - return $this->publicMessage; + $e = new self(sprintf('Provided authorization type %s is not supported. Use Bearer instead.', $providedType)); + + $e->detail = $e->getMessage(); + $e->title = 'Invalid authorization'; + $e->type = 'INVALID_AUTHORIZATION'; + $e->status = StatusCodeInterface::STATUS_UNAUTHORIZED; + + return $e; } } diff --git a/module/Rest/src/Middleware/AuthenticationMiddleware.php b/module/Rest/src/Middleware/AuthenticationMiddleware.php index 855f5490..4fd44bcf 100644 --- a/module/Rest/src/Middleware/AuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/AuthenticationMiddleware.php @@ -6,54 +6,28 @@ namespace Shlinkio\Shlink\Rest\Middleware; use Fig\Http\Message\RequestMethodInterface; use Fig\Http\Message\StatusCodeInterface; -use Psr\Container\ContainerExceptionInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; -use Psr\Log\LoggerInterface; -use Psr\Log\NullLogger; -use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPlugin; use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPluginInterface; -use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException; -use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; -use Shlinkio\Shlink\Rest\Util\RestUtils; -use Zend\Diactoros\Response\JsonResponse; use Zend\Expressive\Router\RouteResult; use function Functional\contains; -use function implode; -use function sprintf; class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterface, RequestMethodInterface { - /** @var LoggerInterface */ - private $logger; /** @var array */ private $routesWhitelist; /** @var RequestToHttpAuthPluginInterface */ private $requestToAuthPlugin; - public function __construct( - RequestToHttpAuthPluginInterface $requestToAuthPlugin, - array $routesWhitelist, - ?LoggerInterface $logger = null - ) { + public function __construct(RequestToHttpAuthPluginInterface $requestToAuthPlugin, array $routesWhitelist) + { $this->routesWhitelist = $routesWhitelist; $this->requestToAuthPlugin = $requestToAuthPlugin; - $this->logger = $logger ?: new NullLogger(); } - /** - * Process an incoming server request and return a response, optionally delegating - * to the next middleware component to create the response. - * - * @param Request $request - * @param RequestHandlerInterface $handler - * - * @return Response - * @throws \InvalidArgumentException - */ public function process(Request $request, RequestHandlerInterface $handler): Response { /** @var RouteResult|null $routeResult */ @@ -67,33 +41,10 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa return $handler->handle($request); } - try { - $plugin = $this->requestToAuthPlugin->fromRequest($request); - } catch (ContainerExceptionInterface | NoAuthenticationException $e) { - $this->logger->warning('Invalid or no authentication provided. {e}', ['e' => $e]); - return $this->createErrorResponse(sprintf( - 'Expected one of the following authentication headers, but none were provided, ["%s"]', - implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS) - )); - } + $plugin = $this->requestToAuthPlugin->fromRequest($request); + $plugin->verify($request); + $response = $handler->handle($request); - try { - $plugin->verify($request); - $response = $handler->handle($request); - return $plugin->update($request, $response); - } catch (VerifyAuthenticationException $e) { - $this->logger->warning('Authentication verification failed. {e}', ['e' => $e]); - return $this->createErrorResponse($e->getPublicMessage(), $e->getErrorCode()); - } - } - - private function createErrorResponse( - string $message, - string $errorCode = RestUtils::INVALID_AUTHORIZATION_ERROR - ): JsonResponse { - return new JsonResponse([ - 'error' => $errorCode, - 'message' => $message, - ], self::STATUS_UNAUTHORIZED); + return $plugin->update($request, $response); } } diff --git a/module/Rest/src/Middleware/BackwardsCompatibleProblemDetailsMiddleware.php b/module/Rest/src/Middleware/BackwardsCompatibleProblemDetailsMiddleware.php new file mode 100644 index 00000000..0812c7e0 --- /dev/null +++ b/module/Rest/src/Middleware/BackwardsCompatibleProblemDetailsMiddleware.php @@ -0,0 +1,84 @@ + 'type', + 'message' => 'detail', + ]; + + /** @var array */ + private $defaultTypeFallbacks; + /** @var int */ + private $jsonFlags; + + public function __construct(array $defaultTypeFallbacks, int $jsonFlags) + { + $this->defaultTypeFallbacks = $defaultTypeFallbacks; + $this->jsonFlags = $jsonFlags; + } + + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + $resp = $handler->handle($request); + if ($resp->getHeaderLine('Content-type') !== 'application/problem+json') { + return $resp; + } + + try { + $body = (string) $resp->getBody(); + $payload = json_decode($body); + } catch (Throwable $e) { + return $resp; + } + + $payload = $this->mapStandardErrorTypes($payload, $resp->getStatusCode()); + + if ($this->isVersionOne($request)) { + $payload = $this->makePayloadBackwardsCompatible($payload); + } + + return new JsonResponse($payload, $resp->getStatusCode(), $resp->getHeaders(), $this->jsonFlags); + } + + private function mapStandardErrorTypes(array $payload, int $respStatusCode): array + { + $type = $payload['type'] ?? ''; + if (strpos($type, 'https://httpstatus.es') === 0) { + $payload['type'] = $this->defaultTypeFallbacks[$respStatusCode] ?? $type; + } + + return $payload; + } + + /** @deprecated When Shlink 2 is released, do not chekc the version */ + private function isVersionOne(ServerRequestInterface $request): bool + { + $path = $request->getUri()->getPath(); + return strpos($path, '/v') === false || strpos($path, '/v1') === 0; + } + + /** @deprecated When Shlink v2 is released, do not map old fields */ + private function makePayloadBackwardsCompatible(array $payload): array + { + return reduce_left(self::BACKWARDS_COMPATIBLE_FIELDS, function (string $newKey, string $oldKey, $c, $acc) { + $acc[$oldKey] = $acc[$newKey]; + return $acc; + }, $payload); + } +} diff --git a/module/Rest/src/Middleware/PathVersionMiddleware.php b/module/Rest/src/Middleware/PathVersionMiddleware.php index c08c8dbb..84921710 100644 --- a/module/Rest/src/Middleware/PathVersionMiddleware.php +++ b/module/Rest/src/Middleware/PathVersionMiddleware.php @@ -15,23 +15,12 @@ class PathVersionMiddleware implements MiddlewareInterface { // TODO The /health endpoint needs this middleware in order to work without the version. // Take it into account if this middleware is ever removed. - - /** - * Process an incoming server request and return a response, optionally delegating - * to the next middleware component to create the response. - * - * @param Request $request - * @param RequestHandlerInterface $handler - * - * @return Response - * @throws \InvalidArgumentException - */ public function process(Request $request, RequestHandlerInterface $handler): Response { $uri = $request->getUri(); $path = $uri->getPath(); - // If the path does not begin with the version number, prepend v1 by default for BC compatibility purposes + // If the path does not begin with the version number, prepend v1 by default for BC purposes if (strpos($path, '/v') !== 0) { $request = $request->withUri($uri->withPath('/v1' . $uri->getPath())); } diff --git a/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php b/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php index 64da53d5..98b64401 100644 --- a/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php +++ b/module/Rest/src/Middleware/ShortUrl/CreateShortUrlContentNegotiationMiddleware.php @@ -21,12 +21,6 @@ class CreateShortUrlContentNegotiationMiddleware implements MiddlewareInterface private const PLAIN_TEXT = 'text'; private const JSON = 'json'; - /** - * Process an incoming server request and return a response, optionally delegating - * response creation to a handler. - * @throws \RuntimeException - * @throws \InvalidArgumentException - */ public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { $response = $handler->handle($request); diff --git a/module/Rest/src/Util/RestUtils.php b/module/Rest/src/Util/RestUtils.php deleted file mode 100644 index d4ad46dc..00000000 --- a/module/Rest/src/Util/RestUtils.php +++ /dev/null @@ -1,48 +0,0 @@ -createShortUrl(['customSlug' => $slug, 'domain' => $domain]); $this->assertEquals(self::STATUS_BAD_REQUEST, $statusCode); - $this->assertEquals(RestUtils::INVALID_SLUG_ERROR, $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 */ @@ -201,6 +216,24 @@ class CreateShortUrlActionTest extends ApiTestCase yield ['http://téstb.shlink.io']; // Redirects to http://tést.shlink.io } + /** @test */ + public function failsToCreateShortUrlWithInvalidOriginalUrl(): void + { + $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(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']); + } + /** * @return array { * @var int $statusCode diff --git a/module/Rest/test-api/Action/DeleteShortUrlActionTest.php b/module/Rest/test-api/Action/DeleteShortUrlActionTest.php new file mode 100644 index 00000000..4680a6f6 --- /dev/null +++ b/module/Rest/test-api/Action/DeleteShortUrlActionTest.php @@ -0,0 +1,49 @@ +callApiWithKey(self::METHOD_DELETE, '/short-urls/invalid'); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); + $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 unprocessableEntityIsReturnedWhenTryingToDeleteUrlWithTooManyVisits(): void + { + // Generate visits first + 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'); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_UNPROCESSABLE_ENTITY, $resp->getStatusCode()); + $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 new file mode 100644 index 00000000..bbcb043a --- /dev/null +++ b/module/Rest/test-api/Action/EditShortUrlActionTest.php @@ -0,0 +1,48 @@ +callApiWithKey(self::METHOD_PATCH, '/short-urls/invalid', [RequestOptions::JSON => []]); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); + $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', + ]]); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_BAD_REQUEST, $resp->getStatusCode()); + $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 new file mode 100644 index 00000000..e2922092 --- /dev/null +++ b/module/Rest/test-api/Action/EditShortUrlTagsActionTest.php @@ -0,0 +1,48 @@ +callApiWithKey(self::METHOD_PUT, '/short-urls/abc123/tags', [RequestOptions::JSON => []]); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_BAD_REQUEST, $resp->getStatusCode()); + $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'], + ]]); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); + $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 new file mode 100644 index 00000000..0db06848 --- /dev/null +++ b/module/Rest/test-api/Action/GetVisitsActionTest.php @@ -0,0 +1,28 @@ +callApiWithKey(self::METHOD_GET, '/short-urls/invalid/visits'); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); + $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 new file mode 100644 index 00000000..e4d45f4a --- /dev/null +++ b/module/Rest/test-api/Action/ResolveShortUrlActionTest.php @@ -0,0 +1,28 @@ +callApiWithKey(self::METHOD_GET, '/short-urls/invalid'); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); + $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 new file mode 100644 index 00000000..8c14774c --- /dev/null +++ b/module/Rest/test-api/Action/UpdateTagActionTest.php @@ -0,0 +1,58 @@ +callApiWithKey(self::METHOD_PUT, '/tags', [RequestOptions::JSON => $body]); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_BAD_REQUEST, $resp->getStatusCode()); + $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 + { + yield [[]]; + yield [['oldName' => 'foo']]; + yield [['newName' => 'foo']]; + } + + /** @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', + ]]); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); + $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 02836c85..3526c5f5 100644 --- a/module/Rest/test-api/Middleware/AuthenticationTest.php +++ b/module/Rest/test-api/Middleware/AuthenticationTest.php @@ -4,9 +4,8 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Middleware; -use Shlinkio\Shlink\Rest\Authentication\Plugin\ApiKeyHeaderPlugin; +use Shlinkio\Shlink\Rest\Authentication\Plugin; use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPlugin; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; use function implode; @@ -17,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(RestUtils::INVALID_AUTHORIZATION_ERROR, $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']); } /** @@ -37,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' => [ - ApiKeyHeaderPlugin::HEADER_NAME => $apiKey, + 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(RestUtils::INVALID_API_KEY_ERROR, $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 @@ -55,4 +63,53 @@ class AuthenticationTest extends ApiTestCase yield 'key which is expired' => ['expired_api_key']; yield 'key which is disabled' => ['disabled_api_key']; } + + /** + * @test + * @dataProvider provideInvalidAuthorizations + */ + public function authorizationErrorIsReturnedIfInvalidDataIsProvided( + string $authValue, + string $expectedDetail, + string $expectedType, + string $expectedTitle + ): void { + $resp = $this->callApi(self::METHOD_GET, '/short-codes', [ + 'headers' => [ + Plugin\AuthorizationHeaderPlugin::HEADER_NAME => $authValue, + ], + ]); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_UNAUTHORIZED, $resp->getStatusCode()); + $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 + { + yield 'no type' => [ + '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', + ]; + } } diff --git a/module/Rest/test/Action/HealthActionFactoryTest.php b/module/Rest/test/Action/HealthActionFactoryTest.php deleted file mode 100644 index 3ecb15c1..00000000 --- a/module/Rest/test/Action/HealthActionFactoryTest.php +++ /dev/null @@ -1,44 +0,0 @@ -factory = new Action\HealthActionFactory(); - } - - /** @test */ - public function serviceIsCreatedExtractingConnectionFromEntityManager() - { - $em = $this->prophesize(EntityManager::class); - $conn = $this->prophesize(Connection::class); - - $getConnection = $em->getConnection()->willReturn($conn->reveal()); - - $sm = new ServiceManager(['services' => [ - 'Logger_Shlink' => $this->prophesize(LoggerInterface::class)->reveal(), - AppOptions::class => $this->prophesize(AppOptions::class)->reveal(), - EntityManager::class => $em->reveal(), - ]]); - - $instance = ($this->factory)($sm, ''); - - $this->assertInstanceOf(Action\HealthAction::class, $instance); - $getConnection->shouldHaveBeenCalledOnce(); - } -} diff --git a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php index cea67f0f..92a3c2aa 100644 --- a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php @@ -4,18 +4,13 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Action\ShortUrl; -use Exception; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\InvalidUrlException; -use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; -use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Rest\Action\ShortUrl\CreateShortUrlAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; -use Zend\Diactoros\Response\JsonResponse; use Zend\Diactoros\ServerRequest; use Zend\Diactoros\Uri; @@ -42,8 +37,8 @@ class CreateShortUrlActionTest extends TestCase /** @test */ public function missingLongUrlParamReturnsError(): void { - $response = $this->action->handle(new ServerRequest()); - $this->assertEquals(400, $response->getStatusCode()); + $this->expectException(ValidationException::class); + $this->action->handle(new ServerRequest()); } /** @test */ @@ -62,21 +57,6 @@ class CreateShortUrlActionTest extends TestCase $this->assertTrue(strpos($response->getBody()->getContents(), $shortUrl->toString(self::DOMAIN_CONFIG)) > 0); } - /** @test */ - public function anInvalidUrlReturnsError(): void - { - $this->urlShortener->urlToShortCode(Argument::type(Uri::class), Argument::type('array'), Argument::cetera()) - ->willThrow(InvalidUrlException::class) - ->shouldBeCalledOnce(); - - $request = (new ServerRequest())->withParsedBody([ - 'longUrl' => 'http://www.domain.com/foo/bar', - ]); - $response = $this->action->handle($request); - $this->assertEquals(400, $response->getStatusCode()); - $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::INVALID_URL_ERROR) > 0); - } - /** * @test * @dataProvider provideInvalidDomains @@ -90,13 +70,11 @@ class CreateShortUrlActionTest extends TestCase 'longUrl' => 'http://www.domain.com/foo/bar', 'domain' => $domain, ]); - /** @var JsonResponse $response */ - $response = $this->action->handle($request); - $payload = $response->getPayload(); - $this->assertEquals(400, $response->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_ARGUMENT_ERROR, $payload['error']); - $urlToShortCode->shouldNotHaveBeenCalled(); + $this->expectException(ValidationException::class); + $urlToShortCode->shouldNotBeCalled(); + + $this->action->handle($request); } public function provideInvalidDomains(): iterable @@ -105,38 +83,4 @@ class CreateShortUrlActionTest extends TestCase yield ['127.0.0.1']; yield ['???/&%$&']; } - - /** @test */ - public function nonUniqueSlugReturnsError(): void - { - $this->urlShortener->urlToShortCode( - Argument::type(Uri::class), - Argument::type('array'), - ShortUrlMeta::createFromRawData(['customSlug' => 'foo']), - Argument::cetera() - )->willThrow(NonUniqueSlugException::class)->shouldBeCalledOnce(); - - $request = (new ServerRequest())->withParsedBody([ - 'longUrl' => 'http://www.domain.com/foo/bar', - 'customSlug' => 'foo', - ]); - $response = $this->action->handle($request); - $this->assertEquals(400, $response->getStatusCode()); - $this->assertStringContainsString(RestUtils::INVALID_SLUG_ERROR, (string) $response->getBody()); - } - - /** @test */ - public function aGenericExceptionWillReturnError(): void - { - $this->urlShortener->urlToShortCode(Argument::type(Uri::class), Argument::type('array'), Argument::cetera()) - ->willThrow(Exception::class) - ->shouldBeCalledOnce(); - - $request = (new ServerRequest())->withParsedBody([ - 'longUrl' => 'http://www.domain.com/foo/bar', - ]); - $response = $this->action->handle($request); - $this->assertEquals(500, $response->getStatusCode()); - $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::UNKNOWN_ERROR) > 0); - } } diff --git a/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php index 35f87dc8..782181ce 100644 --- a/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php @@ -7,12 +7,8 @@ namespace ShlinkioTest\Shlink\Rest\Action\ShortUrl; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Core\Exception; use Shlinkio\Shlink\Core\Service\ShortUrl\DeleteShortUrlServiceInterface; use Shlinkio\Shlink\Rest\Action\ShortUrl\DeleteShortUrlAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; -use Throwable; -use Zend\Diactoros\Response\JsonResponse; use Zend\Diactoros\ServerRequest; class DeleteShortUrlActionTest extends TestCase @@ -39,31 +35,4 @@ class DeleteShortUrlActionTest extends TestCase $this->assertEquals(204, $resp->getStatusCode()); $deleteByShortCode->shouldHaveBeenCalledOnce(); } - - /** - * @test - * @dataProvider provideExceptions - */ - public function returnsErrorResponseInCaseOfException(Throwable $e, string $error, int $statusCode): void - { - $deleteByShortCode = $this->service->deleteByShortCode(Argument::any())->willThrow($e); - - /** @var JsonResponse $resp */ - $resp = $this->action->handle(new ServerRequest()); - $payload = $resp->getPayload(); - - $this->assertEquals($statusCode, $resp->getStatusCode()); - $this->assertEquals($error, $payload['error']); - $deleteByShortCode->shouldHaveBeenCalledOnce(); - } - - public function provideExceptions(): iterable - { - yield 'not found' => [new Exception\InvalidShortCodeException(), RestUtils::INVALID_SHORTCODE_ERROR, 404]; - yield 'bad request' => [ - new Exception\DeleteShortUrlException(5), - RestUtils::INVALID_SHORTCODE_DELETION_ERROR, - 400, - ]; - } } diff --git a/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php index d8e62c20..ff86117e 100644 --- a/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php @@ -8,11 +8,9 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Rest\Action\ShortUrl\EditShortUrlAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; -use Zend\Diactoros\Response\JsonResponse; use Zend\Diactoros\ServerRequest; class EditShortUrlActionTest extends TestCase @@ -29,44 +27,19 @@ class EditShortUrlActionTest extends TestCase } /** @test */ - public function invalidDataReturnsError() + public function invalidDataThrowsError(): void { $request = (new ServerRequest())->withParsedBody([ 'maxVisits' => 'invalid', ]); - /** @var JsonResponse $resp */ - $resp = $this->action->handle($request); - $payload = $resp->getPayload(); + $this->expectException(ValidationException::class); - $this->assertEquals(400, $resp->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_ARGUMENT_ERROR, $payload['error']); - $this->assertEquals('Provided data is invalid.', $payload['message']); + $this->action->handle($request); } /** @test */ - public function incorrectShortCodeReturnsError() - { - $request = (new ServerRequest())->withAttribute('shortCode', 'abc123') - ->withParsedBody([ - 'maxVisits' => 5, - ]); - $updateMeta = $this->shortUrlService->updateMetadataByShortCode(Argument::cetera())->willThrow( - InvalidShortCodeException::class - ); - - /** @var JsonResponse $resp */ - $resp = $this->action->handle($request); - $payload = $resp->getPayload(); - - $this->assertEquals(404, $resp->getStatusCode()); - $this->assertEquals(RestUtils::INVALID_SHORTCODE_ERROR, $payload['error']); - $this->assertEquals('No URL found for short code "abc123"', $payload['message']); - $updateMeta->shouldHaveBeenCalled(); - } - - /** @test */ - public function correctShortCodeReturnsSuccess() + public function correctShortCodeReturnsSuccess(): void { $request = (new ServerRequest())->withAttribute('shortCode', 'abc123') ->withParsedBody([ diff --git a/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php b/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php index d56bbaf1..17293f05 100644 --- a/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php @@ -7,7 +7,7 @@ namespace ShlinkioTest\Shlink\Rest\Action\ShortUrl; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Service\ShortUrlService; use Shlinkio\Shlink\Rest\Action\ShortUrl\EditShortUrlTagsAction; use Zend\Diactoros\ServerRequest; @@ -26,28 +26,14 @@ class EditShortUrlTagsActionTest extends TestCase } /** @test */ - public function notProvidingTagsReturnsError() + public function notProvidingTagsReturnsError(): void { - $response = $this->action->handle((new ServerRequest())->withAttribute('shortCode', 'abc123')); - $this->assertEquals(400, $response->getStatusCode()); + $this->expectException(ValidationException::class); + $this->action->handle((new ServerRequest())->withAttribute('shortCode', 'abc123')); } /** @test */ - public function anInvalidShortCodeReturnsNotFound() - { - $shortCode = 'abc123'; - $this->shortUrlService->setTagsByShortCode($shortCode, [])->willThrow(InvalidShortCodeException::class) - ->shouldBeCalledOnce(); - - $response = $this->action->handle( - (new ServerRequest())->withAttribute('shortCode', 'abc123') - ->withParsedBody(['tags' => []]) - ); - $this->assertEquals(404, $response->getStatusCode()); - } - - /** @test */ - public function tagsListIsReturnedIfCorrectShortCodeIsProvided() + public function tagsListIsReturnedIfCorrectShortCodeIsProvided(): void { $shortCode = 'abc123'; $this->shortUrlService->setTagsByShortCode($shortCode, [])->willReturn(new ShortUrl('')) diff --git a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php index 7bb2d623..4197aba8 100644 --- a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Action\ShortUrl; -use Exception; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Psr\Log\LoggerInterface; @@ -79,26 +78,4 @@ class ListShortUrlsActionTest extends TestCase 'tags' => $tags = ['one', 'two'], ], 2, null, $tags, $orderBy]; } - - /** @test */ - public function anExceptionReturnsErrorResponse(): void - { - $page = 3; - $e = new Exception(); - - $this->service->listShortUrls($page, null, [], null)->willThrow($e) - ->shouldBeCalledOnce(); - $logError = $this->logger->error( - 'Unexpected error while listing short URLs. {e}', - ['e' => $e] - )->will(function () { - }); - - $response = $this->action->handle((new ServerRequest())->withQueryParams([ - 'page' => $page, - ])); - - $this->assertEquals(500, $response->getStatusCode()); - $logError->shouldHaveBeenCalledOnce(); - } } diff --git a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php index e0c23d31..b77a6e45 100644 --- a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php @@ -4,15 +4,11 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Action\ShortUrl; -use Exception; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; -use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Rest\Action\ShortUrl\ResolveShortUrlAction; -use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\ServerRequest; use function strpos; @@ -30,19 +26,6 @@ class ResolveShortUrlActionTest extends TestCase $this->action = new ResolveShortUrlAction($this->urlShortener->reveal(), []); } - /** @test */ - public function incorrectShortCodeReturnsError(): void - { - $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(EntityDoesNotExistException::class) - ->shouldBeCalledOnce(); - - $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); - $response = $this->action->handle($request); - $this->assertEquals(404, $response->getStatusCode()); - $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::INVALID_ARGUMENT_ERROR) > 0); - } - /** @test */ public function correctShortCodeReturnsSuccess(): void { @@ -56,30 +39,4 @@ class ResolveShortUrlActionTest extends TestCase $this->assertEquals(200, $response->getStatusCode()); $this->assertTrue(strpos($response->getBody()->getContents(), 'http://domain.com/foo/bar') > 0); } - - /** @test */ - public function invalidShortCodeExceptionReturnsError(): void - { - $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(InvalidShortCodeException::class) - ->shouldBeCalledOnce(); - - $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); - $response = $this->action->handle($request); - $this->assertEquals(400, $response->getStatusCode()); - $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::INVALID_SHORTCODE_ERROR) > 0); - } - - /** @test */ - public function unexpectedExceptionWillReturnError(): void - { - $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(Exception::class) - ->shouldBeCalledOnce(); - - $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); - $response = $this->action->handle($request); - $this->assertEquals(500, $response->getStatusCode()); - $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::UNKNOWN_ERROR) > 0); - } } diff --git a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php index f3452e9a..b0d8c65d 100644 --- a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php @@ -10,11 +10,11 @@ use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\UriInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Rest\Action\ShortUrl\SingleStepCreateShortUrlAction; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; -use Zend\Diactoros\Response\JsonResponse; use Zend\Diactoros\ServerRequest; class SingleStepCreateShortUrlActionTest extends TestCase @@ -42,39 +42,31 @@ class SingleStepCreateShortUrlActionTest extends TestCase } /** @test */ - public function errorResponseIsReturnedIfInvalidApiKeyIsProvided() + public function errorResponseIsReturnedIfInvalidApiKeyIsProvided(): void { $request = (new ServerRequest())->withQueryParams(['apiKey' => 'abc123']); $findApiKey = $this->apiKeyService->check('abc123')->willReturn(false); - /** @var JsonResponse $resp */ - $resp = $this->action->handle($request); - $payload = $resp->getPayload(); + $this->expectException(ValidationException::class); + $findApiKey->shouldBeCalledOnce(); - $this->assertEquals(400, $resp->getStatusCode()); - $this->assertEquals('INVALID_ARGUMENT', $payload['error']); - $this->assertEquals('No API key was provided or it is not valid', $payload['message']); - $findApiKey->shouldHaveBeenCalled(); + $this->action->handle($request); } /** @test */ - public function errorResponseIsReturnedIfNoUrlIsProvided() + public function errorResponseIsReturnedIfNoUrlIsProvided(): void { $request = (new ServerRequest())->withQueryParams(['apiKey' => 'abc123']); $findApiKey = $this->apiKeyService->check('abc123')->willReturn(true); - /** @var JsonResponse $resp */ - $resp = $this->action->handle($request); - $payload = $resp->getPayload(); + $this->expectException(ValidationException::class); + $findApiKey->shouldBeCalledOnce(); - $this->assertEquals(400, $resp->getStatusCode()); - $this->assertEquals('INVALID_ARGUMENT', $payload['error']); - $this->assertEquals('A URL was not provided', $payload['message']); - $findApiKey->shouldHaveBeenCalled(); + $this->action->handle($request); } /** @test */ - public function properDataIsPassedWhenGeneratingShortCode() + public function properDataIsPassedWhenGeneratingShortCode(): void { $request = (new ServerRequest())->withQueryParams([ 'apiKey' => 'abc123', diff --git a/module/Rest/test/Action/Tag/UpdateTagActionTest.php b/module/Rest/test/Action/Tag/UpdateTagActionTest.php index 9ded1716..3b068add 100644 --- a/module/Rest/test/Action/Tag/UpdateTagActionTest.php +++ b/module/Rest/test/Action/Tag/UpdateTagActionTest.php @@ -7,7 +7,7 @@ namespace ShlinkioTest\Shlink\Rest\Action\Tag; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\Tag; -use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; +use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\Tag\UpdateTagAction; use Zend\Diactoros\ServerRequest; @@ -32,9 +32,10 @@ class UpdateTagActionTest extends TestCase public function whenInvalidParamsAreProvidedAnErrorIsReturned(array $bodyParams): void { $request = (new ServerRequest())->withParsedBody($bodyParams); - $resp = $this->action->handle($request); - $this->assertEquals(400, $resp->getStatusCode()); + $this->expectException(ValidationException::class); + + $this->action->handle($request); } public function provideParams(): iterable @@ -44,21 +45,6 @@ class UpdateTagActionTest extends TestCase yield 'no params' => [[]]; } - /** @test */ - public function requestingInvalidTagReturnsError(): void - { - $request = (new ServerRequest())->withParsedBody([ - 'oldName' => 'foo', - 'newName' => 'bar', - ]); - $rename = $this->tagService->renameTag('foo', 'bar')->willThrow(EntityDoesNotExistException::class); - - $resp = $this->action->handle($request); - - $this->assertEquals(404, $resp->getStatusCode()); - $rename->shouldHaveBeenCalled(); - } - /** @test */ public function correctInvocationRenamesTag(): void { diff --git a/module/Rest/test/Action/Visit/GetVisitsActionTest.php b/module/Rest/test/Action/Visit/GetVisitsActionTest.php index 6a4b533c..80e06085 100644 --- a/module/Rest/test/Action/Visit/GetVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/GetVisitsActionTest.php @@ -8,7 +8,6 @@ use Cake\Chronos\Chronos; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Service\VisitsTracker; @@ -31,7 +30,7 @@ class GetVisitsActionTest extends TestCase } /** @test */ - public function providingCorrectShortCodeReturnsVisits() + public function providingCorrectShortCodeReturnsVisits(): void { $shortCode = 'abc123'; $this->visitsTracker->info($shortCode, Argument::type(VisitsParams::class))->willReturn( @@ -43,19 +42,7 @@ class GetVisitsActionTest extends TestCase } /** @test */ - public function providingInvalidShortCodeReturnsError() - { - $shortCode = 'abc123'; - $this->visitsTracker->info($shortCode, Argument::type(VisitsParams::class))->willThrow( - InvalidArgumentException::class - )->shouldBeCalledOnce(); - - $response = $this->action->handle((new ServerRequest())->withAttribute('shortCode', $shortCode)); - $this->assertEquals(404, $response->getStatusCode()); - } - - /** @test */ - public function paramsAreReadFromQuery() + public function paramsAreReadFromQuery(): void { $shortCode = 'abc123'; $this->visitsTracker->info($shortCode, new VisitsParams( diff --git a/module/Rest/test/Authentication/RequestToAuthPluginTest.php b/module/Rest/test/Authentication/RequestToAuthPluginTest.php index e9d68489..8d11b4d8 100644 --- a/module/Rest/test/Authentication/RequestToAuthPluginTest.php +++ b/module/Rest/test/Authentication/RequestToAuthPluginTest.php @@ -11,7 +11,7 @@ use Shlinkio\Shlink\Rest\Authentication\Plugin\ApiKeyHeaderPlugin; use Shlinkio\Shlink\Rest\Authentication\Plugin\AuthenticationPluginInterface; use Shlinkio\Shlink\Rest\Authentication\Plugin\AuthorizationHeaderPlugin; use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPlugin; -use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException; +use Shlinkio\Shlink\Rest\Exception\MissingAuthenticationException; use Zend\Diactoros\ServerRequest; use function implode; @@ -35,9 +35,9 @@ class RequestToAuthPluginTest extends TestCase { $request = new ServerRequest(); - $this->expectException(NoAuthenticationException::class); + $this->expectException(MissingAuthenticationException::class); $this->expectExceptionMessage(sprintf( - 'None of the valid authentication mechanisms where provided. Expected one of ["%s"]', + 'Expected one of the following authentication headers, but none were provided, ["%s"]', implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS) )); diff --git a/module/Rest/test/ConfigProviderTest.php b/module/Rest/test/ConfigProviderTest.php index 57959367..3cd574b3 100644 --- a/module/Rest/test/ConfigProviderTest.php +++ b/module/Rest/test/ConfigProviderTest.php @@ -18,11 +18,10 @@ class ConfigProviderTest extends TestCase } /** @test */ - public function properConfigIsReturned() + public function properConfigIsReturned(): void { $config = $this->configProvider->__invoke(); - $this->assertArrayHasKey('error_handler', $config); $this->assertArrayHasKey('routes', $config); $this->assertArrayHasKey('dependencies', $config); } diff --git a/module/Rest/test/ErrorHandler/JsonErrorResponseGeneratorTest.php b/module/Rest/test/ErrorHandler/JsonErrorResponseGeneratorTest.php deleted file mode 100644 index d9b99bdd..00000000 --- a/module/Rest/test/ErrorHandler/JsonErrorResponseGeneratorTest.php +++ /dev/null @@ -1,62 +0,0 @@ -errorHandler = new JsonErrorResponseGenerator(); - } - - /** @test */ - public function noErrorStatusReturnsInternalServerError(): void - { - /** @var Response\JsonResponse $response */ - $response = $this->errorHandler->__invoke(null, new ServerRequest(), new Response()); - $payload = $response->getPayload(); - - $this->assertInstanceOf(Response\JsonResponse::class, $response); - $this->assertEquals(500, $response->getStatusCode()); - $this->assertEquals('Internal Server Error', $payload['message']); - } - - /** - * @test - * @dataProvider provideStatus - */ - public function errorStatusReturnsThatStatus(int $status, string $message): void - { - /** @var Response\JsonResponse $response */ - $response = $this->errorHandler->__invoke( - null, - new ServerRequest(), - (new Response())->withStatus($status, $message) - ); - $payload = $response->getPayload(); - - $this->assertInstanceOf(Response\JsonResponse::class, $response); - $this->assertEquals($status, $response->getStatusCode()); - $this->assertEquals($message, $payload['message']); - } - - public function provideStatus(): iterable - { - return array_map(function (int $status) { - return [$status, 'Some message']; - }, range(400, 500, 20)); - } -} diff --git a/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php b/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php index d19fe0f6..28563c5f 100644 --- a/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php +++ b/module/Rest/test/Exception/VerifyAuthenticationExceptionTest.php @@ -4,92 +4,16 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Exception; -use Exception; use PHPUnit\Framework\TestCase; -use Shlinkio\Shlink\Common\Util\StringUtilsTrait; use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; -use Throwable; - -use function array_map; -use function random_int; -use function range; -use function sprintf; class VerifyAuthenticationExceptionTest extends TestCase { - use StringUtilsTrait; - - /** - * @test - * @dataProvider provideExceptionData - */ - public function withErrorCreatesExpectedException(string $code, string $message, ?Throwable $prev): void - { - $e = VerifyAuthenticationException::withError($code, $message, $prev); - - $this->assertEquals(0, $e->getCode()); - $this->assertEquals( - sprintf('Authentication verification failed with the public message "%s"', $message), - $e->getMessage() - ); - $this->assertEquals($code, $e->getErrorCode()); - $this->assertEquals($message, $e->getPublicMessage()); - $this->assertEquals($prev, $e->getPrevious()); - } - - public function provideExceptionData(): iterable - { - return array_map(function () { - return [ - $this->generateRandomString(), - $this->generateRandomString(50), - random_int(0, 1) === 1 ? new Exception('Prev') : null, - ]; - }, range(1, 10)); - } - - /** - * @test - * @dataProvider provideConstructorData - */ - public function constructCreatesExpectedException( - string $errorCode, - string $publicMessage, - string $message, - int $code, - ?Throwable $prev - ): void { - $e = new VerifyAuthenticationException($errorCode, $publicMessage, $message, $code, $prev); - - $this->assertEquals($code, $e->getCode()); - $this->assertEquals($message, $e->getMessage()); - $this->assertEquals($errorCode, $e->getErrorCode()); - $this->assertEquals($publicMessage, $e->getPublicMessage()); - $this->assertEquals($prev, $e->getPrevious()); - } - - public function provideConstructorData(): iterable - { - return array_map(function (int $i) { - return [ - $this->generateRandomString(), - $this->generateRandomString(30), - $this->generateRandomString(50), - $i, - random_int(0, 1) === 1 ? new Exception('Prev') : null, - ]; - }, range(10, 20)); - } - /** @test */ - public function defaultConstructorValuesAreKept(): void + public function createsExpectedExceptionForInvalidApiKey(): void { - $e = new VerifyAuthenticationException('foo', 'bar'); + $e = VerifyAuthenticationException::forInvalidApiKey(); - $this->assertEquals(0, $e->getCode()); - $this->assertEquals('', $e->getMessage()); - $this->assertEquals('foo', $e->getErrorCode()); - $this->assertEquals('bar', $e->getPublicMessage()); - $this->assertNull($e->getPrevious()); + $this->assertEquals('Provided API key does not exist or is invalid.', $e->getMessage()); } } diff --git a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php index e373fbea..36dfaeee 100644 --- a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php @@ -4,12 +4,10 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Middleware; -use Exception; use Fig\Http\Message\RequestMethodInterface; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; -use Psr\Container\ContainerExceptionInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; @@ -17,20 +15,13 @@ use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Rest\Action\AuthenticateAction; use Shlinkio\Shlink\Rest\Authentication\Plugin\AuthenticationPluginInterface; -use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPlugin; use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPluginInterface; -use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException; -use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; -use Shlinkio\Shlink\Rest\Util\RestUtils; -use Throwable; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequest; use Zend\Expressive\Router\Route; use Zend\Expressive\Router\RouteResult; -use function implode; -use function sprintf; use function Zend\Stratigility\middleware; class AuthenticationMiddlewareTest extends TestCase @@ -93,72 +84,6 @@ class AuthenticationMiddlewareTest extends TestCase )->withMethod(RequestMethodInterface::METHOD_OPTIONS)]; } - /** - * @test - * @dataProvider provideExceptions - */ - public function errorIsReturnedWhenNoValidAuthIsProvided(Throwable $e): void - { - $request = (new ServerRequest())->withAttribute( - RouteResult::class, - RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []) - ); - $fromRequest = $this->requestToPlugin->fromRequest(Argument::any())->willThrow($e); - $logWarning = $this->logger->warning('Invalid or no authentication provided. {e}', ['e' => $e])->will( - function () { - } - ); - - /** @var Response\JsonResponse $response */ - $response = $this->middleware->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal()); - $payload = $response->getPayload(); - - $this->assertEquals(RestUtils::INVALID_AUTHORIZATION_ERROR, $payload['error']); - $this->assertEquals(sprintf( - 'Expected one of the following authentication headers, but none were provided, ["%s"]', - implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS) - ), $payload['message']); - $fromRequest->shouldHaveBeenCalledOnce(); - $logWarning->shouldHaveBeenCalledOnce(); - } - - public function provideExceptions(): iterable - { - $containerException = new class extends Exception implements ContainerExceptionInterface { - }; - - yield 'container exception' => [$containerException]; - yield 'authentication exception' => [NoAuthenticationException::fromExpectedTypes([])]; - } - - /** @test */ - public function errorIsReturnedWhenVerificationFails(): void - { - $request = (new ServerRequest())->withAttribute( - RouteResult::class, - RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []) - ); - $e = VerifyAuthenticationException::withError('the_error', 'the_message'); - $plugin = $this->prophesize(AuthenticationPluginInterface::class); - - $verify = $plugin->verify($request)->willThrow($e); - $fromRequest = $this->requestToPlugin->fromRequest(Argument::any())->willReturn($plugin->reveal()); - $logWarning = $this->logger->warning('Authentication verification failed. {e}', ['e' => $e])->will( - function () { - } - ); - - /** @var Response\JsonResponse $response */ - $response = $this->middleware->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal()); - $payload = $response->getPayload(); - - $this->assertEquals('the_error', $payload['error']); - $this->assertEquals('the_message', $payload['message']); - $verify->shouldHaveBeenCalledOnce(); - $fromRequest->shouldHaveBeenCalledOnce(); - $logWarning->shouldHaveBeenCalledOnce(); - } - /** @test */ public function updatedResponseIsReturnedWhenVerificationPasses(): void { diff --git a/module/Rest/test/Middleware/BackwardsCompatibleProblemDetailsMiddlewareTest.php b/module/Rest/test/Middleware/BackwardsCompatibleProblemDetailsMiddlewareTest.php new file mode 100644 index 00000000..4d47c4cb --- /dev/null +++ b/module/Rest/test/Middleware/BackwardsCompatibleProblemDetailsMiddlewareTest.php @@ -0,0 +1,122 @@ +handler = $this->prophesize(RequestHandlerInterface::class); + $this->middleware = new BackwardsCompatibleProblemDetailsMiddleware([ + 404 => 'NOT_FOUND', + 500 => 'INTERNAL_SERVER_ERROR', + ], 0); + } + + /** + * @test + * @dataProvider provideNonProcessableResponses + */ + public function nonProblemDetailsOrInvalidResponsesAreReturnedAsTheyAre(Response $response): void + { + $request = ServerRequestFactory::fromGlobals(); + $response = new Response(); + $handle = $this->handler->handle($request)->willReturn($response); + + $result = $this->middleware->process($request, $this->handler->reveal()); + + $this->assertSame($response, $result); + $handle->shouldHaveBeenCalledOnce(); + } + + public function provideNonProcessableResponses(): iterable + { + yield 'no problem details' => [new Response()]; + yield 'invalid JSON' => [(new Response('data://text/plain,{invalid-json'))->withHeader( + 'Content-Type', + 'application/problem+json' + )]; + } + + /** + * @test + * @dataProvider provideStatusAndTypes + */ + public function properlyMapsTypesBasedOnResponseStatus(Response\JsonResponse $response, string $expectedType): void + { + $request = ServerRequestFactory::fromGlobals()->withUri(new Uri('/v2/something')); + $handle = $this->handler->handle($request)->willReturn($response); + + /** @var Response\JsonResponse $result */ + $result = $this->middleware->process($request, $this->handler->reveal()); + $payload = $result->getPayload(); + + $this->assertEquals($expectedType, $payload['type']); + $this->assertArrayNotHasKey('error', $payload); + $this->assertArrayNotHasKey('message', $payload); + $handle->shouldHaveBeenCalledOnce(); + } + + public function provideStatusAndTypes(): iterable + { + yield [$this->jsonResponse(['type' => 'https://httpstatus.es/404'], 404), 'NOT_FOUND']; + yield [$this->jsonResponse(['type' => 'https://httpstatus.es/500'], 500), 'INTERNAL_SERVER_ERROR']; + yield [$this->jsonResponse(['type' => 'https://httpstatus.es/504'], 504), 'https://httpstatus.es/504']; + yield [$this->jsonResponse(['type' => 'something_else'], 404), 'something_else']; + yield [$this->jsonResponse(['type' => 'something_else'], 500), 'something_else']; + yield [$this->jsonResponse(['type' => 'something_else'], 504), 'something_else']; + } + + /** + * @test + * @dataProvider provideRequestPath + */ + public function mapsDeprecatedPropertiesWhenRequestIsPerformedToVersionOne(string $requestPath): void + { + $request = ServerRequestFactory::fromGlobals()->withUri(new Uri($requestPath)); + $response = $this->jsonResponse([ + 'type' => 'the_type', + 'detail' => 'the_detail', + ]); + $handle = $this->handler->handle($request)->willReturn($response); + + /** @var Response\JsonResponse $result */ + $result = $this->middleware->process($request, $this->handler->reveal()); + $payload = $result->getPayload(); + + $this->assertEquals([ + 'type' => 'the_type', + 'detail' => 'the_detail', + 'error' => 'the_type', + 'message' => 'the_detail', + ], $payload); + $handle->shouldHaveBeenCalledOnce(); + } + + public function provideRequestPath(): iterable + { + yield 'no version' => ['/foo']; + yield 'version one' => ['/v1/foo']; + } + + private function jsonResponse(array $payload, int $status = 200): Response\JsonResponse + { + $headers = ['Content-Type' => 'application/problem+json']; + return new Response\JsonResponse($payload, $status, $headers); + } +} diff --git a/module/Rest/test/Util/RestUtilsTest.php b/module/Rest/test/Util/RestUtilsTest.php deleted file mode 100644 index c958594b..00000000 --- a/module/Rest/test/Util/RestUtilsTest.php +++ /dev/null @@ -1,41 +0,0 @@ -assertEquals( - RestUtils::INVALID_SHORTCODE_ERROR, - RestUtils::getRestErrorCodeFromException(new InvalidShortCodeException()) - ); - $this->assertEquals( - RestUtils::INVALID_URL_ERROR, - RestUtils::getRestErrorCodeFromException(new InvalidUrlException()) - ); - $this->assertEquals( - RestUtils::INVALID_ARGUMENT_ERROR, - RestUtils::getRestErrorCodeFromException(new InvalidArgumentException()) - ); - $this->assertEquals( - RestUtils::INVALID_CREDENTIALS_ERROR, - RestUtils::getRestErrorCodeFromException(new AuthenticationException()) - ); - $this->assertEquals( - RestUtils::UNKNOWN_ERROR, - RestUtils::getRestErrorCodeFromException(new WrongIpException()) - ); - } -} diff --git a/phpstan.neon b/phpstan.neon index 7df6d88f..2d9d960d 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,6 +1,5 @@ parameters: ignoreErrors: - - '#is not subtype of Throwable#' - '#Undefined variable: \$metadata#' - '#AbstractQuery::setParameters()#' - '#mustRun()#'