Repository navigation
CAMEL-25752: camel-core - document skipBindingOnErrorCode with its runtime default true - #28134
Conversation
…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>
|
🌟 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: 593 of 707 tested, 23 compile-only — current: 70 all testedMaveniverse Scalpel detected 593 affected modules (current approach: 70). Skip-tests mode would test 593 modules (9 direct + 588 downstream), skip tests for 23 (generated code, meta-modules)
|
| Module | Duration | Status |
|---|---|---|
| Camel :: Catalog :: Camel Catalog | 39.0s | SUCCESS |
| Camel :: Java DSL IO | 30.3s | SUCCESS |
| Camel :: Docs | 19.5s | SUCCESS |
| Camel :: YAML DSL | 18.5s | SUCCESS |
| Camel :: Maven Plugins :: Camel Maven Package | 14.7s | SUCCESS |
| Camel :: XML IO | 12.6s | SUCCESS |
| Camel :: Core Model | 11.0s | SUCCESS |
| Camel :: YAML IO | 7.8s | SUCCESS |
| Camel :: YAML DSL :: Deserializers | 5.8s | SUCCESS |
Top 20 slowest modules:
Camel :: Catalog :: Camel Catalog(39.0s)Camel :: Java DSL IO(30.3s)Camel :: Docs(19.5s)Camel :: YAML DSL(18.5s)Camel :: Maven Plugins :: Camel Maven Package(14.7s)Camel :: XML IO(12.6s)Camel :: Core Model(11.0s)Camel :: YAML IO(7.8s)Camel :: YAML DSL :: Deserializers(5.8s)
davsclaus
left a comment
There was a problem hiding this comment.
LGTM, thanks. The runtime default really is true (RestConfiguration, RestBindingReifier), and all the generated model, catalog, XSD and YAML schema files are included. The apiDocs fallback to the rest's value is real (RestOpenApiReader), so VerbDefinition#apiDocs in INHERITED_OPTIONS is correct.
Nits:
- The
apiDocschange is unrelated to CAMEL-25752, but the commit only names CAMEL-25752. Could you mention it in the commit body (or open a small JIRA), sogit logonModelWriterGeneratorMojoexplains it? - Not for this PR: the
apiDocsdescriptions look swapped.RestDefinition#apiDocssays it overrides a parent level, but the verb is the one that overrides its parent. The verb's documenteddefaultValue = "true"is misleading in the same wayskipBindingOnErrorCodewas.
Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Description
CAMEL-25752
The rest DSL documents
skipBindingOnErrorCodewith the default valuefalse(the@MetadataofRestConfigurationDefinition,RestDefinition,VerbDefinitionandRestBindingDefinition, and from there the catalog, the XML and YAML DSL schemas and the docs). At runtime an option that is not set falls back toorg.apache.camel.spi.RestConfiguration, which initialises it totrue(RestBindingReifierstarts fromconfig.isSkipBindingOnErrorCode()and only overrides it when the model sets a value). camel-main (camel.rest.skipBindingOnErrorCode) andrest-dsl-binding.adocalready saytrue; only the model saidfalse.Raised by Claus Ibsen in the review of #28029 as a separate issue.
This PR changes the documented default to
true, as the ticket suggests. Runtime behaviour does not change: no runtime code reads the@Metadatadefault, andRestConfigurationis untouched.Regenerated from the metadata, with the projects' generator goals:
rest,restConfiguration,restBinding,get,post,put,patch,delete,head).ModelDeserializers,camelYamlDsl.json,camelYamlDsl-canonical.json,camelYamlDsl-model.json.camel-xml-io.xsd,camel-spring.xsdandcamelYamlDsl-model.json. These are verbatim copies; I built the XSDs (camel-xml-io and camel-spring-xmlpackage) and copied them with the model files, rather than runningprepare-catalog, which would download every component artifact.Every regenerated line is the default of
skipBindingOnErrorCode(false→true); no other default changed. camel-main and its generated docs/configurer already hadtrueand are unchanged.The route writers do not change for
skipBindingOnErrorCode: since #28029 (CAMEL-25721) they write the rest binding options whenever they are set, whatever their default. What users can notice is only that tools that read the default from the catalog or the schemas now showtrue. I added a note to the 4.24 upgrade guide.Also in this PR:
apiDocsof a rest verb (follow-up of #28029)This is outside the scope of CAMEL-25752; it is the non-blocking point from the review of #28029, which I said would go here because it regenerates the same writers.
VerbDefinition#apiDocs(documented defaulttrue) is inherited too: inRestOpenApiReader, a verb whoseapiDocsis not set takes the value of its rest. So a verb withapiDocs="true"under a rest withapiDocs="false"lost its value in a route dump, and once the dump was loaded the verb was left out of the OpenAPI document. Only the API docs are affected, not routing.ModelWriterGeneratorMojo.Property.isInheritedOptionnow listsorg.apache.camel.model.rest.VerbDefinition#apiDocs.apiDocsdefault"true"→null. The rest's ownapiDocskeeps its default, as a rest with no value is included.apiDocsof a rest verb with the other options.Tests:
RestBindingOptionsDefaultValueTest(camel-core-model): forbindingMode,skipBindingOnErrorCode,clientRequestValidation,clientResponseValidation,enableCORSandenableNoContentResponse, the@Metadatadefault on each of the four model classes must equal theRestConfigurationdefault. Without the fix it fails on the fourskipBindingOnErrorCodefields only.skipBindingOnErrorCode=false: a newrestSkipBindingOnErrorCode.xmlused by the existingModelWriterTestXML round trips (also added toModelParserTest's rest files);YamlModelWriterTest.testRestSkipBindingOnErrorCodeFalse;LwModelToJavaDumperTest.testDumpRestKeepsSkipBindingOnErrorCodeFalse. They failed before CAMEL-25721: camel-xml-io, camel-yaml-io, camel-java-io - route dumps keep options set to their default value #28029; with CAMEL-25721: camel-xml-io, camel-yaml-io, camel-java-io - route dumps keep options set to their default value #28029 merged they pass on main too, and stay as regression tests for this option.apiDocs: a newrestVerbApiDocs.xml(a rest withapiDocs="false", one verb withoutapiDocsand one withapiDocs="true") used by theModelWriterTestXML round trips andModelParserTest;YamlModelWriterTest.testRestVerbApiDocsTrueUnderRestApiDocsFalse;LwModelToJavaDumperTest.testDumpRestVerbApiDocsTrueUnderRestApiDocsFalse. Negative control: with the writers generated without theisInheritedOptionline (as on main), all of them fail:ModelWriterTest4 failures (testRestsandtestRestsWithDifffor the new file in both namespaces,rests[0].verbs[1].apiDocs ==> expected: <true> but was: <null>), and the YAML and Java tests 1 failure each (the verb'sapiDocsis missing from the dump). With the line they pass.maven.packaging,model,xml,yamlandjavapackages). Before the rebase onto CAMEL-25721: camel-xml-io, camel-yaml-io, camel-java-io - route dumps keep options set to their default value #28029 (the metadata and the catalog/schema files are unchanged by it): camel-catalogCamelCatalog*Test1096, camel-route-parser rest tests 4, camel-yaml-dsl*Rest*andYamlCompletionTreeTest24, 0 failures. The formatter and import sort ran in the same builds.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 the affected modules and their generators, and committed every file they changed. 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