From 2064b17e3f3e2cf1d298ff308bf204da944adcc2 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Fri, 2 Oct 2015 21:35:53 +0200 Subject: [PATCH] Simplify similarity module and friends SimilarityModule was binding two different interfaces SimilaritySerivce and SimilarityLookupSerice. Both used a class Similarities which was holding some default impls etc. Thit commit folds all the logic into a rather simplified SimilarityService which has not construction time dependency to any other service in the system anymore. It's soely constructued from custom similarities, the index name and index settings and SimilarityModule is just trivial glue code with out much logic. This also adds a simple unittest for basic test coverage of the service. --- .../metadata/MetaDataIndexUpgradeService.java | 6 +- .../org/elasticsearch/index/LocalNodeId.java | 40 ------ .../index/LocalNodeIdModule.java | 39 ------ .../index/mapper/DocumentMapperParser.java | 10 +- .../index/mapper/FieldMapper.java | 4 +- .../elasticsearch/index/mapper/Mapper.java | 18 +-- .../index/mapper/MapperService.java | 6 +- .../index/mapper/core/TypeParsers.java | 4 +- .../index/mapper/internal/AllFieldMapper.java | 4 +- .../index/query/QueryShardContext.java | 2 +- .../elasticsearch/index/shard/IndexShard.java | 2 +- .../similarity/BM25SimilarityProvider.java | 5 +- .../similarity/DFRSimilarityProvider.java | 3 +- .../similarity/DefaultSimilarityProvider.java | 5 +- .../similarity/IBSimilarityProvider.java | 5 +- .../LMDirichletSimilarityProvider.java | 5 +- .../LMJelinekMercerSimilarityProvider.java | 3 +- .../PreBuiltSimilarityProvider.java | 73 ----------- .../index/similarity/Similarities.java | 56 --------- .../similarity/SimilarityLookupService.java | 85 ------------- .../index/similarity/SimilarityModule.java | 53 +++----- .../index/similarity/SimilarityProvider.java | 15 --- .../index/similarity/SimilarityService.java | 87 +++++++++---- .../elasticsearch/indices/IndicesService.java | 4 +- .../common/inject/ModuleTestCase.java | 16 --- .../index/engine/InternalEngineTests.java | 8 +- .../index/query/AbstractQueryTestCase.java | 2 +- .../index/query/TemplateQueryParserTests.java | 2 +- .../similarity/SimilarityModuleTests.java | 117 ++++++++++++++++++ .../index/similarity/SimilarityTests.java | 33 +++-- .../similarity/SimilarityIT.java | 1 - 31 files changed, 252 insertions(+), 461 deletions(-) delete mode 100644 core/src/main/java/org/elasticsearch/index/LocalNodeId.java delete mode 100644 core/src/main/java/org/elasticsearch/index/LocalNodeIdModule.java delete mode 100644 core/src/main/java/org/elasticsearch/index/similarity/PreBuiltSimilarityProvider.java delete mode 100644 core/src/main/java/org/elasticsearch/index/similarity/Similarities.java delete mode 100644 core/src/main/java/org/elasticsearch/index/similarity/SimilarityLookupService.java create mode 100644 core/src/test/java/org/elasticsearch/index/similarity/SimilarityModuleTests.java diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java index a17fe044dcb..6e29f31dffb 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java @@ -34,7 +34,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.analysis.AnalysisService; import org.elasticsearch.index.analysis.NamedAnalyzer; import org.elasticsearch.index.mapper.MapperService; -import org.elasticsearch.index.similarity.SimilarityLookupService; +import org.elasticsearch.index.similarity.SimilarityService; import org.elasticsearch.index.store.IndexStoreModule; import org.elasticsearch.script.ScriptService; @@ -322,11 +322,11 @@ public class MetaDataIndexUpgradeService extends AbstractComponent { Index index = new Index(indexMetaData.getIndex()); Settings settings = indexMetaData.settings(); try { - SimilarityLookupService similarityLookupService = new SimilarityLookupService(index, settings); + SimilarityService similarityService = new SimilarityService(index, settings); // We cannot instantiate real analysis server at this point because the node might not have // been started yet. However, we don't really need real analyzers at this stage - so we can fake it try (AnalysisService analysisService = new FakeAnalysisService(index, settings)) { - try (MapperService mapperService = new MapperService(index, settings, analysisService, similarityLookupService, scriptService)) { + try (MapperService mapperService = new MapperService(index, settings, analysisService, similarityService, scriptService)) { for (ObjectCursor cursor : indexMetaData.getMappings().values()) { MappingMetaData mappingMetaData = cursor.value; mapperService.merge(mappingMetaData.type(), mappingMetaData.source(), false, false); diff --git a/core/src/main/java/org/elasticsearch/index/LocalNodeId.java b/core/src/main/java/org/elasticsearch/index/LocalNodeId.java deleted file mode 100644 index a045636a688..00000000000 --- a/core/src/main/java/org/elasticsearch/index/LocalNodeId.java +++ /dev/null @@ -1,40 +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; - -import org.elasticsearch.common.inject.BindingAnnotation; - -import java.lang.annotation.Documented; -import java.lang.annotation.Retention; -import java.lang.annotation.Target; - -import static java.lang.annotation.ElementType.FIELD; -import static java.lang.annotation.ElementType.PARAMETER; -import static java.lang.annotation.RetentionPolicy.RUNTIME; - -/** - * - */ -@BindingAnnotation -@Target({FIELD, PARAMETER}) -@Retention(RUNTIME) -@Documented -public @interface LocalNodeId { -} diff --git a/core/src/main/java/org/elasticsearch/index/LocalNodeIdModule.java b/core/src/main/java/org/elasticsearch/index/LocalNodeIdModule.java deleted file mode 100644 index 82e36cd6efb..00000000000 --- a/core/src/main/java/org/elasticsearch/index/LocalNodeIdModule.java +++ /dev/null @@ -1,39 +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; - -import org.elasticsearch.common.inject.AbstractModule; - -/** - * - */ -public class LocalNodeIdModule extends AbstractModule { - - private final String localNodeId; - - public LocalNodeIdModule(String localNodeId) { - this.localNodeId = localNodeId; - } - - @Override - protected void configure() { - bind(String.class).annotatedWith(LocalNodeId.class).toInstance(localNodeId); - } -} diff --git a/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java b/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java index bb991ef2603..82ff5fb0a99 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java @@ -43,7 +43,7 @@ import org.elasticsearch.index.mapper.ip.IpFieldMapper; import org.elasticsearch.index.mapper.object.ObjectMapper; import org.elasticsearch.index.mapper.object.RootObjectMapper; import org.elasticsearch.index.settings.IndexSettings; -import org.elasticsearch.index.similarity.SimilarityLookupService; +import org.elasticsearch.index.similarity.SimilarityService; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService; @@ -57,7 +57,7 @@ public class DocumentMapperParser { final MapperService mapperService; final AnalysisService analysisService; private static final ESLogger logger = Loggers.getLogger(DocumentMapperParser.class); - private final SimilarityLookupService similarityLookupService; + private final SimilarityService similarityService; private final ScriptService scriptService; private final RootObjectMapper.TypeParser rootObjectTypeParser = new RootObjectMapper.TypeParser(); @@ -71,12 +71,12 @@ public class DocumentMapperParser { private volatile SortedMap additionalRootMappers; public DocumentMapperParser(@IndexSettings Settings indexSettings, MapperService mapperService, AnalysisService analysisService, - SimilarityLookupService similarityLookupService, ScriptService scriptService) { + SimilarityService similarityService, ScriptService scriptService) { this.indexSettings = indexSettings; this.parseFieldMatcher = new ParseFieldMatcher(indexSettings); this.mapperService = mapperService; this.analysisService = analysisService; - this.similarityLookupService = similarityLookupService; + this.similarityService = similarityService; this.scriptService = scriptService; MapBuilder typeParsersBuilder = new MapBuilder() .put(ByteFieldMapper.CONTENT_TYPE, new ByteFieldMapper.TypeParser()) @@ -142,7 +142,7 @@ public class DocumentMapperParser { } public Mapper.TypeParser.ParserContext parserContext(String type) { - return new Mapper.TypeParser.ParserContext(type, analysisService, similarityLookupService, mapperService, typeParsers, indexVersionCreated, parseFieldMatcher); + return new Mapper.TypeParser.ParserContext(type, analysisService, similarityService::getSimilarity, mapperService, typeParsers::get, indexVersionCreated, parseFieldMatcher); } public DocumentMapper parse(String source) throws MapperParsingException { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index ec53fbaa0b4..45bef68ee00 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -34,8 +34,8 @@ import org.elasticsearch.index.analysis.NamedAnalyzer; import org.elasticsearch.index.fielddata.FieldDataType; import org.elasticsearch.index.mapper.core.TypeParsers; import org.elasticsearch.index.mapper.internal.AllFieldMapper; -import org.elasticsearch.index.similarity.SimilarityLookupService; import org.elasticsearch.index.similarity.SimilarityProvider; +import org.elasticsearch.index.similarity.SimilarityService; import java.io.IOException; import java.util.ArrayList; @@ -447,7 +447,7 @@ public abstract class FieldMapper extends Mapper { if (fieldType().similarity() != null) { builder.field("similarity", fieldType().similarity().name()); } else if (includeDefaults) { - builder.field("similarity", SimilarityLookupService.DEFAULT_SIMILARITY); + builder.field("similarity", SimilarityService.DEFAULT_SIMILARITY); } if (includeDefaults || hasCustomFieldDataSettings()) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/Mapper.java b/core/src/main/java/org/elasticsearch/index/mapper/Mapper.java index ab561461f83..9ca34e1c573 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/Mapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/Mapper.java @@ -19,7 +19,6 @@ package org.elasticsearch.index.mapper; -import com.google.common.collect.ImmutableMap; import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseFieldMatcher; @@ -27,9 +26,10 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.index.analysis.AnalysisService; -import org.elasticsearch.index.similarity.SimilarityLookupService; +import org.elasticsearch.index.similarity.SimilarityProvider; import java.util.Map; +import java.util.function.Function; public abstract class Mapper implements ToXContent, Iterable { @@ -85,18 +85,18 @@ public abstract class Mapper implements ToXContent, Iterable { private final AnalysisService analysisService; - private final SimilarityLookupService similarityLookupService; + private final Function similarityLookupService; private final MapperService mapperService; - private final ImmutableMap typeParsers; + private final Function typeParsers; private final Version indexVersionCreated; private final ParseFieldMatcher parseFieldMatcher; - public ParserContext(String type, AnalysisService analysisService, SimilarityLookupService similarityLookupService, - MapperService mapperService, ImmutableMap typeParsers, + public ParserContext(String type, AnalysisService analysisService, Function similarityLookupService, + MapperService mapperService, Function typeParsers, Version indexVersionCreated, ParseFieldMatcher parseFieldMatcher) { this.type = type; this.analysisService = analysisService; @@ -115,8 +115,8 @@ public abstract class Mapper implements ToXContent, Iterable { return analysisService; } - public SimilarityLookupService similarityLookupService() { - return similarityLookupService; + public SimilarityProvider getSimilarity(String name) { + return similarityLookupService.apply(name); } public MapperService mapperService() { @@ -124,7 +124,7 @@ public abstract class Mapper implements ToXContent, Iterable { } public TypeParser typeParser(String type) { - return typeParsers.get(Strings.toUnderscoreCase(type)); + return typeParsers.apply(Strings.toUnderscoreCase(type)); } public Version indexVersionCreated() { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java index c205e636b45..256a67354ec 100755 --- a/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -52,7 +52,7 @@ import org.elasticsearch.index.mapper.Mapper.BuilderContext; import org.elasticsearch.index.mapper.internal.TypeFieldMapper; import org.elasticsearch.index.mapper.object.ObjectMapper; import org.elasticsearch.index.settings.IndexSettings; -import org.elasticsearch.index.similarity.SimilarityLookupService; +import org.elasticsearch.index.similarity.SimilarityService; import org.elasticsearch.indices.InvalidTypeNameException; import org.elasticsearch.indices.TypeMissingException; import org.elasticsearch.percolator.PercolatorService; @@ -125,12 +125,12 @@ public class MapperService extends AbstractIndexComponent implements Closeable { @Inject public MapperService(Index index, @IndexSettings Settings indexSettings, AnalysisService analysisService, - SimilarityLookupService similarityLookupService, + SimilarityService similarityService, ScriptService scriptService) { super(index, indexSettings); this.analysisService = analysisService; this.fieldTypes = new FieldTypeLookup(); - this.documentParser = new DocumentMapperParser(indexSettings, this, analysisService, similarityLookupService, scriptService); + this.documentParser = new DocumentMapperParser(indexSettings, this, analysisService, similarityService, scriptService); this.indexAnalyzer = new MapperAnalyzerWrapper(analysisService.defaultIndexAnalyzer(), p -> p.indexAnalyzer()); this.searchAnalyzer = new MapperAnalyzerWrapper(analysisService.defaultSearchAnalyzer(), p -> p.searchAnalyzer()); this.searchQuoteAnalyzer = new MapperAnalyzerWrapper(analysisService.defaultSearchQuoteAnalyzer(), p -> p.searchQuoteAnalyzer()); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/TypeParsers.java b/core/src/main/java/org/elasticsearch/index/mapper/core/TypeParsers.java index 0588bd1e044..3f142cc2f9c 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/TypeParsers.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/TypeParsers.java @@ -173,7 +173,7 @@ public class TypeParsers { builder.omitNorms(nodeBooleanValue(propNode)); iterator.remove(); } else if (propName.equals("similarity")) { - builder.similarity(parserContext.similarityLookupService().similarity(propNode.toString())); + builder.similarity(parserContext.getSimilarity(propNode.toString())); iterator.remove(); } else if (parseMultiField(builder, name, parserContext, propName, propNode)) { iterator.remove(); @@ -277,7 +277,7 @@ public class TypeParsers { // ignore for old indexes iterator.remove(); } else if (propName.equals("similarity")) { - builder.similarity(parserContext.similarityLookupService().similarity(propNode.toString())); + builder.similarity(parserContext.getSimilarity(propNode.toString())); iterator.remove(); } else if (propName.equals("fielddata")) { final Settings settings = Settings.builder().put(SettingsLoader.Helper.loadNestedFromMap(nodeMapValue(propNode, "fielddata"))).build(); diff --git a/core/src/main/java/org/elasticsearch/index/mapper/internal/AllFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/internal/AllFieldMapper.java index e538a00da16..59b664dbd65 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/internal/AllFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/internal/AllFieldMapper.java @@ -41,7 +41,7 @@ import org.elasticsearch.index.mapper.MergeResult; import org.elasticsearch.index.mapper.MetadataFieldMapper; import org.elasticsearch.index.mapper.ParseContext; import org.elasticsearch.index.query.QueryShardContext; -import org.elasticsearch.index.similarity.SimilarityLookupService; +import org.elasticsearch.index.similarity.SimilarityService; import java.io.IOException; import java.util.Iterator; @@ -300,7 +300,7 @@ public class AllFieldMapper extends MetadataFieldMapper { if (fieldType().similarity() != null) { builder.field("similarity", fieldType().similarity().name()); } else if (includeDefaults) { - builder.field("similarity", SimilarityLookupService.DEFAULT_SIMILARITY); + builder.field("similarity", SimilarityService.DEFAULT_SIMILARITY); } } diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 7862bb25280..5b12b2d1e1d 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -148,7 +148,7 @@ public class QueryShardContext { } public Similarity searchSimilarity() { - return indexQueryParser.similarityService != null ? indexQueryParser.similarityService.similarity() : null; + return indexQueryParser.similarityService != null ? indexQueryParser.similarityService.similarity(indexQueryParser.mapperService) : null; } public String defaultField() { diff --git a/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java index 4db52b65cb6..c3e6461e6f0 100644 --- a/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -1394,7 +1394,7 @@ public class IndexShard extends AbstractIndexShardComponent implements IndexSett }; return new EngineConfig(shardId, threadPool, indexingService, indexSettings, warmer, store, deletionPolicy, mergePolicyConfig.getMergePolicy(), mergeSchedulerConfig, - mapperService.indexAnalyzer(), similarityService.similarity(), codecService, failedEngineListener, translogRecoveryPerformer, indexCache.query(), cachingPolicy, translogConfig); + mapperService.indexAnalyzer(), similarityService.similarity(mapperService), codecService, failedEngineListener, translogRecoveryPerformer, indexCache.query(), cachingPolicy, translogConfig); } private static class IndexShardOperationCounter extends AbstractRefCounted { diff --git a/core/src/main/java/org/elasticsearch/index/similarity/BM25SimilarityProvider.java b/core/src/main/java/org/elasticsearch/index/similarity/BM25SimilarityProvider.java index 1983c4e8ecf..68e50da1348 100644 --- a/core/src/main/java/org/elasticsearch/index/similarity/BM25SimilarityProvider.java +++ b/core/src/main/java/org/elasticsearch/index/similarity/BM25SimilarityProvider.java @@ -21,8 +21,6 @@ package org.elasticsearch.index.similarity; import org.apache.lucene.search.similarities.BM25Similarity; import org.apache.lucene.search.similarities.Similarity; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.inject.assistedinject.Assisted; import org.elasticsearch.common.settings.Settings; /** @@ -40,8 +38,7 @@ public class BM25SimilarityProvider extends AbstractSimilarityProvider { private final BM25Similarity similarity; - @Inject - public BM25SimilarityProvider(@Assisted String name, @Assisted Settings settings) { + public BM25SimilarityProvider(String name, Settings settings) { super(name); float k1 = settings.getAsFloat("k1", 1.2f); float b = settings.getAsFloat("b", 0.75f); diff --git a/core/src/main/java/org/elasticsearch/index/similarity/DFRSimilarityProvider.java b/core/src/main/java/org/elasticsearch/index/similarity/DFRSimilarityProvider.java index b5a5cdcfeff..10ba1d44c8f 100644 --- a/core/src/main/java/org/elasticsearch/index/similarity/DFRSimilarityProvider.java +++ b/core/src/main/java/org/elasticsearch/index/similarity/DFRSimilarityProvider.java @@ -62,8 +62,7 @@ public class DFRSimilarityProvider extends AbstractSimilarityProvider { private final DFRSimilarity similarity; - @Inject - public DFRSimilarityProvider(@Assisted String name, @Assisted Settings settings) { + public DFRSimilarityProvider(String name, Settings settings) { super(name); BasicModel basicModel = parseBasicModel(settings); AfterEffect afterEffect = parseAfterEffect(settings); diff --git a/core/src/main/java/org/elasticsearch/index/similarity/DefaultSimilarityProvider.java b/core/src/main/java/org/elasticsearch/index/similarity/DefaultSimilarityProvider.java index 0f9feba952b..3acbd9821af 100644 --- a/core/src/main/java/org/elasticsearch/index/similarity/DefaultSimilarityProvider.java +++ b/core/src/main/java/org/elasticsearch/index/similarity/DefaultSimilarityProvider.java @@ -20,8 +20,6 @@ package org.elasticsearch.index.similarity; import org.apache.lucene.search.similarities.DefaultSimilarity; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.inject.assistedinject.Assisted; import org.elasticsearch.common.settings.Settings; /** @@ -37,8 +35,7 @@ public class DefaultSimilarityProvider extends AbstractSimilarityProvider { private final DefaultSimilarity similarity = new DefaultSimilarity(); - @Inject - public DefaultSimilarityProvider(@Assisted String name, @Assisted Settings settings) { + public DefaultSimilarityProvider(String name, Settings settings) { super(name); boolean discountOverlaps = settings.getAsBoolean("discount_overlaps", true); this.similarity.setDiscountOverlaps(discountOverlaps); diff --git a/core/src/main/java/org/elasticsearch/index/similarity/IBSimilarityProvider.java b/core/src/main/java/org/elasticsearch/index/similarity/IBSimilarityProvider.java index 161ca9c10ea..eb8d20abb2b 100644 --- a/core/src/main/java/org/elasticsearch/index/similarity/IBSimilarityProvider.java +++ b/core/src/main/java/org/elasticsearch/index/similarity/IBSimilarityProvider.java @@ -22,8 +22,6 @@ package org.elasticsearch.index.similarity; import com.google.common.collect.ImmutableMap; import org.apache.lucene.search.similarities.*; import org.elasticsearch.common.collect.MapBuilder; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.inject.assistedinject.Assisted; import org.elasticsearch.common.settings.Settings; /** @@ -56,8 +54,7 @@ public class IBSimilarityProvider extends AbstractSimilarityProvider { private final IBSimilarity similarity; - @Inject - public IBSimilarityProvider(@Assisted String name, @Assisted Settings settings) { + public IBSimilarityProvider(String name, Settings settings) { super(name); Distribution distribution = parseDistribution(settings); Lambda lambda = parseLambda(settings); diff --git a/core/src/main/java/org/elasticsearch/index/similarity/LMDirichletSimilarityProvider.java b/core/src/main/java/org/elasticsearch/index/similarity/LMDirichletSimilarityProvider.java index efea285639b..24494dc0b75 100644 --- a/core/src/main/java/org/elasticsearch/index/similarity/LMDirichletSimilarityProvider.java +++ b/core/src/main/java/org/elasticsearch/index/similarity/LMDirichletSimilarityProvider.java @@ -21,8 +21,6 @@ package org.elasticsearch.index.similarity; import org.apache.lucene.search.similarities.LMDirichletSimilarity; import org.apache.lucene.search.similarities.Similarity; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.inject.assistedinject.Assisted; import org.elasticsearch.common.settings.Settings; /** @@ -38,8 +36,7 @@ public class LMDirichletSimilarityProvider extends AbstractSimilarityProvider { private final LMDirichletSimilarity similarity; - @Inject - public LMDirichletSimilarityProvider(@Assisted String name, @Assisted Settings settings) { + public LMDirichletSimilarityProvider(String name, Settings settings) { super(name); float mu = settings.getAsFloat("mu", 2000f); this.similarity = new LMDirichletSimilarity(mu); diff --git a/core/src/main/java/org/elasticsearch/index/similarity/LMJelinekMercerSimilarityProvider.java b/core/src/main/java/org/elasticsearch/index/similarity/LMJelinekMercerSimilarityProvider.java index 5d30b300d5c..3d5a40fc153 100644 --- a/core/src/main/java/org/elasticsearch/index/similarity/LMJelinekMercerSimilarityProvider.java +++ b/core/src/main/java/org/elasticsearch/index/similarity/LMJelinekMercerSimilarityProvider.java @@ -38,8 +38,7 @@ public class LMJelinekMercerSimilarityProvider extends AbstractSimilarityProvide private final LMJelinekMercerSimilarity similarity; - @Inject - public LMJelinekMercerSimilarityProvider(@Assisted String name, @Assisted Settings settings) { + public LMJelinekMercerSimilarityProvider(String name, Settings settings) { super(name); float lambda = settings.getAsFloat("lambda", 0.1f); this.similarity = new LMJelinekMercerSimilarity(lambda); diff --git a/core/src/main/java/org/elasticsearch/index/similarity/PreBuiltSimilarityProvider.java b/core/src/main/java/org/elasticsearch/index/similarity/PreBuiltSimilarityProvider.java deleted file mode 100644 index 4b3f0ccf035..00000000000 --- a/core/src/main/java/org/elasticsearch/index/similarity/PreBuiltSimilarityProvider.java +++ /dev/null @@ -1,73 +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.apache.lucene.search.similarities.Similarity; -import org.elasticsearch.common.settings.Settings; - -/** - * {@link SimilarityProvider} for pre-built Similarities - */ -public class PreBuiltSimilarityProvider extends AbstractSimilarityProvider { - - public static class Factory implements SimilarityProvider.Factory { - - private final PreBuiltSimilarityProvider similarity; - - public Factory(String name, Similarity similarity) { - this.similarity = new PreBuiltSimilarityProvider(name, similarity); - } - - @Override - public SimilarityProvider create(String name, Settings settings) { - return similarity; - } - - public String name() { - return similarity.name(); - } - - public SimilarityProvider get() { - return similarity; - } - } - - private final Similarity similarity; - - /** - * Creates a new {@link PreBuiltSimilarityProvider} with the given name and given - * pre-built Similarity - * - * @param name Name of the Provider - * @param similarity Pre-built Similarity - */ - public PreBuiltSimilarityProvider(String name, Similarity similarity) { - super(name); - this.similarity = similarity; - } - - /** - * {@inheritDoc} - */ - @Override - public Similarity get() { - return similarity; - } -} diff --git a/core/src/main/java/org/elasticsearch/index/similarity/Similarities.java b/core/src/main/java/org/elasticsearch/index/similarity/Similarities.java deleted file mode 100644 index b40acb8a840..00000000000 --- a/core/src/main/java/org/elasticsearch/index/similarity/Similarities.java +++ /dev/null @@ -1,56 +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 com.google.common.collect.ImmutableMap; -import org.apache.lucene.search.similarities.BM25Similarity; -import org.apache.lucene.search.similarities.DefaultSimilarity; -import org.elasticsearch.common.collect.MapBuilder; - -import java.util.Collection; - -/** - * Cache of pre-defined Similarities - */ -public class Similarities { - - private static final ImmutableMap PRE_BUILT_SIMILARITIES; - - static { - MapBuilder similarities = MapBuilder.newMapBuilder(); - similarities.put(SimilarityLookupService.DEFAULT_SIMILARITY, - new PreBuiltSimilarityProvider.Factory(SimilarityLookupService.DEFAULT_SIMILARITY, new DefaultSimilarity())); - similarities.put("BM25", new PreBuiltSimilarityProvider.Factory("BM25", new BM25Similarity())); - - PRE_BUILT_SIMILARITIES = similarities.immutableMap(); - } - - private Similarities() { - } - - /** - * Returns the list of pre-defined SimilarityProvider Factories - * - * @return Pre-defined SimilarityProvider Factories - */ - public static Collection listFactories() { - return PRE_BUILT_SIMILARITIES.values(); - } -} diff --git a/core/src/main/java/org/elasticsearch/index/similarity/SimilarityLookupService.java b/core/src/main/java/org/elasticsearch/index/similarity/SimilarityLookupService.java deleted file mode 100644 index 8b8b68812cb..00000000000 --- a/core/src/main/java/org/elasticsearch/index/similarity/SimilarityLookupService.java +++ /dev/null @@ -1,85 +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 com.google.common.collect.ImmutableMap; -import org.elasticsearch.common.collect.MapBuilder; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.index.AbstractIndexComponent; -import org.elasticsearch.index.Index; -import org.elasticsearch.index.settings.IndexSettings; - -import java.util.Map; - -/** - * Service for looking up configured {@link SimilarityProvider} implementations by name. - *

- * The service instantiates the Providers through their Factories using configuration - * values found with the {@link SimilarityModule#SIMILARITY_SETTINGS_PREFIX} prefix. - */ -public class SimilarityLookupService extends AbstractIndexComponent { - - public final static String DEFAULT_SIMILARITY = "default"; - - private final ImmutableMap similarities; - - public SimilarityLookupService(Index index, Settings indexSettings) { - this(index, indexSettings, ImmutableMap.of()); - } - - @Inject - public SimilarityLookupService(Index index, @IndexSettings Settings indexSettings, Map similarities) { - super(index, indexSettings); - - MapBuilder providers = MapBuilder.newMapBuilder(); - - Map similaritySettings = indexSettings.getGroups(SimilarityModule.SIMILARITY_SETTINGS_PREFIX); - for (Map.Entry entry : similarities.entrySet()) { - String name = entry.getKey(); - SimilarityProvider.Factory factory = entry.getValue(); - - Settings settings = similaritySettings.get(name); - if (settings == null) { - settings = Settings.Builder.EMPTY_SETTINGS; - } - providers.put(name, factory.create(name, settings)); - } - - // For testing - for (PreBuiltSimilarityProvider.Factory factory : Similarities.listFactories()) { - if (!providers.containsKey(factory.name())) { - providers.put(factory.name(), factory.get()); - } - } - - this.similarities = providers.immutableMap(); - } - - /** - * Returns the {@link SimilarityProvider} with the given name - * - * @param name Name of the SimilarityProvider to find - * @return {@link SimilarityProvider} with the given name, or {@code null} if no Provider exists - */ - public SimilarityProvider similarity(String name) { - return similarities.get(name); - } -} diff --git a/core/src/main/java/org/elasticsearch/index/similarity/SimilarityModule.java b/core/src/main/java/org/elasticsearch/index/similarity/SimilarityModule.java index 6e03bcf0848..29312f2557b 100644 --- a/core/src/main/java/org/elasticsearch/index/similarity/SimilarityModule.java +++ b/core/src/main/java/org/elasticsearch/index/similarity/SimilarityModule.java @@ -20,19 +20,18 @@ package org.elasticsearch.index.similarity; import org.elasticsearch.common.inject.AbstractModule; -import org.elasticsearch.common.inject.Scopes; -import org.elasticsearch.common.inject.assistedinject.FactoryProvider; -import org.elasticsearch.common.inject.multibindings.MapBinder; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.Index; 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 SimilarityLookupService} and {@link SimilarityService}. + * implementations and making them available through the {@link SimilarityService}. * - * New {@link SimilarityProvider} implementations can be registered through {@link #addSimilarity(String, Class)} + * 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 * "index.similarity.my_similarity.type : "BM25" can be used. @@ -42,16 +41,12 @@ public class SimilarityModule extends AbstractModule { public static final String SIMILARITY_SETTINGS_PREFIX = "index.similarity"; private final Settings settings; - private final Map> similarities = new HashMap<>(); + private final Map> similarities = new HashMap<>(); + private final Index index; - public SimilarityModule(Settings settings) { + public SimilarityModule(Index index, Settings settings) { this.settings = settings; - addSimilarity("default", DefaultSimilarityProvider.class); - addSimilarity("BM25", BM25SimilarityProvider.class); - addSimilarity("DFR", DFRSimilarityProvider.class); - addSimilarity("IB", IBSimilarityProvider.class); - addSimilarity("LMDirichlet", LMDirichletSimilarityProvider.class); - addSimilarity("LMJelinekMercer", LMJelinekMercerSimilarityProvider.class); + this.index = index; } /** @@ -60,36 +55,16 @@ public class SimilarityModule extends AbstractModule { * @param name Name of the SimilarityProvider * @param similarity SimilarityProvider to register */ - public void addSimilarity(String name, Class similarity) { + public void addSimilarity(String name, BiFunction 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() { - MapBinder similarityBinder = - MapBinder.newMapBinder(binder(), String.class, SimilarityProvider.Factory.class); - - Map similaritySettings = settings.getGroups(SIMILARITY_SETTINGS_PREFIX); - for (Map.Entry entry : similaritySettings.entrySet()) { - String name = entry.getKey(); - Settings settings = entry.getValue(); - - String typeName = settings.get("type"); - if (typeName == null) { - throw new IllegalArgumentException("Similarity [" + name + "] must have an associated type"); - } else if (similarities.containsKey(typeName) == false) { - throw new IllegalArgumentException("Unknown Similarity type [" + typeName + "] for [" + name + "]"); - } - similarityBinder.addBinding(entry.getKey()).toProvider(FactoryProvider.newFactory(SimilarityProvider.Factory.class, similarities.get(typeName))).in(Scopes.SINGLETON); - } - - for (PreBuiltSimilarityProvider.Factory factory : Similarities.listFactories()) { - if (!similarities.containsKey(factory.name())) { - similarityBinder.addBinding(factory.name()).toInstance(factory); - } - } - - bind(SimilarityLookupService.class).asEagerSingleton(); - bind(SimilarityService.class).asEagerSingleton(); + SimilarityService service = new SimilarityService(index, settings, new HashMap<>(similarities)); + bind(SimilarityService.class).toInstance(service); } } diff --git a/core/src/main/java/org/elasticsearch/index/similarity/SimilarityProvider.java b/core/src/main/java/org/elasticsearch/index/similarity/SimilarityProvider.java index 38f56af4514..6433181dd6d 100644 --- a/core/src/main/java/org/elasticsearch/index/similarity/SimilarityProvider.java +++ b/core/src/main/java/org/elasticsearch/index/similarity/SimilarityProvider.java @@ -40,19 +40,4 @@ public interface SimilarityProvider { * @return Provided {@link Similarity} */ Similarity get(); - - /** - * Factory for creating {@link SimilarityProvider} instances - */ - public static interface Factory { - - /** - * Creates a new {@link SimilarityProvider} instance - * - * @param name Name of the provider - * @param settings Settings to be used by the Provider - * @return {@link SimilarityProvider} instance created by the Factory - */ - SimilarityProvider create(String name, Settings settings); - } } diff --git a/core/src/main/java/org/elasticsearch/index/similarity/SimilarityService.java b/core/src/main/java/org/elasticsearch/index/similarity/SimilarityService.java index 98faa87e94b..a77a2de4dff 100644 --- a/core/src/main/java/org/elasticsearch/index/similarity/SimilarityService.java +++ b/core/src/main/java/org/elasticsearch/index/similarity/SimilarityService.java @@ -25,55 +25,96 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.AbstractIndexComponent; import org.elasticsearch.index.Index; -import org.elasticsearch.index.mapper.FieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.settings.IndexSettings; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.function.BiFunction; + /** * */ public class SimilarityService extends AbstractIndexComponent { - private final SimilarityLookupService similarityLookupService; - private final MapperService mapperService; - - private final Similarity perFieldSimilarity; - + public final static String DEFAULT_SIMILARITY = "default"; + private final Similarity defaultSimilarity; + private final Similarity baseSimilarity; + private final Map similarities; + static final Map> DEFAULTS; + static final Map> BUILT_IN; + static { + Map> defaults = new HashMap<>(); + Map> buildIn = new HashMap<>(); + defaults.put("default", DefaultSimilarityProvider::new); + defaults.put("BM25", BM25SimilarityProvider::new); + buildIn.put("default", DefaultSimilarityProvider::new); + buildIn.put("BM25", BM25SimilarityProvider::new); + buildIn.put("DFR", DFRSimilarityProvider::new); + buildIn.put("IB", IBSimilarityProvider::new); + buildIn.put("LMDirichlet", LMDirichletSimilarityProvider::new); + buildIn.put("LMJelinekMercer", LMJelinekMercerSimilarityProvider::new); + DEFAULTS = Collections.unmodifiableMap(defaults); + BUILT_IN = Collections.unmodifiableMap(buildIn); + } public SimilarityService(Index index) { this(index, Settings.Builder.EMPTY_SETTINGS); } public SimilarityService(Index index, Settings settings) { - this(index, settings, new SimilarityLookupService(index, settings), null); + this(index, settings, Collections.EMPTY_MAP); } @Inject - public SimilarityService(Index index, @IndexSettings Settings indexSettings, - final SimilarityLookupService similarityLookupService, final MapperService mapperService) { + public SimilarityService(Index index, @IndexSettings Settings indexSettings, Map> similarities) { super(index, indexSettings); - this.similarityLookupService = similarityLookupService; - this.mapperService = mapperService; - - Similarity defaultSimilarity = similarityLookupService.similarity(SimilarityLookupService.DEFAULT_SIMILARITY).get(); + Map providers = new HashMap<>(similarities.size()); + Map similaritySettings = indexSettings.getGroups(SimilarityModule.SIMILARITY_SETTINGS_PREFIX); + for (Map.Entry entry : similaritySettings.entrySet()) { + String name = entry.getKey(); + Settings settings = entry.getValue(); + String typeName = settings.get("type"); + if (typeName == null) { + throw new IllegalArgumentException("Similarity [" + name + "] must have an associated type"); + } else if ((similarities.containsKey(typeName) || BUILT_IN.containsKey(typeName)) == false) { + throw new IllegalArgumentException("Unknown Similarity type [" + typeName + "] for [" + name + "]"); + } + BiFunction factory = similarities.getOrDefault(typeName, BUILT_IN.get(typeName)); + if (settings == null) { + settings = Settings.Builder.EMPTY_SETTINGS; + } + providers.put(name, factory.apply(name, settings)); + } + addSimilarities(similaritySettings, providers, DEFAULTS); + this.similarities = providers; + defaultSimilarity = providers.get(SimilarityService.DEFAULT_SIMILARITY).get(); // Expert users can configure the base type as being different to default, but out-of-box we use default. - Similarity baseSimilarity = (similarityLookupService.similarity("base") != null) ? similarityLookupService.similarity("base").get() : - defaultSimilarity; - - this.perFieldSimilarity = (mapperService != null) ? new PerFieldSimilarity(defaultSimilarity, baseSimilarity, mapperService) : + baseSimilarity = (providers.get("base") != null) ? providers.get("base").get() : defaultSimilarity; } - public Similarity similarity() { - return perFieldSimilarity; + public Similarity similarity(MapperService mapperService) { + // TODO we can maybe factor out MapperService here entirely by introducing an interface for the lookup? + return (mapperService != null) ? new PerFieldSimilarity(defaultSimilarity, baseSimilarity, mapperService) : + defaultSimilarity; } - public SimilarityLookupService similarityLookupService() { - return similarityLookupService; + private void addSimilarities(Map similaritySettings, Map providers, Map> similarities) { + for (Map.Entry> entry : similarities.entrySet()) { + String name = entry.getKey(); + BiFunction factory = entry.getValue(); + Settings settings = similaritySettings.get(name); + if (settings == null) { + settings = Settings.Builder.EMPTY_SETTINGS; + } + providers.put(name, factory.apply(name, settings)); + } } - public MapperService mapperService() { - return mapperService; + public SimilarityProvider getSimilarity(String name) { + return similarities.get(name); } static class PerFieldSimilarity extends PerFieldSimilarityWrapper { diff --git a/core/src/main/java/org/elasticsearch/indices/IndicesService.java b/core/src/main/java/org/elasticsearch/indices/IndicesService.java index ae69eee8c8d..8601d76cdae 100644 --- a/core/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/core/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -52,7 +52,6 @@ import org.elasticsearch.index.IndexModule; import org.elasticsearch.index.IndexNameModule; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.index.IndexService; -import org.elasticsearch.index.LocalNodeIdModule; import org.elasticsearch.index.analysis.AnalysisModule; import org.elasticsearch.index.analysis.AnalysisService; import org.elasticsearch.index.cache.IndexCache; @@ -330,7 +329,6 @@ public class IndicesService extends AbstractLifecycleComponent i ModulesBuilder modules = new ModulesBuilder(); modules.add(new IndexNameModule(index)); - modules.add(new LocalNodeIdModule(localNodeId)); modules.add(new IndexSettingsModule(index, indexSettings)); // plugin modules must be added here, before others or we can get crazy injection errors... for (Module pluginModule : pluginsService.indexModules(indexSettings)) { @@ -338,7 +336,7 @@ public class IndicesService extends AbstractLifecycleComponent i } modules.add(new IndexStoreModule(indexSettings)); modules.add(new AnalysisModule(indexSettings, indicesAnalysisService)); - modules.add(new SimilarityModule(indexSettings)); + modules.add(new SimilarityModule(index, indexSettings)); modules.add(new IndexCacheModule(indexSettings)); modules.add(new IndexModule()); 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 255def77eb2..9b327fb3112 100644 --- a/core/src/test/java/org/elasticsearch/common/inject/ModuleTestCase.java +++ b/core/src/test/java/org/elasticsearch/common/inject/ModuleTestCase.java @@ -60,22 +60,6 @@ public abstract class ModuleTestCase extends ESTestCase { fail("Did not find any binding to " + to.getName() + ". Found these bindings:\n" + s); } -// /** Configures the module and asserts "instance" is bound to "to". */ -// public void assertInstanceBinding(Module module, Class to, Object instance) { -// List elements = Elements.getElements(module); -// for (Element element : elements) { -// if (element instanceof ProviderInstanceBinding) { -// assertEquals(instance, ((ProviderInstanceBinding) element).getProviderInstance().get()); -// return; -// } -// } -// StringBuilder s = new StringBuilder(); -// for (Element element : elements) { -// s.append(element + "\n"); -// } -// fail("Did not find any binding to " + to.getName() + ". Found these bindings:\n" + s); -// } - /** * Attempts to configure the module, and asserts an {@link IllegalArgumentException} is * caught, containing the given messages diff --git a/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java index 8accf4d49ce..3dcb5482fc7 100644 --- a/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -68,7 +68,7 @@ import org.elasticsearch.index.mapper.internal.SourceFieldMapper; import org.elasticsearch.index.mapper.internal.UidFieldMapper; import org.elasticsearch.index.mapper.object.RootObjectMapper; import org.elasticsearch.index.shard.*; -import org.elasticsearch.index.similarity.SimilarityLookupService; +import org.elasticsearch.index.similarity.SimilarityService; import org.elasticsearch.index.store.DirectoryService; import org.elasticsearch.index.store.DirectoryUtils; import org.elasticsearch.index.store.Store; @@ -1875,10 +1875,10 @@ public class InternalEngineTests extends ESTestCase { RootObjectMapper.Builder rootBuilder = new RootObjectMapper.Builder("test"); Index index = new Index(indexName); AnalysisService analysisService = new AnalysisService(index, settings); - SimilarityLookupService similarityLookupService = new SimilarityLookupService(index, settings); - MapperService mapperService = new MapperService(index, settings, analysisService, similarityLookupService, null); + SimilarityService similarityService = new SimilarityService(index, settings); + MapperService mapperService = new MapperService(index, settings, analysisService, similarityService, null); DocumentMapper.Builder b = new DocumentMapper.Builder(settings, rootBuilder, mapperService); - DocumentMapperParser parser = new DocumentMapperParser(settings, mapperService, analysisService, similarityLookupService, null); + DocumentMapperParser parser = new DocumentMapperParser(settings, mapperService, analysisService, similarityService, null); this.docMapper = b.build(mapperService, parser); } diff --git a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java index 5837d5b16ff..fce0a9478b8 100644 --- a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java @@ -215,7 +215,7 @@ public abstract class AbstractQueryTestCase> new IndexSettingsModule(index, indexSettings), new IndexCacheModule(indexSettings), new AnalysisModule(indexSettings, new IndicesAnalysisService(indexSettings)), - new SimilarityModule(indexSettings), + new SimilarityModule(index, indexSettings), new IndexNameModule(index), new AbstractModule() { @Override diff --git a/core/src/test/java/org/elasticsearch/index/query/TemplateQueryParserTests.java b/core/src/test/java/org/elasticsearch/index/query/TemplateQueryParserTests.java index 40a002db881..985fbfde894 100644 --- a/core/src/test/java/org/elasticsearch/index/query/TemplateQueryParserTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/TemplateQueryParserTests.java @@ -96,7 +96,7 @@ public class TemplateQueryParserTests extends ESTestCase { new IndexSettingsModule(index, settings), new IndexCacheModule(settings), new AnalysisModule(settings, new IndicesAnalysisService(settings)), - new SimilarityModule(settings), + new SimilarityModule(index, settings), new IndexNameModule(index), new AbstractModule() { @Override diff --git a/core/src/test/java/org/elasticsearch/index/similarity/SimilarityModuleTests.java b/core/src/test/java/org/elasticsearch/index/similarity/SimilarityModuleTests.java new file mode 100644 index 00000000000..a73d2a5dac4 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/index/similarity/SimilarityModuleTests.java @@ -0,0 +1,117 @@ +/* + * 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.apache.lucene.index.FieldInvertState; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.search.CollectionStatistics; +import org.apache.lucene.search.TermStatistics; +import org.apache.lucene.search.similarities.BM25Similarity; +import org.apache.lucene.search.similarities.Similarity; +import org.elasticsearch.common.inject.ModuleTestCase; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.Index; + +import java.io.IOException; + +public class SimilarityModuleTests extends ModuleTestCase { + + public void testAddSimilarity() { + Settings indexSettings = Settings.settingsBuilder() + .put("index.similarity.my_similarity.type", "test_similarity") + .put("index.similarity.my_similarity.key", "there is a key") + .build(); + SimilarityModule module = new SimilarityModule(new Index("foo"), indexSettings); + 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") + .build(); + SimilarityModule module = new SimilarityModule(new Index("foo"), indexSettings); + 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") + .build(); + SimilarityModule module = new SimilarityModule(new Index("foo"), indexSettings); + 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); + } + } +} diff --git a/core/src/test/java/org/elasticsearch/index/similarity/SimilarityTests.java b/core/src/test/java/org/elasticsearch/index/similarity/SimilarityTests.java index 6a42ba7e8ce..28f5e5c62f6 100644 --- a/core/src/test/java/org/elasticsearch/index/similarity/SimilarityTests.java +++ b/core/src/test/java/org/elasticsearch/index/similarity/SimilarityTests.java @@ -22,6 +22,7 @@ package org.elasticsearch.index.similarity; import org.apache.lucene.search.similarities.*; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.index.IndexService; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.test.ESSingleNodeTestCase; import org.junit.Test; @@ -35,11 +36,9 @@ public class SimilarityTests extends ESSingleNodeTestCase { @Test public void testResolveDefaultSimilarities() { - SimilarityLookupService similarityLookupService = createIndex("foo").similarityService().similarityLookupService(); - assertThat(similarityLookupService.similarity("default"), instanceOf(PreBuiltSimilarityProvider.class)); - assertThat(similarityLookupService.similarity("default").get(), instanceOf(DefaultSimilarity.class)); - assertThat(similarityLookupService.similarity("BM25"), instanceOf(PreBuiltSimilarityProvider.class)); - assertThat(similarityLookupService.similarity("BM25").get(), instanceOf(BM25Similarity.class)); + SimilarityService similarityService = createIndex("foo").similarityService(); + assertThat(similarityService.getSimilarity("default").get(), instanceOf(DefaultSimilarity.class)); + assertThat(similarityService.getSimilarity("BM25").get(), instanceOf(BM25Similarity.class)); } @Test @@ -54,8 +53,8 @@ public class SimilarityTests extends ESSingleNodeTestCase { .put("index.similarity.my_similarity.type", "default") .put("index.similarity.my_similarity.discount_overlaps", false) .build(); - SimilarityService similarityService = createIndex("foo", indexSettings).similarityService(); - DocumentMapper documentMapper = similarityService.mapperService().documentMapperParser().parse(mapping); + IndexService indexService = createIndex("foo", indexSettings); + DocumentMapper documentMapper = indexService.mapperService().documentMapperParser().parse(mapping); assertThat(documentMapper.mappers().getMapper("field1").fieldType().similarity(), instanceOf(DefaultSimilarityProvider.class)); DefaultSimilarity similarity = (DefaultSimilarity) documentMapper.mappers().getMapper("field1").fieldType().similarity().get(); @@ -76,8 +75,8 @@ public class SimilarityTests extends ESSingleNodeTestCase { .put("index.similarity.my_similarity.b", 1.5f) .put("index.similarity.my_similarity.discount_overlaps", false) .build(); - SimilarityService similarityService = createIndex("foo", indexSettings).similarityService(); - DocumentMapper documentMapper = similarityService.mapperService().documentMapperParser().parse(mapping); + IndexService indexService = createIndex("foo", indexSettings); + DocumentMapper documentMapper = indexService.mapperService().documentMapperParser().parse(mapping); assertThat(documentMapper.mappers().getMapper("field1").fieldType().similarity(), instanceOf(BM25SimilarityProvider.class)); BM25Similarity similarity = (BM25Similarity) documentMapper.mappers().getMapper("field1").fieldType().similarity().get(); @@ -101,8 +100,8 @@ public class SimilarityTests extends ESSingleNodeTestCase { .put("index.similarity.my_similarity.normalization", "h2") .put("index.similarity.my_similarity.normalization.h2.c", 3f) .build(); - SimilarityService similarityService = createIndex("foo", indexSettings).similarityService(); - DocumentMapper documentMapper = similarityService.mapperService().documentMapperParser().parse(mapping); + IndexService indexService = createIndex("foo", indexSettings); + DocumentMapper documentMapper = indexService.mapperService().documentMapperParser().parse(mapping); assertThat(documentMapper.mappers().getMapper("field1").fieldType().similarity(), instanceOf(DFRSimilarityProvider.class)); DFRSimilarity similarity = (DFRSimilarity) documentMapper.mappers().getMapper("field1").fieldType().similarity().get(); @@ -127,8 +126,8 @@ public class SimilarityTests extends ESSingleNodeTestCase { .put("index.similarity.my_similarity.normalization", "h2") .put("index.similarity.my_similarity.normalization.h2.c", 3f) .build(); - SimilarityService similarityService = createIndex("foo", indexSettings).similarityService(); - DocumentMapper documentMapper = similarityService.mapperService().documentMapperParser().parse(mapping); + IndexService indexService = createIndex("foo", indexSettings); + DocumentMapper documentMapper = indexService.mapperService().documentMapperParser().parse(mapping); assertThat(documentMapper.mappers().getMapper("field1").fieldType().similarity(), instanceOf(IBSimilarityProvider.class)); IBSimilarity similarity = (IBSimilarity) documentMapper.mappers().getMapper("field1").fieldType().similarity().get(); @@ -150,8 +149,8 @@ public class SimilarityTests extends ESSingleNodeTestCase { .put("index.similarity.my_similarity.type", "LMDirichlet") .put("index.similarity.my_similarity.mu", 3000f) .build(); - SimilarityService similarityService = createIndex("foo", indexSettings).similarityService(); - DocumentMapper documentMapper = similarityService.mapperService().documentMapperParser().parse(mapping); + IndexService indexService = createIndex("foo", indexSettings); + DocumentMapper documentMapper = indexService.mapperService().documentMapperParser().parse(mapping); assertThat(documentMapper.mappers().getMapper("field1").fieldType().similarity(), instanceOf(LMDirichletSimilarityProvider.class)); LMDirichletSimilarity similarity = (LMDirichletSimilarity) documentMapper.mappers().getMapper("field1").fieldType().similarity().get(); @@ -170,8 +169,8 @@ public class SimilarityTests extends ESSingleNodeTestCase { .put("index.similarity.my_similarity.type", "LMJelinekMercer") .put("index.similarity.my_similarity.lambda", 0.7f) .build(); - SimilarityService similarityService = createIndex("foo", indexSettings).similarityService(); - DocumentMapper documentMapper = similarityService.mapperService().documentMapperParser().parse(mapping); + IndexService indexService = createIndex("foo", indexSettings); + DocumentMapper documentMapper = indexService.mapperService().documentMapperParser().parse(mapping); assertThat(documentMapper.mappers().getMapper("field1").fieldType().similarity(), instanceOf(LMJelinekMercerSimilarityProvider.class)); LMJelinekMercerSimilarity similarity = (LMJelinekMercerSimilarity) documentMapper.mappers().getMapper("field1").fieldType().similarity().get(); diff --git a/core/src/test/java/org/elasticsearch/similarity/SimilarityIT.java b/core/src/test/java/org/elasticsearch/similarity/SimilarityIT.java index 229f2a8ed61..d486cdba220 100644 --- a/core/src/test/java/org/elasticsearch/similarity/SimilarityIT.java +++ b/core/src/test/java/org/elasticsearch/similarity/SimilarityIT.java @@ -30,7 +30,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; public class SimilarityIT extends ESIntegTestCase { - @Test public void testCustomBM25Similarity() throws Exception {