Thomas Raddatz created CAMEL-25135:
--------------------------------------

             Summary: 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
         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)

Reply via email to