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.
elastic · Nov 23, 2016 · 10a945a · 10a945a
1 parent 66d135c
commit 10a945a
Show file tree

Hide file tree

Showing 4 changed files with 0 additions and 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
@@ -151,7 +151,6 @@ private static ClientTemplate buildTemplate(Settings providedSettings, Settings
                 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
@@ -401,7 +401,6 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
 
             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
@@ -76,8 +76,6 @@ public class PluginsService extends AbstractComponent {
     public static final Setting<List<String>> MANDATORY_SETTING =
         Setting.listSetting("plugin.mandatory", Collections.emptyList(), Function.identity(), Property.NodeScope);
 
-    private final Map<Plugin, List<OnModuleReference>> onModuleReferences;
-
     public List<Setting<?>> getPluginSettings() {
         return plugins.stream().flatMap(p -> p.v2().getSettings().stream()).collect(Collectors.toList());
     }
@@ -86,16 +84,6 @@ public List<String> getPluginSettingsFilter() {
         return plugins.stream().flatMap(p -> p.v2().getSettingsFilter().stream()).collect(Collectors.toList());
     }
 
-    static class OnModuleReference {
-        public final Class<? extends Module> moduleClass;
-        public final Method onModuleMethod;
-
-        OnModuleReference(Class<? extends Module> 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 PluginsService(Settings settings, Path modulesDirectory, Path pluginsDire
         // but for now: just be transparent so we can debug any potential issues
         logPluginInfo(info.getModuleInfos(), "module", logger);
         logPluginInfo(info.getPluginInfos(), "plugin", logger);
-
-        Map<Plugin, List<OnModuleReference>> onModuleReferences = new HashMap<>();
-        for (Tuple<PluginInfo, Plugin> pluginEntry : this.plugins) {
-            Plugin plugin = pluginEntry.v2();
-            List<OnModuleReference> 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<PluginInfo> pluginInfos, final String type, final Logger logger) {
@@ -222,38 +176,6 @@ private static void logPluginInfo(final List<PluginInfo> pluginInfos, final Stri
         }
     }
 
-    private List<Tuple<PluginInfo, Plugin>> plugins() {
-        return plugins;
-    }
-
-    public void processModules(Iterable<Module> modules) {
-        for (Module module : modules) {
-            processModule(module);
-        }
-    }
-
-    public void processModule(Module module) {
-        for (Tuple<PluginInfo, Plugin> plugin : plugins()) {
-            // see if there are onModule references
-            List<OnModuleReference> 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<String, String> 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
@@ -45,21 +45,8 @@ public Settings additionalSettings() {
         }
     }
 
-    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<? extends Plugin>... classpathPlugins) {
         return new PluginsService(settings, null, new Environment(settings).pluginsFile(), Arrays.asList(classpathPlugins));
     }
@@ -91,19 +78,6 @@ public void testAdditionalSettingsClash() {
         }
     }
 
-    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"));