diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml index 2095d3d3790..10cb3197a84 100644 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ b/buildSrc/src/main/resources/checkstyle_suppressions.xml @@ -556,7 +556,6 @@ - @@ -936,7 +935,6 @@ - @@ -1068,7 +1066,6 @@ - diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index 95c2d0aab71..8ecc997dbb7 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -63,6 +63,8 @@ import org.elasticsearch.search.dfs.DfsPhase; import org.elasticsearch.search.dfs.DfsSearchResult; import org.elasticsearch.search.fetch.FetchPhase; import org.elasticsearch.search.fetch.FetchSearchResult; +import org.elasticsearch.search.fetch.FetchSubPhaseContext; +import org.elasticsearch.search.fetch.FetchSubPhaseParser; import org.elasticsearch.search.fetch.QueryFetchSearchResult; import org.elasticsearch.search.fetch.ScrollQueryFetchSearchResult; import org.elasticsearch.search.fetch.ShardFetchRequest; @@ -143,7 +145,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv private final ConcurrentMapLong activeContexts = ConcurrentCollections.newConcurrentMapLongWithAggressiveConcurrency(); - private final Map elementParsers; + private final Map fetchPhaseParsers; private final ParseFieldMatcher parseFieldMatcher; @@ -163,7 +165,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv TimeValue keepAliveInterval = KEEPALIVE_INTERVAL_SETTING.get(settings); this.defaultKeepAlive = DEFAULT_KEEPALIVE_SETTING.get(settings).millis(); - this.elementParsers = unmodifiableMap(fetchPhase.parseElements()); + this.fetchPhaseParsers = unmodifiableMap(fetchPhase.parsers()); this.keepAliveReaper = threadPool.scheduleWithFixedDelay(new Reaper(), keepAliveInterval, Names.SAME); @@ -762,8 +764,8 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = extParser.currentName(); } else { - SearchParseElement parseElement = this.elementParsers.get(currentFieldName); - if (parseElement == null) { + FetchSubPhaseParser fetchSubPhaseParser = this.fetchPhaseParsers.get(currentFieldName); + if (fetchSubPhaseParser == null) { if (currentFieldName != null && currentFieldName.equals("suggest")) { throw new SearchParseException(context, "suggest is not supported in [ext], please use SearchSourceBuilder#suggest(SuggestBuilder) instead", @@ -772,7 +774,8 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv throw new SearchParseException(context, "Unknown element [" + currentFieldName + "] in [ext]", extParser.getTokenLocation()); } else { - parseElement.parse(extParser, context); + FetchSubPhaseContext fetchSubPhaseContext = fetchSubPhaseParser.parse(extParser); + context.putFetchSubPhaseContext(currentFieldName, fetchSubPhaseContext); } } } diff --git a/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java index 18c313b409c..1649c0f449c 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java @@ -43,7 +43,6 @@ import org.elasticsearch.index.mapper.ObjectMapper; import org.elasticsearch.index.mapper.SourceFieldMapper; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.SearchHitField; -import org.elasticsearch.search.SearchParseElement; import org.elasticsearch.search.SearchPhase; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.InnerHitsFetchSubPhase; @@ -78,12 +77,12 @@ public class FetchPhase implements SearchPhase { this.fetchSubPhases[fetchSubPhases.size()] = new InnerHitsFetchSubPhase(this); } - public Map parseElements() { - Map parseElements = new HashMap<>(); + public Map parsers() { + Map parsers = new HashMap<>(); for (FetchSubPhase fetchSubPhase : fetchSubPhases) { - parseElements.putAll(fetchSubPhase.parseElements()); + parsers.putAll(fetchSubPhase.parsers()); } - return unmodifiableMap(parseElements); + return unmodifiableMap(parsers); } @Override diff --git a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java index 145dbd1053f..afe67c5d78d 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java @@ -22,7 +22,6 @@ import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.LeafReader; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.IndexSearcher; -import org.elasticsearch.search.SearchParseElement; import org.elasticsearch.search.internal.InternalSearchHit; import org.elasticsearch.search.internal.SearchContext; @@ -78,7 +77,7 @@ public interface FetchSubPhase { } - default Map parseElements() { + default Map parsers() { return Collections.emptyMap(); } @@ -89,23 +88,4 @@ public interface FetchSubPhase { default void hitsExecute(SearchContext context, InternalSearchHit[] hits) {} - - /** - * This interface is in the fetch phase plugin mechanism. - * Whenever a new search is executed we create a new {@link SearchContext} that holds individual contexts for each {@link org.elasticsearch.search.fetch.FetchSubPhase}. - * Fetch phases that use the plugin mechanism must provide a ContextFactory to the SearchContext that creates the fetch phase context and also associates them with a name. - * See {@link SearchContext#getFetchSubPhaseContext(FetchSubPhase.ContextFactory)} - */ - interface ContextFactory { - - /** - * The name of the context. - */ - String getName(); - - /** - * Creates a new instance of a FetchSubPhaseContext that holds all information a FetchSubPhase needs to execute on hits. - */ - SubPhaseContext newContextInstance(); - } } diff --git a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseContext.java b/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseContext.java index 856c0ad902f..f2aa1b88138 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseContext.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseContext.java @@ -19,15 +19,12 @@ package org.elasticsearch.search.fetch; -import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; - /** * All configuration and context needed by the FetchSubPhase to execute on hits. * The only required information in this base class is whether or not the sub phase needs to be run at all. * It can be extended by FetchSubPhases to hold information the phase needs to execute on hits. - * See {@link org.elasticsearch.search.fetch.FetchSubPhase.ContextFactory} and also {@link DocValueFieldsContext} for an example. */ -public class FetchSubPhaseContext { +public abstract class FetchSubPhaseContext { // This is to store if the FetchSubPhase should be executed at all. private boolean hitExecutionNeeded = false; diff --git a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseParser.java b/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseParser.java new file mode 100644 index 00000000000..c88b1995688 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseParser.java @@ -0,0 +1,30 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.search.fetch; + +import org.elasticsearch.common.xcontent.XContentParser; + +/** + * Parser for the ext section of a search request, which can hold custom fetch sub phase + */ +public interface FetchSubPhaseParser { + + Context parse(XContentParser parser) throws Exception; +} diff --git a/core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java b/core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java index 39e0aa17332..bf968e8f0cd 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java @@ -50,7 +50,6 @@ import org.elasticsearch.index.mapper.TypeFieldMapper; import org.elasticsearch.index.query.AbstractQueryBuilder; import org.elasticsearch.index.query.ParsedQuery; import org.elasticsearch.index.query.QueryShardContext; -import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.similarity.SimilarityService; import org.elasticsearch.script.ScriptService; @@ -59,8 +58,8 @@ import org.elasticsearch.search.aggregations.SearchContextAggregations; import org.elasticsearch.search.dfs.DfsSearchResult; import org.elasticsearch.search.fetch.FetchPhase; import org.elasticsearch.search.fetch.FetchSearchResult; -import org.elasticsearch.search.fetch.FetchSubPhase; import org.elasticsearch.search.fetch.FetchSubPhaseContext; +import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.ScriptFieldsContext; @@ -81,9 +80,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -/** - * - */ public class DefaultSearchContext extends SearchContext { private final long id; @@ -390,15 +386,15 @@ public class DefaultSearchContext extends SearchContext { } @Override - public SubPhaseContext getFetchSubPhaseContext( - FetchSubPhase.ContextFactory contextFactory) { - String subPhaseName = contextFactory.getName(); - if (subPhaseContexts.get(subPhaseName) == null) { - subPhaseContexts.put(subPhaseName, contextFactory.newContextInstance()); - } - return (SubPhaseContext) subPhaseContexts.get(subPhaseName); + @SuppressWarnings("unchecked") + public SubPhaseContext getFetchSubPhaseContext(String name) { + return (SubPhaseContext) subPhaseContexts.get(name); } + @Override + public void putFetchSubPhaseContext(String name, SubPhaseContext subPhaseContext) { + subPhaseContexts.put(name, subPhaseContext); + } @Override public SearchContextHighlight highlight() { diff --git a/core/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java b/core/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java index 7959e78b98f..250721ba27d 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java @@ -35,7 +35,6 @@ import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.ObjectMapper; import org.elasticsearch.index.query.ParsedQuery; import org.elasticsearch.index.query.QueryShardContext; -import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.similarity.SimilarityService; import org.elasticsearch.script.ScriptService; @@ -44,8 +43,8 @@ import org.elasticsearch.search.aggregations.SearchContextAggregations; import org.elasticsearch.search.dfs.DfsSearchResult; import org.elasticsearch.search.fetch.FetchPhase; import org.elasticsearch.search.fetch.FetchSearchResult; -import org.elasticsearch.search.fetch.FetchSubPhase; import org.elasticsearch.search.fetch.FetchSubPhaseContext; +import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.InnerHitsContext; import org.elasticsearch.search.fetch.subphase.ScriptFieldsContext; @@ -512,9 +511,13 @@ public abstract class FilteredSearchContext extends SearchContext { } @Override - public SubPhaseContext getFetchSubPhaseContext( - FetchSubPhase.ContextFactory contextFactory) { - return in.getFetchSubPhaseContext(contextFactory); + public SubPhaseContext getFetchSubPhaseContext(String name) { + return in.getFetchSubPhaseContext(name); + } + + @Override + public void putFetchSubPhaseContext(String name, SubPhaseContext subPhaseContext) { + in.putFetchSubPhaseContext(name, subPhaseContext); } @Override diff --git a/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java b/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java index dfa35e8724e..d92fc79dd32 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java @@ -49,7 +49,6 @@ import org.elasticsearch.search.aggregations.SearchContextAggregations; import org.elasticsearch.search.dfs.DfsSearchResult; import org.elasticsearch.search.fetch.FetchPhase; import org.elasticsearch.search.fetch.FetchSearchResult; -import org.elasticsearch.search.fetch.FetchSubPhase; import org.elasticsearch.search.fetch.FetchSubPhaseContext; import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; @@ -186,8 +185,10 @@ public abstract class SearchContext extends AbstractRefCounted implements Releas public abstract SearchContext aggregations(SearchContextAggregations aggregations); - public abstract SubPhaseContext getFetchSubPhaseContext( - FetchSubPhase.ContextFactory contextFactory); + public abstract void putFetchSubPhaseContext(String name, + SubPhaseContext subPhaseContext); + + public abstract SubPhaseContext getFetchSubPhaseContext(String name); public abstract SearchContextHighlight highlight(); diff --git a/core/src/main/java/org/elasticsearch/search/internal/SubSearchContext.java b/core/src/main/java/org/elasticsearch/search/internal/SubSearchContext.java index 8ba76971de9..dc385a0e120 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/SubSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/SubSearchContext.java @@ -37,8 +37,6 @@ import org.elasticsearch.search.suggest.SuggestionSearchContext; import java.util.List; -/** - */ public class SubSearchContext extends FilteredSearchContext { // By default return 3 hits per bucket. A higher default would make the response really large by default, since diff --git a/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java b/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java index a7585864be9..7f4b0911941 100644 --- a/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java +++ b/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java @@ -33,7 +33,6 @@ import org.elasticsearch.index.termvectors.TermVectorsService; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.SearchPlugin; import org.elasticsearch.search.SearchHitField; -import org.elasticsearch.search.SearchParseElement; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.internal.InternalSearchHitField; import org.elasticsearch.search.internal.SearchContext; @@ -106,42 +105,32 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { public static final class TermVectorsFetchSubPhase implements FetchSubPhase { - public static final ContextFactory CONTEXT_FACTORY = new ContextFactory() { - - @Override - public String getName() { - return NAMES[0]; - } - - @Override - public TermVectorsFetchContext newContextInstance() { - return new TermVectorsFetchContext(); - } - }; - - public static final String[] NAMES = {"term_vectors_fetch"}; + public static final String NAME = "term_vectors_fetch"; @Override - public Map parseElements() { - return singletonMap("term_vectors_fetch", new TermVectorsFetchParseElement()); + public Map parsers() { + return singletonMap("term_vectors_fetch", new TermVectorsFetchParser()); } @Override public void hitExecute(SearchContext context, HitContext hitContext) { - if (context.getFetchSubPhaseContext(CONTEXT_FACTORY).hitExecutionNeeded() == false) { + TermVectorsFetchContext fetchSubPhaseContext = context.getFetchSubPhaseContext(NAME); + if (fetchSubPhaseContext.hitExecutionNeeded() == false) { return; } - String field = context.getFetchSubPhaseContext(CONTEXT_FACTORY).getField(); + String field = fetchSubPhaseContext.getField(); if (hitContext.hit().fieldsOrNull() == null) { hitContext.hit().fields(new HashMap<>()); } - SearchHitField hitField = hitContext.hit().fields().get(NAMES[0]); + SearchHitField hitField = hitContext.hit().fields().get(NAME); if (hitField == null) { - hitField = new InternalSearchHitField(NAMES[0], new ArrayList<>(1)); - hitContext.hit().fields().put(NAMES[0], hitField); + hitField = new InternalSearchHitField(NAME, new ArrayList<>(1)); + hitContext.hit().fields().put(NAME, hitField); } - TermVectorsResponse termVector = TermVectorsService.getTermVectors(context.indexShard(), new TermVectorsRequest(context.indexShard().shardId().getIndex().getName(), hitContext.hit().type(), hitContext.hit().id())); + TermVectorsRequest termVectorsRequest = new TermVectorsRequest(context.indexShard().shardId().getIndex().getName(), + hitContext.hit().type(), hitContext.hit().id()); + TermVectorsResponse termVector = TermVectorsService.getTermVectors(context.indexShard(), termVectorsRequest); try { Map tv = new HashMap<>(); TermsEnum terms = termVector.getFields().terms(field).iterator(); @@ -156,20 +145,21 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { } } - public static class TermVectorsFetchParseElement implements SearchParseElement { + public static class TermVectorsFetchParser implements FetchSubPhaseParser { @Override - public void parse(XContentParser parser, SearchContext context) throws Exception { - TermVectorsFetchContext fetchSubPhaseContext = context.getFetchSubPhaseContext(TermVectorsFetchSubPhase.CONTEXT_FACTORY); + public TermVectorsFetchContext parse(XContentParser parser) throws Exception { // this is to make sure that the SubFetchPhase knows it should execute - fetchSubPhaseContext.setHitExecutionNeeded(true); + TermVectorsFetchContext termVectorsFetchContext = new TermVectorsFetchContext(); + termVectorsFetchContext.setHitExecutionNeeded(true); XContentParser.Token token = parser.currentToken(); if (token == XContentParser.Token.VALUE_STRING) { String fieldName = parser.text(); - fetchSubPhaseContext.setField(fieldName); + termVectorsFetchContext.setField(fieldName); } else { throw new IllegalStateException("Expected a VALUE_STRING but got " + token); } + return termVectorsFetchContext; } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java b/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java index 7b05de164f0..67c3ba5b55f 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java +++ b/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java @@ -36,7 +36,6 @@ import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.ObjectMapper; import org.elasticsearch.index.query.ParsedQuery; import org.elasticsearch.index.query.QueryShardContext; -import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.similarity.SimilarityService; import org.elasticsearch.script.ScriptService; @@ -45,8 +44,8 @@ import org.elasticsearch.search.aggregations.SearchContextAggregations; import org.elasticsearch.search.dfs.DfsSearchResult; import org.elasticsearch.search.fetch.FetchPhase; import org.elasticsearch.search.fetch.FetchSearchResult; -import org.elasticsearch.search.fetch.FetchSubPhase; import org.elasticsearch.search.fetch.FetchSubPhaseContext; +import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.ScriptFieldsContext; @@ -198,12 +197,14 @@ public class TestSearchContext extends SearchContext { } @Override - public SubPhaseContext getFetchSubPhaseContext(FetchSubPhase.ContextFactory contextFactory) { - String subPhaseName = contextFactory.getName(); - if (subPhaseContexts.get(subPhaseName) == null) { - subPhaseContexts.put(subPhaseName, contextFactory.newContextInstance()); - } - return (SubPhaseContext) subPhaseContexts.get(subPhaseName); + @SuppressWarnings("unchecked") + public SubPhaseContext getFetchSubPhaseContext(String name) { + return (SubPhaseContext) subPhaseContexts.get(name); + } + + @Override + public void putFetchSubPhaseContext(String name, SubPhaseContext subPhaseContext) { + subPhaseContexts.put(name, subPhaseContext); } @Override