davsclaus commented on code in PR #27535:
URL: https://github.com/apache/camel/pull/27535#discussion_r4216654766
##########
components/camel-dataweave/src/main/java/org/apache/camel/component/dataweave/DataWeaveConverter.java:
##########
@@ -253,8 +263,23 @@ private String emitFieldAccess(FieldAccess fa) {
}
private String emitMultiValueSelector(MultiValueSelector mv) {
+ needsCamelLib = true;
String collection = emitNode(mv.object());
- return "std.map(function(x) x." + mv.field() + ", " + collection + ")";
+ // DataWeave .*field collects all child elements named 'field'
(handling one-or-many XML children).
+ // Translated to c.multiValue(collection, 'field') which gracefully
handles both cases.
+ return "c.multiValue(" + collection + ", \"" + mv.field() + "\")";
+ }
+
+ 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? returns true/false; DataSonnet: use != null check.
+ needsCamelLib = true;
+ return "c.exists(" + emitNode(ec.expr()) + ")";
Review Comment:
Two problems here:
1. `c.exists` is not defined in `camel.libsonnet`, so this fails at
evaluation.
2. Even with a helper, the argument is evaluated first: in Jsonnet `body.a`
on a missing field is an error, so the call can never return `false`.
DataWeave's key-present selector (`keyName?`, `keyName.@type?`) returns
`true`/`false` ([selectors
docs](https://docs.mulesoft.com/dataweave/latest/dataweave-selectors)).
When `ec.expr()` is a `FieldAccess` or `AttributeAccess`, this can be
emitted as `std.objectHas(<object>, "a")` (or `"@type"`). For other shapes,
counting it as a TODO is safer than emitting something that throws.
##########
components/camel-dataweave/src/main/java/org/apache/camel/component/dataweave/DataWeaveConverter.java:
##########
@@ -253,8 +263,23 @@ private String emitFieldAccess(FieldAccess fa) {
}
private String emitMultiValueSelector(MultiValueSelector mv) {
+ needsCamelLib = true;
String collection = emitNode(mv.object());
- return "std.map(function(x) x." + mv.field() + ", " + collection + ")";
+ // DataWeave .*field collects all child elements named 'field'
(handling one-or-many XML children).
+ // Translated to c.multiValue(collection, 'field') which gracefully
handles both cases.
+ return "c.multiValue(" + collection + ", \"" + mv.field() + "\")";
Review Comment:
`c` is `camel.libsonnet`, which has no `multiValue` function (and `CML` has
none either), so this fails at evaluation with a missing-field error. The
previous `std.map(function(x) x.Item, ...)` output at least worked when the
collection was an array.
If the goal is one-or-many XML children, the helper needs to be added to
`camel.libsonnet` (with a docs row in the `c.` function table), for example
something like `multiValue(v, f):: if std.isArray(v) then
std.flatMap(function(x) if std.objectHas(x, f) then (if std.isArray(x[f]) then
x[f] else [x[f]]) else [], v) else ...`, and covered by an executed test with
both a single child and several children.
##########
components/camel-dataweave/src/main/java/org/apache/camel/component/dataweave/DataWeaveConverter.java:
##########
@@ -214,8 +220,12 @@ private String emitNode(DataWeaveAst node) {
}
private String emitStringLit(StringLit s) {
- // The lexer preserves escape sequences as-is, so don't double-escape
- return "\"" + s.value().replace("\"", "\\\"") + "\"";
+ // The lexer preserves escape sequences verbatim (e.g. \" is stored as
\").
+ // Wrapping in double-quotes directly is correct; do NOT replace \"
with \\\"
+ // as that would double-escape an already-escaped sequence.
+ // DataWeave string interpolation "Hello $(expr)" -> DataSonnet "Hello
%(expr)"
+ String value = s.value().replace("$(", "%(");
Review Comment:
Jsonnet has no in-string interpolation. `%(name)` only has a meaning as a
format spec of the `%` operator / `std.format`, and it needs a conversion
character plus an object of values ([Jsonnet tutorial, string
formatting](https://jsonnet.org/learning/tutorial.html)). So `"Hello
%(payload.name)"` is just a literal, and the user gets the text `Hello
%(payload.name)` instead of the name, with no error. The embedded expression is
not converted either: the test asserts `%(payload.name)`, while the description
says `%(body.name)`.
A faithful conversion would parse each `$(...)` as an expression and emit
concatenation, e.g. `"Hello " + std.toString(body.name)`. Until then, counting
interpolated strings as a TODO would make it fail loudly, especially once
#27552 turns TODOs into errors.
##########
components/camel-dataweave/src/main/java/org/apache/camel/component/dataweave/DataWeaveConverter.java:
##########
@@ -214,8 +220,12 @@ private String emitNode(DataWeaveAst node) {
}
private String emitStringLit(StringLit s) {
- // The lexer preserves escape sequences as-is, so don't double-escape
- return "\"" + s.value().replace("\"", "\\\"") + "\"";
+ // The lexer preserves escape sequences verbatim (e.g. \" is stored as
\").
+ // Wrapping in double-quotes directly is correct; do NOT replace \"
with \\\"
+ // as that would double-escape an already-escaped sequence.
+ // DataWeave string interpolation "Hello $(expr)" -> DataSonnet "Hello
%(expr)"
+ String value = s.value().replace("$(", "%(");
+ return "\"" + value + "\"";
Review Comment:
The lexer reads both `"..."` and `'...'` with `readString(quote, ...)` and
keeps the content verbatim. For a single-quoted DataWeave string that contains
a bare double quote, e.g. `'say "hi"'`, removing the escape now emits `"say
"hi""`, which is not valid Jsonnet. Escaping only the `"` characters that are
not already preceded by a backslash, or passing the original quote character
through `StringLit`, would keep both cases right. A test with a single-quoted
string would cover it.
##########
components/camel-dataweave/src/main/java/org/apache/camel/component/dataweave/DataWeaveConverter.java:
##########
@@ -149,6 +149,12 @@ private String emitNode(DataWeaveAst node) {
return emitNode(ia.object()) + "[" + emitNode(ia.index()) + "]";
} else if (node instanceof MultiValueSelector mv) {
return emitMultiValueSelector(mv);
+ } else if (node instanceof AttributeAccess aa) {
+ return emitAttributeAccess(aa);
+ } else if (node instanceof ExistenceCheck ec) {
+ return emitExistenceCheck(ec);
+ } else if (node instanceof DoubleDollar) {
+ return "acc"; // $$ is the reduce accumulator; use the 'acc' name
convention
Review Comment:
This only works when the surrounding lambda happens to name its accumulator
`acc`. For the shorthand form from the description, `payload.items reduce ($$ +
$.price)`, `parseLambda()` now throws on `$$` and the fallback parses `($$ +
$.price)` as a plain expression. `emitReduce` then takes its non-`Lambda`
branch and emits roughly `std.foldl(acc + function(x) x.price, body.items,
null)`: `acc` is unbound and the first argument is not a function.
DataWeave's shorthand binds `$` to the item and `$$` to the accumulator, and
without an initial value the first element is the starting accumulator ([reduce
docs](https://docs.mulesoft.com/dataweave/latest/dw-core-functions-reduce)). So
the faithful form is `std.foldl(function(acc, item) acc + item.price, arr[1:],
arr[0])`, with an empty-array guard. Note that `testDoubleDollarInReduce` uses
an explicit `(item, acc = 0) ->` lambda, so it does not exercise `$$` at all.
##########
components/camel-dataweave/src/main/java/org/apache/camel/component/dataweave/DataWeaveParser.java:
##########
@@ -85,9 +94,47 @@ private DataWeaveAst.Header parseHeader() {
} else if (checkIdentifier("import")) {
// Skip import directives
while (!check(TokenType.EOF) && !checkIdentifier("output") &&
!checkIdentifier("input")
+ && !checkIdentifier("var") && !checkIdentifier("fun")
&& !check(TokenType.HEADER_SEPARATOR)) {
advance();
}
+ } else if (checkIdentifier("var")) {
+ // var declaration in header: emit as local binding in body
+ advance(); // var
+ String name = current().value();
+ advance(); // name
+ expect(TokenType.ASSIGN); // =
+ DataWeaveAst value = parseOr();
+ declarations.add(new DataWeaveAst.VarDecl(name, value, null));
+ } else if (checkIdentifier("fun")) {
+ // fun declaration in header: emit as local function in body
+ advance(); // fun
+ String name = current().value();
+ advance(); // name
+ expect(TokenType.LPAREN);
+ List<String> params = new ArrayList<>();
+ while (!check(TokenType.RPAREN) && !check(TokenType.EOF)) {
+ String paramName = current().value();
+ advance();
+ // Skip type annotation: fun f(a: Number) -> skip ":
Number"
Review Comment:
Skipping exactly one token after `:` handles `a: Number` but not `a:
Array<Number>`, `a: String | Null` or `a: {name: String}`. Here (and in
`parseFunDecl` at line 211) the remaining tokens become parameter names, which
gives a silently wrong function. Skipping a balanced type expression up to the
next `,` / `)` at depth 0, or throwing so the construct is reported as
unsupported, would avoid that.
--
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]
