Add Jackson 3 annotation support - #1740
Conversation
|
Have you tried to build/test it locally using JDK 8 ? |
|
I just tested and build on jdk8 is broken (because of Jackson 3 classes being compiled with target level 17) . It makes sense, as Jackson 3 has raised its JDK baseline to 17 (see item 1 in Jackson 3 Migration guide ; there are links to the voting process / decision of uplifting this baseline ). There's no way for jsonschema2pojo to support Jackson 3 without also uplifting baseline jdk to 17 I think. |
|
I've just seen your work for uplifting baseline jdk to 17, great stuff! I'll rebase my PR right away |
6085102 to
36cb2c5
Compare
|
Rebased it is :) |
unkish
left a comment
There was a problem hiding this comment.
Additionally seems like mentioning jackson3 is missing at least in following places:
GenerationConfigREADME.mdbuild.gradlefiles inexample/android/**
|
|
||
| @Override | ||
| protected void addJsonDeserializeAnnotation(JFieldVar field) { | ||
| field.annotate(JsonDeserialize.class).param("as", LinkedHashSet.class); |
There was a problem hiding this comment.
Perhaps the method should just return annotation class to apply ?
There was a problem hiding this comment.
I don't think this is viable, the problem is that the JsonDeserialize annotation don't have any common ancestor in Jackson 2 / Jackson 3 (they're in different packages now). Returning the bare class forces you to deal with casts from different packages, just the kind of details that you want delegated in the subclasses
There was a problem hiding this comment.
the problem is that the JsonDeserialize annotation don't have any common ancestor in Jackson 2 / Jackson 3
Not really a problem IMO
- annotations can't have any common ancestor other than implicit superinterface
java.lang.annotation.Annotation JFieldVar::annotatedoesn't care much if there's common ancestor or not
I don't think this is viable
Both overrides applying same logic with different argument just doesn't feel right to me.
I'll leave this "open" for someone else to express their opinion
|
Great addition @diegogranados, thank you! And thanks for reviewing @unkish. Just a bit of tidying to do and we can merge. |
|
Thank you guys for the guidance and support! I'm far-from-laptops for a few
days, I'll rework your suggestions as soon as I'm back
Merry Christmas btw!
El mar, 23 dic 2025, 22:29, Joe Littlejohn ***@***.***>
escribió:
… *joelittlejohn* left a comment (joelittlejohn/jsonschema2pojo#1740)
<#1740 (comment)>
Great addition @diegogranados <https://github.com/diegogranados>, thank
you! And thanks for reviewing @unkish <https://github.com/unkish>. Just a
bit of tidying to do and we can merge.
—
Reply to this email directly, view it on GitHub
<#1740 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFICAQM5UUHVSXBZX5YPQH34DGX2PAVCNFSM6AAAAACNZTVHZ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTMOBYGAYTGOBXGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
36cb2c5 to
5915dbc
Compare
|
Thanks a lot for the detailed review @unkish. I pushed an update |
unkish
left a comment
There was a problem hiding this comment.
jsonschema2pojo-gradle-plugin\README.md seems to be missing mentioning of jackson3
|
|
||
| @Override | ||
| protected void addJsonDeserializeAnnotation(JFieldVar field) { | ||
| field.annotate(JsonDeserialize.class).param("as", LinkedHashSet.class); |
There was a problem hiding this comment.
the problem is that the JsonDeserialize annotation don't have any common ancestor in Jackson 2 / Jackson 3
Not really a problem IMO
- annotations can't have any common ancestor other than implicit superinterface
java.lang.annotation.Annotation JFieldVar::annotatedoesn't care much if there's common ancestor or not
I don't think this is viable
Both overrides applying same logic with different argument just doesn't feel right to me.
I'll leave this "open" for someone else to express their opinion
5915dbc to
4e4b8ed
Compare
|
@unkish, I understand your proposal now for the Jackson2/3 annotator. It is implemented, I hope this is what you meant. |
4e4b8ed to
6526988
Compare
|
@unkish The Android action is also working in my fork (I think I understand the problem: you can have one project configured with the jackson2 databind or with the jackson3 databind dependency, but not both simultaneously). I was hesitant about leaving the dependencies commented for this in jsonschema2pojo-gradle-plugin/example/android/app/build.gradle + ../lib.gradle, but at the end I preferred to leave it clean, as this is just a concrete example. Just to learn: I tried to run the action locally before pushing doing the same preparation steps that the task does, but the gradle build was failing for me with error Any chance you know the cause? I configured the same Android Sdk version that the action downloaded and the same Gradle version (8.9) |
|
@diegogranados It looks like the Android action is working okay (see checks, I just approved these to run). |
The change introduces a new Jackson3 annotator, that modifies the existing Jackson-2 annotator only for annotations from jackson-databind package, that needs to be changed to the new tools.jackson package in Jackson 3.x The default annotator continues to be Jackson 2
6526988 to
ad07174
Compare
|
Great work and really fantastic to have Jackson 3 support, thank you @diegogranados and @unkish! |
|
Thank you @unkish, @joelittlejohn! It has been both a joy and an honour working with you on this. Looking forward to have this released! |
|
@diegogranados I think we should include more annotations in the Jackson3Annotator. I would expect that all annotations come from Jackson 3 if using the Jackson3Annotator, no? |
|
Sorry, I understand more about the split between the two now after consulting the migration guide. Ignore me! |
The change introduces a new Jackson3 annotator, that modifies the existing Jackson-2 annotator only for annotations from jackson-databind package, that needs to be changed to the new tools.jackson package in Jackson 3.x
The default annotator continues to be Jackson 2