gnodet-bot commented on code in PR #29342:
URL: https://github.com/apache/camel/pull/29342#discussion_r4494053268
##########
tooling/maven/camel-package-maven-plugin/src/main/java/org/apache/camel/maven/packaging/PrepareCamelJBangMojo.java:
##########
@@ -68,62 +64,63 @@ public PrepareCamelJBangMojo(MavenProjectHelper
projectHelper, BuildContext buil
}
/**
- * Parses the Camel CLI configuration java source file.
+ * Reads the Camel CLI configuration of a compiled configuration class:
its static fields with {@code @Metadata}.
+ * The compiler has resolved the values (a description written as string
literals joined with {@code +} is one
+ * text), so the source is not parsed.
*/
- public static List<JBangModel.JBangOptionModel>
parseConfigurationSource(String fileName) throws IOException {
- return parseConfigurationSource(new File(fileName));
- }
-
- /**
- * Parses the Camel CLI configuration java source file.
- */
- public static List<JBangModel.JBangOptionModel>
parseConfigurationSource(File file) throws IOException {
+ public static List<JBangModel.JBangOptionModel>
parseConfigurationClass(Class<?> clazz) {
final List<JBangModel.JBangOptionModel> answer = new ArrayList<>();
-
- JavaClassSource clazz = (JavaClassSource) Roaster.parse(file);
- List<FieldSource<JavaClassSource>> fields = clazz.getFields();
- fields = fields.stream().filter(StaticCapable::isStatic).toList();
- fields.forEach(f -> {
- AnnotationSource<?> as = f.getAnnotation(Metadata.class);
- if (as != null) {
- String name = f.getStringInitializer();
- String javaType = as.getStringValue("javaType");
- String defaultValue = as.getStringValue("defaultValue");
- String desc = as.getStringValue("description");
- String label = as.getStringValue("label");
- boolean secret = "true".equals(as.getStringValue("secret"));
- String security = as.getStringValue("security");
- if ("secret".equals(security) && !secret) {
- secret = true;
- }
- boolean required =
"true".equals(as.getStringValue("required"));
- boolean deprecated = clazz.getAnnotation(Deprecated.class) !=
null || f.getAnnotation(Deprecated.class) != null;
- JBangModel.JBangOptionModel model = new
JBangModel.JBangOptionModel();
- model.setName(name);
- model.setJavaType(javaType);
- model.setDescription(JavadocHelper.sanitizeDescription(desc,
false));
- model.setLabel(label);
- model.setSourceType(null);
- model.setDeprecated(deprecated);
- model.setSecret(secret);
- model.setSecurity(security);
- model.setRequired(required);
- List<String> enums = null;
- String text = as.getStringValue("enums");
- if (text != null) {
- enums = Arrays.asList(text.split(","));
- }
- model.setEnums(enums);
- String type = MojoHelper.getType(javaType, enums != null &&
!enums.isEmpty(), false);
- model.setType(type);
- model.setDefaultValue(asDefaultValue(type, defaultValue));
- answer.add(model);
+ for (Field f : clazz.getDeclaredFields()) {
+ Metadata as = f.getAnnotation(Metadata.class);
+ if (as == null || !Modifier.isStatic(f.getModifiers()) ||
f.getType() != String.class) {
+ continue;
}
- });
-
+ String name;
+ try {
+ f.trySetAccessible();
Review Comment:
⚠️ **`trySetAccessible()` return value is discarded.** If it returns `false`
(access denied without throwing), the next line `f.get(null)` will throw
`IllegalAccessException`, which is caught and re-thrown as
`IllegalStateException`. No runtime bug — but calling `trySetAccessible()`
without checking the return value is misleading. Consider:
```suggestion
if (!f.trySetAccessible()) {
throw new IllegalStateException("Cannot access " +
clazz.getName() + "." + f.getName());
}
```
This makes the intent explicit and fails at the right point with a clear
message.
##########
tooling/maven/camel-package-maven-plugin/src/main/java/org/apache/camel/maven/packaging/PrepareCamelJBangMojo.java:
##########
@@ -68,62 +64,63 @@ public PrepareCamelJBangMojo(MavenProjectHelper
projectHelper, BuildContext buil
}
/**
- * Parses the Camel CLI configuration java source file.
+ * Reads the Camel CLI configuration of a compiled configuration class:
its static fields with {@code @Metadata}.
+ * The compiler has resolved the values (a description written as string
literals joined with {@code +} is one
+ * text), so the source is not parsed.
*/
- public static List<JBangModel.JBangOptionModel>
parseConfigurationSource(String fileName) throws IOException {
- return parseConfigurationSource(new File(fileName));
- }
-
- /**
- * Parses the Camel CLI configuration java source file.
- */
- public static List<JBangModel.JBangOptionModel>
parseConfigurationSource(File file) throws IOException {
+ public static List<JBangModel.JBangOptionModel>
parseConfigurationClass(Class<?> clazz) {
final List<JBangModel.JBangOptionModel> answer = new ArrayList<>();
-
- JavaClassSource clazz = (JavaClassSource) Roaster.parse(file);
- List<FieldSource<JavaClassSource>> fields = clazz.getFields();
- fields = fields.stream().filter(StaticCapable::isStatic).toList();
- fields.forEach(f -> {
- AnnotationSource<?> as = f.getAnnotation(Metadata.class);
- if (as != null) {
- String name = f.getStringInitializer();
- String javaType = as.getStringValue("javaType");
- String defaultValue = as.getStringValue("defaultValue");
- String desc = as.getStringValue("description");
- String label = as.getStringValue("label");
- boolean secret = "true".equals(as.getStringValue("secret"));
- String security = as.getStringValue("security");
- if ("secret".equals(security) && !secret) {
- secret = true;
- }
- boolean required =
"true".equals(as.getStringValue("required"));
- boolean deprecated = clazz.getAnnotation(Deprecated.class) !=
null || f.getAnnotation(Deprecated.class) != null;
- JBangModel.JBangOptionModel model = new
JBangModel.JBangOptionModel();
- model.setName(name);
- model.setJavaType(javaType);
- model.setDescription(JavadocHelper.sanitizeDescription(desc,
false));
- model.setLabel(label);
- model.setSourceType(null);
- model.setDeprecated(deprecated);
- model.setSecret(secret);
- model.setSecurity(security);
- model.setRequired(required);
- List<String> enums = null;
- String text = as.getStringValue("enums");
- if (text != null) {
- enums = Arrays.asList(text.split(","));
- }
- model.setEnums(enums);
- String type = MojoHelper.getType(javaType, enums != null &&
!enums.isEmpty(), false);
- model.setType(type);
- model.setDefaultValue(asDefaultValue(type, defaultValue));
- answer.add(model);
+ for (Field f : clazz.getDeclaredFields()) {
+ Metadata as = f.getAnnotation(Metadata.class);
+ if (as == null || !Modifier.isStatic(f.getModifiers()) ||
f.getType() != String.class) {
+ continue;
}
- });
-
+ String name;
+ try {
+ f.trySetAccessible();
+ name = (String) f.get(null);
+ } catch (IllegalAccessException e) {
+ throw new IllegalStateException("Cannot read " +
clazz.getName() + "." + f.getName(), e);
+ }
+ String javaType = text(as.javaType());
+ String defaultValue = text(as.defaultValue());
+ String security = text(as.security());
+ boolean secret = as.secret() || "secret".equals(security);
+ boolean deprecated = clazz.isAnnotationPresent(Deprecated.class)
|| f.isAnnotationPresent(Deprecated.class);
+ JBangModel.JBangOptionModel model = new
JBangModel.JBangOptionModel();
+ model.setName(name);
+ model.setJavaType(javaType);
+
model.setDescription(JavadocHelper.sanitizeDescription(text(as.description()),
false));
+ model.setLabel(text(as.label()));
+ model.setSourceType(null);
+ model.setDeprecated(deprecated);
+ model.setSecret(secret);
+ model.setSecurity(security);
+ model.setRequired(as.required());
+ String enumText = text(as.enums());
+ List<String> enums = enumText != null ?
Arrays.asList(enumText.split(",")) : null;
+ model.setEnums(enums);
+ String type = MojoHelper.getType(javaType, enums != null &&
!enums.isEmpty(), false);
+ model.setType(type);
+ model.setDefaultValue(asDefaultValue(type, defaultValue));
+ answer.add(model);
+ }
return answer;
}
+ // an annotation attribute that is not set is an empty text
+ private static String text(String value) {
+ return value == null || value.isEmpty() ? null : value;
+ }
+
+ /** The class of a configuration source file: its path under
src/main/java. */
+ static String className(File file) {
+ String path = file.getPath().replace(File.separatorChar, '/');
+ int root = path.indexOf("src/main/java/");
+ String relative = root >= 0 ? path.substring(root +
"src/main/java/".length()) : file.getName();
Review Comment:
⚠️ **Fallback branch silently produces a package-less class name.** When
`file.getPath()` doesn't contain `src/main/java/` (e.g. the file is in a
non-standard layout), `file.getName()` returns just the filename (e.g.
`CamelJBangConstants.java`), and after stripping `.java` you get
`CamelJBangConstants` — a simple name with no package.
`ClassLoader.loadClass("CamelJBangConstants")` will then throw
`ClassNotFoundException` with an opaque message that doesn't point to the root
cause.
Since the method can't compute a valid class name in this case, it should
fail fast:
```suggestion
if (root < 0) {
throw new IllegalArgumentException("Cannot determine class name
for " + file + ": not under src/main/java/");
}
String relative = path.substring(root + "src/main/java/".length());
```
--
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]
