feat(go): add support for write conflict writes#611
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 WalkthroughAdds conflict-handling options for Go client writes/deletes in templates, updates example and README template, expands tests (client, API, models), updates Go client config overrides to include models tests, and sets an OPEN_API_REF env in CI. Changelog documents the feature. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Application
participant Client as Go SDK Client
participant API as OpenFGA API
App->>Client: Write(tuple writes/deletes, options{Conflict})
note right of Client: If request > chunk size, split into sub-requests
alt Non-chunked
Client->>API: POST /stores/{storeId}/write<br/>body: {writes{on_duplicate}, deletes{on_missing}}
API-->>Client: WriteResponse
else Chunked
loop For each chunk
Client->>API: POST /write with same Conflict options
API-->>Client: Partial response
end
end
Client-->>App: Final WriteResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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 |
cb76c77 to
76cde48
Compare
76cde48 to
7efdc40
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
config/clients/go/template/client/client.mustache (2)
1587-1593: Guard against nil body in WriteExecute.Calling Execute without Body will panic on len(request.GetBody().Writes).
Apply:
func (client *{{appShortName}}Client) WriteExecute(request SdkClientWriteRequestInterface) (*ClientWriteResponse, error) { + if request.GetBody() == nil { + return nil, FgaRequiredParamError{param: "body"} + } options := request.GetOptions()
1728-1731: Bug: delete chunk end uses writeChunkSize.This will overshoot or undershoot delete chunking.
Apply:
- end := int(math.Min(float64(i+writeChunkSize), float64(len(request.GetBody().Deletes)))) + end := int(math.Min(float64(i+deleteChunkSize), float64(len(request.GetBody().Deletes))))config/clients/go/template/README_calling_api.mustache (6)
47-53: Fix missing method-chain dot.
Options(options)Execute()should beOptions(options).Execute().Apply this diff:
- store, err := fgaClient.GetStore(context.Background()).Options(options)Execute() + store, err := fgaClient.GetStore(context.Background()).Options(options).Execute()
183-190: Fix missing method-chain dot.
Options(options)Execute()should beOptions(options).Execute().Apply this diff:
- data, err := fgaClient.ReadLatestAuthorizationModel(context.Background()).Options(options)Execute() + data, err := fgaClient.ReadLatestAuthorizationModel(context.Background()).Options(options).Execute()
233-271: Incorrect variable name passed to Body().You define
body := ClientReadRequest{...}but callBody(requestBody). Usebodyto avoid a compile-time error in examples.Apply this diff:
- data, err := fgaClient.Read(context.Background()).Body(requestBody).Options(options).Execute() + data, err := fgaClient.Read(context.Background()).Body(body).Options(options).Execute()
574-592: Mismatched variable name in ListObjects example.You define
body := ClientListObjectsRequest{...}but callBody(requestBody). Usebody.Apply this diff:
- data, err := fgaClient.ListObjects(context.Background()). - Body(requestBody). + data, err := fgaClient.ListObjects(context.Background()). + Body(body). Options(options). Execute()
609-623: Mismatched variable name in ListRelations example.You define
body := ClientListRelationsRequest{...}but callBody(requestBody). Usebody.Apply this diff:
- data, err := fgaClient.ListRelations(context.Background()). - Body(requestBody). + data, err := fgaClient.ListRelations(context.Background()). + Body(body). Options(options). Execute()
9-18: Unify package alias (openfga → {{packageName}}) and fix method-chain typos in README template.
- Replace openfga.* usages with {{packageName}}.* in config/clients/go/template/README_calling_api.mustache at lines: 13, 543, 545, 607, 640, 645, 647, 650.
- Fix broken method chaining (missing dot before Execute) in the same file at lines 52 and 189: change Options(... )Execute() → Options(...).Execute().
config/clients/go/template/client/client_test.mustache (1)
2719-2723: Leftover ‘openfga’ alias will not compile after alias switch.These lines still reference
openfga.PtrStringandopenfga.ExpandResponse, but the file imports{{packageName}}and notopenfga. Replace with{{packageName}}.Apply this diff:
- options := ClientExpandOptions{ - AuthorizationModelId: openfga.PtrString("01GAHCE4YVKPQEKZQHT2R89MQV"), - } - - var expectedResponse openfga.ExpandResponse + options := ClientExpandOptions{ + AuthorizationModelId: {{packageName}}.PtrString("01GAHCE4YVKPQEKZQHT2R89MQV"), + } + + var expectedResponse {{packageName}}.ExpandResponse
🧹 Nitpick comments (5)
config/clients/go/CHANGELOG.md.mustache (1)
74-75: Fix stray “k” in compare link.The URL currently has an extra “k” after {{gitRepoId}}.
Apply:
-### [0.6.2](https://{{gitHost}}/{{gitUserId}}/{{gitRepoId}}k/compare/v0.6.1...v0.6.2) (2024-10-21) +### [0.6.2](https://{{gitHost}}/{{gitUserId}}/{{gitRepoId}}/compare/v0.6.1...v0.6.2) (2024-10-21)config/clients/go/template/client/client.mustache (2)
1115-1120: Use pointer for Client in ReadLatestAuthorizationModel request (consistency).Other request structs keep a pointer; align to avoid copying the client.
Apply:
-type SdkClientReadLatestAuthorizationModelRequest struct { +type SdkClientReadLatestAuthorizationModelRequest struct { ctx _context.Context - Client {{appShortName}}Client + Client *{{appShortName}}Client options *ClientReadLatestAuthorizationModelOptions } @@ - return &SdkClientReadLatestAuthorizationModelRequest{ - Client: *client, + return &SdkClientReadLatestAuthorizationModelRequest{ + Client: client, ctx: ctx, } @@ - return request.Client.ReadLatestAuthorizationModelExecute(request) + return request.Client.ReadLatestAuthorizationModelExecute(request)Also applies to: 1127-1129
2491-2526: Minor style nit: unify API field access.Mixing
client.{{appShortName}}Apiandclient.OpenFgaApi; prefer the templated form for consistency.config/clients/go/template/README_calling_api.mustache (1)
364-398: New “Conflict Options” section looks good, but keep code style consistent.
- Content is accurate and helpful.
- Nit: elsewhere this doc uses “
golang”; here it’s “go”. Pick one and use consistently.config/clients/go/template/api_test.mustache (1)
790-1090: Solid coverage for OnDuplicate/OnMissing on WriteRequest.
- Tests correctly assert presence and values of
writes.on_duplicateanddeletes.on_missing.- Optional: factor the repeated “register responder + decode body + assert field” into a small helper to reduce duplication and tighten failures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/workflows/main.yaml(1 hunks)config/clients/go/CHANGELOG.md.mustache(1 hunks)config/clients/go/config.overrides.json(1 hunks)config/clients/go/template/README_calling_api.mustache(1 hunks)config/clients/go/template/api_test.mustache(1 hunks)config/clients/go/template/client/client.mustache(4 hunks)config/clients/go/template/client/client_test.mustache(21 hunks)config/clients/go/template/example/example1/example1.go(1 hunks)config/clients/go/template/models_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/api_test.mustacheconfig/clients/go/template/README_calling_api.mustacheconfig/clients/go/CHANGELOG.md.mustacheconfig/clients/go/template/models_test.mustacheconfig/clients/go/template/client/client.mustacheconfig/clients/go/template/client/client_test.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/api_test.mustacheconfig/clients/go/config.overrides.jsonconfig/clients/go/template/README_calling_api.mustacheconfig/clients/go/CHANGELOG.md.mustacheconfig/clients/go/template/models_test.mustacheconfig/clients/go/template/client/client.mustacheconfig/clients/go/template/client/client_test.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
⏰ 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). (2)
- GitHub Check: build-and-test-java-sdk
- GitHub Check: build-and-test-go-sdk
🔇 Additional comments (10)
.github/workflows/main.yaml (1)
68-69: Go job sets OPEN_API_REF — LGTM.Matches PR description and scopes the override to the Go SDK tests step.
config/clients/go/CHANGELOG.md.mustache (2)
5-7: Changelog entry for conflict options — LGTM.Clear note about server requirement and pointer to README.
5-7: Resolved — README anchor exists.
README template contains "Conflict Options for Write Operations" at config/clients/go/template/README_calling_api.mustache: line 364.config/clients/go/template/client/client.mustache (1)
1705-1706: Propagate Conflict in chunked paths — LGTM.Conflict options correctly flow to sub-requests.
Also applies to: 1751-1751
config/clients/go/template/example/example1/example1.go (1)
203-206: Good example usage of Conflict options.Demonstrates ignoring duplicate writes clearly.
config/clients/go/config.overrides.json (2)
101-104: Addition of models_test.mustache — LGTM.Improves coverage for WriteRequest and conflict defaults.
1-9: Verified — template exists and packageVersion is consistent.models_test.mustache found at config/clients/go/template/models_test.mustache; config/clients/go/config.overrides.json packageVersion=0.7.1 and config/common/config.base.json has no packageVersion set (no mismatch).
config/clients/go/template/models_test.mustache (1)
1-759: Thorough model tests and sensible defaults.Comprehensive checks for defaults, getters, setters, presence flags, and JSON marshalling for
WriteRequest{Writes,Deletes}andWriteRequest. Nice.config/clients/go/template/client/client_test.mustache (2)
15-16: Template package alias import LGTM.Using
{{packageName}}as the import alias keeps tests generation-friendly.
3562-3679: BatchCheck max-batch sizing tests look correct.Validates request chunking count using MaxBatchSize/MaxParallelRequests; mutex around the counter is a good touch.
Description
Note: This requires generation using the following API Ref: openfga/api@0ac19aa
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
addresses the go part of #610
generates openfga/go-sdk#230
Review Checklist
mainSummary by CodeRabbit
New Features
Documentation
Tests
Chores