Merge pull request elastic#12156 from cbuescher/feature/query-refacto…

…ring-spannear

Query refactoring: SpanNearQueryBuilder and Parser

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

@@ -312,8 +312,8 @@ public static SpanFirstQueryBuilder spanFirstQuery(SpanQueryBuilder match, int e
         return new SpanFirstQueryBuilder(match, end);
     }
 
-    public static SpanNearQueryBuilder spanNearQuery() {
-        return new SpanNearQueryBuilder();
+    public static SpanNearQueryBuilder spanNearQuery(int slop) {
+        return new SpanNearQueryBuilder(slop);
     }
 
     public static SpanNotQueryBuilder spanNotQuery() {

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

@@ -19,70 +19,180 @@
 
 package org.elasticsearch.index.query;
 
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.spans.SpanNearQuery;
+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.ArrayList;
+import java.util.List;
+import java.util.Objects;
 
+/**
+ * Matches spans which are near one another. One can specify slop, the maximum number
+ * of intervening unmatched positions, as well as whether matches are required to be in-order.
+ * The span near query maps to Lucene {@link SpanNearQuery}.
+ */
 public class SpanNearQueryBuilder extends AbstractQueryBuilder<SpanNearQueryBuilder> implements SpanQueryBuilder<SpanNearQueryBuilder> {
 
     public static final String NAME = "span_near";
 
-    private ArrayList<SpanQueryBuilder> clauses = new ArrayList<>();
+    /** Default for flag controlling whether matches are required to be in-order */
+    public static boolean DEFAULT_IN_ORDER = true;
+
+    /** Default for flag controlling whether payloads are collected */
+    public static boolean DEFAULT_COLLECT_PAYLOADS = true;
+
+    private final ArrayList<SpanQueryBuilder> clauses = new ArrayList<>();
 
-    private Integer slop = null;
+    private final int slop;
 
-    private Boolean inOrder;
+    private boolean inOrder = DEFAULT_IN_ORDER;
 
-    private Boolean collectPayloads;
+    private boolean collectPayloads = DEFAULT_COLLECT_PAYLOADS;
 
     static final SpanNearQueryBuilder PROTOTYPE = new SpanNearQueryBuilder();
 
+    /**
+     * @param slop controls the maximum number of intervening unmatched positions permitted
+     */
+    public SpanNearQueryBuilder(int slop) {
+        this.slop = slop;
+    }
+
+    /**
+     * only used for prototype
+     */
+    private SpanNearQueryBuilder() {
+        this.slop = 0;
+    }
+
+    /**
+     * @return the maximum number of intervening unmatched positions permitted
+     */
+    public int slop() {
+        return this.slop;
+    }
+
     public SpanNearQueryBuilder clause(SpanQueryBuilder clause) {
-        clauses.add(clause);
+        clauses.add(Objects.requireNonNull(clause));
         return this;
     }
 
-    public SpanNearQueryBuilder slop(int slop) {
-        this.slop = slop;
-        return this;
+    /**
+     * @return the {@link SpanQueryBuilder} clauses that were set for this query
+     */
+    public List<SpanQueryBuilder> clauses() {
+        return this.clauses;
     }
 
+    /**
+     * When <code>inOrder</code> is true, the spans from each clause
+     * must be in the same order as in <code>clauses</code> and must be non-overlapping.
+     * Defaults to <code>true</code>
+     */
     public SpanNearQueryBuilder inOrder(boolean inOrder) {
         this.inOrder = inOrder;
         return this;
     }
 
+    /**
+     * @see SpanNearQueryBuilder#inOrder(boolean))
+     */
+    public boolean inOrder() {
+        return this.inOrder;
+    }
+
+    /**
+     * @param collectPayloads flag controlling whether payloads are collected
+     */
     public SpanNearQueryBuilder collectPayloads(boolean collectPayloads) {
         this.collectPayloads = collectPayloads;
         return this;
     }
 
+    /**
+     * @see SpanNearQueryBuilder#collectPayloads(boolean))
+     */
+    public boolean collectPayloads() {
+        return this.collectPayloads;
+    }
+
     @Override
     protected void doXContent(XContentBuilder builder, Params params) throws IOException {
-        if (clauses.isEmpty()) {
-            throw new IllegalArgumentException("Must have at least one clause when building a spanNear query");
-        }
-        if (slop == null) {
-            throw new IllegalArgumentException("Must set the slop when building a spanNear query");
-        }
         builder.startObject(NAME);
         builder.startArray("clauses");
         for (SpanQueryBuilder clause : clauses) {
             clause.toXContent(builder, params);
         }
         builder.endArray();
-        builder.field("slop", slop.intValue());
-        if (inOrder != null) {
-            builder.field("in_order", inOrder);
-        }
-        if (collectPayloads != null) {
-            builder.field("collect_payloads", collectPayloads);
-        }
+        builder.field("slop", slop);
+        builder.field("in_order", inOrder);
+        builder.field("collect_payloads", collectPayloads);
         printBoostAndQueryName(builder);
         builder.endObject();
     }
 
+    @Override
+    protected Query doToQuery(QueryParseContext parseContext) throws IOException {
+        SpanQuery[] spanQueries = new SpanQuery[clauses.size()];
+        for (int i = 0; i < clauses.size(); i++) {
+            Query query = clauses.get(i).toQuery(parseContext);
+            assert query instanceof SpanQuery;
+            spanQueries[i] = (SpanQuery) query;
+        }
+        return new SpanNearQuery(spanQueries, slop, inOrder, collectPayloads);
+    }
+
+    @Override
+    public QueryValidationException validate() {
+        QueryValidationException validationExceptions = null;
+        if (clauses.isEmpty()) {
+            validationExceptions =  addValidationError("query must include [clauses]", validationExceptions);
+        }
+        for (SpanQueryBuilder innerClause : clauses) {
+            validationExceptions = validateInnerQuery(innerClause, validationExceptions);
+        }
+        return validationExceptions;
+    }
+
+    @Override
+    protected SpanNearQueryBuilder doReadFrom(StreamInput in) throws IOException {
+        SpanNearQueryBuilder queryBuilder = new SpanNearQueryBuilder(in.readVInt());
+        List<SpanQueryBuilder> clauses = in.readNamedWriteableList();
+        for (SpanQueryBuilder subClause : clauses) {
+            queryBuilder.clause(subClause);
+        }
+        queryBuilder.collectPayloads = in.readBoolean();
+        queryBuilder.inOrder = in.readBoolean();
+        return queryBuilder;
+
+    }
+
+    @Override
+    protected void doWriteTo(StreamOutput out) throws IOException {
+        out.writeVInt(slop);
+        out.writeNamedWriteableList(clauses);
+        out.writeBoolean(collectPayloads);
+        out.writeBoolean(inOrder);
+    }
+
+    @Override
+    protected int doHashCode() {
+        return Objects.hash(clauses, slop, collectPayloads, inOrder);
+    }
+
+    @Override
+    protected boolean doEquals(SpanNearQueryBuilder other) {
+        return Objects.equals(clauses, other.clauses) &&
+               Objects.equals(slop, other.slop) &&
+               Objects.equals(collectPayloads, other.collectPayloads) &&
+               Objects.equals(inOrder, other.inOrder);
+    }
+
     @Override
     public String getName() {
         return NAME;

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

@@ -19,9 +19,6 @@
 
 package org.elasticsearch.index.query;
 
-import org.apache.lucene.search.Query;
-import org.apache.lucene.search.spans.SpanNearQuery;
-import org.apache.lucene.search.spans.SpanQuery;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -34,7 +31,7 @@
 /**
  *
  */
-public class SpanNearQueryParser extends BaseQueryParserTemp {
+public class SpanNearQueryParser extends BaseQueryParser {
 
     @Inject
     public SpanNearQueryParser() {
@@ -46,16 +43,16 @@ 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;
         Integer slop = null;
-        boolean inOrder = true;
-        boolean collectPayloads = true;
+        boolean inOrder = SpanNearQueryBuilder.DEFAULT_IN_ORDER;
+        boolean collectPayloads = SpanNearQueryBuilder.DEFAULT_COLLECT_PAYLOADS;
         String queryName = null;
 
-        List<SpanQuery> clauses = newArrayList();
+        List<SpanQueryBuilder> clauses = newArrayList();
 
         String currentFieldName = null;
         XContentParser.Token token;
@@ -65,11 +62,11 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
             } else if (token == XContentParser.Token.START_ARRAY) {
                 if ("clauses".equals(currentFieldName)) {
                     while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
-                        Query query = parseContext.parseInnerQuery();
-                        if (!(query instanceof SpanQuery)) {
+                        QueryBuilder query = parseContext.parseInnerQueryBuilder();
+                        if (!(query instanceof SpanQueryBuilder)) {
                             throw new QueryParsingException(parseContext, "spanNear [clauses] must be of type span query");
                         }
-                        clauses.add((SpanQuery) query);
+                        clauses.add((SpanQueryBuilder) query);
                     }
                 } else {
                     throw new QueryParsingException(parseContext, "[span_near] query does not support [" + currentFieldName + "]");
@@ -92,19 +89,20 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
                 throw new QueryParsingException(parseContext, "[span_near] query does not support [" + currentFieldName + "]");
             }
         }
-        if (clauses.isEmpty()) {
-            throw new QueryParsingException(parseContext, "span_near must include [clauses]");
-        }
+
         if (slop == null) {
             throw new QueryParsingException(parseContext, "span_near must include [slop]");
         }
 
-        SpanNearQuery query = new SpanNearQuery(clauses.toArray(new SpanQuery[clauses.size()]), slop.intValue(), inOrder, collectPayloads);
-        query.setBoost(boost);
-        if (queryName != null) {
-            parseContext.addNamedQuery(queryName, query);
+        SpanNearQueryBuilder queryBuilder = new SpanNearQueryBuilder(slop);
+        for (SpanQueryBuilder subQuery : clauses) {
+            queryBuilder.clause(subQuery);
         }
-        return query;
+        queryBuilder.inOrder(inOrder);
+        queryBuilder.collectPayloads(collectPayloads);
+        queryBuilder.boost(boost);
+        queryBuilder.queryName(queryName);
+        return queryBuilder;
     }
 
     @Override

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

@@ -1384,7 +1384,7 @@ public void testSpanFirstQuery() throws IOException {
     @Test
     public void testSpanNearQueryBuilder() throws IOException {
         IndexQueryParserService queryParser = queryParser();
-        Query parsedQuery = queryParser.parse(spanNearQuery().clause(spanTermQuery("age", 34)).clause(spanTermQuery("age", 35)).clause(spanTermQuery("age", 36)).slop(12).inOrder(false).collectPayloads(false)).query();
+        Query parsedQuery = queryParser.parse(spanNearQuery(12).clause(spanTermQuery("age", 34)).clause(spanTermQuery("age", 35)).clause(spanTermQuery("age", 36)).inOrder(false).collectPayloads(false)).query();
         assertThat(parsedQuery, instanceOf(SpanNearQuery.class));
         SpanNearQuery spanNearQuery = (SpanNearQuery) parsedQuery;
         assertThat(spanNearQuery.getClauses().length, equalTo(3));

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

@@ -0,0 +1,85 @@
+/*
+ * 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.SpanNearQuery;
+import org.apache.lucene.search.spans.SpanQuery;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.List;
+
+public class SpanNearQueryBuilderTest extends BaseQueryTestCase<SpanNearQueryBuilder> {
+
+    @Override
+    protected Query doCreateExpectedQuery(SpanNearQueryBuilder testQueryBuilder, QueryParseContext context) throws IOException {
+        List<SpanQueryBuilder> clauses = testQueryBuilder.clauses();
+        SpanQuery[] spanQueries = new SpanQuery[clauses.size()];
+        for (int i = 0; i < clauses.size(); i++) {
+            Query query = clauses.get(i).toQuery(context);
+            assert query instanceof SpanQuery;
+            spanQueries[i] = (SpanQuery) query;
+        }
+        return new SpanNearQuery(spanQueries, testQueryBuilder.slop(), testQueryBuilder.inOrder(), testQueryBuilder.collectPayloads());
+
+    }
+
+    @Override
+    protected SpanNearQueryBuilder doCreateTestQueryBuilder() {
+        SpanNearQueryBuilder queryBuilder = new SpanNearQueryBuilder(randomIntBetween(-10, 10));
+        int clauses = randomIntBetween(1, 6);
+        // we use one random SpanTermQueryBuilder to determine same field name for subsequent clauses
+        String fieldName = new SpanTermQueryBuilderTest().createTestQueryBuilder().fieldName();
+        for (int i = 0; i < clauses; i++) {
+            // we need same field name in all clauses, so we only randomize value
+            Object value;
+            switch (fieldName) {
+                case BOOLEAN_FIELD_NAME: value = randomBoolean(); break;
+                case INT_FIELD_NAME: value = randomInt(); break;
+                case DOUBLE_FIELD_NAME: value = randomDouble(); break;
+                case STRING_FIELD_NAME: value = randomAsciiOfLengthBetween(1, 10); break;
+                default : value = randomAsciiOfLengthBetween(1, 10);
+            }
+            queryBuilder.clause(new SpanTermQueryBuilder(fieldName, value));
+        }
+        queryBuilder.inOrder(randomBoolean());
+        queryBuilder.collectPayloads(randomBoolean());
+        return queryBuilder;
+    }
+
+    @Test
+    public void testValidate() {
+        SpanNearQueryBuilder queryBuilder = new SpanNearQueryBuilder(1);
+        assertValidate(queryBuilder, 1); // empty clause list
+
+        int totalExpectedErrors = 0;
+        int clauses = randomIntBetween(1, 10);
+        for (int i = 0; i < clauses; i++) {
+            if (randomBoolean()) {
+                queryBuilder.clause(new SpanTermQueryBuilder("", "test"));
+                totalExpectedErrors++;
+            } else {
+                queryBuilder.clause(new SpanTermQueryBuilder("name", "value"));
+            }
+        }
+        assertValidate(queryBuilder, totalExpectedErrors);
+    }
+}