fix: errors without compatibility flag

soyuka · Sep 20, 2023 · 971466d · 971466d
1 parent b66fdcf
commit 971466d
Show file tree

Hide file tree

Showing 12 changed files with 115 additions and 32 deletions.
diff --git a/features/hydra/error.feature b/features/hydra/error.feature
@@ -42,13 +42,12 @@ Feature: Error handling
     """
     Then the response status code should be 400
     And the response should be in JSON
-    And the header "Content-Type" should be equal to "application/problem+json; charset=utf-8"
+    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
     And the JSON node "@context" should be equal to "/contexts/Error"
     And the JSON node "@type" should be equal to "hydra:Error"
     And the JSON node "hydra:title" should be equal to "An error occurred"
     And the JSON node "hydra:description" should be equal to 'Nested documents for attribute "relatedDummy" are not allowed. Use IRIs instead.'
     And the JSON node "trace" should exist
-    And the header "Link" should contain '<http://www.w3.org/ns/hydra/error>; rel="http://www.w3.org/ns/json-ld#error"'
 
   Scenario: Get an error during deserialization of collection
     When I add "Content-Type" header equal to "application/ld+json"
@@ -63,7 +62,7 @@ Feature: Error handling
     """
     Then the response status code should be 400
     And the response should be in JSON
-    And the header "Content-Type" should be equal to "application/problem+json; charset=utf-8"
+    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
     And the JSON node "@context" should be equal to "/contexts/Error"
     And the JSON node "@type" should be equal to "hydra:Error"
     And the JSON node "hydra:title" should be equal to "An error occurred"
@@ -80,7 +79,7 @@ Feature: Error handling
     """
     Then the response status code should be 400
     And the response should be in JSON
-    And the header "Content-Type" should be equal to "application/problem+json; charset=utf-8"
+    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
     And the JSON node "@context" should be equal to "/contexts/Error"
     And the JSON node "@type" should be equal to "hydra:Error"
     And the JSON node "hydra:title" should be equal to "An error occurred"
@@ -98,7 +97,7 @@ Feature: Error handling
       """
     Then the response status code should be 400
     And the response should be in JSON
-    And the header "Content-Type" should be equal to "application/problem+json; charset=utf-8"
+    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
     And the JSON node "@context" should be equal to "/contexts/Error"
     And the JSON node "@type" should be equal to "hydra:Error"
     And the JSON node "hydra:title" should be equal to "An error occurred"
@@ -152,3 +151,39 @@ Feature: Error handling
     Then the response status code should be 201
     And the response should be in JSON
     And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
+
+  Scenario: Get an rfc 7807 validation error
+    When I add "Content-Type" header equal to "application/ld+json"
+    And I send a "POST" request to "/validation_exception_problems" with body:
+    """
+    {}
+    """
+    Then the response status code should be 422
+    And the response should be in JSON
+    And the header "Content-Type" should be equal to "application/problem+json; charset=utf-8"
+    And the header "Link" should contain '<http://www.w3.org/ns/hydra/error>; rel="http://www.w3.org/ns/json-ld#error"'
+    And the JSON node "@context" should not exist
+
+  Scenario: Get an rfc 7807 error
+    When I add "Content-Type" header equal to "application/ld+json"
+    And I send a "POST" request to "/exception_problems" with body:
+    """
+    {}
+    """
+    Then the response status code should be 400
+    And the response should be in JSON
+    And the header "Content-Type" should be equal to "application/problem+json; charset=utf-8"
+    And the header "Link" should contain '<http://www.w3.org/ns/hydra/error>; rel="http://www.w3.org/ns/json-ld#error"'
+    And the JSON node "@context" should not exist
+
+  Scenario: Get an rfc 7807 error with backward compatibility
+    When I add "Content-Type" header equal to "application/ld+json"
+    And I send a "POST" request to "/exception_problems_with_compatibility" with body:
+    """
+    {}
+    """
+    Then the response status code should be 400
+    And the response should be in JSON
+    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
+    And the header "Link" should not contain '<http://www.w3.org/ns/hydra/error>; rel="http://www.w3.org/ns/json-ld#error"'
+    And the JSON node "@context" should exist
diff --git a/src/Hydra/EventListener/AddLinkHeaderListener.php b/src/Hydra/EventListener/AddLinkHeaderListener.php
@@ -51,7 +51,7 @@ public function onKernelResponse(ResponseEvent $event): void
 
         $apiDocUrl = $this->urlGenerator->generate('api_doc', ['_format' => 'jsonld'], UrlGeneratorInterface::ABS_URL);
         $apiDocLink = new Link(ContextBuilder::HYDRA_NS.'apiDocumentation', $apiDocUrl);
-        $linkProvider = $request->attributes->get('_links', new GenericLinkProvider());
+        $linkProvider = $request->attributes->get('_api_platform_links', new GenericLinkProvider());
 
         if (!$linkProvider instanceof EvolvableLinkProviderInterface) {
             return;
@@ -63,6 +63,6 @@ public function onKernelResponse(ResponseEvent $event): void
             }
         }
 
-        $request->attributes->set('_links', $linkProvider->withLink($apiDocLink));
+        $request->attributes->set('_api_platform_links', $linkProvider->withLink($apiDocLink));
     }
 }
diff --git a/src/Hydra/State/HydraLinkProcessor.php b/src/Hydra/State/HydraLinkProcessor.php
@@ -40,14 +40,14 @@ public function process(mixed $data, Operation $operation, array $uriVariables =
         }
 
         $apiDocUrl = $this->urlGenerator->generate('api_doc', ['_format' => 'jsonld'], UrlGeneratorInterface::ABS_URL);
-        $linkProvider = $request->attributes->get('_links') ?? new GenericLinkProvider();
+        $linkProvider = $request->attributes->get('_api_platform_links') ?? new GenericLinkProvider();
 
         foreach ($operation->getLinks() ?? [] as $link) {
             $linkProvider = $linkProvider->withLink($link);
         }
 
         $link = new Link(ContextBuilder::HYDRA_NS.'apiDocumentation', $apiDocUrl);
-        $request->attributes->set('_links', $linkProvider->withLink($link));
+        $request->attributes->set('_api_platform_links', $linkProvider->withLink($link));
 
         return $this->decorated->process($data, $operation, $uriVariables, $context);
     }

diff --git a/src/JsonLd/Serializer/JsonLdContextTrait.php b/src/JsonLd/Serializer/JsonLdContextTrait.php
@@ -15,6 +15,7 @@
 
 use ApiPlatform\JsonLd\AnonymousContextBuilderInterface;
 use ApiPlatform\JsonLd\ContextBuilderInterface;
+use ApiPlatform\Metadata\Error;
 
 /**
  * Creates and manipulates the Serializer context.
@@ -42,6 +43,10 @@ private function addJsonLdContext(ContextBuilderInterface $contextBuilder, strin
             return $data;
         }
 
+        if (($operation = $context['operation'] ?? null) && ($operation?->getExtraProperties()['rfc_7807_compliant_errors'] ?? false) && $operation instanceof Error) {
+            return $data;
+        }
+
         $data['@context'] = $contextBuilder->getResourceContextUri($resourceClass);
 
         return $data;

diff --git a/src/State/Processor/AddLinkHeaderProcessor.php b/src/State/Processor/AddLinkHeaderProcessor.php
@@ -36,11 +36,9 @@ public function process(mixed $data, Operation $operation, array $uriVariables =
         }
 
         // We add our header here as Symfony does it only for the main Request and we want it to be done on errors (sub-request) as well
-        $linksProvider = $request->attributes->get('_links');
+        $linksProvider = $request->attributes->get('_api_platform_links');
         if ($this->serializer && ($links = $linksProvider->getLinks())) {
             $response->headers->set('Link', $this->serializer->serialize($links));
-            // We don't want Symfony WebLink component do add links twice
-            $request->attributes->set('_links', []);
         }
 
         return $response;

diff --git a/src/Symfony/EventListener/ErrorListener.php b/src/Symfony/EventListener/ErrorListener.php
@@ -107,7 +107,7 @@ protected function duplicateRequest(\Throwable $exception, Request $request): Re
             }
         } else {
             /** @var HttpOperation $operation */
-            $operation = new ErrorOperation(name: '_api_errors_problem', class: Error::class, outputFormats: ['jsonld' => ['application/ld+json']], normalizationContext: ['groups' => ['jsonld'], 'skip_null_values' => true]);
+            $operation = new ErrorOperation(name: '_api_errors_problem', class: Error::class, outputFormats: ['jsonld' => ['application/problem+json']], normalizationContext: ['groups' => ['jsonld'], 'skip_null_values' => true]);
             $operation = $operation->withStatus($this->getStatusCode($apiOperation, $request, $operation, $exception));
             $errorResource = Error::createFromException($exception, $operation->getStatus());
         }
@@ -128,13 +128,17 @@ protected function duplicateRequest(\Throwable $exception, Request $request): Re
         } catch (\Exception $e) {
         }
 
-        if ($exception instanceof ValidationException) {
-            if (!($apiOperation?->getExtraProperties()['rfc_7807_compliant_errors'] ?? false)) {
-                $operation = $operation->withNormalizationContext([
-                    'groups' => ['legacy_'.$format],
-                    'force_iri_generation' => false,
-                ]);
-            }
+        if ($exception instanceof ValidationException && !($apiOperation?->getExtraProperties()['rfc_7807_compliant_errors'] ?? false)) {
+            $operation = $operation->withNormalizationContext([
+                'groups' => ['legacy_'.$format],
+                'force_iri_generation' => false,
+            ]);
+        }
+
+        if (!($apiOperation?->getExtraProperties()['rfc_7807_compliant_errors'] ?? false)) {
+            $operation = $operation->withOutputFormats(['jsonld' => ['application/ld+json']])
+                                   ->withLinks([])
+                                   ->withExtraProperties(['rfc_7807_compliant_errors' => false] + $operation->getExtraProperties());
         }
 
         $dup->attributes->set('_api_resource_class', $operation->getClass());

diff --git a/src/Symfony/EventListener/SerializeListener.php b/src/Symfony/EventListener/SerializeListener.php
@@ -142,11 +142,11 @@ public function onKernelView(ViewEvent $event): void
             return;
         }
 
-        $linkProvider = $request->attributes->get('_links', new GenericLinkProvider());
+        $linkProvider = $request->attributes->get('_api_platform_links', new GenericLinkProvider());
         foreach ($resourcesToPush as $resourceToPush) {
             $linkProvider = $linkProvider->withLink((new Link('preload', $resourceToPush))->withAttribute('as', 'fetch'));
         }
-        $request->attributes->set('_links', $linkProvider);
+        $request->attributes->set('_api_platform_links', $linkProvider);
     }
 
     /**

diff --git a/src/Symfony/State/SerializeProcessor.php b/src/Symfony/State/SerializeProcessor.php
@@ -65,11 +65,11 @@ public function process(mixed $data, Operation $operation, array $uriVariables =
         $serialized = $this->serializer->serialize($data, $request->getRequestFormat(), $serializerContext);
         $request->attributes->set('_resources', $request->attributes->get('_resources', []) + (array) $resources);
         if (\count($resourcesToPush)) {
-            $linkProvider = $request->attributes->get('_links', new GenericLinkProvider());
+            $linkProvider = $request->attributes->get('_api_platform_links', new GenericLinkProvider());
             foreach ($resourcesToPush as $resourceToPush) {
                 $linkProvider = $linkProvider->withLink((new Link('preload', $resourceToPush))->withAttribute('as', 'fetch'));
             }
-            $request->attributes->set('_links', $linkProvider);
+            $request->attributes->set('_api_platform_links', $linkProvider);
         }
 
         return $this->processor->process($serialized, $operation, $uriVariables, $context);

diff --git a/src/Symfony/Validator/Exception/ValidationException.php b/src/Symfony/Validator/Exception/ValidationException.php
@@ -13,6 +13,7 @@
 
 namespace ApiPlatform\Symfony\Validator\Exception;
 
+use ApiPlatform\JsonLd\ContextBuilderInterface;
 use ApiPlatform\Metadata\Error as ErrorOperation;
 use ApiPlatform\Metadata\ErrorResource;
 use ApiPlatform\Metadata\Exception\ProblemExceptionInterface;
@@ -21,6 +22,7 @@
 use Symfony\Component\Serializer\Annotation\Groups;
 use Symfony\Component\Serializer\Annotation\SerializedName;
 use Symfony\Component\Validator\ConstraintViolationListInterface;
+use Symfony\Component\WebLink\Link;
 
 /**
  * Thrown when a validation error occurs.
@@ -34,8 +36,15 @@
     uriVariables: ['id'],
     shortName: 'ConstraintViolationList',
     operations: [
-        new ErrorOperation(name: '_api_validation_errors_hydra', outputFormats: ['jsonld' => ['application/ld+json']], normalizationContext: ['groups' => ['jsonld'], 'skip_null_values' => true]),
-        new ErrorOperation(name: '_api_validation_errors_problem', outputFormats: ['jsonproblem' => ['application/problem+json'], 'json' => ['application/problem+json']], normalizationContext: ['groups' => ['json'], 'skip_null_values' => true]),
+        new ErrorOperation(
+            name: '_api_validation_errors_hydra',
+            outputFormats: ['jsonld' => ['application/problem+json']],
+            links: [new Link(rel: ContextBuilderInterface::JSONLD_NS.'error', href: 'http://www.w3.org/ns/hydra/error')],
+            normalizationContext: [
+                'groups' => ['jsonld'],
+                'skip_null_values' => true,
+            ]
+        ),
         new ErrorOperation(name: '_api_validation_errors_jsonapi', outputFormats: ['jsonapi' => ['application/vnd.api+json']], normalizationContext: ['groups' => ['jsonapi'], 'skip_null_values' => true]),
     ]
 )]

diff --git a/tests/Fixtures/TestBundle/ApiResource/ValidationExceptionProblem.php b/tests/Fixtures/TestBundle/ApiResource/ValidationExceptionProblem.php
@@ -0,0 +1,35 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\Tests\Fixtures\TestBundle\ApiResource;
+
+use ApiPlatform\Metadata\Post;
+use ApiPlatform\Symfony\Validator\Exception\ValidationException;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+use Symfony\Component\Validator\ConstraintViolationList;
+
+#[Post(read: true, provider: [ValidationExceptionProblem::class, 'provide'])]
+#[Post(uriTemplate: '/exception_problems', read: true, provider: [ValidationExceptionProblem::class, 'provideException'])]
+#[Post(uriTemplate: '/exception_problems_with_compatibility', read: true, provider: [ValidationExceptionProblem::class, 'provideException'], extraProperties: ['rfc_7807_compliant_errors' => false])]
+class ValidationExceptionProblem
+{
+    public static function provide(): void
+    {
+        throw new ValidationException(new ConstraintViolationList());
+    }
+
+    public static function provideException(): void
+    {
+        throw new BadRequestHttpException();
+    }
+}
diff --git a/tests/Hydra/EventListener/AddLinkHeaderListenerTest.php b/tests/Hydra/EventListener/AddLinkHeaderListenerTest.php
@@ -21,9 +21,7 @@
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\ResponseEvent;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
-use Symfony\Component\WebLink\GenericLinkProvider;
 use Symfony\Component\WebLink\HttpHeaderSerializer;
-use Symfony\Component\WebLink\Link;
 
 /**
  * @author Kévin Dunglas <[email protected]>
@@ -49,13 +47,12 @@ public function testAddLinkHeader(string $expected, Request $request): void
 
         $listener = new AddLinkHeaderListener($urlGenerator->reveal());
         $listener->onKernelResponse($event);
-        $this->assertSame($expected, (new HttpHeaderSerializer())->serialize($request->attributes->get('_links')->getLinks()));
+        $this->assertSame($expected, (new HttpHeaderSerializer())->serialize($request->attributes->get('_api_platform_links')->getLinks()));
     }
 
     public static function provider(): \Iterator
     {
         yield ['<http://example.com/docs>; rel="http://www.w3.org/ns/hydra/core#apiDocumentation"', new Request()];
-        yield ['<https://demo.mercure.rocks>; rel="mercure",<http://example.com/docs>; rel="http://www.w3.org/ns/hydra/core#apiDocumentation"', new Request([], [], ['_links' => new GenericLinkProvider([new Link('mercure', 'https://demo.mercure.rocks')])])];
     }
 
     public function testSkipWhenPreflightRequest(): void
@@ -75,6 +72,6 @@ public function testSkipWhenPreflightRequest(): void
         $listener = new AddLinkHeaderListener($urlGenerator->reveal());
         $listener->onKernelResponse($event);
 
-        $this->assertFalse($request->attributes->has('_links'));
+        $this->assertFalse($request->attributes->has('_api_platform_links'));
     }
 }
diff --git a/tests/Hydra/State/HydraLinkProcessorTest.php b/tests/Hydra/State/HydraLinkProcessorTest.php
@@ -33,7 +33,7 @@ public function testProcess(): void
         $request = $this->createMock(Request::class);
         $request->attributes = $this->createMock(ParameterBag::class);
 
-        $request->attributes->expects($this->once())->method('set')->with('_links', $this->callback(function ($linkProvider) {
+        $request->attributes->expects($this->once())->method('set')->with('_api_platform_links', $this->callback(function ($linkProvider) {
             $this->assertInstanceOf(GenericLinkProvider::class, $linkProvider);
             $this->assertEquals($linkProvider->getLinks(), [new Link('a', 'b'), new Link(ContextBuilder::HYDRA_NS.'apiDocumentation', '/docs')]);