Best practices for handling optional arguments?

We just updated graphql-java-tools from 5.4.1 and the biggest change is around nullable input fields (I think caused by #315).

Previously, we were defining our resolver methods with Optional<> arguments for those that are nullable in the schema, e.g. first in

customers(input: CustomerSearchInput!, first: Int): [Customer!]

... mapped to the following resolver method:

public List<Customer> getCustomers(CustomerSearchInput input, Optional<Integer> first) { ... }

Previously, the first argument was Optional.empty() regardless of whether the client passed an explicit first: null or didn't pass the argument at all. However, now we are getting null when the client doesn't pass the argument, which requires us to handle both null and Optional.empty in all resolver methods with optional arguments.

Is there a way to opt-out of this new behavior? Or if not, a recommended way to unify handling of null and absence? e.g. in the object mapper configurer or something?

This was done so that it could be detected whether the client passed in a parameter or not. If you are not interested in the distinction, can you just use Integer first and do a simple null check on it?

@vojtapol

@vojtapol this is what we've started doing (updating the arguments to the unwrapped type, and doing an Optional.ofNullable on it inside the method body), but it will be duplicated in a lot of places, almost all of which the distinction between null and absent isn't meaningful.

Another question... we already have a custom class that we use to differentiate null vs. absent called Omissible, which is basically a wrapper around Optional<Optional<T>>, so Omissible.absent == Optional.empty, Omissible.null == Optional.of(Optional.empty), and Omissible.of(value) == Optional.of(Optional.of(value)). Is there a way to hook up the schema object mapper to deserialize nullable arguments into Omissible?

Edit: also want to mention that we have been using a custom deserializer for our omissible class on the schema object mapper for a while now, and it works for deserializing input object fields into omissibles, but we're seeing now that it doesn't work for top level field arguments. It seems like it should work the same for both?

@vojtapol

@vojtapol this is what we've started doing (updating the arguments to the unwrapped type, and doing an Optional.ofNullable on it inside the method body), but it will be duplicated in a lot of places, almost all of which the distinction between null and absent isn't meaningful.

I think the right way to solve this would be to add a configuration option that changes how Optional behaves. The current behavior would be the default but you could switch it to the old behavior.

also want to mention that we have been using a custom deserializer for our omissible class on the schema object mapper for a while now, and it works for deserializing input object fields into omissibles, but we're seeing now that it doesn't work for top level field arguments. It seems like it should work the same for both?

Can you show me how are you passing the custom deserializer? I don't need to see its implementation, just how it's provided to GraphQL Java Tools.

I think the right way to solve this would be to add a configuration option that changes how Optional behaves.

👍 to this, we definitely don't want to have to null check top level arguments, when input sub-fields just work.

Can you show me how are you passing the custom deserializer?

GraphQLSchema graphQLSchema = SchemaParser.newParser()
  ...
  .options(SchemaParserOptions.newOptions()
    .objectMapperConfigurer((mapper, context) -> {
      mapper.registerModule(new OmissibleModule());
    })
    .build())
.build()
.makeExecutableSchema();

OmissibleModule is basically

public class OmissibleModule extends SimpleModule {
  public OmissibleModule() {
    addDeserializer(Omissible.class, new OmissibleDeserializer());
  }
}

Any thoughts on this? This is basically blocking us from upgrading to the latest version

#404 addresses the first half of this issue. Let me know if it would fix your issue.

Another question... we already have a custom class that we use to differentiate null vs. absent called Omissible, which is basically a wrapper around Optional<Optional>, so Omissible.absent == Optional.empty, Omissible.null == Optional.of(Optional.empty), and Omissible.of(value) == Optional.of(Optional.of(value)). Is there a way to hook up the schema object mapper to deserialize nullable arguments into Omissible?

Edit: also want to mention that we have been using a custom deserializer for our omissible class on the schema object mapper for a while now, and it works for deserializing input object fields into omissibles, but we're seeing now that it doesn't work for top level field arguments. It seems like it should work the same for both?

This doesn't work because types that are a GraphQL scalars bypass the object mapper. We might want to look into changing that.

@vojtapol

@vojtapol any chance for a patch release in the near future?

Yeah. There is a chance for that. We are planning to make the input argument mapping a lot more flexible for users that need more customization.

Navigation Menu

Repository navigation

Best practices for handling optional arguments? #388

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Best practices for handling optional arguments? #388

Description

Activity

vojtapol commented on Apr 9, 2020

alexgenco commented on Apr 9, 2020

alexgenco commented on Apr 9, 2020

vojtapol commented on Apr 12, 2020

alexgenco commented on Apr 14, 2020

alexgenco commented on May 15, 2020

vojtapol commented on May 21, 2020

alexgenco commented on May 22, 2020

vojtapol commented on May 27, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions