Merge pull request #14306 from s1monw/no_lenient_on_module
Don't be lenient in PluginService#processModule(Module)
This commit is contained in:
commit
935a8fc3d4
|
@ -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
|
||||
|
|
|
@ -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));
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -156,7 +156,6 @@ import java.util.Set;
|
|||
*/
|
||||
public class SearchModule extends AbstractModule {
|
||||
|
||||
private final Settings settings;
|
||||
private final Set<Class<? extends Aggregator.Parser>> aggParsers = new HashSet<>();
|
||||
private final Set<Class<? extends PipelineAggregator.Parser>> 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<? extends SearchService> 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<? extends Highlighter> clazz) {
|
||||
highlighters.registerExtension(key, clazz);
|
||||
}
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
|
@ -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<? extends Plugin>... 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());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -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);
|
||||
|
|
Loading…
Reference in New Issue