Skip to content

Commit 80c9d45

Browse files
aaa2000aaa2000
committed
Fix review comments
1 parent a51cefc commit 80c9d45

File tree

2 files changed

+93
-22
lines changed

2 files changed

+93
-22
lines changed

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

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ private function filterProperty(string $property, mixed $value, QueryBuilder $qu
6767
$alias = $queryBuilder->getRootAliases()[0];
6868
$field = $property;
6969

70-
$values = (array) $value;
7170
$associations = [];
7271
if ($this->isPropertyNested($property, $resourceClass)) {
7372
[$alias, $field, $associations] = $this->addJoinsForNestedProperty($property, $alias, $queryBuilder, $queryNameGenerator, $resourceClass, Join::INNER_JOIN);
@@ -76,8 +75,8 @@ private function filterProperty(string $property, mixed $value, QueryBuilder $qu
7675
$metadata = $this->getNestedMetadata($resourceClass, $associations);
7776

7877
if ($metadata->hasField($field)) {
79-
$values = $this->convertValuesToTheDatabaseRepresentationIfNecessary($queryBuilder, $this->getDoctrineFieldType($property, $resourceClass), $values);
80-
$this->addWhere($queryBuilder, $queryNameGenerator, $alias, $field, $values);
78+
$value = $this->convertValuesToTheDatabaseRepresentationIfNecessary($queryBuilder, $this->getDoctrineFieldType($property, $resourceClass), $value);
79+
$this->addWhere($queryBuilder, $queryNameGenerator, $alias, $field, $value);
8180

8281
return;
8382
}
@@ -107,56 +106,67 @@ private function filterProperty(string $property, mixed $value, QueryBuilder $qu
107106
$associationField = $associationFieldIdentifier;
108107
}
109108

110-
$values = $this->convertValuesToTheDatabaseRepresentationIfNecessary($queryBuilder, $doctrineTypeField, $values);
111-
$this->addWhere($queryBuilder, $queryNameGenerator, $associationAlias, $associationField, $values);
109+
$value = $this->convertValuesToTheDatabaseRepresentationIfNecessary($queryBuilder, $doctrineTypeField, $value);
110+
$this->addWhere($queryBuilder, $queryNameGenerator, $associationAlias, $associationField, $value);
112111
}
113112

114113
/**
115114
* Converts values to their database representation.
116115
*/
117-
private function convertValuesToTheDatabaseRepresentationIfNecessary(QueryBuilder $queryBuilder, ?string $doctrineFieldType, array $values): array
116+
private function convertValuesToTheDatabaseRepresentationIfNecessary(QueryBuilder $queryBuilder, ?string $doctrineFieldType, mixed $value): mixed
118117
{
119118
if (null === $doctrineFieldType || !Type::hasType($doctrineFieldType)) {
120-
throw new \LogicException(\sprintf('The Doctrine type "%s" is not valid or not registered.', $doctrineFieldType));
119+
throw new InvalidArgumentException(\sprintf('The Doctrine type "%s" is not valid or not registered.', $doctrineFieldType));
121120
}
122121

123122
$doctrineType = Type::getType($doctrineFieldType);
124123
$platform = $queryBuilder->getEntityManager()->getConnection()->getDatabasePlatform();
125-
$databaseValues = [];
126-
127-
foreach ($values as $value) {
128-
try {
129-
$databaseValues[] = $doctrineType->convertToDatabaseValue($value, $platform);
130-
} catch (ConversionException $e) {
131-
$this->logger->notice('Invalid value conversion value to its database representation', [
132-
'exception' => $e,
133-
]);
134-
$databaseValues[] = null;
124+
125+
if (\is_array($value)) {
126+
$databaseValues = [];
127+
foreach ($value as $val) {
128+
try {
129+
$databaseValues[] = $doctrineType->convertToDatabaseValue($val, $platform);
130+
} catch (ConversionException $e) {
131+
$this->logger->notice('Invalid value conversion to database representation', [
132+
'exception' => $e,
133+
]);
134+
}
135135
}
136+
137+
return $databaseValues;
136138
}
137139

138-
return $databaseValues;
140+
try {
141+
return $doctrineType->convertToDatabaseValue($value, $platform);
142+
} catch (ConversionException $e) {
143+
$this->logger->notice('Invalid value conversion to database representation', [
144+
'exception' => $e,
145+
]);
146+
147+
return null;
148+
}
139149
}
140150

141151
/**
142152
* Adds where clause.
143153
*/
144-
private function addWhere(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $field, array $values): void
154+
private function addWhere(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $field, mixed $value): void
145155
{
146156
$valueParameter = ':'.$queryNameGenerator->generateParameterName($field);
147157
$aliasedField = \sprintf('%s.%s', $alias, $field);
148158

149-
if (1 === \count($values)) {
159+
if (!\is_array($value)) {
150160
$queryBuilder
151161
->andWhere($queryBuilder->expr()->eq($aliasedField, $valueParameter))
152-
->setParameter($valueParameter, $values[0], $this->getDoctrineParameterType());
162+
->setParameter($valueParameter, $value, $this->getDoctrineParameterType());
153163

154164
return;
155165
}
156166

157167
$queryBuilder
158168
->andWhere($queryBuilder->expr()->in($aliasedField, $valueParameter))
159-
->setParameter($valueParameter, $values, $this->getDoctrineArrayParameterType());
169+
->setParameter($valueParameter, $value, $this->getDoctrineArrayParameterType());
160170
}
161171

162172
protected function getDoctrineParameterType(): ?ParameterType

‎tests/Functional/Uuid/UuidFilterBaseTestCase.php‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,33 @@ public function testSearchFilterByInvalidUuid(): void
159159
);
160160
}
161161

162+
public function testSearchFilterByManyInvalidUuid(): void
163+
{
164+
$this->recreateSchema(static::getResources());
165+
166+
$manager = $this->getManager();
167+
$manager->persist($this->createDevice());
168+
$manager->persist($this->createDevice());
169+
$manager->flush();
170+
171+
$response = self::createClient()->request('GET', '/'.$this->getUrlPrefix().'_devices', [
172+
'query' => [
173+
'id' => ['invalid-uuid', 'other-invalid-uuid'],
174+
],
175+
]);
176+
177+
self::assertResponseIsSuccessful();
178+
$json = $response->toArray();
179+
180+
self::assertArraySubset(['hydra:totalItems' => 0], $json);
181+
self::assertArraySubset(
182+
[
183+
'hydra:member' => [],
184+
],
185+
$json
186+
);
187+
}
188+
162189
public function testSearchFilterOnManyToOneRelationByUuid(): void
163190
{
164191
$this->recreateSchema(static::getResources());
@@ -239,6 +266,40 @@ public function testSearchFilterOnManyToOneRelationByManyUuid(): void
239266
);
240267
}
241268

269+
public function testSearchFilterOnManyToOneRelationByInvalidUuids(): void
270+
{
271+
$this->recreateSchema(static::getResources());
272+
273+
$manager = $this->getManager();
274+
$manager->persist($fooDevice = $this->createDevice());
275+
$manager->persist($barDevice = $this->createDevice());
276+
$manager->persist($bazDevice = $this->createDevice());
277+
$manager->persist($this->createDeviceEndpoint(null, $fooDevice));
278+
$manager->persist($this->createDeviceEndpoint(null, $barDevice));
279+
$manager->persist($this->createDeviceEndpoint(null, $bazDevice));
280+
$manager->flush();
281+
282+
$response = self::createClient()->request('GET', '/'.$this->getUrlPrefix().'_device_endpoints', [
283+
'query' => [
284+
'myDevice' => [
285+
'invalid-uuid',
286+
'other-invalid-uuid',
287+
],
288+
],
289+
]);
290+
291+
self::assertResponseIsSuccessful();
292+
$json = $response->toArray();
293+
294+
self::assertArraySubset(['hydra:totalItems' => 0], $json);
295+
self::assertArraySubset(
296+
[
297+
'hydra:member' => [],
298+
],
299+
$json
300+
);
301+
}
302+
242303
public function testGetOpenApiDescription(): void
243304
{
244305
$response = self::createClient()->request('GET', '/docs', [

0 commit comments

Comments
 (0)