diff --git a/core/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationModule.java b/core/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationModule.java index 23c721d187a..62e14ec0471 100644 --- a/core/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationModule.java +++ b/core/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationModule.java @@ -21,6 +21,7 @@ package org.elasticsearch.cluster.routing.allocation; import org.elasticsearch.cluster.routing.allocation.allocator.BalancedShardsAllocator; import org.elasticsearch.cluster.routing.allocation.allocator.ShardsAllocator; +import org.elasticsearch.cluster.routing.allocation.allocator.ShardsAllocators; import org.elasticsearch.cluster.routing.allocation.decider.AllocationDecider; import org.elasticsearch.cluster.routing.allocation.decider.AllocationDeciders; import org.elasticsearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; @@ -42,6 +43,7 @@ import org.elasticsearch.common.inject.multibindings.Multibinder; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.ExtensionPoint; import org.elasticsearch.gateway.GatewayAllocator; import java.util.Arrays; @@ -84,55 +86,43 @@ public class AllocationModule extends AbstractModule { DiskThresholdDecider.class, SnapshotInProgressAllocationDecider.class)); + private final Settings settings; - private final Map> shardsAllocators = new HashMap<>(); - private final Set> allocationDeciders = new HashSet<>(); + private final ExtensionPoint.TypeExtensionPoint shardsAllocators = new ExtensionPoint.TypeExtensionPoint<>("shards_allocator", ShardsAllocator.class); + private final ExtensionPoint.SetExtensionPoint allocationDeciders = new ExtensionPoint.SetExtensionPoint<>("allocation_decider", AllocationDecider.class, AllocationDeciders.class); public AllocationModule(Settings settings) { this.settings = settings; - this.allocationDeciders.addAll(DEFAULT_ALLOCATION_DECIDERS); - registerShardAllocator(BALANCED_ALLOCATOR, BalancedShardsAllocator.class); - registerShardAllocator(EVEN_SHARD_COUNT_ALLOCATOR, BalancedShardsAllocator.class); + for (Class decider : DEFAULT_ALLOCATION_DECIDERS) { + allocationDeciders.registerExtension(decider); + } + shardsAllocators.registerExtension(BALANCED_ALLOCATOR, BalancedShardsAllocator.class); + shardsAllocators.registerExtension(EVEN_SHARD_COUNT_ALLOCATOR, BalancedShardsAllocator.class); } /** Register a custom allocation decider */ public void registerAllocationDecider(Class allocationDecider) { - boolean isNew = allocationDeciders.add(allocationDecider); - if (isNew == false) { - throw new IllegalArgumentException("Cannot register AllocationDecider " + allocationDecider.getName() + " twice"); - } + allocationDeciders.registerExtension(allocationDecider); } /** Register a custom shard allocator with the given name */ public void registerShardAllocator(String name, Class clazz) { - Class existing = shardsAllocators.put(name, clazz); - if (existing != null) { - throw new IllegalArgumentException("Cannot register ShardAllocator [" + name + "] to " + clazz.getName() + ", already registered to " + existing.getName()); - } + shardsAllocators.registerExtension(name, clazz); } @Override protected void configure() { - // bind ShardsAllocator - final String shardsAllocatorType = settings.get(AllocationModule.SHARDS_ALLOCATOR_TYPE_KEY, AllocationModule.BALANCED_ALLOCATOR); - final Class shardsAllocator = shardsAllocators.get(shardsAllocatorType); - if (shardsAllocator == null) { - throw new IllegalArgumentException("Unknown ShardsAllocator type [" + shardsAllocatorType + "]"); - } else if (shardsAllocatorType.equals(EVEN_SHARD_COUNT_ALLOCATOR)) { + String shardsAllocatorType = shardsAllocators.bindType(binder(), settings, AllocationModule.SHARDS_ALLOCATOR_TYPE_KEY, AllocationModule.BALANCED_ALLOCATOR); + if (shardsAllocatorType.equals(EVEN_SHARD_COUNT_ALLOCATOR)) { final ESLogger logger = Loggers.getLogger(getClass(), settings); logger.warn("{} allocator has been removed in 2.0 using {} instead", AllocationModule.EVEN_SHARD_COUNT_ALLOCATOR, AllocationModule.BALANCED_ALLOCATOR); } - bind(ShardsAllocator.class).to(shardsAllocator).asEagerSingleton(); - // bind AllocationDeciders - Multibinder allocationMultibinder = Multibinder.newSetBinder(binder(), AllocationDecider.class); - for (Class allocation : allocationDeciders) { - allocationMultibinder.addBinding().to(allocation).asEagerSingleton(); - } + allocationDeciders.bind(binder()); bind(GatewayAllocator.class).asEagerSingleton(); - bind(AllocationDeciders.class).asEagerSingleton(); bind(AllocationService.class).asEagerSingleton(); } + } diff --git a/core/src/main/java/org/elasticsearch/common/util/ExtensionPoint.java b/core/src/main/java/org/elasticsearch/common/util/ExtensionPoint.java new file mode 100644 index 00000000000..435c3ae4066 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/common/util/ExtensionPoint.java @@ -0,0 +1,194 @@ +/* + * 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.util; + +import org.elasticsearch.common.inject.Binder; +import org.elasticsearch.common.inject.multibindings.MapBinder; +import org.elasticsearch.common.inject.multibindings.Multibinder; +import org.elasticsearch.common.settings.Settings; + +import java.util.*; + +/** + * This class defines an official elasticsearch extension point. It registers + * all extensions by a single name and ensures that extensions are not registered + * more than once. + */ +public abstract class ExtensionPoint { + protected final String name; + protected final Class extensionClass; + protected final Class[] singletons; + + /** + * Creates a new extension point + * + * @param name the human readable underscore case name of the extension poing. This is used in error messages etc. + * @param extensionClass the base class that should be extended + * @param singletons a list of singletons to bind with this extension point - these are bound in {@link #bind(Binder)} + */ + public ExtensionPoint(String name, Class extensionClass, Class... singletons) { + this.name = name; + this.extensionClass = extensionClass; + this.singletons = singletons; + } + + /** + * Binds the extension as well as the singletons to the given guice binder. + * + * @param binder the binder to use + */ + public final void bind(Binder binder) { + if (singletons == null || singletons.length == 0) { + throw new IllegalStateException("Can't bind empty or null singletons"); + } + for (Class c : singletons) { + binder.bind(c).asEagerSingleton(); + } + bindExtensions(binder); + } + + /** + * Subclasses can bind their type, map or set exentions here. + */ + protected abstract void bindExtensions(Binder binder); + + /** + * A map based extension point which allows to register keyed implementations ie. parsers or some kind of strategies. + */ + public static class MapExtensionPoint extends ExtensionPoint { + private final Map> extensions = new HashMap<>(); + private final Set reservedKeys; + + /** + * Creates a new {@link org.elasticsearch.common.util.ExtensionPoint.MapExtensionPoint} + * + * @param name the human readable underscore case name of the extension poing. This is used in error messages etc. + * @param extensionClass the base class that should be extended + * @param singletons a list of singletons to bind with this extension point - these are bound in {@link #bind(Binder)} + * @param reservedKeys a set of reserved keys by internal implementations + */ + public MapExtensionPoint(String name, Class extensionClass, Set reservedKeys, Class... singletons) { + super(name, extensionClass, singletons); + this.reservedKeys = reservedKeys; + + } + + /** + * Returns the extension for the given key or null + */ + public Class getExtension(String type) { + return extensions.get(type); + } + + /** + * Registers an extension class for a given key. This method will thr + * + * @param key the extensions key + * @param extension the extension + * @throws IllegalArgumentException iff the key is already registered or if the key is a reserved key for an internal implementation + */ + public final void registerExtension(String key, Class extension) { + if (extensions.containsKey(key) || reservedKeys.contains(key)) { + throw new IllegalArgumentException("Can't register the same [" + this.name + "] more than once for [" + key + "]"); + } + extensions.put(key, extension); + } + + @Override + protected final void bindExtensions(Binder binder) { + MapBinder parserMapBinder = MapBinder.newMapBinder(binder, String.class, extensionClass); + for (Map.Entry> clazz : extensions.entrySet()) { + parserMapBinder.addBinding(clazz.getKey()).to(clazz.getValue()); + } + } + } + + /** + * A Type extension point which basically allows to registerd keyed extensions like {@link org.elasticsearch.common.util.ExtensionPoint.MapExtensionPoint} + * but doesn't instantiate and bind all the registered key value pairs but instead replace a singleton based on a given setting via {@link #bindType(Binder, Settings, String, String)} + * Note: {@link #bind(Binder)} is not supported by this class + */ + public static final class TypeExtensionPoint extends MapExtensionPoint { + + public TypeExtensionPoint(String name, Class extensionClass) { + super(name, extensionClass, Collections.EMPTY_SET); + } + + /** + * Binds the extension class to the class that is registered for the give configured for the settings key in + * the settings object. + * + * @param binder the binder to use + * @param settings the settings to look up the key to find the implemetation to bind + * @param settingsKey the key to use with the settings + * @param defaultValue the default value if they settings doesn't contain the key + * @return the actual bound type key + */ + public String bindType(Binder binder, Settings settings, String settingsKey, String defaultValue) { + final String type = settings.get(settingsKey, defaultValue); + final Class instance = getExtension(type); + if (instance == null) { + throw new IllegalArgumentException("Unknown [" + this.name + "] type [" + type + "]"); + } + binder.bind(extensionClass).to(instance).asEagerSingleton(); + return type; + } + + } + + /** + * A set based extension point which allows to register extended classes that might be used to chain additional functionality etc. + */ + public final static class SetExtensionPoint extends ExtensionPoint { + private final Set> extensions = new HashSet<>(); + + /** + * Creates a new {@link org.elasticsearch.common.util.ExtensionPoint.SetExtensionPoint} + * + * @param name the human readable underscore case name of the extension poing. This is used in error messages etc. + * @param extensionClass the base class that should be extended + * @param singletons a list of singletons to bind with this extension point - these are bound in {@link #bind(Binder)} + */ + public SetExtensionPoint(String name, Class extensionClass, Class... singletons) { + super(name, extensionClass, singletons); + } + + /** + * Registers a new extension + * + * @param extension the extension to register + * @throws IllegalArgumentException iff the class is already registered + */ + public final void registerExtension(Class extension) { + if (extensions.contains(extension)) { + throw new IllegalArgumentException("Can't register the same [" + this.name + "] more than once for [" + extension.getName() + "]"); + } + extensions.add(extension); + } + + @Override + protected final void bindExtensions(Binder binder) { + Multibinder allocationMultibinder = Multibinder.newSetBinder(binder, extensionClass); + for (Class clazz : extensions) { + allocationMultibinder.addBinding().to(clazz); + } + } + } +} diff --git a/core/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionParserMapper.java b/core/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionParserMapper.java index 4fd5233889a..837837a92b2 100644 --- a/core/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionParserMapper.java +++ b/core/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionParserMapper.java @@ -19,7 +19,6 @@ package org.elasticsearch.index.query.functionscore; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryParsingException; diff --git a/core/src/main/java/org/elasticsearch/search/SearchModule.java b/core/src/main/java/org/elasticsearch/search/SearchModule.java index b4d15b35d16..fcebb12485e 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/core/src/main/java/org/elasticsearch/search/SearchModule.java @@ -19,8 +19,6 @@ package org.elasticsearch.search; -import com.google.common.collect.Lists; - import org.elasticsearch.common.Classes; import org.elasticsearch.common.inject.AbstractModule; import org.elasticsearch.common.inject.multibindings.Multibinder; @@ -150,7 +148,7 @@ import org.elasticsearch.search.suggest.SuggestPhase; import org.elasticsearch.search.suggest.Suggester; import org.elasticsearch.search.suggest.Suggesters; -import java.util.List; +import java.util.*; /** * @@ -160,14 +158,14 @@ public class SearchModule extends AbstractModule { public static final String SEARCH_SERVICE_IMPL = "search.service_impl"; private final Settings settings; - private final List> aggParsers = Lists.newArrayList(); - private final List> pipelineAggParsers = Lists.newArrayList(); - private final List> highlighters = Lists.newArrayList(); - private final List> suggesters = Lists.newArrayList(); - private final List> functionScoreParsers = Lists.newArrayList(); - private final List> fetchSubPhases = Lists.newArrayList(); - private final List> heuristicParsers = Lists.newArrayList(); - private final List> modelParsers = Lists.newArrayList(); + private final Set> aggParsers = new HashSet<>(); + private final Set> pipelineAggParsers = new HashSet<>(); + private final Highlighters highlighters = new Highlighters(); + private final Suggesters suggesters = new Suggesters(); + private final Set> functionScoreParsers = new HashSet<>(); + private final Set> fetchSubPhases = new HashSet<>(); + private final Set> heuristicParsers = new HashSet<>(); + private final Set> modelParsers = new HashSet<>(); public SearchModule(Settings settings) { this.settings = settings; @@ -182,12 +180,12 @@ public class SearchModule extends AbstractModule { MovAvgModelStreams.registerStream(stream); } - public void registerHighlighter(Class clazz) { - highlighters.add(clazz); + public void registerHighlighter(String key, Class clazz) { + highlighters.registerExtension(key, clazz); } - public void registerSuggester(Class suggester) { - suggesters.add(suggester); + public void registerSuggester(String key, Class suggester) { + suggesters.registerExtension(key, suggester); } public void registerFunctionScoreParser(Class parser) { @@ -245,14 +243,7 @@ public class SearchModule extends AbstractModule { } protected void configureSuggesters() { - Multibinder suggesterMultibinder = Multibinder.newSetBinder(binder(), Suggester.class); - for (Class clazz : suggesters) { - suggesterMultibinder.addBinding().to(clazz); - } - - bind(SuggestParseElement.class).asEagerSingleton(); - bind(SuggestPhase.class).asEagerSingleton(); - bind(Suggesters.class).asEagerSingleton(); + suggesters.bind(binder()); } protected void configureFunctionScore() { @@ -264,11 +255,7 @@ public class SearchModule extends AbstractModule { } protected void configureHighlighters() { - Multibinder multibinder = Multibinder.newSetBinder(binder(), Highlighter.class); - for (Class highlighter : highlighters) { - multibinder.addBinding().to(highlighter); - } - bind(Highlighters.class).asEagerSingleton(); + highlighters.bind(binder()); } protected void configureAggs() { @@ -346,7 +333,6 @@ public class SearchModule extends AbstractModule { bind(FetchPhase.class).asEagerSingleton(); bind(SearchServiceTransportAction.class).asEagerSingleton(); bind(MoreLikeThisFetchService.class).asEagerSingleton(); - // search service -- testing only! String impl = settings.get(SEARCH_SERVICE_IMPL); if (impl == null) { @@ -414,4 +400,5 @@ public class SearchModule extends AbstractModule { BucketSelectorPipelineAggregator.registerStreams(); SerialDiffPipelineAggregator.registerStreams(); } + } 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 51dd11c5e87..18a6f6b93dd 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 @@ -64,6 +64,9 @@ public class SignificanceHeuristicStreams { * @param stream The stream to register */ public static synchronized void registerStream(Stream stream) { + if (STREAMS.containsKey(stream.getName())) { + throw new IllegalArgumentException("Can't register stream with name [" + stream.getName() + "] more than once"); + } HashMap map = new HashMap<>(); map.putAll(STREAMS); map.put(stream.getName(), stream); 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 d12b0cdfef4..faee8a9f75b 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 @@ -64,6 +64,9 @@ public class MovAvgModelStreams { * @param stream The stream to register */ public static synchronized void registerStream(Stream stream) { + if (STREAMS.containsKey(stream.getName())) { + throw new IllegalArgumentException("Can't register stream with name [" + stream.getName() + "] more than once"); + } HashMap map = new HashMap<>(); map.putAll(STREAMS); map.put(stream.getName(), stream); diff --git a/core/src/main/java/org/elasticsearch/search/highlight/FastVectorHighlighter.java b/core/src/main/java/org/elasticsearch/search/highlight/FastVectorHighlighter.java index 67b42a5d866..de73a898334 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/FastVectorHighlighter.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/FastVectorHighlighter.java @@ -49,11 +49,6 @@ public class FastVectorHighlighter implements Highlighter { this.termVectorMultiValue = settings.getAsBoolean("search.highlight.term_vector_multi_value", true); } - @Override - public String[] names() { - return new String[]{"fvh", "fast-vector-highlighter"}; - } - @Override public HighlightField highlight(HighlighterContext highlighterContext) { SearchContextHighlight.Field field = highlighterContext.field; diff --git a/core/src/main/java/org/elasticsearch/search/highlight/Highlighter.java b/core/src/main/java/org/elasticsearch/search/highlight/Highlighter.java index 26c3dc0bf21..af4801f3633 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/Highlighter.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/Highlighter.java @@ -25,8 +25,6 @@ import org.elasticsearch.index.mapper.FieldMapper; */ public interface Highlighter { - String[] names(); - HighlightField highlight(HighlighterContext highlighterContext); boolean canHighlight(FieldMapper fieldMapper); diff --git a/core/src/main/java/org/elasticsearch/search/highlight/Highlighters.java b/core/src/main/java/org/elasticsearch/search/highlight/Highlighters.java index 9f14b0f7ed1..349227f5c57 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/Highlighters.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/Highlighters.java @@ -18,44 +18,74 @@ */ package org.elasticsearch.search.highlight; -import com.google.common.collect.ImmutableMap; -import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.ExtensionPoint; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; +import java.util.*; /** - * + * An extensions point and registry for all the highlighters a node supports. */ -public class Highlighters { +public class Highlighters extends ExtensionPoint.MapExtensionPoint { + + @Deprecated // remove in 3.0 + private static final String FAST_VECTOR_HIGHLIGHTER = "fast-vector-highlighter"; + private static final String FVH = "fvh"; + @Deprecated // remove in 3.0 + private static final String HIGHLIGHTER = "highlighter"; + private static final String PLAIN = "plain"; + @Deprecated // remove in 3.0 + private static final String POSTINGS_HIGHLIGHTER = "postings-highlighter"; + private static final String POSTINGS = "postings"; + private final Map parsers; + private final DeprecationLogger deprecationLogger = new DeprecationLogger(ESLoggerFactory.getLogger(Highlighters.class.getName())); + + public Highlighters(){ + this(Collections.EMPTY_MAP); + } + + private Highlighters(Map parsers) { + super("highlighter", Highlighter.class, new HashSet<>(Arrays.asList(FVH, FAST_VECTOR_HIGHLIGHTER, PLAIN, HIGHLIGHTER, POSTINGS, POSTINGS_HIGHLIGHTER)), + Highlighters.class); + this.parsers = Collections.unmodifiableMap(parsers); + } @Inject - public Highlighters(Settings settings, Set parsers) { + public Highlighters(Settings settings, Map parsers) { + this(addBuiltIns(settings, parsers)); + } + + private static Map addBuiltIns(Settings settings, Map parsers) { // build in highlighers Map map = new HashMap<>(); - add(map, new FastVectorHighlighter(settings)); - add(map, new PlainHighlighter()); - add(map, new PostingsHighlighter()); - for (Highlighter highlighter : parsers) { - add(map, highlighter); - } - this.parsers = Collections.unmodifiableMap(map); + map.put(FVH, new FastVectorHighlighter(settings)); + map.put(FAST_VECTOR_HIGHLIGHTER, map.get(FVH)); + map.put(PLAIN, new PlainHighlighter()); + map.put(HIGHLIGHTER, map.get(PLAIN)); + map.put(POSTINGS, new PostingsHighlighter()); + map.put(POSTINGS_HIGHLIGHTER, map.get(POSTINGS)); + map.putAll(parsers); + return map; } public Highlighter get(String type) { + switch (type) { + case FAST_VECTOR_HIGHLIGHTER: + deprecationLogger.deprecated("highlighter key [{}] is deprecated and will be removed in 3.x use [{}] instead", FAST_VECTOR_HIGHLIGHTER, FVH); + break; + case HIGHLIGHTER: + deprecationLogger.deprecated("highlighter key [{}] is deprecated and will be removed in 3.x use [{}] instead", HIGHLIGHTER, PLAIN); + break; + case POSTINGS_HIGHLIGHTER: + deprecationLogger.deprecated("highlighter key [{}] is deprecated and will be removed in 3.x use [{}] instead", POSTINGS_HIGHLIGHTER, POSTINGS); + break; + } return parsers.get(type); } - private void add(Map map, Highlighter highlighter) { - for (String type : highlighter.names()) { - map.put(type, highlighter); - } - } - } diff --git a/core/src/main/java/org/elasticsearch/search/highlight/PlainHighlighter.java b/core/src/main/java/org/elasticsearch/search/highlight/PlainHighlighter.java index a9094f9ceaf..27f439a5c33 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/PlainHighlighter.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/PlainHighlighter.java @@ -47,11 +47,6 @@ public class PlainHighlighter implements Highlighter { private static final String CACHE_KEY = "highlight-plain"; - @Override - public String[] names() { - return new String[] { "plain", "highlighter" }; - } - @Override public HighlightField highlight(HighlighterContext highlighterContext) { SearchContextHighlight.Field field = highlighterContext.field; diff --git a/core/src/main/java/org/elasticsearch/search/highlight/PostingsHighlighter.java b/core/src/main/java/org/elasticsearch/search/highlight/PostingsHighlighter.java index 35f6560899e..270401a9108 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/PostingsHighlighter.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/PostingsHighlighter.java @@ -40,11 +40,6 @@ public class PostingsHighlighter implements Highlighter { private static final String CACHE_KEY = "highlight-postings"; - @Override - public String[] names() { - return new String[]{"postings", "postings-highlighter"}; - } - @Override public HighlightField highlight(HighlighterContext highlighterContext) { diff --git a/core/src/main/java/org/elasticsearch/search/suggest/Suggester.java b/core/src/main/java/org/elasticsearch/search/suggest/Suggester.java index 51f5f21b460..7b3f7bdb89f 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/Suggester.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/Suggester.java @@ -29,8 +29,6 @@ public abstract class Suggester> innerExecute(String name, T suggestion, IndexSearcher searcher, CharsRefBuilder spare) throws IOException; - public abstract String[] names(); - public abstract SuggestContextParser getContextParser(); public Suggest.Suggestion> diff --git a/core/src/main/java/org/elasticsearch/search/suggest/Suggesters.java b/core/src/main/java/org/elasticsearch/search/suggest/Suggesters.java index 264720b8b90..1be80b57502 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/Suggesters.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/Suggesters.java @@ -18,45 +18,46 @@ */ package org.elasticsearch.search.suggest; -import com.google.common.collect.ImmutableMap; -import org.elasticsearch.common.collect.MapBuilder; +import org.elasticsearch.common.inject.Binder; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.util.ExtensionPoint; import org.elasticsearch.script.ScriptService; -import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristicParser; import org.elasticsearch.search.suggest.completion.CompletionSuggester; import org.elasticsearch.search.suggest.phrase.PhraseSuggester; import org.elasticsearch.search.suggest.term.TermSuggester; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; +import java.util.*; /** * */ -public class Suggesters { +public final class Suggesters extends ExtensionPoint.MapExtensionPoint { private final Map parsers; + public Suggesters() { + this(Collections.EMPTY_MAP); + } + + public Suggesters(Map suggesters) { + super("suggester", Suggester.class, new HashSet<>(Arrays.asList("phrase", "term", "completion")), Suggesters.class, SuggestParseElement.class, SuggestPhase.class); + this.parsers = Collections.unmodifiableMap(suggesters); + } + @Inject - public Suggesters(Set suggesters, ScriptService scriptService) { + public Suggesters(Map suggesters, ScriptService scriptService) { + this(addBuildIns(suggesters, scriptService)); + } + + private static Map addBuildIns(Map suggesters, ScriptService scriptService) { final Map map = new HashMap<>(); - add(map, new PhraseSuggester(scriptService)); - add(map, new TermSuggester()); - add(map, new CompletionSuggester()); - for (Suggester suggester : suggesters) { - add(map, suggester); - } - this.parsers = Collections.unmodifiableMap(map); + map.put("phrase", new PhraseSuggester(scriptService)); + map.put("term", new TermSuggester()); + map.put("completion", new CompletionSuggester()); + map.putAll(suggesters); + return map; } public Suggester get(String type) { return parsers.get(type); } - - private void add(Map map, Suggester suggester) { - for (String type : suggester.names()) { - map.put(type, suggester); - } - } } diff --git a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java index 4af360fa05f..4a1d5d1d28c 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java @@ -101,11 +101,6 @@ public class CompletionSuggester extends Suggester return completionSuggestion; } - @Override - public String[] names() { - return new String[] { "completion" }; - } - @Override public SuggestContextParser getContextParser() { return new CompletionSuggestParser(this); diff --git a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java index 30c1b63de21..e7d0eb378c3 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java @@ -150,11 +150,6 @@ public final class PhraseSuggester extends Suggester { return scriptService; } - @Override - public String[] names() { - return new String[] {"phrase"}; - } - @Override public SuggestContextParser getContextParser() { return new PhraseSuggestParser(this); diff --git a/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggester.java b/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggester.java index 70dfefe9522..4c1b176c990 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggester.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggester.java @@ -65,11 +65,6 @@ public final class TermSuggester extends Suggester { return response; } - @Override - public String[] names() { - return new String[] {"term"}; - } - @Override public SuggestContextParser getContextParser() { return new TermSuggestParser(this); diff --git a/core/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationModuleTests.java b/core/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationModuleTests.java index 678bfa3b0e5..7b57ef07190 100644 --- a/core/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationModuleTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationModuleTests.java @@ -59,8 +59,7 @@ public class AllocationModuleTests extends ModuleTestCase { try { module.registerAllocationDecider(EnableAllocationDecider.class); } catch (IllegalArgumentException e) { - assertTrue(e.getMessage().contains("Cannot register AllocationDecider")); - assertTrue(e.getMessage().contains("twice")); + assertEquals(e.getMessage(), "Can't register the same [allocation_decider] more than once for [" + EnableAllocationDecider.class.getName() + "]"); } } @@ -82,14 +81,14 @@ public class AllocationModuleTests extends ModuleTestCase { try { module.registerShardAllocator(AllocationModule.BALANCED_ALLOCATOR, FakeShardsAllocator.class); } catch (IllegalArgumentException e) { - assertTrue(e.getMessage().contains("already registered")); + assertEquals(e.getMessage(), "Can't register the same [shards_allocator] more than once for [balanced]"); } } public void testUnknownShardsAllocator() { Settings settings = Settings.builder().put(AllocationModule.SHARDS_ALLOCATOR_TYPE_KEY, "dne").build(); AllocationModule module = new AllocationModule(settings); - assertBindingFailure(module, "Unknown ShardsAllocator"); + assertBindingFailure(module, "Unknown [shards_allocator]"); } public void testEvenShardsAllocatorBackcompat() { diff --git a/core/src/test/java/org/elasticsearch/common/inject/ModuleTestCase.java b/core/src/test/java/org/elasticsearch/common/inject/ModuleTestCase.java index d96b89d382c..60c3ca126d5 100644 --- a/core/src/test/java/org/elasticsearch/common/inject/ModuleTestCase.java +++ b/core/src/test/java/org/elasticsearch/common/inject/ModuleTestCase.java @@ -72,6 +72,37 @@ public abstract class ModuleTestCase extends ESTestCase { } } + /** + * Configures the module and checks a Map of the "to" class + * is bound to "theClas". + */ + public void assertMapMultiBinding(Module module, Class to, Class theClass) { + List elements = Elements.getElements(module); + Set bindings = new HashSet<>(); + boolean providerFound = false; + for (Element element : elements) { + if (element instanceof LinkedKeyBinding) { + LinkedKeyBinding binding = (LinkedKeyBinding)element; + if (to.equals(binding.getKey().getTypeLiteral().getType())) { + bindings.add(binding.getLinkedKey().getTypeLiteral().getType()); + } + } else if (element instanceof ProviderInstanceBinding) { + ProviderInstanceBinding binding = (ProviderInstanceBinding)element; + String setType = binding.getKey().getTypeLiteral().getType().toString(); + if (setType.equals("java.util.Map")) { + providerFound = true; + } + } + } + + if (bindings.contains(theClass) == false) { + fail("Expected to find " + theClass.getName() + " as binding to " + to.getName() + ", found these classes:\n" + bindings); + } + assertTrue("Did not find provider for map of " + to.getName(), providerFound); + } + + + /** * Configures the module and checks a Set of the "to" class * is bound to "classes". There may be more classes bound diff --git a/core/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/core/src/test/java/org/elasticsearch/search/SearchModuleTests.java new file mode 100644 index 00000000000..efdcf0062c3 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -0,0 +1,69 @@ +/* + * 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.search; + +import org.elasticsearch.common.inject.ModuleTestCase; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.search.highlight.CustomHighlighter; +import org.elasticsearch.search.highlight.Highlighter; +import org.elasticsearch.search.highlight.PlainHighlighter; +import org.elasticsearch.search.suggest.CustomSuggester; +import org.elasticsearch.search.suggest.Suggester; +import org.elasticsearch.search.suggest.phrase.PhraseSuggester; +/** + */ +public class SearchModuleTests extends ModuleTestCase { + + public void testDoubleRegister() { + SearchModule module = new SearchModule(Settings.EMPTY); + try { + module.registerHighlighter("fvh", PlainHighlighter.class); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), "Can't register the same [highlighter] more than once for [fvh]"); + } + + try { + module.registerSuggester("term", PhraseSuggester.class); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), "Can't register the same [suggester] more than once for [term]"); + } + } + + public void testRegisterSuggester() { + SearchModule module = new SearchModule(Settings.EMPTY); + module.registerSuggester("custom", CustomSuggester.class); + try { + module.registerSuggester("custom", CustomSuggester.class); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), "Can't register the same [suggester] more than once for [custom]"); + } + assertMapMultiBinding(module, Suggester.class, CustomSuggester.class); + } + + public void testRegisterHighlighter() { + SearchModule module = new SearchModule(Settings.EMPTY); + module.registerHighlighter("custom", CustomHighlighter.class); + try { + module.registerHighlighter("custom", CustomHighlighter.class); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), "Can't register the same [highlighter] more than once for [custom]"); + } + assertMapMultiBinding(module, Highlighter.class, CustomHighlighter.class); + } +} diff --git a/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighter.java b/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighter.java index 3a9135cb731..e193d2ad69b 100644 --- a/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighter.java +++ b/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighter.java @@ -32,11 +32,6 @@ import java.util.Map; */ public class CustomHighlighter implements Highlighter { - @Override - public String[] names() { - return new String[] { "test-custom" }; - } - @Override public HighlightField highlight(HighlighterContext highlighterContext) { SearchContextHighlight.Field field = highlighterContext.field; diff --git a/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighterPlugin.java b/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighterPlugin.java index e7c69793c2e..705265ea5f6 100644 --- a/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighterPlugin.java +++ b/core/src/test/java/org/elasticsearch/search/highlight/CustomHighlighterPlugin.java @@ -35,6 +35,6 @@ public class CustomHighlighterPlugin extends AbstractPlugin { } public void onModule(SearchModule highlightModule) { - highlightModule.registerHighlighter(CustomHighlighter.class); + highlightModule.registerHighlighter("test-custom", CustomHighlighter.class); } } diff --git a/core/src/test/java/org/elasticsearch/search/suggest/CustomSuggester.java b/core/src/test/java/org/elasticsearch/search/suggest/CustomSuggester.java index 6e57390a165..e3dfe3b96d3 100644 --- a/core/src/test/java/org/elasticsearch/search/suggest/CustomSuggester.java +++ b/core/src/test/java/org/elasticsearch/search/suggest/CustomSuggester.java @@ -55,11 +55,6 @@ public class CustomSuggester extends Suggester