feat!: migrate json serialization to jackson 3 - #392
Open
Adrastopoulos wants to merge 3 commits into
Open
Adrastopoulos wants to merge 3 commits into
Adrastopoulos wants to merge 3 commits into
Conversation
4 tasks
Contributor
|
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.33%) 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.33% -0.01%
+ Complexity 1336 1330 -6
============================================
Files 202 202
Lines 7791 7757 -34
Branches 912 912
============================================
- Hits 3065 3051 -14
+ Misses 4579 4549 -30
- Partials 147 157 +10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
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.
| ```java | ||
| // Hypothetical endpoint whose response wraps a generic Page<Item> | ||
| TypeReference<StreamResult<Page<Item>>> typeRef = new TypeReference<StreamResult<Page<Item>>>() {}; | ||
| SdkTypeToken<StreamResult<Page<Item>>> type = new SdkTypeToken<StreamResult<Page<Item>>>() {}; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.Check status
Local Snyk scans found no vulnerable paths in the SDK production runtime. The root broad scans found identical advisories and dependency paths on the base and PR.
The two DSL examples retain a Jackson 2 BOM for
openfga-languagealongside Jackson 3. Their OpenTelemetry BOM now matches the SDK. Each example dropped from 16 Snyk findings to three existing ANTLR findings, for which Snyk lists no fix.Both examples built and ran against OpenFGA. The executor example streamed 200 objects with each overload; the streaming example returned 2,000 objects. The hosted Snyk report remains inaccessible, so these results do not establish its exact failure cause.
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.