gnodet-bot commented on code in PR #27535:
URL: https://github.com/apache/camel/pull/27535#discussion_r4222142644
##########
components/camel-dataweave/src/main/java/org/apache/camel/component/dataweave/DataWeaveConverter.java:
##########
@@ -254,9 +281,36 @@ private String emitFieldAccess(FieldAccess fa) {
private String emitMultiValueSelector(MultiValueSelector mv) {
String collection = emitNode(mv.object());
+ // DataWeave .*field collects all values for key 'field' from each
element of the collection.
+ // In DataSonnet/Jsonnet: std.map(function(x) x.<field>, collection).
+ // Note: camel.libsonnet has no multiValue helper, so we emit directly.
return "std.map(function(x) x." + mv.field() + ", " + collection + ")";
}
+ private String emitAttributeAccess(AttributeAccess aa) {
+ // DataWeave .@attr accesses an XML attribute; in DataSonnet XML
attributes are exposed
+ // as object keys prefixed with '@', e.g. body.Order['@id'].
+ return emitNode(aa.object()) + "[\"@" + aa.attribute() + "\"]";
+ }
+
+ private String emitExistenceCheck(ExistenceCheck ec) {
+ // DataWeave expr? (key-present selector) returns true/false.
+ // In Jsonnet, accessing a missing field raises an error, so we cannot
simply wrap the expression.
+ // For FieldAccess and AttributeAccess we can use std.objectHas(obj,
"key") safely.
+ // For other shapes the semantics cannot be faithfully reproduced
without a helper; emit as TODO.
+ if (ec.expr() instanceof FieldAccess fa) {
+ return "std.objectHas(" + emitNode(fa.object()) + ", \"" +
fa.field() + "\")";
+ }
+ if (ec.expr() instanceof AttributeAccess aa) {
+ return "std.objectHas(" + emitNode(aa.object()) + ", \"@" +
aa.attribute() + "\")";
+ }
+ todoCount++;
+ return includeComments
+ ? "/* TODO: manual conversion needed -- existence check on
non-field expression: "
+ + emitNode(ec.expr()) + "*/\nfalse"
+ : "false";
+ }
Review Comment:
🐛 **`*/` injection in `emitExistenceCheck`**
`emitNode(ec.expr())` is concatenated raw into a `/* … */` block comment. If
the emitted expression contains the sequence `*/`, the comment closes early and
the remainder becomes dangling source text that will break the generated
Jsonnet.
The same PR already fixes this in `emitStringLit` (`.replace("*/", "* /")`);
the same guard is needed here:
```suggestion
return includeComments
? "/* TODO: manual conversion needed -- existence check on
non-field expression: "
+ emitNode(ec.expr()).replace("*/", "* /") + "*/\nfalse"
: "false";
```
##########
components/camel-dataweave/src/main/java/org/apache/camel/component/dataweave/DataWeaveConverter.java:
##########
@@ -479,7 +533,110 @@ private String emitReduce(ReduceExpr re) {
+ collection + ", " + init + ")";
}
}
- return "std.foldl(" + emitNode(re.lambda()) + ", " + collection + ",
null)";
+ // Shorthand form: payload.items reduce ($$ + $.price)
+ // $$ is the accumulator ($$ -> acc) and $ is the current item ($ ->
item).
+ // DataWeave shorthand without an explicit initial value uses the
first element
+ // as the starting accumulator: std.foldl(function(acc, item) body,
arr[1:], arr[0]).
+ String body = emitReduceShorthandBody(re.lambda());
+ return "(local _arr = " + collection + ";\n"
+ + "if std.length(_arr) == 0 then null else
std.foldl(function(acc, item) " + body + ", _arr[1:], _arr[0]))";
+ }
+
+ /**
+ * Emit a reduce shorthand body, rewriting {@code $$} to {@code acc} and
{@code $} (optionally with field access) to
+ * {@code item} or {@code item.field}. Falls back to normal {@code
emitNode} for any sub-expression that doesn't
+ * contain shorthand references.
+ */
+ private String emitReduceShorthandBody(DataWeaveAst node) {
+ if (node instanceof DoubleDollar) {
+ return "acc";
+ }
+ if (node instanceof LambdaShorthand ls) {
+ if (ls.fields().isEmpty()) {
+ return "item";
+ }
+ return "item." + String.join(".", ls.fields());
+ }
+ if (node instanceof BinaryOp op) {
+ String left = emitReduceShorthandBody(op.left());
+ String right = emitReduceShorthandBody(op.right());
+ return switch (op.op()) {
+ case "++" -> left + " + " + right;
+ case "and" -> left + " && " + right;
+ case "or" -> left + " || " + right;
+ default -> left + " " + op.op() + " " + right;
+ };
+ }
+ if (node instanceof Parens p) {
+ return "(" + emitReduceShorthandBody(p.expr()) + ")";
+ }
+ if (node instanceof FieldAccess fa) {
+ return emitReduceShorthandBody(fa.object()) + "." + fa.field();
+ }
+ if (node instanceof IndexAccess ia) {
+ return emitReduceShorthandBody(ia.object()) + "[" +
emitReduceShorthandBody(ia.index()) + "]";
+ }
+ if (node instanceof UnaryOp op) {
+ return switch (op.op()) {
+ case "not" -> "!" + emitReduceShorthandBody(op.operand());
+ default -> op.op() + emitReduceShorthandBody(op.operand());
+ };
+ }
+ // For anything else: if the sub-expression contains a shorthand
reference ($ or $$)
+ // that emitNode cannot rewrite, emit a TODO to avoid silently
producing wrong code.
+ // Pure literals and identifiers without shorthand references are safe
to emit normally.
+ if (containsShorthand(node)) {
+ todoCount++;
+ return includeComments
+ ? "/* TODO: manual conversion needed -- reduce shorthand
in unsupported context: "
+ + node.getClass().getSimpleName() + "*/\nnull"
+ : "null";
+ }
Review Comment:
💡 **Missing space before `*/` in reduce-shorthand TODO (style)**
`node.getClass().getSimpleName()` is safe here (Java class names cannot
contain `*/`), but for consistency with the other TODO comments in this file
(which all have a space before `*/`), consider adding one:
```suggestion
return includeComments
? "/* TODO: manual conversion needed -- reduce shorthand
in unsupported context: "
+ node.getClass().getSimpleName() + " */\nnull"
: "null";
```
##########
components/camel-dataweave/src/main/java/org/apache/camel/component/dataweave/DataWeaveConverter.java:
##########
@@ -593,7 +753,7 @@ private String emitUnsupported(Unsupported u) {
todoCount++;
convertedCount--;
return includeComments
- ? "// TODO: manual conversion needed -- " + u.reason() + ": "
+ u.originalText() + "\nnull"
+ ? "/* TODO: manual conversion needed -- " + u.reason() + ": "
+ u.originalText() + " */\nnull"
: "null";
}
Review Comment:
🐛 **`*/` injection in `emitUnsupported`**
This method was switched from a single-line `//` comment to a `/* … */`
block comment, but `u.originalText()` (raw DataWeave source) and `u.reason()`
are still concatenated without sanitization. DataWeave source can trivially
contain the sequence `*/` (e.g. `1 */ 2` in a malformed expression), which
would close the comment early and corrupt the generated Jsonnet.
Apply the same guard used in `emitStringLit`:
```suggestion
? "/* TODO: manual conversion needed -- " +
u.reason().replace("*/", "* /") + ": " + u.originalText().replace("*/", "* /")
+ " */\nnull"
```
--
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]