Skip to content

Commit a774f4c

Browse files
authored
fix(doctrine): searchfilter with nested custom identifiers (#5760)
1 parent b7258ef commit a774f4c

File tree

16 files changed

+580
-8
lines changed

16 files changed

+580
-8
lines changed

features/doctrine/search_filter.feature

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,3 +1033,22 @@ Feature: Search filter on collections
10331033
Then the response status code should be 200
10341034
And the response should be in JSON
10351035
And the JSON node "hydra:totalItems" should be equal to 1
1036+
1037+
@!mongodb
1038+
@createSchema
1039+
Scenario: Search on nested sub-entity that doesn't use "id" as its ORM identifier
1040+
Given there is a dummy entity with a sub entity with id "stringId" and name "someName"
1041+
When I send a "GET" request to "/dummy_with_subresource?subEntity=/dummy_subresource/stringId"
1042+
Then the response status code should be 200
1043+
And the response should be in JSON
1044+
And the JSON node "hydra:totalItems" should be equal to 1
1045+
1046+
@!mongodb
1047+
@createSchema
1048+
Scenario: Filters can use UUIDs
1049+
Given there is a group object with uuid "61817181-0ecc-42fb-a6e7-d97f2ddcb344" and 2 users
1050+
And there is a group object with uuid "32510d53-f737-4e70-8d9d-58e292c871f8" and 1 users
1051+
When I send a "GET" request to "/issue5735/issue5735_users?groups[]=/issue5735/groups/61817181-0ecc-42fb-a6e7-d97f2ddcb344&groups[]=/issue5735/groups/32510d53-f737-4e70-8d9d-58e292c871f8"
1052+
Then the response status code should be 200
1053+
And the response should be in JSON
1054+
And the JSON node "hydra:totalItems" should be equal to 3

src/Doctrine/Common/Filter/SearchFilterTrait.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,13 @@ protected function getIdFromValue(string $value): mixed
124124
$iriConverter = $this->getIriConverter();
125125
$item = $iriConverter->getResourceFromIri($value, ['fetch_data' => false]);
126126

127-
return $this->getPropertyAccessor()->getValue($item, 'id');
127+
if (null === $this->identifiersExtractor) {
128+
return $this->getPropertyAccessor()->getValue($item, 'id');
129+
}
130+
131+
$identifiers = $this->identifiersExtractor->getIdentifiersFromItem($item);
132+
133+
return 1 === \count($identifiers) ? array_pop($identifiers) : $identifiers;
128134
} catch (InvalidArgumentException) {
129135
// Do nothing, return the raw value
130136
}

src/Doctrine/Orm/Filter/SearchFilter.php

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
196196
if ($metadata->hasField($field)) {
197197
if ('id' === $field) {
198198
$values = array_map($this->getIdFromValue(...), $values);
199+
// todo: handle composite IDs
199200
}
200201

201202
if (!$this->hasValidValues($values, $this->getDoctrineFieldType($property, $resourceClass))) {
@@ -216,13 +217,44 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
216217
return;
217218
}
218219

219-
$values = array_map($this->getIdFromValue(...), $values);
220-
220+
// association, let's fetch the entity (or reference to it) if we can so we can make sure we get its orm id
221221
$associationResourceClass = $metadata->getAssociationTargetClass($field);
222-
$associationFieldIdentifier = $metadata->getIdentifierFieldNames()[0];
222+
$associationMetadata = $this->getClassMetadata($associationResourceClass);
223+
$associationFieldIdentifier = $associationMetadata->getIdentifierFieldNames()[0];
223224
$doctrineTypeField = $this->getDoctrineFieldType($associationFieldIdentifier, $associationResourceClass);
224225

225-
if (!$this->hasValidValues($values, $doctrineTypeField)) {
226+
$values = array_map(function ($value) use ($associationFieldIdentifier, $doctrineTypeField) {
227+
if (is_numeric($value)) {
228+
return $value;
229+
}
230+
try {
231+
$item = $this->getIriConverter()->getResourceFromIri($value, ['fetch_data' => false]);
232+
233+
return $this->propertyAccessor->getValue($item, $associationFieldIdentifier);
234+
} catch (InvalidArgumentException) {
235+
/*
236+
* Can we do better? This is not the ApiResource the call was made on,
237+
* so we don't get any kind of api metadata for it without (a lot of?) work elsewhere...
238+
* Let's just pretend it's always the ORM id for now.
239+
*/
240+
if (!$this->hasValidValues([$value], $doctrineTypeField)) {
241+
$this->logger->notice('Invalid filter ignored', [
242+
'exception' => new InvalidArgumentException(sprintf('Values for field "%s" are not valid according to the doctrine type.', $associationFieldIdentifier)),
243+
]);
244+
245+
return null;
246+
}
247+
248+
return $value;
249+
}
250+
}, $values);
251+
252+
$expected = \count($values);
253+
$values = array_filter($values, static fn ($value) => null !== $value);
254+
if ($expected > \count($values)) {
255+
/*
256+
* Shouldn't this actually fail harder?
257+
*/
226258
$this->logger->notice('Invalid filter ignored', [
227259
'exception' => new InvalidArgumentException(sprintf('Values for field "%s" are not valid according to the doctrine type.', $field)),
228260
]);

src/Doctrine/Orm/State/ItemProvider.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
5252
$manager = $this->managerRegistry->getManagerForClass($entityClass);
5353

5454
$fetchData = $context['fetch_data'] ?? true;
55-
if (!$fetchData) {
55+
if (!$fetchData && \array_key_exists('id', $uriVariables)) {
56+
// todo : if uriVariables don't contain the id, this fails. This should behave like it does in the following code
5657
return $manager->getReference($entityClass, $uriVariables);
5758
}
5859

src/Symfony/Bundle/Resources/config/doctrine_mongodb_odm.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@
116116
<service id="api_platform.doctrine_mongodb.odm.search_filter" class="ApiPlatform\Doctrine\Odm\Filter\SearchFilter" public="false" abstract="true">
117117
<argument type="service" id="doctrine_mongodb" />
118118
<argument type="service" id="api_platform.iri_converter" />
119-
<argument type="service" id="api_platform.identifiers_extractor.cached" on-invalid="ignore" />
119+
<argument type="service" id="api_platform.identifiers_extractor" on-invalid="ignore" />
120120
<argument type="service" id="api_platform.property_accessor" />
121121
<argument type="service" id="logger" on-invalid="ignore" />
122122
<argument key="$nameConverter" type="service" id="api_platform.name_converter" on-invalid="ignore"/>

src/Symfony/Bundle/Resources/config/doctrine_orm.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@
152152
<argument type="service" id="api_platform.iri_converter" />
153153
<argument type="service" id="api_platform.property_accessor" />
154154
<argument type="service" id="logger" on-invalid="ignore" />
155-
<argument key="$identifiersExtractor" type="service" id="api_platform.identifiers_extractor.cached" on-invalid="ignore" />
155+
<argument key="$identifiersExtractor" type="service" id="api_platform.identifiers_extractor" on-invalid="ignore" />
156156
<argument key="$nameConverter" type="service" id="api_platform.name_converter" on-invalid="ignore" />
157157
</service>
158158
<service id="ApiPlatform\Doctrine\Orm\Filter\SearchFilter" alias="api_platform.doctrine.orm.search_filter" />

tests/Behat/DoctrineContext.php

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,10 @@
128128
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyPassenger;
129129
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyProduct;
130130
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyProperty;
131+
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummySubEntity;
131132
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyTableInheritanceNotApiResourceChild;
132133
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyTravel;
134+
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyWithSubEntity;
133135
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\EmbeddableDummy;
134136
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\EmbeddedDummy;
135137
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\EntityClassWithDateTime;
@@ -144,6 +146,7 @@
144146
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\IriOnlyDummy;
145147
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue5722\Event;
146148
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue5722\ItemLog;
149+
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue5735\Group;
147150
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\MaxDepthDummy;
148151
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\MultiRelationsDummy;
149152
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\MultiRelationsRelatedDummy;
@@ -2149,6 +2152,43 @@ public function thereIsAResourceUsingEntityClassAndDateTime(): void
21492152
$this->manager->flush();
21502153
}
21512154

2155+
/**
2156+
* @Given there is a dummy entity with a sub entity with id :strId and name :name
2157+
*/
2158+
public function thereIsADummyWithSubEntity(string $strId, string $name): void
2159+
{
2160+
$subEntity = new DummySubEntity($strId, $name);
2161+
$mainEntity = new DummyWithSubEntity();
2162+
$mainEntity->setSubEntity($subEntity);
2163+
$mainEntity->setName('main');
2164+
$this->manager->persist($subEntity);
2165+
$this->manager->persist($mainEntity);
2166+
$this->manager->flush();
2167+
}
2168+
2169+
/**
2170+
* @Given there is a group object with uuid :uuid and :nbUsers users
2171+
*/
2172+
public function thereIsAGroupWithUuidAndNUsers(string $uuid, int $nbUsers): void
2173+
{
2174+
$group = new Group();
2175+
$group->setUuid(\Symfony\Component\Uid\Uuid::fromString($uuid));
2176+
2177+
$this->manager->persist($group);
2178+
2179+
for ($i = 0; $i < $nbUsers; ++$i) {
2180+
$user = new \ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue5735\Issue5735User();
2181+
$user->addGroup($group);
2182+
$this->manager->persist($user);
2183+
}
2184+
2185+
// add another user not in this group
2186+
$user = new \ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue5735\Issue5735User();
2187+
$this->manager->persist($user);
2188+
2189+
$this->manager->flush();
2190+
}
2191+
21522192
/**
21532193
* @Given there are logs on an event
21542194
*/
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
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\Fixtures\TestBundle\ApiResource\Issue5605;
15+
16+
use ApiPlatform\Doctrine\Orm\Filter\SearchFilter;
17+
use ApiPlatform\Doctrine\Orm\State\Options;
18+
use ApiPlatform\Metadata\ApiFilter;
19+
use ApiPlatform\Metadata\ApiProperty;
20+
use ApiPlatform\Metadata\ApiResource;
21+
use ApiPlatform\Metadata\Get;
22+
use ApiPlatform\Metadata\GetCollection;
23+
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyWithSubEntity;
24+
use ApiPlatform\Tests\Fixtures\TestBundle\State\Issue5605\MainResourceProvider;
25+
26+
#[ApiResource(
27+
operations : [
28+
new Get(uriTemplate: '/dummy_with_subresource/{id}', uriVariables: ['id']),
29+
new GetCollection(uriTemplate: '/dummy_with_subresource'),
30+
],
31+
provider : MainResourceProvider::class,
32+
stateOptions: new Options(entityClass: DummyWithSubEntity::class)
33+
)]
34+
#[ApiFilter(SearchFilter::class, properties: ['subEntity'])]
35+
class MainResource
36+
{
37+
#[ApiProperty(identifier: true)]
38+
public int $id;
39+
public string $name;
40+
public SubResource $subResource;
41+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
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\Fixtures\TestBundle\ApiResource\Issue5605;
15+
16+
use ApiPlatform\Doctrine\Orm\State\Options;
17+
use ApiPlatform\Metadata\ApiProperty;
18+
use ApiPlatform\Metadata\ApiResource;
19+
use ApiPlatform\Metadata\Get;
20+
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummySubEntity;
21+
use ApiPlatform\Tests\Fixtures\TestBundle\State\Issue5605\SubResourceProvider;
22+
23+
#[ApiResource(
24+
operations : [
25+
new Get(
26+
uriTemplate: '/dummy_subresource/{strId}',
27+
uriVariables: ['strId']
28+
),
29+
],
30+
provider: SubResourceProvider::class,
31+
stateOptions: new Options(entityClass: DummySubEntity::class)
32+
)]
33+
class SubResource
34+
{
35+
#[ApiProperty(identifier: true)]
36+
public string $strId;
37+
38+
public string $name;
39+
}
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
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\Fixtures\TestBundle\Entity;
15+
16+
use Doctrine\ORM\Mapping as ORM;
17+
18+
#[ORM\Entity]
19+
class DummySubEntity
20+
{
21+
#[ORM\Id]
22+
#[ORM\Column(type: 'string')]
23+
private string $strId;
24+
25+
#[ORM\Column]
26+
private string $name;
27+
28+
#[ORM\OneToOne(inversedBy: 'subEntity', cascade: ['persist'])]
29+
private ?DummyWithSubEntity $mainEntity = null;
30+
31+
public function __construct($strId, $name)
32+
{
33+
$this->strId = $strId;
34+
$this->name = $name;
35+
}
36+
37+
public function getStrId(): string
38+
{
39+
return $this->strId;
40+
}
41+
42+
public function getMainEntity(): ?DummyWithSubEntity
43+
{
44+
return $this->mainEntity;
45+
}
46+
47+
public function setMainEntity(DummyWithSubEntity $mainEntity): void
48+
{
49+
$this->mainEntity = $mainEntity;
50+
}
51+
52+
public function getName(): string
53+
{
54+
return $this->name;
55+
}
56+
57+
public function setName(string $name): void
58+
{
59+
$this->name = $name;
60+
}
61+
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
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\Fixtures\TestBundle\Entity;
15+
16+
use Doctrine\ORM\Mapping as ORM;
17+
18+
#[ORM\Entity]
19+
class DummyWithSubEntity
20+
{
21+
#[ORM\Id]
22+
#[ORM\Column(type: 'integer')]
23+
#[ORM\GeneratedValue(strategy: 'AUTO')]
24+
private int $id;
25+
26+
#[ORM\Column]
27+
private string $name;
28+
29+
#[ORM\OneToOne(mappedBy: 'mainEntity', cascade: ['persist'], fetch: 'EAGER')]
30+
private ?DummySubEntity $subEntity = null;
31+
32+
public function getId(): int
33+
{
34+
return $this->id;
35+
}
36+
37+
public function getName(): string
38+
{
39+
return $this->name;
40+
}
41+
42+
public function setName(string $name): void
43+
{
44+
$this->name = $name;
45+
}
46+
47+
public function getSubEntity(): ?DummySubEntity
48+
{
49+
return $this->subEntity;
50+
}
51+
52+
public function setSubEntity(?DummySubEntity $subEntity): void
53+
{
54+
if (null !== $subEntity && $subEntity->getMainEntity() !== $this) {
55+
$subEntity->setMainEntity($this);
56+
}
57+
58+
$this->subEntity = $subEntity;
59+
}
60+
}

0 commit comments

Comments
 (0)