fix(metadata): configure exceptionToStatus on an operation (#4861)

ArnoudThibaut · alanpoulain · soyuka · web-flow · commit 2d705258978c · 2022-09-02T10:53:17.000+02:00
* fix: fix exception to status mapping in ApiResource and Operations

Extend Symfony ErrorListener to customize ErrorListener::duplicateRequest behavior and allow to pass the '_api_operation' request attribute to the ExceptionAction in order to retrieve the exception to status mapping configured on the Resource or Operation

Co-authored-by: Alan Poulain &lt;contact@alanpoulain.eu&gt;
Co-authored-by: soyuka &lt;soyuka@users.noreply.github.com&gt;
diff --git a/features/main/exception_to_status.feature b/features/main/exception_to_status.feature
@@ -0,0 +1,33 @@
+Feature: Using exception_to_status config
+  As an API developer
+  I can customize the status code returned if the application throws an exception
+
+  @createSchema
+  @!mongodb
+  Scenario: Configure status code via the operation exceptionToStatus to map custom NotFound error to 404
+    When I add "Content-Type" header equal to "application/ld+json"
+    And I send a "GET" request to "/dummy_exception_to_statuses/123"
+    Then the response status code should be 404
+    And the response should be in JSON
+    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
+
+  @!mongodb
+  Scenario: Configure status code via the resource exceptionToStatus to map custom NotFound error to 400
+    When I add "Content-Type" header equal to "application/ld+json"
+    And I send a "PUT" request to "/dummy_exception_to_statuses/123" with body:
+    """
+    {
+        "name": "black"
+    }
+    """
+    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"
+
+  @!mongodb
+  Scenario: Configure status code via the config file to map FilterValidationException to 400
+    When I add "Content-Type" header equal to "application/ld+json"
+    And I send a "GET" request to "/dummy_exception_to_statuses"
+    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"
diff --git a/src/Action/ExceptionAction.php b/src/Action/ExceptionAction.php
@@ -96,6 +96,11 @@ public function __invoke($exception, Request $request): Response
 
     private function getOperationExceptionToStatus(Request $request): array
     {
+        // TODO: remove legacy layer in 3.0
+        if ($request->attributes->has('_api_exception_to_status')) {
+            return $request->attributes->get('_api_exception_to_status');
+        }
+
         $attributes = RequestAttributesExtractor::extractAttributes($request);
 
         if ([] === $attributes || null === $this->resourceMetadataFactory) {
diff --git a/src/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php
@@ -43,6 +43,7 @@
 use ApiPlatform\Symfony\Validator\Metadata\Property\Restriction\PropertySchemaRestrictionMetadataInterface;
 use ApiPlatform\Symfony\Validator\ValidationGroupsGeneratorInterface;
 use Doctrine\Common\Annotations\Annotation;
+use Doctrine\Persistence\ManagerRegistry;
 use phpDocumentor\Reflection\DocBlockFactoryInterface;
 use Ramsey\Uuid\Uuid;
 use Symfony\Component\Cache\Adapter\ArrayAdapter;
@@ -657,6 +658,11 @@ private function registerDoctrineOrmConfiguration(ContainerBuilder $container, a
             return;
         }
 
+        // For older versions of doctrine bridge this allows autoconfiguration for filters
+        if (!$container->has(ManagerRegistry::class)) {
+            $container->setAlias(ManagerRegistry::class, 'doctrine');
+        }
+
         $container->registerForAutoconfiguration(QueryItemExtensionInterface::class)
             ->addTag('api_platform.doctrine.orm.query_extension.item');
         $container->registerForAutoconfiguration(DoctrineQueryCollectionExtensionInterface::class)
diff --git a/src/Symfony/Bundle/Resources/config/api.xml b/src/Symfony/Bundle/Resources/config/api.xml
@@ -175,5 +175,8 @@
             <tag name="api_platform.uri_variables.transformer" priority="-100" />
         </service>
 
+        <service id="api_platform.error_listener" class="ApiPlatform\Symfony\EventListener\ErrorListener" parent="exception_listener">
+            <argument key="$controller">api_platform.action.exception</argument>
+        </service>
     </services>
 </container>
diff --git a/src/Symfony/Bundle/Resources/config/legacy/api.xml b/src/Symfony/Bundle/Resources/config/legacy/api.xml
@@ -103,7 +103,7 @@
             <argument>api_platform.action.exception</argument>
             <argument type="service" id="logger" on-invalid="null" />
             <argument>false</argument>
-            <argument type="service" id="exception_listener" on-invalid="null" />
+            <argument type="service" id="api_platform.error_listener" on-invalid="null" />
 
             <tag name="kernel.event_listener" event="kernel.exception" method="onKernelException" priority="-96" />
             <tag name="monolog.logger" channel="request" />
diff --git a/src/Symfony/Bundle/Resources/config/symfony.xml b/src/Symfony/Bundle/Resources/config/symfony.xml
@@ -70,7 +70,7 @@
             <argument>api_platform.action.exception</argument>
             <argument type="service" id="logger" on-invalid="null" />
             <argument>false</argument>
-            <argument type="service" id="exception_listener" on-invalid="null" />
+            <argument type="service" id="api_platform.error_listener" on-invalid="null" />
 
             <tag name="kernel.event_listener" event="kernel.exception" method="onKernelException" priority="-96" />
             <tag name="monolog.logger" channel="request" />
diff --git a/src/Symfony/EventListener/ErrorListener.php b/src/Symfony/EventListener/ErrorListener.php
@@ -0,0 +1,42 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <dunglas@gmail.com>
+ *
+ * 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\Symfony\EventListener;
+
+use ApiPlatform\Action\ExceptionAction;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\EventListener\ErrorListener as SymfonyErrorListener;
+
+/**
+ * This error listener extends the Symfony one in order to add
+ * the `_api_operation` attribute when the request is duplicated.
+ * It will later be used to retrieve the exceptionToStatus from the operation ({@see ExceptionAction}).
+ */
+final class ErrorListener extends SymfonyErrorListener
+{
+    protected function duplicateRequest(\Throwable $exception, Request $request): Request
+    {
+        $dup = parent::duplicateRequest($exception, $request);
+
+        if ($request->attributes->has('_api_operation')) {
+            $dup->attributes->set('_api_operation', $request->attributes->get('_api_operation'));
+        }
+
+        // TODO: remove legacy layer in 3.0
+        if ($request->attributes->has('_api_exception_to_status')) {
+            $dup->attributes->set('_api_exception_to_status', $request->attributes->get('_api_exception_to_status'));
+        }
+
+        return $dup;
+    }
+}
diff --git a/src/Symfony/EventListener/ExceptionListener.php b/src/Symfony/EventListener/ExceptionListener.php
@@ -28,13 +28,16 @@
 final class ExceptionListener
 {
     /**
-     * @var ErrorListener
+     * @phpstan-ignore-next-line legacy may not exist
+     *
+     * @var ErrorListener|LegacyExceptionListener
      */
     private $exceptionListener;
 
     public function __construct($controller, LoggerInterface $logger = null, $debug = false, ErrorListener $errorListener = null)
     {
-        $this->exceptionListener = $errorListener ? new ErrorListener($controller, $logger, $debug) : new LegacyExceptionListener($controller, $logger, $debug); // @phpstan-ignore-line
+        // @phpstan-ignore-next-line legacy may not exist
+        $this->exceptionListener = $errorListener ?: new LegacyExceptionListener($controller, $logger, $debug);
     }
 
     public function onKernelException(ExceptionEvent $event): void
@@ -48,7 +51,7 @@ public function onKernelException(ExceptionEvent $event): void
             return;
         }
 
-        $this->exceptionListener->onKernelException($event);
+        $this->exceptionListener->onKernelException($event); // @phpstan-ignore-line
     }
 }
 
diff --git a/src/Symfony/Routing/ApiLoader.php b/src/Symfony/Routing/ApiLoader.php
@@ -336,6 +336,7 @@ private function addRoute(RouteCollection $routeCollection, string $resourceClas
                 '_api_resource_class' => $resourceClass,
                 '_api_identifiers' => $operation['identifiers'] ?? [],
                 '_api_has_composite_identifier' => $operation['has_composite_identifier'] ?? true,
+                '_api_exception_to_status' => $operation['exception_to_status'] ?? $resourceMetadata->getAttribute('exception_to_status') ?? [],
                 '_api_operation_name' => RouteNameGenerator::generate($operationName, $resourceShortName, $operationType),
                 sprintf('_api_%s_operation_name', $operationType) => $operationName,
             ] + ($operation['defaults'] ?? []),
diff --git a/src/Test/DoctrineMongoDbOdmTestCase.php b/src/Test/DoctrineMongoDbOdmTestCase.php
@@ -20,6 +20,7 @@
 use Doctrine\ODM\MongoDB\Mapping\Driver\AnnotationDriver;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Cache\Adapter\ArrayAdapter;
+
 use function sys_get_temp_dir;
 
 /**
diff --git a/tests/Core/Bridge/Symfony/Routing/ApiLoaderTest.php b/tests/Core/Bridge/Symfony/Routing/ApiLoaderTest.php
@@ -328,6 +328,7 @@ private function getRoute(string $path, string $controller, string $resourceClas
                 '_api_has_composite_identifier' => false,
                 sprintf('_api_%s_operation_name', $collection ? 'collection' : 'item') => $legacyOperationName,
                 '_api_operation_name' => $operationName,
+                '_api_exception_to_status' => [],
             ] + $extraDefaults,
             $requirements,
             $options,
diff --git a/tests/Fixtures/TestBundle/Entity/DummyExceptionToStatus.php b/tests/Fixtures/TestBundle/Entity/DummyExceptionToStatus.php
@@ -0,0 +1,116 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <dunglas@gmail.com>
+ *
+ * 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\Entity;
+
+use ApiPlatform\Core\Annotation\ApiFilter;
+use ApiPlatform\Core\Annotation\ApiResource;
+use ApiPlatform\Tests\Fixtures\TestBundle\Exception\NotFoundException;
+use ApiPlatform\Tests\Fixtures\TestBundle\Filter\RequiredFilter;
+use Doctrine\ORM\Mapping as ORM;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
+
+/**
+ * @ApiResource(
+ *     itemOperations={
+ *         "get"={"exception_to_status"={NotFoundException::class=404}},
+ *         "put"
+ *     },
+ *     collectionOperations={"get"},
+ *     exceptionToStatus={
+ *         NotFoundHttpException::class=400
+ *     }
+ * )
+ * @ApiFilter(RequiredFilter::class)
+ * @ORM\Entity
+ */
+class DummyExceptionToStatus
+{
+    /**
+     * @var int|null The id
+     *
+     * @ORM\Column(type="integer")
+     * @ORM\Id
+     * @ORM\GeneratedValue(strategy="AUTO")
+     */
+    private $id = null;
+
+    /**
+     * @var string|null The dummy name
+     *
+     * @ORM\Column(nullable=true)
+     */
+    private $name = null;
+
+    /**
+     * @var string|null The dummy title
+     *
+     * @ORM\Column(nullable=true)
+     */
+    private $title = null;
+
+    /**
+     * @var string The dummy code
+     *
+     * @ORM\Column
+     */
+    private $code;
+
+    public function getId(): ?int
+    {
+        return $this->id;
+    }
+
+    public function setId(int $id): self
+    {
+        $this->id = $id;
+
+        return $this;
+    }
+
+    public function getName(): ?string
+    {
+        return $this->name;
+    }
+
+    public function setName(?string $name): self
+    {
+        $this->name = $name;
+
+        return $this;
+    }
+
+    public function getTitle(): ?string
+    {
+        return $this->title;
+    }
+
+    public function setTitle(?string $title): self
+    {
+        $this->title = $title;
+
+        return $this;
+    }
+
+    public function getCode(): ?string
+    {
+        return $this->code;
+    }
+
+    public function setCode(string $code): self
+    {
+        $this->code = $code;
+
+        return $this;
+    }
+}
diff --git a/tests/Fixtures/TestBundle/Exception/NotFoundException.php b/tests/Fixtures/TestBundle/Exception/NotFoundException.php
@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <dunglas@gmail.com>
+ *
+ * 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\Exception;
+
+final class NotFoundException extends \Exception
+{
+    public function __construct(string $message = '', int $code = 0, ?\Throwable $previous = null)
+    {
+        parent::__construct($message, $code, $previous);
+    }
+}
diff --git a/tests/Fixtures/TestBundle/State/DummyExceptionToStatusProvider.php b/tests/Fixtures/TestBundle/State/DummyExceptionToStatusProvider.php
@@ -0,0 +1,26 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <dunglas@gmail.com>
+ *
+ * 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\State;
+
+use ApiPlatform\Metadata\Operation;
+use ApiPlatform\State\ProviderInterface;
+use ApiPlatform\Tests\Fixtures\TestBundle\Exception\NotFoundException;
+
+class DummyExceptionToStatusProvider implements ProviderInterface
+{
+    public function provide(Operation $operation, array $uriVariables = [], array $context = [])
+    {
+        throw new NotFoundException();
+    }
+}
diff --git a/tests/Fixtures/app/config/config_common.yml b/tests/Fixtures/app/config/config_common.yml
diff --git a/tests/Symfony/EventListener/ExceptionListenerTest.php b/tests/Symfony/EventListener/ExceptionListenerTest.php

Original file line number	Diff line number	Diff line change
`@@ -28,13 +28,16 @@`
`28`	`28`	`final class ExceptionListener`
`29`	`29`	`{`
`30`	`30`	`/**`
`31`		`- * @var ErrorListener`
	`31`	`+ * @phpstan-ignore-next-line legacy may not exist`
	`32`	`+ *`
	`33`	`+ * @var ErrorListener\|LegacyExceptionListener`
`32`	`34`	`*/`
`33`	`35`	`private $exceptionListener;`
`34`	`36`
`35`	`37`	`public function __construct($controller, LoggerInterface $logger = null, $debug = false, ErrorListener $errorListener = null)`
`36`	`38`	`{`
`37`		`- $this->exceptionListener = $errorListener ? new ErrorListener($controller, $logger, $debug) : new LegacyExceptionListener($controller, $logger, $debug); // @phpstan-ignore-line`
	`39`	`+ // @phpstan-ignore-next-line legacy may not exist`
	`40`	`+ $this->exceptionListener = $errorListener ?: new LegacyExceptionListener($controller, $logger, $debug);`
`38`	`41`	`}`
`39`	`42`
`40`	`43`	`public function onKernelException(ExceptionEvent $event): void`
`@@ -48,7 +51,7 @@ public function onKernelException(ExceptionEvent $event): void`
`48`	`51`	`return;`
`49`	`52`	`}`
`50`	`53`
`51`		`- $this->exceptionListener->onKernelException($event);`
	`54`	`+ $this->exceptionListener->onKernelException($event); // @phpstan-ignore-line`
`52`	`55`	`}`
`53`	`56`	`}`
`54`	`57`