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 17551012edb..10ef7bcb13c 100644 --- a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java +++ b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java @@ -143,7 +143,7 @@ public class TransportClient extends AbstractClient { modules.add(new ClusterNameModule(this.settings)); modules.add(new ThreadPoolModule(threadPool)); modules.add(new TransportModule(this.settings)); - modules.add(new SearchModule(this.settings) { + modules.add(new SearchModule() { @Override protected void configure() { // noop diff --git a/core/src/main/java/org/elasticsearch/node/Node.java b/core/src/main/java/org/elasticsearch/node/Node.java index 345eb56a3fa..15279fc0743 100644 --- a/core/src/main/java/org/elasticsearch/node/Node.java +++ b/core/src/main/java/org/elasticsearch/node/Node.java @@ -182,7 +182,7 @@ public class Node implements Releasable { modules.add(new HttpServerModule(settings)); } modules.add(new IndicesModule()); - modules.add(new SearchModule(settings)); + modules.add(new SearchModule()); modules.add(new ActionModule(false)); modules.add(new MonitorModule(settings)); modules.add(new GatewayModule(settings)); diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java index 9582d3f1714..63377a92c11 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -40,6 +40,7 @@ import org.elasticsearch.common.settings.Settings; import java.io.Closeable; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.net.URL; import java.net.URLClassLoader; @@ -192,8 +193,12 @@ public class PluginsService extends AbstractComponent { if (reference.moduleClass.isAssignableFrom(module.getClass())) { try { reference.onModuleMethod.invoke(plugin.v2(), module); + } catch (IllegalAccessException | InvocationTargetException e) { + logger.warn("plugin {}, failed to invoke custom onModule method", e, plugin.v2().name()); + throw new ElasticsearchException("failed to invoke onModule", e); } catch (Exception e) { logger.warn("plugin {}, failed to invoke custom onModule method", e, plugin.v2().name()); + throw e; } } } diff --git a/core/src/main/java/org/elasticsearch/search/SearchModule.java b/core/src/main/java/org/elasticsearch/search/SearchModule.java index 054cc5b9500..b84a5804c05 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/core/src/main/java/org/elasticsearch/search/SearchModule.java @@ -156,7 +156,6 @@ import java.util.Set; */ public class SearchModule extends AbstractModule { - private final Settings settings; private final Set> aggParsers = new HashSet<>(); private final Set> pipelineAggParsers = new HashSet<>(); private final Highlighters highlighters = new Highlighters(); @@ -169,19 +168,6 @@ public class SearchModule extends AbstractModule { // pkg private so tests can mock Class searchServiceImpl = SearchService.class; - public SearchModule(Settings settings) { - this.settings = settings; - } - - // TODO document public API - public void registerStream(SignificanceHeuristicStreams.Stream stream) { - SignificanceHeuristicStreams.registerStream(stream); - } - - public void registerStream(MovAvgModelStreams.Stream stream) { - MovAvgModelStreams.registerStream(stream); - } - public void registerHighlighter(String key, Class clazz) { highlighters.registerExtension(key, clazz); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristicStreams.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristicStreams.java index 18a6f6b93dd..c58e923280d 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristicStreams.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristicStreams.java @@ -79,7 +79,7 @@ public class SignificanceHeuristicStreams { * @param name The given name * @return The associated stream */ - public static synchronized Stream stream(String name) { + private static synchronized Stream stream(String name) { return STREAMS.get(name); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/MovAvgModelStreams.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/MovAvgModelStreams.java index faee8a9f75b..d0314f4c4f6 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/MovAvgModelStreams.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/movavg/models/MovAvgModelStreams.java @@ -79,7 +79,7 @@ public class MovAvgModelStreams { * @param name The given name * @return The associated stream */ - public static synchronized Stream stream(String name) { + private static synchronized Stream stream(String name) { return STREAMS.get(name); } diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java index e5150ada63e..fd528f48f33 100644 --- a/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java +++ b/core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java @@ -19,6 +19,8 @@ package org.elasticsearch.plugins; +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.common.inject.AbstractModule; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.index.IndexModule; @@ -56,6 +58,28 @@ public class PluginsServiceTests extends ESTestCase { } } + public static class FailOnModule extends Plugin { + @Override + public String name() { + return "fail-on-module"; + } + @Override + public String description() { + return "fails in onModule"; + } + + public void onModule(BrokenModule brokenModule) { + throw new IllegalStateException("boom"); + } + } + + public static class BrokenModule extends AbstractModule { + + @Override + protected void configure() { + } + } + static PluginsService newPluginsService(Settings settings, Class... classpathPlugins) { return new PluginsService(settings, new Environment(settings).pluginsFile(), Arrays.asList(classpathPlugins)); } @@ -86,4 +110,17 @@ public class PluginsServiceTests extends ESTestCase { assertTrue(msg, msg.contains("plugin [additional-settings2]")); } } + + public void testOnModuleExceptionsArePropagated() { + Settings settings = Settings.builder() + .put("path.home", 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()); + } + } } diff --git a/core/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/core/src/test/java/org/elasticsearch/search/SearchModuleTests.java index efdcf0062c3..376e8578e2e 100644 --- a/core/src/test/java/org/elasticsearch/search/SearchModuleTests.java +++ b/core/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -31,7 +31,7 @@ import org.elasticsearch.search.suggest.phrase.PhraseSuggester; public class SearchModuleTests extends ModuleTestCase { public void testDoubleRegister() { - SearchModule module = new SearchModule(Settings.EMPTY); + SearchModule module = new SearchModule(); try { module.registerHighlighter("fvh", PlainHighlighter.class); } catch (IllegalArgumentException e) { @@ -46,7 +46,7 @@ public class SearchModuleTests extends ModuleTestCase { } public void testRegisterSuggester() { - SearchModule module = new SearchModule(Settings.EMPTY); + SearchModule module = new SearchModule(); module.registerSuggester("custom", CustomSuggester.class); try { module.registerSuggester("custom", CustomSuggester.class); @@ -57,7 +57,7 @@ public class SearchModuleTests extends ModuleTestCase { } public void testRegisterHighlighter() { - SearchModule module = new SearchModule(Settings.EMPTY); + SearchModule module = new SearchModule(); module.registerHighlighter("custom", CustomHighlighter.class); try { module.registerHighlighter("custom", CustomHighlighter.class); diff --git a/plugins/lang-groovy/src/test/java/org/elasticsearch/messy/tests/SignificantTermsSignificanceScoreTests.java b/plugins/lang-groovy/src/test/java/org/elasticsearch/messy/tests/SignificantTermsSignificanceScoreTests.java index 8828d064b68..90c96771dd5 100644 --- a/plugins/lang-groovy/src/test/java/org/elasticsearch/messy/tests/SignificantTermsSignificanceScoreTests.java +++ b/plugins/lang-groovy/src/test/java/org/elasticsearch/messy/tests/SignificantTermsSignificanceScoreTests.java @@ -174,6 +174,10 @@ public class SignificantTermsSignificanceScoreTests extends ESIntegTestCase { public static class CustomSignificanceHeuristicPlugin extends Plugin { + static { + SignificanceHeuristicStreams.registerStream(SimpleHeuristic.STREAM); + } + @Override public String name() { return "test-plugin-significance-heuristic"; @@ -186,7 +190,6 @@ public class SignificantTermsSignificanceScoreTests extends ESIntegTestCase { public void onModule(SearchModule significanceModule) { significanceModule.registerHeuristicParser(SimpleHeuristic.SimpleHeuristicParser.class); - significanceModule.registerStream(SimpleHeuristic.STREAM); } public void onModule(ScriptModule module) { module.registerScript(NativeSignificanceScoreScriptNoParams.NATIVE_SIGNIFICANCE_SCORE_SCRIPT_NO_PARAMS, NativeSignificanceScoreScriptNoParams.Factory.class);