feat: add support for Write API with on_duplicate and on_missing options #233
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 WalkthroughIntroduces configurable idempotence controls for the Write API by adding OnDuplicate and OnMissing enums, extending write/delete request schemas and proto messages with corresponding fields, updating OpenAPI docs, and adding enum validations. Also adjusts README code fence languages for syntax highlighting. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as OpenFGA Write API
participant Store as Tuple Store
Client->>API: WriteRequest(writes{on_duplicate}, deletes{on_missing})
API->>API: Validate enums (on_duplicate/on_missing)
API->>Store: Apply writes
alt on_duplicate=ERROR and tuple exists
Store-->>API: Duplicate detected
API-->>Client: Error (duplicate)
else on_duplicate=IGNORE or not duplicate
Store-->>API: Write ok
API->>Store: Apply deletes
alt on_missing=ERROR and tuple missing
Store-->>API: Missing detected
API-->>Client: Error (missing)
else on_missing=IGNORE or exists
Store-->>API: Delete ok
API-->>Client: Success
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
rhamzeh
changed the title
![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 (6)
README.md (1)
24-27: Align code fence language specifier with the rest of the doc (use bash).Earlier blocks in this file use bash. For consistency and better syntax highlighting, prefer bash here too.
Apply this diff:
- ```shell + ```bash @@ - ```shell + ```bashAlso applies to: 29-31
openfga/v1/openfga_service.proto (3)
157-160: Tighten wording and remove a stray double-space in Write API description.Minor copy edits and clarify where to set the flags.
- "The API is not idempotent by default: if, later on, you try to add the same tuple key (even if the `condition` is different), or if you try to delete a non-existing tuple, it will throw an error.\n" - "In order to allow writes in case an identical tuple already exists in the database, you may modify the behavior using `\"on_duplicate\": \"ON_DUPLICATE_IGNORE\"`\n" - "In order to allow deletes in case a tuple was already deleted from the database, you may modify the behavior using `\"on_missing\": \"ON_MISSING_IGNORE\"`\n" + "The API is not idempotent by default: if, later on, you try to add the same tuple key (even if the `condition` is different), or if you try to delete a non-existing tuple, it will throw an error.\n" + "To allow writes when an identical tuple already exists in the database, set `\"on_duplicate\": \"ON_DUPLICATE_IGNORE\"` on the `writes` object.\n" + "To allow deletes when a tuple was already removed from the database, set `\"on_missing\": \"ON_MISSING_IGNORE\"` on the `deletes` object.\n"
1248-1252: Correct OnDuplicate comments: remove “context” and fix minor grammar/spacing.“Context” is not part of tuple attributes for writes; also tighten phrasing.
enum OnDuplicate { - ON_DUPLICATE_UNSPECIFIED = 0; // If set to 'ON_DUPLICATE_UNSPECIFIED' OR not set, the API will behave as if it was set to 'ON_DUPLICATE_ERROR'. - ON_DUPLICATE_ERROR = 1; // If set to 'ON_DUPLICATE_ERROR', the API will return an error if a tuple already exists. - ON_DUPLICATE_IGNORE = 2; // If set to 'ON_DUPLICATE_IGNORE', the API will not return an error if a tuple already exists and all the attributes (including condition and context ) are matching. + ON_DUPLICATE_UNSPECIFIED = 0; // If set to 'ON_DUPLICATE_UNSPECIFIED' or not set, the API behaves as if it were set to 'ON_DUPLICATE_ERROR'. + ON_DUPLICATE_ERROR = 1; // Return an error if a tuple already exists. + ON_DUPLICATE_IGNORE = 2; // Treat identical writes as no-ops if all attributes (including the condition) match. }
1261-1269: Clarify field descriptions for on_duplicate/on_missing.Make field help text explicit about what “identical” means and avoid ambiguity around “attributes”.
OnDuplicate on_duplicate = 2 [ json_name = "on_duplicate", (validate.rules).enum.defined_only = true, (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { example: "\"ON_DUPLICATE_ERROR\"" - description: "If set to 'ON_DUPLICATE_ERROR', the API will return an error if a tuple already exists. If set to 'ON_DUPLICATE_IGNORE', the API will not return an error if a tuple already exists and all the attributes are matching." + description: "If 'ON_DUPLICATE_ERROR', the API returns an error if an identical tuple already exists. If 'ON_DUPLICATE_IGNORE', identical writes are treated as no-ops (matching on user, relation, object, and condition)." } ];OnMissing on_missing = 2 [ json_name = "on_missing", (validate.rules).enum.defined_only = true, (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { example: "\"ON_MISSING_ERROR\"" - description: "If set to 'ON_MISSING_ERROR', the API will return an error if a tuple does not exist. If set to 'ON_MISSING_IGNORE', the API will not return an error if a tuple does not exist." + description: "If 'ON_MISSING_ERROR', the API returns an error when deleting a tuple that does not exist. If 'ON_MISSING_IGNORE', deletes of non-existent tuples are treated as no-ops." } ];Also applies to: 1284-1291
docs/openapiv2/apidocs.swagger.json (2)
2414-2423: Tighten OnDuplicate description formatting and clarify equality criteriaThere’s a stray space before the closing parenthesis and the equality criteria can be clarified.
- "description": " - ON_DUPLICATE_UNSPECIFIED: If set to 'ON_DUPLICATE_UNSPECIFIED' OR not set, the API will behave as if it was set to 'ON_DUPLICATE_ERROR'.\n - ON_DUPLICATE_ERROR: If set to 'ON_DUPLICATE_ERROR', the API will return an error if a tuple already exists.\n - ON_DUPLICATE_IGNORE: If set to 'ON_DUPLICATE_IGNORE', the API will not return an error if a tuple already exists and all the attributes (including condition and context ) are matching." + "description": " - ON_DUPLICATE_UNSPECIFIED: If set to 'ON_DUPLICATE_UNSPECIFIED' OR not set, the API will behave as if it was set to 'ON_DUPLICATE_ERROR'.\n - ON_DUPLICATE_ERROR: If set to 'ON_DUPLICATE_ERROR', the API will return an error if a tuple already exists.\n - ON_DUPLICATE_IGNORE: If set to 'ON_DUPLICATE_IGNORE', the API will not return an error if a tuple already exists and all attributes match (including the condition name and its persisted context)."
3108-3113: Align on_duplicate property description with enum semanticsTo avoid ambiguity, mirror the enum’s wording and explicitly mention condition/context equality.
- "description": "If set to 'ON_DUPLICATE_ERROR', the API will return an error if a tuple already exists. If set to 'ON_DUPLICATE_IGNORE', the API will not return an error if a tuple already exists and all the attributes are matching." + "description": "If set to 'ON_DUPLICATE_ERROR', the API returns an error if a tuple already exists. If set to 'ON_DUPLICATE_IGNORE', the API will not return an error if a tuple already exists and all attributes match (including the condition name and its persisted context)."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
proto/openfga/v1/openfga_service.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (4)
README.md(1 hunks)docs/openapiv2/apidocs.swagger.json(4 hunks)openfga/v1/openfga_service.proto(5 hunks)proto/openfga/v1/openfga_service.pb.validate.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
proto/openfga/v1/openfga_service.pb.validate.go (1)
proto/openfga/v1/openfga_service.pb.go (2)
OnDuplicate_name(40-44)OnMissing_name(89-93)
🔇 Additional comments (2)
proto/openfga/v1/openfga_service.pb.validate.go (1)
1749-1758: LGTM: enum validation added for on_duplicate/on_missing.The checks correctly ensure values are among defined enum members, aligning with the proto validate annotations. No action needed.
Also applies to: 1907-1916
docs/openapiv2/apidocs.swagger.json (1)
3087-3092: LGTM: on_missing field added correctly to WriteRequestDeletesThe new optional on_missing enum reference and description are correctly modeled and consistent with the new OnMissing definition.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
cikasfm
force-pushed
the
duplicate-writes
branch
from
cfa8c1d to
e995050
Compare
cikasfm
changed the title
cikasfm
changed the title
There was a problem hiding this comment.
Can we:
- Keep this in draft until the RFC is up and approved and the code in openfga/openfga is ready for it? Otherwise we'll have it merged and SDKs and docs will start including it while it's not ready.
- While I'm OK with this in general - can we ideate on other ideas? Such as combining write and delete enums? e.g.
on_conflict
On conflict:
- Error
- Update
- Ignore
- I would prefer that in the enum, on error (being the current functionality and the default) is either the first (replacing unspecified) or the second (after unspecified), currently, the flow of:
- unspecified (error)
- ignore (no error)
- error (error)
seems weird
Also ignore should only ignore if the tuples are EXACT matches. If the conditions are different, it absolutely should error as you are in an unexpected state as far as the user is concerned.
You may want to support on conflict update, but that can also come later if implementation delays this work
|
…for proper JSON values
Introducing new
on_duplicateandon_missingflags for Write API to allow future implementation of writing duplicate tuples and deleting tuples that don't exist.Description
This pull request updates Write API contract as required for the related issue openfga/roadmap#79. This feature allows API requests to be sent without failing due to duplicate or missing tuples, which significantly improves the developer experience.
What problem is being solved?
The current
WriteAPI fails an entire batched request if any single tuple being written already exists. This forces developers to implement complex client-side logic for pre-checking for existing tuples or handling de-duplication in their retry mechanisms. This behavior makes batched operations brittle and is a common source of developer friction.How is it being solved?
The solution introduces optional
on_duplicateandon_missingflags to theWriteAPI request. These flags allow a developer to specify that duplicate writes or missing deletes should be ignored as a no-op, rather than causing the entire request to fail. The backend logic will be a SQL implementation that uses a single, atomic database transaction to ensure consistency and prevent race conditions.What changes are made to solve it?
on_duplicateandon_missingflags.Example:
{ "writes": { "tuple_keys": [ { "user": "user:anne", "relation": "reader", "object": "document:2021-budget", "condition": { "name": "condition1", "context": {} } } ], "on_duplicate": "ignore" }, "deletes": { "tuple_keys": [ { "user": "user:anne", "relation": "reader", "object": "document:2021-budget" } ], "on_missing": "ignore" }, "authorization_model_id": "01G5JAVJ41T49E9TT3SKVS7X1J" }Swagger API
References
Review Checklist
main