feat(java-sdk): use common streaming utils, reverse sync streaming API layer #668
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR adds Java streaming API support by introducing template files for streaming response handling. Three new classes are defined: Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant StreamedListObjectsApi
participant BaseStreamingApi
participant HttpClient
participant Consumer
Client->>StreamedListObjectsApi: streamedListObjects(storeId, body, consumer)
StreamedListObjectsApi->>StreamedListObjectsApi: validateParameters()
StreamedListObjectsApi->>StreamedListObjectsApi: buildHttpRequest(POST, path, body)
StreamedListObjectsApi->>BaseStreamingApi: processStreamingResponse(request, consumer, errorConsumer)
BaseStreamingApi->>HttpClient: send(httpRequest)
HttpClient-->>BaseStreamingApi: response stream
loop For each line
BaseStreamingApi->>BaseStreamingApi: processLine(line)
BaseStreamingApi->>BaseStreamingApi: deserialize to StreamResult<T>
alt Error in StreamResult
BaseStreamingApi->>BaseStreamingApi: invoke errorConsumer(error)
else Success
BaseStreamingApi->>Consumer: accept(result)
end
end
BaseStreamingApi-->>Client: CompletableFuture<Void>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
config/clients/java/template/libraries/native/BaseStreamingApi.mustache (1)
131-137: Redundant null check.The
result != nullcheck on line 134 is unnecessary sincestreamResult.getResult() != nullwas already verified on line 131.} else if (streamResult.getResult() != null) { // Deliver the response object to the consumer - T result = streamResult.getResult(); - if (result != null) { - consumer.accept(result); - } + consumer.accept(streamResult.getResult()); }config/clients/java/template/libraries/native/StreamedListObjectsApi.mustache (2)
11-12: Unused imports.
StatusandStreamResultare imported but not used in this class—they're only referenced in the parentBaseStreamingApi.-import {{modelPackage}}.Status; -import {{modelPackage}}.StreamResult;
133-134: Redundant.toString()on String parameter.
storeIdis already aString, so.toString()is unnecessary.String path = "/stores/{store_id}/streamed-list-objects" - .replace("{store_id}", StringUtil.urlEncode(storeId.toString())); + .replace("{store_id}", StringUtil.urlEncode(storeId));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
config/clients/java/config.overrides.json(1 hunks)config/clients/java/template/libraries/native/BaseStreamingApi.mustache(1 hunks)config/clients/java/template/libraries/native/StreamResult.mustache(1 hunks)config/clients/java/template/libraries/native/StreamedListObjectsApi.mustache(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
config/**/*.mustache
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Validate mustache syntax and variable references across all template files, including CHANGELOG.md.mustache
Files:
config/clients/java/template/libraries/native/BaseStreamingApi.mustacheconfig/clients/java/template/libraries/native/StreamedListObjectsApi.mustacheconfig/clients/java/template/libraries/native/StreamResult.mustache
config/**/*.{json,mustache}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Never hardcode API keys or credentials in configuration or template files
Files:
config/clients/java/template/libraries/native/BaseStreamingApi.mustacheconfig/clients/java/template/libraries/native/StreamedListObjectsApi.mustacheconfig/clients/java/config.overrides.jsonconfig/clients/java/template/libraries/native/StreamResult.mustache
config/clients/*/config.overrides.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
config/clients/*/config.overrides.json: Always update the packageVersion in each language-specific config.overrides.json when making version changes
Maintain FOSSA compliance notice IDs in each language’s config.overrides.json
Files:
config/clients/java/config.overrides.json
config/{common/config.base.json,clients/*/config.overrides.json}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Ensure consistent versioning across base and language override configuration files to avoid version conflicts
Files:
config/clients/java/config.overrides.json
🧠 Learnings (3)
📚 Learning: 2025-08-12T14:18:58.827Z
Learnt from: rhamzeh
Repo: openfga/sdk-generator PR: 573
File: config/clients/java/config.overrides.json:184-191
Timestamp: 2025-08-12T14:18:58.827Z
Learning: In the OpenFGA SDK generator, templates are inherited from the upstream OpenAPI Generator (openapitools/openapi-generator-cli). Only custom or modified templates need to be explicitly defined in the config overrides files. Base templates like `settings.gradle.mustache` are provided by the upstream generator and don't require override mappings.
Applied to files:
config/clients/java/config.overrides.json
📚 Learning: 2025-09-04T17:35:34.111Z
Learnt from: CR
Repo: openfga/sdk-generator PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-04T17:35:34.111Z
Learning: Applies to config/clients/*/config.overrides.json : Maintain FOSSA compliance notice IDs in each language’s config.overrides.json
Applied to files:
config/clients/java/config.overrides.json
📚 Learning: 2025-09-04T17:35:34.111Z
Learnt from: CR
Repo: openfga/sdk-generator PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-04T17:35:34.111Z
Learning: Applies to scripts/build_client.sh : Implement and maintain OpenAPI transformations in scripts/build_client.sh (remove streaming endpoints, rename Object to FgaObject, strip v1. prefixes) when API schemas change
Applied to files:
config/clients/java/config.overrides.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Agent
- GitHub Check: build-and-test-dotnet-sdk
- GitHub Check: build-and-test-go-sdk
- GitHub Check: build-and-test-java-sdk
🔇 Additional comments (5)
config/clients/java/template/libraries/native/StreamResult.mustache (1)
1-111: LGTM! Well-structured streaming result wrapper.The generic
StreamResult<T>class is properly designed with:
- Correct Jackson annotations for JSON serialization
- Fluent setter pattern consistent with other SDK model classes
- Proper
equals,hashCode, andtoStringimplementationsThe generic type handling is correctly delegated to
TypeReferencein the consumingBaseStreamingApiclass.config/clients/java/template/libraries/native/BaseStreamingApi.mustache (1)
59-108: Solid async streaming implementation.The
processStreamingResponsemethod correctly:
- Uses
try-with-resourcesonStream<String>for proper resource cleanup- Unwraps
CompletionExceptionto expose the actual cause- Notifies
errorConsumerwhile also keeping theCompletableFuturein a failed stateThis dual error-handling approach (callback + future state) gives callers flexibility.
config/clients/java/template/libraries/native/StreamedListObjectsApi.mustache (1)
44-117: Clean API overload pattern.The four public overloads provide good ergonomics for callers—allowing optional
errorConsumerandconfigurationOverridewithout requiring null parameters. Delegation to the private helper method keeps the implementation DRY.config/clients/java/config.overrides.json (2)
6-6: Consider version bump for new streaming API feature.This PR introduces new streaming API classes. Depending on your release process, you may want to bump
packageVersion(currently0.9.2) to reflect the new functionality.Based on coding guidelines, version updates should be considered when making feature changes.
40-52: Based on my verification, I can see that:
- ✅ All three template files exist at the correct paths
- ✅ Template mappings are properly configured with correct destination paths
⚠️ The packageVersion (0.9.2) was NOT updated, but this is a feature addition ("feat:" commit)The original review comment's concern about version consistency is valid. Per the commit message "feat(java-sdk): use common streaming utils" and coding guidelines, a version bump should be considered.
Here's the rewritten review comment:
Update packageVersion for new streaming API support.
Template mappings for StreamResult, BaseStreamingApi, and StreamedListObjectsApi are correctly configured and files exist at the expected locations. However, per coding guidelines, the
packageVersioninconfig/clients/java/config.overrides.jsonshould be updated from 0.9.2 to reflect this feature addition (currently Python SDK is at 0.9.7, indicating independent versioning per language). Consider bumping to 0.9.3 for this minor feature."packageVersion": "0.9.3"⛔ Skipped due to learnings
Learnt from: rhamzeh Repo: openfga/sdk-generator PR: 573 File: config/clients/java/config.overrides.json:184-191 Timestamp: 2025-08-12T14:18:58.827Z Learning: In the OpenFGA SDK generator, templates are inherited from the upstream OpenAPI Generator (openapitools/openapi-generator-cli). Only custom or modified templates need to be explicitly defined in the config overrides files. Base templates like `settings.gradle.mustache` are provided by the upstream generator and don't require override mappings.
There was a problem hiding this comment.
Pull request overview
This PR adds streaming API support to the Java SDK by introducing common streaming utilities and a new API layer for the streamedListObjects endpoint. The implementation follows the pattern established in the Go SDK, using a generic StreamResult wrapper and a base class for shared streaming functionality.
Key Changes
- Introduced
BaseStreamingApiabstract class providing reusable streaming logic with async CompletableFuture-based processing - Added
StreamResult<T>generic model for deserializing streaming responses that contain either results or errors - Implemented
StreamedListObjectsApiextending the base class for the streaming list objects endpoint
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| config/clients/java/template/libraries/native/BaseStreamingApi.mustache | Base class providing common streaming functionality with HTTP client integration, line-by-line JSON parsing, and error handling |
| config/clients/java/template/libraries/native/StreamResult.mustache | Generic wrapper model for streaming responses with result/error union type, following Go SDK pattern |
| config/clients/java/template/libraries/native/StreamedListObjectsApi.mustache | API implementation for streaming list objects with multiple overloads supporting configuration and error handling |
| config/clients/java/config.overrides.json | Configuration mappings for the three new template files to their destination paths in the generated SDK |
config/clients/java/template/libraries/native/StreamedListObjectsApi.mustache
Outdated
Show resolved
Hide resolved
Description
generates openfga/java-sdk#261
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
main