diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 006a2c915d..8b8f17b005 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -7,6 +7,7 @@ on: pull_request: branches: - master + - 21.x - 20.x - 19.x - 18.x diff --git a/build.gradle b/build.gradle index d545b92572..4768218f76 100644 --- a/build.gradle +++ b/build.gradle @@ -50,6 +50,7 @@ def reactiveStreamsVersion = '1.0.3' def slf4jVersion = '2.0.7' def releaseVersion = System.env.RELEASE_VERSION def antlrVersion = '4.11.1' // https://mvnrepository.com/artifact/org.antlr/antlr4-runtime +def guavaVersion = '32.1.1-jre' version = releaseVersion ? releaseVersion : getDevelopmentVersion() group = 'com.graphql-java' @@ -89,7 +90,7 @@ dependencies { api 'com.graphql-java:java-dataloader:3.2.0' api 'org.reactivestreams:reactive-streams:' + reactiveStreamsVersion antlr 'org.antlr:antlr4:' + antlrVersion - implementation 'com.google.guava:guava:32.1.1-jre' + implementation 'com.google.guava:guava:' + guavaVersion testImplementation group: 'junit', name: 'junit', version: '4.13.2' testImplementation 'org.spockframework:spock-core:2.0-groovy-3.0' testImplementation 'org.codehaus.groovy:groovy:3.0.18' @@ -124,7 +125,7 @@ shadowJar { } relocate('org.antlr.v4.runtime', 'graphql.org.antlr.v4.runtime') dependencies { - include(dependency('com.google.guava:guava:32.1.1-jre')) + include(dependency('com.google.guava:guava:' + guavaVersion)) include(dependency('org.antlr:antlr4-runtime:' + antlrVersion)) } from "LICENSE.md" diff --git a/src/main/java/graphql/execution/ValuesResolverConversion.java b/src/main/java/graphql/execution/ValuesResolverConversion.java index 83f5285b0d..eff4d1cef1 100644 --- a/src/main/java/graphql/execution/ValuesResolverConversion.java +++ b/src/main/java/graphql/execution/ValuesResolverConversion.java @@ -69,11 +69,17 @@ static Object valueToLiteralImpl(GraphqlFieldVisibility fieldVisibility, if (valueMode == NORMALIZED) { return assertShouldNeverHappen("can't infer normalized structure"); } - return ValuesResolverLegacy.valueToLiteralLegacy( + Value value = ValuesResolverLegacy.valueToLiteralLegacy( inputValueWithState.getValue(), type, graphqlContext, locale); + // + // the valueToLiteralLegacy() nominally cant know if null means never set or is set to a null value + // but this code can know - its is SET to a value so, it MUST be a Null Literal + // this method would assert at the end of it if inputValueWithState.isNotSet() were true + // + return value == null ? NullValue.of() : value; } if (inputValueWithState.isLiteral()) { return inputValueWithState.getValue(); diff --git a/src/main/java/graphql/validation/TraversalContext.java b/src/main/java/graphql/validation/TraversalContext.java index dfd3e4920d..93bbdb62e6 100644 --- 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 outputTypeStack = new ArrayList<>(); private final List parentTypeStack = new ArrayList<>(); private final List inputTypeStack = new ArrayList<>(); + private final List defaultValueStack = new ArrayList<>(); private final List fieldDefStack = new ArrayList<>(); private final List 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 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 arguments, String name) { @Override public void leave(Node node, List 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 lastElement(List list) { - if (list.size() == 0) return null; + if (list.isEmpty()) { + return null; + } return list.get(list.size() - 1); } + private T pop(List 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 index 2646afcdbb..18fe678177 100644 --- 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 index de395850af..ef7c4069f4 100644 --- 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 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 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 index 8208eba393..0b47fcec78 100644 --- 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 index 9733f79738..4a6a551fdf 100644 --- 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 index def39f94ea..c15e3bb550 100644 --- a/src/main/resources/i18n/Validation_de.properties +++ b/src/main/resources/i18n/Validation_de.properties @@ -76,7 +76,7 @@ VariableDefaultValuesOfCorrectType.badDefault=Validierungsfehler ({0}) : Ung # 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}'' überein +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 # diff --git a/src/test/groovy/graphql/execution/ValuesResolverE2ETest.groovy b/src/test/groovy/graphql/execution/ValuesResolverE2ETest.groovy new file mode 100644 index 0000000000..590d84e277 --- /dev/null +++ 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 + 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 + 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 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 + 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!'" + } +} diff --git a/src/test/groovy/graphql/introspection/IntrospectionTest.groovy b/src/test/groovy/graphql/introspection/IntrospectionTest.groovy index 712ac8048c..bee6b161e8 100644 --- a/src/test/groovy/graphql/introspection/IntrospectionTest.groovy +++ b/src/test/groovy/graphql/introspection/IntrospectionTest.groovy @@ -1,5 +1,6 @@ package graphql.introspection + import graphql.TestUtil import graphql.schema.DataFetcher import graphql.schema.FieldCoordinates @@ -438,7 +439,7 @@ class IntrospectionTest extends Specification { def "test AST printed introspection query is equivalent to original string"() { when: - def oldIntrospectionQuery = "\n" + + def oldIntrospectionQuery = "\n" + " query IntrospectionQuery {\n" + " __schema {\n" + " queryType { name }\n" + @@ -540,12 +541,11 @@ class IntrospectionTest extends Specification { " }\n" + "\n" - def newIntrospectionQuery = IntrospectionQuery.INTROSPECTION_QUERY; + def newIntrospectionQuery = IntrospectionQuery.INTROSPECTION_QUERY then: - oldIntrospectionQuery.replaceAll("\\s+","").equals( - newIntrospectionQuery.replaceAll("\\s+","") - ) + oldIntrospectionQuery.replaceAll("\\s+", "") == + newIntrospectionQuery.replaceAll("\\s+", "") } def "test parameterized introspection queries"() { @@ -582,44 +582,66 @@ class IntrospectionTest extends Specification { def parseExecutionResult = { [ - it.data["__schema"]["types"].find{it["name"] == "Query"}["fields"].find{it["name"] == "notDeprecated"}["description"] != null, // descriptions is true - it.data["__schema"]["types"].find{it["name"] == "UUID"}["specifiedByURL"] != null, // specifiedByUrl is true - it.data["__schema"]["directives"].find{it["name"] == "repeatableDirective"}["isRepeatable"] != null, // directiveIsRepeatable is true - it.data["__schema"]["description"] != null, // schemaDescription is true - it.data["__schema"]["types"].find { it['name'] == 'InputType' }["inputFields"].find({ it["name"] == "inputField" }) != null // inputValueDeprecation is true + it.data["__schema"]["types"].find { it["name"] == "Query" }["fields"].find { it["name"] == "notDeprecated" }["description"] != null, // descriptions is true + it.data["__schema"]["types"].find { it["name"] == "UUID" }["specifiedByURL"] != null, // specifiedByUrl is true + it.data["__schema"]["directives"].find { it["name"] == "repeatableDirective" }["isRepeatable"] != null, // directiveIsRepeatable is true + it.data["__schema"]["description"] != null, // schemaDescription is true + it.data["__schema"]["types"].find { it['name'] == 'InputType' }["inputFields"].find({ it["name"] == "inputField" }) != null // inputValueDeprecation is true ] } when: - def allFalseExecutionResult = graphQL.execute( + def allFalseExecutionResult = graphQL.execute( IntrospectionQueryBuilder.build( - IntrospectionQueryBuilder.Options.defaultOptions() - .descriptions(false) - .specifiedByUrl(false) - .directiveIsRepeatable(false) - .schemaDescription(false) - .inputValueDeprecation(false) - .typeRefFragmentDepth(5) + IntrospectionQueryBuilder.Options.defaultOptions() + .descriptions(false) + .specifiedByUrl(false) + .directiveIsRepeatable(false) + .schemaDescription(false) + .inputValueDeprecation(false) + .typeRefFragmentDepth(5) ) - ) + ) then: - !parseExecutionResult(allFalseExecutionResult).any() - allFalseExecutionResult.data["__schema"]["types"].find{it["name"] == "Query"}["fields"].find{it["name"] == "tenDimensionalList"}["type"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"] == null // typeRefFragmentDepth is 5 + !parseExecutionResult(allFalseExecutionResult).any() + allFalseExecutionResult.data["__schema"]["types"].find { it["name"] == "Query" }["fields"].find { it["name"] == "tenDimensionalList" }["type"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"] == null // typeRefFragmentDepth is 5 when: - def allTrueExecutionResult = graphQL.execute( + def allTrueExecutionResult = graphQL.execute( IntrospectionQueryBuilder.build( - IntrospectionQueryBuilder.Options.defaultOptions() - .descriptions(true) - .specifiedByUrl(true) - .directiveIsRepeatable(true) - .schemaDescription(true) - .inputValueDeprecation(true) - .typeRefFragmentDepth(7) + IntrospectionQueryBuilder.Options.defaultOptions() + .descriptions(true) + .specifiedByUrl(true) + .directiveIsRepeatable(true) + .schemaDescription(true) + .inputValueDeprecation(true) + .typeRefFragmentDepth(7) ) - ) + ) + then: + parseExecutionResult(allTrueExecutionResult).every() + allTrueExecutionResult.data["__schema"]["types"].find { it["name"] == "Query" }["fields"].find { it["name"] == "tenDimensionalList" }["type"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"] == null // typeRefFragmentDepth is 7 + } + + def "issue 3285 - deprecated defaultValue on programmatic args prints AST literal as expected"() { + def queryObjType = newObject().name("Query") + .field(newFieldDefinition().name("f").type(GraphQLString) + .argument(newArgument().name("arg").type(GraphQLString).defaultValue(null))) + .build() + def schema = newSchema().query(queryObjType).build() + def graphQL = newGraphQL(schema).build() + + + when: + def executionResult = graphQL.execute(IntrospectionQuery.INTROSPECTION_QUERY) then: - parseExecutionResult(allTrueExecutionResult).every() - allTrueExecutionResult.data["__schema"]["types"].find{it["name"] == "Query"}["fields"].find{it["name"] == "tenDimensionalList"}["type"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"] == null // typeRefFragmentDepth is 7 + executionResult.errors.isEmpty() + + def types = executionResult.data['__schema']['types'] as List + def queryType = types.find { it['name'] == 'Query' } + def fField = (queryType['fields'] as List)[0] + def arg = (fField['args'] as List)[0] + arg['name'] == "arg" + arg['defaultValue'] == "null" // printed AST } } diff --git a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy index e946a06278..cff7a5e6e0 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy @@ -2247,6 +2247,24 @@ type Query { type TestObjectB { field: String } +''' + } + + def "issue 3285 - deprecated defaultValue on programmatic args prints as expected"() { + def queryObjType = newObject().name("Query") + .field(newFieldDefinition().name("f").type(GraphQLString) + .argument(newArgument().name("arg").type(GraphQLString).defaultValue(null))) + .build() + def schema = GraphQLSchema.newSchema().query(queryObjType).build() + + + when: + def options = defaultOptions().includeDirectiveDefinitions(false) + def sdl = new SchemaPrinter(options).print(schema) + then: + sdl == '''type Query { + f(arg: String = null): String +} ''' } } \ No newline at end of file diff --git a/src/test/groovy/graphql/validation/rules/VariableTypesMatchTest.groovy b/src/test/groovy/graphql/validation/rules/VariableTypesMatchTest.groovy index dab3510daa..5ccaff13dd 100644 --- a/src/test/groovy/graphql/validation/rules/VariableTypesMatchTest.groovy +++ b/src/test/groovy/graphql/validation/rules/VariableTypesMatchTest.groovy @@ -2,8 +2,10 @@ package graphql.validation.rules import graphql.StarWarsSchema +import graphql.TestUtil import graphql.i18n.I18n import graphql.parser.Parser +import graphql.schema.GraphQLSchema import graphql.validation.LanguageTraversal import graphql.validation.RulesVisitor import graphql.validation.ValidationContext @@ -14,10 +16,10 @@ import spock.lang.Specification class VariableTypesMatchTest extends Specification { ValidationErrorCollector errorCollector = new ValidationErrorCollector() - def traverse(String query) { + def traverse(String query, GraphQLSchema schema = StarWarsSchema.starWarsSchema) { def document = Parser.parse(query) I18n i18n = I18n.i18n(I18n.BundleType.Validation, Locale.ENGLISH) - def validationContext = new ValidationContext(StarWarsSchema.starWarsSchema, document, i18n) + def validationContext = new ValidationContext(schema, document, i18n) def variableTypesMatchRule = new VariableTypesMatch(validationContext, errorCollector) def languageTraversal = new LanguageTraversal() languageTraversal.traverse(document, new RulesVisitor(validationContext, [variableTypesMatchRule])) @@ -56,7 +58,7 @@ class VariableTypesMatchTest extends Specification { then: errorCollector.containsValidationError(ValidationErrorType.VariableTypeMismatch) // #991: describe which types were mismatched in error message - errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[human]) : Variable type 'String' does not match expected type 'String!'" + errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[human]) : Variable 'id' of type 'String' used in position expecting type 'String!'" } def "invalid variables in fragment spread"() { @@ -78,7 +80,7 @@ class VariableTypesMatchTest extends Specification { then: errorCollector.containsValidationError(ValidationErrorType.VariableTypeMismatch) - errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable type 'String' does not match expected type 'String!'" + errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable 'xid' of type 'String' used in position expecting type 'String!'" } def "mixed validity operations, valid first"() { @@ -104,7 +106,7 @@ class VariableTypesMatchTest extends Specification { then: errorCollector.containsValidationError(ValidationErrorType.VariableTypeMismatch) - errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable type 'String' does not match expected type 'String!'" + errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable 'id' of type 'String' used in position expecting type 'String!'" } def "mixed validity operations, invalid first"() { @@ -130,7 +132,7 @@ class VariableTypesMatchTest extends Specification { then: errorCollector.containsValidationError(ValidationErrorType.VariableTypeMismatch) - errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable type 'String' does not match expected type 'String!'" + errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable 'id' of type 'String' used in position expecting type 'String!'" } def "multiple invalid operations"() { @@ -158,11 +160,97 @@ class VariableTypesMatchTest extends Specification { errorCollector.getErrors().size() == 2 errorCollector.errors.any { it.validationErrorType == ValidationErrorType.VariableTypeMismatch && - it.message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable type 'String' does not match expected type 'String!'" + it.message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable 'id' of type 'String' used in position expecting type 'String!'" } errorCollector.errors.any { it.validationErrorType == ValidationErrorType.VariableTypeMismatch && - it.message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable type 'Boolean' does not match expected type 'String!'" + it.message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable 'id' of type 'Boolean' used in position expecting type 'String!'" } } + + + def "issue 3276 - invalid variables in object field values with no defaults in location"() { + + def sdl = ''' + type Query { + items(pagination: Pagination = {limit: 1, offset: 1}): [String] + } + input Pagination { + limit: Int! + offset: Int! + } + ''' + def schema = TestUtil.schema(sdl) + given: + def query = ''' + query Items( $limit: Int, $offset: Int) { + items( + pagination: {limit: $limit, offset: $offset} + ) + } + ''' + + when: + traverse(query, schema) + + then: + errorCollector.containsValidationError(ValidationErrorType.VariableTypeMismatch) + errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[items]) : Variable 'limit' of type 'Int' used in position expecting type 'Int!'" + } + + def "issue 3276 - valid variables because of schema defaults with nullable variable"() { + + def sdl = ''' + type Query { + items(pagination: Pagination! = {limit: 1, offset: 1}): [String] + } + input Pagination { + limit: Int! + offset: Int! + } + ''' + def schema = TestUtil.schema(sdl) + given: + def query = ''' + query Items( $var : Pagination) { + items( + pagination: $var + ) + } + ''' + + when: + traverse(query, schema) + + then: + errorCollector.errors.isEmpty() + } + + def "issue 3276 - valid variables because of variable defaults"() { + + def sdl = ''' + type Query { + items(pagination: Pagination!): [String] + } + input Pagination { + limit: Int! + offset: Int! + } + ''' + def schema = TestUtil.schema(sdl) + given: + def query = ''' + query Items( $var : Pagination = {limit: 1, offset: 1}) { + items( + pagination: $var + ) + } + ''' + + when: + traverse(query, schema) + + then: + errorCollector.errors.isEmpty() + } } diff --git a/src/test/groovy/graphql/validation/rules/VariablesTypesMatcherTest.groovy b/src/test/groovy/graphql/validation/rules/VariablesTypesMatcherTest.groovy index c6bead4ef6..b05e425090 100644 --- a/src/test/groovy/graphql/validation/rules/VariablesTypesMatcherTest.groovy +++ b/src/test/groovy/graphql/validation/rules/VariablesTypesMatcherTest.groovy @@ -1,6 +1,7 @@ package graphql.validation.rules import graphql.language.BooleanValue +import graphql.language.StringValue import spock.lang.Specification import spock.lang.Unroll @@ -18,7 +19,7 @@ class VariablesTypesMatcherTest extends Specification { def "#variableType with default value #defaultValue and expected #expectedType should result: #result "() { expect: - typesMatcher.doesVariableTypesMatch(variableType, defaultValue, expectedType) == result + typesMatcher.doesVariableTypesMatch(variableType, defaultValue, expectedType, null) == result where: variableType | defaultValue | expectedType || result @@ -27,4 +28,18 @@ class VariablesTypesMatcherTest extends Specification { nonNull(GraphQLBoolean) | new BooleanValue(true) | GraphQLBoolean || true nonNull(GraphQLString) | null | list(GraphQLString) || false } + + @Unroll + def "issue 3276 - #variableType with default value #defaultValue and expected #expectedType with #locationDefaultValue should result: #result "() { + + expect: + typesMatcher.doesVariableTypesMatch(variableType, defaultValue, expectedType, locationDefaultValue) == result + + where: + variableType | defaultValue | expectedType | locationDefaultValue || result + GraphQLString | null | nonNull(GraphQLString) | null || false + GraphQLString | null | nonNull(GraphQLString) | StringValue.of("x") || true + GraphQLString | StringValue.of("x") | nonNull(GraphQLString) | StringValue.of("x") || true + GraphQLString | StringValue.of("x") | nonNull(GraphQLString) | null || true + } }