Support filtering of null values also via variables (#178)

Andy2003 · web-flow · commit 2021d1afa973 · 2021-03-25T16:16:07.000+01:00
resolves #47
diff --git a/core/src/main/kotlin/org/neo4j/graphql/handler/QueryHandler.kt b/core/src/main/kotlin/org/neo4j/graphql/handler/QueryHandler.kt
@@ -92,7 +92,7 @@ class QueryHandler private constructor(
 
         val ongoingReading = if ((env.getContext() as? QueryContext)?.optimizedQuery?.contains(QueryContext.OptimizationStrategy.FILTER_AS_MATCH) == true) {
 
-            OptimizedFilterHandler(type).generateFilterQuery(variable, fieldDefinition, field, match, propertyContainer)
+            OptimizedFilterHandler(type).generateFilterQuery(variable, fieldDefinition, field, match, propertyContainer, env.variables)
 
         } else {
 
diff --git a/core/src/main/kotlin/org/neo4j/graphql/handler/filter/OptimizedFilterHandler.kt b/core/src/main/kotlin/org/neo4j/graphql/handler/filter/OptimizedFilterHandler.kt
@@ -34,7 +34,7 @@ typealias ConditionBuilder = (ExposesWith) -> OrderableOngoingReadingAndWithWith
  */
 class OptimizedFilterHandler(val type: GraphQLFieldsContainer) : ProjectionBase() {
 
-    fun generateFilterQuery(variable: String, fieldDefinition: GraphQLFieldDefinition, field: Field, readingWithoutWhere: OngoingReadingWithoutWhere, rootNode: PropertyContainer): OngoingReading {
+    fun generateFilterQuery(variable: String, fieldDefinition: GraphQLFieldDefinition, field: Field, readingWithoutWhere: OngoingReadingWithoutWhere, rootNode: PropertyContainer, variables: Map<String, Any>): OngoingReading {
         if (type.isRelationType()) {
             throw OptimizedQueryException("Optimization for relationship entity type is not implemented. Please provide a test case to help adding further cases.")
         }
@@ -43,16 +43,16 @@ class OptimizedFilterHandler(val type: GraphQLFieldsContainer) : ProjectionBase(
 
         val filteredArguments = field.arguments.filterNot { setOf(FIRST, OFFSET, ORDER_BY, FILTER).contains(it.name) }
         if (filteredArguments.isNotEmpty()) {
-            val parsedQuery = QueryParser.parseArguments(filteredArguments, fieldDefinition, type)
-            val condition = handleQuery(variable, "", rootNode, parsedQuery, type)
+            val parsedQuery = QueryParser.parseArguments(filteredArguments, fieldDefinition, type, variables)
+            val condition = handleQuery(variable, "", rootNode, parsedQuery, type, variables)
             ongoingReading = readingWithoutWhere.where(condition)
         }
         for (argument in field.arguments) {
             if (argument.name == FILTER) {
-                val parsedQuery = parseFilter(argument.value as ObjectValue, type)
+                val parsedQuery = parseFilter(argument.value as ObjectValue, type, variables)
                 ongoingReading = NestingLevelHandler(parsedQuery, false, rootNode, variable, ongoingReading
                         ?: readingWithoutWhere,
-                        type, argument.value, linkedSetOf(rootNode.requiredSymbolicName))
+                        type, argument.value, linkedSetOf(rootNode.requiredSymbolicName), variables)
                     .parseFilter()
             }
         }
@@ -77,7 +77,8 @@ class OptimizedFilterHandler(val type: GraphQLFieldsContainer) : ProjectionBase(
             private val matchQueryWithoutWhere: OngoingReading,
             private val type: GraphQLFieldsContainer,
             private val value: Value<*>?,
-            private val parentPassThroughWiths: Collection<Expression>
+            private val parentPassThroughWiths: Collection<Expression>,
+            private val variables: Map<String, Any>
     ) {
 
         private fun currentNode() = current as? Node
@@ -164,7 +165,7 @@ class OptimizedFilterHandler(val type: GraphQLFieldsContainer) : ProjectionBase(
                 levelPassThroughWiths: LinkedHashSet<Expression>
         ): OrderableOngoingReadingAndWithWithoutWhere {
             val objectField = relFilter.queryField
-            val nestedParsedQuery = parseFilter(objectField.value as ObjectValue, relFilter.fieldDefinition.type.getInnerFieldsContainer())
+            val nestedParsedQuery = parseFilter(objectField.value as ObjectValue, relFilter.fieldDefinition.type.getInnerFieldsContainer(), variables)
             val hasPredicates = nestedParsedQuery.fieldPredicates.isNotEmpty() || nestedParsedQuery.relationPredicates.isNotEmpty()
 
             var queryWithoutWhere = query
@@ -185,7 +186,7 @@ class OptimizedFilterHandler(val type: GraphQLFieldsContainer) : ProjectionBase(
             }
 
             val nestingLevelHandler = NestingLevelHandler(nestedParsedQuery, true, relVariable, relVariableName,
-                    readingWithoutWhere, relFilter.relationshipInfo.type, objectField.value, levelPassThroughWiths)
+                    readingWithoutWhere, relFilter.relationshipInfo.type, objectField.value, levelPassThroughWiths, variables)
 
             when (relFilter.op) {
                 RelationOperator.SOME -> queryWithoutWhere = nestingLevelHandler.parseFilter()
diff --git a/core/src/main/kotlin/org/neo4j/graphql/handler/projection/ProjectionBase.kt b/core/src/main/kotlin/org/neo4j/graphql/handler/projection/ProjectionBase.kt
@@ -72,8 +72,8 @@ open class ProjectionBase {
 
         val filteredArguments = field.arguments.filterNot { setOf(FIRST, OFFSET, ORDER_BY, FILTER).contains(it.name) }
 
-        val parsedQuery = parseArguments(filteredArguments, fieldDefinition, type)
-        val result = handleQuery(variable, "", propertyContainer, parsedQuery, type)
+        val parsedQuery = parseArguments(filteredArguments, fieldDefinition, type, variables)
+        val result = handleQuery(variable, "", propertyContainer, parsedQuery, type, variables)
 
         return field.arguments.find { FILTER == it.name }
             ?.let { arg ->
@@ -83,9 +83,9 @@ open class ProjectionBase {
                     else -> throw IllegalArgumentException("")
                 }
             }
-            ?.let { parseFilter(it as ObjectValue, type) }
+            ?.let { parseFilter(it as ObjectValue, type, variables) }
             ?.let {
-                val filterCondition = handleQuery(normalizeName(FILTER, variable), "", propertyContainer, it, type)
+                val filterCondition = handleQuery(normalizeName(FILTER, variable), "", propertyContainer, it, type, variables)
                 result.and(filterCondition)
             }
                 ?: result
@@ -96,7 +96,8 @@ open class ProjectionBase {
             variableSuffix: String,
             propertyContainer: PropertyContainer,
             parsedQuery: ParsedQuery,
-            type: GraphQLFieldsContainer
+            type: GraphQLFieldsContainer,
+            variables: Map<String, Any>
     ): Condition {
         var result = parsedQuery.getFieldConditions(propertyContainer, variablePrefix, variableSuffix)
 
@@ -122,8 +123,8 @@ open class ProjectionBase {
                 else -> null
             }?.let {
                 val targetNode = predicate.relNode.named(normalizeName(variablePrefix, predicate.relationshipInfo.typeName))
-                val parsedQuery2 = parseFilter(objectField.value as ObjectValue, type)
-                val condition = handleQuery(targetNode.requiredSymbolicName.value, "", targetNode, parsedQuery2, type)
+                val parsedQuery2 = parseFilter(objectField.value as ObjectValue, type, variables)
+                val condition = handleQuery(targetNode.requiredSymbolicName.value, "", targetNode, parsedQuery2, type, variables)
                 var where = it
                     .`in`(listBasedOn(predicate.relationshipInfo.createRelation(propertyContainer as Node, targetNode)).returning(condition))
                     .where(cond.asCondition())
@@ -135,19 +136,19 @@ open class ProjectionBase {
             }
         }
 
-        fun handleLogicalOperator(value: Value<*>, classifier: String): Condition {
+        fun handleLogicalOperator(value: Value<*>, classifier: String, variables: Map<String, Any>): Condition {
             val objectValue = value as? ObjectValue
                     ?: throw IllegalArgumentException("Only object values are supported for logical operations, but got ${value.javaClass.name}")
 
-            val parsedNestedQuery = parseFilter(objectValue, type)
-            return handleQuery(variablePrefix + classifier, variableSuffix, propertyContainer, parsedNestedQuery, type)
+            val parsedNestedQuery = parseFilter(objectValue, type, variables)
+            return handleQuery(variablePrefix + classifier, variableSuffix, propertyContainer, parsedNestedQuery, type, variables)
         }
 
         fun handleLogicalOperators(values: List<Value<*>>?, classifier: String): List<Condition> {
             return when {
                 values?.isNotEmpty() == true -> when {
-                    values.size > 1 -> values.mapIndexed { index, value -> handleLogicalOperator(value, "${classifier}${index + 1}") }
-                    else -> values.map { value -> handleLogicalOperator(value, "") }
+                    values.size > 1 -> values.mapIndexed { index, value -> handleLogicalOperator(value, "${classifier}${index + 1}", variables) }
+                    else -> values.map { value -> handleLogicalOperator(value, "", variables) }
                 }
                 else -> emptyList()
             }
diff --git a/core/src/main/kotlin/org/neo4j/graphql/parser/QueryParser.kt b/core/src/main/kotlin/org/neo4j/graphql/parser/QueryParser.kt
@@ -91,7 +91,7 @@ object QueryParser {
     /**
      * This parser takes an filter object an transform it to the internal [ParsedQuery]-representation
      */
-    fun parseFilter(objectValue: ObjectValue, type: GraphQLFieldsContainer): ParsedQuery {
+    fun parseFilter(objectValue: ObjectValue, type: GraphQLFieldsContainer, variables: Map<String, Any>): ParsedQuery {
         // Map of all queried fields
         // we remove all matching fields from this map, so we can ensure that only known fields got queried
         val queriedFields = objectValue.objectFields
@@ -108,13 +108,13 @@ object QueryParser {
                     ?: throw IllegalArgumentException("AND on type `${type.name}` is expected to be a list")
         }
 
-        return createParsedQuery(queriedFields, type, null, or, and)
+        return createParsedQuery(queriedFields, type, variables, null, or, and)
     }
 
     /**
      * This parser takes all non-filter arguments of a graphql-field an transform it to the internal [ParsedQuery]-representation
      */
-    fun parseArguments(arguments: List<Argument>, fieldDefinition: GraphQLFieldDefinition, type: GraphQLFieldsContainer): ParsedQuery {
+    fun parseArguments(arguments: List<Argument>, fieldDefinition: GraphQLFieldDefinition, type: GraphQLFieldsContainer, variables: Map<String, Any>): ParsedQuery {
         // TODO we should check if the argument is defined on the field definition and throw an error otherwise
         // Map of all queried fields
         // we remove all matching fields from this map, so we can ensure that only known fields got queried
@@ -130,13 +130,14 @@ object QueryParser {
                 queriedFields[argument.name] = index++ to ObjectField(argument.name, argument.defaultValue.asGraphQLValue())
             }
 
-        return createParsedQuery(queriedFields, type, fieldDefinition)
+        return createParsedQuery(queriedFields, type, variables, fieldDefinition)
     }
 
 
     private fun createParsedQuery(
             queriedFields: MutableMap<String, Pair<Int, ObjectField>>,
             type: GraphQLFieldsContainer,
+            variables: Map<String, Any>,
             fieldDefinition: GraphQLFieldDefinition? = null,
             or: List<Value<Value<*>>>? = null,
             and: List<Value<Value<*>>>? = null
@@ -160,7 +161,7 @@ object QueryParser {
                     .map { it to definedField.name + it.suffix }
                     .mapNotNull { (predicate, queryFieldName) ->
                         queriedFields[queryFieldName]?.let { (index, objectField) ->
-                            if (predicate.requireParam xor (objectField.value !is NullValue)) {
+                            if (predicate.requireParam xor (!objectField.value.isNullValue(variables))) {
                                 // if we got a value but the predicate requires none
                                 // or we got a no value but the predicate requires one
                                 // we skip this operator
@@ -187,5 +188,8 @@ object QueryParser {
     }
 }
 
+private fun Value<*>.isNullValue(variables: Map<String, Any>): Boolean =
+        this is NullValue || (this is VariableReference && variables[this.name] == null)
+
 
 
diff --git a/core/src/test/resources/filter-tests.adoc b/core/src/test/resources/filter-tests.adoc
diff --git a/core/src/test/resources/issues/gh-47.adoc b/core/src/test/resources/issues/gh-47.adoc
