From 1eb4908417dd6041d3fce286b85d820f090f6197 Mon Sep 17 00:00:00 2001 From: Ahmed Eben Hassine Date: Sat, 31 Oct 2020 14:28:19 +0100 Subject: [PATCH 1/6] feat: support changing validation status code --- .../Symfony/Bundle/Resources/config/api.xml | 1 + .../ValidationExceptionListener.php | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/Bridge/Symfony/Bundle/Resources/config/api.xml b/src/Bridge/Symfony/Bundle/Resources/config/api.xml index eb8ec8d60a3..f5b94eb917d 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/api.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/api.xml @@ -202,6 +202,7 @@ %api_platform.error_formats% + %api_platform.exception_to_status% diff --git a/src/Bridge/Symfony/Validator/EventListener/ValidationExceptionListener.php b/src/Bridge/Symfony/Validator/EventListener/ValidationExceptionListener.php index 5f72211c5e5..aa259d1806c 100644 --- a/src/Bridge/Symfony/Validator/EventListener/ValidationExceptionListener.php +++ b/src/Bridge/Symfony/Validator/EventListener/ValidationExceptionListener.php @@ -28,11 +28,13 @@ final class ValidationExceptionListener { private $serializer; private $errorFormats; + private $exceptionToStatus; - public function __construct(SerializerInterface $serializer, array $errorFormats) + public function __construct(SerializerInterface $serializer, array $errorFormats, array $exceptionToStatus = []) { $this->serializer = $serializer; $this->errorFormats = $errorFormats; + $this->exceptionToStatus = $exceptionToStatus; } /** @@ -44,12 +46,22 @@ public function onKernelException(ExceptionEvent $event): void if (!$exception instanceof ValidationException) { return; } + $exceptionClass = get_class($exception); + $statusCode = Response::HTTP_BAD_REQUEST; + + foreach ($this->exceptionToStatus as $class => $status) { + if (is_a($exceptionClass, $class, true)) { + $statusCode = $status; + + break; + } + } $format = ErrorFormatGuesser::guessErrorFormat($event->getRequest(), $this->errorFormats); $event->setResponse(new Response( $this->serializer->serialize($exception->getConstraintViolationList(), $format['key']), - Response::HTTP_BAD_REQUEST, + $statusCode, [ 'Content-Type' => sprintf('%s; charset=utf-8', $format['value'][0]), 'X-Content-Type-Options' => 'nosniff', From 912fb6e11d0e4fffacebd0993803c714d2ccab50 Mon Sep 17 00:00:00 2001 From: Ahmed Eben Hassine Date: Sat, 31 Oct 2020 15:09:48 +0100 Subject: [PATCH 2/6] fix: apply phpcs fixer --- .../Validator/EventListener/ValidationExceptionListener.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Bridge/Symfony/Validator/EventListener/ValidationExceptionListener.php b/src/Bridge/Symfony/Validator/EventListener/ValidationExceptionListener.php index aa259d1806c..9eea4322015 100644 --- a/src/Bridge/Symfony/Validator/EventListener/ValidationExceptionListener.php +++ b/src/Bridge/Symfony/Validator/EventListener/ValidationExceptionListener.php @@ -46,7 +46,7 @@ public function onKernelException(ExceptionEvent $event): void if (!$exception instanceof ValidationException) { return; } - $exceptionClass = get_class($exception); + $exceptionClass = \get_class($exception); $statusCode = Response::HTTP_BAD_REQUEST; foreach ($this->exceptionToStatus as $class => $status) { From 60ec9a85517f11ea499f7b52f8adb83a6b446030 Mon Sep 17 00:00:00 2001 From: Ahmed Eben Hassine Date: Fri, 6 Nov 2020 12:55:24 +0100 Subject: [PATCH 3/6] fix: ValidationExceptionListener - default to 422 status --- .../Validator/EventListener/ValidationExceptionListener.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Bridge/Symfony/Validator/EventListener/ValidationExceptionListener.php b/src/Bridge/Symfony/Validator/EventListener/ValidationExceptionListener.php index 9eea4322015..d0da61f1d2d 100644 --- a/src/Bridge/Symfony/Validator/EventListener/ValidationExceptionListener.php +++ b/src/Bridge/Symfony/Validator/EventListener/ValidationExceptionListener.php @@ -47,7 +47,7 @@ public function onKernelException(ExceptionEvent $event): void return; } $exceptionClass = \get_class($exception); - $statusCode = Response::HTTP_BAD_REQUEST; + $statusCode = Response::HTTP_UNPROCESSABLE_ENTITY; foreach ($this->exceptionToStatus as $class => $status) { if (is_a($exceptionClass, $class, true)) { From 096f79fdb07e84bd83ed3a7aa66a3eb6733f56eb Mon Sep 17 00:00:00 2001 From: Ahmed Eben Hassine Date: Fri, 6 Nov 2020 12:56:05 +0100 Subject: [PATCH 4/6] fix: grafql default to 422 status --- .../Bundle/Resources/config/graphql.xml | 2 ++ .../ValidationExceptionNormalizer.php | 20 ++++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml b/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml index 48638b0e99a..d9400932005 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml @@ -239,6 +239,8 @@ + %api_platform.exception_to_status% + diff --git a/src/GraphQl/Serializer/Exception/ValidationExceptionNormalizer.php b/src/GraphQl/Serializer/Exception/ValidationExceptionNormalizer.php index d62986ef561..f00473ff966 100644 --- a/src/GraphQl/Serializer/Exception/ValidationExceptionNormalizer.php +++ b/src/GraphQl/Serializer/Exception/ValidationExceptionNormalizer.php @@ -30,6 +30,13 @@ */ final class ValidationExceptionNormalizer implements NormalizerInterface { + private $exceptionToStatus; + + public function __construct(array $exceptionToStatus = []) + { + $this->exceptionToStatus = $exceptionToStatus; + } + /** * {@inheritdoc} */ @@ -39,7 +46,18 @@ public function normalize($object, $format = null, array $context = []): array $validationException = $object->getPrevious(); $error = FormattedError::createFromException($object); $error['message'] = $validationException->getMessage(); - $error['extensions']['status'] = Response::HTTP_BAD_REQUEST; + + $exceptionClass = \get_class($validationException); + $statusCode = Response::HTTP_UNPROCESSABLE_ENTITY; + + foreach ($this->exceptionToStatus as $class => $status) { + if (is_a($exceptionClass, $class, true)) { + $statusCode = $status; + + break; + } + } + $error['extensions']['status'] = $statusCode; $error['extensions']['category'] = 'user'; $error['extensions']['violations'] = []; From 07e88306a162643e505b2b6cbe2574eeb7b1fe41 Mon Sep 17 00:00:00 2001 From: Ahmed Eben Hassine Date: Fri, 6 Nov 2020 12:56:45 +0100 Subject: [PATCH 5/6] test: update behat features --- features/graphql/mutation.feature | 2 +- features/hal/problem.feature | 2 +- features/hydra/error.feature | 2 +- features/jsonapi/errors.feature | 4 ++-- features/main/validation.feature | 4 ++-- features/security/send_security_headers.feature | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/features/graphql/mutation.feature b/features/graphql/mutation.feature index 0baa2ce0b10..a624047ce62 100644 --- a/features/graphql/mutation.feature +++ b/features/graphql/mutation.feature @@ -674,7 +674,7 @@ Feature: GraphQL mutation support Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/json" - And the JSON node "errors[0].extensions.status" should be equal to "400" + And the JSON node "errors[0].extensions.status" should be equal to "422" And the JSON node "errors[0].message" should be equal to "name: This value should not be blank." And the JSON node "errors[0].extensions.violations" should exist And the JSON node "errors[0].extensions.violations[0].path" should be equal to "name" diff --git a/features/hal/problem.feature b/features/hal/problem.feature index fe92ff4ba3f..878d4b21b2f 100644 --- a/features/hal/problem.feature +++ b/features/hal/problem.feature @@ -10,7 +10,7 @@ Feature: Error handling valid according to RFC 7807 (application/problem+json) """ {} """ - Then the response status code should be 400 + 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 JSON should be equal to: diff --git a/features/hydra/error.feature b/features/hydra/error.feature index 9ec12d469fa..79a3e9e9a04 100644 --- a/features/hydra/error.feature +++ b/features/hydra/error.feature @@ -9,7 +9,7 @@ Feature: Error handling """ {} """ - Then the response status code should be 400 + 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/ld+json; charset=utf-8" And the JSON should be equal to: diff --git a/features/jsonapi/errors.feature b/features/jsonapi/errors.feature index 7d37824e052..587ee74a837 100644 --- a/features/jsonapi/errors.feature +++ b/features/jsonapi/errors.feature @@ -18,7 +18,7 @@ Feature: JSON API error handling } } """ - Then the response status code should be 400 + Then the response status code should be 422 And the response should be in JSON And the JSON should be valid according to the JSON API schema And the JSON should be equal to: @@ -49,7 +49,7 @@ Feature: JSON API error handling } } """ - Then the response status code should be 400 + Then the response status code should be 422 And the response should be in JSON And the JSON should be valid according to the JSON API schema And the JSON should be equal to: diff --git a/features/main/validation.feature b/features/main/validation.feature index 960f10718be..95b41b7db8c 100644 --- a/features/main/validation.feature +++ b/features/main/validation.feature @@ -24,7 +24,7 @@ Feature: Using validations groups "code": "My Dummy" } """ - Then the response status code should be 400 + Then the response status code should be 422 And the response should be in JSON And the JSON should be equal to: """ @@ -52,7 +52,7 @@ Feature: Using validations groups "code": "My Dummy" } """ - Then the response status code should be 400 + Then the response status code should be 422 And the response should be in JSON And the JSON should be equal to: """ diff --git a/features/security/send_security_headers.feature b/features/security/send_security_headers.feature index d4b91f77491..b09afc7c316 100644 --- a/features/security/send_security_headers.feature +++ b/features/security/send_security_headers.feature @@ -27,7 +27,7 @@ Feature: Send security header """ {"name": ""} """ - Then the response status code should be 400 + Then the response status code should be 422 And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8" And the header "X-Content-Type-Options" should be equal to "nosniff" And the header "X-Frame-Options" should be equal to "deny" From 293d884ccd5172ef20a289d86c96d7d75f58a743 Mon Sep 17 00:00:00 2001 From: Ahmed Eben Hassine Date: Fri, 6 Nov 2020 12:57:08 +0100 Subject: [PATCH 6/6] test: update phpunit tests --- .../Validator/EventListener/ValidationExceptionListenerTest.php | 2 +- .../Serializer/Exception/ValidationExceptionNormalizerTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Bridge/Symfony/Validator/EventListener/ValidationExceptionListenerTest.php b/tests/Bridge/Symfony/Validator/EventListener/ValidationExceptionListenerTest.php index 7151e553f36..b1128b477c0 100644 --- a/tests/Bridge/Symfony/Validator/EventListener/ValidationExceptionListenerTest.php +++ b/tests/Bridge/Symfony/Validator/EventListener/ValidationExceptionListenerTest.php @@ -57,7 +57,7 @@ public function testValidationException() $response = $event->getResponse(); $this->assertInstanceOf(Response::class, $response); $this->assertSame($exceptionJson, $response->getContent()); - $this->assertSame(Response::HTTP_BAD_REQUEST, $response->getStatusCode()); + $this->assertSame(Response::HTTP_UNPROCESSABLE_ENTITY, $response->getStatusCode()); $this->assertSame('application/ld+json; charset=utf-8', $response->headers->get('Content-Type')); $this->assertSame('nosniff', $response->headers->get('X-Content-Type-Options')); $this->assertSame('deny', $response->headers->get('X-Frame-Options')); diff --git a/tests/GraphQl/Serializer/Exception/ValidationExceptionNormalizerTest.php b/tests/GraphQl/Serializer/Exception/ValidationExceptionNormalizerTest.php index 1f76bc2a17b..bef54b2fbf7 100644 --- a/tests/GraphQl/Serializer/Exception/ValidationExceptionNormalizerTest.php +++ b/tests/GraphQl/Serializer/Exception/ValidationExceptionNormalizerTest.php @@ -48,7 +48,7 @@ public function testNormalize(): void $normalizedError = $this->validationExceptionNormalizer->normalize($error); $this->assertSame($exceptionMessage, $normalizedError['message']); - $this->assertSame(400, $normalizedError['extensions']['status']); + $this->assertSame(422, $normalizedError['extensions']['status']); $this->assertSame('user', $normalizedError['extensions']['category']); $this->assertArrayHasKey('violations', $normalizedError['extensions']); $this->assertSame([