Merge pull request #14284 from s1monw/fold_similarity_module_into_index_module

Fold SimilarityModule into IndexModule

core/src/main/java/org/elasticsearch/index/IndexModule.java

@@ -28,6 +28,9 @@ import org.elasticsearch.index.fielddata.IndexFieldDataService;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.shard.IndexEventListener;
 import org.elasticsearch.index.shard.IndexSearcherWrapper;
+import org.elasticsearch.index.similarity.BM25SimilarityProvider;
+import org.elasticsearch.index.similarity.SimilarityProvider;
+import org.elasticsearch.index.similarity.SimilarityService;
 import org.elasticsearch.index.store.IndexStore;
 import org.elasticsearch.indices.store.IndicesStore;
 
@@ -36,11 +39,21 @@ import java.util.function.BiFunction;
 import java.util.function.Consumer;
 
 /**
- *
+ * IndexModule represents the central extension point for index level custom implementations like:
+ * <ul>
+ *     <li>{@link SimilarityProvider} - New {@link SimilarityProvider} implementations can be registered through {@link #addSimilarity(String, BiFunction)}
+ *         while existing Providers can be referenced through Settings under the {@link IndexModule#SIMILARITY_SETTINGS_PREFIX} prefix
+ *         along with the "type" value.  For example, to reference the {@link BM25SimilarityProvider}, the configuration
+ *         <tt>"index.similarity.my_similarity.type : "BM25"</tt> can be used.</li>
+ *      <li>{@link IndexStore} - Custom {@link IndexStore} instances can be registered via {@link #addIndexStore(String, BiFunction)}</li>
+ *      <li>{@link IndexEventListener} - Custom {@link IndexEventListener} instances can be registered via {@link #addIndexEventListener(IndexEventListener)}</li>
+ *      <li>Settings update listener - Custom settings update listener can be registered via {@link #addIndexSettingsListener(Consumer)}</li>
+ * </ul>
  */
 public class IndexModule extends AbstractModule {
 
     public static final String STORE_TYPE = "index.store.type";
+    public static final String SIMILARITY_SETTINGS_PREFIX = "index.similarity";
     private final IndexSettings indexSettings;
     private final IndicesStore indicesStore;
     // pkg private so tests can mock
@@ -50,6 +63,7 @@ public class IndexModule extends AbstractModule {
     private final Set<IndexEventListener> indexEventListeners = new HashSet<>();
     private IndexEventListener listener;
     private final Map<String, BiFunction<IndexSettings, IndicesStore, IndexStore>> storeTypes = new HashMap<>();
+    private final Map<String, BiFunction<String, Settings, SimilarityProvider>> similarities = new HashMap<>();
 
 
     public IndexModule(IndexSettings indexSettings, IndicesStore indicesStore) {
@@ -127,6 +141,21 @@ public class IndexModule extends AbstractModule {
         storeTypes.put(type, provider);
     }
 
+
+    /**
+     * Registers the given {@link SimilarityProvider} with the given name
+     *
+     * @param name Name of the SimilarityProvider
+     * @param similarity SimilarityProvider to register
+     */
+    public void addSimilarity(String name, BiFunction<String, Settings, SimilarityProvider> similarity) {
+        if (similarities.containsKey(name) || SimilarityService.BUILT_IN.containsKey(name)) {
+            throw new IllegalArgumentException("similarity for name: [" + name + " is already registered");
+        }
+        similarities.put(name, similarity);
+    }
+
+
     public IndexEventListener freeze() {
         // TODO somehow we need to make this pkg private...
         if (listener == null) {
@@ -175,6 +204,7 @@ public class IndexModule extends AbstractModule {
             }
         }
         bind(IndexStore.class).toInstance(store);
+        bind(SimilarityService.class).toInstance(new SimilarityService(settings, similarities));
     }
 
     public enum Type {

core/src/main/java/org/elasticsearch/index/similarity/SimilarityModule.java

@@ -1,67 +0,0 @@
-/*
- * 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.index.similarity;
-
-import org.elasticsearch.common.inject.AbstractModule;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.index.IndexSettings;
-
-import java.util.HashMap;
-import java.util.Map;
-import java.util.function.BiFunction;
-
-/**
- * {@link SimilarityModule} is responsible gathering registered and configured {@link SimilarityProvider}
- * implementations and making them available through the {@link SimilarityService}.
- *
- * New {@link SimilarityProvider} implementations can be registered through {@link #addSimilarity(String, BiFunction)}
- * while existing Providers can be referenced through Settings under the {@link #SIMILARITY_SETTINGS_PREFIX} prefix
- * along with the "type" value.  For example, to reference the {@link BM25SimilarityProvider}, the configuration
- * <tt>"index.similarity.my_similarity.type : "BM25"</tt> can be used.
- */
-public class SimilarityModule extends AbstractModule {
-
-    public static final String SIMILARITY_SETTINGS_PREFIX = "index.similarity";
-
-    private final Map<String, BiFunction<String, Settings, SimilarityProvider>> similarities = new HashMap<>();
-    private final IndexSettings indexSettings;
-    public SimilarityModule(IndexSettings indexSettings) {
-        this.indexSettings = indexSettings;
-    }
-
-    /**
-     * Registers the given {@link SimilarityProvider} with the given name
-     *
-     * @param name Name of the SimilarityProvider
-     * @param similarity SimilarityProvider to register
-     */
-    public void addSimilarity(String name, BiFunction<String, Settings, SimilarityProvider> similarity) {
-        if (similarities.containsKey(name) || SimilarityService.BUILT_IN.containsKey(name)) {
-            throw new IllegalArgumentException("similarity for name: [" + name + " is already registered");
-        }
-        similarities.put(name, similarity);
-    }
-
-    @Override
-    protected void configure() {
-        SimilarityService service = new SimilarityService(indexSettings, new HashMap<>(similarities));
-        bind(SimilarityService.class).toInstance(service);
-    }
-}

core/src/main/java/org/elasticsearch/index/similarity/SimilarityService.java

@@ -21,9 +21,9 @@ package org.elasticsearch.index.similarity;
 
 import org.apache.lucene.search.similarities.PerFieldSimilarityWrapper;
 import org.apache.lucene.search.similarities.Similarity;
-import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.AbstractIndexComponent;
+import org.elasticsearch.index.IndexModule;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.MapperService;
@@ -33,9 +33,6 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.function.BiFunction;
 
-/**
- *
- */
 public class SimilarityService extends AbstractIndexComponent {
 
     public final static String DEFAULT_SIMILARITY = "default";
@@ -43,7 +40,7 @@ public class SimilarityService extends AbstractIndexComponent {
     private final Similarity baseSimilarity;
     private final Map<String, SimilarityProvider> similarities;
     static final Map<String, BiFunction<String, Settings, SimilarityProvider>> DEFAULTS;
-    static final Map<String, BiFunction<String, Settings, SimilarityProvider>> BUILT_IN;
+    public static final Map<String, BiFunction<String, Settings, SimilarityProvider>> BUILT_IN;
     static {
         Map<String, BiFunction<String, Settings, SimilarityProvider>> defaults = new HashMap<>();
         Map<String, BiFunction<String, Settings, SimilarityProvider>> buildIn = new HashMap<>();
@@ -59,11 +56,10 @@ public class SimilarityService extends AbstractIndexComponent {
         BUILT_IN = Collections.unmodifiableMap(buildIn);
     }
 
-    @Inject
     public SimilarityService(IndexSettings indexSettings, Map<String, BiFunction<String, Settings, SimilarityProvider>> similarities) {
         super(indexSettings);
         Map<String, SimilarityProvider> providers = new HashMap<>(similarities.size());
-        Map<String, Settings> similaritySettings = this.indexSettings.getSettings().getGroups(SimilarityModule.SIMILARITY_SETTINGS_PREFIX);
+        Map<String, Settings> similaritySettings = this.indexSettings.getSettings().getGroups(IndexModule.SIMILARITY_SETTINGS_PREFIX);
         for (Map.Entry<String, Settings> entry : similaritySettings.entrySet()) {
             String name = entry.getKey();
             Settings settings = entry.getValue();

core/src/main/java/org/elasticsearch/indices/IndicesService.java

@@ -59,7 +59,6 @@ import org.elasticsearch.index.shard.IllegalIndexShardStateException;
 import org.elasticsearch.index.shard.IndexShard;
 import org.elasticsearch.index.shard.IndexEventListener;
 import org.elasticsearch.index.shard.ShardId;
-import org.elasticsearch.index.similarity.SimilarityModule;
 import org.elasticsearch.indices.analysis.IndicesAnalysisService;
 import org.elasticsearch.indices.recovery.RecoverySettings;
 import org.elasticsearch.indices.store.IndicesStore;
@@ -313,7 +312,6 @@ public class IndicesService extends AbstractLifecycleComponent<IndicesService> i
         }
         indexModule.addIndexEventListener(oldShardsStats);
         modules.add(new AnalysisModule(idxSettings.getSettings(), indicesAnalysisService));
-        modules.add(new SimilarityModule(idxSettings));
         modules.add(new IndexCacheModule(idxSettings.getSettings()));
         modules.add(indexModule);
         pluginsService.processModules(modules);

core/src/test/java/org/elasticsearch/index/IndexModuleTests.java

@@ -19,7 +19,13 @@
 package org.elasticsearch.index;
 
 import org.apache.lucene.index.DirectoryReader;
+import org.apache.lucene.index.FieldInvertState;
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.search.CollectionStatistics;
 import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.TermStatistics;
+import org.apache.lucene.search.similarities.BM25Similarity;
+import org.apache.lucene.search.similarities.Similarity;
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.inject.ModuleTestCase;
@@ -30,11 +36,14 @@ import org.elasticsearch.index.engine.EngineFactory;
 import org.elasticsearch.index.engine.InternalEngineFactory;
 import org.elasticsearch.index.shard.IndexEventListener;
 import org.elasticsearch.index.shard.IndexSearcherWrapper;
+import org.elasticsearch.index.similarity.SimilarityProvider;
+import org.elasticsearch.index.similarity.SimilarityService;
 import org.elasticsearch.index.store.IndexStore;
 import org.elasticsearch.indices.store.IndicesStore;
 import org.elasticsearch.test.IndexSettingsModule;
 import org.elasticsearch.test.engine.MockEngineFactory;
 
+import java.io.IOException;
 import java.util.Collections;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Consumer;
@@ -128,8 +137,92 @@ public class IndexModuleTests extends ModuleTestCase {
         }
         assertInstanceBinding(module, IndexSettings.class, (x) -> x.getUpdateListeners().size() == 1);
         assertInstanceBinding(module, IndexSettings.class, (x) -> x.getUpdateListeners().get(0) == listener);
+    }
+
+    public void testAddSimilarity() {
+        Settings indexSettings = Settings.settingsBuilder()
+                .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
+                .put("index.similarity.my_similarity.type", "test_similarity")
+                .put("index.similarity.my_similarity.key", "there is a key")
+                .build();
+        IndexModule module = new IndexModule(IndexSettingsModule.newIndexSettings(new Index("foo"), indexSettings, Collections.EMPTY_LIST), null);
+        module.addSimilarity("test_similarity", (string, settings) -> new SimilarityProvider() {
+            @Override
+            public String name() {
+                return string;
+            }
+
+            @Override
+            public Similarity get() {
+                return new TestSimilarity(settings.get("key"));
+            }
+        });
+        assertInstanceBinding(module, SimilarityService.class, (inst) -> {
+            if (inst instanceof SimilarityService) {
+                assertNotNull(inst.getSimilarity("my_similarity"));
+                assertTrue(inst.getSimilarity("my_similarity").get() instanceof TestSimilarity);
+                assertEquals("my_similarity", inst.getSimilarity("my_similarity").name());
+                assertEquals("there is a key" , ((TestSimilarity)inst.getSimilarity("my_similarity").get()).key);
+                return true;
+            }
+            return false;
+        });
+    }
+
+    public void testSetupUnknownSimilarity() {
+        Settings indexSettings = Settings.settingsBuilder()
+                .put("index.similarity.my_similarity.type", "test_similarity")
+                .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
+                .build();
+        IndexModule module = new IndexModule(IndexSettingsModule.newIndexSettings(new Index("foo"), indexSettings, Collections.EMPTY_LIST), null);
+        try {
+            assertInstanceBinding(module, SimilarityService.class, (inst) -> inst instanceof SimilarityService);
+        } catch (IllegalArgumentException ex) {
+            assertEquals("Unknown Similarity type [test_similarity] for [my_similarity]", ex.getMessage());
+        }
+    }
 
 
+    public void testSetupWithoutType() {
+        Settings indexSettings = Settings.settingsBuilder()
+                .put("index.similarity.my_similarity.foo", "bar")
+                .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
+                .build();
+        IndexModule module = new IndexModule(IndexSettingsModule.newIndexSettings(new Index("foo"), indexSettings, Collections.EMPTY_LIST), null);
+        try {
+            assertInstanceBinding(module, SimilarityService.class, (inst) -> inst instanceof SimilarityService);
+        } catch (IllegalArgumentException ex) {
+            assertEquals("Similarity [my_similarity] must have an associated type", ex.getMessage());
+        }
+    }
+
+
+    private static class TestSimilarity extends Similarity {
+        private final Similarity delegate = new BM25Similarity();
+        private final String key;
+
+
+        public TestSimilarity(String key) {
+            if (key == null) {
+                throw new AssertionError("key is null");
+            }
+            this.key = key;
+        }
+
+        @Override
+        public long computeNorm(FieldInvertState state) {
+            return delegate.computeNorm(state);
+        }
+
+        @Override
+        public SimWeight computeWeight(CollectionStatistics collectionStats, TermStatistics... termStats) {
+            return delegate.computeWeight(collectionStats, termStats);
+        }
+
+        @Override
+        public SimScorer simScorer(SimWeight weight, LeafReaderContext context) throws IOException {
+            return delegate.simScorer(weight, context);
+        }
     }
 
     public static final class Wrapper extends IndexSearcherWrapper {

core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java

@@ -65,7 +65,7 @@ import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.query.functionscore.ScoreFunctionParser;
 import org.elasticsearch.index.query.functionscore.ScoreFunctionParserMapper;
 import org.elasticsearch.index.query.support.QueryParsers;
-import org.elasticsearch.index.similarity.SimilarityModule;
+import org.elasticsearch.index.similarity.SimilarityService;
 import org.elasticsearch.indices.IndicesModule;
 import org.elasticsearch.indices.analysis.IndicesAnalysisService;
 import org.elasticsearch.indices.breaker.CircuitBreakerService;
@@ -215,10 +215,11 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
                 new IndexSettingsModule(index, indexSettings),
                 new IndexCacheModule(indexSettings),
                 new AnalysisModule(indexSettings, new IndicesAnalysisService(indexSettings)),
-                new SimilarityModule(IndexSettingsModule.newIndexSettings(index, indexSettings, Collections.EMPTY_LIST)),
                 new AbstractModule() {
                     @Override
                     protected void configure() {
+                        SimilarityService service = new SimilarityService(IndexSettingsModule.newIndexSettings(index, indexSettings, Collections.EMPTY_LIST), Collections.EMPTY_MAP);
+                        bind(SimilarityService.class).toInstance(service);
                         bind(Client.class).toInstance(proxy);
                         Multibinder.newSetBinder(binder(), ScoreFunctionParser.class);
                         bind(ScoreFunctionParserMapper.class).asEagerSingleton();

core/src/test/java/org/elasticsearch/index/query/TemplateQueryParserTests.java

@@ -37,11 +37,10 @@ import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.EnvironmentModule;
 import org.elasticsearch.index.Index;
-import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.analysis.AnalysisModule;
 import org.elasticsearch.index.cache.IndexCacheModule;
 import org.elasticsearch.index.query.functionscore.ScoreFunctionParser;
-import org.elasticsearch.index.similarity.SimilarityModule;
+import org.elasticsearch.index.similarity.SimilarityService;
 import org.elasticsearch.indices.IndicesModule;
 import org.elasticsearch.indices.analysis.IndicesAnalysisService;
 import org.elasticsearch.indices.breaker.CircuitBreakerService;
@@ -98,10 +97,11 @@ public class TemplateQueryParserTests extends ESTestCase {
                 new IndexSettingsModule(index, settings),
                 new IndexCacheModule(settings),
                 new AnalysisModule(settings, new IndicesAnalysisService(settings)),
-                new SimilarityModule(IndexSettingsModule.newIndexSettings(index, settings, Collections.EMPTY_LIST)),
                 new AbstractModule() {
                     @Override
                     protected void configure() {
+                        SimilarityService service = new SimilarityService(IndexSettingsModule.newIndexSettings(index, settings, Collections.EMPTY_LIST), Collections.EMPTY_MAP);
+                        bind(SimilarityService.class).toInstance(service);
                         bind(Client.class).toInstance(proxy); // not needed here
                         Multibinder.newSetBinder(binder(), ScoreFunctionParser.class);
                         bind(ClusterService.class).toProvider(Providers.of((ClusterService) null));

core/src/test/java/org/elasticsearch/index/similarity/SimilarityModuleTests.java

@@ -37,89 +37,5 @@ import java.util.Collections;
 
 public class SimilarityModuleTests extends ModuleTestCase {
 
-    public void testAddSimilarity() {
-        Settings indexSettings = Settings.settingsBuilder()
-                .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
-                .put("index.similarity.my_similarity.type", "test_similarity")
-                .put("index.similarity.my_similarity.key", "there is a key")
-                .build();
-        SimilarityModule module = new SimilarityModule(IndexSettingsModule.newIndexSettings(new Index("foo"), indexSettings, Collections.EMPTY_LIST));
-        module.addSimilarity("test_similarity", (string, settings) -> new SimilarityProvider() {
-            @Override
-            public String name() {
-                return string;
-            }
 
-            @Override
-            public Similarity get() {
-                return new TestSimilarity(settings.get("key"));
-            }
-        });
-        assertInstanceBinding(module, SimilarityService.class, (inst) -> {
-            if (inst instanceof SimilarityService) {
-                assertNotNull(inst.getSimilarity("my_similarity"));
-                assertTrue(inst.getSimilarity("my_similarity").get() instanceof TestSimilarity);
-                assertEquals("my_similarity", inst.getSimilarity("my_similarity").name());
-                assertEquals("there is a key" , ((TestSimilarity)inst.getSimilarity("my_similarity").get()).key);
-                return true;
-            }
-            return false;
-        });
-    }
-
-    public void testSetupUnknownSimilarity() {
-        Settings indexSettings = Settings.settingsBuilder()
-                .put("index.similarity.my_similarity.type", "test_similarity")
-                .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
-                .build();
-        SimilarityModule module = new SimilarityModule(IndexSettingsModule.newIndexSettings(new Index("foo"), indexSettings, Collections.EMPTY_LIST));
-        try {
-            assertInstanceBinding(module, SimilarityService.class, (inst) -> inst instanceof SimilarityService);
-        } catch (IllegalArgumentException ex) {
-            assertEquals("Unknown Similarity type [test_similarity] for [my_similarity]", ex.getMessage());
-        }
-    }
-
-
-    public void testSetupWithoutType() {
-        Settings indexSettings = Settings.settingsBuilder()
-                .put("index.similarity.my_similarity.foo", "bar")
-                .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
-                .build();
-        SimilarityModule module = new SimilarityModule(IndexSettingsModule.newIndexSettings(new Index("foo"), indexSettings, Collections.EMPTY_LIST));
-        try {
-            assertInstanceBinding(module, SimilarityService.class, (inst) -> inst instanceof SimilarityService);
-        } catch (IllegalArgumentException ex) {
-            assertEquals("Similarity [my_similarity] must have an associated type", ex.getMessage());
-        }
-    }
-
-
-    private static class TestSimilarity extends Similarity {
-        private final Similarity delegate = new BM25Similarity();
-        private final String key;
-
-
-        public TestSimilarity(String key) {
-            if (key == null) {
-                throw new AssertionError("key is null");
-            }
-            this.key = key;
-        }
-
-        @Override
-        public long computeNorm(FieldInvertState state) {
-            return delegate.computeNorm(state);
-        }
-
-        @Override
-        public SimWeight computeWeight(CollectionStatistics collectionStats, TermStatistics... termStats) {
-            return delegate.computeWeight(collectionStats, termStats);
-        }
-
-        @Override
-        public SimScorer simScorer(SimWeight weight, LeafReaderContext context) throws IOException {
-            return delegate.simScorer(weight, context);
-        }
-    }
 }