Support multiple typeNames on @SchemaMapping and @BatchMapping

[francis-a]

GitHub issue: #235

This draft aims to provide a simple sketch for supporting multiple typeNames on mapping annotations. The use-case for why this may be a helpful inclusion set out in the attached issue.

[pivotal-cla]

@francis-a Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-cla]

@francis-a Thank you for signing the Contributor License Agreement!

[jord1e]

typeNames seems unintuitive

DGS does not do this https://netflix.github.io/dgs/datafetching/#the-dgsdata-dgsquery-dgsmutation-and-dgssubscription-annotations

NestJS does not do this https://docs.nestjs.com/graphql/resolvers

I am also afraid of the added complexity and how tooling would implement this, GraphQL Java also does not implement such a feature

Adding a second SchemMapping is just a copy+paste action and also gives the added ability to use security optimally

[rstoyanchev]

Sorry for the delayed response. I can't say I completely understand the scenario, nor why exactly it doesn't work, at least not without a sample to run.

That said, I can say we don't really want to change the annotation attributes to support a use case that's a bit difficult to explain, and may open up other unexpected possibilities. For a one-off case, declaring multiple @BatchMapping that delegate to the same internal method seems like a reasonable trade-off vs having everyone who looks at the annotations wonder what could multiple values be used for and why that's there.

[francis-a]

Hi @rstoyanchev I totally understand your point. Let me try to describe my use-case a bit more in-case it's helpful. Generally using multiple mapping annotations does work and is a fair tradeoff.

Given the following Schema

interface User {
    username: String!

    "Populated on request, is expensive to calculate"
    expensiveField: String!
}

type UserTypeA implements User {
   ....
}

type UserTypeB implements User {
   ....
}

union BothUsers = UserTypeA | UserTypeB

type Query {
   userFind(username: String!): BothUsers!
}

To go along with this we have the following Kotlin model.

interface User {
   val username: String
   // no expensiveField on this interface, we don't want consumers of this object to need to populate this
}

// in the controller
    @QueryMapping(name = "userFind")
    fun userFind(
        @Argument(name = "username")
        username: String
    ): User {
       // ....
    }

   // here is where we need multiple mappings
   // this is because we need to map expensiveField to both UserTypeA and UserTypeB
    @BatchMapping(typeName = "UserTypeA", field = "expensiveField")
    fun batchUserTypeAexpensiveField(users: List<User>) = ....
    @BatchMapping(typeName = "UserTypeB", field = "expensiveField")
    fun batchUserTypeBexpensiveField(users: List<User>) = ....
 

Hopefully this example makes the use-case a bit more clear.

[jord1e]

How often does that realistically happen for it to warrant such a feature?

More importantly:
In the above example I don't really see a compelling use-case, why are you duplicating fields across types which share the same logic?

[rstoyanchev]

I'm also not quite sure why it is a union when the two types already have a common parent interface.

In any case, thanks for elaborating. We'll leave at that for now. We can always reconsider if this gathers more interest or more uses cases emerge.

[pinguinjkeke]

I'm also not quite sure why it is a union when the two types already have a common parent interface.

In any case, thanks for elaborating. We'll leave at that for now. We can always reconsider if this gathers more interest or more uses cases emerge.

We're facing the same issue in a large collection of interfaces.

Any workarounds?

[earandap]

We are encountering the same problem.

[adrian-skybaker]

How often does that realistically happen for it to warrant such a feature?

Pretty much anytime you use an interface in your GraphQL schema.

[rstoyanchev]

This is now superseded by #871.