feat(dotnet-sdk): add write conflict resolution options#644
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 write conflict resolution options for the .NET client, adding enums for duplicate-write and missing-delete behaviors, interfaces for conflict configuration, and client-side mapping helpers that propagate conflict settings through API requests. Additionally, per-request header support is expanded, with comprehensive tests validating header precedence, conflict option propagation, and reserved header protection. Changes
Sequence DiagramsequenceDiagram
participant Client as Client Code
participant SDK as {{appShortName}}Client
participant Mapper as Mapping Helpers
participant API as OpenFgaApi
Client->>SDK: Write(request, options: ClientWriteOptions)
Note over SDK: Extract headers via ExtractHeaders(options)
SDK->>Mapper: MapOnDuplicateWrites(options?.Conflict?.OnDuplicateWrites)
Mapper-->>SDK: WriteRequestWrites.OnDuplicateEnum
SDK->>Mapper: MapOnMissingDeletes(options?.Conflict?.OnMissingDeletes)
Mapper-->>SDK: WriteRequestDeletes.OnMissingEnum
Note over SDK: Build clientWriteOpts with<br/>Conflict = options?.Conflict<br/>Headers from ExtractHeaders(options)
SDK->>API: Call API with mapped conflict behavior<br/>and propagated headers
API-->>SDK: Response
SDK-->>Client: Result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span multiple files with new API surface additions (enums, interfaces, classes) and internal logic updates for conflict mapping and header propagation. While individual changes follow a consistent pattern, the extent of new test coverage and cross-file coordination requires moderate review attention to validate correctness and completeness of the implementation. Possibly related issues
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: 4
🧹 Nitpick comments (1)
config/clients/dotnet/CHANGELOG.md.mustache (1)
10-10: Fix typo in “overiding”.Please correct the spelling to “overriding” for clarity.
Apply this diff:
- - add header validation to prevent overiding of reserved headers + - add header validation to prevent overriding of reserved headers
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
config/clients/dotnet/CHANGELOG.md.mustache(7 hunks)config/clients/dotnet/config.overrides.json(1 hunks)config/clients/dotnet/template/Client/Client.mustache(26 hunks)config/clients/dotnet/template/Client/ClientConfiguration.mustache(4 hunks)config/clients/dotnet/template/Client/Model/ClientBatchCheckOptions.mustache(2 hunks)config/clients/dotnet/template/Client/Model/ClientCheckOptions.mustache(2 hunks)config/clients/dotnet/template/Client/Model/ClientConsistencyOptions.mustache(2 hunks)config/clients/dotnet/template/Client/Model/ClientCreateStoreOptions.mustache(1 hunks)config/clients/dotnet/template/Client/Model/ClientExpandOptions.mustache(2 hunks)config/clients/dotnet/template/Client/Model/ClientListObjectsOptions.mustache(2 hunks)config/clients/dotnet/template/Client/Model/ClientListRelationsOptions.mustache(2 hunks)config/clients/dotnet/template/Client/Model/ClientListStoresOptions.mustache(2 hunks)config/clients/dotnet/template/Client/Model/ClientListUsersOptions.mustache(2 hunks)config/clients/dotnet/template/Client/Model/ClientReadAssertionsOptions.mustache(2 hunks)config/clients/dotnet/template/Client/Model/ClientReadAuthorizaionModelOptions.mustache(2 hunks)config/clients/dotnet/template/Client/Model/ClientReadAuthorizaionModelsOptions.mustache(2 hunks)config/clients/dotnet/template/Client/Model/ClientReadChangesOptions.mustache(2 hunks)config/clients/dotnet/template/Client/Model/ClientReadOptions.mustache(2 hunks)config/clients/dotnet/template/Client/Model/ClientRequestOptions.mustache(1 hunks)config/clients/dotnet/template/Client/Model/ClientRequestOptsWithStoreId.mustache(1 hunks)config/clients/dotnet/template/Client/Model/ClientWriteAssertionsOptions.mustache(2 hunks)config/clients/dotnet/template/Client/Model/ClientWriteOptions.mustache(3 hunks)config/clients/dotnet/template/Client_ApiClient.mustache(4 hunks)config/clients/dotnet/template/OpenFgaClientTests.mustache(6 hunks)config/clients/dotnet/template/README_calling_api.mustache(1 hunks)config/clients/dotnet/template/README_initializing.mustache(1 hunks)config/clients/dotnet/template/api.mustache(3 hunks)config/clients/dotnet/template/modelRequestOptions.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/dotnet/template/README_initializing.mustacheconfig/clients/dotnet/template/Client/Model/ClientRequestOptsWithStoreId.mustacheconfig/clients/dotnet/template/Client/Model/ClientReadChangesOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientReadAssertionsOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientBatchCheckOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientExpandOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientReadAuthorizaionModelsOptions.mustacheconfig/clients/dotnet/template/README_calling_api.mustacheconfig/clients/dotnet/template/Client/Model/ClientWriteAssertionsOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientWriteOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientListUsersOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientCreateStoreOptions.mustacheconfig/clients/dotnet/template/modelRequestOptions.mustacheconfig/clients/dotnet/template/api.mustacheconfig/clients/dotnet/template/Client/Model/ClientConsistencyOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientListStoresOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientListRelationsOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientListObjectsOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientReadAuthorizaionModelOptions.mustacheconfig/clients/dotnet/template/Client_ApiClient.mustacheconfig/clients/dotnet/template/Client/ClientConfiguration.mustacheconfig/clients/dotnet/template/Client/Model/ClientCheckOptions.mustacheconfig/clients/dotnet/CHANGELOG.md.mustacheconfig/clients/dotnet/template/OpenFgaClientTests.mustacheconfig/clients/dotnet/template/Client/Model/ClientRequestOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientReadOptions.mustacheconfig/clients/dotnet/template/Client/Client.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/dotnet/template/README_initializing.mustacheconfig/clients/dotnet/template/Client/Model/ClientRequestOptsWithStoreId.mustacheconfig/clients/dotnet/template/Client/Model/ClientReadChangesOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientReadAssertionsOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientBatchCheckOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientExpandOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientReadAuthorizaionModelsOptions.mustacheconfig/clients/dotnet/template/README_calling_api.mustacheconfig/clients/dotnet/template/Client/Model/ClientWriteAssertionsOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientWriteOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientListUsersOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientCreateStoreOptions.mustacheconfig/clients/dotnet/template/modelRequestOptions.mustacheconfig/clients/dotnet/template/api.mustacheconfig/clients/dotnet/template/Client/Model/ClientConsistencyOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientListStoresOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientListRelationsOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientListObjectsOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientReadAuthorizaionModelOptions.mustacheconfig/clients/dotnet/template/Client_ApiClient.mustacheconfig/clients/dotnet/template/Client/ClientConfiguration.mustacheconfig/clients/dotnet/template/Client/Model/ClientCheckOptions.mustacheconfig/clients/dotnet/CHANGELOG.md.mustacheconfig/clients/dotnet/config.overrides.jsonconfig/clients/dotnet/template/OpenFgaClientTests.mustacheconfig/clients/dotnet/template/Client/Model/ClientRequestOptions.mustacheconfig/clients/dotnet/template/Client/Model/ClientReadOptions.mustacheconfig/clients/dotnet/template/Client/Client.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/dotnet/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/dotnet/config.overrides.json
🧠 Learnings (1)
📚 Learning: 2025-08-12T14:18:58.827Z
Learnt from: rhamzeh
PR: openfga/sdk-generator#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/dotnet/config.overrides.json
🪛 Gitleaks (8.28.0)
config/clients/dotnet/template/OpenFgaClientTests.mustache
[high] 2867-2867: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (3)
- GitHub Check: build-and-test-java-sdk
- GitHub Check: build-and-test-dotnet-sdk
- GitHub Check: build-and-test-go-sdk
🔇 Additional comments (12)
config/clients/dotnet/template/Client/Model/ClientListObjectsOptions.mustache (1)
1-23: LGTM! Header support correctly implemented.The changes properly add header support to
ClientListObjectsOptions:
- Templated namespace using
{{packageName}}provides appropriate flexibilitySystem.Collections.Genericimport correctly supports the newIDictionaryproperty- Headers property follows C# conventions with nullable type and auto-property syntax
<inheritdoc />comment assumes the parent interface hierarchy (likelyIClientRequestOptionsWithAuthZModelIdAndConsistency) defines the Headers property, which aligns with the broader refactoring described in the PRThe implementation is consistent with the pattern being applied across multiple option classes in this PR.
config/clients/dotnet/template/Client/Model/ClientExpandOptions.mustache (3)
3-3: LGTM: Proper template parameterization.The change from hardcoded namespace to templated
{{packageName}}.Modelimproves flexibility and follows mustache templating best practices.
4-4: LGTM: Required import for IDictionary.The import is necessary for the new Headers property and is correctly placed.
21-22: The Headers property is correctly defined in the parent interface hierarchy throughIClientRequestOptions, which is accessible via the inheritance chain:IClientExpandOptions→IClientRequestOptionsWithAuthZModelIdAndConsistency→IClientRequestOptionsWithAuthZModelId→IClientRequestOptionsWithStoreId→IClientRequestOptions.All four properties in
ClientExpandOptionsclass—StoreId,AuthorizationModelId,Consistency, andHeaders—are properly defined in their respective parent interfaces and use validinheritdocreferences:
StoreIdfromStoreIdOptionsAuthorizationModelIdfromAuthorizationModelIdOptionsConsistencyfromIClientConsistencyOptionsHeadersfromIClientRequestOptionsThe code is correct.
config/clients/dotnet/template/Client/Model/ClientReadChangesOptions.mustache (1)
3-4: LGTM! Headers property follows the established pattern.The addition of the
Headersproperty withIDictionary<string, string>?type and the correspondingusing System.Collections.Generic;directive aligns with the broader header support introduced across client option models.Also applies to: 23-25
config/clients/dotnet/template/Client/Model/ClientConsistencyOptions.mustache (1)
3-3: LGTM! Namespace migration is appropriate.The change from
OpenFga.Sdk.Modelto{{packageName}}.Modelaligns with the template-driven package scoping approach used across the codebase.config/clients/dotnet/template/Client/Model/ClientListRelationsOptions.mustache (1)
3-4: LGTM! Consistent header support implementation.The namespace change and
Headersproperty addition follow the same pattern as other client option models in this PR.Also applies to: 25-28
config/clients/dotnet/template/README_initializing.mustache (1)
97-136: LGTM! Comprehensive header documentation.The documentation for custom headers is clear and well-structured, covering both default headers (client-level) and per-request headers with practical code examples.
config/clients/dotnet/template/Client/Model/ClientListUsersOptions.mustache (1)
3-4: LGTM! Consistent implementation.The namespace migration and
Headersproperty addition follow the established pattern across client option models.Also applies to: 20-23
config/clients/dotnet/template/Client/Model/ClientReadAuthorizaionModelOptions.mustache (1)
3-4: LGTM! Headers property addition is consistent.The addition of the
Headersproperty and the correspondingusingdirective follows the same pattern as other client option models.Also applies to: 20-22
config/clients/dotnet/template/README_calling_api.mustache (1)
321-358: LGTM! Clear conflict options documentation.The documentation section effectively explains the new conflict handling options for write operations, with a comprehensive code example demonstrating how to configure
OnDuplicateWritesandOnMissingDeletesbehavior.config/clients/dotnet/template/Client/Model/ClientBatchCheckOptions.mustache (1)
3-4: LGTM! Consistent with the pattern.The namespace migration and
Headersproperty addition are consistent with the broader header support implementation across client option models.Also applies to: 29-31
evansims
changed the base branch from
main
to
feat/dotnet/per-request-custom-headers
There was a problem hiding this comment.
Pull Request Overview
Adds per-request custom headers support and introduces write conflict resolution options for the .NET SDK templates.
- Exposes per-request headers via IRequestOptions at API level and IClientRequestOptions for high-level client options; validates and merges with defaults.
- Implements ConflictOptions with OnDuplicateWrites and OnMissingDeletes for write operations and ensures defaults are explicitly sent.
- Updates tests and docs to cover header behavior and conflict options.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| config/clients/dotnet/template/modelRequestOptions.mustache | Introduces IRequestOptions/RequestOptions for per-request headers at API layer. |
| config/clients/dotnet/template/api.mustache | Adds options parameter (headers) to all API methods and forwards headers to ApiClient. |
| config/clients/dotnet/template/Client_ApiClient.mustache | Merges OAuth and per-request headers, adds header-building/validation. |
| config/clients/dotnet/template/Client/Client.mustache | Extracts headers from options, passes to API, and maps conflict options for write calls. |
| config/clients/dotnet/template/Client/Model/* | Adds Headers to all client options; adds ConflictOptions and enums for write behavior. |
| config/clients/dotnet/template/OpenFgaClientTests.mustache | Adds comprehensive tests for headers and write conflict options. |
| config/clients/dotnet/template/README_*.mustache | Documents default and per-request headers and write conflict options. |
| config/clients/dotnet/config.overrides.json | Maps new RequestOptions template to generated output. |
| config/clients/dotnet/CHANGELOG.md.mustache | Notes new features and breaking changes, incl. header support and options rename. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
config/clients/dotnet/template/OpenFgaClientTests.mustache (1)
2900-2904: Use the correct options type for ReadLatestAuthorizationModel headers [duplicate of prior review]Pass
ClientReadAuthorizationModelsOptions(or the interface it implements) instead ofClientWriteOptionsto match the method’s expected options type.Apply:
- var options = new ClientWriteOptions { + var options = new ClientReadAuthorizationModelsOptions { Headers = new Dictionary<string, string> { { "X-Latest-Model", "latest-xyz" } } };
🧹 Nitpick comments (3)
config/clients/dotnet/CHANGELOG.md.mustache (1)
11-15: Clarify version requirement and explicit defaults in changelogSuggest adding:
- Requires OpenFGA v1.10.0+.
- SDK explicitly sends Error defaults when options are not specified (as implemented/tests assert).
Example:
- Requires OpenFGA v1.10.0+.
- When not specified, the SDK explicitly sends
Errorfor both behaviors.config/clients/dotnet/template/Client/Client.mustache (1)
227-239: Mapping helpers default to Error (as desired)Null → Error behavior matches API expectations and tests. Optionally, consider a switch expression for readability.
config/clients/dotnet/template/OpenFgaClientTests.mustache (1)
2202-2203: Avoid hard-coding OpenFgaClient in a templateUse
{{appShortName}}Clientconsistently to keep the template resilient to naming changes and aligned with the rest of the file.Example fix (apply to each occurrence):
-var fgaClient = new OpenFgaClient(_config, httpClient); +var fgaClient = new {{appShortName}}Client(_config, httpClient);Also applies to: 2251-2252, 2300-2301, 2349-2350, 2398-2399, 2460-2461, 2535-2536, 2596-2597, 2655-2656
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
config/clients/dotnet/CHANGELOG.md.mustache(2 hunks)config/clients/dotnet/template/Client/Client.mustache(3 hunks)config/clients/dotnet/template/OpenFgaClientTests.mustache(5 hunks)config/clients/dotnet/template/README_initializing.mustache(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- config/clients/dotnet/template/README_initializing.mustache
🔇 Additional comments (5)
config/clients/dotnet/CHANGELOG.md.mustache (1)
10-10: Reserved headers validation entry looks goodSpelling fix and intent are correct.
config/clients/dotnet/template/Client/Client.mustache (2)
370-379: Conflict options correctly propagated into WriteRequestWrites/Deletes map OnDuplicate/OnMissing via the helpers. Defaults handled when null. LGTM.
395-400: Non-transaction chunk path preserves per-request StoreId, headers, and conflictUsing
options?.StoreId ?? StoreId, carryingHeadersandConflictfixes the previously flagged StoreId override issue in chunked writes. Good catch.config/clients/dotnet/template/OpenFgaClientTests.mustache (2)
1015-1017: Minor test cleanup: LGTMSwitching to fire-and-forget Write calls removes unused variables; no behavior change.
Also applies to: 1061-1063, 1114-1116
2177-2677: Conflict options tests are comprehensiveGood coverage for Ignore/Error behaviors, combinations, non-transaction chunking, and defaulting to Error when unspecified. Assertions reflect the wire format correctly.
evansims
force-pushed
the
feat/dotnet/write-conflict-options
branch
from
53921d5 to
8380689
Compare
evansims
dismissed stale reviews from SoulPancake and ewanharris
The base branch was changed.
evansims
force-pushed
the
feat/dotnet/write-conflict-options
branch
from
70f2692 to
200396c
Compare
Description
Implements support for write conflict resolution options introduced in OpenFGA v1.10.0, allowing users to control behavior for duplicate writes and missing deletes.
OnDuplicateWritesenum:Error(default) |Ignore- controls behavior when writing a tuple that already existsOnMissingDeletesenum:Error(default) |Ignore- controls behavior when deleting a non-existent tupleConflictOptionsclass with properties for both enum typesClientWriteOptions.ConflictpropertyError) are explicitly sent to the API when not specified by the userUsage Example:
References
Generates → openfga/dotnet-sdk#136
Issue → #610
Review Checklist
mainSummary by CodeRabbit
New Features
Bug Fixes
Documentation