[
https://issues.apache.org/jira/browse/CAMEL-25135?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18120689#comment-18120689
]
Guillaume Nodet commented on CAMEL-25135:
-----------------------------------------
This issue is being investigated by a coding agent (on behalf of gnodet).
Initial triage confirmed the root cause: the Java DSL
RouteConfigurationDefinition.onCompletion() explicitly sets routeScoped=false,
while the YAML DSL leaves it at the model default (true). The
RouteDefinitionHelper.initOnCompletions() method lacks the normalization that
initOnExceptions() already has (output.setRouteScoped(false)). This causes
OnCompletionProcessor.shouldSkip() to incorrectly skip the processor when the
consumer route ID differs from the opting-in route ID, which is the common
shape of REST DSL + direct: routes.
Fix scope: add normalization of routeScoped in initOnCompletions, plus a test
covering the indirect-consumer case. Single-PR scope.
_Note: This comment was generated by an AI coding agent and requires manual
verification._
> camel-core - routeConfiguration onCompletion is skipped in the Java DSL
> -----------------------------------------------------------------------
>
> Key: CAMEL-25135
> URL: https://issues.apache.org/jira/browse/CAMEL-25135
> Project: Camel
> Issue Type: Bug
> Components: came-core
> Affects Versions: 4.22.1
> Reporter: Thomas Raddatz
> Assignee: Guillaume Nodet
> Priority: Major
> Attachments: camel-routeconfig-oncompletion-1.zip
>
>
> h2. Summary
> The same route configuration behaves differently depending on the DSL it is
> written in. A
> {{routeConfiguration}} that carries an {{onCompletion}} works when it is
> defined in YAML and silently
> does nothing when the identical configuration is defined with a
> {{{}RouteConfigurationBuilder{}}}, as soon
> as the opting-in route is not itself the consumer route — which is the normal
> shape of a
> contract-first REST application, where the rest-dsl binds every operation to
> a {{direct:}} route.
> The configuration is matched and merged in both cases: the route logs
> {code:java}
> Route: probe-get is using route configurations ids: [probe-config]
> {code}
> in the Java case too. Only the processor is skipped at runtime, so there is
> no warning and no error —
> the response simply keeps its default status.
> {{onException}} in the same configuration is unaffected, which makes the
> failure look arbitrary.
> h2. Reproducer
> {{camel-routeconfig-oncompletion}} in this directory, about 60 lines: an
> OpenAPI contract with one operation, a {{direct:}} route carrying
> {{routeConfigurationId: probe-config}} that sets the exchange property
> {{httpStatus}} to 404, and the configuration itself — once as
> {{{}probe.JavaConfig{}}}, once as {{{}yaml-variant/config.camel.yaml{}}}.
> Both define one {{onCompletion}} with {{mode: BeforeConsumer}} that logs a
> marker and copies {{httpStatus}} into {{{}CamelHttpResponseCode{}}}. See
> {{README.md}} for the two commands.
> ||Configuration DSL||{{GET /probe}}||marker logged||
> |YAML|*404*|yes|
> |Java {{RouteConfigurationBuilder}}|*200*|no|
> Expected: both answer 404.
> h2. Analysis
> {{OnCompletionDefinition.routeScoped}} defaults to {{true}}
> ({{{}core/camel-core-model/.../OnCompletionDefinition.java:49{}}}). No YAML
> or XML deserializer touches it. In {{core/camel-core-model/src/main}} there
> are only four writers: three Java-DSL call sites
> ({{{}RouteConfigurationDefinition:237{}}}, {{RoutesDefinition:406}} and
> {{{}:418{}}}) and one normalisation in {{{}RouteDefinitionHelper:525{}}},
> discussed below.
> {{RouteConfigurationDefinition.onCompletion()}} is one of them
> ({{{}core/camel-core-model/.../RouteConfigurationDefinition.java:233{}}}):
> {code:java}
> public OnCompletionDefinition onCompletion() {
> OnCompletionDefinition answer = new OnCompletionDefinition();
> answer.setRouteConfiguration(this);
> // is global scoped by default
> answer.setRouteScoped(false); // <-- not what the YAML side
> produces
> onCompletions.add(answer);
> return answer;
> }
> {code}
> {{OnCompletionProcessor.shouldSkip
> }}({{{}core/camel-core-processor/.../OnCompletionProcessor.java:365{}}}) then
> reads it:
> {code:java}
> String currentRouteId = ExchangeHelper.getRouteId(exchange);
> if (!routeScoped && currentRouteId != null &&
> !routeId.equals(currentRouteId)) {
> return true; // skipped
> }
> {code}
> {{routeId}} is the route the processor belongs to — the {{direct:}} operation
> route that opted in.
> {{currentRouteId}} is the route the exchange is in when it completes — the
> consumer route the rest-dsl generated, which does not carry the
> configuration. The two are never equal, so the processor is skipped on every
> exchange.
> That check is the de-duplication mechanism for a genuinely context-scoped
> {{{}nCompletion{}}}, which is added to _every_ route: each copy asks "am I
> the route the exchange is actually in?" and only one answers yes. A
> configuration-scoped {{onCompletion}} is added only to the routes that opt
> in, so the premise does not hold for it.
> This also explains why a minimal reproducer can look healthy: if the
> opting-in route _is_ the
> consumer route (a plain {{platform-http}} route with
> {{{}routeConfigurationId{}}}), then {{routeId.equals(currentRouteId)}} and
> nothing is skipped. The divergence only surfaces one hop away from the
> consumer.
> h3. Why {{onException}} is not affected
> {{RouteConfigurationDefinition.onException()}} (line 205) does *not* set the
> flag — but that is not what saves it.
> {{RouteDefinitionHelper.initOnExceptions}} normalises it for every merged
> clause, whatever the DSL left behind:
> {code:java}
> for (OnExceptionDefinition output : onExceptions) {
> // these are context scoped on exceptions so set this flag
> output.setRouteScoped(false);
> abstracts.add(output);
> }
> {code}
> {{RouteDefinitionHelper.initOnCompletions}} (line 683) has no counterpart to
> that line. It calls {{initParent(global)}} and nothing else, so the
> definition-time value survives into reification — and that value is the one
> thing the two DSLs disagree about. The asymmetry between the two {{init*
> }}helpers is the actual defect; the Java DSL call site is only where the
> differing value comes from.
> h2. Related issues
> Searched the CAMEL project (summary, description and comments) for
> {{routeConfigurationId}} + {{{}onCompletion{}}}, {{routeConfiguration}} +
> {{{}onCompletion{}}}, {{routeScoped}} and {{{}BeforeConsumer{}}}, and the
> GitHub issues of {{{}apache/camel{}}}. Nothing covers this. Two neighbours
> are worth knowing about:
> *CAMEL-22820* — *routeConfiguration onException does not propagate to direct
> endpoints for consumer-level exceptions* (fixed in 4.10.9 / 4.14.5 / 4.18.0,
> so before the version reported here).
> Same family, other half: an {{onException}} written in a
> {{RouteConfigurationBuilder}} behaved differently from the identical clause
> written inline in a {{{}RouteBuilder{}}}, and {{direct:}} endpoints were not
> reached. The {{onException}} side was repaired then; the {{onCompletion}}
> side of the same construct is what this report is about.
> *CAMEL-16083* — _OnCompletion with After Consumer mode does not fire if
> defined in routeScope_
> (fixed in 3.7.2 / 3.8.0, a regression from CAMEL-13553). The mechanism is the
> one at work here: the {{onCompletion}} did not fire *because the route id it
> is scoped to does not match the route id of the route the exchange is in*.
> That was corrected for the route-scoped case; the same comparison now bites
> through the {{routeScoped = false}} branch of {{shouldSkip}} for the
> configuration-scoped case.
> h2. Why the obvious patch is wrong
> Deleting {{answer.setRouteScoped(false)}} from
> {{RouteConfigurationDefinition.onCompletion()}} makes the reproducer pass,
> and breaks {{RouteConfigurationOnCompletionTest}} (3 failures). That test
> uses a configuration *without* an id, which applies to every route; with
> {{routeScoped = true}} the {{onCompletion}} then fires once per visited route
> instead of once per exchange (expected 1, actual 2).
> So the flag does earn its keep for a wildcard configuration.
> That leaves a semantic question rather than a one-line fix, and it is the
> maintainers' to answer:
> - Should a configuration matched *by id* be route-scoped, and only a
> wildcard configuration global?
> That would match what the ids express and would fix this without touching the
> wildcard case.
> - Or should {{initOnCompletions}} normalise the flag the way
> {{initOnExceptions}} does, and the
> de-duplication be solved differently for the configuration-scoped case?
> Either way, the DSL divergence itself looks like a bug worth fixing on its
> own: the same
> configuration should not depend on the language it is written in.
> h2. Workaround
> Set the flag back to the model default at definition time:
> {code:java}
> OnCompletionDefinition oc = routeConfiguration("my-config").onCompletion();
> oc.setRouteScoped(true);
> oc.modeBeforeConsumer()
> .process(...);
> {code}
> {{setRouteScoped}} is public, and this produces exactly what the YAML
> deserializer produces. Verified with the reproducer and in the production
> module this was found in.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
