From 10a945ae72fadeb6e83946efeab6df7a82068f05 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 22 Nov 2016 23:12:14 -0800 Subject: [PATCH] Plugins: Remove support for onModule (#21416) All plugin extension points have been converted to pull based interfaces. This change removes the infrastructure for the black-magic onModule methods. --- .../client/transport/TransportClient.java | 1 - .../java/org/elasticsearch/node/Node.java | 1 - .../elasticsearch/plugins/PluginsService.java | 78 ------------------- .../plugins/PluginsServiceTests.java | 26 ------- 4 files changed, 106 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java index f680c336e35..5f1386fbd6a 100644 --- a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java +++ b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java @@ -151,7 +151,6 @@ public abstract class TransportClient extends AbstractClient { pluginsService.filterPlugins(ActionPlugin.class)); modules.add(actionModule); - pluginsService.processModules(modules); CircuitBreakerService circuitBreakerService = Node.createCircuitBreakerService(settingsModule.getSettings(), settingsModule.getClusterSettings()); resourcesToClose.add(circuitBreakerService); diff --git a/core/src/main/java/org/elasticsearch/node/Node.java b/core/src/main/java/org/elasticsearch/node/Node.java index 3be9f757125..fa300be47af 100644 --- a/core/src/main/java/org/elasticsearch/node/Node.java +++ b/core/src/main/java/org/elasticsearch/node/Node.java @@ -401,7 +401,6 @@ public class Node implements Closeable { final DiscoveryModule discoveryModule = new DiscoveryModule(this.settings, threadPool, transportService, networkService, clusterService, pluginsService.filterPlugins(DiscoveryPlugin.class)); - pluginsService.processModules(modules); modules.add(b -> { b.bind(IndicesQueriesRegistry.class).toInstance(searchModule.getQueryParserRegistry()); b.bind(SearchRequestParsers.class).toInstance(searchModule.getSearchRequestParsers()); diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java index d14890c7d17..7d988737058 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -76,8 +76,6 @@ public class PluginsService extends AbstractComponent { public static final Setting> MANDATORY_SETTING = Setting.listSetting("plugin.mandatory", Collections.emptyList(), Function.identity(), Property.NodeScope); - private final Map> onModuleReferences; - public List> getPluginSettings() { return plugins.stream().flatMap(p -> p.v2().getSettings().stream()).collect(Collectors.toList()); } @@ -86,16 +84,6 @@ public class PluginsService extends AbstractComponent { return plugins.stream().flatMap(p -> p.v2().getSettingsFilter().stream()).collect(Collectors.toList()); } - static class OnModuleReference { - public final Class moduleClass; - public final Method onModuleMethod; - - OnModuleReference(Class moduleClass, Method onModuleMethod) { - this.moduleClass = moduleClass; - this.onModuleMethod = onModuleMethod; - } - } - /** * Constructs a new PluginService * @param settings The settings of the system @@ -175,40 +163,6 @@ public class PluginsService extends AbstractComponent { // but for now: just be transparent so we can debug any potential issues logPluginInfo(info.getModuleInfos(), "module", logger); logPluginInfo(info.getPluginInfos(), "plugin", logger); - - Map> onModuleReferences = new HashMap<>(); - for (Tuple pluginEntry : this.plugins) { - Plugin plugin = pluginEntry.v2(); - List list = new ArrayList<>(); - for (Method method : plugin.getClass().getMethods()) { - if (!method.getName().equals("onModule")) { - continue; - } - // this is a deprecated final method, so all Plugin subclasses have it - if (method.getParameterTypes().length == 1 && method.getParameterTypes()[0].equals(IndexModule.class)) { - continue; - } - if (method.getParameterTypes().length == 0 || method.getParameterTypes().length > 1) { - logger.warn("Plugin: {} implementing onModule with no parameters or more than one parameter", pluginEntry.v1().getName()); - continue; - } - Class moduleClass = method.getParameterTypes()[0]; - if (!Module.class.isAssignableFrom(moduleClass)) { - if (method.getDeclaringClass() == Plugin.class) { - // These are still part of the Plugin class to point the user to the new implementations - continue; - } - throw new RuntimeException( - "Plugin: [" + pluginEntry.v1().getName() + "] implements onModule taking a parameter that isn't a Module [" - + moduleClass.getSimpleName() + "]"); - } - list.add(new OnModuleReference(moduleClass, method)); - } - if (!list.isEmpty()) { - onModuleReferences.put(plugin, list); - } - } - this.onModuleReferences = Collections.unmodifiableMap(onModuleReferences); } private static void logPluginInfo(final List pluginInfos, final String type, final Logger logger) { @@ -222,38 +176,6 @@ public class PluginsService extends AbstractComponent { } } - private List> plugins() { - return plugins; - } - - public void processModules(Iterable modules) { - for (Module module : modules) { - processModule(module); - } - } - - public void processModule(Module module) { - for (Tuple plugin : plugins()) { - // see if there are onModule references - List references = onModuleReferences.get(plugin.v2()); - if (references != null) { - for (OnModuleReference reference : references) { - if (reference.moduleClass.isAssignableFrom(module.getClass())) { - try { - reference.onModuleMethod.invoke(plugin.v2(), module); - } catch (IllegalAccessException | InvocationTargetException e) { - logger.warn((Supplier) () -> new ParameterizedMessage("plugin {}, failed to invoke custom onModule method", plugin.v1().getName()), e); - throw new ElasticsearchException("failed to invoke onModule", e); - } catch (Exception e) { - logger.warn((Supplier) () -> new ParameterizedMessage("plugin {}, failed to invoke custom onModule method", plugin.v1().getName()), e); - throw e; - } - } - } - } - } - } - public Settings updatedSettings() { Map foundSettings = new HashMap<>(); final Settings.Builder builder = Settings.builder(); diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java index 38b072b6dd7..bb70c58b8c8 100644 --- a/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java +++ b/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java @@ -45,21 +45,8 @@ public class PluginsServiceTests extends ESTestCase { } } - public static class FailOnModule extends Plugin { - public void onModule(BrokenModule brokenModule) { - throw new IllegalStateException("boom"); - } - } - public static class FilterablePlugin extends Plugin implements ScriptPlugin {} - public static class BrokenModule extends AbstractModule { - - @Override - protected void configure() { - } - } - static PluginsService newPluginsService(Settings settings, Class... classpathPlugins) { return new PluginsService(settings, null, new Environment(settings).pluginsFile(), Arrays.asList(classpathPlugins)); } @@ -91,19 +78,6 @@ public class PluginsServiceTests extends ESTestCase { } } - public void testOnModuleExceptionsArePropagated() { - Settings settings = Settings.builder() - .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()).build(); - PluginsService service = newPluginsService(settings, FailOnModule.class); - try { - service.processModule(new BrokenModule()); - fail("boom"); - } catch (ElasticsearchException ex) { - assertEquals("failed to invoke onModule", ex.getMessage()); - assertEquals("boom", ex.getCause().getCause().getMessage()); - } - } - public void testExistingPluginMissingDescriptor() throws Exception { Path pluginsDir = createTempDir(); Files.createDirectory(pluginsDir.resolve("plugin-missing-descriptor"));