From c43171895161c8eb342bc5fc5eb21760dd91b646 Mon Sep 17 00:00:00 2001 From: Djordy Koert Date: Fri, 8 Nov 2024 16:00:51 +0100 Subject: [PATCH] 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`) --- CHANGELOG.md | 19 ++++ phpunit-ignore.txt | 6 +- phpunit.xml.dist | 1 + src/Attribute/Model.php | 11 ++- src/Model/Model.php | 15 ++-- src/Model/ModelRegistry.php | 2 +- src/ModelDescriber/FormModelDescriber.php | 2 +- src/ModelDescriber/JMSModelDescriber.php | 2 +- .../ObjectPropertyDescriber.php | 4 +- .../Controller/DeprecationController.php | 31 +++++++ tests/Functional/ControllerTest.php | 7 ++ .../Fixtures/DeprecationController.json | 86 +++++++++++++++++++ tests/Model/ModelRegistryTest.php | 8 +- 13 files changed, 176 insertions(+), 18 deletions(-) create mode 100644 tests/Functional/Controller/DeprecationController.php create mode 100644 tests/Functional/Fixtures/DeprecationController.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 70dc95021..f23be4a92 100644 --- 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 index c62fbbb5f..0f455835a 100644 --- 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\./ \ No newline at end of file +/^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\./ \ No newline at end of file diff --git a/phpunit.xml.dist b/phpunit.xml.dist index bd5c8def7..910850797 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -9,6 +9,7 @@ failOnRisky="true" stopOnFailure="false" bootstrap="vendor/autoload.php" + displayDetailsOnTestsThatTriggerWarnings="true" > diff --git a/src/Attribute/Model.php b/src/Attribute/Model.php index 67be0c493..143834d47 100644 --- 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 @@ -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 index 59504e5d5..1a911d156 100644 --- 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 index d258d3f14..544b3037e 100644 --- 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 index 9c19611c4..ed9f06f9d 100644 --- 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 index 91006dd16..9520bc3a7 100644 --- 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 index fddd38970..09385fff9 100644 --- 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 new file mode 100644 index 000000000..eec971a02 --- /dev/null +++ b/tests/Functional/Controller/DeprecationController.php @@ -0,0 +1,31 @@ + [ + [ + '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 new file mode 100644 index 000000000..03c2b3fbb --- /dev/null +++ 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" + } + } + } +} \ No newline at end of file diff --git a/tests/Model/ModelRegistryTest.php b/tests/Model/ModelRegistryTest.php index c6bb4fba5..304053091 100644 --- a/tests/Model/ModelRegistryTest.php +++ b/tests/Model/ModelRegistryTest.php @@ -50,7 +50,7 @@ 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'], @@ -58,7 +58,7 @@ public function testNameCollisionsAreLogged(Type $type, array $arrayType): void ], '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']], ],