review comment fixes

This commit is contained in:
Colin Goodheart-Smithe 2015-10-15 11:34:31 +01:00
parent c618f75b76
commit 63c51b78b2
13 changed files with 79 additions and 59 deletions

View File

@ -100,6 +100,7 @@ public class SearchRequest extends ActionRequest<SearchRequest> implements Indic
*/
public SearchRequest(ActionRequest request) {
super(request);
this.source = new SearchSourceBuilder();
}
/**
@ -107,7 +108,7 @@ public class SearchRequest extends ActionRequest<SearchRequest> implements Indic
* will run against all indices.
*/
public SearchRequest(String... indices) {
indices(indices);
this(indices, new SearchSourceBuilder());
}
/**
@ -331,9 +332,7 @@ public class SearchRequest extends ActionRequest<SearchRequest> implements Indic
if (in.readBoolean()) {
scroll = readScroll(in);
}
if (in.readBoolean()) {
source = SearchSourceBuilder.readSearchSourceFrom(in);
}
types = in.readStringArray();
indicesOptions = IndicesOptions.readIndicesOptions(in);
@ -361,12 +360,7 @@ public class SearchRequest extends ActionRequest<SearchRequest> implements Indic
out.writeBoolean(true);
scroll.writeTo(out);
}
if (source == null) {
out.writeBoolean(false);
} else {
out.writeBoolean(true);
source.writeTo(out);
}
out.writeStringArray(types);
indicesOptions.writeIndicesOptions(out);
out.writeOptionalBoolean(requestCache);

View File

@ -353,10 +353,10 @@ public class SearchRequestBuilder extends ActionRequestBuilder<SearchRequest, Se
}
/**
* Adds the fields to load and return as part of the search request. If none are specified,
* the source of the document will be returned.
* Sets the fields to load and return as part of the search request. If none
* are specified, the source of the document will be returned.
*/
public SearchRequestBuilder addFields(String... fields) {
public SearchRequestBuilder fields(String... fields) {
sourceBuilder().fields(Arrays.asList(fields));
return this;
}

View File

@ -72,7 +72,7 @@ public class RestPutWarmerAction extends BaseRestHandler {
PutWarmerRequest putWarmerRequest = new PutWarmerRequest(request.param("name"));
BytesReference sourceBytes = RestActions.getRestContent(request);
SearchSourceBuilder source = RestActions.getRestSearchSource(sourceBytes, queryRegistry);
SearchSourceBuilder source = RestActions.getRestSearchSource(sourceBytes, queryRegistry, parseFieldMatcher);
SearchRequest searchRequest = new SearchRequest(Strings.splitStringByCommaToArray(request.param("index")))
.types(Strings.splitStringByCommaToArray(request.param("type")))
.requestCache(request.paramAsBoolean("request_cache", null)).source(source);

View File

@ -70,6 +70,7 @@ public class RestCountAction extends AbstractCatAction {
String source = request.param("source");
if (source != null) {
QueryParseContext context = new QueryParseContext(indicesQueriesRegistry);
context.parseFieldMatcher(parseFieldMatcher);
countRequest.query(RestActions.getQueryContent(new BytesArray(source), context));
} else {
QueryBuilder<?> queryBuilder = RestActions.urlParamsToQueryBuilder(request);

View File

@ -71,6 +71,7 @@ public class RestCountAction extends BaseRestHandler {
if (RestActions.hasBodyContent(request)) {
BytesReference restContent = RestActions.getRestContent(request);
QueryParseContext context = new QueryParseContext(indicesQueriesRegistry);
context.parseFieldMatcher(parseFieldMatcher);
countRequest.query(RestActions.getQueryContent(restContent, context));
} else {
QueryBuilder<?> queryBuilder = RestActions.urlParamsToQueryBuilder(request);

View File

@ -24,6 +24,7 @@ import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.client.Client;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseFieldMatcher;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.inject.Inject;
@ -90,7 +91,9 @@ public class RestMultiSearchAction extends BaseRestHandler {
String path = request.path();
boolean isTemplateRequest = isTemplateRequest(path);
IndicesOptions indicesOptions = IndicesOptions.fromRequest(request, multiSearchRequest.indicesOptions());
parseRequest(multiSearchRequest, RestActions.getRestContent(request), isTemplateRequest, indices, types, request.param("search_type"), request.param("routing"), indicesOptions, allowExplicitIndex, indicesQueriesRegistry);
parseRequest(multiSearchRequest, RestActions.getRestContent(request), isTemplateRequest, indices, types,
request.param("search_type"), request.param("routing"), indicesOptions, allowExplicitIndex, indicesQueriesRegistry,
parseFieldMatcher);
client.multiSearch(multiSearchRequest, new RestToXContentListener<>(channel));
}
@ -104,7 +107,8 @@ public class RestMultiSearchAction extends BaseRestHandler {
@Nullable String searchType,
@Nullable String routing,
IndicesOptions indicesOptions,
boolean allowExplicitIndex, IndicesQueriesRegistry indicesQueriesRegistry) throws Exception {
boolean allowExplicitIndex, IndicesQueriesRegistry indicesQueriesRegistry,
ParseFieldMatcher parseFieldMatcher) throws Exception {
XContent xContent = XContentFactory.xContent(data);
int from = 0;
int length = data.length();
@ -178,6 +182,7 @@ public class RestMultiSearchAction extends BaseRestHandler {
if (isTemplateRequest) {
try (XContentParser parser = XContentFactory.xContent(slice).createParser(slice)) {
queryParseContext.reset(parser);
queryParseContext.parseFieldMatcher(parseFieldMatcher);
Template template = TemplateQueryParser.parse(parser, queryParseContext.parseFieldMatcher(), "params", "template");
searchRequest.template(template);
}

View File

@ -110,12 +110,13 @@ public class RestSearchAction extends BaseRestHandler {
if (isTemplateRequest) {
try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) {
context.reset(parser);
context.parseFieldMatcher(parseFieldMatcher);
Template template = TemplateQueryParser.parse(parser, context.parseFieldMatcher(), "params", "template");
searchRequest.template(template);
}
builder = null;
} else {
builder = RestActions.getRestSearchSource(restContent, indicesQueriesRegistry);
builder = RestActions.getRestSearchSource(restContent, indicesQueriesRegistry, parseFieldMatcher);
}
} else {
builder = null;
@ -155,7 +156,7 @@ public class RestSearchAction extends BaseRestHandler {
return searchRequest;
}
public static boolean parseSearchSource(final SearchSourceBuilder searchSourceBuilder, RestRequest request) {
private static boolean parseSearchSource(final SearchSourceBuilder searchSourceBuilder, RestRequest request) {
boolean modified = false;
QueryBuilder<?> queryBuilder = RestActions.urlParamsToQueryBuilder(request);

View File

@ -23,6 +23,7 @@ import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.action.ShardOperationFailedException;
import org.elasticsearch.action.support.broadcast.BroadcastResponse;
import org.elasticsearch.common.ParseFieldMatcher;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.lucene.uid.Versions;
@ -112,11 +113,13 @@ public class RestActions {
return queryBuilder;
}
public static SearchSourceBuilder getRestSearchSource(BytesReference sourceBytes, IndicesQueriesRegistry queryRegistry)
public static SearchSourceBuilder getRestSearchSource(BytesReference sourceBytes, IndicesQueriesRegistry queryRegistry,
ParseFieldMatcher parseFieldMatcher)
throws IOException {
XContentParser parser = XContentFactory.xContent(sourceBytes).createParser(sourceBytes);
QueryParseContext queryParseContext = new QueryParseContext(queryRegistry);
queryParseContext.reset(parser);
queryParseContext.parseFieldMatcher(parseFieldMatcher);
SearchSourceBuilder source = SearchSourceBuilder.parseSearchSource(parser, queryParseContext);
return source;
}
@ -142,7 +145,11 @@ public class RestActions {
public static QueryBuilder<?> getQueryContent(BytesReference source, QueryParseContext context) {
try (XContentParser requestParser = XContentFactory.xContent(source).createParser(source)) {
// Save the parseFieldMatcher because its about to be trashed in the
// QueryParseContext
ParseFieldMatcher parseFieldMatcher = context.parseFieldMatcher();
context.reset(requestParser);
context.parseFieldMatcher(parseFieldMatcher);
return context.parseInnerQueryBuilder();
} catch (IOException e) {
throw new ElasticsearchException("failed to parse source", e);

View File

@ -578,6 +578,7 @@ public class SearchService extends AbstractLifecycleComponent<SearchService> {
try (XContentParser parser = XContentFactory.xContent(run).createParser(run)) {
QueryParseContext queryParseContext = new QueryParseContext(indexService.queryParserService().indicesQueriesRegistry());
queryParseContext.reset(parser);
queryParseContext.parseFieldMatcher(parseFieldMatcher);
parseSource(context, SearchSourceBuilder.parseSearchSource(parser, queryParseContext));
}
}
@ -686,10 +687,10 @@ public class SearchService extends AbstractLifecycleComponent<SearchService> {
}
}
if (source.query() != null) {
context.parsedQuery(context.queryParserService().parse(source.query()));
context.parsedQuery(context.queryParserService().toQuery(source.query()));
}
if (source.postFilter() != null) {
context.parsedPostFilter(context.queryParserService().parse(source.postFilter()));
context.parsedPostFilter(context.queryParserService().toQuery(source.postFilter()));
}
if (source.sorts() != null) {
XContentParser completeSortParser = null;
@ -1187,9 +1188,12 @@ public class SearchService extends AbstractLifecycleComponent<SearchService> {
try {
long now = System.nanoTime();
final IndexService indexService = indicesService.indexServiceSafe(indexShard.shardId().index().name());
QueryParseContext queryParseContext = new QueryParseContext(indexService.queryParserService().indicesQueriesRegistry());
// NOCOMMIT get a parseFieldMatcher from somewhere and set it on the queryParseContext
queryParseContext.parseFieldMatcher(indexService.queryParserService().parseFieldMatcher());
ShardSearchRequest request = new ShardSearchLocalRequest(indexShard.shardId(), indexMetaData
.getNumberOfShards(),
SearchType.QUERY_THEN_FETCH, entry.source().build(new QueryParseContext(indexService.queryParserService().indicesQueriesRegistry())), entry.types(), entry.requestCache());
SearchType.QUERY_THEN_FETCH, entry.source().build(queryParseContext), entry.types(), entry.requestCache());
context = createContext(request, warmerContext.searcher());
// if we use sort, we need to do query to sort on
// it and load relevant field data

View File

@ -20,18 +20,19 @@
package org.elasticsearch.action.search;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.common.ParseFieldMatcher;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.query.MatchAllQueryParser;
import org.elasticsearch.indices.query.IndicesQueriesRegistry;
import org.elasticsearch.rest.action.search.RestMultiSearchAction;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.test.StreamsUtils;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.StreamsUtils;
import org.junit.Test;
import java.io.IOException;
@ -46,7 +47,8 @@ public class MultiSearchRequestTests extends ESTestCase {
public void simpleAdd() throws Exception {
IndicesQueriesRegistry registry = new IndicesQueriesRegistry(Settings.EMPTY, Collections.singleton(new MatchAllQueryParser()), new NamedWriteableRegistry());
byte[] data = StreamsUtils.copyToBytesFromClasspath("/org/elasticsearch/action/search/simple-msearch1.json");
MultiSearchRequest request = RestMultiSearchAction.parseRequest(new MultiSearchRequest(), new BytesArray(data), false, null, null, null, null, IndicesOptions.strictExpandOpenAndForbidClosed(),true, registry);
MultiSearchRequest request = RestMultiSearchAction.parseRequest(new MultiSearchRequest(), new BytesArray(data), false, null, null,
null, null, IndicesOptions.strictExpandOpenAndForbidClosed(), true, registry, ParseFieldMatcher.EMPTY);
assertThat(request.requests().size(), equalTo(8));
assertThat(request.requests().get(0).indices()[0], equalTo("test"));
assertThat(request.requests().get(0).indicesOptions(), equalTo(IndicesOptions.fromOptions(true, true, true, true, IndicesOptions.strictExpandOpenAndForbidClosed())));
@ -73,7 +75,8 @@ public class MultiSearchRequestTests extends ESTestCase {
public void simpleAdd2() throws Exception {
IndicesQueriesRegistry registry = new IndicesQueriesRegistry(Settings.EMPTY, Collections.singleton(new MatchAllQueryParser()), new NamedWriteableRegistry());
byte[] data = StreamsUtils.copyToBytesFromClasspath("/org/elasticsearch/action/search/simple-msearch2.json");
MultiSearchRequest request =RestMultiSearchAction.parseRequest(new MultiSearchRequest(), new BytesArray(data), false, null, null, null, null, IndicesOptions.strictExpandOpenAndForbidClosed(), true, registry);
MultiSearchRequest request = RestMultiSearchAction.parseRequest(new MultiSearchRequest(), new BytesArray(data), false, null, null,
null, null, IndicesOptions.strictExpandOpenAndForbidClosed(), true, registry, ParseFieldMatcher.EMPTY);
assertThat(request.requests().size(), equalTo(5));
assertThat(request.requests().get(0).indices()[0], equalTo("test"));
assertThat(request.requests().get(0).types().length, equalTo(0));
@ -92,7 +95,8 @@ public class MultiSearchRequestTests extends ESTestCase {
public void simpleAdd3() throws Exception {
IndicesQueriesRegistry registry = new IndicesQueriesRegistry(Settings.EMPTY, Collections.singleton(new MatchAllQueryParser()), new NamedWriteableRegistry());
byte[] data = StreamsUtils.copyToBytesFromClasspath("/org/elasticsearch/action/search/simple-msearch3.json");
MultiSearchRequest request =RestMultiSearchAction.parseRequest(new MultiSearchRequest(), new BytesArray(data), false, null, null, null, null, IndicesOptions.strictExpandOpenAndForbidClosed(), true, registry);
MultiSearchRequest request = RestMultiSearchAction.parseRequest(new MultiSearchRequest(), new BytesArray(data), false, null, null,
null, null, IndicesOptions.strictExpandOpenAndForbidClosed(), true, registry, ParseFieldMatcher.EMPTY);
assertThat(request.requests().size(), equalTo(4));
assertThat(request.requests().get(0).indices()[0], equalTo("test0"));
assertThat(request.requests().get(0).indices()[1], equalTo("test1"));
@ -112,7 +116,8 @@ public class MultiSearchRequestTests extends ESTestCase {
public void simpleAdd4() throws Exception {
IndicesQueriesRegistry registry = new IndicesQueriesRegistry(Settings.EMPTY, Collections.singleton(new MatchAllQueryParser()), new NamedWriteableRegistry());
byte[] data = StreamsUtils.copyToBytesFromClasspath("/org/elasticsearch/action/search/simple-msearch4.json");
MultiSearchRequest request = RestMultiSearchAction.parseRequest(new MultiSearchRequest(), new BytesArray(data), false, null, null, null, null, IndicesOptions.strictExpandOpenAndForbidClosed(), true, registry);
MultiSearchRequest request = RestMultiSearchAction.parseRequest(new MultiSearchRequest(), new BytesArray(data), false, null, null,
null, null, IndicesOptions.strictExpandOpenAndForbidClosed(), true, registry, ParseFieldMatcher.EMPTY);
assertThat(request.requests().size(), equalTo(3));
assertThat(request.requests().get(0).indices()[0], equalTo("test0"));
assertThat(request.requests().get(0).indices()[1], equalTo("test1"));
@ -134,7 +139,8 @@ public class MultiSearchRequestTests extends ESTestCase {
public void simpleAdd5() throws Exception {
IndicesQueriesRegistry registry = new IndicesQueriesRegistry(Settings.EMPTY, Collections.singleton(new MatchAllQueryParser()), new NamedWriteableRegistry());
byte[] data = StreamsUtils.copyToBytesFromClasspath("/org/elasticsearch/action/search/simple-msearch5.json");
MultiSearchRequest request = RestMultiSearchAction.parseRequest(new MultiSearchRequest(), new BytesArray(data), true, null, null, null, null, IndicesOptions.strictExpandOpenAndForbidClosed(), true, registry);
MultiSearchRequest request = RestMultiSearchAction.parseRequest(new MultiSearchRequest(), new BytesArray(data), true, null, null,
null, null, IndicesOptions.strictExpandOpenAndForbidClosed(), true, registry, ParseFieldMatcher.EMPTY);
assertThat(request.requests().size(), equalTo(3));
assertThat(request.requests().get(0).indices()[0], equalTo("test0"));
assertThat(request.requests().get(0).indices()[1], equalTo("test1"));

View File

@ -209,7 +209,7 @@ public class ChildQuerySearchIT extends ESIntegTestCase {
refresh();
// TEST FETCHING _parent from child
SearchResponse searchResponse = client().prepareSearch("test").setQuery(idsQuery("child").addIds("c1")).addFields("_parent").execute()
SearchResponse searchResponse = client().prepareSearch("test").setQuery(idsQuery("child").addIds("c1")).fields("_parent").execute()
.actionGet();
assertNoFailures(searchResponse);
assertThat(searchResponse.getHits().totalHits(), equalTo(1l));
@ -217,7 +217,7 @@ public class ChildQuerySearchIT extends ESIntegTestCase {
assertThat(searchResponse.getHits().getAt(0).field("_parent").value().toString(), equalTo("p1"));
// TEST matching on parent
searchResponse = client().prepareSearch("test").setQuery(termQuery("_parent", "p1")).addFields("_parent").get();
searchResponse = client().prepareSearch("test").setQuery(termQuery("_parent", "p1")).fields("_parent").get();
assertNoFailures(searchResponse);
assertThat(searchResponse.getHits().totalHits(), equalTo(2l));
assertThat(searchResponse.getHits().getAt(0).id(), anyOf(equalTo("c1"), equalTo("c2")));
@ -225,7 +225,7 @@ public class ChildQuerySearchIT extends ESIntegTestCase {
assertThat(searchResponse.getHits().getAt(1).id(), anyOf(equalTo("c1"), equalTo("c2")));
assertThat(searchResponse.getHits().getAt(1).field("_parent").value().toString(), equalTo("p1"));
searchResponse = client().prepareSearch("test").setQuery(queryStringQuery("_parent:p1")).addFields("_parent").get();
searchResponse = client().prepareSearch("test").setQuery(queryStringQuery("_parent:p1")).fields("_parent").get();
assertNoFailures(searchResponse);
assertThat(searchResponse.getHits().totalHits(), equalTo(2l));
assertThat(searchResponse.getHits().getAt(0).id(), anyOf(equalTo("c1"), equalTo("c2")));

View File

@ -71,6 +71,7 @@ public class RestDeleteByQueryAction extends BaseRestHandler {
XContentParser requestParser = XContentFactory.xContent(request.content()).createParser(request.content());
QueryParseContext context = new QueryParseContext(indicesQueriesRegistry);
context.reset(requestParser);
context.parseFieldMatcher(parseFieldMatcher);
final QueryBuilder<?> builder = context.parseInnerQueryBuilder();
delete.query(builder);
} else {

View File

@ -197,7 +197,7 @@ public class TransportDeleteByQueryActionTests extends ESSingleNodeTestCase {
SearchResponse searchResponse = client().prepareSearch("test")
.setScroll(TimeValue.timeValueSeconds(10))
.setQuery(boolQuery().must(rangeQuery("num").lte(limit)))
.addFields("_routing", "_parent")
.fields("_routing", "_parent")
.setFetchSource(false)
.setVersion(true)
.get();