api-platform / api-platform/core

IriConverter: localOperationCache is always written but never read

オープン 初心者向け
#8,471 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
PHP
スター
2.6k
フォーク
980
平均マージ
2日 4時間
マージ済み PR(30日)
49

説明

**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`:
```php
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:

```diff
--- 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`:
```php
$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.

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

評価

技術スタック
php, symfony
領域
api, backend, performance
issue の種類
バグ
難易度
2/5
見積もり時間
1〜3時間
活発さ
活発
明瞭さ
明確に書かれている
初心者へのやさしさ
85/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。