Skip to content

Commit

Permalink
Internal: Flatten IndicesModule and add tests
Browse files Browse the repository at this point in the history
The IndicesModule was made up of two submodules, one which
handled registering queries, and the other for registering
hunspell dictionaries. This change moves those into
IndicesModule. It also adds a new extension point type,
InstanceMap. This is simply a Map<K,V>, where K and V are
actual objects, not classes like most other extension points.
I also added a test method to help testing instance map extensions.
This was particularly painful because of how guice binds the key
and value as separate bindings, and then reconstitutes them
into a Map at injection time. In order to gain access to the
object which links the key and value, I had to tweak our
guice copy to not use an anonymous inner class for the Provider.

Note that I also renamed the existing extension point types, since
they were very redundant. For example, ExtensionPoint.MapExtensionPoint
is now ExtensionPoint.ClassMap.

See #12783.

Backport of #12921
  • Loading branch information
rjernst committed Aug 19, 2015
1 parent d93d87b commit ad4f015
Show file tree
Hide file tree
Showing 27 changed files with 364 additions and 263 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ public class ClusterModule extends AbstractModule {
private final Settings settings;
private final DynamicSettings.Builder clusterDynamicSettings = new DynamicSettings.Builder();
private final DynamicSettings.Builder indexDynamicSettings = new DynamicSettings.Builder();
private final ExtensionPoint.TypeExtensionPoint<ShardsAllocator> shardsAllocators = new ExtensionPoint.TypeExtensionPoint<>("shards_allocator", ShardsAllocator.class);
private final ExtensionPoint.SetExtensionPoint<AllocationDecider> allocationDeciders = new ExtensionPoint.SetExtensionPoint<>("allocation_decider", AllocationDecider.class, AllocationDeciders.class);
private final ExtensionPoint.SetExtensionPoint<IndexTemplateFilter> indexTemplateFilters = new ExtensionPoint.SetExtensionPoint<>("index_template_filter", IndexTemplateFilter.class);
private final ExtensionPoint.SelectedType<ShardsAllocator> shardsAllocators = new ExtensionPoint.SelectedType<>("shards_allocator", ShardsAllocator.class);
private final ExtensionPoint.ClassSet<AllocationDecider> allocationDeciders = new ExtensionPoint.ClassSet<>("allocation_decider", AllocationDecider.class, AllocationDeciders.class);
private final ExtensionPoint.ClassSet<IndexTemplateFilter> indexTemplateFilters = new ExtensionPoint.ClassSet<>("index_template_filter", IndexTemplateFilter.class);

// pkg private so tests can mock
Class<? extends ClusterInfoService> clusterInfoServiceImpl = InternalClusterInfoService.class;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

package org.elasticsearch.common.geo;

import org.elasticsearch.common.Classes;

/**
*/
public class ShapesAvailability {
Expand Down Expand Up @@ -48,8 +46,5 @@ public class ShapesAvailability {
JTS_AVAILABLE = xJTS_AVAILABLE;
}


private ShapesAvailability() {

}
private ShapesAvailability() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,31 @@
* @since 2.0
*/
public final class ProviderLookup<T> implements Element {

// NOTE: this class is not part of guice and was added so the provder lookup's key can be acessible for tests
public static class ProviderImpl<T> implements Provider<T> {
private ProviderLookup<T> lookup;

private ProviderImpl(ProviderLookup<T> lookup) {
this.lookup = lookup;
}

@Override
public T get() {
checkState(lookup.delegate != null,
"This Provider cannot be used until the Injector has been created.");
return lookup.delegate.get();
}

@Override
public String toString() {
return "Provider<" + lookup.key.getTypeLiteral() + ">";
}

public Key<T> getKey() {
return lookup.getKey();
}
}
private final Object source;
private final Key<T> key;
private Provider<T> delegate;
Expand Down Expand Up @@ -86,18 +111,6 @@ public Provider<T> getDelegate() {
* IllegalStateException} if you try to use it beforehand.
*/
public Provider<T> getProvider() {
return new Provider<T>() {
@Override
public T get() {
checkState(delegate != null,
"This Provider cannot be used until the Injector has been created.");
return delegate.get();
}

@Override
public String toString() {
return "Provider<" + key.getTypeLiteral() + ">";
}
};
return new ProviderImpl<>(this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,18 @@
* all extensions by a single name and ensures that extensions are not registered
* more than once.
*/
public abstract class ExtensionPoint<T> {
public abstract class ExtensionPoint {
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 point. 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) {
public ExtensionPoint(String name, Class<?>... singletons) {
this.name = name;
this.extensionClass = extensionClass;
this.singletons = singletons;
}

Expand All @@ -62,29 +59,30 @@ public final void bind(Binder binder) {
}

/**
* Subclasses can bind their type, map or set exentions here.
* Subclasses can bind their type, map or set extensions 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> {
public static class ClassMap<T> extends ExtensionPoint {
protected final Class<T> extensionClass;
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}
* Creates a new {@link ClassMap}
*
* @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);
public ClassMap(String name, Class<T> extensionClass, Set<String> reservedKeys, Class<?>... singletons) {
super(name, singletons);
this.extensionClass = extensionClass;
this.reservedKeys = reservedKeys;

}

/**
Expand Down Expand Up @@ -118,13 +116,13 @@ protected final void bindExtensions(Binder binder) {
}

/**
* A Type extension point which basically allows to registerd keyed extensions like {@link org.elasticsearch.common.util.ExtensionPoint.MapExtensionPoint}
* A Type extension point which basically allows to registerd keyed extensions like {@link ClassMap}
* 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 static final class SelectedType<T> extends ClassMap<T> {

public TypeExtensionPoint(String name, Class<T> extensionClass) {
public SelectedType(String name, Class<T> extensionClass) {
super(name, extensionClass, Collections.EMPTY_SET);
}

Expand Down Expand Up @@ -153,18 +151,20 @@ public String bindType(Binder binder, Settings settings, String settingsKey, Str
/**
* 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> {
public final static class ClassSet<T> extends ExtensionPoint {
protected final Class<T> extensionClass;
private final Set<Class<? extends T>> extensions = new HashSet<>();

/**
* Creates a new {@link org.elasticsearch.common.util.ExtensionPoint.SetExtensionPoint}
* Creates a new {@link ClassSet}
*
* @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);
public ClassSet(String name, Class<T> extensionClass, Class<?>... singletons) {
super(name, singletons);
this.extensionClass = extensionClass;
}

/**
Expand All @@ -188,4 +188,46 @@ protected final void bindExtensions(Binder binder) {
}
}
}

/**
* A an instance of a map, mapping one instance value to another. Both key and value are instances, not classes
* like with other extension points.
*/
public final static class InstanceMap<K, V> extends ExtensionPoint {
private final Map<K, V> map = new HashMap<>();
private final Class<K> keyType;
private final Class<V> valueType;

/**
* Creates a new {@link ClassSet}
*
* @param name the human readable underscore case name of the extension point. This is used in error messages.
* @param singletons a list of singletons to bind with this extension point - these are bound in {@link #bind(Binder)}
*/
public InstanceMap(String name, Class<K> keyType, Class<V> valueType, Class<?>... singletons) {
super(name, singletons);
this.keyType = keyType;
this.valueType = valueType;
}

/**
* Registers a mapping from {@param key} to {@param value}
*
* @throws IllegalArgumentException iff the key is already registered
*/
public final void registerExtension(K key, V value) {
V old = map.put(key, value);
if (old != null) {
throw new IllegalArgumentException("Cannot register [" + this.name + "] with key [" + key + "] to [" + value + "], already registered to [" + old + "]");
}
}

@Override
protected void bindExtensions(Binder binder) {
MapBinder<K, V> mapBinder = MapBinder.newMapBinder(binder, keyType, valueType);
for (Map.Entry<K, V> entry : map.entrySet()) {
mapBinder.addBinding(entry.getKey()).toInstance(entry.getValue());
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ public class IndexCacheModule extends AbstractModule {
public static final String QUERY_CACHE_EVERYTHING = "index.queries.cache.everything";

private final Settings indexSettings;
private final ExtensionPoint.TypeExtensionPoint<QueryCache> queryCaches;
private final ExtensionPoint.SelectedType<QueryCache> queryCaches;

public IndexCacheModule(Settings settings) {
this.indexSettings = settings;
this.queryCaches = new ExtensionPoint.TypeExtensionPoint<>("query_cache", QueryCache.class);
this.queryCaches = new ExtensionPoint.SelectedType<>("query_cache", QueryCache.class);

registerQueryCache(INDEX_QUERY_CACHE, IndexQueryCache.class);
registerQueryCache(NONE_QUERY_CACHE, NoneQueryCache.class);
Expand Down
Loading

0 comments on commit ad4f015

Please sign in to comment.