From b0e9e4335769ff04ec69a38d9c15c5d8298e6e23 Mon Sep 17 00:00:00 2001 From: soyuka Date: Thu, 17 Sep 2026 17:24:49 +0200 Subject: [PATCH] fix(metadata): format-agnostic uriTemplate match `itemUriTemplate` and `ApiProperty::uriTemplate` were compared to `Operation::getUriTemplate()` with a strict `===`. Auto-generated operations store the format suffix, so `itemUriTemplate: '/books/{id}'` never matched `/books/{id}{._format}`. `OperationMetadataFactory::create()` returned null and the IriConverter silently fell back to the resource's default item operation, emitting a wrong IRI instead of an error. `OperationMetadataFactory::create()` and `ResourceMetadataCollection::getOperation()` now keep the first candidate that matches once the `{._format}`/`.{_format}` suffix is stripped from both sides, and return it only after the full scan finds no exact match, so two operations differing only by the suffix keep their identities whatever their declaration order. --- .../Factory/OperationMetadataFactory.php | 12 ++++ ...plateResourceMetadataCollectionFactory.php | 5 +- .../Resource/ResourceMetadataCollection.php | 11 ++++ .../Factory/OperationMetadataFactoryTest.php | 58 +++++++++++++++++++ .../ResourceMetadataCollectionTest.php | 34 +++++++++++ src/Metadata/Util/UriTemplateHelper.php | 33 +++++++++++ .../UriTemplateFormatSuffixResource.php | 57 ++++++++++++++++++ ...ItemUriTemplateWithoutFormatSuffixTest.php | 43 ++++++++++++++ 8 files changed, 250 insertions(+), 3 deletions(-) create mode 100644 src/Metadata/Util/UriTemplateHelper.php create mode 100644 tests/Fixtures/TestBundle/ApiResource/ItemUriTemplateWithoutFormatSuffix/UriTemplateFormatSuffixResource.php create mode 100644 tests/Functional/ItemUriTemplateWithoutFormatSuffixTest.php diff --git a/src/Metadata/Operation/Factory/OperationMetadataFactory.php b/src/Metadata/Operation/Factory/OperationMetadataFactory.php index bcaa7bf63ed..90155b3317e 100644 --- a/src/Metadata/Operation/Factory/OperationMetadataFactory.php +++ b/src/Metadata/Operation/Factory/OperationMetadataFactory.php @@ -16,6 +16,7 @@ use ApiPlatform\Metadata\Operation; use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; use ApiPlatform\Metadata\Resource\Factory\ResourceNameCollectionFactoryInterface; +use ApiPlatform\Metadata\Util\UriTemplateHelper; final class OperationMetadataFactory implements OperationMetadataFactoryInterface { @@ -31,16 +32,27 @@ public function create(string $uriTemplate, array $context = []): ?Operation return $this->localCache[$uriTemplate]; } + $fallback = null; + $strippedUriTemplate = UriTemplateHelper::withoutFormatSuffix($uriTemplate); + foreach ($this->resourceNameCollectionFactory->create() as $resourceClass) { foreach ($this->resourceMetadataCollectionFactory->create($resourceClass) as $resource) { foreach ($resource->getOperations() as $operation) { if ($operation->getUriTemplate() === $uriTemplate || $operation->getName() === $uriTemplate) { return $this->localCache[$uriTemplate] = $operation; } + + if (null === $fallback && null !== ($operationUriTemplate = $operation->getUriTemplate()) && UriTemplateHelper::withoutFormatSuffix($operationUriTemplate) === $strippedUriTemplate) { + $fallback = $operation; + } } } } + if (null !== $fallback) { + return $this->localCache[$uriTemplate] = $fallback; + } + return null; } } diff --git a/src/Metadata/Resource/Factory/UriTemplateResourceMetadataCollectionFactory.php b/src/Metadata/Resource/Factory/UriTemplateResourceMetadataCollectionFactory.php index feace7c512c..c207ec859cb 100644 --- a/src/Metadata/Resource/Factory/UriTemplateResourceMetadataCollectionFactory.php +++ b/src/Metadata/Resource/Factory/UriTemplateResourceMetadataCollectionFactory.php @@ -20,6 +20,7 @@ use ApiPlatform\Metadata\Operation\PathSegmentNameGeneratorInterface; use ApiPlatform\Metadata\Operations; use ApiPlatform\Metadata\Resource\ResourceMetadataCollection; +use ApiPlatform\Metadata\Util\UriTemplateHelper; use Symfony\Component\Routing\Route; /** @@ -153,9 +154,7 @@ private function configureUriVariables(ApiResource|HttpOperation $operation): Ap } $operation = $operation->withUriVariables($uriVariables); - if (str_ends_with($uriTemplate, '{._format}') || str_ends_with($uriTemplate, '.{_format}')) { - $uriTemplate = substr($uriTemplate, 0, -10); - } + $uriTemplate = UriTemplateHelper::withoutFormatSuffix($uriTemplate); // TODO: move this to the Symfony bridge if (class_exists(Route::class)) { diff --git a/src/Metadata/Resource/ResourceMetadataCollection.php b/src/Metadata/Resource/ResourceMetadataCollection.php index f55f83dc1c7..c51f084441b 100644 --- a/src/Metadata/Resource/ResourceMetadataCollection.php +++ b/src/Metadata/Resource/ResourceMetadataCollection.php @@ -17,6 +17,7 @@ use ApiPlatform\Metadata\CollectionOperationInterface; use ApiPlatform\Metadata\Exception\OperationNotFoundException; use ApiPlatform\Metadata\Operation; +use ApiPlatform\Metadata\Util\UriTemplateHelper; /** * @extends \ArrayObject @@ -51,6 +52,8 @@ public function getOperation(?string $operationName = null, bool $forceCollectio $it = $this->getIterator(); $metadata = null; + $fallback = null; + $strippedOperationName = '' !== $operationName ? UriTemplateHelper::withoutFormatSuffix($operationName) : ''; while ($it->valid()) { /** @var ApiResource $metadata */ @@ -72,6 +75,10 @@ public function getOperation(?string $operationName = null, bool $forceCollectio if ($operation->getUriTemplate() === $operationName) { return $this->operationCache[$httpCacheKey] = $operation; } + + if ('' !== $operationName && null === $fallback && null !== ($operationUriTemplate = $operation->getUriTemplate()) && UriTemplateHelper::withoutFormatSuffix($operationUriTemplate) === $strippedOperationName) { + $fallback = $operation; + } } } @@ -89,6 +96,10 @@ public function getOperation(?string $operationName = null, bool $forceCollectio $it->next(); } + if (null !== $fallback) { + return $this->operationCache[$httpCacheKey] = $fallback; + } + // Idea: // if ($metadata) { // return (new class extends HttpOperation {})->withResource($metadata); diff --git a/src/Metadata/Tests/Operation/Factory/OperationMetadataFactoryTest.php b/src/Metadata/Tests/Operation/Factory/OperationMetadataFactoryTest.php index 9e4210ebde0..43b7980db58 100644 --- a/src/Metadata/Tests/Operation/Factory/OperationMetadataFactoryTest.php +++ b/src/Metadata/Tests/Operation/Factory/OperationMetadataFactoryTest.php @@ -45,4 +45,62 @@ public function testCreate(): void $this->assertEquals($operation, $operationMetadata->create('/one')); $this->assertNull($operationMetadata->create('none')); } + + public function testCreateResolvesUriTemplateWithoutFormatSuffix(): void + { + $operation = new Get('/one{._format}', name: 'one'); + + $resourceNameCollectionFactory = $this->createStub(ResourceNameCollectionFactoryInterface::class); + $resourceNameCollectionFactory->method('create')->willReturn(new ResourceNameCollection(['one'])); + + $resourceMetadataCollectionFactory = $this->createStub(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataCollectionFactory->method('create')->willReturn(new ResourceMetadataCollection('one', [ + new ApiResource(operations: ['one' => $operation]), + ])); + + $operationMetadata = new OperationMetadataFactory($resourceNameCollectionFactory, $resourceMetadataCollectionFactory); + $this->assertSame($operation, $operationMetadata->create('/one')); + $this->assertSame($operation, $operationMetadata->create('/one.{_format}')); + } + + public function testCreateExactMatchWinsOverLenientMatchRegardlessOfOrder(): void + { + $exact = new Get('/one', name: 'exact'); + $lenient = new Get('/one{._format}', name: 'lenient'); + + $resourceNameCollectionFactory = $this->createStub(ResourceNameCollectionFactoryInterface::class); + $resourceNameCollectionFactory->method('create')->willReturn(new ResourceNameCollection(['one'])); + + $resourceMetadataCollectionFactory = $this->createStub(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataCollectionFactory->method('create')->willReturn(new ResourceMetadataCollection('one', [ + new ApiResource(operations: ['exact' => $exact, 'lenient' => $lenient]), + ])); + + $operationMetadata = new OperationMetadataFactory($resourceNameCollectionFactory, $resourceMetadataCollectionFactory); + $this->assertSame($exact, $operationMetadata->create('/one')); + + $resourceMetadataCollectionFactoryReversed = $this->createStub(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataCollectionFactoryReversed->method('create')->willReturn(new ResourceMetadataCollection('one', [ + new ApiResource(operations: ['lenient' => $lenient, 'exact' => $exact]), + ])); + + $operationMetadataReversed = new OperationMetadataFactory($resourceNameCollectionFactory, $resourceMetadataCollectionFactoryReversed); + $this->assertSame($exact, $operationMetadataReversed->create('/one')); + } + + public function testCreateReturnsNullForUnrelatedTemplate(): void + { + $operation = new Get('/one{._format}', name: 'one'); + + $resourceNameCollectionFactory = $this->createStub(ResourceNameCollectionFactoryInterface::class); + $resourceNameCollectionFactory->method('create')->willReturn(new ResourceNameCollection(['one'])); + + $resourceMetadataCollectionFactory = $this->createStub(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataCollectionFactory->method('create')->willReturn(new ResourceMetadataCollection('one', [ + new ApiResource(operations: ['one' => $operation]), + ])); + + $operationMetadata = new OperationMetadataFactory($resourceNameCollectionFactory, $resourceMetadataCollectionFactory); + $this->assertNull($operationMetadata->create('/unrelated')); + } } diff --git a/src/Metadata/Tests/Resource/ResourceMetadataCollectionTest.php b/src/Metadata/Tests/Resource/ResourceMetadataCollectionTest.php index 8b09aa7262d..f7024c9cbc4 100644 --- a/src/Metadata/Tests/Resource/ResourceMetadataCollectionTest.php +++ b/src/Metadata/Tests/Resource/ResourceMetadataCollectionTest.php @@ -90,4 +90,38 @@ public function testCache(): void $this->assertInstanceOf(OperationNotFoundException::class, $e); } } + + public function testGetOperationResolvesUriTemplateWithoutFormatSuffix(): void + { + $operation = (new Get())->withUriTemplate('/one{._format}'); + $resource = (new ApiResource())->withOperations(new Operations(['name' => $operation])); + $resourceMetadataCollection = new ResourceMetadataCollection('class', [$resource]); + + $this->assertSame($operation, $resourceMetadataCollection->getOperation('/one')); + $this->assertSame($operation, $resourceMetadataCollection->getOperation('/one.{_format}')); + } + + public function testGetOperationExactMatchWinsOverLenientMatchRegardlessOfOrder(): void + { + $exact = (new Get())->withUriTemplate('/one'); + $lenient = (new Get())->withUriTemplate('/one{._format}'); + + $resource = (new ApiResource())->withOperations(new Operations(['exact' => $exact, 'lenient' => $lenient])); + $resourceMetadataCollection = new ResourceMetadataCollection('class', [$resource]); + $this->assertSame($exact, $resourceMetadataCollection->getOperation('/one')); + + $resourceReversed = (new ApiResource())->withOperations(new Operations(['lenient' => $lenient, 'exact' => $exact])); + $resourceMetadataCollectionReversed = new ResourceMetadataCollection('class', [$resourceReversed]); + $this->assertSame($exact, $resourceMetadataCollectionReversed->getOperation('/one')); + } + + public function testGetOperationReturnsNullForUnrelatedTemplate(): void + { + $operation = (new Get())->withUriTemplate('/one{._format}'); + $resource = (new ApiResource())->withOperations(new Operations(['name' => $operation])); + $resourceMetadataCollection = new ResourceMetadataCollection('class', [$resource]); + + $this->expectException(OperationNotFoundException::class); + $resourceMetadataCollection->getOperation('/unrelated'); + } } diff --git a/src/Metadata/Util/UriTemplateHelper.php b/src/Metadata/Util/UriTemplateHelper.php new file mode 100644 index 00000000000..e07c9fc7c85 --- /dev/null +++ b/src/Metadata/Util/UriTemplateHelper.php @@ -0,0 +1,33 @@ + + * + * 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\Util; + +/** + * @internal + */ +final class UriTemplateHelper +{ + private function __construct() + { + } + + public static function withoutFormatSuffix(string $uriTemplate): string + { + if (str_ends_with($uriTemplate, '{._format}') || str_ends_with($uriTemplate, '.{_format}')) { + return substr($uriTemplate, 0, -10); + } + + return $uriTemplate; + } +} diff --git a/tests/Fixtures/TestBundle/ApiResource/ItemUriTemplateWithoutFormatSuffix/UriTemplateFormatSuffixResource.php b/tests/Fixtures/TestBundle/ApiResource/ItemUriTemplateWithoutFormatSuffix/UriTemplateFormatSuffixResource.php new file mode 100644 index 00000000000..13f2255d610 --- /dev/null +++ b/tests/Fixtures/TestBundle/ApiResource/ItemUriTemplateWithoutFormatSuffix/UriTemplateFormatSuffixResource.php @@ -0,0 +1,57 @@ + + * + * 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\Tests\Fixtures\TestBundle\ApiResource\ItemUriTemplateWithoutFormatSuffix; + +use ApiPlatform\Metadata\ApiProperty; +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\GetCollection; +use ApiPlatform\Metadata\Operation; + +#[ApiResource( + operations: [ + new GetCollection( + uriTemplate: '/uri_template_format_suffix_resource_collection', + itemUriTemplate: '/uri_template_format_suffix_resource_items/{id}', + provider: [self::class, 'provideCollection'], + ), + new Get( + provider: [self::class, 'provide'], + ), + new Get( + uriTemplate: '/uri_template_format_suffix_resource_items/{id}{._format}', + uriVariables: ['id'], + provider: [self::class, 'provide'], + ), + ], +)] +class UriTemplateFormatSuffixResource +{ + public function __construct(#[ApiProperty(identifier: true)] public string $id = '1') + { + } + + public static function provide(Operation $operation, array $uriVariables = [], array $context = []): self + { + return new self((string) ($uriVariables['id'] ?? '1')); + } + + /** + * @return self[] + */ + public static function provideCollection(): array + { + return [new self('1'), new self('2')]; + } +} diff --git a/tests/Functional/ItemUriTemplateWithoutFormatSuffixTest.php b/tests/Functional/ItemUriTemplateWithoutFormatSuffixTest.php new file mode 100644 index 00000000000..a772a1c8a1d --- /dev/null +++ b/tests/Functional/ItemUriTemplateWithoutFormatSuffixTest.php @@ -0,0 +1,43 @@ + + * + * 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\Tests\Functional; + +use ApiPlatform\Test\ApiTestCase; +use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\ItemUriTemplateWithoutFormatSuffix\UriTemplateFormatSuffixResource; +use ApiPlatform\Tests\SetupClassResourcesTrait; + +final class ItemUriTemplateWithoutFormatSuffixTest extends ApiTestCase +{ + use SetupClassResourcesTrait; + + protected static ?bool $alwaysBootKernel = false; + + /** + * @return class-string[] + */ + public static function getResources(): array + { + return [UriTemplateFormatSuffixResource::class]; + } + + public function testItemUriTemplateResolvesWithoutFormatSuffix(): void + { + $response = self::createClient()->request('GET', '/uri_template_format_suffix_resource_collection'); + $this->assertResponseIsSuccessful(); + $data = $response->toArray(); + + $this->assertSame('/uri_template_format_suffix_resource_items/1', $data['hydra:member'][0]['@id']); + $this->assertSame('/uri_template_format_suffix_resource_items/2', $data['hydra:member'][1]['@id']); + } +}