gnodet-bot commented on code in PR #28047:
URL: https://github.com/apache/camel/pull/28047#discussion_r4306625674
##########
dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/TextBodyFlow.java:
##########
@@ -235,11 +327,21 @@ private static void walkBlock(
}
/** What is known about the body after a step. */
- private static Text after(
- String name, JsonNode value, JsonNode step, Text text, Map<String,
JsonNode> byFrom, Set<String> seen) {
+ private static Body after(
+ String name, JsonNode value, JsonNode step, Body body, Map<String,
JsonNode> byFrom, Set<String> seen) {
switch (name) {
case "setBody", "transform" -> {
- return setBodyText(value);
+ Text text = setBodyText(value);
+ if (text != null || !(body instanceof Parsed parsed)) {
+ return text;
+ }
+ String call = keepsData(value);
+ return call != null
+ ? new Parsed(parsed.origin() + ", and groovy " + call
+ " keeps it data", parsed.format())
Review Comment:
🔤 **Nit (from prior review, still open):** Awkward phrasing — missing "as".
```suggestion
? new Parsed(parsed.origin() + ", and groovy " +
call + " keeps it as data", parsed.format())
```
##########
dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/TextBodyFlow.java:
##########
@@ -316,6 +417,136 @@ private static Text setBodyText(JsonNode value) {
return null;
}
+ /** The body after an unmarshal: parsed data for the data formats that
make a Map or a List of the text. */
+ private static Parsed unmarshalled(JsonNode value) {
+ if (value == null || !value.isObject() ||
!value.fieldNames().hasNext()) {
+ return null;
+ }
+ String format = value.fieldNames().next();
+ if (!PARSES.contains(format)) {
+ return null;
+ }
+ JsonNode options = value.get(format);
+ for (String option : NOT_A_MAP) {
+ if (options != null && options.isObject() && options.has(option)) {
+ return null;
+ }
+ }
+ return new Parsed("unmarshal: " + format + " turns the text into
data", format);
+ }
+
+ /**
+ * A Groovy setBody whose result is data again: one collection method on
the body and nothing after it, such as
+ * body.findAll { ... }, or body.find { it.sku == ... }, which picks an
element whose fields it reads. The call, or
+ * null.
+ */
+ static String keepsData(JsonNode value) {
+ JsonNode node = value != null && value.has("expression") ?
value.get("expression") : value;
+ String script = node != null && node.isObject() ?
expressionText(node.get("groovy")) : null;
+ if (script == null) {
+ return null;
+ }
+ String s = script.trim();
+ Matcher m = COLLECTION_CALL.matcher(s);
+ if (!m.lookingAt()) {
+ return null;
+ }
+ String method = m.group(1);
+ int depth = 1;
+ int close = m.end();
+ for (; close < s.length() && depth > 0; close++) {
+ char c = s.charAt(close);
+ depth += c == '{' ? 1 : c == '}' ? -1 : 0;
+ }
+ if (depth != 0 || close != s.length()) {
+ // the closure does not end the script: something is done with its
result
+ return null;
+ }
+ String closure = s.substring(m.end(), close - 1);
+ Matcher p = CLOSURE_PARAMETER.matcher(closure);
+ String element = p.find() ? p.group(1) : "it";
+ boolean readsFields = Pattern.compile("(?<![\\w$])" + element +
"\\s*(\\??\\.\\s*[A-Za-z_]|\\[)")
+ .matcher(closure).find();
+ return GIVES_DATA.contains(method) || PICKS.contains(method) &&
readsFields
Review Comment:
🔤 **Nit (from prior review, still open):** The `&&` has higher precedence
than `||`, so this evaluates correctly as written — but the intent is clearer
with explicit parentheses.
```suggestion
return GIVES_DATA.contains(method) || (PICKS.contains(method) &&
readsFields)
```
##########
dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/TextBodyFlow.java:
##########
@@ -136,10 +201,28 @@ static void check(JsonNode target, NodePath path,
List<Error> errors) {
at = at.append("from").append("steps");
}
String id = entry.has("route") && entry.get("route").has("id") ?
entry.get("route").get("id").asText() : null;
- walk(steps, consumerText(from), at, id, byFrom, errors, new
HashSet<>());
+ String consumer = scheme(endpointOf(from));
+ boolean post = NO_HTTP_HEADERS.contains(consumer) &&
!setsHttpHeaders(steps);
+ Body end = walk(steps, consumerText(from), at, id, byFrom, errors,
new HashSet<>(), post);
+ // the body at the end of a platform-http route is the reply,
written as a stream
+ if ("platform-http".equals(consumer) && end instanceof Parsed
parsed && steps != null && steps.isArray()
+ && !steps.isEmpty() && steps.get(steps.size() -
1).fieldNames().hasNext()) {
+ int last = steps.size() - 1;
+
errors.add(error(at.append(last).append(steps.get(last).fieldNames().next()),
id,
+ new Need("the body at the end of the route is the
reply to the HTTP request", "as the last step"),
+ parsed));
+ }
}
}
+ /**
+ * Whether the steps set or read an HTTP control header (CamelHttpMethod,
CamelHttpQuery...), which can make an http
+ * producer call with GET and leave the body out.
+ */
+ private static boolean setsHttpHeaders(JsonNode steps) {
+ return steps != null &&
steps.toString().toLowerCase(Locale.ROOT).contains("camelhttp");
Review Comment:
⚠️ **Low-severity concern:** `steps.toString()` serializes the entire
`JsonNode` tree to a JSON string before doing a substring search. Any step
argument that happens to contain the text `"camelhttp"` — a string literal, an
expression, a header value — would be a false positive. Consider checking only
the DSL step `uri` fields rather than the full serialized representation.
--
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]
