Allow for more customization in GraphQlArgumentBinder

[ooraini]

To customize controller argument binding you have to use HandlerMethodArgumentResolver which can bind any number GraphQl argument to single method parameter, the interface is quite flexible but requires more work. Also, no more @Argumenton the handler parameters, which makes things less obvious.

As a motivation, GraphQL Java recently added support for @oneof directive for input types, to get the most out of it, you will need to customize the handling for arguments based on a directive(rolling out the usage of Converter).

I really like the signature of the bind method on GraphQlArgumentBinder:

public Object bind(DataFetchingEnvironment environment, @Nullable String name, ResolvableType targetType)
			throws BindException {...}

(Can we get rid of the @Nullable? matching the Argument in GraphQlArgumentBinder)

One solution would be to introduce a new abstraction and do Spring's standard way of registering beans implementing said abstraction and the framework would auto wire them in the right place.

Another solution is to change GraphQlArgumentBinder and make it more friendly to user extension. e.g. make most methods protected, add an extension point for implementers, and register it as a bean so users can replace it.

[rstoyanchev]

Thanks for raising this.

From what I can see @oneof is a kind of union input type. How would you like this to be supported, what do you expect it to look like on a controller method?

For the @Nullable name on GraphQlArgumentBinder, we do pass null when the intent is to bind the full map to the target object, e.g. @Arguments (rather than @Argument). So that's there for a reason, and it's mentioned in the Javadoc. Before discussing specific changes, it's important to understand what is required.

[ooraini]

Hi @rstoyanchev

I would like to start by saying that this issue is not meant to be "Here is problem that needs a fix", it's more of "here a suggestion, if you think it's worthwhile to implement, please consider it". I mentioned @oneof as a motivating example, but by no means it should drive the design/implementation(however, it should be considered) .

What I tried to convey is that HandlerMethodArgumentResolver is very powerful, but not very obvious, for example:

    @QueryMapping
    Page<Company> companies(Pageable pageable) {
        return companyRepository.findAll(pageable);
    }

In the method signature, it's not very clear how the GraphQl field argument(s) is related to the method parameter, and the parameter name has no bearing to the argument name. In fact, you could rename it to Pageable p and it will still work.
The second thing is that the implementation of the HandlerMethodArgumentResolver has an implicit contract with the argument name:

    @Override
    public Object resolveArgument(MethodParameter parameter, DataFetchingEnvironment environment) {
        Map<String, Object> = environment.getArgument("page"); // implicit contract, must be named "page" in the schema
         ....
    }

In the opposite side, the @Argument annotation:

    @QueryMapping
    Page<Company> companies(@Argument Pageable page {
        return companyRepository.findAll(page);
    }

It's very clear that the field contains and argument named page and some binder/resolver/whatever does the type conversion.

It's hard to configure/extend GraphQlArgumentBinder as it's, so here is my suggestions.

Binding GraphQl argument happens in more than one context: 1) field arguments. 2) schema directives. 3) operation directives. All have the one thing in common, they only support input types GraphQLInputType.

So we may come up with an interface like this:

import graphql.language.Directive;
import graphql.schema.DataFetchingEnvironment;
import graphql.schema.GraphQLAppliedDirective;
import org.springframework.core.ResolvableType;
import org.springframework.validation.BindException;

public abstract class GraphQlInputBinder {
    // specific argument with a name
    abstract Object bind(DataFetchingEnvironment environment, String name, ResolvableType targetType) throws BindException;
     // all arguments
    abstract Object bind(DataFetchingEnvironment environment, ResolvableType targetType) throws BindException;

    abstract Object bind(Directive directive, String name, ResolvableType targetType) throws BindException;
    abstract Object bind(Directive directive, ResolvableType targetType) throws BindException;

    abstract Object bind(GraphQLAppliedDirective directive, String name, ResolvableType targetType) throws BindException;
    abstract Object bind(GraphQLAppliedDirective directive, ResolvableType targetType) throws BindException;

    // Other protected helper methods just as in GraphQlArgumentBinder
   // bindMapToObjectViaConstructor, bindCollection, ...etc
}

So facilitate extension, GraphQlInputBinder is registered as a bean, and users can replace it. So If I wanted to support @oneof I would register a bean that override one of the methods:

    @Override
    Object bind(DataFetchingEnvironment environment, String name, ResolvableType targetType) throws BindException {
        GraphQLAppliedDirective oneOf = environment.getFieldDefinition().getArgument(name).getAppliedDirective("oneOf");
        if (oneOf == null) return super.bind(environment, name, targetType);
        ...
        // call one of the bind methods
    }

This is a rough sketch. If you don't think it's worthwhile, feel free to close this. As it's I can do everything that I need.

From what I can see @oneof is a kind of union input type. How would you like this to be supported, what do you expect it to look like on a controller method?

There is more than one way to do it, and we will not agree on single solution, so it's best to leave it for users to implement. I myself will try to use sealed types and somehow do field -> java type mapping.

Side note: I wanted to open this as a "Discussion", but they are not enabled.

[hwhh]

Apologies I know this isn't entirely relevant to the initial issue, but I was wondering if the spring-graphql library does/will support the oneOf directive , it seems that support was added for the annotation in java-graphql v21.2 and this library is already using v22 but whenever I try and use the directive I get the following error: Unknown directive "oneOf"