fix(SymfonyConstraintAnnotationReader): disallow null if NotNull attr…

…ibute is present (#2329) | Q | A | |---------------|---------------------------------------------------------------------------------------------------------------------------| | Bug fix? | yes | | New feature? | no | | Deprecations? | no | | Issues | Fix #2309 | if a property is set as something like this: ```php #[OA\Property()] #[Assert\NotNull(groups: ['validation'])] private ?string $prop = null; ``` that property will be marked as nullable AND required. This PR ensures that the nullable property is also set appropriately when a NotNull is set NOTE: PHPUnit already fails on master
nelmio · Aug 16, 2024 · ddeb3d4 · ddeb3d4
1 parent fbb94eb
commit ddeb3d4
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 2 deletions.
diff --git a/src/ModelDescriber/Annotations/SymfonyConstraintAnnotationReader.php b/src/ModelDescriber/Annotations/SymfonyConstraintAnnotationReader.php
@@ -90,6 +90,7 @@ private function processPropertyAnnotations($reflection, OA\Property $property,
                 $existingRequiredFields[] = $propertyName;
 
                 $this->schema->required = array_values(array_unique($existingRequiredFields));
+                $property->nullable = false;
             } elseif ($annotation instanceof Assert\Length) {
                 if (isset($annotation->min)) {
                     $property->minLength = $annotation->min;

diff --git a/tests/Functional/Entity/SymfonyConstraintsWithValidationGroups80.php b/tests/Functional/Entity/SymfonyConstraintsWithValidationGroups80.php
@@ -43,4 +43,13 @@ class SymfonyConstraintsWithValidationGroups80
      * @Assert\Valid
      */
     public $propertyArray;
+
+    /**
+     * @var ?string
+     *
+     * @Groups({"test"})
+     *
+     * @Assert\NotNull(groups={"test"})
+     */
+    public $propertyNotNullOnSpecificGroup;
 }
diff --git a/tests/Functional/Entity/SymfonyConstraintsWithValidationGroups81.php b/tests/Functional/Entity/SymfonyConstraintsWithValidationGroups81.php
@@ -37,4 +37,11 @@ class SymfonyConstraintsWithValidationGroups81
     #[OA\Property(type: 'array', items: new OA\Items(type: 'string'))]
     #[Assert\Valid]
     public $propertyArray;
+
+    /**
+     * @var ?string
+     */
+    #[Groups(['test'])]
+    #[Assert\NotNull(groups: ['test'])]
+    public $propertyNotNullOnSpecificGroup;
 }
diff --git a/tests/Functional/Fixtures/MapQueryStringCleanupComponents.json b/tests/Functional/Fixtures/MapQueryStringCleanupComponents.json
@@ -155,6 +155,14 @@
                                 "type": "string"
                             }
                         }
+                    },
+                    {
+                        "name": "propertyNotNullOnSpecificGroup",
+                        "in": "query",
+                        "required": true,
+                        "schema": {
+                            "type": "string"
+                        }
                     }
                 ],
                 "responses": {

diff --git a/tests/Functional/Fixtures/MapQueryStringController.json b/tests/Functional/Fixtures/MapQueryStringController.json
@@ -155,6 +155,14 @@
                                 "type": "string"
                             }
                         }
+                    },
+                    {
+                        "name": "propertyNotNullOnSpecificGroup",
+                        "in": "query",
+                        "required": true,
+                        "schema": {
+                            "type": "string"
+                        }
                     }
                 ],
                 "responses": {
@@ -1095,7 +1103,8 @@
             "SymfonyConstraintsWithValidationGroups": {
                 "required": [
                     "property",
-                    "propertyInDefaultGroup"
+                    "propertyInDefaultGroup",
+                    "propertyNotNullOnSpecificGroup"
                 ],
                 "properties": {
                     "property": {
@@ -1113,6 +1122,9 @@
                         "items": {
                             "type": "string"
                         }
+                    },
+                    "propertyNotNullOnSpecificGroup": {
+                        "type": "string"
                     }
                 },
                 "type": "object"

diff --git a/tests/Functional/Fixtures/MapRequestPayloadController.json b/tests/Functional/Fixtures/MapRequestPayloadController.json
@@ -147,7 +147,8 @@
             "SymfonyConstraintsWithValidationGroups": {
                 "required": [
                     "property",
-                    "propertyInDefaultGroup"
+                    "propertyInDefaultGroup",
+                    "propertyNotNullOnSpecificGroup"
                 ],
                 "properties": {
                     "property": {
@@ -165,6 +166,9 @@
                         "items": {
                             "type": "string"
                         }
+                    },
+                    "propertyNotNullOnSpecificGroup": {
+                        "type": "string"
                     }
                 },
                 "type": "object"

diff --git a/tests/Functional/ValidationGroupsFunctionalTest.php b/tests/Functional/ValidationGroupsFunctionalTest.php
@@ -35,13 +35,17 @@ public function testConstraintGroupsAreRespectedWhenDescribingModels(): void
         $expected = [
             'required' => [
                 'property',
+                'propertyNotNullOnSpecificGroup',
             ],
             '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
                 ],
+                'propertyNotNullOnSpecificGroup' => [
+                    'type' => 'string',
+                ],
             ],
             'type' => 'object',
             'schema' => 'SymfonyConstraintsTestGroup',
@@ -75,6 +79,10 @@ public function testConstraintDefaultGroupsAreRespectedWhenReadingAnnotations():
                         'type' => 'string',
                     ],
                 ],
+                'propertyNotNullOnSpecificGroup' => [
+                    'type' => 'string',
+                    'nullable' => true,
+                ],
             ],
             'type' => 'object',
             'schema' => 'SymfonyConstraintsDefaultGroup',