diff --git a/core/src/main/java/org/elasticsearch/index/query/FilteredQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/FilteredQueryBuilder.java index 1dc5c92997acf..0b4426ae753dd 100644 --- a/core/src/main/java/org/elasticsearch/index/query/FilteredQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/FilteredQueryBuilder.java @@ -19,10 +19,15 @@ package org.elasticsearch.index.query; -import org.elasticsearch.common.Nullable; +import org.apache.lucene.search.ConstantScoreQuery; +import org.apache.lucene.search.Query; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; +import java.util.Objects; /** * A query that applies a filter to the results of another query. @@ -31,36 +36,103 @@ @Deprecated public class FilteredQueryBuilder extends AbstractQueryBuilder { + /** Name of the query in the REST API. */ public static final String NAME = "filtered"; - + /** The query to filter. */ private final QueryBuilder queryBuilder; - + /** The filter to apply to the query. */ private final QueryBuilder filterBuilder; static final FilteredQueryBuilder PROTOTYPE = new FilteredQueryBuilder(null, null); + /** + * Returns a {@link MatchAllQueryBuilder} instance that will be used as + * default queryBuilder if none is supplied by the user. Feel free to + * set queryName and boost on that instance - it's always a new one. + * */ + private static QueryBuilder generateDefaultQuery() { + return new MatchAllQueryBuilder(); + } + + /** + * A query that applies a filter to the results of a match_all query. + * @param filterBuilder The filter to apply on the query (Can be null) + * */ + public FilteredQueryBuilder(QueryBuilder filterBuilder) { + this(generateDefaultQuery(), filterBuilder); + } + /** * A query that applies a filter to the results of another query. * - * @param queryBuilder The query to apply the filter to (Can be null) + * @param queryBuilder The query to apply the filter to * @param filterBuilder The filter to apply on the query (Can be null) */ - public FilteredQueryBuilder(@Nullable QueryBuilder queryBuilder, @Nullable QueryBuilder filterBuilder) { - this.queryBuilder = queryBuilder; - this.filterBuilder = filterBuilder; + public FilteredQueryBuilder(QueryBuilder queryBuilder, QueryBuilder filterBuilder) { + this.queryBuilder = (queryBuilder != null) ? queryBuilder : generateDefaultQuery(); + this.filterBuilder = (filterBuilder != null) ? filterBuilder : EmptyQueryBuilder.PROTOTYPE; + } + + /** Returns the query to apply the filter to. */ + public QueryBuilder query() { + return queryBuilder; + } + + /** Returns the filter to apply to the query results. */ + public QueryBuilder filter() { + return filterBuilder; } @Override - protected void doXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(NAME); - if (queryBuilder != null) { - builder.field("query"); - queryBuilder.toXContent(builder, params); + protected boolean doEquals(FilteredQueryBuilder other) { + return Objects.equals(queryBuilder, other.queryBuilder) && + Objects.equals(filterBuilder, other.filterBuilder); + } + + @Override + public int doHashCode() { + return Objects.hash(queryBuilder, filterBuilder); + } + + @Override + public Query doToQuery(QueryParseContext parseContext) throws QueryParsingException, IOException { + Query query = queryBuilder.toQuery(parseContext); + Query filter = filterBuilder.toQuery(parseContext); + + if (query == null) { + // Most likely this query was generated from the JSON query DSL - it parsed to an EmptyQueryBuilder so we ignore + // the whole filtered query as there is nothing to filter on. See FilteredQueryParser for an example. + return null; } - if (filterBuilder != null) { - builder.field("filter"); - filterBuilder.toXContent(builder, params); + + if (filter == null || Queries.isConstantMatchAllQuery(filter)) { + // no filter, or match all filter + return query; + } else if (Queries.isConstantMatchAllQuery(query)) { + // if its a match_all query, use constant_score + return new ConstantScoreQuery(filter); } + + // use a BooleanQuery + return Queries.filtered(query, filter); + } + + @Override + public QueryValidationException validate() { + QueryValidationException validationException = null; + validationException = validateInnerQuery(queryBuilder, validationException); + validationException = validateInnerQuery(filterBuilder, validationException); + return validationException; + + } + + @Override + protected void doXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(NAME); + builder.field("query"); + queryBuilder.toXContent(builder, params); + builder.field("filter"); + filterBuilder.toXContent(builder, params); printBoostAndQueryName(builder); builder.endObject(); } @@ -69,4 +141,18 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep public String getName() { return NAME; } + + @Override + public FilteredQueryBuilder doReadFrom(StreamInput in) throws IOException { + QueryBuilder query = in.readNamedWriteable(); + QueryBuilder filter = in.readNamedWriteable(); + FilteredQueryBuilder qb = new FilteredQueryBuilder(query, filter); + return qb; + } + + @Override + public void doWriteTo(StreamOutput out) throws IOException { + out.writeNamedWriteable(queryBuilder); + out.writeNamedWriteable(filterBuilder); + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/FilteredQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/FilteredQueryParser.java index d5298a23ab2cf..3653d23cc0965 100644 --- a/core/src/main/java/org/elasticsearch/index/query/FilteredQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/FilteredQueryParser.java @@ -19,20 +19,17 @@ package org.elasticsearch.index.query; -import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.ConstantScoreQuery; -import org.apache.lucene.search.Query; import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; /** - * + * @deprecated Use {@link BoolQueryParser} instead. */ + @Deprecated -public class FilteredQueryParser extends BaseQueryParserTemp { +public class FilteredQueryParser extends BaseQueryParser { @Inject public FilteredQueryParser() { @@ -44,12 +41,11 @@ public String[] names() { } @Override - public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException { + public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException { XContentParser parser = parseContext.parser(); - Query query = Queries.newMatchAllQuery(); - Query filter = null; - boolean filterFound = false; + QueryBuilder query = null; + QueryBuilder filter = null; float boost = AbstractQueryBuilder.DEFAULT_BOOST; String queryName = null; @@ -63,10 +59,9 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars // skip } else if (token == XContentParser.Token.START_OBJECT) { if ("query".equals(currentFieldName)) { - query = parseContext.parseInnerQuery(); + query = parseContext.parseInnerQueryBuilder(); } else if ("filter".equals(currentFieldName)) { - filterFound = true; - filter = parseContext.parseInnerFilter(); + filter = parseContext.parseInnerFilterToQueryBuilder(); } else { throw new QueryParsingException(parseContext, "[filtered] query does not support [" + currentFieldName + "]"); } @@ -83,44 +78,15 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars } } - // parsed internally, but returned null during parsing... - if (query == null) { - return null; - } - - if (filter == null) { - if (!filterFound) { - // we allow for null filter, so it makes compositions on the client side to be simpler - return query; - } else { - // even if the filter is not found, and its null, we should simply ignore it, and go - // by the query - return query; - } - } - if (Queries.isConstantMatchAllQuery(filter)) { - // this is an instance of match all filter, just execute the query - return query; - } - - // if its a match_all query, use constant_score - if (Queries.isConstantMatchAllQuery(query)) { - Query q = new ConstantScoreQuery(filter); - q.setBoost(boost); - return q; - } - - BooleanQuery filteredQuery = Queries.filtered(query, filter); - - filteredQuery.setBoost(boost); - if (queryName != null) { - parseContext.addNamedQuery(queryName, filteredQuery); - } - return filteredQuery; + FilteredQueryBuilder qb = new FilteredQueryBuilder(query, filter); + qb.boost(boost); + qb.queryName(queryName); + return qb; } @Override public FilteredQueryBuilder getBuilderPrototype() { return FilteredQueryBuilder.PROTOTYPE; } + } 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 fdce80c33edbe..77439fa020297 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java @@ -218,8 +218,8 @@ public void testFromXContent() throws IOException { QueryBuilder newQuery = queryParserService.queryParser(testQuery.getName()).fromXContent(context); assertNotSame(newQuery, testQuery); - assertEquals("Queries should be equal", testQuery, newQuery); - assertEquals("Queries should have equal hashcodes", testQuery.hashCode(), newQuery.hashCode()); + assertEquals(testQuery, newQuery); + assertEquals(testQuery.hashCode(), newQuery.hashCode()); } /** diff --git a/core/src/test/java/org/elasticsearch/index/query/FilteredQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/FilteredQueryBuilderTest.java new file mode 100644 index 0000000000000..edff743baeb41 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/index/query/FilteredQueryBuilderTest.java @@ -0,0 +1,105 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.query; + +import org.apache.lucene.search.ConstantScoreQuery; +import org.apache.lucene.search.Query; +import org.elasticsearch.common.lucene.search.Queries; +import org.junit.Test; + +import java.io.IOException; + +@SuppressWarnings("deprecation") +public class FilteredQueryBuilderTest extends BaseQueryTestCase { + + @Override + protected FilteredQueryBuilder doCreateTestQueryBuilder() { + QueryBuilder queryBuilder = RandomQueryBuilder.createQuery(random()); + QueryBuilder filterBuilder = RandomQueryBuilder.createQuery(random()); + + FilteredQueryBuilder query = new FilteredQueryBuilder(queryBuilder, filterBuilder); + return query; + } + + @Override + protected Query doCreateExpectedQuery(FilteredQueryBuilder qb, QueryParseContext context) throws IOException { + Query query = qb.query().toQuery(context); + Query filter = qb.filter().toQuery(context); + + if (query == null) { + return null; + } + + Query result; + if (filter == null || Queries.isConstantMatchAllQuery(filter)) { + result = qb.query().toQuery(context); + } else if (Queries.isConstantMatchAllQuery(query)) { + result = new ConstantScoreQuery(filter); + } else { + result = Queries.filtered(qb.query().toQuery(context), filter); + } + result.setBoost(qb.boost()); + return result; + } + + @Test + public void testValidation() { + QueryBuilder valid = RandomQueryBuilder.createQuery(random()); + QueryBuilder invalid = RandomQueryBuilder.createInvalidQuery(random()); + + // invalid cases + FilteredQueryBuilder qb = new FilteredQueryBuilder(invalid); + QueryValidationException result = qb.validate(); + assertNotNull(result); + assertEquals(1, result.validationErrors().size()); + + qb = new FilteredQueryBuilder(valid, invalid); + result = qb.validate(); + assertNotNull(result); + assertEquals(1, result.validationErrors().size()); + + qb = new FilteredQueryBuilder(invalid, valid); + result = qb.validate(); + assertNotNull(result); + assertEquals(1, result.validationErrors().size()); + + qb = new FilteredQueryBuilder(invalid, invalid); + result = qb.validate(); + assertNotNull(result); + assertEquals(2, result.validationErrors().size()); + + // valid cases + qb = new FilteredQueryBuilder(valid); + assertNull(qb.validate()); + + qb = new FilteredQueryBuilder(null); + assertNull(qb.validate()); + + qb = new FilteredQueryBuilder(null, valid); + assertNull(qb.validate()); + + qb = new FilteredQueryBuilder(valid, null); + assertNull(qb.validate()); + + qb = new FilteredQueryBuilder(valid, valid); + assertNull(qb.validate()); + } + +}