api-platform / api-platform/core
IriConverter: localOperationCache is always written but never read
- 主要语言
- PHP
- 星标
- 2.6k
- 派生
- 980
- 平均合并
- 2 天 4 小时
- 30 天内合并 PR
- 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