-
Notifications
You must be signed in to change notification settings - Fork 14
feat: add StreamedListObjects support #156
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
base: main
Are you sure you want to change the base?
Conversation
- Add StreamedListObjects method returning IAsyncEnumerable<StreamedListObjectsResponse> - Implement NDJSON streaming parser in BaseClient - Add comprehensive test coverage (10 tests) matching JS SDK - Add example application demonstrating usage - Support all frameworks: netstandard2.0, net48, net8.0, net9.0 - Proper resource cleanup on early termination and cancellation
|
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 WalkthroughIntroduces streaming support to the OpenFGA .NET SDK via a new Changes
Sequence DiagramsequenceDiagram
participant Caller
participant OpenFgaClient
participant OpenFgaApi
participant ApiClient
participant BaseClient
participant HttpClient
participant NDJSON Stream
Caller->>OpenFgaClient: StreamedListObjects(request, options)
OpenFgaClient->>OpenFgaApi: StreamedListObjects(storeId, body, options, cancellationToken)
OpenFgaApi->>ApiClient: SendStreamingRequestAsync<ListObjectsRequest, StreamedListObjectsResponse>
ApiClient->>ApiClient: GetAuthenticationToken()
ApiClient->>BaseClient: SendStreamingRequestAsync<ListObjectsRequest, StreamedListObjectsResponse>
BaseClient->>HttpClient: SendAsync(request, HttpCompletionOption.ResponseHeadersRead)
HttpClient->>NDJSON Stream: Stream response body (NDJSON)
loop For each line in stream
NDJSON Stream->>BaseClient: Read line
alt Valid JSON with "result"
BaseClient->>BaseClient: Parse JSON, extract result
BaseClient->>ApiClient: Yield<StreamedListObjectsResponse>
ApiClient->>OpenFgaApi: Yield<StreamedListObjectsResponse>
OpenFgaApi->>OpenFgaClient: Yield<StreamedListObjectsResponse>
OpenFgaClient->>Caller: Yield<StreamedListObjectsResponse>
else Empty/Whitespace line
BaseClient->>BaseClient: Skip line
else Invalid JSON
BaseClient->>BaseClient: Skip line (log if applicable)
else Cancellation requested
BaseClient->>BaseClient: Break loop, dispose stream
else HTTP error status
BaseClient->>BaseClient: Throw ApiException via CreateSpecificExceptionAsync
end
end
Caller->>Caller: Complete iteration or break early
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes
Possibly related PRs
Suggested labels
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 |
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: 6
🧹 Nitpick comments (2)
example/StreamedListObjectsExample/README.md (1)
24-31: Switch to repo-relative build instructions.The absolute path under
/Users/danieljonathan/...is specific to your workstation and will confuse users cloning the repo elsewhere. Consider replacing it with a repo-relative sequence (e.g.,cd path/to/dotnet-sdkfrom the clone root) to keep the instructions portable.src/OpenFga.Sdk.Test/Client/StreamedListObjectsTests.cs (1)
19-426: Comprehensive test coverage with one minor gap.The test suite provides excellent coverage of the streaming functionality including happy paths, error conditions, cancellation, and resource cleanup. Test naming follows good conventions.
However, while
ContextualTuplesis tested (lines 164-204), theContextparameter (mentioned inListObjectsRequestper Client.cs line 293) is not explicitly verified in the request body. Consider adding a test similar toStreamedListObjects_WithContextualTuples_IncludesContextualTuplesInRequestfor theContextparameter.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
CHANGELOG.md(1 hunks)example/StreamedListObjectsExample/README.md(1 hunks)example/StreamedListObjectsExample/StreamedListObjectsExample.cs(1 hunks)example/StreamedListObjectsExample/StreamedListObjectsExample.csproj(1 hunks)src/OpenFga.Sdk.Test/ApiClient/StreamingTests.cs(1 hunks)src/OpenFga.Sdk.Test/Client/StreamedListObjectsTests.cs(1 hunks)src/OpenFga.Sdk/Api/OpenFgaApi.cs(2 hunks)src/OpenFga.Sdk/ApiClient/ApiClient.cs(2 hunks)src/OpenFga.Sdk/ApiClient/BaseClient.cs(2 hunks)src/OpenFga.Sdk/Client/Client.cs(2 hunks)src/OpenFga.Sdk/Model/StreamedListObjectsResponse.cs(1 hunks)src/OpenFga.Sdk/OpenFga.Sdk.csproj(1 hunks)src/OpenFga.Sdk/Telemetry/Attributes.cs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
src/OpenFga.Sdk/Client/Client.cs (9)
src/OpenFga.Sdk/Api/OpenFgaApi.cs (1)
IAsyncEnumerable(282-313)src/OpenFga.Sdk/ApiClient/ApiClient.cs (1)
IAsyncEnumerable(153-167)src/OpenFga.Sdk/ApiClient/BaseClient.cs (2)
IAsyncEnumerable(143-210)IAsyncEnumerable(222-230)src/OpenFga.Sdk/Model/StreamedListObjectsResponse.cs (2)
StreamedListObjectsResponse(39-46)StreamedListObjectsResponse(75-77)src/OpenFga.Sdk/Model/ListObjectsRequest.cs (2)
ListObjectsRequest(54-75)ListObjectsRequest(145-147)src/OpenFga.Sdk/Model/ContextualTupleKeys.cs (2)
ContextualTupleKeys(41-48)ContextualTupleKeys(77-79)src/OpenFga.Sdk/Client/Model/ClientTupleKey.cs (1)
TupleKey(50-50)src/OpenFga.Sdk/Client/Model/ClientTupleKeyWithoutCondition.cs (1)
TupleKey(39-39)src/OpenFga.Sdk/Model/TupleKey.cs (2)
TupleKey(44-62)TupleKey(115-117)
src/OpenFga.Sdk/ApiClient/ApiClient.cs (3)
src/OpenFga.Sdk/Api/OpenFgaApi.cs (1)
IAsyncEnumerable(282-313)src/OpenFga.Sdk/ApiClient/BaseClient.cs (2)
IAsyncEnumerable(143-210)IAsyncEnumerable(222-230)src/OpenFga.Sdk/ApiClient/RequestBuilder.cs (2)
RequestBuilder(15-107)RequestBuilder(16-19)
src/OpenFga.Sdk/Api/OpenFgaApi.cs (7)
src/OpenFga.Sdk/ApiClient/ApiClient.cs (1)
IAsyncEnumerable(153-167)src/OpenFga.Sdk/ApiClient/BaseClient.cs (2)
IAsyncEnumerable(143-210)IAsyncEnumerable(222-230)src/OpenFga.Sdk/Client/Client.cs (1)
IAsyncEnumerable(513-533)src/OpenFga.Sdk/Model/StreamedListObjectsResponse.cs (2)
StreamedListObjectsResponse(39-46)StreamedListObjectsResponse(75-77)src/OpenFga.Sdk/Model/ListObjectsRequest.cs (2)
ListObjectsRequest(54-75)ListObjectsRequest(145-147)src/OpenFga.Sdk/Exceptions/RequiredParamError.cs (3)
FgaRequiredParamError(6-23)FgaRequiredParamError(11-13)FgaRequiredParamError(20-22)src/OpenFga.Sdk/ApiClient/RequestBuilder.cs (2)
RequestBuilder(15-107)RequestBuilder(16-19)
src/OpenFga.Sdk/Model/StreamedListObjectsResponse.cs (1)
src/OpenFga.Sdk/Constants/FgaConstants.cs (1)
FgaConstants(19-156)
example/StreamedListObjectsExample/StreamedListObjectsExample.cs (4)
src/OpenFga.Sdk/Client/Client.cs (2)
OpenFgaClient(22-635)OpenFgaClient(26-33)src/OpenFga.Sdk/Client/Model/ClientCreateStoreRequest.cs (1)
ClientCreateStoreRequest(6-7)src/OpenFga.Sdk/Client/Model/ClientWriteAuthorizationModelRequest.cs (1)
ClientWriteAuthorizationModelRequest(6-7)src/OpenFga.Sdk/Client/Model/ClientListObjectsOptions.cs (1)
ClientListObjectsOptions(9-21)
src/OpenFga.Sdk/ApiClient/BaseClient.cs (3)
src/OpenFga.Sdk/ApiClient/ApiClient.cs (2)
IAsyncEnumerable(153-167)IDictionary(218-255)src/OpenFga.Sdk/ApiClient/RequestBuilder.cs (3)
HttpRequestMessage(100-106)RequestBuilder(15-107)RequestBuilder(16-19)src/OpenFga.Sdk/Exceptions/ApiException.cs (5)
ApiException(9-71)ApiException(13-15)ApiException(21-23)ApiException(32-34)ApiException(37-39)
src/OpenFga.Sdk.Test/ApiClient/StreamingTests.cs (3)
src/OpenFga.Sdk/ApiClient/ApiClient.cs (2)
ApiClient(19-271)ApiClient(31-57)src/OpenFga.Sdk/ApiClient/BaseClient.cs (2)
BaseClient(26-247)BaseClient(41-53)src/OpenFga.Sdk/Model/StreamedListObjectsResponse.cs (2)
StreamedListObjectsResponse(39-46)StreamedListObjectsResponse(75-77)
src/OpenFga.Sdk.Test/Client/StreamedListObjectsTests.cs (3)
src/OpenFga.Sdk.Test/ApiClient/StreamingTests.cs (1)
Mock(21-35)src/OpenFga.Sdk/Client/Client.cs (2)
OpenFgaClient(22-635)OpenFgaClient(26-33)src/OpenFga.Sdk/Client/Model/ClientListObjectsOptions.cs (1)
ClientListObjectsOptions(9-21)
⏰ 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: Socket Security: Pull Request Alerts
- GitHub Check: Analyze (csharp)
- GitHub Check: Test net8.0 on windows-latest
- GitHub Check: Test net48 on windows-latest
- GitHub Check: Test netcoreapp3.1 on windows-latest
- GitHub Check: Test net8.0 on ubuntu-latest
- GitHub Check: Test net8.0 on macos-latest
- GitHub Check: Test net9.0 on ubuntu-latest
🔇 Additional comments (1)
src/OpenFga.Sdk.Test/Client/StreamedListObjectsTests.cs (1)
27-51: LGTM: Well-designed helper methods.The helper methods are well-structured.
CreateMockHttpHandlerprovides flexible request validation via optional callback, andCreateNDJSONResponsecorrectly formats the test data. The use of Moq.Protected to mockSendAsyncis appropriate.
- Add StreamedListObjects method returning IAsyncEnumerable<StreamedListObjectsResponse> - Implement NDJSON streaming parser in BaseClient with proper cancellation support - Add comprehensive test coverage (22 tests) matching JS SDK - Add example application demonstrating usage - Support all frameworks: netstandard2.0, net48, net8.0, net9.0 - Proper resource cleanup on early termination and cancellation - Register cancellation token to unblock stalled reads immediately Closes #110
- Apply CodeQL suggestion for better resource management - Change to using var for disposeResponseRegistration - Remove manual Dispose() call in finally block
- Use using statement for cancellation registration (CodeQL suggestion) - Register cancellation token to unblock stalled reads immediately - Handle ObjectDisposedException and IOException during cancellation - Fix test exception type: expect FgaRequiredParamError for missing StoreId - Simplify request validation tests for .NET Framework 4.8 compatibility
rhamzeh
left a comment
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 you please make sure to add the README documentation too
- Replace hardcoded 'http://localhost:8080' with FgaConstants.TestApiUrl - Replace hardcoded 'http://localhost' with FgaConstants.TestApiUrl - Add using OpenFga.Sdk.Constants to test files - Use fully qualified type name to avoid namespace conflicts - Consistent with other tests in the codebase Addresses feedback from PR #156
- Simplify example from 199 to 101 lines - Demonstrate 2000 tuples to show streaming value at scale - Show progress indicators (first 3 and every 500th) - Remove complex demonstrations, focus on core functionality - Enhance README with detailed explanations and code examples - Document IAsyncEnumerable pattern, early break, cancellation - List benefits and performance considerations - Consistent with JS SDK example format Addresses feedback from PR #156
- Add Streamed List Objects documentation in Relationship Queries section - Explain two key differences from ListObjects - Include code example with IAsyncEnumerable pattern - Show consistency option usage - Consistent with JS SDK README format - Addresses missing documentation from PR #156
- Add custom headers test (matches JS SDK) - Add rate limit error test (429 handling) - Remove Arrange/Act/Assert comments for consistency with codebase style - Fix retry test (streaming can't retry mid-stream, so test error handling instead) - Complete test coverage parity with JS SDK Addresses feedback from PR #156
- Apply CodeRabbit suggestion for proper disposal - Use 'using var' for CancellationTokenSource - Remove extra comment line Addresses feedback from PR #156
- Switch from build-client to build-client-streamed - Aligns with Python SDK and JS SDK (PR #657) - Includes StreamedListObjects endpoint in generated SDK - No library parameter needed (not applicable to .NET generator) Related: - openfga/dotnet-sdk#156 - #76
example/StreamedListObjectsExample/StreamedListObjectsExample.cs
Outdated
Show resolved
Hide resolved
- Remove redundant response.Dispose() in BaseClient finally block (response already disposed via 'using var' declaration) - Add 'using var' for CancellationTokenSource in StreamedListObjectsTests - Add 'using var' for CancellationTokenSource in RetryHandlerTests - Add 'using var' for HttpClient in StreamedListObjectsTests - Fix HttpStatusCode.TooManyRequests for net48 compatibility (use (HttpStatusCode)429 instead) Addresses CodeQL, CodeRabbit, and Copilot feedback from PR review. All 277 tests passing on net9.0.
…CodeRabbit, and Copilot feedback from PR review. All 277 tests passing on net9.0.
- Add 'using var' for all HttpClient instances in StreamedListObjectsTests - Add 'using var' for all OpenFgaClient instances in StreamedListObjectsTests - Ensures proper disposal of all IDisposable resources in tests - Addresses CodeRabbit feedback for comprehensive resource management All 13 StreamedListObjects tests passing.
- Add '_' in iterator where 'response' is not used in StreamedListObjectsTests All 13 StreamedListObjects tests passing.
- Update authorization model to show owner/viewer/can_read pattern - Write tuples to base relations (owner, viewer) - Query computed relation (can_read = owner OR viewer) - Demonstrates OpenFGA's value: derived permissions from base relations - Update CHANGELOG to be brief with link to README (per @rhamzeh feedback) - Remove OPENFGA_LIST_OBJECTS_DEADLINE from manually-written docs - Clarify no pagination limit vs server timeout in all documentation - Add detailed explanation of computed relations in example README Addresses feedback from @aaguiarz and @SoulPancake: - openfga/js-sdk#280 (comment) - Shows why StreamedListObjects is valuable for computed relations All tests passing. Example builds successfully.
- Write tuples in batches of 100 (OpenFGA write limit) - Prevents validation error when writing 2000 tuples - Matches JS SDK batching pattern - Verified working with live OpenFGA server Example successfully streams 2000 objects from computed can_read relation.
- Sanitize error logging to avoid exposing config values - Handle FgaValidationError separately with generic message - Keep helpful connection refused hint - Log only exception type name for other errors Addresses CodeQL security finding: Clear-text logging of sensitive information Example verified working.
- Add ChunkedStreamContent helper class to simulate real-world chunked streaming - Add 10 new test cases covering partial NDJSON scenarios: - JSON objects split mid-line across multiple chunks - Multiple splits across many small reads - Very small chunks (single character) - Large objects (10KB+) - Empty lines and whitespace handling - Invalid JSON fragments (graceful skipping) - Missing result properties - Cancellation with chunked data - NDJSON without trailing newlines - All 20 streaming tests passing across all target frameworks This verifies the existing BaseClient.SendStreamingRequestAsync implementation correctly handles partial NDJSON streaming, matching the JS SDK behavior.
SoulPancake
left a comment
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.
Implement StreamedListObjects API for streaming large result sets without pagination limits. API Changes: - Add StreamedListObjects method returning IAsyncEnumerable<StreamedListObjectsResponse> - Full support for authorization model ID, consistency, contextual tuples, and context - Proper cancellation token support throughout streaming pipeline Code Quality: - Reorder imports (System first, then third-party) - Whitespace cleanup in ApiClient, BaseClient, and Client Documentation: - Add StreamedListObjects section to README with usage examples - Generate API documentation for new endpoint and models - Highlight key differences from standard ListObjects API Testing: All 287 tests pass including: - 13 StreamedListObjects client integration tests - 20 streaming infrastructure tests (NDJSON, chunking, cancellation) Benefits: - No 1000-object pagination limit - Memory efficient streaming - Early termination support - Idiomatic .NET with IAsyncEnumerable Resolves #110
Address CodeQL warning about inefficient ContainsKey usage.
Changes:
- Replace ContainsKey + indexer with TryGetValue in all model Equals methods
- Reduces dictionary lookups from 2 to 1 per key comparison
- Applied to 79 model files with AdditionalProperties
Before (inefficient):
input.AdditionalProperties.ContainsKey(kv.Key) &&
Equals(kv.Value, input.AdditionalProperties[kv.Key])
After (efficient):
input.AdditionalProperties.TryGetValue(kv.Key, out var inputValue) &&
Equals(kv.Value, inputValue)
Resolves CodeQL warning from PR #156.
Generated from updated sdk-generator template.
| ( | ||
| this.Result == input.Result || | ||
| (this.Result != null && | ||
| this.Result.Equals(input.Result)) | ||
| ) && | ||
| ( | ||
| this.Error == input.Error || | ||
| (this.Error != null && | ||
| this.Error.Equals(input.Error)) | ||
| ) | ||
| && (this.AdditionalProperties.Count == input.AdditionalProperties.Count && this.AdditionalProperties.All(kv => input.AdditionalProperties.TryGetValue(kv.Key, out var inputValue) && Equals(kv.Value, inputValue))); |
Check notice
Code scanning / CodeQL
Complex condition Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 27 days ago
To fix the issue, refactor the complex return statement in the Equals(StreamResultOfStreamedListObjectsResponse input) method (line 107) by breaking it into multiple local boolean variables, each expressing a single logical check. Then combine these named variables in the final return statement. This improves readability and maintainability without altering existing functionality.
Steps:
- Define local boolean variables for each significant comparison:
resultEquals: equality logic forResulterrorEquals: equality logic forErroradditionalPropertiesEquals: equality logic forAdditionalProperties
- Update the return statement to combine these variables.
- All changes are limited to the body of
Equals(StreamResultOfStreamedListObjectsResponse input), starting after the null-check.
-
Copy modified lines R106-R121
| @@ -103,18 +103,22 @@ | ||
| if (input == null) { | ||
| return false; | ||
| } | ||
| return | ||
| ( | ||
| this.Result == input.Result || | ||
| (this.Result != null && | ||
| this.Result.Equals(input.Result)) | ||
| ) && | ||
| ( | ||
| this.Error == input.Error || | ||
| (this.Error != null && | ||
| this.Error.Equals(input.Error)) | ||
| ) | ||
| && (this.AdditionalProperties.Count == input.AdditionalProperties.Count && this.AdditionalProperties.All(kv => input.AdditionalProperties.TryGetValue(kv.Key, out var inputValue) && Equals(kv.Value, inputValue))); | ||
|
|
||
| bool resultEquals = | ||
| this.Result == input.Result || | ||
| (this.Result != null && | ||
| this.Result.Equals(input.Result)); | ||
|
|
||
| bool errorEquals = | ||
| this.Error == input.Error || | ||
| (this.Error != null && | ||
| this.Error.Equals(input.Error)); | ||
|
|
||
| bool additionalPropertiesEquals = | ||
| this.AdditionalProperties.Count == input.AdditionalProperties.Count && | ||
| this.AdditionalProperties.All(kv => input.AdditionalProperties.TryGetValue(kv.Key, out var inputValue) && Equals(kv.Value, inputValue)); | ||
|
|
||
| return resultEquals && errorEquals && additionalPropertiesEquals; | ||
| } | ||
|
|
||
| /// <summary> |
Address two CodeQL warnings in all model Equals methods: 1. Inefficient ContainsKey usage (79 files) - Replace ContainsKey + indexer with TryGetValue - Reduces dictionary lookups from 2 to 1 per key - Improves performance for AdditionalProperties comparison 2. Equals should not apply "as" cast (79 files) - Add GetType() check before casting - Ensures proper equality behavior for subclasses - Prevents incorrect equality results All 287 tests pass on .NET 9.0. Resolves CodeQL warnings from PR #156. Generated from updated sdk-generator template.

StreamedListObjects Implementation Summary
Issue: openfga/dotnet-sdk#110
Overview
Implemented
StreamedListObjectsfunctionality in the .NET SDK, providing feature parity with the JavaScript and Python SDKs.What Was Implemented
Core Functionality
StreamedListObjectsmethod returningIAsyncEnumerable<StreamedListObjectsResponse>BaseClientListObjectsparameters (authorization model ID, consistency, contextual tuples, context)CancellationTokensupportCross-Framework Support
Files Changed
New Files
src/OpenFga.Sdk/Model/StreamedListObjectsResponse.cssrc/OpenFga.Sdk.Test/ApiClient/StreamingTests.cs(11 tests)src/OpenFga.Sdk.Test/Client/StreamedListObjectsTests.cs(11 tests)example/StreamedListObjectsExample/(complete example application)Modified Files
src/OpenFga.Sdk/ApiClient/BaseClient.cs- NDJSON streaming supportsrc/OpenFga.Sdk/ApiClient/ApiClient.cs- Streaming request handlingsrc/OpenFga.Sdk/Api/OpenFgaApi.cs- API endpointsrc/OpenFga.Sdk/Client/Client.cs- Client-level methodsrc/OpenFga.Sdk/Telemetry/Attributes.cs- Telemetry supportsrc/OpenFga.Sdk/OpenFga.Sdk.csproj- AddedMicrosoft.Bcl.AsyncInterfacespackageCHANGELOG.md- Added feature entryTest Results
All 22 tests passing (10 streaming + 11 client integration):
ListObjectsoptions (consistency, contextual tuples, etc.)Usage Example
Key Benefits
IAsyncEnumerable<T>patternReferences
Summary by CodeRabbit
New Features
Documentation