[
https://issues.apache.org/jira/browse/CAMEL-24696?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claus Ibsen updated CAMEL-24696:
--------------------------------
Description:
h3. Background
Almost every scalar attribute in the Camel model is declared as a {{String}}
field with the real type in {{@Metadata(javaType = ...)}}, so that property
placeholders can be used and the text is converted when the route starts. Out
of ~1660 generated YAML deserializer cases, only five convert eagerly while
deserializing ({{Boolean.valueOf}} / {{Integer.valueOf}}), so a property
placeholder is never resolved for them.
CAMEL-24694 ported the two that are actually reachable from the generated YAML
DSL JSON schema:
* {{org.apache.camel.model.rest.ParamDefinition.required}}
* {{org.apache.camel.model.RouteTemplateParameterDefinition.required}}
h3. What is left
Three attributes are still converted eagerly. None of them is reachable from
the generated schema today, which is why CAMEL-24694 could leave them alone
without the validator accepting something the runtime gets wrong:
||attribute||why it is not reachable from the schema||
|{{CircuitBreakerDefinition.inheritErrorHandler}}|{{GenerateYamlSchemaMojo}}
deliberately skips {{inheritErrorHandler}} ("we want to skip
inheritErrorHandler which is only applicable for the load-balancer")|
|{{loadbalancer.FailoverLoadBalancerDefinition.inheritErrorHandler}}|same|
|{{app.BeanConstructorDefinition.index}}|{{BeanConstructorDefinition}} and
{{BeanConstructorsDefinition}} are orphan definitions -
{{BeanFactoryDefinition.constructors}} is emitted as a free-form {{object}}, so
nothing {{$ref}}s them|
This matters if any of them is ever added to the schema: the placeholder
leniency added to {{YamlValidator}} in CAMEL-24694 would then start accepting a
placeholder there while the runtime gets it wrong -
{{Boolean.valueOf("{{flag}}")}} silently yields {{false}},
{{Integer.valueOf("{{idx}}")}} throws. So either port the attribute first, or
exclude it in {{YamlValidator.isRuntimeAcceptedScalar}}.
h3. inheritErrorHandler
Both are {{@Override}} of {{ProcessorDefinition.getInheritErrorHandler()}}
which returns {{Boolean}}, and that contract is read by
{{ProcessorReifier#wrapChannel}}, {{LoadBalanceReifier}},
{{org.apache.camel.component.kamelet.Kamelet}}, the generated {{ModelWriter}} /
{{YamlModelWriter}} / {{JavaDslModelWriter}}, and camel-jta. Options:
# Keep {{getInheritErrorHandler()}} returning {{Boolean}} on
{{ProcessorDefinition}}, add a String-valued accessor on the two subclasses,
and move the {{parseBoolean}} into {{ProcessorReifier}} /
{{LoadBalanceReifier}}. Doable on 4.x.
# Change the base contract to {{String}} across the hierarchy, consistent with
the rest of the model. Public API break - better suited to Camel 5.0.
The flag is {{label=advanced}}, seldom used, and dates back to the early days
of Camel, so this is low urgency. Needs a decision on 4.x vs 5.0.
h3. BeanConstructorDefinition.index
This one is a different problem. The value ends up as a {{Map<Integer,
Object>}} key in {{BeanFactoryDefinition.constructors}}, ordering the
constructor arguments ({{BeanModelHelper}} sorts on it). A property placeholder
cannot be a map key, so porting {{index}} to String would mean changing that
map's type - probably not worth doing. Worth closing out explicitly rather than
leaving it implied.
h3. Done when
* a decision is recorded for each of the three (port on 4.x, defer to 5.0, or
won't fix);
* for any that is ported, a property placeholder at that attribute resolves at
route startup like every other scalar attribute;
* upgrade guide entry if a public signature changes.
Related: CAMEL-24694.
_Claude Code on behalf of davsclaus_
was:
h3. Background
Almost every scalar attribute in the Camel model is declared as a {{String}}
field with the real type in {{@Metadata(javaType = ...)}}, so that property
placeholders can be used and the value is converted at route startup. Out of
~1660 generated YAML deserializer cases, only five converted the text eagerly
at deserialization time. CAMEL-24694 ported three of them:
* {{BeanConstructorDefinition.index}} ({{Integer}})
* {{ParamDefinition.required}} ({{Boolean}})
* {{RouteTemplateParameterDefinition.required}} ({{Boolean}})
h3. What is left
Two remain on {{Boolean}}:
* {{org.apache.camel.model.CircuitBreakerDefinition.inheritErrorHandler}}
*
{{org.apache.camel.model.loadbalancer.FailoverLoadBalancerDefinition.inheritErrorHandler}}
The generated deserializer does
{{target.setInheritErrorHandler(java.lang.Boolean.valueOf(val))}}, so a
property placeholder is *not* resolved - {{Boolean.valueOf("{{myFlag}}")}}
silently yields {{false}} rather than failing. Because of that, CAMEL-24694
deliberately allowlisted {{inheritErrorHandler}} in the YAML validator's
placeholder filter so validation keeps rejecting a placeholder there instead of
accepting something the runtime gets wrong. That allowlist should be removed as
part of this issue.
h3. Why it was not done in CAMEL-24694
Both are {{@Override}} of {{ProcessorDefinition.getInheritErrorHandler()}}
which returns {{Boolean}}, and that contract is read by:
* {{org.apache.camel.reifier.ProcessorReifier#wrapChannel}}
* {{org.apache.camel.reifier.LoadBalanceReifier}}
* {{org.apache.camel.component.kamelet.Kamelet}}
* the generated {{ModelWriter}} / {{YamlModelWriter}} / {{JavaDslModelWriter}}
* camel-jta (the base field carries a "used for camel-jta" note)
So converting the field is a wider blast radius than the rest of CAMEL-24694
combined, for a {{label=advanced}} flag that is seldom used and dates back to
the early days of Camel.
h3. Options
# Keep {{getInheritErrorHandler()}} returning {{Boolean}} on
{{ProcessorDefinition}}, add a String-valued accessor on the two subclasses,
and move the {{parseBoolean}} into {{ProcessorReifier}} /
{{LoadBalanceReifier}}. Doable on 4.x.
# Change the base contract to {{String}} across the hierarchy, consistent with
the rest of the model. Public API break - better suited to Camel 5.0.
Needs a decision on whether this lands on 4.x or waits for 5.0.
h3. Done when
* a property placeholder at {{inheritErrorHandler}} resolves at route startup
like every other scalar attribute;
* the {{inheritErrorHandler}} allowlist is removed from {{YamlValidator}} and
its test asserts the placeholder is now accepted;
* upgrade guide entry if the public signature changes.
Related: CAMEL-24694.
_Claude Code on behalf of davsclaus_
> camel-core-model: port inheritErrorHandler to a String field so property
> placeholders work
> ------------------------------------------------------------------------------------------
>
> Key: CAMEL-24696
> URL: https://issues.apache.org/jira/browse/CAMEL-24696
> Project: Camel
> Issue Type: Task
> Components: camel-core
> Reporter: Claus Ibsen
> Priority: Major
>
> h3. Background
> Almost every scalar attribute in the Camel model is declared as a {{String}}
> field with the real type in {{@Metadata(javaType = ...)}}, so that property
> placeholders can be used and the text is converted when the route starts. Out
> of ~1660 generated YAML deserializer cases, only five convert eagerly while
> deserializing ({{Boolean.valueOf}} / {{Integer.valueOf}}), so a property
> placeholder is never resolved for them.
> CAMEL-24694 ported the two that are actually reachable from the generated
> YAML DSL JSON schema:
> * {{org.apache.camel.model.rest.ParamDefinition.required}}
> * {{org.apache.camel.model.RouteTemplateParameterDefinition.required}}
> h3. What is left
> Three attributes are still converted eagerly. None of them is reachable from
> the generated schema today, which is why CAMEL-24694 could leave them alone
> without the validator accepting something the runtime gets wrong:
> ||attribute||why it is not reachable from the schema||
> |{{CircuitBreakerDefinition.inheritErrorHandler}}|{{GenerateYamlSchemaMojo}}
> deliberately skips {{inheritErrorHandler}} ("we want to skip
> inheritErrorHandler which is only applicable for the load-balancer")|
> |{{loadbalancer.FailoverLoadBalancerDefinition.inheritErrorHandler}}|same|
> |{{app.BeanConstructorDefinition.index}}|{{BeanConstructorDefinition}} and
> {{BeanConstructorsDefinition}} are orphan definitions -
> {{BeanFactoryDefinition.constructors}} is emitted as a free-form {{object}},
> so nothing {{$ref}}s them|
> This matters if any of them is ever added to the schema: the placeholder
> leniency added to {{YamlValidator}} in CAMEL-24694 would then start accepting
> a placeholder there while the runtime gets it wrong -
> {{Boolean.valueOf("{{flag}}")}} silently yields {{false}},
> {{Integer.valueOf("{{idx}}")}} throws. So either port the attribute first, or
> exclude it in {{YamlValidator.isRuntimeAcceptedScalar}}.
> h3. inheritErrorHandler
> Both are {{@Override}} of {{ProcessorDefinition.getInheritErrorHandler()}}
> which returns {{Boolean}}, and that contract is read by
> {{ProcessorReifier#wrapChannel}}, {{LoadBalanceReifier}},
> {{org.apache.camel.component.kamelet.Kamelet}}, the generated {{ModelWriter}}
> / {{YamlModelWriter}} / {{JavaDslModelWriter}}, and camel-jta. Options:
> # Keep {{getInheritErrorHandler()}} returning {{Boolean}} on
> {{ProcessorDefinition}}, add a String-valued accessor on the two subclasses,
> and move the {{parseBoolean}} into {{ProcessorReifier}} /
> {{LoadBalanceReifier}}. Doable on 4.x.
> # Change the base contract to {{String}} across the hierarchy, consistent
> with the rest of the model. Public API break - better suited to Camel 5.0.
> The flag is {{label=advanced}}, seldom used, and dates back to the early days
> of Camel, so this is low urgency. Needs a decision on 4.x vs 5.0.
> h3. BeanConstructorDefinition.index
> This one is a different problem. The value ends up as a {{Map<Integer,
> Object>}} key in {{BeanFactoryDefinition.constructors}}, ordering the
> constructor arguments ({{BeanModelHelper}} sorts on it). A property
> placeholder cannot be a map key, so porting {{index}} to String would mean
> changing that map's type - probably not worth doing. Worth closing out
> explicitly rather than leaving it implied.
> h3. Done when
> * a decision is recorded for each of the three (port on 4.x, defer to 5.0, or
> won't fix);
> * for any that is ported, a property placeholder at that attribute resolves
> at route startup like every other scalar attribute;
> * upgrade guide entry if a public signature changes.
> Related: CAMEL-24694.
> _Claude Code on behalf of davsclaus_
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
