From 2360674a7bd8bbf5fb834b08e89662b6ad851618 Mon Sep 17 00:00:00 2001 From: Djordy Koert Date: Mon, 15 Jan 2024 17:16:29 +0100 Subject: [PATCH] fix: attribute validation groups not passed (#2189) * fix: attribute validation groups not passed * add test cases * fix baseline --- .../SymfonyMapQueryStringDescriber.php | 26 +++++++++ .../SymfonyMapRequestPayloadDescriber.php | 26 +++++++++ .../Functional/Controller/ApiController81.php | 14 +++++ Tests/Functional/SymfonyFunctionalTest.php | 54 +++++++++++++++++++ phpunit-baseline.json | 30 +++++++++++ 5 files changed, 150 insertions(+) diff --git a/RouteDescriber/RouteArgumentDescriber/SymfonyMapQueryStringDescriber.php b/RouteDescriber/RouteArgumentDescriber/SymfonyMapQueryStringDescriber.php index 95e998a88..3050ec25e 100644 --- a/RouteDescriber/RouteArgumentDescriber/SymfonyMapQueryStringDescriber.php +++ b/RouteDescriber/RouteArgumentDescriber/SymfonyMapQueryStringDescriber.php @@ -11,6 +11,7 @@ use Symfony\Component\HttpKernel\Attribute\MapQueryString; use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata; use Symfony\Component\PropertyInfo\Type; +use Symfony\Component\Validator\Constraints\GroupSequence; final class SymfonyMapQueryStringDescriber implements RouteArgumentDescriberInterface, ModelRegistryAwareInterface { @@ -28,10 +29,35 @@ public function describe(ArgumentMetadata $argumentMetadata, OA\Operation $opera $modelRef = $this->modelRegistry->register(new Model( new Type(Type::BUILTIN_TYPE_OBJECT, $argumentMetadata->isNullable(), $argumentMetadata->getType()), + groups: $this->getGroups($attribute), serializationContext: $attribute->serializationContext, )); $operation->_context->{self::CONTEXT_ARGUMENT_METADATA} = $argumentMetadata; $operation->_context->{self::CONTEXT_MODEL_REF} = $modelRef; } + + /** + * @return string[]|null + */ + private function getGroups(MapQueryString $attribute): ?array + { + if (null === $attribute->validationGroups) { + return null; + } + + if (is_string($attribute->validationGroups)) { + return [$attribute->validationGroups]; + } + + if (is_array($attribute->validationGroups)) { + return $attribute->validationGroups; + } + + if ($attribute->validationGroups instanceof GroupSequence) { + return $attribute->validationGroups->groups; + } + + return null; + } } diff --git a/RouteDescriber/RouteArgumentDescriber/SymfonyMapRequestPayloadDescriber.php b/RouteDescriber/RouteArgumentDescriber/SymfonyMapRequestPayloadDescriber.php index 5deb86744..ae098dd16 100644 --- a/RouteDescriber/RouteArgumentDescriber/SymfonyMapRequestPayloadDescriber.php +++ b/RouteDescriber/RouteArgumentDescriber/SymfonyMapRequestPayloadDescriber.php @@ -11,6 +11,7 @@ use Symfony\Component\HttpKernel\Attribute\MapRequestPayload; use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata; use Symfony\Component\PropertyInfo\Type; +use Symfony\Component\Validator\Constraints\GroupSequence; final class SymfonyMapRequestPayloadDescriber implements RouteArgumentDescriberInterface, ModelRegistryAwareInterface { @@ -28,10 +29,35 @@ public function describe(ArgumentMetadata $argumentMetadata, OA\Operation $opera $modelRef = $this->modelRegistry->register(new Model( new Type(Type::BUILTIN_TYPE_OBJECT, false, $argumentMetadata->getType()), + groups: $this->getGroups($attribute), serializationContext: $attribute->serializationContext, )); $operation->_context->{self::CONTEXT_ARGUMENT_METADATA} = $argumentMetadata; $operation->_context->{self::CONTEXT_MODEL_REF} = $modelRef; } + + /** + * @return string[]|null + */ + private function getGroups(MapRequestPayload $attribute): ?array + { + if (null === $attribute->validationGroups) { + return null; + } + + if (is_string($attribute->validationGroups)) { + return [$attribute->validationGroups]; + } + + if (is_array($attribute->validationGroups)) { + return $attribute->validationGroups; + } + + if ($attribute->validationGroups instanceof GroupSequence) { + return $attribute->validationGroups->groups; + } + + return null; + } } diff --git a/Tests/Functional/Controller/ApiController81.php b/Tests/Functional/Controller/ApiController81.php index 3797cb343..772215dc1 100644 --- a/Tests/Functional/Controller/ApiController81.php +++ b/Tests/Functional/Controller/ApiController81.php @@ -478,6 +478,13 @@ public function fetchArticleFromMapQueryStringNullable( ) { } + #[Route('/article_map_query_string_passes_validation_groups')] + #[OA\Response(response: '200', description: '')] + public function fetchArticleFromMapQueryStringHandlesValidationGroups( + #[MapQueryString(validationGroups: ['test'])] SymfonyConstraintsWithValidationGroups $symfonyConstraintsWithValidationGroups, + ) { + } + #[Route('/article_map_query_string_overwrite_parameters')] #[OA\Parameter( name: 'id', @@ -590,4 +597,11 @@ public function createArticleFromMapRequestPayloadHandlesAlreadySetContent( #[MapRequestPayload] Article81 $article81, ) { } + + #[Route('/article_map_request_payload_validation_groups', methods: ['POST'])] + #[OA\Response(response: '200', description: '')] + public function createArticleFromMapRequestPayloadPassedValidationGroups( + #[MapRequestPayload(validationGroups: ['test'])] SymfonyConstraintsWithValidationGroups $symfonyConstraintsWithValidationGroups, + ) { + } } diff --git a/Tests/Functional/SymfonyFunctionalTest.php b/Tests/Functional/SymfonyFunctionalTest.php index f318cd438..84bf27e86 100644 --- a/Tests/Functional/SymfonyFunctionalTest.php +++ b/Tests/Functional/SymfonyFunctionalTest.php @@ -488,4 +488,58 @@ public function testMapRequestPayloadHandlesAlreadySetContent(): void ], ], json_decode($this->getOperation('/api/article_map_request_payload_handles_already_set_content', 'post')->toJson(), true)); } + + public function testMapRequestPayloadPassesValidationGroups(): void + { + if (!class_exists(MapRequestPayload::class)) { + self::markTestSkipped('Symfony 6.3 MapRequestPayload attribute not found'); + } + + self::assertEquals([ + 'operationId' => 'post_api_nelmio_apidoc_tests_functional_api_createarticlefrommaprequestpayloadpassedvalidationgroups', + 'responses' => [ + '200' => [ + 'description' => '', + ], + ], + 'requestBody' => [ + 'content' => [ + 'application/json' => [ + 'schema' => [ + '$ref' => '#/components/schemas/SymfonyConstraintsTestGroup', + ], + ], + ], + 'required' => true, + ], + ], json_decode($this->getOperation('/api/article_map_request_payload_validation_groups', 'post')->toJson(), true)); + } + + public function testMapQueryStringPassesValidationGroups(): void + { + if (!class_exists(MapQueryString::class)) { + self::markTestSkipped('Symfony 6.3 MapQueryString attribute not found'); + } + + self::assertEquals([ + 'operationId' => 'post_api_nelmio_apidoc_tests_functional_api_fetcharticlefrommapquerystringhandlesvalidationgroups', + 'responses' => [ + '200' => [ + 'description' => '', + ], + ], + 'parameters' => [ + [ + 'name' => 'property', + 'in' => 'query', + 'required' => true, + 'schema' => [ + 'type' => 'integer', + 'maximum' => 100, + 'minimum' => 1, + ], + ], + ], + ], json_decode($this->getOperation('/api/article_map_query_string_passes_validation_groups', 'post')->toJson(), true)); + } } diff --git a/phpunit-baseline.json b/phpunit-baseline.json index 7f1f2acfe..0b6ee32a0 100644 --- a/phpunit-baseline.json +++ b/phpunit-baseline.json @@ -7143,5 +7143,35 @@ "location": "Nelmio\\ApiDocBundle\\Tests\\Functional\\SymfonyFunctionalTest::testMapRequestPayloadHandlesAlreadySetContent", "message": "Since sensio/framework-extra-bundle 5.2: The \"sensio_framework_extra.routing.loader.annot_file\" service is deprecated since version 5.2", "count": 1 + }, + { + "location": "Nelmio\\ApiDocBundle\\Tests\\Functional\\SymfonyFunctionalTest::testMapRequestPayloadPassesValidationGroups", + "message": "Since sensio/framework-extra-bundle 5.2: The \"sensio_framework_extra.routing.loader.annot_class\" service is deprecated since version 5.2", + "count": 1 + }, + { + "location": "Nelmio\\ApiDocBundle\\Tests\\Functional\\SymfonyFunctionalTest::testMapRequestPayloadPassesValidationGroups", + "message": "Since sensio/framework-extra-bundle 5.2: The \"sensio_framework_extra.routing.loader.annot_dir\" service is deprecated since version 5.2", + "count": 1 + }, + { + "location": "Nelmio\\ApiDocBundle\\Tests\\Functional\\SymfonyFunctionalTest::testMapRequestPayloadPassesValidationGroups", + "message": "Since sensio/framework-extra-bundle 5.2: The \"sensio_framework_extra.routing.loader.annot_file\" service is deprecated since version 5.2", + "count": 1 + }, + { + "location": "Nelmio\\ApiDocBundle\\Tests\\Functional\\SymfonyFunctionalTest::testMapQueryStringPassesValidationGroups", + "message": "Since sensio/framework-extra-bundle 5.2: The \"sensio_framework_extra.routing.loader.annot_class\" service is deprecated since version 5.2", + "count": 1 + }, + { + "location": "Nelmio\\ApiDocBundle\\Tests\\Functional\\SymfonyFunctionalTest::testMapQueryStringPassesValidationGroups", + "message": "Since sensio/framework-extra-bundle 5.2: The \"sensio_framework_extra.routing.loader.annot_dir\" service is deprecated since version 5.2", + "count": 1 + }, + { + "location": "Nelmio\\ApiDocBundle\\Tests\\Functional\\SymfonyFunctionalTest::testMapQueryStringPassesValidationGroups", + "message": "Since sensio/framework-extra-bundle 5.2: The \"sensio_framework_extra.routing.loader.annot_file\" service is deprecated since version 5.2", + "count": 1 } ]