Skip to content

Commit 8f57f08

Browse files
soyukasoyuka
authored
Merge pull request #1844 from Simperfit/feature/improve-ids-management-for-association
feature: improve ids management for association and fix invalid iri/uuid in SearchFilter
2 parents e03b925 + 6d657b8 commit 8f57f08

File tree

14 files changed

+340
-67
lines changed

14 files changed

+340
-67
lines changed

‎features/doctrine/search_filter.feature‎

Lines changed: 102 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -20,37 +20,62 @@ Feature: Search filter on collections
2020
When I send a "GET" request to "/dummy_cars?colors.prop=red"
2121
Then the response status code should be 200
2222
And the JSON should be deep equal to:
23-
"""
23+
"""
2424
{
25-
"@context": "/contexts/DummyCar",
26-
"@id": "/dummy_cars",
27-
"@type": "hydra:Collection",
28-
"hydra:member": [
25+
"@context": "/contexts/DummyCar",
26+
"@id": "/dummy_cars",
27+
"@type": "hydra:Collection",
28+
"hydra:member": [
29+
{
30+
"@id": "/dummy_cars/1",
31+
"@type": "DummyCar",
32+
"colors": [
2933
{
30-
"@id": "/dummy_cars/1",
31-
"@type": "DummyCar",
32-
"colors": [
33-
{
34-
"@id": "/dummy_car_colors/1",
35-
"@type": "DummyCarColor",
36-
"prop": "red"
37-
},
38-
{
39-
"@id": "/dummy_car_colors/2",
40-
"@type": "DummyCarColor",
41-
"prop": "blue"
42-
}
43-
]
34+
"@id": "/dummy_car_colors/1",
35+
"@type": "DummyCarColor",
36+
"prop": "red"
37+
},
38+
{
39+
"@id": "/dummy_car_colors/2",
40+
"@type": "DummyCarColor",
41+
"prop": "blue"
4442
}
45-
],
46-
"hydra:totalItems": 1,
47-
"hydra:view": {
48-
"@id": "/dummy_cars?colors.prop=red",
49-
"@type": "hydra:PartialCollectionView"
50-
},
51-
"hydra:search": {
43+
],
44+
"secondColors": [
45+
{
46+
"@id": "/dummy_car_colors/1",
47+
"@type": "DummyCarColor",
48+
"prop": "red"
49+
},
50+
{
51+
"@id": "/dummy_car_colors/2",
52+
"@type": "DummyCarColor",
53+
"prop": "blue"
54+
}
55+
],
56+
"thirdColors": [
57+
{
58+
"@id": "/dummy_car_colors/1",
59+
"@type": "DummyCarColor",
60+
"prop": "red"
61+
},
62+
{
63+
"@id": "/dummy_car_colors/2",
64+
"@type": "DummyCarColor",
65+
"prop": "blue"
66+
}
67+
],
68+
"uuid": []
69+
}
70+
],
71+
"hydra:totalItems": 1,
72+
"hydra:view": {
73+
"@id": "/dummy_cars?colors.prop=red",
74+
"@type": "hydra:PartialCollectionView"
75+
},
76+
"hydra:search": {
5277
"@type": "hydra:IriTemplate",
53-
"hydra:template": "\/dummy_cars{?availableAt[before],availableAt[strictly_before],availableAt[after],availableAt[strictly_after],canSell,foobar[],foobargroups[],foobargroups_override[],colors.prop,name}",
78+
"hydra:template": "/dummy_cars{?availableAt[before],availableAt[strictly_before],availableAt[after],availableAt[strictly_after],canSell,foobar[],foobargroups[],foobargroups_override[],colors.prop,colors,colors[],secondColors,secondColors[],thirdColors,thirdColors[],uuid,uuid[],name}",
5479
"hydra:variableRepresentation": "BasicRepresentation",
5580
"hydra:mapping": [
5681
{
@@ -83,12 +108,24 @@ Feature: Search filter on collections
83108
"property": "canSell",
84109
"required": false
85110
},
111+
{
112+
"@type": "IriTemplateMapping",
113+
"variable": "colors",
114+
"property": "colors",
115+
"required": false
116+
},
86117
{
87118
"@type": "IriTemplateMapping",
88119
"variable": "colors.prop",
89120
"property": "colors.prop",
90121
"required": false
91122
},
123+
{
124+
"@type": "IriTemplateMapping",
125+
"variable": "colors[]",
126+
"property": "colors",
127+
"required": false
128+
},
92129
{
93130
"@type": "IriTemplateMapping",
94131
"variable": "foobar[]",
@@ -112,6 +149,42 @@ Feature: Search filter on collections
112149
"variable": "name",
113150
"property": "name",
114151
"required": false
152+
},
153+
{
154+
"@type": "IriTemplateMapping",
155+
"variable": "secondColors",
156+
"property": "secondColors",
157+
"required": false
158+
},
159+
{
160+
"@type": "IriTemplateMapping",
161+
"variable": "secondColors[]",
162+
"property": "secondColors",
163+
"required": false
164+
},
165+
{
166+
"@type": "IriTemplateMapping",
167+
"variable": "thirdColors",
168+
"property": "thirdColors",
169+
"required": false
170+
},
171+
{
172+
"@type": "IriTemplateMapping",
173+
"variable": "thirdColors[]",
174+
"property": "thirdColors",
175+
"required": false
176+
},
177+
{
178+
"@type": "IriTemplateMapping",
179+
"variable": "uuid",
180+
"property": "uuid",
181+
"required": false
182+
},
183+
{
184+
"@type": "IriTemplateMapping",
185+
"variable": "uuid[]",
186+
"property": "uuid",
187+
"required": false
115188
}
116189
]
117190
}
@@ -389,8 +462,7 @@ Feature: Search filter on collections
389462
"@id": {"pattern": "^/dummies$"},
390463
"@type": {"pattern": "^hydra:Collection$"},
391464
"hydra:member": {
392-
"type": "array",
393-
"maxItems": 0
465+
"type": "array"
394466
},
395467
"hydra:view": {
396468
"type": "object",
@@ -655,3 +727,4 @@ Feature: Search filter on collections
655727
}
656728
}
657729
"""
730+

‎src/Bridge/Doctrine/Common/Filter/SearchFilterTrait.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ trait SearchFilterTrait
3131

3232
protected $iriConverter;
3333
protected $propertyAccessor;
34+
protected $identifiersExtractor;
3435

3536
/**
3637
* {@inheritdoc}

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

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,15 @@
1313

1414
namespace ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Filter;
1515

16+
use ApiPlatform\Core\Api\IdentifiersExtractorInterface;
1617
use ApiPlatform\Core\Api\IriConverterInterface;
1718
use ApiPlatform\Core\Bridge\Doctrine\Common\Filter\SearchFilterInterface;
1819
use ApiPlatform\Core\Bridge\Doctrine\Common\Filter\SearchFilterTrait;
1920
use ApiPlatform\Core\Exception\InvalidArgumentException;
2021
use Doctrine\Common\Persistence\ManagerRegistry;
2122
use Doctrine\Common\Persistence\Mapping\ClassMetadata;
2223
use Doctrine\ODM\MongoDB\Aggregation\Builder;
24+
use Doctrine\ODM\MongoDB\Mapping\ClassMetadata as MongoDBClassMetadata;
2325
use Doctrine\ODM\MongoDB\Types\Type as MongoDbType;
2426
use MongoDB\BSON\Regex;
2527
use Psr\Log\LoggerInterface;
@@ -40,12 +42,17 @@ final class SearchFilter extends AbstractFilter implements SearchFilterInterface
4042

4143
public const DOCTRINE_INTEGER_TYPE = MongoDbType::INTEGER;
4244

43-
public function __construct(ManagerRegistry $managerRegistry, IriConverterInterface $iriConverter, PropertyAccessorInterface $propertyAccessor = null, LoggerInterface $logger = null, array $properties = null)
45+
public function __construct(ManagerRegistry $managerRegistry, IriConverterInterface $iriConverter, PropertyAccessorInterface $propertyAccessor = null, LoggerInterface $logger = null, array $properties = null, IdentifiersExtractorInterface $identifiersExtractor = null)
4446
{
4547
parent::__construct($managerRegistry, $logger, $properties);
4648

49+
if (null === $identifiersExtractor) {
50+
@trigger_error('Not injecting ItemIdentifiersExtractor is deprecated since API Platform 2.5 and can lead to unexpected behaviors, it will not be possible anymore in API Platform 3.0.', E_USER_DEPRECATED);
51+
}
52+
4753
$this->iriConverter = $iriConverter;
4854
$this->propertyAccessor = $propertyAccessor ?: PropertyAccess::createPropertyAccessor();
55+
$this->identifiersExtractor = $identifiersExtractor;
4956
}
5057

5158
protected function getIriConverter(): IriConverterInterface
@@ -77,6 +84,10 @@ protected function filterProperty(string $property, $value, Builder $aggregation
7784
if ($this->isPropertyNested($property, $resourceClass)) {
7885
[$matchField, $field, $associations] = $this->addLookupsForNestedProperty($property, $aggregationBuilder, $resourceClass);
7986
}
87+
88+
/**
89+
* @var MongoDBClassMetadata
90+
*/
8091
$metadata = $this->getNestedMetadata($resourceClass, $associations);
8192

8293
$values = $this->normalizeValues((array) $value, $property);
@@ -124,8 +135,16 @@ protected function filterProperty(string $property, $value, Builder $aggregation
124135
}
125136

126137
$values = array_map([$this, 'getIdFromValue'], $values);
138+
$associationFieldIdentifier = 'id';
139+
$doctrineTypeField = $this->getDoctrineFieldType($property, $resourceClass);
140+
141+
if (null !== $this->identifiersExtractor) {
142+
$associationResourceClass = $metadata->getAssociationTargetClass($field);
143+
$associationFieldIdentifier = $this->identifiersExtractor->getIdentifiersFromResourceClass($associationResourceClass)[0];
144+
$doctrineTypeField = $this->getDoctrineFieldType($associationFieldIdentifier, $associationResourceClass);
145+
}
127146

128-
if (!$this->hasValidValues($values, $this->getDoctrineFieldType($property, $resourceClass))) {
147+
if (!$this->hasValidValues($values, $doctrineTypeField)) {
129148
$this->logger->notice('Invalid filter ignored', [
130149
'exception' => new InvalidArgumentException(sprintf('Values for field "%s" are not valid according to the doctrine type.', $property)),
131150
]);

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

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
namespace ApiPlatform\Core\Bridge\Doctrine\Orm\Filter;
1515

16+
use ApiPlatform\Core\Api\IdentifiersExtractorInterface;
1617
use ApiPlatform\Core\Api\IriConverterInterface;
1718
use ApiPlatform\Core\Bridge\Doctrine\Common\Filter\SearchFilterInterface;
1819
use ApiPlatform\Core\Bridge\Doctrine\Common\Filter\SearchFilterTrait;
@@ -21,6 +22,7 @@
2122
use ApiPlatform\Core\Exception\InvalidArgumentException;
2223
use Doctrine\Common\Persistence\ManagerRegistry;
2324
use Doctrine\DBAL\Types\Type as DBALType;
25+
use Doctrine\ORM\Mapping\ClassMetadata;
2426
use Doctrine\ORM\QueryBuilder;
2527
use Psr\Log\LoggerInterface;
2628
use Symfony\Component\HttpFoundation\RequestStack;
@@ -38,12 +40,17 @@ class SearchFilter extends AbstractContextAwareFilter implements SearchFilterInt
3840

3941
public const DOCTRINE_INTEGER_TYPE = DBALType::INTEGER;
4042

41-
public function __construct(ManagerRegistry $managerRegistry, ?RequestStack $requestStack, IriConverterInterface $iriConverter, PropertyAccessorInterface $propertyAccessor = null, LoggerInterface $logger = null, array $properties = null)
43+
public function __construct(ManagerRegistry $managerRegistry, ?RequestStack $requestStack, IriConverterInterface $iriConverter, PropertyAccessorInterface $propertyAccessor = null, LoggerInterface $logger = null, array $properties = null, IdentifiersExtractorInterface $identifiersExtractor = null)
4244
{
4345
parent::__construct($managerRegistry, $requestStack, $logger, $properties);
4446

47+
if (null === $identifiersExtractor) {
48+
@trigger_error('Not injecting ItemIdentifiersExtractor is deprecated since API Platform 2.5 and can lead to unexpected behaviors, it will not be possible anymore in API Platform 3.0.', E_USER_DEPRECATED);
49+
}
50+
4551
$this->iriConverter = $iriConverter;
4652
$this->propertyAccessor = $propertyAccessor ?: PropertyAccess::createPropertyAccessor();
53+
$this->identifiersExtractor = $identifiersExtractor;
4754
}
4855

4956
protected function getIriConverter(): IriConverterInterface
@@ -76,6 +83,10 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
7683
if ($this->isPropertyNested($property, $resourceClass)) {
7784
[$alias, $field, $associations] = $this->addJoinsForNestedProperty($property, $alias, $queryBuilder, $queryNameGenerator, $resourceClass);
7885
}
86+
87+
/**
88+
* @var ClassMetadata
89+
*/
7990
$metadata = $this->getNestedMetadata($resourceClass, $associations);
8091

8192
$values = $this->normalizeValues((array) $value, $property);
@@ -84,7 +95,6 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
8495
}
8596

8697
$caseSensitive = true;
87-
8898
if ($metadata->hasField($field)) {
8999
if ('id' === $field) {
90100
$values = array_map([$this, 'getIdFromValue'], $values);
@@ -134,8 +144,16 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
134144
}
135145

136146
$values = array_map([$this, 'getIdFromValue'], $values);
147+
$associationFieldIdentifier = 'id';
148+
$doctrineTypeField = $this->getDoctrineFieldType($property, $resourceClass);
137149

138-
if (!$this->hasValidValues($values, $this->getDoctrineFieldType($property, $resourceClass))) {
150+
if (null !== $this->identifiersExtractor) {
151+
$associationResourceClass = $metadata->getAssociationTargetClass($field);
152+
$associationFieldIdentifier = $this->identifiersExtractor->getIdentifiersFromResourceClass($associationResourceClass)[0];
153+
$doctrineTypeField = $this->getDoctrineFieldType($associationFieldIdentifier, $associationResourceClass);
154+
}
155+
156+
if (!$this->hasValidValues($values, $doctrineTypeField)) {
139157
$this->logger->notice('Invalid filter ignored', [
140158
'exception' => new InvalidArgumentException(sprintf('Values for field "%s" are not valid according to the doctrine type.', $field)),
141159
]);
@@ -145,10 +163,9 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
145163

146164
$association = $field;
147165
$valueParameter = $queryNameGenerator->generateParameterName($association);
148-
149166
if ($metadata->isCollectionValuedAssociation($association)) {
150167
$associationAlias = QueryBuilderHelper::addJoinOnce($queryBuilder, $queryNameGenerator, $alias, $association);
151-
$associationField = 'id';
168+
$associationField = $associationFieldIdentifier;
152169
} else {
153170
$associationAlias = $alias;
154171
$associationField = $field;

‎src/Bridge/Symfony/Bundle/Resources/config/api.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@
270270
<argument type="service" id="api_platform.property_accessor" />
271271
<argument type="service" id="api_platform.resource_class_resolver" />
272272
</service>
273+
<service id="ApiPlatform\Core\Api\IdentifiersExtractorInterface" alias="api_platform.identifiers_extractor.cached" />
273274

274275
<service id="api_platform.identifier.converter" class="ApiPlatform\Core\Identifier\IdentifierConverter" public="false">
275276
<argument type="service" id="api_platform.identifiers_extractor.cached" />

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,9 @@
6666
<argument type="service" id="api_platform.iri_converter" />
6767
<argument type="service" id="api_platform.property_accessor" />
6868
<argument type="service" id="logger" on-invalid="ignore" />
69+
<argument key="$identifiersExtractor" type="service" id="api_platform.identifiers_extractor.cached" />
6970
</service>
71+
7072
<service id="ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Filter\SearchFilter" alias="api_platform.doctrine_mongodb.odm.search_filter" />
7173

7274
<service id="api_platform.doctrine_mongodb.odm.boolean_filter" class="ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Filter\BooleanFilter" public="false" abstract="true">

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
<argument type="service" id="api_platform.iri_converter" />
5656
<argument type="service" id="api_platform.property_accessor" />
5757
<argument type="service" id="logger" on-invalid="ignore" />
58+
<argument key="$identifiersExtractor" type="service" id="api_platform.identifiers_extractor.cached" />
5859
</service>
5960
<service id="ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\SearchFilter" alias="api_platform.doctrine.orm.search_filter" />
6061

‎src/Test/DoctrineMongoDbOdmFilterTestCase.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ protected function doTestApply(?array $properties, array $filterParameters, arra
8181
$pipeline = $aggregationBuilder->getPipeline();
8282
} catch (\OutOfRangeException $e) {
8383
}
84+
8485
$this->assertEquals($expectedPipeline, $pipeline);
8586
}
8687

0 commit comments

Comments
 (0)