diff --git a/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java index 3e2a6aebff8..82e5a4b7f9f 100644 --- a/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java @@ -233,22 +233,6 @@ public abstract class AbstractQueryBuilder exte return queries; } - /** - * Utility method that converts inner query builders to xContent and - * checks for null values, rendering out empty object in this case. - */ - protected static void doXContentInnerBuilder(XContentBuilder xContentBuilder, String fieldName, - QueryBuilder queryBuilder, Params params) throws IOException { - xContentBuilder.field(fieldName); - if (queryBuilder != null) { - queryBuilder.toXContent(xContentBuilder, params); - } else { - // we output an empty object, QueryParseContext#parseInnerQueryBuilder will parse this back to `null` value - xContentBuilder.startObject(); - xContentBuilder.endObject(); - } - } - protected static QueryValidationException validateInnerQueries(List queryBuilders, QueryValidationException initialValidationException) { QueryValidationException validationException = initialValidationException; for (QueryBuilder queryBuilder : queryBuilders) { diff --git a/core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java index 68a4799f1ac..bdcf3d565f0 100644 --- a/core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java @@ -98,6 +98,10 @@ public class AndQueryBuilder extends AbstractQueryBuilder { query.add(innerQuery, Occur.MUST); } } + if (query.clauses().isEmpty()) { + // no inner lucene query exists, ignore upstream + return null; + } return query; } diff --git a/core/src/main/java/org/elasticsearch/index/query/AndQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/AndQueryParser.java index bad092c1d11..4f5c24dbefd 100644 --- a/core/src/main/java/org/elasticsearch/index/query/AndQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/AndQueryParser.java @@ -54,9 +54,7 @@ public class AndQueryParser extends BaseQueryParser { while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { queriesFound = true; QueryBuilder filter = parseContext.parseInnerFilterToQueryBuilder(); - if (filter != null) { - queries.add(filter); - } + queries.add(filter); } } else { while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { @@ -69,17 +67,13 @@ public class AndQueryParser extends BaseQueryParser { queriesFound = true; while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { QueryBuilder filter = parseContext.parseInnerFilterToQueryBuilder(); - if (filter != null) { - queries.add(filter); - } + queries.add(filter); } } else { queriesFound = true; while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { QueryBuilder filter = parseContext.parseInnerFilterToQueryBuilder(); - if (filter != null) { - queries.add(filter); - } + queries.add(filter); } } } else if (token.isValue()) { 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 1f604c23bbe..32b092dc48d 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java @@ -255,16 +255,16 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { @Override protected Query doToQuery(QueryParseContext parseContext) throws IOException { - if (!hasClauses()) { - return new MatchAllDocsQuery(); - } - BooleanQuery booleanQuery = new BooleanQuery(disableCoord); addBooleanClauses(parseContext, booleanQuery, mustClauses, BooleanClause.Occur.MUST); addBooleanClauses(parseContext, booleanQuery, mustNotClauses, BooleanClause.Occur.MUST_NOT); addBooleanClauses(parseContext, booleanQuery, shouldClauses, BooleanClause.Occur.SHOULD); addBooleanClauses(parseContext, booleanQuery, filterClauses, BooleanClause.Occur.FILTER); + if (booleanQuery.clauses().isEmpty()) { + return new MatchAllDocsQuery(); + } + Queries.applyMinimumShouldMatch(booleanQuery, minimumShouldMatch); return adjustPureNegative ? fixNegativeQueryIfNeeded(booleanQuery) : booleanQuery; } diff --git a/core/src/main/java/org/elasticsearch/index/query/BoolQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/BoolQueryParser.java index 5fb0f3c9059..ad919de1f0f 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoolQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoolQueryParser.java @@ -71,14 +71,13 @@ public class BoolQueryParser extends BaseQueryParser { switch (currentFieldName) { case "must": query = parseContext.parseInnerQueryBuilder(); - if (query != null) { - mustClauses.add(query); - } + mustClauses.add(query); break; case "should": query = parseContext.parseInnerQueryBuilder(); - if (query != null) { - shouldClauses.add(query); + shouldClauses.add(query); + // EmptyQueryBuilder does not add lucene query later, skip setting minuminShouldMatch + if (query != EmptyQueryBuilder.PROTOTYPE) { if (parseContext.isFilter() && minimumShouldMatch == null) { minimumShouldMatch = "1"; } @@ -86,16 +85,12 @@ public class BoolQueryParser extends BaseQueryParser { break; case "filter": query = parseContext.parseInnerFilterToQueryBuilder(); - if (query != null) { - filterClauses.add(query); - } + filterClauses.add(query); break; case "must_not": case "mustNot": query = parseContext.parseInnerFilterToQueryBuilder(); - if (query != null) { - mustNotClauses.add(query); - } + mustNotClauses.add(query); break; default: throw new QueryParsingException(parseContext, "[bool] query does not support [" + currentFieldName + "]"); @@ -105,14 +100,13 @@ public class BoolQueryParser extends BaseQueryParser { switch (currentFieldName) { case "must": query = parseContext.parseInnerQueryBuilder(); - if (query != null) { - mustClauses.add(query); - } + mustClauses.add(query); break; case "should": query = parseContext.parseInnerQueryBuilder(); - if (query != null) { - shouldClauses.add(query); + shouldClauses.add(query); + // EmptyQueryBuilder does not add lucene query later, skip setting minuminShouldMatch + if (query != EmptyQueryBuilder.PROTOTYPE) { if (parseContext.isFilter() && minimumShouldMatch == null) { minimumShouldMatch = "1"; } @@ -120,16 +114,12 @@ public class BoolQueryParser extends BaseQueryParser { break; case "filter": query = parseContext.parseInnerFilterToQueryBuilder(); - if (query != null) { - filterClauses.add(query); - } + filterClauses.add(query); break; case "must_not": case "mustNot": query = parseContext.parseInnerFilterToQueryBuilder(); - if (query != null) { - mustNotClauses.add(query); - } + mustNotClauses.add(query); break; default: throw new QueryParsingException(parseContext, "bool query does not support [" + currentFieldName + "]"); 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 91d79f590e2..c2f5d1f537d 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java @@ -44,23 +44,31 @@ public class BoostingQueryBuilder extends AbstractQueryBuilder { + + public static final String NAME = "empty_query"; + + /** the one and only empty query builder */ + public static final EmptyQueryBuilder PROTOTYPE = new EmptyQueryBuilder(); + + private EmptyQueryBuilder() { + // prevent other constructors + } + + @Override + public String getName() { + return NAME; + } + + @Override + protected void doXContent(XContentBuilder builder, Params params) throws IOException { + // empty + } + + /** + * @returns null to signal to caller that this query should be ignored + * in the context of the DSL. + */ + @Override + public Query doToQuery(QueryParseContext parseContext) throws QueryParsingException, IOException { + return null; + } + + @Override + public QueryValidationException validate() { + // nothing to validate + return null; + } + + @Override + public EmptyQueryBuilder doReadFrom(StreamInput in) throws IOException { + return EmptyQueryBuilder.PROTOTYPE; + } + + @Override + public void doWriteTo(StreamOutput out) throws IOException { + // nothing to serialize + } +} diff --git a/core/src/main/java/org/elasticsearch/index/query/FQueryFilterBuilder.java b/core/src/main/java/org/elasticsearch/index/query/FQueryFilterBuilder.java index 64fffe75447..08b7ced73af 100644 --- a/core/src/main/java/org/elasticsearch/index/query/FQueryFilterBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/FQueryFilterBuilder.java @@ -39,7 +39,7 @@ public class FQueryFilterBuilder extends AbstractQueryBuilder { private final QueryBuilder filter; - static final NotQueryBuilder PROTOTYPE = new NotQueryBuilder(null); + static final NotQueryBuilder PROTOTYPE = new NotQueryBuilder(); public NotQueryBuilder(QueryBuilder filter) { - this.filter = filter; + this.filter = Objects.requireNonNull(filter); + } + + private NotQueryBuilder() { + this.filter = null; } /** @@ -53,16 +57,14 @@ public class NotQueryBuilder extends AbstractQueryBuilder { @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(NAME); - doXContentInnerBuilder(builder, "query", filter, params); + builder.field("query"); + filter.toXContent(builder, params); printBoostAndQueryName(builder); builder.endObject(); } @Override protected Query doToQuery(QueryParseContext parseContext) throws IOException { - if (filter == null) { - return null; - } Query luceneQuery = filter.toQuery(parseContext); if (luceneQuery == null) { return null; diff --git a/core/src/main/java/org/elasticsearch/index/query/OrQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/OrQueryBuilder.java index 049821a5eca..3838d9a5a20 100644 --- a/core/src/main/java/org/elasticsearch/index/query/OrQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/OrQueryBuilder.java @@ -95,6 +95,10 @@ public class OrQueryBuilder extends AbstractQueryBuilder { query.add(innerQuery, Occur.SHOULD); } } + if (query.clauses().isEmpty()) { + // no inner lucene query exists, ignore upstream + return null; + } return query; } diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryBuilders.java b/core/src/main/java/org/elasticsearch/index/query/QueryBuilders.java index 90b2efca348..55789b8df4d 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryBuilders.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryBuilders.java @@ -277,8 +277,8 @@ public abstract class QueryBuilders { * Unlike the "NOT" clause, this still selects documents that contain undesirable terms, * but reduces their overall score: */ - public static BoostingQueryBuilder boostingQuery() { - return new BoostingQueryBuilder(); + public static BoostingQueryBuilder boostingQuery(QueryBuilder positiveQuery, QueryBuilder negativeQuery) { + return new BoostingQueryBuilder(positiveQuery, negativeQuery); } /** diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java b/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java index 7fe48b32531..a7cd8d1c216 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java @@ -40,7 +40,7 @@ public class QueryFilterBuilder extends AbstractQueryBuilder private final QueryBuilder queryBuilder; - static final QueryFilterBuilder PROTOTYPE = new QueryFilterBuilder(null); + static final QueryFilterBuilder PROTOTYPE = new QueryFilterBuilder(); /** * A filter that simply wraps a query. @@ -48,7 +48,11 @@ public class QueryFilterBuilder extends AbstractQueryBuilder * @param queryBuilder The query to wrap as a filter */ public QueryFilterBuilder(QueryBuilder queryBuilder) { - this.queryBuilder = queryBuilder; + this.queryBuilder = Objects.requireNonNull(queryBuilder); + } + + private QueryFilterBuilder() { + this.queryBuilder = null; } /** @@ -72,15 +76,13 @@ public class QueryFilterBuilder extends AbstractQueryBuilder @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { - doXContentInnerBuilder(builder, NAME, queryBuilder, params); + builder.field(NAME); + queryBuilder.toXContent(builder, params); } @Override protected Query doToQuery(QueryParseContext parseContext) throws IOException { // inner query builder can potentially be `null`, in that case we ignore it - if (this.queryBuilder == null) { - return null; - } Query innerQuery = this.queryBuilder.toQuery(parseContext); if (innerQuery == null) { return null; 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 22e63571a68..569e5679405 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java @@ -232,7 +232,7 @@ public class QueryParseContext { token = parser.nextToken(); if (token == XContentParser.Token.END_OBJECT) { // empty query - return null; + return EmptyQueryBuilder.PROTOTYPE; } if (token != XContentParser.Token.FIELD_NAME) { throw new QueryParsingException(this, "[_na] query malformed, no field after start_object"); diff --git a/core/src/main/java/org/elasticsearch/indices/query/IndicesQueriesRegistry.java b/core/src/main/java/org/elasticsearch/indices/query/IndicesQueriesRegistry.java index 3c099f5b35f..682daec4afa 100644 --- a/core/src/main/java/org/elasticsearch/indices/query/IndicesQueriesRegistry.java +++ b/core/src/main/java/org/elasticsearch/indices/query/IndicesQueriesRegistry.java @@ -26,6 +26,7 @@ import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.query.EmptyQueryBuilder; import org.elasticsearch.index.query.QueryParser; import java.util.Map; @@ -45,6 +46,9 @@ public class IndicesQueriesRegistry extends AbstractComponent { } namedWriteableRegistry.registerPrototype(queryParser.getBuilderPrototype()); } + // EmptyQueryBuilder is not registered as query parser but used internally. + // We need to register it with the NamedWriteableRegistry in order to serialize it + namedWriteableRegistry.registerPrototype(EmptyQueryBuilder.PROTOTYPE); this.queryParsers = ImmutableMap.copyOf(queryParsers); } diff --git a/core/src/test/java/org/elasticsearch/index/query/AndQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/AndQueryBuilderTest.java index 5ac0d69b270..e134ef87db4 100644 --- a/core/src/test/java/org/elasticsearch/index/query/AndQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/AndQueryBuilderTest.java @@ -43,6 +43,9 @@ public class AndQueryBuilderTest extends BaseQueryTestCase { query.add(innerQuery, Occur.MUST); } } + if (query.clauses().isEmpty()) { + return null; + } return query; } @@ -78,6 +81,16 @@ public class AndQueryBuilderTest extends BaseQueryTestCase { context.indexQueryParserService().queryParser(AndQueryBuilder.PROTOTYPE.getName()).fromXContent(context); } + @Test(expected=NullPointerException.class) + public void testNullConstructor() { + new AndQueryBuilder(EmptyQueryBuilder.PROTOTYPE, null); + } + + @Test(expected=NullPointerException.class) + public void testAddNull() { + new AndQueryBuilder(EmptyQueryBuilder.PROTOTYPE).add(null); + } + @Test public void testValidate() { AndQueryBuilder andQuery = new AndQueryBuilder(); diff --git a/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java b/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java index b7647ea2f58..fdce80c33ed 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java @@ -234,14 +234,22 @@ public abstract class BaseQueryTestCase> ext Query expectedQuery = createExpectedQuery(testQuery, context); Query actualQuery = testQuery.toQuery(context); - assertThat(actualQuery, instanceOf(expectedQuery.getClass())); - assertThat(actualQuery, equalTo(expectedQuery)); - assertLuceneQuery(testQuery, actualQuery, context); + // expectedQuery can be null, e.g. in case of BoostingQueryBuilder + // with inner clause that returns null itself + if (expectedQuery == null) { + assertNull("Expected a null query, saw some object.", actualQuery); + } else { + assertThat(actualQuery, instanceOf(expectedQuery.getClass())); + assertThat(actualQuery, equalTo(expectedQuery)); + assertLuceneQuery(testQuery, actualQuery, context); + } } protected final Query createExpectedQuery(QB queryBuilder, QueryParseContext context) throws IOException { Query expectedQuery = doCreateExpectedQuery(queryBuilder, context); - expectedQuery.setBoost(queryBuilder.boost()); + if (expectedQuery != null) { + expectedQuery.setBoost(queryBuilder.boost()); + } return expectedQuery; } diff --git a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTest.java index bc379157736..bfa92e02140 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTest.java @@ -72,12 +72,14 @@ public class BoolQueryBuilderTest extends BaseQueryTestCase { } BooleanQuery boolQuery = new BooleanQuery(queryBuilder.disableCoord()); - boolQuery.setBoost(queryBuilder.boost()); addBooleanClauses(context, boolQuery, queryBuilder.must(), BooleanClause.Occur.MUST); addBooleanClauses(context, boolQuery, queryBuilder.mustNot(), BooleanClause.Occur.MUST_NOT); addBooleanClauses(context, boolQuery, queryBuilder.should(), BooleanClause.Occur.SHOULD); addBooleanClauses(context, boolQuery, queryBuilder.filter(), BooleanClause.Occur.FILTER); + if (boolQuery.clauses().isEmpty()) { + return new MatchAllDocsQuery(); + } Queries.applyMinimumShouldMatch(boolQuery, queryBuilder.minimumNumberShouldMatch()); return queryBuilder.adjustPureNegative() ? fixNegativeQueryIfNeeded(boolQuery) : boolQuery; } @@ -85,7 +87,10 @@ public class BoolQueryBuilderTest extends BaseQueryTestCase { private static void addBooleanClauses(QueryParseContext parseContext, BooleanQuery booleanQuery, List clauses, Occur occurs) throws IOException { for (QueryBuilder query : clauses) { - booleanQuery.add(new BooleanClause(query.toQuery(parseContext), occurs)); + Query innerQuery = query.toQuery(parseContext); + if (innerQuery != null) { + booleanQuery.add(new BooleanClause(innerQuery, occurs)); + } } } @@ -129,4 +134,24 @@ public class BoolQueryBuilderTest extends BaseQueryTestCase { } assertValidate(booleanQuery, totalExpectedErrors); } + + @Test(expected=NullPointerException.class) + public void testAddNullMust() { + new BoolQueryBuilder().must(null); + } + + @Test(expected=NullPointerException.class) + public void testAddNullMustNot() { + new BoolQueryBuilder().mustNot(null); + } + + @Test(expected=NullPointerException.class) + public void testAddNullShould() { + new BoolQueryBuilder().should(null); + } + + @Test(expected=NullPointerException.class) + public void testAddNullFilter() { + new BoolQueryBuilder().filter(null); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTest.java index 1db0f7305c8..8eee5cc1097 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTest.java @@ -21,9 +21,6 @@ package org.elasticsearch.index.query; import org.apache.lucene.queries.BoostingQuery; import org.apache.lucene.search.Query; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentParser; import org.junit.Test; import java.io.IOException; @@ -32,9 +29,7 @@ public class BoostingQueryBuilderTest extends BaseQueryTestCasenull - * if any of the inner query builder is not set - */ - @Test - public void testInnerQueryBuilderNull() throws IOException { - BoostingQueryBuilder boostingQueryBuilder = new BoostingQueryBuilder(); - if (randomBoolean()) { - boostingQueryBuilder.positive(new MatchAllQueryBuilder()).negative(null); - } else { - boostingQueryBuilder.positive(null).negative(new MatchAllQueryBuilder()); - } - assertNull(boostingQueryBuilder.toQuery(createContext())); - } - - @Test - public void testInnerQueryBuilderReturnsNull() throws IOException { - QueryBuilder noOpBuilder = new AbstractQueryBuilder() { - - @Override - public String getName() { - return "dummy"; - } - - @Override - protected void doXContent(XContentBuilder builder, Params params) throws IOException { - } - - @Override - protected Query doToQuery(QueryParseContext parseContext) throws IOException { - return null; - } - }; - BoostingQueryBuilder boostingQueryBuilder; - if (randomBoolean()) { - boostingQueryBuilder = new BoostingQueryBuilder().positive(new MatchAllQueryBuilder()).negative(noOpBuilder); - } else { - boostingQueryBuilder = new BoostingQueryBuilder().positive(noOpBuilder).negative(new MatchAllQueryBuilder()); - } - assertNull(boostingQueryBuilder.toQuery(createContext())); - } - @Test public void testValidate() { - BoostingQueryBuilder boostingQuery = new BoostingQueryBuilder(); int totalExpectedErrors = 0; - if (randomBoolean()) { - boostingQuery.negative(RandomQueryBuilder.createInvalidQuery(random())); + QueryBuilder positive; + QueryBuilder negative; + if (frequently()) { + negative = RandomQueryBuilder.createInvalidQuery(random()); totalExpectedErrors++; - } else if(rarely()) { - boostingQuery.negative(RandomQueryBuilder.createQuery(random())); + } else { + negative = RandomQueryBuilder.createQuery(random()); } - if (randomBoolean()) { - boostingQuery.positive(RandomQueryBuilder.createInvalidQuery(random())); + if (frequently()) { + positive = RandomQueryBuilder.createInvalidQuery(random()); totalExpectedErrors++; - } else if(rarely()) { - boostingQuery.positive(RandomQueryBuilder.createQuery(random())); + } else { + positive = RandomQueryBuilder.createQuery(random()); } + BoostingQueryBuilder boostingQuery = new BoostingQueryBuilder(positive, negative); if (frequently()) { boostingQuery.negativeBoost(0.5f); } else { @@ -134,4 +70,13 @@ public class BoostingQueryBuilderTest extends BaseQueryTestCase { @Override protected Query doCreateExpectedQuery(DisMaxQueryBuilder testBuilder, QueryParseContext context) throws QueryParsingException, IOException { - return new DisjunctionMaxQuery(AbstractQueryBuilder.toQueries(testBuilder.queries(), context), testBuilder.tieBreaker()); + Collection queries = AbstractQueryBuilder.toQueries(testBuilder.queries(), context); + if (queries.isEmpty()) { + return null; + } + return new DisjunctionMaxQuery(queries, testBuilder.tieBreaker()); } /** @@ -82,6 +87,11 @@ public class DisMaxQueryBuilderTest extends BaseQueryTestCase { @Override protected Query doCreateExpectedQuery(NotQueryBuilder queryBuilder, QueryParseContext context) throws QueryParsingException, IOException { - if (queryBuilder.filter() == null) { + if (queryBuilder.filter().toQuery(context) == null) { return null; } return Queries.not(queryBuilder.filter().toQuery(context)); @@ -45,25 +45,9 @@ public class NotQueryBuilderTest extends BaseQueryTestCase { return new NotQueryBuilder(RandomQueryBuilder.createQuery(random())); } - /** - * test corner case where no inner query exist - */ - @Test - public void testNoInnerQuerry() throws QueryParsingException, IOException { - NotQueryBuilder notQuery = new NotQueryBuilder(null); - assertNull(notQuery.toQuery(createContext())); - } - - /** - * Test corner case where inner queries returns null. - * This is legal, e.g. here {@link ConstantScoreQueryBuilder} can wrap a filter - * element that itself parses to null, e.g. - * '{ "constant_score" : "filter {} }' - */ - @Test - public void testInnerQuery() throws QueryParsingException, IOException { - NotQueryBuilder notQuery = new NotQueryBuilder(new ConstantScoreQueryBuilder(null)); - assertNull(notQuery.toQuery(createContext())); + @Test(expected=NullPointerException.class) + public void testNotQueryBuilderNull() { + new NotQueryBuilder(null); } /** diff --git a/core/src/test/java/org/elasticsearch/index/query/OrQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/OrQueryBuilderTest.java index e9222576d8c..5c15450b0d7 100644 --- a/core/src/test/java/org/elasticsearch/index/query/OrQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/OrQueryBuilderTest.java @@ -44,6 +44,9 @@ public class OrQueryBuilderTest extends BaseQueryTestCase { query.add(innerQuery, Occur.SHOULD); } } + if (query.clauses().isEmpty()) { + return null; + } return query; } @@ -94,4 +97,14 @@ public class OrQueryBuilderTest extends BaseQueryTestCase { } assertValidate(orQuery, totalExpectedErrors); } + + @Test(expected=NullPointerException.class) + public void testNullConstructor() { + new OrQueryBuilder(EmptyQueryBuilder.PROTOTYPE, null); + } + + @Test(expected=NullPointerException.class) + public void testAddNull() { + new OrQueryBuilder().add(null); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTest.java index 696d61c35c0..67b0f9dba87 100644 --- a/core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTest.java @@ -21,8 +21,6 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.Query; -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentParser; import org.junit.Test; import java.io.IOException; @@ -32,11 +30,12 @@ public class QueryFilterBuilderTest extends BaseQueryTestCasenull to pass on upwards - */ - @Test - public void testInnerQueryReturnsNull() throws IOException { - QueryParseContext context = createContext(); - - // create inner filter - String queryString = "{ \"constant_score\" : { \"filter\" : {} }"; - XContentParser parser = XContentFactory.xContent(queryString).createParser(queryString); - context.reset(parser); - assertQueryHeader(parser, ConstantScoreQueryBuilder.PROTOTYPE.getName()); - QueryBuilder innerQuery = context.indexQueryParserService().queryParser(ConstantScoreQueryBuilder.PROTOTYPE.getName()).fromXContent(context); - - // check that when wrapping this filter, toQuery() returns null - QueryFilterBuilder queryFilterQuery = new QueryFilterBuilder(innerQuery); - assertNull(queryFilterQuery.toQuery(createContext())); + @Test(expected=NullPointerException.class) + public void testQueryFilterBuilderNull() { + new QueryFilterBuilder(null); } @Test diff --git a/core/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java b/core/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java index 7d647b93b71..e355d922618 100644 --- a/core/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java +++ b/core/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java @@ -36,13 +36,15 @@ public class RandomQueryBuilder { * @return a random {@link QueryBuilder} */ public static QueryBuilder createQuery(Random r) { - switch (RandomInts.randomIntBetween(r, 0, 2)) { + switch (RandomInts.randomIntBetween(r, 0, 3)) { case 0: return new MatchAllQueryBuilderTest().createTestQueryBuilder(); case 1: return new TermQueryBuilderTest().createTestQueryBuilder(); case 2: return new IdsQueryBuilderTest().createTestQueryBuilder(); + case 3: + return EmptyQueryBuilder.PROTOTYPE; default: throw new UnsupportedOperationException(); } @@ -59,7 +61,7 @@ public class RandomQueryBuilder { case 0: return new TermQueryBuilder("", "test"); case 1: - return new BoostingQueryBuilder().negativeBoost(-1f); + return new BoostingQueryBuilder(new MatchAllQueryBuilder(), new MatchAllQueryBuilder()).negativeBoost(-1f); case 2: return new CommonTermsQueryBuilder("", "text"); case 3: diff --git a/core/src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java b/core/src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java index 78a8adaf9b6..59c2ce3cb19 100644 --- a/core/src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/SimpleIndexQueryParserTests.java @@ -964,7 +964,7 @@ public class SimpleIndexQueryParserTests extends ElasticsearchSingleNodeTest { @Test public void testBoostingQueryBuilder() throws IOException { IndexQueryParserService queryParser = queryParser(); - Query parsedQuery = queryParser.parse(boostingQuery().positive(termQuery("field1", "value1")).negative(termQuery("field1", "value2")).negativeBoost(0.2f)).query(); + Query parsedQuery = queryParser.parse(boostingQuery(termQuery("field1", "value1"), termQuery("field1", "value2")).negativeBoost(0.2f)).query(); assertThat(parsedQuery, instanceOf(BoostingQuery.class)); } diff --git a/core/src/test/java/org/elasticsearch/search/highlight/HighlighterSearchTests.java b/core/src/test/java/org/elasticsearch/search/highlight/HighlighterSearchTests.java index e5ba8a34db2..7f498e6a127 100644 --- a/core/src/test/java/org/elasticsearch/search/highlight/HighlighterSearchTests.java +++ b/core/src/test/java/org/elasticsearch/search/highlight/HighlighterSearchTests.java @@ -50,7 +50,6 @@ import static org.elasticsearch.index.query.QueryBuilders.*; import static org.elasticsearch.search.builder.SearchSourceBuilder.highlight; import static org.elasticsearch.search.builder.SearchSourceBuilder.searchSource; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.*; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; import static org.elasticsearch.test.hamcrest.RegexMatcher.matches; import static org.hamcrest.Matchers.*; @@ -1368,7 +1367,7 @@ public class HighlighterSearchTests extends ElasticsearchIntegrationTest { logger.info("--> highlighting and searching on field1"); SearchSourceBuilder source = searchSource() - .query(boostingQuery().positive(termQuery("field2", "brown")).negative(termQuery("field2", "foobar")).negativeBoost(0.5f)) + .query(boostingQuery(termQuery("field2", "brown"), termQuery("field2", "foobar")).negativeBoost(0.5f)) .highlight(highlight().field("field2").order("score").preTags("").postTags("")); SearchResponse searchResponse = client().prepareSearch("test").setSource(source.buildAsBytes()).get(); @@ -1386,7 +1385,7 @@ public class HighlighterSearchTests extends ElasticsearchIntegrationTest { logger.info("--> highlighting and searching on field1"); SearchSourceBuilder source = searchSource() - .query(boostingQuery().positive(termQuery("field2", "brown")).negative(termQuery("field2", "foobar")).negativeBoost(0.5f)) + .query(boostingQuery(termQuery("field2", "brown"), termQuery("field2", "foobar")).negativeBoost(0.5f)) .highlight(highlight().field("field2").order("score").preTags("").postTags("")); SearchResponse searchResponse = client().prepareSearch("test").setSource(source.buildAsBytes()).get(); @@ -2300,7 +2299,7 @@ public class HighlighterSearchTests extends ElasticsearchIntegrationTest { logger.info("--> highlighting and searching on field1"); SearchSourceBuilder source = searchSource() - .query(boostingQuery().positive(termQuery("field2", "brown")).negative(termQuery("field2", "foobar")).negativeBoost(0.5f)) + .query(boostingQuery(termQuery("field2", "brown"), termQuery("field2", "foobar")).negativeBoost(0.5f)) .highlight(highlight().field("field2").preTags("").postTags("")); SearchResponse searchResponse = client().search(searchRequest("test").source(source)).actionGet(); @@ -2603,10 +2602,10 @@ public class HighlighterSearchTests extends ElasticsearchIntegrationTest { assertHighlight(response, 0, "field1", 0, 1, highlightedMatcher); phrase.boost(1); // Try with a boosting query - response = search.setQuery(boostingQuery().positive(phrase).negative(terms).boost(boost).negativeBoost(1)).get(); + response = search.setQuery(boostingQuery(phrase, terms).boost(boost).negativeBoost(1)).get(); assertHighlight(response, 0, "field1", 0, 1, highlightedMatcher); // Try with a boosting query using a negative boost - response = search.setQuery(boostingQuery().positive(phrase).negative(terms).boost(1).negativeBoost(1/boost)).get(); + response = search.setQuery(boostingQuery(phrase, terms).boost(1).negativeBoost(1/boost)).get(); assertHighlight(response, 0, "field1", 0, 1, highlightedMatcher); } } diff --git a/docs/reference/migration/migrate_query_refactoring.asciidoc b/docs/reference/migration/migrate_query_refactoring.asciidoc index ba425d78fd5..464e43e1982 100644 --- a/docs/reference/migration/migrate_query_refactoring.asciidoc +++ b/docs/reference/migration/migrate_query_refactoring.asciidoc @@ -6,6 +6,11 @@ on the query-refactoring feature branch. === Java-API +==== BoostingQueryBuilder + +Removed setters for mandatory positive/negative query. Both arguments now have +to be supplied at construction time already and have to be non-null. + ==== QueryFilterBuilder Removed the setter `queryName(String queryName)` since this field is not supported