gnodet-bot commented on code in PR #28435:
URL: https://github.com/apache/camel/pull/28435#discussion_r4370071288
##########
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 (previously raised, still not fixed)** — the
condition on lines 166–167 fires when `option` IS in `dependsOn` or `byOption`,
but the message says _"which is **not** in dependsOn or byOption as well"_.
This is exactly backwards: a developer adding a new language who hits this
build error will see a message claiming the option is absent from `dependsOn`,
when the actual problem is the opposite.
```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 must not also be
in dependsOn or byOption");
}
```
##########
tooling/camel-tooling-model/src/main/java/org/apache/camel/tooling/model/BodyTypeModel.java:
##########
@@ -145,6 +154,84 @@ 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, and the
+ * options of {@link #getDependsOn()} keep their defaults, the body is of
the class it names, or null, and the
+ * bodies of {@link #getByOption()} for other options do not hold. 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, unknown
for Object, which can be anything, 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 (previously raised, still not
fixed)** — `byte` is a primitive type, not a class in `java.lang`. No JVM or
Camel code ever produces the string `"java.lang.byte[]"` as a type name, so
this map entry is dead code. The plain `"byte[]"` entry (already in the same
`List.of(...)`) is the correct one.
```suggestion
for (String name : List.of("byte[]")) {
```
--
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]
