From 30dacf29df79ca5403460f2ef0ab00b4a3a91c1b Mon Sep 17 00:00:00 2001 From: soyuka Date: Fri, 15 Sep 2023 12:21:31 +0200 Subject: [PATCH] fix: exception to status on error resource --- features/main/exception_to_status.feature | 7 +++ src/Action/EntrypointAction.php | 2 +- src/ApiResource/Error.php | 7 ++- .../Action/DocumentationAction.php | 4 +- src/JsonLd/Action/ContextAction.php | 3 +- .../Exception/ProblemExceptionInterface.php | 2 + src/State/Provider/ReadProvider.php | 3 +- src/Symfony/Controller/MainController.php | 13 +++++ src/Symfony/EventListener/ErrorListener.php | 55 ++++++++++++------- src/Symfony/State/DeserializeProvider.php | 10 ++-- src/Symfony/State/SerializeProcessor.php | 2 +- .../Exception/ValidationException.php | 9 ++- .../ApiResource/ErrorWithOverridenStatus.php | 32 +++++++++++ .../Entity/DummyExceptionToStatus.php | 2 + tests/Symfony/Bundle/Test/ApiTestCaseTest.php | 3 +- 15 files changed, 119 insertions(+), 35 deletions(-) create mode 100644 tests/Fixtures/TestBundle/ApiResource/ErrorWithOverridenStatus.php diff --git a/features/main/exception_to_status.feature b/features/main/exception_to_status.feature index 019a5d77375..34cda8c5d13 100644 --- a/features/main/exception_to_status.feature +++ b/features/main/exception_to_status.feature @@ -31,3 +31,10 @@ Feature: Using exception_to_status config 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" + + @!mongodb + Scenario: Override validation exception status code from delete operation + When I add "Content-Type" header equal to "application/ld+json" + And I send a "DELETE" request to "/error_with_overriden_status" + Then the response status code should be 403 + And the JSON node "status" should be equal to 403 diff --git a/src/Action/EntrypointAction.php b/src/Action/EntrypointAction.php index 38c31c5f0ba..08c0c737130 100644 --- a/src/Action/EntrypointAction.php +++ b/src/Action/EntrypointAction.php @@ -42,7 +42,7 @@ public function __invoke(Request $request = null) { if ($this->provider && $this->processor) { $context = ['request' => $request]; - $operation = new Get(class: Entrypoint::class, provider: fn () => new Entrypoint($this->resourceNameCollectionFactory->create())); + $operation = new Get(read: true, serialize: true, class: Entrypoint::class, provider: fn () => new Entrypoint($this->resourceNameCollectionFactory->create())); $body = $this->provider->provide($operation, [], $context); return $this->processor->process($body, $operation, [], $context); diff --git a/src/ApiResource/Error.php b/src/ApiResource/Error.php index 5c4abdcc915..aab3eb305f9 100644 --- a/src/ApiResource/Error.php +++ b/src/ApiResource/Error.php @@ -48,7 +48,7 @@ class Error extends \Exception implements ProblemExceptionInterface, HttpExcepti public function __construct( private readonly string $title, private readonly string $detail, - #[ApiProperty(identifier: true)] private readonly int $status, + #[ApiProperty(identifier: true)] private int $status, private readonly array $originalTrace, private ?string $instance = null, private string $type = 'about:blank', @@ -132,6 +132,11 @@ public function getStatus(): ?int return $this->status; } + public function setStatus(int $status): void + { + $this->status = $status; + } + #[Groups(['jsonld', 'jsonproblem', 'legacy_jsonproblem'])] public function getDetail(): ?string { diff --git a/src/Documentation/Action/DocumentationAction.php b/src/Documentation/Action/DocumentationAction.php index 12d199a1782..533f84b3455 100644 --- a/src/Documentation/Action/DocumentationAction.php +++ b/src/Documentation/Action/DocumentationAction.php @@ -78,7 +78,7 @@ private function getOpenApiDocumentation(array $context, string $format, Request { if ($this->provider && $this->processor) { $context['request'] = $request; - $operation = new Get(class: OpenApi::class, provider: fn () => $this->openApiFactory->__invoke($context), normalizationContext: [ApiGatewayNormalizer::API_GATEWAY => $context['api_gateway'] ?? null], outputFormats: $this->documentationFormats); + $operation = new Get(class: OpenApi::class, read: true, serialize: true, provider: fn () => $this->openApiFactory->__invoke($context), normalizationContext: [ApiGatewayNormalizer::API_GATEWAY => $context['api_gateway'] ?? null], outputFormats: $this->documentationFormats); if ('html' === $format) { $operation = $operation->withProcessor('api_platform.swagger_ui.processor')->withWrite(true); } @@ -104,6 +104,8 @@ private function getHydraDocumentation(array $context, Request $request): Docume $context['request'] = $request; $operation = new Get( class: Documentation::class, + read: true, + serialize: true, provider: fn () => new Documentation($this->resourceNameCollectionFactory->create(), $this->title, $this->description, $this->version) ); diff --git a/src/JsonLd/Action/ContextAction.php b/src/JsonLd/Action/ContextAction.php index 40483482ecc..f737f56cbcf 100644 --- a/src/JsonLd/Action/ContextAction.php +++ b/src/JsonLd/Action/ContextAction.php @@ -61,7 +61,8 @@ public function __invoke(string $shortName, Request $request = null): array|Resp outputFormats: ['jsonld' => ['application/ld+json']], validate: false, provider: fn () => $this->getContext($shortName), - serialize: false + serialize: false, + read: true ); $context = ['request' => $request]; $jsonLdContext = $this->provider->provide($operation, [], $context); diff --git a/src/Metadata/Exception/ProblemExceptionInterface.php b/src/Metadata/Exception/ProblemExceptionInterface.php index 6b4ef927ac3..6761865bdaa 100644 --- a/src/Metadata/Exception/ProblemExceptionInterface.php +++ b/src/Metadata/Exception/ProblemExceptionInterface.php @@ -28,6 +28,8 @@ public function getTitle(): ?string; public function getStatus(): ?int; + public function setStatus(int $status): void; + public function getDetail(): ?string; public function getInstance(): ?string; diff --git a/src/State/Provider/ReadProvider.php b/src/State/Provider/ReadProvider.php index 556c10bb37f..a8b6d6e21f2 100644 --- a/src/State/Provider/ReadProvider.php +++ b/src/State/Provider/ReadProvider.php @@ -49,7 +49,8 @@ public function provide(Operation $operation, array $uriVariables = [], array $c } $request = ($context['request'] ?? null); - if (!($operation->canRead() ?? true) || (!$operation->getUriVariables() && !$request?->isMethodSafe())) { + + if (!$operation->canRead()) { return null; } diff --git a/src/Symfony/Controller/MainController.php b/src/Symfony/Controller/MainController.php index c2c3e219aca..1098e863466 100644 --- a/src/Symfony/Controller/MainController.php +++ b/src/Symfony/Controller/MainController.php @@ -16,6 +16,7 @@ use ApiPlatform\Api\UriVariablesConverterInterface; use ApiPlatform\Exception\InvalidIdentifierException; use ApiPlatform\Exception\InvalidUriVariableException; +use ApiPlatform\Metadata\HttpOperation; use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; use ApiPlatform\State\ProcessorInterface; use ApiPlatform\State\ProviderInterface; @@ -62,6 +63,14 @@ public function __invoke(Request $request): Response $operation = $operation->withValidate(!$request->isMethodSafe() && !$request->isMethod('DELETE')); } + if (null === $operation->canRead() && $operation instanceof HttpOperation) { + $operation = $operation->withRead($operation->getUriVariables() || $request?->isMethodSafe()); + } + + if (null === $operation->canDeserialize() && $operation instanceof HttpOperation) { + $operation = $operation->withDeserialize(\in_array($operation->getMethod(), ['POST', 'PUT', 'PATCH'], true)); + } + $body = $this->provider->provide($operation, $uriVariables, $context); // The provider can change the Operation, extract it again from the Request attributes @@ -84,6 +93,10 @@ public function __invoke(Request $request): Response $operation = $operation->withWrite(!$request->isMethodSafe()); } + if (null === $operation->canSerialize()) { + $operation = $operation->withSerialize(true); + } + return $this->processor->process($body, $operation, $uriVariables, $context); } } diff --git a/src/Symfony/EventListener/ErrorListener.php b/src/Symfony/EventListener/ErrorListener.php index 905a4a76b36..1133d80c729 100644 --- a/src/Symfony/EventListener/ErrorListener.php +++ b/src/Symfony/EventListener/ErrorListener.php @@ -16,6 +16,7 @@ use ApiPlatform\Api\IdentifiersExtractorInterface; use ApiPlatform\ApiResource\Error; use ApiPlatform\Metadata\Error as ErrorOperation; +use ApiPlatform\Metadata\Exception\ProblemExceptionInterface; use ApiPlatform\Metadata\HttpOperation; use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; use ApiPlatform\Metadata\ResourceClassResolverInterface; @@ -64,32 +65,44 @@ protected function duplicateRequest(\Throwable $exception, Request $request): Re $apiOperation = $this->initializeOperation($request); $format = $this->getRequestFormat($request, $this->errorFormats, false); - if ($this->resourceClassResolver?->isResourceClass($exception::class)) { - $resourceCollection = $this->resourceMetadataCollectionFactory->create($exception::class); - - $operation = null; - foreach ($resourceCollection as $resource) { - foreach ($resource->getOperations() as $op) { - foreach ($op->getOutputFormats() as $key => $value) { - if ($key === $format) { - $operation = $op; - break 3; + if ($this->resourceMetadataCollectionFactory) { + if ($this->resourceClassResolver?->isResourceClass($exception::class)) { + $resourceCollection = $this->resourceMetadataCollectionFactory->create($exception::class); + + $operation = null; + foreach ($resourceCollection as $resource) { + foreach ($resource->getOperations() as $op) { + foreach ($op->getOutputFormats() as $key => $value) { + if ($key === $format) { + $operation = $op; + break 3; + } } } } - } - // No operation found for the requested format, we take the first available - if (!$operation) { - $operation = $resourceCollection->getOperation(); + // No operation found for the requested format, we take the first available + if (!$operation) { + $operation = $resourceCollection->getOperation(); + } + $errorResource = $exception; + if ($errorResource instanceof ProblemExceptionInterface) { + $statusCode = $this->getStatusCode($apiOperation, $request, $operation, $exception); + if ($operation instanceof HttpOperation) { + $operation = $operation->withStatus($statusCode); + } + $errorResource->setStatus($statusCode); + } + } else { + // Create a generic, rfc7807 compatible error according to the wanted format + $operation = $this->resourceMetadataCollectionFactory->create(Error::class)->getOperation($this->getFormatOperation($format)); + // status code may be overriden by the exceptionToStatus option + $statusCode = $this->getStatusCode($apiOperation, $request, $operation, $exception); + if ($operation instanceof HttpOperation) { + $operation = $operation->withStatus($statusCode); + } + $errorResource = Error::createFromException($exception, $statusCode); } - $errorResource = $exception; - } elseif ($this->resourceMetadataCollectionFactory) { - // Create a generic, rfc7807 compatible error according to the wanted format - /** @var HttpOperation $operation */ - $operation = $this->resourceMetadataCollectionFactory->create(Error::class)->getOperation($this->getFormatOperation($format)); - $operation = $operation->withStatus($this->getStatusCode($apiOperation, $request, $operation, $exception)); - $errorResource = Error::createFromException($exception, $operation->getStatus()); } 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]); diff --git a/src/Symfony/State/DeserializeProvider.php b/src/Symfony/State/DeserializeProvider.php index a7ac41dfa78..36b239e96e8 100644 --- a/src/Symfony/State/DeserializeProvider.php +++ b/src/Symfony/State/DeserializeProvider.php @@ -51,10 +51,7 @@ public function provide(Operation $operation, array $uriVariables = [], array $c return $data; } - if ( - !($operation->canDeserialize() ?? true) - || !\in_array($method = $operation->getMethod(), ['POST', 'PUT', 'PATCH'], true) - ) { + if (!$operation->canDeserialize()) { return $data; } @@ -74,10 +71,11 @@ public function provide(Operation $operation, array $uriVariables = [], array $c throw new UnsupportedMediaTypeHttpException('Format not supported.'); } + $method = $operation->getMethod(); + if ( null !== $data - && ( - 'POST' === $method + && ('POST' === $method || 'PATCH' === $method || ('PUT' === $method && !($operation->getExtraProperties()['standard_put'] ?? false)) ) diff --git a/src/Symfony/State/SerializeProcessor.php b/src/Symfony/State/SerializeProcessor.php index 47c45df41aa..ce9b4e8a765 100644 --- a/src/Symfony/State/SerializeProcessor.php +++ b/src/Symfony/State/SerializeProcessor.php @@ -36,7 +36,7 @@ public function __construct(private readonly ProcessorInterface $processor, priv public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []) { - if ($data instanceof Response || !($operation->canSerialize() ?? true) || !($request = $context['request'] ?? null)) { + if ($data instanceof Response || !$operation->canSerialize() || !($request = $context['request'] ?? null)) { return $this->processor->process($data, $operation, $uriVariables, $context); } diff --git a/src/Symfony/Validator/Exception/ValidationException.php b/src/Symfony/Validator/Exception/ValidationException.php index 561a0e6ff70..70800ab7677 100644 --- a/src/Symfony/Validator/Exception/ValidationException.php +++ b/src/Symfony/Validator/Exception/ValidationException.php @@ -41,6 +41,8 @@ )] final class ValidationException extends BaseValidationException implements ConstraintViolationListAwareExceptionInterface, \Stringable, ProblemExceptionInterface { + private int $status = 422; + public function __construct(private readonly ConstraintViolationListInterface $constraintViolationList, string $message = '', int $code = 0, \Throwable $previous = null, string $errorTitle = null) { parent::__construct($message ?: $this->__toString(), $code, $previous, $errorTitle); @@ -119,7 +121,12 @@ public function getDetail(): ?string #[Groups(['jsonld', 'json', 'legacy_jsonproblem', 'legacy_json'])] public function getStatus(): ?int { - return 422; + return $this->status; + } + + public function setStatus(int $status): void + { + $this->status = $status; } #[Groups(['jsonld', 'json'])] diff --git a/tests/Fixtures/TestBundle/ApiResource/ErrorWithOverridenStatus.php b/tests/Fixtures/TestBundle/ApiResource/ErrorWithOverridenStatus.php new file mode 100644 index 00000000000..3aaf203ee42 --- /dev/null +++ b/tests/Fixtures/TestBundle/ApiResource/ErrorWithOverridenStatus.php @@ -0,0 +1,32 @@ + + * + * 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\Delete; +use ApiPlatform\Symfony\Validator\Exception\ValidationException; +use Symfony\Component\Validator\ConstraintViolationList; + +#[Delete( + uriTemplate: '/error_with_overriden_status', + read: true, + provider: [ErrorWithOverridenStatus::class, 'throw'], + exceptionToStatus: [ValidationException::class => 403] +)] +class ErrorWithOverridenStatus +{ + public static function throw(): void + { + throw new ValidationException(new ConstraintViolationList()); + } +} diff --git a/tests/Fixtures/TestBundle/Entity/DummyExceptionToStatus.php b/tests/Fixtures/TestBundle/Entity/DummyExceptionToStatus.php index 3cd965dd05a..2ccd9a3c5eb 100644 --- a/tests/Fixtures/TestBundle/Entity/DummyExceptionToStatus.php +++ b/tests/Fixtures/TestBundle/Entity/DummyExceptionToStatus.php @@ -20,11 +20,13 @@ use ApiPlatform\Metadata\Put; use ApiPlatform\Tests\Fixtures\TestBundle\Exception\NotFoundException; use ApiPlatform\Tests\Fixtures\TestBundle\Filter\RequiredFilter; +use ApiPlatform\Tests\Fixtures\TestBundle\State\DummyExceptionToStatusProvider; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; #[ApiResource( exceptionToStatus: [NotFoundHttpException::class => 400], + provider: DummyExceptionToStatusProvider::class, operations: [ new Get(uriTemplate: '/dummy_exception_to_statuses/{id}', exceptionToStatus: [NotFoundException::class => 404]), new Put(uriTemplate: '/dummy_exception_to_statuses/{id}'), diff --git a/tests/Symfony/Bundle/Test/ApiTestCaseTest.php b/tests/Symfony/Bundle/Test/ApiTestCaseTest.php index 5a2984a59c5..50827ffc25c 100644 --- a/tests/Symfony/Bundle/Test/ApiTestCaseTest.php +++ b/tests/Symfony/Bundle/Test/ApiTestCaseTest.php @@ -29,7 +29,8 @@ class ApiTestCaseTest extends ApiTestCase { public function testAssertJsonContains(): void { - self::createClient()->request('GET', '/'); + $client = self::createClient(); + $client->request('GET', '/'); $this->assertJsonContains(['@context' => '/contexts/Entrypoint']); }