refactor: deprecate options possibly being null (#2387)

## Description Deprecates `options` nullable type for `Model` classes. This reduces complexity and streamlines it with the `$serializationContext`. ## What type of PR is this? (check all applicable) - [ ] Bug Fix - [ ] Feature - [x] Refactor - [x] Deprecation - [ ] Breaking Change - [ ] Documentation Update - [ ] CI ## Checklist - [ ] I have made corresponding changes to the documentation (`docs/`) - [x] I have made corresponding changes to the changelog (`CHANGELOG.md`)
nelmio · Nov 8, 2024 · c431718 · c431718
1 parent 0dcdc7d
commit c431718
Show file tree

Hide file tree

Showing 13 changed files with 176 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,24 @@
 # CHANGELOG
 
+## 4.33.4
+* Deprecated `null` type from `$options` in `Nelmio\ApiDocBundle\Attribute\Model::__construct()`. Pass an empty array (`[]`) instead.
+* Deprecated `null` type from `$options` in `NNelmio\ApiDocBundle\Attribute\Model::__construct()`. Pass an empty array (`[]`) instead.
+
+## 4.33.3
+* Bumped swagger-ui files from `5.18.1` to `5.18.2`
+* Bumped redoc files to `2.2.0`
+
+## 4.33.2
+* Fixed incorrect directory updated for swagger-ui files from version `4.33.2`
+
+## 4.33.1
+* Bumped swagger-ui files to `5.18.1`
+* Fixed explicitly set default values defined in `#[OA\Property]` being overwritten
+
+## 4.33.0
+* Fixed custom JMS enum type handling
+* Added support for name based serialisation of JMS enums
+
 ## 4.32.3
 
 * Deprecated `Nelmio\ApiDocBundle\Annotation` namespace in favor of `Nelmio\ApiDocBundle\Attribute` namespace in preparation for 5.x. Consider upgrading to the new attribute syntax.

diff --git a/phpunit-ignore.txt b/phpunit-ignore.txt
@@ -2,4 +2,8 @@
 /^Since nelmio\/api-doc-bundle 4\.17\.0: Using the \$groups parameter of "Nelmio\\ApiDocBundle\\PropertyDescriber\\(PropertyDescriber|IntegerPropertyDescriber|NullablePropertyDescriber|StringPropertyDescriber|ObjectPropertyDescriber)::describe\(\)" is deprecated/
 
 # Ignoring deprecations from Nelmio\ApiDocBundle 4.32.3
-/^Since nelmio\/api-doc-bundle 4\.32\.3: The "Nelmio\\ApiDocBundle\\Annotation\\(Operation|Security|Areas|Model)" class is deprecated and will be removed in 5\.0\. Use the "\\Nelmio\\ApiDocBundle\\Attribute\\(Operation|Security|Areas|Model)" attribute instead\./
+/^Since nelmio\/api-doc-bundle 4\.32\.3: The "Nelmio\\ApiDocBundle\\Annotation\\(Operation|Security|Areas|Model)" class is deprecated and will be removed in 5\.0\. Use the "\\Nelmio\\ApiDocBundle\\Attribute\\(Operation|Security|Areas|Model)" attribute instead\./
+
+# Ignoring deprecations from Nelmio\ApiDocBundle 4.33.4
+/^Since nelmio\/api-doc-bundle 4\.33\.4: Passing null to the "\$options" argument of "Nelmio\\ApiDocBundle\\Model\\Model\:\:\_\_construct\(\)" is deprecated\, pass an empty array instead\./
+/^Since nelmio\/api-doc-bundle 4\.33\.4: Passing null to the "\$options" argument of "Nelmio\\ApiDocBundle\\Attribute\\Model\:\:\_\_construct\(\)" is deprecated\, pass an empty array instead\./
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -9,6 +9,7 @@
          failOnRisky="true"
          stopOnFailure="false"
          bootstrap="vendor/autoload.php"
+         displayDetailsOnTestsThatTriggerWarnings="true"
 >
     <php>
         <env name="SHELL_VERBOSITY" value="-1" />

diff --git a/src/Attribute/Model.php b/src/Attribute/Model.php
@@ -41,9 +41,9 @@ class Model extends Attachable
     public ?array $groups;
 
     /**
-     * @var mixed[]|null
+     * @var mixed[]
      */
-    public ?array $options;
+    public array $options;
 
     /**
      * @var array<string, mixed>
@@ -60,9 +60,14 @@ public function __construct(
         array $properties = [],
         string $type = Generator::UNDEFINED,
         ?array $groups = null,
-        ?array $options = null,
+        ?array $options = [],
         array $serializationContext = []
     ) {
+        if (null === $options) {
+            trigger_deprecation('nelmio/api-doc-bundle', '4.33.4', 'Passing null to the "$options" argument of "%s()" is deprecated, pass an empty array instead.', __METHOD__);
+            $options = [];
+        }
+
         parent::__construct($properties + [
             'type' => $type,
             'groups' => $groups,

diff --git a/src/Model/Model.php b/src/Model/Model.php
@@ -18,9 +18,9 @@ final class Model
     private Type $type;
 
     /**
-     * @var mixed[]|null
+     * @var mixed[]
      */
-    private ?array $options;
+    private array $options;
 
     /**
      * @var mixed[]
@@ -32,8 +32,13 @@ final class Model
      * @param mixed[]|null  $options
      * @param mixed[]       $serializationContext
      */
-    public function __construct(Type $type, ?array $groups = null, ?array $options = null, array $serializationContext = [])
+    public function __construct(Type $type, ?array $groups = null, ?array $options = [], array $serializationContext = [])
     {
+        if (null === $options) {
+            trigger_deprecation('nelmio/api-doc-bundle', '4.33.4', 'Passing null to the "$options" argument of "%s()" is deprecated, pass an empty array instead.', __METHOD__);
+            $options = [];
+        }
+
         $this->type = $type;
         $this->options = $options;
         $this->serializationContext = $serializationContext;
@@ -69,9 +74,9 @@ public function getHash(): string
     }
 
     /**
-     * @return mixed[]|null
+     * @return mixed[]
      */
-    public function getOptions(): ?array
+    public function getOptions(): array
     {
         return $this->options;
     }

diff --git a/src/Model/ModelRegistry.php b/src/Model/ModelRegistry.php
@@ -70,7 +70,7 @@ public function __construct($modelDescribers, OA\OpenApi $api, array $alternativ
             $this->alternativeNames[] = $model = new Model(
                 new Type('object', false, $criteria['type']),
                 $criteria['groups'],
-                $criteria['options'] ?? null,
+                $criteria['options'] ?? [],
                 $criteria['serializationContext'] ?? [],
             );
             $this->names[$model->getHash()] = $alternativeName;

diff --git a/src/ModelDescriber/FormModelDescriber.php b/src/ModelDescriber/FormModelDescriber.php
@@ -85,7 +85,7 @@ public function describe(Model $model, OA\Schema $schema): void
 
         $this->setContextFromReflection($schema->_context, new \ReflectionClass($class));
 
-        $form = $this->formFactory->create($class, null, $model->getOptions() ?? []);
+        $form = $this->formFactory->create($class, null, $model->getOptions());
         $this->parseForm($schema, $form);
 
         $this->setContext(null);

diff --git a/src/ModelDescriber/JMSModelDescriber.php b/src/ModelDescriber/JMSModelDescriber.php
@@ -350,7 +350,7 @@ public function describeItem(array $type, OA\Schema $property, Context $context,
             $groups = $this->computeGroups($context, $type);
             unset($serializationContext['groups']);
 
-            $model = new Model(new Type(Type::BUILTIN_TYPE_OBJECT, false, $type['name']), $groups, null, $serializationContext);
+            $model = new Model(new Type(Type::BUILTIN_TYPE_OBJECT, false, $type['name']), $groups, [], $serializationContext);
             $modelRef = $this->modelRegistry->register($model);
 
             $customFields = (array) $property->jsonSerialize();

diff --git a/src/PropertyDescriber/ObjectPropertyDescriber.php b/src/PropertyDescriber/ObjectPropertyDescriber.php
@@ -56,13 +56,13 @@ public function describe(array $types, OA\Schema $property, ?array $groups = nul
 
         if ($types[0]->isNullable()) {
             $weakContext = Util::createWeakContext($property->_context);
-            $schemas = [new OA\Schema(['ref' => $this->modelRegistry->register(new Model($type, $groups, null, $context)), '_context' => $weakContext])];
+            $schemas = [new OA\Schema(['ref' => $this->modelRegistry->register(new Model($type, $groups, [], $context)), '_context' => $weakContext])];
             $property->oneOf = $schemas;
 
             return;
         }
 
-        $property->ref = $this->modelRegistry->register(new Model($type, $groups, null, $context));
+        $property->ref = $this->modelRegistry->register(new Model($type, $groups, [], $context));
     }
 
     public function supports(array $types): bool

diff --git a/tests/Functional/Controller/DeprecationController.php b/tests/Functional/Controller/DeprecationController.php
@@ -0,0 +1,31 @@
+<?php
+
+/*
+ * This file is part of the NelmioApiDocBundle package.
+ *
+ * (c) Nelmio
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Nelmio\ApiDocBundle\Tests\Functional\Controller;
+
+use Nelmio\ApiDocBundle\Attribute\Model;
+use Nelmio\ApiDocBundle\Tests\Functional\Entity\Article81;
+use OpenApi\Attributes as OA;
+use Symfony\Component\HttpFoundation\JsonResponse;
+use Symfony\Component\Routing\Annotation\Route;
+
+class DeprecationController
+{
+    #[Route(path: '/legacy/null_options', name: 'legacy_null_options', methods: 'POST')]
+    #[OA\Response(
+        response: 200,
+        description: 'Legacy null options',
+        content: new Model(type: Article81::class, options: null),
+    )]
+    public function __invoke(Article81 $requestDTO): JsonResponse
+    {
+    }
+}
diff --git a/tests/Functional/ControllerTest.php b/tests/Functional/ControllerTest.php
@@ -105,6 +105,13 @@ public static function provideAttributeTestCases(): \Generator
             [__DIR__.'/Configs/JMS.yaml'],
         ];
 
+        yield 'Deprecation null model options' => [
+            [
+                'name' => 'DeprecationController',
+                'type' => $type,
+            ],
+        ];
+
         if (version_compare(Kernel::VERSION, '6.3.0', '>=')) {
             yield 'https://github.com/nelmio/NelmioApiDocBundle/issues/2209' => [
                 [

diff --git a/tests/Functional/Fixtures/DeprecationController.json b/tests/Functional/Fixtures/DeprecationController.json
@@ -0,0 +1,86 @@
+{
+    "openapi": "3.0.0",
+    "info": {
+        "title": "",
+        "version": "0.0.0"
+    },
+    "paths": {
+        "/legacy/null_options": {
+            "post": {
+                "operationId": "post_legacy_null_options",
+                "responses": {
+                    "200": {
+                        "description": "Legacy null options",
+                        "content": {
+                            "application/json": {
+                                "schema": {
+                                    "$ref": "#/components/schemas/Article81"
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    },
+    "components": {
+        "schemas": {
+            "Article81": {
+                "required": [
+                    "id",
+                    "type",
+                    "intBackedType",
+                    "notBackedType"
+                ],
+                "properties": {
+                    "id": {
+                        "type": "integer"
+                    },
+                    "type": {
+                        "$ref": "#/components/schemas/ArticleType81"
+                    },
+                    "intBackedType": {
+                        "$ref": "#/components/schemas/ArticleType81IntBacked"
+                    },
+                    "notBackedType": {
+                        "$ref": "#/components/schemas/ArticleType81NotBacked"
+                    },
+                    "nullableType": {
+                        "nullable": true,
+                        "oneOf": [
+                            {
+                                "$ref": "#/components/schemas/ArticleType81"
+                            }
+                        ]
+                    }
+                },
+                "type": "object"
+            },
+            "ArticleType81": {
+                "type": "string",
+                "enum": [
+                    "draft",
+                    "final"
+                ]
+            },
+            "ArticleType81IntBacked": {
+                "type": "integer",
+                "enum": [
+                    0,
+                    1
+                ]
+            },
+            "ArticleType81NotBacked": {
+                "required": [
+                    "name"
+                ],
+                "properties": {
+                    "name": {
+                        "type": "string"
+                    }
+                },
+                "type": "object"
+            }
+        }
+    }
+}
diff --git a/tests/Model/ModelRegistryTest.php b/tests/Model/ModelRegistryTest.php
@@ -50,15 +50,15 @@ public function testNameCollisionsAreLogged(Type $type, array $arrayType): void
                 'Can not assign a name for the model, the name "ModelRegistryTest" has already been taken.', [
                     'model' => [
                         'type' => $arrayType,
-                        'options' => null,
+                        'options' => [],
                         'groups' => ['group2'],
                         'serialization_context' => [
                             'groups' => ['group2'],
                         ],
                     ],
                     'taken_by' => [
                         'type' => $arrayType,
-                        'options' => null,
+                        'options' => [],
                         'groups' => ['group1'],
                         'serialization_context' => [
                             'groups' => ['group1'],
@@ -134,7 +134,7 @@ public function testNameCollisionsAreLoggedWithAlternativeNames(): void
                             'collection_key_types' => null,
                             'collection_value_types' => null,
                         ],
-                        'options' => null,
+                        'options' => [],
                         'groups' => ['group2'],
                         'serialization_context' => ['groups' => ['group2']],
                     ],
@@ -147,7 +147,7 @@ public function testNameCollisionsAreLoggedWithAlternativeNames(): void
                             'collection_key_types' => null,
                             'collection_value_types' => null,
                         ],
-                        'options' => null,
+                        'options' => [],
                         'groups' => ['group1'],
                         'serialization_context' => ['groups' => ['group1']],
                     ],