feat: streaming endpoint utils, using templates for streaming #666
Conversation
|
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. WalkthroughAdds streaming support to the Go SDK through new template configurations and implementations. Introduces generic streaming helper types, NDJSON response processors, and public executors for streamed list objects operations, with comprehensive error handling, context cancellation, and backward compatibility. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Code
participant Executor as ExecuteStreamedListObjects
participant Helper as executeStreamingRequest
participant HTTP as HTTP Client
participant Processor as ProcessStreamingResponse
participant Channel as StreamingChannel
Client->>Executor: Call with store_id, request
Executor->>Helper: Delegate execution
Helper->>Helper: Build request, prepare headers
Helper->>HTTP: Execute streaming request
HTTP-->>Helper: *http.Response (NDJSON stream)
Helper->>Processor: Process response with context
Processor->>Channel: Create channel with buffer
Processor->>Processor: Read NDJSON lines in goroutine
Processor->>Processor: Decode JSON per line
Processor->>Channel: Emit result or error
Processor->>Channel: Close on stream end
Helper-->>Executor: Return StreamingChannel
Executor-->>Client: Return StreamingChannel
Client->>Channel: Read from Results/Errors channels
alt Stream Success
Channel-->>Client: Result objects
else Stream Error
Channel-->>Client: Error on Errors channel
end
Client->>Channel: Call Close()
Channel->>Channel: Cancel context, cleanup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Pull Request Overview
This pull request adds streaming endpoint utilities to the Go SDK by introducing templated implementations for streaming support. The changes enable proper handling of NDJSON streaming responses from the OpenFGA API, specifically for the StreamedListObjects endpoint.
Key changes:
- Introduced generic streaming utilities (
StreamingChannel,ProcessStreamingResponse,executeStreamingRequest) - Added backward-compatible wrappers for StreamedListObjects operations
- Comprehensive test coverage for streaming functionality including error handling, context cancellation, and custom buffer sizes
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| config/clients/go/template/streaming.mustache | Core streaming implementation with generic utilities and StreamedListObjects-specific functions |
| config/clients/go/template/streaming_test.mustache | Comprehensive test suite for streaming functionality covering success, error, and edge cases |
| config/clients/go/config.overrides.json | Configuration to include the new streaming template files in the generated SDK |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
config/clients/go/template/streaming_test.mustache (1)
266-322: Context cancellation test is good, consider slightly relaxing error‑message matchingThe cancellation test correctly:
- Flushes one item from the server.
- Cancels the client context.
- Asserts that a cancellation‑style error surfaces on the error channel.
You already match both
"context canceled"and"operation was canceled", which covers common Unix/Windows differences. If you see flakiness on other platforms or transports, consider also accepting a generic prefix like"context"to be more robust, but this isn’t strictly required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
config/clients/go/config.overrides.json(1 hunks)config/clients/go/template/streaming.mustache(1 hunks)config/clients/go/template/streaming_test.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/go/template/streaming_test.mustacheconfig/clients/go/template/streaming.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/go/template/streaming_test.mustacheconfig/clients/go/config.overrides.jsonconfig/clients/go/template/streaming.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/go/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/go/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/go/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/go/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/go/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-java-sdk
- GitHub Check: build-and-test-dotnet-sdk
- GitHub Check: build-and-test-go-sdk
🔇 Additional comments (11)
config/clients/go/template/streaming_test.mustache (5)
1-12: Mustache usage and imports look correct
{{>partial_header}}andpackage {{packageName}}align with existing templates, and all imported packages are used by the tests. No config/templating issues here.
14-46: Close semantics tests are tight and aligned with implementationBoth
TestStreamingChannel_CloseandTestStreamedListObjectsChannel_Closecorrectly assert thatClosecancels the associated context without making assumptions about channel closure order. This matches the implementation that only invokes the storedcancelfunction.
48-160: Happy‑path and empty‑line coverage for streamed list objects is solidThe success and empty‑line tests validate:
- Correct URL and HTTP method for
/stores/{store_id}/streamed-list-objects.- NDJSON content type.
- Proper aggregation of all objects and ignoring blank lines.
The pattern
for obj := range channel.Objectsfollowed by a single read fromchannel.Errorsis compatible with the producer’s close‑order (errors closed before results, results last), so no race or deadlock here.
162-264: Error and invalid JSON handling tests accurately reflect stream behaviorThe
ErrorInStreamandInvalidJSONtests:
- Ensure objects before the error are delivered.
- Assert that the first error short‑circuits the stream and is propagated via
channel.Errors.- Verify error contents (server
error.message) and object counts.This matches the
ProcessStreamingResponsebehavior (fail fast on eitherstreamResult.Erroror JSON unmarshal error).
324-423: Buffer‑size and generic processing coverage complete the story
TestStreamedListObjectsWithChannel_CustomBufferSizeandTestProcessStreamingResponse_Genericvalidate:
- That custom buffer sizes work for high‑volume streams.
- That
ProcessStreamingResponse[T]functions independently of the executor and correctly surfaces typed results and errors.Together with earlier tests, this gives good confidence in the streaming helpers’ behavior.
config/clients/go/template/streaming.mustache (5)
1-27: Template header and core types are consistent and idiomaticMustache usage (
{{>partial_header}},{{packageName}}) is minimal and correct. The genericStreamResult[T]andStreamingChannel[T]definitions are idiomatic Go and align with the NDJSON schema used in tests (resultvserror).
28-124: Streaming loop and cancellation semantics look sound
ProcessStreamingResponse:
- Uses a dedicated
streamCtxwith a storedcancelforStreamingChannel.Close, without exposing channels to external closing.- Handles:
- Nil responses (
response or response body is nil).- Large NDJSON entries (scanner buffer bumped to 10MB).
- Empty lines (skipped).
- Structured stream errors via
StreamResult.Error, preferring the server’sStatus.Message.- Context cancellation both before and during send operations, surfacing the context error instead of low‑level network errors.
- Ensures cleanup via defers: closing
ResultsandErrors, canceling the context, and closing the HTTP body.This all matches the test expectations and avoids double‑sending on
Errors.
126-156: Backward‑compatibility wrapper is straightforward and correct
StreamedListObjectsChannelandProcessStreamedListObjectsResponsecorrectly adapt the genericStreamingChannel[StreamedListObjectsResponse]into the previousObjects/Errorsshape while sharing the same underlying channels and cancel function. This keeps the old API surface intact without extra goroutines.
158-185: ListObjects streaming executors are well wired
ExecuteStreamedListObjectsandExecuteStreamedListObjectsWithBufferSize:
- Use the expected path template
/stores/{store_id}/streamed-list-objects(matching tests).- Delegate to the generic executor with the right type parameters (
ListObjectsRequest,StreamedListObjectsResponse).- Map the generic channel into the backward‑compatible
StreamedListObjectsChannel.The zero/negative buffer case is safely handled downstream by
ProcessStreamingResponse’s defaulting logic.
202-234: HTTP request construction and error handling are aligned with existing patterns
executeStreamingRequest:
- Validates
storeIdand usesurl.PathEscape(parameterToString(...))when substituting into the path.- Sets
Content-Type: application/jsonandAccept: application/x-ndjson, matching server and test expectations.- Merges custom headers from
options.Headers.- On non‑2xx responses, reads and closes the body, then delegates to
client.handleAPIErrorwith the right context (body,operationName,storeId).This is consistent with typical OpenFGA SDK patterns and looks good.
config/clients/go/config.overrides.json (1)
14-31: Streaming templates are correctly registered; FOSSA and versioning remain consistentThe new
filesentries forstreaming.mustache→streaming.goandstreaming_test.mustache→streaming_test.gousetemplateType: "SupportingFiles", which matches how other Go templates are wired.
packageVersionandfossaComplianceNoticeIdare unchanged, so versioning and FOSSA compliance stay consistent with existing guidance. No hardcoded credentials are introduced in this JSON.Based on learnings
Description
Generates openfga/go-sdk#259
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.