From 74854b3dace92e178d35c2c811dc6d909fb98798 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 22 Nov 2019 19:49:14 +0100 Subject: [PATCH] Added zend problem details to the project --- composer.json | 3 +- config/autoload/error-handler.global.php | 26 ++++++++++++++++ .../autoload/middleware-pipeline.global.php | 31 ++++++++++++------- config/config.php | 2 ++ module/Core/src/Response/NotFoundHandler.php | 9 ------ .../test/Response/NotFoundHandlerTest.php | 24 -------------- 6 files changed, 49 insertions(+), 46 deletions(-) create mode 100644 config/autoload/error-handler.global.php diff --git a/composer.json b/composer.json index 5b222782..5d035b60 100644 --- a/composer.json +++ b/composer.json @@ -33,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", @@ -52,6 +52,7 @@ "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" }, diff --git a/config/autoload/error-handler.global.php b/config/autoload/error-handler.global.php new file mode 100644 index 00000000..4ef36e0a --- /dev/null +++ b/config/autoload/error-handler.global.php @@ -0,0 +1,26 @@ + [ + 'listeners' => [Logger\ErrorLogger::class], + ], + + 'dependencies' => [ + 'delegators' => [ + ErrorHandler::class => [ + Logger\ErrorHandlerListenerAttachingDelegator::class, + ], + ProblemDetailsMiddleware::class => [ + Logger\ErrorHandlerListenerAttachingDelegator::class, + ], + ], + ], + +]; diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index 9ea5e260..2508b191 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink; use Zend\Expressive; +use Zend\ProblemDetails; use Zend\Stratigility\Middleware\ErrorHandler; return [ @@ -14,20 +15,19 @@ return [ 'middleware' => [ ErrorHandler::class, ], - 'priority' => 15, ], -// 'error-handler-rest' => [ -// 'path' => '/rest', -// 'middleware' => [], -// 'priority' => 14, -// ], + 'error-handler-rest' => [ + 'path' => '/rest', + 'middleware' => [ + ProblemDetails\ProblemDetailsMiddleware::class, + ], + ], 'pre-routing' => [ 'middleware' => [ Expressive\Helper\ContentLengthMiddleware::class, Common\Middleware\CloseDbConnectionMiddleware::class, ], - 'priority' => 12, ], 'pre-routing-rest' => [ 'path' => '/rest', @@ -35,14 +35,12 @@ return [ Rest\Middleware\PathVersionMiddleware::class, Rest\Middleware\ShortUrl\ShortCodePathMiddleware::class, ], - 'priority' => 11, ], 'routing' => [ 'middleware' => [ Expressive\Router\Middleware\RouteMiddleware::class, ], - 'priority' => 10, ], 'rest' => [ @@ -53,15 +51,24 @@ return [ Rest\Middleware\BodyParserMiddleware::class, Rest\Middleware\AuthenticationMiddleware::class, ], - 'priority' => 5, ], - 'post-routing' => [ + 'dispatch' => [ 'middleware' => [ Expressive\Router\Middleware\DispatchMiddleware::class, + ], + ], + + 'not-found-rest' => [ + 'path' => '/rest', + 'middleware' => [ + ProblemDetails\ProblemDetailsNotFoundHandler::class, + ], + ], + 'not-found' => [ + 'middleware' => [ Core\Response\NotFoundHandler::class, ], - 'priority' => 1, ], ], ]; diff --git a/config/config.php b/config/config.php index 2c817d1e..ad18fd20 100644 --- a/config/config.php +++ b/config/config.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink; use Zend\ConfigAggregator; use Zend\Expressive; +use Zend\ProblemDetails; use function Shlinkio\Shlink\Common\env; @@ -15,6 +16,7 @@ return (new ConfigAggregator\ConfigAggregator([ Expressive\Router\FastRouteRouter\ConfigProvider::class, Expressive\Plates\ConfigProvider::class, Expressive\Swoole\ConfigProvider::class, + ProblemDetails\ConfigProvider::class, Common\ConfigProvider::class, IpGeolocation\ConfigProvider::class, Core\ConfigProvider::class, diff --git a/module/Core/src/Response/NotFoundHandler.php b/module/Core/src/Response/NotFoundHandler.php index 6bf245c1..7b288b54 100644 --- a/module/Core/src/Response/NotFoundHandler.php +++ b/module/Core/src/Response/NotFoundHandler.php @@ -18,7 +18,6 @@ use Zend\Expressive\Template\TemplateRendererInterface; use function array_shift; use function explode; -use function Functional\contains; use function rtrim; class NotFoundHandler implements RequestHandlerInterface @@ -64,14 +63,6 @@ class NotFoundHandler implements RequestHandlerInterface $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); } diff --git a/module/Core/test/Response/NotFoundHandlerTest.php b/module/Core/test/Response/NotFoundHandlerTest.php index 8f1d6f51..50211229 100644 --- a/module/Core/test/Response/NotFoundHandlerTest.php +++ b/module/Core/test/Response/NotFoundHandlerTest.php @@ -13,7 +13,6 @@ use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Core\Response\NotFoundHandler; use Zend\Diactoros\Response; -use Zend\Diactoros\ServerRequest; use Zend\Diactoros\ServerRequestFactory; use Zend\Diactoros\Uri; use Zend\Expressive\Router\Route; @@ -37,29 +36,6 @@ class NotFoundHandlerTest extends TestCase $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