Skip to content

Commit 4bf9863

Browse files
author
abluchet
committed
Correct filter id needs split
1 parent 88a9e92 commit 4bf9863

File tree

5 files changed

+138
-14
lines changed

5 files changed

+138
-14
lines changed

‎src/Api/IriConverterInterface.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,12 @@ public function getItemIriFromResourceClass(string $resourceClass, array $identi
8585
* @return string
8686
*/
8787
public function getSubresourceIriFromResourceClass(string $resourceClass, array $identifiers, int $referenceType = UrlGeneratorInterface::ABS_PATH): string;
88+
89+
/**
90+
* Gets identifiers from a given IRI.
91+
*
92+
* @throws InvalidArgumentException
93+
* @throws RuntimeException
94+
*/
95+
public function getIdentifiersFromIri(string $iri, array $context = []): array;
8896
}

‎src/Bridge/Doctrine/Orm/Filter/SearchFilter.php

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
1919
use ApiPlatform\Core\Exception\InvalidArgumentException;
2020
use Doctrine\Common\Persistence\ManagerRegistry;
21+
use Doctrine\Common\Persistence\Mapping\ClassMetadata;
2122
use Doctrine\DBAL\Types\Type;
23+
use Doctrine\ORM\Mapping\MappingException;
2224
use Doctrine\ORM\QueryBuilder;
2325
use Psr\Log\LoggerInterface;
2426
use Symfony\Component\HttpFoundation\RequestStack;
@@ -138,7 +140,7 @@ public function getDescription(string $resourceClass): array
138140
*
139141
* @return string
140142
*/
141-
private function getType(string $doctrineType): string
143+
private function getType(string $doctrineType = null): string
142144
{
143145
switch ($doctrineType) {
144146
case Type::TARRAY:
@@ -207,8 +209,16 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
207209
$caseSensitive = true;
208210

209211
if ($metadata->hasField($field)) {
210-
if ('id' === $field) {
211-
$values = array_map([$this, 'getIdFromValue'], $values);
212+
if ($metadata->isIdentifier($field)) {
213+
$values = $this->getValueIdentifiers($values, $field);
214+
}
215+
216+
if (!$this->hasValidValues($values, $field, $metadata)) {
217+
$this->logger->notice('Invalid filter ignored', [
218+
'exception' => new InvalidArgumentException(sprintf('Values for field "%s" are not valid according to the doctrine type.', $field)),
219+
]);
220+
221+
return;
212222
}
213223

214224
$strategy = $this->properties[$property] ?? self::STRATEGY_EXACT;
@@ -246,14 +256,33 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
246256
return;
247257
}
248258

249-
$values = array_map([$this, 'getIdFromValue'], $values);
259+
$values = $this->getValueIdentifiers($values, $field);
260+
261+
if (!$this->hasValidValues($values, $field, $metadata)) {
262+
$this->logger->notice('Invalid filter ignored', [
263+
'exception' => new InvalidArgumentException(sprintf('Values for field "%s" are not valid according to the doctrine type.', $field)),
264+
]);
265+
266+
return;
267+
}
250268

251269
$association = $field;
252270
$valueParameter = $queryNameGenerator->generateParameterName($association);
253271

254272
if ($metadata->isCollectionValuedAssociation($association)) {
255273
$associationAlias = QueryBuilderHelper::addJoinOnce($queryBuilder, $queryNameGenerator, $alias, $association);
256-
$associationField = 'id';
274+
$targetClass = $metadata->getAssociationTargetClass($association);
275+
$associationMetadata = $this->getClassMetadata($targetClass);
276+
277+
try {
278+
$associationField = $associationMetadata->getSingleIdentifierFieldName();
279+
} catch (MappingException $e) {
280+
$this->logger->notice('Invalid filter ignored', [
281+
'exception' => new InvalidArgumentException(sprintf('"%s" association has a composite identifier which is not supported.', $association)),
282+
]);
283+
284+
return;
285+
}
257286
} else {
258287
$associationAlias = $alias;
259288
$associationField = $field;
@@ -347,10 +376,14 @@ protected function createWrapCase(bool $caseSensitive): \Closure
347376
*
348377
* @param string $value
349378
*
379+
* @deprecated The "getValueIdentifiers" method should be used instead. Indeed this method has a hard-coded `id` property.
380+
*
350381
* @return mixed
351382
*/
352383
protected function getIdFromValue(string $value)
353384
{
385+
@trigger_error(sprintf('The method "%s"::"%s" is deprecated since API Platform 2.2 and will be removed in API Platform 3. Please use "getValueIdentifiers" instead.', __CLASS__, __METHOD__), E_USER_DEPRECATED);
386+
354387
try {
355388
if ($item = $this->iriConverter->getItemFromIri($value, ['fetch_data' => false])) {
356389
return $this->propertyAccessor->getValue($item, 'id');
@@ -362,6 +395,42 @@ protected function getIdFromValue(string $value)
362395
return $value;
363396
}
364397

398+
/**
399+
* Transforms IRI to raw identifiers if possible.
400+
*/
401+
protected function getValueIdentifiers(array $values, string $field): array
402+
{
403+
foreach ($values as $key => $value) {
404+
if (is_numeric($value)) {
405+
continue;
406+
}
407+
408+
try {
409+
$identifiers = $this->iriConverter->getIdentifiersFromIri($value, ['fetch_data' => false]);
410+
$values[$key] = $identifiers[$field] ?? reset($identifiers); //an assumption is being made for composite identifiers
411+
} catch (InvalidArgumentException $e) {
412+
// Do nothing, return the raw value
413+
}
414+
}
415+
416+
return $values;
417+
}
418+
419+
/**
420+
* When the field should be an integer, check that the given value is a valid one.
421+
*/
422+
protected function hasValidValues(array $values, string $field, ClassMetadata $metadata): bool
423+
{
424+
foreach ($values as $key => $value) {
425+
$type = $metadata->getTypeOfField($field);
426+
if (Type::INTEGER === $type && null !== $value && false === filter_var($value, FILTER_VALIDATE_INT)) {
427+
return false;
428+
}
429+
}
430+
431+
return true;
432+
}
433+
365434
/**
366435
* Normalize the values array.
367436
*

‎src/Bridge/Symfony/Routing/IriConverter.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,14 @@ public function getItemFromIri(string $iri, array $context = [])
8686
throw new ItemNotFoundException(sprintf('Item not found for "%s".', $iri));
8787
}
8888

89+
/**
90+
* {@inheritdoc}
91+
*/
92+
public function getIdentifiersFromIri(string $iri, array $context = []): array
93+
{
94+
return $this->identifiersExtractor->getIdentifiersFromItem($this->getItemFromIri($iri, $context));
95+
}
96+
8997
/**
9098
* {@inheritdoc}
9199
*/

‎tests/Bridge/Doctrine/Orm/Filter/SearchFilterTest.php

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,11 @@ protected function setUp()
6868
$manager = DoctrineTestHelper::createTestEntityManager();
6969
$this->managerRegistry = self::$kernel->getContainer()->get('doctrine');
7070

71-
$relatedDummyProphecy = $this->prophesize(RelatedDummy::class);
72-
7371
$iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
7472

75-
$iriConverterProphecy->getItemFromIri(Argument::type('string'), ['fetch_data' => false])->will(function ($args) use ($relatedDummyProphecy) {
73+
$iriConverterProphecy->getIdentifiersFromIri(Argument::type('string'), ['fetch_data' => false])->will(function ($args) {
7674
if (false !== strpos($args[0], '/related_dummies')) {
77-
$relatedDummyProphecy->getId()->shouldBeCalled()->willReturn(1);
78-
79-
return $relatedDummyProphecy->reveal();
75+
return ['id' => 1];
8076
}
8177

8278
throw new InvalidArgumentException();
@@ -746,7 +742,7 @@ public function testDoubleJoin()
746742

747743
public function testTripleJoin()
748744
{
749-
$request = Request::create('/api/dummies', 'GET', ['relatedDummy.symfony' => 'foo', 'relatedDummy.thirdLevel.level' => 'bar']);
745+
$request = Request::create('/api/dummies', 'GET', ['relatedDummy.symfony' => 'foo', 'relatedDummy.thirdLevel.level' => '2']);
750746
$requestStack = new RequestStack();
751747
$requestStack->push($request);
752748
$queryBuilder = $this->repository->createQueryBuilder('o');
@@ -770,7 +766,7 @@ public function testTripleJoin()
770766

771767
public function testJoinLeft()
772768
{
773-
$request = Request::create('/api/dummies', 'GET', ['relatedDummy.symfony' => 'foo', 'relatedDummy.thirdLevel.level' => 'bar']);
769+
$request = Request::create('/api/dummies', 'GET', ['relatedDummy.symfony' => 'foo', 'relatedDummy.thirdLevel.level' => '3']);
774770
$requestStack = new RequestStack();
775771
$requestStack->push($request);
776772
$queryBuilder = $this->repository->createQueryBuilder('o');

‎tests/Bridge/Symfony/Routing/IriConverterTest.php

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,11 @@
2121
use ApiPlatform\Core\DataProvider\ItemDataProviderInterface;
2222
use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
2323
use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
24+
use ApiPlatform\Core\Metadata\Property\PropertyMetadata;
25+
use ApiPlatform\Core\Metadata\Property\PropertyNameCollection;
2426
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy;
2527
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedDummy;
28+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\User;
2629
use PHPUnit\Framework\TestCase;
2730
use Prophecy\Argument;
2831
use Symfony\Component\Routing\Exception\RouteNotFoundException;
@@ -164,7 +167,8 @@ public function testGetItemFromIri()
164167
null,
165168
new IdentifiersExtractor($propertyNameCollectionFactory, $propertyMetadataFactory)
166169
);
167-
$converter->getItemFromIri('/users/3', ['fetch_data' => true]);
170+
171+
$this->assertEquals('foo', $converter->getItemFromIri('/users/3', ['fetch_data' => true]));
168172
}
169173

170174
public function testGetIriFromResourceClass()
@@ -352,4 +356,43 @@ public function testNotAbleToGenerateGetItemIriFromResourceClass()
352356
);
353357
$converter->getItemIriFromResourceClass(Dummy::class, ['id' => 1]);
354358
}
359+
360+
public function testGetIdentifiersFromIri()
361+
{
362+
$propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
363+
$propertyNameCollectionFactoryProphecy->create(Argument::type('string'))->willReturn(new PropertyNameCollection(['id']));
364+
365+
$propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
366+
$propertyMetadataFactoryProphecy->create(Argument::type('string'), 'id')->willReturn((new PropertyMetadata())->withIdentifier(true));
367+
368+
$userProphecy = $this->prophesize(User::class);
369+
$userProphecy->getId()->willReturn(3);
370+
371+
$itemDataProviderProphecy = $this->prophesize(ItemDataProviderInterface::class);
372+
$itemDataProviderProphecy->getItem(User::class, 3, null, ['fetch_data' => false])
373+
->willReturn($userProphecy->reveal())
374+
->shouldBeCalledTimes(1);
375+
376+
$routeNameResolverProphecy = $this->prophesize(RouteNameResolverInterface::class);
377+
378+
$routerProphecy = $this->prophesize(RouterInterface::class);
379+
$routerProphecy->match('/users/3')->willReturn([
380+
'_api_resource_class' => User::class,
381+
'id' => 3,
382+
])->shouldBeCalledTimes(1);
383+
384+
$propertyNameCollectionFactory = $propertyNameCollectionFactoryProphecy->reveal();
385+
$propertyMetadataFactory = $propertyMetadataFactoryProphecy->reveal();
386+
387+
$converter = new IriConverter(
388+
$propertyNameCollectionFactory,
389+
$propertyMetadataFactory,
390+
$itemDataProviderProphecy->reveal(),
391+
$routeNameResolverProphecy->reveal(),
392+
$routerProphecy->reveal(),
393+
null,
394+
new IdentifiersExtractor($propertyNameCollectionFactory, $propertyMetadataFactory)
395+
);
396+
$this->assertEquals(['id' => 3], $converter->getIdentifiersFromIri('/users/3', ['fetch_data' => false]));
397+
}
355398
}

0 commit comments

Comments
 (0)