Merge pull request #13034 from rjernst/module_culling4

Remove SpawnModules

core/src/main/java/org/elasticsearch/client/transport/TransportClient.java

@@ -35,6 +35,7 @@ import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.component.LifecycleComponent;
 import org.elasticsearch.common.inject.Injector;
+import org.elasticsearch.common.inject.Module;
 import org.elasticsearch.common.inject.ModulesBuilder;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.settings.Settings;
@@ -132,7 +133,7 @@ public class TransportClient extends AbstractClient {
             try {
                 ModulesBuilder modules = new ModulesBuilder();
                 modules.add(new Version.Module(version));
-                modules.add(new PluginsModule(this.settings, pluginsService));
+                modules.add(new PluginsModule(pluginsService));
                 modules.add(new EnvironmentModule(environment));
                 modules.add(new SettingsModule(this.settings));
                 modules.add(new NetworkModule());
@@ -149,6 +150,11 @@ public class TransportClient extends AbstractClient {
                 modules.add(new ClientTransportModule());
                 modules.add(new CircuitBreakerModule(this.settings));
 
+                for (Module pluginModule : pluginsService.nodeModules()) {
+                    modules.add(pluginModule);
+                }
+                pluginsService.processModules(modules);
+
                 Injector injector = modules.createInjector();
                 injector.getInstance(TransportService.class).start();
                 TransportClient transportClient = new TransportClient(injector);

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

@@ -1,65 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.common.inject;
-
-import org.elasticsearch.ElasticsearchException;
-import org.elasticsearch.common.Nullable;
-import org.elasticsearch.common.settings.Settings;
-
-import java.lang.reflect.Constructor;
-
-/**
- *
- */
-public class Modules {
-
-    public static Module createModule(Class<? extends Module> moduleClass, @Nullable Settings settings) {
-        Constructor<? extends Module> constructor;
-        try {
-            constructor = moduleClass.getConstructor(Settings.class);
-            try {
-                return constructor.newInstance(settings);
-            } catch (Exception e) {
-                throw new ElasticsearchException("Failed to create module [" + moduleClass + "]", e);
-            }
-        } catch (NoSuchMethodException e) {
-            try {
-                constructor = moduleClass.getConstructor();
-                try {
-                    return constructor.newInstance();
-                } catch (Exception e1) {
-                    throw new ElasticsearchException("Failed to create module [" + moduleClass + "]", e);
-                }
-            } catch (NoSuchMethodException e1) {
-                throw new ElasticsearchException("No constructor for [" + moduleClass + "]");
-            }
-        }
-    }
-
-    public static void processModules(Iterable<Module> modules) {
-        for (Module module : modules) {
-            if (module instanceof PreProcessModule) {
-                for (Module module1 : modules) {
-                    ((PreProcessModule) module).processModule(module1);
-                }
-            }
-        }
-    }
-}

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

@@ -31,20 +31,9 @@ public class ModulesBuilder implements Iterable<Module> {
 
     private final List<Module> modules = Lists.newArrayList();
 
-    public ModulesBuilder add(Module... modules) {
+    public ModulesBuilder add(Module... newModules) {
-        for (Module module : modules) {
+        for (Module module : newModules) {
-            add(module);
-        }
-        return this;
-    }
-
-    public ModulesBuilder add(Module module) {
             modules.add(module);
-        if (module instanceof SpawnModules) {
-            Iterable<? extends Module> spawned = ((SpawnModules) module).spawnModules();
-            for (Module spawn : spawned) {
-                add(spawn);
-            }
         }
         return this;
     }
@@ -55,7 +44,6 @@ public class ModulesBuilder implements Iterable<Module> {
     }
 
     public Injector createInjector() {
-        Modules.processModules(modules);
         Injector injector = Guice.createInjector(modules);
         Injectors.cleanCaches(injector);
         // in ES, we always create all instances as if they are eager singletons
@@ -65,7 +53,6 @@ public class ModulesBuilder implements Iterable<Module> {
     }
 
     public Injector createChildInjector(Injector injector) {
-        Modules.processModules(modules);
         Injector childInjector = injector.createChildInjector(modules);
         Injectors.cleanCaches(childInjector);
         // in ES, we always create all instances as if they are eager singletons

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

@@ -1,36 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-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 {
-
-    Iterable<? extends Module> spawnModules();
-}

core/src/main/java/org/elasticsearch/index/IndexService.java

@@ -56,7 +56,6 @@ import org.elasticsearch.indices.IndicesService;
 import org.elasticsearch.indices.InternalIndicesLifecycle;
 import org.elasticsearch.indices.cache.query.IndicesQueryCache;
 import org.elasticsearch.plugins.PluginsService;
-import org.elasticsearch.plugins.ShardsPluginsModule;
 
 import java.io.Closeable;
 import java.io.IOException;
@@ -317,7 +316,6 @@ public class IndexService extends AbstractIndexComponent implements IndexCompone
             final boolean canDeleteShardContent = IndexMetaData.isOnSharedFilesystem(indexSettings) == false ||
                     (primary && IndexMetaData.isOnSharedFilesystem(indexSettings));
             ModulesBuilder modules = new ModulesBuilder();
-            modules.add(new ShardsPluginsModule(indexSettings, pluginsService));
             modules.add(new IndexShardModule(shardId, primary, indexSettings));
             modules.add(new StoreModule(injector.getInstance(IndexStore.class).shardDirectory(), lock,
                     new StoreCloseListener(shardId, canDeleteShardContent,  new Closeable() {
@@ -327,6 +325,12 @@ public class IndexService extends AbstractIndexComponent implements IndexCompone
                         }
                     }), path));
             modules.add(new DeletionPolicyModule());
+
+            for (Module pluginModule : pluginsService.shardModules(indexSettings)) {
+                modules.add(pluginModule);
+            }
+            pluginsService.processModules(modules);
+
             try {
                 shardInjector = modules.createChildInjector(injector);
             } catch (CreationException e) {

core/src/main/java/org/elasticsearch/indices/IndicesService.java

@@ -20,7 +20,12 @@
 package org.elasticsearch.indices;
 
 import com.google.common.base.Function;
-import com.google.common.collect.*;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Iterators;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import org.apache.lucene.store.LockObtainFailedException;
 import org.apache.lucene.util.CollectionUtil;
 import org.apache.lucene.util.IOUtils;
@@ -35,7 +40,12 @@ import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.component.AbstractLifecycleComponent;
-import org.elasticsearch.common.inject.*;
+import org.elasticsearch.common.inject.CreationException;
+import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.inject.Injector;
+import org.elasticsearch.common.inject.Injectors;
+import org.elasticsearch.common.inject.Module;
+import org.elasticsearch.common.inject.ModulesBuilder;
 import org.elasticsearch.common.io.FileSystemUtils;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
@@ -43,7 +53,12 @@ import org.elasticsearch.common.util.concurrent.EsExecutors;
 import org.elasticsearch.env.NodeEnvironment;
 import org.elasticsearch.env.ShardLock;
 import org.elasticsearch.gateway.MetaDataStateFormat;
-import org.elasticsearch.index.*;
+import org.elasticsearch.index.Index;
+import org.elasticsearch.index.IndexModule;
+import org.elasticsearch.index.IndexNameModule;
+import org.elasticsearch.index.IndexNotFoundException;
+import org.elasticsearch.index.IndexService;
+import org.elasticsearch.index.LocalNodeIdModule;
 import org.elasticsearch.index.aliases.IndexAliasesServiceModule;
 import org.elasticsearch.index.analysis.AnalysisModule;
 import org.elasticsearch.index.analysis.AnalysisService;
@@ -71,13 +86,16 @@ import org.elasticsearch.index.store.IndexStore;
 import org.elasticsearch.index.store.IndexStoreModule;
 import org.elasticsearch.indices.analysis.IndicesAnalysisService;
 import org.elasticsearch.indices.recovery.RecoverySettings;
-import org.elasticsearch.plugins.IndexPluginsModule;
 import org.elasticsearch.plugins.PluginsService;
 
 import java.io.Closeable;
 import java.io.IOException;
 import java.nio.file.Files;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -306,7 +324,6 @@ public class IndicesService extends AbstractLifecycleComponent<IndicesService> i
         modules.add(new IndexNameModule(index));
         modules.add(new LocalNodeIdModule(localNodeId));
         modules.add(new IndexSettingsModule(index, indexSettings));
-        modules.add(new IndexPluginsModule(indexSettings, pluginsService));
         modules.add(new IndexStoreModule(indexSettings));
         modules.add(new AnalysisModule(indexSettings, indicesAnalysisService));
         modules.add(new SimilarityModule(indexSettings));
@@ -316,6 +333,11 @@ public class IndicesService extends AbstractLifecycleComponent<IndicesService> i
         modules.add(new IndexAliasesServiceModule());
         modules.add(new IndexModule(indexSettings));
 
+        for (Module pluginModule : pluginsService.indexModules(indexSettings)) {
+            modules.add(pluginModule);
+        }
+        pluginsService.processModules(modules);
+
         Injector indexInjector;
         try {
             indexInjector = modules.createChildInjector(injector);

core/src/main/java/org/elasticsearch/node/Node.java

@@ -35,6 +35,7 @@ import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.component.Lifecycle;
 import org.elasticsearch.common.component.LifecycleComponent;
 import org.elasticsearch.common.inject.Injector;
+import org.elasticsearch.common.inject.Module;
 import org.elasticsearch.common.inject.ModulesBuilder;
 import org.elasticsearch.common.lease.Releasable;
 import org.elasticsearch.common.lease.Releasables;
@@ -159,7 +160,7 @@ public class Node implements Releasable {
             ModulesBuilder modules = new ModulesBuilder();
             modules.add(new Version.Module(version));
             modules.add(new CircuitBreakerModule(settings));
-            modules.add(new PluginsModule(settings, pluginsService));
+            modules.add(new PluginsModule(pluginsService));
             modules.add(new SettingsModule(settings));
             modules.add(new NodeModule(this));
             modules.add(new NetworkModule());
@@ -187,6 +188,11 @@ public class Node implements Releasable {
             modules.add(new RepositoriesModule());
             modules.add(new TribeModule());
 
+            for (Module pluginModule : pluginsService.nodeModules()) {
+                modules.add(pluginModule);
+            }
+            pluginsService.processModules(modules);
+
             injector = modules.createInjector();
 
             client = injector.getInstance(Client.class);

core/src/main/java/org/elasticsearch/plugins/IndexPluginsModule.java

@@ -1,55 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.plugins;
-
-import org.elasticsearch.common.inject.AbstractModule;
-import org.elasticsearch.common.inject.Module;
-import org.elasticsearch.common.inject.PreProcessModule;
-import org.elasticsearch.common.inject.SpawnModules;
-import org.elasticsearch.common.settings.Settings;
-
-/**
- *
- */
-public class IndexPluginsModule extends AbstractModule implements SpawnModules, PreProcessModule {
-
-    private final Settings settings;
-
-    private final PluginsService pluginsService;
-
-    public IndexPluginsModule(Settings settings, PluginsService pluginsService) {
-        this.settings = settings;
-        this.pluginsService = pluginsService;
-    }
-
-    @Override
-    public Iterable<? extends Module> spawnModules() {
-        return pluginsService.indexModules(settings);
-    }
-
-    @Override
-    public void processModule(Module module) {
-        pluginsService.processModule(module);
-    }
-
-    @Override
-    protected void configure() {
-    }
-}

core/src/main/java/org/elasticsearch/plugins/PluginsModule.java

@@ -20,35 +20,15 @@
 package org.elasticsearch.plugins;
 
 import org.elasticsearch.common.inject.AbstractModule;
-import org.elasticsearch.common.inject.Module;
-import org.elasticsearch.common.inject.PreProcessModule;
-import org.elasticsearch.common.inject.SpawnModules;
-import org.elasticsearch.common.settings.Settings;
 
-/**
+public class PluginsModule extends AbstractModule {
- *
- */
-public class PluginsModule extends AbstractModule implements SpawnModules, PreProcessModule {
-
-    private final Settings settings;
 
     private final PluginsService pluginsService;
 
-    public PluginsModule(Settings settings, PluginsService pluginsService) {
+    public PluginsModule(PluginsService pluginsService) {
-        this.settings = settings;
         this.pluginsService = pluginsService;
     }
 
-    @Override
-    public Iterable<? extends Module> spawnModules() {
-        return pluginsService.nodeModules();
-    }
-
-    @Override
-    public void processModule(Module module) {
-        pluginsService.processModule(module);
-    }
-
     @Override
     protected void configure() {
         bind(PluginsService.class).toInstance(pluginsService);

core/src/main/java/org/elasticsearch/plugins/ShardsPluginsModule.java

@@ -1,55 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.plugins;
-
-import org.elasticsearch.common.inject.AbstractModule;
-import org.elasticsearch.common.inject.Module;
-import org.elasticsearch.common.inject.PreProcessModule;
-import org.elasticsearch.common.inject.SpawnModules;
-import org.elasticsearch.common.settings.Settings;
-
-/**
- *
- */
-public class ShardsPluginsModule extends AbstractModule implements SpawnModules, PreProcessModule {
-
-    private final Settings settings;
-
-    private final PluginsService pluginsService;
-
-    public ShardsPluginsModule(Settings settings, PluginsService pluginsService) {
-        this.settings = settings;
-        this.pluginsService = pluginsService;
-    }
-
-    @Override
-    public Iterable<? extends Module> spawnModules() {
-        return pluginsService.shardModules(settings);
-    }
-
-    @Override
-    public void processModule(Module module) {
-        pluginsService.processModule(module);
-    }
-
-    @Override
-    protected void configure() {
-    }
-}

core/src/main/java/org/elasticsearch/repositories/RepositoryModule.java

@@ -20,14 +20,8 @@
 package org.elasticsearch.repositories;
 
 import org.elasticsearch.common.inject.AbstractModule;
-import org.elasticsearch.common.inject.Module;
-import org.elasticsearch.common.inject.Modules;
-import org.elasticsearch.common.inject.SpawnModules;
 import org.elasticsearch.common.settings.Settings;
 
-import java.util.Arrays;
-import java.util.Collections;
-
 /**
  * Binds repository classes for the specific repository type.
  */

core/src/test/java/org/elasticsearch/transport/ContextAndHeaderTransportIT.java

@@ -42,7 +42,6 @@ import org.elasticsearch.client.FilterClient;
 import org.elasticsearch.common.inject.AbstractModule;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.inject.Module;
-import org.elasticsearch.common.inject.PreProcessModule;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.http.HttpServerTransport;
@@ -403,7 +402,7 @@ public class ContextAndHeaderTransportIT extends ESIntegTestCase {
         }
     }
 
-    public static class ActionLoggingModule extends AbstractModule implements PreProcessModule {
+    public static class ActionLoggingModule extends AbstractModule {
 
 
         @Override
@@ -411,11 +410,8 @@ public class ContextAndHeaderTransportIT extends ESIntegTestCase {
             bind(LoggingFilter.class).asEagerSingleton();
         }
 
-        @Override
+        public void onModule(ActionModule module) {
-        public void processModule(Module module) {
+            module.registerFilter(LoggingFilter.class);
-            if (module instanceof ActionModule) {
-                ((ActionModule)module).registerFilter(LoggingFilter.class);
-            }
         }
     }