Query refactoring: SpanFirstQueryBuilder 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

Author: cbuescher

core/src/main/java/org/elasticsearch/index/query/SpanFirstQueryBuilder.java

@@ -19,9 +19,15 @@
 
 package org.elasticsearch.index.query;
 
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.spans.SpanFirstQuery;
+import org.apache.lucene.search.spans.SpanQuery;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 
 import java.io.IOException;
+import java.util.Objects;
 
 public class SpanFirstQueryBuilder extends AbstractQueryBuilder<SpanFirstQueryBuilder> implements SpanQueryBuilder<SpanFirstQueryBuilder>{
 
@@ -31,11 +37,44 @@ public class SpanFirstQueryBuilder extends AbstractQueryBuilder<SpanFirstQueryBu
 
     private final int end;
 
-    static final SpanFirstQueryBuilder SPAN_FIRST_QUERY_BUILDER = new SpanFirstQueryBuilder(null, -1);
+    static final SpanFirstQueryBuilder SPAN_FIRST_QUERY_BUILDER = new SpanFirstQueryBuilder();
 
+    /**
+     * Query that matches spans queries defined in <code>matchBuilder</code>
+     * whose end position is less than or equal to <code>end</code>.
+     * @param matchBuilder inner {@link SpanQueryBuilder}
+     * @param end maximum end position of the match, needs to be positive
+     * @throws IllegalArgumentException for negative <code>end</code> positions
+     */
     public SpanFirstQueryBuilder(SpanQueryBuilder matchBuilder, int end) {
-        this.matchBuilder = matchBuilder;
-        this.end = end;
+        this.matchBuilder = Objects.requireNonNull(matchBuilder);
+        if (end >= 0) {
+            this.end = end;
+        } else {
+            throw new IllegalArgumentException("end parameter needs to be positive");
+        }
+    }
+
+    /**
+     * only used for prototype
+     */
+    private SpanFirstQueryBuilder() {
+        this.matchBuilder = null;
+        this.end = -1;
+    }
+
+    /**
+     * @return the inner {@link SpanQueryBuilder} defined in this query
+     */
+    public SpanQueryBuilder matchBuilder() {
+        return this.matchBuilder;
+    }
+
+    /**
+     * @return maximum end position of the matching inner span query
+     */
+    public int end() {
+        return this.end;
     }
 
     @Override
@@ -48,6 +87,42 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
         builder.endObject();
     }
 
+    @Override
+    protected Query doToQuery(QueryParseContext parseContext) throws IOException {
+        Query innerSpanQuery = matchBuilder.toQuery(parseContext);
+        assert innerSpanQuery instanceof SpanQuery;
+        return new SpanFirstQuery((SpanQuery) innerSpanQuery, end);
+    }
+
+    @Override
+    public QueryValidationException validate() {
+        return validateInnerQuery(matchBuilder, null);
+    }
+
+    @Override
+    protected SpanFirstQueryBuilder doReadFrom(StreamInput in) throws IOException {
+        SpanQueryBuilder matchBuilder = in.readNamedWriteable();
+        int end = in.readInt();
+        return new SpanFirstQueryBuilder(matchBuilder, end);
+    }
+
+    @Override
+    protected void doWriteTo(StreamOutput out) throws IOException {
+        out.writeNamedWriteable(matchBuilder);
+        out.writeInt(end);
+    }
+
+    @Override
+    protected int doHashCode() {
+        return Objects.hash(matchBuilder, end);
+    }
+
+    @Override
+    protected boolean doEquals(SpanFirstQueryBuilder other) {
+        return Objects.equals(matchBuilder, other.matchBuilder) &&
+               Objects.equals(end, other.end);
+    }
+
     @Override
     public String getName() {
         return NAME;

core/src/main/java/org/elasticsearch/index/query/SpanFirstQueryParser.java

@@ -19,9 +19,6 @@
 
 package org.elasticsearch.index.query;
 
-import org.apache.lucene.search.Query;
-import org.apache.lucene.search.spans.SpanFirstQuery;
-import org.apache.lucene.search.spans.SpanQuery;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -31,7 +28,7 @@
 /**
  *
  */
-public class SpanFirstQueryParser extends BaseQueryParserTemp {
+public class SpanFirstQueryParser extends BaseQueryParser {
 
     @Inject
     public SpanFirstQueryParser() {
@@ -43,12 +40,12 @@ public String[] names() {
     }
 
     @Override
-    public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException {
+    public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException {
         XContentParser parser = parseContext.parser();
 
         float boost = AbstractQueryBuilder.DEFAULT_BOOST;
 
-        SpanQuery match = null;
+        SpanQueryBuilder match = null;
         int end = -1;
         String queryName = null;
 
@@ -59,11 +56,11 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
                 currentFieldName = parser.currentName();
             } else if (token == XContentParser.Token.START_OBJECT) {
                 if ("match".equals(currentFieldName)) {
-                    Query query = parseContext.parseInnerQuery();
-                    if (!(query instanceof SpanQuery)) {
+                    QueryBuilder query = parseContext.parseInnerQueryBuilder();
+                    if (!(query instanceof SpanQueryBuilder)) {
                         throw new QueryParsingException(parseContext, "spanFirst [match] must be of type span query");
                     }
-                    match = (SpanQuery) query;
+                    match = (SpanQueryBuilder) query;
                 } else {
                     throw new QueryParsingException(parseContext, "[span_first] query does not support [" + currentFieldName + "]");
                 }
@@ -85,13 +82,9 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
         if (end == -1) {
             throw new QueryParsingException(parseContext, "spanFirst must have [end] set for it");
         }
-
-        SpanFirstQuery query = new SpanFirstQuery(match, end);
-        query.setBoost(boost);
-        if (queryName != null) {
-            parseContext.addNamedQuery(queryName, query);
-        }
-        return query;
+        SpanFirstQueryBuilder queryBuilder = new SpanFirstQueryBuilder(match, end);
+        queryBuilder.boost(boost).queryName(queryName);
+        return queryBuilder;
     }
 
     @Override

core/src/test/java/org/elasticsearch/index/query/SpanFirstQueryBuilderTest.java

@@ -0,0 +1,66 @@
+/*
+ * 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.Query;
+import org.apache.lucene.search.spans.SpanFirstQuery;
+import org.apache.lucene.search.spans.SpanQuery;
+import org.junit.Test;
+
+import java.io.IOException;
+
+public class SpanFirstQueryBuilderTest extends BaseQueryTestCase<SpanFirstQueryBuilder> {
+
+    @Override
+    protected Query doCreateExpectedQuery(SpanFirstQueryBuilder testQueryBuilder, QueryParseContext context) throws IOException {
+        SpanQuery innerQuery = (SpanQuery) testQueryBuilder.matchBuilder().toQuery(context);
+        return new SpanFirstQuery(innerQuery, testQueryBuilder.end());
+    }
+
+    @Override
+    protected SpanFirstQueryBuilder doCreateTestQueryBuilder() {
+        SpanTermQueryBuilder innerQueryBuilder = new SpanTermQueryBuilderTest().createTestQueryBuilder();
+        return new SpanFirstQueryBuilder(innerQueryBuilder, randomIntBetween(0, 1000));
+    }
+
+    @Test
+    public void testValidate() {
+        int totalExpectedErrors = 0;
+        SpanQueryBuilder innerSpanQueryBuilder;
+        if (randomBoolean()) {
+            innerSpanQueryBuilder = new SpanTermQueryBuilder("", "test");
+            totalExpectedErrors++;
+        } else {
+            innerSpanQueryBuilder = new SpanTermQueryBuilder("name", "value");
+        }
+        SpanFirstQueryBuilder queryBuilder = new SpanFirstQueryBuilder(innerSpanQueryBuilder, 10);
+        assertValidate(queryBuilder, totalExpectedErrors);
+    }
+
+    @Test(expected=IllegalArgumentException.class)
+    public void testEndValueNegative() {
+        new SpanFirstQueryBuilder(new SpanTermQueryBuilder("name", "value"), -1);
+    }
+
+    @Test(expected=NullPointerException.class)
+    public void testInnerQueryNull() {
+        new SpanFirstQueryBuilder(null, 1);
+    }
+}