Default values not working for @GraphQLArgument in 0.9.8

[szantogab]

HI!

Great release for 0.9.8, it was long awaited :)

One issue I encountered with 0.9.8 is that @GraphQLArgument seems to ignore defaultValue.

I have the following code:

    @GraphQLQuery(description = "Queries nearby ShoutOuts, that should be displayed on the map for the given user.")
    fun discoverShoutOuts(lat: Double, lng: Double, @GraphQLArgument(name = "radiusInMeters", defaultValue = "50000") radiusInMeters: Double): Collection<ShoutOut> = shoutOutService.discoverShoutOuts(lat, lng, radiusInMeters)

After upgrading to 0.9.8 the radiusInMeters argument is required and doesn't have a default value.

Is there anything to do with it to make it work?

I tried to debug it, and I see that in AnnotatedArgumentBuilder, the defaultValue() method correctly returns the value I specified, so the problem must be afterwards.

[kaqqao]

Ouch 😫
Will check it. If this is broken, 0.9.9 will have to come out immediately...

[szantogab]

I'm glad to help you if you can point me to the class where it is likely broken.

[kaqqao]

A quick test in Java doesn't seem to replicate the issue for me:

@GraphQLQuery(description = "Queries nearby ShoutOuts, that should be displayed on the map for the given user.")
public Collection<String> discoverShoutOuts(Double lat, Double lng, @GraphQLArgument(name = "radiusInMeters", defaultValue = "50000") Double radiusInMeters) {
    return Collections.emptyList();
}

AnnotatedArgumentBuilder#defaultValue would indeed be my first suspect. If that's fine, could you maybe break at OperationMapper#toGraphQLArgument and check what happens there? Same for DefaultOperationBuilder.java:106?

[szantogab]

After digging deeper, it seems like when I execute a GraphQL request with the value not specified, it works fine and it uses the default value. However, GraphiQL shows me an error that a required argument is not specified.

Looking at the introspection query response, it seems like the defaultValue is there, but the field is NON_NULL:

{"name":"radiusInMeters","description":"","type":{"kind":"NON_NULL","name":null,"ofType":{"kind":"SCALAR","name":"Float","ofType":null}},"defaultValue":"50000.0"}

So I can't tell it it's a bug or it is supposed to be like this.

[szantogab]

I checked the code at OperationMapper#toGraphQLArgument and it seemed fine, except that the type was GraphQLNonNull, which I think is wrong, but please correct me if this is not how it's working :)

[kaqqao]

In 0.9.8, primitives are always non-null, and I unfortunately don't know enough Kotlin to say what is primitive and what isn't 😕
Could you perhaps break at NonNullMapper#supports and check if that specific argument is detected as primitive?
That would explain the non-nullness.

[kaqqao]

Also, it might be worth to try the latest version of GraphiQL, as it sounds like a possible bug in there... but I'm not sure how to interpret this behavior yet.

[szantogab]

Yes, it is recognized as a primitive. If I make a type nullable in Kotlin, for example: Int?, that's when it is represented as Integer in Java.

So going back to the previous example:

    @GraphQLQuery(description = "Queries nearby ShoutOuts, that should be displayed on the map for the given user.")
    fun discoverShoutOuts(lat: Double, lng: Double, @GraphQLArgument(name = "radiusInMeters", defaultValue = "50000") radiusInMeters: Double): Collection<ShoutOut> = shoutOutService.discoverShoutOuts(lat, lng, radiusInMeters)

It is clear that radiusInMeters is a double primitive and it's non-nullable, and that's exactly what I want in the CODE. But since a defaultValue is provided, it is not required to specify this argument.

Conclusion: is it possible to make arguments that have a default value nullable in the schema? I guess it would make sense?!

Thanks :)

[kaqqao]

I remembered having a reason to leave primitives alone, but I forgot what it was and making them non-null seemed logical... Now the reason is apparent.
For the moment, I'd say either use the non-primitive double or replace the NonNullMapper (just extend it and override the supports method), and I'll release 0.9.9 with a fix this weekend.

To replace a mapper:

generator.withTypeMappers((conf, current) -> current.replace(NonNullMapper.class, new CustomNonNullMapper()))

[kaqqao]

The GraphQL spec has changed to better deal with non-null arguments with a default value.
graphql/graphql-spec#418
Not sure if/when this will take effect in graphql-java.

[kaqqao]

My commit makes sure everything with a default value is nullable, so that will be the behavior in 0.9.9.

When graphql-java catches up with the spec, I'll probably have to revert that bit.