Repository navigation
CAMEL-26981: camel-main - the Rest DSL binding uses the camel.dataformat options - #30012
Merged
davsclaus merged 2 commits intoNov 15, 2026
Merged
Conversation
…mat options The json and xml data formats of the Rest DSL binding start from the options of the data format configured on the context, as route data formats do; the rest dataFormatProperties still take precedence.
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet-bot
reviewed
Nov 15, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
LGTM. The fix correctly mirrors the DataFormatReifier pattern to propagate camel.dataformat.<name>.* options into the REST DSL binding's data formats, with a well-motivated exclusion set for mapper instances and type-header options. Test coverage is solid (include propagation, json.out.* precedence, allowUnmarshallType exclusion).
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 585 of 726 tested, 0 compile-only — current: 585 all testedMaveniverse Scalpel detected 585 affected modules (current approach: 585). Skip-tests mode would test 585 modules (4 direct + 33 downstream), skip tests for 0 (generated code, meta-modules) Modules Scalpel would test (585)
Build reactor — dependencies compiled but only changed modules were tested (4 modules, 1m 23s total)Total reactor time: 1m 23s
Top 20 slowest modules:
|
davsclaus
reviewed
Nov 15, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thanks @Croway, this closes a real gap between marshal().json() and the REST binding. createDataFormat copies only when the generic data format is known to the context and has the same class, and the json.in./json.out./xml.in./xml.out. properties are applied afterwards, so they still win (restDataFormatPropertiesComeFirst). Leaving out allowUnmarshallType/allowJmsType is the right security call: otherwise a global opt-in meant for a route would let an external HTTP caller pick the request body class with CamelJacksonUnmarshalType, and restBindingDoesNotTakeTheUnmarshalTypeHeader pins it. The Jackson 3 FAIL_ON_UNKNOWN_PROPERTIES default still applies, and JAXB sets contextPath from the verb type after the copy.
Findings:
- The body-type options are copied too:
unmarshalType,useListandcollectionType(Jackson, Jackson 3, Jackson XML;unmarshalTypealso for Gson, Fastjson and JSON-B).setupJsonsets them only when the verb has atype/outType. Socamel.dataformat.jackson.unmarshal-type=com.acme.Order, meant for anunmarshal().json()route, now makes every REST verb withouttypebind its request body toOrder, where before it bound to maps/lists, and a body that does not fitOrdernow fails. In the binding the verb decides the body type, the same reasoning as forallowUnmarshallType. Could we add them toNOT_INHERITED_OPTIONS(RestBindingAdviceFactory.java:51) and mention them in the upgrade guide and rest-dsl-binding.adoc next to the other four?A test withprivate static final Set<String> NOT_INHERITED_OPTIONS = Set.of( "objectmapper", "xmlmapper", "allowunmarshalltype", "allowjmstype", "unmarshaltype", "uselist", "collectiontype");
camel.dataformat.jackson.unmarshal-typeand a verb withouttypewould pin it. - [Suggestion]
copyOptionsrepeats the configurer lookup and thegetAllOptions/getOptionValueloop ofDataFormatReifier.configureDataFormat(CAMEL-22352). One helper in camel-support used by both would keep them in step.
Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
… of the configured data format unmarshalType, useList, collectionType and the JAXB partClass are not copied from the camel.dataformat options: the type and outType of the verb decide the body type.
Contributor
Author
Claude Code on behalf of Federico Mariani |
gnodet-bot
reviewed
Nov 15, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Following up on my earlier review — the previous findings are all addressed.
davsclaus's finding #1 (body-type options leaking into REST binding) is correctly fixed: unmarshalType, useList, collectionType, and partClass are added to NOT_INHERITED_OPTIONS. The new tests restBindingDoesNotTakeTheBodyTypeOptions and JaxbRestBindingPartClassTest both verify the exclusion with assertions that would fail without the fix. Docs (camel-4x-upgrade-guide-4_24.adoc and rest-dsl-binding.adoc) list all 8 excluded options accurately.
LGTM.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
davsclaus
approved these changes
Nov 15, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thanks @Croway, 5bff9ae closes finding 1. unmarshalType, useList, collectionType and the JAXB partClass are now in NOT_INHERITED_OPTIONS (RestBindingAdviceFactory.java:51), compared lower-case against the capitalised configurer keys, so Gson, JSON-B and Fastjson unmarshalType are covered too. Both new tests pin the change:
- In
restBindingDoesNotTakeTheBodyTypeOptions, each of the three options would break the untypedput()on its own if it were still copied:unmarshalTypewould give aTestOtherPojo, anduseListorcollectionTypewould make the JSON object fail to unmarshal. JaxbRestBindingPartClassTestwould get aBar(unwrapped byignoreJAXBElement) instead of aPurchaseOrderwithout the fix.
The upgrade guide and rest-dsl-binding.adoc list the same eight options as the code. Keeping the shared camel-support helper out of this PR is fine with me, given that the reifier merges route options and copies everything.
LGTM (the JDK 21 build is still running and gates the merge).
Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JIRA: CAMEL-26981
camel.dataformat.<name>.*configured the data format used bymarshal()/unmarshal(), but the Rest DSL binding created new json/xml data formats and only appliedrestConfigurationdata format properties (e.g.camel.dataformat.jackson.include=NON_NULLwas ignored in REST responses).RestBindingAdviceFactorynow copies the options of the configured data format into the binding's data formats, asDataFormatReifierdoes for route data formats (CAMEL-22352); thejson.in./json.out./xml.in./xml.out.properties are applied afterwards and still win.objectMapperandxmlMapper(the generic data format may have created its mapper when it started, the request and response data formats keep their own),allowUnmarshallTypeandallowJmsType(a header must not choose the request body type), andunmarshalType,useList,collectionTypeand the JAXBpartClass(the verb'stype/outTypedecides the body type).Tests:
JacksonRestBindingMainTest(camel-jackson, new test-scopedcamel-maindependency),JaxbRestBindingPartClassTest(camel-jaxb).Upgrade guide: 4.24, "camel-main - the camel.dataformat options apply to the Rest DSL binding";
rest-dsl-binding.adocdocuments the precedence and the options that are not copied.Claude Code on behalf of Federico Mariani