gnodet-bot commented on code in PR #29599:
URL: https://github.com/apache/camel/pull/29599#discussion_r4535075960
##########
core/camel-core-engine/src/main/java/org/apache/camel/impl/DefaultRouteTopologyDumper.java:
##########
@@ -107,16 +124,67 @@ public TopologyResult dumpTopology(CamelContext context) {
private static void addEdges(
CamelContext context, List<TopologyEdge> edges, Map<String,
List<String>> inputUriToRouteIds,
- String fromRouteId, String uri, String kind, String via, String
handling) {
+ String fromRouteId, String uri, String kind, String via, String
handling, boolean reply) {
String outputUri = normalizeUri(context, uri);
List<String> targetRouteIds = inputUriToRouteIds.get(outputUri);
if (targetRouteIds != null) {
String scheme = extractScheme(outputUri);
String connType = INTERNAL_SCHEMES.contains(scheme) ? "internal" :
"external";
for (String targetId : targetRouteIds) {
- edges.add(new TopologyEdge(fromRouteId, targetId, outputUri,
connType, kind, via, handling));
+ edges.add(new TopologyEdge(fromRouteId, targetId, outputUri,
connType, kind, via, handling, reply));
+ }
+ }
+ }
+
+ /**
+ * Whether the route waits for the answer of what it sends to, as the send
processor decides the exchange pattern:
+ * the exchangePattern of the endpoint uri, else the pattern of the send
(to with pattern InOut, a wire tap is
+ * InOnly), else the pattern of the message the route consumed (InOut from
platform-http, InOnly from a timer).
+ * Direct always runs the target before the route goes on.
+ */
+ static boolean waitsForAnswer(CamelContext context, RouteDefinition route,
ProcessorDefinition<?> send, String uri) {
+ if (uri == null || send instanceof WireTapDefinition) {
+ // a wire tap sends a copy on another thread: the route goes on
without its answer
+ return false;
+ }
+ if (WAITING_SCHEMES.contains(extractScheme(uri))) {
+ return true;
+ }
+ ExchangePattern pattern = null;
+ try {
+ pattern =
EndpointHelper.resolveExchangePatternFromUrl(context.resolvePropertyPlaceholders(uri));
+ } catch (Exception e) {
+ // a uri that cannot be resolved says nothing
+ }
+ if (pattern == null) {
+ String sendPattern = send instanceof SendDefinition<?> sd ?
sd.getPattern()
+ : send instanceof ToDynamicDefinition td ? td.getPattern()
: null;
+ if (sendPattern != null) {
+ pattern =
context.getTypeConverter().tryConvertTo(ExchangePattern.class,
+ context.resolvePropertyPlaceholders(sendPattern));
Review Comment:
⚠️ **Uncaught `IllegalArgumentException`**: `resolvePropertyPlaceholders`
throws when a placeholder cannot be resolved. The first call on `uri` is
guarded by the try-catch above, but this second call on `sendPattern` is not —
if the pattern is `{{myPattern}}` and unresolvable at analysis time, the entire
topology dump crashes. Wrap this block:
```suggestion
if (sendPattern != null) {
try {
pattern =
context.getTypeConverter().tryConvertTo(ExchangePattern.class,
context.resolvePropertyPlaceholders(sendPattern));
} catch (Exception e) {
// a pattern that cannot be resolved says nothing
}
```
##########
core/camel-console/src/generated/resources/META-INF/org/apache/camel/dev-console/route-topology.json:
##########
@@ -146,8 +146,15 @@
"handling": {
"type": "string",
"description": "For an error path, what happens to the failure:
handled (the message ends there), continued (the route goes on after it) or
notHandled (the failure goes on to the caller); absent when a predicate decides
it"
+ },
+ "reply": {
+ "type": "boolean",
+ "description": "Whether the source route waits for the answer of
the target route: a call (direct) or a request-reply over a broker, rather than
a hand-off"
}
- }
+ },
+ "required": [
+ "reply"
Review Comment:
⚠️ **Schema/implementation mismatch**: `required: ["reply"]` claims every
edge object must include `reply`, but
`DefaultDumpRoutesStrategy.doDumpTopologyAsJSon()` only writes `reply` when it
is `true` (omits it when `false`). The upgrade guide explicitly states this is
intentional, so the schema is wrong. Remove `reply` from `required` — it is an
optional field present only when true:
```suggestion
"required": [
```
Actually remove the entire `required` block (lines 155-157) — `reply` is
optional in the JSON output.
##########
core/camel-core-engine/src/main/java/org/apache/camel/impl/DefaultDumpRoutesStrategy.java:
##########
@@ -341,6 +341,9 @@ protected void doDumpTopologyAsJSon(CamelContext
camelContext) {
if (edge.handling() != null) {
jo.put("handling", edge.handling());
}
+ if (edge.reply()) {
+ jo.put("reply", true);
Review Comment:
If the fix is to always write `reply` (matching the `required` constraint),
replace this conditional write:
```suggestion
jo.put("reply", edge.reply());
```
Otherwise, remove `reply` from `required` in the JSON schema (see the other
comment). Pick one — schema and implementation must agree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
