[Question] - Parsing ScalarType with Object input

[sfriberg]

Hi,

I've tried using a scalar that takes an object as input, which makes it easier to write the query in graphiql as it is basically json rather than a regular string.

So now I can write a query with the following format and I can parse the key and value using the Value classes (ObjectValue, IntValue, StringValue, etc.)

{ query(parameter: {key: value}) { result }}

However if I send the query using variables I instead get a VariableReference instead of a regular Value class, and I can't get the value as I don't have the DataFetchingEnv when parsing a scalar.

Query($value: String!) { query(parameter: {key: $value}) { result }}

Is this expected or should graphql-java expand the scalar object with the value from the variable?
Or am I simply just misusing the Scalar implementation and instead should parse the object/json input in different way?

Thanks,
Staffan

[bbakerman]

I've tried using a scalar that takes an object as input.

I am not sure what that means. in graphql only "fields" take arguments. A scalar is a type (both input and output type) and represents an indivisable value.

I suspect you are "misusing the Scalar implementation"

Can you give more concrete examples. graphql-java should be doing all the execution and parsing for you

[sfriberg]

Hi,

Here is a small example of what I was trying to do.

Basically the variable is used as part of the input json object.

https://github.com/sfriberg/graphqlScalar

Thanks,
Staffan

[bbakerman]

Yeah you have your terminology and type systems wrong.

query testOk {
  field(arg: {test: "ok"} )
}

type Query {
	field(arg: Test): String
}

input Test {
     test : String   
}

You should not use a scalar here but an input type. This is the way we describe compound object values for field arguments

Scalars are leaf nodes that have a defined value set. You have a compound "object" here not a scalar

https://graphql.org/learn/schema/#scalar-types

[sfriberg]

The use case here is a basically a where clause similar to graphcool/prisma, so encoding this by hand would be tedious. So the best solution here would be to autogenerate this for each call. As a first iteration tried to basically have a generic object instead (parse JSON through scalar). Is it possible to have a generic object as input type?

[sfriberg]

Would it be possible to have the scalar call contain the variable list/map?
Which seems to be roughly what is done here, https://github.com/taion/graphql-type-json.

[sfriberg]

Hi @bbakerman

Sorry for pinging you. Just wanted to understand if supporting something like taion/graphql-type-json would be acceptable in graphql-java?

Did a quick prototype and supporting the regular primitives at least as part of the final scalar parsing is rather non-intrusive as the variable map is available there. Could be done with a default method taking the variable map so it is backwards compatible.

The first call scalar parsing that happens as part of verification seems to require a bit more work as the variables weren't part of that call chain (still need to read the code a bit more to understand the flow).

Thanks,
Staffan

[bbakerman]

We would not accept a JSON scalar type in graphql-java. You can however write one and exposes it as a library say based on graphql-java.

The idea of making the variables map part of the "Coercing" callback is just wrong. A Scalar is meant to be an atomic type - that is cant be broken down further.

So you can have a JSON scalar type say but its does not reach into the variables map to grab part of itsself like you have done. Eg your have

query A($value: String!) { 
    query(parameter: {key: $value}) { 
           result 
     }
}

where the $value is actually a string but you want your JSON scalar to reach into the variables map to get a small part of it. That is mix AST with variables. Graphql is not meant to be used like that.

A more proper example would be

query A($value: JsonScalar) { 
    query(parameter: $value) { 
           result 
     }
}

Notice here we have your JsonScalar and it is passed to the field as a parameter. But as a whole variable - not as part AST and part variable. Mixing the two is NOT how graphql should work.

[bbakerman]

I write a really simple groovy test showing this

    def "map like scalar"() {

        def mapScalar = GraphQLScalarType.newScalar().name("MapScalar").coercing(new Coercing() {
            @Override
            Object serialize(Object dataFetcherResult) throws CoercingSerializeException {
                return dataFetcherResult
            }

            @Override
            Object parseValue(Object input) throws CoercingParseValueException {
                return input
            }

            @Override
            Object parseLiteral(Object input) throws CoercingParseLiteralException {
                return input
            }
        }).build()

        def spec = '''
            scalar MapScalar
            
            type Query {
                query(parameter : MapScalar) : Result
            }
            
            type Result {
                result : MapScalar
            }
        '''

        def dataFetcher = new DataFetcher() {
            @Override
            Object get(DataFetchingEnvironment environment) {
                def argument = environment.getArgument("parameter")
                return argument
            }
        }

        def runtimeWiring = newRuntimeWiring().type(newTypeWiring("Query")
                .dataFetcher("query", dataFetcher)
        )
                .scalar(mapScalar)
                .build()

        def schema = TestUtil.schema(spec, runtimeWiring)

        def graphql = GraphQL.newGraphQL(schema).build()

        def query = '''
            query A { 
                query(parameter: {result : "value"}) { 
                    result
                }
            }'''
        
         // use variables
         query = '''
            query A($value: MapScalar) {
                query(parameter: $value) {
                    result
                }
            }'''
        def executionInput = ExecutionInput.newExecutionInput().variables([value: [result: "value"]]).query(query).build()

        when:
        def er = graphql.execute(executionInput)
        then:
        er.errors.isEmpty()
        println(er.data)
    }

[bbakerman]

This shows a Java Map scalar but it could be a JSON one where you call Jackson or GSON to parse/serialise JSON say

[bbakerman]

Ok I looked further into this and the spec says nothing about whether the Scalar.parseLiteral method should take a variables map (allowing you to mix AST and variables)

However looking at graphql-js (the reference implementation) I see the following definition

parseLiteral: GraphQLScalarLiteralParser<*>;
export type GraphQLScalarLiteralParser<TInternal> = (
  valueNode: ValueNode,
  variables: ?ObjMap<mixed>,
) => ?TInternal;

So it in fact it has a variables object defined as argument

HOWEVER there is some validation code that never gives in the variables map and there are NOT built in scalars that use variables.

So it seems you CAN reach in and mix AST with variable references kinda sort of but the spec has nothing to see on it

[sfriberg]

Hi Brad,

Thank you for taking the time to dive into this and your detailed answer. Much appreciated!

Will simply provide the full object as a parameter.

I will let you decide if you want to pursue providing the variable map or not to parseLiteral. Feel free to close this as Will-Not-Fix if you don't, or keep it open for tracking if you do.

Thanks,
Staffan

[bbakerman]

Yeah trying to decide what do do here. The spec is absent on variables being available and what I am calling partial AST eg `field(argument : { objectKey : $variableReference })

The JavaScript implementation seems to have it based on reading the code.

There is 2 calls to parseLiteral - one for validation (we ask that a value be created so we can let them say if its ok or not (currently no variables yet since they havent been translated)

The JS implementation does

 // Scalars determine if a literal value is valid via parseLiteral() which
  // may throw or return an invalid value to indicate failure.
  try {
    const parseResult = type.parseLiteral(node, undefined /* variables */);
    if (isInvalid(parseResult)) {
      context.reportError(
        new GraphQLError(
          badValueMessage(inspect(locationType), print(node)),
          node,
        ),
      );
    }
  } catch (error) {
    // Ensure a reference to the original error is maintained.
    context.reportError(
      

So no variables being passed here

And then there is the call to get values for arguments during execution, which is when variables are available

In JS its

  if (isScalarType(type)) {
    // Scalars fulfill parsing a literal value via parseLiteral().
    // Invalid values represent a failure to parse correctly, in which case
    // no value is returned.
    let result;
    try {
      result = type.parseLiteral(valueNode, variables);
    } catch (_error) {
      return; // Invalid: intentionally return no value.
    }

We could have a default method so we don't break ALL Scalars

default Object parseLiteral(Object input, Map<String,Object> variables) {
       return this.parseLiteral(input)
}

@andimarek ? Thoughts? This is a tricky one

[bbakerman]

I have asked on the spec site as well

graphql/graphql-spec#495

[bbakerman]

This is now in place