Support @SchemaMapping and @BatchMapping for schema interface types

[frenchtoast747]

Instead of requesting to re-open #294, I'm creating a new issue.

First off, thank you for this library! It has come a long way since I first started looking into Java + GraphQL over 2 years ago. There have been a lot of great QoL improvements everywhere! Thank you!

Between issues #294 and #236 the suggestion was to create a base batching function and then write a new batching function for each concrete type citing that DGS and NestJS don't support the functionality requested. I'd like to revisit this.

Here's my use-case:

interface Activity {
    id: ID!
    completedBy: [User!]!  # requires a dataloader
    labels: [Lable!]!  # requires a dataloader
    # other fields here...
}

type FooActivity implements Activity {
    # repeat Activity fields here...
    foo: Foo!
    # other fields related to Foo...
}

type BarActivity implements Activity {
    # repeat Activity fields here...
    bar: Bar!
    # other fields related to Bar...
}

Query {
    activities(...): [Activity!]!
}

I should be able to write a single test that executes activities() testing all of the Activity fields which should always pass no matter whether I have one or ten concrete implementations of Activity (though, per the GraphQL spec, I must have at least one).

Right now, I only have a handful of Activity types, but I plan to add more in the future. As I do, with the current state of the library, I will have to just remember to make sure to implement dataloaders for each new concrete type or risk a 500 error on a frontend query that is fetching one of the .users or .labels attributes.

I would like to point out that Python's Graphene library supports setting resolvers on an Interface type and therefore dataloaders so that I don't have to implement the same resolvers on all concrete types (Sadly, this isn't clearly documented as an example, otherwise I'd post a link. I've used it in production code and it just works). That being said, I have to usually supply a TypeResolver to help it out a bit.

Here is what I expect to work:

interface Activity { 
    // ... fields here
}

public class FooActivity implements Activity {}
public class BarActivity implements Activity {}

@BatchMapping
Mono<Map<Activity, List<User>>> completedBy(List<Activity> activities) {
    // use repositories to fetch data
}

However, here is my workaround:

// just a regular function on the controller
Mono<Map<Activity, List<User>>> activityCompletedBy(List<? extends Activity> activities) {
    // use repositories to fetch data
}

// due to type erasure, I have to name each method explicitly 
// and therefore must specify the GraphQL field name explicitly
@BatchMapping(field = "completedBy")
Mono<Map<Activity, List<User>>> fooActivityCompletedBy(List<FooActivity> activities) {
    return activityCompletedBy(activities);
}

@BatchMapping(field = "completedBy")
Mono<Map<Activity, List<User>>> barActivityCompletedBy(List<BarActivity> activities) {
    return activityCompletedBy(activities);
}

// same for the `labels` field, just swapping fields.

[adrian-skybaker]

This is a noticeable gap when migrating from graphql-kickstart, which was able to identify schema mappings to interface based resolvers.

[rstoyanchev]

Thanks for the good description, and helping me to understand the problem.

We can make a change such that if the type of a SchemaMapping or BatchMapping registration is an interface, we instead make registrations for each concrete type conditionally, if there isn't one already. This should help to address the issue transparently while still allowing an application to make registrations for specific concrete types if it needs to. Either way, it is not useful to perform a registration with an interface type as it won't be used.

Let me know if you have any comments. I think we can get this into 1.3.

[frenchtoast747]

we instead make registrations for each concrete type conditionally

Just to check my understanding, you will register the same batch function for each concrete type automatically unless a concrete type already has a concrete type-specific batch function registered?

If so, that sounds great!

[rstoyanchev]

Yes, that is what I meant. This is now available in 1.3.0-SNAPSHOT and we'll be releasing 1.3.0-RC1 tomorrow. If you'd like to give this a try and provide feedback, that would be much appreciated!

[frenchtoast747]

I apologize for the long delay and for not being able to test out the SNAPSHOT and RC versions, but I was finally able to try out 1.3.0 today and it works exactly as I'd expect. Thank you!