Skip to content

Commit 9161117

Browse files
soyukasoyuka
authored
Merge pull request #2277 from ArnoudThibaut/allow-numeric-filter-to-filter-over-multiple-values
allow to use multiple values with NumericFilter
2 parents 77da03e + a46fd26 commit 9161117

File tree

5 files changed

+195
-11
lines changed

5 files changed

+195
-11
lines changed

‎features/doctrine/date_filter.feature

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ Feature: Date filter on collections
404404
},
405405
"hydra:search": {
406406
"@type": "hydra:IriTemplate",
407-
"hydra:template": "/dummies{?dummyBoolean,relatedDummy.embeddedDummy.dummyBoolean,dummyDate[before],dummyDate[strictly_before],dummyDate[after],dummyDate[strictly_after],relatedDummy.dummyDate[before],relatedDummy.dummyDate[strictly_before],relatedDummy.dummyDate[after],relatedDummy.dummyDate[strictly_after],description[exists],relatedDummy.name[exists],dummyBoolean[exists],relatedDummy[exists],dummyFloat,dummyPrice,order[id],order[name],order[description],order[relatedDummy.name],order[relatedDummy.symfony],order[dummyDate],dummyFloat[between],dummyFloat[gt],dummyFloat[gte],dummyFloat[lt],dummyFloat[lte],dummyPrice[between],dummyPrice[gt],dummyPrice[gte],dummyPrice[lt],dummyPrice[lte],id,id[],name,alias,description,relatedDummy.name,relatedDummy.name[],relatedDummies,relatedDummies[],dummy,relatedDummies.name,properties[]}",
407+
"hydra:template": "/dummies{?dummyBoolean,relatedDummy.embeddedDummy.dummyBoolean,dummyDate[before],dummyDate[strictly_before],dummyDate[after],dummyDate[strictly_after],relatedDummy.dummyDate[before],relatedDummy.dummyDate[strictly_before],relatedDummy.dummyDate[after],relatedDummy.dummyDate[strictly_after],description[exists],relatedDummy.name[exists],dummyBoolean[exists],relatedDummy[exists],dummyFloat,dummyFloat[],dummyPrice,dummyPrice[],order[id],order[name],order[description],order[relatedDummy.name],order[relatedDummy.symfony],order[dummyDate],dummyFloat[between],dummyFloat[gt],dummyFloat[gte],dummyFloat[lt],dummyFloat[lte],dummyPrice[between],dummyPrice[gt],dummyPrice[gte],dummyPrice[lt],dummyPrice[lte],id,id[],name,alias,description,relatedDummy.name,relatedDummy.name[],relatedDummies,relatedDummies[],dummy,relatedDummies.name,properties[]}",
408408
"hydra:variableRepresentation": "BasicRepresentation",
409409
"hydra:mapping": [
410410
{
@@ -497,12 +497,24 @@ Feature: Date filter on collections
497497
"property": "dummyFloat",
498498
"required": false
499499
},
500+
{
501+
"@type": "IriTemplateMapping",
502+
"variable": "dummyFloat[]",
503+
"property": "dummyFloat",
504+
"required": false
505+
},
500506
{
501507
"@type": "IriTemplateMapping",
502508
"variable": "dummyPrice",
503509
"property": "dummyPrice",
504510
"required": false
505511
},
512+
{
513+
"@type": "IriTemplateMapping",
514+
"variable": "dummyPrice[]",
515+
"property": "dummyPrice",
516+
"required": false
517+
},
506518
{
507519
"@type": "IriTemplateMapping",
508520
"variable": "order[id]",

‎features/doctrine/numeric_filter.feature

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,51 @@ Feature: Numeric filter on collections
4747
}
4848
"""
4949

50+
@createSchema
51+
Scenario: Get collection by multiple dummyPrice
52+
Given there are 10 dummy objects with dummyPrice
53+
When I send a "GET" request to "/dummies?dummyPrice[]=9.99&dummyPrice[]=12.99"
54+
Then the response status code should be 200
55+
And the response should be in JSON
56+
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
57+
And print last JSON response
58+
And the JSON should be valid according to this schema:
59+
"""
60+
{
61+
"type": "object",
62+
"properties": {
63+
"@context": {"pattern": "^/contexts/Dummy$"},
64+
"@id": {"pattern": "^/dummies$"},
65+
"@type": {"pattern": "^hydra:Collection$"},
66+
"hydra:member": {
67+
"type": "array",
68+
"items": {
69+
"type": "object",
70+
"properties": {
71+
"@id": {
72+
"oneOf": [
73+
{"pattern": "^/dummies/1$"},
74+
{"pattern": "^/dummies/2$"},
75+
{"pattern": "^/dummies/5$"}
76+
]
77+
}
78+
}
79+
},
80+
"maxItems": 3,
81+
"uniqueItems": true
82+
},
83+
"hydra:totalItems": {"pattern": "^6$"},
84+
"hydra:view": {
85+
"type": "object",
86+
"properties": {
87+
"@id": {"pattern": "^/dummies\\?dummyPrice%5B%5D=9.99&dummyPrice%5B%5D=12.99"},
88+
"@type": {"pattern": "^hydra:PartialCollectionView$"}
89+
}
90+
}
91+
}
92+
}
93+
"""
94+
5095
Scenario: Get collection by non-numeric dummyPrice=marty
5196
Given there are 10 dummy objects with dummyPrice
5297
When I send a "GET" request to "/dummies?dummyPrice=marty"

‎features/main/crud.feature

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ Feature: Create-Retrieve-Update-Delete
135135
"hydra:totalItems": 1,
136136
"hydra:search": {
137137
"@type": "hydra:IriTemplate",
138-
"hydra:template": "/dummies{?dummyBoolean,relatedDummy.embeddedDummy.dummyBoolean,dummyDate[before],dummyDate[strictly_before],dummyDate[after],dummyDate[strictly_after],relatedDummy.dummyDate[before],relatedDummy.dummyDate[strictly_before],relatedDummy.dummyDate[after],relatedDummy.dummyDate[strictly_after],description[exists],relatedDummy.name[exists],dummyBoolean[exists],relatedDummy[exists],dummyFloat,dummyPrice,order[id],order[name],order[description],order[relatedDummy.name],order[relatedDummy.symfony],order[dummyDate],dummyFloat[between],dummyFloat[gt],dummyFloat[gte],dummyFloat[lt],dummyFloat[lte],dummyPrice[between],dummyPrice[gt],dummyPrice[gte],dummyPrice[lt],dummyPrice[lte],id,id[],name,alias,description,relatedDummy.name,relatedDummy.name[],relatedDummies,relatedDummies[],dummy,relatedDummies.name,properties[]}",
138+
"hydra:template": "/dummies{?dummyBoolean,relatedDummy.embeddedDummy.dummyBoolean,dummyDate[before],dummyDate[strictly_before],dummyDate[after],dummyDate[strictly_after],relatedDummy.dummyDate[before],relatedDummy.dummyDate[strictly_before],relatedDummy.dummyDate[after],relatedDummy.dummyDate[strictly_after],description[exists],relatedDummy.name[exists],dummyBoolean[exists],relatedDummy[exists],dummyFloat,dummyFloat[],dummyPrice,dummyPrice[],order[id],order[name],order[description],order[relatedDummy.name],order[relatedDummy.symfony],order[dummyDate],dummyFloat[between],dummyFloat[gt],dummyFloat[gte],dummyFloat[lt],dummyFloat[lte],dummyPrice[between],dummyPrice[gt],dummyPrice[gte],dummyPrice[lt],dummyPrice[lte],id,id[],name,alias,description,relatedDummy.name,relatedDummy.name[],relatedDummies,relatedDummies[],dummy,relatedDummies.name,properties[]}",
139139
"hydra:variableRepresentation": "BasicRepresentation",
140140
"hydra:mapping": [
141141
{
@@ -228,12 +228,24 @@ Feature: Create-Retrieve-Update-Delete
228228
"property": "dummyFloat",
229229
"required": false
230230
},
231+
{
232+
"@type": "IriTemplateMapping",
233+
"variable": "dummyFloat[]",
234+
"property": "dummyFloat",
235+
"required": false
236+
},
231237
{
232238
"@type": "IriTemplateMapping",
233239
"variable": "dummyPrice",
234240
"property": "dummyPrice",
235241
"required": false
236242
},
243+
{
244+
"@type": "IriTemplateMapping",
245+
"variable": "dummyPrice[]",
246+
"property": "dummyPrice",
247+
"required": false
248+
},
237249
{
238250
"@type": "IriTemplateMapping",
239251
"variable": "order[id]",

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

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,16 @@ public function getDescription(string $resourceClass): array
6161
continue;
6262
}
6363

64-
$description[$property] = [
65-
'property' => $property,
66-
'type' => $this->getType((string) $this->getDoctrineFieldType($property, $resourceClass)),
67-
'required' => false,
68-
];
64+
$filterParameterNames = [$property, $property.'[]'];
65+
66+
foreach ($filterParameterNames as $filterParameterName) {
67+
$description[$filterParameterName] = [
68+
'property' => $property,
69+
'type' => $this->getType((string) $this->getDoctrineFieldType($property, $resourceClass)),
70+
'required' => false,
71+
'is_collection' => '[]' === substr($filterParameterName, -2),
72+
];
73+
}
6974
}
7075

7176
return $description;
@@ -99,14 +104,24 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
99104
return;
100105
}
101106

102-
if (!is_numeric($value)) {
107+
if (!is_numeric($value) && (!\is_array($value) || !$this->isNumericArray($value))) {
103108
$this->logger->notice('Invalid filter ignored', [
104109
'exception' => new InvalidArgumentException(sprintf('Invalid numeric value for "%s::%s" property', $resourceClass, $property)),
105110
]);
106111

107112
return;
108113
}
109114

115+
$values = $this->normalizeValues((array) $value);
116+
117+
if (empty($values)) {
118+
$this->logger->notice('Invalid filter ignored', [
119+
'exception' => new InvalidArgumentException(sprintf('At least one value is required, multiple values should be in "%1$s[]=firstvalue&%1$s[]=secondvalue" format', $property)),
120+
]);
121+
122+
return;
123+
}
124+
110125
$alias = $queryBuilder->getRootAliases()[0];
111126
$field = $property;
112127

@@ -124,9 +139,15 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
124139

125140
$valueParameter = $queryNameGenerator->generateParameterName($field);
126141

127-
$queryBuilder
128-
->andWhere(sprintf('%s.%s = :%s', $alias, $field, $valueParameter))
129-
->setParameter($valueParameter, $value, (string) $this->getDoctrineFieldType($property, $resourceClass));
142+
if (1 === \count($values)) {
143+
$queryBuilder
144+
->andWhere(sprintf('%s.%s = :%s', $alias, $field, $valueParameter))
145+
->setParameter($valueParameter, $values[0], (string) $this->getDoctrineFieldType($property, $resourceClass));
146+
} else {
147+
$queryBuilder
148+
->andWhere(sprintf('%s.%s IN (:%s)', $alias, $field, $valueParameter))
149+
->setParameter($valueParameter, $values);
150+
}
130151
}
131152

132153
/**
@@ -139,4 +160,26 @@ protected function isNumericField(string $property, string $resourceClass): bool
139160

140161
return isset(self::DOCTRINE_NUMERIC_TYPES[(string) $metadata->getTypeOfField($propertyParts['field'])]);
141162
}
163+
164+
protected function isNumericArray(array $values): bool
165+
{
166+
foreach ($values as $value) {
167+
if (!is_numeric($value)) {
168+
return false;
169+
}
170+
}
171+
172+
return true;
173+
}
174+
175+
protected function normalizeValues(array $values): array
176+
{
177+
foreach ($values as $key => $value) {
178+
if (!\is_int($key)) {
179+
unset($values[$key]);
180+
}
181+
}
182+
183+
return array_values($values);
184+
}
142185
}

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ public function testGetDescription()
3838
'property' => 'id',
3939
'type' => 'int',
4040
'required' => false,
41+
'is_collection' => false,
42+
],
43+
'id[]' => [
44+
'property' => 'id',
45+
'type' => 'int',
46+
'required' => false,
47+
'is_collection' => true,
4148
],
4249
], $filter->getDescription($this->resourceClass));
4350
}
@@ -51,16 +58,37 @@ public function testGetDescriptionDefaultFields()
5158
'property' => 'id',
5259
'type' => 'int',
5360
'required' => false,
61+
'is_collection' => false,
62+
],
63+
'id[]' => [
64+
'property' => 'id',
65+
'type' => 'int',
66+
'required' => false,
67+
'is_collection' => true,
5468
],
5569
'dummyFloat' => [
5670
'property' => 'dummyFloat',
5771
'type' => 'float',
5872
'required' => false,
73+
'is_collection' => false,
74+
],
75+
'dummyFloat[]' => [
76+
'property' => 'dummyFloat',
77+
'type' => 'float',
78+
'required' => false,
79+
'is_collection' => true,
5980
],
6081
'dummyPrice' => [
6182
'property' => 'dummyPrice',
6283
'type' => 'string',
6384
'required' => false,
85+
'is_collection' => false,
86+
],
87+
'dummyPrice[]' => [
88+
'property' => 'dummyPrice',
89+
'type' => 'string',
90+
'required' => false,
91+
'is_collection' => true,
6492
],
6593
], $filter->getDescription($this->resourceClass));
6694
}
@@ -79,6 +107,50 @@ public function provideApplyTestData(): array
79107
],
80108
sprintf('SELECT o FROM %s o WHERE o.dummyPrice = :dummyPrice_p1', Dummy::class),
81109
],
110+
'multiple numeric string (positive integer)' => [
111+
[
112+
'id' => null,
113+
'name' => null,
114+
'dummyPrice' => null,
115+
],
116+
[
117+
'dummyPrice' => ['21', '22'],
118+
],
119+
sprintf('SELECT o FROM %s o WHERE o.dummyPrice IN (:dummyPrice_p1)', Dummy::class),
120+
],
121+
'multiple numeric string with one invalid property key' => [
122+
[
123+
'id' => null,
124+
'name' => null,
125+
'dummyPrice' => null,
126+
],
127+
[
128+
'dummyPrice' => ['invalid' => '21', '22'],
129+
],
130+
sprintf('SELECT o FROM %s o WHERE o.dummyPrice = :dummyPrice_p1', Dummy::class),
131+
],
132+
'multiple numeric string with invalid value keys' => [
133+
[
134+
'id' => null,
135+
'name' => null,
136+
'dummyPrice' => null,
137+
],
138+
[
139+
'dummyPrice' => ['invalid' => '21', 'invalid2' => '22'],
140+
],
141+
sprintf('SELECT o FROM %s o', Dummy::class),
142+
],
143+
'multiple non-numeric' => [
144+
[
145+
'id' => null,
146+
'name' => null,
147+
'dummyPrice' => null,
148+
],
149+
[
150+
'dummyPrice' => ['test', 'invalid'],
151+
],
152+
sprintf('SELECT o FROM %s o', Dummy::class),
153+
],
82154
'numeric string (negative integer)' => [
83155
[
84156
'id' => null,

0 commit comments

Comments
 (0)