davsclaus commented on code in PR #28500:
URL: https://github.com/apache/camel/pull/28500#discussion_r4375875666
##########
core/camel-support/src/main/java/org/apache/camel/support/BodyFacts.java:
##########
@@ -277,6 +284,42 @@ private static String unmarshalFirst(Exchange exchange,
Object body) {
};
}
+ /**
+ * When a marshal wrote the body from a Map or a List, as the message
history says: the marshal is wanted, for a
+ * later step that needs text (a file write), and the field is read after
it, too late; an unmarshal would undo it.
+ * Null when the history does not say so, as for a marshal of text, where
an unmarshal was meant.
+ */
+ private static String marshalledData(Exchange exchange, Object body) {
+ List<MessageHistory> history =
exchange.getProperty(ExchangePropertyKey.MESSAGE_HISTORY, List.class);
+ if (history == null || history.isEmpty()) {
+ return null;
+ }
+ String now = ObjectHelper.classCanonicalName(body);
+ for (int i = history.size() - 1; i >= 0; i--) {
+ MessageHistory step = history.get(i);
+ String type = step.getBodyType();
+ if (type == null || type.equals(now)) {
+ continue;
+ }
+ // the step that set the body, and the body it was reached with
+ if (step.getNode() == null ||
!"marshal".equals(step.getNode().getShortName())
+ || !isDataType(exchange.getContext(), type)) {
+ return null;
+ }
+ return "the " + step(step.getNode()) + " wrote it from data (" +
StringHelper.afterLast(type, ".", type)
+ + "): read the field before the marshal, for example into a
variable with setVariable, or keep the data"
+ + " in the body and marshal into a variable (marshal with
variableReceive, and send that variable with"
+ + " variableSend)";
+ }
+ return null;
+ }
+
+ /** Whether a class, by its name, is a Map or a collection. */
+ private static boolean isDataType(CamelContext context, String type) {
+ Class<?> clazz = context != null ?
context.getClassResolver().resolveClass(type) : null;
Review Comment:
Thanks for raising this. I'd rather keep it as it is, and here is why:
- `context.getClassResolver()` is not the thread-context loader in those
runtimes. Karaf and Quarkus install their own `ClassResolver`, which is the one
meant to see application classes.
- The types this looks at are what a data format leaves before a marshal:
`LinkedHashMap`, `ArrayList` and similar JDK collections, which any loader can
see.
- The check is only advisory. When the class cannot be resolved, the hint
falls back to the earlier "unmarshal it first" text. It can be less specific,
never wrong, and the route is not affected.
_Claude Code on behalf of davsclaus_
##########
dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/TextBodyFlow.java:
##########
@@ -565,14 +657,317 @@ private static Text setBodyText(JsonNode value) {
* iterator).
*/
private static Parsed unmarshalled(JsonNode value, CatalogBodyTypes
bodyTypes) {
- if (value == null || !value.isObject() ||
!value.fieldNames().hasNext()) {
+ String format = dataFormatOf(value);
+ if (format == null) {
return null;
}
- String format = value.fieldNames().next();
CatalogBodyTypes.Answer out = bodyTypes.unmarshalOut(format,
value.get(format));
return out != null && out.data() ? new Parsed("unmarshal: " + format +
" turns the text into data", format) : null;
}
+ /** The text a marshal writes; marshalled when the body it writes is known
to be parsed data. */
+ private static Text marshalled(JsonNode value, Body body) {
+ String format = dataFormatOf(value);
+ return new Text(
+ "marshal" + (format != null ? ": " + format : "")
+ + " writes data as text (marshal turns data into text,
unmarshal turns text into data)",
+ format, false, body instanceof Parsed);
+ }
+
+ /** The data format of an unmarshal or a marshal: the key that is not one
of its options, such as json. */
+ private static String dataFormatOf(JsonNode value) {
+ if (value == null || !value.isObject()) {
+ return null;
+ }
+ for (var it = value.fieldNames(); it.hasNext();) {
+ String key = it.next();
+ if (!DATA_FORMAT_OPTIONS.contains(key)) {
+ return key;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * An unmarshal of data that is parsed already: the body after an
unmarshal, or a variable that holds data
+ * (variableSend). The data format reads text, and a Map is no text: "No
type converter available to convert from
+ * type: java.util.LinkedHashMap to the required type:
java.io.InputStream". Reports it, and answers whether it did.
+ */
+ private static boolean parsesData(String name, JsonNode value, Body body,
NodePath at, Walk w) {
+ String format = "unmarshal".equals(name) ? dataFormatOf(value) : null;
+ if (format == null) {
+ return false;
+ }
+ String variable = fixedName(value.get("variableSend"));
+ Body input = variable != null ? w.variables().get(variable) :
value.has("variableSend") ? null : body;
+ CatalogBodyTypes.Answer in = input instanceof Parsed ?
w.bodyTypes().unmarshalIn(format, value.get(format)) : null;
+ if (in == null || !in.onlyPayload()) {
+ return false;
+ }
+ String held = variable != null ? "the variable " + variable + " holds"
: "the body here is";
+ String fix = variable != null
+ ? "the variable is data already, so leave this unmarshal out,
and read its fields as they are, such as"
+ + " ${variable." + variable + "[name]}"
+ : "the body is data already, so leave this unmarshal out, and
read its fields as they are; to parse text"
+ + " in a variable, unmarshal it with variableSend and
variableReceive set to its name";
+ w.errors().add(Error.builder()
+ .keyword("type")
+ .instanceLocation(at)
+ .messageKey("type")
+ .format(new MessageFormat("{0}"))
+ .arguments((w.routeId() != null ? "route " + w.routeId() + ":
" : "") + "unmarshal: " + format
+ + " parses text (" + in.inWords() + "), but " + held
+ + " parsed data (a Map or a List), not text - " +
input.origin() + ": " + fix)
+ .build());
+ return true;
+ }
+
+ /**
+ * Remembers what a step puts in a variable: the result of a step with
variableReceive, which is what the step would
+ * leave in the body, and the body or a constant a setVariable sets. A
step that may set any variable (a bean, a
+ * call of a route the walk does not follow, a block with such a step)
makes the walk forget them all, so that it
+ * only reports a variable it is certain of.
+ */
+ private static void remember(String name, JsonNode value, Body body, Walk
w) {
+ Map<String, Body> variables = w.variables();
+ String uri = sendUri(name, value);
+ boolean callsARoute = uri != null && ROUTES.contains(scheme(uri))
+ && !("to".equals(name) && !value.has("variableReceive") &&
w.byFrom().containsKey(normalize(uri)));
+ if (SETS_VARIABLES.contains(name) || callsARoute ||
RouteGraph.SENDS.contains(name) && uri == null
+ || mayChangeVariables(value)) {
+ variables.clear();
+ }
+ String receive = value != null && value.isObject() ?
fixedName(value.get("variableReceive")) : null;
+ if (receive != null) {
+ put(variables, receive, received(name, value, uri, body, w));
+ } else if (value != null && value.isObject() &&
value.has("variableReceive")) {
+ // a variable named by an expression
+ variables.clear();
+ } else if ("setVariable".equals(name) ||
"removeVariable".equals(name)) {
+ JsonNode n = value != null && value.isObject() ? value.get("name")
: value;
+ String variable = fixedName(n);
+ if (variable == null) {
+ variables.clear();
+ } else if ("setVariable".equals(name)) {
+ put(variables, variable, setVariableBody(value, body));
+ } else {
+ variables.remove(variable);
+ }
+ }
+ }
+
+ /** Whether the steps of a block may set a variable: a step that does, or
a call of a route. */
+ private static boolean mayChangeVariables(JsonNode value) {
+ if (value == null || !value.isContainerNode()) {
+ return false;
+ }
+ String text = value.toString();
Review Comment:
Good point. Fixed in c8fe9f968d8a: `remember()` now returns before the check
when no variable is known, which is the case for almost every step, because
clearing an empty map does nothing. The text only comes from the route YAML (a
step's subtree), not from a message body, so it stays small even when it does
run.
_Claude Code on behalf of davsclaus_
##########
dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/TextBodyFlow.java:
##########
@@ -149,11 +188,15 @@ sealed interface Body permits Text, Parsed {
record Parsed(String origin, String format) implements Body {
}
- /** Text: the payload as it was read or written. */
- record Text(String origin, String format, boolean file) implements Body {
+ /** Text: the payload as it was read or written; marshalled when a marshal
wrote it from parsed data. */
+ record Text(String origin, String format, boolean file, boolean
marshalled) implements Body {
Review Comment:
I'd keep the record as it is. Only one call site passes all four arguments
(`marshalled(...)`, which computes the flag from the body before the marshal).
The others use the two- and three-argument constructors, so the extra boolean
doesn't appear anywhere else.
_Claude Code on behalf of davsclaus_
##########
dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/TextBodyFlow.java:
##########
@@ -171,16 +214,25 @@ Set<String> readable() {
* What a walk knows besides the body: the route, the routes of the file,
where reports go (null for a route a step
* calls, which reports from its own walk), whether an http producer sends
the body with POST, the verbs of the
* OpenAPI operations a rest-openapi call can be judged by (empty when a
REST binding may write the body), with the
- * fields their JSON requests need, and the body types of the catalog.
+ * fields their JSON requests need, the body types of the catalog, and
what is known about the variables of the
+ * message.
*/
private record Walk(String routeId, Map<String, JsonNode> byFrom,
List<Error> errors, Set<String> seen, boolean post,
Map<String, Map<String, String>> operations, Map<String,
Map<String, List<String>>> requestFields,
Map<String, ProjectFacts.Observed> observed, CatalogBodyTypes
bodyTypes,
- Map<String, ProjectFacts.KameletDataTypes> kamelets) {
+ Map<String, ProjectFacts.KameletDataTypes> kamelets, Map<String,
Body> variables) {
- /** The walk of a route a step calls: for its outcome alone. */
+ /** The walk of a route a step calls: for its outcome alone, and the
variables of the message it changes. */
Walk called() {
Review Comment:
Thanks, done in c8fe9f968d8a. The javadoc now says that the called route
runs on the same exchange, so it shares the variables, whereas a branch gets a
copy.
_Claude Code on behalf of davsclaus_
##########
dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/TextBodyFlow.java:
##########
@@ -134,6 +152,27 @@ final class TextBodyFlow {
/** The REST binding modes that write a Map or a List for the route. */
private static final Set<String> BINDS = Set.of("auto", "json", "xml",
"json_xml");
+ /** The steps that send the body to an endpoint, and can need it as text.
*/
+ private static final Set<String> SENDS_THE_BODY = Set.of("to", "toD",
"wireTap", "enrich");
+
+ /** The steps whose uri, and whose endpoint options, are simple templates
read for each message. */
+ private static final Set<String> TEMPLATE_URIS = Set.of("to", "toD",
"wireTap");
+
+ /** The steps that may set any variable, which the walk then forgets. */
+ private static final Set<String> SETS_VARIABLES = Set.of("bean",
"process", "script", "setVariables",
+ "removeVariables", "convertVariableTo", "kamelet", "claimCheck");
+
+ /** The components whose endpoint is a route, which may set variables of
the message. */
+ private static final Set<String> ROUTES = Set.of("direct", "seda",
"direct-vm", "vm", "kamelet");
+
+ /** What a block that may set variables has in it: a step that does, or a
call of a route. */
+ private static final List<String> MAY_SET_VARIABLES = List.of("ariable",
"\"bean\"", "\"process\"", "\"script\"",
Review Comment:
Done in c8fe9f968d8a, with a comment on the constant: "ariable" is one
substring for setVariable, removeVariable, variableReceive, ${variable.x} and
the like, whether the v is upper or lower case. Matching "direct" and "seda" is
intended, by the way: they are in the same list, because a call of a route may
set variables.
_Claude Code on behalf of davsclaus_
--
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]
