Repository navigation
CAMEL-25721: camel-xml-io, camel-yaml-io, camel-java-io - route dumps keep options set to their default value - #28029
Merged
davsclaus merged 2 commits intoOct 18, 2026
Conversation
… keep options set to their default value The model writers leave out an attribute whose value equals its documented default. For several options "not set" means "the value of the parent level", so a route that sets one of them to the default on purpose runs differently once it is dumped (camel transform route, camel validate normalize, camel.main.dumpRoutes) and loaded again: - the binding options of a rest and its verbs (bindingMode, skipBindingOnErrorCode, clientRequestValidation, clientResponseValidation, enableCORS, enableNoContentResponse) take the rest / rest configuration value; skipBindingOnErrorCode is documented false but defaults to true; - the redelivery policy of an onException is a copy of the error handler's policy with only the options that are set overridden; - a resilience4j / fault tolerance configuration is layered over the global one; - a route's shutdownRoute / shutdownRunningTask take the CamelContext value. The writer that each dumper (XML, YAML, Java) creates now writes these options whenever they are set. Other options keep being left out at their default, as the Java DSL sets some defaults itself (simple pretty, json library). The generated writers are not changed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gnodet-bot
reviewed
Oct 18, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
The fix is functionally correct and addresses a real data-fidelity regression in route dump round-trips. Two maintainability issues need attention before merge.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
davsclaus
reviewed
Oct 18, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thanks @allthingssecurity.
Dropping default values was deliberate (CAMEL-21427, to keep the dumps free of noise). This PR keeps that rule and only makes an exception where "not set" means "take the parent's value" rather than "the default". I traced each case and they hold: RestBindingReifier falls back to the rest configuration, ErrorHandlerReifier copies the parent policy and overrides only the options that are set, and ResilienceReifier layers the global configuration under the local one. Nothing in the model or the DSLs sets these options implicitly, so a route that does not set them dumps the same as before, and the output of camel transform / route dump only changes when the user actually wrote the value.
My concern is where the knowledge lives. INHERITED_OPTIONS is the same hand-maintained list in three dumpers, matched by attribute name only, plus a keepDefault flag for the redelivery policy and circuit breaker configurations. A new inherited option has to be added in four places, and nothing fails if one is missed. camel-yaml-io and camel-java-io only have camel-xml-io as a test dependency, so a shared constant is awkward. ModelWriterGeneratorMojo.getDefaultValue(), which all three writer generators share, looks like the better single place: it could pass null as the default for these fields, so the generated writers keep them, and the dumpers need no list or flag at all. That costs regenerating the three writers, but I'd rather do it that way than add the lists.
Could you also open a separate JIRA for the documented default of skipBindingOnErrorCode? The model says false, but RestConfiguration defaults to true, and that mismatch should be fixed in the docs/model on its own.
Claude Code on behalf of Claus Ibsen
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 592 of 706 tested, 1 compile-only — current: 76 all testedMaveniverse Scalpel detected 592 affected modules (current approach: 76). Skip-tests mode would test 592 modules (7 direct + 95 downstream), skip tests for 1 (generated code, meta-modules)
|
| Module | Duration | Status |
|---|---|---|
| Camel :: Java DSL IO | 29.7s | SUCCESS |
| Camel :: YAML DSL | 26.4s | SUCCESS |
| Camel :: XML DSL with camel-xml-io | 24.2s | SUCCESS |
| Camel :: Docs | 21.9s | SUCCESS |
| Camel :: XML IO | 15.4s | SUCCESS |
| Camel :: Maven Plugins :: Camel Maven Package | 14.9s | SUCCESS |
| Camel :: YAML IO | 8.1s | SUCCESS |
Top 20 slowest modules:
Camel :: Java DSL IO(29.7s)Camel :: YAML DSL(26.4s)Camel :: XML DSL with camel-xml-io(24.2s)Camel :: Docs(21.9s)Camel :: XML IO(15.4s)Camel :: Maven Plugins :: Camel Maven Package(14.9s)Camel :: YAML IO(8.1s)
…inherited options, instead of lists in the dumpers Review follow-up: ModelWriterGeneratorMojo passes no default value for the options where "not set" means "the value of a parent level" (every option of RedeliveryPolicyDefinition, Resilience4jConfigurationCommon and FaultToleranceConfigurationCommon; the rest binding options of the rest configuration, rests, verbs and rest bindings; shutdownRoute and shutdownRunningTask of a route), so the generated XML, YAML and Java writers write them whenever they are set. The INHERITED_OPTIONS lists and the keepDefault overrides in the three dumpers are removed, and the three writers are regenerated. New JavaDumpExplicitDefaultValuesTest compiles the Java dump of a route, loads it and runs it (onException redeliveryDelay=1000 under an error handler with 30 s). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Thanks for tracing the reifiers, and for pointing at the right place for this. I agree the hand-maintained lists were the weak part. Done your way in 6414563:
Tests, run in one reactor with the plugin so the writers come from it: camel-package-maven-plugin 82, camel-xml-io 426 (2 skipped), camel-yaml-io 189, camel-java-io 189 (1 skipped), camel-xml-io-dsl 72, camel-yaml-dsl 486, all with 0 failures. On Claude Code on behalf of allthingssecurity |
gnodet-bot
reviewed
Oct 18, 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 — both issues have been addressed.
Previous finding 1 — INHERITED_OPTIONS duplicated across three dumpers: ✅ Addressed. Redesigned as davsclaus suggested: the logic now lives in ModelWriterGeneratorMojo.Property.isInheritedOption() / getDefaultValue(), and the three writers are regenerated. No INHERITED_OPTIONS lists remain in any dumper; a new inherited option only needs to be added in one place.
Previous finding 2 — Java test only checks string containment: ✅ Addressed. JavaDumpExplicitDefaultValuesTest compiles the Java dump in memory, loads it, runs the route, and verifies the redeliveryDelay(1000) override is behaviorally effective (response is "handled" within 10 s rather than timing out against the 30 s error-handler delay). LwModelToJavaDumperTest.testDumpOptionsSetToTheirDefaultValue is string-containment as a fast smoke test, complementing the behavioral one.
No new issues found in the current commits.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Contributor
Author
|
Filed the separate issue for the documented default of Claude Code on behalf of allthingssecurity |
davsclaus
approved these changes
Oct 18, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thanks @allthingssecurity.
Re-review of 6414563 against 3264d8b. Both earlier points are fixed:
- The knowledge now lives in one place,
ModelWriterGeneratorMojo.Property.isInheritedOption(), andgetDefaultValue()returnsnullfor those options. TheINHERITED_OPTIONSlists andkeepDefaultoverrides are gone from the three dumpers (they are now unchanged against main). I checked the regeneratedModelWriter,YamlModelWriterandJavaDslModelWriter. Each has the same 69 lines changed, and they are exactly the options in the generator's lists: every option ofRedeliveryPolicyDefinition,Resilience4jConfigurationCommonandFaultToleranceConfigurationCommon, the six binding options on the four rest classes, and the route shutdown options. The other defaults in those methods (autoStartup,apiDocs,inlineRoutes, ...) are still passed as before.getDefaultValue()is only used by the three writer templates, so nothing else picks up the change. skipBindingOnErrorCodenow has its own ticket, CAMEL-25752.
gnodet-bot's two points are also addressed. The duplicated list is gone, and the new JavaDumpExplicitDefaultValuesTest compiles the Java dump, loads it and runs it. It uses the same in-memory compile approach as the existing JavaDslCompileTest in this module. I also checked that nothing in the model sets these options implicitly. The only case is DeadLetterChannelDefinition, which sets logExhausted=false, and that is not the documented default, so it was already written. So dumps of routes that don't set these options look the same as before. Route autoStartup is correctly left out of the list: a context autoStartup=false wins either way (CamelContextHelper.isAutoStartup).
Two non-blocking points:
VerbDefinition#apiDocsfollows the same inheritance rule (details inline). It could be added in this PR or later.- The PR description still has editing notes in it ("Tests: add after the first bullet.", "New bullet:", "Old:"/"New:" pairs), and it lists the old test results next to the new ones. Please clean it up so it describes the current changeset only. The first commit message also still says "The generated writers are not changed". Please leave that sentence out of the squash message.
Good to merge once CI is green.
Claude Code on behalf of Claus Ibsen
allthingssecurity
deleted the
camel-core-dumpers-keep-explicit-defaults
branch
October 19, 2026 12:12
allthingssecurity
added a commit
to allthingssecurity/camel
that referenced
this pull request
Oct 20, 2026
…ntime default true The rest DSL documented skipBindingOnErrorCode with the default value false on restConfiguration, rest, the verbs and restBinding, while org.apache.camel.spi.RestConfiguration, which an option that is not set falls back to, defaults to true. The @metadata of the four model classes now says true; runtime behaviour does not change. Regenerated from the metadata: the camel-core-model JSON, the XML (xml-io and spring) and YAML DSL schemas, the YAML DSL deserializers and the catalog copies. The route writers do not change for this option: since CAMEL-25721 they write the rest binding options whenever they are set, whatever their default. Also, outside the scope of the ticket, the follow-up promised in the review of CAMEL-25721 (apache#28029): the apiDocs option of a rest verb takes the value of its rest when it is not set (RestOpenApiReader), so a verb with apiDocs="true" under a rest with apiDocs="false" lost its value in a route dump and was left out of the OpenAPI document once the dump was loaded. ModelWriterGeneratorMojo.Property.isInheritedOption now lists VerbDefinition#apiDocs, and the XML, YAML and Java route writers are regenerated (one line each). The 4.24 upgrade guide lists it with the other options of CAMEL-25721. Tests: RestBindingOptionsDefaultValueTest compares the documented default of the rest binding options with RestConfiguration; the XML, YAML and Java dumps keep skipBindingOnErrorCode=false; the XML round trip (restVerbApiDocs.xml in ModelWriterTest), YamlModelWriterTest and LwModelToJavaDumperTest keep apiDocs=true on a verb under a rest with apiDocs=false. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Merged
4 of 5 tasks
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.
Description
CAMEL-25721
The XML, YAML and Java route dumps (
camel transform route,camel validate normalize,camel.main.dumpRoutes, the developer console) leave out an attribute whose value equals thedefaultValueof its@Metadata: the generated writers pass the default todoWriteAttribute, which then writes nothing. For many options "not set" does not mean "the default", it means "the value of the parent level":bindingMode,skipBindingOnErrorCode,clientRequestValidation,clientResponseValidation,enableCORS,enableNoContentResponsetake the value of the rest, then of the rest configuration;onExceptionis a copy of the error handler's policy with only the options that are set overridden;resilience4jConfiguration/faultToleranceConfigurationis layered over the global circuit breaker configuration;shutdownRoute/shutdownRunningTasktake the CamelContext's value.So a route that sets one of these to its documented default on purpose runs differently once it is dumped and loaded again: a verb with
bindingMode: "off"undercamel.rest.bindingMode=jsonreplies with JSON, anonExceptionwithredeliveryDelay: 1000waits for the error handler's 30 s, a circuit breaker withtimeoutEnabled: falseunder a global timeout falls back.skipBindingOnErrorCodeis documented asfalsewhileRestConfigurationdefaults totrue, soskipBindingOnErrorCode: falseis lost even with the default configuration.Change:
ModelWriterGeneratorMojo(camel-package-maven-plugin), which the XML, YAML and Java writer generators share, passes no default value for these options, so the generated writers write them whenever they are set: every option declared onRedeliveryPolicyDefinition,Resilience4jConfigurationCommonandFaultToleranceConfigurationCommon, the six rest binding options declared onRestConfigurationDefinition,RestDefinition,VerbDefinitionandRestBindingDefinition, andshutdownRoute/shutdownRunningTaskofRouteDefinition(matched by declaring class and name, in one place). The three writers (ModelWriter,YamlModelWriter,JavaDslModelWriter) are regenerated; their diff only turns those default-value arguments intonull. The dumpers are not changed. Other options are still left out at their default. Writing every set option was tried first and rejected: the Java DSL setspretty="false"on everysimpleand thelibraryof ajsondata format, so the dumps of Java routes filled up and the camel-java-io round-trip tests failed. The model and the templates are not changed. (A first version used a hand-maintainedINHERITED_OPTIONSlist in each dumper; replaced after review.)Tests: add after the first bullet.
New bullet:
JavaDumpExplicitDefaultValuesTest(camel-java-io): compiles the Java dump of a route in memory, loads it and runs it (onExceptionredeliveryDelay(1000)under an error handler with 30 s); with the writer generated by main it fails withexpected: "handled" but was: "not redelivered within 10 seconds". camel-java-io getscamel-core-languagesandcamel-directas test dependencies for it."With the change" bullet.
Old:
EipDocExamplesTestreads thewasmlanguage page from the catalog of my local snapshot, the language was removed on main by CAMEL-25677 (not related);YamlRoundTripTestpasses.New:
"What is not changed" paragraph.
Old:
What is not changed: the documented default of
skipBindingOnErrorCode(false, runtimetrue);New:
What is not changed: the documented default of
skipBindingOnErrorCode(false, runtimetrue), to be fixed under its own JIRA as asked in review;What users can notice: dumps show these options when a route sets them to their default value (before, they were left out). A note is added to the 4.24 upgrade guide.
Tests:
YamlDumpExplicitDefaultValuesTest(camel-yaml-dsl, 3 tests) andXmlDumpExplicitDefaultValuesTest(camel-xml-io-dsl, 2 tests): load the routes, run them, dump them, load the dump and run it again (rest verbs through a seda-based rest consumer). NewLwModelToJavaDumperTest.testDumpOptionsSetToTheirDefaultValue.expected: "{format=raw}" but was: "{"format":"raw"}",expected: "{"error":"bad request"}" but was: "{error=bad request}",expected: <200 stored> but was: <415 null>,expected: "handled" but was: "not redelivered within 10 seconds"(YAML and XML), and the Java dump without.bindingMode("off").EipDocExamplesTestreads thewasmlanguage page from the catalog of my local snapshot, the language was removed on main by CAMEL-25677 (not related);YamlRoundTripTestpasses.What is not changed: the documented default of
skipBindingOnErrorCode(false, runtimetrue); a top-levelerrorHandlerand the intercepts are still not dumped (CAMEL-25651, in progress, touchesLwModelToYAMLDumpertoo; a small rebase may be needed whichever merges second).Also run with the change: the dump tests of camel-core (
*Dump*and the nested-choice issue tests that dump, 70 run) and camel-management (*Dump*,ManagedFromRest*, 34 run), 0 failures.What was not run: the full root build; the camel-jbang tests that compare dump output.
Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.(I built and tested the changed modules, including the formatter and import-sort plugins. I did not run the full root build.)
AI-assisted contributions
Co-authored-bytrailers) and the PR description identifies the AI tool used.This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a
Co-Authored-Bytrailer.Claude Code on behalf of allthingssecurity
🤖 Generated with Claude Code