Introduce a formal ExtensionPoint class to stream line extensions

This commit tries to add some infrastructure to streamline how extension
points should be strucutred. It's a simple approache with 4 implementations
for `highlighter`, `suggester`, `allocation_decider` and `shards_allocator`.
It simplifies adding new extension points and forces to register classes instead
of strings.

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<String, Class<? extends ShardsAllocator>> shardsAllocators = new HashMap<>();
+    private final ExtensionPoint.TypeExtensionPoint<ShardsAllocator> shardsAllocators = new ExtensionPoint.TypeExtensionPoint<>("shards_allocator", ShardsAllocator.class);
-    private final Set<Class<? extends AllocationDecider>> allocationDeciders = new HashSet<>();
+    private final ExtensionPoint.SetExtensionPoint<AllocationDecider> allocationDeciders = new ExtensionPoint.SetExtensionPoint<>("allocation_decider", AllocationDecider.class, AllocationDeciders.class);
 
     public AllocationModule(Settings settings) {
         this.settings = settings;
-        this.allocationDeciders.addAll(DEFAULT_ALLOCATION_DECIDERS);
+        for (Class<? extends AllocationDecider> decider : DEFAULT_ALLOCATION_DECIDERS) {
-        registerShardAllocator(BALANCED_ALLOCATOR, BalancedShardsAllocator.class);
+            allocationDeciders.registerExtension(decider);
-        registerShardAllocator(EVEN_SHARD_COUNT_ALLOCATOR, BalancedShardsAllocator.class);
+        }
+        shardsAllocators.registerExtension(BALANCED_ALLOCATOR, BalancedShardsAllocator.class);
+        shardsAllocators.registerExtension(EVEN_SHARD_COUNT_ALLOCATOR, BalancedShardsAllocator.class);
     }
 
     /** Register a custom allocation decider */
     public void registerAllocationDecider(Class<? extends AllocationDecider> allocationDecider) {
-        boolean isNew = allocationDeciders.add(allocationDecider);
+        allocationDeciders.registerExtension(allocationDecider);
-        if (isNew == false) {
-            throw new IllegalArgumentException("Cannot register AllocationDecider " + allocationDecider.getName() + " twice");
-        }
     }
 
     /** Register a custom shard allocator with the given name */
     public void registerShardAllocator(String name, Class<? extends ShardsAllocator> clazz) {
-        Class<? extends ShardsAllocator> existing = shardsAllocators.put(name, clazz);
+        shardsAllocators.registerExtension(name, clazz);
-        if (existing != null) {
-            throw new IllegalArgumentException("Cannot register ShardAllocator [" + name + "] to " + clazz.getName() + ", already registered to " + existing.getName());
-        }
     }
 
     @Override
     protected void configure() {
-
         // bind ShardsAllocator
-        final String shardsAllocatorType = settings.get(AllocationModule.SHARDS_ALLOCATOR_TYPE_KEY, AllocationModule.BALANCED_ALLOCATOR);
+        String shardsAllocatorType = shardsAllocators.bindType(binder(), settings, AllocationModule.SHARDS_ALLOCATOR_TYPE_KEY, AllocationModule.BALANCED_ALLOCATOR);
-        final Class<? extends ShardsAllocator> shardsAllocator = shardsAllocators.get(shardsAllocatorType);
+        if (shardsAllocatorType.equals(EVEN_SHARD_COUNT_ALLOCATOR)) {
-        if (shardsAllocator == null) {
-            throw new IllegalArgumentException("Unknown ShardsAllocator type [" + shardsAllocatorType + "]");
-        } else 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<AllocationDecider> allocationMultibinder = Multibinder.newSetBinder(binder(), AllocationDecider.class);
+        allocationDeciders.bind(binder());
-        for (Class<? extends AllocationDecider> allocation : allocationDeciders) {
-            allocationMultibinder.addBinding().to(allocation).asEagerSingleton();
-        }
 
         bind(GatewayAllocator.class).asEagerSingleton();
-        bind(AllocationDeciders.class).asEagerSingleton();
         bind(AllocationService.class).asEagerSingleton();
     }
+
 }

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<T> {
+    protected final String name;
+    protected final Class<T> 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<T> 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<T> extends ExtensionPoint<T> {
+        private final Map<String, Class<? extends T>> extensions = new HashMap<>();
+        private final Set<String> 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<T> extensionClass, Set<String> reservedKeys, Class<?>... singletons) {
+            super(name, extensionClass, singletons);
+            this.reservedKeys = reservedKeys;
+
+        }
+
+        /**
+         * Returns the extension for the given key or <code>null</code>
+         */
+        public Class<? extends T> 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<? extends T> 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<String, T> parserMapBinder = MapBinder.newMapBinder(binder, String.class, extensionClass);
+            for (Map.Entry<String, Class<? extends T>> 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<T> extends MapExtensionPoint<T> {
+
+        public TypeExtensionPoint(String name, Class<T> 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<? extends T> 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<T> extends ExtensionPoint<T> {
+        private final Set<Class<? extends T>> 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<T> 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<? extends T> 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<T> allocationMultibinder = Multibinder.newSetBinder(binder, extensionClass);
+            for (Class<? extends T> clazz : extensions) {
+                allocationMultibinder.addBinding().to(clazz);
+            }
+        }
+    }
+}

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;

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<Class<? extends Aggregator.Parser>> aggParsers = Lists.newArrayList();
+    private final Set<Class<? extends Aggregator.Parser>> aggParsers = new HashSet<>();
-    private final List<Class<? extends PipelineAggregator.Parser>> pipelineAggParsers = Lists.newArrayList();
+    private final Set<Class<? extends PipelineAggregator.Parser>> pipelineAggParsers = new HashSet<>();
-    private final List<Class<? extends Highlighter>> highlighters = Lists.newArrayList();
+    private final Highlighters highlighters = new Highlighters();
-    private final List<Class<? extends Suggester>> suggesters = Lists.newArrayList();
+    private final Suggesters suggesters = new Suggesters();
-    private final List<Class<? extends ScoreFunctionParser>> functionScoreParsers = Lists.newArrayList();
+    private final Set<Class<? extends ScoreFunctionParser>> functionScoreParsers = new HashSet<>();
-    private final List<Class<? extends FetchSubPhase>> fetchSubPhases = Lists.newArrayList();
+    private final Set<Class<? extends FetchSubPhase>> fetchSubPhases = new HashSet<>();
-    private final List<Class<? extends SignificanceHeuristicParser>> heuristicParsers = Lists.newArrayList();
+    private final Set<Class<? extends SignificanceHeuristicParser>> heuristicParsers = new HashSet<>();
-    private final List<Class<? extends MovAvgModel.AbstractModelParser>> modelParsers = Lists.newArrayList();
+    private final Set<Class<? extends MovAvgModel.AbstractModelParser>> 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<? extends Highlighter> clazz) {
+    public void registerHighlighter(String key, Class<? extends Highlighter> clazz) {
-        highlighters.add(clazz);
+        highlighters.registerExtension(key, clazz);
     }
 
-    public void registerSuggester(Class<? extends Suggester> suggester) {
+    public void registerSuggester(String key, Class<? extends Suggester> suggester) {
-        suggesters.add(suggester);
+        suggesters.registerExtension(key, suggester);
     }
 
     public void registerFunctionScoreParser(Class<? extends ScoreFunctionParser> parser) {
@@ -245,14 +243,7 @@ public class SearchModule extends AbstractModule {
     }
 
     protected void configureSuggesters() {
-        Multibinder<Suggester> suggesterMultibinder = Multibinder.newSetBinder(binder(), Suggester.class);
+        suggesters.bind(binder());
-        for (Class<? extends Suggester> clazz : suggesters) {
-            suggesterMultibinder.addBinding().to(clazz);
-        }
-
-        bind(SuggestParseElement.class).asEagerSingleton();
-        bind(SuggestPhase.class).asEagerSingleton();
-        bind(Suggesters.class).asEagerSingleton();
     }
 
     protected void configureFunctionScore() {
@@ -264,11 +255,7 @@ public class SearchModule extends AbstractModule {
     }
 
     protected void configureHighlighters() {
-        Multibinder<Highlighter> multibinder = Multibinder.newSetBinder(binder(), Highlighter.class);
+       highlighters.bind(binder());
-        for (Class<? extends Highlighter> highlighter : highlighters) {
-            multibinder.addBinding().to(highlighter);
-        }
-        bind(Highlighters.class).asEagerSingleton();
     }
 
     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();
     }
+
 }

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<String, Stream> map = new HashMap<>();
         map.putAll(STREAMS);
         map.put(stream.getName(), stream);

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<String, Stream> map = new HashMap<>();
         map.putAll(STREAMS);
         map.put(stream.getName(), stream);

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;

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);

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.*;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Set;
 
 /**
- *
+ * An extensions point and registry for all the highlighters a node supports.
  */
-public class Highlighters {
+public class Highlighters extends ExtensionPoint.MapExtensionPoint<Highlighter> {
+
+    @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<String, Highlighter> parsers;
+    private final DeprecationLogger deprecationLogger = new DeprecationLogger(ESLoggerFactory.getLogger(Highlighters.class.getName()));
+
+    public Highlighters(){
+        this(Collections.EMPTY_MAP);
+    }
+
+    private Highlighters(Map<String, Highlighter> 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<Highlighter> parsers) {
+    public Highlighters(Settings settings, Map<String, Highlighter> parsers) {
+        this(addBuiltIns(settings, parsers));
+    }
+
+    private static Map<String, Highlighter> addBuiltIns(Settings settings, Map<String, Highlighter> parsers) {
         // build in highlighers
         Map<String, Highlighter> map = new HashMap<>();
-        add(map, new FastVectorHighlighter(settings));
+        map.put(FVH,  new FastVectorHighlighter(settings));
-        add(map, new PlainHighlighter());
+        map.put(FAST_VECTOR_HIGHLIGHTER, map.get(FVH));
-        add(map, new PostingsHighlighter());
+        map.put(PLAIN, new PlainHighlighter());
-        for (Highlighter highlighter : parsers) {
+        map.put(HIGHLIGHTER, map.get(PLAIN));
-            add(map, highlighter);
+        map.put(POSTINGS, new PostingsHighlighter());
-        }
+        map.put(POSTINGS_HIGHLIGHTER, map.get(POSTINGS));
-        this.parsers = Collections.unmodifiableMap(map);
+        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<String, Highlighter> map, Highlighter highlighter) {
-        for (String type : highlighter.names()) {
-            map.put(type, highlighter);
-        }
-    }
-
 }

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;

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) {
 

core/src/main/java/org/elasticsearch/search/suggest/Suggester.java

@@ -29,8 +29,6 @@ public abstract class Suggester<T extends SuggestionSearchContext.SuggestionCont
     protected abstract Suggest.Suggestion<? extends Suggest.Suggestion.Entry<? extends Suggest.Suggestion.Entry.Option>>
         innerExecute(String name, T suggestion, IndexSearcher searcher, CharsRefBuilder spare) throws IOException;
 
-    public abstract String[] names();
-
     public abstract SuggestContextParser getContextParser();
 
     public Suggest.Suggestion<? extends Suggest.Suggestion.Entry<? extends Suggest.Suggestion.Entry.Option>>

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.inject.Binder;
-import org.elasticsearch.common.collect.MapBuilder;
 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.*;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Set;
 
 /**
  *
  */
-public class Suggesters {
+public final class Suggesters extends ExtensionPoint.MapExtensionPoint<Suggester> {
     private final Map<String, Suggester> parsers;
 
+    public Suggesters() {
+        this(Collections.EMPTY_MAP);
+    }
+
+    public Suggesters(Map<String, Suggester> 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<Suggester> suggesters, ScriptService scriptService) {
+    public Suggesters(Map<String, Suggester> suggesters, ScriptService scriptService) {
+        this(addBuildIns(suggesters, scriptService));
+    }
+
+    private static Map<String, Suggester> addBuildIns(Map<String, Suggester> suggesters, ScriptService scriptService) {
         final Map<String, Suggester> map = new HashMap<>();
-        add(map, new PhraseSuggester(scriptService));
+        map.put("phrase", new PhraseSuggester(scriptService));
-        add(map, new TermSuggester());
+        map.put("term", new TermSuggester());
-        add(map, new CompletionSuggester());
+        map.put("completion", new CompletionSuggester());
-        for (Suggester suggester : suggesters) {
+        map.putAll(suggesters);
-           add(map, suggester);
+        return map;
-        }
-        this.parsers = Collections.unmodifiableMap(map);
     }
 
     public Suggester get(String type) {
         return parsers.get(type);
     }
-
-    private void add(Map<String, Suggester> map, Suggester suggester) {
-        for (String type : suggester.names()) {
-            map.put(type, suggester);
-        }
-    }
 }

core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java

@@ -101,11 +101,6 @@ public class CompletionSuggester extends Suggester<CompletionSuggestionContext>
         return completionSuggestion;
     }
 
-    @Override
-    public String[] names() {
-        return new String[] { "completion" };
-    }
-
     @Override
     public SuggestContextParser getContextParser() {
         return new CompletionSuggestParser(this);

core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java

@@ -150,11 +150,6 @@ public final class PhraseSuggester extends Suggester<PhraseSuggestionContext> {
         return scriptService;
     }
     
-    @Override
-    public String[] names() {
-        return new String[] {"phrase"};
-    }
-
     @Override
     public SuggestContextParser getContextParser() {
         return new PhraseSuggestParser(this);

core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggester.java

@@ -65,11 +65,6 @@ public final class TermSuggester extends Suggester<TermSuggestionContext> {
         return response;
     }
 
-    @Override
-    public String[] names() {
-        return new String[] {"term"};
-    }
-
     @Override
     public SuggestContextParser getContextParser() {
         return new TermSuggestParser(this);

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"));
+            assertEquals(e.getMessage(), "Can't register the same [allocation_decider] more than once for [" + EnableAllocationDecider.class.getName() + "]");
-            assertTrue(e.getMessage().contains("twice"));
         }
     }
 
@@ -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() {

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<String, Class> of the "to" class
+     * is bound to "theClas".
+     */
+    public void assertMapMultiBinding(Module module, Class to, Class theClass) {
+        List<Element> elements = Elements.getElements(module);
+        Set<Type> 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<java.lang.String, " + to.getName() + ">")) {
+                    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

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);
+    }
+}

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;

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);
     }
 }

core/src/test/java/org/elasticsearch/search/suggest/CustomSuggester.java

@@ -55,11 +55,6 @@ public class CustomSuggester extends Suggester<CustomSuggester.CustomSuggestions
         return response;
     }
 
-    @Override
-    public String[] names() {
-        return new String[] {"custom"};
-    }
-
     @Override
     public SuggestContextParser getContextParser() {
         return new SuggestContextParser() {

core/src/test/java/org/elasticsearch/search/suggest/CustomSuggesterPlugin.java

@@ -37,7 +37,7 @@ public class CustomSuggesterPlugin extends AbstractPlugin {
     }
 
     public void onModule(SearchModule searchModule) {
-        searchModule.registerSuggester(CustomSuggester.class);
+        searchModule.registerSuggester("custom", CustomSuggester.class);
     }
 
 }