Skip to content

Commit b3bd652

Browse files
alanpoulainalanpoulain
authored
[GraphQL] Avoid to call serialize in ItemNormalizer (#2576)
* Avoid to call serialize in ItemNormalizer * Allow composite identifiers in getItemIriFromResourceClass
1 parent ea153d1 commit b3bd652

File tree

9 files changed

+65
-41
lines changed

9 files changed

+65
-41
lines changed

‎src/Bridge/Symfony/Bundle/Resources/config/graphql.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
<argument type="service" id="api_platform.collection_data_provider" />
1414
<argument type="service" id="api_platform.subresource_data_provider" />
1515
<argument type="service" id="serializer" />
16-
<argument type="service" id="api_platform.identifiers_extractor.cached" />
1716
<argument type="service" id="api_platform.metadata.resource.metadata_factory" />
1817
<argument type="service" id="api_platform.security.resource_access_checker" on-invalid="null" />
1918
<argument type="service" id="request_stack" />
@@ -85,6 +84,7 @@
8584
<argument type="service" id="api_platform.metadata.property.name_collection_factory" />
8685
<argument type="service" id="api_platform.metadata.property.metadata_factory" />
8786
<argument type="service" id="api_platform.iri_converter" />
87+
<argument type="service" id="api_platform.identifiers_extractor.cached" />
8888
<argument type="service" id="api_platform.resource_class_resolver" />
8989
<argument type="service" id="api_platform.property_accessor" />
9090
<argument type="service" id="api_platform.name_converter" on-invalid="ignore" />

‎src/Bridge/Symfony/Routing/IriConverter.php

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -116,23 +116,17 @@ public function getItemFromIri(string $iri, array $context = [])
116116
public function getIriFromItem($item, int $referenceType = UrlGeneratorInterface::ABS_PATH): string
117117
{
118118
$resourceClass = $this->getObjectClass($item);
119-
$routeName = $this->routeNameResolver->getRouteName($resourceClass, OperationType::ITEM);
120119

121120
try {
122-
$identifiers = $this->generateIdentifiersUrl($this->identifiersExtractor->getIdentifiersFromItem($item), $resourceClass);
123-
124-
return $this->router->generate($routeName, ['id' => implode(';', $identifiers)], $referenceType);
121+
$identifiers = $this->identifiersExtractor->getIdentifiersFromItem($item);
125122
} catch (RuntimeException $e) {
126123
throw new InvalidArgumentException(sprintf(
127124
'Unable to generate an IRI for the item of type "%s"',
128125
$resourceClass
129126
), $e->getCode(), $e);
130-
} catch (RoutingExceptionInterface $e) {
131-
throw new InvalidArgumentException(sprintf(
132-
'Unable to generate an IRI for the item of type "%s"',
133-
$resourceClass
134-
), $e->getCode(), $e);
135127
}
128+
129+
return $this->getItemIriFromResourceClass($resourceClass, $identifiers, $referenceType);
136130
}
137131

138132
/**
@@ -152,10 +146,17 @@ public function getIriFromResourceClass(string $resourceClass, int $referenceTyp
152146
*/
153147
public function getItemIriFromResourceClass(string $resourceClass, array $identifiers, int $referenceType = UrlGeneratorInterface::ABS_PATH): string
154148
{
149+
$routeName = $this->routeNameResolver->getRouteName($resourceClass, OperationType::ITEM);
150+
155151
try {
156-
return $this->router->generate($this->routeNameResolver->getRouteName($resourceClass, OperationType::ITEM), $identifiers, $referenceType);
152+
$identifiers = $this->generateIdentifiersUrl($identifiers, $resourceClass);
153+
154+
return $this->router->generate($routeName, ['id' => implode(';', $identifiers)], $referenceType);
157155
} catch (RoutingExceptionInterface $e) {
158-
throw new InvalidArgumentException(sprintf('Unable to generate an IRI for "%s".', $resourceClass), $e->getCode(), $e);
156+
throw new InvalidArgumentException(sprintf(
157+
'Unable to generate an IRI for "%s".',
158+
$resourceClass
159+
), $e->getCode(), $e);
159160
}
160161
}
161162

‎src/GraphQl/Resolver/Factory/CollectionResolverFactory.php

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
namespace ApiPlatform\Core\GraphQl\Resolver\Factory;
1515

16-
use ApiPlatform\Core\Api\IdentifiersExtractorInterface;
1716
use ApiPlatform\Core\DataProvider\CollectionDataProviderInterface;
1817
use ApiPlatform\Core\DataProvider\PaginatorInterface;
1918
use ApiPlatform\Core\DataProvider\SubresourceDataProviderInterface;
@@ -44,18 +43,16 @@ final class CollectionResolverFactory implements ResolverFactoryInterface
4443
private $collectionDataProvider;
4544
private $subresourceDataProvider;
4645
private $normalizer;
47-
private $identifiersExtractor;
4846
private $resourceAccessChecker;
4947
private $requestStack;
5048
private $paginationEnabled;
5149
private $resourceMetadataFactory;
5250

53-
public function __construct(CollectionDataProviderInterface $collectionDataProvider, SubresourceDataProviderInterface $subresourceDataProvider, NormalizerInterface $normalizer, IdentifiersExtractorInterface $identifiersExtractor, ResourceMetadataFactoryInterface $resourceMetadataFactory, ResourceAccessCheckerInterface $resourceAccessChecker = null, RequestStack $requestStack = null, bool $paginationEnabled = false)
51+
public function __construct(CollectionDataProviderInterface $collectionDataProvider, SubresourceDataProviderInterface $subresourceDataProvider, NormalizerInterface $normalizer, ResourceMetadataFactoryInterface $resourceMetadataFactory, ResourceAccessCheckerInterface $resourceAccessChecker = null, RequestStack $requestStack = null, bool $paginationEnabled = false)
5452
{
5553
$this->subresourceDataProvider = $subresourceDataProvider;
5654
$this->collectionDataProvider = $collectionDataProvider;
5755
$this->normalizer = $normalizer;
58-
$this->identifiersExtractor = $identifiersExtractor;
5956
$this->resourceAccessChecker = $resourceAccessChecker;
6057
$this->requestStack = $requestStack;
6158
$this->paginationEnabled = $paginationEnabled;
@@ -82,8 +79,8 @@ public function __invoke(string $resourceClass = null, string $rootClass = null,
8279
$dataProviderContext['filters'] = $this->getNormalizedFilters($args);
8380
$dataProviderContext['graphql'] = true;
8481

85-
if (isset($rootClass, $source[$rootProperty = $info->fieldName], $source[ItemNormalizer::ITEM_KEY])) {
86-
$rootResolvedFields = $this->identifiersExtractor->getIdentifiersFromItem(unserialize($source[ItemNormalizer::ITEM_KEY]));
82+
if (isset($rootClass, $source[$rootProperty = $info->fieldName], $source[ItemNormalizer::ITEM_IDENTIFIERS_KEY])) {
83+
$rootResolvedFields = $source[ItemNormalizer::ITEM_IDENTIFIERS_KEY];
8784
$subresource = $this->getSubresource($rootClass, $rootResolvedFields, array_keys($rootResolvedFields), $rootProperty, $resourceClass, true, $dataProviderContext);
8885
$collection = $subresource ?? [];
8986
} else {

‎src/GraphQl/Resolver/ResourceFieldResolver.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ public function __construct(IriConverterInterface $iriConverter)
3636
public function __invoke($source, $args, $context, ResolveInfo $info)
3737
{
3838
$property = null;
39-
if ('id' === $info->fieldName && isset($source[ItemNormalizer::ITEM_KEY])) {
40-
return $this->iriConverter->getIriFromItem(unserialize($source[ItemNormalizer::ITEM_KEY]));
39+
if ('id' === $info->fieldName && isset($source[ItemNormalizer::ITEM_RESOURCE_CLASS_KEY], $source[ItemNormalizer::ITEM_IDENTIFIERS_KEY])) {
40+
return $this->iriConverter->getItemIriFromResourceClass($source[ItemNormalizer::ITEM_RESOURCE_CLASS_KEY], $source[ItemNormalizer::ITEM_IDENTIFIERS_KEY]);
4141
}
4242

4343
if ('_id' === $info->fieldName && isset($source['id'])) {

‎src/GraphQl/Serializer/ItemNormalizer.php

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,21 @@
1313

1414
namespace ApiPlatform\Core\GraphQl\Serializer;
1515

16+
use ApiPlatform\Core\Api\IdentifiersExtractorInterface;
17+
use ApiPlatform\Core\Api\IriConverterInterface;
18+
use ApiPlatform\Core\Api\ResourceClassResolverInterface;
19+
use ApiPlatform\Core\DataProvider\ItemDataProviderInterface;
20+
use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
21+
use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
1622
use ApiPlatform\Core\Metadata\Property\PropertyMetadata;
23+
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
1724
use ApiPlatform\Core\Serializer\ItemNormalizer as BaseItemNormalizer;
25+
use ApiPlatform\Core\Util\ClassInfoTrait;
26+
use Psr\Log\LoggerInterface;
27+
use Psr\Log\NullLogger;
28+
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
29+
use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactoryInterface;
30+
use Symfony\Component\Serializer\NameConverter\NameConverterInterface;
1831

1932
/**
2033
* GraphQL normalizer.
@@ -23,8 +36,20 @@
2336
*/
2437
final class ItemNormalizer extends BaseItemNormalizer
2538
{
26-
const FORMAT = 'graphql';
27-
const ITEM_KEY = '#item';
39+
use ClassInfoTrait;
40+
41+
public const FORMAT = 'graphql';
42+
public const ITEM_RESOURCE_CLASS_KEY = '#itemResourceClass';
43+
public const ITEM_IDENTIFIERS_KEY = '#itemIdentifiers';
44+
45+
private $identifiersExtractor;
46+
47+
public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, IriConverterInterface $iriConverter, IdentifiersExtractorInterface $identifiersExtractor, ResourceClassResolverInterface $resourceClassResolver, PropertyAccessorInterface $propertyAccessor = null, NameConverterInterface $nameConverter = null, ClassMetadataFactoryInterface $classMetadataFactory = null, ItemDataProviderInterface $itemDataProvider = null, bool $allowPlainIdentifiers = false, LoggerInterface $logger = null, iterable $dataTransformers = [], ResourceMetadataFactoryInterface $resourceMetadataFactory = null, $allowUnmappedClass = false)
48+
{
49+
parent::__construct($propertyNameCollectionFactory, $propertyMetadataFactory, $iriConverter, $resourceClassResolver, $propertyAccessor, $nameConverter, $classMetadataFactory, $itemDataProvider, $allowPlainIdentifiers, $logger ?: new NullLogger(), $dataTransformers, $resourceMetadataFactory, $allowUnmappedClass);
50+
51+
$this->identifiersExtractor = $identifiersExtractor;
52+
}
2853

2954
/**
3055
* {@inheritdoc}
@@ -41,7 +66,8 @@ public function normalize($object, $format = null, array $context = [])
4166
{
4267
$object = $this->transformOutput($object, $context);
4368
$data = parent::normalize($object, $format, $context);
44-
$data[self::ITEM_KEY] = serialize($object); // calling serialize prevent weird normalization process done by Webonyx's GraphQL PHP
69+
$data[self::ITEM_RESOURCE_CLASS_KEY] = $this->getObjectClass($object);
70+
$data[self::ITEM_IDENTIFIERS_KEY] = $this->identifiersExtractor->getIdentifiersFromItem($object);
4571

4672
return $data;
4773
}

‎src/GraphQl/Type/SchemaBuilder.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,11 @@ private function getNodeInterface(): InterfaceType
131131
],
132132
],
133133
'resolveType' => function ($value) {
134-
if (!isset($value[ItemNormalizer::ITEM_KEY])) {
134+
if (!isset($value[ItemNormalizer::ITEM_RESOURCE_CLASS_KEY])) {
135135
return null;
136136
}
137137

138-
$shortName = (new \ReflectionObject(unserialize($value[ItemNormalizer::ITEM_KEY])))->getShortName();
138+
$shortName = (new \ReflectionClass($value[ItemNormalizer::ITEM_RESOURCE_CLASS_KEY]))->getShortName();
139139

140140
return $this->graphqlTypes[$shortName] ?? null;
141141
},

‎tests/GraphQl/Resolver/Factory/CollectionResolverFactoryTest.php

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
namespace ApiPlatform\Core\Tests\GraphQl\Resolver\Factory;
1515

16-
use ApiPlatform\Core\Api\IdentifiersExtractorInterface;
1716
use ApiPlatform\Core\DataProvider\ArrayPaginator;
1817
use ApiPlatform\Core\DataProvider\CollectionDataProviderInterface;
1918
use ApiPlatform\Core\DataProvider\PaginatorInterface;
@@ -26,7 +25,6 @@
2625
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedDummy;
2726
use GraphQL\Type\Definition\ObjectType;
2827
use GraphQL\Type\Definition\ResolveInfo;
29-
use GraphQL\Type\Definition\Type;
3028
use GraphQL\Type\Schema;
3129
use PHPUnit\Framework\TestCase;
3230
use Prophecy\Argument;
@@ -80,21 +78,19 @@ public function testCreateCollectionResolverNoPagination()
8078
*/
8179
public function testCreateSubresourceCollectionResolverNoPagination(array $subcollection, array $expected)
8280
{
81+
$identifiers = ['id' => 1];
8382
$factory = $this->createCollectionResolverFactory([
8483
'Object1',
8584
'Object2',
86-
], $subcollection, ['id' => 1], false);
85+
], $subcollection, $identifiers, false);
8786

8887
$resolver = $factory(RelatedDummy::class, Dummy::class, 'operationName');
8988

9089
$resolveInfo = new ResolveInfo('relatedDummies', [], null, new ObjectType(['name' => '']), '', new Schema([]), null, null, null, null);
9190

92-
$dummy = new Dummy();
93-
$dummy->setId(1);
94-
9591
$source = [
9692
'relatedDummies' => [],
97-
ItemNormalizer::ITEM_KEY => serialize($dummy),
93+
ItemNormalizer::ITEM_IDENTIFIERS_KEY => $identifiers,
9894
];
9995

10096
$this->assertEquals($expected, $resolver($source, [], null, $resolveInfo));
@@ -206,9 +202,6 @@ private function createCollectionResolverFactory($collection, array $subcollecti
206202
$normalizerProphecy->normalize($object, Argument::cetera())->willReturn('normalized'.$object);
207203
}
208204

209-
$identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class);
210-
$identifiersExtractorProphecy->getIdentifiersFromItem(Argument::type(Dummy::class))->willReturn($identifiers);
211-
212205
$resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
213206
$resourceMetadataFactoryProphecy->create(RelatedDummy::class)->willReturn(new ResourceMetadata('RelatedDummy', null, null, null, null, ['normalization_context' => ['groups' => ['foo']]]));
214207

@@ -221,7 +214,6 @@ private function createCollectionResolverFactory($collection, array $subcollecti
221214
$collectionDataProviderProphecy->reveal(),
222215
$subresourceDataProviderProphecy->reveal(),
223216
$normalizerProphecy->reveal(),
224-
$identifiersExtractorProphecy->reveal(),
225217
$resourceMetadataFactoryProphecy->reveal(),
226218
null,
227219
$requestStack,

‎tests/GraphQl/Resolver/ResourceFieldResolverTest.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,13 @@ class ResourceFieldResolverTest extends TestCase
2626
{
2727
public function testId()
2828
{
29-
$dummy = new Dummy();
30-
3129
$iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
32-
$iriConverterProphecy->getIriFromItem($dummy)->willReturn('/dummies/1')->shouldBeCalled();
30+
$iriConverterProphecy->getItemIriFromResourceClass(Dummy::class, ['id' => 1])->willReturn('/dummies/1')->shouldBeCalled();
3331

3432
$resolveInfo = new ResolveInfo('id', [], null, new ObjectType(['name' => '']), '', new Schema([]), null, null, null, null);
3533

3634
$resolver = new ResourceFieldResolver($iriConverterProphecy->reveal());
37-
$this->assertEquals('/dummies/1', $resolver([ItemNormalizer::ITEM_KEY => serialize($dummy)], [], [], $resolveInfo));
35+
$this->assertEquals('/dummies/1', $resolver([ItemNormalizer::ITEM_RESOURCE_CLASS_KEY => Dummy::class, ItemNormalizer::ITEM_IDENTIFIERS_KEY => ['id' => 1]], [], [], $resolveInfo));
3836
}
3937

4038
public function testOriginalId()

‎tests/GraphQl/Serializer/ItemNormalizerTest.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
namespace ApiPlatform\Core\Tests\GraphQl\Serializer;
1515

16+
use ApiPlatform\Core\Api\IdentifiersExtractorInterface;
1617
use ApiPlatform\Core\Api\IriConverterInterface;
1718
use ApiPlatform\Core\Api\ResourceClassResolverInterface;
1819
use ApiPlatform\Core\GraphQl\Serializer\ItemNormalizer;
@@ -45,6 +46,7 @@ public function testSupportNormalization()
4546
$propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
4647
$propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
4748
$iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
49+
$identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class);
4850

4951
$resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
5052
$resourceClassResolverProphecy->isResourceClass(Dummy::class)->willReturn(true)->shouldBeCalled();
@@ -54,6 +56,7 @@ public function testSupportNormalization()
5456
$propertyNameCollectionFactoryProphecy->reveal(),
5557
$propertyMetadataFactoryProphecy->reveal(),
5658
$iriConverterProphecy->reveal(),
59+
$identifiersExtractorProphecy->reveal(),
5760
$resourceClassResolverProphecy->reveal()
5861
);
5962

@@ -82,6 +85,9 @@ public function testNormalize()
8285
$iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
8386
$iriConverterProphecy->getIriFromItem($dummy)->willReturn('/dummies/1')->shouldBeCalled();
8487

88+
$identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class);
89+
$identifiersExtractorProphecy->getIdentifiersFromItem($dummy)->willReturn(['id' => 1])->shouldBeCalled();
90+
8591
$resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
8692
$resourceClassResolverProphecy->getResourceClass($dummy, null, true)->willReturn(Dummy::class)->shouldBeCalled();
8793

@@ -93,6 +99,7 @@ public function testNormalize()
9399
$propertyNameCollectionFactoryProphecy->reveal(),
94100
$propertyMetadataFactoryProphecy->reveal(),
95101
$iriConverterProphecy->reveal(),
102+
$identifiersExtractorProphecy->reveal(),
96103
$resourceClassResolverProphecy->reveal(),
97104
null,
98105
null,
@@ -106,7 +113,7 @@ public function testNormalize()
106113
);
107114
$normalizer->setSerializer($serializerProphecy->reveal());
108115

109-
$this->assertEquals(['name' => 'hello', ItemNormalizer::ITEM_KEY => serialize($dummy)], $normalizer->normalize($dummy, ItemNormalizer::FORMAT, ['resources' => []]));
116+
$this->assertEquals(['name' => 'hello', ItemNormalizer::ITEM_RESOURCE_CLASS_KEY => Dummy::class, ItemNormalizer::ITEM_IDENTIFIERS_KEY => ['id' => 1]], $normalizer->normalize($dummy, ItemNormalizer::FORMAT, ['resources' => []]));
110117
}
111118

112119
public function testDenormalize()
@@ -123,6 +130,8 @@ public function testDenormalize()
123130

124131
$iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
125132

133+
$identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class);
134+
126135
$resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
127136

128137
$serializerProphecy = $this->prophesize(SerializerInterface::class);
@@ -132,6 +141,7 @@ public function testDenormalize()
132141
$propertyNameCollectionFactoryProphecy->reveal(),
133142
$propertyMetadataFactoryProphecy->reveal(),
134143
$iriConverterProphecy->reveal(),
144+
$identifiersExtractorProphecy->reveal(),
135145
$resourceClassResolverProphecy->reveal(),
136146
null,
137147
null,

0 commit comments

Comments
 (0)