This method on settings loaded a class, based on a setting value, using

the default classloader. It had all kinds of leniency in how the
classname was found, and simply cannot work with plugins having isolated
classloaders.

This change removes that method. Some of the uses of it were for custom
extension points, like custom repository or discovery types. A lot were
just there to plugin mock implementations for tests. For the settings
that were legitimate, all now support plugins adding the given setting
via onModule. For those that were specific to tests for mocks, they now
use Classes.loadClass (a helper around Class.forName). This is a
temporary measure until (in a future PR) tests can change the
implementation via package private statics.

I also removed a number of unnecessary intermediate modules, added a
"jvm-example" plugin that can be filled in in the future as a smoke test
for breaking plugins, and gave some documentation to "spawn" modules
interface.

closes elastic#12643
closes elastic#12656

core/src/main/java/org/elasticsearch/cache/recycler/PageCacheRecyclerModule.java

@@ -19,17 +19,13 @@
 
 package org.elasticsearch.cache.recycler;
 
-import com.google.common.collect.ImmutableList;
+import org.elasticsearch.common.Classes;
 import org.elasticsearch.common.inject.AbstractModule;
-import org.elasticsearch.common.inject.Module;
-import org.elasticsearch.common.inject.SpawnModules;
 import org.elasticsearch.common.settings.Settings;
 
-import static org.elasticsearch.common.inject.Modules.createModule;
-
 /**
  */
-public class PageCacheRecyclerModule extends AbstractModule implements SpawnModules {
+public class PageCacheRecyclerModule extends AbstractModule {
 
     public static final String CACHE_IMPL = "cache.recycler.page_cache_impl";
 
@@ -41,10 +37,12 @@ public PageCacheRecyclerModule(Settings settings) {
 
     @Override
     protected void configure() {
-    }
-
-    @Override
-    public Iterable<? extends Module> spawnModules() {
-        return ImmutableList.of(createModule(settings.getAsClass(CACHE_IMPL, DefaultPageCacheRecyclerModule.class), settings));
+        String impl = settings.get(CACHE_IMPL);
+        if (impl == null) {
+            bind(PageCacheRecycler.class).asEagerSingleton();
+        } else {
+            Class<? extends PageCacheRecycler> implClass = Classes.loadClass(getClass().getClassLoader(), impl);
+            bind(PageCacheRecycler.class).to(implClass).asEagerSingleton();
+        }
     }
 }

core/src/main/java/org/elasticsearch/cluster/ClusterModule.java

@@ -31,6 +31,7 @@
 import org.elasticsearch.cluster.routing.allocation.AllocationModule;
 import org.elasticsearch.cluster.service.InternalClusterService;
 import org.elasticsearch.cluster.settings.ClusterDynamicSettingsModule;
+import org.elasticsearch.common.Classes;
 import org.elasticsearch.common.inject.AbstractModule;
 import org.elasticsearch.common.inject.Module;
 import org.elasticsearch.common.inject.SpawnModules;
@@ -88,7 +89,12 @@ protected void configure() {
         bind(NodeMappingRefreshAction.class).asEagerSingleton();
         bind(MappingUpdatedAction.class).asEagerSingleton();
 
-        bind(ClusterInfoService.class).to(settings.getAsClass(CLUSTER_SERVICE_IMPL, InternalClusterInfoService.class)).asEagerSingleton();
+        String impl = settings.get(CLUSTER_SERVICE_IMPL);
+        Class<? extends ClusterInfoService> implClass = InternalClusterInfoService.class;
+        if (impl != null) {
+            implClass = Classes.loadClass(getClass().getClassLoader(), impl);
+        }
+        bind(ClusterInfoService.class).to(implClass).asEagerSingleton();
 
         Multibinder<IndexTemplateFilter> mbinder = Multibinder.newSetBinder(binder(), IndexTemplateFilter.class);
         for (Class<? extends IndexTemplateFilter> indexTemplateFilter : indexTemplateFilters) {

core/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java

@@ -32,6 +32,7 @@
 import org.elasticsearch.cluster.node.DiscoveryNodeFilters;
 import org.elasticsearch.cluster.routing.HashFunction;
 import org.elasticsearch.cluster.routing.Murmur3HashFunction;
+import org.elasticsearch.common.Classes;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.ParseFieldMatcher;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
@@ -245,10 +246,11 @@ private IndexMetaData(String index, long version, State state, Settings settings
         } else {
             this.minimumCompatibleLuceneVersion = null;
         }
-        final Class<? extends HashFunction> hashFunctionClass = settings.getAsClass(SETTING_LEGACY_ROUTING_HASH_FUNCTION, null);
-        if (hashFunctionClass == null) {
+        final String hashFunction = settings.get(SETTING_LEGACY_ROUTING_HASH_FUNCTION);
+        if (hashFunction == null) {
             routingHashFunction = MURMUR3_HASH_FUNCTION;
         } else {
+            final Class<? extends HashFunction> hashFunctionClass = Classes.loadClass(getClass().getClassLoader(), hashFunction);
             try {
                 routingHashFunction = hashFunctionClass.newInstance();
             } catch (InstantiationException | IllegalAccessException e) {

core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java

@@ -25,6 +25,7 @@
 import org.elasticsearch.cluster.routing.HashFunction;
 import org.elasticsearch.cluster.routing.SimpleHashFunction;
 import org.elasticsearch.cluster.routing.UnassignedInfo;
+import org.elasticsearch.common.Classes;
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
@@ -66,14 +67,18 @@ public MetaDataIndexUpgradeService(Settings settings, ScriptService scriptServic
         // the hash function package has changed we replace the two hash functions if their fully qualified name is used.
         if (hasCustomPre20HashFunction) {
             switch (pre20HashFunctionName) {
+                case "Simple":
+                case "simple":
                 case "org.elasticsearch.cluster.routing.operation.hash.simple.SimpleHashFunction":
                     pre20HashFunction = SimpleHashFunction.class;
                     break;
+                case "Djb":
+                case "djb":
                 case "org.elasticsearch.cluster.routing.operation.hash.djb.DjbHashFunction":
                     pre20HashFunction = DjbHashFunction.class;
                     break;
                 default:
-                    pre20HashFunction = settings.getAsClass(DEPRECATED_SETTING_ROUTING_HASH_FUNCTION, DjbHashFunction.class, "org.elasticsearch.cluster.routing.", "HashFunction");
+                    pre20HashFunction = Classes.loadClass(getClass().getClassLoader(), pre20HashFunctionName);
             }
         } else {
             pre20HashFunction = DjbHashFunction.class;

core/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/ShardsAllocatorModule.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.cluster.routing.allocation.allocator;
 
+import org.elasticsearch.common.Classes;
 import org.elasticsearch.common.inject.AbstractModule;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.logging.Loggers;
@@ -64,8 +65,7 @@ private Class<? extends ShardsAllocator> loadShardsAllocator(Settings settings)
             logger.warn("{} allocator has been removed in 2.0 using {} instead", EVEN_SHARD_COUNT_ALLOCATOR_KEY, BALANCED_ALLOCATOR_KEY);
             shardsAllocator = BalancedShardsAllocator.class;
         } else {
-            shardsAllocator = settings.getAsClass(TYPE_KEY, BalancedShardsAllocator.class,
-                    "org.elasticsearch.cluster.routing.allocation.allocator.", "Allocator");
+            throw new IllegalArgumentException("Unknown ShardsAllocator type [" + type + "]");
         }
         return shardsAllocator;
     }

core/src/main/java/org/elasticsearch/common/Classes.java

@@ -19,6 +19,8 @@
 
 package org.elasticsearch.common;
 
+import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.bootstrap.Elasticsearch;
 import org.elasticsearch.common.inject.Module;
 import org.elasticsearch.common.settings.NoClassSettingsException;
 
@@ -81,14 +83,6 @@ public static String getPackageName(Class<?> clazz) {
         return (lastDotIndex != -1 ? className.substring(0, lastDotIndex) : "");
     }
 
-    public static String getPackageNameNoDomain(Class<?> clazz) {
-        String fullPackage = getPackageName(clazz);
-        if (fullPackage.startsWith("org.") || fullPackage.startsWith("com.") || fullPackage.startsWith("net.")) {
-            return fullPackage.substring(4);
-        }
-        return fullPackage;
-    }
-
     public static boolean isInnerClass(Class<?> clazz) {
         return !Modifier.isStatic(clazz.getModifiers())
                 && clazz.getEnclosingClass() != null;
@@ -99,47 +93,13 @@ public static boolean isConcrete(Class<?> clazz) {
         return !clazz.isInterface() && !Modifier.isAbstract(modifiers);
     }
 
-    public static <T> Class<? extends T> loadClass(ClassLoader classLoader, String className, String prefixPackage, String suffixClassName) {
-        return loadClass(classLoader, className, prefixPackage, suffixClassName, null);
-    }
-
-    @SuppressWarnings({"unchecked"})
-    public static <T> Class<? extends T> loadClass(ClassLoader classLoader, String className, String prefixPackage, String suffixClassName, String errorPrefix) {
-        Throwable t = null;
-        String[] classNames = classNames(className, prefixPackage, suffixClassName);
-        for (String fullClassName : classNames) {
-            try {
-                return (Class<? extends T>) classLoader.loadClass(fullClassName);
-            } catch (ClassNotFoundException ex) {
-                t = ex;
-            } catch (NoClassDefFoundError er) {
-                t = er;
-            }
-        }
-        if (errorPrefix == null) {
-            errorPrefix = "failed to load class";
-        }
-        throw new NoClassSettingsException(errorPrefix + " with value [" + className + "]; tried " + Arrays.toString(classNames), t);
-    }
-
-    private static String[] classNames(String className, String prefixPackage, String suffixClassName) {
-        String prefixValue = prefixPackage;
-        int packageSeparator = className.lastIndexOf('.');
-        String classNameValue = className;
-        // If class name contains package use it as package prefix instead of specified default one
-        if (packageSeparator > 0) {
-            prefixValue = className.substring(0, packageSeparator + 1);
-            classNameValue = className.substring(packageSeparator + 1);
+    public static <T> Class<? extends T> loadClass(ClassLoader classLoader, String className) {
+        try {
+            return (Class<? extends T>) classLoader.loadClass(className);
+        } catch (ClassNotFoundException|NoClassDefFoundError e) {
+            throw new ElasticsearchException("failed to load class [" + className + "]", e);
         }
-        return new String[]{
-                className,
-                prefixValue + Strings.capitalize(toCamelCase(classNameValue)) + suffixClassName,
-                prefixValue + toCamelCase(classNameValue) + "." + Strings.capitalize(toCamelCase(classNameValue)) + suffixClassName,
-                prefixValue + toCamelCase(classNameValue).toLowerCase(Locale.ROOT) + "." + Strings.capitalize(toCamelCase(classNameValue)) + suffixClassName,
-        };
     }
 
-    private Classes() {
-
-    }
+    private Classes() {}
 }

core/src/main/java/org/elasticsearch/common/inject/SpawnModules.java

@@ -20,7 +20,15 @@
 package org.elasticsearch.common.inject;
 
 /**
+ * This interface can be added to a Module to spawn sub modules. DO NOT USE.
  *
+ * This is fundamentally broken.
+ * <ul>
+ * <li>If you have a plugin with multiple modules, return all the modules at once.</li>
+ * <li>If you are trying to make the implementation of a module "pluggable", don't do it.
+ * This is not extendable because custom implementations (using onModule) cannot be
+ * registered before spawnModules() is called.</li>
+ * </ul>
  */
 public interface SpawnModules {
 

core/src/main/java/org/elasticsearch/common/settings/Settings.java

@@ -533,78 +533,6 @@ public SizeValue getAsSize(String[] settings, SizeValue defaultValue) throws Set
         return parseSizeValue(get(settings), defaultValue);
     }
 
-    /**
-     * Returns the setting value (as a class) associated with the setting key. If it does not exists,
-     * returns the default class provided.
-     *
-     * @param setting      The setting key
-     * @param defaultClazz The class to return if no value is associated with the setting
-     * @param <T>          The type of the class
-     * @return The class setting value, or the default class provided is no value exists
-     * @throws org.elasticsearch.common.settings.NoClassSettingsException Failure to load a class
-     */
-    @SuppressWarnings({"unchecked"})
-    public <T> Class<? extends T> getAsClass(String setting, Class<? extends T> defaultClazz) throws NoClassSettingsException {
-        String sValue = get(setting);
-        if (sValue == null) {
-            return defaultClazz;
-        }
-        try {
-            return (Class<? extends T>) getClassLoader().loadClass(sValue);
-        } catch (ClassNotFoundException e) {
-            throw new NoClassSettingsException("Failed to load class setting [" + setting + "] with value [" + sValue + "]", e);
-        }
-    }
-
-    /**
-     * Returns the setting value (as a class) associated with the setting key. If the value itself fails to
-     * represent a loadable class, the value will be appended to the <tt>prefixPackage</tt> and suffixed with the
-     * <tt>suffixClassName</tt> and it will try to be loaded with it.
-     *
-     * @param setting         The setting key
-     * @param defaultClazz    The class to return if no value is associated with the setting
-     * @param prefixPackage   The prefix package to prefix the value with if failing to load the class as is
-     * @param suffixClassName The suffix class name to prefix the value with if failing to load the class as is
-     * @param <T>             The type of the class
-     * @return The class represented by the setting value, or the default class provided if no value exists
-     * @throws org.elasticsearch.common.settings.NoClassSettingsException Failure to load the class
-     */
-    @SuppressWarnings({"unchecked"})
-    public <T> Class<? extends T> getAsClass(String setting, Class<? extends T> defaultClazz, String prefixPackage, String suffixClassName) throws NoClassSettingsException {
-        String sValue = get(setting);
-        if (sValue == null) {
-            return defaultClazz;
-        }
-        String fullClassName = sValue;
-        try {
-            return (Class<? extends T>) getClassLoader().loadClass(fullClassName);
-        } catch (ClassNotFoundException e) {
-            String prefixValue = prefixPackage;
-            int packageSeparator = sValue.lastIndexOf('.');
-            if (packageSeparator > 0) {
-                prefixValue = sValue.substring(0, packageSeparator + 1);
-                sValue = sValue.substring(packageSeparator + 1);
-            }
-            fullClassName = prefixValue + Strings.capitalize(toCamelCase(sValue)) + suffixClassName;
-            try {
-                return (Class<? extends T>) getClassLoader().loadClass(fullClassName);
-            } catch (ClassNotFoundException e1) {
-                return loadClass(prefixValue, sValue, suffixClassName, setting);
-            } catch (NoClassDefFoundError e1) {
-                return loadClass(prefixValue, sValue, suffixClassName, setting);
-            }
-        }
-    }
-
-    private <T> Class<? extends T> loadClass(String prefixValue, String sValue, String suffixClassName, String setting) {
-        String fullClassName = prefixValue + toCamelCase(sValue).toLowerCase(Locale.ROOT) + "." + Strings.capitalize(toCamelCase(sValue)) + suffixClassName;
-        try {
-            return (Class<? extends T>) getClassLoader().loadClass(fullClassName);
-        } catch (ClassNotFoundException e2) {
-            throw new NoClassSettingsException("Failed to load class setting [" + setting + "] with value [" + get(setting) + "]", e2);
-        }
-    }
-
     /**
      * The values associated with a setting prefix as an array. The settings array is in the format of:
      * <tt>settingPrefix.[index]</tt>.
@@ -858,6 +786,43 @@ public String remove(String key) {
             return map.remove(key);
         }
 
+        /**
+         * Removes the specified value from the given key.
+         * Returns true if the value was found and removed, false otherwise.
+         */
+        public boolean removeArrayElement(String key, String value) {
+            // TODO: this is too crazy, we should just have a multimap...
+            String oldValue = get(key);
+            if (oldValue != null) {
+                // single valued case
+                boolean match = oldValue.equals(value);
+                if (match) {
+                    remove(key);
+                }
+                return match;
+            }
+
+            // multi valued
+            int i = 0;
+            while (true) {
+                String toCheck = map.get(key + '.' + i++);
+                if (toCheck == null) {
+                    return false;
+                } else if (toCheck.equals(value)) {
+                    break;
+                }
+            }
+            // found the value, shift values after it back one index
+            int j = i + 1;
+            while (true) {
+                String toMove = map.get(key + '.' + j++);
+                if (toMove == null) {
+                    return true;
+                }
+                put(key + '.' + i++, toMove);
+            }
+        }
+
         /**
          * Returns a setting value based on the setting key.
          */
@@ -1028,6 +993,26 @@ public Builder putArray(String setting, String... values) {
             return this;
         }
 
+        /**
+         * Sets the setting as an array of values, but keeps existing elements for the key.
+         */
+        public Builder extendArray(String setting, String... values) {
+            // check for a singular (non array) value
+            String oldSingle = remove(setting);
+            // find the highest array index
+            int counter = 0;
+            while (map.containsKey(setting + '.' + counter)) {
+                ++counter;
+            }
+            if (oldSingle != null) {
+                put(setting + '.' + counter++, oldSingle);
+            }
+            for (String value : values) {
+                put(setting + '.' + counter++, value);
+            }
+            return this;
+        }
+
         /**
          * Sets the setting group.
          */