feat!: migrate json serialization to jackson 3 - #392
Adrastopoulos wants to merge 4 commits into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (34)
💤 Files with no reviewable changes (5)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe SDK migrates JSON serialization from Jackson 2 to Jackson 3. It removes deprecated ChangesJackson 3 migration
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant Application
participant OpenFgaClient
participant ApiClient
participant JsonSerializer
Application->>OpenFgaClient: request streamingApiExecutor(SdkTypeToken)
OpenFgaClient->>ApiClient: execute request
ApiClient->>JsonSerializer: deserialize response
JsonSerializer-->>OpenFgaClient: typed StreamResult
OpenFgaClient-->>Application: streaming response
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 60 functions across 23 files. (6 skipped: 6 unsupported.) ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (39.34%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #392 +/- ##
=========================================
Coverage 39.34% 39.34%
+ Complexity 1336 1331 -5
=========================================
Files 202 202
Lines 7791 7757 -34
Branches 912 912
=========================================
- Hits 3065 3052 -13
+ Misses 4579 4549 -30
- Partials 147 156 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Remaining README examples reference removed Jackson 2 APIs and no longer compile with the new dependency scopes.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
What changed in this PR
Migrates the SDK’s internal JSON serialization from Jackson 2 to Jackson 3 while keeping public serialization library-neutral.
Changes:
- Adds the Jackson 3 serializer and preserves wire compatibility.
- Removes deprecated Jackson 2 mapper and
TypeReferenceAPIs. - Updates dependencies, tests, examples, and migration documentation.
| File | Description |
|---|---|
src/test/java/dev/openfga/sdk/TestJsonSerializer.java |
Adds a Jackson 3 test serializer. |
src/test/java/dev/openfga/sdk/LegacyStreamingApiTest.java |
Removes legacy Jackson 2 streaming tests. |
src/test/java/dev/openfga/sdk/api/StreamingApiTest.java |
Tests streaming through SDK serialization APIs. |
src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java |
Uses the test serializer. |
src/test/java/dev/openfga/sdk/api/client/StreamingApiExecutorTest.java |
Removes TypeReference compatibility coverage. |
src/test/java/dev/openfga/sdk/api/client/StreamedListObjectsTest.java |
Uses the default serializer. |
src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java |
Replaces mapper-based client construction. |
src/test/java/dev/openfga/sdk/api/client/OpenFgaClientHeadersTest.java |
Uses the test serializer. |
src/test/java/dev/openfga/sdk/api/client/Jackson3JsonSerializerTest.java |
Covers Jackson 3 compatibility and failures. |
src/test/java/dev/openfga/sdk/api/client/Jackson2JsonSerializerTest.java |
Removes Jackson 2 serializer tests. |
src/test/java/dev/openfga/sdk/api/client/ApiClientTest.java |
Tests custom serializer configuration. |
src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java |
Uses the default serializer. |
src/test-integration/java/dev/openfga/sdk/example/Example1.java |
Migrates integration parsing to Jackson 3. |
src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java |
Migrates error tests to Jackson 3. |
src/test-integration/java/dev/openfga/sdk/api/OpenFgaApiIntegrationTest.java |
Migrates mapper and exception APIs. |
src/test-integration/java/dev/openfga/sdk/api/client/OpenFgaClientIntegrationTest.java |
Migrates client integration parsing. |
src/test-integration/java/dev/openfga/sdk/api/client/ApiExecutorIntegrationTest.java |
Migrates executor integration parsing. |
src/main/java/dev/openfga/sdk/api/client/StreamingApiExecutor.java |
Removes the Jackson TypeReference constructor. |
src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java |
Removes the TypeReference overload. |
src/main/java/dev/openfga/sdk/api/client/JsonSerializer.java |
Selects Jackson 3 and documents its contract. |
src/main/java/dev/openfga/sdk/api/client/Jackson3JsonSerializer.java |
Implements default Jackson 3 serialization. |
src/main/java/dev/openfga/sdk/api/client/Jackson2JsonSerializer.java |
Removes the Jackson 2 implementation. |
src/main/java/dev/openfga/sdk/api/client/ApiClient.java |
Removes deprecated mapper APIs. |
src/main/java/dev/openfga/sdk/api/BaseStreamingApi.java |
Removes Jackson compatibility fields and constructors. |
examples/streamed-list-objects/src/main/java/dev/openfga/sdk/example/StreamedListObjectsExample.java |
Migrates example parsing to Jackson 3. |
examples/streamed-list-objects/build.gradle |
Updates example dependencies. |
examples/basic-examples/src/main/kotlin/dev/openfga/sdk/example/KotlinExample1.kt |
Migrates Kotlin parsing to Jackson 3. |
examples/basic-examples/src/main/java/dev/openfga/sdk/example/Example1.java |
Migrates Java parsing to Jackson 3. |
examples/basic-examples/build.gradle |
Updates basic example dependencies. |
examples/api-executor/src/main/java/dev/openfga/sdk/example/StreamingApiExecutorExample.java |
Replaces TypeReference with SdkTypeToken. |
examples/api-executor/src/main/java/dev/openfga/sdk/example/ApiExecutorExample.java |
Applies formatting cleanup. |
examples/api-executor/build.gradle |
Updates executor example dependencies. |
docs/ApiExecutor.md |
Documents SdkTypeToken streaming. |
CHANGELOG.md |
Adds migration guidance. |
build.gradle |
Moves production dependencies to Jackson 3. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@SoulPancake any insight into the failing snyk security test? Otherwise this is ready! |
| @@ -1,5 +1,36 @@ | |||
| # Changelog | |||
|
|
|||
There was a problem hiding this comment.
Hey @Adrastopoulos
No need to update the changelog right now as we do not maintain the unreleased section anymore, but we can add it in the description using this https://github.com/openfga/mapper/blob/b38530aff3e280b9c347bec7dddd25380944465b/RELEASING.md?plain=1#L41 so that modifies the changelog which gets autogenerated on the next release PR using release-please and we can also refine the language we use over there to reflect in the release notes.
Description
Moves the SDK to Jackson 3 after the serialization bridge. Request, response, error and streaming JSON handling stay behind the SDK-owned API.
What problem is being solved?
The bridge still uses Jackson 2. The next major needs to switch the implementation without putting Jackson types back into the public API.
How is it being solved?
Jackson3JsonSerializeruses the existing mapper configuration on Jackson 3. The deprecated mapper andTypeReferenceAPIs are removed; callers useJsonSerializerandSdkTypeTokeninstead.This is for the next major. The Jackson 2 bridge must ship before this lands. Release versions are unchanged.
What changes are made to solve it?
References
Test plan
./gradlew build: 566 unit tests passed.Jackson3JsonSerializerhas 26/26 lines and 7/7 methods covered../gradlew test-integration: 60 passed, one existing skip. Used the local Colima socket.Review Checklist
Summary by CodeRabbit
Changed
SdkTypeTokeninstead of JacksonTypeReference.Migration
TypeReferenceintegrations must migrate to the serializer and type-token APIs described in the updated documentation.