From d3484b0f1bf06e518c83cd15e67ed10e9a75fe03 Mon Sep 17 00:00:00 2001 From: Antoine Bluchet Date: Fri, 29 Dec 2023 19:05:17 +0100 Subject: [PATCH] fix(serializer): integrate root_resource_class to cache key (#6073) * fix: operation cache key without operation name on related resources (de)normalization * test: cache key --------- Co-authored-by: Marius J --- src/Serializer/AbstractItemNormalizer.php | 9 ++- .../Serializer/AbstractItemNormalizerTest.php | 78 +++++++++++++++++++ 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php index c24fc563a4a..d5669e49eeb 100644 --- a/src/Serializer/AbstractItemNormalizer.php +++ b/src/Serializer/AbstractItemNormalizer.php @@ -580,9 +580,10 @@ protected function getFactoryOptions(array $context): array $options['serializer_groups'] = (array) $context[self::GROUPS]; } - $operationCacheKey = ($context['resource_class'] ?? '').($context['operation_name'] ?? '').($context['api_normalize'] ?? ''); - if ($operationCacheKey && isset($this->localFactoryOptionsCache[$operationCacheKey])) { - return $options + $this->localFactoryOptionsCache[$operationCacheKey]; + $operationCacheKey = ($context['resource_class'] ?? '').($context['operation_name'] ?? '').($context['root_operation_name'] ?? ''); + $suffix = ($context['api_normalize'] ?? '') ? 'n' : ''; + if ($operationCacheKey && isset($this->localFactoryOptionsCache[$operationCacheKey.$suffix])) { + return $options + $this->localFactoryOptionsCache[$operationCacheKey.$suffix]; } // This is a hot spot @@ -595,7 +596,7 @@ protected function getFactoryOptions(array $context): array } } - return $options + $this->localFactoryOptionsCache[$operationCacheKey] = $options; + return $options + $this->localFactoryOptionsCache[$operationCacheKey.$suffix] = $options; } /** diff --git a/tests/Serializer/AbstractItemNormalizerTest.php b/tests/Serializer/AbstractItemNormalizerTest.php index 66f074fdfec..4c82141317e 100644 --- a/tests/Serializer/AbstractItemNormalizerTest.php +++ b/tests/Serializer/AbstractItemNormalizerTest.php @@ -1399,6 +1399,84 @@ public function testDenormalizeObjectWithNullDisabledTypeEnforcement(): void $this->assertInstanceOf(DtoWithNullValue::class, $actual); $this->assertEquals(new DtoWithNullValue(), $actual); } + + public function testCacheKey(): void + { + $relatedDummy = new RelatedDummy(); + + $dummy = new Dummy(); + $dummy->setName('foo'); + $dummy->setAlias('ignored'); + $dummy->setRelatedDummy($relatedDummy); + $dummy->relatedDummies->add(new RelatedDummy()); + + $relatedDummies = new ArrayCollection([$relatedDummy]); + + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + $propertyNameCollectionFactoryProphecy->create(Dummy::class, Argument::type('array'))->willReturn(new PropertyNameCollection(['name', 'alias', 'relatedDummy', 'relatedDummies'])); + + $relatedDummyType = new Type(Type::BUILTIN_TYPE_OBJECT, false, RelatedDummy::class); + $relatedDummiesType = new Type(Type::BUILTIN_TYPE_OBJECT, false, ArrayCollection::class, true, new Type(Type::BUILTIN_TYPE_INT), $relatedDummyType); + + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'name', Argument::type('array'))->willReturn((new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_STRING)])->withDescription('')->withReadable(true)); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'alias', Argument::type('array'))->willReturn((new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_STRING)])->withDescription('')->withReadable(true)); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummy', Argument::type('array'))->willReturn((new ApiProperty())->withBuiltinTypes([$relatedDummyType])->withDescription('')->withReadable(true)->withWritable(false)->withReadableLink(false)); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummies', Argument::type('array'))->willReturn((new ApiProperty())->withBuiltinTypes([$relatedDummiesType])->withReadable(true)->withWritable(false)->withReadableLink(false)); + + $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + $iriConverterProphecy->getIriFromResource($dummy, Argument::cetera())->willReturn('/dummies/1'); + $iriConverterProphecy->getIriFromResource($relatedDummy, Argument::cetera())->willReturn('/dummies/2'); + + $propertyAccessorProphecy = $this->prophesize(PropertyAccessorInterface::class); + $propertyAccessorProphecy->getValue($dummy, 'name')->willReturn('foo'); + $propertyAccessorProphecy->getValue($dummy, 'relatedDummy')->willReturn($relatedDummy); + $propertyAccessorProphecy->getValue($dummy, 'relatedDummies')->willReturn($relatedDummies); + + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + $resourceClassResolverProphecy->getResourceClass(null, Dummy::class)->willReturn(Dummy::class); + $resourceClassResolverProphecy->getResourceClass($dummy, null)->willReturn(Dummy::class); + $resourceClassResolverProphecy->getResourceClass($relatedDummy, RelatedDummy::class)->willReturn(RelatedDummy::class); + $resourceClassResolverProphecy->getResourceClass($relatedDummies, RelatedDummy::class)->willReturn(RelatedDummy::class); + $resourceClassResolverProphecy->isResourceClass(Dummy::class)->willReturn(true); + $resourceClassResolverProphecy->isResourceClass(RelatedDummy::class)->willReturn(true); + + $serializerProphecy = $this->prophesize(SerializerInterface::class); + $serializerProphecy->willImplement(NormalizerInterface::class); + $serializerProphecy->normalize('foo', null, Argument::type('array'))->willReturn('foo'); + $serializerProphecy->normalize(['/dummies/2'], null, Argument::type('array'))->willReturn(['/dummies/2']); + + $normalizer = $this->getMockForAbstractClass(AbstractItemNormalizer::class, [ + $propertyNameCollectionFactoryProphecy->reveal(), + $propertyMetadataFactoryProphecy->reveal(), + $iriConverterProphecy->reveal(), + $resourceClassResolverProphecy->reveal(), + $propertyAccessorProphecy->reveal(), + null, + null, + [], + null, + null, + ]); + $normalizer->setSerializer($serializerProphecy->reveal()); + + $expected = [ + 'name' => 'foo', + 'relatedDummy' => '/dummies/2', + 'relatedDummies' => ['/dummies/2'], + ]; + $this->assertSame($expected, $normalizer->normalize($dummy, null, [ + 'resources' => [], + 'groups' => ['group'], + 'ignored_attributes' => ['alias'], + 'operation_name' => 'operation_name', + 'root_operation_name' => 'root_operation_name', + ])); + + $operationCacheKey = (new \ReflectionClass($normalizer))->getProperty('localFactoryOptionsCache')->getValue($normalizer); + $this->assertEquals(array_keys($operationCacheKey), [sprintf('%s%s%s%s', Dummy::class, 'operation_name', 'root_operation_name', 'n')]); + $this->assertEquals(current($operationCacheKey), ['serializer_groups' => ['group']]); + } } class ObjectWithBasicProperties