Skip to content

Commit

Permalink
Merge pull request #11885 from MaineC/feature/filter-query-builder-re…
Browse files Browse the repository at this point in the history
…factoring

Refactors FilteredQueryBuilder.

Relates to #10217
  • Loading branch information
Isabel Drost-Fromm committed Jul 8, 2015
2 parents de277d9 + 0202c99 commit a0cccec
Show file tree
Hide file tree
Showing 4 changed files with 221 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -31,36 +36,103 @@
@Deprecated
public class FilteredQueryBuilder extends AbstractQueryBuilder<FilteredQueryBuilder> {

/** 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();
}
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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;

Expand All @@ -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 + "]");
}
Expand All @@ -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;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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<FilteredQueryBuilder> {

@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());
}

}

0 comments on commit a0cccec

Please sign in to comment.