Aggregations Refactor: Refactor Value Count Aggregation

Author: colings86

core/src/main/java/org/elasticsearch/search/aggregations/metrics/valuecount/ValueCountAggregator.java

@@ -19,9 +19,12 @@
 package org.elasticsearch.search.aggregations.metrics.valuecount;
 
 import org.apache.lucene.index.LeafReaderContext;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.lease.Releasables;
 import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.common.util.LongArray;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.index.fielddata.SortedBinaryDocValues;
 import org.elasticsearch.search.aggregations.Aggregator;
 import org.elasticsearch.search.aggregations.InternalAggregation;
@@ -30,9 +33,10 @@
 import org.elasticsearch.search.aggregations.metrics.NumericMetricsAggregator;
 import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
 import org.elasticsearch.search.aggregations.support.AggregationContext;
+import org.elasticsearch.search.aggregations.support.ValueType;
 import org.elasticsearch.search.aggregations.support.ValuesSource;
 import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory;
-import org.elasticsearch.search.aggregations.support.ValuesSourceParser;
+import org.elasticsearch.search.aggregations.support.ValuesSourceType;
 import org.elasticsearch.search.aggregations.support.format.ValueFormatter;
 
 import java.io.IOException;
@@ -110,8 +114,8 @@ public void doClose() {
 
     public static class Factory<VS extends ValuesSource> extends ValuesSourceAggregatorFactory.LeafOnly<VS> {
 
-        public Factory(String name, ValuesSourceParser.Input<VS> input) {
-            super(name, InternalValueCount.TYPE.name(), input);
+        public Factory(String name, ValuesSourceType valuesSourceType, ValueType valueType) {
+            super(name, InternalValueCount.TYPE.name(), valuesSourceType, valueType);
         }
 
         @Override
@@ -128,6 +132,32 @@ protected Aggregator doCreateInternal(VS valuesSource, AggregationContext aggreg
                     metaData);
         }
 
+        @Override
+        protected ValuesSourceAggregatorFactory<VS> innerReadFrom(String name, ValuesSourceType valuesSourceType,
+                ValueType targetValueType, StreamInput in) {
+            return new ValueCountAggregator.Factory<VS>(name, valuesSourceType, targetValueType);
+        }
+
+        @Override
+        protected void innerWriteTo(StreamOutput out) {
+            // Do nothing, no extra state to write to stream
+        }
+
+        @Override
+        public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
+            return builder;
+        }
+
+        @Override
+        protected int innerHashCode() {
+            return 0;
+        }
+
+        @Override
+        protected boolean innerEquals(Object obj) {
+            return true;
+        }
+
     }
 
 }

core/src/main/java/org/elasticsearch/search/aggregations/metrics/valuecount/ValueCountParser.java

@@ -18,48 +18,47 @@
  */
 package org.elasticsearch.search.aggregations.metrics.valuecount;
 
+import org.elasticsearch.common.ParseField;
+import org.elasticsearch.common.ParseFieldMatcher;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.search.SearchParseException;
-import org.elasticsearch.search.aggregations.Aggregator;
 import org.elasticsearch.search.aggregations.AggregatorFactory;
-import org.elasticsearch.search.aggregations.support.ValuesSourceParser;
-import org.elasticsearch.search.internal.SearchContext;
+import org.elasticsearch.search.aggregations.support.AbstractValuesSourceParser.AnyValuesSourceParser;
+import org.elasticsearch.search.aggregations.support.ValueType;
+import org.elasticsearch.search.aggregations.support.ValuesSource;
+import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory;
+import org.elasticsearch.search.aggregations.support.ValuesSourceType;
 
 import java.io.IOException;
+import java.util.Map;
 
 /**
  *
  */
-public class ValueCountParser implements Aggregator.Parser {
+public class ValueCountParser extends AnyValuesSourceParser {
+
+    public ValueCountParser() {
+        super(true, true);
+    }
 
     @Override
     public String type() {
         return InternalValueCount.TYPE.name();
     }
 
     @Override
-    public AggregatorFactory parse(String aggregationName, XContentParser parser, SearchContext context) throws IOException {
-
-        ValuesSourceParser vsParser = ValuesSourceParser.any(aggregationName, InternalValueCount.TYPE, context)
-                .build();
-
-        XContentParser.Token token;
-        String currentFieldName = null;
-        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
-            if (token == XContentParser.Token.FIELD_NAME) {
-                currentFieldName = parser.currentName();
-            } else if (!vsParser.token(currentFieldName, token, parser)) {
-                throw new SearchParseException(context, "Unexpected token " + token + " in [" + aggregationName + "].",
-                        parser.getTokenLocation());
-            }
-        }
+    protected boolean token(String aggregationName, String currentFieldName, XContentParser.Token token, XContentParser parser,
+            ParseFieldMatcher parseFieldMatcher, Map<ParseField, Object> otherOptions) throws IOException {
+        return false;
+    }
 
-        return new ValueCountAggregator.Factory(aggregationName, vsParser.input());
+    @Override
+    protected ValuesSourceAggregatorFactory<ValuesSource> createFactory(String aggregationName, ValuesSourceType valuesSourceType,
+            ValueType targetValueType, Map<ParseField, Object> otherOptions) {
+        return new ValueCountAggregator.Factory<ValuesSource>(aggregationName, valuesSourceType, targetValueType);
     }
 
-    // NORELEASE implement this method when refactoring this aggregation
     @Override
     public AggregatorFactory getFactoryPrototype() {
-        return null;
+        return new ValueCountAggregator.Factory<ValuesSource>(null, null, null);
     }
 }

core/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountTests.java

@@ -0,0 +1,54 @@
+/*
+ * 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.search.aggregations.metrics;
+
+import org.elasticsearch.script.Script;
+import org.elasticsearch.search.aggregations.BaseAggregationTestCase;
+import org.elasticsearch.search.aggregations.metrics.valuecount.ValueCountAggregator;
+import org.elasticsearch.search.aggregations.support.ValuesSource;
+import org.elasticsearch.search.aggregations.support.ValuesSourceType;
+
+public class ValueCountTests extends BaseAggregationTestCase<ValueCountAggregator.Factory<? extends ValuesSource>> {
+
+    @Override
+    protected final ValueCountAggregator.Factory<? extends ValuesSource> createTestAggregatorFactory() {
+        ValueCountAggregator.Factory<ValuesSource> factory = new ValueCountAggregator.Factory<ValuesSource>("foo", ValuesSourceType.ANY,
+                null);
+        String field = randomNumericField();
+        int randomFieldBranch = randomInt(3);
+        switch (randomFieldBranch) {
+        case 0:
+            factory.field(field);
+            break;
+        case 1:
+            factory.field(field);
+            factory.script(new Script("_value + 1"));
+            break;
+        case 2:
+            factory.script(new Script("doc[" + field + "] + 1"));
+            break;
+        }
+        if (randomBoolean()) {
+            factory.missing("MISSING");
+        }
+        return factory;
+    }
+
+}