From c685a613f628017888b83e9471de716687f858d1 Mon Sep 17 00:00:00 2001 From: Andreas Heigl Date: Wed, 6 Nov 2024 11:28:58 +0100 Subject: [PATCH 1/6] Override code based defaults with explicit ones This change allows that defaults that are set in attributes can override the ones that are read from the code. This fixes https://github.com/nelmio/NelmioApiDocBundle/issues/2330 --- src/ModelDescriber/ObjectModelDescriber.php | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/ModelDescriber/ObjectModelDescriber.php b/src/ModelDescriber/ObjectModelDescriber.php index f511629b7..7b767cbf1 100644 --- a/src/ModelDescriber/ObjectModelDescriber.php +++ b/src/ModelDescriber/ObjectModelDescriber.php @@ -154,13 +154,6 @@ public function describe(Model $model, OA\Schema $schema) $property = Util::getProperty($schema, $serializedName); - // Fix for https://github.com/nelmio/NelmioApiDocBundle/issues/2222 - // Property default value has to be set before SymfonyConstraintAnnotationReader::processPropertyAnnotations() - // is called to prevent wrongly detected required properties - if (Generator::UNDEFINED === $property->default && array_key_exists($propertyName, $defaultValues)) { - $property->default = $defaultValues[$propertyName]; - } - // Interpret additional options $groups = $model->getGroups(); if (isset($groups[$propertyName]) && is_array($groups[$propertyName])) { @@ -175,6 +168,10 @@ public function describe(Model $model, OA\Schema $schema) continue; } + if (Generator::UNDEFINED === $property->default && array_key_exists($propertyName, $defaultValues)) { + $property->default = $defaultValues[$propertyName]; + } + $types = $this->propertyInfo->getTypes($class, $propertyName); if (null === $types || 0 === count($types)) { throw new \LogicException(sprintf('The PropertyInfo component was not able to guess the type of %s::$%s. You may need to add a `@var` annotation or use `@OA\Property(type="")` to make its type explicit.', $class, $propertyName)); From 0a818505969bd638cdd2edf3477ab67197021af7 Mon Sep 17 00:00:00 2001 From: Andreas Heigl Date: Wed, 6 Nov 2024 11:56:49 +0100 Subject: [PATCH 2/6] Remove fields with default from required entry --- src/ModelDescriber/ObjectModelDescriber.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/ModelDescriber/ObjectModelDescriber.php b/src/ModelDescriber/ObjectModelDescriber.php index 7b767cbf1..4e08cf9de 100644 --- a/src/ModelDescriber/ObjectModelDescriber.php +++ b/src/ModelDescriber/ObjectModelDescriber.php @@ -172,6 +172,18 @@ public function describe(Model $model, OA\Schema $schema) $property->default = $defaultValues[$propertyName]; } + if (Generator::UNDEFINED !== $property->default) { + // Fix for https://github.com/nelmio/NelmioApiDocBundle/issues/2222 + // When a default value has been set for the property, we can + // remove the field from the required fields. + if (is_array($schema->required) && ($key = array_search($propertyName, $schema->required, true)) !== false) { + unset($schema->required[$key]); + } + } + if ([] === $schema->required) { + $schema->required = Generator::UNDEFINED; + } + $types = $this->propertyInfo->getTypes($class, $propertyName); if (null === $types || 0 === count($types)) { throw new \LogicException(sprintf('The PropertyInfo component was not able to guess the type of %s::$%s. You may need to add a `@var` annotation or use `@OA\Property(type="")` to make its type explicit.', $class, $propertyName)); From a4efff3dd55897e9f7064d71f86ddf7b8ec42a0e Mon Sep 17 00:00:00 2001 From: Andreas Heigl Date: Thu, 7 Nov 2024 09:01:32 +0100 Subject: [PATCH 3/6] Move Reflection handling into own Reader This allows to fetch default values via reflection should they so far not have been set via other means. As that happens before the SymfonyConstraint handling, the required fields should be set as expected but should also take into account the default values set via Attributes or Annotations. --- .../Annotations/AnnotationsReader.php | 4 + .../Annotations/ReflectionReader.php | 150 ++++++++++++++++++ src/ModelDescriber/ObjectModelDescriber.php | 33 ---- 3 files changed, 154 insertions(+), 33 deletions(-) create mode 100644 src/ModelDescriber/Annotations/ReflectionReader.php diff --git a/src/ModelDescriber/Annotations/AnnotationsReader.php b/src/ModelDescriber/Annotations/AnnotationsReader.php index 8eae411bf..8e578fe65 100644 --- a/src/ModelDescriber/Annotations/AnnotationsReader.php +++ b/src/ModelDescriber/Annotations/AnnotationsReader.php @@ -24,6 +24,7 @@ class AnnotationsReader private PropertyPhpDocReader $phpDocReader; private OpenApiAnnotationsReader $openApiAnnotationsReader; private SymfonyConstraintAnnotationReader $symfonyConstraintAnnotationReader; + private ReflectionReader $reflectionReader; /** * @param string[] $mediaTypes @@ -40,12 +41,14 @@ public function __construct( $annotationsReader, $useValidationGroups ); + $this->reflectionReader = new ReflectionReader(); } public function updateDefinition(\ReflectionClass $reflectionClass, OA\Schema $schema): bool { $this->openApiAnnotationsReader->updateSchema($reflectionClass, $schema); $this->symfonyConstraintAnnotationReader->setSchema($schema); + $this->reflectionReader->setSchema($schema); return $this->shouldDescribeModelProperties($schema); } @@ -66,6 +69,7 @@ public function updateProperty($reflection, OA\Property $property, ?array $seria { $this->openApiAnnotationsReader->updateProperty($reflection, $property, $serializationGroups); $this->phpDocReader->updateProperty($reflection, $property); + $this->reflectionReader->updateProperty($reflection, $property); $this->symfonyConstraintAnnotationReader->updateProperty($reflection, $property, $serializationGroups); } diff --git a/src/ModelDescriber/Annotations/ReflectionReader.php b/src/ModelDescriber/Annotations/ReflectionReader.php new file mode 100644 index 000000000..33ff2747e --- /dev/null +++ b/src/ModelDescriber/Annotations/ReflectionReader.php @@ -0,0 +1,150 @@ +default) { + return; + } + + $serializedName = $reflection->getName(); + foreach (['', 'get', 'is', 'has', 'can', 'add', 'remove', 'set'] as $prefix) { + if (0 === strpos($serializedName, $prefix)) { + $serializedName = substr($serializedName, strlen($prefix)); + } + } + + if ($reflection instanceof \ReflectionMethod) { + $methodDefault = $this->getDefaultFromMethodReflection($reflection); + if (Generator::UNDEFINED !== $methodDefault) { + $property->default = $methodDefault; + + return; + } + } + + if ($reflection instanceof \ReflectionProperty) { + $methodDefault = $this->getDefaultFromPropertyReflection($reflection); + if (Generator::UNDEFINED !== $methodDefault) { + $property->default = $methodDefault; + + return; + } + } + // Fix for https://github.com/nelmio/NelmioApiDocBundle/issues/2222 + // Promoted properties with a value initialized by the constructor are not considered to have a default value + // and are therefore not returned by ReflectionClass::getDefaultProperties(); see https://bugs.php.net/bug.php?id=81386 + $reflClassConstructor = $reflection->getDeclaringClass()->getConstructor(); + $reflClassConstructorParameters = null !== $reflClassConstructor ? $reflClassConstructor->getParameters() : []; + foreach ($reflClassConstructorParameters as $parameter) { + if ($parameter->name !== $serializedName) { + continue; + } + if (!$parameter->isDefaultValueAvailable()) { + continue; + } + + if (null === $this->schema) { + continue; + } + + if (!Generator::isDefault($property->default)) { + continue; + } + + $property->default = $parameter->getDefaultValue(); + } + } + + public function setSchema(OA\Schema $schema): void + { + $this->schema = $schema; + } + + private function getDefaultFromMethodReflection(\ReflectionMethod $reflection): mixed + { + if (0 !== strpos($reflection->name, 'set')) { + return Generator::UNDEFINED; + } + + if (1 !== $reflection->getNumberOfParameters()) { + return Generator::UNDEFINED; + } + + $param = $reflection->getParameters()[0]; + + if (!$param->isDefaultValueAvailable()) { + return Generator::UNDEFINED; + } + + if (null === $param->getDefaultValue()) { + return Generator::UNDEFINED; + } + + return $param->getDefaultValue(); + } + + public function getDefaultFromPropertyReflection(\ReflectionProperty $reflection): mixed + { + $propertyName = $reflection->name; + if (!$reflection->getDeclaringClass()->hasProperty($propertyName)) { + return Generator::UNDEFINED; + } + + $reflectionProp = $reflection->getDeclaringClass()->getProperty($propertyName); + if (!$reflectionProp->hasDefaultValue()) { + return Generator::UNDEFINED; + } + + if (null === $reflectionProp->getDefaultValue()) { + return Generator::UNDEFINED; + } + + return $reflectionProp->getDefaultValue(); + } +} diff --git a/src/ModelDescriber/ObjectModelDescriber.php b/src/ModelDescriber/ObjectModelDescriber.php index 4e08cf9de..309def099 100644 --- a/src/ModelDescriber/ObjectModelDescriber.php +++ b/src/ModelDescriber/ObjectModelDescriber.php @@ -125,23 +125,6 @@ public function describe(Model $model, OA\Schema $schema) // The SerializerExtractor does expose private/protected properties for some reason, so we eliminate them here $propertyInfoProperties = array_intersect($propertyInfoProperties, $this->propertyInfo->getProperties($class, []) ?? []); - $defaultValues = array_filter($reflClass->getDefaultProperties(), static function ($value) { - return null !== $value; - }); - - // Fix for https://github.com/nelmio/NelmioApiDocBundle/issues/2222 - // Promoted properties with a value initialized by the constructor are not considered to have a default value - // and are therefore not returned by ReflectionClass::getDefaultProperties(); see https://bugs.php.net/bug.php?id=81386 - $reflClassConstructor = $reflClass->getConstructor(); - $reflClassConstructorParameters = null !== $reflClassConstructor ? $reflClassConstructor->getParameters() : []; - foreach ($reflClassConstructorParameters as $parameter) { - if (!$parameter->isDefaultValueAvailable()) { - continue; - } - - $defaultValues[$parameter->name] = $parameter->getDefaultValue(); - } - foreach ($propertyInfoProperties as $propertyName) { $serializedName = null !== $this->nameConverter ? $this->nameConverter->normalize($propertyName, $class, null, $model->getSerializationContext()) : $propertyName; @@ -168,22 +151,6 @@ public function describe(Model $model, OA\Schema $schema) continue; } - if (Generator::UNDEFINED === $property->default && array_key_exists($propertyName, $defaultValues)) { - $property->default = $defaultValues[$propertyName]; - } - - if (Generator::UNDEFINED !== $property->default) { - // Fix for https://github.com/nelmio/NelmioApiDocBundle/issues/2222 - // When a default value has been set for the property, we can - // remove the field from the required fields. - if (is_array($schema->required) && ($key = array_search($propertyName, $schema->required, true)) !== false) { - unset($schema->required[$key]); - } - } - if ([] === $schema->required) { - $schema->required = Generator::UNDEFINED; - } - $types = $this->propertyInfo->getTypes($class, $propertyName); if (null === $types || 0 === count($types)) { throw new \LogicException(sprintf('The PropertyInfo component was not able to guess the type of %s::$%s. You may need to add a `@var` annotation or use `@OA\Property(type="")` to make its type explicit.', $class, $propertyName)); From 43eca976bb7649db0d06565573ae661741cc93fe Mon Sep 17 00:00:00 2001 From: Andreas Heigl Date: Thu, 7 Nov 2024 09:27:31 +0100 Subject: [PATCH 4/6] Adapt tests to reflect default values Two tests didn't handle the provided defaults properly, so I had to fix them to now also validate that the defaults are set appropriately --- tests/Functional/FunctionalTest.php | 1 + tests/Functional/JMSFunctionalTest.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Functional/FunctionalTest.php b/tests/Functional/FunctionalTest.php index 0d4e89972..848dd25af 100644 --- a/tests/Functional/FunctionalTest.php +++ b/tests/Functional/FunctionalTest.php @@ -237,6 +237,7 @@ public function testUserModel(): void '$ref' => '#/components/schemas/User', ], 'type' => 'array', + 'default' => [], ], 'dummy' => [ '$ref' => '#/components/schemas/Dummy2', diff --git a/tests/Functional/JMSFunctionalTest.php b/tests/Functional/JMSFunctionalTest.php index 68a161d28..c0dc3a774 100644 --- a/tests/Functional/JMSFunctionalTest.php +++ b/tests/Functional/JMSFunctionalTest.php @@ -328,9 +328,9 @@ public function testNamingStrategyWithConstraints(): void 'type' => 'string', 'maxLength' => 10, 'minLength' => 3, + 'default' => 'default' ], ], - 'required' => ['beautifulName'], 'schema' => 'JMSNamingStrategyConstraints', ], json_decode($this->getModel('JMSNamingStrategyConstraints')->toJson(), true)); } From bc59009f609647878e0fbd0ccd4c2d81b16bd6d8 Mon Sep 17 00:00:00 2001 From: Andreas Heigl Date: Thu, 7 Nov 2024 09:41:33 +0100 Subject: [PATCH 5/6] Make usable with PHP<8.0 As the library is set to be usable with PHP7.4 and the ReflectionProperty::has/getDefaultValue is only available for PHP8+ I needed to slightly refactor the class. --- .../Annotations/ReflectionReader.php | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/ModelDescriber/Annotations/ReflectionReader.php b/src/ModelDescriber/Annotations/ReflectionReader.php index 33ff2747e..c6df8fa94 100644 --- a/src/ModelDescriber/Annotations/ReflectionReader.php +++ b/src/ModelDescriber/Annotations/ReflectionReader.php @@ -43,9 +43,6 @@ public function updateProperty( OA\Property $property, ?array $validationGroups = null ): void { - if (PHP_VERSION_ID < 80000) { - return; - } // The default has been set by an Annotation or Attribute // We leave that as it is! if (Generator::UNDEFINED !== $property->default) { @@ -53,7 +50,7 @@ public function updateProperty( } $serializedName = $reflection->getName(); - foreach (['', 'get', 'is', 'has', 'can', 'add', 'remove', 'set'] as $prefix) { + foreach (['get', 'is', 'has', 'can', 'add', 'remove', 'set'] as $prefix) { if (0 === strpos($serializedName, $prefix)) { $serializedName = substr($serializedName, strlen($prefix)); } @@ -106,7 +103,10 @@ public function setSchema(OA\Schema $schema): void $this->schema = $schema; } - private function getDefaultFromMethodReflection(\ReflectionMethod $reflection): mixed + /** + * @return mixed|string + */ + private function getDefaultFromMethodReflection(\ReflectionMethod $reflection) { if (0 !== strpos($reflection->name, 'set')) { return Generator::UNDEFINED; @@ -129,22 +129,22 @@ private function getDefaultFromMethodReflection(\ReflectionMethod $reflection): return $param->getDefaultValue(); } - public function getDefaultFromPropertyReflection(\ReflectionProperty $reflection): mixed + /** + * @return mixed|string + */ + public function getDefaultFromPropertyReflection(\ReflectionProperty $reflection) { $propertyName = $reflection->name; if (!$reflection->getDeclaringClass()->hasProperty($propertyName)) { return Generator::UNDEFINED; } - $reflectionProp = $reflection->getDeclaringClass()->getProperty($propertyName); - if (!$reflectionProp->hasDefaultValue()) { - return Generator::UNDEFINED; - } + $defaultValue = $reflection->getDeclaringClass()->getDefaultProperties()[$propertyName] ?? null; - if (null === $reflectionProp->getDefaultValue()) { + if (null === $defaultValue) { return Generator::UNDEFINED; } - return $reflectionProp->getDefaultValue(); + return $defaultValue; } } From 6a2bffd3b1c8c5aeb40e68a41bf06096e88f55fc Mon Sep 17 00:00:00 2001 From: Andreas Heigl Date: Thu, 7 Nov 2024 09:50:22 +0100 Subject: [PATCH 6/6] Implement recommendations from the PR-review --- src/ModelDescriber/Annotations/ReflectionReader.php | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/ModelDescriber/Annotations/ReflectionReader.php b/src/ModelDescriber/Annotations/ReflectionReader.php index c6df8fa94..4ea3f26f9 100644 --- a/src/ModelDescriber/Annotations/ReflectionReader.php +++ b/src/ModelDescriber/Annotations/ReflectionReader.php @@ -18,30 +18,25 @@ /** * Read default values of a property from the function or property signature. * - * This needs to be called before the SymfonyConstraintAnnotationReader, + * This needs to be called before the {@see SymfonyConstraintAnnotationReader}, * otherwise required properties might be considered wrongly. * * @internal */ -class ReflectionReader +final class ReflectionReader { use SetsContextTrait; - /** - * @var OA\Schema - */ - private $schema; + private ?OA\Schema $schema; /** * Update the given property and schema with defined Symfony constraints. * * @param \ReflectionProperty|\ReflectionMethod $reflection - * @param string[]|null $validationGroups */ public function updateProperty( $reflection, - OA\Property $property, - ?array $validationGroups = null + OA\Property $property ): void { // The default has been set by an Annotation or Attribute // We leave that as it is!