gnodet-bot commented on code in PR #29663:
URL: https://github.com/apache/camel/pull/29663#discussion_r4549399402
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc:
##########
@@ -1166,6 +1166,25 @@ built for Camel 4.24:
* A test sets the options of a command with
`CommandLineTestHelper.parse(command, args...)` (in the test jar of
`camel-jbang-core`) instead of `CommandLine.populateCommand`.
+=== camel-jbang - plugins declare which commands they contribute to
+
+A CLI plugin can now declare which built-in commands it participates in beyond
its own command, using the
+`appliesTo` attribute on `@CamelJBangPlugin`. For example, a plugin that
provides a `PluginExporter` should declare
+`@CamelJBangPlugin(name = "...", firstVersion = "...", appliesTo =
{"export"})` so it is loaded during `camel export`
+but not during `camel run`.
+
+Existing plugins that do not declare `appliesTo` are loaded for `camel run`
and `camel export` as before (backward
+compatible). A plugin opts in to filtering by listing at least one command in
`appliesTo`; in the plugin configuration
+file, an empty `appliesTo: []` array means the plugin is only loaded for its
own command.
+
+When adding a plugin via `camel plugin add`, the `appliesTo` value from the
list of known Camel plugins is stored in
+the configuration. After a plugin is downloaded for the first time, its
`appliesTo` is learned from its
+`@CamelJBangPlugin` annotation and cached for subsequent invocations.
+
+NOTE: The annotation default (`appliesTo = {}`) is not equivalent to
`appliesTo: []` in the config file.
+An unset annotation value means no filtering (backward compatible — the plugin
is loaded for `run` and `export`),
+whereas an explicit empty list in the config restricts the plugin to its own
command only.
Review Comment:
The NOTE correctly distinguishes the annotation default from an explicit
empty list in the config, but the backward-compatible statement applies only to
**third-party** plugins. For built-in Camel plugins (TUI, MCP, KUBERNETES,
etc.), `effectiveAppliesTo` resolves via the `PluginType` enum to `List.of()` —
not `null` — so they are **never** treated as legacy contributors even when the
annotation has the default `appliesTo = {}`. Consider qualifying the scope:
```suggestion
NOTE: The annotation default (`appliesTo = {}`) is not equivalent to
`appliesTo: []` in the config file.
An unset annotation value means no filtering (backward compatible — a
_third-party_ plugin is loaded for `run` and `export`),
whereas an explicit empty list in the config restricts the plugin to its own
command only.
Built-in Camel plugins are governed by their `PluginType` enum entry, which
takes precedence over the annotation default.
```
##########
dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/common/PluginFilteringTest.java:
##########
@@ -0,0 +1,517 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.dsl.jbang.core.common;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
+import java.util.ArrayList;
+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.catalog.DefaultCamelCatalog;
+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;
+import org.apache.camel.dsl.jbang.run.export.ExportContext;
+import org.apache.camel.util.json.JsonArray;
+import org.apache.camel.util.json.JsonObject;
+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 static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+/**
+ * Tests that plugins not relevant to the current command are skipped entirely
— not downloaded, not customized.
+ * <p>
+ * Each test configures cached fake plugins (so no Maven download is needed).
To detect whether a plugin was loaded or
+ * skipped, some tests use a "broken" plugin entry whose cache points to a
non-existent jar: if the resolution code
+ * attempts to load the broken plugin, it will fail with a {@link
CliException}. If the plugin is properly filtered out,
+ * the resolution succeeds.
+ */
+class PluginFilteringTest {
+
+ @TempDir
+ Path tempDir;
+
+ private Path previousHome;
+ private String camelVersion;
+
+ @BeforeEach
+ void setup() {
+ previousHome = CommandLineHelper.getHomeDir();
+ CommandLineHelper.useHomeDir(tempDir.toString());
+ camelVersion = new DefaultCamelCatalog().getCatalogVersion();
+ }
+
+ @AfterEach
+ void restoreHome() {
+ CommandLineHelper.useHomeDir(previousHome.toString());
+ }
+
+ // ---- Target-based filtering in getActivePlugins ----
+
+ @Test
+ void pluginNotMatchingTargetIsNotLoaded() throws Exception {
+ writeTwoPluginConfig("alpha", "beta");
+
+ CamelJBangMain main = new CamelJBangMain();
+ Map<String, Plugin> plugins = PluginHelper.getActivePlugins(main,
null, "alpha");
+
+ assertThat(plugins).containsKey("alpha")
+ .doesNotContainKey("beta");
+ }
+
+ @Test
+ void nullTargetLoadsAllPlugins() throws Exception {
+ writeTwoPluginConfig("alpha", "beta");
+
+ CamelJBangMain main = new CamelJBangMain();
+ Map<String, Plugin> plugins = PluginHelper.getActivePlugins(main,
null, null);
+
+ assertThat(plugins).containsKey("alpha")
+ .containsKey("beta");
+ }
+
+ @Test
+ void shellTargetLoadsAllPlugins() throws Exception {
+ writeTwoPluginConfig("alpha", "beta");
+
+ CamelJBangMain main = new CamelJBangMain();
+ Map<String, Plugin> plugins = PluginHelper.getActivePlugins(main,
null, "shell");
+
+ assertThat(plugins).containsKey("alpha")
+ .containsKey("beta");
+ }
+
+ // ---- Legacy plugins without appliesTo: backward compat only for
CliExportContext ----
+
+ @Test
+ void legacyPluginIsLoadedForRunTargetWithLegacyContributors() throws
Exception {
+ writeTwoPluginConfig("alpha", "beta");
+
+ CamelJBangMain main = new CamelJBangMain();
+ Map<String, Plugin> plugins = PluginHelper.getActivePlugins(main,
null, "run", true);
+
+ assertThat(plugins).containsKey("alpha")
+ .containsKey("beta");
+ }
+
+ @Test
+ void legacyPluginIsLoadedForExportTargetWithLegacyContributors() throws
Exception {
+ writeTwoPluginConfig("alpha", "beta");
+
+ CamelJBangMain main = new CamelJBangMain();
+ Map<String, Plugin> plugins = PluginHelper.getActivePlugins(main,
null, "export", true);
+
+ assertThat(plugins).containsKey("alpha")
+ .containsKey("beta");
+ }
+
+ @Test
+ void legacyPluginIsSkippedForRunTargetWithoutLegacyContributors() throws
Exception {
+ writeTwoPluginConfig("alpha", "beta");
+
+ CamelJBangMain main = new CamelJBangMain();
+ Map<String, Plugin> plugins = PluginHelper.getActivePlugins(main,
null, "run");
+
+ assertThat(plugins).doesNotContainKey("alpha")
+ .doesNotContainKey("beta");
+ }
+
+ // ---- A broken unrelated plugin should not break the "run" target ----
+
+ @Test
+ void brokenUnrelatedPluginDoesNotBreakTargetedResolution() throws
Exception {
+ Path goodJar = tempDir.resolve("fake-good.jar");
+ FakePluginJar.write(goodJar, "good");
+
+ writeConfig(
+ buildEntry("good", camelVersion, goodJar,
Files.getLastModifiedTime(goodJar).toMillis()),
+ buildBrokenEntry("broken"));
+
+ CamelJBangMain main = new CamelJBangMain();
+
+ // targeting "good" should skip the broken plugin entirely
+ assertThatCode(() -> PluginHelper.getActivePlugins(main, null, "good"))
+ .doesNotThrowAnyException();
+
+ Map<String, Plugin> plugins = PluginHelper.getActivePlugins(main,
null, "good");
+ assertThat(plugins).containsKey("good")
+ .doesNotContainKey("broken");
+ }
+
+ @Test
+ void brokenUnrelatedPluginBreaksNullTarget() throws Exception {
+ Path goodJar = tempDir.resolve("fake-good.jar");
+ FakePluginJar.write(goodJar, "good");
+
+ writeConfig(
+ buildEntry("good", camelVersion, goodJar,
Files.getLastModifiedTime(goodJar).toMillis()),
+ buildBrokenEntry("broken"));
+
+ CamelJBangMain main = new CamelJBangMain();
+
+ // null target loads ALL plugins, so the broken one causes a failure
+ assertThatThrownBy(() -> PluginHelper.getActivePlugins(main, null,
null))
+ .isInstanceOf(CliException.class)
+ .hasMessageContaining("camel-jbang-plugin-broken not found");
+ }
+
+ // ---- appliesTo filtering ----
+
+ @Test
+ void pluginWithAppliesToIsLoadedForMatchingTarget() throws Exception {
+ Path jar = tempDir.resolve("fake-exporter.jar");
+ FakePluginJar.write(jar, "exporter");
+
+ JsonObject entry = buildEntry("exporter", camelVersion, jar,
Files.getLastModifiedTime(jar).toMillis());
+ entry.put("appliesTo", jsonArray("export"));
+ writeConfig(entry);
+
+ CamelJBangMain main = new CamelJBangMain();
+ Map<String, Plugin> plugins = PluginHelper.getActivePlugins(main,
null, "export");
+
+ assertThat(plugins).containsKey("exporter");
+ }
+
+ @Test
+ void pluginWithAppliesToIsNotLoadedForNonMatchingTarget() throws Exception
{
+ Path jar = tempDir.resolve("fake-exporter.jar");
+ FakePluginJar.write(jar, "exporter");
+
+ JsonObject entry = buildEntry("exporter", camelVersion, jar,
Files.getLastModifiedTime(jar).toMillis());
+ entry.put("appliesTo", jsonArray("export"));
+ writeConfig(entry);
+
+ CamelJBangMain main = new CamelJBangMain();
+ Map<String, Plugin> plugins = PluginHelper.getActivePlugins(main,
null, "run");
+
+ assertThat(plugins).doesNotContainKey("exporter");
+ }
+
+ @Test
+ void pluginWithEmptyAppliesToIsNotLoadedForBuiltinTarget() throws
Exception {
+ Path jar = tempDir.resolve("fake-alpha.jar");
+ FakePluginJar.write(jar, "alpha");
+
+ JsonObject entry = buildEntry("alpha", camelVersion, jar,
Files.getLastModifiedTime(jar).toMillis());
+ entry.put("appliesTo", jsonArray());
+ writeConfig(entry);
+
+ CamelJBangMain main = new CamelJBangMain();
+ Map<String, Plugin> plugins = PluginHelper.getActivePlugins(main,
null, "export");
+
+ assertThat(plugins).doesNotContainKey("alpha");
+ }
+
+ @Test
+ void pluginWithAppliesToExportIsLoadedByCliExportContext() throws
Exception {
+ // A broken entry with appliesTo ["export"]: if CliExportContext
includes it, resolution
+ // fails — proving the plugin was included in the export target.
+ JsonObject entry = buildBrokenEntry("exporter");
+ entry.put("appliesTo", jsonArray("export"));
+ writeConfig(entry);
+
+ StringPrinter printer = new StringPrinter();
+ CamelJBangMain main = new CamelJBangMain().withPrinter(printer);
+
+ ExportContext context = CliExportContext.create(main, printer, true);
+
+ assertThatThrownBy(() -> context.getPlugins(null))
+ .isInstanceOf(CliException.class)
+ .hasMessageContaining("camel-jbang-plugin-exporter not found");
+ }
+
+ // ---- Known PluginType appliesTo is used at runtime without config entry
----
+
+ @Test
+ void knownPluginTypeAppliesToIsUsedAtRuntime() throws Exception {
+ Path jar = tempDir.resolve("fake-test.jar");
Review Comment:
This test covers the positive case (PluginType.TEST with `appliesTo =
{"export"}` is loaded for export, skipped for run) but misses the **negative**
case: a known `PluginType` with an **empty** enum `appliesTo` (e.g. TUI, MCP)
must also be excluded when `legacyContributors=true`. That exercises the
`List.of()` branch of `effectiveAppliesTo` — distinct from the
`Arrays.asList(at)` branch tested here. Example:
```java
@Test
void knownPluginWithEmptyAppliesToIsExcludedFromRunWithLegacyContributors()
throws Exception {
// TUI has PluginType.TUI with no appliesTo entries — effectiveAppliesTo
returns List.of()
// Write a config entry for "tui" without an explicit appliesTo key
Path jar = tempDir.resolve("fake-tui.jar");
FakePluginJar.write(jar, "tui");
writeConfig(buildEntry("tui", camelVersion, jar,
Files.getLastModifiedTime(jar).toMillis()));
CamelJBangMain main = new CamelJBangMain();
Map<String, Plugin> plugins = PluginHelper.getActivePlugins(main, null,
"run", true);
// TUI is a known PluginType with empty appliesTo — must be excluded
even with legacyContributors
assertThat(plugins).doesNotContainKey("tui");
}
```
--
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]
