Support filter operator with null value

PowerKiKi · PowerKiKi · commit c4384ec98798 · 2018-05-13T12:14:55.000+09:00
In most cases an operator receiving no arguments will not be applied
and be entirely ignored. This is now supported.
diff --git a/src/Definition/Operator/AbstractOperator.php b/src/Definition/Operator/AbstractOperator.php
@@ -65,9 +65,9 @@ abstract protected function getConfiguration(Types $types, LeafType $leafType):
      * @param QueryBuilder $queryBuilder
      * @param string $alias the alias for the entity on which to apply the filter
      * @param string $field the field for the entity on which to apply the filter
-     * @param array $args all arguments specific to this operator as declared in its configuration
+     * @param null|array $args all arguments specific to this operator as declared in its configuration
      *
      * @return null|string
      */
-    abstract public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string;
+    abstract public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string;
 }
diff --git a/src/Definition/Operator/AbstractSimpleOperator.php b/src/Definition/Operator/AbstractSimpleOperator.php
@@ -35,8 +35,12 @@ final protected function getConfiguration(Types $types, LeafType $leafType): arr
         ];
     }
 
-    final public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string
+    final public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string
     {
+        if ($args === null) {
+            return null;
+        }
+
         $param = $uniqueNameFactory->createParameterName();
         $queryBuilder->setParameter($param, $args['value']);
 
diff --git a/src/Definition/Operator/BetweenOperatorType.php b/src/Definition/Operator/BetweenOperatorType.php
@@ -34,8 +34,12 @@ protected function getConfiguration(Types $types, LeafType $leafType): array
         ];
     }
 
-    public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string
+    public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string
     {
+        if ($args === null) {
+            return null;
+        }
+
         $from = $uniqueNameFactory->createParameterName();
         $to = $uniqueNameFactory->createParameterName();
         $queryBuilder->setParameter($from, $args['from']);
diff --git a/src/Definition/Operator/ContainOperatorType.php b/src/Definition/Operator/ContainOperatorType.php
@@ -30,8 +30,12 @@ protected function getConfiguration(Types $types, LeafType $leafType): array
         ];
     }
 
-    public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string
+    public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string
     {
+        if ($args === null) {
+            return null;
+        }
+
         $values = $uniqueNameFactory->createParameterName();
         $queryBuilder->setParameter($values, $args['values']);
         $not = $args['not'] ? 'NOT ' : '';
diff --git a/src/Definition/Operator/EmptyOperatorType.php b/src/Definition/Operator/EmptyOperatorType.php
@@ -26,8 +26,12 @@ protected function getConfiguration(Types $types, LeafType $leafType): array
         ];
     }
 
-    public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string
+    public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string
     {
+        if ($args === null) {
+            return null;
+        }
+
         $not = $args['not'] ? 'NOT ' : '';
 
         return $alias . '.' . $field . ' IS ' . $not . 'EMPTY';
diff --git a/src/Definition/Operator/InOperatorType.php b/src/Definition/Operator/InOperatorType.php
@@ -30,8 +30,12 @@ protected function getConfiguration(Types $types, LeafType $leafType): array
         ];
     }
 
-    public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string
+    public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string
     {
+        if ($args === null) {
+            return null;
+        }
+
         $values = $uniqueNameFactory->createParameterName();
         $queryBuilder->setParameter($values, $args['values']);
         $not = $args['not'] ? 'NOT ' : '';
diff --git a/src/Definition/Operator/NullOperatorType.php b/src/Definition/Operator/NullOperatorType.php
@@ -26,8 +26,12 @@ protected function getConfiguration(Types $types, LeafType $leafType): array
         ];
     }
 
-    public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string
+    public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string
     {
+        if ($args === null) {
+            return null;
+        }
+
         $not = $args['not'] ? 'NOT ' : '';
 
         return $alias . '.' . $field . ' IS ' . $not . 'NULL';
diff --git a/src/Factory/FilteredQueryBuilderFactory.php b/src/Factory/FilteredQueryBuilderFactory.php
@@ -81,6 +81,9 @@ private function applyFilters(ClassMetadata $metadata, InputObjectType $type, ar
         foreach ($filter['conditions'] ?? [] as $conditions) {
             $dqlConditions = [];
             foreach ($conditions['fields'] as $field => $fieldConditions) {
+                if ($fieldConditions === null) {
+                    return;
+                }
 
                 /** @var InputObjectType $typeField */
                 $typeField = $typeFields->getField($field)->type;
diff --git a/tests/Blog/Filtering/ModuloOperatorType.php b/tests/Blog/Filtering/ModuloOperatorType.php
@@ -26,7 +26,7 @@ protected function getConfiguration(Types $types, LeafType $leafType): array
         ];
     }
 
-    public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string
+    public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string
     {
         $param = $uniqueNameFactory->createParameterName();
         $queryBuilder->setParameter($param, $args['value']);
diff --git a/tests/Blog/Filtering/SearchOperatorType.php b/tests/Blog/Filtering/SearchOperatorType.php
@@ -26,7 +26,7 @@ protected function getConfiguration(Types $types, LeafType $leafType): array
         ];
     }
 
-    public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string
+    public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string
     {
         $words = preg_split('/[[:space:]]+/', $args['term'], -1, PREG_SPLIT_NO_EMPTY);
         if (!$words) {
diff --git a/tests/Blog/Model/AbstractModel.php b/tests/Blog/Model/AbstractModel.php
@@ -14,7 +14,7 @@
  * @ORM\MappedSuperclass
  * @API\Sorting({"GraphQLTests\Doctrine\Blog\Sorting\PseudoRandom"})
  * @API\Filters({
- *     @API\Filter(field="id", operator="GraphQLTests\Doctrine\Blog\Filtering\ModuloOperatorType", type="int")
+ *     @API\Filter(field="id", operator="GraphQLTests\Doctrine\Blog\Filtering\ModuloOperatorType", type="int"),
  * })
  */
 abstract class AbstractModel
diff --git a/tests/Blog/Model/Post.php b/tests/Blog/Model/Post.php
@@ -14,7 +14,7 @@
  * @ORM\Entity
  * @API\Sorting({"GraphQLTests\Doctrine\Blog\Sorting\UserName"})
  * @API\Filters({
- *     @API\Filter(field="custom", operator="GraphQLTests\Doctrine\Blog\Filtering\SearchOperatorType", type="string")
+ *     @API\Filter(field="custom", operator="GraphQLTests\Doctrine\Blog\Filtering\SearchOperatorType", type="string"),
  * })
  */
 final class Post extends AbstractModel
diff --git a/tests/Blog/Model/Special/InvalidFilter.php b/tests/Blog/Model/Special/InvalidFilter.php
@@ -11,7 +11,7 @@
 /**
  * @ORM\Entity
  * @API\Filters({
- *     @API\Filter(field="custom", operator="invalid_class_name", type="string")
+ *     @API\Filter(field="custom", operator="invalid_class_name", type="string"),
  * })
  */
 final class InvalidFilter extends AbstractModel
diff --git a/tests/OperatorsTest.php b/tests/OperatorsTest.php
@@ -31,7 +31,7 @@ final class OperatorsTest extends \PHPUnit\Framework\TestCase
      * @param string $className
      * @param array $args
      */
-    public function testOperator(string $expected, string $className, array $args): void
+    public function testOperator(?string $expected, string $className, ?array $args): void
     {
         /** @var AbstractOperator $operator */
         $operator = new $className($this->types, Type::string());
@@ -44,14 +44,20 @@ public function testOperator(string $expected, string $className, array $args):
         $actual = $operator->getDqlCondition($uniqueNameFactory, $metadata, $queryBuilder, $alias, $field, $args);
 
         self::assertEquals($expected, $actual, 'DQL condition should match');
-        self::assertSame(mb_substr_count($expected, ':'), $queryBuilder->getParameters()->count(), 'should declare the same number of parameters that are actually used');
+        if (is_string($expected)) {
+            self::assertSame(mb_substr_count($expected, ':'), $queryBuilder->getParameters()->count(), 'should declare the same number of parameters that are actually used');
+        }
     }
 
     public function providerOperator(): array
     {
         return [
             [
-                'alias.field BETWEEN :filter1 AND :filter2',
+                null,
+                BetweenOperatorType::class,
+                null,
+            ],
+            ['alias.field BETWEEN :filter1 AND :filter2',
                 BetweenOperatorType::class,
                 [
                     'from' => 123,
@@ -68,6 +74,11 @@ public function providerOperator(): array
                     'not' => true,
                 ],
             ],
+            [
+                null,
+                ContainOperatorType::class,
+                null,
+            ],
             [
                 ':filter1 MEMBER OF alias.field',
                 ContainOperatorType::class,
@@ -84,6 +95,11 @@ public function providerOperator(): array
                     'not' => true,
                 ],
             ],
+            [
+                null,
+                EmptyOperatorType::class,
+                null,
+            ],
             [
                 'alias.field IS EMPTY',
                 EmptyOperatorType::class,
@@ -98,6 +114,11 @@ public function providerOperator(): array
                     'not' => true,
                 ],
             ],
+            [
+                null,
+                EqualOperatorType::class,
+                null,
+            ],
             [
                 'alias.field = :filter1',
                 EqualOperatorType::class,
@@ -114,6 +135,11 @@ public function providerOperator(): array
                     'not' => true,
                 ],
             ],
+            [
+                null,
+                GreaterOperatorType::class,
+                null,
+            ],
             [
                 'alias.field > :filter1',
                 GreaterOperatorType::class,
@@ -130,6 +156,11 @@ public function providerOperator(): array
                     'not' => true,
                 ],
             ],
+            [
+                null,
+                GreaterOrEqualOperatorType::class,
+                null,
+            ],
             [
                 'alias.field >= :filter1',
                 GreaterOrEqualOperatorType::class,
@@ -146,6 +177,11 @@ public function providerOperator(): array
                     'not' => true,
                 ],
             ],
+            [
+                null,
+                InOperatorType::class,
+                null,
+            ],
             [
                 'alias.field IN (:filter1)',
                 InOperatorType::class,
@@ -162,6 +198,11 @@ public function providerOperator(): array
                     'not' => true,
                 ],
             ],
+            [
+                null,
+                LessOperatorType::class,
+                null,
+            ],
             [
                 'alias.field < :filter1',
                 LessOperatorType::class,
@@ -178,6 +219,11 @@ public function providerOperator(): array
                     'not' => true,
                 ],
             ],
+            [
+                null,
+                LessOrEqualOperatorType::class,
+                null,
+            ],
             [
                 'alias.field <= :filter1',
                 LessOrEqualOperatorType::class,
@@ -194,6 +240,11 @@ public function providerOperator(): array
                     'not' => true,
                 ],
             ],
+            [
+                null,
+                LikeOperatorType::class,
+                null,
+            ],
             [
                 'alias.field LIKE :filter1',
                 LikeOperatorType::class,
@@ -210,6 +261,11 @@ public function providerOperator(): array
                     'not' => true,
                 ],
             ],
+            [
+                null,
+                NullOperatorType::class,
+                null,
+            ],
             [
                 'alias.field IS NULL',
                 NullOperatorType::class,
diff --git a/tests/data/query-builder/filter-with-null-value.php b/tests/data/query-builder/filter-with-null-value.php
@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+// all posts whose title contains "foo"
+return [
+    'SELECT post1 FROM GraphQLTests\Doctrine\Blog\Model\Post post1',
+    \GraphQLTests\Doctrine\Blog\Model\Post::class,
+    [
+        'conditions' => [
+            [
+                'conditionLogic' => 'AND',
+                'fieldsLogic' => 'AND',
+                'fields' => [
+                    'title' => null,
+                ],
+            ],
+        ],
+    ],
+    [],
+];

Original file line number	Diff line number	Diff line change
`@@ -65,9 +65,9 @@ abstract protected function getConfiguration(Types $types, LeafType $leafType):`
`65`	`65`	`* @param QueryBuilder $queryBuilder`
`66`	`66`	`* @param string $alias the alias for the entity on which to apply the filter`
`67`	`67`	`* @param string $field the field for the entity on which to apply the filter`
`68`		`- * @param array $args all arguments specific to this operator as declared in its configuration`
	`68`	`+ * @param null\|array $args all arguments specific to this operator as declared in its configuration`
`69`	`69`	`*`
`70`	`70`	`* @return null\|string`
`71`	`71`	`*/`
`72`		`- abstract public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string;`
	`72`	`+ abstract public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string;`
`73`	`73`	`}`
Original file line number	Diff line number	Diff line change
`@@ -35,8 +35,12 @@ final protected function getConfiguration(Types $types, LeafType $leafType): arr`
`35`	`35`	`];`
`36`	`36`	`}`
`37`	`37`
`38`		`- final public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string`
	`38`	`+ final public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string`
`39`	`39`	`{`
	`40`	`+ if ($args === null) {`
	`41`	`+ return null;`
	`42`	`+ }`
	`43`	`+`
`40`	`44`	`$param = $uniqueNameFactory->createParameterName();`
`41`	`45`	`$queryBuilder->setParameter($param, $args['value']);`
`42`	`46`
Original file line number	Diff line number	Diff line change
`@@ -34,8 +34,12 @@ protected function getConfiguration(Types $types, LeafType $leafType): array`
`34`	`34`	`];`
`35`	`35`	`}`
`36`	`36`
`37`		`- public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string`
	`37`	`+ public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string`
`38`	`38`	`{`
	`39`	`+ if ($args === null) {`
	`40`	`+ return null;`
	`41`	`+ }`
	`42`	`+`
`39`	`43`	`$from = $uniqueNameFactory->createParameterName();`
`40`	`44`	`$to = $uniqueNameFactory->createParameterName();`
`41`	`45`	`$queryBuilder->setParameter($from, $args['from']);`
Original file line number	Diff line number	Diff line change
`@@ -30,8 +30,12 @@ protected function getConfiguration(Types $types, LeafType $leafType): array`
`30`	`30`	`];`
`31`	`31`	`}`
`32`	`32`
`33`		`- public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string`
	`33`	`+ public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string`
`34`	`34`	`{`
	`35`	`+ if ($args === null) {`
	`36`	`+ return null;`
	`37`	`+ }`
	`38`	`+`
`35`	`39`	`$values = $uniqueNameFactory->createParameterName();`
`36`	`40`	`$queryBuilder->setParameter($values, $args['values']);`
`37`	`41`	`$not = $args['not'] ? 'NOT ' : '';`
Original file line number	Diff line number	Diff line change
`@@ -26,8 +26,12 @@ protected function getConfiguration(Types $types, LeafType $leafType): array`
`26`	`26`	`];`
`27`	`27`	`}`
`28`	`28`
`29`		`- public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string`
	`29`	`+ public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string`
`30`	`30`	`{`
	`31`	`+ if ($args === null) {`
	`32`	`+ return null;`
	`33`	`+ }`
	`34`	`+`
`31`	`35`	`$not = $args['not'] ? 'NOT ' : '';`
`32`	`36`
`33`	`37`	`return $alias . '.' . $field . ' IS ' . $not . 'EMPTY';`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ protected function getConfiguration(Types $types, LeafType $leafType): array`
`26`	`26`	`];`
`27`	`27`	`}`
`28`	`28`
`29`		`- public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, array $args): ?string`
	`29`	`+ public function getDqlCondition(UniqueNameFactory $uniqueNameFactory, ClassMetadata $metadata, QueryBuilder $queryBuilder, string $alias, string $field, ?array $args): ?string`
`30`	`30`	`{`
`31`	`31`	`$param = $uniqueNameFactory->createParameterName();`
`32`	`32`	`$queryBuilder->setParameter($param, $args['value']);`