gnodet-bot commented on code in PR #28435:
URL: https://github.com/apache/camel/pull/28435#discussion_r4369967252
##########
tooling/maven/camel-package-maven-plugin/src/main/java/org/apache/camel/maven/packaging/BodyTypesHelper.java:
##########
@@ -141,6 +145,32 @@ private static void checkFrom(Class<?> owner, String side,
BodyType bodyType) {
}
}
+ /**
+ * The option that names the Java type says the body after a step when it
is set, whatever other options say: the
+ * class of what a step needs is not a choice of the step, and an option
that is also one the body depends on, or
+ * has bodies for its values, would say two things.
+ */
+ private static void checkJavaTypeOption(
+ Class<?> owner, String side, BodyType bodyType, Map<String,
BaseOptionModel> options) {
+ String option = bodyType.javaTypeOption();
+ checkOption(owner, side, option, options);
+ if (IN.contains(side)) {
+ throw new IllegalArgumentException(
+ owner.getName() + ": the @BodyTypes " + side + " is what a
step needs, which has no javaTypeOption");
+ }
+ if (!bodyType.from().isEmpty()) {
+ throw new IllegalArgumentException(
+ owner.getName() + ": the @BodyTypes " + side + " is from "
+ bodyType.from()
+ + ", so it has no
javaTypeOption of its own");
+ }
+ if (Arrays.asList(bodyType.dependsOn()).contains(option)
+ || Arrays.stream(bodyType.byOption()).anyMatch(w ->
w.option().equals(option))) {
+ throw new IllegalArgumentException(
+ owner.getName() + ": the @BodyTypes " + side + " has the
javaTypeOption " + option
+ + ", which is not in dependsOn
or byOption as well");
Review Comment:
⚠️ **Misleading error message** — the condition fires when `option` IS in
`dependsOn` or `byOption`, but the message says _"which is not in dependsOn or
byOption as well"_. The negation is inverted: the message should say _"which is
also in dependsOn or byOption"_ (or similar) to match the actual logic.
```suggestion
if (Arrays.asList(bodyType.dependsOn()).contains(option)
|| Arrays.stream(bodyType.byOption()).anyMatch(w ->
w.option().equals(option))) {
throw new IllegalArgumentException(
owner.getName() + ": the @BodyTypes " + side + " has the
javaTypeOption " + option
+ ", which is also in
dependsOn or byOption");
}
```
##########
components/camel-hl7/src/main/java/org/apache/camel/component/hl7/Hl7TerserLanguage.java:
##########
@@ -46,7 +46,8 @@
+ " in the charset of
MSH-18, a HAPI Message, or a DOM of HL7 XML. Another"
+ " body fails"),
expressionOut = @BodyType(value = { Carrier.TEXT, Carrier.NULL },
javaType = "java.lang.String",
- dependsOn = "resultType",
+
Review Comment:
🔧 Stray blank line inside the annotation block — left behind when
`dependsOn` was replaced by `javaTypeOption`. The empty line between `javaType
= "java.lang.String",` and `javaTypeOption = "resultType",` is unnecessary.
##########
tooling/camel-tooling-model/src/main/java/org/apache/camel/tooling/model/BodyTypeModel.java:
##########
@@ -145,6 +154,80 @@ public void setFrom(String from) {
this.from = from;
}
+ /**
+ * The option that names the Java type of the body, such as resultType of
a language: when it is set, the body is of
+ * the class it names, or null. The carriers say the body when it is not
set.
+ */
+ public String getJavaTypeOption() {
+ return javaTypeOption;
+ }
+
+ public void setJavaTypeOption(String javaTypeOption) {
+ this.javaTypeOption = javaTypeOption;
+ }
+
+ /**
+ * The carrier of a body of a Java type, such as a type option names: text
for String, bytes for byte[], stream,
+ * file, value, map, list (a Collection or an array) and iterator, and
pojo for any other class. The type is a class
+ * name, with or without generics, or a short name Camel takes, such as
String or int.
+ */
+ public static String carrierOf(String javaType) {
+ if (javaType == null || javaType.isBlank()) {
+ return "unknown";
+ }
+ String name = javaType.trim();
+ int generic = name.indexOf('<');
+ if (generic > 0) {
+ name = name.substring(0, generic).trim();
+ }
+ String answer = CARRIERS.get(name);
+ if (answer != null) {
+ return answer;
+ }
+ if (name.endsWith("[]")) {
+ return "list";
+ }
+ return "pojo";
+ }
+
+ private static Map<String, String> carriers() {
+ Map<String, String> answer = new HashMap<>();
+ for (String name : List.of("String", "java.lang.String",
"java.lang.CharSequence", "java.lang.StringBuilder")) {
+ answer.put(name, "text");
+ }
+ for (String name : List.of("byte[]", "java.lang.byte[]")) {
Review Comment:
💡 `java.lang.byte[]` is not a valid Java FQN — `byte` is a primitive type,
not a class in `java.lang`. This entry will never match any real input and is
dead code in the carrier map. Only `byte[]` is needed here.
```suggestion
for (String name : List.of("byte[]")) {
```
##########
components/camel-quickjs/src/main/java/org/apache/camel/language/quickjs/QuickjsLanguage.java:
##########
@@ -64,7 +64,8 @@
description = "A JSON copy of the body, in
the variable body: bytes as Base64 text, and a"
+ " POJO through Jackson. A
stream fails: convert it to text first"),
expressionOut = @BodyType(value = { Carrier.TEXT, Carrier.VALUE,
Carrier.MAP, Carrier.LIST, Carrier.NULL },
- dependsOn = "resultType",
+
Review Comment:
🔧 Same stray blank line as in `Hl7TerserLanguage.java` — the empty line
inside the annotation block between the `value = { ... }` attribute and
`javaTypeOption = "resultType",` can be removed.
--
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]
