gnodet-bot commented on code in PR #29838:
URL: https://github.com/apache/camel/pull/29838#discussion_r4562833233
##########
components/camel-openfga/src/main/java/org/apache/camel/component/openfga/OpenFgaProducer.java:
##########
@@ -604,11 +618,82 @@ private void deleteTuples(Exchange exchange,
OpenFgaAuthorizer authorizer) throw
tuples.add(new ClientTupleKeyWithoutCondition()
.user(tuple.user).relation(tuple.relation)._object(tuple.object));
}
- authorizer.await(exchange, "delete " + tuples.size() + " tuple(s)",
- () -> authorizer.getClient().deleteTuples(tuples));
+ if (getEndpoint().getConfiguration().getOnMissing() ==
OpenFgaWriteConflictMode.ignore) {
+ ClientDeleteTuplesOptions options
+ = new
ClientDeleteTuplesOptions().onMissing(WriteRequestDeletes.OnMissingEnum.IGNORE);
+ authorizer.await(exchange, "delete " + tuples.size() + " tuple(s)",
+ () -> authorizer.getClient().deleteTuples(tuples,
options));
+ } else {
+ authorizer.await(exchange, "delete " + tuples.size() + " tuple(s)",
+ () -> authorizer.getClient().deleteTuples(tuples));
+ }
exchange.getMessage().setHeader(OpenFgaConstants.DELETED_TUPLES,
tuples.size());
}
+ private void write(Exchange exchange, OpenFgaAuthorizer authorizer) throws
Exception {
+ List<Tuple> writes;
+ List<Tuple> deletes;
+ if (authorizer.hasConfiguredTuple()) {
+ // The same configured-tuple precedence as writeTuples: a body
never adds extra mutations to it.
+ writes = resolveTuples(exchange, authorizer);
+ deletes = List.of();
+ } else {
+ Object body = exchange.getMessage().getBody();
+ if (!(body instanceof Map<?, ?> map) || !Set.of("writes",
"deletes").containsAll(map.keySet())) {
+ throw new IllegalArgumentException("write requires a Map
containing writes and/or deletes collections");
+ }
+ writes = mutationTuples(map.get("writes"));
+ deletes = mutationTuples(map.get("deletes"));
+ }
+ int count = writes.size() + deletes.size();
+ if (count < 1 || count > 100) {
+ throw new IllegalArgumentException("write requires between 1 and
100 tuples across writes and deletes");
+ }
+ List<ClientTupleKey> writeKeys = new ArrayList<>(writes.size());
+ for (Tuple tuple : writes) {
+ writeKeys.add(new
ClientTupleKey().user(tuple.user).relation(tuple.relation)._object(tuple.object));
+ }
+ List<ClientTupleKeyWithoutCondition> deleteKeys = new
ArrayList<>(deletes.size());
+ for (Tuple tuple : deletes) {
+ deleteKeys
+ .add(new
ClientTupleKeyWithoutCondition().user(tuple.user).relation(tuple.relation)._object(tuple.object));
+ }
+ OpenFgaConfiguration configuration = getEndpoint().getConfiguration();
+ ClientWriteOptions options = new
ClientWriteOptions().transactions(true)
+ .onDuplicate(configuration.getOnDuplicate() ==
OpenFgaWriteConflictMode.ignore
+ ? WriteRequestWrites.OnDuplicateEnum.IGNORE :
WriteRequestWrites.OnDuplicateEnum.ERROR)
+ .onMissing(configuration.getOnMissing() ==
OpenFgaWriteConflictMode.ignore
+ ? WriteRequestDeletes.OnMissingEnum.IGNORE :
WriteRequestDeletes.OnMissingEnum.ERROR);
+ if (ObjectHelper.isNotEmpty(configuration.getAuthorizationModelId())) {
+
options.authorizationModelId(configuration.getAuthorizationModelId());
+ }
+ ClientWriteRequest request = new
ClientWriteRequest().writes(writeKeys).deletes(deleteKeys);
+ authorizer.await(exchange, "write " + writes.size() + " and delete " +
deletes.size() + " tuple(s)",
+ () -> authorizer.getClient().write(request, options));
+ exchange.getMessage().setHeader(OpenFgaConstants.WRITTEN_TUPLES,
writes.size());
+ exchange.getMessage().setHeader(OpenFgaConstants.DELETED_TUPLES,
deletes.size());
+ }
+
+ private List<Tuple> mutationTuples(Object value) {
+ if (value == null) {
+ return List.of();
+ }
+ if (!(value instanceof Collection<?> collection)) {
+ throw new IllegalArgumentException("write mutation lists must be
collections of user, relation and object Maps");
+ }
+ if (collection.size() > 100) {
+ throw new IllegalArgumentException("write requires between 1 and
100 tuples across writes and deletes");
Review Comment:
⚠️ **Misleading error message in `mutationTuples()`**
This message says `"write requires between 1 and 100 tuples across writes
and deletes"`, but `mutationTuples()` only inspects a **single** collection
(either writes or deletes) — not the combined count. The combined check at line
650 uses the same wording and is correct there, but here, at the per-side
level, the message is misleading: a single side with 101 entries doesn't
violate the combined constraint.
Suggested fix:
```suggestion
throw new IllegalArgumentException("a single write mutation list
cannot exceed 100 entries");
```
##########
components/camel-openfga/src/main/docs/openfga-component.adoc:
##########
@@ -347,6 +347,38 @@ producer fails and names the part that is missing.
A typed wildcard — `user:*`, OpenFGA's "everyone" — is accepted here, because
writing that tuple is how a resource
is shared publicly and is a deliberate act by the route. It is *refused* as
the subject of a check; see below.
+=== Mutation conflicts and atomic updates
+
+By default, `onDuplicate=error` makes an existing relationship fail a write,
and `onMissing=error` makes an absent
+relationship fail a delete. On OpenFGA 1.10.0 or later, select
`onDuplicate=ignore` or `onMissing=ignore` explicitly
+for reconciliation routes that should leave an already-satisfied operation
unchanged. These options apply to
+`writeTuples` and `deleteTuples`, respectively, and both apply to `write`.
+
+Use `write` to grant and revoke relationships in one atomic request. With no
configured `user`/`relation`/`object`,
+the body is a Map containing `writes` and/or `deletes` collections. Each entry
is an ordinary Map containing exactly
+`user`, `relation` and `object`; other body shapes or entry fields are
rejected. For example:
+
+[source,java]
+------------------------------------------------------------
+Map<String, Object> changes = Map.of(
+ "writes", List.of(Map.of("user", "user:anne", "relation", "writer",
"object", "document:budget")),
+ "deletes", List.of(Map.of("user", "user:anne", "relation", "reader",
"object", "document:budget")));
+template.requestBody("openfga:write?storeId={{fga.store}}&authorizationModelId={{fga.model}}",
changes);
+------------------------------------------------------------
+
+At least one tuple is required, and the sum of writes and deletes cannot
exceed 100. Larger transactions are rejected
+before calling OpenFGA; the component never splits this atomic update into
independently committed requests. A failed
+transaction remains a failure even when `failOpen=true`. An
endpoint-configured model revision is applied to the
Review Comment:
💡 **Passive voice** — `"An endpoint-configured model revision is applied to
the transaction"`.
Consider: `"The component applies an endpoint-configured model revision to
the transaction."`
##########
components/camel-openfga/src/main/docs/openfga-component.adoc:
##########
@@ -347,6 +347,38 @@ producer fails and names the part that is missing.
A typed wildcard — `user:*`, OpenFGA's "everyone" — is accepted here, because
writing that tuple is how a resource
is shared publicly and is a deliberate act by the route. It is *refused* as
the subject of a check; see below.
+=== Mutation conflicts and atomic updates
+
+By default, `onDuplicate=error` makes an existing relationship fail a write,
and `onMissing=error` makes an absent
+relationship fail a delete. On OpenFGA 1.10.0 or later, select
`onDuplicate=ignore` or `onMissing=ignore` explicitly
+for reconciliation routes that should leave an already-satisfied operation
unchanged. These options apply to
+`writeTuples` and `deleteTuples`, respectively, and both apply to `write`.
+
+Use `write` to grant and revoke relationships in one atomic request. With no
configured `user`/`relation`/`object`,
+the body is a Map containing `writes` and/or `deletes` collections. Each entry
is an ordinary Map containing exactly
+`user`, `relation` and `object`; other body shapes or entry fields are
rejected. For example:
+
+[source,java]
+------------------------------------------------------------
+Map<String, Object> changes = Map.of(
+ "writes", List.of(Map.of("user", "user:anne", "relation", "writer",
"object", "document:budget")),
+ "deletes", List.of(Map.of("user", "user:anne", "relation", "reader",
"object", "document:budget")));
+template.requestBody("openfga:write?storeId={{fga.store}}&authorizationModelId={{fga.model}}",
changes);
+------------------------------------------------------------
+
+At least one tuple is required, and the sum of writes and deletes cannot
exceed 100. Larger transactions are rejected
+before calling OpenFGA; the component never splits this atomic update into
independently committed requests. A failed
+transaction remains a failure even when `failOpen=true`. An
endpoint-configured model revision is applied to the
+transaction. A server configured with a lower tuple limit can reject a request
within the component's limit.
+
+The configured-tuple precedence also applies to `write`: when any of
`user`/`relation`/`object` is configured, that
+triple supplies one write, the body is ignored, and no deletes are taken from
it. A missing configured part fails
+rather than falling back to the body.
+
+Successful mutations preserve the input body. `CamelOpenFgaWrittenTuples` and
`CamelOpenFgaDeletedTuples` report the
+number of submitted operations, including any skipped by the selected ignore
mode; they do not report the number of
Review Comment:
💡 **Passive voice** — `"including any skipped by the selected ignore mode"`.
Consider: `"including any that the selected ignore mode skipped"`
##########
components/camel-openfga/src/main/java/org/apache/camel/component/openfga/OpenFgaConfiguration.java:
##########
@@ -476,6 +482,30 @@ public void setMaxParallelRequests(int
maxParallelRequests) {
this.maxParallelRequests = maxParallelRequests;
}
+ public OpenFgaWriteConflictMode getOnDuplicate() {
+ return onDuplicate;
+ }
+
+ /**
+ * How writeTuples and write handle an existing relationship: error (the
default) fails the operation, while ignore
+ * leaves that relationship unchanged and continues. The ignore mode
requires OpenFGA 1.10.0 or later.
+ */
+ public void setOnDuplicate(OpenFgaWriteConflictMode onDuplicate) {
Review Comment:
💡 **Javadoc on setter instead of getter** — The convention in this file puts
Javadoc on the getter (e.g. `getMaxParallelRequests()` at line 471 carries the
description). This PR inverts the pattern: `getOnDuplicate()` and
`getOnMissing()` have no Javadoc, while their setters do.
Consider moving the Javadoc block to the getter in both cases.
##########
components/camel-openfga/src/main/docs/openfga-component.adoc:
##########
@@ -347,6 +347,38 @@ producer fails and names the part that is missing.
A typed wildcard — `user:*`, OpenFGA's "everyone" — is accepted here, because
writing that tuple is how a resource
is shared publicly and is a deliberate act by the route. It is *refused* as
the subject of a check; see below.
+=== Mutation conflicts and atomic updates
+
+By default, `onDuplicate=error` makes an existing relationship fail a write,
and `onMissing=error` makes an absent
+relationship fail a delete. On OpenFGA 1.10.0 or later, select
`onDuplicate=ignore` or `onMissing=ignore` explicitly
+for reconciliation routes that should leave an already-satisfied operation
unchanged. These options apply to
+`writeTuples` and `deleteTuples`, respectively, and both apply to `write`.
+
+Use `write` to grant and revoke relationships in one atomic request. With no
configured `user`/`relation`/`object`,
+the body is a Map containing `writes` and/or `deletes` collections. Each entry
is an ordinary Map containing exactly
+`user`, `relation` and `object`; other body shapes or entry fields are
rejected. For example:
+
+[source,java]
+------------------------------------------------------------
+Map<String, Object> changes = Map.of(
+ "writes", List.of(Map.of("user", "user:anne", "relation", "writer",
"object", "document:budget")),
+ "deletes", List.of(Map.of("user", "user:anne", "relation", "reader",
"object", "document:budget")));
+template.requestBody("openfga:write?storeId={{fga.store}}&authorizationModelId={{fga.model}}",
changes);
+------------------------------------------------------------
+
+At least one tuple is required, and the sum of writes and deletes cannot
exceed 100. Larger transactions are rejected
Review Comment:
💡 **Passive voice** — `"At least one tuple is required"` reads as passive.
Consider: `"The component requires at least one tuple and rejects an
all-empty write."`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
