refactor(serializer): move isCacheKeySafe to AbstractItemNormalizer

src/GraphQl/Serializer/ItemNormalizer.php

@@ -24,7 +24,6 @@
 use ApiPlatform\Metadata\ResourceAccessCheckerInterface;
 use ApiPlatform\Metadata\ResourceClassResolverInterface;
 use ApiPlatform\Metadata\Util\ClassInfoTrait;
-use ApiPlatform\Serializer\CacheKeyTrait;
 use ApiPlatform\Serializer\ItemNormalizer as BaseItemNormalizer;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
@@ -40,15 +39,12 @@
  */
 final class ItemNormalizer extends BaseItemNormalizer
 {
-    use CacheKeyTrait;
     use ClassInfoTrait;
 
     public const FORMAT = 'graphql';
     public const ITEM_RESOURCE_CLASS_KEY = '#itemResourceClass';
     public const ITEM_IDENTIFIERS_KEY = '#itemIdentifiers';
 
-    private array $safeCacheKeysCache = [];
-
     public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, IriConverterInterface $iriConverter, private readonly IdentifiersExtractorInterface $identifiersExtractor, ResourceClassResolverInterface $resourceClassResolver, ?PropertyAccessorInterface $propertyAccessor = null, ?NameConverterInterface $nameConverter = null, ?ClassMetadataFactoryInterface $classMetadataFactory = null, ?LoggerInterface $logger = null, ?ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory = null, ?ResourceAccessCheckerInterface $resourceAccessChecker = null)
     {
         parent::__construct($propertyNameCollectionFactory, $propertyMetadataFactory, $iriConverter, $resourceClassResolver, $propertyAccessor, $nameConverter, $classMetadataFactory, $logger ?: new NullLogger(), $resourceMetadataCollectionFactory, $resourceAccessChecker);
@@ -90,12 +86,6 @@ public function normalize(mixed $data, ?string $format = null, array $context =
             return parent::normalize($data, $format, $context);
         }
 
-        if ($this->isCacheKeySafe($context)) {
-            $context['cache_key'] = $this->getCacheKey($format, $context);
-        } else {
-            $context['cache_key'] = false;
-        }
-
         unset($context['operation_name'], $context['operation']); // Remove operation and operation_name only when cache key has been created
         $normalizedData = parent::normalize($data, $format, $context);
         if (!\is_array($normalizedData)) {
@@ -161,32 +151,4 @@ protected function setAttributeValue(object $object, string $attribute, mixed $v
 
         parent::setAttributeValue($object, $attribute, $value, $format, $context);
     }
-
-    /**
-     * Check if any property contains a security grants, which makes the cache key not safe,
-     * as allowed_properties can differ for 2 instances of the same object.
-     */
-    private function isCacheKeySafe(array $context): bool
-    {
-        if (!isset($context['resource_class']) || !$this->resourceClassResolver->isResourceClass($context['resource_class'])) {
-            return false;
-        }
-        $resourceClass = $this->resourceClassResolver->getResourceClass(null, $context['resource_class']);
-        if (isset($this->safeCacheKeysCache[$resourceClass])) {
-            return $this->safeCacheKeysCache[$resourceClass];
-        }
-        $options = $this->getFactoryOptions($context);
-        $propertyNames = $this->propertyNameCollectionFactory->create($resourceClass, $options);
-
-        $this->safeCacheKeysCache[$resourceClass] = true;
-        foreach ($propertyNames as $propertyName) {
-            $propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $propertyName, $options);
-            if (null !== $propertyMetadata->getSecurity()) {
-                $this->safeCacheKeysCache[$resourceClass] = false;
-                break;
-            }
-        }
-
-        return $this->safeCacheKeysCache[$resourceClass];
-    }
 }

src/GraphQl/composer.json

@@ -23,7 +23,7 @@
         "php": ">=8.2",
         "api-platform/metadata": "^4.3",
         "api-platform/state": "^4.3",
-        "api-platform/serializer": "^4.3",
+        "api-platform/serializer": "^4.3.7",
         "symfony/property-info": "^7.1 || ^8.0",
         "symfony/serializer": "^6.4 || ^7.1 || ^8.0",
         "symfony/type-info": "^7.3 || ^8.0",

src/Hal/Serializer/ItemNormalizer.php

@@ -23,7 +23,6 @@
 use ApiPlatform\Metadata\Util\ClassInfoTrait;
 use ApiPlatform\Metadata\Util\TypeHelper;
 use ApiPlatform\Serializer\AbstractItemNormalizer;
-use ApiPlatform\Serializer\CacheKeyTrait;
 use ApiPlatform\Serializer\ContextTrait;
 use ApiPlatform\Serializer\OperationResourceClassResolverInterface;
 use ApiPlatform\Serializer\TagCollectorInterface;
@@ -49,7 +48,6 @@
  */
 final class ItemNormalizer extends AbstractItemNormalizer
 {
-    use CacheKeyTrait;
     use ClassInfoTrait;
     use ContextTrait;
 
@@ -113,7 +111,7 @@ public function normalize(mixed $data, ?string $format = null, array $context =
         $context['api_normalize'] = true;
 
         if (!isset($context['cache_key'])) {
-            $context['cache_key'] = $this->getCacheKey($format, $context);
+            $context['cache_key'] = $this->isCacheKeySafe($context) ? $this->getCacheKey($format, $context) : false;
         }
 
         $normalizedData = parent::normalize($data, $format, $context);

src/Hal/Tests/Serializer/ItemNormalizerTest.php

@@ -170,6 +170,52 @@ public function testNormalize(): void
         $this->assertEquals($expected, $normalizer->normalize($dummy));
     }
 
+    public function testCacheKeyIsFalseWhenAPropertyHasSecurity(): void
+    {
+        $dummy = new Dummy();
+        $dummy->setName('hello');
+
+        $propertyNameCollection = new PropertyNameCollection(['name']);
+        $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
+        $propertyNameCollectionFactoryProphecy->create(Dummy::class, Argument::type('array'))->willReturn($propertyNameCollection);
+
+        $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
+        $propertyMetadataFactoryProphecy->create(Dummy::class, 'name', Argument::type('array'))->willReturn(
+            (new ApiProperty())->withNativeType(Type::string())->withDescription('')->withReadable(true)->withSecurity('is_granted(\'ROLE_ADMIN\')')
+        );
+
+        $iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
+        $iriConverterProphecy->getIriFromResource($dummy, Argument::cetera())->willReturn('/dummies/1');
+
+        $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
+        $resourceClassResolverProphecy->isResourceClass(Dummy::class)->willReturn(true);
+        $resourceClassResolverProphecy->getResourceClass($dummy, null)->willReturn(Dummy::class);
+        $resourceClassResolverProphecy->getResourceClass(null, Dummy::class)->willReturn(Dummy::class);
+        $resourceClassResolverProphecy->getResourceClass($dummy, Dummy::class)->willReturn(Dummy::class);
+
+        $serializerProphecy = $this->prophesize(SerializerInterface::class);
+        $serializerProphecy->willImplement(NormalizerInterface::class);
+        $serializerProphecy->normalize('hello', null, Argument::type('array'))->willReturn('hello');
+
+        $nameConverter = $this->prophesize(NameConverterInterface::class);
+        $nameConverter->normalize('name', Argument::any(), Argument::any(), Argument::any())->willReturn('name');
+
+        $normalizer = new ItemNormalizer(
+            $propertyNameCollectionFactoryProphecy->reveal(),
+            $propertyMetadataFactoryProphecy->reveal(),
+            $iriConverterProphecy->reveal(),
+            $resourceClassResolverProphecy->reveal(),
+            null,
+            $nameConverter->reveal()
+        );
+        $normalizer->setSerializer($serializerProphecy->reveal());
+
+        $normalizer->normalize($dummy);
+
+        $componentsCacheRef = new \ReflectionProperty(ItemNormalizer::class, 'componentsCache');
+        $this->assertSame([], $componentsCacheRef->getValue($normalizer), 'componentsCache must not be populated when a property has security set');
+    }
+
     public function testNormalizeWithUnionIntersectTypes(): void
     {
         $author = new Author(id: 2, name: 'Isaac Asimov');

src/Hal/composer.json

@@ -25,7 +25,7 @@
         "api-platform/state": "^4.3",
         "api-platform/metadata": "^4.3",
         "api-platform/documentation": "^4.3",
-        "api-platform/serializer": "^4.3",
+        "api-platform/serializer": "^4.3.7",
         "symfony/type-info": "^7.3 || ^8.0"
     },
     "autoload": {

src/JsonApi/Serializer/ItemNormalizer.php

@@ -28,7 +28,6 @@
 use ApiPlatform\Metadata\Util\CompositeIdentifierParser;
 use ApiPlatform\Metadata\Util\TypeHelper;
 use ApiPlatform\Serializer\AbstractItemNormalizer;
-use ApiPlatform\Serializer\CacheKeyTrait;
 use ApiPlatform\Serializer\ContextTrait;
 use ApiPlatform\Serializer\OperationResourceClassResolverInterface;
 use ApiPlatform\Serializer\TagCollectorInterface;
@@ -55,7 +54,6 @@
  */
 final class ItemNormalizer extends AbstractItemNormalizer
 {
-    use CacheKeyTrait;
     use ClassInfoTrait;
     use ContextTrait;
 
@@ -127,7 +125,7 @@ public function normalize(mixed $data, ?string $format = null, array $context =
         $context['api_normalize'] = true;
 
         if (!isset($context['cache_key'])) {
-            $context['cache_key'] = $this->getCacheKey($format, $context);
+            $context['cache_key'] = $this->isCacheKeySafe($context) ? $this->getCacheKey($format, $context) : false;
         }
 
         $normalizedData = parent::normalize($data, $format, $context);

src/JsonApi/Tests/Serializer/ItemNormalizerTest.php

@@ -126,6 +126,62 @@ public function testNormalize(): void
         $this->assertEquals($expected, $normalizer->normalize($dummy, ItemNormalizer::FORMAT));
     }
 
+    public function testCacheKeyIsFalseWhenAPropertyHasSecurity(): void
+    {
+        $dummy = new Dummy();
+        $dummy->setId(11);
+        $dummy->setName('hello');
+
+        $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
+        $propertyNameCollectionFactoryProphecy->create(Dummy::class, Argument::type('array'))->willReturn(new PropertyNameCollection(['id', 'name']));
+
+        $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
+        $propertyMetadataFactoryProphecy->create(Dummy::class, 'id', Argument::type('array'))->willReturn((new ApiProperty())->withReadable(true)->withIdentifier(true));
+        $propertyMetadataFactoryProphecy->create(Dummy::class, 'name', Argument::type('array'))->willReturn((new ApiProperty())->withReadable(true)->withSecurity('is_granted(\'ROLE_ADMIN\')'));
+
+        $iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
+        $iriConverterProphecy->getIriFromResource($dummy, Argument::cetera())->willReturn('/dummies/11');
+
+        $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
+        $resourceClassResolverProphecy->getResourceClass($dummy, null)->willReturn(Dummy::class);
+        $resourceClassResolverProphecy->getResourceClass(null, Dummy::class)->willReturn(Dummy::class);
+        $resourceClassResolverProphecy->getResourceClass($dummy, Dummy::class)->willReturn(Dummy::class);
+        $resourceClassResolverProphecy->isResourceClass(Dummy::class)->willReturn(true);
+
+        $propertyAccessorProphecy = $this->prophesize(PropertyAccessorInterface::class);
+        $propertyAccessorProphecy->getValue($dummy, 'id')->willReturn(11);
+        $propertyAccessorProphecy->getValue($dummy, 'name')->willReturn('hello');
+
+        $resourceMetadataCollectionFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class);
+        $resourceMetadataCollectionFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadataCollection('Dummy', [
+            (new ApiResource())
+                ->withShortName('Dummy')
+                ->withOperations(new Operations(['get' => (new Get())->withShortName('Dummy')])),
+        ]));
+
+        $serializerProphecy = $this->prophesize(SerializerInterface::class);
+        $serializerProphecy->willImplement(NormalizerInterface::class);
+        $serializerProphecy->normalize(Argument::any(), ItemNormalizer::FORMAT, Argument::type('array'))->will(static fn ($args) => $args[0]);
+
+        $normalizer = new ItemNormalizer(
+            $propertyNameCollectionFactoryProphecy->reveal(),
+            $propertyMetadataFactoryProphecy->reveal(),
+            $iriConverterProphecy->reveal(),
+            $resourceClassResolverProphecy->reveal(),
+            $propertyAccessorProphecy->reveal(),
+            new ReservedAttributeNameConverter(),
+            null,
+            [],
+            $resourceMetadataCollectionFactoryProphecy->reveal(),
+        );
+        $normalizer->setSerializer($serializerProphecy->reveal());
+
+        $normalizer->normalize($dummy, ItemNormalizer::FORMAT);
+
+        $componentsCacheRef = new \ReflectionProperty(ItemNormalizer::class, 'componentsCache');
+        $this->assertSame([], $componentsCacheRef->getValue($normalizer), 'componentsCache must not be populated when a property has security set');
+    }
+
     public function testNormalizeCircularReference(): void
     {
         $circularReferenceEntity = new CircularReference();
@@ -144,7 +200,7 @@ public function testNormalizeCircularReference(): void
         $resourceMetadataCollectionFactoryProphecy->create(CircularReference::class)->willReturn(new ResourceMetadataCollection('CircularReference'));
 
         $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
-        $propertyNameCollectionFactoryProphecy->create(CircularReference::class, [])->willReturn(new PropertyNameCollection());
+        $propertyNameCollectionFactoryProphecy->create(CircularReference::class, Argument::type('array'))->willReturn(new PropertyNameCollection());
 
         $normalizer = new ItemNormalizer(
             $propertyNameCollectionFactoryProphecy->reveal(),

src/JsonApi/composer.json

@@ -25,7 +25,7 @@
         "api-platform/documentation": "^4.3",
         "api-platform/json-schema": "^4.3",
         "api-platform/metadata": "^4.3",
-        "api-platform/serializer": "^4.3",
+        "api-platform/serializer": "^4.3.7",
         "api-platform/state": "^4.3",
         "symfony/error-handler": "^6.4 || ^7.0 || ^8.0",
         "symfony/http-foundation": "^6.4.14 || ^7.0 || ^8.0",

src/Serializer/AbstractItemNormalizer.php

@@ -63,6 +63,7 @@
  */
 abstract class AbstractItemNormalizer extends AbstractObjectNormalizer
 {
+    use CacheKeyTrait;
     use ClassInfoTrait;
     use CloneTrait;
     use ContextTrait;
@@ -77,6 +78,7 @@ abstract class AbstractItemNormalizer extends AbstractObjectNormalizer
     protected PropertyAccessorInterface $propertyAccessor;
     protected array $localCache = [];
     protected array $localFactoryOptionsCache = [];
+    protected array $safeCacheKeysCache = [];
     protected ?ResourceAccessCheckerInterface $resourceAccessChecker;
 
     public function __construct(protected PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, protected PropertyMetadataFactoryInterface $propertyMetadataFactory, protected IriConverterInterface $iriConverter, protected ResourceClassResolverInterface $resourceClassResolver, ?PropertyAccessorInterface $propertyAccessor = null, ?NameConverterInterface $nameConverter = null, ?ClassMetadataFactoryInterface $classMetadataFactory = null, array $defaultContext = [], ?ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory = null, ?ResourceAccessCheckerInterface $resourceAccessChecker = null, protected ?TagCollectorInterface $tagCollector = null, protected ?OperationResourceClassResolverInterface $operationResourceResolver = null)
@@ -191,6 +193,10 @@ public function normalize(mixed $data, ?string $format = null, array $context =
             $context['resources'][$iri] = $iri;
         }
 
+        if (!isset($context['cache_key'])) {
+            $context['cache_key'] = $this->isCacheKeySafe($context) ? $this->getCacheKey($format, $context) : false;
+        }
+
         $context['object'] = $data;
         $context['format'] = $format;
 
@@ -525,6 +531,36 @@ protected function canAccessAttribute(?object $object, string $attribute, array
         return true;
     }
 
+    /**
+     * Check if any property contains a security grant, which makes the cache key not safe,
+     * as allowed_properties can differ for two instances of the same object.
+     */
+    protected function isCacheKeySafe(array $context): bool
+    {
+        if (!isset($context['resource_class']) || !$this->resourceClassResolver->isResourceClass($context['resource_class'])) {
+            return false;
+        }
+
+        $resourceClass = $this->resourceClassResolver->getResourceClass(null, $context['resource_class']);
+        if (isset($this->safeCacheKeysCache[$resourceClass])) {
+            return $this->safeCacheKeysCache[$resourceClass];
+        }
+
+        $options = $this->getFactoryOptions($context);
+        $propertyNames = $this->propertyNameCollectionFactory->create($resourceClass, $options);
+
+        $this->safeCacheKeysCache[$resourceClass] = true;
+        foreach ($propertyNames as $propertyName) {
+            $propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $propertyName, $options);
+            if (null !== $propertyMetadata->getSecurity()) {
+                $this->safeCacheKeysCache[$resourceClass] = false;
+                break;
+            }
+        }
+
+        return $this->safeCacheKeysCache[$resourceClass];
+    }
+
     /**
      * Check if access to the attribute is granted.
      */

src/Serializer/CacheKeyTrait.php

@@ -21,7 +21,7 @@
  */
 trait CacheKeyTrait
 {
-    private function getCacheKey(?string $format, array $context): string|bool
+    protected function getCacheKey(?string $format, array $context): string|bool
     {
         foreach ($context[self::EXCLUDE_FROM_CACHE_KEY] ?? $this->defaultContext[self::EXCLUDE_FROM_CACHE_KEY] as $key) {
             unset($context[$key]);