-
Notifications
You must be signed in to change notification settings - Fork 21
feat: Streamed list objects SDK support #244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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. WalkthroughThe PR introduces streaming support for listing objects in the OpenFGA Java SDK. It adds two new model classes, a new streaming API endpoint handler, helper classes for NDJSON response parsing, updated client methods, and a complete example project demonstrating the streaming workflow. Changes
Sequence DiagramsequenceDiagram
participant Client
participant OpenFgaClient
participant StreamedListObjectsApi
participant HttpClient as HTTP Client
participant Server
participant Iterator as StreamedResponseIterator
Client->>OpenFgaClient: streamedListObjects(request)
OpenFgaClient->>StreamedListObjectsApi: streamedListObjects(storeId, body, config)
StreamedListObjectsApi->>HttpClient: POST /streamed-list-objects
HttpClient->>Server: Send request
Server-->>HttpClient: NDJSON stream (line-delimited)
HttpClient-->>StreamedListObjectsApi: ApiResponse\<StreamingResponseString\>
StreamedListObjectsApi-->>OpenFgaClient: CompletableFuture\<ApiResponse\>
OpenFgaClient->>Iterator: new StreamedResponseIterator(ndjsonString)
OpenFgaClient-->>Client: CompletableFuture\<Stream\<StreamedListObjectsResponse\>\>
Client->>Iterator: iterate via stream
Iterator-->>Client: StreamedListObjectsResponse items
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 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 |
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (36.83%) 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 #244 +/- ##
============================================
+ Coverage 36.39% 36.83% +0.43%
- Complexity 1141 1184 +43
============================================
Files 188 195 +7
Lines 7185 7464 +279
Branches 822 853 +31
============================================
+ Hits 2615 2749 +134
- Misses 4465 4591 +126
- Partials 105 124 +19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements support for the OpenFGA Streamed ListObjects API, which returns results as a stream rather than collecting all results before responding. The implementation allows clients to process results as they arrive, which is beneficial for large result sets.
Key changes:
- Added new model classes
StreamedListObjectsResponseandStreamResultOfStreamedListObjectsResponseto handle streaming responses - Implemented
StreamedListObjectsApito handle the streaming endpoint - Created
StreamedResponseIteratorto parse NDJSON streaming responses - Added
streamedListObjects()method toOpenFgaClientthat returns aStream<StreamedListObjectsResponse>
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| StreamedListObjectsResponse.java | Model class representing individual streamed response items |
| StreamResultOfStreamedListObjectsResponse.java | Wrapper model for streaming results with error handling support |
| StreamingResponseString.java | Marker class for raw streaming responses to bypass standard deserialization |
| StreamedResponseIterator.java | Iterator implementation for parsing NDJSON streaming responses |
| OpenFgaClient.java | Added streamedListObjects methods to the client API |
| HttpRequestAttempt.java | Added special handling for StreamingResponseString responses |
| StreamedListObjectsApi.java | New API handler for the streaming endpoint |
| OpenFgaApi.java | Added streamedListObjects methods to the generated API |
| OpenFgaClientTest.java | Added comprehensive tests for streaming functionality |
| examples/streamed-list-objects/* | Complete working example demonstrating streaming API usage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/dev/openfga/sdk/api/client/StreamedResponseIterator.java
Outdated
Show resolved
Hide resolved
src/main/java/dev/openfga/sdk/api/client/StreamedResponseIterator.java
Outdated
Show resolved
Hide resolved
src/main/java/dev/openfga/sdk/api/model/StreamResultOfStreamedListObjectsResponse.java
Show resolved
Hide resolved
src/main/java/dev/openfga/sdk/api/model/StreamResultOfStreamedListObjectsResponse.java
Show resolved
Hide resolved
src/main/java/dev/openfga/sdk/api/model/StreamedListObjectsResponse.java
Show resolved
Hide resolved
src/main/java/dev/openfga/sdk/api/model/StreamedListObjectsResponse.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (1)
docs/OpenFgaApi.md (1)
2369-2380: Minor: Add blank lines around tables per markdown standards.Linting flags (MD058) indicate tables should be surrounded by blank lines. Add a blank line before line 2369 (the HTTP response table header) and before line 2454.
| **200** | A successful response.(streaming responses) | - | + | Status code | Description | Response headers | |-------------|-------------|------------------| | **200** | A successful response.(streaming responses) | - |Also applies to: 2454-2465
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
.openapi-generator/FILES(2 hunks)README.md(2 hunks)docs/OpenFgaApi.md(2 hunks)docs/StreamResultOfStreamedListObjectsResponse.md(1 hunks)docs/StreamedListObjectsResponse.md(1 hunks)examples/streamed-list-objects/.env.example(1 hunks)examples/streamed-list-objects/Makefile(1 hunks)examples/streamed-list-objects/README.md(1 hunks)examples/streamed-list-objects/build.gradle(1 hunks)examples/streamed-list-objects/gradle.properties(1 hunks)examples/streamed-list-objects/gradle/wrapper/gradle-wrapper.properties(1 hunks)examples/streamed-list-objects/gradlew(1 hunks)examples/streamed-list-objects/model.json(1 hunks)examples/streamed-list-objects/settings.gradle(1 hunks)examples/streamed-list-objects/src/main/java/dev/openfga/sdk/example/streamedlistobjects/StreamedListObjectsExample.java(1 hunks)src/main/java/dev/openfga/sdk/api/OpenFgaApi.java(2 hunks)src/main/java/dev/openfga/sdk/api/StreamedListObjectsApi.java(1 hunks)src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java(1 hunks)src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java(3 hunks)src/main/java/dev/openfga/sdk/api/client/StreamedResponseIterator.java(1 hunks)src/main/java/dev/openfga/sdk/api/client/StreamingResponseString.java(1 hunks)src/main/java/dev/openfga/sdk/api/model/StreamResultOfStreamedListObjectsResponse.java(1 hunks)src/main/java/dev/openfga/sdk/api/model/StreamedListObjectsResponse.java(1 hunks)src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/main/java/dev/openfga/sdk/api/StreamedListObjectsApi.java (10)
src/main/java/dev/openfga/sdk/util/Validation.java (1)
Validation(7-15)src/main/java/dev/openfga/sdk/api/client/ApiClient.java (1)
ApiClient(36-327)src/main/java/dev/openfga/sdk/api/client/ApiResponse.java (1)
ApiResponse(13-50)src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java (1)
HttpRequestAttempt(24-265)src/main/java/dev/openfga/sdk/api/client/StreamingResponseString.java (1)
StreamingResponseString(7-17)src/main/java/dev/openfga/sdk/api/configuration/Configuration.java (1)
Configuration(21-326)src/main/java/dev/openfga/sdk/errors/ApiException.java (1)
ApiException(6-94)src/main/java/dev/openfga/sdk/errors/FgaInvalidParameterException.java (1)
FgaInvalidParameterException(3-19)src/main/java/dev/openfga/sdk/telemetry/Attribute.java (1)
Attribute(6-26)src/main/java/dev/openfga/sdk/telemetry/Attributes.java (1)
Attributes(19-204)
src/main/java/dev/openfga/sdk/api/model/StreamedListObjectsResponse.java (1)
src/main/java/dev/openfga/sdk/api/model/StreamResultOfStreamedListObjectsResponse.java (1)
JsonPropertyOrder(24-168)
src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java (1)
src/main/java/dev/openfga/sdk/api/client/model/ClientTupleKey.java (1)
ClientTupleKey(9-69)
src/main/java/dev/openfga/sdk/api/OpenFgaApi.java (2)
src/main/java/dev/openfga/sdk/util/StringUtil.java (1)
StringUtil(12-51)src/main/java/dev/openfga/sdk/telemetry/Attributes.java (1)
Attributes(19-204)
src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java (1)
src/main/java/dev/openfga/sdk/constants/FgaConstants.java (1)
FgaConstants(19-104)
src/main/java/dev/openfga/sdk/api/model/StreamResultOfStreamedListObjectsResponse.java (1)
src/main/java/dev/openfga/sdk/api/model/StreamedListObjectsResponse.java (1)
JsonPropertyOrder(26-139)
examples/streamed-list-objects/src/main/java/dev/openfga/sdk/example/streamedlistobjects/StreamedListObjectsExample.java (1)
src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java (1)
OpenFgaClient(23-1385)
🪛 checkmake (0.2.2)
examples/streamed-list-objects/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
[warning] 1-1: Target "all" should be declared PHONY.
(phonydeclared)
🪛 dotenv-linter (4.0.0)
examples/streamed-list-objects/.env.example
[warning] 4-4: [UnorderedKey] The FGA_MODEL_ID key should go before the FGA_STORE_ID key
(UnorderedKey)
[warning] 9-9: [UnorderedKey] The FGA_API_AUDIENCE key should go before the FGA_CLIENT_ID key
(UnorderedKey)
[warning] 10-10: [UnorderedKey] The FGA_API_TOKEN_ISSUER key should go before the FGA_CLIENT_ID key
(UnorderedKey)
🪛 LanguageTool
examples/streamed-list-objects/README.md
[style] ~17-~17: To reduce wordiness, try specifying a number or using “many” or “numerous” instead.
Context: ...med ListObjects API when: - You expect a large number of results that would take a long time to ...
(LARGE_NUMBER_OF)
[style] ~20-~20: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...p after finding a certain number) - You want to avoid timeout issues with very large re...
(REP_WANT_TO_VB)
[style] ~20-~20: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...- You want to avoid timeout issues with very large result sets ## Prerequisites - Java 1...
(EN_WEAK_ADJECTIVE)
🪛 markdownlint-cli2 (0.18.1)
docs/OpenFgaApi.md
2370-2370: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
2455-2455: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
⏰ 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). (8)
- GitHub Check: CodeQL analysis (java)
- GitHub Check: Test and Build OpenFGA (17)
- GitHub Check: Test and Build OpenFGA (21)
- GitHub Check: Test and Build OpenFGA (11)
- GitHub Check: Analyze (java)
- GitHub Check: Test and Build OpenFGA (21)
- GitHub Check: Test and Build OpenFGA (17)
- GitHub Check: Test and Build OpenFGA (11)
🔇 Additional comments (11)
README.md (2)
1188-1188: LGTM! Documentation properly reflects the new streaming endpoint.The addition of the
streamedListObjectsendpoint to the API documentation is consistent with the streaming feature implementation.
1314-1316: LGTM! New model documentation links are properly added.The documentation correctly references the two new streaming-related models introduced in this PR.
docs/StreamedListObjectsResponse.md (1)
1-14: LGTM! Model documentation is clear and complete.The documentation properly describes the
StreamedListObjectsResponsemodel and its single_objectproperty.examples/streamed-list-objects/gradle.properties (1)
1-1: LGTM! Standard Gradle configuration.The language property is correctly set for the Java example project.
src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java (1)
218-223: LGTM! Clean implementation for streaming response handling.The special case for
StreamingResponseStringproperly bypasses JSON deserialization and returns the raw response body, enabling NDJSON streaming to be handled at a higher layer. The implementation follows the existing pattern forVoidresponses and the unchecked cast is safe given the type check.docs/StreamResultOfStreamedListObjectsResponse.md (1)
1-14: LGTM! Documentation clearly describes the streaming result wrapper.The documentation properly describes the
StreamResultOfStreamedListObjectsResponsemodel with its optionalresultanderrorproperties, following the standard documentation format.examples/streamed-list-objects/gradle/wrapper/gradle-wrapper.properties (1)
1-7: LGTM! Standard Gradle wrapper configuration.The Gradle wrapper properties are correctly configured with Gradle 8.6 and standard settings for distribution management and validation.
examples/streamed-list-objects/.env.example (1)
1-11: LGTM! Environment configuration is well-organized.The configuration file is logically grouped into required and optional sections with clear comments. The current organization is more intuitive than alphabetical ordering suggested by the linter.
examples/streamed-list-objects/model.json (1)
1-121: LGTM!The authorization model is well-structured and provides a comprehensive example for the streaming API. The schema demonstrates key OpenFGA concepts (relations, computed usersets, wildcards, and metadata) in a practical user-document-folder-group hierarchy suitable for streaming scenarios.
docs/OpenFgaApi.md (2)
35-36: ✓ Streamed endpoints added to API surface.The two new streaming methods are correctly added to the method summary table in alphabetical order, following the established pattern.
2306-2465: ✓ Comprehensive streaming endpoint documentation.The documentation for both
streamedListObjectsandstreamedListObjectsWithHttpInfofollows the established pattern and correctly notes the key streaming differences: lazy evaluation and no upper-bound result limits (except execution timeout).
| if (streamResult.getError() != null) { | ||
| // Handle error in stream - convert to exception | ||
| Status error = streamResult.getError(); | ||
| String errorMessage = String.format( | ||
| "Error in streaming response: code=%d, message=%s", | ||
| error.getCode(), error.getMessage() != null ? error.getMessage() : "Unknown error"); | ||
| pendingException = new RuntimeException(errorMessage); | ||
| hasNext = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against null error codes when formatting.
If the server sends an error object without a code (it’s optional in some responses), String.format(... "%d" ...) will throw a NullPointerException, hiding the original error. Please switch to a null-safe format, e.g. default to "unknown" when the code is missing.
🤖 Prompt for AI Agents
In src/main/java/dev/openfga/sdk/api/client/StreamedResponseIterator.java around
lines 52 to 59, the String.format uses "%d" with error.getCode() which can be
null and will throw; change to a null-safe formatting approach (use "%s" or
convert toString with a default) and default the code to "unknown" when null,
e.g. compute a codeStr = error.getCode() != null ?
String.valueOf(error.getCode()) : "unknown" and use that in the formatted
message so the exception message is always created without throwing; keep the
existing null-check for error.getMessage() and set pendingException to the new
RuntimeException with the null-safe message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java:1172
- This InputStreamReader is not always closed on method exit.
new java.io.InputStreamReader(srb.getBody(), java.nio.charset.StandardCharsets.UTF_8));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/dev/openfga/sdk/api/client/StreamedResponseIterator.java
Outdated
Show resolved
Hide resolved
...ts/src/main/java/dev/openfga/sdk/example/streamedlistobjects/StreamedListObjectsExample.java
Outdated
Show resolved
Hide resolved
| if (!HttpStatusCode.isSuccessful(statusCode)) { | ||
| try { | ||
| String bodyStr = new String(response.body().readAllBytes(), StandardCharsets.UTF_8); | ||
| HttpResponse<String> stringResponse = new HttpResponse<>() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of this anonymous inner class here, can we extract this to a static inner class?
| return processHttpResponse(response, retryNumber, previousError); | ||
| }) | ||
| .thenCompose(Function.identity()); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we extract the duplicate code introduced here by the branching?
| } | ||
|
|
||
| // Special handling for streaming responses - don't deserialize, just wrap the raw string | ||
| if (clazz == StreamingResponseString.class) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed, or is it here from earlier iteration of work?
| return stream.onClose(() -> { | ||
| try { | ||
| iterator.close(); | ||
| } catch (java.io.IOException ignore) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we be ignoring exceptions closing the iterator or response body? Seems like it could cause issues to swallow these errors. If we can recover we should, if not can we throw back to caller with applicable information? Or if there is good reason to swallow exceptions, let's add clear code comment explaining why.
| * This class is separate from the generated OpenFgaApi to avoid modifications to generated code. | ||
| */ | ||
| public class StreamedListObjectsApi { | ||
| private final Configuration configuration; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configuration is assigned but never read. Should it be merged with requestConfiguration when making request, or handled like other requests?
| StreamedResponseIterator iterator = | ||
| new StreamedResponseIterator(reader, apiClient.getObjectMapper()); | ||
| Stream<StreamedListObjectsResponse> stream = java.util.stream.StreamSupport.stream( | ||
| ((Iterable<StreamedListObjectsResponse>) () -> iterator).spliterator(), false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: do we need to cast to Iterable from a lambda to get a stream, or can we use any Spliterator functionality, like Spliterators.spliteratorUnknownSize or similar?
|
|
||
| if (!HttpStatusCode.isSuccessful(statusCode)) { | ||
| try { | ||
| String bodyStr = new String(response.body().readAllBytes(), StandardCharsets.UTF_8); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to ensure we close the resource stream from response.body() in the error condition handling here? I asked claude, it said
When
readAllBytes()succeeds: The stream is consumed but never closed before returning. WhenreadAllBytes()throwsIOException: The exception is caught but the stream is never closed before returning.
| * large result sets. | ||
| * | ||
| * @throws FgaInvalidParameterException When the Store ID is null, empty, or whitespace | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be very explicit about the need for callers to close the stream themselves. If the stream isn’t closed, the underlying HTTP connection and resources remain open, causing connection-pool exhaustion and potential memory or socket leaks until the server or client eventually times out.
|
Thank you for your comments and review, have accomodated those |
Description
for #167
What problem is being solved?
This adds StreamedListObjects endpoitn support in Java SDK.
How is it being solved?
The API layer for streaming is custom written for parsing NDJSON responses by line.
The client layer utilises this API layer for the client side API.
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
New Features
Documentation