Merge pull request #3309 from graphql-java/20.x-backport-of-3278-defa…

…ult-value 20.x backport of #3278 default value bugfix
graphql-java · Aug 29, 2023 · 55d7fc5 · 55d7fc5
2 parents 3c3d875 + e4862a6
commit 55d7fc5
Show file tree

Hide file tree

Showing 9 changed files with 337 additions and 42 deletions.
diff --git a/src/main/java/graphql/validation/TraversalContext.java b/src/main/java/graphql/validation/TraversalContext.java
@@ -32,6 +32,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;
@@ -47,6 +48,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;
@@ -155,6 +157,7 @@ private void enterImpl(Argument argument) {
         }
 
         addInputType(argumentType != null ? argumentType.getType() : null);
+        addDefaultValue(argumentType != null ? argumentType.getArgumentDefaultValue() : null);
         this.argument = argumentType;
     }
 
@@ -165,23 +168,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;
     }
@@ -190,29 +200,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);
         }
     }
 
@@ -249,10 +262,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
      */
@@ -267,11 +286,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
@@ -14,6 +14,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;
@@ -41,8 +42,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);
         }
@@ -72,6 +75,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, getValidationContext().getGraphQLContext(), getValidationContext().getI18n().getLocale());
-        }
-        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, getValidationContext().getGraphQLContext(), getValidationContext().getI18n().getLocale());
+        }
+        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}''
 #
 UniqueObjectFieldName.duplicateFieldName=Validation Error ({0}) : There can be only one field named ''{1}''
 #

diff --git a/src/main/resources/i18n/Validation_de.properties b/src/main/resources/i18n/Validation_de.properties
@@ -79,7 +79,7 @@ VariableDefaultValuesOfCorrectType.badDefault=Validierungsfehler ({0}) : Ung\u00
 #
 VariablesAreInputTypes.wrongType=Validierungsfehler ({0}) : Eingabevariable ''{1}'' Typ ''{2}'' ist kein Eingabetyp
 #
-VariableTypesMatchRule.unexpectedType=Validierungsfehler ({0}) : Der Variablentyp ''{1}'' stimmt nicht mit dem erwarteten Typ ''{2}'' \u00fcberein
+VariableTypesMatchRule.unexpectedType=Validierungsfehler ({0}) : Variable ''{1}'' vom Typ ''{2}'' verwendet in Position, die Typ ''{3}'' erwartet
 #
 # These are used but IDEA cant find them easily as being called
 #