From 359f45988fa5550ff44a0f805fa050eb9a221405 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Wed, 20 Apr 2016 13:19:12 +0200 Subject: [PATCH 1/4] Handle empty query bodies at parse time and remove EmptyQueryBuilder Currently we support empty query clauses like the filter in "constant_score" : { "filter" : { } } How these clauses are handled depends on the surrounding query. They later are either ignored, converted to match all or no documents or passed up further in the query hierarchy. During parsing these claues are currently represented as EmptyQueryBuilders. When not handled anywhere else, these special cases need to be checked for on the shard when building the lucene query. This is trappy, so this PR changes the parsing of compound queries. Instead of returning QueryBuilder, the core query parsing method QueryShardContext#parseInnerQueryBuilder() now return an Optional which can be empty in the case of empty query clauses. This has the advantage of forcing callers to deal with this sooner or later. When encountering empty Optionals, compound query builders now have the choice to ignore them, pass them on or rewrite to a different query, depending on context. --- .../cluster/metadata/AliasValidator.java | 8 +- .../org/elasticsearch/index/IndexService.java | 8 +- .../index/query/BoolQueryBuilder.java | 30 +++---- .../index/query/BoostingQueryBuilder.java | 14 ++-- .../index/query/CommonTermsQueryBuilder.java | 7 +- .../query/ConstantScoreQueryBuilder.java | 14 +++- .../index/query/DisMaxQueryBuilder.java | 11 ++- .../index/query/ExistsQueryBuilder.java | 5 +- .../query/FieldMaskingSpanQueryBuilder.java | 11 +-- .../index/query/FuzzyQueryBuilder.java | 7 +- .../query/GeoBoundingBoxQueryBuilder.java | 5 +- .../index/query/GeoDistanceQueryBuilder.java | 5 +- .../query/GeoDistanceRangeQueryBuilder.java | 5 +- .../index/query/GeoPolygonQueryBuilder.java | 5 +- .../index/query/GeoShapeQueryBuilder.java | 5 +- .../index/query/GeohashCellQuery.java | 7 +- .../index/query/HasChildQueryBuilder.java | 15 +++- .../index/query/HasParentQueryBuilder.java | 13 ++- .../index/query/IdsQueryBuilder.java | 5 +- .../index/query/IndicesQueryBuilder.java | 12 +-- .../index/query/InnerHitBuilder.java | 5 +- .../index/query/MatchAllQueryBuilder.java | 5 +- .../index/query/MatchNoneQueryBuilder.java | 5 +- .../query/MatchPhrasePrefixQueryBuilder.java | 5 +- .../index/query/MatchPhraseQueryBuilder.java | 6 +- .../index/query/MatchQueryBuilder.java | 5 +- .../index/query/MoreLikeThisQueryBuilder.java | 4 +- .../index/query/MultiMatchQueryBuilder.java | 7 +- .../index/query/NestedQueryBuilder.java | 14 +++- .../index/query/ParentIdQueryBuilder.java | 5 +- .../index/query/PrefixQueryBuilder.java | 7 +- .../index/query/QueryParseContext.java | 22 +++-- .../index/query/QueryParser.java | 10 ++- .../index/query/QueryStringQueryBuilder.java | 5 +- .../index/query/RangeQueryBuilder.java | 5 +- .../index/query/RegexpQueryBuilder.java | 7 +- .../index/query/ScriptQueryBuilder.java | 9 +- .../index/query/SimpleQueryStringBuilder.java | 5 +- .../query/SpanContainingQueryBuilder.java | 17 ++-- .../index/query/SpanFirstQueryBuilder.java | 11 +-- .../query/SpanMultiTermQueryBuilder.java | 11 +-- .../index/query/SpanNearQueryBuilder.java | 11 +-- .../index/query/SpanNotQueryBuilder.java | 17 ++-- .../index/query/SpanOrQueryBuilder.java | 11 +-- .../index/query/SpanTermQueryBuilder.java | 5 +- .../index/query/SpanWithinQueryBuilder.java | 17 ++-- .../index/query/TemplateQueryBuilder.java | 9 +- .../index/query/TermQueryBuilder.java | 5 +- .../index/query/TermsQueryBuilder.java | 7 +- .../index/query/TypeQueryBuilder.java | 7 +- .../index/query/WildcardQueryBuilder.java | 7 +- .../index/query/WrapperQueryBuilder.java | 8 +- .../FunctionScoreQueryBuilder.java | 16 ++-- .../elasticsearch/search/SearchModule.java | 4 - .../filter/FilterAggregationBuilder.java | 20 +---- .../filters/FiltersAggregationBuilder.java | 9 +- .../bucket/filters/FiltersAggregator.java | 7 +- .../significant/SignificantTermsParser.java | 7 +- .../search/builder/SearchSourceBuilder.java | 6 +- .../highlight/AbstractHighlighterBuilder.java | 2 +- .../search/rescore/QueryRescorerBuilder.java | 3 +- .../search/sort/FieldSortBuilder.java | 7 +- .../search/sort/GeoDistanceSortBuilder.java | 11 +-- .../search/sort/ScriptSortBuilder.java | 7 +- .../suggest/phrase/PhraseSuggester.java | 6 +- .../index/query/BoolQueryBuilderTests.java | 25 ++++++ .../query/BoostingQueryBuilderTests.java | 39 +++++++++ .../query/ConstantScoreQueryBuilderTests.java | 25 ++++++ .../index/query/DisMaxQueryBuilderTests.java | 18 ++-- .../query/HasChildQueryBuilderTests.java | 43 +++++++--- .../query/HasParentQueryBuilderTests.java | 36 ++++++-- .../index/query/NestedQueryBuilderTests.java | 10 +-- .../index/query/RandomQueryBuilder.java | 4 +- .../index/query/TermsQueryBuilderTests.java | 8 +- .../FunctionScoreQueryBuilderTests.java | 27 +++++- .../query/plugin/CustomQueryParserIT.java | 22 ++--- .../query/plugin/DummyQueryBuilder.java} | 83 +++++++++---------- .../query/plugin/DummyQueryParserPlugin.java | 61 +------------- .../search/aggregations/bucket/FilterIT.java | 21 ++++- .../search/aggregations/bucket/FiltersIT.java | 26 ++++-- .../builder/SearchSourceBuilderTests.java | 24 ++++-- .../messy/tests/TemplateQueryParserTests.java | 12 +-- .../percolator/PercolateQueryBuilder.java | 7 +- .../percolator/PercolatorFieldMapper.java | 3 +- .../percolator/TransportPercolateAction.java | 4 +- .../PercolatorFieldMapperTests.java | 2 +- .../test/AbstractQueryTestCase.java | 26 +++--- 87 files changed, 653 insertions(+), 444 deletions(-) rename core/src/{main/java/org/elasticsearch/index/query/EmptyQueryBuilder.java => test/java/org/elasticsearch/index/query/plugin/DummyQueryBuilder.java} (55%) diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/AliasValidator.java b/core/src/main/java/org/elasticsearch/cluster/metadata/AliasValidator.java index 735916504da..647e30cd85c 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/AliasValidator.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/AliasValidator.java @@ -32,6 +32,7 @@ import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.indices.InvalidAliasNameException; import java.io.IOException; +import java.util.Optional; /** * Validator for an alias, to be used before adding an alias to the index metadata @@ -141,7 +142,10 @@ public class AliasValidator extends AbstractComponent { private static void validateAliasFilter(XContentParser parser, QueryShardContext queryShardContext) throws IOException { QueryParseContext queryParseContext = queryShardContext.newParseContext(parser); - QueryBuilder queryBuilder = QueryBuilder.rewriteQuery(queryParseContext.parseInnerQueryBuilder(), queryShardContext); - queryBuilder.toFilter(queryShardContext); + Optional parseInnerQueryBuilder = queryParseContext.parseInnerQueryBuilder(); + if (parseInnerQueryBuilder.isPresent()) { + QueryBuilder queryBuilder = QueryBuilder.rewriteQuery(parseInnerQueryBuilder.get(), queryShardContext); + queryBuilder.toFilter(queryShardContext); + } } } diff --git a/core/src/main/java/org/elasticsearch/index/IndexService.java b/core/src/main/java/org/elasticsearch/index/IndexService.java index f5e5ce91d80..e2d2ea4b8fe 100644 --- a/core/src/main/java/org/elasticsearch/index/IndexService.java +++ b/core/src/main/java/org/elasticsearch/index/IndexService.java @@ -51,6 +51,7 @@ import org.elasticsearch.index.fielddata.IndexFieldDataCache; import org.elasticsearch.index.fielddata.IndexFieldDataService; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.query.ParsedQuery; +import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.shard.IndexEventListener; import org.elasticsearch.index.shard.IndexSearcherWrapper; @@ -80,6 +81,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -601,7 +603,11 @@ public final class IndexService extends AbstractIndexComponent implements IndexC try { byte[] filterSource = alias.filter().uncompressed(); try (XContentParser parser = XContentFactory.xContent(filterSource).createParser(filterSource)) { - ParsedQuery parsedFilter = shardContext.toFilter(shardContext.newParseContext(parser).parseInnerQueryBuilder()); + Optional innerQueryBuilder = shardContext.newParseContext(parser).parseInnerQueryBuilder(); + ParsedQuery parsedFilter = null; + if (innerQueryBuilder.isPresent()) { + parsedFilter = shardContext.toFilter(innerQueryBuilder.get()); + } return parsedFilter == null ? null : parsedFilter.query(); } } catch (IOException ex) { diff --git a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java index e5aa774addc..863bdb25c71 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java @@ -37,6 +37,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.function.Consumer; import static org.elasticsearch.common.lucene.search.Queries.fixNegativeQueryIfNeeded; @@ -300,7 +301,7 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { builder.endArray(); } - public static BoolQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, ParsingException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException, ParsingException { XContentParser parser = parseContext.parser(); boolean disableCoord = BoolQueryBuilder.DISABLE_COORD_DEFAULT; @@ -316,7 +317,6 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { String currentFieldName = null; XContentParser.Token token; - QueryBuilder query; while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); @@ -325,21 +325,17 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { } else if (token == XContentParser.Token.START_OBJECT) { switch (currentFieldName) { case MUST: - query = parseContext.parseInnerQueryBuilder(); - mustClauses.add(query); + parseContext.parseInnerQueryBuilder().ifPresent(mustClauses::add); break; case SHOULD: - query = parseContext.parseInnerQueryBuilder(); - shouldClauses.add(query); + parseContext.parseInnerQueryBuilder().ifPresent(shouldClauses::add); break; case FILTER: - query = parseContext.parseInnerQueryBuilder(); - filterClauses.add(query); + parseContext.parseInnerQueryBuilder().ifPresent(filterClauses::add); break; case MUST_NOT: case MUSTNOT: - query = parseContext.parseInnerQueryBuilder(); - mustNotClauses.add(query); + parseContext.parseInnerQueryBuilder().ifPresent(mustNotClauses::add); break; default: throw new ParsingException(parser.getTokenLocation(), "[bool] query does not support [" + currentFieldName + "]"); @@ -348,21 +344,17 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { switch (currentFieldName) { case MUST: - query = parseContext.parseInnerQueryBuilder(); - mustClauses.add(query); + parseContext.parseInnerQueryBuilder().ifPresent(mustClauses::add); break; case SHOULD: - query = parseContext.parseInnerQueryBuilder(); - shouldClauses.add(query); + parseContext.parseInnerQueryBuilder().ifPresent(shouldClauses::add); break; case FILTER: - query = parseContext.parseInnerQueryBuilder(); - filterClauses.add(query); + parseContext.parseInnerQueryBuilder().ifPresent(filterClauses::add); break; case MUST_NOT: case MUSTNOT: - query = parseContext.parseInnerQueryBuilder(); - mustNotClauses.add(query); + parseContext.parseInnerQueryBuilder().ifPresent(mustNotClauses::add); break; default: throw new ParsingException(parser.getTokenLocation(), "bool query does not support [" + currentFieldName + "]"); @@ -404,7 +396,7 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { boolQuery.adjustPureNegative(adjustPureNegative); boolQuery.minimumNumberShouldMatch(minimumShouldMatch); boolQuery.queryName(queryName); - return boolQuery; + return Optional.of(boolQuery); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java index 496cb7ec8a7..3afa4339b5c 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java @@ -31,6 +31,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; import java.util.Map; import java.util.Objects; +import java.util.Optional; /** * The BoostingQuery class can be used to effectively demote results that match a given query. @@ -138,12 +139,12 @@ public class BoostingQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); - QueryBuilder positiveQuery = null; + Optional positiveQuery = null; boolean positiveQueryFound = false; - QueryBuilder negativeQuery = null; + Optional negativeQuery = null; boolean negativeQueryFound = false; float boost = AbstractQueryBuilder.DEFAULT_BOOST; float negativeBoost = -1; @@ -187,12 +188,15 @@ public class BoostingQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); XContentParser.Token token = parser.nextToken(); if (token != XContentParser.Token.FIELD_NAME) { @@ -352,7 +353,7 @@ public class CommonTermsQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); - QueryBuilder query = null; + Optional query = Optional.empty(); boolean queryFound = false; String queryName = null; float boost = AbstractQueryBuilder.DEFAULT_BOOST; @@ -131,10 +132,15 @@ public class ConstantScoreQueryBuilder extends AbstractQueryBuilder builder.endObject(); } - public static DisMaxQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); float boost = AbstractQueryBuilder.DEFAULT_BOOST; @@ -140,8 +141,7 @@ public class DisMaxQueryBuilder extends AbstractQueryBuilder } else if (token == XContentParser.Token.START_OBJECT) { if (parseContext.getParseFieldMatcher().match(currentFieldName, QUERIES_FIELD)) { queriesFound = true; - QueryBuilder query = parseContext.parseInnerQueryBuilder(); - queries.add(query); + parseContext.parseInnerQueryBuilder().ifPresent(queries::add); } else { throw new ParsingException(parser.getTokenLocation(), "[dis_max] query does not support [" + currentFieldName + "]"); } @@ -149,8 +149,7 @@ public class DisMaxQueryBuilder extends AbstractQueryBuilder if (parseContext.getParseFieldMatcher().match(currentFieldName, QUERIES_FIELD)) { queriesFound = true; while (token != XContentParser.Token.END_ARRAY) { - QueryBuilder query = parseContext.parseInnerQueryBuilder(); - queries.add(query); + parseContext.parseInnerQueryBuilder().ifPresent(queries::add); token = parser.nextToken(); } } else { @@ -180,7 +179,7 @@ public class DisMaxQueryBuilder extends AbstractQueryBuilder for (QueryBuilder query : queries) { disMaxQuery.add(query); } - return disMaxQuery; + return Optional.of(disMaxQuery); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java index afcff164bec..c933bae68c0 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java @@ -37,6 +37,7 @@ import java.io.IOException; import java.util.Collection; import java.util.Collections; import java.util.Objects; +import java.util.Optional; /** * Constructs a query that only match on documents that the field has a value in them. @@ -85,7 +86,7 @@ public class ExistsQueryBuilder extends AbstractQueryBuilder builder.endObject(); } - public static ExistsQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String fieldPattern = null; @@ -121,7 +122,7 @@ public class ExistsQueryBuilder extends AbstractQueryBuilder ExistsQueryBuilder builder = new ExistsQueryBuilder(fieldPattern); builder.queryName(queryName); builder.boost(boost); - return builder; + return Optional.of(builder); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/FieldMaskingSpanQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/FieldMaskingSpanQueryBuilder.java index 0382f353cb3..801b0ae5a81 100644 --- a/core/src/main/java/org/elasticsearch/index/query/FieldMaskingSpanQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/FieldMaskingSpanQueryBuilder.java @@ -33,6 +33,7 @@ import org.elasticsearch.index.mapper.MappedFieldType; import java.io.IOException; import java.util.Objects; +import java.util.Optional; public class FieldMaskingSpanQueryBuilder extends AbstractQueryBuilder implements SpanQueryBuilder { @@ -103,7 +104,7 @@ public class FieldMaskingSpanQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); float boost = AbstractQueryBuilder.DEFAULT_BOOST; @@ -119,11 +120,11 @@ public class FieldMaskingSpanQueryBuilder extends AbstractQueryBuilder query = parseContext.parseInnerQueryBuilder(); + if (query.isPresent() == false || query.get() instanceof SpanQueryBuilder == false) { throw new ParsingException(parser.getTokenLocation(), "[field_masking_span] query must be of type span query"); } - inner = (SpanQueryBuilder) query; + inner = (SpanQueryBuilder) query.get(); } else { throw new ParsingException(parser.getTokenLocation(), "[field_masking_span] query does not support [" + currentFieldName + "]"); @@ -151,7 +152,7 @@ public class FieldMaskingSpanQueryBuilder extends AbstractQueryBuilder i builder.endObject(); } - public static FuzzyQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); XContentParser.Token token = parser.nextToken(); @@ -317,14 +318,14 @@ public class FuzzyQueryBuilder extends AbstractQueryBuilder i if (value == null) { throw new ParsingException(parser.getTokenLocation(), "no value specified for fuzzy query"); } - return new FuzzyQueryBuilder(fieldName, value) + return Optional.of(new FuzzyQueryBuilder(fieldName, value) .fuzziness(fuzziness) .prefixLength(prefixLength) .maxExpansions(maxExpansions) .transpositions(transpositions) .rewrite(rewrite) .boost(boost) - .queryName(queryName); + .queryName(queryName)); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java index 7f5d5e2ec8d..c1925fe3f39 100644 --- a/core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java @@ -43,6 +43,7 @@ import org.elasticsearch.index.search.geo.IndexedGeoBoundingBoxQuery; import java.io.IOException; import java.util.Objects; +import java.util.Optional; /** * Creates a Lucene query that will filter for all documents that lie within the specified @@ -385,7 +386,7 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String fieldName = null; @@ -496,7 +497,7 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); XContentParser.Token token; @@ -445,7 +446,7 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); XContentParser.Token token; @@ -603,7 +604,7 @@ public class GeoDistanceRangeQueryBuilder extends AbstractQueryBuilder { @@ -241,7 +242,7 @@ public class GeoPolygonQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String fieldName = null; @@ -322,7 +323,7 @@ public class GeoPolygonQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String fieldName = null; @@ -559,7 +560,7 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String fieldName = null; @@ -362,7 +363,7 @@ public class GeohashCellQuery { builder.boost(boost); } builder.ignoreUnmapped(ignoreUnmapped); - return builder; + return Optional.of(builder); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java index 990b5a35fd9..410fda41aef 100644 --- a/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java @@ -43,6 +43,7 @@ import java.io.IOException; import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Optional; /** * A query builder for has_child query. @@ -226,7 +227,7 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); float boost = AbstractQueryBuilder.DEFAULT_BOOST; String childType = null; @@ -238,7 +239,7 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder iqb = Optional.empty(); while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); @@ -272,7 +273,13 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); float boost = AbstractQueryBuilder.DEFAULT_BOOST; String parentType = null; @@ -238,7 +239,7 @@ public class HasParentQueryBuilder extends AbstractQueryBuilder iqb = Optional.empty(); while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); @@ -276,14 +277,18 @@ public class HasParentQueryBuilder extends AbstractQueryBuilder { builder.endObject(); } - public static IdsQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); List ids = new ArrayList<>(); List types = new ArrayList<>(); @@ -191,7 +192,7 @@ public class IdsQueryBuilder extends AbstractQueryBuilder { IdsQueryBuilder query = new IdsQueryBuilder(types.toArray(new String[types.size()])); query.addIds(ids.toArray(new String[ids.size()])); query.boost(boost).queryName(queryName); - return query; + return Optional.of(query); } diff --git a/core/src/main/java/org/elasticsearch/index/query/IndicesQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/IndicesQueryBuilder.java index 7cfdf1baa1e..9758ad3f542 100644 --- a/core/src/main/java/org/elasticsearch/index/query/IndicesQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/IndicesQueryBuilder.java @@ -34,6 +34,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Objects; +import java.util.Optional; /** * A query that will execute the wrapped query only for the specified indices, @@ -141,7 +142,7 @@ public class IndicesQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, ParsingException { XContentParser parser = parseContext.parser(); QueryBuilder innerQuery = null; @@ -158,9 +159,10 @@ public class IndicesQueryBuilder extends AbstractQueryBuilder PARSER = new ObjectParser<>("inner_hits", InnerHitBuilder::new); @@ -131,7 +132,7 @@ public final class InnerHitBuilder extends ToXContentToBytes implements Writeabl private boolean trackScores; private List fieldNames; - private QueryBuilder query = new MatchAllQueryBuilder(); + private QueryBuilder query = DEFAULT_INNER_HIT_QUERY; private List> sorts; private List fieldDataFields; private Set scriptFields; @@ -394,7 +395,7 @@ public final class InnerHitBuilder extends ToXContentToBytes implements Writeabl return this; } - public InnerHitBuilder addSort(SortBuilder sort) { + public InnerHitBuilder addSort(SortBuilder sort) { if (sorts == null) { sorts = new ArrayList<>(); } diff --git a/core/src/main/java/org/elasticsearch/index/query/MatchAllQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/MatchAllQueryBuilder.java index 2d415c2b2a8..21742eb8771 100644 --- a/core/src/main/java/org/elasticsearch/index/query/MatchAllQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/MatchAllQueryBuilder.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; +import java.util.Optional; /** * A query that matches on all documents. @@ -60,7 +61,7 @@ public class MatchAllQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String currentFieldName = null; @@ -87,7 +88,7 @@ public class MatchAllQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String currentFieldName = null; @@ -88,7 +89,7 @@ public class MatchNoneQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); XContentParser.Token token = parser.nextToken(); @@ -257,6 +258,6 @@ public class MatchPhrasePrefixQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); XContentParser.Token token = parser.nextToken(); @@ -223,6 +225,6 @@ public class MatchPhraseQueryBuilder extends AbstractQueryBuilder { return NAME; } - public static MatchQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); XContentParser.Token token = parser.nextToken(); @@ -633,7 +634,7 @@ public class MatchQueryBuilder extends AbstractQueryBuilder { matchQuery.zeroTermsQuery(zeroTermsQuery); matchQuery.queryName(queryName); matchQuery.boost(boost); - return matchQuery; + return Optional.of(matchQuery); } } diff --git a/core/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java index 66f623cbbb3..5fa36ec9777 100644 --- a/core/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java @@ -805,7 +805,7 @@ public class MoreLikeThisQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); // document inputs @@ -955,7 +955,7 @@ public class MoreLikeThisQueryBuilder extends AbstractQueryBuilder texts, List items) throws IOException { diff --git a/core/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java index 032feac9193..0d93b1331bb 100644 --- a/core/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/MultiMatchQueryBuilder.java @@ -43,6 +43,7 @@ import java.util.HashMap; import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.TreeMap; /** @@ -556,7 +557,7 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); Object value = null; @@ -660,7 +661,7 @@ public class MultiMatchQueryBuilder extends AbstractQueryBuilder fieldsBoosts) throws IOException { diff --git a/core/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java index 5d74b540116..1d0e6d11f5f 100644 --- a/core/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java @@ -36,6 +36,7 @@ import org.elasticsearch.index.mapper.object.ObjectMapper; import java.io.IOException; import java.util.Map; import java.util.Objects; +import java.util.Optional; public class NestedQueryBuilder extends AbstractQueryBuilder { @@ -156,12 +157,12 @@ public class NestedQueryBuilder extends AbstractQueryBuilder builder.endObject(); } - public static NestedQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); float boost = AbstractQueryBuilder.DEFAULT_BOOST; ScoreMode scoreMode = ScoreMode.Avg; String queryName = null; - QueryBuilder query = null; + Optional query = Optional.empty(); String path = null; String currentFieldName = null; InnerHitBuilder innerHitBuilder = null; @@ -194,14 +195,19 @@ public class NestedQueryBuilder extends AbstractQueryBuilder } } } - NestedQueryBuilder queryBuilder = new NestedQueryBuilder(path, query, scoreMode) + + if (query.isPresent() == false) { + // if inner query is empty, bubble this up to caller so they can decide how to deal with it + return Optional.empty(); + } + NestedQueryBuilder queryBuilder = new NestedQueryBuilder(path, query.get(), scoreMode) .ignoreUnmapped(ignoreUnmapped) .queryName(queryName) .boost(boost); if (innerHitBuilder != null) { queryBuilder.innerHit(innerHitBuilder); } - return queryBuilder; + return Optional.of(queryBuilder); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/ParentIdQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ParentIdQueryBuilder.java index 419149ce992..ede449cbd39 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ParentIdQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ParentIdQueryBuilder.java @@ -38,6 +38,7 @@ import org.elasticsearch.index.mapper.internal.TypeFieldMapper; import java.io.IOException; import java.util.Objects; +import java.util.Optional; public final class ParentIdQueryBuilder extends AbstractQueryBuilder { @@ -117,7 +118,7 @@ public final class ParentIdQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); float boost = AbstractQueryBuilder.DEFAULT_BOOST; String type = null; @@ -151,7 +152,7 @@ public final class ParentIdQueryBuilder extends AbstractQueryBuilder builder.endObject(); } - public static PrefixQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String fieldName = parser.currentName(); @@ -163,10 +164,10 @@ public class PrefixQueryBuilder extends AbstractQueryBuilder if (value == null) { throw new ParsingException(parser.getTokenLocation(), "No value specified for prefix query"); } - return new PrefixQueryBuilder(fieldName, value) + return Optional.of(new PrefixQueryBuilder(fieldName, value) .rewrite(rewrite) .boost(boost) - .queryName(queryName); + .queryName(queryName)); } @Override 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 62662914fd8..93bd07c7bf9 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java @@ -23,14 +23,19 @@ import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParseFieldMatcherSupplier; import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.indices.query.IndicesQueriesRegistry; import java.io.IOException; import java.util.Objects; +import java.util.Optional; public class QueryParseContext implements ParseFieldMatcherSupplier { + private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(QueryParseContext.class)); + private static final ParseField CACHE = new ParseField("_cache").withAllDeprecated("Elasticsearch makes its own caching decisions"); private static final ParseField CACHE_KEY = new ParseField("_cache_key").withAllDeprecated("Filters are always used as cache keys"); @@ -62,7 +67,7 @@ public class QueryParseContext implements ParseFieldMatcherSupplier { if (token == XContentParser.Token.FIELD_NAME) { String fieldName = parser.currentName(); if ("query".equals(fieldName)) { - queryBuilder = parseInnerQueryBuilder(); + queryBuilder = parseInnerQueryBuilder().orElse(null); } else { throw new ParsingException(parser.getTokenLocation(), "request does not support [" + parser.currentName() + "]"); } @@ -82,7 +87,7 @@ public class QueryParseContext implements ParseFieldMatcherSupplier { /** * Parses a query excluding the query element that wraps it */ - public QueryBuilder parseInnerQueryBuilder() throws IOException { + public Optional parseInnerQueryBuilder() throws IOException { // move to START object XContentParser.Token token; if (parser.currentToken() != XContentParser.Token.START_OBJECT) { @@ -93,8 +98,13 @@ public class QueryParseContext implements ParseFieldMatcherSupplier { } token = parser.nextToken(); if (token == XContentParser.Token.END_OBJECT) { - // empty query - return new EmptyQueryBuilder(); + // we encountered '{}' for a query clause + String msg = "query malformed, empty clause found at [" + parser.getTokenLocation() +"]"; + DEPRECATION_LOGGER.deprecated(msg); + if (parseFieldMatcher.isStrict()) { + throw new IllegalArgumentException(msg); + } + return Optional.empty(); } if (token != XContentParser.Token.FIELD_NAME) { throw new ParsingException(parser.getTokenLocation(), "[_na] query malformed, no field after start_object"); @@ -105,7 +115,9 @@ public class QueryParseContext implements ParseFieldMatcherSupplier { if (token != XContentParser.Token.START_OBJECT && token != XContentParser.Token.START_ARRAY) { throw new ParsingException(parser.getTokenLocation(), "[_na] query malformed, no field after start_object"); } - QueryBuilder result = indicesQueriesRegistry.lookup(queryName, parseFieldMatcher, parser.getTokenLocation()).fromXContent(this); + @SuppressWarnings("unchecked") + Optional result = (Optional) indicesQueriesRegistry.lookup(queryName, parseFieldMatcher, + parser.getTokenLocation()).fromXContent(this); if (parser.currentToken() == XContentParser.Token.END_OBJECT || parser.currentToken() == XContentParser.Token.END_ARRAY) { // if we are at END_OBJECT, move to the next one... parser.nextToken(); diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryParser.java b/core/src/main/java/org/elasticsearch/index/query/QueryParser.java index 069dc86cf8a..222d3bd4963 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryParser.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.query; import java.io.IOException; +import java.util.Optional; /** * Defines a query parser that is able to parse {@link QueryBuilder}s from {@link org.elasticsearch.common.xcontent.XContent}. @@ -36,5 +37,12 @@ public interface QueryParser { * call * @return the new QueryBuilder */ - QB fromXContent(QueryParseContext parseContext) throws IOException; + Optional fromXContent(QueryParseContext parseContext) throws IOException; + + /** + * @return an empty {@link QueryBuilder} instance for this parser that can be used for deserialization + */ + default QB getBuilderPrototype() { // TODO remove this when nothing implements it + throw new UnsupportedOperationException(); + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryStringQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/QueryStringQueryBuilder.java index 6806ae944c2..c390507a785 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryStringQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryStringQueryBuilder.java @@ -46,6 +46,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.TreeMap; /** @@ -631,7 +632,7 @@ public class QueryStringQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String currentFieldName = null; XContentParser.Token token; @@ -792,7 +793,7 @@ public class QueryStringQueryBuilder extends AbstractQueryBuilder i builder.endObject(); } - public static RangeQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String fieldName = null; @@ -381,7 +382,7 @@ public class RangeQueryBuilder extends AbstractQueryBuilder i if (format != null) { rangeQuery.format(format); } - return rangeQuery; + return Optional.of(rangeQuery); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/RegexpQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/RegexpQueryBuilder.java index 703b2463b11..27462cfb26e 100644 --- a/core/src/main/java/org/elasticsearch/index/query/RegexpQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/RegexpQueryBuilder.java @@ -37,6 +37,7 @@ import org.elasticsearch.index.query.support.QueryParsers; import java.io.IOException; import java.util.Objects; +import java.util.Optional; /** * A Query that does fuzzy matching for a specific value. @@ -179,7 +180,7 @@ public class RegexpQueryBuilder extends AbstractQueryBuilder builder.endObject(); } - public static RegexpQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String fieldName = parser.currentName(); @@ -237,12 +238,12 @@ public class RegexpQueryBuilder extends AbstractQueryBuilder if (value == null) { throw new ParsingException(parser.getTokenLocation(), "No value specified for regexp query"); } - return new RegexpQueryBuilder(fieldName, value) + return Optional.of(new RegexpQueryBuilder(fieldName, value) .flags(flagsValue) .maxDeterminizedStates(maxDeterminizedStates) .rewrite(rewrite) .boost(boost) - .queryName(queryName); + .queryName(queryName)); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java index 12aba8ae879..6d563f22a06 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java @@ -35,9 +35,9 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.script.LeafSearchScript; import org.elasticsearch.script.Script; import org.elasticsearch.script.Script.ScriptField; -import org.elasticsearch.script.ScriptParameterParser.ScriptParameterValue; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptParameterParser; +import org.elasticsearch.script.ScriptParameterParser.ScriptParameterValue; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.SearchScript; import org.elasticsearch.search.lookup.SearchLookup; @@ -47,6 +47,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Objects; +import java.util.Optional; public class ScriptQueryBuilder extends AbstractQueryBuilder { @@ -94,7 +95,7 @@ public class ScriptQueryBuilder extends AbstractQueryBuilder builder.endObject(); } - public static ScriptQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); ScriptParameterParser scriptParameterParser = new ScriptParameterParser(); @@ -149,9 +150,9 @@ public class ScriptQueryBuilder extends AbstractQueryBuilder throw new ParsingException(parser.getTokenLocation(), "script must be provided with a [script] filter"); } - return new ScriptQueryBuilder(script) + return Optional.of(new ScriptQueryBuilder(script) .boost(boost) - .queryName(queryName); + .queryName(queryName)); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java b/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java index 5e2c88af923..0bedf67820b 100644 --- a/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java @@ -39,6 +39,7 @@ import java.util.HashMap; import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.TreeMap; /** @@ -417,7 +418,7 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String currentFieldName = null; @@ -514,7 +515,7 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); float boost = AbstractQueryBuilder.DEFAULT_BOOST; String queryName = null; @@ -116,17 +117,17 @@ public class SpanContainingQueryBuilder extends AbstractQueryBuilder query = parseContext.parseInnerQueryBuilder(); + if (query.isPresent() == false || query.get() instanceof SpanQueryBuilder == false) { throw new ParsingException(parser.getTokenLocation(), "span_containing [big] must be of type span query"); } - big = (SpanQueryBuilder) query; + big = (SpanQueryBuilder) query.get(); } else if (parseContext.getParseFieldMatcher().match(currentFieldName, LITTLE_FIELD)) { - QueryBuilder query = parseContext.parseInnerQueryBuilder(); - if (!(query instanceof SpanQueryBuilder)) { + Optional query = parseContext.parseInnerQueryBuilder(); + if (query.isPresent() == false || query.get() instanceof SpanQueryBuilder == false) { throw new ParsingException(parser.getTokenLocation(), "span_containing [little] must be of type span query"); } - little = (SpanQueryBuilder) query; + little = (SpanQueryBuilder) query.get(); } else { throw new ParsingException(parser.getTokenLocation(), "[span_containing] query does not support [" + currentFieldName + "]"); @@ -143,7 +144,7 @@ public class SpanContainingQueryBuilder extends AbstractQueryBuilder implements SpanQueryBuilder { @@ -101,7 +102,7 @@ public class SpanFirstQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); float boost = AbstractQueryBuilder.DEFAULT_BOOST; @@ -117,11 +118,11 @@ public class SpanFirstQueryBuilder extends AbstractQueryBuilder query = parseContext.parseInnerQueryBuilder(); + if (query.isPresent() == false || query.get() instanceof SpanQueryBuilder == false) { throw new ParsingException(parser.getTokenLocation(), "spanFirst [match] must be of type span query"); } - match = (SpanQueryBuilder) query; + match = (SpanQueryBuilder) query.get(); } else { throw new ParsingException(parser.getTokenLocation(), "[span_first] query does not support [" + currentFieldName + "]"); } @@ -145,7 +146,7 @@ public class SpanFirstQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String currentFieldName = null; MultiTermQueryBuilder subQuery = null; @@ -94,12 +95,12 @@ public class SpanMultiTermQueryBuilder extends AbstractQueryBuilder query = parseContext.parseInnerQueryBuilder(); + if (query.isPresent() == false || query.get() instanceof MultiTermQueryBuilder == false) { throw new ParsingException(parser.getTokenLocation(), "[span_multi] [" + MATCH_FIELD.getPreferredName() + "] must be of type multi term query"); } - subQuery = (MultiTermQueryBuilder) innerQuery; + subQuery = (MultiTermQueryBuilder) query.get(); } else { throw new ParsingException(parser.getTokenLocation(), "[span_multi] query does not support [" + currentFieldName + "]"); } @@ -119,7 +120,7 @@ public class SpanMultiTermQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); float boost = AbstractQueryBuilder.DEFAULT_BOOST; @@ -164,11 +165,11 @@ public class SpanNearQueryBuilder extends AbstractQueryBuilder query = parseContext.parseInnerQueryBuilder(); + if (query.isPresent() == false || query.get() instanceof SpanQueryBuilder == false) { throw new ParsingException(parser.getTokenLocation(), "spanNear [clauses] must be of type span query"); } - clauses.add((SpanQueryBuilder) query); + clauses.add((SpanQueryBuilder) query.get()); } } else { throw new ParsingException(parser.getTokenLocation(), "[span_near] query does not support [" + currentFieldName + "]"); @@ -207,7 +208,7 @@ public class SpanNearQueryBuilder extends AbstractQueryBuilder implements SpanQueryBuilder { @@ -162,7 +163,7 @@ public class SpanNotQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); float boost = AbstractQueryBuilder.DEFAULT_BOOST; @@ -183,17 +184,17 @@ public class SpanNotQueryBuilder extends AbstractQueryBuilder query = parseContext.parseInnerQueryBuilder(); + if (query.isPresent() == false || query.get() instanceof SpanQueryBuilder == false) { throw new ParsingException(parser.getTokenLocation(), "spanNot [include] must be of type span query"); } - include = (SpanQueryBuilder) query; + include = (SpanQueryBuilder) query.get(); } else if (parseContext.getParseFieldMatcher().match(currentFieldName, EXCLUDE_FIELD)) { - QueryBuilder query = parseContext.parseInnerQueryBuilder(); - if (!(query instanceof SpanQueryBuilder)) { + Optional query = parseContext.parseInnerQueryBuilder(); + if (query.isPresent() == false || query.get() instanceof SpanQueryBuilder == false) { throw new ParsingException(parser.getTokenLocation(), "spanNot [exclude] must be of type span query"); } - exclude = (SpanQueryBuilder) query; + exclude = (SpanQueryBuilder) query.get(); } else { throw new ParsingException(parser.getTokenLocation(), "[span_not] query does not support [" + currentFieldName + "]"); } @@ -235,7 +236,7 @@ public class SpanNotQueryBuilder extends AbstractQueryBuilder builder.endObject(); } - public static SpanOrQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); float boost = AbstractQueryBuilder.DEFAULT_BOOST; @@ -115,11 +116,11 @@ public class SpanOrQueryBuilder extends AbstractQueryBuilder } else if (token == XContentParser.Token.START_ARRAY) { if (parseContext.getParseFieldMatcher().match(currentFieldName, CLAUSES_FIELD)) { while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { - QueryBuilder query = parseContext.parseInnerQueryBuilder(); - if (!(query instanceof SpanQueryBuilder)) { + Optional query = parseContext.parseInnerQueryBuilder(); + if (query.isPresent() == false || query.get() instanceof SpanQueryBuilder == false) { throw new ParsingException(parser.getTokenLocation(), "spanOr [clauses] must be of type span query"); } - clauses.add((SpanQueryBuilder) query); + clauses.add((SpanQueryBuilder) query.get()); } } else { throw new ParsingException(parser.getTokenLocation(), "[span_or] query does not support [" + currentFieldName + "]"); @@ -145,7 +146,7 @@ public class SpanOrQueryBuilder extends AbstractQueryBuilder } queryBuilder.boost(boost); queryBuilder.queryName(queryName); - return queryBuilder; + return Optional.of(queryBuilder); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/SpanTermQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/SpanTermQueryBuilder.java index 3bb374ff276..56581c39918 100644 --- a/core/src/main/java/org/elasticsearch/index/query/SpanTermQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/SpanTermQueryBuilder.java @@ -31,6 +31,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.mapper.MappedFieldType; import java.io.IOException; +import java.util.Optional; /** * A Span Query that matches documents containing a term. @@ -93,7 +94,7 @@ public class SpanTermQueryBuilder extends BaseTermQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, ParsingException { XContentParser parser = parseContext.parser(); XContentParser.Token token = parser.currentToken(); @@ -142,7 +143,7 @@ public class SpanTermQueryBuilder extends BaseTermQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); float boost = AbstractQueryBuilder.DEFAULT_BOOST; @@ -122,17 +123,17 @@ public class SpanWithinQueryBuilder extends AbstractQueryBuilder query = parseContext.parseInnerQueryBuilder(); + if (query.isPresent() == false || query.get() instanceof SpanQueryBuilder == false) { throw new ParsingException(parser.getTokenLocation(), "span_within [big] must be of type span query"); } - big = (SpanQueryBuilder) query; + big = (SpanQueryBuilder) query.get(); } else if (parseContext.getParseFieldMatcher().match(currentFieldName, LITTLE_FIELD)) { - QueryBuilder query = parseContext.parseInnerQueryBuilder(); - if (query instanceof SpanQueryBuilder == false) { + Optional query = parseContext.parseInnerQueryBuilder(); + if (query.isPresent() == false || query.get() instanceof SpanQueryBuilder == false) { throw new ParsingException(parser.getTokenLocation(), "span_within [little] must be of type span query"); } - little = (SpanQueryBuilder) query; + little = (SpanQueryBuilder) query.get(); } else { throw new ParsingException(parser.getTokenLocation(), "[span_within] query does not support [" + currentFieldName + "]"); @@ -155,7 +156,7 @@ public class SpanWithinQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); Template template = parse(parser, parseContext.getParseFieldMatcher()); - return new TemplateQueryBuilder(template); + return Optional.of(new TemplateQueryBuilder(template)); } public static Template parse(XContentParser parser, ParseFieldMatcher parseFieldMatcher, String... parameters) throws IOException { @@ -179,7 +181,8 @@ public class TemplateQueryBuilder extends AbstractQueryBuilder new ParsingException(qSourceParser.getTokenLocation(), "inner query in [" + NAME + "] cannot be empty"));; if (boost() != DEFAULT_BOOST || queryName() != null) { final BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder(); boolQueryBuilder.must(queryBuilder); diff --git a/core/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java index ffdf6a3337d..bfb55c9956c 100644 --- a/core/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java @@ -30,6 +30,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.mapper.MappedFieldType; import java.io.IOException; +import java.util.Optional; /** * A Query that matches documents containing a term. @@ -84,7 +85,7 @@ public class TermQueryBuilder extends BaseTermQueryBuilder { super(in); } - public static TermQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String queryName = null; @@ -140,7 +141,7 @@ public class TermQueryBuilder extends BaseTermQueryBuilder { if (queryName != null) { termQuery.queryName(queryName); } - return termQuery; + return Optional.of(termQuery); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java index 5b0650eb7e4..7d27a911f88 100644 --- a/core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java @@ -47,6 +47,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -238,7 +239,7 @@ public class TermsQueryBuilder extends AbstractQueryBuilder { builder.endObject(); } - public static TermsQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); String fieldName = null; @@ -289,9 +290,9 @@ public class TermsQueryBuilder extends AbstractQueryBuilder { throw new ParsingException(parser.getTokenLocation(), "[" + TermsQueryBuilder.NAME + "] query requires a field name, " + "followed by array of terms or a document lookup specification"); } - return new TermsQueryBuilder(fieldName, values, termsLookup) + return Optional.of(new TermsQueryBuilder(fieldName, values, termsLookup) .boost(boost) - .queryName(queryName); + .queryName(queryName)); } private static List parseValues(XContentParser parser) throws IOException { diff --git a/core/src/main/java/org/elasticsearch/index/query/TypeQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/TypeQueryBuilder.java index 129b0275527..00cd108c003 100644 --- a/core/src/main/java/org/elasticsearch/index/query/TypeQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/TypeQueryBuilder.java @@ -33,6 +33,7 @@ import org.elasticsearch.index.mapper.DocumentMapper; import java.io.IOException; import java.util.Objects; +import java.util.Optional; public class TypeQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "type"; @@ -81,7 +82,7 @@ public class TypeQueryBuilder extends AbstractQueryBuilder { builder.endObject(); } - public static TypeQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); BytesRef type = null; @@ -114,9 +115,9 @@ public class TypeQueryBuilder extends AbstractQueryBuilder { throw new ParsingException(parser.getTokenLocation(), "[" + TypeQueryBuilder.NAME + "] filter needs to be provided with a value for the type"); } - return new TypeQueryBuilder(type) + return Optional.of(new TypeQueryBuilder(type) .boost(boost) - .queryName(queryName); + .queryName(queryName)); } diff --git a/core/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java index ef49472b8cc..007dff92340 100644 --- a/core/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java @@ -36,6 +36,7 @@ import org.elasticsearch.index.query.support.QueryParsers; import java.io.IOException; import java.util.Objects; +import java.util.Optional; /** * Implements the wildcard search query. Supported wildcards are *, which @@ -135,7 +136,7 @@ public class WildcardQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); XContentParser.Token token = parser.nextToken(); @@ -180,10 +181,10 @@ public class WildcardQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); XContentParser.Token token = parser.nextToken(); @@ -136,7 +137,7 @@ public class WrapperQueryBuilder extends AbstractQueryBuilder new ParsingException(qSourceParser.getTokenLocation(), "inner query cannot be empty")); if (boost() != DEFAULT_BOOST || queryName() != null) { final BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder(); boolQueryBuilder.must(queryBuilder); diff --git a/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java index 98f878d1c2e..3e52322d22f 100644 --- a/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java @@ -37,12 +37,13 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentLocation; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.AbstractQueryBuilder; +import org.elasticsearch.index.query.InnerHitBuilder; import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.index.query.QueryShardContext; -import org.elasticsearch.index.query.InnerHitBuilder; import java.io.IOException; import java.util.ArrayList; @@ -51,6 +52,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Optional; /** * A query that uses a filters with a script associated with them to compute the @@ -431,15 +433,13 @@ public class FunctionScoreQueryBuilder extends AbstractQueryBuilder innerHits) { InnerHitBuilder.extractInnerHits(query(), innerHits); } - public static FunctionScoreQueryBuilder fromXContent(ParseFieldRegistry> scoreFunctionsRegistry, - QueryParseContext parseContext) throws IOException { + public static Optional fromXContent(ParseFieldRegistry> scoreFunctionsRegistry, + QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); QueryBuilder query = null; @@ -468,7 +468,7 @@ public class FunctionScoreQueryBuilder extends AbstractQueryBuilder new ParsingException(context.parser().getTokenLocation(), + "filter cannot be null in filter aggregation [{}]", aggregationName)); return new FilterAggregationBuilder(aggregationName, filter); } - @Override protected int doHashCode() { return Objects.hash(filter); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregationBuilder.java index 5056e3a67bb..9b2b9cee7ff 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregationBuilder.java @@ -26,7 +26,6 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.search.aggregations.AbstractAggregationBuilder; import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; @@ -235,8 +234,8 @@ public class FiltersAggregationBuilder extends AbstractAggregationBuilder(); while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { - QueryBuilder filter = context.parseInnerQueryBuilder(); - nonKeyedFilters.add(filter == null ? QueryBuilders.matchAllQuery() : filter); + QueryBuilder filter = context.parseInnerQueryBuilder().orElse(matchAllQuery()); + nonKeyedFilters.add(filter); } } else { throw new ParsingException(parser.getTokenLocation(), diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregator.java index a5ce89c4666..08bbdaf3e3b 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregator.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filters/FiltersAggregator.java @@ -29,7 +29,6 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.index.query.EmptyQueryBuilder; import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.search.aggregations.Aggregator; @@ -69,11 +68,7 @@ public class FiltersAggregator extends BucketsAggregator { throw new IllegalArgumentException("[filter] must not be null"); } this.key = key; - if (filter instanceof EmptyQueryBuilder) { - this.filter = new MatchAllQueryBuilder(); - } else { - this.filter = filter; - } + this.filter = filter; } /** diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsParser.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsParser.java index 33db8f97335..ba87f0917a0 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsParser.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTermsParser.java @@ -38,6 +38,7 @@ import org.elasticsearch.search.aggregations.support.ValuesSourceType; import java.io.IOException; import java.util.Map; +import java.util.Optional; /** * @@ -91,8 +92,10 @@ public class SignificantTermsParser extends AbstractTermsParser { return true; } else if (parseFieldMatcher.match(currentFieldName, SignificantTermsAggregationBuilder.BACKGROUND_FILTER)) { QueryParseContext queryParseContext = new QueryParseContext(queriesRegistry, parser, parseFieldMatcher); - QueryBuilder filter = queryParseContext.parseInnerQueryBuilder(); - otherOptions.put(SignificantTermsAggregationBuilder.BACKGROUND_FILTER, filter); + Optional filter = queryParseContext.parseInnerQueryBuilder(); + if (filter.isPresent()) { + otherOptions.put(SignificantTermsAggregationBuilder.BACKGROUND_FILTER, filter.get()); + } return true; } } 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 a2c2f3a0fa6..4c47304330a 100644 --- a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java @@ -611,7 +611,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ /** * Add an aggregation to perform as part of the search. */ - public SearchSourceBuilder aggregation(PipelineAggregatorBuilder aggregation) { + public SearchSourceBuilder aggregation(PipelineAggregatorBuilder aggregation) { if (aggregations == null) { aggregations = AggregatorFactories.builder(); } @@ -1003,9 +1003,9 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ } } else if (token == XContentParser.Token.START_OBJECT) { if (context.getParseFieldMatcher().match(currentFieldName, QUERY_FIELD)) { - queryBuilder = context.parseInnerQueryBuilder(); + queryBuilder = context.parseInnerQueryBuilder().orElse(null); } else if (context.getParseFieldMatcher().match(currentFieldName, POST_FILTER_FIELD)) { - postQueryBuilder = context.parseInnerQueryBuilder(); + postQueryBuilder = context.parseInnerQueryBuilder().orElse(null); } else if (context.getParseFieldMatcher().match(currentFieldName, _SOURCE_FIELD)) { fetchSourceContext = FetchSourceContext.parse(context); } else if (context.getParseFieldMatcher().match(currentFieldName, SCRIPT_FIELDS_FIELD)) { diff --git a/core/src/main/java/org/elasticsearch/search/highlight/AbstractHighlighterBuilder.java b/core/src/main/java/org/elasticsearch/search/highlight/AbstractHighlighterBuilder.java index 557567fe354..816be79a9e5 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/AbstractHighlighterBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/AbstractHighlighterBuilder.java @@ -539,7 +539,7 @@ public abstract class AbstractHighlighterBuilder { try { - return c.parseInnerQueryBuilder(); + return c.parseInnerQueryBuilder().orElse(null); } catch (IOException e) { throw new RuntimeException("Error parsing query", e); } diff --git a/core/src/main/java/org/elasticsearch/search/rescore/QueryRescorerBuilder.java b/core/src/main/java/org/elasticsearch/search/rescore/QueryRescorerBuilder.java index e91468c805f..6f59b446dc2 100644 --- a/core/src/main/java/org/elasticsearch/search/rescore/QueryRescorerBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/rescore/QueryRescorerBuilder.java @@ -26,6 +26,7 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.search.rescore.QueryRescorer.QueryRescoreContext; @@ -56,7 +57,7 @@ public class QueryRescorerBuilder extends RescoreBuilder { static { QUERY_RESCORE_PARSER.declareObject(InnerBuilder::setQueryBuilder, (p, c) -> { try { - return c.parseInnerQueryBuilder(); + return c.parseInnerQueryBuilder().orElse(QueryBuilders.matchAllQuery()); } catch (IOException e) { throw new ParsingException(p.getTokenLocation(), "Could not parse inner query", e); } diff --git a/core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java index 892673f890a..d519f740870 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -39,6 +39,7 @@ import org.elasticsearch.search.MultiValueMode; import java.io.IOException; import java.util.Objects; +import java.util.Optional; /** * A sort builder to sort based on a document field. @@ -328,7 +329,7 @@ public class FieldSortBuilder extends SortBuilder { public static FieldSortBuilder fromXContent(QueryParseContext context, String fieldName) throws IOException { XContentParser parser = context.parser(); - QueryBuilder nestedFilter = null; + Optional nestedFilter = Optional.empty(); String nestedPath = null; Object missing = null; SortOrder order = null; @@ -371,9 +372,7 @@ public class FieldSortBuilder extends SortBuilder { } FieldSortBuilder builder = new FieldSortBuilder(fieldName); - if (nestedFilter != null) { - builder.setNestedFilter(nestedFilter); - } + nestedFilter.ifPresent(builder::setNestedFilter); if (nestedPath != null) { builder.setNestedPath(nestedPath); } diff --git a/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java index dce9a7ec3fe..f33dd0e2b15 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java @@ -60,6 +60,7 @@ import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.Objects; +import java.util.Optional; /** * A geo distance based sorting on a geo point like field. @@ -257,7 +258,7 @@ public class GeoDistanceSortBuilder extends SortBuilder } /** - * Sets validation method for this sort builder. + * Sets validation method for this sort builder. */ public GeoDistanceSortBuilder validation(GeoValidationMethod method) { this.validation = method; @@ -265,7 +266,7 @@ public class GeoDistanceSortBuilder extends SortBuilder } /** - * Returns the validation method to use for this sort builder. + * Returns the validation method to use for this sort builder. */ public GeoValidationMethod validation() { return validation; @@ -407,7 +408,7 @@ public class GeoDistanceSortBuilder extends SortBuilder GeoDistance geoDistance = GeoDistance.DEFAULT; SortOrder order = SortOrder.ASC; SortMode sortMode = null; - QueryBuilder nestedFilter = null; + Optional nestedFilter = Optional.empty(); String nestedPath = null; boolean coerce = GeoValidationMethod.DEFAULT_LENIENT_PARSING; @@ -492,7 +493,7 @@ public class GeoDistanceSortBuilder extends SortBuilder if (sortMode != null) { result.sortMode(sortMode); } - result.setNestedFilter(nestedFilter); + nestedFilter.ifPresent(result::setNestedFilter); result.setNestedPath(nestedPath); if (validation == null) { // looks like either validation was left unset or we are parsing old validation json @@ -517,7 +518,7 @@ public class GeoDistanceSortBuilder extends SortBuilder for (GeoPoint point : localPoints) { if (GeoUtils.isValidLatitude(point.lat()) == false) { throw new ElasticsearchParseException( - "illegal latitude value [{}] for [GeoDistanceSort] for field [{}].", + "illegal latitude value [{}] for [GeoDistanceSort] for field [{}].", point.lat(), fieldName); } diff --git a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java index eeb418c0a97..f7baee509de 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java @@ -61,6 +61,7 @@ import java.util.HashMap; import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Optional; /** * Script sort builder allows to sort based on a custom script expression. @@ -237,7 +238,7 @@ public class ScriptSortBuilder extends SortBuilder { ScriptSortType type = null; SortMode sortMode = null; SortOrder order = null; - QueryBuilder nestedFilter = null; + Optional nestedFilter = Optional.empty(); String nestedPath = null; Map params = new HashMap<>(); @@ -292,9 +293,7 @@ public class ScriptSortBuilder extends SortBuilder { if (sortMode != null) { result.sortMode(sortMode); } - if (nestedFilter != null) { - result.setNestedFilter(nestedFilter); - } + nestedFilter.ifPresent(result::setNestedFilter); if (nestedPath != null) { result.setNestedPath(nestedPath); } diff --git a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java index 9aed90f55e5..25f589794f3 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggester.java @@ -34,7 +34,9 @@ import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.text.Text; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.index.query.MatchNoneQueryBuilder; import org.elasticsearch.index.query.ParsedQuery; +import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.script.CompiledScript; @@ -53,6 +55,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Optional; public final class PhraseSuggester extends Suggester { private final BytesRef SEPARATOR = new BytesRef(" "); @@ -123,7 +126,8 @@ public final class PhraseSuggester extends Suggester { final ExecutableScript executable = scriptService.executable(collateScript, vars); final BytesReference querySource = (BytesReference) executable.run(); try (XContentParser parser = XContentFactory.xContent(querySource).createParser(querySource)) { - final ParsedQuery parsedQuery = shardContext.toQuery(shardContext.newParseContext(parser).parseInnerQueryBuilder()); + Optional innerQueryBuilder = shardContext.newParseContext(parser).parseInnerQueryBuilder(); + final ParsedQuery parsedQuery = shardContext.toQuery(innerQueryBuilder.orElse(new MatchNoneQueryBuilder())); collateMatch = Lucene.exists(searcher, parsedQuery.query()); } } diff --git a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java index 15180b9d989..89da227df87 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java @@ -24,6 +24,7 @@ import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; +import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; @@ -43,6 +44,7 @@ import static org.elasticsearch.index.query.QueryBuilders.constantScoreQuery; import static org.elasticsearch.index.query.QueryBuilders.termQuery; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.startsWith; public class BoolQueryBuilderTests extends AbstractQueryTestCase { @Override @@ -345,6 +347,29 @@ public class BoolQueryBuilderTests extends AbstractQueryTestCase parseQuery(query, ParseFieldMatcher.STRICT)); + assertThat(ex.getMessage(), startsWith("query malformed, empty clause found at")); + } + public void testRewrite() throws IOException { BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder(); boolean mustRewrite = false; diff --git a/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTests.java index 34c15d63577..343c6270746 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTests.java @@ -21,12 +21,17 @@ package org.elasticsearch.index.query; import org.apache.lucene.queries.BoostingQuery; import org.apache.lucene.search.Query; +import org.elasticsearch.common.ParseFieldMatcher; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.AbstractQueryTestCase; import java.io.IOException; +import java.util.Optional; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.CoreMatchers.startsWith;; public class BoostingQueryBuilderTests extends AbstractQueryTestCase { @@ -105,6 +110,40 @@ public class BoostingQueryBuilderTests extends AbstractQueryTestCase innerQueryBuilder = context.parseInnerQueryBuilder(); + assertTrue(innerQueryBuilder.isPresent() == false); + + query = + "{ \"boosting\" : {" + + " \"positive\" : { \"match_all\" : {} }, " + + " \"negative\" : { }, " + + " \"negative_boost\" : 23.0" + + " }" + + "}"; + parser = XContentFactory.xContent(query).createParser(query); + context = createParseContext(parser, ParseFieldMatcher.EMPTY); + innerQueryBuilder = context.parseInnerQueryBuilder(); + assertTrue(innerQueryBuilder.isPresent() == false); + + parser = XContentFactory.xContent(query).createParser(query); + QueryParseContext otherContext = createParseContext(parser, ParseFieldMatcher.STRICT); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> otherContext.parseInnerQueryBuilder()); + assertThat(ex.getMessage(), startsWith("query malformed, empty clause found at")); + } + public void testRewrite() throws IOException { QueryBuilder positive = randomBoolean() ? new MatchAllQueryBuilder() : new WrapperQueryBuilder(new TermQueryBuilder("pos", "bar").toString()); QueryBuilder negative = randomBoolean() ? new MatchAllQueryBuilder() : new WrapperQueryBuilder(new TermQueryBuilder("neg", "bar").toString()); diff --git a/core/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java index c24dd25b987..86381c135a5 100644 --- a/core/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java @@ -21,13 +21,18 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.Query; +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.test.AbstractQueryTestCase; import java.io.IOException; +import java.util.Optional; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.Matchers.containsString; public class ConstantScoreQueryBuilderTests extends AbstractQueryTestCase { @@ -126,4 +131,24 @@ public class ConstantScoreQueryBuilderTests extends AbstractQueryTestCase innerQueryBuilder = context.parseInnerQueryBuilder(); + assertTrue(innerQueryBuilder.isPresent() == false); + + parser = XContentFactory.xContent(query).createParser(query); + QueryParseContext otherContext = createParseContext(parser, ParseFieldMatcher.STRICT); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> otherContext.parseInnerQueryBuilder()); + assertThat(ex.getMessage(), startsWith("query malformed, empty clause found at")); + } + } diff --git a/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java index 6d2348660c6..61c786d1b01 100644 --- a/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java @@ -24,6 +24,7 @@ import org.apache.lucene.search.BoostQuery; import org.apache.lucene.search.DisjunctionMaxQuery; import org.apache.lucene.search.PrefixQuery; import org.apache.lucene.search.Query; +import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.test.AbstractQueryTestCase; import java.io.IOException; @@ -96,15 +97,16 @@ public class DisMaxQueryBuilderTests extends AbstractQueryTestCasenull. - * Those should be ignored upstream. To test this, we use inner {@link ConstantScoreQueryBuilder} - * with empty inner filter. + * Test with empty inner query body, this should be ignored upstream. + * To test this, we use inner {@link ConstantScoreQueryBuilder} with empty inner filter. */ - public void testInnerQueryReturnsNull() throws IOException { - String queryString = "{ \"" + ConstantScoreQueryBuilder.NAME + "\" : { \"filter\" : { } } }"; - QueryBuilder innerQueryBuilder = parseQuery(queryString); - DisMaxQueryBuilder disMaxBuilder = new DisMaxQueryBuilder().add(innerQueryBuilder); - assertNull(disMaxBuilder.toQuery(createShardContext())); + public void testInnerQueryEmptyIgnored() throws IOException { + String queryString = "{ \"" + DisMaxQueryBuilder.NAME + "\" :" + + " { \"queries\" : [ {\"" + ConstantScoreQueryBuilder.NAME + "\" : { \"filter\" : { } } } ] " + + " }" + + " }"; + DisMaxQueryBuilder builder = (DisMaxQueryBuilder) parseQuery(queryString, ParseFieldMatcher.EMPTY); + assertTrue("the inner query has an empty body, so it should be ignored", builder.innerQueries().isEmpty()); } public void testIllegalArguments() { diff --git a/core/src/test/java/org/elasticsearch/index/query/HasChildQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/HasChildQueryBuilderTests.java index 1c6a99ca099..d0a924be0ac 100644 --- a/core/src/test/java/org/elasticsearch/index/query/HasChildQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/HasChildQueryBuilderTests.java @@ -35,9 +35,12 @@ import org.apache.lucene.search.similarities.Similarity; import org.apache.lucene.util.BytesRef; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; +import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.Uid; import org.elasticsearch.index.mapper.internal.TypeFieldMapper; @@ -56,11 +59,13 @@ import java.io.IOException; import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.Matchers.is; public class HasChildQueryBuilderTests extends AbstractQueryTestCase { @@ -116,16 +121,11 @@ public class HasChildQueryBuilderTests extends AbstractQueryTestCase innerQueryBuilder = context.parseInnerQueryBuilder(); + assertTrue(innerQueryBuilder.isPresent() == false); + + parser = XContentFactory.xContent(query).createParser(query); + QueryParseContext otherContext = createParseContext(parser, ParseFieldMatcher.STRICT); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> otherContext.parseInnerQueryBuilder()); + assertThat(ex.getMessage(), startsWith("query malformed, empty clause found at")); + } + public void testToQueryInnerQueryType() throws IOException { String[] searchTypes = new String[]{PARENT_TYPE}; QueryShardContext shardContext = createShardContext(); diff --git a/core/src/test/java/org/elasticsearch/index/query/HasParentQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/HasParentQueryBuilderTests.java index 7b700cbfc7f..c4ba395db51 100644 --- a/core/src/test/java/org/elasticsearch/index/query/HasParentQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/HasParentQueryBuilderTests.java @@ -32,6 +32,7 @@ import org.elasticsearch.common.compress.CompressedXContent; 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.index.mapper.MapperService; import org.elasticsearch.script.Script.ScriptParseException; import org.elasticsearch.search.fetch.innerhits.InnerHitsContext; @@ -45,11 +46,13 @@ import org.junit.BeforeClass; import java.io.IOException; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.startsWith; public class HasParentQueryBuilderTests extends AbstractQueryTestCase { protected static final String PARENT_TYPE = "parent"; @@ -99,14 +102,10 @@ public class HasParentQueryBuilderTests extends AbstractQueryTestCase innerQueryBuilder = context.parseInnerQueryBuilder(); + assertTrue(innerQueryBuilder.isPresent() == false); + + parser = XContentFactory.xContent(query).createParser(query); + QueryParseContext otherContext = createParseContext(parser, ParseFieldMatcher.STRICT); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> otherContext.parseInnerQueryBuilder()); + assertThat(ex.getMessage(), startsWith("query malformed, empty clause found at")); + } + public void testIgnoreUnmapped() throws IOException { final HasParentQueryBuilder queryBuilder = new HasParentQueryBuilder("unmapped", new MatchAllQueryBuilder(), false); queryBuilder.ignoreUnmapped(true); diff --git a/core/src/test/java/org/elasticsearch/index/query/NestedQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/NestedQueryBuilderTests.java index 04bbadb02c2..d93bcce6dca 100644 --- a/core/src/test/java/org/elasticsearch/index/query/NestedQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/NestedQueryBuilderTests.java @@ -83,13 +83,9 @@ public class NestedQueryBuilderTests extends AbstractQueryTestCase values = copy.values(); assertEquals(Arrays.asList(1, 3, 4), values); } { TermsQueryBuilder builder = new TermsQueryBuilder("foo", new double[]{1, 3, 4}); - TermsQueryBuilder copy = assertSerialization(builder); + TermsQueryBuilder copy = (TermsQueryBuilder) assertSerialization(builder); List values = copy.values(); assertEquals(Arrays.asList(1d, 3d, 4d), values); } { TermsQueryBuilder builder = new TermsQueryBuilder("foo", new float[]{1, 3, 4}); - TermsQueryBuilder copy = assertSerialization(builder); + TermsQueryBuilder copy = (TermsQueryBuilder) assertSerialization(builder); List values = copy.values(); assertEquals(Arrays.asList(1f, 3f, 4f), values); } { TermsQueryBuilder builder = new TermsQueryBuilder("foo", new long[]{1, 3, 4}); - TermsQueryBuilder copy = assertSerialization(builder); + TermsQueryBuilder copy = (TermsQueryBuilder) assertSerialization(builder); List values = copy.values(); assertEquals(Arrays.asList(1L, 3L, 4L), values); } 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 52632edcf47..2597675095a 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 @@ -26,6 +26,7 @@ import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.io.stream.StreamInput; @@ -599,8 +600,30 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase { +public class DummyQueryBuilder extends AbstractQueryBuilder { + private static final String NAME = "dummy"; + static final ParseField QUERY_NAME_FIELD = new ParseField(NAME); - public static final String NAME = "empty_query"; - - /** - * Construct an empty query. This query can *technically* be named and given a boost. - */ - public EmptyQueryBuilder() { + public DummyQueryBuilder() { } - /** - * Read from a stream. - */ - public EmptyQueryBuilder(StreamInput in) throws IOException { + public DummyQueryBuilder(StreamInput in) throws IOException { super(in); } @Override protected void doWriteTo(StreamOutput out) throws IOException { + // only the superclass has state + } + + @Override + protected void doXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(NAME).endObject(); + } + + public static Optional fromXContent(QueryParseContext parseContext) throws IOException { + XContentParser.Token token = parseContext.parser().nextToken(); + assert token == XContentParser.Token.END_OBJECT; + return Optional.of(new DummyQueryBuilder()); + } + + @Override + protected Query doToQuery(QueryShardContext context) throws IOException { + return new DummyQuery(context.isFilter()); + } + + @Override + protected int doHashCode() { + return 0; + } + + @Override + protected boolean doEquals(DummyQueryBuilder other) { + return true; } @Override public String getWriteableName() { return NAME; } - - @Override - protected Query doToQuery(QueryShardContext context) throws IOException { - return null; - } - - @Override - public String getName() { - return getWriteableName(); - } - - @Override - protected void doXContent(XContentBuilder builder, Params params) throws IOException { - } - - @Override - protected int doHashCode() { - return 31; - } - - @Override - protected boolean doEquals(EmptyQueryBuilder other) { - return true; - } -} +} \ No newline at end of file diff --git a/core/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java b/core/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java index 54d1026a7f0..fbb744f799c 100644 --- a/core/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java +++ b/core/src/test/java/org/elasticsearch/index/query/plugin/DummyQueryParserPlugin.java @@ -23,14 +23,6 @@ import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.Weight; -import org.elasticsearch.common.ParseField; -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.XContentParser; -import org.elasticsearch.index.query.AbstractQueryBuilder; -import org.elasticsearch.index.query.QueryParseContext; -import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.search.SearchModule; @@ -52,62 +44,11 @@ public class DummyQueryParserPlugin extends Plugin { module.registerQuery(DummyQueryBuilder::new, DummyQueryBuilder::fromXContent, DummyQueryBuilder.QUERY_NAME_FIELD); } - public static class DummyQueryBuilder extends AbstractQueryBuilder { - private static final String NAME = "dummy"; - private static final ParseField QUERY_NAME_FIELD = new ParseField(NAME); - - public DummyQueryBuilder() { - } - - /** - * Read from a stream. - */ - public DummyQueryBuilder(StreamInput in) throws IOException { - super(in); - } - - @Override - protected void doWriteTo(StreamOutput out) throws IOException { - // only the superclass has state - } - - @Override - protected void doXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(NAME).endObject(); - } - - public static DummyQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { - XContentParser.Token token = parseContext.parser().nextToken(); - assert token == XContentParser.Token.END_OBJECT; - return new DummyQueryBuilder(); - } - - @Override - protected Query doToQuery(QueryShardContext context) throws IOException { - return new DummyQuery(context.isFilter()); - } - - @Override - protected int doHashCode() { - return 0; - } - - @Override - protected boolean doEquals(DummyQueryBuilder other) { - return true; - } - - @Override - public String getWriteableName() { - return NAME; - } - } - public static class DummyQuery extends Query { public final boolean isFilter; private final Query matchAllDocsQuery = new MatchAllDocsQuery(); - private DummyQuery(boolean isFilter) { + public DummyQuery(boolean isFilter) { this.isFilter = isFilter; } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterIT.java index 9859936b410..ddb62d57ffe 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterIT.java @@ -21,11 +21,17 @@ package org.elasticsearch.search.aggregations.bucket; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.BoolQueryBuilder; -import org.elasticsearch.index.query.EmptyQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryParseContext; +import org.elasticsearch.indices.query.IndicesQueriesRegistry; +import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.bucket.filter.Filter; +import org.elasticsearch.search.aggregations.bucket.filter.FilterAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.metrics.avg.Avg; import org.elasticsearch.test.ESIntegTestCase; @@ -42,8 +48,8 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.filter; import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.is; import static org.hamcrest.core.IsNull.notNullValue; /** @@ -119,9 +125,16 @@ public class FilterIT extends ESIntegTestCase { assertThat(filter.getDocCount(), equalTo((long) numDocs)); } + /** + * test that "{ "filter" : {} }" is regarded as match_all when not parsing strict + */ public void testEmptyFilter() throws Exception { - QueryBuilder emptyFilter = new EmptyQueryBuilder(); - SearchResponse response = client().prepareSearch("idx").addAggregation(filter("tag1", emptyFilter)).execute().actionGet(); + String emtpyFilterBody = "{ }"; + XContentParser parser = XContentFactory.xContent(emtpyFilterBody).createParser(emtpyFilterBody); + QueryParseContext parseContext = new QueryParseContext(new IndicesQueriesRegistry(), parser, ParseFieldMatcher.EMPTY); + AggregationBuilder filterAgg = FilterAggregationBuilder.parse("tag1", parseContext); + + SearchResponse response = client().prepareSearch("idx").addAggregation(filterAgg).execute().actionGet(); assertSearchResponse(response); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersIT.java index fd4db82f2db..0be0f76de90 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersIT.java @@ -22,11 +22,17 @@ package org.elasticsearch.search.aggregations.bucket; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.BoolQueryBuilder; -import org.elasticsearch.index.query.EmptyQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryParseContext; +import org.elasticsearch.indices.query.IndicesQueriesRegistry; +import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.bucket.filters.Filters; +import org.elasticsearch.search.aggregations.bucket.filters.FiltersAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.filters.FiltersAggregator.KeyedFilter; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.metrics.avg.Avg; @@ -207,8 +213,13 @@ public class FiltersIT extends ESIntegTestCase { } public void testEmptyFilter() throws Exception { - QueryBuilder emptyFilter = new EmptyQueryBuilder(); - SearchResponse response = client().prepareSearch("idx").addAggregation(filters("tag1", emptyFilter)).execute().actionGet(); + String emtpyFilterBody = "{ \"filters\" : [ {} ] }"; + XContentParser parser = XContentFactory.xContent(emtpyFilterBody).createParser(emtpyFilterBody); + parser.nextToken(); + QueryParseContext parseContext = new QueryParseContext(new IndicesQueriesRegistry(), parser, ParseFieldMatcher.EMPTY); + AggregationBuilder filtersAgg = FiltersAggregationBuilder.parse("tag1", parseContext); + + SearchResponse response = client().prepareSearch("idx").addAggregation(filtersAgg).execute().actionGet(); assertSearchResponse(response); @@ -219,8 +230,13 @@ public class FiltersIT extends ESIntegTestCase { } public void testEmptyKeyedFilter() throws Exception { - QueryBuilder emptyFilter = new EmptyQueryBuilder(); - SearchResponse response = client().prepareSearch("idx").addAggregation(filters("tag1", new KeyedFilter("foo", emptyFilter))) + String emtpyFilterBody = "{ \"filters\" : {\"foo\" : {} } }"; + XContentParser parser = XContentFactory.xContent(emtpyFilterBody).createParser(emtpyFilterBody); + parser.nextToken(); + QueryParseContext parseContext = new QueryParseContext(new IndicesQueriesRegistry(), parser, ParseFieldMatcher.EMPTY); + AggregationBuilder filtersAgg = FiltersAggregationBuilder.parse("tag1", parseContext); + + SearchResponse response = client().prepareSearch("idx").addAggregation(filtersAgg) .execute().actionGet(); assertSearchResponse(response); diff --git a/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java b/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java index f8236f66e48..fc3c90b5165 100644 --- a/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java @@ -48,8 +48,6 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.env.Environment; import org.elasticsearch.env.EnvironmentModule; import org.elasticsearch.index.Index; -import org.elasticsearch.test.AbstractQueryTestCase; -import org.elasticsearch.index.query.EmptyQueryBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.indices.IndicesModule; @@ -80,6 +78,7 @@ import org.elasticsearch.search.sort.SortBuilders; import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.search.suggest.SuggestBuilderTests; import org.elasticsearch.search.suggest.Suggesters; +import org.elasticsearch.test.AbstractQueryTestCase; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.IndexSettingsModule; import org.elasticsearch.test.InternalSettingsPlugin; @@ -440,11 +439,17 @@ public class SearchSourceBuilderTests extends ESTestCase { assertParseSearchSource(testSearchSourceBuilder, builder.bytes()); } - private void assertParseSearchSource(SearchSourceBuilder testBuilder, BytesReference searchSourceAsBytes) throws IOException { + private static void assertParseSearchSource(SearchSourceBuilder testBuilder, BytesReference searchSourceAsBytes) throws IOException { + assertParseSearchSource(testBuilder, searchSourceAsBytes, ParseFieldMatcher.STRICT); + } + + private static void assertParseSearchSource(SearchSourceBuilder testBuilder, BytesReference searchSourceAsBytes, ParseFieldMatcher pfm) + throws IOException { XContentParser parser = XContentFactory.xContent(searchSourceAsBytes).createParser(searchSourceAsBytes); - QueryParseContext parseContext = createParseContext(parser); + QueryParseContext parseContext = new QueryParseContext(indicesQueriesRegistry, parser, pfm); if (randomBoolean()) { - parser.nextToken(); // sometimes we move it on the START_OBJECT to test the embedded case + parser.nextToken(); // sometimes we move it on the START_OBJECT to + // test the embedded case } SearchSourceBuilder newBuilder = SearchSourceBuilder.fromXContent(parseContext, aggParsers, suggesters); assertNull(parser.nextToken()); @@ -614,8 +619,13 @@ public class SearchSourceBuilderTests extends ESTestCase { public void testEmptyPostFilter() throws IOException { SearchSourceBuilder builder = new SearchSourceBuilder(); - builder.postFilter(new EmptyQueryBuilder()); String query = "{ \"post_filter\": {} }"; - assertParseSearchSource(builder, new BytesArray(query)); + assertParseSearchSource(builder, new BytesArray(query), ParseFieldMatcher.EMPTY); + } + + public void testEmptyQuery() throws IOException { + SearchSourceBuilder builder = new SearchSourceBuilder(); + String query = "{ \"query\": {} }"; + assertParseSearchSource(builder, new BytesArray(query), ParseFieldMatcher.EMPTY); } } diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/messy/tests/TemplateQueryParserTests.java b/modules/lang-mustache/src/test/java/org/elasticsearch/messy/tests/TemplateQueryParserTests.java index 7097718c6fc..9aa5e1892ef 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/messy/tests/TemplateQueryParserTests.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/messy/tests/TemplateQueryParserTests.java @@ -169,7 +169,7 @@ public class TemplateQueryParserTests extends ESTestCase { QueryShardContext context = contextFactory.get(); templateSourceParser.nextToken(); - Query query = QueryBuilder.rewriteQuery(TemplateQueryBuilder.fromXContent(context.newParseContext(templateSourceParser)), + Query query = QueryBuilder.rewriteQuery(TemplateQueryBuilder.fromXContent(context.newParseContext(templateSourceParser)).get(), context).toQuery(context); assertTrue("Parsing template query failed.", query instanceof MatchAllDocsQuery); } @@ -180,7 +180,9 @@ public class TemplateQueryParserTests extends ESTestCase { XContentParser templateSourceParser = XContentFactory.xContent(templateString).createParser(templateString); QueryShardContext context = contextFactory.get(); - Query query = QueryBuilder.rewriteQuery(TemplateQueryBuilder.fromXContent(context.newParseContext(templateSourceParser)), context).toQuery(context); + Query query = QueryBuilder + .rewriteQuery(TemplateQueryBuilder.fromXContent(context.newParseContext(templateSourceParser)).get(), context) + .toQuery(context); assertTrue("Parsing template query failed.", query instanceof MatchAllDocsQuery); } @@ -197,7 +199,7 @@ public class TemplateQueryParserTests extends ESTestCase { QueryShardContext context = contextFactory.get(); try { - TemplateQueryBuilder.fromXContent(context.newParseContext(templateSourceParser)).rewrite(context); + TemplateQueryBuilder.fromXContent(context.newParseContext(templateSourceParser)).get().rewrite(context); fail("Expected ParsingException"); } catch (ParsingException e) { assertThat(e.getMessage(), containsString("query malformed, no field after start_object")); @@ -212,7 +214,7 @@ public class TemplateQueryParserTests extends ESTestCase { templateSourceParser.nextToken(); - Query query = QueryBuilder.rewriteQuery(TemplateQueryBuilder.fromXContent(context.newParseContext(templateSourceParser)), + Query query = QueryBuilder.rewriteQuery(TemplateQueryBuilder.fromXContent(context.newParseContext(templateSourceParser)).get(), context).toQuery(context); assertTrue("Parsing template query failed.", query instanceof MatchAllDocsQuery); } @@ -224,7 +226,7 @@ public class TemplateQueryParserTests extends ESTestCase { QueryShardContext context = contextFactory.get(); templateSourceParser.nextToken(); try { - TemplateQueryBuilder.fromXContent(context.newParseContext(templateSourceParser)).toQuery(context); + TemplateQueryBuilder.fromXContent(context.newParseContext(templateSourceParser)).get().toQuery(context); fail(); } catch (UnsupportedOperationException ex) { assertEquals("this query must be rewritten first", ex.getMessage()); diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index 627cb09da19..aadcabda006 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -76,6 +76,7 @@ import org.elasticsearch.index.query.QueryShardException; import java.io.IOException; import java.util.List; import java.util.Objects; +import java.util.Optional; import static org.elasticsearch.index.mapper.SourceToParse.source; import static org.elasticsearch.percolator.PercolatorFieldMapper.parseQuery; @@ -235,7 +236,7 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); float boost = AbstractQueryBuilder.DEFAULT_BOOST; @@ -316,7 +317,7 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder docs = doc.docs(); int rootDocIndex = docs.size() - 1; diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java index bf5c4c0ae8b..a19a207eb76 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java @@ -250,7 +250,8 @@ public class PercolatorFieldMapper extends FieldMapper { private static QueryBuilder parseQueryBuilder(QueryParseContext context, XContentLocation location) { try { - return context.parseInnerQueryBuilder(); + return context.parseInnerQueryBuilder() + .orElseThrow(() -> new ParsingException(location, "Failed to parse inner query, was empty")); } catch (IOException e) { throw new ParsingException(location, "Failed to parse", e); } diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/TransportPercolateAction.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/TransportPercolateAction.java index f3ee9230eaa..9e86ae44cdd 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/TransportPercolateAction.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/TransportPercolateAction.java @@ -42,7 +42,6 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.ConstantScoreQueryBuilder; -import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.indices.query.IndicesQueriesRegistry; @@ -202,9 +201,8 @@ public class TransportPercolateAction extends HandledTransportAction> /** * Parses the query provided as string argument and compares it with the expected result provided as argument as a {@link QueryBuilder} */ - protected final void assertParsedQuery(String queryAsString, QueryBuilder expectedQuery) throws IOException { + protected final static void assertParsedQuery(String queryAsString, QueryBuilder expectedQuery) throws IOException { assertParsedQuery(queryAsString, expectedQuery, ParseFieldMatcher.STRICT); } - protected final void assertParsedQuery(String queryAsString, QueryBuilder expectedQuery, ParseFieldMatcher matcher) throws IOException { + protected final static void assertParsedQuery(String queryAsString, QueryBuilder expectedQuery, ParseFieldMatcher matcher) throws IOException { QueryBuilder newQuery = parseQuery(queryAsString, matcher); assertNotSame(newQuery, expectedQuery); assertEquals(expectedQuery, newQuery); @@ -350,38 +350,39 @@ public abstract class AbstractQueryTestCase> /** * Parses the query provided as bytes argument and compares it with the expected result provided as argument as a {@link QueryBuilder} */ - protected final void assertParsedQuery(BytesReference queryAsBytes, QueryBuilder expectedQuery) throws IOException { + protected final static void assertParsedQuery(BytesReference queryAsBytes, QueryBuilder expectedQuery) throws IOException { assertParsedQuery(queryAsBytes, expectedQuery, ParseFieldMatcher.STRICT); } - protected final void assertParsedQuery(BytesReference queryAsBytes, QueryBuilder expectedQuery, ParseFieldMatcher matcher) throws IOException { + protected final static void assertParsedQuery(BytesReference queryAsBytes, QueryBuilder expectedQuery, ParseFieldMatcher matcher) throws IOException { QueryBuilder newQuery = parseQuery(queryAsBytes, matcher); assertNotSame(newQuery, expectedQuery); assertEquals(expectedQuery, newQuery); assertEquals(expectedQuery.hashCode(), newQuery.hashCode()); } - protected final QueryBuilder parseQuery(String queryAsString) throws IOException { + protected final static QueryBuilder parseQuery(String queryAsString) throws IOException { return parseQuery(queryAsString, ParseFieldMatcher.STRICT); } - protected final QueryBuilder parseQuery(String queryAsString, ParseFieldMatcher matcher) throws IOException { + protected final static QueryBuilder parseQuery(String queryAsString, ParseFieldMatcher matcher) throws IOException { XContentParser parser = XContentFactory.xContent(queryAsString).createParser(queryAsString); return parseQuery(parser, matcher); } - protected final QueryBuilder parseQuery(BytesReference queryAsBytes) throws IOException { + protected final static QueryBuilder parseQuery(BytesReference queryAsBytes) throws IOException { return parseQuery(queryAsBytes, ParseFieldMatcher.STRICT); } - protected final QueryBuilder parseQuery(BytesReference queryAsBytes, ParseFieldMatcher matcher) throws IOException { + protected final static QueryBuilder parseQuery(BytesReference queryAsBytes, ParseFieldMatcher matcher) throws IOException { XContentParser parser = XContentFactory.xContent(queryAsBytes).createParser(queryAsBytes); return parseQuery(parser, matcher); } - private QueryBuilder parseQuery(XContentParser parser, ParseFieldMatcher matcher) throws IOException { + private static QueryBuilder parseQuery(XContentParser parser, ParseFieldMatcher matcher) throws IOException { QueryParseContext context = createParseContext(parser, matcher); - QueryBuilder parseInnerQueryBuilder = context.parseInnerQueryBuilder(); + QueryBuilder parseInnerQueryBuilder = context.parseInnerQueryBuilder() + .orElseThrow(() -> new IllegalArgumentException("inner query body cannot be empty")); assertNull(parser.nextToken()); return parseInnerQueryBuilder; } @@ -518,8 +519,7 @@ public abstract class AbstractQueryTestCase> /** * Serialize the given query builder and asserts that both are equal */ - @SuppressWarnings("unchecked") - protected QB assertSerialization(QB testQuery) throws IOException { + protected QueryBuilder assertSerialization(QueryBuilder testQuery) throws IOException { try (BytesStreamOutput output = new BytesStreamOutput()) { output.writeNamedWriteable(testQuery); try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), serviceHolder.namedWriteableRegistry)) { @@ -527,7 +527,7 @@ public abstract class AbstractQueryTestCase> assertEquals(testQuery, deserializedQuery); assertEquals(testQuery.hashCode(), deserializedQuery.hashCode()); assertNotSame(testQuery, deserializedQuery); - return (QB) deserializedQuery; + return deserializedQuery; } } } From cb145aec684ccb029943bcbf5d0fb6a51557e26c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Wed, 25 May 2016 11:18:40 +0200 Subject: [PATCH 2/4] Removing handling of null lucene query where we catch this at parse time --- .../org/elasticsearch/index/IndexService.java | 6 ++---- .../index/query/BoolQueryBuilder.java | 4 +--- .../index/query/BoostingQueryBuilder.java | 6 ------ .../index/query/ConstantScoreQueryBuilder.java | 4 ---- .../index/query/DisMaxQueryBuilder.java | 8 ++------ .../index/query/HasChildQueryBuilder.java | 3 --- .../index/query/HasParentQueryBuilder.java | 3 --- .../index/query/NestedQueryBuilder.java | 3 --- .../bucket/filter/FilterAggregationBuilder.java | 5 ++--- .../index/query/DisMaxQueryBuilderTests.java | 15 ++++----------- 10 files changed, 11 insertions(+), 46 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/IndexService.java b/core/src/main/java/org/elasticsearch/index/IndexService.java index e2d2ea4b8fe..5780dc256a5 100644 --- a/core/src/main/java/org/elasticsearch/index/IndexService.java +++ b/core/src/main/java/org/elasticsearch/index/IndexService.java @@ -50,7 +50,6 @@ import org.elasticsearch.index.engine.EngineFactory; import org.elasticsearch.index.fielddata.IndexFieldDataCache; import org.elasticsearch.index.fielddata.IndexFieldDataService; import org.elasticsearch.index.mapper.MapperService; -import org.elasticsearch.index.query.ParsedQuery; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.shard.IndexEventListener; @@ -604,11 +603,10 @@ public final class IndexService extends AbstractIndexComponent implements IndexC byte[] filterSource = alias.filter().uncompressed(); try (XContentParser parser = XContentFactory.xContent(filterSource).createParser(filterSource)) { Optional innerQueryBuilder = shardContext.newParseContext(parser).parseInnerQueryBuilder(); - ParsedQuery parsedFilter = null; if (innerQueryBuilder.isPresent()) { - parsedFilter = shardContext.toFilter(innerQueryBuilder.get()); + return shardContext.toFilter(innerQueryBuilder.get()).query(); } - return parsedFilter == null ? null : parsedFilter.query(); + return null; } } catch (IOException ex) { throw new AliasFilterParsingException(shardContext.index(), alias.getAlias(), "Invalid alias filter", ex); diff --git a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java index 863bdb25c71..052ddd85663 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java @@ -441,9 +441,7 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { luceneQuery = query.toFilter(context); break; } - if (luceneQuery != null) { - booleanQueryBuilder.add(new BooleanClause(luceneQuery, occurs)); - } + booleanQueryBuilder.add(new BooleanClause(luceneQuery, occurs)); } } diff --git a/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java index 3afa4339b5c..3912bcff26e 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java @@ -208,12 +208,6 @@ public class BoostingQueryBuilder extends AbstractQueryBuilder } } - if (!queriesFound) { - throw new ParsingException(parser.getTokenLocation(), "[dis_max] requires 'queries' field"); + if (!queriesFound || queries.isEmpty()) { + throw new ParsingException(parser.getTokenLocation(), "[dis_max] requires 'queries' field with at least one clause"); } DisMaxQueryBuilder disMaxQuery = new DisMaxQueryBuilder(); @@ -186,10 +186,6 @@ public class DisMaxQueryBuilder extends AbstractQueryBuilder protected Query doToQuery(QueryShardContext context) throws IOException { // return null if there are no queries at all Collection luceneQueries = toQueries(queries, context); - if (luceneQueries.isEmpty()) { - return null; - } - return new DisjunctionMaxQuery(luceneQueries, tieBreaker); } diff --git a/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java index 410fda41aef..9fb44845f22 100644 --- a/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java @@ -330,9 +330,6 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder try { context.nestedScope().nextLevel(nestedObjectMapper); innerQuery = this.query.toQuery(context); - if (innerQuery == null) { - return null; - } } finally { context.nestedScope().previousLevel(); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregationBuilder.java index a973d69da4f..4026f846e6d 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregationBuilder.java @@ -20,10 +20,10 @@ package org.elasticsearch.search.aggregations.bucket.filter; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.search.aggregations.AbstractAggregationBuilder; @@ -84,8 +84,7 @@ public class FilterAggregationBuilder extends AbstractAggregationBuilder new ParsingException(context.parser().getTokenLocation(), - "filter cannot be null in filter aggregation [{}]", aggregationName)); + QueryBuilder filter = context.parseInnerQueryBuilder().orElse(new MatchAllQueryBuilder()); return new FilterAggregationBuilder(aggregationName, filter); } diff --git a/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java index 61c786d1b01..f18db46346c 100644 --- a/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java @@ -25,6 +25,7 @@ import org.apache.lucene.search.DisjunctionMaxQuery; import org.apache.lucene.search.PrefixQuery; import org.apache.lucene.search.Query; import org.elasticsearch.common.ParseFieldMatcher; +import org.elasticsearch.common.ParsingException; import org.elasticsearch.test.AbstractQueryTestCase; import java.io.IOException; @@ -88,25 +89,17 @@ public class DisMaxQueryBuilderTests extends AbstractQueryTestCase parseQuery(queryString, ParseFieldMatcher.EMPTY)); + assertEquals("[dis_max] requires 'queries' field with at least one clause", ex.getMessage()); } public void testIllegalArguments() { From e2b6dbc020838dc36efd7921118f817754774faf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 31 May 2016 16:51:06 +0200 Subject: [PATCH 3/4] Add tests to check that toQuery() doesn't return null --- .../test/AbstractQueryTestCase.java | 72 ++++++++++--------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index 2e0bfe28911..cb73ebc0752 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -19,27 +19,33 @@ package org.elasticsearch.test; -import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator; -import com.fasterxml.jackson.core.JsonParseException; -import com.fasterxml.jackson.core.io.JsonStringEncoder; -import org.apache.lucene.util.IOUtils; -import org.elasticsearch.common.inject.Module; -import org.elasticsearch.common.settings.IndexScopedSettings; -import org.elasticsearch.index.query.AbstractQueryBuilder; -import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.index.query.QueryParseContext; -import org.elasticsearch.index.query.QueryRewriteContext; -import org.elasticsearch.index.query.QueryShardContext; -import org.elasticsearch.node.internal.InternalSettingsPreparer; -import org.elasticsearch.plugins.Plugin; -import org.elasticsearch.plugins.PluginsModule; -import org.elasticsearch.plugins.PluginsService; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.either; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.not; + +import java.io.Closeable; +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.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ExecutionException; import org.apache.lucene.search.BoostQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; import org.apache.lucene.search.spans.SpanBoostQuery; import org.apache.lucene.util.Accountable; +import org.apache.lucene.util.IOUtils; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.Version; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; @@ -62,6 +68,7 @@ import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.inject.AbstractModule; import org.elasticsearch.common.inject.Injector; +import org.elasticsearch.common.inject.Module; import org.elasticsearch.common.inject.ModulesBuilder; import org.elasticsearch.common.inject.multibindings.Multibinder; import org.elasticsearch.common.inject.util.Providers; @@ -69,6 +76,7 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsModule; import org.elasticsearch.common.unit.Fuzziness; @@ -87,6 +95,11 @@ import org.elasticsearch.index.cache.bitset.BitsetFilterCache; import org.elasticsearch.index.fielddata.IndexFieldDataCache; import org.elasticsearch.index.fielddata.IndexFieldDataService; import org.elasticsearch.index.mapper.MapperService; +import org.elasticsearch.index.query.AbstractQueryBuilder; +import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryParseContext; +import org.elasticsearch.index.query.QueryRewriteContext; +import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.support.QueryParsers; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.index.similarity.SimilarityService; @@ -96,6 +109,10 @@ import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.indices.fielddata.cache.IndicesFieldDataCache; import org.elasticsearch.indices.mapper.MapperRegistry; import org.elasticsearch.indices.query.IndicesQueriesRegistry; +import org.elasticsearch.node.internal.InternalSettingsPreparer; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.PluginsModule; +import org.elasticsearch.plugins.PluginsService; import org.elasticsearch.script.MockScriptEngine; import org.elasticsearch.script.Script.ScriptParseException; import org.elasticsearch.script.ScriptContext; @@ -115,26 +132,9 @@ import org.junit.After; import org.junit.AfterClass; import org.junit.Before; -import java.io.Closeable; -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.Collection; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.ExecutionException; - -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.either; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.not; +import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator; +import com.fasterxml.jackson.core.JsonParseException; +import com.fasterxml.jackson.core.io.JsonStringEncoder; public abstract class AbstractQueryTestCase> extends ESTestCase { @@ -399,6 +399,7 @@ public abstract class AbstractQueryTestCase> QB controlQuery = copyQuery(firstQuery); setSearchContext(randomTypes, context); // only set search context for toQuery to be more realistic Query firstLuceneQuery = rewriteQuery(firstQuery, context).toQuery(context); + assertNotNull("toQuery should not return null", firstLuceneQuery); assertLuceneQuery(firstQuery, firstLuceneQuery, context); SearchContext.removeCurrent(); // remove after assertLuceneQuery since the assertLuceneQuery impl might access the context as well assertTrue( @@ -1005,6 +1006,7 @@ public abstract class AbstractQueryTestCase> this.namedWriteableRegistry = injector.getInstance(NamedWriteableRegistry.class); } + @Override public void close() throws IOException { injector.getInstance(ClusterService.class).close(); try { From 9067407cdd10bb6035feaac56632aa5698feb064 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Wed, 1 Jun 2016 19:48:22 +0200 Subject: [PATCH 4/4] Adressing review comments --- .../query/ConstantScoreQueryBuilder.java | 2 +- .../index/query/DisMaxQueryBuilder.java | 7 ++- .../search/builder/SearchSourceBuilder.java | 2 +- .../search/rescore/QueryRescorerBuilder.java | 3 ++ .../index/query/DisMaxQueryBuilderTests.java | 30 ++++++----- .../search/aggregations/bucket/FilterIT.java | 2 +- .../search/aggregations/bucket/FiltersIT.java | 4 +- .../rescore/QueryRescoreBuilderTests.java | 6 +++ .../test/AbstractQueryTestCase.java | 50 +++++++++---------- 9 files changed, 59 insertions(+), 47 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java index 574835c6b7a..8848a5ce558 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java @@ -91,7 +91,7 @@ public class ConstantScoreQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); - Optional query = Optional.empty(); + Optional query = null; boolean queryFound = false; String queryName = null; float boost = AbstractQueryBuilder.DEFAULT_BOOST; diff --git a/core/src/main/java/org/elasticsearch/index/query/DisMaxQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/DisMaxQueryBuilder.java index 3ceca1a07a0..b2d8f8b8d15 100644 --- a/core/src/main/java/org/elasticsearch/index/query/DisMaxQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/DisMaxQueryBuilder.java @@ -25,6 +25,7 @@ import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -168,7 +169,7 @@ public class DisMaxQueryBuilder extends AbstractQueryBuilder } } - if (!queriesFound || queries.isEmpty()) { + if (!queriesFound) { throw new ParsingException(parser.getTokenLocation(), "[dis_max] requires 'queries' field with at least one clause"); } @@ -186,6 +187,10 @@ public class DisMaxQueryBuilder extends AbstractQueryBuilder protected Query doToQuery(QueryShardContext context) throws IOException { // return null if there are no queries at all Collection luceneQueries = toQueries(queries, context); + if (luceneQueries.isEmpty()) { + return Queries.newMatchNoDocsQuery("no clauses for dismax query."); + } + return new DisjunctionMaxQuery(luceneQueries, tieBreaker); } 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 4c47304330a..8c4be225127 100644 --- a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java @@ -611,7 +611,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ /** * Add an aggregation to perform as part of the search. */ - public SearchSourceBuilder aggregation(PipelineAggregatorBuilder aggregation) { + public SearchSourceBuilder aggregation(PipelineAggregatorBuilder aggregation) { if (aggregations == null) { aggregations = AggregatorFactories.builder(); } diff --git a/core/src/main/java/org/elasticsearch/search/rescore/QueryRescorerBuilder.java b/core/src/main/java/org/elasticsearch/search/rescore/QueryRescorerBuilder.java index 6f59b446dc2..5603446914c 100644 --- a/core/src/main/java/org/elasticsearch/search/rescore/QueryRescorerBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/rescore/QueryRescorerBuilder.java @@ -72,6 +72,9 @@ public class QueryRescorerBuilder extends RescoreBuilder { * @param builder the query builder to build the rescore query from */ public QueryRescorerBuilder(QueryBuilder builder) { + if (builder == null) { + throw new IllegalArgumentException("rescore_query cannot be null"); + } this.queryBuilder = builder; } diff --git a/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java index f18db46346c..ebd6446f80a 100644 --- a/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTests.java @@ -25,7 +25,7 @@ import org.apache.lucene.search.DisjunctionMaxQuery; import org.apache.lucene.search.PrefixQuery; import org.apache.lucene.search.Query; import org.elasticsearch.common.ParseFieldMatcher; -import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.lucene.search.MatchNoDocsQuery; import org.elasticsearch.test.AbstractQueryTestCase; import java.io.IOException; @@ -35,7 +35,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.Matchers.closeTo; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; @@ -60,17 +59,13 @@ public class DisMaxQueryBuilderTests extends AbstractQueryTestCase queries = AbstractQueryBuilder.toQueries(queryBuilder.innerQueries(), context); - if (queries.isEmpty()) { - assertThat(query, nullValue()); - } else { - assertThat(query, instanceOf(DisjunctionMaxQuery.class)); - DisjunctionMaxQuery disjunctionMaxQuery = (DisjunctionMaxQuery) query; - assertThat(disjunctionMaxQuery.getTieBreakerMultiplier(), equalTo(queryBuilder.tieBreaker())); - assertThat(disjunctionMaxQuery.getDisjuncts().size(), equalTo(queries.size())); - Iterator queryIterator = queries.iterator(); - for (int i = 0; i < disjunctionMaxQuery.getDisjuncts().size(); i++) { - assertThat(disjunctionMaxQuery.getDisjuncts().get(i), equalTo(queryIterator.next())); - } + assertThat(query, instanceOf(DisjunctionMaxQuery.class)); + DisjunctionMaxQuery disjunctionMaxQuery = (DisjunctionMaxQuery) query; + assertThat(disjunctionMaxQuery.getTieBreakerMultiplier(), equalTo(queryBuilder.tieBreaker())); + assertThat(disjunctionMaxQuery.getDisjuncts().size(), equalTo(queries.size())); + Iterator queryIterator = queries.iterator(); + for (int i = 0; i < disjunctionMaxQuery.getDisjuncts().size(); i++) { + assertThat(disjunctionMaxQuery.getDisjuncts().get(i), equalTo(queryIterator.next())); } } @@ -90,7 +85,7 @@ public class DisMaxQueryBuilderTests extends AbstractQueryTestCase parseQuery(queryString, ParseFieldMatcher.EMPTY)); - assertEquals("[dis_max] requires 'queries' field with at least one clause", ex.getMessage()); + QueryBuilder queryBuilder = parseQuery(queryString, ParseFieldMatcher.EMPTY); + QueryShardContext context = createShardContext(); + Query luceneQuery = queryBuilder.toQuery(context); + assertThat(luceneQuery, instanceOf(MatchNoDocsQuery.class)); + assertThat(((MatchNoDocsQuery) luceneQuery).toString(), equalTo("MatchNoDocsQuery[\"no clauses for dismax query.\"]")); } public void testIllegalArguments() { diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterIT.java index ddb62d57ffe..a48facc4d66 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FilterIT.java @@ -132,7 +132,7 @@ public class FilterIT extends ESIntegTestCase { String emtpyFilterBody = "{ }"; XContentParser parser = XContentFactory.xContent(emtpyFilterBody).createParser(emtpyFilterBody); QueryParseContext parseContext = new QueryParseContext(new IndicesQueriesRegistry(), parser, ParseFieldMatcher.EMPTY); - AggregationBuilder filterAgg = FilterAggregationBuilder.parse("tag1", parseContext); + AggregationBuilder filterAgg = FilterAggregationBuilder.parse("tag1", parseContext); SearchResponse response = client().prepareSearch("idx").addAggregation(filterAgg).execute().actionGet(); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersIT.java index 0be0f76de90..a95df3ff5e6 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/FiltersIT.java @@ -217,7 +217,7 @@ public class FiltersIT extends ESIntegTestCase { XContentParser parser = XContentFactory.xContent(emtpyFilterBody).createParser(emtpyFilterBody); parser.nextToken(); QueryParseContext parseContext = new QueryParseContext(new IndicesQueriesRegistry(), parser, ParseFieldMatcher.EMPTY); - AggregationBuilder filtersAgg = FiltersAggregationBuilder.parse("tag1", parseContext); + AggregationBuilder filtersAgg = FiltersAggregationBuilder.parse("tag1", parseContext); SearchResponse response = client().prepareSearch("idx").addAggregation(filtersAgg).execute().actionGet(); @@ -234,7 +234,7 @@ public class FiltersIT extends ESIntegTestCase { XContentParser parser = XContentFactory.xContent(emtpyFilterBody).createParser(emtpyFilterBody); parser.nextToken(); QueryParseContext parseContext = new QueryParseContext(new IndicesQueriesRegistry(), parser, ParseFieldMatcher.EMPTY); - AggregationBuilder filtersAgg = FiltersAggregationBuilder.parse("tag1", parseContext); + AggregationBuilder filtersAgg = FiltersAggregationBuilder.parse("tag1", parseContext); SearchResponse response = client().prepareSearch("idx").addAggregation(filtersAgg) .execute().actionGet(); diff --git a/core/src/test/java/org/elasticsearch/search/rescore/QueryRescoreBuilderTests.java b/core/src/test/java/org/elasticsearch/search/rescore/QueryRescoreBuilderTests.java index 5cc59046433..f965d3ac5fd 100644 --- a/core/src/test/java/org/elasticsearch/search/rescore/QueryRescoreBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/rescore/QueryRescoreBuilderTests.java @@ -54,6 +54,7 @@ import org.junit.AfterClass; import org.junit.BeforeClass; import java.io.IOException; + import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; @@ -180,6 +181,11 @@ public class QueryRescoreBuilderTests extends ESTestCase { } } + public void testRescoreQueryNull() throws IOException { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new QueryRescorerBuilder((QueryBuilder) null)); + assertEquals("rescore_query cannot be null", e.getMessage()); + } + /** * test parsing exceptions for incorrect rescorer syntax */ diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index cb73ebc0752..b7a0d5a16f6 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -19,26 +19,9 @@ package org.elasticsearch.test; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.either; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.not; - -import java.io.Closeable; -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.Collection; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.ExecutionException; +import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator; +import com.fasterxml.jackson.core.JsonParseException; +import com.fasterxml.jackson.core.io.JsonStringEncoder; import org.apache.lucene.search.BoostQuery; import org.apache.lucene.search.Query; @@ -132,9 +115,26 @@ import org.junit.After; import org.junit.AfterClass; import org.junit.Before; -import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator; -import com.fasterxml.jackson.core.JsonParseException; -import com.fasterxml.jackson.core.io.JsonStringEncoder; +import java.io.Closeable; +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.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ExecutionException; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.either; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.not; public abstract class AbstractQueryTestCase> extends ESTestCase { @@ -418,13 +418,13 @@ public abstract class AbstractQueryTestCase> } setSearchContext(randomTypes, context); Query secondLuceneQuery = rewriteQuery(secondQuery, context).toQuery(context); + assertNotNull("toQuery should not return null", secondLuceneQuery); assertLuceneQuery(secondQuery, secondLuceneQuery, context); SearchContext.removeCurrent(); assertEquals("two equivalent query builders lead to different lucene queries", rewrite(secondLuceneQuery), rewrite(firstLuceneQuery)); - // if the initial lucene query is null, changing its boost won't have any effect, we shouldn't test that - if (firstLuceneQuery != null && supportsBoostAndQueryName()) { + if (supportsBoostAndQueryName()) { secondQuery.boost(firstQuery.boost() + 1f + randomFloat()); setSearchContext(randomTypes, context); Query thirdLuceneQuery = rewriteQuery(secondQuery, context).toQuery(context);