Define PluginInfos from test files for classpath plugins

Signed-off-by: Craig Perkins <[email protected]>

server/src/internalClusterTest/java/org/opensearch/plugins/ClasspathPluginIT.java

@@ -12,6 +12,7 @@
 
 import java.io.IOException;
 import java.util.Collection;
+import java.util.Map;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -46,6 +47,11 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
             .collect(Collectors.toList());
     }
 
+    @Override
+    protected Map<Class<? extends Plugin>, Class<? extends Plugin>> extendedPlugins() {
+        return Map.of(SampleExtendingPlugin.class, SampleExtensiblePlugin.class);
+    }
+
     public void testPluginExtensionWithClasspathPlugins() throws IOException {
         internalCluster().startNode();
     }

server/src/main/java/org/opensearch/node/Node.java

@@ -212,6 +212,7 @@
 import org.opensearch.plugins.NetworkPlugin;
 import org.opensearch.plugins.PersistentTaskPlugin;
 import org.opensearch.plugins.Plugin;
+import org.opensearch.plugins.PluginInfo;
 import org.opensearch.plugins.PluginsService;
 import org.opensearch.plugins.RepositoryPlugin;
 import org.opensearch.plugins.ScriptPlugin;
@@ -460,11 +461,7 @@ public Node(Environment environment) {
      * @param forbidPrivateIndexSettings whether or not private index settings are forbidden when creating an index; this is used in the
      *                                   test framework for tests that rely on being able to set private settings
      */
-    protected Node(
-        final Environment initialEnvironment,
-        Collection<Class<? extends Plugin>> classpathPlugins,
-        boolean forbidPrivateIndexSettings
-    ) {
+    protected Node(final Environment initialEnvironment, Collection<PluginInfo> classpathPlugins, boolean forbidPrivateIndexSettings) {
         final List<Closeable> resourcesToClose = new ArrayList<>(); // register everything we need to release in the case of an error
         boolean success = false;
         try {

server/src/main/java/org/opensearch/plugins/PluginsService.java

@@ -125,12 +125,13 @@ public List<String> getPluginSettingsFilter() {
      * @param pluginsDirectory The directory plugins exist in, or null if plugins should not be loaded from the filesystem
      * @param classpathPlugins Plugins that exist in the classpath which should be loaded
      */
+    @SuppressWarnings("unchecked")
     public PluginsService(
         Settings settings,
         Path configPath,
         Path modulesDirectory,
         Path pluginsDirectory,
-        Collection<Class<? extends Plugin>> classpathPlugins
+        Collection<PluginInfo> classpathPlugins
     ) {
         this.settings = settings;
         this.configPath = configPath;
@@ -140,25 +141,19 @@ public PluginsService(
         // we need to build a List of plugins for checking mandatory plugins
         final List<String> pluginsNames = new ArrayList<>();
         // first we load plugins that are on the classpath. this is for tests
-        for (Class<? extends Plugin> pluginClass : classpathPlugins) {
-            Plugin plugin = loadPlugin(pluginClass, settings, configPath);
-            PluginInfo pluginInfo = new PluginInfo(
-                pluginClass.getName(),
-                "classpath plugin",
-                "NA",
-                Version.CURRENT,
-                "1.8",
-                pluginClass.getName(),
-                null,
-                classpathPlugins.stream().map(Class::getName).filter(cp -> !pluginClass.getName().equals(cp)).collect(Collectors.toList()),
-                false
-            );
-            if (logger.isTraceEnabled()) {
-                logger.trace("plugin loaded from classpath [{}]", pluginInfo);
+        for (PluginInfo pluginInfo : classpathPlugins) {
+            try {
+                Class<? extends Plugin> pluginClazz = (Class<? extends Plugin>) Class.forName(pluginInfo.getClassname());
+                Plugin plugin = loadPlugin(pluginClazz, settings, configPath);
+                if (logger.isTraceEnabled()) {
+                    logger.trace("plugin loaded from classpath [{}]", pluginInfo);
+                }
+                pluginsLoaded.add(new Tuple<>(pluginInfo, plugin));
+                pluginsList.add(pluginInfo);
+                pluginsNames.add(pluginInfo.getName());
+            } catch (ClassNotFoundException e) {
+                logger.error("Failed to load classpath plugin: " + pluginInfo.getClassname());
             }
-            pluginsLoaded.add(new Tuple<>(pluginInfo, plugin));
-            pluginsList.add(pluginInfo);
-            pluginsNames.add(pluginInfo.getName());
         }
         loadExtensions(pluginsLoaded);
 
@@ -576,17 +571,9 @@ private static void loadExtensionsForPlugin(ExtensiblePlugin extensiblePlugin, L
         ExtensiblePlugin.ExtensionLoader extensionLoader = new ExtensiblePlugin.ExtensionLoader() {
             @Override
             public <T> List<T> loadExtensions(Class<T> extensionPointType) {
-                Set<Class<?>> seenClasses = new LinkedHashSet<>();
                 List<T> result = new ArrayList<>();
-
                 for (Plugin extendingPlugin : extendingPlugins) {
-                    List<? extends T> extensions = createExtensions(extensionPointType, extendingPlugin);
-                    for (T extension : extensions) {
-                        // Only add if we haven't seen this class before, needed for classpath extensions for testing
-                        if (seenClasses.add(extension.getClass())) {
-                            result.add(extension);
-                        }
-                    }
+                    result.addAll(createExtensions(extensionPointType, extendingPlugin));
                 }
                 return Collections.unmodifiableList(result);
             }

server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java

@@ -57,6 +57,7 @@
 import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -100,13 +101,23 @@ public Settings additionalSettings() {
     public static class FilterablePlugin extends Plugin implements ScriptPlugin {}
 
     static PluginsService newPluginsService(Settings settings, Class<? extends Plugin>... classpathPlugins) {
-        return new PluginsService(
-            settings,
-            null,
-            null,
-            TestEnvironment.newEnvironment(settings).pluginsDir(),
-            Arrays.asList(classpathPlugins)
-        );
+        Collection<PluginInfo> pluginInfos = new ArrayList<>();
+        for (Class<? extends Plugin> plugin : classpathPlugins) {
+            pluginInfos.add(
+                new PluginInfo(
+                    plugin.getName(),
+                    "classpath plugin",
+                    "NA",
+                    Version.CURRENT,
+                    "1.8",
+                    plugin.getName(),
+                    null,
+                    Collections.emptyList(),
+                    false
+                )
+            );
+        }
+        return new PluginsService(settings, null, null, TestEnvironment.newEnvironment(settings).pluginsDir(), pluginInfos);
     }
 
     public void testAdditionalSettings() {

test/framework/src/main/java/org/opensearch/node/MockNode.java

@@ -32,6 +32,7 @@
 
 package org.opensearch.node;
 
+import org.opensearch.Version;
 import org.opensearch.client.node.NodeClient;
 import org.opensearch.cluster.ClusterInfoService;
 import org.opensearch.cluster.MockInternalClusterInfoService;
@@ -51,6 +52,7 @@
 import org.opensearch.http.HttpServerTransport;
 import org.opensearch.indices.IndicesService;
 import org.opensearch.plugins.Plugin;
+import org.opensearch.plugins.PluginInfo;
 import org.opensearch.script.MockScriptService;
 import org.opensearch.script.ScriptContext;
 import org.opensearch.script.ScriptEngine;
@@ -72,10 +74,12 @@
 import java.nio.file.Path;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.Executor;
 import java.util.function.Function;
+import java.util.stream.Collectors;
 
 /**
  * A node for testing which allows:
@@ -88,10 +92,6 @@ public class MockNode extends Node {
 
     private final Collection<Class<? extends Plugin>> classpathPlugins;
 
-    public MockNode(final Settings settings, final Collection<Class<? extends Plugin>> classpathPlugins) {
-        this(settings, classpathPlugins, true);
-    }
-
     public MockNode(
         final Settings settings,
         final Collection<Class<? extends Plugin>> classpathPlugins,
@@ -100,6 +100,21 @@ public MockNode(
         this(settings, classpathPlugins, null, forbidPrivateIndexSettings);
     }
 
+    public MockNode(
+        final Settings settings,
+        final Collection<Class<? extends Plugin>> classpathPlugins,
+        final Path configPath,
+        final boolean forbidPrivateIndexSettings,
+        final Map<Class<? extends Plugin>, Class<? extends Plugin>> extendedPlugins
+    ) {
+        this(
+            InternalSettingsPreparer.prepareEnvironment(settings, Collections.emptyMap(), configPath, () -> "mock_ node"),
+            classpathPlugins,
+            forbidPrivateIndexSettings,
+            extendedPlugins
+        );
+    }
+
     public MockNode(
         final Settings settings,
         final Collection<Class<? extends Plugin>> classpathPlugins,
@@ -109,19 +124,58 @@ public MockNode(
         this(
             InternalSettingsPreparer.prepareEnvironment(settings, Collections.emptyMap(), configPath, () -> "mock_ node"),
             classpathPlugins,
-            forbidPrivateIndexSettings
+            forbidPrivateIndexSettings,
+            Collections.emptyMap()
         );
     }
 
     private MockNode(
         final Environment environment,
         final Collection<Class<? extends Plugin>> classpathPlugins,
-        final boolean forbidPrivateIndexSettings
+        final boolean forbidPrivateIndexSettings,
+        final Map<Class<? extends Plugin>, Class<? extends Plugin>> extendedPlugins
     ) {
-        super(environment, classpathPlugins, forbidPrivateIndexSettings);
+        super(
+            environment,
+            classpathPlugins.stream()
+                .map(
+                    p -> new PluginInfo(
+                        p.getName(),
+                        "classpath plugin",
+                        "NA",
+                        Version.CURRENT,
+                        "1.8",
+                        p.getName(),
+                        null,
+                        (extendedPlugins != null && extendedPlugins.containsKey(p))
+                            ? List.of(extendedPlugins.get(p).getName())
+                            : Collections.emptyList(),
+                        false
+                    )
+                )
+                .collect(Collectors.toList()),
+            forbidPrivateIndexSettings
+        );
         this.classpathPlugins = classpathPlugins;
     }
 
+    public MockNode(final Settings settings, final Collection<Class<? extends Plugin>> classpathPlugins) {
+        this(settings, classpathPlugins, true);
+    }
+
+    public MockNode(
+        final Settings settings,
+        final Collection<Class<? extends Plugin>> classpathPlugins,
+        final Map<Class<? extends Plugin>, Class<? extends Plugin>> extendedPlugins
+    ) {
+        this(
+            InternalSettingsPreparer.prepareEnvironment(settings, Collections.emptyMap(), null, () -> "mock_ node"),
+            classpathPlugins,
+            true,
+            extendedPlugins
+        );
+    }
+
     /**
      * The classpath plugins this node was constructed with.
      */

test/framework/src/main/java/org/opensearch/test/AbstractBuilderTestCase.java

@@ -81,6 +81,7 @@
 import org.opensearch.node.InternalSettingsPreparer;
 import org.opensearch.plugins.MapperPlugin;
 import org.opensearch.plugins.Plugin;
+import org.opensearch.plugins.PluginInfo;
 import org.opensearch.plugins.PluginsService;
 import org.opensearch.plugins.ScriptPlugin;
 import org.opensearch.plugins.SearchPlugin;
@@ -389,7 +390,23 @@ private static class ServiceHolder implements Closeable {
                 throw new AssertionError("node.name must be set");
             });
             PluginsService pluginsService;
-            pluginsService = new PluginsService(nodeSettings, null, env.modulesDir(), env.pluginsDir(), plugins);
+            Collection<PluginInfo> pluginInfos = new ArrayList<>();
+            for (Class<? extends Plugin> plugin : plugins) {
+                pluginInfos.add(
+                    new PluginInfo(
+                        plugin.getName(),
+                        "classpath plugin",
+                        "NA",
+                        Version.CURRENT,
+                        "1.8",
+                        plugin.getName(),
+                        null,
+                        Collections.emptyList(),
+                        false
+                    )
+                );
+            }
+            pluginsService = new PluginsService(nodeSettings, null, env.modulesDir(), env.pluginsDir(), pluginInfos);
 
             client = (Client) Proxy.newProxyInstance(Client.class.getClassLoader(), new Class[] { Client.class }, clientInvocationHandler);
             ScriptModule scriptModule = createScriptModule(pluginsService.filterPlugins(ScriptPlugin.class));

test/framework/src/main/java/org/opensearch/test/ExternalTestCluster.java

@@ -59,6 +59,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Function;
 import java.util.stream.Collectors;
@@ -98,6 +99,7 @@ public ExternalTestCluster(
         Function<Client, Client> clientWrapper,
         String clusterName,
         Collection<Class<? extends Plugin>> pluginClasses,
+        Map<Class<? extends Plugin>, Class<? extends Plugin>> extendedPlugins,
         TransportAddress... transportAddresses
     ) {
         super(0);
@@ -129,7 +131,7 @@ public ExternalTestCluster(
         pluginClasses = new ArrayList<>(pluginClasses);
         pluginClasses.add(MockHttpTransport.TestPlugin.class);
         Settings clientSettings = clientSettingsBuilder.build();
-        MockNode node = new MockNode(clientSettings, pluginClasses);
+        MockNode node = new MockNode(clientSettings, pluginClasses, extendedPlugins);
         Client client = clientWrapper.apply(node.client());
         try {
             node.start();

test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java

@@ -518,6 +518,10 @@ public Collection<Class<? extends Plugin>> getPlugins() {
         return plugins;
     }
 
+    public Map<Class<? extends Plugin>, Class<? extends Plugin>> getExtendedPlugins() {
+        return nodeConfigurationSource.extendedPlugins();
+    }
+
     private static Settings getRandomNodeSettings(long seed) {
         Random random = new Random(seed);
         Builder builder = Settings.builder();
@@ -803,6 +807,7 @@ private synchronized NodeAndClient buildNode(int nodeId, Settings settings, bool
         assert Thread.holdsLock(this);
         ensureOpen();
         Collection<Class<? extends Plugin>> plugins = getPlugins();
+        Map<Class<? extends Plugin>, Class<? extends Plugin>> extendedPlugins = getExtendedPlugins();
         String name = settings.get("node.name");
 
         final NodeAndClient nodeAndClient = nodes.get(name);
@@ -817,7 +822,13 @@ private synchronized NodeAndClient buildNode(int nodeId, Settings settings, bool
             // we clone this here since in the case of a node restart we might need it again
             secureSettings = ((MockSecureSettings) secureSettings).clone();
         }
-        MockNode node = new MockNode(settings, plugins, nodeConfigurationSource.nodeConfigPath(nodeId), forbidPrivateIndexSettings);
+        MockNode node = new MockNode(
+            settings,
+            plugins,
+            nodeConfigurationSource.nodeConfigPath(nodeId),
+            forbidPrivateIndexSettings,
+            extendedPlugins
+        );
         node.injector().getInstance(TransportService.class).addLifecycleListener(new LifecycleListener() {
             @Override
             public void afterStart() {

test/framework/src/main/java/org/opensearch/test/NodeConfigurationSource.java

@@ -37,6 +37,7 @@
 import java.nio.file.Path;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Map;
 
 public abstract class NodeConfigurationSource {
 
@@ -63,4 +64,9 @@ public Path nodeConfigPath(int nodeOrdinal) {
     public Collection<Class<? extends Plugin>> nodePlugins() {
         return Collections.emptyList();
     }
+
+    /** Returns a map corresponding to plugin dependencies to other classpath plugins */
+    public Map<Class<? extends Plugin>, Class<? extends Plugin>> extendedPlugins() {
+        return Collections.emptyMap();
+    }
 }