[Refactor] Make PluginBundle package-private (#86863)

Our plugin loading code uses the PluginBundle class to describe a plugin on
disk. This should be an implementation detail of the plugin loading code, and
not part of the API shared with the plugin CLI.

* Refactor to hide internal code from Plugin CLI
* Add javadoc

distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/cli/InstallPluginAction.java

@@ -39,7 +39,6 @@ import org.elasticsearch.core.Tuple;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.jdk.JarHell;
 import org.elasticsearch.plugins.Platforms;
-import org.elasticsearch.plugins.PluginBundle;
 import org.elasticsearch.plugins.PluginInfo;
 import org.elasticsearch.plugins.PluginsUtils;
 
@@ -73,7 +72,6 @@ import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -885,22 +883,7 @@ public class InstallPluginAction implements Closeable {
                 throw new AssertionError(e);
             }
         }).collect(Collectors.toSet());
-
-        // read existing bundles. this does some checks on the installation too.
-        Set<PluginBundle> bundles = new HashSet<>(PluginsUtils.getPluginBundles(pluginsDir));
-        bundles.addAll(PluginsUtils.getModuleBundles(modulesDir));
-        bundles.add(new PluginBundle(candidateInfo, candidateDir));
-        List<PluginBundle> sortedBundles = PluginsUtils.sortBundles(bundles);
-
-        // check jarhell of all plugins so we know this plugin and anything depending on it are ok together
-        // TODO: optimize to skip any bundles not connected to the candidate plugin?
-        Map<String, Set<URL>> transitiveUrls = new HashMap<>();
-        for (PluginBundle bundle : sortedBundles) {
-            PluginsUtils.checkBundleJarHell(classpath, bundle, transitiveUrls);
-        }
-
-        // TODO: no jars should be an error
-        // TODO: verify the classname exists in one of the jars!
+        PluginsUtils.preInstallJarHellCheck(candidateInfo, candidateDir, pluginsDir, modulesDir, classpath);
     }
 
     /**

distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/cli/RemovePluginAction.java

@@ -13,7 +13,6 @@ import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.cli.UserException;
 import org.elasticsearch.core.IOUtils;
 import org.elasticsearch.env.Environment;
-import org.elasticsearch.plugins.PluginBundle;
 import org.elasticsearch.plugins.PluginsUtils;
 
 import java.io.IOException;
@@ -25,7 +24,6 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.Set;
 import java.util.StringJoiner;
 import java.util.stream.Stream;
 
@@ -90,13 +88,17 @@ public class RemovePluginAction {
     private void ensurePluginsNotUsedByOtherPlugins(List<PluginDescriptor> plugins) throws IOException, UserException {
         // First make sure nothing extends this plugin
         final Map<String, List<String>> usedBy = new HashMap<>();
-        Set<PluginBundle> bundles = PluginsUtils.getPluginBundles(env.pluginsFile());
-        for (PluginBundle bundle : bundles) {
-            for (String extendedPlugin : bundle.plugin.getExtendedPlugins()) {
+
+        // We build a new map where the keys are plugins that extend plugins
+        // we want to remove and the values are the plugins we can't remove
+        // because of this dependency
+        Map<String, List<String>> pluginDependencyMap = PluginsUtils.getDependencyMapView(env.pluginsFile());
+        for (Map.Entry<String, List<String>> entry : pluginDependencyMap.entrySet()) {
+            for (String extendedPlugin : entry.getValue()) {
                 for (PluginDescriptor plugin : plugins) {
                     String pluginId = plugin.getId();
                     if (extendedPlugin.equals(pluginId)) {
-                        usedBy.computeIfAbsent(bundle.plugin.getName(), (_key -> new ArrayList<>())).add(pluginId);
+                        usedBy.computeIfAbsent(entry.getKey(), (_key -> new ArrayList<>())).add(pluginId);
                     }
                 }
             }

server/src/main/java/org/elasticsearch/plugins/PluginBundle.java

@@ -20,13 +20,13 @@ import java.util.Set;
 /**
  * A "bundle" is a group of jars that will be loaded in their own classloader
  */
-public class PluginBundle {
+class PluginBundle {
     public final PluginInfo plugin;
     public final Set<URL> urls;
     public final Set<URL> spiUrls;
     public final Set<URL> allUrls;
 
-    public PluginBundle(PluginInfo plugin, Path dir) throws IOException {
+    PluginBundle(PluginInfo plugin, Path dir) throws IOException {
         this.plugin = Objects.requireNonNull(plugin);
 
         Path spiDir = dir.resolve("spi");

server/src/main/java/org/elasticsearch/plugins/PluginsUtils.java

@@ -21,6 +21,7 @@ import java.nio.file.DirectoryStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
@@ -90,11 +91,13 @@ public class PluginsUtils {
         JarHell.checkJavaVersion(info.getName(), info.getJavaVersion());
     }
 
+    /**
+     * Check for the existence of a marker file that indicates any plugins are in a garbage state from a failed attempt to remove the
+     * plugin.
+     * @param pluginsDirectory Path to plugins directory
+     * @throws IOException if there is an error reading from the filesystem
+     */
     public static void checkForFailedPluginRemovals(final Path pluginsDirectory) throws IOException {
-        /*
-         * Check for the existence of a marker file that indicates any plugins are in a garbage state from a failed attempt to remove the
-         * plugin.
-         */
         try (DirectoryStream<Path> stream = Files.newDirectoryStream(pluginsDirectory, ".removing-*")) {
             final Iterator<Path> iterator = stream.iterator();
             if (iterator.hasNext()) {
@@ -113,15 +116,26 @@ public class PluginsUtils {
     }
 
     /** Get bundles for plugins installed in the given modules directory. */
-    public static Set<PluginBundle> getModuleBundles(Path modulesDirectory) throws IOException {
+    static Set<PluginBundle> getModuleBundles(Path modulesDirectory) throws IOException {
         return findBundles(modulesDirectory, "module");
     }
 
     /** Get bundles for plugins installed in the given plugins directory. */
-    public static Set<PluginBundle> getPluginBundles(final Path pluginsDirectory) throws IOException {
+    static Set<PluginBundle> getPluginBundles(final Path pluginsDirectory) throws IOException {
         return findBundles(pluginsDirectory, "plugin");
     }
 
+    /**
+     * A convenience method for analyzing plugin dependencies
+     * @param pluginsDirectory Directory of plugins to scan
+     * @return a map of plugin names to a list of names of any plugins that they extend
+     * @throws IOException if there is an error reading the plugins
+     */
+    public static Map<String, List<String>> getDependencyMapView(final Path pluginsDirectory) throws IOException {
+        return getPluginBundles(pluginsDirectory).stream()
+            .collect(Collectors.toMap(b -> b.plugin.getName(), b -> b.plugin.getExtendedPlugins()));
+    }
+
     // searches subdirectories under the given directory for plugin directories
     private static Set<PluginBundle> findBundles(final Path directory, String type) throws IOException {
         final Set<PluginBundle> bundles = new HashSet<>();
@@ -154,9 +168,47 @@ public class PluginsUtils {
         return new PluginBundle(info, plugin);
     }
 
+    /**
+     * Given a plugin that we would like to install, perform a series of "jar hell
+     * checks to make sure that we don't have any classname conflicts. Some of these
+     * checks are unique to the "pre-installation" scenario, but we also call the
+     * {@link #checkBundleJarHell(Set, PluginBundle, Map)}.
+     * @param candidateInfo Candidate for installation
+     * @param candidateDir Directory containing the candidate plugin files
+     * @param pluginsDir Directory containing already-installed plugins
+     * @param modulesDir Directory containing Elasticsearch modules
+     * @param classpath Set of URLs to use for a classpath
+     * @throws IOException on failed plugin reads
+     */
+    public static void preInstallJarHellCheck(
+        PluginInfo candidateInfo,
+        Path candidateDir,
+        Path pluginsDir,
+        Path modulesDir,
+        Set<URL> classpath
+    ) throws IOException {
+        // create list of current jars in classpath
+
+        // read existing bundles. this does some checks on the installation too.
+        Set<PluginBundle> bundles = new HashSet<>(getPluginBundles(pluginsDir));
+        bundles.addAll(getModuleBundles(modulesDir));
+        bundles.add(new PluginBundle(candidateInfo, candidateDir));
+        List<PluginBundle> sortedBundles = sortBundles(bundles);
+
+        // check jarhell of all plugins so we know this plugin and anything depending on it are ok together
+        // TODO: optimize to skip any bundles not connected to the candidate plugin?
+        Map<String, Set<URL>> transitiveUrls = new HashMap<>();
+        for (PluginBundle bundle : sortedBundles) {
+            checkBundleJarHell(classpath, bundle, transitiveUrls);
+        }
+
+        // TODO: no jars should be an error
+        // TODO: verify the classname exists in one of the jars!
+    }
+
     // jar-hell check the bundle against the parent classloader and extended plugins
     // the plugin cli does it, but we do it again, in case users mess with jar files manually
-    public static void checkBundleJarHell(Set<URL> systemLoaderURLs, PluginBundle bundle, Map<String, Set<URL>> transitiveUrls) {
+    static void checkBundleJarHell(Set<URL> systemLoaderURLs, PluginBundle bundle, Map<String, Set<URL>> transitiveUrls) {
         // invariant: any plugins this plugin bundle extends have already been added to transitiveUrls
         List<String> exts = bundle.plugin.getExtendedPlugins();
 
@@ -225,7 +277,7 @@ public class PluginsUtils {
      *
      * @throws IllegalStateException if a dependency cycle is found
      */
-    public static List<PluginBundle> sortBundles(Set<PluginBundle> bundles) {
+    static List<PluginBundle> sortBundles(Set<PluginBundle> bundles) {
         Map<String, PluginBundle> namedBundles = bundles.stream().collect(Collectors.toMap(b -> b.plugin.getName(), Function.identity()));
         LinkedHashSet<PluginBundle> sortedBundles = new LinkedHashSet<>();
         LinkedHashSet<String> dependencyStack = new LinkedHashSet<>();
@@ -271,5 +323,4 @@ public class PluginsUtils {
 
         sortedBundles.add(bundle);
     }
-
 }