From 271664c68db57a5e9455d2dc08b8c22a6e4b6dae Mon Sep 17 00:00:00 2001 From: soyuka Date: Wed, 3 Jun 2026 11:15:15 +0200 Subject: [PATCH 1/6] fix(symfony): reject duplicate operation names instead of silently dropping operations Two operations declared on the same resource with an identical explicit `name` silently overwrote each other in the operations collection, leaving only the last one alive. The reporter hit this with two custom operations sharing the URI template `/forms/{id}/submit{._format}` and the same `name`, differing only by HTTP method (POST and PATCH): the first declaration would 405. The operation name is the unique key in the operations collection and is also used verbatim as the Symfony route name, so duplicates can never both work. The attribute and XML/YAML factories now detect a collision at metadata-build time and throw `RuntimeException` with a message pointing to the duplicate name and how to fix it (either drop the duplicate `name` so the framework disambiguates by method, or set distinct names). Fixes #8175 --- ...actorResourceMetadataCollectionFactory.php | 4 ++ .../MetadataCollectionFactoryTrait.php | 18 ++++++++ .../Tests/Extractor/XmlExtractorTest.php | 17 ++++++++ .../xml/duplicate_operation_name.xml | 18 ++++++++ .../SameNameDifferentMethodOperations.php | 42 +++++++++++++++++++ ...sResourceMetadataCollectionFactoryTest.php | 15 +++++++ 6 files changed, 114 insertions(+) create mode 100644 src/Metadata/Tests/Extractor/xml/duplicate_operation_name.xml create mode 100644 src/Metadata/Tests/Fixtures/ApiResource/SameNameDifferentMethodOperations.php 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); + } } From b7d98ec3c9c9292aca4af027d297c511465bd070 Mon Sep 17 00:00:00 2001 From: soyuka Date: Wed, 3 Jun 2026 12:35:04 +0200 Subject: [PATCH 2/6] fix(symfony): auto-suffix duplicate operation names with method instead of throwing The previous commit threw a RuntimeException whenever two operations resolved to the same key in the operations collection. That broke legitimate "override an earlier declaration" patterns used across the test suite (e.g. OverriddenOperationDummy re-declares Get with custom serialization groups, producing the same default key _api_OverriddenOperationDummy_get twice). The throw fired during cache warmup and took out every PHPUnit job. Restore the pre-regression behavior selectively: - Same key + same HTTP method: leave untouched. This is the override pattern; the later declaration wins, matching how Operations::add() already behaves. - Same key + different HTTP method (the issue #8175 case): the colliding entry is auto-suffixed with its method (e.g. _api_/forms/{id}/submit{._format}_patch) so both operations survive in the collection and Symfony route registration. The disambiguation lives on OperationDefaultsTrait so both the attributes/concerns factories (via MetadataCollectionFactoryTrait) and the XML/YAML extractor factory share one implementation. Tests previously asserting the throw now assert that both operations are present with the expected methods. --- ...actorResourceMetadataCollectionFactory.php | 5 +-- .../MetadataCollectionFactoryTrait.php | 20 ++---------- .../Factory/OperationDefaultsTrait.php | 32 +++++++++++++++++++ .../Tests/Extractor/XmlExtractorTest.php | 22 +++++++++---- ...sResourceMetadataCollectionFactoryTest.php | 21 ++++++++---- 5 files changed, 65 insertions(+), 35 deletions(-) diff --git a/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php b/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php index fd7abf9d705..3124635f9a5 100644 --- a/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php +++ b/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php @@ -14,7 +14,6 @@ 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; @@ -135,9 +134,7 @@ 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() ?? '')); - } + [$key, $operation] = $this->disambiguateOperationName($operations, $key, $operation); $operations[$key] = $operation; } diff --git a/src/Metadata/Resource/Factory/MetadataCollectionFactoryTrait.php b/src/Metadata/Resource/Factory/MetadataCollectionFactoryTrait.php index cd74e7776df..57042b8c8aa 100644 --- a/src/Metadata/Resource/Factory/MetadataCollectionFactoryTrait.php +++ b/src/Metadata/Resource/Factory/MetadataCollectionFactoryTrait.php @@ -87,7 +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); + [$key, $operation] = $this->disambiguateOperationName($operations, $key, $operation); $operations[$key] = $operation; } if ($operations) { @@ -146,7 +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); + [$key, $operation] = $this->disambiguateOperationName(iterator_to_array($operations), $key, $operation); $resources[$index] = $resources[$index]->withOperations($operations->add($key, $operation)); } @@ -275,22 +275,6 @@ 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/Resource/Factory/OperationDefaultsTrait.php b/src/Metadata/Resource/Factory/OperationDefaultsTrait.php index fe705960d44..24798d09d8f 100644 --- a/src/Metadata/Resource/Factory/OperationDefaultsTrait.php +++ b/src/Metadata/Resource/Factory/OperationDefaultsTrait.php @@ -242,4 +242,36 @@ private function getDefaultOperationName(HttpOperation $operation, string $resou $operation instanceof CollectionOperationInterface ? '_collection' : '' ); } + + /** + * Two operations sharing the same key would silently overwrite each other because the + * operation name is the unique collection key (and the Symfony route name). When the + * collision is between operations with different HTTP methods (e.g. POST + PATCH on the + * same URI template, both declared with the same explicit `name`), suffix the new entry + * with its method so both survive. Same key + same method is left untouched: that is the + * legitimate "override an earlier declaration" pattern (e.g. a class re-declaring a + * default `Get` with custom serialization groups). + * + * @param array $operations + * + * @return array{0: string, 1: Operation} + */ + private function disambiguateOperationName(array $operations, string $key, Operation $operation): array + { + if (!\array_key_exists($key, $operations) || !$operation instanceof HttpOperation) { + return [$key, $operation]; + } + + $existing = $operations[$key]; + if (!$existing instanceof HttpOperation || strtoupper($existing->getMethod()) === strtoupper($operation->getMethod())) { + return [$key, $operation]; + } + + $newKey = $key.'_'.strtolower($operation->getMethod()); + if (\array_key_exists($newKey, $operations)) { + return [$key, $operation]; + } + + return [$newKey, $operation->withName($newKey)]; + } } diff --git a/src/Metadata/Tests/Extractor/XmlExtractorTest.php b/src/Metadata/Tests/Extractor/XmlExtractorTest.php index 98ca94248e6..7e195c8b400 100644 --- a/src/Metadata/Tests/Extractor/XmlExtractorTest.php +++ b/src/Metadata/Tests/Extractor/XmlExtractorTest.php @@ -14,7 +14,6 @@ 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; @@ -437,17 +436,26 @@ 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. + * Tests issue #8175: two XML operations sharing an explicit name with different + * HTTP methods used to silently overwrite each other. The factory must keep both + * by auto-suffixing the colliding entry with its method. */ - public function testDuplicateOperationNameFromXmlThrows(): void + public function testDuplicateOperationNameWithDifferentMethodsFromXmlAreDisambiguated(): 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\}/'); + $collection = $factory->create(Comment::class); + $operations = $collection[0]->getOperations(); - $factory->create(Comment::class); + $this->assertTrue($operations->has('_api_/forms/{id}/submit{._format}')); + $this->assertTrue($operations->has('_api_/forms/{id}/submit{._format}_patch')); + + $methods = []; + foreach ($operations as $name => $operation) { + $methods[$name] = $operation->getMethod(); + } + $this->assertSame('POST', $methods['_api_/forms/{id}/submit{._format}']); + $this->assertSame('PATCH', $methods['_api_/forms/{id}/submit{._format}_patch']); } } diff --git a/src/Metadata/Tests/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php b/src/Metadata/Tests/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php index f88c59d54af..41fecbc1b0d 100644 --- a/src/Metadata/Tests/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php +++ b/src/Metadata/Tests/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php @@ -315,16 +315,25 @@ public function testWithParameters(): void } /** - * Tests issue #8175: two operations sharing an explicit name silently dropped - * one another because they collided on the operations collection key. + * Tests issue #8175: two operations sharing an explicit name with different HTTP + * methods used to silently overwrite each other. The factory must keep both by + * auto-suffixing the colliding entry with its method. */ - public function testDuplicateOperationNameThrows(): void + public function testDuplicateOperationNameWithDifferentMethodsAreDisambiguated(): void { $factory = new AttributesResourceMetadataCollectionFactory(); - $this->expectException(\ApiPlatform\Metadata\Exception\RuntimeException::class); - $this->expectExceptionMessageMatches('/_api_\/forms\/\{id\}\/submit\{\._format\}/'); + $collection = $factory->create(SameNameDifferentMethodOperations::class); + $operations = $collection[0]->getOperations(); - $factory->create(SameNameDifferentMethodOperations::class); + $this->assertTrue($operations->has('_api_/forms/{id}/submit{._format}')); + $this->assertTrue($operations->has('_api_/forms/{id}/submit{._format}_patch')); + + $names = []; + foreach ($operations as $name => $operation) { + $names[$name] = $operation->getMethod(); + } + $this->assertSame('POST', $names['_api_/forms/{id}/submit{._format}']); + $this->assertSame('PATCH', $names['_api_/forms/{id}/submit{._format}_patch']); } } From d2a51e3fe1d8e5356fc4b648b3c250469673a7ed Mon Sep 17 00:00:00 2001 From: soyuka Date: Wed, 3 Jun 2026 12:51:30 +0200 Subject: [PATCH 3/6] fix(metadata): update deprecation baselines for shifted trigger line The disambiguateOperationName calls added two lines to MetadataCollectionFactoryTrait, shifting the existing shortName-duplicate trigger_deprecation from line 252 to 254. PHPUnit baselines key on the line number; out-of-sync baselines stop ignoring the deprecation, so --fail-on-deprecation jobs (no-deprecations, Symfony dev, metadata) fail on a pre-existing, intentionally-baselined message. --- phpunit.baseline.xml | 2 +- src/Metadata/phpunit.baseline.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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/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 @@ - + From 358aaaab2b4a7d8c6ac161121c9e4db9f948984b Mon Sep 17 00:00:00 2001 From: soyuka Date: Wed, 3 Jun 2026 13:48:22 +0200 Subject: [PATCH 4/6] Revert "fix(metadata): update deprecation baselines for shifted trigger line" This reverts commit d2a51e3fe1d8e5356fc4b648b3c250469673a7ed. --- phpunit.baseline.xml | 2 +- src/Metadata/phpunit.baseline.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/phpunit.baseline.xml b/phpunit.baseline.xml index bb10f6ce0f9..ebb80a94bd9 100644 --- a/phpunit.baseline.xml +++ b/phpunit.baseline.xml @@ -31,7 +31,7 @@ - + diff --git a/src/Metadata/phpunit.baseline.xml b/src/Metadata/phpunit.baseline.xml index 202ea87bbb1..d83ee92be4d 100644 --- a/src/Metadata/phpunit.baseline.xml +++ b/src/Metadata/phpunit.baseline.xml @@ -1,7 +1,7 @@ - + From 899fb577ecbed6f79d77311d5555d9c5fb1f1f86 Mon Sep 17 00:00:00 2001 From: soyuka Date: Wed, 3 Jun 2026 13:48:27 +0200 Subject: [PATCH 5/6] Revert "fix(symfony): auto-suffix duplicate operation names with method instead of throwing" This reverts commit b7d98ec3c9c9292aca4af027d297c511465bd070. --- ...actorResourceMetadataCollectionFactory.php | 5 ++- .../MetadataCollectionFactoryTrait.php | 20 ++++++++++-- .../Factory/OperationDefaultsTrait.php | 32 ------------------- .../Tests/Extractor/XmlExtractorTest.php | 22 ++++--------- ...sResourceMetadataCollectionFactoryTest.php | 21 ++++-------- 5 files changed, 35 insertions(+), 65 deletions(-) diff --git a/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php b/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php index 3124635f9a5..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,7 +135,9 @@ private function addOperations(?array $data, ApiResource $resource): ApiResource } [$key, $operation] = $this->getOperationWithDefaults($resource, $operation); - [$key, $operation] = $this->disambiguateOperationName($operations, $key, $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 57042b8c8aa..cd74e7776df 100644 --- a/src/Metadata/Resource/Factory/MetadataCollectionFactoryTrait.php +++ b/src/Metadata/Resource/Factory/MetadataCollectionFactoryTrait.php @@ -87,7 +87,7 @@ private function buildResourceOperations(array $metadataCollection, string $reso $operations = []; foreach ($resource->getOperations() ?? new Operations() as $operation) { [$key, $operation] = $this->getOperationWithDefaults($resource, $operation); - [$key, $operation] = $this->disambiguateOperationName($operations, $key, $operation); + $this->assertOperationNameIsUnique($operations, $key, $resourceClass); $operations[$key] = $operation; } if ($operations) { @@ -146,7 +146,7 @@ private function buildResourceOperations(array $metadataCollection, string $reso $operation = $operation->withPriority(++$operationPriority); } $operations = $resources[$index]->getOperations() ?? new Operations(); - [$key, $operation] = $this->disambiguateOperationName(iterator_to_array($operations), $key, $operation); + $this->assertOperationNameIsUnique(iterator_to_array($operations), $key, $resourceClass); $resources[$index] = $resources[$index]->withOperations($operations->add($key, $operation)); } @@ -275,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/Resource/Factory/OperationDefaultsTrait.php b/src/Metadata/Resource/Factory/OperationDefaultsTrait.php index 24798d09d8f..fe705960d44 100644 --- a/src/Metadata/Resource/Factory/OperationDefaultsTrait.php +++ b/src/Metadata/Resource/Factory/OperationDefaultsTrait.php @@ -242,36 +242,4 @@ private function getDefaultOperationName(HttpOperation $operation, string $resou $operation instanceof CollectionOperationInterface ? '_collection' : '' ); } - - /** - * Two operations sharing the same key would silently overwrite each other because the - * operation name is the unique collection key (and the Symfony route name). When the - * collision is between operations with different HTTP methods (e.g. POST + PATCH on the - * same URI template, both declared with the same explicit `name`), suffix the new entry - * with its method so both survive. Same key + same method is left untouched: that is the - * legitimate "override an earlier declaration" pattern (e.g. a class re-declaring a - * default `Get` with custom serialization groups). - * - * @param array $operations - * - * @return array{0: string, 1: Operation} - */ - private function disambiguateOperationName(array $operations, string $key, Operation $operation): array - { - if (!\array_key_exists($key, $operations) || !$operation instanceof HttpOperation) { - return [$key, $operation]; - } - - $existing = $operations[$key]; - if (!$existing instanceof HttpOperation || strtoupper($existing->getMethod()) === strtoupper($operation->getMethod())) { - return [$key, $operation]; - } - - $newKey = $key.'_'.strtolower($operation->getMethod()); - if (\array_key_exists($newKey, $operations)) { - return [$key, $operation]; - } - - return [$newKey, $operation->withName($newKey)]; - } } diff --git a/src/Metadata/Tests/Extractor/XmlExtractorTest.php b/src/Metadata/Tests/Extractor/XmlExtractorTest.php index 7e195c8b400..98ca94248e6 100644 --- a/src/Metadata/Tests/Extractor/XmlExtractorTest.php +++ b/src/Metadata/Tests/Extractor/XmlExtractorTest.php @@ -14,6 +14,7 @@ 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; @@ -436,26 +437,17 @@ public static function getInvalidPaths(): array } /** - * Tests issue #8175: two XML operations sharing an explicit name with different - * HTTP methods used to silently overwrite each other. The factory must keep both - * by auto-suffixing the colliding entry with its method. + * Tests issue #8175: two XML operations sharing an explicit name silently + * dropped one another. The factory must reject the duplicate up front. */ - public function testDuplicateOperationNameWithDifferentMethodsFromXmlAreDisambiguated(): void + public function testDuplicateOperationNameFromXmlThrows(): void { $extractor = new XmlResourceExtractor([__DIR__.'/xml/duplicate_operation_name.xml']); $factory = new ExtractorResourceMetadataCollectionFactory($extractor); - $collection = $factory->create(Comment::class); - $operations = $collection[0]->getOperations(); + $this->expectException(RuntimeException::class); + $this->expectExceptionMessageMatches('/_api_\/forms\/\{id\}\/submit\{\._format\}/'); - $this->assertTrue($operations->has('_api_/forms/{id}/submit{._format}')); - $this->assertTrue($operations->has('_api_/forms/{id}/submit{._format}_patch')); - - $methods = []; - foreach ($operations as $name => $operation) { - $methods[$name] = $operation->getMethod(); - } - $this->assertSame('POST', $methods['_api_/forms/{id}/submit{._format}']); - $this->assertSame('PATCH', $methods['_api_/forms/{id}/submit{._format}_patch']); + $factory->create(Comment::class); } } diff --git a/src/Metadata/Tests/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php b/src/Metadata/Tests/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php index 41fecbc1b0d..f88c59d54af 100644 --- a/src/Metadata/Tests/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php +++ b/src/Metadata/Tests/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php @@ -315,25 +315,16 @@ public function testWithParameters(): void } /** - * Tests issue #8175: two operations sharing an explicit name with different HTTP - * methods used to silently overwrite each other. The factory must keep both by - * auto-suffixing the colliding entry with its method. + * Tests issue #8175: two operations sharing an explicit name silently dropped + * one another because they collided on the operations collection key. */ - public function testDuplicateOperationNameWithDifferentMethodsAreDisambiguated(): void + public function testDuplicateOperationNameThrows(): void { $factory = new AttributesResourceMetadataCollectionFactory(); - $collection = $factory->create(SameNameDifferentMethodOperations::class); - $operations = $collection[0]->getOperations(); + $this->expectException(\ApiPlatform\Metadata\Exception\RuntimeException::class); + $this->expectExceptionMessageMatches('/_api_\/forms\/\{id\}\/submit\{\._format\}/'); - $this->assertTrue($operations->has('_api_/forms/{id}/submit{._format}')); - $this->assertTrue($operations->has('_api_/forms/{id}/submit{._format}_patch')); - - $names = []; - foreach ($operations as $name => $operation) { - $names[$name] = $operation->getMethod(); - } - $this->assertSame('POST', $names['_api_/forms/{id}/submit{._format}']); - $this->assertSame('PATCH', $names['_api_/forms/{id}/submit{._format}_patch']); + $factory->create(SameNameDifferentMethodOperations::class); } } From 536eeb4ddc39459fd77c99e2d80f87271c55a791 Mon Sep 17 00:00:00 2001 From: soyuka Date: Wed, 3 Jun 2026 13:57:27 +0200 Subject: [PATCH 6/6] fix(symfony): drop duplicate Get() from OverriddenOperationDummy fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fixture declared two parameterless `new Get()` operations on the same resource, which resolved to identical default operation names. In 4.2.0+ the second declaration silently overwrote the first; with the duplicate-name guard added in 271664c68 this now throws, breaking cache warmup. The first `new Get()` was dead — the second one (with overridden_operation_dummy_get groups) is what every test exercises. Removing it is the same fix users hitting #8175 need to apply to their own resources. Also bump the shortName-deduplication baseline from line 252 to 254 to match the new trigger_deprecation position once the throw guard's assertOperationNameIsUnique calls have shifted the surrounding code. --- phpunit.baseline.xml | 2 +- .../Tests/Fixtures/ApiResource/OverriddenOperationDummy.php | 2 +- src/Metadata/phpunit.baseline.xml | 2 +- tests/Fixtures/TestBundle/Document/OverriddenOperationDummy.php | 2 +- tests/Fixtures/TestBundle/Entity/OverriddenOperationDummy.php | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) 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/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 {