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

Allow JsonbDeserializer based on generic types #14824

Merged

Conversation

loicmathieu
Copy link
Contributor

This PR allows to use the existing JsonbDeserializer not only based on a Class but also on a Type.
This allows to deals with Generic collections https://javaee.github.io/jsonb-spec/docs/user-guide.html#mapping-a-generic-collection

public class ListOfPersonDeserializer extends JsonbDeserializer {
    public ListOfPersonDeserializer() {
        super(new ArrayList<Person>(){}.getClass().getGenericSuperclass());
    }
} 

@ghost ghost added the area/kafka label Feb 4, 2021
Copy link
Contributor

@gunnarmorling gunnarmorling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; shouldn't you be able to get away with a single field of type Type though (which Class implements)? Saving one field and the if/else upon deserialize.

@loicmathieu
Copy link
Contributor Author

@gunnarmorling right but there is two different fromJson method on the JSONB interface so if I only use a Type field the fromJson(InputStream, Type) will be used in both cases.
As it's an interface, we cannot be sure that both impl will be the same.

@gunnarmorling
Copy link
Contributor

gunnarmorling commented Feb 8, 2021 via email

@loicmathieu loicmathieu force-pushed the feat/kafka-deserialize-parameterized-type branch from 373a59a to bc4c70d Compare February 8, 2021 10:58
@loicmathieu
Copy link
Contributor Author

So I'd go for that, but it's your call, I'm not hung up on it :)

@gunnarmorling the simplier the better :)

I updated the PR accordingly

@loicmathieu
Copy link
Contributor Author

@gsmet all native jobs failed, I let you relaunch CI when it's OK for you, hope it will be able to make it inside 1.2

@gsmet gsmet force-pushed the feat/kafka-deserialize-parameterized-type branch from bc4c70d to 24e34fc Compare February 10, 2021 14:25
@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Feb 10, 2021
@gsmet
Copy link
Member

gsmet commented Feb 10, 2021

I rebased.

@gsmet gsmet merged commit 46f518d into quarkusio:master Feb 18, 2021
@quarkus-bot quarkus-bot bot added this to the 1.13 - master milestone Feb 18, 2021
@gsmet gsmet modified the milestones: 1.13 - master, 1.12.1.Final Feb 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kafka triage/waiting-for-ci Ready to merge when CI successfully finishes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants