From bd6243b2acecfdb51553974946d5a12eaf580ff2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 30 Dec 2019 22:42:29 +0100 Subject: [PATCH] Updated to problem-details 1.1, removing custom code --- composer.json | 2 +- config/autoload/error-handler.global.php | 7 ++- module/Rest/config/dependencies.config.php | 1 - ...ardsCompatibleProblemDetailsMiddleware.php | 27 ++--------- ...CompatibleProblemDetailsMiddlewareTest.php | 48 +++++-------------- 5 files changed, 21 insertions(+), 64 deletions(-) diff --git a/composer.json b/composer.json index 1dddb7f8..2f33ce98 100644 --- a/composer.json +++ b/composer.json @@ -52,7 +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-problem-details": "^1.1", "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 index 964b90de..7f5c91c9 100644 --- a/config/autoload/error-handler.global.php +++ b/config/autoload/error-handler.global.php @@ -8,11 +8,14 @@ use Zend\Stratigility\Middleware\ErrorHandler; return [ - 'backwards_compatible_problem_details' => [ - 'default_type_fallbacks' => [ + 'problem-details' => [ + 'default_types_map' => [ 404 => 'NOT_FOUND', 500 => 'INTERNAL_SERVER_ERROR', ], + ], + + 'backwards_compatible_problem_details' => [ 'json_flags' => JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE | JSON_PRESERVE_ZERO_FRACTION, ], diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 17c30642..ace2fda7 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -78,7 +78,6 @@ return [ 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/src/Middleware/BackwardsCompatibleProblemDetailsMiddleware.php b/module/Rest/src/Middleware/BackwardsCompatibleProblemDetailsMiddleware.php index 65bd12b0..11fce5c9 100644 --- a/module/Rest/src/Middleware/BackwardsCompatibleProblemDetailsMiddleware.php +++ b/module/Rest/src/Middleware/BackwardsCompatibleProblemDetailsMiddleware.php @@ -15,6 +15,7 @@ use function Functional\reduce_left; use function Shlinkio\Shlink\Common\json_decode; use function strpos; +/** @deprecated */ class BackwardsCompatibleProblemDetailsMiddleware implements MiddlewareInterface { private const BACKWARDS_COMPATIBLE_FIELDS = [ @@ -22,56 +23,36 @@ class BackwardsCompatibleProblemDetailsMiddleware implements MiddlewareInterface 'message' => 'detail', ]; - private array $defaultTypeFallbacks; private int $jsonFlags; - public function __construct(array $defaultTypeFallbacks, int $jsonFlags) + public function __construct(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') { + if ($resp->getHeaderLine('Content-type') !== 'application/problem+json' || ! $this->isVersionOne($request)) { return $resp; } try { $body = (string) $resp->getBody(); - $payload = json_decode($body); + $payload = $this->makePayloadBackwardsCompatible(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) { diff --git a/module/Rest/test/Middleware/BackwardsCompatibleProblemDetailsMiddlewareTest.php b/module/Rest/test/Middleware/BackwardsCompatibleProblemDetailsMiddlewareTest.php index 5f58e7eb..20a5b04d 100644 --- a/module/Rest/test/Middleware/BackwardsCompatibleProblemDetailsMiddlewareTest.php +++ b/module/Rest/test/Middleware/BackwardsCompatibleProblemDetailsMiddlewareTest.php @@ -9,6 +9,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Rest\Middleware\BackwardsCompatibleProblemDetailsMiddleware; use Zend\Diactoros\Response; +use Zend\Diactoros\ServerRequest; use Zend\Diactoros\ServerRequestFactory; use Zend\Diactoros\Uri; @@ -20,20 +21,18 @@ class BackwardsCompatibleProblemDetailsMiddlewareTest extends TestCase public function setUp(): void { $this->handler = $this->prophesize(RequestHandlerInterface::class); - $this->middleware = new BackwardsCompatibleProblemDetailsMiddleware([ - 404 => 'NOT_FOUND', - 500 => 'INTERNAL_SERVER_ERROR', - ], 0); + $this->middleware = new BackwardsCompatibleProblemDetailsMiddleware(0); } /** * @test * @dataProvider provideNonProcessableResponses */ - public function nonProblemDetailsOrInvalidResponsesAreReturnedAsTheyAre(Response $response): void - { - $request = ServerRequestFactory::fromGlobals(); - $response = new Response(); + public function nonProblemDetailsOrInvalidResponsesAreReturnedAsTheyAre( + Response $response, + ?ServerRequest $request = null + ): void { + $request = $request ?? ServerRequestFactory::fromGlobals(); $handle = $this->handler->handle($request)->willReturn($response); $result = $this->middleware->process($request, $this->handler->reveal()); @@ -49,35 +48,10 @@ class BackwardsCompatibleProblemDetailsMiddlewareTest extends TestCase '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']; + yield 'version 2' => [ + (new Response())->withHeader('Content-type', 'application/problem+json'), + ServerRequestFactory::fromGlobals()->withUri(new Uri('/v2/something')), + ]; } /**