From fe6b9d62a5c5f9dbda772ff7a93b3c7584919c1b Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 7 Sep 2016 15:14:07 +0200 Subject: [PATCH 01/13] Remove parseElements method from SearchPhase interface Parse elements are always empty for all of our search phases. They can be non empty only for sub fetch phases as they are pluggable and search parse element is left to be used only for plugins that plug in their own sub fetch phase. Only FetchPhase needs now the parseElements method then. --- .../java/org/elasticsearch/search/SearchPhase.java | 12 ++++-------- .../java/org/elasticsearch/search/SearchService.java | 6 +----- .../org/elasticsearch/search/fetch/FetchPhase.java | 3 +-- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/SearchPhase.java b/core/src/main/java/org/elasticsearch/search/SearchPhase.java index 48c041f12f5..33260706b3c 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchPhase.java +++ b/core/src/main/java/org/elasticsearch/search/SearchPhase.java @@ -21,22 +21,18 @@ package org.elasticsearch.search; import org.elasticsearch.search.internal.SearchContext; -import java.util.Collections; -import java.util.Map; - /** - * + * Represents a phase of a search request e.g. query, fetch etc. */ public interface SearchPhase { - default Map parseElements() { - return Collections.emptyMap(); - } - /** * Performs pre processing of the search context before the execute. */ void preProcess(SearchContext context); + /** + * Executes the search phase + */ void execute(SearchContext context); } diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index ba39c55a7a2..daebe480e04 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -165,11 +165,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv TimeValue keepAliveInterval = KEEPALIVE_INTERVAL_SETTING.get(settings); this.defaultKeepAlive = DEFAULT_KEEPALIVE_SETTING.get(settings).millis(); - Map elementParsers = new HashMap<>(); - elementParsers.putAll(dfsPhase.parseElements()); - elementParsers.putAll(queryPhase.parseElements()); - elementParsers.putAll(fetchPhase.parseElements()); - this.elementParsers = unmodifiableMap(elementParsers); + this.elementParsers = unmodifiableMap(fetchPhase.parseElements()); this.keepAliveReaper = threadPool.scheduleWithFixedDelay(new Reaper(), keepAliveInterval, Names.SAME); 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 997cb7caa4b..e10d7828a8a 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java @@ -66,7 +66,7 @@ import static java.util.Collections.unmodifiableMap; import static org.elasticsearch.common.xcontent.XContentFactory.contentBuilder; /** - * + * Fetch phase of a search request */ public class FetchPhase implements SearchPhase { @@ -77,7 +77,6 @@ public class FetchPhase implements SearchPhase { this.fetchSubPhases[fetchSubPhases.size()] = new InnerHitsFetchSubPhase(this); } - @Override public Map parseElements() { Map parseElements = new HashMap<>(); for (FetchSubPhase fetchSubPhase : fetchSubPhases) { From 319280bde3f576e520448514fc1c576f9cb6e654 Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 7 Sep 2016 16:00:20 +0200 Subject: [PATCH 02/13] add java docs to all of the SearchPhase implementations --- .../elasticsearch/search/aggregations/AggregationPhase.java | 2 +- core/src/main/java/org/elasticsearch/search/dfs/DfsPhase.java | 3 ++- .../main/java/org/elasticsearch/search/fetch/FetchPhase.java | 3 ++- .../main/java/org/elasticsearch/search/query/QueryPhase.java | 3 ++- .../java/org/elasticsearch/search/rescore/RescorePhase.java | 1 + .../java/org/elasticsearch/search/suggest/SuggestPhase.java | 1 + 6 files changed, 9 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java b/core/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java index 5dc29374310..8acd4f13752 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/AggregationPhase.java @@ -40,7 +40,7 @@ import java.util.Collections; import java.util.List; /** - * + * Aggregation phase of a search request, used to collect aggregations */ public class AggregationPhase implements SearchPhase { diff --git a/core/src/main/java/org/elasticsearch/search/dfs/DfsPhase.java b/core/src/main/java/org/elasticsearch/search/dfs/DfsPhase.java index de06655f414..1359be24a15 100644 --- a/core/src/main/java/org/elasticsearch/search/dfs/DfsPhase.java +++ b/core/src/main/java/org/elasticsearch/search/dfs/DfsPhase.java @@ -37,7 +37,8 @@ import java.util.Collection; import java.util.Iterator; /** - * + * Dfs phase of a search request, used to make scoring 100% accurate by collecting additional info from each shard before the query phase. + * The additional information is used to better compare the scores coming from all the shards, which depend on local factors (e.g. idf) */ public class DfsPhase implements SearchPhase { 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 e10d7828a8a..18c313b409c 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java @@ -66,7 +66,8 @@ import static java.util.Collections.unmodifiableMap; import static org.elasticsearch.common.xcontent.XContentFactory.contentBuilder; /** - * Fetch phase of a search request + * Fetch phase of a search request, used to fetch the actual top matching documents to be returned to the client, identified + * after reducing all of the matches returned by the query phase */ public class FetchPhase implements SearchPhase { diff --git a/core/src/main/java/org/elasticsearch/search/query/QueryPhase.java b/core/src/main/java/org/elasticsearch/search/query/QueryPhase.java index 189fead7813..47fa98856cf 100644 --- a/core/src/main/java/org/elasticsearch/search/query/QueryPhase.java +++ b/core/src/main/java/org/elasticsearch/search/query/QueryPhase.java @@ -68,7 +68,8 @@ import java.util.List; import java.util.concurrent.Callable; /** - * + * Query phase of a search request, used to run the query and get back from each shard information about the matching documents + * (document ids and score or sort criteria) so that matches can be reduced on the coordinating node */ public class QueryPhase implements SearchPhase { diff --git a/core/src/main/java/org/elasticsearch/search/rescore/RescorePhase.java b/core/src/main/java/org/elasticsearch/search/rescore/RescorePhase.java index 395db4cdcd8..d3d4c75cd7b 100644 --- a/core/src/main/java/org/elasticsearch/search/rescore/RescorePhase.java +++ b/core/src/main/java/org/elasticsearch/search/rescore/RescorePhase.java @@ -29,6 +29,7 @@ import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; /** + * Rescore phase of a search request, used to run potentially expensive scoring models against the top matching documents. */ public class RescorePhase extends AbstractComponent implements SearchPhase { diff --git a/core/src/main/java/org/elasticsearch/search/suggest/SuggestPhase.java b/core/src/main/java/org/elasticsearch/search/suggest/SuggestPhase.java index c0567e59e8c..874448b924c 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/SuggestPhase.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/SuggestPhase.java @@ -35,6 +35,7 @@ import java.util.List; import java.util.Map; /** + * Suggest phase of a search request, used to collect suggestions */ public class SuggestPhase extends AbstractComponent implements SearchPhase { From 060e732f5043cf116755881c4b5e2fdcb0535b83 Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 7 Sep 2016 16:01:22 +0200 Subject: [PATCH 03/13] remove unused topLevelSearcher method from FetchSubPhase.HitContext --- .../java/org/elasticsearch/search/fetch/FetchSubPhase.java | 4 ---- 1 file changed, 4 deletions(-) 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 2180cc34128..145dbd1053f 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java @@ -69,10 +69,6 @@ public interface FetchSubPhase { return searcher.getIndexReader(); } - public IndexSearcher topLevelSearcher() { - return searcher; - } - public Map cache() { if (cache == null) { cache = new HashMap<>(); From a33ca70ff530fe65e283ca6b215d03c6f37c470e Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 7 Sep 2016 17:47:08 +0200 Subject: [PATCH 04/13] make docValueFields similar to other standard sub fetch phases Given that doc value fields is our own fetch sub phase, it doesn't need to be implemented like if it was plugged in from the outside. It doesn't need its own fetch sub phase context, but it can just be an instance member in SearchContext --- .../index/query/InnerHitBuilder.java | 8 +---- .../elasticsearch/search/SearchService.java | 8 +---- .../tophits/TopHitsAggregatorFactory.java | 11 ++----- .../fetch/subphase/DocValueFieldsContext.java | 31 +++++-------------- .../subphase/DocValueFieldsFetchSubPhase.java | 26 ++++------------ .../search/internal/DefaultSearchContext.java | 13 ++++++++ .../search/internal/SearchContext.java | 9 ++++-- .../search/internal/SubSearchContext.java | 13 ++++++++ .../elasticsearch/test/TestSearchContext.java | 11 +++++++ 9 files changed, 61 insertions(+), 69 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java b/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java index 27b0138e1e5..4516dfde698 100644 --- a/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java @@ -36,7 +36,6 @@ import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.builder.SearchSourceBuilder.ScriptField; import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; -import org.elasticsearch.search.fetch.subphase.DocValueFieldsFetchSubPhase; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.InnerHitsContext; import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder; @@ -572,12 +571,7 @@ public final class InnerHitBuilder extends ToXContentToBytes implements Writeabl innerHitsContext.storedFieldsContext(storedFieldsContext); } if (docValueFields != null) { - DocValueFieldsContext docValueFieldsContext = innerHitsContext - .getFetchSubPhaseContext(DocValueFieldsFetchSubPhase.CONTEXT_FACTORY); - for (String field : docValueFields) { - docValueFieldsContext.add(new DocValueFieldsContext.DocValueField(field)); - } - docValueFieldsContext.setHitExecutionNeeded(true); + innerHitsContext.docValueFieldsContext(new DocValueFieldsContext(docValueFields)); } if (scriptFields != null) { for (ScriptField field : scriptFields) { diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index daebe480e04..223595cc13b 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -67,8 +67,6 @@ import org.elasticsearch.search.fetch.QueryFetchSearchResult; import org.elasticsearch.search.fetch.ScrollQueryFetchSearchResult; import org.elasticsearch.search.fetch.ShardFetchRequest; import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; -import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext.DocValueField; -import org.elasticsearch.search.fetch.subphase.DocValueFieldsFetchSubPhase; import org.elasticsearch.search.fetch.subphase.ScriptFieldsContext.ScriptField; import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder; import org.elasticsearch.search.internal.DefaultSearchContext; @@ -732,11 +730,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv context.fetchSourceContext(source.fetchSource()); } if (source.docValueFields() != null) { - DocValueFieldsContext docValuesFieldsContext = context.getFetchSubPhaseContext(DocValueFieldsFetchSubPhase.CONTEXT_FACTORY); - for (String field : source.docValueFields()) { - docValuesFieldsContext.add(new DocValueField(field)); - } - docValuesFieldsContext.setHitExecutionNeeded(true); + context.docValueFieldsContext(new DocValueFieldsContext(source.docValueFields())); } if (source.highlighter() != null) { HighlightBuilder highlightBuilder = source.highlighter(); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java index b3389322d9c..7c6a743a20b 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java @@ -19,7 +19,6 @@ package org.elasticsearch.search.aggregations.metrics.tophits; -import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.SearchScript; import org.elasticsearch.search.aggregations.Aggregator; @@ -29,9 +28,8 @@ import org.elasticsearch.search.aggregations.InternalAggregation.Type; import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import org.elasticsearch.search.aggregations.support.AggregationContext; import org.elasticsearch.search.builder.SearchSourceBuilder.ScriptField; +import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; -import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext.DocValueField; -import org.elasticsearch.search.fetch.subphase.DocValueFieldsFetchSubPhase; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder; import org.elasticsearch.search.internal.SubSearchContext; @@ -98,12 +96,7 @@ public class TopHitsAggregatorFactory extends AggregatorFactory fields; - public DocValueField(String name) { - this.name = name; - } - - public String name() { - return name; - } + public DocValueFieldsContext(List fields) { + this.fields = fields; } - private List fields = new ArrayList<>(); - - public DocValueFieldsContext() { - } - - public void add(DocValueField field) { - this.fields.add(field); - } - - public List fields() { + /** + * Returns the required docvalue fields + */ + public List fields() { return this.fields; } } diff --git a/core/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java index 803cbb4348f..befce94a9e4 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java @@ -36,35 +36,21 @@ import java.util.HashMap; */ public final class DocValueFieldsFetchSubPhase implements FetchSubPhase { - public static final String NAME = "docvalue_fields"; - public static final ContextFactory CONTEXT_FACTORY = new ContextFactory() { - - @Override - public String getName() { - return NAME; - } - - @Override - public DocValueFieldsContext newContextInstance() { - return new DocValueFieldsContext(); - } - }; - @Override public void hitExecute(SearchContext context, HitContext hitContext) { - if (context.getFetchSubPhaseContext(CONTEXT_FACTORY).hitExecutionNeeded() == false) { + if (context.docValueFieldsContext() == null) { return; } - for (DocValueFieldsContext.DocValueField field : context.getFetchSubPhaseContext(CONTEXT_FACTORY).fields()) { + for (String field : context.docValueFieldsContext().fields()) { if (hitContext.hit().fieldsOrNull() == null) { hitContext.hit().fields(new HashMap<>(2)); } - SearchHitField hitField = hitContext.hit().fields().get(field.name()); + SearchHitField hitField = hitContext.hit().fields().get(field); if (hitField == null) { - hitField = new InternalSearchHitField(field.name(), new ArrayList<>(2)); - hitContext.hit().fields().put(field.name(), hitField); + hitField = new InternalSearchHitField(field, new ArrayList<>(2)); + hitContext.hit().fields().put(field, hitField); } - MappedFieldType fieldType = context.mapperService().fullName(field.name()); + MappedFieldType fieldType = context.mapperService().fullName(field); if (fieldType != null) { AtomicFieldData data = context.fieldData().getForField(fieldType).load(hitContext.readerContext()); ScriptDocValues values = data.getScriptValues(); 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 780352508bf..edce266c957 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java @@ -61,6 +61,7 @@ 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.subphase.DocValueFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.ScriptFieldsContext; import org.elasticsearch.search.fetch.subphase.highlight.SearchContextHighlight; @@ -110,6 +111,7 @@ public class DefaultSearchContext extends SearchContext { private StoredFieldsContext storedFields; private ScriptFieldsContext scriptFields; private FetchSourceContext fetchSourceContext; + private DocValueFieldsContext docValueFieldsContext; private int from = -1; private int size = -1; private SortAndFormats sort; @@ -470,6 +472,17 @@ public class DefaultSearchContext extends SearchContext { return this; } + @Override + public DocValueFieldsContext docValueFieldsContext() { + return docValueFieldsContext; + } + + @Override + public SearchContext docValueFieldsContext(DocValueFieldsContext docValueFieldsContext) { + this.docValueFieldsContext = docValueFieldsContext; + return this; + } + @Override public ContextIndexSearcher searcher() { return this.searcher; 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 65fe7ddad15..543f97e638f 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java @@ -22,9 +22,7 @@ package org.elasticsearch.search.internal; import org.apache.lucene.search.Collector; import org.apache.lucene.search.FieldDoc; import org.apache.lucene.search.Query; -import org.apache.lucene.store.AlreadyClosedException; import org.apache.lucene.util.Counter; -import org.apache.lucene.util.RefCount; import org.elasticsearch.action.search.SearchType; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseFieldMatcher; @@ -43,7 +41,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; @@ -54,6 +51,8 @@ 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.InnerHitsContext; import org.elasticsearch.search.fetch.subphase.ScriptFieldsContext; @@ -226,6 +225,10 @@ public abstract class SearchContext extends AbstractRefCounted implements Releas public abstract SearchContext fetchSourceContext(FetchSourceContext fetchSourceContext); + public abstract DocValueFieldsContext docValueFieldsContext(); + + public abstract SearchContext docValueFieldsContext(DocValueFieldsContext docValueFieldsContext); + public abstract ContextIndexSearcher searcher(); public abstract IndexShard indexShard(); 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 077a0941a0b..8ba76971de9 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/SubSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/SubSearchContext.java @@ -25,6 +25,7 @@ import org.elasticsearch.index.query.ParsedQuery; import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.aggregations.SearchContextAggregations; import org.elasticsearch.search.fetch.FetchSearchResult; +import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.ScriptFieldsContext; import org.elasticsearch.search.fetch.subphase.highlight.SearchContextHighlight; @@ -60,6 +61,7 @@ public class SubSearchContext extends FilteredSearchContext { private StoredFieldsContext storedFields; private ScriptFieldsContext scriptFields; private FetchSourceContext fetchSourceContext; + private DocValueFieldsContext docValueFieldsContext; private SearchContextHighlight highlight; private boolean explain; @@ -154,6 +156,17 @@ public class SubSearchContext extends FilteredSearchContext { return this; } + @Override + public DocValueFieldsContext docValueFieldsContext() { + return docValueFieldsContext; + } + + @Override + public SearchContext docValueFieldsContext(DocValueFieldsContext docValueFieldsContext) { + this.docValueFieldsContext = docValueFieldsContext; + return this; + } + @Override public void timeout(TimeValue timeout) { throw new UnsupportedOperationException("Not supported"); 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 cc325d2e60f..7b05de164f0 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java +++ b/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java @@ -47,6 +47,7 @@ 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.subphase.DocValueFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.ScriptFieldsContext; import org.elasticsearch.search.fetch.subphase.highlight.SearchContextHighlight; @@ -262,6 +263,16 @@ public class TestSearchContext extends SearchContext { return null; } + @Override + public DocValueFieldsContext docValueFieldsContext() { + return null; + } + + @Override + public SearchContext docValueFieldsContext(DocValueFieldsContext docValueFieldsContext) { + return null; + } + @Override public ContextIndexSearcher searcher() { return searcher; From 4b57a0fd975f10952d3bc37edef4366e48da470a Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 7 Sep 2016 18:04:07 +0200 Subject: [PATCH 05/13] resolve some line length problems and remove some entry from checkstyle suppressions (deleted classes) --- .../resources/checkstyle_suppressions.xml | 16 --------------- .../elasticsearch/search/SearchService.java | 6 ++++-- .../search/builder/SearchSourceBuilder.java | 20 +++++++++---------- .../search/internal/DefaultSearchContext.java | 3 ++- .../internal/FilteredSearchContext.java | 3 ++- .../search/internal/SearchContext.java | 3 ++- .../search/fetch/FetchSubPhasePluginIT.java | 3 ++- 7 files changed, 22 insertions(+), 32 deletions(-) diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml index 14c2bc8ca5a..2095d3d3790 100644 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ b/buildSrc/src/main/resources/checkstyle_suppressions.xml @@ -238,7 +238,6 @@ - @@ -415,7 +414,6 @@ - @@ -488,15 +486,12 @@ - - - @@ -555,7 +550,6 @@ - @@ -563,21 +557,13 @@ - - - - - - - - @@ -586,9 +572,7 @@ - - diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index 223595cc13b..95c2d0aab71 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -459,7 +459,8 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv throw ExceptionsHelper.convertToRuntime(e); } operationListener.onFetchPhase(context, System.nanoTime() - time2); - return new ScrollQueryFetchSearchResult(new QueryFetchSearchResult(context.queryResult(), context.fetchResult()), context.shardTarget()); + return new ScrollQueryFetchSearchResult(new QueryFetchSearchResult(context.queryResult(), context.fetchResult()), + context.shardTarget()); } catch (Exception e) { logger.trace("Fetch phase failed", e); processFailure(context, e); @@ -904,7 +905,8 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv continue; } if ((time - lastAccessTime > context.keepAlive())) { - logger.debug("freeing search context [{}], time [{}], lastAccessTime [{}], keepAlive [{}]", context.id(), time, lastAccessTime, context.keepAlive()); + logger.debug("freeing search context [{}], time [{}], lastAccessTime [{}], keepAlive [{}]", context.id(), time, + lastAccessTime, context.keepAlive()); freeContext(context.id()); } } diff --git a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java index e442db67b5a..e83fc91fd20 100644 --- a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java @@ -957,8 +957,8 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ XContentParser.Token token = parser.currentToken(); String currentFieldName = null; if (token != XContentParser.Token.START_OBJECT && (token = parser.nextToken()) != XContentParser.Token.START_OBJECT) { - throw new ParsingException(parser.getTokenLocation(), "Expected [" + XContentParser.Token.START_OBJECT + "] but found [" + token + "]", - parser.getTokenLocation()); + throw new ParsingException(parser.getTokenLocation(), "Expected [" + XContentParser.Token.START_OBJECT + + "] but found [" + token + "]", parser.getTokenLocation()); } while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { @@ -1017,8 +1017,8 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ } else if (token.isValue()) { indexBoost.put(currentFieldName, parser.floatValue()); } else { - throw new ParsingException(parser.getTokenLocation(), "Unknown key for a " + token + " in [" + currentFieldName + "].", - parser.getTokenLocation()); + throw new ParsingException(parser.getTokenLocation(), "Unknown key for a " + token + + " in [" + currentFieldName + "].", parser.getTokenLocation()); } } } else if (context.getParseFieldMatcher().match(currentFieldName, AGGREGATIONS_FIELD) @@ -1051,8 +1051,8 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ if (token == XContentParser.Token.VALUE_STRING) { docValueFields.add(parser.text()); } else { - throw new ParsingException(parser.getTokenLocation(), "Expected [" + XContentParser.Token.VALUE_STRING + "] in [" - + currentFieldName + "] but found [" + token + "]", parser.getTokenLocation()); + throw new ParsingException(parser.getTokenLocation(), "Expected [" + XContentParser.Token.VALUE_STRING + + "] in [" + currentFieldName + "] but found [" + token + "]", parser.getTokenLocation()); } } } else if (context.getParseFieldMatcher().match(currentFieldName, SORT_FIELD)) { @@ -1068,8 +1068,8 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ if (token == XContentParser.Token.VALUE_STRING) { stats.add(parser.text()); } else { - throw new ParsingException(parser.getTokenLocation(), "Expected [" + XContentParser.Token.VALUE_STRING + "] in [" - + currentFieldName + "] but found [" + token + "]", parser.getTokenLocation()); + throw new ParsingException(parser.getTokenLocation(), "Expected [" + XContentParser.Token.VALUE_STRING + + "] in [" + currentFieldName + "] but found [" + token + "]", parser.getTokenLocation()); } } } else if (context.getParseFieldMatcher().match(currentFieldName, _SOURCE_FIELD)) { @@ -1345,8 +1345,8 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ @Override public int hashCode() { return Objects.hash(aggregations, explain, fetchSourceContext, docValueFields, storedFieldsContext, from, - highlightBuilder, indexBoost, minScore, postQueryBuilder, queryBuilder, rescoreBuilders, scriptFields, - size, sorts, searchAfterBuilder, sliceBuilder, stats, suggestBuilder, terminateAfter, timeout, trackScores, version, profile); + highlightBuilder, indexBoost, minScore, postQueryBuilder, queryBuilder, rescoreBuilders, scriptFields, size, sorts, + searchAfterBuilder, sliceBuilder, stats, suggestBuilder, terminateAfter, timeout, trackScores, version, profile); } @Override 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 edce266c957..39e0aa17332 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java @@ -390,7 +390,8 @@ public class DefaultSearchContext extends SearchContext { } @Override - public SubPhaseContext getFetchSubPhaseContext(FetchSubPhase.ContextFactory contextFactory) { + public SubPhaseContext getFetchSubPhaseContext( + FetchSubPhase.ContextFactory contextFactory) { String subPhaseName = contextFactory.getName(); if (subPhaseContexts.get(subPhaseName) == null) { subPhaseContexts.put(subPhaseName, contextFactory.newContextInstance()); 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 4e52ab0d534..7959e78b98f 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java @@ -512,7 +512,8 @@ public abstract class FilteredSearchContext extends SearchContext { } @Override - public SubPhaseContext getFetchSubPhaseContext(FetchSubPhase.ContextFactory contextFactory) { + public SubPhaseContext getFetchSubPhaseContext( + FetchSubPhase.ContextFactory contextFactory) { return in.getFetchSubPhaseContext(contextFactory); } 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 543f97e638f..dfa35e8724e 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java @@ -186,7 +186,8 @@ public abstract class SearchContext extends AbstractRefCounted implements Releas public abstract SearchContext aggregations(SearchContextAggregations aggregations); - public abstract SubPhaseContext getFetchSubPhaseContext(FetchSubPhase.ContextFactory contextFactory); + public abstract SubPhaseContext getFetchSubPhaseContext( + FetchSubPhase.ContextFactory contextFactory); public abstract SearchContextHighlight highlight(); 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 f8da0312f88..a7585864be9 100644 --- a/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java +++ b/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java @@ -89,7 +89,8 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { .endObject(); SearchResponse response = client().prepareSearch().setSource(new SearchSourceBuilder().ext(extSource)).get(); assertSearchResponse(response); - assertThat(((Map) response.getHits().getAt(0).field("term_vectors_fetch").getValues().get(0)).get("i"), equalTo(2)); + assertThat(((Map) response.getHits().getAt(0).field("term_vectors_fetch").getValues().get(0)).get("i"), + equalTo(2)); assertThat(((Map) response.getHits().getAt(0).field("term_vectors_fetch").getValues().get(0)).get("am"), equalTo(2)); assertThat(((Map) response.getHits().getAt(0).field("term_vectors_fetch").getValues().get(0)).get("sam"), From dc2ba90f48adfa37803ddaf46087ab24debb0be0 Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 7 Sep 2016 23:43:17 +0200 Subject: [PATCH 06/13] clarify that SearchParseElement is only used for custom fetch sub phases and clean up extension point SearchParseElement is renamed to FetchSubPhaseParser and moved to the search.fetch package. Its parse method doesn't get the SearchContext as argument anymore, only the XContentParser, and the return type is what gets parsed (the fetch sub phase context which we may as well rename later). It is the parser that initializes the FetchSubPhaseContext then. SearchService retrieves the parser by name, calls parse against it and stores the result of parsing by name. No need for FetchSubPhase.ContextFactory anymore, which can be removed. --- .../resources/checkstyle_suppressions.xml | 3 -- .../elasticsearch/search/SearchService.java | 13 ++++-- .../search/fetch/FetchPhase.java | 9 ++-- .../search/fetch/FetchSubPhase.java | 22 +-------- .../search/fetch/FetchSubPhaseContext.java | 5 +- .../search/fetch/FetchSubPhaseParser.java | 30 ++++++++++++ .../search/internal/DefaultSearchContext.java | 20 ++++---- .../internal/FilteredSearchContext.java | 13 ++++-- .../search/internal/SearchContext.java | 7 +-- .../search/internal/SubSearchContext.java | 2 - .../search/fetch/FetchSubPhasePluginIT.java | 46 ++++++++----------- .../elasticsearch/test/TestSearchContext.java | 17 +++---- 12 files changed, 91 insertions(+), 96 deletions(-) create mode 100644 core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseParser.java 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 From f9530dfe8f360183b42c32828c62743017fc0e47 Mon Sep 17 00:00:00 2001 From: javanna Date: Thu, 8 Sep 2016 00:01:43 +0200 Subject: [PATCH 07/13] remove FetchSubPhaseContext in favour of generic fetch sub phase builder of type object The context was an object where the parsed info are stored. That is more of what we call the builder since after the search refactoring. No need for generics in FetchSubPhaseParser then. Also the previous setHitsExecutionNeeded wasn't useful, it can be removed as well, given that once there is a parsed ext section, it will become a builder that can be retrieved by the sub fetch phase. The sub fetch phase is responsible for doing nothing in case the builder is not set, meaning that the fetch sub phase is plugged in but the request didn't have the corresponding section. --- .../search/SearchParseElement.java | 31 ------------- .../elasticsearch/search/SearchService.java | 5 +- .../search/fetch/FetchSubPhaseContext.java | 46 ------------------- .../search/fetch/FetchSubPhaseParser.java | 4 +- .../search/internal/DefaultSearchContext.java | 12 ++--- .../internal/FilteredSearchContext.java | 9 ++-- .../search/internal/SearchContext.java | 6 +-- .../search/fetch/FetchSubPhasePluginIT.java | 43 ++++++++--------- .../elasticsearch/test/TestSearchContext.java | 12 ++--- 9 files changed, 39 insertions(+), 129 deletions(-) delete mode 100644 core/src/main/java/org/elasticsearch/search/SearchParseElement.java delete mode 100644 core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseContext.java diff --git a/core/src/main/java/org/elasticsearch/search/SearchParseElement.java b/core/src/main/java/org/elasticsearch/search/SearchParseElement.java deleted file mode 100644 index 9bf680deb55..00000000000 --- a/core/src/main/java/org/elasticsearch/search/SearchParseElement.java +++ /dev/null @@ -1,31 +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.search; - -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.search.internal.SearchContext; - -/** - * - */ -public interface SearchParseElement { - - void parse(XContentParser parser, SearchContext context) throws Exception; -} diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index 8ecc997dbb7..5ed2f62d9cc 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -63,7 +63,6 @@ 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; @@ -774,8 +773,8 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv throw new SearchParseException(context, "Unknown element [" + currentFieldName + "] in [ext]", extParser.getTokenLocation()); } else { - FetchSubPhaseContext fetchSubPhaseContext = fetchSubPhaseParser.parse(extParser); - context.putFetchSubPhaseContext(currentFieldName, fetchSubPhaseContext); + Object fetchSubPhaseBuilder = fetchSubPhaseParser.parse(extParser); + context.putFetchSubPhaseBuilder(currentFieldName, fetchSubPhaseBuilder); } } } diff --git a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseContext.java b/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseContext.java deleted file mode 100644 index f2aa1b88138..00000000000 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseContext.java +++ /dev/null @@ -1,46 +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.search.fetch; - -/** - * 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. - */ -public abstract class FetchSubPhaseContext { - - // This is to store if the FetchSubPhase should be executed at all. - private boolean hitExecutionNeeded = false; - - /** - * Set if this phase should be executed at all. - */ - public void setHitExecutionNeeded(boolean hitExecutionNeeded) { - this.hitExecutionNeeded = hitExecutionNeeded; - } - - /** - * Returns if this phase be executed at all. - */ - public boolean hitExecutionNeeded() { - return hitExecutionNeeded; - } - -} diff --git a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseParser.java b/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseParser.java index c88b1995688..1fddf442d2d 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseParser.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseParser.java @@ -24,7 +24,7 @@ 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 { +public interface FetchSubPhaseParser { - Context parse(XContentParser parser) throws Exception; + Object 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 bf968e8f0cd..35f472da46d 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java @@ -58,7 +58,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.FetchSubPhaseContext; import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; @@ -146,7 +145,7 @@ public class DefaultSearchContext extends SearchContext { private volatile long lastAccessTime = -1; private Profilers profilers; - private final Map subPhaseContexts = new HashMap<>(); + private final Map subPhaseBuilders = new HashMap<>(); private final Map, Collector> queryCollectors = new HashMap<>(); private final QueryShardContext queryShardContext; private FetchPhase fetchPhase; @@ -386,14 +385,13 @@ public class DefaultSearchContext extends SearchContext { } @Override - @SuppressWarnings("unchecked") - public SubPhaseContext getFetchSubPhaseContext(String name) { - return (SubPhaseContext) subPhaseContexts.get(name); + public Object getFetchSubPhaseBuilder(String name) { + return subPhaseBuilders.get(name); } @Override - public void putFetchSubPhaseContext(String name, SubPhaseContext subPhaseContext) { - subPhaseContexts.put(name, subPhaseContext); + public void putFetchSubPhaseBuilder(String name, Object fetchSubPhaseBuilder) { + subPhaseBuilders.put(name, fetchSubPhaseBuilder); } @Override 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 250721ba27d..afd14ce07ab 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java @@ -43,7 +43,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.FetchSubPhaseContext; import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.InnerHitsContext; @@ -511,13 +510,13 @@ public abstract class FilteredSearchContext extends SearchContext { } @Override - public SubPhaseContext getFetchSubPhaseContext(String name) { - return in.getFetchSubPhaseContext(name); + public void putFetchSubPhaseBuilder(String name, Object fetchSubPhaseBuilder) { + in.putFetchSubPhaseBuilder(name, fetchSubPhaseBuilder); } @Override - public void putFetchSubPhaseContext(String name, SubPhaseContext subPhaseContext) { - in.putFetchSubPhaseContext(name, subPhaseContext); + public Object getFetchSubPhaseBuilder(String name) { + return in.getFetchSubPhaseBuilder(name); } @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 d92fc79dd32..4520c866602 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.FetchSubPhaseContext; import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; @@ -185,10 +184,9 @@ public abstract class SearchContext extends AbstractRefCounted implements Releas public abstract SearchContext aggregations(SearchContextAggregations aggregations); - public abstract void putFetchSubPhaseContext(String name, - SubPhaseContext subPhaseContext); + public abstract void putFetchSubPhaseBuilder(String name, Object fetchSubPhaseBuilder); - public abstract SubPhaseContext getFetchSubPhaseContext(String name); + public abstract Object getFetchSubPhaseBuilder(String name); public abstract SearchContextHighlight highlight(); 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 7f4b0911941..a7e2988f16e 100644 --- a/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java +++ b/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java @@ -26,6 +26,7 @@ import org.apache.lucene.util.BytesRef; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.termvectors.TermVectorsRequest; import org.elasticsearch.action.termvectors.TermVectorsResponse; +import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -62,6 +63,7 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { return Collections.singletonList(FetchTermVectorsPlugin.class); } + @SuppressWarnings("unchecked") public void testPlugin() throws Exception { client().admin() .indices() @@ -104,22 +106,20 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { } public static final class TermVectorsFetchSubPhase implements FetchSubPhase { - - public static final String NAME = "term_vectors_fetch"; + private static final String NAME = "term_vectors_fetch"; @Override public Map parsers() { - return singletonMap("term_vectors_fetch", new TermVectorsFetchParser()); + return singletonMap(NAME, new TermVectorsFetchParser()); } @Override public void hitExecute(SearchContext context, HitContext hitContext) { - TermVectorsFetchContext fetchSubPhaseContext = context.getFetchSubPhaseContext(NAME); - if (fetchSubPhaseContext.hitExecutionNeeded() == false) { + TermVectorsFetchBuilder fetchSubPhaseBuilder = (TermVectorsFetchBuilder)context.getFetchSubPhaseBuilder(NAME); + if (fetchSubPhaseBuilder == null) { return; } - String field = fetchSubPhaseContext.getField(); - + String field = fetchSubPhaseBuilder.getField(); if (hitContext.hit().fieldsOrNull() == null) { hitContext.hit().fields(new HashMap<>()); } @@ -145,32 +145,27 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { } } - public static class TermVectorsFetchParser implements FetchSubPhaseParser { - + public static class TermVectorsFetchParser implements FetchSubPhaseParser { @Override - public TermVectorsFetchContext parse(XContentParser parser) throws Exception { - // this is to make sure that the SubFetchPhase knows it should execute - TermVectorsFetchContext termVectorsFetchContext = new TermVectorsFetchContext(); - termVectorsFetchContext.setHitExecutionNeeded(true); + public TermVectorsFetchBuilder parse(XContentParser parser) throws Exception { + String field; XContentParser.Token token = parser.currentToken(); if (token == XContentParser.Token.VALUE_STRING) { - String fieldName = parser.text(); - termVectorsFetchContext.setField(fieldName); + field = parser.text(); } else { - throw new IllegalStateException("Expected a VALUE_STRING but got " + token); + throw new ParsingException(parser.getTokenLocation(), "Expected a VALUE_STRING but got " + token); } - return termVectorsFetchContext; + if (field == null) { + throw new ParsingException(parser.getTokenLocation(), "no fields specified for " + TermVectorsFetchSubPhase.NAME); + } + return new TermVectorsFetchBuilder(field); } } - public static class TermVectorsFetchContext extends FetchSubPhaseContext { + public static class TermVectorsFetchBuilder { + private final String field; - private String field = null; - - public TermVectorsFetchContext() { - } - - public void setField(String field) { + private TermVectorsFetchBuilder(String field) { this.field = field; } 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 67c3ba5b55f..a520b144df1 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java +++ b/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java @@ -44,7 +44,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.FetchSubPhaseContext; import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; @@ -90,7 +89,7 @@ public class TestSearchContext extends SearchContext { private SearchContextAggregations aggregations; private final long originNanoTime = System.nanoTime(); - private final Map subPhaseContexts = new HashMap<>(); + private final Map subPhaseContexts = new HashMap<>(); public TestSearchContext(ThreadPool threadPool, BigArrays bigArrays, ScriptService scriptService, IndexService indexService) { super(ParseFieldMatcher.STRICT); @@ -197,14 +196,13 @@ public class TestSearchContext extends SearchContext { } @Override - @SuppressWarnings("unchecked") - public SubPhaseContext getFetchSubPhaseContext(String name) { - return (SubPhaseContext) subPhaseContexts.get(name); + public void putFetchSubPhaseBuilder(String name, Object fetchSubPhaseBuilder) { + subPhaseContexts.put(name, fetchSubPhaseBuilder); } @Override - public void putFetchSubPhaseContext(String name, SubPhaseContext subPhaseContext) { - subPhaseContexts.put(name, subPhaseContext); + public Object getFetchSubPhaseBuilder(String name) { + return subPhaseContexts.get(name); } @Override From 12eaeb3945e48adfa33c639a786421f80b5d634e Mon Sep 17 00:00:00 2001 From: javanna Date: Thu, 8 Sep 2016 10:30:48 +0200 Subject: [PATCH 08/13] FetchSubPhase to support a single parser that extends SearchExtParser --- .../elasticsearch/search/SearchService.java | 10 +++++----- .../search/fetch/FetchPhase.java | 8 +++++--- .../search/fetch/FetchSubPhase.java | 8 +++++--- ...bPhaseParser.java => SearchExtParser.java} | 12 +++++++++-- .../search/fetch/FetchSubPhasePluginIT.java | 20 ++++++++++++++----- 5 files changed, 40 insertions(+), 18 deletions(-) rename core/src/main/java/org/elasticsearch/search/fetch/{FetchSubPhaseParser.java => SearchExtParser.java} (79%) diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index 5ed2f62d9cc..589dccec50b 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -63,7 +63,7 @@ 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.FetchSubPhaseParser; +import org.elasticsearch.search.fetch.SearchExtParser; import org.elasticsearch.search.fetch.QueryFetchSearchResult; import org.elasticsearch.search.fetch.ScrollQueryFetchSearchResult; import org.elasticsearch.search.fetch.ShardFetchRequest; @@ -144,7 +144,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv private final ConcurrentMapLong activeContexts = ConcurrentCollections.newConcurrentMapLongWithAggressiveConcurrency(); - private final Map fetchPhaseParsers; + private final Map fetchPhaseParsers; private final ParseFieldMatcher parseFieldMatcher; @@ -763,8 +763,8 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = extParser.currentName(); } else { - FetchSubPhaseParser fetchSubPhaseParser = this.fetchPhaseParsers.get(currentFieldName); - if (fetchSubPhaseParser == null) { + SearchExtParser searchExtParser = this.fetchPhaseParsers.get(currentFieldName); + if (searchExtParser == null) { if (currentFieldName != null && currentFieldName.equals("suggest")) { throw new SearchParseException(context, "suggest is not supported in [ext], please use SearchSourceBuilder#suggest(SuggestBuilder) instead", @@ -773,7 +773,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv throw new SearchParseException(context, "Unknown element [" + currentFieldName + "] in [ext]", extParser.getTokenLocation()); } else { - Object fetchSubPhaseBuilder = fetchSubPhaseParser.parse(extParser); + Object fetchSubPhaseBuilder = searchExtParser.parse(extParser); context.putFetchSubPhaseBuilder(currentFieldName, fetchSubPhaseBuilder); } } 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 1649c0f449c..4053ba5adb5 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java @@ -77,10 +77,12 @@ public class FetchPhase implements SearchPhase { this.fetchSubPhases[fetchSubPhases.size()] = new InnerHitsFetchSubPhase(this); } - public Map parsers() { - Map parsers = new HashMap<>(); + public Map parsers() { + Map parsers = new HashMap<>(); for (FetchSubPhase fetchSubPhase : fetchSubPhases) { - parsers.putAll(fetchSubPhase.parsers()); + if (fetchSubPhase.parser() != null) { + parsers.put(fetchSubPhase.parser().getName(), fetchSubPhase.parser()); + } } return unmodifiableMap(parsers); } 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 afe67c5d78d..b230c8dd878 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java @@ -25,7 +25,6 @@ import org.apache.lucene.search.IndexSearcher; import org.elasticsearch.search.internal.InternalSearchHit; import org.elasticsearch.search.internal.SearchContext; -import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -77,8 +76,11 @@ public interface FetchSubPhase { } - default Map parsers() { - return Collections.emptyMap(); + /** + * Returns the parser for the optional config to be put in the ext section of the search request + */ + default SearchExtParser parser() { + return null; } /** diff --git a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseParser.java b/core/src/main/java/org/elasticsearch/search/fetch/SearchExtParser.java similarity index 79% rename from core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseParser.java rename to core/src/main/java/org/elasticsearch/search/fetch/SearchExtParser.java index 1fddf442d2d..01e87951efc 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhaseParser.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/SearchExtParser.java @@ -22,9 +22,17 @@ 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 + * Parser for the ext section of a search request, which can hold custom fetch sub phases config */ -public interface FetchSubPhaseParser { +public interface SearchExtParser { + /** + * Returns the name of the element that this parser is able to parse + */ + String getName(); + + /** + * Parses the element whose name is returned by {@link #getName()} + */ Object parse(XContentParser parser) throws Exception; } 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 a7e2988f16e..5134d62c309 100644 --- a/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java +++ b/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java @@ -50,7 +50,6 @@ import java.util.List; import java.util.Map; import static java.util.Collections.singletonList; -import static java.util.Collections.singletonMap; import static org.elasticsearch.client.Requests.indexRequest; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; @@ -109,8 +108,8 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { private static final String NAME = "term_vectors_fetch"; @Override - public Map parsers() { - return singletonMap(NAME, new TermVectorsFetchParser()); + public SearchExtParser parser() { + return TermVectorsFetchParser.INSTANCE; } @Override @@ -145,7 +144,18 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { } } - public static class TermVectorsFetchParser implements FetchSubPhaseParser { + public static final class TermVectorsFetchParser implements SearchExtParser { + + private static final TermVectorsFetchParser INSTANCE = new TermVectorsFetchParser(); + + private TermVectorsFetchParser() { + } + + @Override + public String getName() { + return TermVectorsFetchSubPhase.NAME; + } + @Override public TermVectorsFetchBuilder parse(XContentParser parser) throws Exception { String field; @@ -162,7 +172,7 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { } } - public static class TermVectorsFetchBuilder { + public static final class TermVectorsFetchBuilder { private final String field; private TermVectorsFetchBuilder(String field) { From 455a2143f1364516d40351b62b5b94f5939d80bd Mon Sep 17 00:00:00 2001 From: javanna Date: Thu, 8 Sep 2016 10:37:06 +0200 Subject: [PATCH 09/13] move SearchExtParser back to o.e.search package The parser is now needed only for sub fetch phases, but doesn't have to be strictly connected to them, it could be used for something else as well potentially --- .../org/elasticsearch/search/{fetch => }/SearchExtParser.java | 2 +- core/src/main/java/org/elasticsearch/search/SearchService.java | 1 - .../main/java/org/elasticsearch/search/fetch/FetchPhase.java | 1 + .../main/java/org/elasticsearch/search/fetch/FetchSubPhase.java | 1 + .../org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java | 1 + 5 files changed, 4 insertions(+), 2 deletions(-) rename core/src/main/java/org/elasticsearch/search/{fetch => }/SearchExtParser.java (96%) diff --git a/core/src/main/java/org/elasticsearch/search/fetch/SearchExtParser.java b/core/src/main/java/org/elasticsearch/search/SearchExtParser.java similarity index 96% rename from core/src/main/java/org/elasticsearch/search/fetch/SearchExtParser.java rename to core/src/main/java/org/elasticsearch/search/SearchExtParser.java index 01e87951efc..c537beaf3f4 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/SearchExtParser.java +++ b/core/src/main/java/org/elasticsearch/search/SearchExtParser.java @@ -17,7 +17,7 @@ * under the License. */ -package org.elasticsearch.search.fetch; +package org.elasticsearch.search; import org.elasticsearch.common.xcontent.XContentParser; diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index 589dccec50b..09c1bda2d8d 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -63,7 +63,6 @@ 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.SearchExtParser; import org.elasticsearch.search.fetch.QueryFetchSearchResult; import org.elasticsearch.search.fetch.ScrollQueryFetchSearchResult; import org.elasticsearch.search.fetch.ShardFetchRequest; 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 4053ba5adb5..b45ae23e0bd 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java @@ -41,6 +41,7 @@ import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.ObjectMapper; import org.elasticsearch.index.mapper.SourceFieldMapper; +import org.elasticsearch.search.SearchExtParser; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.SearchHitField; import org.elasticsearch.search.SearchPhase; 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 b230c8dd878..1873568f8d4 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java @@ -22,6 +22,7 @@ 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.SearchExtParser; import org.elasticsearch.search.internal.InternalSearchHit; import org.elasticsearch.search.internal.SearchContext; 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 5134d62c309..4dbdc850706 100644 --- a/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java +++ b/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java @@ -33,6 +33,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.termvectors.TermVectorsService; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.SearchPlugin; +import org.elasticsearch.search.SearchExtParser; import org.elasticsearch.search.SearchHitField; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.internal.InternalSearchHitField; From 65c7f61ad9ebc3ae7993373583377962ce02ecad Mon Sep 17 00:00:00 2001 From: javanna Date: Thu, 8 Sep 2016 19:15:28 +0200 Subject: [PATCH 10/13] decouple registration of SearchExtParsers from sub fetch phases Search section supports an ext section that is used to provide additional config needed from plugins. It is now tied to sub fetch phases because it is the only section that may need additional config, but there is no reason for the two to be tightly coupled. It is now possible to register a searchExtParser independently from a sub fetch phase. All a search ext parser does is parsing some ext section of a search request, whose parsed resulting object is stored in the search context for later retrieval. --- .../elasticsearch/plugins/SearchPlugin.java | 7 ++++ .../search/SearchExtParserRegistry.java | 32 +++++++++++++++++++ .../elasticsearch/search/SearchModule.java | 11 +++++++ .../elasticsearch/search/SearchService.java | 27 ++++++---------- .../search/fetch/FetchPhase.java | 12 ------- .../search/fetch/FetchSubPhase.java | 8 ----- .../search/internal/DefaultSearchContext.java | 10 +++--- .../internal/FilteredSearchContext.java | 8 ++--- .../search/internal/SearchContext.java | 4 +-- .../search/fetch/FetchSubPhasePluginIT.java | 12 +++---- .../search/MockSearchService.java | 5 +-- .../elasticsearch/test/TestSearchContext.java | 10 +++--- 12 files changed, 84 insertions(+), 62 deletions(-) create mode 100644 core/src/main/java/org/elasticsearch/search/SearchExtParserRegistry.java diff --git a/core/src/main/java/org/elasticsearch/plugins/SearchPlugin.java b/core/src/main/java/org/elasticsearch/plugins/SearchPlugin.java index 97ee715ef6a..a01837509c0 100644 --- a/core/src/main/java/org/elasticsearch/plugins/SearchPlugin.java +++ b/core/src/main/java/org/elasticsearch/plugins/SearchPlugin.java @@ -30,6 +30,7 @@ import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryParser; import org.elasticsearch.index.query.functionscore.ScoreFunctionBuilder; import org.elasticsearch.index.query.functionscore.ScoreFunctionParser; +import org.elasticsearch.search.SearchExtParser; import org.elasticsearch.search.aggregations.Aggregation; import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.Aggregator; @@ -82,6 +83,12 @@ public interface SearchPlugin { default List getFetchSubPhases(FetchPhaseConstructionContext context) { return emptyList(); } + /** + * The new {@link SearchExtParser}s defined by this plugin. + */ + default List getSearchExtParsers() { + return emptyList(); + } /** * Get the {@link Highlighter}s defined by this plugin. */ diff --git a/core/src/main/java/org/elasticsearch/search/SearchExtParserRegistry.java b/core/src/main/java/org/elasticsearch/search/SearchExtParserRegistry.java new file mode 100644 index 00000000000..423ae802874 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/search/SearchExtParserRegistry.java @@ -0,0 +1,32 @@ +/* + * 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; + +import org.elasticsearch.common.xcontent.ParseFieldRegistry; + +/** + * Extensions to ParseFieldRegistry to make Guice happy. + */ +public class SearchExtParserRegistry extends ParseFieldRegistry { + + SearchExtParserRegistry() { + super("ext"); + } +} diff --git a/core/src/main/java/org/elasticsearch/search/SearchModule.java b/core/src/main/java/org/elasticsearch/search/SearchModule.java index ab36f03639c..60cfdf7ab5b 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/core/src/main/java/org/elasticsearch/search/SearchModule.java @@ -306,6 +306,7 @@ public class SearchModule extends AbstractModule { "moving_avg_model"); private final List fetchSubPhases = new ArrayList<>(); + private final SearchExtParserRegistry searchExtParserRegistry = new SearchExtParserRegistry(); private final Settings settings; private final List namedWriteables = new ArrayList<>(); @@ -326,6 +327,7 @@ public class SearchModule extends AbstractModule { registerAggregations(plugins); registerPipelineAggregations(plugins); registerFetchSubPhases(plugins); + registerSearchExtParsers(plugins); registerShapes(); searchRequestParsers = new SearchRequestParsers(queryParserRegistry, aggregatorParsers, getSuggesters()); } @@ -380,6 +382,7 @@ public class SearchModule extends AbstractModule { if (false == transportClient) { bind(IndicesQueriesRegistry.class).toInstance(queryParserRegistry); bind(SearchRequestParsers.class).toInstance(searchRequestParsers); + bind(SearchExtParserRegistry.class).toInstance(searchExtParserRegistry); configureSearch(); } } @@ -725,6 +728,14 @@ public class SearchModule extends AbstractModule { registerFromPlugin(plugins, p -> p.getFetchSubPhases(context), this::registerFetchSubPhase); } + private void registerSearchExtParsers(List plugins) { + registerFromPlugin(plugins, SearchPlugin::getSearchExtParsers, this::registerSearchExtParser); + } + + private void registerSearchExtParser(SearchExtParser searchExtParser) { + searchExtParserRegistry.register(searchExtParser, searchExtParser.getName()); + } + private void registerFetchSubPhase(FetchSubPhase subPhase) { Class subPhaseClass = subPhase.getClass(); if (fetchSubPhases.stream().anyMatch(p -> p.getClass().equals(subPhaseClass))) { diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index 09c1bda2d8d..96f0629beb9 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -100,7 +100,6 @@ import java.util.Optional; import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicLong; -import static java.util.Collections.unmodifiableMap; import static org.elasticsearch.common.unit.TimeValue.timeValueMillis; import static org.elasticsearch.common.unit.TimeValue.timeValueMinutes; @@ -143,13 +142,14 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv private final ConcurrentMapLong activeContexts = ConcurrentCollections.newConcurrentMapLongWithAggressiveConcurrency(); - private final Map fetchPhaseParsers; + private final SearchExtParserRegistry searchExtParserRegistry; private final ParseFieldMatcher parseFieldMatcher; @Inject public SearchService(Settings settings, ClusterSettings clusterSettings, ClusterService clusterService, IndicesService indicesService, - ThreadPool threadPool, ScriptService scriptService, BigArrays bigArrays, FetchPhase fetchPhase) { + ThreadPool threadPool, ScriptService scriptService, BigArrays bigArrays, FetchPhase fetchPhase, + SearchExtParserRegistry searchExtParserRegistry) { super(settings); this.parseFieldMatcher = new ParseFieldMatcher(settings); this.threadPool = threadPool; @@ -163,10 +163,10 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv TimeValue keepAliveInterval = KEEPALIVE_INTERVAL_SETTING.get(settings); this.defaultKeepAlive = DEFAULT_KEEPALIVE_SETTING.get(settings).millis(); - this.fetchPhaseParsers = unmodifiableMap(fetchPhase.parsers()); - this.keepAliveReaper = threadPool.scheduleWithFixedDelay(new Reaper(), keepAliveInterval, Names.SAME); + this.searchExtParserRegistry = searchExtParserRegistry; + defaultSearchTimeout = DEFAULT_SEARCH_TIMEOUT_SETTING.get(settings); clusterSettings.addSettingsUpdateConsumer(DEFAULT_SEARCH_TIMEOUT_SETTING, this::setDefaultSearchTimeout); } @@ -762,19 +762,10 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = extParser.currentName(); } else { - SearchExtParser searchExtParser = this.fetchPhaseParsers.get(currentFieldName); - if (searchExtParser == null) { - if (currentFieldName != null && currentFieldName.equals("suggest")) { - throw new SearchParseException(context, - "suggest is not supported in [ext], please use SearchSourceBuilder#suggest(SuggestBuilder) instead", - extParser.getTokenLocation()); - } - throw new SearchParseException(context, "Unknown element [" + currentFieldName + "] in [ext]", - extParser.getTokenLocation()); - } else { - Object fetchSubPhaseBuilder = searchExtParser.parse(extParser); - context.putFetchSubPhaseBuilder(currentFieldName, fetchSubPhaseBuilder); - } + SearchExtParser searchExtParser = this.searchExtParserRegistry.lookup(currentFieldName, parseFieldMatcher, + extParser.getTokenLocation()); + Object searchExtBuilder = searchExtParser.parse(extParser); + context.putSearchExtBuilder(currentFieldName, searchExtBuilder); } } } catch (Exception e) { 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 b45ae23e0bd..41ea0e294da 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java @@ -41,7 +41,6 @@ import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.ObjectMapper; import org.elasticsearch.index.mapper.SourceFieldMapper; -import org.elasticsearch.search.SearchExtParser; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.SearchHitField; import org.elasticsearch.search.SearchPhase; @@ -62,7 +61,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import static java.util.Collections.unmodifiableMap; import static org.elasticsearch.common.xcontent.XContentFactory.contentBuilder; /** @@ -78,16 +76,6 @@ public class FetchPhase implements SearchPhase { this.fetchSubPhases[fetchSubPhases.size()] = new InnerHitsFetchSubPhase(this); } - public Map parsers() { - Map parsers = new HashMap<>(); - for (FetchSubPhase fetchSubPhase : fetchSubPhases) { - if (fetchSubPhase.parser() != null) { - parsers.put(fetchSubPhase.parser().getName(), fetchSubPhase.parser()); - } - } - return unmodifiableMap(parsers); - } - @Override public void preProcess(SearchContext context) { } 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 1873568f8d4..1783652d120 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.SearchExtParser; import org.elasticsearch.search.internal.InternalSearchHit; import org.elasticsearch.search.internal.SearchContext; @@ -77,13 +76,6 @@ public interface FetchSubPhase { } - /** - * Returns the parser for the optional config to be put in the ext section of the search request - */ - default SearchExtParser parser() { - return null; - } - /** * Executes the hit level phase, with a reader and doc id (note, its a low level reader, and the matching doc). */ 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 35f472da46d..5aacb633416 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java @@ -145,7 +145,7 @@ public class DefaultSearchContext extends SearchContext { private volatile long lastAccessTime = -1; private Profilers profilers; - private final Map subPhaseBuilders = new HashMap<>(); + private final Map searchExtBuilders = new HashMap<>(); private final Map, Collector> queryCollectors = new HashMap<>(); private final QueryShardContext queryShardContext; private FetchPhase fetchPhase; @@ -385,13 +385,13 @@ public class DefaultSearchContext extends SearchContext { } @Override - public Object getFetchSubPhaseBuilder(String name) { - return subPhaseBuilders.get(name); + public Object getSearchExtBuilder(String name) { + return searchExtBuilders.get(name); } @Override - public void putFetchSubPhaseBuilder(String name, Object fetchSubPhaseBuilder) { - subPhaseBuilders.put(name, fetchSubPhaseBuilder); + public void putSearchExtBuilder(String name, Object searchExtBuilder) { + searchExtBuilders.put(name, searchExtBuilder); } @Override 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 afd14ce07ab..c9d10a7aa67 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java @@ -510,13 +510,13 @@ public abstract class FilteredSearchContext extends SearchContext { } @Override - public void putFetchSubPhaseBuilder(String name, Object fetchSubPhaseBuilder) { - in.putFetchSubPhaseBuilder(name, fetchSubPhaseBuilder); + public void putSearchExtBuilder(String name, Object fetchSubPhaseBuilder) { + in.putSearchExtBuilder(name, fetchSubPhaseBuilder); } @Override - public Object getFetchSubPhaseBuilder(String name) { - return in.getFetchSubPhaseBuilder(name); + public Object getSearchExtBuilder(String name) { + return in.getSearchExtBuilder(name); } @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 4520c866602..e060669fd5c 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java @@ -184,9 +184,9 @@ public abstract class SearchContext extends AbstractRefCounted implements Releas public abstract SearchContext aggregations(SearchContextAggregations aggregations); - public abstract void putFetchSubPhaseBuilder(String name, Object fetchSubPhaseBuilder); + public abstract void putSearchExtBuilder(String name, Object fetchSubPhaseBuilder); - public abstract Object getFetchSubPhaseBuilder(String name); + public abstract Object getSearchExtBuilder(String name); public abstract SearchContextHighlight highlight(); 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 4dbdc850706..2517f89ec49 100644 --- a/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java +++ b/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java @@ -103,19 +103,19 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { public List getFetchSubPhases(FetchPhaseConstructionContext context) { return singletonList(new TermVectorsFetchSubPhase()); } + + @Override + public List getSearchExtParsers() { + return Collections.singletonList(TermVectorsFetchParser.INSTANCE); + } } public static final class TermVectorsFetchSubPhase implements FetchSubPhase { private static final String NAME = "term_vectors_fetch"; - @Override - public SearchExtParser parser() { - return TermVectorsFetchParser.INSTANCE; - } - @Override public void hitExecute(SearchContext context, HitContext hitContext) { - TermVectorsFetchBuilder fetchSubPhaseBuilder = (TermVectorsFetchBuilder)context.getFetchSubPhaseBuilder(NAME); + TermVectorsFetchBuilder fetchSubPhaseBuilder = (TermVectorsFetchBuilder)context.getSearchExtBuilder(NAME); if (fetchSubPhaseBuilder == null) { return; } diff --git a/test/framework/src/main/java/org/elasticsearch/search/MockSearchService.java b/test/framework/src/main/java/org/elasticsearch/search/MockSearchService.java index cae5b2ff95b..832a85654c1 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/MockSearchService.java +++ b/test/framework/src/main/java/org/elasticsearch/search/MockSearchService.java @@ -72,8 +72,9 @@ public class MockSearchService extends SearchService { @Inject public MockSearchService(Settings settings, ClusterSettings clusterSettings, ClusterService clusterService, IndicesService indicesService, ThreadPool threadPool, ScriptService scriptService, - BigArrays bigArrays, FetchPhase fetchPhase) { - super(settings, clusterSettings, clusterService, indicesService, threadPool, scriptService, bigArrays, fetchPhase); + BigArrays bigArrays, FetchPhase fetchPhase, SearchExtParserRegistry searchExtParserRegistry) { + super(settings, clusterSettings, clusterService, indicesService, threadPool, scriptService, + bigArrays, fetchPhase, searchExtParserRegistry); } @Override 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 a520b144df1..e9976b93657 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java +++ b/test/framework/src/main/java/org/elasticsearch/test/TestSearchContext.java @@ -89,7 +89,7 @@ public class TestSearchContext extends SearchContext { private SearchContextAggregations aggregations; private final long originNanoTime = System.nanoTime(); - private final Map subPhaseContexts = new HashMap<>(); + private final Map searchExtBuilders = new HashMap<>(); public TestSearchContext(ThreadPool threadPool, BigArrays bigArrays, ScriptService scriptService, IndexService indexService) { super(ParseFieldMatcher.STRICT); @@ -196,13 +196,13 @@ public class TestSearchContext extends SearchContext { } @Override - public void putFetchSubPhaseBuilder(String name, Object fetchSubPhaseBuilder) { - subPhaseContexts.put(name, fetchSubPhaseBuilder); + public void putSearchExtBuilder(String name, Object searchExtBuilders) { + this.searchExtBuilders.put(name, searchExtBuilders); } @Override - public Object getFetchSubPhaseBuilder(String name) { - return subPhaseContexts.get(name); + public Object getSearchExtBuilder(String name) { + return searchExtBuilders.get(name); } @Override From 90ab460fccd52db2fa98890cd1268538f57b1c93 Mon Sep 17 00:00:00 2001 From: javanna Date: Fri, 9 Sep 2016 15:28:12 +0200 Subject: [PATCH 11/13] move parsing of search ext sections to the coordinating node --- .../resources/checkstyle_suppressions.xml | 1 - .../io/stream/NamedWriteableRegistry.java | 6 +- .../common/io/stream/StreamOutput.java | 10 ++ .../elasticsearch/plugins/SearchPlugin.java | 20 ++- .../action/search/RestMultiSearchAction.java | 12 +- .../rest/action/search/RestSearchAction.java | 3 +- .../search/SearchExtBuilder.java | 48 ++++++++ .../elasticsearch/search/SearchExtParser.java | 23 ++-- ...erRegistry.java => SearchExtRegistry.java} | 4 +- .../elasticsearch/search/SearchModule.java | 18 +-- .../search/SearchRequestParsers.java | 18 ++- .../elasticsearch/search/SearchService.java | 45 +------ .../search/builder/SearchSourceBuilder.java | 114 +++++++++--------- .../search/internal/DefaultSearchContext.java | 13 +- .../internal/FilteredSearchContext.java | 9 +- .../search/internal/SearchContext.java | 5 +- .../search/MultiSearchRequestTests.java | 2 +- .../search/SearchRequestTests.java | 5 +- .../builder/SearchSourceBuilderTests.java | 36 +++--- .../search/fetch/FetchSubPhasePluginIT.java | 73 ++++++++--- .../ShardSearchTransportRequestTests.java | 5 +- .../TransportSearchTemplateAction.java | 5 +- .../TransportMultiPercolateAction.java | 5 +- .../percolator/TransportPercolateAction.java | 23 ++-- .../index/reindex/RestReindexAction.java | 6 +- .../index/reindex/RestReindexActionTests.java | 2 +- .../search/MockSearchService.java | 5 +- .../elasticsearch/test/TestSearchContext.java | 9 +- 28 files changed, 309 insertions(+), 216 deletions(-) create mode 100644 core/src/main/java/org/elasticsearch/search/SearchExtBuilder.java rename core/src/main/java/org/elasticsearch/search/{SearchExtParserRegistry.java => SearchExtRegistry.java} (89%) diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml index 10cb3197a84..987087764f7 100644 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ b/buildSrc/src/main/resources/checkstyle_suppressions.xml @@ -989,7 +989,6 @@ - diff --git a/core/src/main/java/org/elasticsearch/common/io/stream/NamedWriteableRegistry.java b/core/src/main/java/org/elasticsearch/common/io/stream/NamedWriteableRegistry.java index 1a3f57052de..8fde972e8e4 100644 --- a/core/src/main/java/org/elasticsearch/common/io/stream/NamedWriteableRegistry.java +++ b/core/src/main/java/org/elasticsearch/common/io/stream/NamedWriteableRegistry.java @@ -25,10 +25,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -import org.elasticsearch.plugins.Plugin; /** * A registry for {@link org.elasticsearch.common.io.stream.Writeable.Reader} readers of {@link NamedWriteable}. @@ -47,7 +43,7 @@ public class NamedWriteableRegistry { /** A name for the writeable which is unique to the {@link #categoryClass}. */ public final String name; - /** A reader captability of reading*/ + /** A reader capability of reading*/ public final Writeable.Reader reader; /** Creates a new entry which can be stored by the registry. */ diff --git a/core/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java b/core/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java index c1932fd4215..0584d37960e 100644 --- a/core/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java +++ b/core/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java @@ -871,6 +871,16 @@ public abstract class StreamOutput extends OutputStream { } } + /** + * Writes a list of strings + */ + public void writeStringList(List list) throws IOException { + writeVInt(list.size()); + for (String string: list) { + this.writeString(string); + } + } + /** * Writes a list of {@link NamedWriteable} objects. */ diff --git a/core/src/main/java/org/elasticsearch/plugins/SearchPlugin.java b/core/src/main/java/org/elasticsearch/plugins/SearchPlugin.java index a01837509c0..364454de0c5 100644 --- a/core/src/main/java/org/elasticsearch/plugins/SearchPlugin.java +++ b/core/src/main/java/org/elasticsearch/plugins/SearchPlugin.java @@ -30,6 +30,7 @@ import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryParser; import org.elasticsearch.index.query.functionscore.ScoreFunctionBuilder; import org.elasticsearch.index.query.functionscore.ScoreFunctionParser; +import org.elasticsearch.search.SearchExtBuilder; import org.elasticsearch.search.SearchExtParser; import org.elasticsearch.search.aggregations.Aggregation; import org.elasticsearch.search.aggregations.AggregationBuilder; @@ -86,7 +87,7 @@ public interface SearchPlugin { /** * The new {@link SearchExtParser}s defined by this plugin. */ - default List getSearchExtParsers() { + default List> getSearchExts() { return emptyList(); } /** @@ -167,7 +168,7 @@ public interface SearchPlugin { /** * Specification for an {@link Aggregation}. */ - public static class AggregationSpec extends SearchExtensionSpec { + class AggregationSpec extends SearchExtensionSpec { private final Map> resultReaders = new TreeMap<>(); /** @@ -224,7 +225,7 @@ public interface SearchPlugin { /** * Specification for a {@link PipelineAggregator}. */ - public static class PipelineAggregationSpec extends SearchExtensionSpec { + class PipelineAggregationSpec extends SearchExtensionSpec { private final Map> resultReaders = new TreeMap<>(); private final Writeable.Reader aggregatorReader; @@ -297,6 +298,19 @@ public interface SearchPlugin { } } + /** + * Specification for a {@link SearchExtBuilder} which represents an additional section that can be + * parsed in a search request (within the ext element). + */ + class SearchExtSpec extends SearchExtensionSpec> { + public SearchExtSpec(ParseField name, Writeable.Reader reader, SearchExtParser parser) { + super(name, reader, parser); + } + + public SearchExtSpec(String name, Writeable.Reader reader, SearchExtParser parser) { + super(name, reader, parser); + } + } /** * Specification of search time behavior extension like a custom {@link MovAvgModel} or {@link ScoreFunction}. diff --git a/core/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java b/core/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java index ae320bccac2..a54e40be731 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java @@ -19,10 +19,6 @@ package org.elasticsearch.rest.action.search; -import java.io.IOException; -import java.util.Map; -import java.util.function.BiConsumer; - import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.action.search.MultiSearchRequest; import org.elasticsearch.action.search.SearchRequest; @@ -40,12 +36,16 @@ import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestController; -import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.action.RestActions; import org.elasticsearch.rest.action.RestToXContentListener; import org.elasticsearch.search.SearchRequestParsers; import org.elasticsearch.search.builder.SearchSourceBuilder; +import java.io.IOException; +import java.util.Map; +import java.util.function.BiConsumer; + import static org.elasticsearch.common.xcontent.support.XContentMapValues.lenientNodeBooleanValue; import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeStringArrayValue; import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeStringValue; @@ -97,7 +97,7 @@ public class RestMultiSearchAction extends BaseRestHandler { final QueryParseContext queryParseContext = new QueryParseContext(searchRequestParsers.queryParsers, requestParser, parseFieldMatcher); searchRequest.source(SearchSourceBuilder.fromXContent(queryParseContext, - searchRequestParsers.aggParsers, searchRequestParsers.suggesters)); + searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers)); multiRequest.add(searchRequest); } catch (IOException e) { throw new ElasticsearchParseException("Exception when parsing search request", e); diff --git a/core/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java b/core/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java index 215165e6bbf..8acfc72dfe1 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java @@ -102,7 +102,8 @@ public class RestSearchAction extends BaseRestHandler { if (restContent != null) { try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) { QueryParseContext context = new QueryParseContext(searchRequestParsers.queryParsers, parser, parseFieldMatcher); - searchRequest.source().parseXContent(context, searchRequestParsers.aggParsers, searchRequestParsers.suggesters); + searchRequest.source().parseXContent(context, searchRequestParsers.aggParsers, searchRequestParsers.suggesters, + searchRequestParsers.searchExtParsers); } } diff --git a/core/src/main/java/org/elasticsearch/search/SearchExtBuilder.java b/core/src/main/java/org/elasticsearch/search/SearchExtBuilder.java new file mode 100644 index 00000000000..205dba1ba0e --- /dev/null +++ b/core/src/main/java/org/elasticsearch/search/SearchExtBuilder.java @@ -0,0 +1,48 @@ +/* + * 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; + +import org.elasticsearch.common.io.stream.NamedWriteable; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.plugins.SearchPlugin; + +/** + * Intermediate serializable representation of a search ext section. To be subclassed by plugins that support + * a custom section as part of a search request, which will be provided within the ext element. + * Any state needs to be serialized as part of the {@link org.elasticsearch.common.io.stream.Writeable#writeTo(StreamOutput)} method and + * read from the incoming stream, usually done adding a constructor that takes {@link org.elasticsearch.common.io.stream.StreamInput} as + * an argument. + * + * Registration happens through {@link SearchPlugin#getSearchExts()}, which also needs a {@link SearchExtParser} that's able to parse + * the incoming request from the REST layer into the proper {@link SearchExtBuilder} subclass. + * + * {@link #getWriteableName()} must return the same name as the one used for the registration + * of the {@link org.elasticsearch.plugins.SearchPlugin.SearchExtSpec}. + * + * @see SearchExtParser + * @see org.elasticsearch.plugins.SearchPlugin.SearchExtSpec + */ +public abstract class SearchExtBuilder implements NamedWriteable, ToXContent { + + public abstract int hashCode(); + + public abstract boolean equals(Object obj); +} diff --git a/core/src/main/java/org/elasticsearch/search/SearchExtParser.java b/core/src/main/java/org/elasticsearch/search/SearchExtParser.java index c537beaf3f4..a2fe4cfe0cb 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchExtParser.java +++ b/core/src/main/java/org/elasticsearch/search/SearchExtParser.java @@ -21,18 +21,23 @@ package org.elasticsearch.search; import org.elasticsearch.common.xcontent.XContentParser; +import java.io.IOException; + /** - * Parser for the ext section of a search request, which can hold custom fetch sub phases config + * Defines a parser that is able to parse {@link org.elasticsearch.search.SearchExtBuilder}s + * from {@link org.elasticsearch.common.xcontent.XContent}. + * + * Registration happens through {@link org.elasticsearch.plugins.SearchPlugin#getSearchExts()}, which also needs a {@link SearchExtBuilder} + * implementation which is the object that this parser returns when reading an incoming request form the REST layer. + * + * @see SearchExtBuilder + * @see org.elasticsearch.plugins.SearchPlugin.SearchExtSpec */ -public interface SearchExtParser { +@FunctionalInterface +public interface SearchExtParser { /** - * Returns the name of the element that this parser is able to parse + * Parses the supported element placed within the ext section of a search request */ - String getName(); - - /** - * Parses the element whose name is returned by {@link #getName()} - */ - Object parse(XContentParser parser) throws Exception; + T fromXContent(XContentParser parser) throws IOException; } diff --git a/core/src/main/java/org/elasticsearch/search/SearchExtParserRegistry.java b/core/src/main/java/org/elasticsearch/search/SearchExtRegistry.java similarity index 89% rename from core/src/main/java/org/elasticsearch/search/SearchExtParserRegistry.java rename to core/src/main/java/org/elasticsearch/search/SearchExtRegistry.java index 423ae802874..dd04145ba7d 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchExtParserRegistry.java +++ b/core/src/main/java/org/elasticsearch/search/SearchExtRegistry.java @@ -24,9 +24,9 @@ import org.elasticsearch.common.xcontent.ParseFieldRegistry; /** * Extensions to ParseFieldRegistry to make Guice happy. */ -public class SearchExtParserRegistry extends ParseFieldRegistry { +public class SearchExtRegistry extends ParseFieldRegistry { - SearchExtParserRegistry() { + public SearchExtRegistry() { super("ext"); } } diff --git a/core/src/main/java/org/elasticsearch/search/SearchModule.java b/core/src/main/java/org/elasticsearch/search/SearchModule.java index 60cfdf7ab5b..0a1d9824ea9 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/core/src/main/java/org/elasticsearch/search/SearchModule.java @@ -93,6 +93,7 @@ import org.elasticsearch.plugins.SearchPlugin.FetchPhaseConstructionContext; import org.elasticsearch.plugins.SearchPlugin.PipelineAggregationSpec; import org.elasticsearch.plugins.SearchPlugin.QuerySpec; import org.elasticsearch.plugins.SearchPlugin.ScoreFunctionSpec; +import org.elasticsearch.plugins.SearchPlugin.SearchExtSpec; import org.elasticsearch.plugins.SearchPlugin.SearchExtensionSpec; import org.elasticsearch.search.action.SearchTransportService; import org.elasticsearch.search.aggregations.AggregationBuilder; @@ -306,7 +307,7 @@ public class SearchModule extends AbstractModule { "moving_avg_model"); private final List fetchSubPhases = new ArrayList<>(); - private final SearchExtParserRegistry searchExtParserRegistry = new SearchExtParserRegistry(); + private final SearchExtRegistry searchExtParserRegistry = new SearchExtRegistry(); private final Settings settings; private final List namedWriteables = new ArrayList<>(); @@ -327,9 +328,9 @@ public class SearchModule extends AbstractModule { registerAggregations(plugins); registerPipelineAggregations(plugins); registerFetchSubPhases(plugins); - registerSearchExtParsers(plugins); + registerSearchExts(plugins); registerShapes(); - searchRequestParsers = new SearchRequestParsers(queryParserRegistry, aggregatorParsers, getSuggesters()); + searchRequestParsers = new SearchRequestParsers(queryParserRegistry, aggregatorParsers, getSuggesters(), searchExtParserRegistry); } public List getNamedWriteables() { @@ -382,7 +383,7 @@ public class SearchModule extends AbstractModule { if (false == transportClient) { bind(IndicesQueriesRegistry.class).toInstance(queryParserRegistry); bind(SearchRequestParsers.class).toInstance(searchRequestParsers); - bind(SearchExtParserRegistry.class).toInstance(searchExtParserRegistry); + bind(SearchExtRegistry.class).toInstance(searchExtParserRegistry); configureSearch(); } } @@ -728,12 +729,13 @@ public class SearchModule extends AbstractModule { registerFromPlugin(plugins, p -> p.getFetchSubPhases(context), this::registerFetchSubPhase); } - private void registerSearchExtParsers(List plugins) { - registerFromPlugin(plugins, SearchPlugin::getSearchExtParsers, this::registerSearchExtParser); + private void registerSearchExts(List plugins) { + registerFromPlugin(plugins, SearchPlugin::getSearchExts, this::registerSearchExt); } - private void registerSearchExtParser(SearchExtParser searchExtParser) { - searchExtParserRegistry.register(searchExtParser, searchExtParser.getName()); + private void registerSearchExt(SearchExtSpec spec) { + searchExtParserRegistry.register(spec.getParser(), spec.getName()); + namedWriteables.add(new Entry(SearchExtBuilder.class, spec.getName().getPreferredName(), spec.getReader())); } private void registerFetchSubPhase(FetchSubPhase subPhase) { diff --git a/core/src/main/java/org/elasticsearch/search/SearchRequestParsers.java b/core/src/main/java/org/elasticsearch/search/SearchRequestParsers.java index 83eebd125d8..e15557c2389 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchRequestParsers.java +++ b/core/src/main/java/org/elasticsearch/search/SearchRequestParsers.java @@ -37,7 +37,8 @@ public class SearchRequestParsers { /** * Query parsers that may be used in search requests. * @see org.elasticsearch.index.query.QueryParseContext - * @see org.elasticsearch.search.builder.SearchSourceBuilder#fromXContent(QueryParseContext, AggregatorParsers, Suggesters) + * @see org.elasticsearch.search.builder.SearchSourceBuilder#fromXContent(QueryParseContext, AggregatorParsers, + * Suggesters, SearchExtRegistry) */ public final IndicesQueriesRegistry queryParsers; @@ -45,20 +46,29 @@ public class SearchRequestParsers { // and pipeline agg parsers should be here /** * Agg and pipeline agg parsers that may be used in search requests. - * @see org.elasticsearch.search.builder.SearchSourceBuilder#fromXContent(QueryParseContext, AggregatorParsers, Suggesters) + * @see org.elasticsearch.search.builder.SearchSourceBuilder#fromXContent(QueryParseContext, AggregatorParsers, + * Suggesters, SearchExtRegistry) */ public final AggregatorParsers aggParsers; // TODO: Suggesters should be removed and the underlying map moved here /** * Suggesters that may be used in search requests. - * @see org.elasticsearch.search.builder.SearchSourceBuilder#fromXContent(QueryParseContext, AggregatorParsers, Suggesters) + * @see org.elasticsearch.search.builder.SearchSourceBuilder#fromXContent(QueryParseContext, AggregatorParsers, + * Suggesters, SearchExtRegistry) */ public final Suggesters suggesters; - public SearchRequestParsers(IndicesQueriesRegistry queryParsers, AggregatorParsers aggParsers, Suggesters suggesters) { + /** + * Pluggable section that can be parsed out of a search section, within the ext element + */ + public final SearchExtRegistry searchExtParsers; + + public SearchRequestParsers(IndicesQueriesRegistry queryParsers, AggregatorParsers aggParsers, Suggesters suggesters, + SearchExtRegistry searchExtParsers) { this.queryParsers = queryParsers; this.aggParsers = aggParsers; this.suggesters = suggesters; + this.searchExtParsers = searchExtParsers; } } diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index 96f0629beb9..2fb87565aa3 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -39,9 +39,6 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.common.util.concurrent.ConcurrentMapLong; -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentLocation; -import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexService; import org.elasticsearch.index.engine.Engine; @@ -142,14 +139,11 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv private final ConcurrentMapLong activeContexts = ConcurrentCollections.newConcurrentMapLongWithAggressiveConcurrency(); - private final SearchExtParserRegistry searchExtParserRegistry; - private final ParseFieldMatcher parseFieldMatcher; @Inject public SearchService(Settings settings, ClusterSettings clusterSettings, ClusterService clusterService, IndicesService indicesService, - ThreadPool threadPool, ScriptService scriptService, BigArrays bigArrays, FetchPhase fetchPhase, - SearchExtParserRegistry searchExtParserRegistry) { + ThreadPool threadPool, ScriptService scriptService, BigArrays bigArrays, FetchPhase fetchPhase) { super(settings); this.parseFieldMatcher = new ParseFieldMatcher(settings); this.threadPool = threadPool; @@ -165,8 +159,6 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv this.keepAliveReaper = threadPool.scheduleWithFixedDelay(new Reaper(), keepAliveInterval, Names.SAME); - this.searchExtParserRegistry = searchExtParserRegistry; - defaultSearchTimeout = DEFAULT_SEARCH_TIMEOUT_SETTING.get(settings); clusterSettings.addSettingsUpdateConsumer(DEFAULT_SEARCH_TIMEOUT_SETTING, this::setDefaultSearchTimeout); } @@ -749,39 +741,8 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv } } if (source.ext() != null) { - XContentParser extParser = null; - try { - extParser = XContentFactory.xContent(source.ext()).createParser(source.ext()); - if (extParser.nextToken() != XContentParser.Token.START_OBJECT) { - throw new SearchParseException(context, "expected start object, found [" + extParser.currentToken() + "] instead", - extParser.getTokenLocation()); - } - XContentParser.Token token; - String currentFieldName = null; - while ((token = extParser.nextToken()) != XContentParser.Token.END_OBJECT) { - if (token == XContentParser.Token.FIELD_NAME) { - currentFieldName = extParser.currentName(); - } else { - SearchExtParser searchExtParser = this.searchExtParserRegistry.lookup(currentFieldName, parseFieldMatcher, - extParser.getTokenLocation()); - Object searchExtBuilder = searchExtParser.parse(extParser); - context.putSearchExtBuilder(currentFieldName, searchExtBuilder); - } - } - } catch (Exception e) { - String sSource = "_na_"; - try { - sSource = source.toString(); - } catch (Exception inner) { - e.addSuppressed(inner); - // ignore - } - XContentLocation location = extParser != null ? extParser.getTokenLocation() : null; - throw new SearchParseException(context, "failed to parse ext source [" + sSource + "]", location, e); - } finally { - if (extParser != null) { - extParser.close(); - } + for (SearchExtBuilder searchExtBuilder : source.ext()) { + context.addSearchExt(searchExtBuilder); } } if (source.version() != null) { diff --git a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java index e83fc91fd20..8a643fe1075 100644 --- a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java @@ -25,7 +25,6 @@ import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -33,13 +32,14 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.script.Script; +import org.elasticsearch.search.SearchExtBuilder; +import org.elasticsearch.search.SearchExtParser; +import org.elasticsearch.search.SearchExtRegistry; import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.AggregatorFactories; import org.elasticsearch.search.aggregations.AggregatorParsers; @@ -108,9 +108,9 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ public static final ParseField SLICE = new ParseField("slice"); public static SearchSourceBuilder fromXContent(QueryParseContext context, AggregatorParsers aggParsers, - Suggesters suggesters) throws IOException { + Suggesters suggesters, SearchExtRegistry searchExtRegistry) throws IOException { SearchSourceBuilder builder = new SearchSourceBuilder(); - builder.parseXContent(context, aggParsers, suggesters); + builder.parseXContent(context, aggParsers, suggesters, searchExtRegistry); return builder; } @@ -164,13 +164,13 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ private SuggestBuilder suggestBuilder; - private List> rescoreBuilders; + private List rescoreBuilders; private ObjectFloatHashMap indexBoost = null; private List stats; - private BytesReference ext = null; + private List extBuilders; private boolean profile = false; @@ -203,18 +203,10 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ postQueryBuilder = in.readOptionalNamedWriteable(QueryBuilder.class); queryBuilder = in.readOptionalNamedWriteable(QueryBuilder.class); if (in.readBoolean()) { - int size = in.readVInt(); - rescoreBuilders = new ArrayList<>(); - for (int i = 0; i < size; i++) { - rescoreBuilders.add(in.readNamedWriteable(RescoreBuilder.class)); - } + rescoreBuilders = in.readNamedWriteableList(RescoreBuilder.class); } if (in.readBoolean()) { - int size = in.readVInt(); - scriptFields = new ArrayList<>(size); - for (int i = 0; i < size; i++) { - scriptFields.add(new ScriptField(in)); - } + scriptFields = in.readList(ScriptField::new); } size = in.readVInt(); if (in.readBoolean()) { @@ -225,18 +217,16 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ } } if (in.readBoolean()) { - int size = in.readVInt(); - stats = new ArrayList<>(); - for (int i = 0; i < size; i++) { - stats.add(in.readString()); - } + stats = in.readList(StreamInput::readString); } suggestBuilder = in.readOptionalWriteable(SuggestBuilder::new); terminateAfter = in.readVInt(); timeout = in.readOptionalWriteable(TimeValue::new); trackScores = in.readBoolean(); version = in.readOptionalBoolean(); - ext = in.readOptionalBytesReference(); + if (in.readBoolean()) { + extBuilders = in.readNamedWriteableList(SearchExtBuilder.class); + } profile = in.readBoolean(); searchAfterBuilder = in.readOptionalWriteable(SearchAfterBuilder::new); sliceBuilder = in.readOptionalWriteable(SliceBuilder::new); @@ -262,18 +252,12 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ boolean hasRescoreBuilders = rescoreBuilders != null; out.writeBoolean(hasRescoreBuilders); if (hasRescoreBuilders) { - out.writeVInt(rescoreBuilders.size()); - for (RescoreBuilder rescoreBuilder : rescoreBuilders) { - out.writeNamedWriteable(rescoreBuilder); - } + out.writeNamedWriteableList(rescoreBuilders); } boolean hasScriptFields = scriptFields != null; out.writeBoolean(hasScriptFields); if (hasScriptFields) { - out.writeVInt(scriptFields.size()); - for (ScriptField scriptField : scriptFields) { - scriptField.writeTo(out); - } + out.writeList(scriptFields); } out.writeVInt(size); boolean hasSorts = sorts != null; @@ -287,17 +271,19 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ boolean hasStats = stats != null; out.writeBoolean(hasStats); if (hasStats) { - out.writeVInt(stats.size()); - for (String stat : stats) { - out.writeString(stat); - } + out.writeStringList(stats); } out.writeOptionalWriteable(suggestBuilder); out.writeVInt(terminateAfter); out.writeOptionalWriteable(timeout); out.writeBoolean(trackScores); out.writeOptionalBoolean(version); - out.writeOptionalBytesReference(ext); + if (extBuilders == null) { + out.writeBoolean(false); + } else { + out.writeBoolean(true); + out.writeNamedWriteableList(extBuilders); + } out.writeBoolean(profile); out.writeOptionalWriteable(searchAfterBuilder); out.writeOptionalWriteable(sliceBuilder); @@ -649,7 +635,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ /** * Gets the bytes representing the rescore builders for this request. */ - public List> rescores() { + public List rescores() { return rescoreBuilders; } @@ -875,13 +861,13 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ return stats; } - public SearchSourceBuilder ext(XContentBuilder ext) { - this.ext = ext.bytes(); + public SearchSourceBuilder ext(List searchExtBuilders) { + this.extBuilders = searchExtBuilders; return this; } - public BytesReference ext() { - return ext; + public List ext() { + return extBuilders; } /** @@ -919,7 +905,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ SearchSourceBuilder rewrittenBuilder = new SearchSourceBuilder(); rewrittenBuilder.aggregations = aggregations; rewrittenBuilder.explain = explain; - rewrittenBuilder.ext = ext; + rewrittenBuilder.extBuilders = extBuilders; rewrittenBuilder.fetchSourceContext = fetchSourceContext; rewrittenBuilder.docValueFields = docValueFields; rewrittenBuilder.storedFieldsContext = storedFieldsContext; @@ -948,9 +934,10 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ /** * Parse some xContent into this SearchSourceBuilder, overwriting any values specified in the xContent. Use this if you need to set up * different defaults than a regular SearchSourceBuilder would have and use - * {@link #fromXContent(QueryParseContext, AggregatorParsers, Suggesters)} if you have normal defaults. + * {@link #fromXContent(QueryParseContext, AggregatorParsers, Suggesters, SearchExtRegistry)} if you have normal defaults. */ - public void parseXContent(QueryParseContext context, AggregatorParsers aggParsers, Suggesters suggesters) + public void parseXContent(QueryParseContext context, AggregatorParsers aggParsers, + Suggesters suggesters, SearchExtRegistry searchExtRegistry) throws IOException { XContentParser parser = context.parser(); @@ -1034,8 +1021,23 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ rescoreBuilders = new ArrayList<>(); rescoreBuilders.add(RescoreBuilder.parseFromXContent(context)); } else if (context.getParseFieldMatcher().match(currentFieldName, EXT_FIELD)) { - XContentBuilder xContentBuilder = XContentFactory.jsonBuilder().copyCurrentStructure(parser); - ext = xContentBuilder.bytes(); + extBuilders = new ArrayList<>(); + String extSectionName = null; + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + extSectionName = parser.currentName(); + } else { + SearchExtParser searchExtParser = searchExtRegistry.lookup(extSectionName, + context.getParseFieldMatcher(), parser.getTokenLocation()); + SearchExtBuilder searchExtBuilder = searchExtParser.fromXContent(parser); + if (searchExtBuilder.getWriteableName().equals(extSectionName) == false) { + throw new IllegalStateException("The parsed [" + searchExtBuilder.getClass().getName() + "] object has a " + + "different writeable name compared to the name of the section that it was parsed from: found [" + + searchExtBuilder.getWriteableName() + "] expected [" + extSectionName + "]"); + } + extBuilders.add(searchExtBuilder); + } + } } else if (context.getParseFieldMatcher().match(currentFieldName, SLICE)) { sliceBuilder = SliceBuilder.fromXContent(context); } else { @@ -1221,12 +1223,12 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ builder.field(STATS_FIELD.getPreferredName(), stats); } - if (ext != null) { - builder.field(EXT_FIELD.getPreferredName()); - try (XContentParser parser = XContentFactory.xContent(XContentType.JSON).createParser(ext)) { - parser.nextToken(); - builder.copyCurrentStructure(parser); + if (extBuilders != null) { + builder.startObject(EXT_FIELD.getPreferredName()); + for (SearchExtBuilder extBuilder : extBuilders) { + extBuilder.toXContent(builder, params); } + builder.endObject(); } } @@ -1344,9 +1346,9 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ @Override public int hashCode() { - return Objects.hash(aggregations, explain, fetchSourceContext, docValueFields, storedFieldsContext, from, - highlightBuilder, indexBoost, minScore, postQueryBuilder, queryBuilder, rescoreBuilders, scriptFields, size, sorts, - searchAfterBuilder, sliceBuilder, stats, suggestBuilder, terminateAfter, timeout, trackScores, version, profile); + return Objects.hash(aggregations, explain, fetchSourceContext, docValueFields, storedFieldsContext, from, highlightBuilder, + indexBoost, minScore, postQueryBuilder, queryBuilder, rescoreBuilders, scriptFields, size, sorts, searchAfterBuilder, + sliceBuilder, stats, suggestBuilder, terminateAfter, timeout, trackScores, version, profile, extBuilders); } @Override @@ -1381,7 +1383,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ && Objects.equals(timeout, other.timeout) && Objects.equals(trackScores, other.trackScores) && Objects.equals(version, other.version) - && Objects.equals(profile, other.profile); + && Objects.equals(profile, other.profile) + && Objects.equals(extBuilders, other.extBuilders); } - } 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 5aacb633416..b1618c3a205 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/DefaultSearchContext.java @@ -53,6 +53,7 @@ import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.similarity.SimilarityService; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.search.SearchExtBuilder; import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.search.aggregations.SearchContextAggregations; import org.elasticsearch.search.dfs.DfsSearchResult; @@ -145,7 +146,7 @@ public class DefaultSearchContext extends SearchContext { private volatile long lastAccessTime = -1; private Profilers profilers; - private final Map searchExtBuilders = new HashMap<>(); + private final Map searchExtBuilders = new HashMap<>(); private final Map, Collector> queryCollectors = new HashMap<>(); private final QueryShardContext queryShardContext; private FetchPhase fetchPhase; @@ -385,13 +386,15 @@ public class DefaultSearchContext extends SearchContext { } @Override - public Object getSearchExtBuilder(String name) { - return searchExtBuilders.get(name); + public void addSearchExt(SearchExtBuilder searchExtBuilder) { + //it's ok to use the writeable name here given that we enforce it to be the same as the name of the element that gets + //parsed by the corresponding parser. There is one single name and one single way to retrieve the parsed object from the context. + searchExtBuilders.put(searchExtBuilder.getWriteableName(), searchExtBuilder); } @Override - public void putSearchExtBuilder(String name, Object searchExtBuilder) { - searchExtBuilders.put(name, searchExtBuilder); + public SearchExtBuilder getSearchExt(String name) { + return searchExtBuilders.get(name); } @Override 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 c9d10a7aa67..18af4873db3 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/FilteredSearchContext.java @@ -38,6 +38,7 @@ import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.similarity.SimilarityService; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.search.SearchExtBuilder; import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.search.aggregations.SearchContextAggregations; import org.elasticsearch.search.dfs.DfsSearchResult; @@ -510,13 +511,13 @@ public abstract class FilteredSearchContext extends SearchContext { } @Override - public void putSearchExtBuilder(String name, Object fetchSubPhaseBuilder) { - in.putSearchExtBuilder(name, fetchSubPhaseBuilder); + public void addSearchExt(SearchExtBuilder searchExtBuilder) { + in.addSearchExt(searchExtBuilder); } @Override - public Object getSearchExtBuilder(String name) { - return in.getSearchExtBuilder(name); + public SearchExtBuilder getSearchExt(String name) { + return in.getSearchExt(name); } @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 e060669fd5c..e96c9856a33 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java @@ -44,6 +44,7 @@ import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.similarity.SimilarityService; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.search.SearchExtBuilder; import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.search.aggregations.SearchContextAggregations; import org.elasticsearch.search.dfs.DfsSearchResult; @@ -184,9 +185,9 @@ public abstract class SearchContext extends AbstractRefCounted implements Releas public abstract SearchContext aggregations(SearchContextAggregations aggregations); - public abstract void putSearchExtBuilder(String name, Object fetchSubPhaseBuilder); + public abstract void addSearchExt(SearchExtBuilder searchExtBuilder); - public abstract Object getSearchExtBuilder(String name); + public abstract SearchExtBuilder getSearchExt(String name); public abstract SearchContextHighlight highlight(); diff --git a/core/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java b/core/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java index 3c9acb4104b..d5b17861e0f 100644 --- a/core/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java +++ b/core/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java @@ -175,6 +175,6 @@ public class MultiSearchRequestTests extends ESTestCase { IndicesQueriesRegistry registry = new IndicesQueriesRegistry(); QueryParser parser = MatchAllQueryBuilder::fromXContent; registry.register(parser, MatchAllQueryBuilder.NAME); - return new SearchRequestParsers(registry, null, null); + return new SearchRequestParsers(registry, null, null, null); } } diff --git a/core/src/test/java/org/elasticsearch/search/SearchRequestTests.java b/core/src/test/java/org/elasticsearch/search/SearchRequestTests.java index 3db3492f415..548a09d37bc 100644 --- a/core/src/test/java/org/elasticsearch/search/SearchRequestTests.java +++ b/core/src/test/java/org/elasticsearch/search/SearchRequestTests.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.indices.IndicesModule; +import org.elasticsearch.search.fetch.FetchSubPhasePluginIT; import org.elasticsearch.test.ESTestCase; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -36,6 +37,7 @@ import org.junit.BeforeClass; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import static java.util.Collections.emptyList; @@ -53,7 +55,8 @@ public class SearchRequestTests extends ESTestCase { bindMapperExtension(); } }; - SearchModule searchModule = new SearchModule(Settings.EMPTY, false, emptyList()) { + SearchModule searchModule = new SearchModule(Settings.EMPTY, false, + Collections.singletonList(new FetchSubPhasePluginIT.FetchTermVectorsPlugin())) { @Override protected void configureSearch() { // Skip me diff --git a/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java b/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java index 0742fca357d..7960e9bf4e8 100644 --- a/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java @@ -59,6 +59,7 @@ import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.SearchModule; import org.elasticsearch.search.SearchRequestParsers; import org.elasticsearch.search.aggregations.AggregationBuilders; +import org.elasticsearch.search.fetch.FetchSubPhasePluginIT; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilderTests; import org.elasticsearch.search.rescore.QueryRescoreBuilderTests; @@ -85,7 +86,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import static java.util.Collections.emptyList; import static org.elasticsearch.test.ClusterServiceUtils.createClusterService; import static org.elasticsearch.test.ClusterServiceUtils.setState; import static org.hamcrest.CoreMatchers.containsString; @@ -132,7 +132,8 @@ public class SearchSourceBuilderTests extends ESTestCase { bindMapperExtension(); } }; - SearchModule searchModule = new SearchModule(settings, false, emptyList()) { + SearchModule searchModule = new SearchModule(settings, false, + Collections.singletonList(new FetchSubPhasePluginIT.FetchTermVectorsPlugin())) { @Override protected void configureSearch() { // Skip me @@ -389,11 +390,7 @@ public class SearchSourceBuilderTests extends ESTestCase { builder.aggregation(AggregationBuilders.avg(randomAsciiOfLengthBetween(5, 20))); } if (randomBoolean()) { - XContentBuilder xContentBuilder = XContentFactory.jsonBuilder(); - xContentBuilder.startObject(); - xContentBuilder.field("term_vectors_fetch", randomAsciiOfLengthBetween(5, 20)); - xContentBuilder.endObject(); - builder.ext(xContentBuilder); + builder.ext(Collections.singletonList(new FetchSubPhasePluginIT.TermVectorsFetchBuilder("test"))); } if (randomBoolean()) { String field = randomBoolean() ? null : randomAsciiOfLengthBetween(5, 20); @@ -431,15 +428,14 @@ public class SearchSourceBuilderTests extends ESTestCase { // test the embedded case } SearchSourceBuilder newBuilder = SearchSourceBuilder.fromXContent(parseContext, searchRequestParsers.aggParsers, - searchRequestParsers.suggesters); + searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers); assertNull(parser.nextToken()); assertEquals(testBuilder, newBuilder); assertEquals(testBuilder.hashCode(), newBuilder.hashCode()); } private static QueryParseContext createParseContext(XContentParser parser) { - QueryParseContext context = new QueryParseContext(searchRequestParsers.queryParsers, parser, parseFieldMatcher); - return context; + return new QueryParseContext(searchRequestParsers.queryParsers, parser, parseFieldMatcher); } public void testSerialization() throws IOException { @@ -497,7 +493,7 @@ public class SearchSourceBuilderTests extends ESTestCase { String restContent = " { \"_source\": { \"includes\": \"include\", \"excludes\": \"*.field2\"}}"; try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) { SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(createParseContext(parser), - searchRequestParsers.aggParsers, searchRequestParsers.suggesters); + searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers); assertArrayEquals(new String[]{"*.field2"}, searchSourceBuilder.fetchSource().excludes()); assertArrayEquals(new String[]{"include"}, searchSourceBuilder.fetchSource().includes()); } @@ -506,7 +502,7 @@ public class SearchSourceBuilderTests extends ESTestCase { String restContent = " { \"_source\": false}"; try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) { SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(createParseContext(parser), - searchRequestParsers.aggParsers, searchRequestParsers.suggesters); + searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers); assertArrayEquals(new String[]{}, searchSourceBuilder.fetchSource().excludes()); assertArrayEquals(new String[]{}, searchSourceBuilder.fetchSource().includes()); assertFalse(searchSourceBuilder.fetchSource().fetchSource()); @@ -519,7 +515,7 @@ public class SearchSourceBuilderTests extends ESTestCase { String restContent = " { \"sort\": \"foo\"}"; try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) { SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(createParseContext(parser), - searchRequestParsers.aggParsers, searchRequestParsers.suggesters); + searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers); assertEquals(1, searchSourceBuilder.sorts().size()); assertEquals(new FieldSortBuilder("foo"), searchSourceBuilder.sorts().get(0)); } @@ -535,7 +531,7 @@ public class SearchSourceBuilderTests extends ESTestCase { " ]}"; try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) { SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(createParseContext(parser), - searchRequestParsers.aggParsers, searchRequestParsers.suggesters); + searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers); assertEquals(5, searchSourceBuilder.sorts().size()); assertEquals(new FieldSortBuilder("post_date"), searchSourceBuilder.sorts().get(0)); assertEquals(new FieldSortBuilder("user"), searchSourceBuilder.sorts().get(1)); @@ -559,7 +555,7 @@ public class SearchSourceBuilderTests extends ESTestCase { "}\n"; try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) { SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(createParseContext(parser), - searchRequestParsers.aggParsers, searchRequestParsers.suggesters); + searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers); assertEquals(1, searchSourceBuilder.aggregations().count()); } } @@ -575,7 +571,7 @@ public class SearchSourceBuilderTests extends ESTestCase { "}\n"; try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) { SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(createParseContext(parser), - searchRequestParsers.aggParsers, searchRequestParsers.suggesters); + searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers); assertEquals(1, searchSourceBuilder.aggregations().count()); } } @@ -601,7 +597,7 @@ public class SearchSourceBuilderTests extends ESTestCase { "}\n"; try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) { SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(createParseContext(parser), - searchRequestParsers.aggParsers, searchRequestParsers.suggesters); + searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers); assertEquals(1, searchSourceBuilder.rescores().size()); assertEquals(new QueryRescorerBuilder(QueryBuilders.matchQuery("content", "baz")).windowSize(50), searchSourceBuilder.rescores().get(0)); @@ -624,7 +620,7 @@ public class SearchSourceBuilderTests extends ESTestCase { "}\n"; try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) { SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.fromXContent(createParseContext(parser), - searchRequestParsers.aggParsers, searchRequestParsers.suggesters); + searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers); assertEquals(1, searchSourceBuilder.rescores().size()); assertEquals(new QueryRescorerBuilder(QueryBuilders.matchQuery("content", "baz")).windowSize(50), searchSourceBuilder.rescores().get(0)); @@ -637,7 +633,7 @@ public class SearchSourceBuilderTests extends ESTestCase { final String query = "{ \"query\": { \"match_all\": {}}, \"timeout\": \"" + timeout + "\"}"; try (XContentParser parser = XContentFactory.xContent(query).createParser(query)) { final SearchSourceBuilder builder = SearchSourceBuilder.fromXContent(createParseContext(parser), - searchRequestParsers.aggParsers, searchRequestParsers.suggesters); + searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers); assertThat(builder.timeout(), equalTo(TimeValue.parseTimeValue(timeout, null, "timeout"))); } } @@ -650,7 +646,7 @@ public class SearchSourceBuilderTests extends ESTestCase { expectThrows( ElasticsearchParseException.class, () -> SearchSourceBuilder.fromXContent(createParseContext(parser), - searchRequestParsers.aggParsers, searchRequestParsers.suggesters)); + searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers)); assertThat(e, hasToString(containsString("unit is missing or unrecognized"))); } } 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 2517f89ec49..87965365fd1 100644 --- a/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java +++ b/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java @@ -27,12 +27,15 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.termvectors.TermVectorsRequest; import org.elasticsearch.action.termvectors.TermVectorsResponse; import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.termvectors.TermVectorsService; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.SearchPlugin; +import org.elasticsearch.search.SearchExtBuilder; import org.elasticsearch.search.SearchExtParser; import org.elasticsearch.search.SearchHitField; import org.elasticsearch.search.builder.SearchSourceBuilder; @@ -49,6 +52,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import static java.util.Collections.singletonList; import static org.elasticsearch.client.Requests.indexRequest; @@ -56,13 +60,18 @@ import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.CoreMatchers.equalTo; -@ClusterScope(scope = Scope.SUITE, supportsDedicatedMasters = false, numDataNodes = 1) +@ClusterScope(scope = Scope.SUITE, supportsDedicatedMasters = false, numDataNodes = 2) public class FetchSubPhasePluginIT extends ESIntegTestCase { @Override protected Collection> nodePlugins() { return Collections.singletonList(FetchTermVectorsPlugin.class); } + @Override + protected Collection> transportClientPlugins() { + return nodePlugins(); + } + @SuppressWarnings("unchecked") public void testPlugin() throws Exception { client().admin() @@ -85,10 +94,8 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { client().admin().indices().prepareRefresh().execute().actionGet(); - XContentBuilder extSource = jsonBuilder().startObject() - .field("term_vectors_fetch", "test") - .endObject(); - SearchResponse response = client().prepareSearch().setSource(new SearchSourceBuilder().ext(extSource)).get(); + SearchResponse response = client().prepareSearch().setSource(new SearchSourceBuilder() + .ext(Collections.singletonList(new TermVectorsFetchBuilder("test")))).get(); assertSearchResponse(response); assertThat(((Map) response.getHits().getAt(0).field("term_vectors_fetch").getValues().get(0)).get("i"), equalTo(2)); @@ -105,8 +112,9 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { } @Override - public List getSearchExtParsers() { - return Collections.singletonList(TermVectorsFetchParser.INSTANCE); + public List> getSearchExts() { + return Collections.singletonList(new SearchExtSpec<>(TermVectorsFetchSubPhase.NAME, + TermVectorsFetchBuilder::new, TermVectorsFetchParser.INSTANCE)); } } @@ -115,7 +123,7 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { @Override public void hitExecute(SearchContext context, HitContext hitContext) { - TermVectorsFetchBuilder fetchSubPhaseBuilder = (TermVectorsFetchBuilder)context.getSearchExtBuilder(NAME); + TermVectorsFetchBuilder fetchSubPhaseBuilder = (TermVectorsFetchBuilder)context.getSearchExt(NAME); if (fetchSubPhaseBuilder == null) { return; } @@ -145,7 +153,7 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { } } - public static final class TermVectorsFetchParser implements SearchExtParser { + public static final class TermVectorsFetchParser implements SearchExtParser { private static final TermVectorsFetchParser INSTANCE = new TermVectorsFetchParser(); @@ -153,12 +161,7 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { } @Override - public String getName() { - return TermVectorsFetchSubPhase.NAME; - } - - @Override - public TermVectorsFetchBuilder parse(XContentParser parser) throws Exception { + public TermVectorsFetchBuilder fromXContent(XContentParser parser) throws IOException { String field; XContentParser.Token token = parser.currentToken(); if (token == XContentParser.Token.VALUE_STRING) { @@ -173,15 +176,51 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { } } - public static final class TermVectorsFetchBuilder { + public static final class TermVectorsFetchBuilder extends SearchExtBuilder { private final String field; - private TermVectorsFetchBuilder(String field) { + public TermVectorsFetchBuilder(String field) { this.field = field; } + public TermVectorsFetchBuilder(StreamInput in) throws IOException { + this.field = in.readString(); + } + public String getField() { return field; } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + TermVectorsFetchBuilder that = (TermVectorsFetchBuilder) o; + return Objects.equals(field, that.field); + } + + @Override + public int hashCode() { + return Objects.hash(field); + } + + @Override + public String getWriteableName() { + return TermVectorsFetchSubPhase.NAME; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(field); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + return builder.field(TermVectorsFetchSubPhase.NAME, field); + } } } diff --git a/core/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java b/core/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java index c2e99b09dc3..eb37299306c 100644 --- a/core/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java +++ b/core/src/test/java/org/elasticsearch/search/internal/ShardSearchTransportRequestTests.java @@ -34,12 +34,14 @@ import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.indices.IndicesModule; import org.elasticsearch.search.SearchModule; import org.elasticsearch.search.SearchRequestTests; +import org.elasticsearch.search.fetch.FetchSubPhasePluginIT; import org.elasticsearch.test.ESTestCase; import org.junit.AfterClass; import org.junit.BeforeClass; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import static java.util.Collections.emptyList; @@ -56,7 +58,8 @@ public class ShardSearchTransportRequestTests extends ESTestCase { bindMapperExtension(); } }; - SearchModule searchModule = new SearchModule(Settings.EMPTY, false, emptyList()) { + SearchModule searchModule = new SearchModule(Settings.EMPTY, false, + Collections.singletonList(new FetchSubPhasePluginIT.FetchTermVectorsPlugin())) { @Override protected void configureSearch() { // Skip me diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java index 473e287c9a2..9df5a29b9c7 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java @@ -32,14 +32,11 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryParseContext; -import org.elasticsearch.indices.query.IndicesQueriesRegistry; import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.SearchRequestParsers; -import org.elasticsearch.search.aggregations.AggregatorParsers; import org.elasticsearch.search.builder.SearchSourceBuilder; -import org.elasticsearch.search.suggest.Suggesters; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; @@ -86,7 +83,7 @@ public class TransportSearchTemplateAction extends HandledTransportAction() { diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/TransportMultiPercolateAction.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/TransportMultiPercolateAction.java index 44914e140b1..8b290d4c9c4 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/TransportMultiPercolateAction.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/TransportMultiPercolateAction.java @@ -37,9 +37,7 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.indices.query.IndicesQueriesRegistry; import org.elasticsearch.search.SearchRequestParsers; -import org.elasticsearch.search.aggregations.AggregatorParsers; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; @@ -160,8 +158,7 @@ public class TransportMultiPercolateAction extends HandledTransportAction searchExtBuilders = new HashMap<>(); + private final Map searchExtBuilders = new HashMap<>(); public TestSearchContext(ThreadPool threadPool, BigArrays bigArrays, ScriptService scriptService, IndexService indexService) { super(ParseFieldMatcher.STRICT); @@ -196,12 +197,12 @@ public class TestSearchContext extends SearchContext { } @Override - public void putSearchExtBuilder(String name, Object searchExtBuilders) { - this.searchExtBuilders.put(name, searchExtBuilders); + public void addSearchExt(SearchExtBuilder searchExtBuilder) { + searchExtBuilders.put(searchExtBuilder.getWriteableName(), searchExtBuilder); } @Override - public Object getSearchExtBuilder(String name) { + public SearchExtBuilder getSearchExt(String name) { return searchExtBuilders.get(name); } From 17d48c1ff674c707aaef4a9009986a16936df2ae Mon Sep 17 00:00:00 2001 From: javanna Date: Fri, 9 Sep 2016 21:17:16 +0200 Subject: [PATCH 12/13] adjust SearchExtBuilder javadocs --- .../org/elasticsearch/search/SearchExtBuilder.java | 11 +++++++---- .../elasticsearch/search/SearchRequestParsers.java | 6 +++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/SearchExtBuilder.java b/core/src/main/java/org/elasticsearch/search/SearchExtBuilder.java index 205dba1ba0e..8d75216fe12 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchExtBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/SearchExtBuilder.java @@ -20,25 +20,28 @@ package org.elasticsearch.search; import org.elasticsearch.common.io.stream.NamedWriteable; +import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.plugins.SearchPlugin; +import org.elasticsearch.plugins.SearchPlugin.SearchExtSpec; /** * Intermediate serializable representation of a search ext section. To be subclassed by plugins that support * a custom section as part of a search request, which will be provided within the ext element. - * Any state needs to be serialized as part of the {@link org.elasticsearch.common.io.stream.Writeable#writeTo(StreamOutput)} method and - * read from the incoming stream, usually done adding a constructor that takes {@link org.elasticsearch.common.io.stream.StreamInput} as + * Any state needs to be serialized as part of the {@link Writeable#writeTo(StreamOutput)} method and + * read from the incoming stream, usually done adding a constructor that takes {@link StreamInput} as * an argument. * * Registration happens through {@link SearchPlugin#getSearchExts()}, which also needs a {@link SearchExtParser} that's able to parse * the incoming request from the REST layer into the proper {@link SearchExtBuilder} subclass. * * {@link #getWriteableName()} must return the same name as the one used for the registration - * of the {@link org.elasticsearch.plugins.SearchPlugin.SearchExtSpec}. + * of the {@link SearchExtSpec}. * * @see SearchExtParser - * @see org.elasticsearch.plugins.SearchPlugin.SearchExtSpec + * @see SearchExtSpec */ public abstract class SearchExtBuilder implements NamedWriteable, ToXContent { diff --git a/core/src/main/java/org/elasticsearch/search/SearchRequestParsers.java b/core/src/main/java/org/elasticsearch/search/SearchRequestParsers.java index e15557c2389..ba3afc15771 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchRequestParsers.java +++ b/core/src/main/java/org/elasticsearch/search/SearchRequestParsers.java @@ -38,7 +38,7 @@ public class SearchRequestParsers { * Query parsers that may be used in search requests. * @see org.elasticsearch.index.query.QueryParseContext * @see org.elasticsearch.search.builder.SearchSourceBuilder#fromXContent(QueryParseContext, AggregatorParsers, - * Suggesters, SearchExtRegistry) + * Suggesters, SearchExtRegistry) */ public final IndicesQueriesRegistry queryParsers; @@ -47,7 +47,7 @@ public class SearchRequestParsers { /** * Agg and pipeline agg parsers that may be used in search requests. * @see org.elasticsearch.search.builder.SearchSourceBuilder#fromXContent(QueryParseContext, AggregatorParsers, - * Suggesters, SearchExtRegistry) + * Suggesters, SearchExtRegistry) */ public final AggregatorParsers aggParsers; @@ -55,7 +55,7 @@ public class SearchRequestParsers { /** * Suggesters that may be used in search requests. * @see org.elasticsearch.search.builder.SearchSourceBuilder#fromXContent(QueryParseContext, AggregatorParsers, - * Suggesters, SearchExtRegistry) + * Suggesters, SearchExtRegistry) */ public final Suggesters suggesters; From 9a84cb99f4af8c2a17459ab565ca32380acb8707 Mon Sep 17 00:00:00 2001 From: javanna Date: Fri, 9 Sep 2016 21:24:18 +0200 Subject: [PATCH 13/13] remove writeBoolean from searchExtBuilders serialization in SearchSourceBuilder The list is not optional anymore, default is empty list --- .../search/builder/SearchSourceBuilder.java | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java index 8a643fe1075..84fc26fdb2f 100644 --- a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java @@ -170,7 +170,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ private List stats; - private List extBuilders; + private List extBuilders = Collections.emptyList(); private boolean profile = false; @@ -224,9 +224,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ timeout = in.readOptionalWriteable(TimeValue::new); trackScores = in.readBoolean(); version = in.readOptionalBoolean(); - if (in.readBoolean()) { - extBuilders = in.readNamedWriteableList(SearchExtBuilder.class); - } + extBuilders = in.readNamedWriteableList(SearchExtBuilder.class); profile = in.readBoolean(); searchAfterBuilder = in.readOptionalWriteable(SearchAfterBuilder::new); sliceBuilder = in.readOptionalWriteable(SliceBuilder::new); @@ -278,12 +276,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ out.writeOptionalWriteable(timeout); out.writeBoolean(trackScores); out.writeOptionalBoolean(version); - if (extBuilders == null) { - out.writeBoolean(false); - } else { - out.writeBoolean(true); - out.writeNamedWriteableList(extBuilders); - } + out.writeNamedWriteableList(extBuilders); out.writeBoolean(profile); out.writeOptionalWriteable(searchAfterBuilder); out.writeOptionalWriteable(sliceBuilder); @@ -862,7 +855,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ } public SearchSourceBuilder ext(List searchExtBuilders) { - this.extBuilders = searchExtBuilders; + this.extBuilders = Objects.requireNonNull(searchExtBuilders, "searchExtBuilders must not be null"); return this; }