graphql-java · dondonz · Aug 29, 2023 · Aug 22, 2023
diff --git a/src/main/java/graphql/validation/TraversalContext.java b/src/main/java/graphql/validation/TraversalContext.java
@@ -33,6 +33,7 @@
 import graphql.schema.GraphQLType;
 import graphql.schema.GraphQLUnionType;
 import graphql.schema.GraphQLUnmodifiedType;
+import graphql.schema.InputValueWithState;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -48,6 +49,7 @@ public class TraversalContext implements DocumentVisitor {
     private final List<GraphQLOutputType> outputTypeStack = new ArrayList<>();
     private final List<GraphQLCompositeType> parentTypeStack = new ArrayList<>();
     private final List<GraphQLInputType> inputTypeStack = new ArrayList<>();
+    private final List<InputValueWithState> defaultValueStack = new ArrayList<>();
     private final List<GraphQLFieldDefinition> fieldDefStack = new ArrayList<>();
     private final List<String> nameStack = new ArrayList<>();
     private GraphQLDirective directive;
@@ -156,6 +158,7 @@ private void enterImpl(Argument argument) {
         }
 
         addInputType(argumentType != null ? argumentType.getType() : null);
+        addDefaultValue(argumentType != null ? argumentType.getArgumentDefaultValue() : null);
         this.argument = argumentType;
     }
 
@@ -166,23 +169,30 @@ private void enterImpl(ArrayValue arrayValue) {
             inputType = (GraphQLInputType) unwrapOne(nullableType);
         }
         addInputType(inputType);
+        // List positions never have a default value. See graphql-js impl for inspiration
+        addDefaultValue(null);
     }
 
     private void enterImpl(ObjectField objectField) {
         GraphQLUnmodifiedType objectType = unwrapAll(getInputType());
         GraphQLInputType inputType = null;
+        GraphQLInputObjectField inputField = null;
         if (objectType instanceof GraphQLInputObjectType) {
             GraphQLInputObjectType inputObjectType = (GraphQLInputObjectType) objectType;
-            GraphQLInputObjectField inputField = schema.getFieldVisibility().getFieldDefinition(inputObjectType, objectField.getName());
-            if (inputField != null)
+            inputField = schema.getFieldVisibility().getFieldDefinition(inputObjectType, objectField.getName());
+            if (inputField != null) {
                 inputType = inputField.getType();
+            }
         }
         addInputType(inputType);
+        addDefaultValue(inputField != null ? inputField.getInputFieldDefaultValue() : null);
     }
 
     private GraphQLArgument find(List<GraphQLArgument> arguments, String name) {
         for (GraphQLArgument argument : arguments) {
-            if (argument.getName().equals(name)) return argument;
+            if (argument.getName().equals(name)) {
+                return argument;
+            }
         }
         return null;
     }
@@ -191,29 +201,32 @@ private GraphQLArgument find(List<GraphQLArgument> arguments, String name) {
     @Override
     public void leave(Node node, List<Node> ancestors) {
         if (node instanceof OperationDefinition) {
-            outputTypeStack.remove(outputTypeStack.size() - 1);
+            pop(outputTypeStack);
         } else if (node instanceof SelectionSet) {
-            parentTypeStack.remove(parentTypeStack.size() - 1);
+            pop(parentTypeStack);
         } else if (node instanceof Field) {
             leaveName(((Field) node).getName());
-            fieldDefStack.remove(fieldDefStack.size() - 1);
-            outputTypeStack.remove(outputTypeStack.size() - 1);
+            pop(fieldDefStack);
+            pop(outputTypeStack);
         } else if (node instanceof Directive) {
             directive = null;
         } else if (node instanceof InlineFragment) {
-            outputTypeStack.remove(outputTypeStack.size() - 1);
+            pop(outputTypeStack);
         } else if (node instanceof FragmentDefinition) {
             leaveName(((FragmentDefinition) node).getName());
-            outputTypeStack.remove(outputTypeStack.size() - 1);
+            pop(outputTypeStack);
         } else if (node instanceof VariableDefinition) {
             inputTypeStack.remove(inputTypeStack.size() - 1);
         } else if (node instanceof Argument) {
             argument = null;
-            inputTypeStack.remove(inputTypeStack.size() - 1);
+            pop(inputTypeStack);
+            pop(defaultValueStack);
         } else if (node instanceof ArrayValue) {
-            inputTypeStack.remove(inputTypeStack.size() - 1);
+            pop(inputTypeStack);
+            pop(defaultValueStack);
         } else if (node instanceof ObjectField) {
-            inputTypeStack.remove(inputTypeStack.size() - 1);
+            pop(inputTypeStack);
+            pop(defaultValueStack);
         }
     }
 
@@ -250,10 +263,16 @@ private void addOutputType(GraphQLOutputType type) {
     }
 
     private <T> T lastElement(List<T> list) {
-        if (list.size() == 0) return null;
+        if (list.isEmpty()) {
+            return null;
+        }
         return list.get(list.size() - 1);
     }
 
+    private <T>  T pop(List<T> list) {
+        return list.remove(list.size() - 1);
+    }
+
     /**
      * @return can be null if the parent is not a CompositeType
      */
@@ -268,11 +287,18 @@ private void addParentType(GraphQLCompositeType compositeType) {
     public GraphQLInputType getInputType() {
         return lastElement(inputTypeStack);
     }
+    public InputValueWithState getDefaultValue() {
+        return lastElement(defaultValueStack);
+    }
 
     private void addInputType(GraphQLInputType graphQLInputType) {
         inputTypeStack.add(graphQLInputType);
     }
 
+    private void addDefaultValue(InputValueWithState defaultValue) {
+        defaultValueStack.add(defaultValue);
+    }
+
     public GraphQLFieldDefinition getFieldDef() {
         return lastElement(fieldDefStack);
     }

diff --git a/src/main/java/graphql/validation/ValidationContext.java b/src/main/java/graphql/validation/ValidationContext.java
@@ -13,6 +13,7 @@
 import graphql.schema.GraphQLInputType;
 import graphql.schema.GraphQLOutputType;
 import graphql.schema.GraphQLSchema;
+import graphql.schema.InputValueWithState;
 
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -37,8 +38,10 @@ public ValidationContext(GraphQLSchema schema, Document document, I18n i18n) {
     }
 
     private void buildFragmentMap() {
-        for (Definition definition : document.getDefinitions()) {
-            if (!(definition instanceof FragmentDefinition)) continue;
+        for (Definition<?> definition : document.getDefinitions()) {
+            if (!(definition instanceof FragmentDefinition)) {
+                continue;
+            }
             FragmentDefinition fragmentDefinition = (FragmentDefinition) definition;
             fragmentDefinitionMap.put(fragmentDefinition.getName(), fragmentDefinition);
         }
@@ -68,6 +71,10 @@ public GraphQLInputType getInputType() {
         return traversalContext.getInputType();
     }
 
+    public InputValueWithState getDefaultValue() {
+        return traversalContext.getDefaultValue();
+    }
+
     public GraphQLFieldDefinition getFieldDef() {
         return traversalContext.getFieldDef();
     }

diff --git a/src/main/java/graphql/validation/rules/VariableTypesMatch.java b/src/main/java/graphql/validation/rules/VariableTypesMatch.java
@@ -59,24 +59,25 @@ public void checkVariable(VariableReference variableReference) {
         if (variableType == null) {
             return;
         }
-        GraphQLInputType expectedType = getValidationContext().getInputType();
-        Optional<InputValueWithState> schemaDefault = Optional.ofNullable(getValidationContext().getArgument()).map(v -> v.getArgumentDefaultValue());
-        Value schemaDefaultValue = null;
-        if (schemaDefault.isPresent() && schemaDefault.get().isLiteral()) {
-            schemaDefaultValue = (Value) schemaDefault.get().getValue();
-        } else if (schemaDefault.isPresent() && schemaDefault.get().isSet()) {
-            schemaDefaultValue = ValuesResolver.valueToLiteral(schemaDefault.get(), expectedType);
-        }
-        if (expectedType == null) {
-            // we must have a unknown variable say to not have a known type
+        GraphQLInputType locationType = getValidationContext().getInputType();
+        Optional<InputValueWithState> locationDefault = Optional.ofNullable(getValidationContext().getDefaultValue());
+        if (locationType == null) {
+            // we must have an unknown variable say to not have a known type
             return;
         }
-        if (!variablesTypesMatcher.doesVariableTypesMatch(variableType, variableDefinition.getDefaultValue(), expectedType) &&
-            !variablesTypesMatcher.doesVariableTypesMatch(variableType, schemaDefaultValue, expectedType)) {
+        Value<?> locationDefaultValue = null;
+        if (locationDefault.isPresent() && locationDefault.get().isLiteral()) {
+            locationDefaultValue = (Value<?>) locationDefault.get().getValue();
+        } else if (locationDefault.isPresent() && locationDefault.get().isSet()) {
+            locationDefaultValue = ValuesResolver.valueToLiteral(locationDefault.get(), locationType);
+        }
+        boolean variableDefMatches = variablesTypesMatcher.doesVariableTypesMatch(variableType, variableDefinition.getDefaultValue(), locationType, locationDefaultValue);
+        if (!variableDefMatches) {
             GraphQLType effectiveType = variablesTypesMatcher.effectiveType(variableType, variableDefinition.getDefaultValue());
             String message = i18n(VariableTypeMismatch, "VariableTypesMatchRule.unexpectedType",
+                    variableDefinition.getName(),
                     GraphQLTypeUtil.simplePrint(effectiveType),
-                    GraphQLTypeUtil.simplePrint(expectedType));
+                    GraphQLTypeUtil.simplePrint(locationType));
             addError(VariableTypeMismatch, variableReference.getSourceLocation(), message);
         }
     }

diff --git a/src/main/java/graphql/validation/rules/VariablesTypesMatcher.java b/src/main/java/graphql/validation/rules/VariablesTypesMatcher.java
@@ -9,16 +9,38 @@
 import static graphql.schema.GraphQLNonNull.nonNull;
 import static graphql.schema.GraphQLTypeUtil.isList;
 import static graphql.schema.GraphQLTypeUtil.isNonNull;
+import static graphql.schema.GraphQLTypeUtil.unwrapNonNull;
 import static graphql.schema.GraphQLTypeUtil.unwrapOne;
 
 @Internal
 public class VariablesTypesMatcher {
 
-    public boolean doesVariableTypesMatch(GraphQLType variableType, Value variableDefaultValue, GraphQLType expectedType) {
-        return checkType(effectiveType(variableType, variableDefaultValue), expectedType);
+    /**
+     * This method and variable naming was inspired from the reference graphql-js implementation
+     *
+     * @param varType              the variable type
+     * @param varDefaultValue      the default value for the variable
+     * @param locationType         the location type where the variable was encountered
+     * @param locationDefaultValue the default value for that location
+     *
+     * @return true if the variable matches ok
+     */
+    public boolean doesVariableTypesMatch(GraphQLType varType, Value<?> varDefaultValue, GraphQLType locationType, Value<?> locationDefaultValue) {
+        if (isNonNull(locationType) && !isNonNull(varType)) {
+            boolean hasNonNullVariableDefaultValue =
+                    varDefaultValue != null && !(varDefaultValue instanceof NullValue);
+            boolean hasLocationDefaultValue = locationDefaultValue != null;
+            if (!hasNonNullVariableDefaultValue && !hasLocationDefaultValue) {
+                return false;
+            }
+            GraphQLType nullableLocationType = unwrapNonNull(locationType);
+            return checkType(varType, nullableLocationType);
+        }
+        return checkType(varType, locationType);
     }
 
-    public GraphQLType effectiveType(GraphQLType variableType, Value defaultValue) {
+
+    public GraphQLType effectiveType(GraphQLType variableType, Value<?> defaultValue) {
         if (defaultValue == null || defaultValue instanceof NullValue) {
             return variableType;
         }

diff --git a/src/main/resources/i18n/Validation.properties b/src/main/resources/i18n/Validation.properties
@@ -76,7 +76,7 @@ VariableDefaultValuesOfCorrectType.badDefault=Validation error ({0}) : Bad defau
 #
 VariablesAreInputTypes.wrongType=Validation error ({0}) : Input variable ''{1}'' type ''{2}'' is not an input type
 #
-VariableTypesMatchRule.unexpectedType=Validation error ({0}) : Variable type ''{1}'' does not match expected type ''{2}''
+VariableTypesMatchRule.unexpectedType=Validation error ({0}) : Variable ''{1}'' of type ''{2}'' used in position expecting type ''{3}''
 #
 # These are used but IDEA cant find them easily as being called
 #

diff --git a/src/test/groovy/graphql/execution/ValuesResolverE2ETest.groovy b/src/test/groovy/graphql/execution/ValuesResolverE2ETest.groovy
@@ -0,0 +1,136 @@
+package graphql.execution
+
+import graphql.ExecutionInput
+import graphql.ExecutionResult
+import graphql.GraphQL
+import graphql.Scalars
+import graphql.TestUtil
+import graphql.schema.DataFetcher
+import graphql.schema.DataFetchingEnvironment
+import graphql.schema.FieldCoordinates
+import graphql.schema.GraphQLCodeRegistry
+import graphql.schema.GraphQLInputObjectType
+import graphql.schema.GraphQLList
+import graphql.schema.GraphQLObjectType
+import graphql.schema.GraphQLSchema
+import spock.lang.Specification
+
+class ValuesResolverE2ETest extends Specification {
+
+    def "issue 3276 - reported bug on validation problems as SDL"() {
+        def sdl = '''
+            type Query {
+              items(pagination: Pagination = {limit: 10, offset: 0}): [String]
+            }
+
+            input Pagination {
+              limit: Int
+              offset: Int
+            }
+        '''
+        DataFetcher df = { DataFetchingEnvironment env ->
+            def pagination = env.getArgument("pagination") as Map<String, Integer>
+            def strings = pagination.entrySet().collect { entry -> entry.key + "=" + entry.value }
+            return strings
+        }
+        def schema = TestUtil.schema(sdl, [Query: [items: df]])
+        def graphQL = GraphQL.newGraphQL(schema).build()
+
+        when:
+        def ei = ExecutionInput.newExecutionInput('''
+            query Items($limit: Int, $offset: Int) {
+                 items(pagination: {limit: $limit, offset: $offset})
+             }
+            ''').variables([limit: 5, offset: 0]).build()
+        def er = graphQL.execute(ei)
+        then:
+        er.errors.isEmpty()
+        er.data == [items : ["limit=5", "offset=0"]]
+    }
+
+    def "issue 3276 - reported bug on validation problems as reported code"() {
+        DataFetcher<?> dataFetcher = { env ->
+            def pagination = env.getArgument("pagination") as Map<String, Integer>
+            def strings = pagination.entrySet().collect { entry -> entry.key + "=" + entry.value }
+            return strings
+        }
+        GraphQLSchema schema = GraphQLSchema.newSchema()
+                .query(GraphQLObjectType.newObject()
+                        .name("Query")
+                        .field(items -> items
+                                .name("items")
+                                .type(GraphQLList.list(Scalars.GraphQLString))
+                                .argument(pagination -> pagination
+                                        .name("pagination")
+                                //skipped adding the default limit/offset values as it doesn't change anything
+                                        .defaultValueProgrammatic(new HashMap<>())
+                                        .type(GraphQLInputObjectType.newInputObject()
+                                                .name("Pagination")
+                                                .field(limit -> limit
+                                                        .name("limit")
+                                                        .type(Scalars.GraphQLInt))
+                                                .field(offset -> offset
+                                                        .name("offset")
+                                                        .type(Scalars.GraphQLInt))
+                                                .build())))
+                        .build())
+                .codeRegistry(GraphQLCodeRegistry.newCodeRegistry()
+                        .dataFetcher(FieldCoordinates.coordinates("Query", "items"), dataFetcher)
+                        .build())
+                .build()
+
+        GraphQL gql = GraphQL.newGraphQL(schema).build()
+
+        Map<String, Object> vars = new HashMap<>()
+        vars.put("limit", 5)
+        vars.put("offset", 0)
+
+        ExecutionInput ei = ExecutionInput.newExecutionInput()
+                .query("query Items( \$limit: Int, \$offset: Int) {\n" +
+                        "  items(\n" +
+                        "    pagination: {limit: \$limit, offset: \$offset} \n" +
+                        "  )\n" +
+                        "}")
+                .variables(vars)
+                .build()
+
+        when:
+        ExecutionResult result = gql.execute( ei)
+        then:
+        result.errors.isEmpty()
+        result.data == [items : ["limit=5", "offset=0"]]
+    }
+
+    def "issue 3276 - should end up in validation errors because location defaults are not present"() {
+        def sdl = '''
+            type Query {
+                items(pagination: Pagination = {limit: 1, offset: 1}): [String]
+            }
+            input Pagination {
+                limit: Int! #non-null this time, no default value
+                offset: Int! #non-null this time, no default value
+            }
+        '''
+        DataFetcher df = { DataFetchingEnvironment env ->
+            def pagination = env.getArgument("pagination") as Map<String, Integer>
+            def strings = pagination.entrySet().collect { entry -> entry.key + "=" + entry.value }
+            return strings
+        }
+        def schema = TestUtil.schema(sdl, [Query: [items: df]])
+        def graphQL = GraphQL.newGraphQL(schema).build()
+
+        when:
+        def ei = ExecutionInput.newExecutionInput('''
+            query Items( $limit: Int, $offset: Int) {
+                 items(
+                    pagination: {limit: $limit, offset: $offset} 
+                )
+            }
+            ''').variables([limit: 5, offset: null]).build()
+        def er = graphQL.execute(ei)
+        then:
+        er.errors.size() == 2
+        er.errors[0].message == "Validation error (VariableTypeMismatch@[items]) : Variable 'limit' of type 'Int' used in position expecting type 'Int!'"
+        er.errors[1].message == "Validation error (VariableTypeMismatch@[items]) : Variable 'offset' of type 'Int' used in position expecting type 'Int!'"
+    }
+}