smallrye / smallrye-graphql

Implementation for MicroProfile GraphQL
Apache License 2.0
160 stars 92 forks source link

Turn Java overrides into GraphQL OneOf #2099

Open t1 opened 5 months ago

t1 commented 5 months ago

Say I have multiple methods in a GraphQL API class with the same name but different parameters:

@GraphQLApi
public class Users {
    @Mutation public String add(User user) {
        ...
    }

    @Mutation public String add(Comment comment) {
        ...
    }
}

Currently, one add method overwrites the other, so we end up with only a single add mutation. We probably should consider this a bug; it should at least throw a validation error.

But we can do even better: we could derive a schema like this:

type Mutation {
    add(comment: CommentInput, user: UserInput): String
}

And we would validate that exactly one parameter is set; and then call the correct method.

But the fact that a client must provide exactly one argument would not be visible in the schema. The @oneOf directive being discussed for the GraphQL spec is only valid on Input types (yet).

I'm not sure we want to live without this information nor define our own directive like oneOfParameters, so we could also go for a regular @oneOf wrapper type:

type Mutation {
    add(oneOf: OneOfUserOrCommentInput): String
}

input OneOfUserOrCommentInput @oneOf {
    comment: CommentInput
    user: UserInput
}

This would be conforming to the upcoming @oneOf spec, but make the queries a bit more verbose:

mutation addUser {
    add(oneOf: {user: { name: "Jane", slug: "foo" }})
}

mutation addComment {
    add(oneOf: {comment: { text: "bar" }})
}

What do you think?

t1 commented 5 months ago

Maybe we could even implement the inverse on the client side. We'd need more thinking on that.

phillip-kruger commented 5 months ago

I think that is a good idea, what would happen if the user set both ?

t1 commented 5 months ago

The server returns an error explaining that the client should set exactly one.

BTW: you really look like an Australian now 😎

phillip-kruger commented 5 months ago

A, ok , so that is what the OneOf will do ? Can this be done with a directive on the client side ?

jmartisk commented 5 months ago

You can already accomplish the same by doing

@Mutation public String add(UserOrComment userOrComment) {
        ...
    }

@OneOf
class UserOrComment {
    // user, comment fields + getters and setters
}

and it feels like less magic to me

t1 commented 5 months ago

The @oneOf is currently only a directive defined on the server side. I'm not sure there is already any validation.

Doing it manually by hand is surely less magic, but more work... one could say that for GraphQL, too 😜

t1 commented 5 months ago

My question in the oneOf-discussion was answered. Now I understand why the @oneOf on a field nor a new @oneOfParameters is not considered good for GraphQL. Maybe we should just follow their lead and only do the automatic wrapping.

jmartisk commented 5 months ago

The @oneOf is currently only a directive defined on the server side. I'm not sure there is already any validation.

There is validation for it built into graphql-java