From 65e940f7f80219ca02745db40977e06b2b26313a Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Mon, 11 Mar 2019 12:53:35 +0100 Subject: [PATCH 1/3] use jms serialization groups detection --- ModelDescriber/JMSModelDescriber.php | 100 ++++++++++++------ .../Entity/NestedGroup/JMSChatFriend.php | 2 +- .../Entity/NestedGroup/JMSChatUser.php | 2 +- Tests/Functional/JMSFunctionalTest.php | 66 +++--------- Tests/Functional/SwaggerUiTest.php | 6 +- Tests/Functional/TestKernel.php | 10 ++ 6 files changed, 99 insertions(+), 87 deletions(-) diff --git a/ModelDescriber/JMSModelDescriber.php b/ModelDescriber/JMSModelDescriber.php index 046fcce..3e9912b 100644 --- a/ModelDescriber/JMSModelDescriber.php +++ b/ModelDescriber/JMSModelDescriber.php @@ -13,6 +13,7 @@ namespace Nelmio\ApiDocBundle\ModelDescriber; use Doctrine\Common\Annotations\Reader; use EXSyst\Component\Swagger\Schema; +use JMS\Serializer\Context; use JMS\Serializer\Exclusion\GroupsExclusionStrategy; use JMS\Serializer\Naming\PropertyNamingStrategyInterface; use JMS\Serializer\SerializationContext; @@ -31,9 +32,14 @@ class JMSModelDescriber implements ModelDescriberInterface, ModelRegistryAwareIn use ModelRegistryAwareTrait; private $factory; + private $namingStrategy; + private $doctrineReader; - private $previousGroups = []; + + private $contexts = []; + + private $metadataStacks = []; /** * @var array @@ -61,40 +67,22 @@ class JMSModelDescriber implements ModelDescriberInterface, ModelRegistryAwareIn throw new \InvalidArgumentException(sprintf('No metadata found for class %s.', $className)); } - $groupsExclusion = null !== $model->getGroups() ? new GroupsExclusionStrategy($model->getGroups()) : null; - $schema->setType('object'); $annotationsReader = new AnnotationsReader($this->doctrineReader, $this->modelRegistry); $annotationsReader->updateDefinition(new \ReflectionClass($className), $schema); $isJmsV1 = null !== $this->namingStrategy; - $properties = $schema->getProperties(); + + $context = $this->getSerializationContext($model); + $context->pushClassMetadata($metadata); foreach ($metadata->propertyMetadata as $item) { // filter groups - if (null !== $groupsExclusion && $groupsExclusion->shouldSkipProperty($item, SerializationContext::create())) { + if (null !== $context->getExclusionStrategy() && $context->getExclusionStrategy()->shouldSkipProperty($item, $context)) { continue; } - $groups = $model->getGroups(); - - $previousGroups = null; - if (isset($groups[$item->name]) && is_array($groups[$item->name])) { - $previousGroups = $groups; - $groups = $groups[$item->name]; - } elseif (!isset($groups[$item->name]) && !empty($this->previousGroups[$model->getHash()])) { - $groups = false === $this->propertyTypeUsesGroups($item->type) - ? null - : ($isJmsV1 ? [GroupsExclusionStrategy::DEFAULT_GROUP] : $this->previousGroups[$model->getHash()]); - } - - if (is_array($groups)) { - $groups = array_filter($groups, 'is_scalar'); - } - - if ([GroupsExclusionStrategy::DEFAULT_GROUP] === $groups) { - $groups = null; - } + $context->pushPropertyMetadata($item); $name = true === $isJmsV1 ? $this->namingStrategy->translateName($item) : $item->serializedName; // read property options from Swagger Property annotation if it exists @@ -106,22 +94,71 @@ class JMSModelDescriber implements ModelDescriberInterface, ModelRegistryAwareIn } $property = $properties->get($annotationsReader->getPropertyName($reflection, $name)); + $groups = $this->computeGroups($context, $item->type); $annotationsReader->updateProperty($reflection, $property, $groups); } catch (\ReflectionException $e) { $property = $properties->get($name); } if (null !== $property->getType() || null !== $property->getRef()) { + $context->popPropertyMetadata(); + continue; } if (null === $item->type) { $properties->remove($name); + $context->popPropertyMetadata(); continue; } - $this->describeItem($item->type, $property, $groups, $previousGroups); + $this->describeItem($item->type, $property, $context, $item); + $context->popPropertyMetadata(); } + $context->popClassMetadata(); + } + + private function getSerializationContext(Model $model): SerializationContext + { + if (isset($this->contexts[$model->getHash()])) { + $context = $this->contexts[$model->getHash()]; + + $stack = $context->getMetadataStack(); + while (!$stack->isEmpty()) { + $stack->pop(); + } + + foreach ($this->metadataStacks[$model->getHash()] as $metadataCopy) { + $stack->unshift($metadataCopy); + } + } else { + $context = SerializationContext::create(); + + if (null !== $model->getGroups()) { + $context->addExclusionStrategy(new GroupsExclusionStrategy($model->getGroups())); + } + } + + return $context; + } + + private function computeGroups(Context $context, array $type = null) + { + if (null === $type || true !== $this->propertyTypeUsesGroups($type)) { + return null; + } + + $groupsExclusion = $context->getExclusionStrategy(); + if (!($groupsExclusion instanceof GroupsExclusionStrategy)) { + return null; + } + + $groups = $groupsExclusion->getGroupsFor($context); + if ([GroupsExclusionStrategy::DEFAULT_GROUP] === $groups) { + return null; + } + + return $groups; } /** @@ -141,7 +178,7 @@ class JMSModelDescriber implements ModelDescriberInterface, ModelRegistryAwareIn return false; } - private function describeItem(array $type, $property, array $groups = null, array $previousGroups = null) + private function describeItem(array $type, $property, Context $context) { $nestedTypeInfo = $this->getNestedTypeInArray($type); if (null !== $nestedTypeInfo) { @@ -156,13 +193,13 @@ class JMSModelDescriber implements ModelDescriberInterface, ModelRegistryAwareIn return; } - $this->describeItem($nestedType, $property->getAdditionalProperties(), $groups, $previousGroups); + $this->describeItem($nestedType, $property->getAdditionalProperties(), $context); return; } $property->setType('array'); - $this->describeItem($nestedType, $property->getItems(), $groups, $previousGroups); + $this->describeItem($nestedType, $property->getItems(), $context); } elseif ('array' === $type['name']) { $property->setType('object'); $property->merge(['additionalProperties' => []]); @@ -177,12 +214,13 @@ class JMSModelDescriber implements ModelDescriberInterface, ModelRegistryAwareIn $property->setType('string'); $property->setFormat('date-time'); } else { + $groups = $this->computeGroups($context, $type); + $model = new Model(new Type(Type::BUILTIN_TYPE_OBJECT, false, $type['name']), $groups); $property->setRef($this->modelRegistry->register($model)); - if ($previousGroups) { - $this->previousGroups[$model->getHash()] = $previousGroups; - } + $this->contexts[$model->getHash()] = $context; + $this->metadataStacks[$model->getHash()] = clone $context->getMetadataStack(); } } diff --git a/Tests/Functional/Entity/NestedGroup/JMSChatFriend.php b/Tests/Functional/Entity/NestedGroup/JMSChatFriend.php index 00792b8..161337e 100644 --- a/Tests/Functional/Entity/NestedGroup/JMSChatFriend.php +++ b/Tests/Functional/Entity/NestedGroup/JMSChatFriend.php @@ -30,7 +30,7 @@ class JMSChatFriend /** * @Serializer\Type("Nelmio\ApiDocBundle\Tests\Functional\Entity\NestedGroup\JMSChatLivingRoom") * @Serializer\Expose - * @Serializer\Groups({"Default"}) + * @Serializer\Groups({"Default", "mini"}) */ private $living; diff --git a/Tests/Functional/Entity/NestedGroup/JMSChatUser.php b/Tests/Functional/Entity/NestedGroup/JMSChatUser.php index 71ea114..a220bfd 100644 --- a/Tests/Functional/Entity/NestedGroup/JMSChatUser.php +++ b/Tests/Functional/Entity/NestedGroup/JMSChatUser.php @@ -28,7 +28,7 @@ class JMSChatUser /** * @Serializer\Type("Nelmio\ApiDocBundle\Tests\Functional\Entity\NestedGroup\JMSPicture") - * @Serializer\Groups({"Default", "mini"}) + * @Serializer\Groups({"mini"}) * @Serializer\Expose */ private $picture; diff --git a/Tests/Functional/JMSFunctionalTest.php b/Tests/Functional/JMSFunctionalTest.php index cda11f8..431609e 100644 --- a/Tests/Functional/JMSFunctionalTest.php +++ b/Tests/Functional/JMSFunctionalTest.php @@ -11,8 +11,6 @@ namespace Nelmio\ApiDocBundle\Tests\Functional; -use JMS\Serializer\Visitor\SerializationVisitorInterface; - class JMSFunctionalTest extends WebTestCase { public function testModelPictureDocumentation() @@ -20,11 +18,20 @@ class JMSFunctionalTest extends WebTestCase $this->assertEquals([ 'type' => 'object', 'properties' => [ - 'only_direct_picture_mini' => [ + 'id' => [ 'type' => 'integer', ], ], ], $this->getModel('JMSPicture')->toArray()); + + $this->assertEquals([ + 'type' => 'object', + 'properties' => [ + 'only_direct_picture_mini' => [ + 'type' => 'integer', + ], + ], + ], $this->getModel('JMSPicture_mini')->toArray()); } public function testModeChatDocumentation() @@ -48,7 +55,7 @@ class JMSFunctionalTest extends WebTestCase 'type' => 'object', 'properties' => [ 'picture' => [ - '$ref' => '#/definitions/JMSPicture2', + '$ref' => '#/definitions/JMSPicture', ], ], ], $this->getModel('JMSChatUser')->toArray()); @@ -212,12 +219,8 @@ class JMSFunctionalTest extends WebTestCase ], $this->getModel('JMSDualComplex')->toArray()); } - public function testNestedGroupsV1() + public function testNestedGroups() { - if (interface_exists(SerializationVisitorInterface::class)){ - $this->markTestSkipped('This applies only for jms/serializer v1.x'); - } - $this->assertEquals([ 'type' => 'object', 'properties' => [ @@ -230,48 +233,18 @@ class JMSFunctionalTest extends WebTestCase 'type' => 'object', 'properties' => [ 'id1' => ['type' => 'integer'], - 'id2' => ['type' => 'integer'], 'id3' => ['type' => 'integer'], ], ], $this->getModel('JMSChatRoom')->toArray()); } - public function testNestedGroupsV2() - { - if (!interface_exists(SerializationVisitorInterface::class)){ - $this->markTestSkipped('This applies only for jms/serializer v2.x'); - } - - $this->assertEquals([ - 'type' => 'object', - 'properties' => [ - 'living' => ['$ref' => '#/definitions/JMSChatLivingRoom'], - 'dining' => ['$ref' => '#/definitions/JMSChatRoom'], - ], - ], $this->getModel('JMSChatFriend')->toArray()); - - $this->assertEquals([ - 'type' => 'object', - 'properties' => [ - 'id2' => ['type' => 'integer'], - ], - ], $this->getModel('JMSChatRoom')->toArray()); - - $this->assertEquals([ - 'type' => 'object', - 'properties' => [ - 'id' => ['type' => 'integer'], - ], - ], $this->getModel('JMSChatLivingRoom')->toArray()); - } - public function testModelComplexDocumentation() { $this->assertEquals([ 'type' => 'object', 'properties' => [ 'id' => ['type' => 'integer'], - 'user' => ['$ref' => '#/definitions/JMSUser2'], + 'user' => ['$ref' => '#/definitions/JMSUser'], 'name' => ['type' => 'string'], 'virtual' => ['$ref' => '#/definitions/JMSUser'], ], @@ -280,19 +253,6 @@ class JMSFunctionalTest extends WebTestCase 'user', ], ], $this->getModel('JMSComplex')->toArray()); - - $this->assertEquals([ - 'type' => 'object', - 'properties' => [ - 'id' => [ - 'type' => 'integer', - 'title' => 'userid', - 'description' => 'User id', - 'readOnly' => true, - 'example' => '1', - ], - ], - ], $this->getModel('JMSUser2')->toArray()); } public function testYamlConfig() diff --git a/Tests/Functional/SwaggerUiTest.php b/Tests/Functional/SwaggerUiTest.php index 4d958e4..b3cbd81 100644 --- a/Tests/Functional/SwaggerUiTest.php +++ b/Tests/Functional/SwaggerUiTest.php @@ -50,7 +50,11 @@ class SwaggerUiTest extends WebTestCase 'responses' => ['200' => ['description' => 'Test']], ]], ]; - $expected['definitions'] = ['Dummy' => $expected['definitions']['Dummy'], 'Test' => ['type' => 'string']]; + $expected['definitions'] = [ + 'Dummy' => $expected['definitions']['Dummy'], + 'Test' => ['type' => 'string'], + 'JMSPicture_mini' => ['type' => 'object'], + ]; yield ['/docs/test', 'test', $expected]; } diff --git a/Tests/Functional/TestKernel.php b/Tests/Functional/TestKernel.php index 31d4b5c..912fe0c 100644 --- a/Tests/Functional/TestKernel.php +++ b/Tests/Functional/TestKernel.php @@ -16,6 +16,7 @@ use Bazinga\Bundle\HateoasBundle\BazingaHateoasBundle; use FOS\RestBundle\FOSRestBundle; use JMS\SerializerBundle\JMSSerializerBundle; use Nelmio\ApiDocBundle\NelmioApiDocBundle; +use Nelmio\ApiDocBundle\Tests\Functional\Entity\NestedGroup\JMSPicture; use Nelmio\ApiDocBundle\Tests\Functional\ModelDescriber\VirtualTypeClassDoesNotExistsHandlerDefinedDescriber; use Sensio\Bundle\FrameworkExtraBundle\SensioFrameworkExtraBundle; use Symfony\Bundle\FrameworkBundle\FrameworkBundle; @@ -164,6 +165,15 @@ class TestKernel extends Kernel ], ], ], + 'models' => [ + 'names' => [ + [ + 'alias' => 'JMSPicture_mini', + 'type' => JMSPicture::class, + 'groups' => ['mini'], + ], + ], + ], ]); $def = new Definition(VirtualTypeClassDoesNotExistsHandlerDefinedDescriber::class); From 54053571a8b453634017fbf2c6826df634ae36a7 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Wed, 10 Apr 2019 15:07:44 +0200 Subject: [PATCH 2/3] let composer decide the stability --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index cd4aa04..bf7568d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -29,6 +29,6 @@ matrix: before_install: - phpenv config-rm xdebug.ini -install: composer update $COMPOSER_FLAGS --prefer-stable +install: composer update $COMPOSER_FLAGS script: ./phpunit From 4fbf09659528c5c26fd86a894bfd4780863bb7dd Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Wed, 10 Apr 2019 13:27:04 +0200 Subject: [PATCH 3/3] require jms v3 --- composer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index f14052f..7da0441 100644 --- a/composer.json +++ b/composer.json @@ -44,7 +44,8 @@ "api-platform/core": "^2.1.0", "friendsofsymfony/rest-bundle": "^2.0", "willdurand/hateoas-bundle": "^1.0|^2.0", - "jms/serializer-bundle": "^2.0|^3.0" + "jms/serializer-bundle": "^2.0|^3.0", + "jms/serializer": "^1.14|^3.0" }, "suggest": { "api-platform/core": "For using an API oriented framework.",