Claus Ibsen created CAMEL-25410:
-----------------------------------
Summary: camel-core-model - TryDefinition: keep doCatch/doFinally
set as properties in outputs (single source of truth)
Key: CAMEL-25410
URL: https://issues.apache.org/jira/browse/CAMEL-25410
Project: Camel
Issue Type: Bug
Components: camel-core
Reporter: Claus Ibsen
Follow-up to CAMEL-25367 (PR https://github.com/apache/camel/pull/27483).
h3. Problem
{{TryDefinition}} keeps {{doCatch}} / {{doFinally}} in two places:
* in {{outputs}} (Java DSL, XML, YAML with the clauses written inside
{{doTry.steps}})
* in the {{@XmlTransient}} fields {{catchClauses}} / {{finallyClause}} (YAML
with {{doCatch}} / {{doFinally}} written as properties of {{doTry}}, via
{{setCatchClauses}} / {{setFinallyClause}})
{{checkInitialized()}} copies clauses from {{outputs}} into the fields, but
never the other way round. Everything that walks {{getOutputs()}} therefore
misses the property-form clauses:
* the lightweight XML/YAML/Java model writers (fixed writer-side by CAMEL-25367
with a special case in 3 velocity templates)
* {{JaxbModelToXMLDumper}} (camel-xml-jaxb) - still drops the clauses, as the
fields are {{@XmlTransient}}
* tooling that has to special-case {{getCatchClauses()}} /
{{getFinallyClause()}}: {{ModelRouteNodeScanner}}, {{JavaRouteScanner}} (TUI),
{{JavaRouteReader}} (camel-jbang-core)
In addition the clause fields only ever grow: {{checkInitialized()}} adds
catches to {{catchClauses}} if missing but never removes, and never resets
{{finallyClause}}. A catch removed from {{outputs}} after the first
initialization (e.g. {{adviceWith}} remove) stays in the cache and is still
reified.
h3. Proposed fix: make {{outputs}} the single source of truth
1. *Setters fold clauses into outputs* (public signatures unchanged):
{code:java}
@XmlTransient
public void setCatchClauses(List<CatchDefinition> catchClauses) {
if (catchClauses != null) {
for (CatchDefinition cd : catchClauses) {
if (!outputs.contains(cd)) {
outputs.add(indexOfFinally(), cd); // before doFinally, if any
}
}
}
initialized = false;
}
@XmlTransient
public void setFinallyClause(FinallyDefinition finallyClause) {
outputs.removeIf(o -> o instanceof FinallyDefinition && o != finallyClause);
if (finallyClause != null && !outputs.contains(finallyClause)) {
outputs.add(finallyClause);
}
initialized = false;
}
{code}
2. *{{checkInitialized()}} rebuilds the fields from outputs each time*
({{catchClauses = new ArrayList<>(); finallyClause = null;}} before the loop)
instead of "create if null, add if missing". The fields become a derived cache,
which also fixes the stale-clause problem above.
3. *Copy constructor must stop deep-copying the clause fields separately* -
{{super(source)}} already deep-copies outputs, which now contain the clauses.
Otherwise a copy holds two distinct instances of each catch ({{contains}} is
identity-based), and route templates would run each catch twice. This step is
mandatory.
4. *YAML key order*: {{steps}} are added via {{addOutput}} (append), so
{{doCatch:}} written *above* {{steps:}} gives {{outputs = [catch, to]}}.
Runtime and the YAML writer cope, but the Java DSL writer would emit
{{.doTry().doCatch(X).to(...)}}, putting the step inside the catch. It cannot
be fixed in {{TryDefinition.addOutput}} because the DSL block stack
({{ProcessorDefinition.blocks}}) is private. Fix it in the YAML deserializer
with the existing {{YamlDeserializerBase.afterPropertiesSet}} hook (as
{{RouteTemplateDefinitionDeserializer}} does):
{code:java}
@Override
protected void afterPropertiesSet(TryDefinition target, Node node) {
// doCatch/doFinally after the steps, whatever the YAML key order (stable
sort)
target.getOutputs().sort(Comparator.comparingInt(
o -> o instanceof FinallyDefinition ? 2 : o instanceof CatchDefinition
? 1 : 0));
}
{code}
{{TryDefinitionDeserializer}} is generated, so this needs a generator special
case or a hand-written deserializer (to be checked).
5. *Clean-up once the model is fixed*: revert the {{TryDefinition}} special
case in {{model-writer.vm}}, {{model-yaml-writer.vm}} and
{{model-java-dsl-writer.vm}} added by CAMEL-25367 (and the regenerated
writers), and drop the clause special-casing in the TUI scanners /
{{JavaRouteReader}}.
h3. Comparison
||Aspect||Writer-side (CAMEL-25367)||Model-side (this issue)||
|Lightweight XML/YAML/Java writers|fixed in 3 templates|fixed, no template code|
|JaxbModelToXMLDumper|still drops clauses|fixed|
|Tooling special cases|still needed|removable|
|Stale clause cache|now visible in dumps|fixed|
|Risk|low (writers only)|touches TryDefinition used by runtime and route
templates|
h3. Tests
* the CAMEL-25367 writer tests ({{TryModelWriterTest}} in camel-xml-io,
camel-yaml-io, camel-java-io) must keep passing with the template special case
removed
* route template: YAML doTry with property clauses, template instantiated
twice, each catch runs exactly once
* YAML with {{doCatch:}} above {{steps:}}, dumped to Java and XML, reloads with
the same behaviour
* {{JaxbModelToXMLDumper}} with property clauses
* {{adviceWith}} removing a doCatch after the route was started: the removed
catch no longer runs
_Claude Code on behalf of davsclaus_
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
