Merge pull request #11629 from cbuescher/feature/query-refactoring-constantscore

Query refactoring: ConstantScoreQueryBuilder and Parser

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<ConstantScoreQueryBuilder> {
+public class ConstantScoreQueryBuilder extends QueryBuilder<ConstantScoreQueryBuilder> implements BoostableQueryBuilder<ConstantScoreQueryBuilder> {
 
     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() {
+    public QueryBuilder query() {
-        this.filterBuilder = null;
+        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);
-
+        builder.field("boost", boost);
-        if (boost != -1) {
-            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);
+    }
 }

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) {
+        ConstantScoreQueryBuilder constantScoreBuilder = new ConstantScoreQueryBuilder(query);
-            return null;
+        constantScoreBuilder.boost(boost);
-        }
+        constantScoreBuilder.validate();
-
+        return constantScoreBuilder;
-        filter = new ConstantScoreQuery(filter);
-        filter.setBoost(boost);
-        return filter;
     }
 
     @Override

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