fix(metadata): getOperation cache matches arguments (#5215)

fixes #5204

Author: soyuka

src/Metadata/Operations.php

@@ -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;
             }

src/Metadata/Resource/ResourceMetadataCollection.php

@@ -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 = [])
@@ -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();
@@ -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;
                 }
             }
 

tests/Metadata/Resource/ResourceMetadataCollectionTest.php

@@ -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;
@@ -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);
+        }
+    }
 }