Skip to content

Commit ae4ea86

Browse files
soyukasoyuka
authored
fix(symfony,laravel): IriConverter local cache key collision between item and collection ops (#7975)
1 parent 91f93e0 commit ae4ea86

4 files changed

Lines changed: 161 additions & 2 deletions

File tree

‎src/Laravel/Routing/IriConverter.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public function getIriFromResource(object|string $resource, int $referenceType =
103103
$operation = $this->operationMetadataFactory->create($context['item_uri_template']);
104104
}
105105

106-
$localOperationCacheKey = ($operation?->getName() ?? '').$resourceClass.(\is_string($resource) ? '_c' : '_i');
106+
$localOperationCacheKey = ($operation?->getName() ?? '').$resourceClass.(\is_string($resource) ? '_s' : '_o').($operation instanceof CollectionOperationInterface ? '_c' : '_i');
107107
if ($operation && isset($this->localOperationCache[$localOperationCacheKey])) {
108108
return $this->generateRoute($resource, $referenceType, $this->localOperationCache[$localOperationCacheKey], $context, $this->localIdentifiersExtractorOperationCache[$localOperationCacheKey] ?? null);
109109
}

‎src/Laravel/Tests/Unit/Routing/IriConverterTest.php‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Laravel\Tests\Unit\Routing;
15+
16+
use ApiPlatform\Laravel\Routing\IriConverter;
17+
use ApiPlatform\Metadata\ApiResource;
18+
use ApiPlatform\Metadata\Get;
19+
use ApiPlatform\Metadata\GetCollection;
20+
use ApiPlatform\Metadata\IdentifiersExtractorInterface;
21+
use ApiPlatform\Metadata\Operation\Factory\OperationMetadataFactoryInterface;
22+
use ApiPlatform\Metadata\Operations;
23+
use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
24+
use ApiPlatform\Metadata\Resource\ResourceMetadataCollection;
25+
use ApiPlatform\Metadata\ResourceClassResolverInterface;
26+
use ApiPlatform\Metadata\UrlGeneratorInterface;
27+
use ApiPlatform\State\ProviderInterface;
28+
use PHPUnit\Framework\TestCase;
29+
use Symfony\Component\Routing\RouterInterface;
30+
use Workbench\App\Models\Book;
31+
32+
class IriConverterTest extends TestCase
33+
{
34+
public function testLocalCacheKeyDistinguishesItemAndCollectionForStringResource(): void
35+
{
36+
$collectionOpName = 'collection_op';
37+
$itemOpName = 'item_op';
38+
39+
$collectionOp = (new GetCollection())->withName($collectionOpName)->withClass(Book::class);
40+
$itemOp = (new Get())->withName($itemOpName)->withClass(Book::class);
41+
42+
$router = $this->createMock(RouterInterface::class);
43+
$router->expects($this->exactly(2))
44+
->method('generate')
45+
->willReturnCallback(function (string $routeName, array $params) use ($collectionOpName, $itemOpName): string {
46+
if ($collectionOpName === $routeName) {
47+
return '/api/books';
48+
}
49+
if ($itemOpName === $routeName) {
50+
return '/api/books/'.$params['id'];
51+
}
52+
$this->fail(\sprintf('Unexpected route name "%s".', $routeName));
53+
});
54+
55+
$metadataCollection = new ResourceMetadataCollection(Book::class, [
56+
(new ApiResource())->withOperations(new Operations([
57+
$collectionOpName => $collectionOp,
58+
$itemOpName => $itemOp,
59+
])),
60+
]);
61+
62+
$resourceMetadataFactory = $this->createStub(ResourceMetadataCollectionFactoryInterface::class);
63+
$resourceMetadataFactory->method('create')->willReturn($metadataCollection);
64+
65+
$resourceClassResolver = $this->createStub(ResourceClassResolverInterface::class);
66+
$resourceClassResolver->method('isResourceClass')->willReturn(true);
67+
68+
$iriConverter = new IriConverter(
69+
$this->createStub(ProviderInterface::class),
70+
$this->createStub(OperationMetadataFactoryInterface::class),
71+
$router,
72+
$this->createStub(IdentifiersExtractorInterface::class),
73+
$resourceClassResolver,
74+
$resourceMetadataFactory,
75+
);
76+
77+
$this->assertSame('/api/books', $iriConverter->getIriFromResource(
78+
Book::class,
79+
UrlGeneratorInterface::ABS_PATH,
80+
new GetCollection(),
81+
));
82+
83+
$this->assertSame('/api/books/1', $iriConverter->getIriFromResource(
84+
Book::class,
85+
UrlGeneratorInterface::ABS_PATH,
86+
new Get(),
87+
['uri_variables' => ['id' => 1]],
88+
));
89+
}
90+
}

‎src/Symfony/Routing/IriConverter.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ public function getIriFromResource(object|string $resource, int $referenceType =
122122
$operation = $this->operationMetadataFactory->create($context['item_uri_template']);
123123
}
124124

125-
$localOperationCacheKey = ($operation?->getName() ?? '').$resourceClass.((\is_string($resource) || $operation instanceof CollectionOperationInterface) ? '_c' : '_i');
125+
$localOperationCacheKey = ($operation?->getName() ?? '').$resourceClass.(\is_string($resource) ? '_s' : '_o').($operation instanceof CollectionOperationInterface ? '_c' : '_i');
126126
if ($operation && isset($this->localOperationCache[$localOperationCacheKey])) {
127127
return $this->generateSymfonyRoute($resource, $referenceType, $this->localOperationCache[$localOperationCacheKey], $context, $this->localIdentifiersExtractorOperationCache[$localOperationCacheKey] ?? null);
128128
}

‎tests/Functional/Json/RelationTest.php‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Functional\Json;
15+
16+
use ApiPlatform\Symfony\Bundle\Test\ApiTestCase;
17+
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\RelatedDummy;
18+
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\ThirdLevel;
19+
use ApiPlatform\Tests\RecreateSchemaTrait;
20+
use ApiPlatform\Tests\SetupClassResourcesTrait;
21+
22+
final class RelationTest extends ApiTestCase
23+
{
24+
use RecreateSchemaTrait;
25+
use SetupClassResourcesTrait;
26+
27+
protected static ?bool $alwaysBootKernel = false;
28+
29+
public static function getResources(): array
30+
{
31+
return [ThirdLevel::class, RelatedDummy::class];
32+
}
33+
34+
protected function setUp(): void
35+
{
36+
parent::setUp();
37+
38+
if ($this->isMongoDB()) {
39+
$this->markTestSkipped('Not tested with MongoDB.');
40+
}
41+
42+
$this->recreateSchema([ThirdLevel::class, RelatedDummy::class]);
43+
}
44+
45+
public function testCreateRelatedDummyWithPlainIdentifierForRelation(): void
46+
{
47+
// Creates a ThirdLevel; PurgeHttpCacheListener::postFlush caches the GetCollection
48+
// operation under the '' + ThirdLevel + '_c' slot of IriConverter::$localOperationCache.
49+
self::createClient()->request('POST', '/third_levels', [
50+
'headers' => ['Content-Type' => 'application/json'],
51+
'json' => ['level' => 3],
52+
]);
53+
$this->assertResponseStatusCodeSame(201);
54+
55+
// RelatedDummyPlainIdentifierDenormalizer calls getIriFromResource(ThirdLevel::class, new Get(), …).
56+
// Without the fix the '_c' slot collision returns the GetCollection op, producing
57+
// "/third_levels?id=1" instead of "/third_levels/1".
58+
$response = self::createClient()->request('POST', '/related_dummies', [
59+
'headers' => ['Content-Type' => 'application/json'],
60+
'json' => ['thirdLevel' => '1'],
61+
]);
62+
$this->assertResponseStatusCodeSame(201);
63+
64+
$data = $response->toArray(false);
65+
$this->assertArrayHasKey('thirdLevel', $data);
66+
$this->assertIsArray($data['thirdLevel']);
67+
$this->assertSame('/third_levels/1', $data['thirdLevel']['@id']);
68+
}
69+
}

0 commit comments

Comments
 (0)