fix searchFilter with binary types (like UUID)

Author: odoucet

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

@@ -21,8 +21,10 @@
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
 use ApiPlatform\Core\Exception\InvalidArgumentException;
 use Doctrine\DBAL\Types\Type as DBALType;
+use Doctrine\ORM\Query\Parameter;
 use Doctrine\ORM\QueryBuilder;
 use Doctrine\Persistence\ManagerRegistry;
+use Doctrine\Persistence\Mapping\ClassMetadata;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\PropertyAccess\PropertyAccess;
@@ -113,7 +115,7 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
                 $caseSensitive = false;
             }
 
-            $this->addWhereByStrategy($strategy, $queryBuilder, $queryNameGenerator, $alias, $field, $values, $caseSensitive);
+            $this->addWhereByStrategy($strategy, $queryBuilder, $queryNameGenerator, $alias, $field, $values, $caseSensitive, $metadata);
 
             return;
         }
@@ -149,14 +151,39 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
             $associationField = $associationFieldIdentifier;
         }
 
-        if (1 === \count($values)) {
+        /*
+         * If field type is string/float, Doctrine does not call convertToDatabaseValueSQL() because it
+         * does not know it needs conversion.
+         * This would lead to incorrect values for Ramsey\Uuid\Doctrine\UuidBinaryType for example.
+         * The only fix is to provide field type to doctrine ...
+         * It's easy if setParameter() sets only one value BUT impossible if multiple values are provided.
+         * The only way to do this is to rewrite the IN() statement to multiple values
+         * and map a global setParameters()
+         */
+        $type = $metadata->getTypeOfField($associationField);
+        $nbValues = \count($values);
+
+        if (1 === $nbValues) {
             $queryBuilder
                 ->andWhere($queryBuilder->expr()->eq($associationAlias.'.'.$associationField, ':'.$valueParameter))
-                ->setParameter($valueParameter, $values[0]);
+                ->setParameter($valueParameter, $values[0], $type);
         } else {
+            // get current parameters, because QueryBuilder->setParameters() erase previous parameters set
+            $parameters = $queryBuilder->getParameters();
+            $inQuery = [];
+
+            // convertToDatabaseValue() can only convert one value at a time ... We can no longer pass an array of values, we should use multiple values
+
+            for ($i = 0; $i < $nbValues; ++$i) {
+                $inQuery[] = ':'.$valueParameter;
+                $parameters->add(new Parameter($valueParameter, $values[$i], $type));
+                $valueParameter = $queryNameGenerator->generateParameterName($associationField);
+            }
+
+            // we cannot use expr->in() here because it considers $inQuery parameters as strings.
             $queryBuilder
-                ->andWhere($queryBuilder->expr()->in($associationAlias.'.'.$associationField, ':'.$valueParameter))
-                ->setParameter($valueParameter, $values);
+                ->andWhere($associationAlias.'.'.$associationField.' IN ('.implode(', ', $inQuery).')')
+                ->setParameters($parameters);
         }
     }
 
@@ -165,28 +192,47 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
      *
      * @throws InvalidArgumentException If strategy does not exist
      */
-    protected function addWhereByStrategy(string $strategy, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $field, $values, bool $caseSensitive)
+    protected function addWhereByStrategy(string $strategy, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $field, $values, bool $caseSensitive/*, ClassMetadata $metadata = null*/)
     {
+        // check if we have metadata
+        if (\func_num_args() > 7 && ($metadata = func_get_arg(7)) instanceof ClassMetadata) {
+            $type = $metadata->getTypeOfField($field);
+        } else {
+            @trigger_error(sprintf('Method %s() will have a 8th argument `$metadata` in version API Platform 3.0.', __FUNCTION__), E_USER_DEPRECATED);
+            $type = null; // default setParameter() value
+        }
+
         if (!\is_array($values)) {
             $values = [$values];
         }
 
+        $nbValues = \count($values);
         $wrapCase = $this->createWrapCase($caseSensitive);
         $valueParameter = ':'.$queryNameGenerator->generateParameterName($field);
         $aliasedField = sprintf('%s.%s', $alias, $field);
 
         if (null == $strategy || self::STRATEGY_EXACT == $strategy) {
-            if (1 == \count($values)) {
+            if (1 == $nbValues) {
                 $queryBuilder
                     ->andWhere($queryBuilder->expr()->eq($wrapCase($aliasedField), $wrapCase($valueParameter)))
-                    ->setParameter($valueParameter, $values[0]);
+                    ->setParameter($valueParameter, $values[0], $type);
 
                 return;
             }
 
+            // get current parameters, because QueryBuilder->setParameters() erase previous parameters set
+            $parameters = $queryBuilder->getParameters();
+            $inQuery = [];
+            for ($i = 0; $i < $nbValues; ++$i) {
+                $inQuery[] = $valueParameter;
+                $parameters->add(new Parameter($valueParameter, $caseSensitive ? $values[$i] : strtolower($values[$i]), $type));
+                $valueParameter = ':'.$queryNameGenerator->generateParameterName($field);
+            }
+
+            // we cannot use expr->in() here because it considers $inQuery parameters as strings.
             $queryBuilder
-                ->andWhere($queryBuilder->expr()->in($wrapCase($aliasedField), $valueParameter))
-                ->setParameter($valueParameter, $caseSensitive ? $values : array_map('strtolower', $values));
+                ->andWhere($wrapCase($aliasedField).' IN ('.implode(', ', $inQuery).')')
+                ->setParameters($parameters);
 
             return;
         }
@@ -228,7 +274,7 @@ protected function addWhereByStrategy(string $strategy, QueryBuilder $queryBuild
         }
 
         $queryBuilder->andWhere($queryBuilder->expr()->orX(...$ors));
-        array_walk($parameters, [$queryBuilder, 'setParameter']);
+        array_walk($parameters, [$queryBuilder, 'setParameter'], $type);
     }
 
     /**

tests/Bridge/Doctrine/Orm/Filter/SearchFilterTest.php

@@ -390,22 +390,18 @@ public function provideApplyTestData(): array
                     $filterFactory,
                 ],
                 'exact (multiple values)' => [
-                    sprintf('SELECT %s FROM %s %1$s WHERE %1$s.name IN(:name_p1)', $this->alias, Dummy::class),
+                    sprintf('SELECT %s FROM %s %1$s WHERE %1$s.name IN (:name_p1, :name_p2)', $this->alias, Dummy::class),
                     [
-                        'name_p1' => [
-                            'CaSE',
-                            'SENSitive',
-                        ],
+                        'name_p1' => 'CaSE',
+                        'name_p2' => 'SENSitive',
                     ],
                     $filterFactory,
                 ],
                 'exact (multiple values; case insensitive)' => [
-                    sprintf('SELECT %s FROM %s %1$s WHERE LOWER(%1$s.name) IN(:name_p1)', $this->alias, Dummy::class),
+                    sprintf('SELECT %s FROM %s %1$s WHERE LOWER(%1$s.name) IN (:name_p1, :name_p2)', $this->alias, Dummy::class),
                     [
-                        'name_p1' => [
-                            'case',
-                            'insensitive',
-                        ],
+                        'name_p1' => 'case',
+                        'name_p2' => 'insensitive',
                     ],
                     $filterFactory,
                 ],
@@ -547,10 +543,11 @@ public function provideApplyTestData(): array
                     $filterFactory,
                 ],
                 'mixed IRI and entity ID values for relations' => [
-                    sprintf('SELECT %s FROM %s %1$s INNER JOIN %1$s.relatedDummies relatedDummies_a1 WHERE %1$s.relatedDummy IN(:relatedDummy_p1) AND relatedDummies_a1.id = :relatedDummies_p2', $this->alias, Dummy::class),
+                    sprintf('SELECT %s FROM %s %1$s INNER JOIN %1$s.relatedDummies relatedDummies_a1 WHERE %1$s.relatedDummy IN (:relatedDummy_p1, :relatedDummy_p2) AND relatedDummies_a1.id = :relatedDummies_p4', $this->alias, Dummy::class),
                     [
-                        'relatedDummy_p1' => [1, 2],
-                        'relatedDummies_p2' => 1,
+                        'relatedDummy_p1' => 1,
+                        'relatedDummy_p2' => 2,
+                        'relatedDummies_p4' => 1,
                     ],
                     $filterFactory,
                 ],