From 302980e0c47366bb02705465e047b2c2d0e20d5c Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 4 Mar 2020 13:06:41 -0500 Subject: [PATCH] Remove some ceremony in agg parsing (#53078) (#53117) With #50871 aggrgations should now be parsed directly by an `ObjectParser` or `ConstructingObjectParser` without the need for the ceremonial `parse` method. This removes 9 of those `parse` methods and parses the aggregation directly from their `ObjectParser`. --- .../java/org/elasticsearch/search/SearchModule.java | 12 ++++++------ .../adjacency/AdjacencyMatrixAggregationBuilder.java | 9 ++++----- .../bucket/adjacency/AdjacencyMatrixAggregator.java | 4 ++-- .../bucket/filter/FilterAggregationBuilder.java | 2 +- .../bucket/global/GlobalAggregationBuilder.java | 10 +++++----- .../bucket/missing/MissingAggregationBuilder.java | 9 ++------- .../sampler/DiversifiedAggregationBuilder.java | 9 ++------- .../bucket/terms/RareTermsAggregationBuilder.java | 9 ++------- .../bucket/terms/TermsAggregationBuilder.java | 9 ++------- .../metrics/CardinalityAggregationBuilder.java | 9 ++------- .../MedianAbsoluteDeviationAggregationBuilder.java | 10 ++-------- .../org/elasticsearch/search/SearchModuleTests.java | 2 +- 12 files changed, 31 insertions(+), 63 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index e3cba4b1ed0..f76badc4800 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -386,14 +386,14 @@ public class SearchModule { .addResultReader(InternalTDigestPercentileRanks.NAME, InternalTDigestPercentileRanks::new) .addResultReader(InternalHDRPercentileRanks.NAME, InternalHDRPercentileRanks::new)); registerAggregation(new AggregationSpec(MedianAbsoluteDeviationAggregationBuilder.NAME, - MedianAbsoluteDeviationAggregationBuilder::new, MedianAbsoluteDeviationAggregationBuilder::parse) + MedianAbsoluteDeviationAggregationBuilder::new, MedianAbsoluteDeviationAggregationBuilder.PARSER) .addResultReader(InternalMedianAbsoluteDeviation::new)); registerAggregation(new AggregationSpec(CardinalityAggregationBuilder.NAME, CardinalityAggregationBuilder::new, - CardinalityAggregationBuilder::parse).addResultReader(InternalCardinality::new)); + CardinalityAggregationBuilder.PARSER).addResultReader(InternalCardinality::new)); registerAggregation(new AggregationSpec(GlobalAggregationBuilder.NAME, GlobalAggregationBuilder::new, GlobalAggregationBuilder::parse).addResultReader(InternalGlobal::new)); registerAggregation(new AggregationSpec(MissingAggregationBuilder.NAME, MissingAggregationBuilder::new, - MissingAggregationBuilder::parse).addResultReader(InternalMissing::new)); + MissingAggregationBuilder.PARSER).addResultReader(InternalMissing::new)); registerAggregation(new AggregationSpec(FilterAggregationBuilder.NAME, FilterAggregationBuilder::new, FilterAggregationBuilder::parse).addResultReader(InternalFilter::new)); registerAggregation(new AggregationSpec(FiltersAggregationBuilder.NAME, FiltersAggregationBuilder::new, @@ -405,16 +405,16 @@ public class SearchModule { .addResultReader(InternalSampler.NAME, InternalSampler::new) .addResultReader(UnmappedSampler.NAME, UnmappedSampler::new)); registerAggregation(new AggregationSpec(DiversifiedAggregationBuilder.NAME, DiversifiedAggregationBuilder::new, - DiversifiedAggregationBuilder::parse) + DiversifiedAggregationBuilder.PARSER) /* Reuses result readers from SamplerAggregator*/); registerAggregation(new AggregationSpec(TermsAggregationBuilder.NAME, TermsAggregationBuilder::new, - TermsAggregationBuilder::parse) + TermsAggregationBuilder.PARSER) .addResultReader(StringTerms.NAME, StringTerms::new) .addResultReader(UnmappedTerms.NAME, UnmappedTerms::new) .addResultReader(LongTerms.NAME, LongTerms::new) .addResultReader(DoubleTerms.NAME, DoubleTerms::new)); registerAggregation(new AggregationSpec(RareTermsAggregationBuilder.NAME, RareTermsAggregationBuilder::new, - RareTermsAggregationBuilder::parse) + RareTermsAggregationBuilder.PARSER) .addResultReader(StringRareTerms.NAME, StringRareTerms::new) .addResultReader(UnmappedRareTerms.NAME, UnmappedRareTerms::new) .addResultReader(LongRareTerms.NAME, LongRareTerms::new)); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregationBuilder.java index 7f861b13d94..9e112a079aa 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregationBuilder.java @@ -57,15 +57,15 @@ public class AdjacencyMatrixAggregationBuilder extends AbstractAggregationBuilde private List filters; private String separator = DEFAULT_SEPARATOR; - private static final ObjectParser PARSER = new ObjectParser<>( - AdjacencyMatrixAggregationBuilder.NAME); + private static final ObjectParser PARSER = + ObjectParser.fromBuilder(NAME, AdjacencyMatrixAggregationBuilder::new); static { PARSER.declareString(AdjacencyMatrixAggregationBuilder::separator, SEPARATOR_FIELD); PARSER.declareNamedObjects(AdjacencyMatrixAggregationBuilder::setFiltersAsList, KeyedFilter.PARSER, FILTERS_FIELD); } - public static AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException { - AdjacencyMatrixAggregationBuilder result = PARSER.parse(parser, new AdjacencyMatrixAggregationBuilder(aggregationName), null); + public static AggregationBuilder parse(XContentParser parser, String name) throws IOException { + AdjacencyMatrixAggregationBuilder result = PARSER.parse(parser, name); result.checkConsistency(); return result; } @@ -76,7 +76,6 @@ public class AdjacencyMatrixAggregationBuilder extends AbstractAggregationBuilde } } - protected void setFiltersAsMap(Map filters) { // Convert uniquely named objects into internal KeyedFilters this.filters = new ArrayList<>(filters.size()); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregator.java index aa36e3a26fb..bea1e4af838 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregator.java @@ -61,8 +61,8 @@ public class AdjacencyMatrixAggregator extends BucketsAggregator { private final String key; private final QueryBuilder filter; - public static final NamedObjectParser PARSER = - (XContentParser p, Void c, String name) -> + public static final NamedObjectParser PARSER = + (XContentParser p, String aggName, String name) -> new KeyedFilter(name, parseInnerQueryBuilder(p)); public KeyedFilter(String key, QueryBuilder filter) { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregationBuilder.java index 151b86f51cc..9f3d4b5a2ba 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregationBuilder.java @@ -106,7 +106,7 @@ public class FilterAggregationBuilder extends AbstractAggregationBuilder { + public static GlobalAggregationBuilder parse(XContentParser parser, String aggregationName) throws IOException { + parser.nextToken(); + return new GlobalAggregationBuilder(aggregationName); + } + public static final String NAME = "global"; public GlobalAggregationBuilder(String name) { @@ -73,11 +78,6 @@ public class GlobalAggregationBuilder extends AbstractAggregationBuilder { public static final String NAME = "missing"; - private static final ObjectParser PARSER; + public static final ObjectParser PARSER = + ObjectParser.fromBuilder(NAME, name -> new MissingAggregationBuilder(name, null)); static { - PARSER = new ObjectParser<>(MissingAggregationBuilder.NAME); ValuesSourceParserHelper.declareAnyFields(PARSER, true, true); } - public static MissingAggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException { - return PARSER.parse(parser, new MissingAggregationBuilder(aggregationName, null), null); - } - public MissingAggregationBuilder(String name, ValueType targetValueType) { super(name, CoreValuesSourceType.ANY, targetValueType); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/sampler/DiversifiedAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/sampler/DiversifiedAggregationBuilder.java index e7781b5586b..62bfa453a54 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/sampler/DiversifiedAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/sampler/DiversifiedAggregationBuilder.java @@ -23,7 +23,6 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; @@ -44,19 +43,15 @@ public class DiversifiedAggregationBuilder extends ValuesSourceAggregationBuilde public static final int MAX_DOCS_PER_VALUE_DEFAULT = 1; - private static final ObjectParser PARSER; + public static final ObjectParser PARSER = + ObjectParser.fromBuilder(NAME, DiversifiedAggregationBuilder::new); static { - PARSER = new ObjectParser<>(DiversifiedAggregationBuilder.NAME); ValuesSourceParserHelper.declareAnyFields(PARSER, true, false); PARSER.declareInt(DiversifiedAggregationBuilder::shardSize, SamplerAggregator.SHARD_SIZE_FIELD); PARSER.declareInt(DiversifiedAggregationBuilder::maxDocsPerValue, SamplerAggregator.MAX_DOCS_PER_VALUE_FIELD); PARSER.declareString(DiversifiedAggregationBuilder::executionHint, SamplerAggregator.EXECUTION_HINT_FIELD); } - public static AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException { - return PARSER.parse(parser, new DiversifiedAggregationBuilder(aggregationName), null); - } - private int shardSize = SamplerAggregationBuilder.DEFAULT_SHARD_SAMPLE_SIZE; private int maxDocsPerValue = MAX_DOCS_PER_VALUE_DEFAULT; private String executionHint = null; diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/RareTermsAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/RareTermsAggregationBuilder.java index d0f6b18d1a5..d4a9dda43c2 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/RareTermsAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/RareTermsAggregationBuilder.java @@ -24,7 +24,6 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; @@ -48,9 +47,9 @@ public class RareTermsAggregationBuilder extends ValuesSourceAggregationBuilder< private static final ParseField PRECISION = new ParseField("precision"); private static final int MAX_MAX_DOC_COUNT = 100; - private static final ObjectParser PARSER; + public static final ObjectParser PARSER = + ObjectParser.fromBuilder(NAME, name -> new RareTermsAggregationBuilder(name, null)); static { - PARSER = new ObjectParser<>(RareTermsAggregationBuilder.NAME); ValuesSourceParserHelper.declareAnyFields(PARSER, true, true); PARSER.declareLong(RareTermsAggregationBuilder::maxDocCount, MAX_DOC_COUNT_FIELD_NAME); @@ -63,10 +62,6 @@ public class RareTermsAggregationBuilder extends ValuesSourceAggregationBuilder< PARSER.declareDouble(RareTermsAggregationBuilder::setPrecision, PRECISION); } - public static AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException { - return PARSER.parse(parser, new RareTermsAggregationBuilder(aggregationName, null), null); - } - private IncludeExclude includeExclude = null; private int maxDocCount = 1; private double precision = 0.001; diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregationBuilder.java index b7a01099597..f57785404f1 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregationBuilder.java @@ -25,7 +25,6 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.search.aggregations.AggregationBuilder; @@ -65,9 +64,9 @@ public class TermsAggregationBuilder extends ValuesSourceAggregationBuilder PARSER; + public static final ObjectParser PARSER = + ObjectParser.fromBuilder(NAME, name -> new TermsAggregationBuilder(name, null)); static { - PARSER = new ObjectParser<>(TermsAggregationBuilder.NAME); ValuesSourceParserHelper.declareAnyFields(PARSER, true, true); PARSER.declareBoolean(TermsAggregationBuilder::showTermDocCountError, @@ -97,10 +96,6 @@ public class TermsAggregationBuilder extends ValuesSourceAggregationBuilder PARSER; + public static final ObjectParser PARSER = + ObjectParser.fromBuilder(NAME, name -> new CardinalityAggregationBuilder(name, null)); static { - PARSER = new ObjectParser<>(CardinalityAggregationBuilder.NAME); ValuesSourceParserHelper.declareAnyFields(PARSER, true, false); PARSER.declareLong(CardinalityAggregationBuilder::precisionThreshold, CardinalityAggregationBuilder.PRECISION_THRESHOLD_FIELD); PARSER.declareLong((b, v) -> {/*ignore*/}, REHASH); } - public static AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException { - return PARSER.parse(parser, new CardinalityAggregationBuilder(aggregationName, null), null); - } - private Long precisionThreshold = null; public CardinalityAggregationBuilder(String name, ValueType targetValueType) { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregationBuilder.java index e02e382fa5d..3f35d4bf5a4 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregationBuilder.java @@ -24,7 +24,6 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.AggregatorFactories; @@ -47,18 +46,13 @@ public class MedianAbsoluteDeviationAggregationBuilder extends LeafOnly PARSER; - + public static final ObjectParser PARSER = + ObjectParser.fromBuilder(NAME, MedianAbsoluteDeviationAggregationBuilder::new); static { - PARSER = new ObjectParser<>(NAME); ValuesSourceParserHelper.declareNumericFields(PARSER, true, true, false); PARSER.declareDouble(MedianAbsoluteDeviationAggregationBuilder::compression, COMPRESSION_FIELD); } - public static MedianAbsoluteDeviationAggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException { - return PARSER.parse(parser, new MedianAbsoluteDeviationAggregationBuilder(aggregationName), null); - } - private double compression = 1000d; public MedianAbsoluteDeviationAggregationBuilder(String name) { diff --git a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java index d2a8b9dd533..8582fc64fac 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -154,7 +154,7 @@ public class SearchModuleTests extends ESTestCase { @Override public List getAggregations() { return singletonList(new AggregationSpec(TermsAggregationBuilder.NAME, TermsAggregationBuilder::new, - TermsAggregationBuilder::parse)); + TermsAggregationBuilder.PARSER)); } }; expectThrows(IllegalArgumentException.class, registryForPlugin(registersDupeAggregation));