From 90ab460fccd52db2fa98890cd1268538f57b1c93 Mon Sep 17 00:00:00 2001 From: javanna Date: Fri, 9 Sep 2016 15:28:12 +0200 Subject: [PATCH] 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); }