Simplify FetchSubPhase registration and detach it from Guice (#18862)

this commit removes FetchSubPhrase registration by class to registration
by instance. No Guice binding needed anymore.
This commit is contained in:
Simon Willnauer 2016-06-15 09:13:02 +02:00 committed by GitHub
parent fa77d4d885
commit 429dd3a876
13 changed files with 67 additions and 87 deletions

View File

@ -34,7 +34,7 @@ public final class Highlighters {
private final Map<String, Highlighter> parsers = new HashMap<>();
Highlighters(Settings settings) {
public Highlighters(Settings settings) {
registerHighlighter("fvh", new FastVectorHighlighter(settings));
registerHighlighter("plain", new PlainHighlighter());
registerHighlighter("postings", new PostingsHighlighter());

View File

@ -24,7 +24,6 @@ import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.geo.ShapesAvailability;
import org.elasticsearch.common.geo.builders.ShapeBuilders;
import org.elasticsearch.common.inject.AbstractModule;
import org.elasticsearch.common.inject.multibindings.Multibinder;
import org.elasticsearch.common.io.stream.NamedWriteable;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.io.stream.Writeable;
@ -92,7 +91,6 @@ import org.elasticsearch.index.query.functionscore.ScriptScoreFunctionBuilder;
import org.elasticsearch.index.query.functionscore.WeightBuilder;
import org.elasticsearch.indices.query.IndicesQueriesRegistry;
import org.elasticsearch.search.action.SearchTransportService;
import org.elasticsearch.search.aggregations.AggregationPhase;
import org.elasticsearch.search.aggregations.Aggregator;
import org.elasticsearch.search.aggregations.AggregationBuilder;
import org.elasticsearch.search.aggregations.AggregatorParsers;
@ -236,12 +234,10 @@ import org.elasticsearch.search.aggregations.pipeline.movavg.models.SimpleModel;
import org.elasticsearch.search.aggregations.pipeline.serialdiff.SerialDiffPipelineAggregator;
import org.elasticsearch.search.aggregations.pipeline.serialdiff.SerialDiffPipelineAggregatorBuilder;
import org.elasticsearch.search.controller.SearchPhaseController;
import org.elasticsearch.search.dfs.DfsPhase;
import org.elasticsearch.search.fetch.FetchPhase;
import org.elasticsearch.search.fetch.FetchSubPhase;
import org.elasticsearch.search.fetch.explain.ExplainFetchSubPhase;
import org.elasticsearch.search.fetch.fielddata.FieldDataFieldsFetchSubPhase;
import org.elasticsearch.search.fetch.innerhits.InnerHitsFetchSubPhase;
import org.elasticsearch.search.fetch.matchedqueries.MatchedQueriesFetchSubPhase;
import org.elasticsearch.search.fetch.parent.ParentFieldSubFetchPhase;
import org.elasticsearch.search.fetch.script.ScriptFieldsFetchSubPhase;
@ -249,7 +245,6 @@ import org.elasticsearch.search.fetch.source.FetchSourceSubPhase;
import org.elasticsearch.search.fetch.version.VersionFetchSubPhase;
import org.elasticsearch.search.highlight.HighlightPhase;
import org.elasticsearch.search.highlight.Highlighter;
import org.elasticsearch.search.query.QueryPhase;
import org.elasticsearch.search.rescore.QueryRescorerBuilder;
import org.elasticsearch.search.rescore.RescoreBuilder;
import org.elasticsearch.search.sort.FieldSortBuilder;
@ -261,6 +256,7 @@ import org.elasticsearch.search.suggest.Suggester;
import org.elasticsearch.search.suggest.Suggesters;
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;
/**
@ -281,7 +277,7 @@ public class SearchModule extends AbstractModule {
private final ParseFieldRegistry<MovAvgModel.AbstractModelParser> movingAverageModelParserRegistry = new ParseFieldRegistry<>(
"moving_avg_model");
private final Set<Class<? extends FetchSubPhase>> fetchSubPhases = new HashSet<>();
private final Set<FetchSubPhase> fetchSubPhases = new HashSet<>();
private final Settings settings;
private final NamedWriteableRegistry namedWriteableRegistry;
@ -303,6 +299,7 @@ public class SearchModule extends AbstractModule {
registerBuiltinValueFormats();
registerBuiltinSignificanceHeuristics();
registerBuiltinMovingAverageModels();
registerBuiltinSubFetchPhases();
}
public void registerHighlighter(String key, Highlighter highligher) {
@ -357,8 +354,19 @@ public class SearchModule extends AbstractModule {
return queryParserRegistry;
}
public void registerFetchSubPhase(Class<? extends FetchSubPhase> subPhase) {
fetchSubPhases.add(subPhase);
/**
* Registers a {@link FetchSubPhase} instance. This sub phase is executed when docuemnts are fetched for instanced to highlight
* documents.
*/
public void registerFetchSubPhase(FetchSubPhase subPhase) {
fetchSubPhases.add(Objects.requireNonNull(subPhase, "FetchSubPhase must not be null"));
}
/**
* Returns the {@link Highlighter} registry
*/
public Highlighters getHighlighters() {
return highlighters;
}
/**
@ -437,31 +445,9 @@ public class SearchModule extends AbstractModule {
bind(Suggesters.class).toInstance(suggesters);
configureSearch();
configureAggs();
configureHighlighters();
configureFetchSubPhase();
configureShapes();
}
protected void configureFetchSubPhase() {
Multibinder<FetchSubPhase> fetchSubPhaseMultibinder = Multibinder.newSetBinder(binder(), FetchSubPhase.class);
fetchSubPhaseMultibinder.addBinding().to(ExplainFetchSubPhase.class);
fetchSubPhaseMultibinder.addBinding().to(FieldDataFieldsFetchSubPhase.class);
fetchSubPhaseMultibinder.addBinding().to(ScriptFieldsFetchSubPhase.class);
fetchSubPhaseMultibinder.addBinding().to(FetchSourceSubPhase.class);
fetchSubPhaseMultibinder.addBinding().to(VersionFetchSubPhase.class);
fetchSubPhaseMultibinder.addBinding().to(MatchedQueriesFetchSubPhase.class);
fetchSubPhaseMultibinder.addBinding().to(HighlightPhase.class);
fetchSubPhaseMultibinder.addBinding().to(ParentFieldSubFetchPhase.class);
for (Class<? extends FetchSubPhase> clazz : fetchSubPhases) {
fetchSubPhaseMultibinder.addBinding().to(clazz);
}
bind(InnerHitsFetchSubPhase.class).asEagerSingleton();
}
protected void configureHighlighters() {
binder().bind(Highlighters.class).toInstance(highlighters);
}
protected void configureAggs() {
registerAggregation(AvgAggregationBuilder::new, new AvgParser(), AvgAggregationBuilder.AGGREGATION_NAME_FIELD);
registerAggregation(SumAggregationBuilder::new, new SumParser(), SumAggregationBuilder.AGGREGATION_NAME_FIELD);
@ -541,18 +527,13 @@ public class SearchModule extends AbstractModule {
BucketSelectorPipelineAggregatorBuilder.AGGREGATION_NAME_FIELD);
registerPipelineAggregation(SerialDiffPipelineAggregatorBuilder::new, SerialDiffPipelineAggregatorBuilder::parse,
SerialDiffPipelineAggregatorBuilder.AGGREGATION_NAME_FIELD);
AggregationPhase aggPhase = new AggregationPhase();
bind(AggregatorParsers.class).toInstance(aggregatorParsers);
bind(AggregationPhase.class).toInstance(aggPhase);
}
protected void configureSearch() {
// configure search private classes...
bind(DfsPhase.class).asEagerSingleton();
bind(QueryPhase.class).asEagerSingleton();
bind(SearchPhaseController.class).asEagerSingleton();
bind(FetchPhase.class).asEagerSingleton();
bind(FetchPhase.class).toInstance(new FetchPhase(fetchSubPhases));
bind(SearchTransportService.class).asEagerSingleton();
if (searchServiceImpl == SearchService.class) {
bind(SearchService.class).asEagerSingleton();
@ -623,6 +604,17 @@ public class SearchModule extends AbstractModule {
registerMovingAverageModel(HoltWintersModel.NAME_FIELD, HoltWintersModel::new, HoltWintersModel.PARSER);
}
private void registerBuiltinSubFetchPhases() {
registerFetchSubPhase(new ExplainFetchSubPhase());
registerFetchSubPhase(new FieldDataFieldsFetchSubPhase());
registerFetchSubPhase(new ScriptFieldsFetchSubPhase());
registerFetchSubPhase(new FetchSourceSubPhase());
registerFetchSubPhase(new VersionFetchSubPhase());
registerFetchSubPhase(new MatchedQueriesFetchSubPhase());
registerFetchSubPhase(new HighlightPhase(settings, highlighters));
registerFetchSubPhase(new ParentFieldSubFetchPhase());
}
private void registerBuiltinQueryParsers() {
registerQuery(MatchQueryBuilder::new, MatchQueryBuilder::fromXContent, MatchQueryBuilder.QUERY_NAME_FIELD);
registerQuery(MatchPhraseQueryBuilder::new, MatchPhraseQueryBuilder::fromXContent, MatchPhraseQueryBuilder.QUERY_NAME_FIELD);

View File

@ -134,7 +134,7 @@ public class SearchService extends AbstractLifecycleComponent<SearchService> imp
private final BigArrays bigArrays;
private final DfsPhase dfsPhase;
private final DfsPhase dfsPhase = new DfsPhase();
private final QueryPhase queryPhase;
@ -158,8 +158,8 @@ public class SearchService extends AbstractLifecycleComponent<SearchService> imp
@Inject
public SearchService(Settings settings, ClusterSettings clusterSettings, ClusterService clusterService, IndicesService indicesService,
ThreadPool threadPool, ScriptService scriptService, BigArrays bigArrays, DfsPhase dfsPhase,
QueryPhase queryPhase, FetchPhase fetchPhase, AggregatorParsers aggParsers, Suggesters suggesters) {
ThreadPool threadPool, ScriptService scriptService, BigArrays bigArrays,
FetchPhase fetchPhase, AggregatorParsers aggParsers, Suggesters suggesters) {
super(settings);
this.aggParsers = aggParsers;
this.suggesters = suggesters;
@ -169,8 +169,7 @@ public class SearchService extends AbstractLifecycleComponent<SearchService> imp
this.indicesService = indicesService;
this.scriptService = scriptService;
this.bigArrays = bigArrays;
this.dfsPhase = dfsPhase;
this.queryPhase = queryPhase;
this.queryPhase = new QueryPhase(settings);
this.fetchPhase = fetchPhase;
TimeValue keepAliveInterval = KEEPALIVE_INTERVAL_SETTING.get(settings);
@ -257,8 +256,7 @@ public class SearchService extends AbstractLifecycleComponent<SearchService> imp
/**
* Try to load the query results from the cache or execute the query phase directly if the cache cannot be used.
*/
private void loadOrExecuteQueryPhase(final ShardSearchRequest request, final SearchContext context,
final QueryPhase queryPhase) throws Exception {
private void loadOrExecuteQueryPhase(final ShardSearchRequest request, final SearchContext context) throws Exception {
final boolean canCache = indicesService.canCache(request, context);
if (canCache) {
indicesService.loadIntoContext(request, context, queryPhase);
@ -275,7 +273,7 @@ public class SearchService extends AbstractLifecycleComponent<SearchService> imp
long time = System.nanoTime();
contextProcessing(context);
loadOrExecuteQueryPhase(request, context, queryPhase);
loadOrExecuteQueryPhase(request, context);
if (context.queryResult().topDocs().scoreDocs.length == 0 && context.scrollContext() == null) {
freeContext(context.id());
@ -367,7 +365,7 @@ public class SearchService extends AbstractLifecycleComponent<SearchService> imp
operationListener.onPreQueryPhase(context);
long time = System.nanoTime();
try {
loadOrExecuteQueryPhase(request, context, queryPhase);
loadOrExecuteQueryPhase(request, context);
} catch (Throwable e) {
operationListener.onFailedQueryPhase(context);
throw ExceptionsHelper.convertToRuntime(e);

View File

@ -29,7 +29,6 @@ import org.apache.lucene.util.BitSet;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.text.Text;
@ -73,11 +72,9 @@ public class FetchPhase implements SearchPhase {
private final FetchSubPhase[] fetchSubPhases;
@Inject
public FetchPhase(Set<FetchSubPhase> fetchSubPhases, InnerHitsFetchSubPhase innerHitsFetchSubPhase) {
innerHitsFetchSubPhase.setFetchPhase(this);
public FetchPhase(Set<FetchSubPhase> fetchSubPhases) {
this.fetchSubPhases = fetchSubPhases.toArray(new FetchSubPhase[fetchSubPhases.size() + 1]);
this.fetchSubPhases[fetchSubPhases.size()] = innerHitsFetchSubPhase;
this.fetchSubPhases[fetchSubPhases.size()] = new InnerHitsFetchSubPhase(this);
}
@Override

View File

@ -23,7 +23,6 @@ import org.apache.lucene.search.FieldDoc;
import org.apache.lucene.search.ScoreDoc;
import org.apache.lucene.search.TopDocs;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.search.SearchParseElement;
import org.elasticsearch.search.fetch.FetchPhase;
import org.elasticsearch.search.fetch.FetchSearchResult;
@ -41,10 +40,10 @@ import java.util.Map;
*/
public class InnerHitsFetchSubPhase implements FetchSubPhase {
private FetchPhase fetchPhase;
private final FetchPhase fetchPhase;
@Inject
public InnerHitsFetchSubPhase() {
public InnerHitsFetchSubPhase(FetchPhase fetchPhase) {
this.fetchPhase = fetchPhase;
}
@Override
@ -102,9 +101,4 @@ public class InnerHitsFetchSubPhase implements FetchSubPhase {
@Override
public void hitsExecute(SearchContext context, InternalSearchHit[] hits) {
}
// To get around cyclic dependency issue
public void setFetchPhase(FetchPhase fetchPhase) {
this.fetchPhase = fetchPhase;
}
}

View File

@ -42,10 +42,10 @@ import org.apache.lucene.search.TopScoreDocCollector;
import org.apache.lucene.search.TotalHitCountCollector;
import org.apache.lucene.search.Weight;
import org.elasticsearch.action.search.SearchType;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.lucene.Lucene;
import org.elasticsearch.common.lucene.MinimumScoreCollector;
import org.elasticsearch.common.lucene.search.FilteredCollector;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.SearchPhase;
import org.elasticsearch.search.SearchService;
@ -76,11 +76,10 @@ public class QueryPhase implements SearchPhase {
private final SuggestPhase suggestPhase;
private RescorePhase rescorePhase;
@Inject
public QueryPhase(AggregationPhase aggregationPhase, SuggestPhase suggestPhase, RescorePhase rescorePhase) {
this.aggregationPhase = aggregationPhase;
this.suggestPhase = suggestPhase;
this.rescorePhase = rescorePhase;
public QueryPhase(Settings settings) {
this.aggregationPhase = new AggregationPhase();
this.suggestPhase = new SuggestPhase(settings);
this.rescorePhase = new RescorePhase(settings);
}
@Override

View File

@ -22,7 +22,6 @@ package org.elasticsearch.search.rescore;
import org.apache.lucene.search.TopDocs;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.search.SearchPhase;
import org.elasticsearch.search.internal.SearchContext;
@ -33,7 +32,6 @@ import java.io.IOException;
*/
public class RescorePhase extends AbstractComponent implements SearchPhase {
@Inject
public RescorePhase(Settings settings) {
super(settings);
}

View File

@ -21,7 +21,6 @@ package org.elasticsearch.search.suggest;
import org.apache.lucene.util.CharsRefBuilder;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.search.SearchPhase;
import org.elasticsearch.search.internal.SearchContext;
@ -39,7 +38,6 @@ import java.util.Map;
*/
public class SuggestPhase extends AbstractComponent implements SearchPhase {
@Inject
public SuggestPhase(Settings settings) {
super(settings);
}

View File

@ -112,7 +112,7 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase {
}
public void onModule(SearchModule searchModule) {
searchModule.registerFetchSubPhase(TermVectorsFetchSubPhase.class);
searchModule.registerFetchSubPhase(new TermVectorsFetchSubPhase());
}
}

View File

@ -28,9 +28,10 @@ import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.Query;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.text.Text;
import org.elasticsearch.index.query.ParsedQuery;
import org.elasticsearch.search.Highlighters;
import org.elasticsearch.search.SearchParseElement;
import org.elasticsearch.search.fetch.FetchSubPhase;
import org.elasticsearch.search.highlight.HighlightPhase;
@ -46,13 +47,10 @@ import java.util.Map;
// Highlighting in the case of the percolate query is a bit different, because the PercolateQuery itself doesn't get highlighted,
// but the source of the PercolateQuery gets highlighted by each hit containing a query.
public class PercolatorHighlightSubFetchPhase implements FetchSubPhase {
public class PercolatorHighlightSubFetchPhase extends HighlightPhase {
private final HighlightPhase highlightPhase;
@Inject
public PercolatorHighlightSubFetchPhase(HighlightPhase highlightPhase) {
this.highlightPhase = highlightPhase;
public PercolatorHighlightSubFetchPhase(Settings settings, Highlighters highlighters) {
super(settings, highlighters);
}
@Override
@ -93,7 +91,7 @@ public class PercolatorHighlightSubFetchPhase implements FetchSubPhase {
percolatorLeafReaderContext, 0, percolatorIndexSearcher
);
hitContext.cache().clear();
highlightPhase.hitExecute(subSearchContext, hitContext);
super.hitExecute(subSearchContext, hitContext);
hit.highlightFields().putAll(hitContext.hit().getHighlightFields());
}
}

View File

@ -28,15 +28,21 @@ import org.elasticsearch.common.settings.SettingsModule;
import org.elasticsearch.indices.IndicesModule;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.search.SearchModule;
import org.elasticsearch.search.fetch.FetchSubPhase;
import org.elasticsearch.search.highlight.HighlightPhase;
import java.util.Optional;
public class PercolatorPlugin extends Plugin {
public static final String NAME = "percolator";
private final boolean transportClientMode;
private final Settings settings;
public PercolatorPlugin(Settings settings) {
this.transportClientMode = transportClientMode(settings);
this.settings = settings;
}
@Override
@ -67,7 +73,7 @@ public class PercolatorPlugin extends Plugin {
public void onModule(SearchModule module) {
module.registerQuery(PercolateQueryBuilder::new, PercolateQueryBuilder::fromXContent, PercolateQueryBuilder.QUERY_NAME_FIELD);
module.registerFetchSubPhase(PercolatorHighlightSubFetchPhase.class);
module.registerFetchSubPhase(new PercolatorHighlightSubFetchPhase(settings, module.getHighlighters()));
}
public void onModule(SettingsModule module) {

View File

@ -25,6 +25,8 @@ import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.MatchAllDocsQuery;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.search.Highlighters;
import org.elasticsearch.search.highlight.SearchContextHighlight;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.test.ESTestCase;
@ -44,7 +46,8 @@ public class PercolatorHighlightSubFetchPhaseTests extends ESTestCase {
Mockito.mock(IndexSearcher.class))
.build();
PercolatorHighlightSubFetchPhase subFetchPhase = new PercolatorHighlightSubFetchPhase(null);
PercolatorHighlightSubFetchPhase subFetchPhase = new PercolatorHighlightSubFetchPhase(Settings.EMPTY,
new Highlighters(Settings.EMPTY));
SearchContext searchContext = Mockito.mock(SearchContext.class);
Mockito.when(searchContext.highlight()).thenReturn(new SearchContextHighlight(Collections.emptyList()));
Mockito.when(searchContext.query()).thenReturn(new MatchAllDocsQuery());

View File

@ -28,10 +28,8 @@ import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.search.aggregations.AggregatorParsers;
import org.elasticsearch.search.dfs.DfsPhase;
import org.elasticsearch.search.fetch.FetchPhase;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.search.query.QueryPhase;
import org.elasticsearch.search.suggest.Suggesters;
import org.elasticsearch.threadpool.ThreadPool;
@ -84,10 +82,9 @@ public class MockSearchService extends SearchService {
@Inject
public MockSearchService(Settings settings, ClusterSettings clusterSettings, ClusterService clusterService,
IndicesService indicesService, ThreadPool threadPool, ScriptService scriptService,
BigArrays bigArrays, DfsPhase dfsPhase, QueryPhase queryPhase, FetchPhase fetchPhase,
AggregatorParsers aggParsers, Suggesters suggesters) {
super(settings, clusterSettings, clusterService, indicesService, threadPool, scriptService, bigArrays, dfsPhase,
queryPhase, fetchPhase, aggParsers, suggesters);
BigArrays bigArrays, FetchPhase fetchPhase, AggregatorParsers aggParsers, Suggesters suggesters) {
super(settings, clusterSettings, clusterService, indicesService, threadPool, scriptService, bigArrays,
fetchPhase, aggParsers, suggesters);
}
@Override