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

GraphQL Mutation exceptions when no setters are present. #1387

Open
zbhavyai opened this issue May 8, 2022 · 12 comments
Open

GraphQL Mutation exceptions when no setters are present. #1387

zbhavyai opened this issue May 8, 2022 · 12 comments

Comments

@zbhavyai
Copy link

zbhavyai commented May 8, 2022

Hello,

With Quarkus GraphQL, I am facing couple of exceptions with mutations when setters are not present in the model class. I made a small reproducer for this, which is available at https://github.com/zbhavyai/quarkus-graphql-nosetters-reproducer.

To explain, the Film class has no setters. The REST endpoints to add a Film works fine. The GraphQL mutations however throw exception - Cannot create instance of a class. No default constructor found. When I add a default constructor, InvalidSchemaException is thrown at build time.

The exceptions faced might be somewhat related to #1342, but I am not sure if the actual issue is related at all.

So, my question is - why exactly are setters required? If REST works fine without them, shouldn't GraphQL too? Is it bug or GraphQL limitation?

Thanks in advance.

@phillip-kruger
Copy link
Member

To be able to do a mutation you are going to need setters. Why do you not have setters ?

@zbhavyai
Copy link
Author

zbhavyai commented May 8, 2022

Because, I am building a GraphQL wrapper on an existing service that doesn't have setters. The REST wrapper on that works without any issues, so I wondered why GraphQL doesn't work.

@zbhavyai
Copy link
Author

zbhavyai commented May 8, 2022

Is there any particular reason why GraphQL mutation need setters? IMHO the parameterized constructors should be enough to create the object using the Jackson deserialization.

Thanks.

@robp94
Copy link
Contributor

robp94 commented May 8, 2022

If I remember correctly, the quarkus version still uses jsonb. If you don't have setters, you need to do something like this for the server input objects

@zbhavyai
Copy link
Author

zbhavyai commented May 8, 2022

@robp94 Yeah this one works after I update my constructor with @JsonbCreator. Thanks for letting this know!

@zbhavyai zbhavyai closed this as completed May 8, 2022
@zbhavyai
Copy link
Author

Want to check, can we have Jackson support for Quarkus GraphQL?

@zbhavyai zbhavyai reopened this May 15, 2022
@phillip-kruger
Copy link
Member

What do you mean by Jackson support?

@zbhavyai
Copy link
Author

zbhavyai commented May 15, 2022

To put it simply, expecting GraphQL mutations to work with @JsonCreater annotated constructor rather than to rely on @JsonbCreator as robp94 suggested.

@phillip-kruger
Copy link
Member

Should be possible. Let's leave this open and we can look at it at some point. Or you can do a PR if you are keen.

@phillip-kruger
Copy link
Member

I think this should work with com.fasterxml.jackson.annotation.JsonCreator ( at least I see the code for it:

public static final DotName JACKSON_CREATOR = DotName.createSimple("com.fasterxml.jackson.annotation.JsonCreator");
)

@zbhavyai
Copy link
Author

zbhavyai commented Aug 2, 2022

Not sure if you meant to do it differently (posted reproducer does uses @JsonCreator), but I see the exact same error. Tried with quarkus version 2.11.1.Final. Again, @JsonbCreator works but not @JsonCreator.

@phillip-kruger
Copy link
Member

phillip-kruger commented Aug 2, 2022

Yes initially I thought we don't support it, but we do, as the annotations are there, so this is a bug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants