Thomas LE GUEN created CAMEL-23735:
--------------------------------------
Summary: camel-dynamic-router: default aggregation keeps the FIRST
reply, contradicting the documented "last reply" behavior (regression since
4.4.0)
Key: CAMEL-23735
URL: https://issues.apache.org/jira/browse/CAMEL-23735
Project: Camel
Issue Type: Bug
Components: camel-core
Affects Versions: 4.4.0
Environment: h2. Summary
Since the CAMEL-20302 refactoring (Camel 4.4.0), the {{dynamic-router}}
component uses a private {{NoopAggregationStrategy}} as its default aggregation
strategy, which keeps the *first* recipient's result. Both the user
documentation and the component's own javadoc still state that the *last* reply
is used by default (the pre-4.4 behavior, {{UseLatestAggregationStrategy}}).
In {{recipientMode=allMatch}}, any modification made by a recipient other than
the first one (body, exchange properties) is silently discarded when the
exchange returns to the calling route. Whether a downstream {{choice()}} sees a
property set by a subscriber now depends on subscription priorities, which is
surprising and undocumented.
h2. Expected behavior (as documented)
The component documentation for the {{aggregationStrategy}} option states:
{quote}Refers to an AggregationStrategy to be used to assemble the replies from
the multicasts, into a single outgoing message from the Multicast. By default,
Camel will use the last reply as the outgoing message.{quote}
The javadoc of {{DynamicRouterRecipientListHelper#createAggregationStrategy}}
also states:
{quote}If all else fails, then a {{UseLatestAggregationStrategy}} is
used.{quote}
h2. Actual behavior
{{DynamicRouterRecipientListHelper.createAggregationStrategy}} falls back to a
private {{NoopAggregationStrategy}}:
{code:java}
.orElse(new NoopAggregationStrategy());
...
public static class NoopAggregationStrategy implements AggregationStrategy {
public Exchange aggregate(Exchange oldExchange, Exchange newExchange) {
return oldExchange == null ? newExchange : oldExchange; // keeps the
FIRST result
}
}
{code}
The aggregated (first) copy is then written back onto the original exchange by
{{MulticastProcessor.doDone}} via {{ExchangeHelper.copyResults}}, so the caller
route only ever sees the first recipient's outcome.
Note that the unused import of {{UseLatestAggregationStrategy}} is still
present in {{DynamicRouterRecipientListHelper}}, which suggests the behavior
change was unintentional.
h2. Regression evidence
* Camel 4.3.0, {{DynamicRouterEndpoint#determineAggregationStrategy}}:
{code:java}
return cfgStrategy != null
? CamelContextHelper.mandatoryLookup(camelContext, cfgStrategy,
AggregationStrategy.class)
: new UseLatestAggregationStrategy();
{code}
* Camel 4.4.0 (commit {{c4711c97}}, CAMEL-20302) introduced
{{DynamicRouterRecipientListHelper}} with the {{NoopAggregationStrategy}}
fallback shown above.
* Still present and verified on 4.14.1 (LTS) and 4.20.0.
h2. Reproduction (JUnit 5, camel-test-junit5, verified on 4.14.1 and 4.20.0)
{code:java}
public class DynamicRouterDefaultAggregationTest extends CamelTestSupport {
@Override
protected RoutesBuilder createRouteBuilder() {
return new RouteBuilder() {
@Override
public void configure() {
from("direct:start")
.to("dynamic-router:demo?recipientMode=allMatch")
.to("mock:result");
// first recipient (priority 1): leaves its copy untouched
from("direct:first").log("first recipient");
// second recipient (priority 2): modifies its copy
from("direct:second")
.setProperty("enrichedBy", constant("second"))
.setBody(constant("modified by second"));
}
};
}
@Test
void defaultAggregationShouldKeepLastReplyAsDocumented() throws Exception {
subscribe("first-sub", "direct:first", 1);
subscribe("second-sub", "direct:second", 2);
getMockEndpoint("mock:result").expectedMessageCount(1);
template.sendBody("direct:start", "original");
MockEndpoint.assertIsSatisfied(context);
Exchange result = getMockEndpoint("mock:result").getExchanges().get(0);
// Documentation: "By default, Camel will use the last reply as the
outgoing message."
// Both assertions FAIL: the body is still "original" and the property
is null,
// because the first recipient's (untouched) copy wins.
assertEquals("modified by second",
result.getMessage().getBody(String.class));
assertEquals("second", result.getProperty("enrichedBy", String.class));
}
private void subscribe(String id, String uri, int priority) {
template.sendBody("dynamic-router-control:subscribe",
DynamicRouterControlMessage.Builder.newBuilder()
.subscribeChannel("demo")
.subscriptionId(id)
.destinationUri(uri)
.priority(priority)
.predicate("${body} != null")
.expressionLanguage("simple")
.build());
}
}
{code}
h2. Suggested resolution
Either:
# restore {{UseLatestAggregationStrategy}} as the default, matching the
documentation and the pre-4.4 behavior, or
# keep {{NoopAggregationStrategy}} but update the option description, the
javadoc of {{createAggregationStrategy}} (and remove the stale
{{UseLatestAggregationStrategy}} import), clearly stating that the first
matching recipient's result is kept by default.
Option 1 seems preferable since the current behavior looks unintentional (stale
import, contradicting javadoc) and silently drops data.
Reporter: Thomas LE GUEN
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
