gnodet-bot commented on code in PR #26914:
URL: https://github.com/apache/camel/pull/26914#discussion_r4111437815
##########
dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/BodyTypeFlow.java:
##########
@@ -263,6 +263,13 @@ private static boolean producesTheBody(JsonNode node) {
}
for (var it = node.fieldNames(); it.hasNext();) {
String name = it.next();
+ // A rest-openapi producer returns the HTTP response as the
exchange body. Keep this
+ // explicit because its request verb (which may be GET) does not
describe its response.
+ if (("to".equals(name) || "toD".equals(name))
+ && endpointOf(node.get(name)) != null
+ && endpointOf(node.get(name)).startsWith("rest-openapi:"))
{
+ return true;
+ }
Review Comment:
💡 **Observation (low):** This entire block is dead code — `"to"` and `"toD"`
are already in `SETS_THE_BODY` (line 60–61), so the existing
`SETS_THE_BODY.contains(name)` check on line 273 already returns `true` for any
`to`/`toD` node, regardless of whether the endpoint is `rest-openapi:` or
anything else.
The intent is clear — documenting *why* a rest-openapi producer sets the
body — but the guard can never change the outcome. If it's kept purely as
documentation, a comment on the existing `SETS_THE_BODY` entry would be
cleaner. If it's preparing for a future split where `to` is removed from
`SETS_THE_BODY`, that should be mentioned.
Also, `endpointOf(node.get(name))` is evaluated twice; if kept, cache it:
```suggestion
// A rest-openapi producer returns the HTTP response as the
exchange body. Keep this
// explicit because its request verb (which may be GET) does not
describe its response.
String ep = ("to".equals(name) || "toD".equals(name)) ?
endpointOf(node.get(name)) : null;
if (ep != null && ep.startsWith("rest-openapi:")) {
return true;
}
```
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/OpenApiVerbs.java:
##########
@@ -62,34 +81,76 @@ public static Set<String> bodylessEndpoints(String content,
Path directory) {
continue;
}
try {
- String text = Files.readString(spec);
- if (!text.stripLeading().startsWith("{")) {
- continue; // a YAML specification: not read here
- }
- JsonObject root = (JsonObject) Jsoner.deserialize(text);
- JsonObject paths = root.getMap("paths");
+ Map<?, ?> paths = paths(Files.readString(spec));
if (paths == null) {
continue;
}
- for (Map.Entry<String, Object> path : paths.entrySet()) {
- if (!(path.getValue() instanceof Map<?, ?> operations)) {
+ for (Object path : paths.values()) {
+ if (!(path instanceof Map<?, ?> operations)) {
continue;
}
for (Map.Entry<?, ?> operation : operations.entrySet()) {
String verb =
String.valueOf(operation.getKey()).toLowerCase(java.util.Locale.ROOT);
- if (!WITHOUT_BODY.contains(verb) ||
!(operation.getValue() instanceof Map<?, ?> details)) {
- continue;
+ if (WITHOUT_BODY.contains(verb) &&
operation.getValue() instanceof Map<?, ?> details
+ && details.get("operationId") != null) {
+ answer.add("direct:" + details.get("operationId"));
}
- Object id = details.get("operationId");
- if (id != null) {
- answer.add("direct:" + id);
+ }
+ }
+ } catch (Exception e) {
+ // Catching Exception handles unreadable and unparseable JSON
or YAML specifications.
+ }
+ }
+ // A producer URI carries its own operation reference. Read its spec
as well so its verb is
+ // resolved using the same rules as a rest binding (in particular, do
not infer from the URI).
+ Matcher producer = REST_OPENAPI_URI_PATTERN.matcher(content);
+ while (producer.find()) {
+ String specName = producer.group(1);
+ String targetOpId = producer.group(2);
+ Path spec = directory.resolve(specName);
+ if (!Files.isRegularFile(spec)) {
+ continue;
+ }
Review Comment:
💡 **Question (low):** The entries added here
(`"rest-openapi:specName#opId"`) feed into `BodyTypeFlow.check()`'s `known`
set, which is added to `restless` and checked against
`normalize(route.fromUri())`. But a `rest-openapi:` URI is a *producer*
endpoint (used in `to:`), not a *consumer* `from:` URI — no route starts with
`from: rest-openapi:stock-api.json#getStock`.
So this path through `restless` / `certainlyWithoutABody` would never match
a real route. The test (`restOpenApiProducerGetOperationSetsResponseBody`)
passes because `to` is already in `SETS_THE_BODY`, not because of this
`bodylessEndpoints` addition.
Is there a planned consumer-side scenario where this would be matched, or
should the entries use a different key that the body flow analysis actually
correlates?
--
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]
