From d932b06bbbfa6b97ed2030ace6154e58b75b8de5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Ben=C4=8Do?= Date: Tue, 16 Jun 2020 13:11:53 +0200 Subject: [PATCH] Add support for compound properties (#1651) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add support for compound properties * Fix CS & Tests * Another fixing :D * Final CS fix * Allow complex compound properties * cs * Update the Upgrading guide * Update php doc * Add Support for Nullable properties * Fix CS * Fix CS * Add Support for Nullable Types & Schemas as in OA3 * Update Nullable Property handling * CS * Fix tests * Accept also nullable config for Alternative model names * Refactor nullable refs * Fix CS & Tests * Another CS * Revert "Another CS" This reverts commit 03ada32b3263f3537d2af63f0abe79bd4a9ac0b5. * Revert "Fix CS & Tests" This reverts commit 369f2ccd170aebeeb9d87e9e00cba5cea62d5529. * Revert "Refactor nullable refs" This reverts commit 91cdf6fd0130f3ebf415de99f8a91edbc764255e. * Revert "Revert "Refactor nullable refs"" This reverts commit 0e50fc1938ce3e620fc655a7d1e9284a9f8c24f0. * Revert "Revert "Fix CS & Tests"" This reverts commit 228d3ca994eb4622c4db81aaa5f32845862e5616. * Revert "Revert "Another CS"" This reverts commit a5b08dedf5bca8fb711b816c62bed2de9f1c9521. * Improve nullable refs description Co-authored-by: Filip Benčo Co-authored-by: Guilhem Niot --- Model/ModelRegistry.php | 1 - ModelDescriber/ObjectModelDescriber.php | 21 ++++---- PropertyDescriber/ArrayPropertyDescriber.php | 15 +++--- .../BooleanPropertyDescriber.php | 9 ++-- .../CompoundPropertyDescriber.php | 54 +++++++++++++++++++ .../DateTimePropertyDescriber.php | 12 +++-- PropertyDescriber/FloatPropertyDescriber.php | 9 ++-- .../IntegerPropertyDescriber.php | 9 ++-- PropertyDescriber/NullablePropertyTrait.php | 25 +++++++++ PropertyDescriber/ObjectPropertyDescriber.php | 15 ++++-- .../PropertyDescriberInterface.php | 10 +++- PropertyDescriber/StringPropertyDescriber.php | 9 ++-- Resources/config/services.xml | 6 +++ Tests/Functional/Controller/ApiController.php | 10 ++++ Tests/Functional/Entity/CompoundEntity.php | 20 +++++++ Tests/Functional/FunctionalTest.php | 20 ++++++- UPGRADE-4.0.md | 9 ++++ 17 files changed, 211 insertions(+), 43 deletions(-) create mode 100644 PropertyDescriber/CompoundPropertyDescriber.php create mode 100644 PropertyDescriber/NullablePropertyTrait.php create mode 100644 Tests/Functional/Entity/CompoundEntity.php diff --git a/Model/ModelRegistry.php b/Model/ModelRegistry.php index 2a5bbe8..48bc4ba 100644 --- a/Model/ModelRegistry.php +++ b/Model/ModelRegistry.php @@ -40,7 +40,6 @@ final class ModelRegistry { $this->modelDescribers = $modelDescribers; $this->api = $api; - $this->alternativeNames = []; // last rule wins foreach (array_reverse($alternativeNames) as $alternativeName => $criteria) { $this->alternativeNames[] = $model = new Model(new Type('object', false, $criteria['type']), $criteria['groups']); diff --git a/ModelDescriber/ObjectModelDescriber.php b/ModelDescriber/ObjectModelDescriber.php index 4a26100..94a37f6 100644 --- a/ModelDescriber/ObjectModelDescriber.php +++ b/ModelDescriber/ObjectModelDescriber.php @@ -43,7 +43,7 @@ class ObjectModelDescriber implements ModelDescriberInterface, ModelRegistryAwar public function __construct( PropertyInfoExtractorInterface $propertyInfo, Reader $reader, - $propertyDescribers, + iterable $propertyDescribers, array $mediaTypes, NameConverterInterface $nameConverter = null ) { @@ -99,31 +99,30 @@ class ObjectModelDescriber implements ModelDescriberInterface, ModelRegistryAwar $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 `@SWG\Property(type="")` to make its type explicit.', $class, $propertyName)); - } - if (count($types) > 1) { - throw new \LogicException(sprintf('Property %s::$%s defines more than one type. You can specify the one that should be documented using `@SWG\Property(type="")`.', $class, $propertyName)); + 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)); } - $type = $types[0]; - $this->describeProperty($type, $model, $property, $propertyName); + $this->describeProperty($types, $model, $property, $propertyName); } } - private function describeProperty(Type $type, Model $model, OA\Schema $property, string $propertyName) + /** + * @param Type[] $types + */ + private function describeProperty(array $types, Model $model, OA\Schema $property, string $propertyName) { foreach ($this->propertyDescribers as $propertyDescriber) { if ($propertyDescriber instanceof ModelRegistryAwareInterface) { $propertyDescriber->setModelRegistry($this->modelRegistry); } - if ($propertyDescriber->supports($type)) { - $propertyDescriber->describe($type, $property, $model->getGroups()); + if ($propertyDescriber->supports($types)) { + $propertyDescriber->describe($types, $property, $model->getGroups()); return; } } - throw new \Exception(sprintf('Type "%s" is not supported in %s::$%s. You may use the `@OA\Property(type="")` annotation to specify it manually.', $type->getBuiltinType(), $model->getType()->getClassName(), $propertyName)); + 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)); } public function supports(Model $model): bool diff --git a/PropertyDescriber/ArrayPropertyDescriber.php b/PropertyDescriber/ArrayPropertyDescriber.php index be54702..f437ccb 100644 --- a/PropertyDescriber/ArrayPropertyDescriber.php +++ b/PropertyDescriber/ArrayPropertyDescriber.php @@ -15,7 +15,6 @@ use Nelmio\ApiDocBundle\Describer\ModelRegistryAwareInterface; use Nelmio\ApiDocBundle\Describer\ModelRegistryAwareTrait; use Nelmio\ApiDocBundle\OpenApiPhp\Util; use OpenApi\Annotations as OA; -use Symfony\Component\PropertyInfo\Type; class ArrayPropertyDescriber implements PropertyDescriberInterface, ModelRegistryAwareInterface { @@ -29,11 +28,11 @@ class ArrayPropertyDescriber implements PropertyDescriberInterface, ModelRegistr $this->propertyDescribers = $propertyDescribers; } - public function describe(Type $type, OA\Schema $property, array $groups = null) + public function describe(array $types, OA\Schema $property, array $groups = null) { - $type = $type->getCollectionValueType(); + $type = $types[0]->getCollectionValueType(); if (null === $type) { - throw new \LogicException(sprintf('Property "%s" is an array, but its items type isn\'t specified. You can specify that by using the type `string[]` for instance or `@SWG\Property(type="array", @SWG\Items(type="string"))`.', $property->title)); + throw new \LogicException(sprintf('Property "%s" is an array, but its items type isn\'t specified. You can specify that by using the type `string[]` for instance or `@OA\Property(type="array", @OA\Items(type="string"))`.', $property->title)); } $property->type = 'array'; @@ -43,16 +42,16 @@ class ArrayPropertyDescriber implements PropertyDescriberInterface, ModelRegistr if ($propertyDescriber instanceof ModelRegistryAwareInterface) { $propertyDescriber->setModelRegistry($this->modelRegistry); } - if ($propertyDescriber->supports($type)) { - $propertyDescriber->describe($type, $property, $groups); + if ($propertyDescriber->supports([$type])) { + $propertyDescriber->describe([$type], $property, $groups); break; } } } - public function supports(Type $type): bool + public function supports(array $types): bool { - return $type->isCollection(); + return 1 === count($types) && $types[0]->isCollection(); } } diff --git a/PropertyDescriber/BooleanPropertyDescriber.php b/PropertyDescriber/BooleanPropertyDescriber.php index 4785331..5406f45 100644 --- a/PropertyDescriber/BooleanPropertyDescriber.php +++ b/PropertyDescriber/BooleanPropertyDescriber.php @@ -16,13 +16,16 @@ use Symfony\Component\PropertyInfo\Type; class BooleanPropertyDescriber implements PropertyDescriberInterface { - public function describe(Type $type, OA\Schema $property, array $groups = null) + use NullablePropertyTrait; + + public function describe(array $types, OA\Schema $property, array $groups = null) { $property->type = 'boolean'; + $this->setNullableProperty($types[0], $property); } - public function supports(Type $type): bool + public function supports(array $types): bool { - return Type::BUILTIN_TYPE_BOOL === $type->getBuiltinType(); + return 1 === count($types) && Type::BUILTIN_TYPE_BOOL === $types[0]->getBuiltinType(); } } diff --git a/PropertyDescriber/CompoundPropertyDescriber.php b/PropertyDescriber/CompoundPropertyDescriber.php new file mode 100644 index 0000000..ba6cf6c --- /dev/null +++ b/PropertyDescriber/CompoundPropertyDescriber.php @@ -0,0 +1,54 @@ +propertyDescribers = $propertyDescribers; + } + + public function describe(array $types, OA\Schema $property, array $groups = null) + { + $property->oneOf = OA\UNDEFINED !== $property->oneOf ? $property->oneOf : []; + + foreach ($types as $type) { + $property->oneOf[] = $schema = Util::createChild($property, OA\Schema::class, []); + foreach ($this->propertyDescribers as $propertyDescriber) { + if ($propertyDescriber instanceof ModelRegistryAwareInterface) { + $propertyDescriber->setModelRegistry($this->modelRegistry); + } + if ($propertyDescriber->supports([$type])) { + $propertyDescriber->describe([$type], $schema, $groups); + + break; + } + } + } + } + + public function supports(array $types): bool + { + return count($types) >= 2; + } +} diff --git a/PropertyDescriber/DateTimePropertyDescriber.php b/PropertyDescriber/DateTimePropertyDescriber.php index e9a3cfb..fa351a0 100644 --- a/PropertyDescriber/DateTimePropertyDescriber.php +++ b/PropertyDescriber/DateTimePropertyDescriber.php @@ -16,15 +16,19 @@ use Symfony\Component\PropertyInfo\Type; class DateTimePropertyDescriber implements PropertyDescriberInterface { - public function describe(Type $type, OA\Schema $property, array $groups = null) + use NullablePropertyTrait; + + public function describe(array $types, OA\Schema $property, array $groups = null) { $property->type = 'string'; $property->format = 'date-time'; + $this->setNullableProperty($types[0], $property); } - public function supports(Type $type): bool + public function supports(array $types): bool { - return Type::BUILTIN_TYPE_OBJECT === $type->getBuiltinType() - && is_a($type->getClassName(), \DateTimeInterface::class, true); + return 1 === count($types) + && Type::BUILTIN_TYPE_OBJECT === $types[0]->getBuiltinType() + && is_a($types[0]->getClassName(), \DateTimeInterface::class, true); } } diff --git a/PropertyDescriber/FloatPropertyDescriber.php b/PropertyDescriber/FloatPropertyDescriber.php index baad8c9..0401cd7 100644 --- a/PropertyDescriber/FloatPropertyDescriber.php +++ b/PropertyDescriber/FloatPropertyDescriber.php @@ -16,14 +16,17 @@ use Symfony\Component\PropertyInfo\Type; class FloatPropertyDescriber implements PropertyDescriberInterface { - public function describe(Type $type, OA\Schema $property, array $groups = null) + use NullablePropertyTrait; + + public function describe(array $types, OA\Schema $property, array $groups = null) { $property->type = 'number'; $property->format = 'float'; + $this->setNullableProperty($types[0], $property); } - public function supports(Type $type): bool + public function supports(array $types): bool { - return Type::BUILTIN_TYPE_FLOAT === $type->getBuiltinType(); + return 1 === count($types) && Type::BUILTIN_TYPE_FLOAT === $types[0]->getBuiltinType(); } } diff --git a/PropertyDescriber/IntegerPropertyDescriber.php b/PropertyDescriber/IntegerPropertyDescriber.php index 91214ec..1a2ddcb 100644 --- a/PropertyDescriber/IntegerPropertyDescriber.php +++ b/PropertyDescriber/IntegerPropertyDescriber.php @@ -16,13 +16,16 @@ use Symfony\Component\PropertyInfo\Type; class IntegerPropertyDescriber implements PropertyDescriberInterface { - public function describe(Type $type, OA\Schema $property, array $groups = null) + use NullablePropertyTrait; + + public function describe(array $types, OA\Schema $property, array $groups = null) { $property->type = 'integer'; + $this->setNullableProperty($types[0], $property); } - public function supports(Type $type): bool + public function supports(array $types): bool { - return Type::BUILTIN_TYPE_INT === $type->getBuiltinType(); + return 1 === count($types) && Type::BUILTIN_TYPE_INT === $types[0]->getBuiltinType(); } } diff --git a/PropertyDescriber/NullablePropertyTrait.php b/PropertyDescriber/NullablePropertyTrait.php new file mode 100644 index 0000000..9b798e9 --- /dev/null +++ b/PropertyDescriber/NullablePropertyTrait.php @@ -0,0 +1,25 @@ +isNullable()) { + $property->nullable = true; + } + } +} diff --git a/PropertyDescriber/ObjectPropertyDescriber.php b/PropertyDescriber/ObjectPropertyDescriber.php index 9c6cc22..45c1da3 100644 --- a/PropertyDescriber/ObjectPropertyDescriber.php +++ b/PropertyDescriber/ObjectPropertyDescriber.php @@ -21,15 +21,22 @@ class ObjectPropertyDescriber implements PropertyDescriberInterface, ModelRegist { use ModelRegistryAwareTrait; - public function describe(Type $type, OA\Schema $property, array $groups = null) + public function describe(array $types, OA\Schema $property, array $groups = null) { - $type = new Type($type->getBuiltinType(), false, $type->getClassName(), $type->isCollection(), $type->getCollectionKeyType(), $type->getCollectionValueType()); // ignore nullable field + $type = new Type($types[0]->getBuiltinType(), false, $types[0]->getClassName(), $types[0]->isCollection(), $types[0]->getCollectionKeyType(), $types[0]->getCollectionValueType()); // ignore nullable field + + if ($types[0]->isNullable()) { + $property->nullable = true; + $property->allOf = [['$ref' => $this->modelRegistry->register(new Model($type, $groups))]]; + + return; + } $property->ref = $this->modelRegistry->register(new Model($type, $groups)); } - public function supports(Type $type): bool + public function supports(array $types): bool { - return Type::BUILTIN_TYPE_OBJECT === $type->getBuiltinType(); + return 1 === count($types) && Type::BUILTIN_TYPE_OBJECT === $types[0]->getBuiltinType(); } } diff --git a/PropertyDescriber/PropertyDescriberInterface.php b/PropertyDescriber/PropertyDescriberInterface.php index 428634e..9cba944 100644 --- a/PropertyDescriber/PropertyDescriberInterface.php +++ b/PropertyDescriber/PropertyDescriberInterface.php @@ -16,7 +16,13 @@ use Symfony\Component\PropertyInfo\Type; interface PropertyDescriberInterface { - public function describe(Type $type, Schema $property, array $groups = null); + /** + * @param Type[] $types + */ + public function describe(array $types, Schema $property, array $groups = null); - public function supports(Type $type): bool; + /** + * @param Type[] $types + */ + public function supports(array $types): bool; } diff --git a/PropertyDescriber/StringPropertyDescriber.php b/PropertyDescriber/StringPropertyDescriber.php index b88d8cd..6dda1bc 100644 --- a/PropertyDescriber/StringPropertyDescriber.php +++ b/PropertyDescriber/StringPropertyDescriber.php @@ -16,13 +16,16 @@ use Symfony\Component\PropertyInfo\Type; class StringPropertyDescriber implements PropertyDescriberInterface { - public function describe(Type $type, OA\Schema $property, array $groups = null) + use NullablePropertyTrait; + + public function describe(array $types, OA\Schema $property, array $groups = null) { $property->type = 'string'; + $this->setNullableProperty($types[0], $property); } - public function supports(Type $type): bool + public function supports(array $types): bool { - return Type::BUILTIN_TYPE_STRING === $type->getBuiltinType(); + return 1 === count($types) && Type::BUILTIN_TYPE_STRING === $types[0]->getBuiltinType(); } } diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 67a499d..889199f 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -89,6 +89,12 @@ + + + + + + diff --git a/Tests/Functional/Controller/ApiController.php b/Tests/Functional/Controller/ApiController.php index 289ca09..f916455 100644 --- a/Tests/Functional/Controller/ApiController.php +++ b/Tests/Functional/Controller/ApiController.php @@ -16,6 +16,7 @@ use Nelmio\ApiDocBundle\Annotation\Model; use Nelmio\ApiDocBundle\Annotation\Operation; use Nelmio\ApiDocBundle\Annotation\Security; use Nelmio\ApiDocBundle\Tests\Functional\Entity\Article; +use Nelmio\ApiDocBundle\Tests\Functional\Entity\CompoundEntity; use Nelmio\ApiDocBundle\Tests\Functional\Entity\SymfonyConstraints; use Nelmio\ApiDocBundle\Tests\Functional\Entity\User; use Nelmio\ApiDocBundle\Tests\Functional\Form\DummyType; @@ -201,4 +202,13 @@ class ApiController public function newAreaAction() { } + + /** + * @Route("/compound", methods={"GET", "POST"}) + * + * @OA\Response(response=200, description="Worked well!", @Model(type=CompoundEntity::class)) + */ + public function compoundEntityAction() + { + } } diff --git a/Tests/Functional/Entity/CompoundEntity.php b/Tests/Functional/Entity/CompoundEntity.php new file mode 100644 index 0000000..ba0a6cf --- /dev/null +++ b/Tests/Functional/Entity/CompoundEntity.php @@ -0,0 +1,20 @@ + 'array', ], 'friend' => [ - '$ref' => '#/components/schemas/User', + 'nullable' => true, + 'allOf' => [ + ['$ref' => '#/components/schemas/User'], + ], ], 'dummy' => [ '$ref' => '#/components/schemas/Dummy2', @@ -430,4 +433,19 @@ class FunctionalTest extends WebTestCase $this->assertNotHasProperty('bar', $model); $this->assertHasProperty('notwhatyouthink', $model); } + + public function testCompoundEntityAction() + { + $model = $this->getModel('CompoundEntity'); + $this->assertCount(1, $model->properties); + + $this->assertHasProperty('complex', $model); + + $property = $model->properties[0]; + $this->assertCount(2, $property->oneOf); + + $this->assertSame('integer', $property->oneOf[0]->type); + $this->assertSame('array', $property->oneOf[1]->type); + $this->assertSame('#/components/schemas/CompoundEntity', $property->oneOf[1]->items->ref); + } } diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index d8dc38a..7435f85 100644 --- a/UPGRADE-4.0.md +++ b/UPGRADE-4.0.md @@ -26,3 +26,12 @@ Here are some additional advices that are more likely to apply to NelmioApiDocBu ``@OA\Response(..., @OA\JsonContent(...))`` or ``@OA\Response(..., @OA\XmlContent(...))``. When you use ``@Model`` directly (``@OA\Response(..., @Model(...)))``), the media type is set by default to ``json``. + +BC Breaks +--------- + +There are also BC breaks that were introduced in 4.0: + +- We migrated from `EXSyst\Component\Swagger\Swagger` to `OpenApi\Annotations\OpenApi` to describe the api in all our describers signature (`DescriberInterface`, `RouteDescriberInterface`, `ModelDescriberInterface`, `PropertyDescriberInterface`). + +- `PropertyDescriberInterface` now takes several types as input to leverage compound types support in OpenApi 3.0 (`int|string` for instance).