diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml index 4c62693a34a..9b86a207af5 100644 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ b/buildSrc/src/main/resources/checkstyle_suppressions.xml @@ -13,6 +13,9 @@ + + + diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java index 5219fabf243..2e0ec0f242e 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -42,6 +42,7 @@ import org.elasticsearch.index.IndexModule; import org.elasticsearch.threadpool.ExecutorBuilder; import java.io.IOException; +import java.lang.reflect.Constructor; import java.net.URL; import java.net.URLClassLoader; import java.nio.file.DirectoryStream; @@ -418,25 +419,44 @@ public class PluginsService extends AbstractComponent { } private Plugin loadPlugin(Class pluginClass, Settings settings, Path configPath) { - try { - try { - return pluginClass.getConstructor(Settings.class, Path.class).newInstance(settings, configPath); - } catch (NoSuchMethodException e) { - try { - return pluginClass.getConstructor(Settings.class).newInstance(settings); - } catch (NoSuchMethodException e1) { - try { - return pluginClass.getConstructor().newInstance(); - } catch (NoSuchMethodException e2) { - throw new ElasticsearchException("No constructor for [" + pluginClass + "]. A plugin class must " + - "have either an empty default constructor, a single argument constructor accepting a " + - "Settings instance, or a single argument constructor accepting a pair of Settings, Path instances"); - } - } - } - } catch (Exception e) { - throw new ElasticsearchException("Failed to load plugin class [" + pluginClass.getName() + "]", e); + final Constructor[] constructors = pluginClass.getConstructors(); + if (constructors.length == 0) { + throw new IllegalStateException("no public constructor for [" + pluginClass.getName() + "]"); } + + if (constructors.length > 1) { + throw new IllegalStateException("no unique public constructor for [" + pluginClass.getName() + "]"); + } + + final Constructor constructor = constructors[0]; + if (constructor.getParameterCount() > 2) { + throw new IllegalStateException(signatureMessage(pluginClass)); + } + + final Class[] parameterTypes = constructor.getParameterTypes(); + try { + if (constructor.getParameterCount() == 2 && parameterTypes[0] == Settings.class && parameterTypes[1] == Path.class) { + return (Plugin)constructor.newInstance(settings, configPath); + } else if (constructor.getParameterCount() == 1 && parameterTypes[0] == Settings.class) { + return (Plugin)constructor.newInstance(settings); + } else if (constructor.getParameterCount() == 0) { + return (Plugin)constructor.newInstance(); + } else { + throw new IllegalStateException(signatureMessage(pluginClass)); + } + } catch (final ReflectiveOperationException e) { + throw new IllegalStateException("failed to load plugin class [" + pluginClass.getName() + "]", e); + } + } + + private String signatureMessage(final Class clazz) { + return String.format( + Locale.ROOT, + "no public constructor of correct signature for [%s]; must be [%s], [%s], or [%s]", + clazz.getName(), + "(org.elasticsearch.common.settings.Settings,java.nio.file.Path)", + "(org.elasticsearch.common.settings.Settings)", + "()"); } public List filterPlugins(Class type) { diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java index 7f91d11acdc..c3fd0b19f73 100644 --- a/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java +++ b/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java @@ -30,6 +30,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; +import java.util.Collection; import java.util.List; import java.util.Locale; @@ -151,4 +152,90 @@ public class PluginsServiceTests extends ESTestCase { assertThat(e, hasToString(containsString(expected))); } + public void testLoadPluginWithNoPublicConstructor() { + class NoPublicConstructorPlugin extends Plugin { + + private NoPublicConstructorPlugin() { + + } + + } + + final Path home = createTempDir(); + final Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), home).build(); + final IllegalStateException e = + expectThrows(IllegalStateException.class, () -> newPluginsService(settings, NoPublicConstructorPlugin.class)); + assertThat(e, hasToString(containsString("no public constructor"))); + } + + public void testLoadPluginWithMultiplePublicConstructors() { + class MultiplePublicConstructorsPlugin extends Plugin { + + @SuppressWarnings("unused") + public MultiplePublicConstructorsPlugin() { + + } + + @SuppressWarnings("unused") + public MultiplePublicConstructorsPlugin(final Settings settings) { + + } + + } + + final Path home = createTempDir(); + final Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), home).build(); + final IllegalStateException e = + expectThrows(IllegalStateException.class, () -> newPluginsService(settings, MultiplePublicConstructorsPlugin.class)); + assertThat(e, hasToString(containsString("no unique public constructor"))); + } + + public void testLoadPluginWithNoPublicConstructorOfCorrectSignature() { + class TooManyParametersPlugin extends Plugin { + + @SuppressWarnings("unused") + public TooManyParametersPlugin(Settings settings, Path configPath, Object object) { + + } + + } + + class TwoParametersFirstIncorrectType extends Plugin { + + @SuppressWarnings("unused") + public TwoParametersFirstIncorrectType(Object object, Path configPath) { + + } + } + + class TwoParametersSecondIncorrectType extends Plugin { + + @SuppressWarnings("unused") + public TwoParametersSecondIncorrectType(Settings settings, Object object) { + + } + + } + + class OneParameterIncorrectType extends Plugin { + + @SuppressWarnings("unused") + public OneParameterIncorrectType(Object object) { + + } + } + + final Collection> classes = Arrays.asList( + TooManyParametersPlugin.class, + TwoParametersFirstIncorrectType.class, + TwoParametersSecondIncorrectType.class, + OneParameterIncorrectType.class); + for (Class pluginClass : classes) { + final Path home = createTempDir(); + final Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), home).build(); + final IllegalStateException e = expectThrows(IllegalStateException.class, () -> newPluginsService(settings, pluginClass)); + assertThat(e, hasToString(containsString("no public constructor of correct signature"))); + } + } + }