This is an automated email from the ASF dual-hosted git repository. christophd pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/camel.git
commit 98bd3f390014156605c5998284f522c688303bbf Author: Christoph Deppisch <[email protected]> AuthorDate: Wed Nov 11 22:52:41 2026 +0100 CAMEL-26764: Address review feedback - Return defensive copy from PluginType.getAppliesTo() - Update stale comment in PluginFilteringTest - Extract toJsonArray helper in PluginHelper to deduplicate - Fix test compilation after picocli-to-aesh migration Co-Authored-By: Claude Opus 4.6 <[email protected]> --- .../dsl/jbang/core/commands/plugin/PluginAdd.java | 6 +--- .../camel/dsl/jbang/core/common/PluginHelper.java | 14 ++++---- .../camel/dsl/jbang/core/common/PluginType.java | 2 +- .../dsl/jbang/core/common/PluginFilteringTest.java | 42 +++++++++++----------- 4 files changed, 31 insertions(+), 33 deletions(-) diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/plugin/PluginAdd.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/plugin/PluginAdd.java index 7e46197ac116..e127c477f324 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/plugin/PluginAdd.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/plugin/PluginAdd.java @@ -16,7 +16,6 @@ */ package org.apache.camel.dsl.jbang.core.commands.plugin; -import java.util.Arrays; import java.util.Optional; import org.aesh.command.CommandDefinition; @@ -40,7 +39,6 @@ import org.apache.camel.dsl.jbang.core.common.PluginVersionResolver.Unavailable; import org.apache.camel.main.download.DependencyDownloader; import org.apache.camel.main.download.MavenDependencyDownloader; import org.apache.camel.tooling.maven.MavenGav; -import org.apache.camel.util.json.JsonArray; import org.apache.camel.util.json.JsonObject; @CommandDefinition(name = "add", description = "Add new plugin", generateHelp = true, @@ -144,9 +142,7 @@ public class PluginAdd extends PluginBaseCommand { plugin.put("description", description != null ? description : "Plugin %s called with command %s".formatted(name, command)); if (appliesTo != null && appliesTo.length > 0) { - JsonArray appliesToArray = new JsonArray(); - appliesToArray.addAll(Arrays.asList(appliesTo)); - plugin.put("appliesTo", appliesToArray); + plugin.put("appliesTo", PluginHelper.toJsonArray(appliesTo)); } boolean explicit = gav != null || isVersionGiven(); diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/PluginHelper.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/PluginHelper.java index 4282cfabdba1..a6ab03f9dd98 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/PluginHelper.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/PluginHelper.java @@ -365,9 +365,7 @@ public final class PluginHelper { if (properties.get("appliesTo") == null) { Optional<PluginType> knownType = PluginType.findByName(name); if (knownType.isPresent() && knownType.get().getAppliesTo().length > 0) { - JsonArray appliesToArray = new JsonArray(); - appliesToArray.addAll(Arrays.asList(knownType.get().getAppliesTo())); - properties.put("appliesTo", appliesToArray); + properties.put("appliesTo", toJsonArray(knownType.get().getAppliesTo())); configDirty = true; main.getOut().printWarn( ("Plugin '%s' configuration updated with 'appliesTo' filter. " @@ -626,6 +624,12 @@ public final class PluginHelper { return o == null ? null : o.toString(); } + public static JsonArray toJsonArray(String[] values) { + JsonArray array = new JsonArray(); + array.addAll(Arrays.asList(values)); + return array; + } + private record ResolveResult(Optional<Plugin> plugin, boolean cacheWritten) { } @@ -837,9 +841,7 @@ public final class PluginHelper { newPlugin.put("firstVersion", pluginType.getFirstVersion()); String[] appliesTo = pluginType.getAppliesTo(); if (appliesTo.length > 0) { - JsonArray appliesToArray = new JsonArray(); - appliesToArray.addAll(Arrays.asList(appliesTo)); - newPlugin.put("appliesTo", appliesToArray); + newPlugin.put("appliesTo", toJsonArray(appliesTo)); } plugins.putIfAbsent(pluginType.getName(), newPlugin); diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/PluginType.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/PluginType.java index 06ad7c5b13cc..52a3af7264c0 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/PluginType.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/PluginType.java @@ -82,6 +82,6 @@ public enum PluginType { } public String[] getAppliesTo() { - return appliesTo; + return appliesTo.clone(); } } diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/common/PluginFilteringTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/common/PluginFilteringTest.java index 63e7d5412d95..7b6948acfe4d 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/common/PluginFilteringTest.java +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/common/PluginFilteringTest.java @@ -24,6 +24,9 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import org.aesh.command.Command; +import org.aesh.command.CommandResult; +import org.aesh.command.invocation.CommandInvocation; import org.apache.camel.dsl.jbang.core.commands.CamelJBangMain; import org.apache.camel.dsl.jbang.core.commands.CliExportContext; import org.apache.camel.dsl.jbang.run.common.StringPrinter; @@ -34,7 +37,6 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -import picocli.CommandLine; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; @@ -278,12 +280,10 @@ class PluginFilteringTest { ExportContext context = CliExportContext.create(main, printer, true); - // CliExportContext.create() calls getActivePlugins(main, repos) WITHOUT a target filter. - // This means it will attempt to resolve the broken plugin, causing a CliException. - // If the filtering were correct, the broken plugin would be skipped and this would succeed. + // CliExportContext.create() passes "export" as target, so the broken plugin + // (whose command is "broken") is filtered out before resolution. assertThatCode(() -> context.getPlugins(null)) - .as("CliExportContext should not resolve plugins unrelated to run/export, " - + "but it currently calls getActivePlugins without a target filter") + .as("CliExportContext should not resolve plugins unrelated to export") .doesNotThrowAnyException(); } @@ -294,8 +294,8 @@ class PluginFilteringTest { writeTwoPluginConfig("alpha", "beta"); CamelJBangMain main = new CamelJBangMain().withPrinter(new StringPrinter()); - CommandLine commandLine = new CommandLine(main); - commandLine.addSubcommand("run", new CommandLine(new NoOpCommand())); + CommandTree commandLine = new CommandTree(main); + commandLine.addSubcommand("run", new CommandTree(new NoOpCommand())); PluginHelper.addPlugins(commandLine, main, "run", "foo.yaml"); @@ -311,8 +311,8 @@ class PluginFilteringTest { writeTwoPluginConfig("alpha", "beta"); CamelJBangMain main = new CamelJBangMain().withPrinter(new StringPrinter()); - CommandLine commandLine = new CommandLine(main); - commandLine.addSubcommand("export", new CommandLine(new NoOpCommand())); + CommandTree commandLine = new CommandTree(main); + commandLine.addSubcommand("export", new CommandTree(new NoOpCommand())); PluginHelper.addPlugins(commandLine, main, "export"); @@ -333,8 +333,8 @@ class PluginFilteringTest { buildBrokenEntry("broken")); CamelJBangMain main = new CamelJBangMain().withPrinter(new StringPrinter()); - CommandLine commandLine = new CommandLine(main); - commandLine.addSubcommand("run", new CommandLine(new NoOpCommand())); + CommandTree commandLine = new CommandTree(main); + commandLine.addSubcommand("run", new CommandTree(new NoOpCommand())); // addPlugins for "run" should skip the broken plugin since it doesn't match assertThatCode(() -> PluginHelper.addPlugins(commandLine, main, "run", "foo.yaml")) @@ -346,12 +346,12 @@ class PluginFilteringTest { @Test void builtInCommandDoesNotTriggerPluginDiscovery() { CamelJBangMain main = new CamelJBangMain(); - CommandLine cl = new CommandLine(main); - cl.addSubcommand("version", new CommandLine(new NoOpCommand())); - cl.addSubcommand("ps", new CommandLine(new NoOpCommand())); - cl.addSubcommand("get", new CommandLine(new NoOpCommand())); - cl.addSubcommand("config", new CommandLine(new NoOpCommand())); - cl.addSubcommand("catalog", new CommandLine(new NoOpCommand())); + CommandTree cl = new CommandTree(main); + cl.addSubcommand("version", new CommandTree(new NoOpCommand())); + cl.addSubcommand("ps", new CommandTree(new NoOpCommand())); + cl.addSubcommand("get", new CommandTree(new NoOpCommand())); + cl.addSubcommand("config", new CommandTree(new NoOpCommand())); + cl.addSubcommand("catalog", new CommandTree(new NoOpCommand())); assertThat(PluginHelper.shouldDiscoverPlugins(cl, "version")).isFalse(); assertThat(PluginHelper.shouldDiscoverPlugins(cl, "ps")).isFalse(); @@ -426,10 +426,10 @@ class PluginFilteringTest { return array; } - @CommandLine.Command(name = "noop") - private static class NoOpCommand implements Runnable { + private static class NoOpCommand implements Command<CommandInvocation> { @Override - public void run() { + public CommandResult execute(CommandInvocation invocation) { + return CommandResult.SUCCESS; } } }
