Query refactoring: MatchAllQueryBuilder

src/main/java/org/elasticsearch/index/query/MatchAllQueryBuilder.java

@@ -19,18 +19,22 @@
 
 package org.elasticsearch.index.query;
 
+import org.apache.lucene.search.MatchAllDocsQuery;
+import org.apache.lucene.search.Query;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.io.stream.Streamable;
+import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 
 import java.io.IOException;
 
 /**
  * A query that matches on all documents.
- *
- *
  */
-public class MatchAllQueryBuilder extends BaseQueryBuilder implements BoostableQueryBuilder<MatchAllQueryBuilder> {
+public class MatchAllQueryBuilder extends BaseQueryBuilder implements Streamable, BoostableQueryBuilder<MatchAllQueryBuilder> {
 
-    private float boost = -1;
+    private float boost = 1.0f;
 
     /**
      * Sets the boost for this query.  Documents matching this query will (in addition to the normal
@@ -42,10 +46,17 @@ public MatchAllQueryBuilder boost(float boost) {
         return this;
     }
 
+    /**
+     * Gets the boost for this query.
+     */
+    public float boost() {
+        return this.boost;
+    }
+
     @Override
     public void doXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject(MatchAllQueryParser.NAME);
-        if (boost != -1) {
+        if (boost != 1.0f) {
             builder.field("boost", boost);
         }
         builder.endObject();
@@ -54,4 +65,42 @@ public void doXContent(XContentBuilder builder, Params params) throws IOExceptio
     final protected String parserName() {
         return MatchAllQueryParser.NAME;
     }
-}
+
+    @Override
+    public Query toQuery(QueryParseContext parseContext) {
+        if (this.boost == 1.0f) {
+            return Queries.newMatchAllQuery();
+        }
+
+        MatchAllDocsQuery query = new MatchAllDocsQuery();
+        query.setBoost(boost);
+        return query;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        MatchAllQueryBuilder that = (MatchAllQueryBuilder) o;
+        return Float.compare(that.boost, boost) == 0;
+    }
+
+    @Override
+    public int hashCode() {
+        return boost != +0.0f ? Float.floatToIntBits(boost) : 0;
+    }
+
+    @Override
+    public void readFrom(StreamInput in) throws IOException {
+        this.boost = in.readFloat();
+    }
+
+    @Override
+    public void writeTo(StreamOutput out) throws IOException {
+        out.writeFloat(this.boost);
+    }
+}

src/main/java/org/elasticsearch/index/query/MatchAllQueryParser.java

@@ -19,19 +19,16 @@
 
 package org.elasticsearch.index.query;
 
-import org.apache.lucene.search.MatchAllDocsQuery;
-import org.apache.lucene.search.Query;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.inject.Inject;
-import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.xcontent.XContentParser;
 
 import java.io.IOException;
 
 /**
- *
+ * Parser code for MatchAllQuery
  */
-public class MatchAllQueryParser extends BaseQueryParserTemp {
+public class MatchAllQueryParser extends BaseQueryParser {
 
     public static final String NAME = "match_all";
 
@@ -44,32 +41,23 @@ public String[] names() {
         return new String[]{NAME, Strings.toCamelCase(NAME)};
     }
 
-    @Override
-    public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException {
+    public MatchAllQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException {
+        MatchAllQueryBuilder queryBuilder = new MatchAllQueryBuilder();
         XContentParser parser = parseContext.parser();
 
-        float boost = 1.0f;
         String currentFieldName = null;
-
         XContentParser.Token token;
         while (((token = parser.nextToken()) != XContentParser.Token.END_OBJECT && token != XContentParser.Token.END_ARRAY)) {
             if (token == XContentParser.Token.FIELD_NAME) {
                 currentFieldName = parser.currentName();
             } else if (token.isValue()) {
                 if ("boost".equals(currentFieldName)) {
-                    boost = parser.floatValue();
+                    queryBuilder.boost(parser.floatValue());
                 } else {
                     throw new QueryParsingException(parseContext.index(), "[match_all] query does not support [" + currentFieldName + "]");
                 }
             }
         }
-
-        if (boost == 1.0f) {
-            return Queries.newMatchAllQuery();
-        }
-
-        MatchAllDocsQuery query = new MatchAllDocsQuery();
-        query.setBoost(boost);
-        return query;
+        return queryBuilder;
     }
-}
+}

src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java

@@ -0,0 +1,166 @@
+/*
+ * 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.elasticsearch.Version;
+import org.elasticsearch.cluster.ClusterService;
+import org.elasticsearch.cluster.metadata.IndexMetaData;
+import org.elasticsearch.common.inject.AbstractModule;
+import org.elasticsearch.common.inject.Injector;
+import org.elasticsearch.common.inject.ModulesBuilder;
+import org.elasticsearch.common.inject.util.Providers;
+import org.elasticsearch.common.io.stream.BytesStreamInput;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.Streamable;
+import org.elasticsearch.common.settings.ImmutableSettings;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.settings.SettingsModule;
+import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.env.Environment;
+import org.elasticsearch.env.EnvironmentModule;
+import org.elasticsearch.index.Index;
+import org.elasticsearch.index.IndexNameModule;
+import org.elasticsearch.index.analysis.AnalysisModule;
+import org.elasticsearch.index.cache.IndexCacheModule;
+import org.elasticsearch.index.query.functionscore.FunctionScoreModule;
+import org.elasticsearch.index.settings.IndexSettingsModule;
+import org.elasticsearch.index.similarity.SimilarityModule;
+import org.elasticsearch.indices.breaker.CircuitBreakerService;
+import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
+import org.elasticsearch.indices.query.IndicesQueriesModule;
+import org.elasticsearch.script.ScriptModule;
+import org.elasticsearch.test.ElasticsearchTestCase;
+import org.elasticsearch.threadpool.ThreadPool;
+import org.elasticsearch.threadpool.ThreadPoolModule;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Ignore;
+import org.junit.Test;
+
+import java.io.IOException;
+
+@Ignore
+public abstract class BaseQueryTestCase<QB extends BaseQueryBuilder & Streamable> extends ElasticsearchTestCase {
+
+    private static Injector injector;
+    private static IndexQueryParserService queryParserService;
+    private static Index index;
+
+    protected QB testQuery = createTestQueryBuilder();
+
+    /**
+     * Setup for the whole base test class.
+     * @throws IOException
+     */
+    @BeforeClass
+    public static void init() throws IOException {
+        Settings settings = ImmutableSettings.settingsBuilder()
+                .put("name", BaseQueryTestCase.class.toString())
+                .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
+                .build();
+
+        index = new Index("test");
+        injector = new ModulesBuilder().add(
+                new EnvironmentModule(new Environment(settings)),
+                new SettingsModule(settings),
+                new ThreadPoolModule(settings),
+                new IndicesQueriesModule(),
+                new ScriptModule(settings),
+                new IndexSettingsModule(index, settings),
+                new IndexCacheModule(settings),
+                new AnalysisModule(settings),
+                new SimilarityModule(settings),
+                new IndexNameModule(index),
+                new IndexQueryParserModule(settings),
+                new FunctionScoreModule(),
+                new AbstractModule() {
+                    @Override
+                    protected void configure() {
+                        bind(ClusterService.class).toProvider(Providers.of((ClusterService) null));
+                        bind(CircuitBreakerService.class).to(NoneCircuitBreakerService.class);
+                    }
+                }
+        ).createInjector();
+        queryParserService = injector.getInstance(IndexQueryParserService.class);
+    }
+
+    @AfterClass
+    public static void after() throws Exception {
+        terminate(injector.getInstance(ThreadPool.class));
+    }
+
+    /**
+     * Create the query that is being tested
+     */
+    protected abstract QB createTestQueryBuilder();
+
+    /**
+     * Subclass should handle assertions on the lucene query produced by the query builder under test here
+     */
+    protected abstract void assertLuceneQuery(QB queryBuilder, Query query) throws IOException;
+
+    /**
+     * Creates an empty builder of the type of query under test
+     */
+    protected abstract QB createEmptyQueryBuilder();
+
+    /**
+     * Generic test that creates new query from the test query and checks both for equality
+     * and asserts equality on the two queries.
+     */
+    @Test
+    public void testFromXContent() throws IOException {
+        QueryParseContext context = new QueryParseContext(index, queryParserService);
+        String contentString = testQuery.toString();
+        context.reset(XContentFactory.xContent(contentString).createParser(contentString));
+
+        QueryBuilder newQuery = queryParserService.queryParser(testQuery.parserName()).fromXContent(context);
+        assertNotSame(newQuery, testQuery);
+        assertEquals(newQuery, testQuery);
+    }
+
+    /**
+     * Test creates the {@link Query} from the {@link QueryBuilder} under test and delegates the
+     * assertions being made on the result to the implementing subclass.
+     */
+    @Test
+    public void testToQuery() throws IOException {
+        QueryParseContext context = new QueryParseContext(index, queryParserService);
+        assertLuceneQuery(this.testQuery, this.testQuery.toQuery(context));
+    }
+
+    /**
+     * Test serialization and deserialization of the test query.
+     * @throws IOException
+     */
+    @Test
+    public void testSerialization() throws IOException {
+        BytesStreamOutput output = new BytesStreamOutput();
+        testQuery.writeTo(output);
+
+        BytesStreamInput in = new BytesStreamInput(output.bytes());
+        QB deserializedQuery = createEmptyQueryBuilder();
+        deserializedQuery.readFrom(in);
+
+        assertEquals(deserializedQuery, testQuery);
+        assertNotSame(deserializedQuery, testQuery);
+    }
+}

src/test/java/org/elasticsearch/index/query/MatchAllQueryBuilderTest.java

@@ -0,0 +1,59 @@
+/*
+ * 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.MatchAllDocsQuery;
+import org.apache.lucene.search.Query;
+
+import java.io.IOException;
+
+import static org.hamcrest.Matchers.*;
+
+public class MatchAllQueryBuilderTest extends BaseQueryTestCase<MatchAllQueryBuilder> {
+
+    @Override
+    protected void assertLuceneQuery(MatchAllQueryBuilder queryBuilder, Query query) throws IOException {
+        if (queryBuilder.boost() != 1.0f) {
+            assertThat(query, instanceOf(MatchAllDocsQuery.class));
+        } else {
+            assertThat(query, instanceOf(ConstantScoreQuery.class));
+        }
+        assertThat(query.getBoost(), is(queryBuilder.boost()));
+    }
+
+    @Override
+    protected MatchAllQueryBuilder createEmptyQueryBuilder() {
+        return new MatchAllQueryBuilder();
+    }
+
+    /**
+     * @return a MatchAllQuery with random boost between 0.1f and 2.0f
+     */
+    @Override
+    protected MatchAllQueryBuilder createTestQueryBuilder() {
+        MatchAllQueryBuilder query = new MatchAllQueryBuilder();
+        if (randomBoolean()) {
+            query.boost(2.0f / randomIntBetween(1, 20));
+        }
+        return query;
+    }
+
+}