Skip to content

Commit f6a68c0

Browse files
soyukasoyuka
committed
human review
1 parent b0c1b14 commit f6a68c0

6 files changed

Lines changed: 39 additions & 74 deletions

File tree

‎src/Doctrine/Odm/Filter/ComparisonFilter.php‎

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,6 @@ final class ComparisonFilter implements FilterInterface, OpenApiParameterFilterI
4646
'lte' => 'lte',
4747
];
4848

49-
public const ALLOWED_COMPARISON_METHODS = ['equals', 'gt', 'gte', 'lt', 'lte'];
50-
5149
public function __construct(private readonly FilterInterface $filter)
5250
{
5351
}
@@ -87,19 +85,21 @@ public function getOpenApiParameters(Parameter $parameter): array
8785
{
8886
$in = $parameter instanceof QueryParameter ? 'query' : 'header';
8987
$key = $parameter->getKey();
90-
$schema = $this->getInnerSchema($parameter);
9188

9289
return [
93-
new OpenApiParameter(name: "{$key}[gt]", in: $in, schema: $schema),
94-
new OpenApiParameter(name: "{$key}[gte]", in: $in, schema: $schema),
95-
new OpenApiParameter(name: "{$key}[lt]", in: $in, schema: $schema),
96-
new OpenApiParameter(name: "{$key}[lte]", in: $in, schema: $schema),
90+
new OpenApiParameter(name: "{$key}[gt]", in: $in),
91+
new OpenApiParameter(name: "{$key}[gte]", in: $in),
92+
new OpenApiParameter(name: "{$key}[lt]", in: $in),
93+
new OpenApiParameter(name: "{$key}[lte]", in: $in),
9794
];
9895
}
9996

10097
public function getSchema(Parameter $parameter): array
10198
{
102-
$innerSchema = $this->getInnerSchema($parameter);
99+
$innerSchema = ['type' => 'string'];
100+
if ($this->filter instanceof JsonSchemaFilterInterface) {
101+
$innerSchema = $this->filter->getSchema($parameter);
102+
}
103103

104104
return [
105105
'type' => 'object',
@@ -130,13 +130,4 @@ private function applyOperator(Builder $aggregationBuilder, string $resourceClas
130130
$context['match'] = $newContext['match'];
131131
}
132132
}
133-
134-
private function getInnerSchema(Parameter $parameter): array
135-
{
136-
if ($this->filter instanceof JsonSchemaFilterInterface) {
137-
return $this->filter->getSchema($parameter);
138-
}
139-
140-
return ['type' => 'string'];
141-
}
142133
}

‎src/Doctrine/Odm/Filter/ExactFilter.php‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,6 @@ public function apply(Builder $aggregationBuilder, string $resourceClass, ?Opera
6262

6363
if (!$classMetadata->hasReference($property)) {
6464
$comparisonMethod = $context['comparisonMethod'] ?? (is_iterable($value) ? 'in' : 'equals');
65-
if (!\in_array($comparisonMethod, ComparisonFilter::ALLOWED_COMPARISON_METHODS, true) && 'in' !== $comparisonMethod) {
66-
throw new InvalidArgumentException(\sprintf('Unsupported comparison method "%s".', $comparisonMethod));
67-
}
6865
$match
6966
->{$operator}($aggregationBuilder->matchExpr()->field($property)->{$comparisonMethod}($value));
7067

‎src/Doctrine/Orm/Filter/ComparisonFilter.php‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -85,19 +85,21 @@ public function getOpenApiParameters(Parameter $parameter): array
8585
{
8686
$in = $parameter instanceof QueryParameter ? 'query' : 'header';
8787
$key = $parameter->getKey();
88-
$schema = $this->getInnerSchema($parameter);
8988

9089
return [
91-
new OpenApiParameter(name: "{$key}[gt]", in: $in, schema: $schema),
92-
new OpenApiParameter(name: "{$key}[gte]", in: $in, schema: $schema),
93-
new OpenApiParameter(name: "{$key}[lt]", in: $in, schema: $schema),
94-
new OpenApiParameter(name: "{$key}[lte]", in: $in, schema: $schema),
90+
new OpenApiParameter(name: "{$key}[gt]", in: $in),
91+
new OpenApiParameter(name: "{$key}[gte]", in: $in),
92+
new OpenApiParameter(name: "{$key}[lt]", in: $in),
93+
new OpenApiParameter(name: "{$key}[lte]", in: $in),
9594
];
9695
}
9796

9897
public function getSchema(Parameter $parameter): array
9998
{
100-
$innerSchema = $this->getInnerSchema($parameter);
99+
$innerSchema = ['type' => 'string'];
100+
if ($this->filter instanceof JsonSchemaFilterInterface) {
101+
$innerSchema = $this->filter->getSchema($parameter);
102+
}
101103

102104
return [
103105
'type' => 'object',
@@ -110,6 +112,9 @@ public function getSchema(Parameter $parameter): array
110112
];
111113
}
112114

115+
/**
116+
* @param array<string,mixed> $context
117+
*/
113118
private function applyOperator(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, ?Operation $operation, array $context, Parameter $parameter, string $operator, mixed $value): void
114119
{
115120
if (!\is_string($value) && !is_numeric($value) && !$value instanceof \DateTimeInterface) {
@@ -125,13 +130,4 @@ private function applyOperator(QueryBuilder $queryBuilder, QueryNameGeneratorInt
125130
['operator' => $operator, 'parameter' => $subParameter] + $context
126131
);
127132
}
128-
129-
private function getInnerSchema(Parameter $parameter): array
130-
{
131-
if ($this->filter instanceof JsonSchemaFilterInterface) {
132-
return $this->filter->getSchema($parameter);
133-
}
134-
135-
return ['type' => 'string'];
136-
}
137133
}

‎tests/Fixtures/TestBundle/Document/Chicken.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@
6363
filter: new ExactFilter(),
6464
properties: ['owner.name'],
6565
),
66-
'idComparison' => new QueryParameter(
66+
'nameComparison' => new QueryParameter(
6767
filter: new ComparisonFilter(new ExactFilter()),
68-
property: 'id',
68+
property: 'name',
6969
),
7070
],
7171
),

‎tests/Fixtures/TestBundle/Entity/Chicken.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@
6363
filter: new ExactFilter(),
6464
properties: ['owner.name'],
6565
),
66-
'idComparison' => new QueryParameter(
66+
'nameComparison' => new QueryParameter(
6767
filter: new ComparisonFilter(new ExactFilter()),
68-
property: 'id',
68+
property: 'name',
6969
),
7070
],
7171
),

‎tests/Functional/Parameters/ComparisonFilterTest.php‎

Lines changed: 16 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -48,26 +48,10 @@ protected function setUp(): void
4848
$this->loadFixtures();
4949
}
5050

51-
/**
52-
* @return array<string, int|string> name => id mapping
53-
*/
54-
private function getChickenIds(): array
55-
{
56-
$response = self::createClient()->request('GET', '/chickens?itemsPerPage=10');
57-
$members = $response->toArray()['member'];
58-
$ids = [];
59-
foreach ($members as $member) {
60-
$ids[$member['name']] = $member['id'];
61-
}
62-
63-
return $ids;
64-
}
65-
6651
public function testGt(): void
6752
{
68-
$ids = $this->getChickenIds();
69-
// gt: id > second chicken should return third and fourth
70-
$response = self::createClient()->request('GET', '/chickens?idComparison[gt]='.$ids['Bravo']);
53+
// gt "Bravo": names > "Bravo" alphabetically → Charlie, Delta
54+
$response = self::createClient()->request('GET', '/chickens?nameComparison[gt]=Bravo');
7155
$this->assertResponseIsSuccessful();
7256
$names = array_map(static fn ($c) => $c['name'], $response->toArray()['member']);
7357
sort($names);
@@ -76,8 +60,8 @@ public function testGt(): void
7660

7761
public function testGte(): void
7862
{
79-
$ids = $this->getChickenIds();
80-
$response = self::createClient()->request('GET', '/chickens?idComparison[gte]='.$ids['Bravo']);
63+
// gte "Bravo": names >= "Bravo" → Bravo, Charlie, Delta
64+
$response = self::createClient()->request('GET', '/chickens?nameComparison[gte]=Bravo');
8165
$this->assertResponseIsSuccessful();
8266
$names = array_map(static fn ($c) => $c['name'], $response->toArray()['member']);
8367
sort($names);
@@ -86,8 +70,8 @@ public function testGte(): void
8670

8771
public function testLt(): void
8872
{
89-
$ids = $this->getChickenIds();
90-
$response = self::createClient()->request('GET', '/chickens?idComparison[lt]='.$ids['Charlie']);
73+
// lt "Charlie": names < "Charlie" → Alpha, Bravo
74+
$response = self::createClient()->request('GET', '/chickens?nameComparison[lt]=Charlie');
9175
$this->assertResponseIsSuccessful();
9276
$names = array_map(static fn ($c) => $c['name'], $response->toArray()['member']);
9377
sort($names);
@@ -96,8 +80,8 @@ public function testLt(): void
9680

9781
public function testLte(): void
9882
{
99-
$ids = $this->getChickenIds();
100-
$response = self::createClient()->request('GET', '/chickens?idComparison[lte]='.$ids['Charlie']);
83+
// lte "Charlie": names <= "Charlie" → Alpha, Bravo, Charlie
84+
$response = self::createClient()->request('GET', '/chickens?nameComparison[lte]=Charlie');
10185
$this->assertResponseIsSuccessful();
10286
$names = array_map(static fn ($c) => $c['name'], $response->toArray()['member']);
10387
sort($names);
@@ -106,8 +90,8 @@ public function testLte(): void
10690

10791
public function testCombinedGtAndLt(): void
10892
{
109-
$ids = $this->getChickenIds();
110-
$response = self::createClient()->request('GET', '/chickens?idComparison[gt]='.$ids['Alpha'].'&idComparison[lt]='.$ids['Delta']);
93+
// gt "Alpha" AND lt "Delta" → Bravo, Charlie
94+
$response = self::createClient()->request('GET', '/chickens?nameComparison[gt]=Alpha&nameComparison[lt]=Delta');
11195
$this->assertResponseIsSuccessful();
11296
$names = array_map(static fn ($c) => $c['name'], $response->toArray()['member']);
11397
sort($names);
@@ -116,16 +100,16 @@ public function testCombinedGtAndLt(): void
116100

117101
public function testGtNoResults(): void
118102
{
119-
$response = self::createClient()->request('GET', '/chickens?idComparison[gt]=999999');
103+
// gt "ZZZZ": no name is alphabetically after "ZZZZ"
104+
$response = self::createClient()->request('GET', '/chickens?nameComparison[gt]=ZZZZ');
120105
$this->assertResponseIsSuccessful();
121106
$this->assertCount(0, $response->toArray()['member']);
122107
}
123108

124109
public function testGteAllResults(): void
125110
{
126-
$ids = $this->getChickenIds();
127-
$minId = min($ids);
128-
$response = self::createClient()->request('GET', '/chickens?idComparison[gte]='.$minId.'&itemsPerPage=10');
111+
// gte "A": all names start with A or later → all 4
112+
$response = self::createClient()->request('GET', '/chickens?nameComparison[gte]=A&itemsPerPage=10');
129113
$this->assertResponseIsSuccessful();
130114
$this->assertCount(4, $response->toArray()['member']);
131115
}
@@ -141,16 +125,13 @@ public function testOpenApiDocumentation(): void
141125
$parameters = $openApiDoc['paths']['/chickens']['get']['parameters'];
142126
$parameterNames = array_column($parameters, 'name');
143127

144-
foreach (['idComparison[gt]', 'idComparison[gte]', 'idComparison[lt]', 'idComparison[lte]'] as $expectedName) {
128+
foreach (['nameComparison[gt]', 'nameComparison[gte]', 'nameComparison[lt]', 'nameComparison[lte]'] as $expectedName) {
145129
$this->assertContains($expectedName, $parameterNames, \sprintf('Expected parameter "%s" in OpenAPI documentation', $expectedName));
146130
}
147131

148-
$comparisonParams = array_filter($parameters, static fn ($p) => str_starts_with($p['name'], 'idComparison['));
132+
$comparisonParams = array_filter($parameters, static fn ($p) => str_starts_with($p['name'], 'nameComparison['));
149133
foreach ($comparisonParams as $param) {
150134
$this->assertSame('query', $param['in']);
151-
$this->assertArrayHasKey('schema', $param);
152-
153-
$this->assertSame('string', $param['schema']['type']);
154135
}
155136
}
156137

0 commit comments

Comments
 (0)