From 8d7290eb9150b3fdc0fb2ef36051a178eb18cdea Mon Sep 17 00:00:00 2001 From: Alan Poulain Date: Wed, 22 Apr 2020 17:23:06 +0200 Subject: [PATCH] [GraphQL] Support serialized name (#3516) --- CHANGELOG.md | 1 + features/doctrine/search_filter.feature | 3 +- features/graphql/query.feature | 12 ++++ .../Serializer/SerializerContextBuilder.php | 36 ++++++----- src/GraphQl/Type/FieldsBuilder.php | 10 +++- .../Fixtures/TestBundle/Document/DummyCar.php | 20 +++++++ tests/Fixtures/TestBundle/Entity/DummyCar.php | 20 +++++++ .../SerializerContextBuilderTest.php | 59 +++++++++++++++---- tests/GraphQl/Type/FieldsBuilderTest.php | 38 +++++++++++- .../AnnotationFilterExtractorTraitTest.php | 2 +- 10 files changed, 168 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df11bfe125a..9a864b0fea9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * GraphQL: Allow to format GraphQL errors based on exceptions (#3063) * GraphQL: Add page-based pagination (#3175) * GraphQL: Possibility to add a custom description for queries, mutations and subscriptions (#3477, #3514) +* GraphQL: Support for field name conversion (serialized name) (#3455, #3516) * OpenAPI: Add PHP default values to the documentation (#2386) * Deprecate using a validation groups generator service not implementing `ApiPlatform\Core\Bridge\Symfony\Validator\ValidationGroupsGeneratorInterface` (#3346) diff --git a/features/doctrine/search_filter.feature b/features/doctrine/search_filter.feature index 857e0c70718..e33b7515c47 100644 --- a/features/doctrine/search_filter.feature +++ b/features/doctrine/search_filter.feature @@ -65,7 +65,8 @@ Feature: Search filter on collections "prop": "blue" } ], - "uuid": [] + "uuid": [], + "carBrand": "DummyBrand" } ], "hydra:totalItems": 1, diff --git a/features/graphql/query.feature b/features/graphql/query.feature index 8fd4058ca3e..93285285dec 100644 --- a/features/graphql/query.feature +++ b/features/graphql/query.feature @@ -126,6 +126,18 @@ Feature: GraphQL query support And the header "Content-Type" should be equal to "application/json" And the JSON node "data.dummyGroup.foo" should be equal to "Foo #1" + Scenario: Query a serialized name + Given there is a DummyCar entity with related colors + When I send the following GraphQL request: + """ + { + dummyCar(id: "/dummy_cars/1") { + carBrand + } + } + """ + Then the JSON node "data.dummyCar.carBrand" should be equal to "DummyBrand" + Scenario: Fetch only the internal id When I send the following GraphQL request: """ diff --git a/src/GraphQl/Serializer/SerializerContextBuilder.php b/src/GraphQl/Serializer/SerializerContextBuilder.php index 49803d82751..a822c35474d 100644 --- a/src/GraphQl/Serializer/SerializerContextBuilder.php +++ b/src/GraphQl/Serializer/SerializerContextBuilder.php @@ -16,6 +16,7 @@ use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; use GraphQL\Type\Definition\ResolveInfo; +use Symfony\Component\Serializer\NameConverter\AdvancedNameConverterInterface; use Symfony\Component\Serializer\NameConverter\NameConverterInterface; /** @@ -45,10 +46,6 @@ public function create(?string $resourceClass, string $operationName, array $res 'graphql_operation_name' => $operationName, ]; - if ($normalization) { - $context['attributes'] = $this->fieldsToAttributes($resourceMetadata, $resolverContext); - } - if (isset($resolverContext['fields'])) { $context['no_resolver_data'] = true; } @@ -61,25 +58,29 @@ public function create(?string $resourceClass, string $operationName, array $res $context = array_merge($resourceMetadata->getGraphqlAttribute($operationName, $key, [], true), $context); } + if ($normalization) { + $context['attributes'] = $this->fieldsToAttributes($resourceClass, $resourceMetadata, $resolverContext, $context); + } + return $context; } /** * Retrieves fields, recursively replaces the "_id" key (the raw id) by "id" (the name of the property expected by the Serializer) and flattens edge and node structures (pagination). */ - private function fieldsToAttributes(?ResourceMetadata $resourceMetadata, array $context): array + private function fieldsToAttributes(?string $resourceClass, ?ResourceMetadata $resourceMetadata, array $resolverContext, array $context): array { - if (isset($context['fields'])) { - $fields = $context['fields']; + if (isset($resolverContext['fields'])) { + $fields = $resolverContext['fields']; } else { /** @var ResolveInfo $info */ - $info = $context['info']; + $info = $resolverContext['info']; $fields = $info->getFieldSelection(PHP_INT_MAX); } - $attributes = $this->replaceIdKeys($fields['edges']['node'] ?? $fields); + $attributes = $this->replaceIdKeys($fields['edges']['node'] ?? $fields, $resourceClass, $context); - if ($context['is_mutation'] || $context['is_subscription']) { + if ($resolverContext['is_mutation'] || $resolverContext['is_subscription']) { if (!$resourceMetadata) { throw new \LogicException('ResourceMetadata should always exist for a mutation or a subscription.'); } @@ -92,7 +93,7 @@ private function fieldsToAttributes(?ResourceMetadata $resourceMetadata, array $ return $attributes; } - private function replaceIdKeys(array $fields): array + private function replaceIdKeys(array $fields, ?string $resourceClass, array $context): array { $denormalizedFields = []; @@ -103,14 +104,21 @@ private function replaceIdKeys(array $fields): array continue; } - $denormalizedFields[$this->denormalizePropertyName((string) $key)] = \is_array($fields[$key]) ? $this->replaceIdKeys($fields[$key]) : $value; + $denormalizedFields[$this->denormalizePropertyName((string) $key, $resourceClass, $context)] = \is_array($fields[$key]) ? $this->replaceIdKeys($fields[$key], $resourceClass, $context) : $value; } return $denormalizedFields; } - private function denormalizePropertyName(string $property): string + private function denormalizePropertyName(string $property, ?string $resourceClass, array $context): string { - return null !== $this->nameConverter ? $this->nameConverter->denormalize($property) : $property; + if (null === $this->nameConverter) { + return $property; + } + if ($this->nameConverter instanceof AdvancedNameConverterInterface) { + return $this->nameConverter->denormalize($property, $resourceClass, null, $context); + } + + return $this->nameConverter->denormalize($property); } } diff --git a/src/GraphQl/Type/FieldsBuilder.php b/src/GraphQl/Type/FieldsBuilder.php index aef61d96449..c073edaeebe 100644 --- a/src/GraphQl/Type/FieldsBuilder.php +++ b/src/GraphQl/Type/FieldsBuilder.php @@ -29,6 +29,7 @@ use Psr\Container\ContainerInterface; use Symfony\Component\Config\Definition\Exception\InvalidTypeException; use Symfony\Component\PropertyInfo\Type; +use Symfony\Component\Serializer\NameConverter\AdvancedNameConverterInterface; use Symfony\Component\Serializer\NameConverter\NameConverterInterface; /** @@ -497,6 +498,13 @@ private function convertType(Type $type, bool $input, ?string $queryName, ?strin private function normalizePropertyName(string $property, string $resourceClass): string { - return null !== $this->nameConverter ? $this->nameConverter->normalize($property, $resourceClass) : $property; + if (null === $this->nameConverter) { + return $property; + } + if ($this->nameConverter instanceof AdvancedNameConverterInterface) { + return $this->nameConverter->normalize($property, $resourceClass); + } + + return $this->nameConverter->normalize($property); } } diff --git a/tests/Fixtures/TestBundle/Document/DummyCar.php b/tests/Fixtures/TestBundle/Document/DummyCar.php index df23c2cfe9a..ea64c6babe7 100644 --- a/tests/Fixtures/TestBundle/Document/DummyCar.php +++ b/tests/Fixtures/TestBundle/Document/DummyCar.php @@ -110,6 +110,16 @@ class DummyCar */ private $availableAt; + /** + * @var string + * + * @Serializer\Groups({"colors"}) + * @Serializer\SerializedName("carBrand") + * + * @ODM\Field + */ + private $brand = 'DummyBrand'; + public function __construct() { $this->colors = new ArrayCollection(); @@ -191,4 +201,14 @@ public function setAvailableAt(\DateTime $availableAt) { $this->availableAt = $availableAt; } + + public function getBrand(): string + { + return $this->brand; + } + + public function setBrand(string $brand): void + { + $this->brand = $brand; + } } diff --git a/tests/Fixtures/TestBundle/Entity/DummyCar.php b/tests/Fixtures/TestBundle/Entity/DummyCar.php index a6bb99575fe..ad0660f65a9 100644 --- a/tests/Fixtures/TestBundle/Entity/DummyCar.php +++ b/tests/Fixtures/TestBundle/Entity/DummyCar.php @@ -115,6 +115,16 @@ class DummyCar */ private $availableAt; + /** + * @var string + * + * @Serializer\Groups({"colors"}) + * @Serializer\SerializedName("carBrand") + * + * @ORM\Column + */ + private $brand = 'DummyBrand'; + public function __construct() { $this->colors = new ArrayCollection(); @@ -199,4 +209,14 @@ public function setAvailableAt(\DateTime $availableAt) { $this->availableAt = $availableAt; } + + public function getBrand(): string + { + return $this->brand; + } + + public function setBrand(string $brand): void + { + $this->brand = $brand; + } } diff --git a/tests/GraphQl/Serializer/SerializerContextBuilderTest.php b/tests/GraphQl/Serializer/SerializerContextBuilderTest.php index 277a696f6fb..b4a3e4d6072 100644 --- a/tests/GraphQl/Serializer/SerializerContextBuilderTest.php +++ b/tests/GraphQl/Serializer/SerializerContextBuilderTest.php @@ -19,6 +19,8 @@ use ApiPlatform\Core\Tests\Fixtures\TestBundle\Serializer\NameConverter\CustomConverter; use GraphQL\Type\Definition\ResolveInfo; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; +use Symfony\Component\Serializer\NameConverter\AdvancedNameConverterInterface; /** * @author Alan Poulain @@ -36,16 +38,18 @@ protected function setUp(): void { $this->resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); - $this->serializerContextBuilder = new SerializerContextBuilder( - $this->resourceMetadataFactoryProphecy->reveal(), - new CustomConverter() - ); + $this->serializerContextBuilder = $this->buildSerializerContextBuilder(); + } + + private function buildSerializerContextBuilder(?AdvancedNameConverterInterface $advancedNameConverter = null): SerializerContextBuilder + { + return new SerializerContextBuilder($this->resourceMetadataFactoryProphecy->reveal(), $advancedNameConverter ?? new CustomConverter()); } /** * @dataProvider createNormalizationContextProvider */ - public function testCreateNormalizationContext(?string $resourceClass, string $operationName, array $fields, bool $isMutation, bool $isSubscription, bool $noInfo, array $expectedContext, ?string $expectedExceptionClass = null, ?string $expectedExceptionMessage = null): void + public function testCreateNormalizationContext(?string $resourceClass, string $operationName, array $fields, bool $isMutation, bool $isSubscription, bool $noInfo, array $expectedContext, ?AdvancedNameConverterInterface $advancedNameConverter = null, ?string $expectedExceptionClass = null, ?string $expectedExceptionMessage = null): void { $resolverContext = [ 'is_mutation' => $isMutation, @@ -76,13 +80,21 @@ public function testCreateNormalizationContext(?string $resourceClass, string $o $this->expectExceptionMessage($expectedExceptionMessage); } - $context = $this->serializerContextBuilder->create($resourceClass, $operationName, $resolverContext, true); + $serializerContextBuilder = $this->serializerContextBuilder; + if ($advancedNameConverter) { + $serializerContextBuilder = $this->buildSerializerContextBuilder($advancedNameConverter); + } + + $context = $serializerContextBuilder->create($resourceClass, $operationName, $resolverContext, true); $this->assertSame($expectedContext, $context); } public function createNormalizationContextProvider(): array { + $advancedNameConverter = $this->prophesize(AdvancedNameConverterInterface::class); + $advancedNameConverter->denormalize('field', 'myResource', null, Argument::type('array'))->willReturn('denormalizedField'); + return [ 'nominal' => [ $resourceClass = 'myResource', @@ -95,13 +107,33 @@ public function createNormalizationContextProvider(): array 'groups' => ['normalization_group'], 'resource_class' => $resourceClass, 'graphql_operation_name' => $operationName, + 'input' => ['class' => 'inputClass'], + 'output' => ['class' => 'outputClass'], 'attributes' => [ 'id' => 3, 'field' => 'foo', ], + ], + ], + 'nominal with advanced name converter' => [ + $resourceClass = 'myResource', + $operationName = 'item_query', + ['_id' => 3, 'field' => 'foo'], + false, + false, + false, + [ + 'groups' => ['normalization_group'], + 'resource_class' => $resourceClass, + 'graphql_operation_name' => $operationName, 'input' => ['class' => 'inputClass'], 'output' => ['class' => 'outputClass'], + 'attributes' => [ + 'id' => 3, + 'denormalizedField' => 'foo', + ], ], + $advancedNameConverter->reveal(), ], 'nominal collection' => [ $resourceClass = 'myResource', @@ -114,11 +146,11 @@ public function createNormalizationContextProvider(): array 'groups' => ['normalization_group'], 'resource_class' => $resourceClass, 'graphql_operation_name' => $operationName, + 'input' => ['class' => 'inputClass'], + 'output' => ['class' => 'outputClass'], 'attributes' => [ 'nodeField' => 'baz', ], - 'input' => ['class' => 'inputClass'], - 'output' => ['class' => 'outputClass'], ], ], 'no resource class' => [ @@ -147,12 +179,12 @@ public function createNormalizationContextProvider(): array 'groups' => ['normalization_group'], 'resource_class' => $resourceClass, 'graphql_operation_name' => $operationName, + 'input' => ['class' => 'inputClass'], + 'output' => ['class' => 'outputClass'], 'attributes' => [ 'id' => 7, 'related' => ['field' => 'bar'], ], - 'input' => ['class' => 'inputClass'], - 'output' => ['class' => 'outputClass'], ], ], 'mutation without resource class' => [ @@ -163,6 +195,7 @@ public function createNormalizationContextProvider(): array false, false, [], + null, \LogicException::class, 'ResourceMetadata should always exist for a mutation or a subscription.', ], @@ -177,13 +210,13 @@ public function createNormalizationContextProvider(): array 'groups' => ['normalization_group'], 'resource_class' => $resourceClass, 'graphql_operation_name' => $operationName, + 'no_resolver_data' => true, + 'input' => ['class' => 'inputClass'], + 'output' => ['class' => 'outputClass'], 'attributes' => [ 'id' => 7, 'related' => ['field' => 'bar'], ], - 'no_resolver_data' => true, - 'input' => ['class' => 'inputClass'], - 'output' => ['class' => 'outputClass'], ], ], ]; diff --git a/tests/GraphQl/Type/FieldsBuilderTest.php b/tests/GraphQl/Type/FieldsBuilderTest.php index dc95d41a093..ee4001044db 100644 --- a/tests/GraphQl/Type/FieldsBuilderTest.php +++ b/tests/GraphQl/Type/FieldsBuilderTest.php @@ -38,6 +38,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Psr\Container\ContainerInterface; use Symfony\Component\PropertyInfo\Type; +use Symfony\Component\Serializer\NameConverter\AdvancedNameConverterInterface; /** * @author Alan Poulain @@ -96,7 +97,12 @@ protected function setUp(): void $this->itemMutationResolverFactoryProphecy = $this->prophesize(ResolverFactoryInterface::class); $this->itemSubscriptionResolverFactoryProphecy = $this->prophesize(ResolverFactoryInterface::class); $this->filterLocatorProphecy = $this->prophesize(ContainerInterface::class); - $this->fieldsBuilder = new FieldsBuilder($this->propertyNameCollectionFactoryProphecy->reveal(), $this->propertyMetadataFactoryProphecy->reveal(), $this->resourceMetadataFactoryProphecy->reveal(), $this->typesContainerProphecy->reveal(), $this->typeBuilderProphecy->reveal(), $this->typeConverterProphecy->reveal(), $this->itemResolverFactoryProphecy->reveal(), $this->collectionResolverFactoryProphecy->reveal(), $this->itemMutationResolverFactoryProphecy->reveal(), $this->itemSubscriptionResolverFactoryProphecy->reveal(), $this->filterLocatorProphecy->reveal(), new Pagination($this->resourceMetadataFactoryProphecy->reveal()), new CustomConverter(), '__'); + $this->fieldsBuilder = $this->buildFieldsBuilder(); + } + + private function buildFieldsBuilder(?AdvancedNameConverterInterface $advancedNameConverter = null): FieldsBuilder + { + return new FieldsBuilder($this->propertyNameCollectionFactoryProphecy->reveal(), $this->propertyMetadataFactoryProphecy->reveal(), $this->resourceMetadataFactoryProphecy->reveal(), $this->typesContainerProphecy->reveal(), $this->typeBuilderProphecy->reveal(), $this->typeConverterProphecy->reveal(), $this->itemResolverFactoryProphecy->reveal(), $this->collectionResolverFactoryProphecy->reveal(), $this->itemMutationResolverFactoryProphecy->reveal(), $this->itemSubscriptionResolverFactoryProphecy->reveal(), $this->filterLocatorProphecy->reveal(), new Pagination($this->resourceMetadataFactoryProphecy->reveal()), $advancedNameConverter ?? new CustomConverter(), '__'); } public function testGetNodeQueryFields(): void @@ -484,7 +490,7 @@ public function subscriptionFieldsProvider(): array /** * @dataProvider resourceObjectTypeFieldsProvider */ - public function testGetResourceObjectTypeFields(string $resourceClass, ResourceMetadata $resourceMetadata, array $properties, bool $input, ?string $queryName, ?string $mutationName, ?string $subscriptionName, ?array $ioMetadata, array $expectedResourceObjectTypeFields): void + public function testGetResourceObjectTypeFields(string $resourceClass, ResourceMetadata $resourceMetadata, array $properties, bool $input, ?string $queryName, ?string $mutationName, ?string $subscriptionName, ?array $ioMetadata, array $expectedResourceObjectTypeFields, ?AdvancedNameConverterInterface $advancedNameConverter = null): void { $this->propertyNameCollectionFactoryProphecy->create($resourceClass)->willReturn(new PropertyNameCollection(array_keys($properties))); foreach ($properties as $propertyName => $propertyMetadata) { @@ -501,13 +507,20 @@ public function testGetResourceObjectTypeFields(string $resourceClass, ResourceM $this->typeBuilderProphecy->isCollection(Argument::type(Type::class))->willReturn(false); $this->resourceMetadataFactoryProphecy->create('subresourceClass')->willReturn(new ResourceMetadata()); - $resourceObjectTypeFields = $this->fieldsBuilder->getResourceObjectTypeFields($resourceClass, $resourceMetadata, $input, $queryName, $mutationName, $subscriptionName, 0, $ioMetadata); + $fieldsBuilder = $this->fieldsBuilder; + if ($advancedNameConverter) { + $fieldsBuilder = $this->buildFieldsBuilder($advancedNameConverter); + } + $resourceObjectTypeFields = $fieldsBuilder->getResourceObjectTypeFields($resourceClass, $resourceMetadata, $input, $queryName, $mutationName, $subscriptionName, 0, $ioMetadata); $this->assertEquals($expectedResourceObjectTypeFields, $resourceObjectTypeFields); } public function resourceObjectTypeFieldsProvider(): array { + $advancedNameConverter = $this->prophesize(AdvancedNameConverterInterface::class); + $advancedNameConverter->normalize('field', 'resourceClass')->willReturn('normalizedField'); + return [ 'query' => ['resourceClass', new ResourceMetadata(), [ @@ -537,6 +550,25 @@ public function resourceObjectTypeFieldsProvider(): array ], ], ], + 'query with advanced name converter' => ['resourceClass', new ResourceMetadata(), + [ + 'field' => new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), null, true, false), + ], + false, 'item_query', null, null, null, + [ + 'id' => [ + 'type' => GraphQLType::nonNull(GraphQLType::id()), + ], + 'normalizedField' => [ + 'type' => GraphQLType::nonNull(GraphQLType::string()), + 'description' => null, + 'args' => [], + 'resolve' => null, + 'deprecationReason' => '', + ], + ], + $advancedNameConverter->reveal(), + ], 'query input' => ['resourceClass', new ResourceMetadata(), [ 'property' => new PropertyMetadata(), diff --git a/tests/Util/AnnotationFilterExtractorTraitTest.php b/tests/Util/AnnotationFilterExtractorTraitTest.php index 4a329ac0f42..c3dff404b1d 100644 --- a/tests/Util/AnnotationFilterExtractorTraitTest.php +++ b/tests/Util/AnnotationFilterExtractorTraitTest.php @@ -40,7 +40,7 @@ public function testReadAnnotations() $this->assertEquals($this->extractor->getFilters($reflectionClass), [ 'annotated_api_platform_core_tests_fixtures_test_bundle_entity_dummy_car_api_platform_core_bridge_doctrine_orm_filter_date_filter' => [ - ['properties' => ['id' => 'exclude_null', 'colors' => 'exclude_null', 'name' => 'exclude_null', 'canSell' => 'exclude_null', 'availableAt' => 'exclude_null', 'secondColors' => 'exclude_null', 'thirdColors' => 'exclude_null', 'uuid' => 'exclude_null']], + ['properties' => ['id' => 'exclude_null', 'colors' => 'exclude_null', 'name' => 'exclude_null', 'canSell' => 'exclude_null', 'availableAt' => 'exclude_null', 'brand' => 'exclude_null', 'secondColors' => 'exclude_null', 'thirdColors' => 'exclude_null', 'uuid' => 'exclude_null']], DateFilter::class, ], 'annotated_api_platform_core_tests_fixtures_test_bundle_entity_dummy_car_api_platform_core_bridge_doctrine_orm_filter_boolean_filter' => [