gnodet-bot commented on code in PR #28723:
URL: https://github.com/apache/camel/pull/28723#discussion_r4422480270
##########
dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/TextBodyFlow.java:
##########
@@ -691,6 +716,16 @@ private static Text setBodyText(JsonNode value) {
if (expression == null) {
return null;
}
+ // r24 connect-http-client: a setBody ${null} before the split of the
lines, and a constant "" before a GET
+ if ("constant".equals(language) && expression.isEmpty()
+ || "simple".equals(language) && expression.trim().isEmpty()) {
Review Comment:
💡 **Readability:** The `&&`/`||` mix is correct by Java precedence (`&&`
binds tighter), but explicit parentheses would make the intent immediately
clear to readers:
```suggestion
if (("constant".equals(language) && expression.isEmpty())
|| ("simple".equals(language) &&
expression.trim().isEmpty())) {
```
##########
dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java:
##########
@@ -1281,12 +1331,100 @@ void checkDynamicUri(JsonNode node, NodePath path,
List<Error> errors) {
+ " is fixed when the route starts, so
it is sent as text: write toD: to build"
+ " the uri for each message")
.build());
+ } else {
+ String option = staticOptionWithExpression(uri, value);
+ if (option != null) {
+ errors.add(Error.builder()
+ .keyword("type")
+ .instanceLocation(at)
+ .messageKey("type")
+ .format(new MessageFormat("{0}"))
+ .arguments("to: " + option)
+ .build());
+ }
}
}
checkDynamicUri(value, path.append(name), errors);
}
}
+ /**
+ * An option of a to: with a simple expression that the component does not
evaluate, said with the header to set
+ * instead; null when there is none. The options of a to: are fixed when
the route starts: r24
+ * connect-service-kafka-orders step 3 wrote key: "${header.orderId}" on a
to: kafka, and every record got that text
+ * as its key. Only an option with a header of its own for each message
(CamelKafkaKey for the key of kafka) is
+ * reported: some components evaluate their options themselves (openfga,
the method of bean), and the catalog marks
+ * only some of them (the fileName of file), or says so in the description.
+ */
+ private String staticOptionWithExpression(String uri, JsonNode to) {
+ if (uri == null) {
+ return null;
+ }
+ int colon = uri.indexOf(':');
+ int question = uri.indexOf('?');
+ String component = colon > 0 && (question < 0 || colon < question) ?
uri.substring(0, colon)
+ : question > 0 ? uri.substring(0, question) : uri;
+ Map<String, String> options = new LinkedHashMap<>();
+ if (question > 0) {
+ for (String pair : uri.substring(question + 1).split("&")) {
+ int eq = pair.indexOf('=');
+ if (eq > 0) {
+ options.put(pair.substring(0, eq), pair.substring(eq + 1));
+ }
+ }
+ }
+ JsonNode parameters = to != null && to.isObject() ?
to.get("parameters") : null;
+ if (parameters != null && parameters.isObject()) {
+ for (var it = parameters.fieldNames(); it.hasNext();) {
+ String name = it.next();
+ if (parameters.get(name).isValueNode()) {
+ options.put(name, parameters.get(name).asText());
+ }
+ }
+ }
+ ComponentModel model;
+ try {
+ model = catalog().componentModel(component);
+ } catch (Exception e) {
+ LOG.debug("Cannot read the catalog model of component {}: its
options are left alone", component, e);
+ return null;
+ }
+ if (model == null) {
+ return null;
+ }
+ for (Map.Entry<String, String> option : options.entrySet()) {
+ String text = option.getValue();
+ int start = text.indexOf("${");
+ if (start < 0 || start >= 2 && text.startsWith(":#", start - 2)) {
+ continue;
+ }
+ var known = model.getEndpointParameterOptions().stream()
+ .filter(o -> o.getName().equals(option.getKey()))
+ .findFirst();
+ String description = known.map(o ->
String.valueOf(o.getDescription()).toLowerCase(Locale.ROOT)).orElse("");
+ if (known.isEmpty() || known.get().isSupportSimpleExpression() ||
description.contains("expression")
+ || description.contains("simple")) {
+ continue;
+ }
+ // the header the producer reads for each message, such as
CamelKafkaKey for the key of kafka
+ String header = model.getEndpointHeaders().stream()
+ .map(BaseOptionModel::getName)
+ .filter(h -> h.equalsIgnoreCase("Camel" + component +
option.getKey())
+ || h.equalsIgnoreCase(component + "." +
option.getKey()))
+ .findFirst()
Review Comment:
⚠️ **Limitation:** `"Camel" + component + option.getKey()` won't match
header names for hyphenated components (e.g. `aws2-s3` would produce
`Camelaws2-s3key` instead of the actual CamelCase header `CamelAws2S3Key`).
This is a false-negative-only issue — the check silently skips validation for
those components rather than producing false positives — but it means the
feature won't fire for a subset of the component catalog. Worth noting in a
comment or a follow-up ticket.
--
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]
