From 1c0b7b3b06c244c8af058ac00d62e25c7f417f36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Fri, 12 Jun 2015 16:46:18 +0200 Subject: [PATCH] Query refactoring: ConstantScoreQueryBuilder and Parser Moving the query building functionality from the parser to the builders new toQuery() method analogous to other recent query refactorings. Relates to #10217 Closes #11629 --- .../query/ConstantScoreQueryBuilder.java | 81 +++++++++++++++--- .../index/query/ConstantScoreQueryParser.java | 21 ++--- .../query/ConstantScoreQueryBuilderTest.java | 82 +++++++++++++++++++ 3 files changed, 159 insertions(+), 25 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTest.java 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 53b2a7bc757..08e55113c65 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java @@ -19,7 +19,10 @@ package org.elasticsearch.index.query; +import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.Query; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; @@ -29,31 +32,31 @@ import java.util.Objects; * A query that wraps a filter and simply returns a constant score equal to the * query boost for every document in the filter. */ -public class ConstantScoreQueryBuilder extends QueryBuilder implements BoostableQueryBuilder { +public class ConstantScoreQueryBuilder extends QueryBuilder implements BoostableQueryBuilder { public static final String NAME = "constant_score"; private final QueryBuilder filterBuilder; - private float boost = -1; + private float boost = 1.0f; - static final ConstantScoreQueryBuilder PROTOTYPE = new ConstantScoreQueryBuilder(); + static final ConstantScoreQueryBuilder PROTOTYPE = new ConstantScoreQueryBuilder(null); /** - * A query that wraps a query and simply returns a constant score equal to the + * A query that wraps another query and simply returns a constant score equal to the * query boost for every document in the query. * * @param filterBuilder The query to wrap in a constant score query */ public ConstantScoreQueryBuilder(QueryBuilder filterBuilder) { - this.filterBuilder = Objects.requireNonNull(filterBuilder); + this.filterBuilder = filterBuilder; } /** - * private constructor only used for serialization + * @return the query that was wrapped in this constant score query */ - private ConstantScoreQueryBuilder() { - this.filterBuilder = null; + public QueryBuilder query() { + return this.filterBuilder; } /** @@ -66,20 +69,74 @@ public class ConstantScoreQueryBuilder extends QueryBuilder implements Boostable return this; } + /** + * @return the boost factor + */ + public float boost() { + return this.boost; + } + @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(NAME); builder.field("filter"); filterBuilder.toXContent(builder, params); - - if (boost != -1) { - builder.field("boost", boost); - } + builder.field("boost", boost); builder.endObject(); } + @Override + public Query toQuery(QueryParseContext parseContext) throws QueryParsingException, IOException { + // current DSL allows empty inner filter clauses, we ignore them + if (filterBuilder == null) { + return null; + } + + Query innerFilter = filterBuilder.toQuery(parseContext); + if (innerFilter == null ) { + // return null so that parent queries (e.g. bool) also ignore this + return null; + } + + Query filter = new ConstantScoreQuery(filterBuilder.toQuery(parseContext)); + filter.setBoost(boost); + return filter; + } + @Override public String queryId() { return NAME; } + + @Override + public int hashCode() { + return Objects.hash(boost, filterBuilder); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + ConstantScoreQueryBuilder other = (ConstantScoreQueryBuilder) obj; + return Objects.equals(boost, other.boost) && + Objects.equals(filterBuilder, other.filterBuilder); + } + + @Override + public ConstantScoreQueryBuilder readFrom(StreamInput in) throws IOException { + QueryBuilder innerFilterBuilder = in.readNamedWriteable(); + ConstantScoreQueryBuilder constantScoreQueryBuilder = new ConstantScoreQueryBuilder(innerFilterBuilder); + constantScoreQueryBuilder.boost = in.readFloat(); + return constantScoreQueryBuilder; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeNamedWriteable(this.filterBuilder); + out.writeFloat(boost); + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryParser.java index 504eb8d5a74..04b4b033178 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryParser.java @@ -19,8 +19,6 @@ package org.elasticsearch.index.query; -import org.apache.lucene.search.ConstantScoreQuery; -import org.apache.lucene.search.Query; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.inject.Inject; @@ -31,7 +29,7 @@ import java.io.IOException; /** * */ -public class ConstantScoreQueryParser extends BaseQueryParserTemp { +public class ConstantScoreQueryParser extends BaseQueryParser { private static final ParseField INNER_QUERY_FIELD = new ParseField("filter", "query"); @@ -45,10 +43,10 @@ public class ConstantScoreQueryParser extends BaseQueryParserTemp { } @Override - public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException { + public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException { XContentParser parser = parseContext.parser(); - Query filter = null; + QueryBuilder query = null; boolean queryFound = false; float boost = 1.0f; @@ -61,7 +59,7 @@ public class ConstantScoreQueryParser extends BaseQueryParserTemp { // skip } else if (token == XContentParser.Token.START_OBJECT) { if (INNER_QUERY_FIELD.match(currentFieldName)) { - filter = parseContext.parseInnerFilter(); + query = parseContext.parseInnerFilterToQueryBuilder(); queryFound = true; } else { throw new QueryParsingException(parseContext, "[constant_score] query does not support [" + currentFieldName + "]"); @@ -78,13 +76,10 @@ public class ConstantScoreQueryParser extends BaseQueryParserTemp { throw new QueryParsingException(parseContext, "[constant_score] requires a 'filter' element"); } - if (filter == null) { - return null; - } - - filter = new ConstantScoreQuery(filter); - filter.setBoost(boost); - return filter; + ConstantScoreQueryBuilder constantScoreBuilder = new ConstantScoreQueryBuilder(query); + constantScoreBuilder.boost(boost); + constantScoreBuilder.validate(); + return constantScoreBuilder; } @Override diff --git a/core/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTest.java new file mode 100644 index 00000000000..8a9094cf33b --- /dev/null +++ b/core/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTest.java @@ -0,0 +1,82 @@ +/* + * 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.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; +import org.junit.Test; + +import java.io.IOException; + +public class ConstantScoreQueryBuilderTest extends BaseQueryTestCase { + + @Override + protected Query createExpectedQuery(ConstantScoreQueryBuilder testBuilder, QueryParseContext context) throws QueryParsingException, IOException { + Query expectedQuery = new ConstantScoreQuery(testBuilder.query().toQuery(context)); + expectedQuery.setBoost(testBuilder.boost()); + return expectedQuery; + } + + /** + * @return a {@link ConstantScoreQueryBuilder} with random boost between 0.1f and 2.0f + */ + @Override + protected ConstantScoreQueryBuilder createTestQueryBuilder() { + ConstantScoreQueryBuilder query = new ConstantScoreQueryBuilder(RandomQueryBuilder.create(random())); + if (randomBoolean()) { + query.boost(2.0f / randomIntBetween(1, 20)); + } + return query; + } + + /** + * test that missing "filter" element causes {@link QueryParsingException} + */ + @Test(expected=QueryParsingException.class) + public void testNoFilterElement() throws IOException { + QueryParseContext context = createContext(); + String queryId = ConstantScoreQueryBuilder.PROTOTYPE.queryId(); + String queryString = "{ \""+queryId+"\" : {}"; + XContentParser parser = XContentFactory.xContent(queryString).createParser(queryString); + context.reset(parser); + assertQueryHeader(parser, queryId); + context.indexQueryParserService().queryParser(queryId).fromXContent(context); + } + + /** + * Test empty "filter" element. + * Current DSL allows inner filter element to be empty, returning a `null` inner filter builder + */ + @Test + public void testEmptyFilterElement() throws IOException { + QueryParseContext context = createContext(); + String queryId = ConstantScoreQueryBuilder.PROTOTYPE.queryId(); + String queryString = "{ \""+queryId+"\" : { \"filter\" : { } }"; + XContentParser parser = XContentFactory.xContent(queryString).createParser(queryString); + context.reset(parser); + assertQueryHeader(parser, queryId); + ConstantScoreQueryBuilder queryBuilder = (ConstantScoreQueryBuilder) context.indexQueryParserService() + .queryParser(queryId).fromXContent(context); + assertNull(queryBuilder.query()); + assertNull(queryBuilder.toQuery(createContext())); + } +}