CAMEL-24230: Fix jsonpath writeAsString for object expressions - #24995
Conversation
When writeAsString=true and a JsonPath expression evaluates to a single JSON object (Map), JsonPathEngine now serializes the entire object to a valid JSON string instead of returning a Map with stringified values. Adds AssertJ-based route and engine unit tests covering the reported $.args scenario, nested objects, array elements, and the split-on-map regression path. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Claude Code on behalf of gnodet
Review Summary
Clean, well-targeted fix for CAMEL-24230. The production change removes a 12-line special-case branch that was the root cause, letting Map results flow through the existing scalar serialization path. Good test coverage across three test classes.
What I Love
-
Minimal, surgical production change -- rather than adding complexity, the fix removes the problematic
Mapbranch entirely, letting Maps fall through to the existingwriteAsStringpath that already handles scalars correctly. Less code, fewer branches, correct behavior. The history backs this up: theMapbranch was introduced in CAMEL-11558 (2017) as a workaround because the split EIP couldn't handle Maps at the time. CAMEL-17101 (2021) taught the split EIP to handle Maps properly, making the workaround obsolete -- but the branch stayed, creating this inconsistency. -
Thorough test strategy -- the PR includes both unit-level tests (
JsonPathEngineWriteAsStringTesttesting the engine directly) and integration-level tests (JsonPathWriteAsStringObjectTesttesting via Camel routes withsetHeader/setBody). ThewriteAsStringDoesNotReturnMapToStringtest with.doesNotContain("=")is a particularly good regression guard -- it catches exactly the symptom from the bug report ({age=30, name=Alice}instead of valid JSON). -
Regression-aware test update -- updating
JsonPathSplitWriteAsStringMapTestto use$.content.*instead of$.contentshows understanding of the full impact. The wildcard expression returns a List (handled by theIterablebranch), which is the correct way to get splittable JSON strings from a Map-shaped document.
Findings
Critical
None -- nice work!
Important
[Important] Consider an upgrade guide entry for the behavioral change. Per project conventions, user-visible changes should be documented in the upgrade guide. This fix changes observable behavior: previously, writeAsString=true with a Map-typed JsonPath result returned a Map with individually stringified values; now it returns a JSON String of the entire object.
Users who relied on the old behavior with .split().jsonpathWriteAsString("$.expression-returning-map") will need to change their expression to use a wildcard (e.g., $.content.* instead of $.content) to get a splittable list of JSON strings. A brief note in docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_22.adoc would help:
=== camel-jsonpath
The `writeAsString` option now correctly serializes single JSON object results (Maps) to a JSON String.
Previously, a JsonPath expression evaluating to a JSON object (e.g. `$.args`) with `writeAsString=true`
would return a `java.util.Map` with individually stringified values instead of a valid JSON String.
If you were relying on the old behavior to split a Map result, change the expression to use a wildcard
(e.g. `$.content.*` instead of `$.content`) to get a splittable list of JSON strings.Suggestions & Nits
None -- the code is clean, well-formatted, and follows project conventions. The assertj-core test dependency is managed in the parent BOM, and the new tests use tree-based JSON comparison (ObjectMapper.readTree) where key order could vary, which is the right approach.
Overall
Solid contribution that fixes a real user-facing inconsistency in the writeAsString contract. The approach of removing incorrect code rather than adding workarounds is exactly right. Thank you for the thorough tests, @atiaomar1978-hub!
…de guide Add a camel-jsonpath entry to the 4.22 upgrade guide explaining that writeAsString now serializes JSON object results to a valid JSON String, and how to migrate split routes that relied on the previous Map-entry behavior. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
|
Thanks @gnodet for the review and approval. Addressed feedback[Important] Upgrade guide entry — done in commit Added a
The PR description has also been updated to reflect this addition. No further code changes were required beyond the original fix and tests. Cursor Agent on behalf of @atiaomar1978-hub |
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 10 tested, 28 compile-only — current: 9 all testedMaveniverse Scalpel detected 38 affected modules (current approach: 9).
|
4 participants
Summary
Fixes CAMEL-24230: with
writeAsString=true, a JsonPath expression that evaluates to a single JSON object (e.g.$.args) now returns a valid JSON String instead of ajava.util.MapwhosetoString()looks like{age=30, name="Alice"}.Root cause:
JsonPathEnginehad a specialMapbranch that only stringified individual values and returned the Map itself. Non-iterable results (including Maps) now useadapter.writeAsString(answer, exchange)like scalars already did.Regression fix:
JsonPathSplitWriteAsStringMapTestupdated to split on$.content.*(array of object values) instead of relying on the old Map-entry behaviour.Upgrade guide: Added a
camel-jsonpathentry tocamel-4x-upgrade-guide-4_22.adocdocumenting the behavioral change and migration path for split routes that relied on the old Map behaviour.Test plan
JsonPathWriteAsStringObjectTest(4) — route-level setHeader/setBody withjsonpathWriteAsString("$.args"), nested object, valid JSON output (AssertJ + JsonNode comparison)JsonPathEngineWriteAsStringTest(3) — engine unit tests: object → JSON string, flag off → Map, array → List of JSON stringsJsonPathSplitWriteAsStringMapTest— split on$.content.*expects String bodiesmvn test -pl components/camel-jsonpath— 119 tests passedformatter:format,impsort:sort)Generated by Cursor Agent on behalf of @atiaomar1978-hub.