CAMEL-24064: Fix module registration in jackson3 type converter and wrap JacksonException in data type transformers#24705
Conversation
…rap JacksonException in data type transformers
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Clean, well-targeted fix for two genuine regressions in the Jackson 2 → Jackson 3 port.
Module registration fix (JacksonTypeConverters.java): The broken code called JsonMapper.builder().addModule(module).build() inside the loop, creating a fresh mapper per iteration and discarding previously registered modules. The fix correctly accumulates modules on a single JsonMapper.Builder instance before calling build() once — matches the Jackson 3 idiomatic pattern.
JacksonException wrapping (JsonPojoDataTypeTransformer / JsonStructDataTypeTransformer): In Jackson 3, JacksonException extends RuntimeException (not IOException as in Jackson 2), so the existing IOException catch clauses missed Jackson parse errors. The added JacksonException catches are correct and consistent with how JsonDataTypeTransformer (the third transformer in the same package) already handles it.
Test coverage is solid — the multi-module test verifies both custom modules are active, and the malformed-JSON tests confirm CamelExecutionException wrapping.
Minor observation (non-blocking): the IOException catch in JsonPojoDataTypeTransformer is now only reachable via getBodyAsStream() I/O errors, not Jackson parse errors. This is technically correct but could be clarified with a comment in a future cleanup.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @gnodet
oscerd
left a comment
oscerd
left a comment
There was a problem hiding this comment.
Thanks Federico — both fixes look correct, and I like that each is backed by a test that fails before / passes after.
Fix 1 — module registration (JacksonTypeConverters): good catch. The old loop rebuilt the mapper on every iteration (mapper = JsonMapper.builder().addModule(module).build();), so only the last discovered module actually survived. Hoisting a single JsonMapper.Builder and calling build() once restores the Jackson-2 registerModule semantics, and JacksonConversionsMultipleModulesTest (Foo + Bar) is exactly the test that would have caught the regression.
Fix 2 — JacksonException in the transformers: correct. In Jackson 3, tools.jackson.core.JacksonException extends RuntimeException (vs Jackson 2's JsonProcessingException extends IOException), so malformed JSON was slipping past the checked-only catch. Adding it to the multi-catch and wrapping in CamelExecutionException(..., exchange, e) preserves the original cause and matches the sibling JsonDataTypeTransformer.
One trivial, non-blocking nit: JsonPojoDataTypeTransformer keeps IOException in its catch list while JsonStructDataTypeTransformer doesn't — for the Jackson read path it's now effectively dead (only kept legal by getJavaObject's throws IOException). Harmless; could drop it for symmetry, but not worth a revision on its own.
A couple of CI checks are still running as I write this — worth letting it go fully green before merge, but the code itself looks good.
Reviewed with Claude Code on behalf of Andrea Cosentino (@oscerd). This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
|
🌟 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: 11 tested, 29 compile-only — current: 11 all testedMaveniverse Scalpel detected 40 affected modules (current approach: 11).
|
4 participants
Fixes two regressions in
camel-jackson3from the Jackson 2 → Jackson 3 port (CAMEL-22857):Type converter drops all but the last configured Jackson module —
JacksonTypeConverters.resolveObjectMapper()replaced the mapper with a freshJsonMapper.builder().addModule(module).build()on every loop iteration overCamelJacksonTypeConverterModuleClassNames, so only the last module was registered. Now all modules are accumulated on a single builder (matching the Jackson 2 behavior ofregisterModule).Jackson exceptions escape data type transformers unwrapped —
JsonStructDataTypeTransformerandJsonPojoDataTypeTransformerstill caught the checkedIOExceptionthat Jackson 3 read/write calls no longer throw, so malformed JSON escaped as a raw uncheckedJacksonExceptioninstead of being wrapped inCamelExecutionExceptionwith the exchange attached (asJsonDataTypeTransformeralready does). AddedJacksonExceptionto the catch clauses.Each new test was verified to fail against the unfixed code and pass with the fix; the full
camel-jackson3suite passes (103 tests).Note:
camel-jackson3was introduced in 4.19.0, so no backport tocamel-4.18.xis applicable.JIRA: https://issues.apache.org/jira/browse/CAMEL-24064
Claude Code on behalf of Croway