Merge pull request elastic#11628 from cbuescher/feature/query-refactoring-and

cbuescher · cbuescher · commit acbc7b2447a4 · 2015-06-16T14:56:35.000+02:00
Query refactoring: AndQueryBuilder and Parser
diff --git a/core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java
@@ -21,17 +21,24 @@
 
 import com.google.common.collect.Lists;
 
+import org.apache.lucene.search.BooleanClause.Occur;
+import org.apache.lucene.search.BooleanQuery;
+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;
 import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
 
 /**
  * A filter that matches documents matching boolean combinations of other filters.
  * @deprecated Use {@link BoolQueryBuilder} instead
  */
 @Deprecated
-public class AndQueryBuilder extends QueryBuilder {
+public class AndQueryBuilder extends QueryBuilder<AndQueryBuilder> {
 
     public static final String NAME = "and";
 
@@ -55,6 +62,13 @@ public AndQueryBuilder add(QueryBuilder filterBuilder) {
         return this;
     }
 
+    /**
+     * @return the list of filters added to "and".
+     */
+    public List<QueryBuilder> filters() {
+        return this.filters;
+    }
+
     /**
      * Sets the filter name for the filter that can be used when searching for matched_filters per hit.
      */
@@ -63,6 +77,14 @@ public AndQueryBuilder queryName(String queryName) {
         return this;
     }
 
+    /**
+     * @return the query name.
+     */
+    public Object queryName() {
+        return this.queryName;
+    }
+
+
     @Override
     protected void doXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject(NAME);
@@ -77,8 +99,61 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
         builder.endObject();
     }
 
+    @Override
+    public Query toQuery(QueryParseContext parseContext) throws QueryParsingException, IOException {
+        if (filters.isEmpty()) {
+            // no filters provided, this should be ignored upstream
+            return null;
+        }
+
+        BooleanQuery query = new BooleanQuery();
+        for (QueryBuilder f : filters) {
+            query.add(f.toQuery(parseContext), Occur.MUST);
+        }
+        if (queryName != null) {
+            parseContext.addNamedQuery(queryName, query);
+        }
+        return query;
+    }
+
     @Override
     public String queryId() {
         return NAME;
     }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(filters, queryName);
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj == null || getClass() != obj.getClass()) {
+            return false;
+        }
+        AndQueryBuilder other = (AndQueryBuilder) obj;
+        return Objects.equals(filters, other.filters) &&
+                Objects.equals(queryName, other.queryName);
+    }
+
+    @Override
+    public AndQueryBuilder readFrom(StreamInput in) throws IOException {
+        AndQueryBuilder andQueryBuilder = new AndQueryBuilder();
+        List<QueryBuilder> queryBuilders = in.readNamedWritableList();
+        for (QueryBuilder queryBuilder : queryBuilders) {
+            andQueryBuilder.add(queryBuilder);
+        }
+        andQueryBuilder.queryName = in.readOptionalString();
+        return andQueryBuilder;
+
+    }
+
+    @Override
+    public void writeTo(StreamOutput out) throws IOException {
+        out.writeNamedWritableList(this.filters);
+        out.writeOptionalString(queryName);
+    }
 }
diff --git a/core/src/main/java/org/elasticsearch/index/query/AndQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/AndQueryParser.java
@@ -19,9 +19,6 @@
 
 package org.elasticsearch.index.query;
 
-import org.apache.lucene.search.BooleanClause.Occur;
-import org.apache.lucene.search.BooleanQuery;
-import org.apache.lucene.search.Query;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.xcontent.XContentParser;
 
@@ -34,7 +31,7 @@
  *
  */
 @Deprecated
-public class AndQueryParser extends BaseQueryParserTemp {
+public class AndQueryParser extends BaseQueryParser {
 
     @Inject
     public AndQueryParser() {
@@ -46,10 +43,10 @@ public String[] names() {
     }
 
     @Override
-    public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException {
+    public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException {
         XContentParser parser = parseContext.parser();
 
-        ArrayList<Query> queries = newArrayList();
+        ArrayList<QueryBuilder> queries = newArrayList();
         boolean queriesFound = false;
 
         String queryName = null;
@@ -58,7 +55,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
         if (token == XContentParser.Token.START_ARRAY) {
             while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
                 queriesFound = true;
-                Query filter = parseContext.parseInnerFilter();
+                QueryBuilder filter = parseContext.parseInnerFilterToQueryBuilder();
                 if (filter != null) {
                     queries.add(filter);
                 }
@@ -73,15 +70,15 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
                     if ("filters".equals(currentFieldName)) {
                         queriesFound = true;
                         while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
-                            Query filter = parseContext.parseInnerFilter();
+                            QueryBuilder filter = parseContext.parseInnerFilterToQueryBuilder();
                             if (filter != null) {
                                 queries.add(filter);
                             }
                         }
                     } else {
                         queriesFound = true;
                         while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
-                            Query filter = parseContext.parseInnerFilter();
+                            QueryBuilder filter = parseContext.parseInnerFilterToQueryBuilder();
                             if (filter != null) {
                                 queries.add(filter);
                             }
@@ -101,19 +98,13 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
             throw new QueryParsingException(parseContext, "[and] query requires 'filters' to be set on it'");
         }
 
-        if (queries.isEmpty()) {
-            // no filters provided, this should be ignored upstream
-            return null;
+        AndQueryBuilder andQuery = new AndQueryBuilder();
+        for (QueryBuilder query : queries) {
+            andQuery.add(query);
         }
-
-        BooleanQuery query = new BooleanQuery();
-        for (Query f : queries) {
-            query.add(f, Occur.MUST);
-        }
-        if (queryName != null) {
-            parseContext.addNamedQuery(queryName, query);
-        }
-        return query;
+        andQuery.queryName(queryName);
+        andQuery.validate();
+        return andQuery;
     }
 
     @Override
diff --git a/core/src/test/java/org/elasticsearch/index/query/AndQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/AndQueryBuilderTest.java
@@ -0,0 +1,90 @@
+/*
+ * 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.BooleanQuery;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.BooleanClause.Occur;
+import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.junit.Test;
+
+import java.io.IOException;
+
+import static org.hamcrest.Matchers.equalTo;
+
+@SuppressWarnings("deprecation")
+public class AndQueryBuilderTest extends BaseQueryTestCase<AndQueryBuilder> {
+
+    @Override
+    protected Query createExpectedQuery(AndQueryBuilder queryBuilder, QueryParseContext context) throws QueryParsingException, IOException {
+        if (queryBuilder.filters().isEmpty()) {
+            return null;
+        }
+        BooleanQuery query = new BooleanQuery();
+        for (QueryBuilder subQuery : queryBuilder.filters()) {
+            query.add(subQuery.toQuery(context), Occur.MUST);
+        }
+        return query;
+    }
+
+    /**
+     * @return a AndQueryBuilder with random limit between 0 and 20
+     */
+    @Override
+    protected AndQueryBuilder createTestQueryBuilder() {
+        AndQueryBuilder query = new AndQueryBuilder();
+        int subQueries = randomIntBetween(1, 5);
+        for (int i = 0; i < subQueries; i++ ) {
+            query.add(RandomQueryBuilder.create(random()));
+        }
+        if (randomBoolean()) {
+            query.queryName(randomAsciiOfLengthBetween(1, 10));
+        }
+        return query;
+    }
+
+    @Override
+    protected void assertLuceneQuery(AndQueryBuilder queryBuilder, Query query, QueryParseContext context) {
+        if (queryBuilder.queryName() != null) {
+            Query namedQuery = context.copyNamedFilters().get(queryBuilder.queryName());
+            assertThat(namedQuery, equalTo(query));
+        }
+    }
+
+    /**
+     * test corner case where no inner queries exist
+     */
+    @Test
+    public void testNoInnerQueries() throws QueryParsingException, IOException {
+        AndQueryBuilder andQuery = new AndQueryBuilder();
+        assertNull(andQuery.toQuery(createContext()));
+    }
+
+    @Test(expected=QueryParsingException.class)
+    public void testMissingFiltersSection() throws IOException {
+        QueryParseContext context = createContext();
+        String queryString = "{ \"and\" : {}";
+        XContentParser parser = XContentFactory.xContent(queryString).createParser(queryString);
+        context.reset(parser);
+        assertQueryHeader(parser, AndQueryBuilder.PROTOTYPE.queryId());
+        context.indexQueryParserService().queryParser(AndQueryBuilder.PROTOTYPE.queryId()).fromXContent(context);
+    }
+}
