Merge pull request #589 from acelaya-forks/feature/problem-details-upgrade

Updated to problem-details 1.1, removing custom code

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"
     },

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,
     ],
 

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',
         ],
     ],

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) {

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')),
+        ];
     }
 
     /**