From dc900a08a6c6f346656105fa0e2484ed186f8461 Mon Sep 17 00:00:00 2001 From: javanna Date: Mon, 26 Oct 2015 12:18:36 +0100 Subject: [PATCH] Remove "query" query and fix related parsing bugs We have two types of parse methods for queries: one for the inner query, to be used once the parser is positioned within the query element, and one for the whole query source, including the query element that wraps the actual query. With the search refactoring we ended up using the former in count, cat count and delete by query, whereas we should have used the former. It ends up working properly given that we have a registered (deprecated) query called "query", which used to allow to wrap a filter into a query, but this has the following downsides: 1) prevents us from removing the deprecated "query" query 2) we end up supporting a top level query that is not wrapped within a query element (pre 1.0 syntax iirc that shouldn't be supported anymore) This commit finally removes the "query" query and fixes the related parsing bugs. We also had some tests that were providing queries in the wrong format, those have been fixed too. Closes #13326 Closes #14304 --- .../query/TransportValidateQueryAction.java | 2 +- .../explain/TransportExplainAction.java | 2 +- .../index/query/IndexQueryParserService.java | 32 ++--- .../index/query/QueryFilterBuilder.java | 111 ------------------ .../index/query/QueryFilterParser.java | 46 -------- .../index/query/QueryParseContext.java | 43 ++++++- .../elasticsearch/indices/IndicesModule.java | 3 - .../rest/action/cat/RestCountAction.java | 5 +- .../rest/action/count/RestCountAction.java | 5 +- .../rest/action/support/RestActions.java | 21 +--- .../search/builder/SearchSourceBuilder.java | 9 +- .../index/query/AbstractQueryTestCase.java | 21 +--- .../index/query/QueryFilterBuilderTests.java | 77 ------------ .../index/query/TemplateQueryIT.java | 13 +- .../FunctionScoreQueryBuilderTests.java | 2 - docs/reference/migration/migrate_3_0.asciidoc | 10 ++ docs/reference/redirects.asciidoc | 2 +- .../RestDeleteByQueryAction.java | 30 +---- .../test/delete_by_query/10_basic.yaml | 16 ++- .../rest-api-spec/test/count/10_basic.yaml | 15 ++- .../rest-api-spec/test/explain/10_basic.yaml | 47 ++++---- .../test/indices.validate_query/10_basic.yaml | 13 +- .../test/search/20_default_values.yaml | 17 ++- 23 files changed, 165 insertions(+), 377 deletions(-) delete mode 100644 core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java delete mode 100644 core/src/main/java/org/elasticsearch/index/query/QueryFilterParser.java delete mode 100644 core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTests.java diff --git a/core/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java b/core/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java index db54fe4278a..9cdd8ce2183 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java +++ b/core/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java @@ -179,7 +179,7 @@ public class TransportValidateQueryAction extends TransportBroadcastAction 0) { - searchContext.parsedQuery(queryParserService.parseQuery(request.source())); + searchContext.parsedQuery(queryParserService.parseTopLevelQuery(request.source())); } searchContext.preProcess(); diff --git a/core/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java b/core/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java index c2e6ddf46ff..e52d4e32071 100644 --- a/core/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java +++ b/core/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java @@ -121,7 +121,7 @@ public class TransportExplainAction extends TransportSingleShardAction queryBuilder = queryShardContext.parseContext().parseTopLevelQueryBuilder(); + Query query = toQuery(queryBuilder, queryShardContext); + return new ParsedQuery(query, queryShardContext.copyNamedQueries()); + } finally { + queryShardContext.reset(null); } - if (parsedQuery == null) { - throw new ParsingException(parser.getTokenLocation(), "Required query is missing"); - } - return parsedQuery; } catch (ParsingException | QueryShardException e) { throw e; } catch (Throwable e) { diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java b/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java deleted file mode 100644 index 4ca9e1598e2..00000000000 --- a/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java +++ /dev/null @@ -1,111 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.index.query; - -import org.apache.lucene.search.ConstantScoreQuery; -import org.apache.lucene.search.Query; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.xcontent.XContentBuilder; - -import java.io.IOException; -import java.util.Objects; - -/** - * A filter that simply wraps a query. - * @deprecated Useless now that queries and filters are merged: pass the - * query as a filter directly. - */ -//TODO: remove when https://github.com/elastic/elasticsearch/issues/13326 is fixed -@Deprecated -public class QueryFilterBuilder extends AbstractQueryBuilder { - - public static final String NAME = "query"; - - private final QueryBuilder queryBuilder; - - static final QueryFilterBuilder PROTOTYPE = new QueryFilterBuilder(EmptyQueryBuilder.PROTOTYPE); - - /** - * A filter that simply wraps a query. - * - * @param queryBuilder The query to wrap as a filter - */ - public QueryFilterBuilder(QueryBuilder queryBuilder) { - if (queryBuilder == null) { - throw new IllegalArgumentException("inner query cannot be null"); - } - this.queryBuilder = queryBuilder; - } - - /** - * @return the query builder that is wrapped by this {@link QueryFilterBuilder} - */ - public QueryBuilder innerQuery() { - return this.queryBuilder; - } - - @Override - protected void doXContent(XContentBuilder builder, Params params) throws IOException { - builder.field(NAME); - queryBuilder.toXContent(builder, params); - } - - @Override - protected Query doToQuery(QueryShardContext context) throws IOException { - // inner query builder can potentially be `null`, in that case we ignore it - Query innerQuery = this.queryBuilder.toQuery(context); - if (innerQuery == null) { - return null; - } - return new ConstantScoreQuery(innerQuery); - } - - @Override - protected void setFinalBoost(Query query) { - //no-op this query doesn't support boost - } - - @Override - protected int doHashCode() { - return Objects.hash(queryBuilder); - } - - @Override - protected boolean doEquals(QueryFilterBuilder other) { - return Objects.equals(queryBuilder, other.queryBuilder); - } - - @Override - protected QueryFilterBuilder doReadFrom(StreamInput in) throws IOException { - QueryBuilder innerQueryBuilder = in.readQuery(); - return new QueryFilterBuilder(innerQueryBuilder); - } - - @Override - protected void doWriteTo(StreamOutput out) throws IOException { - out.writeQuery(queryBuilder); - } - - @Override - public String getWriteableName() { - return NAME; - } -} diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryFilterParser.java b/core/src/main/java/org/elasticsearch/index/query/QueryFilterParser.java deleted file mode 100644 index e13661c814c..00000000000 --- a/core/src/main/java/org/elasticsearch/index/query/QueryFilterParser.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.index.query; - -import java.io.IOException; - -/** - * Parser for query filter - * @deprecated use any query instead directly, possible since queries and filters are merged. - */ -// TODO: remove when https://github.com/elastic/elasticsearch/issues/13326 is fixed -@Deprecated -public class QueryFilterParser implements QueryParser { - - @Override - public String[] names() { - return new String[]{QueryFilterBuilder.NAME}; - } - - @Override - public QueryFilterBuilder fromXContent(QueryParseContext parseContext) throws IOException { - return new QueryFilterBuilder(parseContext.parseInnerQueryBuilder()); - } - - @Override - public QueryFilterBuilder getBuilderPrototype() { - return QueryFilterBuilder.PROTOTYPE; - } -} \ No newline at end of file diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java index 0885d040179..70a6a18aab2 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java @@ -22,6 +22,7 @@ package org.elasticsearch.index.query; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.indices.query.IndicesQueriesRegistry; @@ -65,9 +66,47 @@ public class QueryParseContext { } /** - * @return a new QueryBuilder based on the current state of the parser + * Parses a top level query including the query element that wraps it */ - public QueryBuilder parseInnerQueryBuilder() throws IOException { + public QueryBuilder parseTopLevelQueryBuilder() { + try { + QueryBuilder queryBuilder = null; + for (XContentParser.Token token = parser.nextToken(); token != XContentParser.Token.END_OBJECT; token = parser.nextToken()) { + if (token == XContentParser.Token.FIELD_NAME) { + String fieldName = parser.currentName(); + if ("query".equals(fieldName)) { + queryBuilder = parseInnerQueryBuilder(); + } else if ("query_binary".equals(fieldName) || "queryBinary".equals(fieldName)) { + byte[] querySource = parser.binaryValue(); + XContentParser qSourceParser = XContentFactory.xContent(querySource).createParser(querySource); + QueryParseContext queryParseContext = new QueryParseContext(indicesQueriesRegistry); + queryParseContext.reset(qSourceParser); + try { + queryParseContext.parseFieldMatcher(parseFieldMatcher); + queryBuilder = queryParseContext.parseInnerQueryBuilder(); + } finally { + queryParseContext.reset(null); + } + } else { + throw new ParsingException(parser.getTokenLocation(), "request does not support [" + parser.currentName() + "]"); + } + } + } + if (queryBuilder == null) { + throw new ParsingException(parser.getTokenLocation(), "Required query is missing"); + } + return queryBuilder; + } catch (ParsingException e) { + throw e; + } catch (Throwable e) { + throw new ParsingException(parser == null ? null : parser.getTokenLocation(), "Failed to parse", e); + } + } + + /** + * Parses a query excluding the query element that wraps it + */ + public QueryBuilder parseInnerQueryBuilder() throws IOException { // move to START object XContentParser.Token token; if (parser.currentToken() != XContentParser.Token.START_OBJECT) { diff --git a/core/src/main/java/org/elasticsearch/indices/IndicesModule.java b/core/src/main/java/org/elasticsearch/indices/IndicesModule.java index 89a3ae21fb4..8246ed9e608 100644 --- a/core/src/main/java/org/elasticsearch/indices/IndicesModule.java +++ b/core/src/main/java/org/elasticsearch/indices/IndicesModule.java @@ -24,11 +24,9 @@ import org.elasticsearch.action.update.UpdateHelper; import org.elasticsearch.cluster.metadata.MetaDataIndexUpgradeService; import org.elasticsearch.common.geo.ShapesAvailability; import org.elasticsearch.common.inject.AbstractModule; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.ExtensionPoint; import org.elasticsearch.index.query.*; import org.elasticsearch.index.query.functionscore.FunctionScoreQueryParser; -import org.elasticsearch.index.query.MoreLikeThisQueryParser; import org.elasticsearch.index.termvectors.TermVectorsService; import org.elasticsearch.indices.analysis.HunspellService; import org.elasticsearch.indices.analysis.IndicesAnalysisService; @@ -105,7 +103,6 @@ public class IndicesModule extends AbstractModule { registerQueryParser(GeoBoundingBoxQueryParser.class); registerQueryParser(GeohashCellQuery.Parser.class); registerQueryParser(GeoPolygonQueryParser.class); - registerQueryParser(QueryFilterParser.class); registerQueryParser(ExistsQueryParser.class); registerQueryParser(MissingQueryParser.class); registerQueryParser(MatchNoneQueryParser.class); 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 c234ac62b04..e4d291b5fc8 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 @@ -28,7 +28,6 @@ import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.indices.query.IndicesQueriesRegistry; import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestController; @@ -71,9 +70,7 @@ public class RestCountAction extends AbstractCatAction { SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().size(0); countRequest.source(searchSourceBuilder); if (source != null) { - QueryParseContext context = new QueryParseContext(indicesQueriesRegistry); - context.parseFieldMatcher(parseFieldMatcher); - searchSourceBuilder.query(RestActions.getQueryContent(new BytesArray(source), context)); + searchSourceBuilder.query(RestActions.getQueryContent(new BytesArray(source), indicesQueriesRegistry, parseFieldMatcher)); } else { QueryBuilder queryBuilder = RestActions.urlParamsToQueryBuilder(request); if (queryBuilder != null) { 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 e32e1954c9f..1ce78e33e3f 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 @@ -29,7 +29,6 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.indices.query.IndicesQueriesRegistry; import org.elasticsearch.rest.*; import org.elasticsearch.rest.action.support.RestActions; @@ -68,9 +67,7 @@ public class RestCountAction extends BaseRestHandler { countRequest.source(searchSourceBuilder); if (RestActions.hasBodyContent(request)) { BytesReference restContent = RestActions.getRestContent(request); - QueryParseContext context = new QueryParseContext(indicesQueriesRegistry); - context.parseFieldMatcher(parseFieldMatcher); - searchSourceBuilder.query(RestActions.getQueryContent(restContent, context)); + searchSourceBuilder.query(RestActions.getQueryContent(restContent, indicesQueriesRegistry, parseFieldMatcher)); } else { QueryBuilder queryBuilder = RestActions.urlParamsToQueryBuilder(request); if (queryBuilder != 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 e788f044237..14935f5f9a5 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 @@ -27,17 +27,8 @@ import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.lucene.uid.Versions; -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.common.xcontent.*; +import org.elasticsearch.index.query.*; import org.elasticsearch.indices.query.IndicesQueriesRegistry; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.search.builder.SearchSourceBuilder; @@ -142,14 +133,12 @@ public class RestActions { return content; } - public static QueryBuilder getQueryContent(BytesReference source, QueryParseContext context) { + public static QueryBuilder getQueryContent(BytesReference source, IndicesQueriesRegistry indicesQueriesRegistry, ParseFieldMatcher parseFieldMatcher) { + QueryParseContext context = new QueryParseContext(indicesQueriesRegistry); 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(); + return context.parseTopLevelQueryBuilder(); } catch (IOException e) { throw new ElasticsearchException("failed to parse source", e); } finally { 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 f4cb67b2d2a..e9bbe6e8114 100644 --- a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java @@ -724,8 +724,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ } else if (context.parseFieldMatcher().match(currentFieldName, TRACK_SCORES_FIELD)) { builder.trackScores = parser.booleanValue(); } else if (context.parseFieldMatcher().match(currentFieldName, _SOURCE_FIELD)) { - FetchSourceContext fetchSourceContext = FetchSourceContext.parse(parser, context); - builder.fetchSourceContext = fetchSourceContext; + builder.fetchSourceContext = FetchSourceContext.parse(parser, context); } else if (context.parseFieldMatcher().match(currentFieldName, FIELDS_FIELD)) { List fieldNames = new ArrayList<>(); fieldNames.add(parser.text()); @@ -742,8 +741,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ } else if (context.parseFieldMatcher().match(currentFieldName, POST_FILTER_FIELD)) { builder.postQueryBuilder = context.parseInnerQueryBuilder(); } else if (context.parseFieldMatcher().match(currentFieldName, _SOURCE_FIELD)) { - FetchSourceContext fetchSourceContext = FetchSourceContext.parse(parser, context); - builder.fetchSourceContext = fetchSourceContext; + builder.fetchSourceContext = FetchSourceContext.parse(parser, context); } else if (context.parseFieldMatcher().match(currentFieldName, SCRIPT_FIELDS_FIELD)) { List scriptFields = new ArrayList<>(); while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { @@ -886,8 +884,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ } builder.stats = stats; } else if (context.parseFieldMatcher().match(currentFieldName, _SOURCE_FIELD)) { - FetchSourceContext fetchSourceContext = FetchSourceContext.parse(parser, context); - builder.fetchSourceContext = fetchSourceContext; + builder.fetchSourceContext = FetchSourceContext.parse(parser, context); } else { throw new ParsingException(parser.getTokenLocation(), "Unknown key for a " + token + " in [" + currentFieldName + "].", parser.getTokenLocation()); diff --git a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java index 42ef67733e3..79666cf9d56 100644 --- a/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java @@ -21,7 +21,6 @@ package org.elasticsearch.index.query; import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator; import com.fasterxml.jackson.core.io.JsonStringEncoder; - import org.apache.lucene.search.Query; import org.elasticsearch.Version; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; @@ -73,12 +72,7 @@ import org.elasticsearch.indices.IndicesWarmer; import org.elasticsearch.indices.analysis.IndicesAnalysisService; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; -import org.elasticsearch.script.MockScriptEngine; -import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.ScriptContextRegistry; -import org.elasticsearch.script.ScriptEngineService; -import org.elasticsearch.script.ScriptModule; -import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.*; import org.elasticsearch.script.mustache.MustacheScriptEngineService; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.ESTestCase; @@ -99,12 +93,7 @@ import java.io.IOException; import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; import java.lang.reflect.Proxy; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.concurrent.ExecutionException; import static org.hamcrest.Matchers.equalTo; @@ -220,8 +209,8 @@ public abstract class AbstractQueryTestCase> new AbstractModule() { @Override protected void configure() { - IndexSettings idxSettings = IndexSettingsModule.newIndexSettings(index, indexSettings, Collections.EMPTY_LIST); - SimilarityService service = new SimilarityService(idxSettings, Collections.EMPTY_MAP); + IndexSettings idxSettings = IndexSettingsModule.newIndexSettings(index, indexSettings, Collections.emptyList()); + SimilarityService service = new SimilarityService(idxSettings, Collections.emptyMap()); bind(SimilarityService.class).toInstance(service); BitsetFilterCache bitsetFilterCache = new BitsetFilterCache(idxSettings, new IndicesWarmer(idxSettings.getNodeSettings(), null)); bind(BitsetFilterCache.class).toInstance(bitsetFilterCache); @@ -413,7 +402,7 @@ public abstract class AbstractQueryTestCase> /** * Few queries allow you to set the boost and queryName on the java api, although the corresponding parser doesn't parse them as they are not supported. * This method allows to disable boost and queryName related tests for those queries. Those queries are easy to identify: their parsers - * don't parse `boost` and `_name` as they don't apply to the specific query: filter query, wrapper query and match_none + * don't parse `boost` and `_name` as they don't apply to the specific query: wrapper query and match_none */ protected boolean supportsBoostAndQueryName() { return true; diff --git a/core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTests.java deleted file mode 100644 index d98dbd0cbbe..00000000000 --- a/core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTests.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.index.query; - -import org.apache.lucene.search.ConstantScoreQuery; -import org.apache.lucene.search.Query; - -import java.io.IOException; - -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.instanceOf; -import static org.hamcrest.CoreMatchers.nullValue; - -@SuppressWarnings("deprecation") -public class QueryFilterBuilderTests extends AbstractQueryTestCase { - - @Override - protected QueryFilterBuilder doCreateTestQueryBuilder() { - QueryBuilder innerQuery = RandomQueryBuilder.createQuery(random()); - return new QueryFilterBuilder(innerQuery); - } - - @Override - protected void doAssertLuceneQuery(QueryFilterBuilder queryBuilder, Query query, QueryShardContext context) throws IOException { - Query innerQuery = queryBuilder.innerQuery().toQuery(context); - if (innerQuery == null) { - assertThat(query, nullValue()); - } else { - assertThat(query, instanceOf(ConstantScoreQuery.class)); - ConstantScoreQuery constantScoreQuery = (ConstantScoreQuery) query; - assertThat(constantScoreQuery.getQuery(), equalTo(innerQuery)); - } - } - - @Override - protected boolean supportsBoostAndQueryName() { - return false; - } - - /** - * test that wrapping an inner filter that returns null also returns null to pass on upwards - */ - public void testInnerQueryReturnsNull() throws IOException { - // create inner filter - String queryString = "{ \"constant_score\" : { \"filter\" : {} } }"; - QueryBuilder innerQuery = parseQuery(queryString); - // check that when wrapping this filter, toQuery() returns null - QueryFilterBuilder queryFilterQuery = new QueryFilterBuilder(innerQuery); - assertNull(queryFilterQuery.toQuery(createShardContext())); - } - - public void testValidate() { - try { - new QueryFilterBuilder(null); - fail("cannot be null"); - } catch (IllegalArgumentException e) { - // expected - } - } -} diff --git a/core/src/test/java/org/elasticsearch/index/query/TemplateQueryIT.java b/core/src/test/java/org/elasticsearch/index/query/TemplateQueryIT.java index c4951640cce..ce5eca5d11c 100644 --- a/core/src/test/java/org/elasticsearch/index/query/TemplateQueryIT.java +++ b/core/src/test/java/org/elasticsearch/index/query/TemplateQueryIT.java @@ -88,23 +88,12 @@ public class TemplateQueryIT extends ESIntegTestCase { } public void testTemplateInBodyWithSize() throws IOException { - String request = "{\n" + - " \"size\":0," + - " \"query\": {\n" + - " \"template\": {\n" + - " \"query\": {\"match_{{template}}\": {}},\n" + - " \"params\" : {\n" + - " \"template\" : \"all\"\n" + - " }\n" + - " }\n" + - " }\n" + - "}"; Map params = new HashMap<>(); params.put("template", "all"); SearchResponse sr = client().prepareSearch() .setSource( new SearchSourceBuilder().size(0).query( - QueryBuilders.templateQuery(new Template("{ \"query\": { \"match_{{template}}\": {} } }", + QueryBuilders.templateQuery(new Template("{ \"match_{{template}}\": {} }", ScriptType.INLINE, null, null, params)))).execute() .actionGet(); assertNoFailures(sr); diff --git a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java index 69aeb824d4e..9aa5c0c5e07 100644 --- a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java @@ -579,7 +579,6 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase>. [role="exclude",id="query-dsl-filtered-query"] === Filtered query -The `filtered` query is replaced in favour of the <> query. Instead of +The `filtered` query is replaced by the <> query. Instead of the following: [source,js] diff --git a/plugins/delete-by-query/src/main/java/org/elasticsearch/rest/action/deletebyquery/RestDeleteByQueryAction.java b/plugins/delete-by-query/src/main/java/org/elasticsearch/rest/action/deletebyquery/RestDeleteByQueryAction.java index 71349648138..2b8dc02289c 100644 --- a/plugins/delete-by-query/src/main/java/org/elasticsearch/rest/action/deletebyquery/RestDeleteByQueryAction.java +++ b/plugins/delete-by-query/src/main/java/org/elasticsearch/rest/action/deletebyquery/RestDeleteByQueryAction.java @@ -20,16 +20,12 @@ package org.elasticsearch.rest.action.deletebyquery; import org.elasticsearch.action.deletebyquery.DeleteByQueryRequest; -import org.elasticsearch.action.deletebyquery.DeleteByQueryResponse; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.client.Client; import org.elasticsearch.common.Strings; 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.QueryBuilder; -import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.indices.query.IndicesQueriesRegistry; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestChannel; @@ -67,29 +63,15 @@ public class RestDeleteByQueryAction extends BaseRestHandler { if (request.hasParam("timeout")) { delete.timeout(request.paramAsTime("timeout", null)); } - if (request.hasContent()) { - 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); + if (RestActions.hasBodyContent(request)) { + delete.query(RestActions.getQueryContent(RestActions.getRestContent(request), indicesQueriesRegistry, parseFieldMatcher)); } else { - String source = request.param("source"); - if (source != null) { - XContentParser requestParser = XContentFactory.xContent(source).createParser(source); - QueryParseContext context = new QueryParseContext(indicesQueriesRegistry); - context.reset(requestParser); - final QueryBuilder builder = context.parseInnerQueryBuilder(); - delete.query(builder); - } else { - QueryBuilder queryBuilder = RestActions.urlParamsToQueryBuilder(request); - if (queryBuilder != null) { - delete.query(queryBuilder); - } + QueryBuilder queryBuilder = RestActions.urlParamsToQueryBuilder(request); + if (queryBuilder != null) { + delete.query(queryBuilder); } } delete.types(Strings.splitStringByCommaToArray(request.param("type"))); - client.execute(INSTANCE, delete, new RestToXContentListener(channel)); + client.execute(INSTANCE, delete, new RestToXContentListener<>(channel)); } } diff --git a/plugins/delete-by-query/src/test/resources/rest-api-spec/test/delete_by_query/10_basic.yaml b/plugins/delete-by-query/src/test/resources/rest-api-spec/test/delete_by_query/10_basic.yaml index c253ad8d276..063e959a807 100644 --- a/plugins/delete-by-query/src/test/resources/rest-api-spec/test/delete_by_query/10_basic.yaml +++ b/plugins/delete-by-query/src/test/resources/rest-api-spec/test/delete_by_query/10_basic.yaml @@ -1,5 +1,4 @@ ---- -"Basic delete_by_query": +setup: - do: index: index: test_1 @@ -24,6 +23,8 @@ - do: indices.refresh: {} +--- +"Basic delete_by_query": - do: delete_by_query: index: test_1 @@ -40,3 +41,14 @@ index: test_1 - match: { count: 2 } + +--- +"Delete_by_query body without query element": + - do: + catch: request + delete_by_query: + index: test_1 + body: + match: + foo: bar + diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/count/10_basic.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/count/10_basic.yaml index 2495f296121..f3eb0a5fae6 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/count/10_basic.yaml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/count/10_basic.yaml @@ -1,5 +1,4 @@ ---- -"count with body": +setup: - do: indices.create: index: test @@ -14,6 +13,8 @@ indices.refresh: index: [test] +--- +"count with body": - do: count: index: test @@ -35,3 +36,13 @@ foo: test - match: {count : 0} + +--- +"count body without query element": + - do: + catch: request + count: + index: test + body: + match: + foo: bar diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/explain/10_basic.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/explain/10_basic.yaml index 4e6845c17e8..1a6c57848e0 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/explain/10_basic.yaml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/explain/10_basic.yaml @@ -1,5 +1,14 @@ ---- -"Basic explain": +setup: + - do: + indices.create: + index: test_1 + body: + aliases: + alias_1: {} + - do: + cluster.health: + wait_for_status: yellow + - do: index: index: test_1 @@ -10,6 +19,9 @@ - do: indices.refresh: {} +--- +"Basic explain": + - do: explain: index: test_1 @@ -27,26 +39,6 @@ --- "Basic explain with alias": - - do: - indices.create: - index: test_1 - body: - aliases: - alias_1: {} - - - do: - cluster.health: - wait_for_status: yellow - - - do: - index: - index: test_1 - type: test - id: id_1 - body: { foo: bar, title: howdy } - - - do: - indices.refresh: {} - do: explain: @@ -63,3 +55,14 @@ - match: { _type: test } - match: { _id: id_1 } +--- +"Explain body without query element": + - do: + catch: request + explain: + index: test_1 + type: test + id: id_1 + body: + match_all: {} + diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.validate_query/10_basic.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.validate_query/10_basic.yaml index 2a9ed19221f..87014a09b33 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.validate_query/10_basic.yaml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.validate_query/10_basic.yaml @@ -1,5 +1,4 @@ ---- -"Validate query api": +setup: - do: indices.create: index: testing @@ -11,6 +10,8 @@ cluster.health: wait_for_status: yellow +--- +"Validate query api": - do: indices.validate_query: q: query string @@ -34,3 +35,11 @@ - match: {explanations.0.index: 'testing'} - match: {explanations.0.explanation: '*:*'} +--- +"Validate body without query element": + - do: + indices.validate_query: + body: + match_all: {} + + - is_false: valid diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/20_default_values.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/20_default_values.yaml index 6921a58d886..5cdde2cb696 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/20_default_values.yaml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/20_default_values.yaml @@ -1,5 +1,4 @@ ---- -"Default index": +setup: - do: indices.create: index: test_2 @@ -24,6 +23,9 @@ indices.refresh: index: [test_1, test_2] +--- +"Basic search": + - do: search: index: _all @@ -62,3 +64,14 @@ - match: {hits.hits.0._index: test_2 } - match: {hits.hits.0._type: test } - match: {hits.hits.0._id: "42" } + +--- +"Search body without query element": + + - do: + catch: request + search: + body: + match: + foo: bar +