From a36b6138d7045096df243f8affdf200d0dbe473e Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 14 Apr 2016 14:41:55 -0400 Subject: [PATCH] Cut avg aggregation to registerAggregation and remove its PROTOTYPE Relates to #17085 --- .../elasticsearch/search/SearchModule.java | 3 +- .../aggregations/AggregatorBuilder.java | 1 + .../metrics/avg/AvgAggregatorBuilder.java | 43 ++++++++++++------- .../aggregations/metrics/avg/AvgParser.java | 11 ----- .../ValuesSourceAggregatorBuilder.java | 13 +++++- 5 files changed, 41 insertions(+), 30 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/SearchModule.java b/core/src/main/java/org/elasticsearch/search/SearchModule.java index 44b504d777f..5aa8764d9f9 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/core/src/main/java/org/elasticsearch/search/SearchModule.java @@ -148,6 +148,7 @@ import org.elasticsearch.search.aggregations.bucket.terms.LongTerms; import org.elasticsearch.search.aggregations.bucket.terms.StringTerms; import org.elasticsearch.search.aggregations.bucket.terms.TermsParser; import org.elasticsearch.search.aggregations.bucket.terms.UnmappedTerms; +import org.elasticsearch.search.aggregations.metrics.avg.AvgAggregatorBuilder; import org.elasticsearch.search.aggregations.metrics.avg.AvgParser; import org.elasticsearch.search.aggregations.metrics.avg.InternalAvg; import org.elasticsearch.search.aggregations.metrics.cardinality.CardinalityParser; @@ -442,7 +443,7 @@ public class SearchModule extends AbstractModule { SignificanceHeuristicParserMapper significanceHeuristicParserMapper = new SignificanceHeuristicParserMapper(heuristicParsers); - registerAggregatorParser(new AvgParser()); + registerAggregation(AvgAggregatorBuilder::new, new AvgParser(), AvgAggregatorBuilder.AGGREGATION_NAME_FIELD); registerAggregatorParser(new SumParser()); registerAggregatorParser(new MinParser()); registerAggregatorParser(new MaxParser()); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/AggregatorBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/AggregatorBuilder.java index 04fa29989db..165f047d252 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/AggregatorBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/AggregatorBuilder.java @@ -191,6 +191,7 @@ public abstract class AggregatorBuilder> extend @Override public String getWriteableName() { // NORELEASE remove this before 5.0.0GA - all builders will implement this method on their own. + assert usesNewStyleSerialization() == false: "migrated aggregations should just return their NAME"; return type.stream().toUtf8(); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/avg/AvgAggregatorBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/avg/AvgAggregatorBuilder.java index 351069f79d9..fea1ddd8328 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/avg/AvgAggregatorBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/avg/AvgAggregatorBuilder.java @@ -19,47 +19,53 @@ package org.elasticsearch.search.aggregations.metrics.avg; +import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.ToXContent.Params; -import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; +import org.elasticsearch.search.aggregations.AggregatorFactory; 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.ValuesSource.Numeric; import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorBuilder; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; import org.elasticsearch.search.aggregations.support.ValuesSourceType; -import org.elasticsearch.search.aggregations.support.ValuesSource.Numeric; import java.io.IOException; public class AvgAggregatorBuilder extends ValuesSourceAggregatorBuilder.LeafOnly { - - static final AvgAggregatorBuilder PROTOTYPE = new AvgAggregatorBuilder(""); + public static final String NAME = InternalAvg.TYPE.name(); + public static final ParseField AGGREGATION_NAME_FIELD = new ParseField(NAME); public AvgAggregatorBuilder(String name) { super(name, InternalAvg.TYPE, ValuesSourceType.NUMERIC, ValueType.NUMERIC); } + /** + * Read from a stream. + */ + public AvgAggregatorBuilder(StreamInput in) throws IOException { + super(in, InternalAvg.TYPE, ValuesSourceType.NUMERIC, ValueType.NUMERIC); + } + + @Override + protected void innerWriteTo(StreamOutput out) { + // Do nothing, no extra state to write to stream + } + + @Override + protected boolean usesNewStyleSerialization() { + return true; + } + @Override protected AvgAggregatorFactory innerBuild(AggregationContext context, ValuesSourceConfig config, AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException { return new AvgAggregatorFactory(name, type, config, context, parent, subFactoriesBuilder, metaData); } - @Override - protected AvgAggregatorBuilder innerReadFrom(String name, ValuesSourceType valuesSourceType, - ValueType targetValueType, StreamInput in) { - return new AvgAggregatorBuilder(name); - } - - @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; @@ -74,4 +80,9 @@ public class AvgAggregatorBuilder extends ValuesSourceAggregatorBuilder.LeafOnly protected boolean innerEquals(Object obj) { return true; } + + @Override + public String getWriteableName() { + return NAME; + } } \ No newline at end of file diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/avg/AvgParser.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/avg/AvgParser.java index f51e4070fd7..edb3d8f6620 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/avg/AvgParser.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/avg/AvgParser.java @@ -37,11 +37,6 @@ public class AvgParser extends NumericValuesSourceParser { super(true, true, false); } - @Override - public String type() { - return InternalAvg.TYPE.name(); - } - @Override protected boolean token(String aggregationName, String currentFieldName, XContentParser.Token token, XContentParser parser, ParseFieldMatcher parseFieldMatcher, Map otherOptions) throws IOException { @@ -53,10 +48,4 @@ public class AvgParser extends NumericValuesSourceParser { ValueType targetValueType, Map otherOptions) { return new AvgAggregatorBuilder(aggregationName); } - - @Override - public AvgAggregatorBuilder getFactoryPrototypes() { - return AvgAggregatorBuilder.PROTOTYPE; - } - } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceAggregatorBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceAggregatorBuilder.java index 486fae0d52a..cfec5a8962b 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceAggregatorBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceAggregatorBuilder.java @@ -21,7 +21,6 @@ package org.elasticsearch.search.aggregations.support; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.joda.FormatDateTimeFormatter; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.IndexGeoPointFieldData; @@ -32,9 +31,9 @@ import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.SearchScript; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.aggregations.AggregationInitializationException; -import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; import org.elasticsearch.search.aggregations.AggregatorBuilder; import org.elasticsearch.search.aggregations.AggregatorFactories; +import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.InternalAggregation.Type; import org.elasticsearch.search.internal.SearchContext; @@ -57,6 +56,13 @@ public abstract class ValuesSourceAggregatorBuilder