SearchFilter: Refactor code to reduce code duplication and makes it easier to add new strategies to classes inheriting from SearchFilter class

Author: jpierront

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

@@ -19,7 +19,6 @@
 use ApiPlatform\Core\Bridge\Doctrine\Common\Filter\SearchFilterTrait;
 use ApiPlatform\Core\Exception\InvalidArgumentException;
 use Doctrine\Common\Persistence\ManagerRegistry;
-use Doctrine\Common\Persistence\Mapping\ClassMetadata;
 use Doctrine\ODM\MongoDB\Aggregation\Builder;
 use Doctrine\ODM\MongoDB\Mapping\ClassMetadata as MongoDBClassMetadata;
 use Doctrine\ODM\MongoDB\Types\Type as MongoDbType;
@@ -116,8 +115,9 @@ protected function filterProperty(string $property, $value, Builder $aggregation
             }
 
             $inValues = [];
+            $type = $metadata->getTypeOfField($field);
             foreach ($values as $inValue) {
-                $inValues[] = $this->addEqualityMatchStrategy($strategy, $field, $inValue, $caseSensitive, $metadata);
+                $inValues[] = $this->addEqualityMatchStrategy($strategy, $inValue, $caseSensitive, $type);
             }
 
             $aggregationBuilder
@@ -149,10 +149,15 @@ protected function filterProperty(string $property, $value, Builder $aggregation
             return;
         }
 
+        $inValues = [];
+        foreach ($values as $inValue) {
+            $inValues[] = $this->addEqualityMatchStrategy(self::STRATEGY_EXACT, $inValue, $caseSensitive, $doctrineTypeField);
+        }
+
         $aggregationBuilder
             ->match()
             ->field($matchField)
-            ->in($values);
+            ->in($inValues);
     }
 
     /**
@@ -162,10 +167,8 @@ protected function filterProperty(string $property, $value, Builder $aggregation
      *
      * @return Regex|string
      */
-    private function addEqualityMatchStrategy(string $strategy, string $field, $value, bool $caseSensitive, ClassMetadata $metadata)
+    private function addEqualityMatchStrategy(string $strategy, $value, bool $caseSensitive, string $type)
     {
-        $type = $metadata->getTypeOfField($field);
-
         switch ($strategy) {
             case MongoDbType::STRING !== $type:
                 return MongoDbType::getType($type)->convertToDatabaseValue($value);

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

@@ -112,26 +112,8 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
                 $caseSensitive = false;
             }
 
-            if (1 === \count($values)) {
-                $this->addWhereByStrategy($strategy, $queryBuilder, $queryNameGenerator, $alias, $field, $doctrineTypeField, $values[0], $caseSensitive);
-
-                return;
-            }
-
-            if (self::STRATEGY_EXACT !== $strategy) {
-                $this->logger->notice('Invalid filter ignored', [
-                    'exception' => new InvalidArgumentException(sprintf('"%s" strategy selected for "%s" property, but only "%s" strategy supports multiple values', $strategy, $property, self::STRATEGY_EXACT)),
-                ]);
-
-                return;
-            }
-
-            $wrapCase = $this->createWrapCase($caseSensitive);
             $valueParameter = $queryNameGenerator->generateParameterName($field);
-
-            $queryBuilder
-                ->andWhere(sprintf($wrapCase('%s.%s').' IN (:%s)', $alias, $field, $valueParameter))
-                ->setParameter($valueParameter, $caseSensitive ? $values : array_map('strtolower', $values));
+            $this->addWhereByStrategy($strategy, $queryBuilder, $alias, $field, $doctrineTypeField, $values, $caseSensitive, $valueParameter);
         }
 
         // metadata doesn't have the field, nor an association on the field
@@ -155,63 +137,67 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
             return;
         }
 
-        $association = $field;
-        $valueParameter = $queryNameGenerator->generateParameterName($association);
-        if ($metadata->isCollectionValuedAssociation($association)) {
-            $associationAlias = QueryBuilderHelper::addJoinOnce($queryBuilder, $queryNameGenerator, $alias, $association);
+        $valueParameter = $queryNameGenerator->generateParameterName($field);
+        if ($metadata->isCollectionValuedAssociation($field)) {
+            $associationAlias = QueryBuilderHelper::addJoinOnce($queryBuilder, $queryNameGenerator, $alias, $field);
             $associationField = $associationFieldIdentifier;
         } else {
             $associationAlias = $alias;
             $associationField = $field;
         }
 
-        if (1 === \count($values)) {
-            $queryBuilder
-                ->andWhere(sprintf('%s.%s = :%s', $associationAlias, $associationField, $valueParameter))
-                ->setParameter($valueParameter, $values[0], $doctrineTypeField);
-        } else {
-            $queryBuilder
-                ->andWhere(sprintf('%s.%s IN (:%s)', $associationAlias, $associationField, $valueParameter))
-                ->setParameter($valueParameter, $values, $doctrineTypeField);
-        }
+        $this->addWhereByStrategy(self::STRATEGY_EXACT, $queryBuilder, $associationAlias, $associationField, $doctrineTypeField, $values, $caseSensitive, $valueParameter);
     }
 
     /**
      * Adds where clause according to the strategy.
      *
      * @throws InvalidArgumentException If strategy does not exist
      */
-    protected function addWhereByStrategy(string $strategy, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $field, $fieldType, $value, bool $caseSensitive)
+    protected function addWhereByStrategy(string $strategy, QueryBuilder $queryBuilder, string $alias, string $field, $fieldType, array $values, bool $caseSensitive, string $valueParameter)
     {
         $wrapCase = $this->createWrapCase($caseSensitive);
-        $valueParameter = $queryNameGenerator->generateParameterName($field);
+
+        if (self::STRATEGY_EXACT !== $strategy && 1 !== \count($values)) {
+            $this->logger->notice('Invalid filter ignored', [
+                'exception' => new InvalidArgumentException(sprintf('"%s" strategy selected for "%s" property, but only "%s" strategy supports multiple values', $strategy, $field, self::STRATEGY_EXACT)),
+            ]);
+
+            return;
+        }
 
         switch ($strategy) {
             case null:
             case self::STRATEGY_EXACT:
-                $queryBuilder
-                    ->andWhere(sprintf($wrapCase('%s.%s').' = '.$wrapCase(':%s'), $alias, $field, $valueParameter))
-                    ->setParameter($valueParameter, $value, $fieldType);
+                if (1 === \count($values)) {
+                    $queryBuilder
+                        ->andWhere(sprintf($wrapCase('%s.%s').' = '.$wrapCase(':%s'), $alias, $field, $valueParameter))
+                        ->setParameter($valueParameter, $values[0], $fieldType);
+                } else {
+                    $queryBuilder
+                        ->andWhere(sprintf($wrapCase('%s.%s').' IN (:%s)', $alias, $field, $valueParameter))
+                        ->setParameter($valueParameter, $caseSensitive ? $values : array_map('strtolower', $values));
+                }
                 break;
             case self::STRATEGY_PARTIAL:
                 $queryBuilder
                     ->andWhere(sprintf($wrapCase('%s.%s').' LIKE '.$wrapCase('CONCAT(\'%%\', :%s, \'%%\')'), $alias, $field, $valueParameter))
-                    ->setParameter($valueParameter, $value, $fieldType);
+                    ->setParameter($valueParameter, $values[0], $fieldType);
                 break;
             case self::STRATEGY_START:
                 $queryBuilder
                     ->andWhere(sprintf($wrapCase('%s.%s').' LIKE '.$wrapCase('CONCAT(:%s, \'%%\')'), $alias, $field, $valueParameter))
-                    ->setParameter($valueParameter, $value, $fieldType);
+                    ->setParameter($valueParameter, $values[0], $fieldType);
                 break;
             case self::STRATEGY_END:
                 $queryBuilder
                     ->andWhere(sprintf($wrapCase('%s.%s').' LIKE '.$wrapCase('CONCAT(\'%%\', :%s)'), $alias, $field, $valueParameter))
-                    ->setParameter($valueParameter, $value, $fieldType);
+                    ->setParameter($valueParameter, $values[0], $fieldType);
                 break;
             case self::STRATEGY_WORD_START:
                 $queryBuilder
                     ->andWhere(sprintf($wrapCase('%1$s.%2$s').' LIKE '.$wrapCase('CONCAT(:%3$s, \'%%\')').' OR '.$wrapCase('%1$s.%2$s').' LIKE '.$wrapCase('CONCAT(\'%% \', :%3$s, \'%%\')'), $alias, $field, $valueParameter))
-                    ->setParameter($valueParameter, $value, $fieldType);
+                    ->setParameter($valueParameter, $values[0], $fieldType);
                 break;
             default:
                 throw new InvalidArgumentException(sprintf('strategy %s does not exist.', $strategy));