IriConverter: localOperationCache is always written but never read
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 85/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- php, symfony
- Domain
- api, backend, performance
Research direction
Start in src/Symfony/Routing/IriConverter.php, focusing on getIriFromResource() and the local operation-cache lookup around the metadata-factory call. Review IriConverterTest::testGetIriFromItemWithoutOperation, add coverage proving repeated calls reuse the cache, and run the IriConverter test suite to verify the expensive metadata lookup is avoided.
Written by the indexing model from the issue text.
Description
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():121does try to supply
one via$context['operation'] ?? null, but the context no longer has it —
AbstractCollectionNormalizerbuilds the per-item context throughcreateOperationContext(),
which ends atOperationContextTrait.php:53with
unset($context['operation'], $context['operation_name']);. - for related objects:
AbstractItemNormalizer::normalizeRelation():982passes only two named arguments,
so$operationdefaults tonull:$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.
- Dominant language
- PHP
- Stars
- 2.6k
- Forks
- 982
- Avg merge
- 1d 23h
- Merged PRs (30d)
- 42
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from api-platform/core
-
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
api-platform/core#8549 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
api-platform/core#8495 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
api-platform/core#8475 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
api-platform/core#8447 ·
-
Difficulty 1/5 Under an hour Newbie friendliness 88/100
api-platform/core#8312 · 2 comments ·
All issues in api-platform/core
Similar issues
-
priority: p3
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
googleapis/librarian#7636 ·
-
0. Needs triage bug
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
nextcloud/fulltextsearch#1011 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
phpstan/phpstan-doctrine#794 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
Automattic/static-site-importer#1767 ·