refactor: determining property required logic (#2359)

| Q | A |

|---------------|---------------------------------------------------------------------------------------------------------------------------|
| Bug fix? | yes |
| New feature? | no |
| Deprecations? |no                                                  |

Move the behaviour for determining required property.

config/services.xml

@@ -139,10 +139,6 @@
             <tag name="nelmio_api_doc.object_model.property_describer" />
         </service>
 
-        <service id="nelmio_api_doc.object_model.property_describers.required" class="Nelmio\ApiDocBundle\PropertyDescriber\RequiredPropertyDescriber" public="false">
-            <tag name="nelmio_api_doc.object_model.property_describer" />
-        </service>
-
         <service id="nelmio_api_doc.object_model.property_describers.object" class="Nelmio\ApiDocBundle\PropertyDescriber\ObjectPropertyDescriber" public="false">
             <tag name="nelmio_api_doc.object_model.property_describer" priority="-1000" />
         </service>

phpstan-baseline.neon

@@ -35,11 +35,6 @@ parameters:
 			count: 1
 			path: src/PropertyDescriber/PropertyDescriberInterface.php
 
-		-
-			message: "#^PHPDoc tag @param references unknown parameter\\: \\$schema$#"
-			count: 1
-			path: src/PropertyDescriber/PropertyDescriberInterface.php
-
 		-
 			message: "#^Method Nelmio\\\\ApiDocBundle\\\\PropertyDescriber\\\\PropertyDescriberInterface\\:\\:describe\\(\\) invoked with 5 parameters, 2\\-3 required\\.$#"
 			count: 1

src/ModelDescriber/ObjectModelDescriber.php

@@ -182,6 +182,8 @@ public function describe(Model $model, OA\Schema $schema)
 
             $this->describeProperty($types, $model, $property, $propertyName, $schema);
         }
+
+        $this->markRequiredProperties($schema);
     }
 
     /**
@@ -233,6 +235,36 @@ private function describeProperty(array $types, Model $model, OA\Schema $propert
         throw new \Exception(sprintf('Type "%s" is not supported in %s::$%s. You may use the `@OA\Property(type="")` annotation to specify it manually.', $types[0]->getBuiltinType(), $model->getType()->getClassName(), $propertyName));
     }
 
+    /**
+     * Mark properties as required while ordering them in the same order as the properties of the schema.
+     * Then append the original required properties.
+     */
+    private function markRequiredProperties(OA\Schema $schema): void
+    {
+        if (Generator::isDefault($properties = $schema->properties)) {
+            return;
+        }
+
+        $newRequired = [];
+        foreach ($properties as $property) {
+            if (is_array($schema->required) && \in_array($property->property, $schema->required, true)) {
+                $newRequired[] = $property->property;
+                continue;
+            }
+
+            if (true === $property->nullable || !Generator::isDefault($property->default)) {
+                continue;
+            }
+            $newRequired[] = $property->property;
+        }
+
+        if ([] !== $newRequired) {
+            $originalRequired = Generator::isDefault($schema->required) ? [] : $schema->required;
+
+            $schema->required = array_values(array_unique(array_merge($newRequired, $originalRequired)));
+        }
+    }
+
     public function supports(Model $model): bool
     {
         return Type::BUILTIN_TYPE_OBJECT === $model->getType()->getBuiltinType()

src/PropertyDescriber/PropertyDescriberInterface.php

@@ -19,12 +19,11 @@ interface PropertyDescriberInterface
     /**
      * @param Type[]               $types
      * @param string[]|null        $groups  Deprecated use $context['groups'] instead
-     * @param Schema               $schema  Allows to make changes inside of the schema (e.g. adding required fields)
      * @param array<string, mixed> $context Context options for describing the property
      *
      * @return void
      */
-    public function describe(array $types, Schema $property, ?array $groups = null /* , ?Schema $schema = null */ /* , array $context = [] */);
+    public function describe(array $types, Schema $property, ?array $groups = null /* , array $context = [] */);
 
     /**
      * @param Type[] $types

src/PropertyDescriber/RequiredPropertyDescriber.php

@@ -16,6 +16,8 @@
 
 /**
  * Mark a property as required if it is not nullable.
+ *
+ * @deprecated {@see ObjectModelDescriber::markRequiredProperties()}
  */
 final class RequiredPropertyDescriber implements PropertyDescriberInterface, PropertyDescriberAwareInterface
 {

tests/Functional/Fixtures/MapQueryStringCleanupComponents.json

@@ -148,7 +148,7 @@
                     {
                         "name": "propertyArray[]",
                         "in": "query",
-                        "required": false,
+                        "required": true,
                         "schema": {
                             "type": "array",
                             "items": {
@@ -304,7 +304,7 @@
                         "name": "productIds[]",
                         "in": "query",
                         "description": "List of product ids",
-                        "required": false,
+                        "required": true,
                         "schema": {
                             "description": "List of product ids",
                             "type": "array",

tests/Functional/Fixtures/MapQueryStringController.json

@@ -148,7 +148,7 @@
                     {
                         "name": "propertyArray[]",
                         "in": "query",
-                        "required": false,
+                        "required": true,
                         "schema": {
                             "type": "array",
                             "items": {
@@ -304,7 +304,7 @@
                         "name": "productIds[]",
                         "in": "query",
                         "description": "List of product ids",
-                        "required": false,
+                        "required": true,
                         "schema": {
                             "description": "List of product ids",
                             "type": "array",
@@ -1104,6 +1104,7 @@
                 "required": [
                     "property",
                     "propertyInDefaultGroup",
+                    "propertyArray",
                     "propertyNotNullOnSpecificGroup"
                 ],
                 "properties": {
@@ -1180,7 +1181,8 @@
             },
             "ArrayQueryModel": {
                 "required": [
-                    "ids"
+                    "ids",
+                    "productIds"
                 ],
                 "properties": {
                     "ids": {
@@ -1248,7 +1250,8 @@
             },
             "ArrayQueryModel2": {
                 "required": [
-                    "ids"
+                    "ids",
+                    "productIds"
                 ],
                 "properties": {
                     "ids": {

tests/Functional/Fixtures/MapRequestPayloadController.json

@@ -148,6 +148,7 @@
                 "required": [
                     "property",
                     "propertyInDefaultGroup",
+                    "propertyArray",
                     "propertyNotNullOnSpecificGroup"
                 ],
                 "properties": {

tests/Functional/FunctionalTest.php

@@ -252,6 +252,9 @@ public function testUserModel(): void
                 ],
                 'schema' => 'User',
                 'required' => [
+                    'email',
+                    'location',
+                    'friendsNumber',
                     'creationDate',
                     'users',
                     'status',

tests/Functional/ValidationGroupsFunctionalTest.php

@@ -89,6 +89,7 @@ public function testConstraintDefaultGroupsAreRespectedWhenReadingAnnotations():
             'required' => [
                 'property',
                 'propertyInDefaultGroup',
+                'propertyArray',
             ],
         ];