review fixes

This commit is contained in:
Colin Goodheart-Smithe 2015-10-08 09:30:14 +01:00
parent a16b025f0a
commit c69cde4ee4
13 changed files with 54 additions and 71 deletions

View File

@ -332,7 +332,7 @@ public class SearchRequest extends ActionRequest<SearchRequest> implements Indic
scroll = readScroll(in); scroll = readScroll(in);
} }
if (in.readBoolean()) { if (in.readBoolean()) {
source = SearchSourceBuilder.PROTOTYPE.readFrom(in); source = SearchSourceBuilder.readSearchSourceFrom(in);
} }
types = in.readStringArray(); types = in.readStringArray();

View File

@ -45,8 +45,6 @@ import java.util.List;
*/ */
public class SearchRequestBuilder extends ActionRequestBuilder<SearchRequest, SearchResponse, SearchRequestBuilder> { public class SearchRequestBuilder extends ActionRequestBuilder<SearchRequest, SearchResponse, SearchRequestBuilder> {
private SearchSourceBuilder sourceBuilder;
public SearchRequestBuilder(ElasticsearchClient client, SearchAction action) { public SearchRequestBuilder(ElasticsearchClient client, SearchAction action) {
super(client, action, new SearchRequest()); super(client, action, new SearchRequest());
} }
@ -475,54 +473,18 @@ public class SearchRequestBuilder extends ActionRequestBuilder<SearchRequest, Se
return this; return this;
} }
/**
* Sets the source builder to be used with this request. Note, any operations done
* on this require builder before are discarded as this internal builder replaces
* what has been built up until this point.
*/
public SearchRequestBuilder internalBuilder(SearchSourceBuilder sourceBuilder) {
this.sourceBuilder = sourceBuilder;
return this;
}
/**
* Returns the internal search source builder used to construct the request.
*/
public SearchSourceBuilder internalBuilder() {
return sourceBuilder();
}
@Override @Override
public String toString() { public String toString() {
if (sourceBuilder != null) {
return sourceBuilder.toString();
}
if (request.source() != null) { if (request.source() != null) {
return request.source().toString(); return request.source().toString();
} }
return new SearchSourceBuilder().toString(); return new SearchSourceBuilder().toString();
} }
@Override
public SearchRequest request() {
if (sourceBuilder != null) {
request.source(sourceBuilder());
}
return request;
}
@Override
protected SearchRequest beforeExecute(SearchRequest request) {
if (sourceBuilder != null) {
request.source(sourceBuilder());
}
return request;
}
private SearchSourceBuilder sourceBuilder() { private SearchSourceBuilder sourceBuilder() {
if (sourceBuilder == null) { if (request.source() == null) {
sourceBuilder = new SearchSourceBuilder(); request.source(new SearchSourceBuilder());
} }
return sourceBuilder; return request.source();
} }
} }

View File

@ -60,8 +60,6 @@ import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import static org.elasticsearch.common.Strings.hasLength;
/** /**
* The indices request cache allows to cache a shard level request stage responses, helping with improving * The indices request cache allows to cache a shard level request stage responses, helping with improving
* similar requests that are potentially expensive (because of aggs for example). The cache is fully coherent * similar requests that are potentially expensive (because of aggs for example). The cache is fully coherent
@ -207,6 +205,10 @@ public class IndicesRequestCache extends AbstractComponent implements RemovalLis
* Can the shard request be cached at all? * Can the shard request be cached at all?
*/ */
public boolean canCache(ShardSearchRequest request, SearchContext context) { public boolean canCache(ShardSearchRequest request, SearchContext context) {
if (request.template() != null) {
return false;
}
// for now, only enable it for requests with no hits // for now, only enable it for requests with no hits
if (context.size() != 0) { if (context.size() != 0) {
return false; return false;

View File

@ -72,13 +72,15 @@ public class RestCountAction extends AbstractCatAction {
CountRequest countRequest = new CountRequest(indices); CountRequest countRequest = new CountRequest(indices);
String source = request.param("source"); String source = request.param("source");
if (source != null) { if (source != null) {
QueryParseContext context = new QueryParseContext(indicesQueriesRegistry);
try (XContentParser requestParser = XContentFactory.xContent(source).createParser(source)) { try (XContentParser requestParser = XContentFactory.xContent(source).createParser(source)) {
QueryParseContext context = new QueryParseContext(indicesQueriesRegistry);
context.reset(requestParser); context.reset(requestParser);
final QueryBuilder<?> builder = context.parseInnerQueryBuilder(); final QueryBuilder<?> builder = context.parseInnerQueryBuilder();
countRequest.query(builder); countRequest.query(builder);
} catch (IOException e) { } catch (IOException e) {
throw new ElasticsearchException("failed to parse source", e); throw new ElasticsearchException("failed to parse source", e);
} finally {
context.reset(null);
} }
} else { } else {
QueryBuilder<?> queryBuilder = RestActions.urlParamsToQueryBuilder(request); QueryBuilder<?> queryBuilder = RestActions.urlParamsToQueryBuilder(request);

View File

@ -34,7 +34,10 @@ import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.index.query.TemplateQueryParser; import org.elasticsearch.index.query.TemplateQueryParser;
import org.elasticsearch.indices.query.IndicesQueriesRegistry; import org.elasticsearch.indices.query.IndicesQueriesRegistry;
import org.elasticsearch.rest.*; import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestChannel;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.support.RestActions; import org.elasticsearch.rest.action.support.RestActions;
import org.elasticsearch.rest.action.support.RestToXContentListener; import org.elasticsearch.rest.action.support.RestToXContentListener;
import org.elasticsearch.script.Template; import org.elasticsearch.script.Template;
@ -181,7 +184,7 @@ public class RestMultiSearchAction extends BaseRestHandler {
} else { } else {
try (XContentParser requestParser = XContentFactory.xContent(slice).createParser(slice)) { try (XContentParser requestParser = XContentFactory.xContent(slice).createParser(slice)) {
queryParseContext.reset(requestParser); queryParseContext.reset(requestParser);
searchRequest.source(SearchSourceBuilder.PROTOTYPE.fromXContent(requestParser, queryParseContext)); searchRequest.source(SearchSourceBuilder.parseSearchSource(requestParser, queryParseContext));
} }
} }
// move pointers // move pointers

View File

@ -116,7 +116,7 @@ public class RestActions {
XContentParser parser = XContentFactory.xContent(sourceBytes).createParser(sourceBytes); XContentParser parser = XContentFactory.xContent(sourceBytes).createParser(sourceBytes);
QueryParseContext queryParseContext = new QueryParseContext(queryRegistry); QueryParseContext queryParseContext = new QueryParseContext(queryRegistry);
queryParseContext.reset(parser); queryParseContext.reset(parser);
SearchSourceBuilder source = SearchSourceBuilder.PROTOTYPE.fromXContent(parser, queryParseContext); SearchSourceBuilder source = SearchSourceBuilder.parseSearchSource(parser, queryParseContext);
return source; return source;
} }

View File

@ -578,7 +578,7 @@ public class SearchService extends AbstractLifecycleComponent<SearchService> {
try (XContentParser parser = XContentFactory.xContent(run).createParser(run)) { try (XContentParser parser = XContentFactory.xContent(run).createParser(run)) {
QueryParseContext queryParseContext = new QueryParseContext(indexService.queryParserService().indicesQueriesRegistry()); QueryParseContext queryParseContext = new QueryParseContext(indexService.queryParserService().indicesQueriesRegistry());
queryParseContext.reset(parser); queryParseContext.reset(parser);
parseSource(context, SearchSourceBuilder.PROTOTYPE.fromXContent(parser, queryParseContext)); parseSource(context, SearchSourceBuilder.parseSearchSource(parser, queryParseContext));
} }
} }
parseSource(context, request.source()); parseSource(context, request.source());

View File

@ -60,12 +60,9 @@ import java.util.Objects;
/** /**
* A search source builder allowing to easily build search source. Simple * A search source builder allowing to easily build search source. Simple
* construction using * construction using
* {@link org.elasticsearch.search.builder.NewSearchSourceBuilder#searchSource()}. * {@link org.elasticsearch.search.builder.SearchSourceBuilder#searchSource()}.
*
* @see org.elasticsearch.action.search.SearchRequest#source(NewSearchSourceBuilder)
*/
/**
* *
* @see org.elasticsearch.action.search.SearchRequest#source(SearchSourceBuilder)
*/ */
public final class SearchSourceBuilder extends ToXContentToBytes implements Writeable<SearchSourceBuilder> { public final class SearchSourceBuilder extends ToXContentToBytes implements Writeable<SearchSourceBuilder> {
@ -95,7 +92,15 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ
public static final ParseField STATS_FIELD = new ParseField("stats"); public static final ParseField STATS_FIELD = new ParseField("stats");
public static final ParseField EXT_FIELD = new ParseField("ext"); public static final ParseField EXT_FIELD = new ParseField("ext");
public static final SearchSourceBuilder PROTOTYPE = new SearchSourceBuilder(); private static final SearchSourceBuilder PROTOTYPE = new SearchSourceBuilder();
public static SearchSourceBuilder readSearchSourceFrom(StreamInput in) throws IOException {
return PROTOTYPE.readFrom(in);
}
public static SearchSourceBuilder parseSearchSource(XContentParser parser, QueryParseContext context) throws IOException {
return PROTOTYPE.fromXContent(parser, context);
}
/** /**
* A static factory method to construct a new search source. * A static factory method to construct a new search source.

View File

@ -175,7 +175,7 @@ public class ShardSearchLocalRequest extends ContextAndHeaderHolder implements S
scroll = readScroll(in); scroll = readScroll(in);
} }
if (in.readBoolean()) { if (in.readBoolean()) {
source = SearchSourceBuilder.PROTOTYPE.readFrom(in); source = SearchSourceBuilder.readSearchSourceFrom(in);
} }
types = in.readStringArray(); types = in.readStringArray();
filteringAliases = in.readStringArray(); filteringAliases = in.readStringArray();

View File

@ -30,8 +30,12 @@ import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.util.ByteArray; import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.*; import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentGenerator;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.builder.SearchSourceBuilder;
@ -279,6 +283,9 @@ public class IndexWarmersMetaData extends AbstractDiffable<IndexMetaData.Custom>
private SearchSourceBuilder cached; private SearchSourceBuilder cached;
public SearchSource(BytesReference bytesArray) { public SearchSource(BytesReference bytesArray) {
if (bytesArray == null) {
throw new IllegalArgumentException("bytesArray must not be null");
}
this.binary = bytesArray; this.binary = bytesArray;
} }
@ -299,7 +306,7 @@ public class IndexWarmersMetaData extends AbstractDiffable<IndexMetaData.Custom>
if (cached == null) { if (cached == null) {
try (XContentParser parser = XContentFactory.xContent(binary).createParser(binary)) { try (XContentParser parser = XContentFactory.xContent(binary).createParser(binary)) {
ctx.reset(parser); ctx.reset(parser);
cached = SearchSourceBuilder.PROTOTYPE.fromXContent(parser, ctx); cached = SearchSourceBuilder.parseSearchSource(parser, ctx);
} }
} }
return cached; return cached;

View File

@ -65,11 +65,6 @@ public class DummyQueryParserPlugin extends Plugin {
return new DummyQuery(context.isFilter()); return new DummyQuery(context.isFilter());
} }
@Override
public String getWriteableName() {
return NAME;
}
@Override @Override
protected DummyQueryBuilder doReadFrom(StreamInput in) throws IOException { protected DummyQueryBuilder doReadFrom(StreamInput in) throws IOException {
return new DummyQueryBuilder(); return new DummyQueryBuilder();
@ -89,6 +84,11 @@ public class DummyQueryParserPlugin extends Plugin {
protected boolean doEquals(DummyQueryBuilder other) { protected boolean doEquals(DummyQueryBuilder other) {
return true; return true;
} }
@Override
public String getWriteableName() {
return NAME;
}
} }
public static class DummyQueryParser implements QueryParser<DummyQueryBuilder> { public static class DummyQueryParser implements QueryParser<DummyQueryBuilder> {

View File

@ -677,6 +677,7 @@ public class IndicesOptionsIntegrationIT extends ESIntegTestCase {
@Test @Test
public void testDeleteWarmer() throws Exception { public void testDeleteWarmer() throws Exception {
SearchSourceBuilder source = new SearchSourceBuilder(); SearchSourceBuilder source = new SearchSourceBuilder();
source.query(QueryBuilders.matchAllQuery());
IndexWarmersMetaData.Entry entry = new IndexWarmersMetaData.Entry("test1", new String[] { "typ1" }, false, new IndexWarmersMetaData.SearchSource(source)); IndexWarmersMetaData.Entry entry = new IndexWarmersMetaData.Entry("test1", new String[] { "typ1" }, false, new IndexWarmersMetaData.SearchSource(source));
assertAcked(prepareCreate("foobar").addCustom(new IndexWarmersMetaData(entry))); assertAcked(prepareCreate("foobar").addCustom(new IndexWarmersMetaData(entry)));
ensureYellow(); ensureYellow();
@ -692,6 +693,7 @@ public class IndicesOptionsIntegrationIT extends ESIntegTestCase {
verify(client().admin().indices().prepareDeleteWarmer().setIndices("_all").setNames("test1"), true); verify(client().admin().indices().prepareDeleteWarmer().setIndices("_all").setNames("test1"), true);
SearchSourceBuilder source = new SearchSourceBuilder(); SearchSourceBuilder source = new SearchSourceBuilder();
source.query(QueryBuilders.matchAllQuery());
IndexWarmersMetaData.Entry entry = new IndexWarmersMetaData.Entry("test1", new String[] { "type1" }, false, new IndexWarmersMetaData.SearchSource(source)); IndexWarmersMetaData.Entry entry = new IndexWarmersMetaData.Entry("test1", new String[] { "type1" }, false, new IndexWarmersMetaData.SearchSource(source));
assertAcked(prepareCreate("foo").addCustom(new IndexWarmersMetaData(entry))); assertAcked(prepareCreate("foo").addCustom(new IndexWarmersMetaData(entry)));
assertAcked(prepareCreate("foobar").addCustom(new IndexWarmersMetaData(entry))); assertAcked(prepareCreate("foobar").addCustom(new IndexWarmersMetaData(entry)));

View File

@ -302,7 +302,7 @@ public class SearchSourceBuilderTests extends ESTestCase {
private void assertParseSearchSource(SearchSourceBuilder testBuilder, String builderAsString) throws IOException { private void assertParseSearchSource(SearchSourceBuilder testBuilder, String builderAsString) throws IOException {
XContentParser parser = XContentFactory.xContent(builderAsString).createParser(builderAsString); XContentParser parser = XContentFactory.xContent(builderAsString).createParser(builderAsString);
SearchSourceBuilder newBuilder = SearchSourceBuilder.PROTOTYPE.fromXContent(parser, createParseContext(parser)); SearchSourceBuilder newBuilder = SearchSourceBuilder.parseSearchSource(parser, createParseContext(parser));
assertNotSame(testBuilder, newBuilder); assertNotSame(testBuilder, newBuilder);
assertEquals(testBuilder, newBuilder); assertEquals(testBuilder, newBuilder);
assertEquals(testBuilder.hashCode(), newBuilder.hashCode()); assertEquals(testBuilder.hashCode(), newBuilder.hashCode());
@ -321,7 +321,7 @@ public class SearchSourceBuilderTests extends ESTestCase {
try (BytesStreamOutput output = new BytesStreamOutput()) { try (BytesStreamOutput output = new BytesStreamOutput()) {
testBuilder.writeTo(output); testBuilder.writeTo(output);
try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) { try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) {
SearchSourceBuilder deserializedBuilder = SearchSourceBuilder.PROTOTYPE.readFrom(in); SearchSourceBuilder deserializedBuilder = SearchSourceBuilder.readSearchSourceFrom(in);
assertEquals(deserializedBuilder, testBuilder); assertEquals(deserializedBuilder, testBuilder);
assertEquals(deserializedBuilder.hashCode(), testBuilder.hashCode()); assertEquals(deserializedBuilder.hashCode(), testBuilder.hashCode());
assertNotSame(deserializedBuilder, testBuilder); assertNotSame(deserializedBuilder, testBuilder);
@ -359,7 +359,7 @@ public class SearchSourceBuilderTests extends ESTestCase {
try (BytesStreamOutput output = new BytesStreamOutput()) { try (BytesStreamOutput output = new BytesStreamOutput()) {
builder.writeTo(output); builder.writeTo(output);
try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) { try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) {
return SearchSourceBuilder.PROTOTYPE.readFrom(in); return SearchSourceBuilder.readSearchSourceFrom(in);
} }
} }
} }
@ -368,7 +368,7 @@ public class SearchSourceBuilderTests extends ESTestCase {
{ {
String restContent = " { \"_source\": { \"includes\": \"include\", \"excludes\": \"*.field2\"}}"; String restContent = " { \"_source\": { \"includes\": \"include\", \"excludes\": \"*.field2\"}}";
try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) { try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) {
SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.PROTOTYPE.fromXContent(parser, createParseContext(parser)); SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.parseSearchSource(parser, createParseContext(parser));
assertArrayEquals(new String[]{"*.field2" }, searchSourceBuilder.fetchSource().excludes()); assertArrayEquals(new String[]{"*.field2" }, searchSourceBuilder.fetchSource().excludes());
assertArrayEquals(new String[]{"include" }, searchSourceBuilder.fetchSource().includes()); assertArrayEquals(new String[]{"include" }, searchSourceBuilder.fetchSource().includes());
} }
@ -376,7 +376,7 @@ public class SearchSourceBuilderTests extends ESTestCase {
{ {
String restContent = " { \"_source\": false}"; String restContent = " { \"_source\": false}";
try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) { try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) {
SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.PROTOTYPE.fromXContent(parser, createParseContext(parser)); SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.parseSearchSource(parser, createParseContext(parser));
assertArrayEquals(new String[]{}, searchSourceBuilder.fetchSource().excludes()); assertArrayEquals(new String[]{}, searchSourceBuilder.fetchSource().excludes());
assertArrayEquals(new String[]{}, searchSourceBuilder.fetchSource().includes()); assertArrayEquals(new String[]{}, searchSourceBuilder.fetchSource().includes());
assertFalse(searchSourceBuilder.fetchSource().fetchSource()); assertFalse(searchSourceBuilder.fetchSource().fetchSource());
@ -389,7 +389,7 @@ public class SearchSourceBuilderTests extends ESTestCase {
{ {
String restContent = " { \"sort\": \"foo\"}"; String restContent = " { \"sort\": \"foo\"}";
try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) { try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) {
SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.PROTOTYPE.fromXContent(parser, createParseContext(parser)); SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.parseSearchSource(parser, createParseContext(parser));
assertEquals(1, searchSourceBuilder.sorts().size()); assertEquals(1, searchSourceBuilder.sorts().size());
assertEquals("{\"foo\":{}}", searchSourceBuilder.sorts().get(0).toUtf8()); assertEquals("{\"foo\":{}}", searchSourceBuilder.sorts().get(0).toUtf8());
} }
@ -404,7 +404,7 @@ public class SearchSourceBuilderTests extends ESTestCase {
" \"_score\"\n" + " \"_score\"\n" +
" ]}"; " ]}";
try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) { try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) {
SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.PROTOTYPE.fromXContent(parser, createParseContext(parser)); SearchSourceBuilder searchSourceBuilder = SearchSourceBuilder.parseSearchSource(parser, createParseContext(parser));
assertEquals(5, searchSourceBuilder.sorts().size()); assertEquals(5, searchSourceBuilder.sorts().size());
assertEquals("{\"post_date\":{\"order\":\"asc\"}}", searchSourceBuilder.sorts().get(0).toUtf8()); assertEquals("{\"post_date\":{\"order\":\"asc\"}}", searchSourceBuilder.sorts().get(0).toUtf8());
assertEquals("\"user\"", searchSourceBuilder.sorts().get(1).toUtf8()); assertEquals("\"user\"", searchSourceBuilder.sorts().get(1).toUtf8());