[
https://issues.apache.org/jira/browse/CAMEL-25135?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18123658#comment-18123658
]
Guillaume Nodet commented on CAMEL-25135:
-----------------------------------------
Fix PR opened: https://github.com/apache/camel/pull/27430
*Root cause*: {{RouteConfigurationDefinition.onCompletion()}} (Java DSL)
unconditionally set {{routeScoped=false}} on every {{OnCompletionDefinition}}
it created. The YAML DSL left {{routeScoped}} at its model default ({{true}}).
When the opted-in route ({{direct:}}) is called from a consumer route (REST
DSL), the exchange's current route at completion time is the consumer route.
With {{routeScoped=false}}, {{OnCompletionProcessor.shouldSkip()}} compares
{{routeId}} (the direct: route) against {{currentRouteId}} (the consumer route)
-- they never match, so the processor is skipped.
*Fix*: in {{RouteConfigurationDefinition.onCompletion()}}, only set
{{routeScoped=false}} for wildcard (no-id) route configurations, where it is
needed to de-duplicate the onCompletion across all applicable routes. Named
route configurations (with a specific id) leave {{routeScoped}} at {{true}} so
the route-visit tracking mechanism fires correctly regardless of which route is
the actual consumer.
_This comment was generated by an AI coding agent (Claude Sonnet 4.6) on behalf
of gnodet._
> 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)
