From ba270351842e1d5118fd21deedca70b5cf77b76b Mon Sep 17 00:00:00 2001 From: Vincent Chalamon <407859+vincentchalamon@users.noreply.github.com> Date: Tue, 11 Jul 2023 20:00:09 +0200 Subject: [PATCH 1/3] fix(symfony): fix Symfony IriConverter with item_uri_template --- features/hydra/item_uri_template.feature | 45 +++++++++++- src/JsonLd/Serializer/ItemNormalizer.php | 3 + src/Symfony/Routing/IriConverter.php | 23 +++--- .../TestBundle/Entity/Issue5662/Book.php | 51 +++++++++++++ .../TestBundle/Entity/Issue5662/Review.php | 71 +++++++++++++++++++ tests/Symfony/Routing/IriConverterTest.php | 46 +++++++++--- 6 files changed, 217 insertions(+), 22 deletions(-) create mode 100644 tests/Fixtures/TestBundle/Entity/Issue5662/Book.php create mode 100644 tests/Fixtures/TestBundle/Entity/Issue5662/Review.php diff --git a/features/hydra/item_uri_template.feature b/features/hydra/item_uri_template.feature index a1663297d95..5652215b261 100644 --- a/features/hydra/item_uri_template.feature +++ b/features/hydra/item_uri_template.feature @@ -131,8 +131,7 @@ Feature: Exposing a collection of objects should use the specified operation to """ Scenario: Get a collection referencing another resource for its IRI - When I add "Content-Type" header equal to "application/json" - And I send a "GET" request to "/item_referenced_in_collection" + When I send a "GET" request to "/item_referenced_in_collection" Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8" @@ -159,3 +158,45 @@ Feature: Exposing a collection of objects should use the specified operation to "hydra:totalItems":2 } """ + + Scenario: Get a collection referencing an itemUriTemplate + When I send a "GET" request to "/issue5662/books/a/reviews" + Then the response status code should be 200 + And the response should be in JSON + And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8" + And the JSON should be equal to: + """ + { + "@context":"/contexts/Review", + "@id":"/issue5662/books/a/reviews", + "@type":"hydra:Collection", + "hydra:member":[ + { + "@id":"/issue5662/books/a/reviews/1", + "@type":"Review", + "book":"/issue5662/books/a", + "id":1, + "body":"Best book ever!" + }, + { + "@id":"/issue5662/books/b/reviews/2", + "@type":"Review", + "book":"/issue5662/books/b", + "id":2, + "body":"Worst book ever!" + } + ], + "hydra:totalItems":2 + } + """ + + Scenario: Get a collection referencing an invalid itemUriTemplate + When I send a "GET" request to "/issue5662/admin/reviews" + Then the response status code should be 400 + And the response should be in JSON + And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8" + And the JSON node "@context" should be equal to "/contexts/Error" + And the JSON node "@type" should be equal to "hydra:Error" + And the JSON node "hydra:title" should be equal to "An error occurred" + And the JSON node "hydra:description" should be equal to 'Unable to find operation "/issue5662/reviews/{id}{._format}"' + And the JSON node "trace" should exist diff --git a/src/JsonLd/Serializer/ItemNormalizer.php b/src/JsonLd/Serializer/ItemNormalizer.php index e0cc8a171ed..ffe122b1461 100644 --- a/src/JsonLd/Serializer/ItemNormalizer.php +++ b/src/JsonLd/Serializer/ItemNormalizer.php @@ -110,6 +110,9 @@ public function normalize(mixed $object, string $format = null, array $context = $context['api_normalize'] = true; + /* @see https://github.com/api-platform/core/pull/5663 */ + unset($context['item_uri_template']); + $data = parent::normalize($object, $format, $context); if (!\is_array($data)) { return $data; diff --git a/src/Symfony/Routing/IriConverter.php b/src/Symfony/Routing/IriConverter.php index 91a580c3aee..a6a8cd96df9 100644 --- a/src/Symfony/Routing/IriConverter.php +++ b/src/Symfony/Routing/IriConverter.php @@ -50,7 +50,6 @@ final class IriConverter implements IriConverterInterface use UriVariablesResolverTrait; private $localOperationCache = []; - private $localIdentifiersExtractorOperationCache = []; public function __construct(private readonly ProviderInterface $provider, private readonly RouterInterface $router, private readonly IdentifiersExtractorInterface $identifiersExtractor, ResourceClassResolverInterface $resourceClassResolver, private readonly ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory, UriVariablesConverterInterface $uriVariablesConverter = null, private readonly ?IriConverterInterface $decorated = null, private readonly ?OperationMetadataFactoryInterface $operationMetadataFactory = null) { @@ -108,7 +107,7 @@ public function getIriFromResource(object|string $resource, int $referenceType = $localOperationCacheKey = ($operation?->getName() ?? '').$resourceClass.(\is_string($resource) ? '_c' : '_i'); if ($operation && isset($this->localOperationCache[$localOperationCacheKey])) { - return $this->generateSymfonyRoute($resource, $referenceType, $this->localOperationCache[$localOperationCacheKey], $context, $this->localIdentifiersExtractorOperationCache[$localOperationCacheKey] ?? null); + return $this->generateSymfonyRoute($resource, $referenceType, $this->localOperationCache[$localOperationCacheKey], $context); } if (!$this->resourceClassResolver->isResourceClass($resourceClass)) { @@ -129,10 +128,11 @@ public function getIriFromResource(object|string $resource, int $referenceType = unset($context['uri_variables']); } - $identifiersExtractorOperation = $operation; if ($this->operationMetadataFactory && isset($context['item_uri_template'])) { - $identifiersExtractorOperation = null; $operation = $this->operationMetadataFactory->create($context['item_uri_template']); + if (!$operation) { + throw new InvalidArgumentException(sprintf('Unable to find operation "%s"', $context['item_uri_template'])); + } } // In symfony the operation name is the route name, try to find one if none provided @@ -143,7 +143,6 @@ public function getIriFromResource(object|string $resource, int $referenceType = $forceCollection = $operation instanceof CollectionOperationInterface; try { $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null, $forceCollection, true); - $identifiersExtractorOperation = $operation; } catch (OperationNotFoundException) { } } @@ -153,9 +152,8 @@ public function getIriFromResource(object|string $resource, int $referenceType = } $this->localOperationCache[$localOperationCacheKey] = $operation; - $this->localIdentifiersExtractorOperationCache[$localOperationCacheKey] = $identifiersExtractorOperation; - return $this->generateSymfonyRoute($resource, $referenceType, $operation, $context, $identifiersExtractorOperation); + return $this->generateSymfonyRoute($resource, $referenceType, $operation, $context); } private function generateSkolemIri(object|string $resource, int $referenceType = UrlGeneratorInterface::ABS_PATH, Operation $operation = null, array $context = [], string $resourceClass = null): string @@ -168,17 +166,22 @@ private function generateSkolemIri(object|string $resource, int $referenceType = return $this->decorated->getIriFromResource($resource, $referenceType, $operation, $context); } - private function generateSymfonyRoute(object|string $resource, int $referenceType = UrlGeneratorInterface::ABS_PATH, Operation $operation = null, array $context = [], Operation $identifiersExtractorOperation = null): string + private function generateSymfonyRoute(object|string $resource, int $referenceType = UrlGeneratorInterface::ABS_PATH, Operation $operation = null, array $context = []): string { $identifiers = $context['uri_variables'] ?? []; if (\is_object($resource)) { try { - $identifiers = $this->identifiersExtractor->getIdentifiersFromItem($resource, $identifiersExtractorOperation, $context); + $identifiers = $this->identifiersExtractor->getIdentifiersFromItem($resource, $operation, $context); } catch (InvalidArgumentException|RuntimeException $e) { // We can try using context uri variables if any if (!$identifiers) { - throw new InvalidArgumentException(sprintf('Unable to generate an IRI for the item of type "%s"', $operation->getClass()), $e->getCode(), $e); + // Try with the first operation found + try { + $identifiers = $this->identifiersExtractor->getIdentifiersFromItem($resource, null, $context); + } catch (InvalidArgumentException|RuntimeException $e) { + throw new InvalidArgumentException(sprintf('Unable to generate an IRI for the item of type "%s"', $operation->getClass()), $e->getCode(), $e); + } } } } diff --git a/tests/Fixtures/TestBundle/Entity/Issue5662/Book.php b/tests/Fixtures/TestBundle/Entity/Issue5662/Book.php new file mode 100644 index 00000000000..322ae4a58ae --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/Issue5662/Book.php @@ -0,0 +1,51 @@ + + * + * 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\Entity\Issue5662; + +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\GetCollection; +use ApiPlatform\Metadata\Operation; + +#[GetCollection( + uriTemplate: '/issue5662/books{._format}', + itemUriTemplate: '/issue5662/books/{id}{._format}', + provider: [Book::class, 'getData'] +)] +#[Get( + uriTemplate: '/issue5662/books/{id}{._format}', + provider: [Book::class, 'getDatum'] +)] +class Book +{ + public function __construct(public string $id, public string $title) + { + } + + public static function getData(Operation $operation, array $uriVariables = [], array $context = []): object|array|null + { + return [new self('a', 'hello'), new self('b', 'you')]; + } + + public static function getDatum(Operation $operation, array $uriVariables = [], array $context = []): self|null + { + $id = $uriVariables['id']; + foreach (static::getData($operation, $uriVariables, $context) as $datum) { + if ($id === $datum->id) { + return $datum; + } + } + + return null; + } +} diff --git a/tests/Fixtures/TestBundle/Entity/Issue5662/Review.php b/tests/Fixtures/TestBundle/Entity/Issue5662/Review.php new file mode 100644 index 00000000000..6d37f2bad6a --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/Issue5662/Review.php @@ -0,0 +1,71 @@ + + * + * 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\Entity\Issue5662; + +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\GetCollection; +use ApiPlatform\Metadata\Link; +use ApiPlatform\Metadata\Operation; + +#[GetCollection( + uriTemplate: '/issue5662/admin/reviews{._format}', + itemUriTemplate: '/issue5662/reviews/{id}{._format}', + provider: [Review::class, 'getData'] +)] +#[Get( + uriTemplate: '/issue5662/admin/reviews/{id}{._format}', + provider: [Review::class, 'getDatum'] +)] +#[GetCollection( + uriTemplate: '/issue5662/books/{bookId}/reviews{._format}', + itemUriTemplate: '/issue5662/books/{bookId}/reviews/{id}{._format}', + provider: [Review::class, 'getData'], + uriVariables: [ + 'bookId' => new Link(toProperty: 'book', fromClass: Book::class), + ] +)] +#[Get( + uriTemplate: '/issue5662/books/{bookId}/reviews/{id}{._format}', + provider: [Review::class, 'getDatum'], + uriVariables: [ + 'bookId' => new Link(toProperty: 'book', fromClass: Book::class), + 'id' => new Link(fromClass: Review::class), + ] +)] +class Review +{ + public function __construct(public Book $book, public int $id, public string $body) + { + } + + public static function getData(Operation $operation, array $uriVariables = [], array $context = []): object|array|null + { + return [ + new self(Book::getDatum($operation, ['id' => 'a'], $context), 1, 'Best book ever!'), + new self(Book::getDatum($operation, ['id' => 'b'], $context), 2, 'Worst book ever!'), + ]; + } + + public static function getDatum(Operation $operation, array $uriVariables = [], array $context = []): self|null + { + $id = (int) $uriVariables['id']; + foreach (static::getData($operation, $uriVariables, $context) as $datum) { + if ($id === $datum->id) { + return $datum; + } + } + + return null; + } +} diff --git a/tests/Symfony/Routing/IriConverterTest.php b/tests/Symfony/Routing/IriConverterTest.php index 66f06b392da..25b4e8554d2 100644 --- a/tests/Symfony/Routing/IriConverterTest.php +++ b/tests/Symfony/Routing/IriConverterTest.php @@ -32,6 +32,7 @@ use ApiPlatform\Symfony\Routing\IriConverter; use ApiPlatform\Symfony\Routing\SkolemIriConverter; use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Dummy; +use ApiPlatform\Tests\Fixtures\TestBundle\Entity\RelatedDummy; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; @@ -53,15 +54,15 @@ public function testGetIriFromItemWithOperation(): void $routerProphecy = $this->prophesize(RouterInterface::class); $routerProphecy->generate($operationName, ['id' => 1], UrlGeneratorInterface::ABS_PATH)->shouldBeCalled()->willReturn('/dummies/1'); - $identifiersExtractyorProphecy = $this->prophesize(IdentifiersExtractorInterface::class); - $identifiersExtractyorProphecy->getIdentifiersFromItem($item, $operation, Argument::any())->shouldBeCalled()->willReturn(['id' => 1]); + $identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class); + $identifiersExtractorProphecy->getIdentifiersFromItem($item, $operation, Argument::any())->shouldBeCalled()->willReturn(['id' => 1]); $resourceMetadataCollectionFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class); $resourceMetadataCollectionFactoryProphecy->create(Dummy::class)->shouldNotBeCalled()->willReturn(new ResourceMetadataCollection(Dummy::class, [ (new ApiResource())->withOperations(new Operations([$operationName => $operation])), ])); - $iriConverter = $this->getIriConverter(null, $routerProphecy, $identifiersExtractyorProphecy, $resourceMetadataCollectionFactoryProphecy); + $iriConverter = $this->getIriConverter(null, $routerProphecy, $identifiersExtractorProphecy, $resourceMetadataCollectionFactoryProphecy); $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item, UrlGeneratorInterface::ABS_PATH, $operation)); } @@ -76,15 +77,15 @@ public function testGetIriFromItemWithoutOperation(): void $routerProphecy = $this->prophesize(RouterInterface::class); $routerProphecy->generate($operationName, ['id' => 1], UrlGeneratorInterface::ABS_PATH)->shouldBeCalled()->willReturn('/dummies/1'); - $identifiersExtractyorProphecy = $this->prophesize(IdentifiersExtractorInterface::class); - $identifiersExtractyorProphecy->getIdentifiersFromItem($item, $operation, Argument::any())->shouldBeCalled()->willReturn(['id' => 1]); + $identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class); + $identifiersExtractorProphecy->getIdentifiersFromItem($item, $operation, Argument::any())->shouldBeCalled()->willReturn(['id' => 1]); $resourceMetadataCollectionFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class); $resourceMetadataCollectionFactoryProphecy->create(Dummy::class)->shouldBeCalled()->willReturn(new ResourceMetadataCollection(Dummy::class, [ (new ApiResource())->withOperations(new Operations([$operationName => $operation])), ])); - $iriConverter = $this->getIriConverter(null, $routerProphecy, $identifiersExtractyorProphecy, $resourceMetadataCollectionFactoryProphecy); + $iriConverter = $this->getIriConverter(null, $routerProphecy, $identifiersExtractorProphecy, $resourceMetadataCollectionFactoryProphecy); $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item)); } @@ -99,13 +100,13 @@ public function testGetIriFromItemWithContextOperation(): void $routerProphecy = $this->prophesize(RouterInterface::class); $routerProphecy->generate($operationName, ['id' => 1], UrlGeneratorInterface::ABS_URL)->shouldBeCalled()->willReturn('/dummies/1'); - $identifiersExtractyorProphecy = $this->prophesize(IdentifiersExtractorInterface::class); - $identifiersExtractyorProphecy->getIdentifiersFromItem($item, $operation, Argument::any())->shouldBeCalled()->willReturn(['id' => 1]); + $identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class); + $identifiersExtractorProphecy->getIdentifiersFromItem($item, $operation, Argument::any())->shouldBeCalled()->willReturn(['id' => 1]); $resourceMetadataCollectionFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class); $resourceMetadataCollectionFactoryProphecy->create(Dummy::class)->shouldNotBeCalled(); - $iriConverter = $this->getIriConverter(null, $routerProphecy, $identifiersExtractyorProphecy, $resourceMetadataCollectionFactoryProphecy); + $iriConverter = $this->getIriConverter(null, $routerProphecy, $identifiersExtractorProphecy, $resourceMetadataCollectionFactoryProphecy); $this->assertSame('/dummies/1', $iriConverter->getIriFromResource($item, UrlGeneratorInterface::ABS_URL, $operation)); } @@ -208,7 +209,32 @@ public function testGetIriFromResourceClassWithoutOperation(): void $this->assertSame('/dummies/1/foo', $iriConverter->getIriFromResource(Dummy::class, UrlGeneratorInterface::ABS_URL, $operation, ['uri_variables' => ['id' => 1]])); } - public function testGetIriFromResourceClassWithItemUriTemplate(): void + public function testGetIriFromItemWithItemUriTemplate(): void + { + $item = new Dummy(); + $item->setId(1); + + $operation = new GetCollection(name: 'operation_name', class: Dummy::class, itemUriTemplate: '/dummies/another/{id}{._format}'); + $anotherOperation = new Get(name: 'another_name', uriTemplate: '/dummies/{relatedId}/another/{id}{._format}', uriVariables: [ + 'relatedId' => new Link(fromClass: RelatedDummy::class, toProperty: 'id'), + 'id' => new Link(fromClass: Dummy::class), + ]); + + $routerProphecy = $this->prophesize(RouterInterface::class); + $routerProphecy->generate('another_name', ['id' => 1, 'relatedId' => 6], UrlGeneratorInterface::ABS_URL)->shouldBeCalled()->willReturn('/dummies/6/another/1'); + + $identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class); + $identifiersExtractorProphecy->getIdentifiersFromItem($item, $anotherOperation, Argument::any())->shouldBeCalled()->willReturn(['id' => 1, 'relatedId' => 6]); + + $resourceMetadataCollectionFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class); + + $operationMetadataFactoryProphecy = $this->prophesize(OperationMetadataFactoryInterface::class); + $operationMetadataFactoryProphecy->create('/dummies/{relatedId}/another/{id}{._format}')->willReturn($anotherOperation); + $iriConverter = $this->getIriConverter(null, $routerProphecy, $identifiersExtractorProphecy, $resourceMetadataCollectionFactoryProphecy, null, null, $operationMetadataFactoryProphecy); + $this->assertSame('/dummies/6/another/1', $iriConverter->getIriFromResource($item, UrlGeneratorInterface::ABS_URL, $operation, ['item_uri_template' => '/dummies/{relatedId}/another/{id}{._format}'])); + } + + public function testGetIriFromResourceClassWithItemUriTemplateAndUriVariables(): void { $operation = new GetCollection(name: 'operation_name', class: Dummy::class, itemUriTemplate: '/dummies/another/{id}{._format}'); $anotherOperation = new Get(name: 'another_name', uriTemplate: '/dummies/another/{id}{._format}'); From 4b8a3ce5713428f625527609996d60cba23304c8 Mon Sep 17 00:00:00 2001 From: soyuka Date: Mon, 14 Aug 2023 16:24:26 +0200 Subject: [PATCH 2/3] go further on that fix --- features/hydra/item_uri_template.feature | 32 +++++++++--- src/Hydra/Serializer/CollectionNormalizer.php | 20 +------- src/JsonLd/Serializer/ItemNormalizer.php | 7 --- .../AbstractCollectionNormalizer.php | 24 ++------- src/Serializer/AbstractItemNormalizer.php | 34 ++----------- src/Serializer/OperationContextTrait.php | 50 +++++++++++++++++++ src/Serializer/SerializerContextBuilder.php | 6 +++ src/Symfony/Routing/IriConverter.php | 30 +++++------ .../CompositeKeyWithDifferentType.php | 8 ++- .../ApiResource/Issue5648/DummyResource.php | 3 +- .../Entity/ItemReferencedInCollection.php | 3 +- .../Serializer/AbstractItemNormalizerTest.php | 1 - 12 files changed, 116 insertions(+), 102 deletions(-) create mode 100644 src/Serializer/OperationContextTrait.php diff --git a/features/hydra/item_uri_template.feature b/features/hydra/item_uri_template.feature index 5652215b261..2cb5a57443b 100644 --- a/features/hydra/item_uri_template.feature +++ b/features/hydra/item_uri_template.feature @@ -192,11 +192,31 @@ Feature: Exposing a collection of objects should use the specified operation to Scenario: Get a collection referencing an invalid itemUriTemplate When I send a "GET" request to "/issue5662/admin/reviews" - Then the response status code should be 400 + Then the response status code should be 200 And the response should be in JSON And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8" - And the JSON node "@context" should be equal to "/contexts/Error" - And the JSON node "@type" should be equal to "hydra:Error" - And the JSON node "hydra:title" should be equal to "An error occurred" - And the JSON node "hydra:description" should be equal to 'Unable to find operation "/issue5662/reviews/{id}{._format}"' - And the JSON node "trace" should exist + And the JSON should be equal to: + """ + { + "@context": "/contexts/Review", + "@id": "/issue5662/admin/reviews", + "@type": "hydra:Collection", + "hydra:totalItems": 2, + "hydra:member": [ + { + "@id": "/issue5662/admin/reviews/1", + "@type": "Review", + "book": "/issue5662/books/a", + "id": 1, + "body": "Best book ever!" + }, + { + "@id": "/issue5662/admin/reviews/2", + "@type": "Review", + "book": "/issue5662/books/b", + "id": 2, + "body": "Worst book ever!" + } + ] + } + """ diff --git a/src/Hydra/Serializer/CollectionNormalizer.php b/src/Hydra/Serializer/CollectionNormalizer.php index 05effc13354..f472fb486cd 100644 --- a/src/Hydra/Serializer/CollectionNormalizer.php +++ b/src/Hydra/Serializer/CollectionNormalizer.php @@ -39,7 +39,7 @@ final class CollectionNormalizer extends AbstractCollectionNormalizer self::IRI_ONLY => false, ]; - public function __construct(private readonly ContextBuilderInterface $contextBuilder, ResourceClassResolverInterface $resourceClassResolver, private readonly IriConverterInterface $iriConverter, private readonly ?ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory = null, array $defaultContext = []) + public function __construct(private readonly ContextBuilderInterface $contextBuilder, ResourceClassResolverInterface $resourceClassResolver, private readonly IriConverterInterface $iriConverter, readonly ?ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory = null, array $defaultContext = []) { $this->defaultContext = array_merge($this->defaultContext, $defaultContext); @@ -80,26 +80,8 @@ protected function getItemsData(iterable $object, string $format = null, array $ { $data = []; $data['hydra:member'] = []; - $iriOnly = $context[self::IRI_ONLY] ?? $this->defaultContext[self::IRI_ONLY]; - if (($operation = $context['operation'] ?? null) && method_exists($operation, 'getItemUriTemplate')) { - $context['item_uri_template'] = $operation->getItemUriTemplate(); - } - - // We need to keep this operation for serialization groups for later - if (isset($context['operation'])) { - $context['root_operation'] = $context['operation']; - } - - if (isset($context['operation_name'])) { - $context['root_operation_name'] = $context['operation_name']; - } - - // We need to unset the operation to ensure a proper IRI generation inside items - unset($context['operation']); - unset($context['operation_name'], $context['uri_variables']); - foreach ($object as $obj) { if ($iriOnly) { $data['hydra:member'][] = $this->iriConverter->getIriFromResource($obj); diff --git a/src/JsonLd/Serializer/ItemNormalizer.php b/src/JsonLd/Serializer/ItemNormalizer.php index ffe122b1461..3b662a1387d 100644 --- a/src/JsonLd/Serializer/ItemNormalizer.php +++ b/src/JsonLd/Serializer/ItemNormalizer.php @@ -99,10 +99,6 @@ public function normalize(mixed $object, string $format = null, array $context = unset($context['operation'], $context['operation_name']); } - if (($operation = $context['operation'] ?? null) && method_exists($operation, 'getItemUriTemplate') && ($itemUriTemplate = $operation->getItemUriTemplate())) { - $context['item_uri_template'] = $itemUriTemplate; - } - if ($iri = $this->iriConverter->getIriFromResource($object, UrlGeneratorInterface::ABS_PATH, $context['operation'] ?? null, $context)) { $context['iri'] = $iri; $metadata['@id'] = $iri; @@ -110,9 +106,6 @@ public function normalize(mixed $object, string $format = null, array $context = $context['api_normalize'] = true; - /* @see https://github.com/api-platform/core/pull/5663 */ - unset($context['item_uri_template']); - $data = parent::normalize($object, $format, $context); if (!\is_array($data)) { return $data; diff --git a/src/Serializer/AbstractCollectionNormalizer.php b/src/Serializer/AbstractCollectionNormalizer.php index 9397f4090d7..9d9f439ab8d 100644 --- a/src/Serializer/AbstractCollectionNormalizer.php +++ b/src/Serializer/AbstractCollectionNormalizer.php @@ -34,6 +34,7 @@ abstract class AbstractCollectionNormalizer implements NormalizerInterface, Norm initContext as protected; } use NormalizerAwareTrait; + use OperationContextTrait; /** * This constant must be overridden in the child class. @@ -96,27 +97,12 @@ public function normalize(mixed $object, string $format = null, array $context = } $resourceClass = $this->resourceClassResolver->getResourceClass($object, $context['resource_class']); - $context = $this->initContext($resourceClass, $context); + $collectionContext = $this->initContext($resourceClass, $context); $data = []; - $paginationData = $this->getPaginationData($object, $context); + $paginationData = $this->getPaginationData($object, $collectionContext); - if (($operation = $context['operation'] ?? null) && method_exists($operation, 'getItemUriTemplate')) { - $context['item_uri_template'] = $operation->getItemUriTemplate(); - } - - // We need to keep this operation for serialization groups for later - if (isset($context['operation'])) { - $context['root_operation'] = $context['operation']; - } - - if (isset($context['operation_name'])) { - $context['root_operation_name'] = $context['operation_name']; - } - - unset($context['operation']); - unset($context['operation_type'], $context['operation_name']); - - $itemsData = $this->getItemsData($object, $format, $context); + $childContext = $this->createOperationContext($collectionContext, $resourceClass); + $itemsData = $this->getItemsData($object, $format, $childContext); return array_merge_recursive($data, $paginationData, $itemsData); } diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php index 6120e93266f..ebcd72982a2 100644 --- a/src/Serializer/AbstractItemNormalizer.php +++ b/src/Serializer/AbstractItemNormalizer.php @@ -20,7 +20,6 @@ use ApiPlatform\Exception\ItemNotFoundException; use ApiPlatform\Metadata\ApiProperty; use ApiPlatform\Metadata\CollectionOperationInterface; -use ApiPlatform\Metadata\Exception\OperationNotFoundException; use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface; use ApiPlatform\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface; use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; @@ -56,6 +55,7 @@ abstract class AbstractItemNormalizer extends AbstractObjectNormalizer use CloneTrait; use ContextTrait; use InputOutputMetadataTrait; + use OperationContextTrait; protected PropertyAccessorInterface $propertyAccessor; protected array $localCache = []; @@ -134,6 +134,8 @@ public function normalize(mixed $object, string $format = null, array $context = return $this->serializer->normalize($object, $format, $context); } + // Never remove this, with `application/json` we don't use our AbstractCollectionNormalizer and we need + // to remove the collection operation from our context or we'll introduce security issues if (isset($context['operation']) && $context['operation'] instanceof CollectionOperationInterface) { unset($context['operation_name']); unset($context['operation']); @@ -590,7 +592,7 @@ protected function getFactoryOptions(array $context): array if (!$operation && $this->resourceMetadataCollectionFactory && $this->resourceClassResolver->isResourceClass($context['resource_class'])) { $resourceClass = $this->resourceClassResolver->getResourceClass(null, $context['resource_class']); // fix for abstract classes and interfaces - $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation($context['root_operation_name'] ?? $context['operation_name'] ?? null); + $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null); } if ($operation) { @@ -716,8 +718,7 @@ protected function normalizeRelation(ApiProperty $propertyMetadata, ?object $rel throw new LogicException(sprintf('The injected serializer must be an instance of "%s".', NormalizerInterface::class)); } - $relatedContext = $context; - unset($relatedContext['force_resource_class']); + $relatedContext = $this->createOperationContext($context, $resourceClass); $normalizedRelatedObject = $this->serializer->normalize($relatedObject, $format, $relatedContext); if (!\is_string($normalizedRelatedObject) && !\is_array($normalizedRelatedObject) && !$normalizedRelatedObject instanceof \ArrayObject && null !== $normalizedRelatedObject) { throw new UnexpectedValueException('Expected normalized relation to be an IRI, array, \ArrayObject or null'); @@ -883,29 +884,4 @@ private function setValue(object $object, string $attributeName, mixed $value): // Properties not found are ignored } } - - private function createOperationContext(array $context, string $resourceClass = null): array - { - if (isset($context['operation']) && !isset($context['root_operation'])) { - $context['root_operation'] = $context['operation']; - $context['root_operation_name'] = $context['operation_name']; - } - - unset($context['iri'], $context['uri_variables']); - if (!$resourceClass) { - return $context; - } - - unset($context['operation'], $context['operation_name']); - $context['resource_class'] = $resourceClass; - if ($this->resourceMetadataCollectionFactory) { - try { - $context['operation'] = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(); - $context['operation_name'] = $context['operation']->getName(); - } catch (OperationNotFoundException) { - } - } - - return $context; - } } diff --git a/src/Serializer/OperationContextTrait.php b/src/Serializer/OperationContextTrait.php new file mode 100644 index 00000000000..c0d188a7004 --- /dev/null +++ b/src/Serializer/OperationContextTrait.php @@ -0,0 +1,50 @@ + + * + * 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\Serializer; + +/** + * @internal + */ +trait OperationContextTrait +{ + /** + * This context is created when working on a relation context or items of a collection. It cleans the previously given + * context as the operation changes. + */ + protected function createOperationContext(array $context, string $resourceClass = null): array + { + if (isset($context['operation']) && !isset($context['root_operation'])) { + $context['root_operation'] = $context['operation']; + } + + if (isset($context['operation_name']) || isset($context['graphql_operation_name'])) { + $context['root_operation_name'] = $context['operation_name'] ?? $context['graphql_operation_name']; + } + + unset($context['iri'], $context['uri_variables'], $context['item_uri_template'], $context['force_resource_class']); + + if (!$resourceClass) { + return $context; + } + + if (($operation = $context['operation'] ?? null) && method_exists($operation, 'getItemUriTemplate')) { + $context['item_uri_template'] = $operation->getItemUriTemplate(); + } + + unset($context['operation'], $context['operation_name']); + $context['resource_class'] = $resourceClass; + + return $context; + } +} diff --git a/src/Serializer/SerializerContextBuilder.php b/src/Serializer/SerializerContextBuilder.php index 2fa07c3a1c0..af1432179b0 100644 --- a/src/Serializer/SerializerContextBuilder.php +++ b/src/Serializer/SerializerContextBuilder.php @@ -14,6 +14,7 @@ namespace ApiPlatform\Serializer; use ApiPlatform\Exception\RuntimeException; +use ApiPlatform\Metadata\CollectionOperationInterface; use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; use ApiPlatform\Util\RequestAttributesExtractor; use Symfony\Component\HttpFoundation\Request; @@ -53,6 +54,11 @@ public function createFromRequest(Request $request, bool $normalization, array $ $context['input'] = $operation->getInput(); $context['output'] = $operation->getOutput(); + // Special case as this is usually handled by our OperationContextTrait, here we want to force the IRI in the response + if (!$operation instanceof CollectionOperationInterface && method_exists($operation, 'getItemUriTemplate') && $operation->getItemUriTemplate()) { + $context['item_uri_template'] = $operation->getItemUriTemplate(); + } + if ($operation->getTypes()) { $context['types'] = $operation->getTypes(); } diff --git a/src/Symfony/Routing/IriConverter.php b/src/Symfony/Routing/IriConverter.php index a6a8cd96df9..a2f2a414014 100644 --- a/src/Symfony/Routing/IriConverter.php +++ b/src/Symfony/Routing/IriConverter.php @@ -50,6 +50,7 @@ final class IriConverter implements IriConverterInterface use UriVariablesResolverTrait; private $localOperationCache = []; + private $localIdentifiersExtractorOperationCache = []; public function __construct(private readonly ProviderInterface $provider, private readonly RouterInterface $router, private readonly IdentifiersExtractorInterface $identifiersExtractor, ResourceClassResolverInterface $resourceClassResolver, private readonly ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory, UriVariablesConverterInterface $uriVariablesConverter = null, private readonly ?IriConverterInterface $decorated = null, private readonly ?OperationMetadataFactoryInterface $operationMetadataFactory = null) { @@ -105,9 +106,13 @@ public function getIriFromResource(object|string $resource, int $referenceType = { $resourceClass = $context['force_resource_class'] ?? (\is_string($resource) ? $resource : $this->getObjectClass($resource)); + if ($this->operationMetadataFactory && isset($context['item_uri_template'])) { + $operation = $this->operationMetadataFactory->create($context['item_uri_template']); + } + $localOperationCacheKey = ($operation?->getName() ?? '').$resourceClass.(\is_string($resource) ? '_c' : '_i'); if ($operation && isset($this->localOperationCache[$localOperationCacheKey])) { - return $this->generateSymfonyRoute($resource, $referenceType, $this->localOperationCache[$localOperationCacheKey], $context); + return $this->generateSymfonyRoute($resource, $referenceType, $this->localOperationCache[$localOperationCacheKey], $context, $this->localIdentifiersExtractorOperationCache[$localOperationCacheKey] ?? null); } if (!$this->resourceClassResolver->isResourceClass($resourceClass)) { @@ -128,13 +133,7 @@ public function getIriFromResource(object|string $resource, int $referenceType = unset($context['uri_variables']); } - if ($this->operationMetadataFactory && isset($context['item_uri_template'])) { - $operation = $this->operationMetadataFactory->create($context['item_uri_template']); - if (!$operation) { - throw new InvalidArgumentException(sprintf('Unable to find operation "%s"', $context['item_uri_template'])); - } - } - + $identifiersExtractorOperation = $operation; // In symfony the operation name is the route name, try to find one if none provided if ( !$operation->getName() @@ -143,6 +142,7 @@ public function getIriFromResource(object|string $resource, int $referenceType = $forceCollection = $operation instanceof CollectionOperationInterface; try { $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null, $forceCollection, true); + $identifiersExtractorOperation = $operation; } catch (OperationNotFoundException) { } } @@ -152,8 +152,9 @@ public function getIriFromResource(object|string $resource, int $referenceType = } $this->localOperationCache[$localOperationCacheKey] = $operation; + $this->localIdentifiersExtractorOperationCache[$localOperationCacheKey] = $identifiersExtractorOperation; - return $this->generateSymfonyRoute($resource, $referenceType, $operation, $context); + return $this->generateSymfonyRoute($resource, $referenceType, $operation, $context, $identifiersExtractorOperation); } private function generateSkolemIri(object|string $resource, int $referenceType = UrlGeneratorInterface::ABS_PATH, Operation $operation = null, array $context = [], string $resourceClass = null): string @@ -166,22 +167,17 @@ private function generateSkolemIri(object|string $resource, int $referenceType = return $this->decorated->getIriFromResource($resource, $referenceType, $operation, $context); } - private function generateSymfonyRoute(object|string $resource, int $referenceType = UrlGeneratorInterface::ABS_PATH, Operation $operation = null, array $context = []): string + private function generateSymfonyRoute(object|string $resource, int $referenceType = UrlGeneratorInterface::ABS_PATH, Operation $operation = null, array $context = [], Operation $identifiersExtractorOperation = null): string { $identifiers = $context['uri_variables'] ?? []; if (\is_object($resource)) { try { - $identifiers = $this->identifiersExtractor->getIdentifiersFromItem($resource, $operation, $context); + $identifiers = $this->identifiersExtractor->getIdentifiersFromItem($resource, $identifiersExtractorOperation, $context); } catch (InvalidArgumentException|RuntimeException $e) { // We can try using context uri variables if any if (!$identifiers) { - // Try with the first operation found - try { - $identifiers = $this->identifiersExtractor->getIdentifiersFromItem($resource, null, $context); - } catch (InvalidArgumentException|RuntimeException $e) { - throw new InvalidArgumentException(sprintf('Unable to generate an IRI for the item of type "%s"', $operation->getClass()), $e->getCode(), $e); - } + throw new InvalidArgumentException(sprintf('Unable to generate an IRI for the item of type "%s"', $operation->getClass()), $e->getCode(), $e); } } } diff --git a/tests/Fixtures/TestBundle/ApiResource/Issue5396/CompositeKeyWithDifferentType.php b/tests/Fixtures/TestBundle/ApiResource/Issue5396/CompositeKeyWithDifferentType.php index 51919fbbefd..29f959a1fb9 100644 --- a/tests/Fixtures/TestBundle/ApiResource/Issue5396/CompositeKeyWithDifferentType.php +++ b/tests/Fixtures/TestBundle/ApiResource/Issue5396/CompositeKeyWithDifferentType.php @@ -26,12 +26,16 @@ class CompositeKeyWithDifferentType #[ApiProperty(identifier: true)] public ?string $verificationKey; - public static function provide(Operation $operation, array $uriVariables = [], array $context = []): array + public static function provide(Operation $operation, array $uriVariables = [], array $context = []): self { if (!\is_string($uriVariables['verificationKey'])) { throw new \RuntimeException('verificationKey should be a string.'); } - return $context; + $t = new self(); + $t->id = $uriVariables['id']; + $t->verificationKey = $uriVariables['verificationKey']; + + return $t; } } diff --git a/tests/Fixtures/TestBundle/ApiResource/Issue5648/DummyResource.php b/tests/Fixtures/TestBundle/ApiResource/Issue5648/DummyResource.php index c20a2142ade..21674757fc3 100644 --- a/tests/Fixtures/TestBundle/ApiResource/Issue5648/DummyResource.php +++ b/tests/Fixtures/TestBundle/ApiResource/Issue5648/DummyResource.php @@ -19,12 +19,13 @@ use ApiPlatform\Metadata\ApiResource; use ApiPlatform\Metadata\Get; use ApiPlatform\Metadata\GetCollection; +use ApiPlatform\Metadata\Link; use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Dummy; #[ApiResource( operations: [ new GetCollection(uriTemplate: '/dummy_resource_with_custom_filter', itemUriTemplate: '/dummy_resource_with_custom_filter/{id}'), - new Get(uriTemplate: '/dummy_resource_with_custom_filter/{id}', uriVariables: ['id']), + new Get(uriTemplate: '/dummy_resource_with_custom_filter/{id}', uriVariables: ['id' => new Link(fromClass: Dummy::class)]), ], stateOptions: new Options(entityClass: Dummy::class) )] diff --git a/tests/Fixtures/TestBundle/Entity/ItemReferencedInCollection.php b/tests/Fixtures/TestBundle/Entity/ItemReferencedInCollection.php index 03367318e27..db6687ccff4 100644 --- a/tests/Fixtures/TestBundle/Entity/ItemReferencedInCollection.php +++ b/tests/Fixtures/TestBundle/Entity/ItemReferencedInCollection.php @@ -14,8 +14,9 @@ namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity; use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\Link; -#[Get('/item_referenced_in_collection/{id}{._format}')] +#[Get('/item_referenced_in_collection/{id}{._format}', uriVariables: ['id' => new Link(fromClass: CollectionReferencingItem::class)])] class ItemReferencedInCollection { public $id; diff --git a/tests/Serializer/AbstractItemNormalizerTest.php b/tests/Serializer/AbstractItemNormalizerTest.php index 011936da0e0..66f074fdfec 100644 --- a/tests/Serializer/AbstractItemNormalizerTest.php +++ b/tests/Serializer/AbstractItemNormalizerTest.php @@ -627,7 +627,6 @@ public function testNormalizePolymorphicRelations(): void $serializerProphecy->willImplement(NormalizerInterface::class); $concreteDummyChildContext = Argument::allOf( Argument::type('array'), - Argument::withEntry('resource_class', DummyTableInheritanceChild::class), Argument::not(Argument::withKey('iri')) ); $serializerProphecy->normalize($concreteDummy, null, $concreteDummyChildContext)->willReturn(['foo' => 'concrete']); From 350bdc773864e54ab74d17e14887567f0d6bc03c Mon Sep 17 00:00:00 2001 From: soyuka Date: Tue, 22 Aug 2023 15:03:20 +0200 Subject: [PATCH 3/3] dead code --- src/Serializer/AbstractItemNormalizer.php | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php index ebcd72982a2..ac44b9f6f0c 100644 --- a/src/Serializer/AbstractItemNormalizer.php +++ b/src/Serializer/AbstractItemNormalizer.php @@ -588,14 +588,7 @@ protected function getFactoryOptions(array $context): array // This is a hot spot if (isset($context['resource_class'])) { // Note that the groups need to be read on the root operation - $operation = $context['root_operation'] ?? $context['operation'] ?? null; - - if (!$operation && $this->resourceMetadataCollectionFactory && $this->resourceClassResolver->isResourceClass($context['resource_class'])) { - $resourceClass = $this->resourceClassResolver->getResourceClass(null, $context['resource_class']); // fix for abstract classes and interfaces - $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null); - } - - if ($operation) { + if ($operation = ($context['root_operation'] ?? null)) { $options['normalization_groups'] = $operation->getNormalizationContext()['groups'] ?? null; $options['denormalization_groups'] = $operation->getDenormalizationContext()['groups'] ?? null; $options['operation_name'] = $operation->getName();