From cc962b72c87bda3f8edd6ac2c3c1f210df987a11 Mon Sep 17 00:00:00 2001 From: Christopher Davis Date: Sat, 6 Nov 2021 06:16:15 -0500 Subject: [PATCH 1/8] Add Validation Group Support to SymfonyConstraintAnnotationReader This support is gated behind a flag as turning it on seems like it would be a large backwards incompatible change. --- .../Annotations/AnnotationsReader.php | 2 +- .../SymfonyConstraintAnnotationReader.php | 46 ++++++- .../SymfonyConstraintAnnotationReaderTest.php | 122 ++++++++++++++++++ 3 files changed, 165 insertions(+), 5 deletions(-) diff --git a/ModelDescriber/Annotations/AnnotationsReader.php b/ModelDescriber/Annotations/AnnotationsReader.php index 62641ab..dcd38bb 100644 --- a/ModelDescriber/Annotations/AnnotationsReader.php +++ b/ModelDescriber/Annotations/AnnotationsReader.php @@ -52,6 +52,6 @@ class AnnotationsReader { $this->openApiAnnotationsReader->updateProperty($reflection, $property, $serializationGroups); $this->phpDocReader->updateProperty($reflection, $property); - $this->symfonyConstraintAnnotationReader->updateProperty($reflection, $property); + $this->symfonyConstraintAnnotationReader->updateProperty($reflection, $property, $serializationGroups); } } diff --git a/ModelDescriber/Annotations/SymfonyConstraintAnnotationReader.php b/ModelDescriber/Annotations/SymfonyConstraintAnnotationReader.php index 8fa31f6..e6cb574 100644 --- a/ModelDescriber/Annotations/SymfonyConstraintAnnotationReader.php +++ b/ModelDescriber/Annotations/SymfonyConstraintAnnotationReader.php @@ -32,9 +32,15 @@ class SymfonyConstraintAnnotationReader */ private $schema; - public function __construct(Reader $annotationsReader) + /** + * @var bool + */ + private $useValidationGroups; + + public function __construct(Reader $annotationsReader, bool $useValidationGroups=false) { $this->annotationsReader = $annotationsReader; + $this->useValidationGroups = $useValidationGroups; } /** @@ -42,9 +48,9 @@ class SymfonyConstraintAnnotationReader * * @param \ReflectionProperty|\ReflectionMethod $reflection */ - public function updateProperty($reflection, OA\Property $property): void + public function updateProperty($reflection, OA\Property $property, ?array $validationGroups = null): void { - foreach ($this->getAnnotations($reflection) as $outerAnnotation) { + foreach ($this->getAnnotations($reflection, $validationGroups) as $outerAnnotation) { $innerAnnotations = $outerAnnotation instanceof Assert\Compound ? $outerAnnotation->constraints : [$outerAnnotation]; @@ -175,7 +181,23 @@ class SymfonyConstraintAnnotationReader /** * @param \ReflectionProperty|\ReflectionMethod $reflection */ - private function getAnnotations($reflection): \Traversable + private function getAnnotations($reflection, ?array $validationGroups): iterable + { + foreach ($this->locateAnnotations($reflection) as $annotation) { + if (! $annotation instanceof Constraint) { + continue; + } + + if (! $this->useValidationGroups || $this->isConstraintInGroup($annotation, $validationGroups)) { + yield $annotation; + } + } + } + + /** + * @param \ReflectionProperty|\ReflectionMethod $reflection + */ + private function locateAnnotations($reflection): \Traversable { if (\PHP_VERSION_ID >= 80000 && class_exists(Constraint::class)) { foreach ($reflection->getAttributes(Constraint::class, \ReflectionAttribute::IS_INSTANCEOF) as $attribute) { @@ -189,4 +211,20 @@ class SymfonyConstraintAnnotationReader yield from $this->annotationsReader->getMethodAnnotations($reflection); } } + + /** + * Check to see if the given constraint is in the provided serialization groups. + * + * If no groups are provided the validator would run in the Constraint::DEFAULT_GROUP, + * and constraints without any `groups` passed to them would be in that same + * default group. So even with a null $validationGroups passed here there still + * has to be a check on the default group. + */ + private function isConstraintInGroup(Constraint $annotation, ?array $validationGroups) : bool + { + return count(array_intersect( + $validationGroups ?: [Constraint::DEFAULT_GROUP], + $annotation->groups + )) > 0; + } } diff --git a/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php b/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php index 9ee7fc8..a864720 100644 --- a/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php +++ b/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php @@ -12,6 +12,7 @@ namespace Nelmio\ApiDocBundle\Tests\ModelDescriber\Annotations; use Doctrine\Common\Annotations\AnnotationReader; +use Symfony\Component\Validator\Constraint; use Nelmio\ApiDocBundle\ModelDescriber\Annotations\SymfonyConstraintAnnotationReader; use Nelmio\ApiDocBundle\Tests\Helper; use Nelmio\ApiDocBundle\Tests\ModelDescriber\Annotations\Fixture as CustomAssert; @@ -423,4 +424,125 @@ class SymfonyConstraintAnnotationReaderTest extends TestCase }]; } } + + /** + * re-using another provider here, since all constraints land in the default + * group when `group={"someGroup"}` is not set. + * + * @group https://github.com/nelmio/NelmioApiDocBundle/issues/1857 + * @dataProvider provideRangeConstraintDoesNotSetMinimumIfMinIsNotSet + */ + public function testReaderWithValidationGroupsEnabledChecksForDefaultGroupWhenNoSerializationGroupsArePassed($entity) + { + $schema = new OA\Schema([]); + $schema->merge([new OA\Property(['property' => 'property1'])]); + $reader = $this->createConstraintReaderWithValidationGroupsEnabled(); + $reader->setSchema($schema); + + // no serialization groups passed here + $reader->updateProperty( + new \ReflectionProperty($entity, 'property1'), + $schema->properties[0] + ); + + $this->assertSame(10, $schema->properties[0]->maximum, 'should have read constraints in the default group'); + } + + /** + * @group https://github.com/nelmio/NelmioApiDocBundle/issues/1857 + * @dataProvider provideConstraintsWithGroups + */ + public function testReaderWithValidationGroupsEnabledDoesNotReadAnnotationsWithoutDefaultGroupIfNoGroupsArePassed($entity) + { + $schema = new OA\Schema([]); + $schema->merge([ + new OA\Property(['property' => 'property1']), + ]); + $reader = $this->createConstraintReaderWithValidationGroupsEnabled(); + $reader->setSchema($schema); + + // no serialization groups passed here + $reader->updateProperty( + new \ReflectionProperty($entity, 'property1'), + $schema->properties[0] + ); + + $this->assertSame(['property1'], $schema->required, 'should have read constraint in default group'); + $this->assertSame(OA\UNDEFINED, $schema->properties[0]->minimum, 'should not have read constraint in other group'); + } + + /** + * @group https://github.com/nelmio/NelmioApiDocBundle/issues/1857 + * @dataProvider provideConstraintsWithGroups + */ + public function testReaderWithValidationGroupsEnabledReadsOnlyConstraintsWithGroupsProvided($entity) + { + $schema = new OA\Schema([]); + $schema->merge([ + new OA\Property(['property' => 'property1']), + ]); + $reader = $this->createConstraintReaderWithValidationGroupsEnabled(); + $reader->setSchema($schema); + + // no serialization groups passed here + $reader->updateProperty( + new \ReflectionProperty($entity, 'property1'), + $schema->properties[0], + ["other"], + ); + + $this->assertSame(OA\UNDEFINED, $schema->required, 'should not have read constraint in default group'); + $this->assertSame(1, $schema->properties[0]->minimum, 'should have read constraint in other group'); + } + + /** + * @group https://github.com/nelmio/NelmioApiDocBundle/issues/1857 + * @dataProvider provideConstraintsWithGroups + */ + public function testReaderWithValidationGroupsEnabledCanReadFromMultipleValidationGroups($entity) + { + $schema = new OA\Schema([]); + $schema->merge([ + new OA\Property(['property' => 'property1']), + ]); + $reader = $this->createConstraintReaderWithValidationGroupsEnabled(); + $reader->setSchema($schema); + + // no serialization groups passed here + $reader->updateProperty( + new \ReflectionProperty($entity, 'property1'), + $schema->properties[0], + ["other", Constraint::DEFAULT_GROUP], + ); + + $this->assertSame(['property1'], $schema->required, 'should have read constraint in default group'); + $this->assertSame(1, $schema->properties[0]->minimum, 'should have read constraint in other group'); + } + + public function provideConstraintsWithGroups(): iterable + { + yield 'Annotations' => [new class() { + /** + * @Assert\NotBlank() + * @Assert\Range(min=1, groups={"other"}) + */ + private $property1; + }]; + + if (\PHP_VERSION_ID >= 80000) { + yield 'Attributes' => [new class() { + #[Assert\NotBlank()] + #[Assert\Range(min: 1, group: ["other"])] + private $property1; + }]; + } + } + + private function createConstraintReaderWithValidationGroupsEnabled() : SymfonyConstraintAnnotationReader + { + return new SymfonyConstraintAnnotationReader( + new AnnotationReader(), + true + ); + } } From 7357de9c16d26b49dff0a583402611152c9087e2 Mon Sep 17 00:00:00 2001 From: Christopher Davis Date: Sat, 6 Nov 2021 07:13:56 -0500 Subject: [PATCH 2/8] Add a Configuration Option to Enable Validation Groups If this was turned on by default, that seems like a _large_ BC break as folks entire OpenAPI doc could change underneath them. The config option defaults to false and users can enable it if they desire. --- .../Compiler/ConfigurationPass.php | 1 + DependencyInjection/Configuration.php | 4 + DependencyInjection/NelmioApiDocExtension.php | 2 + .../Annotations/AnnotationsReader.php | 13 +++- ModelDescriber/FormModelDescriber.php | 17 +++- ModelDescriber/JMSModelDescriber.php | 16 +++- ModelDescriber/ObjectModelDescriber.php | 13 +++- Resources/config/services.xml | 1 + Tests/Functional/Controller/ApiController.php | 13 ++++ .../Functional/Entity/SymfonyConstraints.php | 2 +- ...SymfonyConstraintsWithValidationGroups.php | 36 +++++++++ Tests/Functional/FunctionalTest.php | 4 +- Tests/Functional/TestKernel.php | 14 ++++ .../ValidationGroupsFunctionalTest.php | 77 +++++++++++++++++++ 14 files changed, 200 insertions(+), 13 deletions(-) create mode 100644 Tests/Functional/Entity/SymfonyConstraintsWithValidationGroups.php create mode 100644 Tests/Functional/ValidationGroupsFunctionalTest.php diff --git a/DependencyInjection/Compiler/ConfigurationPass.php b/DependencyInjection/Compiler/ConfigurationPass.php index 2bfa5ec..674062f 100644 --- a/DependencyInjection/Compiler/ConfigurationPass.php +++ b/DependencyInjection/Compiler/ConfigurationPass.php @@ -31,6 +31,7 @@ final class ConfigurationPass implements CompilerPassInterface ->addArgument(new Reference('form.factory')) ->addArgument(new Reference('annotations.reader')) ->addArgument($container->getParameter('nelmio_api_doc.media_types')) + ->addArgument($container->getParameter('nelmio_api_doc.use_validation_groups')) ->addTag('nelmio_api_doc.model_describer', ['priority' => 100]); } diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 5a4c2e3..18879d6 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -29,6 +29,10 @@ final class Configuration implements ConfigurationInterface $rootNode ->children() + ->booleanNode('use_validation_groups') + ->info('If true, `groups` passed to @Model annotations will be used to limit validation constraints') + ->defaultFalse() + ->end() ->arrayNode('documentation') ->useAttributeAsKey('key') ->info('The documentation used as base') diff --git a/DependencyInjection/NelmioApiDocExtension.php b/DependencyInjection/NelmioApiDocExtension.php index cd6a622..1b7f113 100644 --- a/DependencyInjection/NelmioApiDocExtension.php +++ b/DependencyInjection/NelmioApiDocExtension.php @@ -64,6 +64,7 @@ final class NelmioApiDocExtension extends Extension implements PrependExtensionI $container->setParameter('nelmio_api_doc.areas', array_keys($config['areas'])); $container->setParameter('nelmio_api_doc.media_types', $config['media_types']); + $container->setParameter('nelmio_api_doc.use_validation_groups', $config['use_validation_groups']); foreach ($config['areas'] as $area => $areaConfig) { $nameAliases = $this->findNameAliases($config['models']['names'], $area); $container->register(sprintf('nelmio_api_doc.generator.%s', $area), ApiDocGenerator::class) @@ -175,6 +176,7 @@ final class NelmioApiDocExtension extends Extension implements PrependExtensionI new Reference('annotations.reader'), $config['media_types'], $jmsNamingStrategy, + $container->getParameter('nelmio_api_doc.use_validation_groups') ]) ->addTag('nelmio_api_doc.model_describer', ['priority' => 50]); diff --git a/ModelDescriber/Annotations/AnnotationsReader.php b/ModelDescriber/Annotations/AnnotationsReader.php index dcd38bb..3fbcbfb 100644 --- a/ModelDescriber/Annotations/AnnotationsReader.php +++ b/ModelDescriber/Annotations/AnnotationsReader.php @@ -27,14 +27,21 @@ class AnnotationsReader private $openApiAnnotationsReader; private $symfonyConstraintAnnotationReader; - public function __construct(Reader $annotationsReader, ModelRegistry $modelRegistry, array $mediaTypes) - { + public function __construct( + Reader $annotationsReader, + ModelRegistry $modelRegistry, + array $mediaTypes, + bool $useValidationGroups = false + ) { $this->annotationsReader = $annotationsReader; $this->modelRegistry = $modelRegistry; $this->phpDocReader = new PropertyPhpDocReader(); $this->openApiAnnotationsReader = new OpenApiAnnotationsReader($annotationsReader, $modelRegistry, $mediaTypes); - $this->symfonyConstraintAnnotationReader = new SymfonyConstraintAnnotationReader($annotationsReader); + $this->symfonyConstraintAnnotationReader = new SymfonyConstraintAnnotationReader( + $annotationsReader, + $useValidationGroups + ); } public function updateDefinition(\ReflectionClass $reflectionClass, OA\Schema $schema): void diff --git a/ModelDescriber/FormModelDescriber.php b/ModelDescriber/FormModelDescriber.php index 6bff0e4..a0f5c2a 100644 --- a/ModelDescriber/FormModelDescriber.php +++ b/ModelDescriber/FormModelDescriber.php @@ -37,9 +37,14 @@ final class FormModelDescriber implements ModelDescriberInterface, ModelRegistry private $formFactory; private $doctrineReader; private $mediaTypes; + private $useValidationGroups; - public function __construct(FormFactoryInterface $formFactory = null, Reader $reader = null, array $mediaTypes = null) - { + public function __construct( + FormFactoryInterface $formFactory = null, + Reader $reader = null, + array $mediaTypes = null, + bool $useValidationGroups = false + ) { $this->formFactory = $formFactory; $this->doctrineReader = $reader; if (null === $reader) { @@ -51,6 +56,7 @@ final class FormModelDescriber implements ModelDescriberInterface, ModelRegistry @trigger_error(sprintf('Not passing media types to the constructor of %s is deprecated since version 4.1 and won\'t be allowed in version 5.', self::class), E_USER_DEPRECATED); } $this->mediaTypes = $mediaTypes; + $this->useValidationGroups = $useValidationGroups; } public function describe(Model $model, OA\Schema $schema) @@ -66,7 +72,12 @@ final class FormModelDescriber implements ModelDescriberInterface, ModelRegistry $class = $model->getType()->getClassName(); - $annotationsReader = new AnnotationsReader($this->doctrineReader, $this->modelRegistry, $this->mediaTypes); + $annotationsReader = new AnnotationsReader( + $this->doctrineReader, + $this->modelRegistry, + $this->mediaTypes, + $this->useValidationGroups + ); $annotationsReader->updateDefinition(new \ReflectionClass($class), $schema); $form = $this->formFactory->create($class, null, $model->getOptions() ?? []); diff --git a/ModelDescriber/JMSModelDescriber.php b/ModelDescriber/JMSModelDescriber.php index c81519a..129c432 100644 --- a/ModelDescriber/JMSModelDescriber.php +++ b/ModelDescriber/JMSModelDescriber.php @@ -49,16 +49,23 @@ class JMSModelDescriber implements ModelDescriberInterface, ModelRegistryAwareIn */ private $propertyTypeUseGroupsCache = []; + /** + * @var bool + */ + private $useValidationGroups; + public function __construct( MetadataFactoryInterface $factory, Reader $reader, array $mediaTypes, - ?PropertyNamingStrategyInterface $namingStrategy = null + ?PropertyNamingStrategyInterface $namingStrategy = null, + bool $useValidationGroups = false ) { $this->factory = $factory; $this->namingStrategy = $namingStrategy; $this->doctrineReader = $reader; $this->mediaTypes = $mediaTypes; + $this->useValidationGroups = $useValidationGroups; } /** @@ -73,7 +80,12 @@ class JMSModelDescriber implements ModelDescriberInterface, ModelRegistryAwareIn } $schema->type = 'object'; - $annotationsReader = new AnnotationsReader($this->doctrineReader, $this->modelRegistry, $this->mediaTypes); + $annotationsReader = new AnnotationsReader( + $this->doctrineReader, + $this->modelRegistry, + $this->mediaTypes, + $this->useValidationGroups + ); $annotationsReader->updateDefinition(new \ReflectionClass($className), $schema); $isJmsV1 = null !== $this->namingStrategy; diff --git a/ModelDescriber/ObjectModelDescriber.php b/ModelDescriber/ObjectModelDescriber.php index 0aa339a..c4c9b87 100644 --- a/ModelDescriber/ObjectModelDescriber.php +++ b/ModelDescriber/ObjectModelDescriber.php @@ -40,19 +40,23 @@ class ObjectModelDescriber implements ModelDescriberInterface, ModelRegistryAwar private $mediaTypes; /** @var NameConverterInterface[] */ private $nameConverter; + /** @var bool */ + private $useValidationGroups; public function __construct( PropertyInfoExtractorInterface $propertyInfo, Reader $reader, iterable $propertyDescribers, array $mediaTypes, - NameConverterInterface $nameConverter = null + NameConverterInterface $nameConverter = null, + bool $useValidationGroups = false ) { $this->propertyInfo = $propertyInfo; $this->doctrineReader = $reader; $this->propertyDescribers = $propertyDescribers; $this->mediaTypes = $mediaTypes; $this->nameConverter = $nameConverter; + $this->useValidationGroups = $useValidationGroups; } public function describe(Model $model, OA\Schema $schema) @@ -68,7 +72,12 @@ class ObjectModelDescriber implements ModelDescriberInterface, ModelRegistryAwar } $reflClass = new \ReflectionClass($class); - $annotationsReader = new AnnotationsReader($this->doctrineReader, $this->modelRegistry, $this->mediaTypes); + $annotationsReader = new AnnotationsReader( + $this->doctrineReader, + $this->modelRegistry, + $this->mediaTypes, + $this->useValidationGroups + ); $annotationsReader->updateDefinition($reflClass, $schema); $discriminatorMap = $this->doctrineReader->getClassAnnotation($reflClass, DiscriminatorMap::class); diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 9d2c38c..bfc845c 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -75,6 +75,7 @@ + %nelmio_api_doc.use_validation_groups% diff --git a/Tests/Functional/Controller/ApiController.php b/Tests/Functional/Controller/ApiController.php index 4a40897..3de4c5d 100644 --- a/Tests/Functional/Controller/ApiController.php +++ b/Tests/Functional/Controller/ApiController.php @@ -18,6 +18,7 @@ use Nelmio\ApiDocBundle\Annotation\Security; use Nelmio\ApiDocBundle\Tests\Functional\Entity\Article; use Nelmio\ApiDocBundle\Tests\Functional\Entity\CompoundEntity; use Nelmio\ApiDocBundle\Tests\Functional\Entity\SymfonyConstraints; +use Nelmio\ApiDocBundle\Tests\Functional\Entity\SymfonyConstraintsWithValidationGroups; use Nelmio\ApiDocBundle\Tests\Functional\Entity\SymfonyDiscriminator; use Nelmio\ApiDocBundle\Tests\Functional\Entity\User; use Nelmio\ApiDocBundle\Tests\Functional\Form\DummyType; @@ -179,6 +180,18 @@ class ApiController { } + /** + * @Route("/swagger/symfonyConstraintsWithValidationGroups", methods={"GET"}) + * @OA\Response( + * response="201", + * description="Used for symfony constraints with validation groups test", + * @Model(type=SymfonyConstraintsWithValidationGroups::class, groups={"test"}) + * ) + */ + public function symfonyConstraintsWithGroupsAction() + { + } + /** * @OA\Response( * response="200", diff --git a/Tests/Functional/Entity/SymfonyConstraints.php b/Tests/Functional/Entity/SymfonyConstraints.php index 05e65c9..86e3d16 100644 --- a/Tests/Functional/Entity/SymfonyConstraints.php +++ b/Tests/Functional/Entity/SymfonyConstraints.php @@ -19,7 +19,7 @@ class SymfonyConstraints /** * @var int * - * @Assert\NotBlank() + * @Assert\NotBlank(groups={"test"}) */ private $propertyNotBlank; diff --git a/Tests/Functional/Entity/SymfonyConstraintsWithValidationGroups.php b/Tests/Functional/Entity/SymfonyConstraintsWithValidationGroups.php new file mode 100644 index 0000000..01057b2 --- /dev/null +++ b/Tests/Functional/Entity/SymfonyConstraintsWithValidationGroups.php @@ -0,0 +1,36 @@ + [ 'propertyNotBlank' => [ 'type' => 'integer', - 'maxItems' => '10', - 'minItems' => '0', + 'maxItems' => 10, + 'minItems' => 0, ], 'propertyNotNull' => [ 'type' => 'integer', diff --git a/Tests/Functional/TestKernel.php b/Tests/Functional/TestKernel.php index 5de564d..fea681b 100644 --- a/Tests/Functional/TestKernel.php +++ b/Tests/Functional/TestKernel.php @@ -19,6 +19,7 @@ use JMS\SerializerBundle\JMSSerializerBundle; use Nelmio\ApiDocBundle\NelmioApiDocBundle; use Nelmio\ApiDocBundle\Tests\Functional\Entity\BazingaUser; use Nelmio\ApiDocBundle\Tests\Functional\Entity\JMSComplex; +use Nelmio\ApiDocBundle\Tests\Functional\Entity\SymfonyConstraintsWithValidationGroups; use Nelmio\ApiDocBundle\Tests\Functional\Entity\NestedGroup\JMSPicture; use Nelmio\ApiDocBundle\Tests\Functional\Entity\PrivateProtectedExposure; use Nelmio\ApiDocBundle\Tests\Functional\ModelDescriber\VirtualTypeClassDoesNotExistsHandlerDefinedDescriber; @@ -32,12 +33,14 @@ use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\HttpKernel\Kernel; use Symfony\Component\Routing\RouteCollectionBuilder; use Symfony\Component\Serializer\Annotation\SerializedName; +use Symfony\Component\Validator\Constraint; class TestKernel extends Kernel { const USE_JMS = 1; const USE_BAZINGA = 2; const ERROR_ARRAY_ITEMS = 4; + const USE_VALIDATION_GROUPS = 8; use MicroKernelTrait; @@ -173,6 +176,7 @@ class TestKernel extends Kernel // Filter routes $c->loadFromExtension('nelmio_api_doc', [ + 'use_validation_groups' => boolval($this->flags & self::USE_VALIDATION_GROUPS), 'documentation' => [ 'servers' => [ // from https://github.com/nelmio/NelmioApiDocBundle/issues/1691 [ @@ -263,6 +267,16 @@ class TestKernel extends Kernel 'type' => JMSComplex::class, 'groups' => null, ], + [ + 'alias' => 'SymfonyConstraintsTestGroup', + 'type' => SymfonyConstraintsWithValidationGroups::class, + 'groups' => ['test'], + ], + [ + 'alias' => 'SymfonyConstraintsDefaultGroup', + 'type' => SymfonyConstraintsWithValidationGroups::class, + 'groups' => null, + ], ], ], ]); diff --git a/Tests/Functional/ValidationGroupsFunctionalTest.php b/Tests/Functional/ValidationGroupsFunctionalTest.php new file mode 100644 index 0000000..611b869 --- /dev/null +++ b/Tests/Functional/ValidationGroupsFunctionalTest.php @@ -0,0 +1,77 @@ + 'api.example.com']); + } + + public function testConstraintGroupsAreRespectedWhenDescribingModels() + { + $expected = [ + 'required' => [ + 'property', + ], + 'properties' => [ + 'property' => [ + 'type' => 'integer', + // the min/max constraint is in the default group only and shouldn't + // be read here with validation groups turned on + ], + ], + 'type' => 'object', + 'schema' => 'SymfonyConstraintsTestGroup', + ]; + + $this->assertEquals( + $expected, + json_decode($this->getModel('SymfonyConstraintsTestGroup')->toJson(), true) + ); + } + + public function testConstraintDefaultGroupsAreRespectedWhenReadingAnnotations() + { + $expected = [ + 'properties' => [ + 'property' => [ + 'type' => 'integer', + // min/max will be read here as they are in th e default group + 'maximum' => 100, + 'minimum' => 1, + ], + 'propertyInDefaultGroup' => [ + 'type' => 'integer', + // min/max will be read here as they are in th e default group + 'maximum' => 100, + 'minimum' => 1, + ], + ], + 'type' => 'object', + 'schema' => 'SymfonyConstraintsDefaultGroup', + ]; + + $this->assertEquals( + $expected, + json_decode($this->getModel('SymfonyConstraintsDefaultGroup')->toJson(), true) + ); + } +} From 76949ca53750826aa253df8abbc600b9c3429ba1 Mon Sep 17 00:00:00 2001 From: Christopher Davis Date: Sat, 6 Nov 2021 07:28:13 -0500 Subject: [PATCH 3/8] Fix a Deprecated Method Call on Symfony 5.3 --- PropertyDescriber/ObjectPropertyDescriber.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/PropertyDescriber/ObjectPropertyDescriber.php b/PropertyDescriber/ObjectPropertyDescriber.php index ef1aabc..d5836c0 100644 --- a/PropertyDescriber/ObjectPropertyDescriber.php +++ b/PropertyDescriber/ObjectPropertyDescriber.php @@ -28,7 +28,10 @@ class ObjectPropertyDescriber implements PropertyDescriberInterface, ModelRegist false, $types[0]->getClassName(), $types[0]->isCollection(), - $types[0]->getCollectionKeyType(), + // BC layer for symfony < 5.3 + method_exists($types[0], 'getCollectionKeyTypes') ? + ($types[0]->getCollectionKeyTypes()[0] ?? null) : + $types[0]->getCollectionKeyType(), // BC layer for symfony < 5.3 method_exists($types[0], 'getCollectionValueTypes') ? ($types[0]->getCollectionValueTypes()[0] ?? null) : From 477442588a1830f229b6c9920784ce62c27664dd Mon Sep 17 00:00:00 2001 From: Christopher Davis Date: Sat, 6 Nov 2021 07:35:50 -0500 Subject: [PATCH 4/8] Fix CS --- DependencyInjection/NelmioApiDocExtension.php | 2 +- .../Annotations/SymfonyConstraintAnnotationReader.php | 6 +++--- .../Entity/SymfonyConstraintsWithValidationGroups.php | 4 +--- Tests/Functional/TestKernel.php | 3 +-- .../SymfonyConstraintAnnotationReaderTest.php | 10 +++++----- 5 files changed, 11 insertions(+), 14 deletions(-) diff --git a/DependencyInjection/NelmioApiDocExtension.php b/DependencyInjection/NelmioApiDocExtension.php index 1b7f113..1a40a42 100644 --- a/DependencyInjection/NelmioApiDocExtension.php +++ b/DependencyInjection/NelmioApiDocExtension.php @@ -176,7 +176,7 @@ final class NelmioApiDocExtension extends Extension implements PrependExtensionI new Reference('annotations.reader'), $config['media_types'], $jmsNamingStrategy, - $container->getParameter('nelmio_api_doc.use_validation_groups') + $container->getParameter('nelmio_api_doc.use_validation_groups'), ]) ->addTag('nelmio_api_doc.model_describer', ['priority' => 50]); diff --git a/ModelDescriber/Annotations/SymfonyConstraintAnnotationReader.php b/ModelDescriber/Annotations/SymfonyConstraintAnnotationReader.php index e6cb574..aac6c1d 100644 --- a/ModelDescriber/Annotations/SymfonyConstraintAnnotationReader.php +++ b/ModelDescriber/Annotations/SymfonyConstraintAnnotationReader.php @@ -184,11 +184,11 @@ class SymfonyConstraintAnnotationReader private function getAnnotations($reflection, ?array $validationGroups): iterable { foreach ($this->locateAnnotations($reflection) as $annotation) { - if (! $annotation instanceof Constraint) { + if (!$annotation instanceof Constraint) { continue; } - if (! $this->useValidationGroups || $this->isConstraintInGroup($annotation, $validationGroups)) { + if (!$this->useValidationGroups || $this->isConstraintInGroup($annotation, $validationGroups)) { yield $annotation; } } @@ -220,7 +220,7 @@ class SymfonyConstraintAnnotationReader * default group. So even with a null $validationGroups passed here there still * has to be a check on the default group. */ - private function isConstraintInGroup(Constraint $annotation, ?array $validationGroups) : bool + private function isConstraintInGroup(Constraint $annotation, ?array $validationGroups): bool { return count(array_intersect( $validationGroups ?: [Constraint::DEFAULT_GROUP], diff --git a/Tests/Functional/Entity/SymfonyConstraintsWithValidationGroups.php b/Tests/Functional/Entity/SymfonyConstraintsWithValidationGroups.php index 01057b2..65c2896 100644 --- a/Tests/Functional/Entity/SymfonyConstraintsWithValidationGroups.php +++ b/Tests/Functional/Entity/SymfonyConstraintsWithValidationGroups.php @@ -11,10 +11,8 @@ namespace Nelmio\ApiDocBundle\Tests\Functional\Entity; -use Nelmio\ApiDocBundle\Tests\ModelDescriber\Annotations\Fixture as CustomAssert; -use Symfony\Component\Validator\Constraint; -use Symfony\Component\Validator\Constraints as Assert; use Symfony\Component\Serializer\Annotation\Groups; +use Symfony\Component\Validator\Constraints as Assert; class SymfonyConstraintsWithValidationGroups { diff --git a/Tests/Functional/TestKernel.php b/Tests/Functional/TestKernel.php index fea681b..cd1890c 100644 --- a/Tests/Functional/TestKernel.php +++ b/Tests/Functional/TestKernel.php @@ -19,9 +19,9 @@ use JMS\SerializerBundle\JMSSerializerBundle; use Nelmio\ApiDocBundle\NelmioApiDocBundle; use Nelmio\ApiDocBundle\Tests\Functional\Entity\BazingaUser; use Nelmio\ApiDocBundle\Tests\Functional\Entity\JMSComplex; -use Nelmio\ApiDocBundle\Tests\Functional\Entity\SymfonyConstraintsWithValidationGroups; use Nelmio\ApiDocBundle\Tests\Functional\Entity\NestedGroup\JMSPicture; use Nelmio\ApiDocBundle\Tests\Functional\Entity\PrivateProtectedExposure; +use Nelmio\ApiDocBundle\Tests\Functional\Entity\SymfonyConstraintsWithValidationGroups; use Nelmio\ApiDocBundle\Tests\Functional\ModelDescriber\VirtualTypeClassDoesNotExistsHandlerDefinedDescriber; use Sensio\Bundle\FrameworkExtraBundle\SensioFrameworkExtraBundle; use Symfony\Bundle\FrameworkBundle\FrameworkBundle; @@ -33,7 +33,6 @@ use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\HttpKernel\Kernel; use Symfony\Component\Routing\RouteCollectionBuilder; use Symfony\Component\Serializer\Annotation\SerializedName; -use Symfony\Component\Validator\Constraint; class TestKernel extends Kernel { diff --git a/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php b/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php index a864720..d9e84e4 100644 --- a/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php +++ b/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php @@ -12,12 +12,12 @@ namespace Nelmio\ApiDocBundle\Tests\ModelDescriber\Annotations; use Doctrine\Common\Annotations\AnnotationReader; -use Symfony\Component\Validator\Constraint; use Nelmio\ApiDocBundle\ModelDescriber\Annotations\SymfonyConstraintAnnotationReader; use Nelmio\ApiDocBundle\Tests\Helper; use Nelmio\ApiDocBundle\Tests\ModelDescriber\Annotations\Fixture as CustomAssert; use OpenApi\Annotations as OA; use PHPUnit\Framework\TestCase; +use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\Constraints as Assert; class SymfonyConstraintAnnotationReaderTest extends TestCase @@ -488,7 +488,7 @@ class SymfonyConstraintAnnotationReaderTest extends TestCase $reader->updateProperty( new \ReflectionProperty($entity, 'property1'), $schema->properties[0], - ["other"], + ['other'], ); $this->assertSame(OA\UNDEFINED, $schema->required, 'should not have read constraint in default group'); @@ -512,7 +512,7 @@ class SymfonyConstraintAnnotationReaderTest extends TestCase $reader->updateProperty( new \ReflectionProperty($entity, 'property1'), $schema->properties[0], - ["other", Constraint::DEFAULT_GROUP], + ['other', Constraint::DEFAULT_GROUP], ); $this->assertSame(['property1'], $schema->required, 'should have read constraint in default group'); @@ -532,13 +532,13 @@ class SymfonyConstraintAnnotationReaderTest extends TestCase if (\PHP_VERSION_ID >= 80000) { yield 'Attributes' => [new class() { #[Assert\NotBlank()] - #[Assert\Range(min: 1, group: ["other"])] + #[Assert\Range(min: 1, group: ['other'])] private $property1; }]; } } - private function createConstraintReaderWithValidationGroupsEnabled() : SymfonyConstraintAnnotationReader + private function createConstraintReaderWithValidationGroupsEnabled(): SymfonyConstraintAnnotationReader { return new SymfonyConstraintAnnotationReader( new AnnotationReader(), From 10d6d57ca7921534c0f02abf05a8e7188b48635b Mon Sep 17 00:00:00 2001 From: Christopher Davis Date: Sat, 6 Nov 2021 07:37:42 -0500 Subject: [PATCH 5/8] Remove Some Trailing Comma --- .../Annotations/SymfonyConstraintAnnotationReaderTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php b/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php index d9e84e4..4baed85 100644 --- a/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php +++ b/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php @@ -488,7 +488,7 @@ class SymfonyConstraintAnnotationReaderTest extends TestCase $reader->updateProperty( new \ReflectionProperty($entity, 'property1'), $schema->properties[0], - ['other'], + ['other'] ); $this->assertSame(OA\UNDEFINED, $schema->required, 'should not have read constraint in default group'); @@ -512,7 +512,7 @@ class SymfonyConstraintAnnotationReaderTest extends TestCase $reader->updateProperty( new \ReflectionProperty($entity, 'property1'), $schema->properties[0], - ['other', Constraint::DEFAULT_GROUP], + ['other', Constraint::DEFAULT_GROUP] ); $this->assertSame(['property1'], $schema->required, 'should have read constraint in default group'); From 1e45fdf1d6822452ee5edf3cfdade9e3ee353672 Mon Sep 17 00:00:00 2001 From: Christopher Davis Date: Sat, 6 Nov 2021 07:38:56 -0500 Subject: [PATCH 6/8] Fix a Named Parameter on PHP 8 --- .../Annotations/SymfonyConstraintAnnotationReaderTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php b/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php index 4baed85..cb628d7 100644 --- a/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php +++ b/Tests/ModelDescriber/Annotations/SymfonyConstraintAnnotationReaderTest.php @@ -532,7 +532,7 @@ class SymfonyConstraintAnnotationReaderTest extends TestCase if (\PHP_VERSION_ID >= 80000) { yield 'Attributes' => [new class() { #[Assert\NotBlank()] - #[Assert\Range(min: 1, group: ['other'])] + #[Assert\Range(min: 1, groups: ['other'])] private $property1; }]; } From b399cb4c7e0db64ad2389cbd78964c8304fb6e51 Mon Sep 17 00:00:00 2001 From: Guilhem Niot Date: Sun, 19 Dec 2021 11:41:36 +0100 Subject: [PATCH 7/8] fix tests --- Tests/Functional/ValidationGroupsFunctionalTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Tests/Functional/ValidationGroupsFunctionalTest.php b/Tests/Functional/ValidationGroupsFunctionalTest.php index 611b869..70d018e 100644 --- a/Tests/Functional/ValidationGroupsFunctionalTest.php +++ b/Tests/Functional/ValidationGroupsFunctionalTest.php @@ -11,9 +11,11 @@ namespace Nelmio\ApiDocBundle\Tests\Functional; +use Symfony\Component\HttpKernel\KernelInterface; + class ValidationGroupsFunctionalTest extends WebTestCase { - protected static function createKernel(array $options = []) + protected static function createKernel(array $options = []): KernelInterface { return new TestKernel(TestKernel::USE_VALIDATION_GROUPS); } From ccd3689faaabc329e7777450cc03a1a4b083aeb3 Mon Sep 17 00:00:00 2001 From: Christopher Davis Date: Wed, 11 May 2022 08:52:48 -0500 Subject: [PATCH 8/8] Expand the Documentation on Groups to Include Validation With some examples of objects and the use of model as well as some implications for generated code. --- Resources/doc/index.rst | 90 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 88 insertions(+), 2 deletions(-) diff --git a/Resources/doc/index.rst b/Resources/doc/index.rst index 6bc43bd..553a9f4 100644 --- a/Resources/doc/index.rst +++ b/Resources/doc/index.rst @@ -233,7 +233,7 @@ The normal PHPdoc block on the controller method is used for the summary and des However, unlike in those examples, when using this bundle you don't need to specify paths and you can easily document models as well as some other properties described below as they can be automatically be documented using the Symfony integration. -Use models +Use Models ---------- As shown in the example above, the bundle provides the ``@Model`` annotation. @@ -259,7 +259,7 @@ This annotation has two options: */ .. code-block:: php-attributes - + #[OA\Response( response: 200, description: 'Successful response', @@ -288,6 +288,92 @@ This annotation has two options: content: new Model(type: User::class, groups: ['non_sensitive_data']) )] +* ``groups`` may also be used to specify the constraint validation groups used + (de)serialize your model, but this must be enabled in configuration:: + +.. code-block:: yaml + + nelmio_api_doc: + use_validation_groups: true + # ... + +With this enabled, groups set in the model will apply to both serializer +properties and validator constraints. Take the model class below: + +.. configuration-block:: + + .. code-block:: php-annotations + + use Symfony\Component\Serializer\Annotation\Group; + use Symfony\Component\Validator\Constraints as Assert; + + class UserDto + { + /** + * @Group({"default", "create", "update"}) + * @Assert\NotBlank(groups={"default", "create"}) + */ + public string $username; + } + + .. code-block:: php-attributes + + use Symfony\Component\Serializer\Annotation\Group; + use Symfony\Component\Validator\Constraints as Assert; + + class UserDto + { + #[Group(["default", "create", "update"]) + #[Assert\NotBlank(groups: ["default", "create"]) + public string $username; + } + +The ``NotBlank`` constraint will apply only to the ``default`` and ``create`` +group, but not ``update``. In more practical terms: the `username` property +would show as ``required`` for both model create and default, but not update. +When using code generators to build API clients, this often translates into +client side validation and types. ``NotBlank`` adding ``required`` will cause +that property type to not be nullable, for example. + +.. configuration-block:: + + .. code-block:: php-annotations + + use OpenApi\Annotations as OA; + + /** + * shows `username` as `required` in the OpenAPI schema (not nullable) + * @OA\Response( + * response=200, + *     @Model(type=UserDto::class, groups={"default"}) + * ) + */ + + /** + * Similarly, this will make the username `required` in the create + * schema + * @OA\RequestBody(@Model(type=UserDto::class, groups={"create"})) + */ + + /** + * But for updates, the `username` property will not be required + * @OA\RequestBody(@Model(type=UserDto::class, groups={"update"})) + */ + + .. code-block:: php-attributes + + use OpenApi\Attributes as OA; + + // shows `username` as `required` in the OpenAPI schema (not nullable) + #[OA\Response(response: 200, content: new Model(type: UserDto::class, groups: ["default"]))] + + // Similarly, this will make the username `required` in the create schema + #[OA\RequestBody(new Model(type: UserDto::class, groups: ["create"]))] + + // But for updates, the `username` property will not be required + #[OA\RequestBody(new Model(type: UserDto::class, groups: ["update"]))] + + .. tip:: When used at the root of ``@OA\Response`` and ``@OA\Parameter``, ``@Model`` is automatically nested