From 12eeb46a26f03dcf5b088a4c137507009a078a61 Mon Sep 17 00:00:00 2001 From: Johannes Werbrouck Date: Sat, 22 Aug 2026 09:56:28 +0200 Subject: [PATCH 1/3] fix(serializer): localOperationCache is always written but never read --- src/Symfony/Routing/IriConverter.php | 15 ++-- tests/Symfony/Routing/IriConverterTest.php | 86 ++++++++++++++++++++++ 2 files changed, 96 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Routing/IriConverter.php b/src/Symfony/Routing/IriConverter.php index eb5ce19576f..00a353081a2 100644 --- a/src/Symfony/Routing/IriConverter.php +++ b/src/Symfony/Routing/IriConverter.php @@ -161,11 +161,16 @@ public function getIriFromResource(object|string $resource, int $referenceType = !$operation->getName() || ($operation instanceof HttpOperation && 'POST' === $operation->getMethod()) ) { - $forceCollection = $operation instanceof CollectionOperationInterface; - try { - $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null, $forceCollection, true); - $identifiersExtractorOperation = $operation; - } catch (OperationNotFoundException) { + if (isset($this->localOperationCache[$localOperationCacheKey])) { + $operation = $this->localOperationCache[$localOperationCacheKey]; + $identifiersExtractorOperation = $this->localIdentifiersExtractorOperationCache[$localOperationCacheKey] ?? $operation; + } else { + $forceCollection = $operation instanceof CollectionOperationInterface; + try { + $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null, $forceCollection, true); + $identifiersExtractorOperation = $operation; + } catch (OperationNotFoundException) { + } } } diff --git a/tests/Symfony/Routing/IriConverterTest.php b/tests/Symfony/Routing/IriConverterTest.php index 0e05743a303..b4be06edf49 100644 --- a/tests/Symfony/Routing/IriConverterTest.php +++ b/tests/Symfony/Routing/IriConverterTest.php @@ -110,6 +110,92 @@ public function testGetIriFromItemWithContextOperation(): void $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item, UrlGeneratorInterface::ABS_URL, $operation)); } + public function testGetIriFromItemWithoutOperationUsesTheLocalOperationCache(): void + { + $item = new Dummy(); + $item->setId(1); + + $operationName = 'operation_name'; + $operation = (new Get())->withName($operationName); + + $routerProphecy = $this->prophesize(RouterInterface::class); + $routerProphecy->generate($operationName, ['id' => 1], UrlGeneratorInterface::ABS_PATH)->shouldBeCalledTimes(2)->willReturn('/dummies/1'); + + $identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class); + $identifiersExtractorProphecy->getIdentifiersFromItem($item, $operation, Argument::any())->shouldBeCalledTimes(2)->willReturn(['id' => 1]); + + $resourceMetadataCollectionFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataCollectionFactoryProphecy->create(Dummy::class)->shouldBeCalledOnce()->willReturn(new ResourceMetadataCollection(Dummy::class, [ + (new ApiResource())->withOperations(new Operations([$operationName => $operation])), + ])); + + $iriConverter = $this->getIriConverter(null, $routerProphecy, $identifiersExtractorProphecy, $resourceMetadataCollectionFactoryProphecy); + + $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item)); + $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item)); + } + + public function testGetIriFromItemWithoutOperationReusesTheCachedOperation(): void + { + $item = new Dummy(); + $item->setId(1); + + $cachedOperation = (new Get())->withName('cached_operation'); + $staleOperation = (new Get())->withName('stale_operation'); + + $routerProphecy = $this->prophesize(RouterInterface::class); + $routerProphecy->generate('cached_operation', ['id' => 1], UrlGeneratorInterface::ABS_PATH)->willReturn('/dummies/1'); + $routerProphecy->generate('stale_operation', ['id' => 1], UrlGeneratorInterface::ABS_PATH)->willReturn('/stale/1'); + + $identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class); + $identifiersExtractorProphecy->getIdentifiersFromItem($item, Argument::cetera())->willReturn(['id' => 1]); + + // Prophecy returns these in order across consecutive calls, repeating the last. + $resourceMetadataCollectionFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataCollectionFactoryProphecy->create(Dummy::class)->willReturn( + new ResourceMetadataCollection(Dummy::class, [(new ApiResource())->withOperations(new Operations(['cached_operation' => $cachedOperation]))]), + new ResourceMetadataCollection(Dummy::class, [(new ApiResource())->withOperations(new Operations(['stale_operation' => $staleOperation]))]), + ); + + $iriConverter = $this->getIriConverter(null, $routerProphecy, $identifiersExtractorProphecy, $resourceMetadataCollectionFactoryProphecy); + + $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item)); + $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item)); + } + + public function testLocalOperationCacheDistinguishesItemAndCollectionIris(): void + { + $item = new Dummy(); + $item->setId(1); + + $itemOperation = (new Get())->withName('item_operation')->withClass(Dummy::class); + $collectionOperation = (new GetCollection())->withName('collection_operation')->withClass(Dummy::class); + + $routerProphecy = $this->prophesize(RouterInterface::class); + $routerProphecy->generate('item_operation', ['id' => 1], UrlGeneratorInterface::ABS_PATH)->shouldBeCalledTimes(2)->willReturn('/dummies/1'); + $routerProphecy->generate('collection_operation', [], UrlGeneratorInterface::ABS_PATH)->shouldBeCalledTimes(2)->willReturn('/dummies'); + + $identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class); + $identifiersExtractorProphecy->getIdentifiersFromItem($item, Argument::cetera())->willReturn(['id' => 1]); + + // getOperation(null, $forceCollection, true) picks between the two according to $forceCollection. + $resourceMetadataCollectionFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataCollectionFactoryProphecy->create(Dummy::class)->shouldBeCalledTimes(2)->willReturn(new ResourceMetadataCollection(Dummy::class, [ + (new ApiResource())->withOperations(new Operations([ + 'item_operation' => $itemOperation, + 'collection_operation' => $collectionOperation, + ])), + ])); + + $iriConverter = $this->getIriConverter(null, $routerProphecy, $identifiersExtractorProphecy, $resourceMetadataCollectionFactoryProphecy); + + // Interleaved on purpose: the third and fourth calls must read the cache entry the first two wrote. + $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item)); + $this->assertSame('/dummies', $iriConverter->getIriFromResource(Dummy::class, UrlGeneratorInterface::ABS_PATH, new GetCollection())); + $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item)); + $this->assertSame('/dummies', $iriConverter->getIriFromResource(Dummy::class, UrlGeneratorInterface::ABS_PATH, new GetCollection())); + } + public function testGetIriFromItemWithNoOperations(): void { $this->expectExceptionMessage(\sprintf('Unable to generate an IRI for the item of type "%s"', Dummy::class)); From f3f91f76e3aa83c424b43040d8f18df19d004018 Mon Sep 17 00:00:00 2001 From: Johannes Werbrouck Date: Sat, 22 Aug 2026 09:56:28 +0200 Subject: [PATCH 2/3] fix(serializer): localOperationCache is always written but never read --- src/Laravel/Routing/IriConverter.php | 15 ++- .../Tests/Unit/Routing/IriConverterTest.php | 109 ++++++++++++++++++ src/Symfony/Routing/IriConverter.php | 15 ++- tests/Symfony/Routing/IriConverterTest.php | 86 ++++++++++++++ 4 files changed, 215 insertions(+), 10 deletions(-) diff --git a/src/Laravel/Routing/IriConverter.php b/src/Laravel/Routing/IriConverter.php index 82afad6be8a..1e2f56ea9d0 100644 --- a/src/Laravel/Routing/IriConverter.php +++ b/src/Laravel/Routing/IriConverter.php @@ -140,11 +140,16 @@ public function getIriFromResource(object|string $resource, int $referenceType = !$operation->getName() || ($operation instanceof HttpOperation && 'POST' === $operation->getMethod()) ) { - $forceCollection = $operation instanceof CollectionOperationInterface; - try { - $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null, $forceCollection, true); - $identifiersExtractorOperation = $operation; - } catch (OperationNotFoundException) { + if (isset($this->localOperationCache[$localOperationCacheKey])) { + $operation = $this->localOperationCache[$localOperationCacheKey]; + $identifiersExtractorOperation = $this->localIdentifiersExtractorOperationCache[$localOperationCacheKey] ?? $operation; + } else { + $forceCollection = $operation instanceof CollectionOperationInterface; + try { + $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null, $forceCollection, true); + $identifiersExtractorOperation = $operation; + } catch (OperationNotFoundException) { + } } } diff --git a/src/Laravel/Tests/Unit/Routing/IriConverterTest.php b/src/Laravel/Tests/Unit/Routing/IriConverterTest.php index c88de228bda..83bfc744087 100644 --- a/src/Laravel/Tests/Unit/Routing/IriConverterTest.php +++ b/src/Laravel/Tests/Unit/Routing/IriConverterTest.php @@ -87,4 +87,113 @@ public function testLocalCacheKeyDistinguishesItemAndCollectionForStringResource ['uri_variables' => ['id' => 1]], )); } + + public function testGetIriFromResourceWithoutOperationUsesTheLocalOperationCache(): void + { + $itemOpName = 'item_op'; + $itemOp = (new Get())->withName($itemOpName)->withClass(Book::class); + + $router = $this->createMock(RouterInterface::class); + $router->expects($this->exactly(2)) + ->method('generate') + ->willReturn('/api/books/1'); + + $resourceMetadataFactory = $this->createMock(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataFactory->expects($this->once()) + ->method('create') + ->with(Book::class) + ->willReturn(new ResourceMetadataCollection(Book::class, [ + (new ApiResource())->withOperations(new Operations([$itemOpName => $itemOp])), + ])); + + $iriConverter = $this->createIriConverter($router, $resourceMetadataFactory); + + // No operation argument: the path every relation and every collection item's @id takes. + $context = ['uri_variables' => ['id' => 1]]; + $this->assertSame('/api/books/1', $iriConverter->getIriFromResource(Book::class, UrlGeneratorInterface::ABS_PATH, null, $context)); + $this->assertSame('/api/books/1', $iriConverter->getIriFromResource(Book::class, UrlGeneratorInterface::ABS_PATH, null, $context)); + } + + public function testGetIriFromResourceWithoutOperationReusesTheCachedOperation(): void + { + $cachedOp = (new Get())->withName('cached_op')->withClass(Book::class); + $staleOp = (new Get())->withName('stale_op')->withClass(Book::class); + + $router = $this->createMock(RouterInterface::class); + $router->method('generate') + ->willReturnCallback(fn (string $routeName): string => 'cached_op' === $routeName ? '/api/books/1' : '/api/stale/1'); + + // Hand back a different operation on a hypothetical second call: if the cache is not + // read, the second IRI visibly changes. + $resourceMetadataFactory = $this->createMock(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataFactory->method('create')->willReturnOnConsecutiveCalls( + new ResourceMetadataCollection(Book::class, [(new ApiResource())->withOperations(new Operations(['cached_op' => $cachedOp]))]), + new ResourceMetadataCollection(Book::class, [(new ApiResource())->withOperations(new Operations(['stale_op' => $staleOp]))]), + ); + + $iriConverter = $this->createIriConverter($router, $resourceMetadataFactory); + + $context = ['uri_variables' => ['id' => 1]]; + $this->assertSame('/api/books/1', $iriConverter->getIriFromResource(Book::class, UrlGeneratorInterface::ABS_PATH, null, $context)); + $this->assertSame('/api/books/1', $iriConverter->getIriFromResource(Book::class, UrlGeneratorInterface::ABS_PATH, null, $context)); + } + + public function testLocalOperationCacheDistinguishesItemAndCollectionAcrossCacheReads(): void + { + $collectionOpName = 'collection_op'; + $itemOpName = 'item_op'; + + $collectionOp = (new GetCollection())->withName($collectionOpName)->withClass(Book::class); + $itemOp = (new Get())->withName($itemOpName)->withClass(Book::class); + + $router = $this->createMock(RouterInterface::class); + $router->expects($this->exactly(4)) + ->method('generate') + ->willReturnCallback(function (string $routeName) use ($collectionOpName, $itemOpName): string { + if ($collectionOpName === $routeName) { + return '/api/books'; + } + if ($itemOpName === $routeName) { + return '/api/books/1'; + } + $this->fail(\sprintf('Unexpected route name "%s".', $routeName)); + }); + + // getOperation(null, $forceCollection, true) picks between the two according to $forceCollection. + $resourceMetadataFactory = $this->createMock(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataFactory->expects($this->exactly(2)) + ->method('create') + ->with(Book::class) + ->willReturn(new ResourceMetadataCollection(Book::class, [ + (new ApiResource())->withOperations(new Operations([ + $collectionOpName => $collectionOp, + $itemOpName => $itemOp, + ])), + ])); + + $iriConverter = $this->createIriConverter($router, $resourceMetadataFactory); + + // Both forms use a string resource, so only the item/collection part of the key differs. + // Interleaved on purpose: the third and fourth calls must read what the first two wrote. + $context = ['uri_variables' => ['id' => 1]]; + $this->assertSame('/api/books/1', $iriConverter->getIriFromResource(Book::class, UrlGeneratorInterface::ABS_PATH, null, $context)); + $this->assertSame('/api/books', $iriConverter->getIriFromResource(Book::class, UrlGeneratorInterface::ABS_PATH, new GetCollection())); + $this->assertSame('/api/books/1', $iriConverter->getIriFromResource(Book::class, UrlGeneratorInterface::ABS_PATH, null, $context)); + $this->assertSame('/api/books', $iriConverter->getIriFromResource(Book::class, UrlGeneratorInterface::ABS_PATH, new GetCollection())); + } + + private function createIriConverter(RouterInterface $router, ResourceMetadataCollectionFactoryInterface $resourceMetadataFactory): IriConverter + { + $resourceClassResolver = $this->createStub(ResourceClassResolverInterface::class); + $resourceClassResolver->method('isResourceClass')->willReturn(true); + + return new IriConverter( + $this->createStub(ProviderInterface::class), + $this->createStub(OperationMetadataFactoryInterface::class), + $router, + $this->createStub(IdentifiersExtractorInterface::class), + $resourceClassResolver, + $resourceMetadataFactory, + ); + } } diff --git a/src/Symfony/Routing/IriConverter.php b/src/Symfony/Routing/IriConverter.php index eb5ce19576f..00a353081a2 100644 --- a/src/Symfony/Routing/IriConverter.php +++ b/src/Symfony/Routing/IriConverter.php @@ -161,11 +161,16 @@ public function getIriFromResource(object|string $resource, int $referenceType = !$operation->getName() || ($operation instanceof HttpOperation && 'POST' === $operation->getMethod()) ) { - $forceCollection = $operation instanceof CollectionOperationInterface; - try { - $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null, $forceCollection, true); - $identifiersExtractorOperation = $operation; - } catch (OperationNotFoundException) { + if (isset($this->localOperationCache[$localOperationCacheKey])) { + $operation = $this->localOperationCache[$localOperationCacheKey]; + $identifiersExtractorOperation = $this->localIdentifiersExtractorOperationCache[$localOperationCacheKey] ?? $operation; + } else { + $forceCollection = $operation instanceof CollectionOperationInterface; + try { + $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null, $forceCollection, true); + $identifiersExtractorOperation = $operation; + } catch (OperationNotFoundException) { + } } } diff --git a/tests/Symfony/Routing/IriConverterTest.php b/tests/Symfony/Routing/IriConverterTest.php index 0e05743a303..b4be06edf49 100644 --- a/tests/Symfony/Routing/IriConverterTest.php +++ b/tests/Symfony/Routing/IriConverterTest.php @@ -110,6 +110,92 @@ public function testGetIriFromItemWithContextOperation(): void $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item, UrlGeneratorInterface::ABS_URL, $operation)); } + public function testGetIriFromItemWithoutOperationUsesTheLocalOperationCache(): void + { + $item = new Dummy(); + $item->setId(1); + + $operationName = 'operation_name'; + $operation = (new Get())->withName($operationName); + + $routerProphecy = $this->prophesize(RouterInterface::class); + $routerProphecy->generate($operationName, ['id' => 1], UrlGeneratorInterface::ABS_PATH)->shouldBeCalledTimes(2)->willReturn('/dummies/1'); + + $identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class); + $identifiersExtractorProphecy->getIdentifiersFromItem($item, $operation, Argument::any())->shouldBeCalledTimes(2)->willReturn(['id' => 1]); + + $resourceMetadataCollectionFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataCollectionFactoryProphecy->create(Dummy::class)->shouldBeCalledOnce()->willReturn(new ResourceMetadataCollection(Dummy::class, [ + (new ApiResource())->withOperations(new Operations([$operationName => $operation])), + ])); + + $iriConverter = $this->getIriConverter(null, $routerProphecy, $identifiersExtractorProphecy, $resourceMetadataCollectionFactoryProphecy); + + $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item)); + $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item)); + } + + public function testGetIriFromItemWithoutOperationReusesTheCachedOperation(): void + { + $item = new Dummy(); + $item->setId(1); + + $cachedOperation = (new Get())->withName('cached_operation'); + $staleOperation = (new Get())->withName('stale_operation'); + + $routerProphecy = $this->prophesize(RouterInterface::class); + $routerProphecy->generate('cached_operation', ['id' => 1], UrlGeneratorInterface::ABS_PATH)->willReturn('/dummies/1'); + $routerProphecy->generate('stale_operation', ['id' => 1], UrlGeneratorInterface::ABS_PATH)->willReturn('/stale/1'); + + $identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class); + $identifiersExtractorProphecy->getIdentifiersFromItem($item, Argument::cetera())->willReturn(['id' => 1]); + + // Prophecy returns these in order across consecutive calls, repeating the last. + $resourceMetadataCollectionFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataCollectionFactoryProphecy->create(Dummy::class)->willReturn( + new ResourceMetadataCollection(Dummy::class, [(new ApiResource())->withOperations(new Operations(['cached_operation' => $cachedOperation]))]), + new ResourceMetadataCollection(Dummy::class, [(new ApiResource())->withOperations(new Operations(['stale_operation' => $staleOperation]))]), + ); + + $iriConverter = $this->getIriConverter(null, $routerProphecy, $identifiersExtractorProphecy, $resourceMetadataCollectionFactoryProphecy); + + $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item)); + $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item)); + } + + public function testLocalOperationCacheDistinguishesItemAndCollectionIris(): void + { + $item = new Dummy(); + $item->setId(1); + + $itemOperation = (new Get())->withName('item_operation')->withClass(Dummy::class); + $collectionOperation = (new GetCollection())->withName('collection_operation')->withClass(Dummy::class); + + $routerProphecy = $this->prophesize(RouterInterface::class); + $routerProphecy->generate('item_operation', ['id' => 1], UrlGeneratorInterface::ABS_PATH)->shouldBeCalledTimes(2)->willReturn('/dummies/1'); + $routerProphecy->generate('collection_operation', [], UrlGeneratorInterface::ABS_PATH)->shouldBeCalledTimes(2)->willReturn('/dummies'); + + $identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class); + $identifiersExtractorProphecy->getIdentifiersFromItem($item, Argument::cetera())->willReturn(['id' => 1]); + + // getOperation(null, $forceCollection, true) picks between the two according to $forceCollection. + $resourceMetadataCollectionFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataCollectionFactoryProphecy->create(Dummy::class)->shouldBeCalledTimes(2)->willReturn(new ResourceMetadataCollection(Dummy::class, [ + (new ApiResource())->withOperations(new Operations([ + 'item_operation' => $itemOperation, + 'collection_operation' => $collectionOperation, + ])), + ])); + + $iriConverter = $this->getIriConverter(null, $routerProphecy, $identifiersExtractorProphecy, $resourceMetadataCollectionFactoryProphecy); + + // Interleaved on purpose: the third and fourth calls must read the cache entry the first two wrote. + $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item)); + $this->assertSame('/dummies', $iriConverter->getIriFromResource(Dummy::class, UrlGeneratorInterface::ABS_PATH, new GetCollection())); + $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item)); + $this->assertSame('/dummies', $iriConverter->getIriFromResource(Dummy::class, UrlGeneratorInterface::ABS_PATH, new GetCollection())); + } + public function testGetIriFromItemWithNoOperations(): void { $this->expectExceptionMessage(\sprintf('Unable to generate an IRI for the item of type "%s"', Dummy::class)); From 22f2dcb375213aed7fe82662bf21c812ca56370a Mon Sep 17 00:00:00 2001 From: Johannes Werbrouck Date: Mon, 24 Aug 2026 08:41:46 +0200 Subject: [PATCH 3/3] fix(serializer): localOperationCache is always written but never read --- src/Laravel/Tests/Unit/Routing/IriConverterTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Laravel/Tests/Unit/Routing/IriConverterTest.php b/src/Laravel/Tests/Unit/Routing/IriConverterTest.php index 83bfc744087..16c28394656 100644 --- a/src/Laravel/Tests/Unit/Routing/IriConverterTest.php +++ b/src/Laravel/Tests/Unit/Routing/IriConverterTest.php @@ -121,7 +121,7 @@ public function testGetIriFromResourceWithoutOperationReusesTheCachedOperation() $router = $this->createMock(RouterInterface::class); $router->method('generate') - ->willReturnCallback(fn (string $routeName): string => 'cached_op' === $routeName ? '/api/books/1' : '/api/stale/1'); + ->willReturnCallback(static fn (string $routeName): string => 'cached_op' === $routeName ? '/api/books/1' : '/api/stale/1'); // Hand back a different operation on a hypothetical second call: if the cache is not // read, the second IRI visibly changes.