Support multiple paths for plugin.dir

Author: gertjanal

core/trino-main/src/main/java/io/trino/server/ServerPluginsProvider.java

@@ -36,13 +36,13 @@
 public class ServerPluginsProvider
         implements PluginsProvider
 {
-    private final File installedPluginsDir;
+    private final List<File> installedPluginsDirs;
     private final Executor executor;
 
     @Inject
     public ServerPluginsProvider(ServerPluginsProviderConfig config, @ForStartup Executor executor)
     {
-        this.installedPluginsDir = config.getInstalledPluginsDir();
+        this.installedPluginsDirs = config.getInstalledPluginsDirs();
         this.executor = requireNonNull(executor, "executor is null");
     }
 
@@ -51,7 +51,8 @@ public void loadPlugins(Loader loader, ClassLoaderFactory createClassLoader)
     {
         executeUntilFailure(
                 executor,
-                listFiles(installedPluginsDir).stream()
+                installedPluginsDirs.stream()
+                        .flatMap(installedPluginsDir -> listFiles(installedPluginsDir).stream())
                         .filter(File::isDirectory)
                         .map(file -> (Callable<?>) () -> {
                             loader.load(file.getAbsolutePath(), () ->

core/trino-main/src/main/java/io/trino/server/ServerPluginsProviderConfig.java

@@ -13,23 +13,28 @@
  */
 package io.trino.server;
 
+import com.google.common.collect.ImmutableList;
 import io.airlift.configuration.Config;
+import io.airlift.configuration.ConfigDescription;
+import io.airlift.configuration.validation.FileExists;
 
 import java.io.File;
+import java.util.List;
 
 public class ServerPluginsProviderConfig
 {
-    private File installedPluginsDir = new File("plugin");
+    private List<File> installedPluginsDirs = ImmutableList.of(new File("plugin"));
 
-    public File getInstalledPluginsDir()
+    public List<@FileExists File> getInstalledPluginsDirs()
     {
-        return installedPluginsDir;
+        return installedPluginsDirs;
     }
 
     @Config("plugin.dir")
-    public ServerPluginsProviderConfig setInstalledPluginsDir(File installedPluginsDir)
+    @ConfigDescription("Comma separated list of root directories where the plugins are located")
+    public ServerPluginsProviderConfig setInstalledPluginsDirs(List<File> installedPluginsDirs)
     {
-        this.installedPluginsDir = installedPluginsDir;
+        this.installedPluginsDirs = ImmutableList.copyOf(installedPluginsDirs);
         return this;
     }
 }

core/trino-main/src/test/java/io/trino/server/TestServerPluginsProviderConfig.java

@@ -17,6 +17,7 @@
 import org.junit.jupiter.api.Test;
 
 import java.io.File;
+import java.util.List;
 import java.util.Map;
 
 import static io.airlift.configuration.testing.ConfigAssertions.assertFullMapping;
@@ -29,7 +30,7 @@ public class TestServerPluginsProviderConfig
     public void testDefaults()
     {
         assertRecordedDefaults(recordDefaults(ServerPluginsProviderConfig.class)
-                .setInstalledPluginsDir(new File("plugin")));
+                .setInstalledPluginsDirs(List.of(new File("plugin"))));
     }
 
     @Test
@@ -38,7 +39,18 @@ public void testExplicitPropertyMappings()
         Map<String, String> properties = ImmutableMap.of("plugin.dir", "plugins-dir");
 
         ServerPluginsProviderConfig expected = new ServerPluginsProviderConfig()
-                .setInstalledPluginsDir(new File("plugins-dir"));
+                .setInstalledPluginsDirs(List.of(new File("plugins-dir")));
+
+        assertFullMapping(properties, expected);
+    }
+
+    @Test
+    public void testExplicitPropertyMappingMultiDir()
+    {
+        Map<String, String> properties = ImmutableMap.of("plugin.dir", "plugins-dir,external-dir");
+
+        ServerPluginsProviderConfig expected = new ServerPluginsProviderConfig()
+                .setInstalledPluginsDirs(List.of(new File("plugins-dir"), new File("external-dir")));
 
         assertFullMapping(properties, expected);
     }

testing/trino-plugin-reader/src/main/java/io/trino/server/PluginLoader.java

@@ -67,10 +67,10 @@ public static void printPluginFeatures(Plugin plugin)
         plugin.getExchangeManagerFactories().forEach(factory -> System.out.println(EXCHANGE_MANAGER + factory.getName()));
     }
 
-    public static List<Plugin> loadPlugins(File path)
+    public static List<Plugin> loadPlugins(List<File> path)
     {
         ServerPluginsProviderConfig config = new ServerPluginsProviderConfig();
-        config.setInstalledPluginsDir(path);
+        config.setInstalledPluginsDirs(path);
         ServerPluginsProvider pluginsProvider = new ServerPluginsProvider(config, directExecutor());
         ImmutableList.Builder<Plugin> plugins = ImmutableList.builder();
         pluginsProvider.loadPlugins((plugin, createClassLoader) -> loadPlugin(createClassLoader, plugins), PluginManager::createClassLoader);

testing/trino-plugin-reader/src/main/java/io/trino/server/PluginReader.java

@@ -45,8 +45,8 @@ public class PluginReader
     @Option(names = {"-i", "--impacted-modules"}, description = "Impacted modules file generated by the gitflow-incremental-builder (GIB) Maven plugin")
     private Optional<File> impactedModulesFile;
 
-    @Option(names = {"-p", "--plugin-dir"}, description = "Trino plugin directory")
-    private File pluginDir = new File("plugin");
+    @Option(names = {"-p", "--plugin-dir"}, description = "Trino plugin directories", arity = "1..*")
+    private List<File> pluginDirs = List.of(new File("plugin"));
 
     @Option(names = {"-r", "--root-pom"}, description = "Trino root module pom.xml")
     private File rootPom = new File("pom.xml");
@@ -82,7 +82,7 @@ public Integer call()
             }
         }
 
-        Map<String, Plugin> plugins = loadPlugins(pluginDir).stream()
+        Map<String, Plugin> plugins = loadPlugins(pluginDirs).stream()
                 .collect(toMap(plugin -> plugin.getClass().getName(), identity()));
         modulesStream.forEach(entry -> {
             if (!plugins.containsKey(entry.getValue())) {

testing/trino-plugin-reader/src/test/java/io/trino/server/TestPluginReader.java

@@ -14,10 +14,12 @@
 package io.trino.server;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
 import picocli.CommandLine;
 
 import java.io.PrintWriter;
 import java.io.StringWriter;
+import java.nio.file.Path;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -39,4 +41,22 @@ void testCall()
         assertThat(exitCode).isEqualTo(0);
         assertThat(writer.toString()).isEqualTo("");
     }
+
+    @Test
+    void testCallMultiplePluginDirs(@TempDir Path tempDir)
+    {
+        PluginReader pluginReader = new PluginReader();
+        StringWriter writer = new StringWriter();
+        CommandLine cmd = new CommandLine(pluginReader)
+                .setOut(new PrintWriter(writer))
+                .setErr(new PrintWriter(writer));
+
+        int exitCode = cmd.execute(
+                "--impacted-modules", "src/test/resources/gib-impacted.log",
+                "--plugin-dir", "src/test/resources/server-plugins",
+                "--plugin-dir", tempDir.toString(),
+                "--root-pom", "src/test/resources/pom.xml");
+        assertThat(exitCode).isEqualTo(0);
+        assertThat(writer.toString()).isEqualTo("");
+    }
 }