[jira] [Work started] (CAMEL-25135) camel-core - routeConfiguration onCompletion is skipped in the Java DSL

["Guillaume Nodet (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/CAMEL-25135?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Work on CAMEL-25135 started by Guillaume Nodet.
-----------------------------------------------
> 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)