fix for coerceVariableValues (#2083)

dugenkui03 · dugenkui · web-flow · commit 39c567ed5b4a · 2020-11-10T20:36:00.000+11:00
Co-authored-by: dugenkui &lt;dugenkui@dugenkuideMBP.lan&gt;
diff --git a/src/main/java/graphql/execution/ValuesResolver.java b/src/main/java/graphql/execution/ValuesResolver.java
@@ -67,23 +67,14 @@ public Map<String, Object> coerceVariableValues(GraphQLSchema schema, List<Varia
                 }
             } else {
                 Object value = variableValues.get(variableName);
-                Object coercedValue = getVariableValue(fieldVisibility, variableDefinition, variableType, value);
+                Object coercedValue = coerceValue(fieldVisibility, variableDefinition, variableDefinition.getName(), variableType, value);
                 coercedValues.put(variableName, coercedValue);
             }
         }
 
         return coercedValues;
     }
 
-    private Object getVariableValue(GraphqlFieldVisibility fieldVisibility, VariableDefinition variableDefinition, GraphQLType variableType, Object value) {
-
-        if (value == null && variableDefinition.getDefaultValue() != null) {
-            return coerceValueAst(fieldVisibility, variableType, variableDefinition.getDefaultValue(), null);
-        }
-
-        return coerceValue(fieldVisibility, variableDefinition, variableDefinition.getName(), variableType, value);
-    }
-
     public Map<String, Object> getArgumentValues(List<GraphQLArgument> argumentTypes, List<Argument> arguments, Map<String, Object> variables) {
         GraphQLCodeRegistry codeRegistry = GraphQLCodeRegistry.newCodeRegistry().fieldVisibility(DEFAULT_FIELD_VISIBILITY).build();
         return getArgumentValuesImpl(codeRegistry, argumentTypes, arguments, variables);
diff --git a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy
@@ -451,6 +451,44 @@ class ValuesResolverTest extends Specification {
         thrown(GraphQLException)
     }
 
+    def "coerceVariableValues: use null when variable defined in variableValuesMap is null"() {
+        given:
+        def schema = TestUtil.schemaWithInputType(nonNull(GraphQLString))
+
+        def defaultValueForFoo = new StringValue("defaultValueForFoo")
+        VariableDefinition fooVarDef = new VariableDefinition("foo", new TypeName("String"), defaultValueForFoo)
+
+        def defaultValueForBar = new StringValue("defaultValueForBar")
+        VariableDefinition barVarDef = new VariableDefinition("bar", new TypeName("String"), defaultValueForBar)
+
+        def variableValuesMap = ["foo": null, "bar": "barValue"]
+
+        when:
+        def resolvedVars = resolver.coerceVariableValues(schema, [fooVarDef, barVarDef], variableValuesMap)
+
+        then:
+        resolvedVars['foo'] == null
+        resolvedVars['bar'] == "barValue"
+    }
+
+    def "coerceVariableValues: if variableType is a Non‐Nullable type and value is null, throw a query error"() {
+        given:
+        def schema = TestUtil.schemaWithInputType(nonNull(GraphQLString))
+
+        def defaultValueForFoo = new StringValue("defaultValueForFoo")
+        VariableDefinition fooVarDef = new VariableDefinition("foo", new NonNullType(new TypeName("String")), defaultValueForFoo)
+
+
+        def variableValuesMap = ["foo": null]
+
+        when:
+        resolver.coerceVariableValues(schema, [fooVarDef], variableValuesMap)
+
+        then:
+        def error = thrown(NonNullableValueCoercedAsNullException)
+        error.message == "Field 'foo' of variable 'foo' has coerced Null value for NonNull type 'String!'"
+    }
+
     // Note: use NullValue defined in Field when it exists,
     // and ignore defaultValue defined in type system
     def "getArgumentValues: use null value when argumentValue defined in Field is null"() {

Original file line number	Diff line number	Diff line change
`@@ -67,23 +67,14 @@ public Map<String, Object> coerceVariableValues(GraphQLSchema schema, List<Varia`
`67`	`67`	`}`
`68`	`68`	`} else {`
`69`	`69`	`Object value = variableValues.get(variableName);`
`70`		`- Object coercedValue = getVariableValue(fieldVisibility, variableDefinition, variableType, value);`
	`70`	`+ Object coercedValue = coerceValue(fieldVisibility, variableDefinition, variableDefinition.getName(), variableType, value);`
`71`	`71`	`coercedValues.put(variableName, coercedValue);`
`72`	`72`	`}`
`73`	`73`	`}`
`74`	`74`
`75`	`75`	`return coercedValues;`
`76`	`76`	`}`
`77`	`77`
`78`		`- private Object getVariableValue(GraphqlFieldVisibility fieldVisibility, VariableDefinition variableDefinition, GraphQLType variableType, Object value) {`
`79`		`-`
`80`		`- if (value == null && variableDefinition.getDefaultValue() != null) {`
`81`		`- return coerceValueAst(fieldVisibility, variableType, variableDefinition.getDefaultValue(), null);`
`82`		`- }`
`83`		`-`
`84`		`- return coerceValue(fieldVisibility, variableDefinition, variableDefinition.getName(), variableType, value);`
`85`		`- }`
`86`		`-`
`87`	`78`	`public Map<String, Object> getArgumentValues(List<GraphQLArgument> argumentTypes, List<Argument> arguments, Map<String, Object> variables) {`
`88`	`79`	`GraphQLCodeRegistry codeRegistry = GraphQLCodeRegistry.newCodeRegistry().fieldVisibility(DEFAULT_FIELD_VISIBILITY).build();`
`89`	`80`	`return getArgumentValuesImpl(codeRegistry, argumentTypes, arguments, variables);`