-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Description
Setup
I'll use the following simple schema to illustrate the issue:
type Query {
items(pagination: Pagination = {limit: 10, offset: 0}): [String]
}
input Pagination {
limit: Int
offset: Int
}While variables are commonly used as the whole argument value:
query Items($pagination: Pagination) {
items(pagination: $pagination) { #variable is the entire input object (Pagination)
...
}
}they can also be used as the values of fields inside an input object:
query Items($limit: Int) {
items(pagination: {limit: $limit, offset: 0}) { #variable is only a part of the input
...
}
}I've scoured the spec for a confirmation on whether this is indeed legal usage of variables and, while this situation is never directly addressed either way, the section on Input Coercion lists the following examples which seem to indicate this is legitimate usage:
input ExampleInputObject {
a: String
b: Int!
}| Literal Value | Variables | Coerced Value |
|---|---|---|
{ a: $var, b: 123 } |
{ var: null } |
{ a: null, b: 123 } |
{ b: $var } |
{ var: 123 } |
{ b: 123 } |
The bug
When a variable is used inside an input object which has a default value (like in my Pagination example), graphql-java's variable type validation freaks out, because it compares the type of the variable (Int) against the default value of the entire argument (Pagination) here:
//Notice the getValidationContext().getArgument()
Optional<InputValueWithState> schemaDefault = Optional.ofNullable(getValidationContext().getArgument()).map(v -> v.getArgumentDefaultValue());
...
//expectedType is Int
schemaDefaultValue = ValuesResolver.valueToLiteral(schemaDefault.get(), expectedType, getValidationContext().getGraphQLContext(), getValidationContext().getI18n().getLocale());To Reproduce
Here's a self-contained example:
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<?>) env -> Collections.singletonList("test"))
.build())
.build();
GraphQL gql = GraphQLRuntime.newGraphQL(schema).build();
Map<String, Object> vars = new HashMap<>();
vars.put("limit", 5);
vars.put("offset", 0);
ExecutionInput in = ExecutionInput.newExecutionInput()
.query("query Items( $limit: Int, $offset: Int) {\n" +
" items(\n" +
" pagination: {limit: $limit, offset: $offset} \n" +
" )\n" +
"}")
.variables(vars)
.build();
ExecutionResult result = gql.execute(in);Leads to:
graphql.AssertException: Expected a value that can be converted to type 'Int' but it was a 'HashMap'