gnodet-bot commented on code in PR #29838:
URL: https://github.com/apache/camel/pull/29838#discussion_r4563267463
##########
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:
💡 **Still open — 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."`
##########
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:
⚠️ **Still open — Misleading error message in `mutationTuples()`**
`mutationTuples()` checks only a single collection (`collection.size() >
100`), but the error message says "across writes and deletes". That combined
constraint is checked in `write()` at the call site — this per-side check has a
different invariant.
```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
+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:
💡 **Still open — Passive voice**: `"including any skipped by the selected
ignore mode"`.
Consider: `"including any that the selected ignore mode skipped"`
##########
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.
Review Comment:
💡 **Still open — 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/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) {
+ this.onDuplicate = onDuplicate;
+ }
Review Comment:
💡 **Still open — Javadoc on setter instead of getter** — The convention in
this file puts Javadoc on the getter (e.g. `getMaxParallelRequests()` carries
the description). This PR inverts the pattern: `getOnDuplicate()` and
`getOnMissing()` have no Javadoc, while their setters do.
Move the Javadoc block to the getter in both cases.
--
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]
