diff --git a/phpunit.baseline.xml b/phpunit.baseline.xml index ebb80a94bd9..bb10f6ce0f9 100644 --- a/phpunit.baseline.xml +++ b/phpunit.baseline.xml @@ -31,7 +31,7 @@ - + diff --git a/src/JsonSchema/Tests/Fixtures/ApiResource/OverriddenOperationDummy.php b/src/JsonSchema/Tests/Fixtures/ApiResource/OverriddenOperationDummy.php index 9285dcc75fc..8e98c20b3d4 100644 --- a/src/JsonSchema/Tests/Fixtures/ApiResource/OverriddenOperationDummy.php +++ b/src/JsonSchema/Tests/Fixtures/ApiResource/OverriddenOperationDummy.php @@ -28,7 +28,7 @@ * * @author Amrouche Hamza */ -#[ApiResource(operations: [new Get(), new Get(normalizationContext: ['groups' => ['overridden_operation_dummy_get']], denormalizationContext: ['groups' => ['overridden_operation_dummy_get']]), new Put(normalizationContext: ['groups' => ['overridden_operation_dummy_put']], denormalizationContext: ['groups' => ['overridden_operation_dummy_put']]), new Delete(), new GetCollection(), new Post(), new GetCollection(uriTemplate: '/override/swagger')], normalizationContext: ['groups' => ['overridden_operation_dummy_read']], denormalizationContext: ['groups' => ['overridden_operation_dummy_write']])] +#[ApiResource(operations: [new Get(normalizationContext: ['groups' => ['overridden_operation_dummy_get']], denormalizationContext: ['groups' => ['overridden_operation_dummy_get']]), new Put(normalizationContext: ['groups' => ['overridden_operation_dummy_put']], denormalizationContext: ['groups' => ['overridden_operation_dummy_put']]), new Delete(), new GetCollection(), new Post(), new GetCollection(uriTemplate: '/override/swagger')], normalizationContext: ['groups' => ['overridden_operation_dummy_read']], denormalizationContext: ['groups' => ['overridden_operation_dummy_write']])] class OverriddenOperationDummy { /** diff --git a/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php b/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php index 391eca8c491..fd7abf9d705 100644 --- a/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php +++ b/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php @@ -14,6 +14,7 @@ namespace ApiPlatform\Metadata\Resource\Factory; use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Exception\RuntimeException; use ApiPlatform\Metadata\Extractor\ResourceExtractorInterface; use ApiPlatform\Metadata\GraphQl\Operation as GraphQlOperation; use ApiPlatform\Metadata\HttpOperation; @@ -134,6 +135,9 @@ private function addOperations(?array $data, ApiResource $resource): ApiResource } [$key, $operation] = $this->getOperationWithDefaults($resource, $operation); + if (\array_key_exists($key, $operations)) { + throw new RuntimeException(\sprintf('Operation name "%s" is declared twice on resource "%s". Operation names must be unique because they are also used as Symfony route names. Remove the duplicate "name" so the framework can disambiguate by method, or 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 2f45e36f93d..cd74e7776df 100644 --- a/src/Metadata/Resource/Factory/MetadataCollectionFactoryTrait.php +++ b/src/Metadata/Resource/Factory/MetadataCollectionFactoryTrait.php @@ -87,6 +87,7 @@ private function buildResourceOperations(array $metadataCollection, string $reso $operations = []; foreach ($resource->getOperations() ?? new Operations() as $operation) { [$key, $operation] = $this->getOperationWithDefaults($resource, $operation); + $this->assertOperationNameIsUnique($operations, $key, $resourceClass); $operations[$key] = $operation; } if ($operations) { @@ -145,6 +146,7 @@ private function buildResourceOperations(array $metadataCollection, string $reso $operation = $operation->withPriority(++$operationPriority); } $operations = $resources[$index]->getOperations() ?? new Operations(); + $this->assertOperationNameIsUnique(iterator_to_array($operations), $key, $resourceClass); $resources[$index] = $resources[$index]->withOperations($operations->add($key, $operation)); } @@ -273,6 +275,22 @@ private function deduplicateShortNames(array $resources): array return $resources; } + /** + * Two operations sharing an explicit name silently overwrite each other because + * the operation name is the unique key (and the Symfony route name). + * Detect and reject upfront so users get a clear, actionable error. + * + * @param array $operations + */ + private function assertOperationNameIsUnique(array $operations, string $key, string $resourceClass): void + { + if (!\array_key_exists($key, $operations)) { + return; + } + + throw new RuntimeException(\sprintf('Operation name "%s" is declared twice on resource "%s". Operation names must be unique because they are also used as Symfony route names. Remove the duplicate "name" so the framework can disambiguate by method, or set distinct names.', $key, $resourceClass)); + } + /** * @template T of Metadata * diff --git a/src/Metadata/Tests/Extractor/XmlExtractorTest.php b/src/Metadata/Tests/Extractor/XmlExtractorTest.php index 9dedfc19906..98ca94248e6 100644 --- a/src/Metadata/Tests/Extractor/XmlExtractorTest.php +++ b/src/Metadata/Tests/Extractor/XmlExtractorTest.php @@ -14,10 +14,12 @@ namespace ApiPlatform\Metadata\Tests\Extractor; use ApiPlatform\Metadata\Exception\InvalidArgumentException; +use ApiPlatform\Metadata\Exception\RuntimeException; use ApiPlatform\Metadata\Extractor\XmlResourceExtractor; use ApiPlatform\Metadata\Get; use ApiPlatform\Metadata\GetCollection; use ApiPlatform\Metadata\QueryParameter; +use ApiPlatform\Metadata\Resource\Factory\ExtractorResourceMetadataCollectionFactory; use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\Comment; use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\User; use PHPUnit\Framework\Attributes\DataProvider; @@ -433,4 +435,19 @@ public static function getInvalidPaths(): array ], ]; } + + /** + * Tests issue #8175: two XML operations sharing an explicit name silently + * dropped one another. The factory must reject the duplicate up front. + */ + public function testDuplicateOperationNameFromXmlThrows(): void + { + $extractor = new XmlResourceExtractor([__DIR__.'/xml/duplicate_operation_name.xml']); + $factory = new ExtractorResourceMetadataCollectionFactory($extractor); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessageMatches('/_api_\/forms\/\{id\}\/submit\{\._format\}/'); + + $factory->create(Comment::class); + } } diff --git a/src/Metadata/Tests/Extractor/xml/duplicate_operation_name.xml b/src/Metadata/Tests/Extractor/xml/duplicate_operation_name.xml new file mode 100644 index 00000000000..061a88e7130 --- /dev/null +++ b/src/Metadata/Tests/Extractor/xml/duplicate_operation_name.xml @@ -0,0 +1,18 @@ + + + + + + + + + diff --git a/src/Metadata/Tests/Fixtures/ApiResource/SameNameDifferentMethodOperations.php b/src/Metadata/Tests/Fixtures/ApiResource/SameNameDifferentMethodOperations.php new file mode 100644 index 00000000000..4926cfdf0f4 --- /dev/null +++ b/src/Metadata/Tests/Fixtures/ApiResource/SameNameDifferentMethodOperations.php @@ -0,0 +1,42 @@ + + * + * 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\Patch; +use ApiPlatform\Metadata\Post; + +/** + * Reproduces issue #8175: two operations declared with the same explicit `name` + * but different methods on the same URI template. + */ +#[ApiResource( + shortName: 'SameNameDifferentMethodOperations', + operations: [ + new Post( + uriTemplate: '/forms/{id}/submit{._format}', + name: '_api_/forms/{id}/submit{._format}', + ), + new Patch( + uriTemplate: '/forms/{id}/submit{._format}', + name: '_api_/forms/{id}/submit{._format}', + ), + ], +)] +class SameNameDifferentMethodOperations +{ + #[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 3f744979a03..f88c59d54af 100644 --- a/src/Metadata/Tests/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php +++ b/src/Metadata/Tests/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php @@ -34,6 +34,7 @@ use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\AttributeResources; use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\ExtraPropertiesResource; use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\PasswordResource; +use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\SameNameDifferentMethodOperations; use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\WithParameter; use ApiPlatform\Metadata\Tests\Fixtures\State\AttributeResourceProcessor; use ApiPlatform\Metadata\Tests\Fixtures\State\AttributeResourceProvider; @@ -312,4 +313,18 @@ public function testWithParameters(): void $parameters = $metadataCollection->getOperation('collection')->getParameters(); $this->assertCount(3, $parameters); } + + /** + * Tests issue #8175: two operations sharing an explicit name silently dropped + * one another because they collided on the operations collection key. + */ + public function testDuplicateOperationNameThrows(): void + { + $factory = new AttributesResourceMetadataCollectionFactory(); + + $this->expectException(\ApiPlatform\Metadata\Exception\RuntimeException::class); + $this->expectExceptionMessageMatches('/_api_\/forms\/\{id\}\/submit\{\._format\}/'); + + $factory->create(SameNameDifferentMethodOperations::class); + } } diff --git a/src/Metadata/phpunit.baseline.xml b/src/Metadata/phpunit.baseline.xml index d83ee92be4d..202ea87bbb1 100644 --- a/src/Metadata/phpunit.baseline.xml +++ b/src/Metadata/phpunit.baseline.xml @@ -1,7 +1,7 @@ - + diff --git a/tests/Fixtures/TestBundle/Document/OverriddenOperationDummy.php b/tests/Fixtures/TestBundle/Document/OverriddenOperationDummy.php index 1fa4a150f4b..730cd42a476 100644 --- a/tests/Fixtures/TestBundle/Document/OverriddenOperationDummy.php +++ b/tests/Fixtures/TestBundle/Document/OverriddenOperationDummy.php @@ -29,7 +29,7 @@ * * @author Amrouche Hamza */ -#[ApiResource(operations: [new Get(), new Get(normalizationContext: ['groups' => ['overridden_operation_dummy_get']], denormalizationContext: ['groups' => ['overridden_operation_dummy_get']]), new Put(normalizationContext: ['groups' => ['overridden_operation_dummy_put']], denormalizationContext: ['groups' => ['overridden_operation_dummy_put']]), new Delete(), new GetCollection(), new Post(), new GetCollection(uriTemplate: '/override/swagger')], normalizationContext: ['groups' => ['overridden_operation_dummy_read']], denormalizationContext: ['groups' => ['overridden_operation_dummy_write']])] +#[ApiResource(operations: [new Get(normalizationContext: ['groups' => ['overridden_operation_dummy_get']], denormalizationContext: ['groups' => ['overridden_operation_dummy_get']]), new Put(normalizationContext: ['groups' => ['overridden_operation_dummy_put']], denormalizationContext: ['groups' => ['overridden_operation_dummy_put']]), new Delete(), new GetCollection(), new Post(), new GetCollection(uriTemplate: '/override/swagger')], normalizationContext: ['groups' => ['overridden_operation_dummy_read']], denormalizationContext: ['groups' => ['overridden_operation_dummy_write']])] #[ODM\Document] class OverriddenOperationDummy { diff --git a/tests/Fixtures/TestBundle/Entity/OverriddenOperationDummy.php b/tests/Fixtures/TestBundle/Entity/OverriddenOperationDummy.php index aa56dbcfbdc..a73ecf795b9 100644 --- a/tests/Fixtures/TestBundle/Entity/OverriddenOperationDummy.php +++ b/tests/Fixtures/TestBundle/Entity/OverriddenOperationDummy.php @@ -29,7 +29,7 @@ * * @author Amrouche Hamza */ -#[ApiResource(operations: [new Get(), new Get(normalizationContext: ['groups' => ['overridden_operation_dummy_get']], denormalizationContext: ['groups' => ['overridden_operation_dummy_get']]), new Put(normalizationContext: ['groups' => ['overridden_operation_dummy_put']], denormalizationContext: ['groups' => ['overridden_operation_dummy_put']]), new Delete(), new GetCollection(), new Post(), new GetCollection(uriTemplate: '/override/swagger')], normalizationContext: ['groups' => ['overridden_operation_dummy_read']], denormalizationContext: ['groups' => ['overridden_operation_dummy_write']])] +#[ApiResource(operations: [new Get(normalizationContext: ['groups' => ['overridden_operation_dummy_get']], denormalizationContext: ['groups' => ['overridden_operation_dummy_get']]), new Put(normalizationContext: ['groups' => ['overridden_operation_dummy_put']], denormalizationContext: ['groups' => ['overridden_operation_dummy_put']]), new Delete(), new GetCollection(), new Post(), new GetCollection(uriTemplate: '/override/swagger')], normalizationContext: ['groups' => ['overridden_operation_dummy_read']], denormalizationContext: ['groups' => ['overridden_operation_dummy_write']])] #[ORM\Entity] class OverriddenOperationDummy {