Skip to content

Commit

Permalink
fix(symfony): fix Symfony IriConverter with item_uri_template
Browse files Browse the repository at this point in the history
  • Loading branch information
vincentchalamon committed Aug 4, 2023
1 parent 0fe4f4d commit 7690b1d
Show file tree
Hide file tree
Showing 6 changed files with 217 additions and 22 deletions.
45 changes: 43 additions & 2 deletions features/hydra/item_uri_template.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
3 changes: 3 additions & 0 deletions src/JsonLd/Serializer/ItemNormalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,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;
Expand Down
23 changes: 13 additions & 10 deletions src/Symfony/Routing/IriConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)) {
Expand All @@ -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
Expand All @@ -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) {
}
}
Expand All @@ -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
Expand All @@ -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);
}
}
}
}
Expand Down
51 changes: 51 additions & 0 deletions tests/Fixtures/TestBundle/Entity/Issue5662/Book.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <dunglas@gmail.com>
*
* 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;
}
}
71 changes: 71 additions & 0 deletions tests/Fixtures/TestBundle/Entity/Issue5662/Review.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <dunglas@gmail.com>
*
* 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;
}
}
46 changes: 36 additions & 10 deletions tests/Symfony/Routing/IriConverterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));
}

Expand All @@ -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));
}

Expand All @@ -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));
}

Expand Down Expand Up @@ -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}');
Expand Down

0 comments on commit 7690b1d

Please sign in to comment.