Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn Java overrides into GraphQL OneOf #2099

Open
t1 opened this issue Apr 28, 2024 · 8 comments
Open

Turn Java overrides into GraphQL OneOf #2099

t1 opened this issue Apr 28, 2024 · 8 comments
Labels
Client This issue applies to the Client side enhancement New feature or request Server This issue applies to the Server side

Comments

@t1
Copy link
Collaborator

t1 commented Apr 28, 2024

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 t1 added enhancement New feature or request Server This issue applies to the Server side Client This issue applies to the Client side labels Apr 28, 2024
@t1
Copy link
Collaborator Author

t1 commented Apr 28, 2024

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

@phillip-kruger
Copy link
Member

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

@t1
Copy link
Collaborator Author

t1 commented Apr 29, 2024

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

BTW: you really look like an Australian now 😎

@phillip-kruger
Copy link
Member

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

@jmartisk
Copy link
Member

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
Copy link
Collaborator Author

t1 commented Apr 29, 2024

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
Copy link
Collaborator Author

t1 commented Apr 29, 2024

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue applies to the Client side enhancement New feature or request Server This issue applies to the Server side
Projects
None yet
Development

No branches or pull requests

3 participants