Skip to content

fix(metadata): getOperation cache matches arguments #5215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 23, 2022
Merged

fix(metadata): getOperation cache matches arguments #5215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Metadata/Operations.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public function __construct(array $operations = [])
{
foreach ($operations as $operationName => $operation) {
// When we use an int-indexed array in the constructor, compute priorities
if (\is_int($operationName)) {
if (\is_int($operationName) && null === $operation->getPriority()) {
$operation = $operation->withPriority($operationName);
$operationName = (string) $operationName;
}
Expand Down
24 changes: 15 additions & 9 deletions src/Metadata/Resource/ResourceMetadataCollection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ final class ResourceMetadataCollection extends \ArrayObject
{
private const GRAPHQL_PREFIX = 'g_';
private const HTTP_PREFIX = 'h_';
private const FORCE_COLLECTION = 'co_';
private const HTTP_OPERATION = 'ht_';

private array $operationCache = [];

public function __construct(private readonly string $resourceClass, array $input = [])
Expand All @@ -36,12 +39,15 @@ public function __construct(private readonly string $resourceClass, array $input
public function getOperation(?string $operationName = null, bool $forceCollection = false, bool $httpOperation = false): Operation
{
$operationName ??= '';
if (isset($this->operationCache[self::HTTP_PREFIX.$operationName])) {
return $this->operationCache[self::HTTP_PREFIX.$operationName];
$cachePrefix = ($forceCollection ? self::FORCE_COLLECTION : '').($httpOperation ? self::HTTP_OPERATION : '');
$httpCacheKey = self::HTTP_PREFIX.$cachePrefix.$operationName;
if (isset($this->operationCache[$httpCacheKey])) {
return $this->operationCache[$httpCacheKey];
}

if (isset($this->operationCache[self::GRAPHQL_PREFIX.$operationName])) {
return $this->operationCache[self::GRAPHQL_PREFIX.$operationName];
$gqlCacheKey = self::GRAPHQL_PREFIX.$cachePrefix.$operationName;
if (isset($this->operationCache[$gqlCacheKey])) {
return $this->operationCache[$gqlCacheKey];
}

$it = $this->getIterator();
Expand All @@ -56,26 +62,26 @@ public function getOperation(?string $operationName = null, bool $forceCollectio
$method = $operation->getMethod() ?? HttpOperation::METHOD_GET;
$isGetOperation = HttpOperation::METHOD_GET === $method || HttpOperation::METHOD_OPTIONS === $method || HttpOperation::METHOD_HEAD === $method;
if ('' === $operationName && $isGetOperation && ($forceCollection ? $isCollection : !$isCollection)) {
return $this->operationCache[self::HTTP_PREFIX.$operationName] = $operation;
return $this->operationCache[$httpCacheKey] = $operation;
}

if ($name === $operationName) {
return $this->operationCache[self::HTTP_PREFIX.$operationName] = $operation;
return $this->operationCache[$httpCacheKey] = $operation;
}

if ($operation->getUriTemplate() === $operationName) {
return $this->operationCache[self::HTTP_PREFIX.$operationName] = $operation;
return $this->operationCache[$httpCacheKey] = $operation;
}
}

foreach ($metadata->getGraphQlOperations() ?? [] as $name => $operation) {
$isCollection = $operation instanceof CollectionOperationInterface;
if ('' === $operationName && ($forceCollection ? $isCollection : !$isCollection) && false === $httpOperation) {
return $this->operationCache[self::GRAPHQL_PREFIX.$operationName] = $operation;
return $this->operationCache[$gqlCacheKey] = $operation;
}

if ($name === $operationName) {
return $this->operationCache[self::GRAPHQL_PREFIX.$operationName] = $operation;
return $this->operationCache[$httpCacheKey] = $operation;
}
}

Expand Down
44 changes: 44 additions & 0 deletions tests/Metadata/Resource/ResourceMetadataCollectionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
use ApiPlatform\Exception\OperationNotFoundException;
use ApiPlatform\Metadata\ApiResource;
use ApiPlatform\Metadata\Get;
use ApiPlatform\Metadata\GetCollection;
use ApiPlatform\Metadata\GraphQl\Query;
use ApiPlatform\Metadata\GraphQl\QueryCollection;
use ApiPlatform\Metadata\Operations;
use ApiPlatform\Metadata\Resource\ResourceMetadataCollection;
use PHPUnit\Framework\TestCase;
Expand Down Expand Up @@ -46,4 +48,46 @@ public function testOperationNotFound(): void

$this->assertSame($operation, $resourceMetadataCollection->getOperation('noname'));
}

public function testCache(): void
{
$defaultOperation = new Get(name: 'get');
$defaultGqlOperation = new Query();
$defaultCollectionOperation = new GetCollection(name: 'get_collection');
$defaultGqlCollectionOperation = new QueryCollection();
$resource = new ApiResource(
operations: [
'get' => $defaultOperation,
'get_collection' => $defaultCollectionOperation,
],
graphQlOperations: [
'query' => $defaultGqlOperation,
'query_collection' => $defaultGqlCollectionOperation,
]
);

$resourceMetadataCollection = new ResourceMetadataCollection('class', [$resource]);

$this->assertEquals($resourceMetadataCollection->getOperation(), $defaultOperation);
$this->assertEquals($resourceMetadataCollection->getOperation(null, true), $defaultCollectionOperation);
$this->assertEquals($resourceMetadataCollection->getOperation(null, false, true), $defaultOperation);

$resource = new ApiResource(
graphQlOperations: [
'query' => $defaultGqlOperation,
'query_collection' => $defaultGqlCollectionOperation,
]
);

$resourceMetadataCollection = new ResourceMetadataCollection('class', [$resource]);

$this->assertEquals($resourceMetadataCollection->getOperation(), $defaultGqlOperation);
$this->assertEquals($resourceMetadataCollection->getOperation(null, true), $defaultGqlCollectionOperation);

try {
$resourceMetadataCollection->getOperation(null, false, true);
} catch (\Exception $e) {
$this->assertInstanceOf(OperationNotFoundException::class, $e);
}
}
}