Skip to content

IriConverter: localOperationCache is always written but never read #8471

Description

@Johannes-Werbrouck

API Platform version(s) affected: 4.3.3

Description
In a getCollection, the AbstractItemNormalizer will use the IriConverter to find the IRIs for all entities in the collection, including related entities. To find these IRI's, the resource metadata has to be created to find what operation to use to generate the IRI. This resource metadata is cached in the IriConverter, but will only be read if an $operation was specified. The AbstractItemNormalizer does not specify this, so the cache is always written but never read.

This is the relevant method in ApiPlatform\Symfony\Routing\IriConverter:

public function getIriFromResource(object|string $resource, int $referenceType = UrlGeneratorInterface::ABS_PATH, ?Operation $operation = null, array $context = []): string
    {
        $resourceClass = $context['force_resource_class'] ?? (\is_string($resource) ? $resource : $this->getObjectClass($resource));

        if ($this->operationMetadataFactory && isset($context['item_uri_template'])) {
            $operation = $this->operationMetadataFactory->create($context['item_uri_template']);
        }

        $localOperationCacheKey = ($operation?->getName() ?? '').$resourceClass.(\is_string($resource) ? '_s' : '_o').($operation instanceof CollectionOperationInterface ? '_c' : '_i');
        // ❗️This short-circuits on $operation, so if no $operation is present the localOperationCache will never be used
        if ($operation && isset($this->localOperationCache[$localOperationCacheKey])) {
            return $this->generateSymfonyRoute($resource, $referenceType, $this->localOperationCache[$localOperationCacheKey], $context, $this->localIdentifiersExtractorOperationCache[$localOperationCacheKey] ?? null);
        }

        if (!($isResourceClass = $this->resourceClassResolver->isResourceClass($resourceClass)) && !isset($context['item_uri_template'])) {
            return $this->generateSkolemIri($resource, $referenceType, $operation, $context, $resourceClass);
        }

        $context['is_resource_class'] = $isResourceClass;
        $context['current_resource_class'] = $resourceClass;

        // This is only for when a class (that is not a resource) extends another one that is a resource, we should remove this behavior
        if (!\is_string($resource) && !isset($context['force_resource_class']) && !isset($context['item_uri_template'])) {
            $resourceClass = $this->getResourceClass($resource, true);
        }

        if (!$operation) {
            $operation = (new Get())->withClass($resourceClass);
        }

        if ($operation instanceof HttpOperation && 301 === $operation->getStatus()) {
            $operation = ($operation instanceof CollectionOperationInterface ? new GetCollection() : new Get())->withClass($operation->getClass());
            unset($context['uri_variables']);
        }

        $identifiersExtractorOperation = $operation;
        // In symfony the operation name is the route name, try to find one if none provided
        if (
            !$operation->getName()
            || ($operation instanceof HttpOperation && 'POST' === $operation->getMethod())
        ) {
            $forceCollection = $operation instanceof CollectionOperationInterface;
            try {
                // ❗️This is the actual expensive call
                $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null, $forceCollection, true);
                $identifiersExtractorOperation = $operation;
            } catch (OperationNotFoundException) {
            }
        }

        if (!$operation->getName() || ($operation instanceof HttpOperation && $operation->getUriTemplate() && str_starts_with($operation->getUriTemplate(), SkolemIriConverter::$skolemUriTemplate))) {
            return $this->generateSkolemIri($resource, $referenceType, $operation, $context, $resourceClass);
        }

        $this->localOperationCache[$localOperationCacheKey] = $operation;
        $this->localIdentifiersExtractorOperationCache[$localOperationCacheKey] = $identifiersExtractorOperation;

        return $this->generateSymfonyRoute($resource, $referenceType, $operation, $context, $identifiersExtractorOperation);
    }

How to reproduce
IriConverterTest::testGetIriFromItemWithoutOperation touches on the logic that's affected by this bug, but does not assert the cache was used.

Possible Solution
Do not check if $operation is truthy before trying to read the cache:

--- a/src/Symfony/Routing/IriConverter.php
+++ b/src/Symfony/Routing/IriConverter.php
@@ -160,10 +160,15 @@
             !$operation->getName()
             || ($operation instanceof HttpOperation && 'POST' === $operation->getMethod())
         ) { 
-            $forceCollection = $operation instanceof CollectionOperationInterface;
-            try {
-                $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null, $forceCollection, true);
-                $identifiersExtractorOperation = $operation;
-            } catch (OperationNotFoundException) {
+            if (isset($this->localOperationCache[$localOperationCacheKey])) {
+                $operation = $this->localOperationCache[$localOperationCacheKey];
+                $identifiersExtractorOperation = $this->localIdentifiersExtractorOperationCache[$localOperationCacheKey] ?? $operation;
+            } else {
+                $forceCollection = $operation instanceof CollectionOperationInterface;
+                try {
+                    $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null, $forceCollection, true);
+                    $identifiersExtractorOperation = $operation;
+                } catch (OperationNotFoundException) {
+                }   
             }   
         }       

Additional Context
Entry points:

  • for each item's own '@id': JsonLd\Serializer\ItemNormalizer::normalize():121 does try to supply
    one via $context['operation'] ?? null, but the context no longer has it —
    AbstractCollectionNormalizer builds the per-item context through createOperationContext(),
    which ends at OperationContextTrait.php:53 with
    unset($context['operation'], $context['operation_name']);.
  • for related objects: AbstractItemNormalizer::normalizeRelation():982 passes only two named arguments,
    so $operation defaults to null:
    $context['iri'] = $iri = $this->iriConverter->getIriFromResource(resource: $relatedObject, context: $context);

On a real endpoint, 2,337 items with ~6 IRIs each, Xdebug profile, PHP 8.4.20, Symfony 7.4.8, api-platform/core 4.3.3: getIriFromResource() is called 14,362 times, and fell through to $this->resourceMetadataCollectionFactory->create 14,360 times.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions