diff --git a/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php b/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php index fd7abf9d70..0c4a9e6345 100644 --- a/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php +++ b/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php @@ -177,6 +177,9 @@ private function addGraphQlOperations(?array $data, ApiResource $resource): ApiR } [$key, $operation] = $this->getOperationWithDefaults($resource, $operation); + if (\array_key_exists($key, $operations)) { + throw new RuntimeException(\sprintf('GraphQL operation name "%s" is declared twice on resource "%s". GraphQL operation names must be unique, set distinct names.', $key, $resource->getClass() ?? '')); + } $operations[$key] = $operation; } diff --git a/src/Metadata/Resource/Factory/MetadataCollectionFactoryTrait.php b/src/Metadata/Resource/Factory/MetadataCollectionFactoryTrait.php index e4ca44459e..dc2c144f8a 100644 --- a/src/Metadata/Resource/Factory/MetadataCollectionFactoryTrait.php +++ b/src/Metadata/Resource/Factory/MetadataCollectionFactoryTrait.php @@ -117,6 +117,7 @@ private function buildResourceOperations(array $metadataCollection, string $reso } [$key, $operation] = $this->getOperationWithDefaults($resources[$index], $metadata); $graphQlOperations = $resources[$index]->getGraphQlOperations(); + $this->assertGraphQlOperationNameIsUnique($graphQlOperations ?? [], $key, $resourceClass); $graphQlOperations[$key] = $operation; $resources[$index] = $resources[$index]->withGraphQlOperations($graphQlOperations); continue; @@ -205,6 +206,7 @@ private function buildResourceOperations(array $metadataCollection, string $reso $operation = $this->mergeOperationParameters($operation, $parameters); } + $this->assertGraphQlOperationNameIsUnique($graphQlOperationsWithDefaults, $key, $resourceClass); $graphQlOperationsWithDefaults[$key] = $operation; } @@ -270,6 +272,20 @@ private function deduplicateShortNames(array $resources): array return $resources; } + /** + * GraphQL operations are indexed by name, a duplicate would silently replace the first one. + * + * @param array $operations + */ + private function assertGraphQlOperationNameIsUnique(array $operations, string $key, string $resourceClass): void + { + if (!\array_key_exists($key, $operations)) { + return; + } + + throw new RuntimeException(\sprintf('GraphQL operation name "%s" is declared twice on resource "%s". GraphQL operation names must be unique, set distinct names.', $key, $resourceClass)); + } + /** * Two operations sharing an explicit name silently overwrite each other because * the operation name is the unique key (and the Symfony route name). diff --git a/src/Metadata/Tests/Extractor/XmlExtractorTest.php b/src/Metadata/Tests/Extractor/XmlExtractorTest.php index 1c1e8545de..c9c3e9ee6e 100644 --- a/src/Metadata/Tests/Extractor/XmlExtractorTest.php +++ b/src/Metadata/Tests/Extractor/XmlExtractorTest.php @@ -493,6 +493,17 @@ public function testDuplicateOperationNameFromXmlThrows(): void $factory->create(Comment::class); } + public function testDuplicateGraphQlOperationNameFromXmlThrows(): void + { + $extractor = new XmlResourceExtractor([__DIR__.'/xml/duplicate_graphql_operation_name.xml']); + $factory = new ExtractorResourceMetadataCollectionFactory($extractor, graphQlEnabled: true); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage(\sprintf('GraphQL operation name "item_query" is declared twice on resource "%s".', Comment::class)); + + $factory->create(Comment::class); + } + public function testOpenApiParametersAreAList(): void { $extractor = new XmlResourceExtractor([__DIR__.'/xml/openapi_parameters.xml']); diff --git a/src/Metadata/Tests/Extractor/xml/duplicate_graphql_operation_name.xml b/src/Metadata/Tests/Extractor/xml/duplicate_graphql_operation_name.xml new file mode 100644 index 0000000000..1070d86215 --- /dev/null +++ b/src/Metadata/Tests/Extractor/xml/duplicate_graphql_operation_name.xml @@ -0,0 +1,15 @@ + + + + + + + + + diff --git a/src/Metadata/Tests/Fixtures/ApiResource/SameNameGraphQlOperationAttributes.php b/src/Metadata/Tests/Fixtures/ApiResource/SameNameGraphQlOperationAttributes.php new file mode 100644 index 0000000000..9617e08c9e --- /dev/null +++ b/src/Metadata/Tests/Fixtures/ApiResource/SameNameGraphQlOperationAttributes.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Metadata\Tests\Fixtures\ApiResource; + +use ApiPlatform\Metadata\ApiProperty; +use ApiPlatform\Metadata\GraphQl\Query; + +#[Query(name: 'item_query', security: 'is_granted("ROLE_ADMIN")')] +#[Query(name: 'item_query')] +class SameNameGraphQlOperationAttributes +{ + #[ApiProperty(identifier: true)] + public ?string $id = null; +} diff --git a/src/Metadata/Tests/Fixtures/ApiResource/SameNameGraphQlOperations.php b/src/Metadata/Tests/Fixtures/ApiResource/SameNameGraphQlOperations.php new file mode 100644 index 0000000000..1b28e250cf --- /dev/null +++ b/src/Metadata/Tests/Fixtures/ApiResource/SameNameGraphQlOperations.php @@ -0,0 +1,30 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Metadata\Tests\Fixtures\ApiResource; + +use ApiPlatform\Metadata\ApiProperty; +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\GraphQl\Query; + +#[ApiResource( + graphQlOperations: [ + new Query(name: 'item_query', security: 'is_granted("ROLE_ADMIN")'), + new Query(name: 'item_query'), + ], +)] +class SameNameGraphQlOperations +{ + #[ApiProperty(identifier: true)] + public ?string $id = null; +} diff --git a/src/Metadata/Tests/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php b/src/Metadata/Tests/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php index 1a995b3736..b1edcdb40e 100644 --- a/src/Metadata/Tests/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php +++ b/src/Metadata/Tests/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php @@ -42,6 +42,8 @@ use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\ResourceClassPropagation; use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\ResourceClassPropagationTarget; use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\SameNameDifferentMethodOperations; +use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\SameNameGraphQlOperationAttributes; +use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\SameNameGraphQlOperations; use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\WithParameter; use ApiPlatform\Metadata\Tests\Fixtures\State\AttributeResourceProcessor; use ApiPlatform\Metadata\Tests\Fixtures\State\AttributeResourceProvider; @@ -440,4 +442,24 @@ public function testDuplicateOperationNameThrows(): void $factory->create(SameNameDifferentMethodOperations::class); } + + public function testDuplicateGraphQlOperationNameThrows(): void + { + $factory = new AttributesResourceMetadataCollectionFactory(graphQlEnabled: true); + + $this->expectException(\ApiPlatform\Metadata\Exception\RuntimeException::class); + $this->expectExceptionMessage(\sprintf('GraphQL operation name "item_query" is declared twice on resource "%s".', SameNameGraphQlOperations::class)); + + $factory->create(SameNameGraphQlOperations::class); + } + + public function testDuplicateGraphQlOperationNameOnAttributesThrows(): void + { + $factory = new AttributesResourceMetadataCollectionFactory(graphQlEnabled: true); + + $this->expectException(\ApiPlatform\Metadata\Exception\RuntimeException::class); + $this->expectExceptionMessage(\sprintf('GraphQL operation name "item_query" is declared twice on resource "%s".', SameNameGraphQlOperationAttributes::class)); + + $factory->create(SameNameGraphQlOperationAttributes::class); + } }