[
https://issues.apache.org/jira/browse/CAMEL-25410?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claus Ibsen updated CAMEL-25410:
--------------------------------
Fix Version/s: 4.24.0
Issue Type: Improvement (was: Bug)
> 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: Improvement
> Components: camel-core
> Reporter: Claus Ibsen
> Priority: Major
> Fix For: 4.24.0
>
>
> 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)
