From 7fbd565c5ccb47c5254e6d2ba3d017968a82f864 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Mon, 5 Oct 2015 13:27:14 +0100 Subject: [PATCH] review comment fixes --- .../warmer/get/GetWarmersResponse.java | 12 ++++++--- .../action/count/CountRequest.java | 27 +++++-------------- .../action/count/CountRequestBuilder.java | 17 ++---------- .../termvectors/dfs/DfsOnlyRequest.java | 5 +++- .../cluster/metadata/IndexMetaData.java | 18 ++++--------- .../warmer/put/RestPutWarmerAction.java | 8 +----- .../rest/action/cat/RestCountAction.java | 7 ++--- .../rest/action/count/RestCountAction.java | 6 ++--- .../rest/action/search/RestSearchAction.java | 5 +--- .../rest/action/support/RestActions.java | 13 +++++++++ .../count/CountRequestBuilderTests.java | 16 ----------- 11 files changed, 45 insertions(+), 89 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/admin/indices/warmer/get/GetWarmersResponse.java b/core/src/main/java/org/elasticsearch/action/admin/indices/warmer/get/GetWarmersResponse.java index 4719cb8c597..57e0b746496 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/indices/warmer/get/GetWarmersResponse.java +++ b/core/src/main/java/org/elasticsearch/action/admin/indices/warmer/get/GetWarmersResponse.java @@ -25,7 +25,6 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.warmer.IndexWarmersMetaData; import java.io.IOException; @@ -69,7 +68,10 @@ public class GetWarmersResponse extends ActionResponse { for (int j = 0; j < valueSize; j++) { String name = in.readString(); String[] types = in.readStringArray(); - IndexWarmersMetaData.SearchSource source = new IndexWarmersMetaData.SearchSource(in); + IndexWarmersMetaData.SearchSource source = null; + if (in.readBoolean()) { + source = new IndexWarmersMetaData.SearchSource(in); + } Boolean queryCache = null; queryCache = in.readOptionalBoolean(); warmerEntryBuilder.add(new IndexWarmersMetaData.Entry( @@ -94,7 +96,11 @@ public class GetWarmersResponse extends ActionResponse { for (IndexWarmersMetaData.Entry warmerEntry : indexEntry.value) { out.writeString(warmerEntry.name()); out.writeStringArray(warmerEntry.types()); - warmerEntry.source().writeTo(out); + boolean hasWarmerSource = warmerEntry != null; + out.writeBoolean(hasWarmerSource); + if (hasWarmerSource) { + warmerEntry.source().writeTo(out); + } out.writeOptionalBoolean(warmerEntry.requestCache()); } } diff --git a/core/src/main/java/org/elasticsearch/action/count/CountRequest.java b/core/src/main/java/org/elasticsearch/action/count/CountRequest.java index 80c03e6f073..a74ac54e6ce 100644 --- a/core/src/main/java/org/elasticsearch/action/count/CountRequest.java +++ b/core/src/main/java/org/elasticsearch/action/count/CountRequest.java @@ -19,26 +19,18 @@ package org.elasticsearch.action.count; -import org.elasticsearch.ElasticsearchGenerationException; import org.elasticsearch.action.search.SearchRequest; -import org.elasticsearch.action.support.QuerySourceBuilder; import org.elasticsearch.action.support.broadcast.BroadcastRequest; -import org.elasticsearch.client.Requests; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.bytes.BytesArray; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.search.builder.SearchSourceBuilder; import java.io.IOException; import java.util.Arrays; -import java.util.Map; import static org.elasticsearch.search.internal.SearchContext.DEFAULT_TERMINATE_AFTER; @@ -66,7 +58,7 @@ public class CountRequest extends BroadcastRequest { private int terminateAfter = DEFAULT_TERMINATE_AFTER; - private SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); + private QueryBuilder queryBuilder = null; /** * Constructs a new count request against the provided indices. No indices provided means it will @@ -96,13 +88,8 @@ public class CountRequest extends BroadcastRequest { /** * The query to execute */ - public CountRequest query(QueryBuilder queryBuilder) { - this.searchSourceBuilder = new SearchSourceBuilder().query(queryBuilder); - return this; - } - - public CountRequest searchSource(SearchSourceBuilder searchSourceBuilder) { - this.searchSourceBuilder = searchSourceBuilder; + public CountRequest query(QueryBuilder queryBuilder) { + this.queryBuilder = queryBuilder; return this; } @@ -184,7 +171,7 @@ public class CountRequest extends BroadcastRequest { public String toString() { String sSource = "_na_"; try { - sSource = XContentHelper.toString(searchSourceBuilder); + sSource = XContentHelper.toString(queryBuilder); } catch (Exception e) { // ignore } @@ -192,6 +179,8 @@ public class CountRequest extends BroadcastRequest { } public SearchRequest toSearchRequest() { + SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); + searchSourceBuilder.query(queryBuilder); searchSourceBuilder.size(0); if (minScore() != DEFAULT_MIN_SCORE) { searchSourceBuilder.minScore(minScore()); @@ -207,8 +196,4 @@ public class CountRequest extends BroadcastRequest { searchRequest.preference(preference()); return searchRequest; } - - SearchSourceBuilder sourceBuilder() { - return searchSourceBuilder; - } } diff --git a/core/src/main/java/org/elasticsearch/action/count/CountRequestBuilder.java b/core/src/main/java/org/elasticsearch/action/count/CountRequestBuilder.java index 068e197eefd..c10c54e14de 100644 --- a/core/src/main/java/org/elasticsearch/action/count/CountRequestBuilder.java +++ b/core/src/main/java/org/elasticsearch/action/count/CountRequestBuilder.java @@ -19,7 +19,6 @@ package org.elasticsearch.action.count; -import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.search.SearchAction; import org.elasticsearch.action.search.SearchResponse; @@ -27,11 +26,7 @@ import org.elasticsearch.action.support.DelegatingActionListener; import org.elasticsearch.action.support.QuerySourceBuilder; import org.elasticsearch.action.support.broadcast.BroadcastOperationRequestBuilder; import org.elasticsearch.client.ElasticsearchClient; -import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.search.builder.SearchSourceBuilder; /** * A count action request builder. @@ -86,14 +81,6 @@ public class CountRequestBuilder extends BroadcastOperationRequestBuilder { @Override public String toString() { - String sSource = searchRequest.source().toString(); + String sSource = "_na_"; + if (searchRequest.source() != null) { + sSource = searchRequest.source().toString(); + } return "[" + Arrays.toString(indices) + "]" + Arrays.toString(types()) + ", source[" + sSource + "]"; } diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java b/core/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java index 5d2fe4208d8..6ea1d0e6e61 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java @@ -21,7 +21,6 @@ package org.elasticsearch.cluster.metadata; import com.carrotsearch.hppc.cursors.ObjectCursor; import com.carrotsearch.hppc.cursors.ObjectObjectCursor; - import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; import org.elasticsearch.cluster.Diff; @@ -41,11 +40,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.loader.SettingsLoader; -import org.elasticsearch.common.xcontent.FromXContentBuilder; -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.*; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.search.warmer.IndexWarmersMetaData; @@ -61,14 +56,12 @@ import java.util.Map; import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.OpType.AND; import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.OpType.OR; -import static org.elasticsearch.common.settings.Settings.readSettingsFromStream; -import static org.elasticsearch.common.settings.Settings.settingsBuilder; -import static org.elasticsearch.common.settings.Settings.writeSettingsToStream; +import static org.elasticsearch.common.settings.Settings.*; /** * */ -public class IndexMetaData implements Diffable, FromXContentBuilder, ToXContent { +public class IndexMetaData implements Diffable, FromXContentBuilder, ToXContent { public static final IndexMetaData PROTO = IndexMetaData.builder("") .settings(Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)) @@ -531,8 +524,7 @@ public class IndexMetaData implements Diffable, FromXContentBuild } @Override - public IndexMetaData fromXContent(XContentParser parser, ParseFieldMatcher parseFieldMatcher) - throws IOException { + public IndexMetaData fromXContent(XContentParser parser, ParseFieldMatcher parseFieldMatcher) throws IOException { return Builder.fromXContent(parser); } @@ -713,7 +705,7 @@ public class IndexMetaData implements Diffable, FromXContentBuild public int numberOfReplicas() { return settings.getAsInt(SETTING_NUMBER_OF_REPLICAS, -1); } - + public Builder creationDate(long creationDate) { settings = settingsBuilder().put(settings).put(SETTING_CREATION_DATE, creationDate).build(); return this; diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/warmer/put/RestPutWarmerAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/warmer/put/RestPutWarmerAction.java index c50cf18b906..2a4650b425a 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/warmer/put/RestPutWarmerAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/warmer/put/RestPutWarmerAction.java @@ -26,9 +26,6 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.inject.Inject; 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.rest.BaseRestHandler; import org.elasticsearch.rest.RestChannel; @@ -75,10 +72,7 @@ public class RestPutWarmerAction extends BaseRestHandler { PutWarmerRequest putWarmerRequest = new PutWarmerRequest(request.param("name")); BytesReference sourceBytes = RestActions.getRestContent(request); - XContentParser parser = XContentFactory.xContent(sourceBytes).createParser(sourceBytes); - QueryParseContext queryParseContext = new QueryParseContext(queryRegistry); - queryParseContext.reset(parser); - SearchSourceBuilder source = SearchSourceBuilder.PROTOTYPE.fromXContent(parser, queryParseContext); + SearchSourceBuilder source = RestActions.getRestSearchSource(sourceBytes, queryRegistry); SearchRequest searchRequest = new SearchRequest(Strings.splitStringByCommaToArray(request.param("index"))) .types(Strings.splitStringByCommaToArray(request.param("type"))) .requestCache(request.paramAsBoolean("request_cache", null)).source(source); diff --git a/core/src/main/java/org/elasticsearch/rest/action/cat/RestCountAction.java b/core/src/main/java/org/elasticsearch/rest/action/cat/RestCountAction.java index 86029e5343f..9ff8fb31277 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/cat/RestCountAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/cat/RestCountAction.java @@ -23,7 +23,6 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.count.CountRequest; import org.elasticsearch.action.count.CountResponse; import org.elasticsearch.action.support.QuerySourceBuilder; -import org.elasticsearch.bootstrap.Elasticsearch; import org.elasticsearch.client.Client; import org.elasticsearch.common.Strings; import org.elasticsearch.common.Table; @@ -38,11 +37,9 @@ import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestResponse; -import org.elasticsearch.rest.action.search.RestSearchAction; import org.elasticsearch.rest.action.support.RestActions; import org.elasticsearch.rest.action.support.RestResponseListener; import org.elasticsearch.rest.action.support.RestTable; -import org.elasticsearch.search.builder.SearchSourceBuilder; import org.joda.time.format.DateTimeFormat; import org.joda.time.format.DateTimeFormatter; @@ -78,8 +75,8 @@ public class RestCountAction extends AbstractCatAction { try (XContentParser requestParser = XContentFactory.xContent(source).createParser(source)) { QueryParseContext context = new QueryParseContext(indicesQueriesRegistry); context.reset(requestParser); - final SearchSourceBuilder builder = SearchSourceBuilder.PROTOTYPE.fromXContent(requestParser, context); - countRequest.searchSource(builder); + final QueryBuilder builder = context.parseInnerQueryBuilder(); + countRequest.query(builder); } catch (IOException e) { throw new ElasticsearchException("failed to parse source", e); } diff --git a/core/src/main/java/org/elasticsearch/rest/action/count/RestCountAction.java b/core/src/main/java/org/elasticsearch/rest/action/count/RestCountAction.java index c0548cefcb1..6d3fd0818af 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/count/RestCountAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/count/RestCountAction.java @@ -23,7 +23,6 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.count.CountRequest; import org.elasticsearch.action.count.CountResponse; import org.elasticsearch.action.support.IndicesOptions; -import org.elasticsearch.action.support.QuerySourceBuilder; import org.elasticsearch.client.Client; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; @@ -43,7 +42,6 @@ import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestResponse; import org.elasticsearch.rest.action.support.RestActions; import org.elasticsearch.rest.action.support.RestBuilderListener; -import org.elasticsearch.search.builder.SearchSourceBuilder; import java.io.IOException; @@ -81,8 +79,8 @@ public class RestCountAction extends BaseRestHandler { try (XContentParser requestParser = XContentFactory.xContent(restContent).createParser(restContent)) { QueryParseContext context = new QueryParseContext(indicesQueriesRegistry); context.reset(requestParser); - final SearchSourceBuilder builder = SearchSourceBuilder.PROTOTYPE.fromXContent(requestParser, context); - countRequest.searchSource(builder); + final QueryBuilder builder = context.parseInnerQueryBuilder(); + countRequest.query(builder); } catch (IOException e) { throw new ElasticsearchException("failed to parse source", 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 30d1966c969..29dda6c12eb 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 @@ -115,10 +115,7 @@ public class RestSearchAction extends BaseRestHandler { } builder = null; } else { - try (XContentParser requestParser = XContentFactory.xContent(restContent).createParser(restContent)) { - context.reset(requestParser); - builder = SearchSourceBuilder.PROTOTYPE.fromXContent(requestParser, context); - } + builder = RestActions.getRestSearchSource(restContent, indicesQueriesRegistry); } } else { builder = null; diff --git a/core/src/main/java/org/elasticsearch/rest/action/support/RestActions.java b/core/src/main/java/org/elasticsearch/rest/action/support/RestActions.java index 464870ac4c6..95d479860fd 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/support/RestActions.java +++ b/core/src/main/java/org/elasticsearch/rest/action/support/RestActions.java @@ -29,12 +29,16 @@ import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentBuilderString; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.query.Operator; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryStringQueryBuilder; +import org.elasticsearch.indices.query.IndicesQueriesRegistry; import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.search.builder.SearchSourceBuilder; import java.io.IOException; @@ -107,6 +111,15 @@ public class RestActions { return queryBuilder; } + public static SearchSourceBuilder getRestSearchSource(BytesReference sourceBytes, IndicesQueriesRegistry queryRegistry) + throws IOException { + XContentParser parser = XContentFactory.xContent(sourceBytes).createParser(sourceBytes); + QueryParseContext queryParseContext = new QueryParseContext(queryRegistry); + queryParseContext.reset(parser); + SearchSourceBuilder source = SearchSourceBuilder.PROTOTYPE.fromXContent(parser, queryParseContext); + return source; + } + /** * Get Rest content from either payload or source parameter * @param request Rest request diff --git a/core/src/test/java/org/elasticsearch/action/count/CountRequestBuilderTests.java b/core/src/test/java/org/elasticsearch/action/count/CountRequestBuilderTests.java index 4bf5e7f7487..9993c1c1965 100644 --- a/core/src/test/java/org/elasticsearch/action/count/CountRequestBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/action/count/CountRequestBuilderTests.java @@ -22,23 +22,14 @@ package org.elasticsearch.action.count; import org.elasticsearch.action.support.QuerySourceBuilder; import org.elasticsearch.client.Client; import org.elasticsearch.client.transport.TransportClient; -import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentHelper; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.query.MatchAllQueryBuilder; -import org.elasticsearch.index.query.MatchQueryBuilder; import org.elasticsearch.index.query.QueryBuilders; -import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.test.ESTestCase; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; -import java.io.IOException; - import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; @@ -81,11 +72,4 @@ public class CountRequestBuilderTests extends ESTestCase { countRequestBuilder.setQuery(new MatchAllQueryBuilder()); assertThat(countRequestBuilder.toString(), containsString("match_all")); } - - @Test - public void testStringSourceToString() { - CountRequestBuilder countRequestBuilder = client.prepareCount(); - countRequestBuilder.setSource(new SearchSourceBuilder().query(new MatchAllQueryBuilder())); - assertThat(countRequestBuilder.toString(), containsString("match_all")); - } }