Repository navigation
CAMEL-25481: camel-dataweave - convert XML namespaces, attributes and the DataSonnet XML model - #27612
Conversation
… the DataSonnet XML model The DataWeave to DataSonnet conversion now follows the DataWeave XML semantics. XML input: a selector matches an element by its local name whatever its namespace prefix (such as payload.Envelope.Body of a SOAP message), .ns#name only in the namespace of an ns header directive, .@ gives all the attributes and ..* all the descendants, an empty element is null, and the object functions see the child elements in document order. An output other than XML writes the elements without the attributes, namespace declarations and positions of the DataSonnet XML model (dw.output). XML output: attributes (key @(name: value)), namespace-qualified keys (ns0#key), null as an empty element, arrays, object spreads and keys that repeat as elements that repeat (in the order of the script), elements of the input with their attributes and namespaces, and the writeDeclaration and skipNullOn writer properties. DataSonnet does not keep the value of a function argument (each use evaluates it again), so nested calls of the camel-dataweave.libsonnet functions took exponential time in the depth of the nesting. The functions now evaluate their arguments once. The corpus has 29 new XML entries, verified with the DataWeave CLI, and compares XML outputs without formatting. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet-bot
left a comment
There was a problem hiding this comment.
Two minor style findings on this DataWeave conversion PR.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 10 of 704 tested, 25 compile-only — current: 10 all testedMaveniverse Scalpel detected 10 affected modules (current approach: 10). Skip-tests mode would test 10 modules (4 direct + 8 downstream), skip tests for 25 (generated code, meta-modules) Modules Scalpel would test (10)
Modules with tests skipped (25)
All tested modules (37 modules, 7m 9s total)Total reactor time: 7m 9s
Top 20 slowest modules:
|
…e guide note Review feedback. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
gnodet-bot
left a comment
There was a problem hiding this comment.
Both prior findings are addressed in the new commits. The XML namespace/attribute support looks solid: strict() memoization is correct (forces evaluation once via std.foldl init), namespace scoping via inScope propagates declarations through selector chains correctly, and the 29 new XML corpus entries provide thorough coverage (SOAP envelopes, namespace-qualified selectors, XML-to-XML passthrough, repeated elements, skipNullOn variants, writeDeclaration). No issues found.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
oscerd
left a comment
There was a problem hiding this comment.
Strong PR and a legitimate follow-on to the 4.23 fail-fast work (CAMEL-25323 / 25324 / 25471). CI is green on head 6a72ee49, the corpus additions are substantial, and the tests would not even compile against main — so this is well pinned.
Requesting changes for one narrow but real silent-wrong-result bug. It is a one-line fix plus a corpus entry. Everything else below is non-blocking.
I did not rely on the diff alone for these: I extracted datasonnet-mapper-3.0.1.3, ran this PR's own camel-dataweave.libsonnet (fetched at the head SHA) against sample XML through com.datasonnet.Mapper, and compiled the 5-file camel-dataweave converter package standalone to inspect generated scripts. The findings are reproduced, not inferred.
Blocking — CDATA adjacent to plain text silently duplicates the text
camel-dataweave.libsonnet:52-53 (textOf):
local textOf(x) = strict(std.join('', [x[k] for k in std.objectFields(x) if std.startsWith(k, '$')]), ...The DataSonnet reader emits both a collated $ and numbered $1/$3 segments when an element has several text segments and no child elements. Verified raw model:
<root><b>x<![CDATA[y]]>z</b></root>
-> {"$":"xyz","$1":"x","#2":"y","$3":"z","~":1}
textOf joins all three $-prefixed keys:
| expression | actual | expected |
|---|---|---|
payload.root.b |
"xyzxz" |
"xyz" |
dw.fromXml(payload.root) |
{"b":"xyzxz"} |
{"b":"xyz"} |
dw.entries(...b) |
3 __text entries |
2 |
The entries fallout propagates to keysOf, sizeOf, pluck, entriesOf, mapObject, groupBy, orderBy. Comments and entity references do not trigger it (verified: <b>x<!--c-->y</b> → "xy", a&b → "a&b") — only CDATA mixed with text, which is common in SOAP payloads.
This is exactly the failure class the PR sets out to eliminate ("converted without error but gave wrong results"), which is why I'd rather it not ship. Prefer the collated key when present:
local textOf(x) = strict(if std.objectHas(x, '$') then x['$']
else std.join('', [x[k] for k in std.objectFields(x) if std.startsWith(k, '$')]),
function(t) if t == '' then null else t),and in entries (:87-89) drop the collated $ when numbered segments exist.
Corpus XM15 covers only pure CDATA (<a><![CDATA[x < y]]></a>), which passes — hence the miss. Worth adding <a>x<![CDATA[y]]>z</a>.
Important (non-blocking)
Two namespaces, same local name: .*name and ..*name return only the first prefix. childKey (:60), used by multiRaw:155 and descendants:163, returns the first field whose local name matches:
<root xmlns:p="urn:p" xmlns:q="urn:q"><p:item>1</p:item><q:item>2</q:item></root>
dw.multi(root,'item') -> ["1"] // drops q:item
dw.descAll(payload,'item') -> ["1"] // drops q:item
dw.entries(root) -> [item, item] // but this path sees both
This contradicts the PR's own doc ("a selector matches an element by its local name whatever its namespace prefix", ".*field all of them") and is internally inconsistent with the entries path. A childKeys(x,k) returning every matching field, flattened, would fix it. No corpus entry covers two prefixes sharing a local name.
An empty array disappears once the object goes through xmlObject. fieldsOf flattens [] to zero entries, while ordered's own comment says "an empty array is written as an empty element":
dw.xmlOutput({root: {a: [], b: 1}}, {}) -> <root><a></a><b>1</b></root> correct
dw.xmlOutput({root: dw.xmlObject([{a: []},{b: 1}])}, {}) -> <root><b>1</b></root> key lost
xmlObject engages as soon as the object also has a spread or a repeated key. XM32 covers ea: [] on the plain path and XM27 has a spread but only non-empty arrays, so the combination is untested. Mapping an empty array to a single {k: f.k, v: null} entry would do it.
Nested lambda params with the same name clear the rawNames flag early. In DataWeaveConverter.java, rawNames is a flat Set<String>; emitLambda/emitImplicitLambda add then remove without saving prior membership, and jsonnetName doesn't uniquify across nesting. Reproduced with the standalone-compiled converter:
payload.*order map ((o) -> { id: o.@id, v: o })
-> function(o, _1) { id: dw.attr(o, "id"), v: dw.text(o) } correct
payload.*order map ((o) -> { items: (o.*item map ((o) -> o.@sku)), v: o })
-> function(o, _1) { items: (...), v: o } dw.text(o) lost
The bare v: o then leaks the raw DataSonnet element object (with ~, @…) into JSON. Obscure — needs a shadowing same-named inner lambda that also reads attributes — but silently wrong. Save/restore prior membership, or use a counting multiset.
Suggestions
- Namespace prefixes with
-or.fail with a misleading error.DataWeaveLexerNAMESPACE_DIRECTIVEuses[A-Za-z_][A-Za-z0-9_]*, narrower than XML NCName. Verified:ns my-ns http://example.com/x→expected STRING but found MINUS ('-'). Fail-fast so no wrong output, but the message doesn't mention namespace prefixes. Widening to[A-Za-z_][A-Za-z0-9_.-]*is cheap. - Untested: default (unprefixed) namespace on input.
selNsRaw:144maps a colon-less field to prefix'$', which I verified is correct — but no corpus entry hasxmlns="…", so the branch is unexercised. - [Nit]
keyNameusesprefix#namewhilestaticNameusesprefix:name. Harmless today, but a literal key"a#b"would collide withQName(a,b)inhasRepeatedKey. - [Nit]
DescendantSelector's 2-arg compat constructor is package-private whileObjectEntry's ispublic— worth making consistent.
Security: no script-injection issue (checked specifically)
The only input-derived values spliced into generated DataSonnet source are the namespace prefix and URI from the script's own ns header — i.e. the route's script, trusted under Camel's threat model. They also pass through DataWeaveConverter.string() (escapes ", \, control chars) or isJsonnetIdentifier(). Untrusted message XML — element names, namespace URIs, attribute names — never reaches generated source; it is handled at runtime by the libsonnet against the already-parsed object model, with no eval or parseJson of message data. Clean.
Also worth noting the new canonicalXml test comparator sets disallow-doctype-decl=true — good XXE hygiene in a helper that could easily have skipped it.
Test strength: would they fail without the change? Decisively yes
DataWeaveParserTest asserts AST types that don't exist on main (AllAttributes, QName, QualifiedFieldAccess, DescendantSelector.multi()) — it wouldn't compile. DataWeaveConverterTest asserts exact generated text for functions that don't exist before. The corpus harness asserts assertThrows(DataWeaveConversionException…) for @unsupported entries, so the 29 new XML entries would throw on main. The coverage gaps map exactly onto findings 1, 2, 3 and the default-namespace note.
Checklist: docs ✅ · catalog mirror ✅ (blob hashes identical on both sides) · upgrade guide ✅ correctly amends the existing "DataWeave auto-conversion now fails fast" 4.23 entry rather than adding a feature note · commit convention ✅ · public-API compat ✅ all churn is within unreleased 4.23 · no FQCNs ✅ (scripted scan of added Java lines: 0) · no Thread.sleep ✅ · assertion style ✅ · CI green on head SHA ✅ · both gnodet-bot threads resolved ✅
Two process notes: the PR has no human approval yet — only COMMENTED from gnodet-bot (AI) and from you as author — and CAMEL-25481 has empty fixVersions, which must be set to 4.23.0 before resolving, since it can't be set once closed.
Could not verify: that DataWeave 2.12 itself returns both elements for .*item across two prefixes — I didn't run the dw CLI. The finding stands on its own, since it contradicts this PR's documented rule and its own entries path. Also could not get a clean strict() memoisation timing (my harness timed out at depth 15, consistent with the exponential-blowup claim).
Reviewed with Claude Code (Claude Opus 5) on behalf of @oscerd. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying. It is a rules-and-conventions review and does not replace CodeRabbit, Sourcery, or SonarCloud.
…h the same local name, empty arrays and nested lambdas Review fixes: - The text of an element with text and CDATA segments was read twice (the reader also gives it in '$'); adjacent text and CDATA in mixed content are one __text. - .*name and ..*name give the elements of every namespace with that local name, in document order (and .name the first in document order). - An empty array is an empty element also in an object with a spread or a repeated key. - A lambda parameter kept as an XML element for its attributes is tracked in its scope, so an inner lambda with a parameter of the same name does not change it. Five corpus entries for these, verified with the DataWeave CLI. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
Thanks @oscerd. All of these reproduced, and I checked each against the DataWeave CLI (
camel-dataweave: 101 tests pass. camel-datasonnet: 450 tests pass, including 365 corpus entries. Claude Code on behalf of davsclaus |
Summary
CAMEL-25481: the DataWeave to DataSonnet conversion (camel-dataweave, with the
camel-dataweave.libsonnetruntime library of camel-datasonnet) now follows the DataWeave XML semantics. Before this change, XML namespaces and attributes in the output failed the conversion. Worse, several XML input cases converted without error but gave wrong results: a SOAP selector such aspayload.Envelope.Bodyreturnednull, andpayload.orderwritten as JSON included the attributes, namespace declarations and~positions of the DataSonnet XML model.XML input
.*nameand..*namegive the elements of every namespace with that local name, in document order.nsheader directives are supported, and.ns#namematches only in that namespace (payload.s#Envelope.o#Bodyisnull, as in DataWeave)..@gives all the attributes, and..*nameall the descendants (..nametakes the first of an element that repeats).null..nameof an element that repeats gives the first,.*namegives all of them. Text next to CDATA is read once, and adjacent text and CDATA in mixed content are one__text.o.@id,$.@id).keysOf,sizeOf,mapObject,pluck,entriesOf(with the attributes),groupBy,orderBy,-,--and{ (x) }see the child elements in document order.dw.output).XML output
key @(name: value): a null attribute is written as"null", and is left out withskipNullOn="attributes".ns0#keyand attributes, declared on the root element. An undeclared prefix fails the conversion.nulland an empty array are written as an empty element.{ items: { (payload.items map { item: $.sku }) } }used to keep only the last item.writeDeclarationandskipNullOn(elements,attributes,everywhere) are supported.indent,encodingandinlineCloseOnare formatting only.Performance fix in the runtime library
DataSonnet 3.0.1.3 does not memoize lazy values:
Val.Lazy.force()uses a locallazy val, so every use of a function argument, and every access to an array element, evaluates it again. Nested calls of library functions therefore took exponential time in the depth of the nesting. A 4-level SOAP selection did not finish within 20 s. With 300 items,map,filterandorderBytook more than 120 s with the library onmain; it now takes 0.6 s. Every library function that uses an argument more than once now evaluates it once (strict, through the initial value ofstd.foldl, which DataSonnet does evaluate only once).orderBycomputes each key once.Tests
@unsupportedor?. All 46 XML entries match the DataWeave CLI (dw2.12).DataWeaveCorpusTestcompares XML outputs without formatting or namespace-declaration placement.camel-dataweave: 101 tests pass.camel-datasonnet: 450 tests pass, including 365 corpus entries.Review fixes
&&/||conditions parenthesized, and the upgrade guide note rewrapped..*and..*, an empty array lost in an object with a spread, and nested lambdas with the same parameter name. Each one has a corpus entry (XM33 to XM37).Known differences (documented)
A DataSonnet object cannot repeat a key. So in a JSON or Java output, elements that repeat under one name, and mixed-content
__text, become an array, where DataWeave writes the key repeatedly. The XML output does write repeated elements...@is still not converted: the conversion fails.Claude Code on behalf of davsclaus
🤖 Generated with Claude Code