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 <marius@microdon.org>
api-platform · Dec 29, 2023 · d3484b0 · d3484b0
1 parent 3685010
commit d3484b0
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 4 deletions.
diff --git 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
@@ -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