federation-jvm
federation-jvm copied to clipboard
_Any scalar not handling variable correctly
query($arg1: String!, $arg2: String!, $arg3: String!){
_entities(representations: [{
__typename: "SomeEntity",
arg1: $arg1,
arg2: $arg2,
arg3: $arg3
}]) {
... on SomeEntity {
someField
}
}
}
fails on this line Looks like it's missing the case of VariableReference.
It's minor because gateway doesn't send this kind of query, but it can happen on manual testing and unit tests.
@v3i1r4in This has to do with a limitation of the GraphQL spec with regards to scalars.
In your example, you are providing a literal value
{
__typename: "SomeEntity",
arg1: $arg1,
arg2: $arg2,
arg3: $arg3
}
for the scalar type _Any.
However, the rules for Input Coercion for Scalars do not make any mention of substitution of a variable's runtime value (this is in contrast to e.g. the rules for Input Coercion for Input Objects).
Accordingly, the APIs of GraphQL libraries usually don't support this kind of substitution in scalars. Even if we were to add case-handling for VariableReference in the line you've cited, we wouldn't be able to perform variable substitution with the variable's runtime value, as graphql-java doesn't provide us with those values during scalar input coercion. (Arguably though in such cases, _Any's parseLiteral() function should throw CoercingParseLiteralException instead of an AssertException in order to abide by the method contract, so I'll file a PR to fix that.)
The general alternative is to either fully embed the scalar in the GraphQL operation as a literal value, e.g.
{
__typename: "SomeEntity",
arg1: "foo",
arg2: "bar",
arg3: "baz"
}
or pass a variable reference for the scalar type _Any instead of a literal value.
as graphql-java doesn't provide us with those values during scalar input coercion
I think it does, https://github.com/graphql-java/graphql-java/blob/5396ab20f6aae315327c14a77ff2f66c8fd403c7/src/main/java/graphql/schema/Coercing.java#L98. Although not on the spec, this is supported by the javascript version, it would be nice to match the behavior.
@v3i1r4in
Ah, that's a good catch! It looks like it was added in graphql-java/graphql-java#1170 , which was released in graphql-java v9.7.
Given that graphql-js and graphql-java both support this, I'd be fine with doing the variable substitution in parseLiteral(), and will file a PR for the update.
that's perfect, thanks!