gnodet-bot commented on code in PR #29578:
URL: https://github.com/apache/camel/pull/29578#discussion_r4536027100
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/PluginHelper.java:
##########
@@ -86,6 +88,49 @@ private PluginHelper() {
// prevent instantiation of utility class
}
+ public static Path getCustomPluginConfigPath() {
+ return getCustomPluginConfigPath(null);
+ }
+
+ /**
+ * Resolves the path to the plugin configuration file. Checks, in order:
+ * <ol>
+ * <li>A path set programmatically via method argument</li>
+ * <li>The {@value #PLUGIN_CONFIG_ENV} environment variable</li>
+ * <li>The default {@value #PLUGIN_CONFIG} in the user home directory</li>
+ * </ol>
+ */
+ public static Path getCustomPluginConfigPath(Path configPath) {
+ return resolvePluginConfigPath(configPath,
System.getenv(PLUGIN_CONFIG_ENV));
+ }
+
+ static Path resolvePluginConfigPath(Path configPath, String envValue) {
+ Path pluginConfigPath;
+ if (configPath != null) {
+ pluginConfigPath = configPath;
+ } else {
+ if (envValue != null && !envValue.isBlank()) {
+ try {
+ pluginConfigPath = Path.of(envValue);
+ } catch (java.nio.file.InvalidPathException e) {
+ throw new IllegalStateException(
+ "Invalid path in " + PLUGIN_CONFIG_ENV + "
environment variable: " + envValue, e);
+ }
+ } else {
+ return CommandLineHelper.getHomeDir().resolve(PLUGIN_CONFIG);
+ }
+ }
+
+ if (Files.isDirectory(pluginConfigPath)) {
+ return pluginConfigPath.resolve(PLUGIN_CONFIG);
+ }
+ Path fileName = pluginConfigPath.getFileName();
+ if (fileName != null &&
fileName.toString().toLowerCase().endsWith(".json")) {
+ return pluginConfigPath;
+ }
+ return pluginConfigPath.resolve(PLUGIN_CONFIG);
Review Comment:
⚠️ **Undocumented path resolution fallback** — when the supplied path is
neither an existing directory nor a `.json`-suffixed filename, this falls
through and appends `/.camel-jbang-plugins.json` to it (treating it as a
directory). A user passing `--plugin-config /tmp/myplugins` (a plain name
without extension) would silently get
`/tmp/myplugins/.camel-jbang-plugins.json` instead of an error.
The docs say "a JSON file or a directory" with no mention of this third
case. Consider either rejecting it with a clear error (preferred) or at least
documenting it in the Javadoc and the `--plugin-config` description:
```suggestion
if (Files.isDirectory(pluginConfigPath)) {
return pluginConfigPath.resolve(PLUGIN_CONFIG);
}
Path fileName = pluginConfigPath.getFileName();
if (fileName != null &&
fileName.toString().toLowerCase().endsWith(".json")) {
return pluginConfigPath;
}
// Path has no .json suffix and does not exist as a directory yet;
// treat it as a directory path and resolve the default filename
inside it.
return pluginConfigPath.resolve(PLUGIN_CONFIG);
```
--
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]
