Implement Jackson3 support and maintain Jackson2 support#117
Implement Jackson3 support and maintain Jackson2 support#117smals-mavh wants to merge 9 commits intoOpenAPITools:masterfrom
Conversation
There was a problem hiding this comment.
1 issue found across 36 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="pom.xml">
<violation number="1" location="pom.xml:40">
P2: Jackson 3 requires Java 17+, but the build is pinned to Java 8. Adding the Jackson 3 BOM/tools.jackson dependencies will make builds fail with class file version incompatibility unless the toolchain is upgraded or Jackson 3 is isolated.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Some extra context:
|
|
I'm waiting this PR to be released because I can't migrate to spring 4 without breaking my app behavior... 💯 |
…upport' into implement-jackson3-with-jackson2-support # Conflicts: # .github/workflows/maven_release.yml # .github/workflows/maven_test.yml
|
@wing328 is there any chance that this PR gets merged? This could be useful to add it once |
|
I'm waiting :) |
|
Is there an ETA for releasing this PR? That will be very helpful for the migration to Spring Boot 4. Thanks! |
|
I'll try to get it merged this week and cut a release |
|
please review the build failure when you've time |
|
I just tested the PR and I found a regression. The following code doesn't work - name is always undefined even if a The following code does work: |
…with-jackson2-support # Conflicts: # pom.xml
|
I updated the PR to fix the integration tests (working together with @smals-mavh). The jackson dependencies aren't transitively added anymore, either jackson 2 or 3 has to be added explicitly. We still have to investigate the regression reported by @mikomarrache . For projects using a module path, there's an issue: the SPI mechanism with We could support SPI from Java 25 onwards using multi-release jar, but I don't see a way to support SPI for Java 17 to 24 without making jackson 2 and 3 a mandatory dependency. Any preference on how to go forward with this or other ideas? |
|
@mikomarrache, could you tell a bit more about your setup. I was unable to reproduce. This is also more or less tested here: https://github.com/smals-mavh/jackson-databind-nullable/blob/implement-jackson3-with-jackson2-support/src/test/java/org/openapitools/jackson/nullable/JsonNullableSimpleTest.java#L134 Could you tell us more about the setup / share a reproducer? |
|
Sorry, my bad, I didn't explain the regression correctly. You can use this test: |
|
@mikomarrache , good catch! I don't think this is really a regression in the json-nullable lib, but more a behavioral change in Jackson 3. Note that the jackson 2 test you provided still passes, but to let it work on jackson 3 I suppose we have two options. But to be seen how future proof this will be.
I don't think this is an issue for this library, but might be an annoying behavioral change in Jackson 3. |
Summary by cubic
Adds Jackson 3 support alongside Jackson 2 so JsonNullable works on both runtimes. Also adds a Java 17 module and updates CI to JDK 17.
New Features
Refactors
Written for commit 460051d. Summary will update on new commits.