Skip to content

Commit 302980e

Browse files
nik9000nik9000
authored
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`.
1 parent a5e82d7 commit 302980e

12 files changed

+31
-63
lines changed

‎server/src/main/java/org/elasticsearch/search/SearchModule.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -386,14 +386,14 @@ private void registerAggregations(List<SearchPlugin> plugins) {
386386
.addResultReader(InternalTDigestPercentileRanks.NAME, InternalTDigestPercentileRanks::new)
387387
.addResultReader(InternalHDRPercentileRanks.NAME, InternalHDRPercentileRanks::new));
388388
registerAggregation(new AggregationSpec(MedianAbsoluteDeviationAggregationBuilder.NAME,
389-
MedianAbsoluteDeviationAggregationBuilder::new, MedianAbsoluteDeviationAggregationBuilder::parse)
389+
MedianAbsoluteDeviationAggregationBuilder::new, MedianAbsoluteDeviationAggregationBuilder.PARSER)
390390
.addResultReader(InternalMedianAbsoluteDeviation::new));
391391
registerAggregation(new AggregationSpec(CardinalityAggregationBuilder.NAME, CardinalityAggregationBuilder::new,
392-
CardinalityAggregationBuilder::parse).addResultReader(InternalCardinality::new));
392+
CardinalityAggregationBuilder.PARSER).addResultReader(InternalCardinality::new));
393393
registerAggregation(new AggregationSpec(GlobalAggregationBuilder.NAME, GlobalAggregationBuilder::new,
394394
GlobalAggregationBuilder::parse).addResultReader(InternalGlobal::new));
395395
registerAggregation(new AggregationSpec(MissingAggregationBuilder.NAME, MissingAggregationBuilder::new,
396-
MissingAggregationBuilder::parse).addResultReader(InternalMissing::new));
396+
MissingAggregationBuilder.PARSER).addResultReader(InternalMissing::new));
397397
registerAggregation(new AggregationSpec(FilterAggregationBuilder.NAME, FilterAggregationBuilder::new,
398398
FilterAggregationBuilder::parse).addResultReader(InternalFilter::new));
399399
registerAggregation(new AggregationSpec(FiltersAggregationBuilder.NAME, FiltersAggregationBuilder::new,
@@ -405,16 +405,16 @@ private void registerAggregations(List<SearchPlugin> plugins) {
405405
.addResultReader(InternalSampler.NAME, InternalSampler::new)
406406
.addResultReader(UnmappedSampler.NAME, UnmappedSampler::new));
407407
registerAggregation(new AggregationSpec(DiversifiedAggregationBuilder.NAME, DiversifiedAggregationBuilder::new,
408-
DiversifiedAggregationBuilder::parse)
408+
DiversifiedAggregationBuilder.PARSER)
409409
/* Reuses result readers from SamplerAggregator*/);
410410
registerAggregation(new AggregationSpec(TermsAggregationBuilder.NAME, TermsAggregationBuilder::new,
411-
TermsAggregationBuilder::parse)
411+
TermsAggregationBuilder.PARSER)
412412
.addResultReader(StringTerms.NAME, StringTerms::new)
413413
.addResultReader(UnmappedTerms.NAME, UnmappedTerms::new)
414414
.addResultReader(LongTerms.NAME, LongTerms::new)
415415
.addResultReader(DoubleTerms.NAME, DoubleTerms::new));
416416
registerAggregation(new AggregationSpec(RareTermsAggregationBuilder.NAME, RareTermsAggregationBuilder::new,
417-
RareTermsAggregationBuilder::parse)
417+
RareTermsAggregationBuilder.PARSER)
418418
.addResultReader(StringRareTerms.NAME, StringRareTerms::new)
419419
.addResultReader(UnmappedRareTerms.NAME, UnmappedRareTerms::new)
420420
.addResultReader(LongRareTerms.NAME, LongRareTerms::new));

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregationBuilder.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,15 @@ public class AdjacencyMatrixAggregationBuilder extends AbstractAggregationBuilde
5757
private List<KeyedFilter> filters;
5858
private String separator = DEFAULT_SEPARATOR;
5959

60-
private static final ObjectParser<AdjacencyMatrixAggregationBuilder, Void> PARSER = new ObjectParser<>(
61-
AdjacencyMatrixAggregationBuilder.NAME);
60+
private static final ObjectParser<AdjacencyMatrixAggregationBuilder, String> PARSER =
61+
ObjectParser.fromBuilder(NAME, AdjacencyMatrixAggregationBuilder::new);
6262
static {
6363
PARSER.declareString(AdjacencyMatrixAggregationBuilder::separator, SEPARATOR_FIELD);
6464
PARSER.declareNamedObjects(AdjacencyMatrixAggregationBuilder::setFiltersAsList, KeyedFilter.PARSER, FILTERS_FIELD);
6565
}
6666

67-
public static AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
68-
AdjacencyMatrixAggregationBuilder result = PARSER.parse(parser, new AdjacencyMatrixAggregationBuilder(aggregationName), null);
67+
public static AggregationBuilder parse(XContentParser parser, String name) throws IOException {
68+
AdjacencyMatrixAggregationBuilder result = PARSER.parse(parser, name);
6969
result.checkConsistency();
7070
return result;
7171
}
@@ -76,7 +76,6 @@ protected void checkConsistency() {
7676
}
7777
}
7878

79-
8079
protected void setFiltersAsMap(Map<String, QueryBuilder> filters) {
8180
// Convert uniquely named objects into internal KeyedFilters
8281
this.filters = new ArrayList<>(filters.size());

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregator.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ protected static class KeyedFilter implements Writeable, ToXContentFragment {
6161
private final String key;
6262
private final QueryBuilder filter;
6363

64-
public static final NamedObjectParser<KeyedFilter, Void> PARSER =
65-
(XContentParser p, Void c, String name) ->
64+
public static final NamedObjectParser<KeyedFilter, String> PARSER =
65+
(XContentParser p, String aggName, String name) ->
6666
new KeyedFilter(name, parseInnerQueryBuilder(p));
6767

6868
public KeyedFilter(String key, QueryBuilder filter) {

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregationBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ protected XContentBuilder internalXContent(XContentBuilder builder, Params param
106106
return builder;
107107
}
108108

109-
public static FilterAggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
109+
public static FilterAggregationBuilder parse(XContentParser parser, String aggregationName) throws IOException {
110110
QueryBuilder filter = parseInnerQueryBuilder(parser);
111111
return new FilterAggregationBuilder(aggregationName, filter);
112112
}

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/global/GlobalAggregationBuilder.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@
3333
import java.util.Map;
3434

3535
public class GlobalAggregationBuilder extends AbstractAggregationBuilder<GlobalAggregationBuilder> {
36+
public static GlobalAggregationBuilder parse(XContentParser parser, String aggregationName) throws IOException {
37+
parser.nextToken();
38+
return new GlobalAggregationBuilder(aggregationName);
39+
}
40+
3641
public static final String NAME = "global";
3742

3843
public GlobalAggregationBuilder(String name) {
@@ -73,11 +78,6 @@ protected XContentBuilder internalXContent(XContentBuilder builder, Params param
7378
return builder;
7479
}
7580

76-
public static GlobalAggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
77-
parser.nextToken();
78-
return new GlobalAggregationBuilder(aggregationName);
79-
}
80-
8181
@Override
8282
public String getType() {
8383
return NAME;

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/missing/MissingAggregationBuilder.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.elasticsearch.common.io.stream.StreamOutput;
2525
import org.elasticsearch.common.xcontent.ObjectParser;
2626
import org.elasticsearch.common.xcontent.XContentBuilder;
27-
import org.elasticsearch.common.xcontent.XContentParser;
2827
import org.elasticsearch.index.query.QueryShardContext;
2928
import org.elasticsearch.search.aggregations.AggregationBuilder;
3029
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
@@ -43,16 +42,12 @@
4342
public class MissingAggregationBuilder extends ValuesSourceAggregationBuilder<ValuesSource, MissingAggregationBuilder> {
4443
public static final String NAME = "missing";
4544

46-
private static final ObjectParser<MissingAggregationBuilder, Void> PARSER;
45+
public static final ObjectParser<MissingAggregationBuilder, String> PARSER =
46+
ObjectParser.fromBuilder(NAME, name -> new MissingAggregationBuilder(name, null));
4747
static {
48-
PARSER = new ObjectParser<>(MissingAggregationBuilder.NAME);
4948
ValuesSourceParserHelper.declareAnyFields(PARSER, true, true);
5049
}
5150

52-
public static MissingAggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
53-
return PARSER.parse(parser, new MissingAggregationBuilder(aggregationName, null), null);
54-
}
55-
5651
public MissingAggregationBuilder(String name, ValueType targetValueType) {
5752
super(name, CoreValuesSourceType.ANY, targetValueType);
5853
}

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/sampler/DiversifiedAggregationBuilder.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.elasticsearch.common.io.stream.StreamOutput;
2424
import org.elasticsearch.common.xcontent.ObjectParser;
2525
import org.elasticsearch.common.xcontent.XContentBuilder;
26-
import org.elasticsearch.common.xcontent.XContentParser;
2726
import org.elasticsearch.index.query.QueryShardContext;
2827
import org.elasticsearch.search.aggregations.AggregationBuilder;
2928
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
@@ -44,19 +43,15 @@ public class DiversifiedAggregationBuilder extends ValuesSourceAggregationBuilde
4443

4544
public static final int MAX_DOCS_PER_VALUE_DEFAULT = 1;
4645

47-
private static final ObjectParser<DiversifiedAggregationBuilder, Void> PARSER;
46+
public static final ObjectParser<DiversifiedAggregationBuilder, String> PARSER =
47+
ObjectParser.fromBuilder(NAME, DiversifiedAggregationBuilder::new);
4848
static {
49-
PARSER = new ObjectParser<>(DiversifiedAggregationBuilder.NAME);
5049
ValuesSourceParserHelper.declareAnyFields(PARSER, true, false);
5150
PARSER.declareInt(DiversifiedAggregationBuilder::shardSize, SamplerAggregator.SHARD_SIZE_FIELD);
5251
PARSER.declareInt(DiversifiedAggregationBuilder::maxDocsPerValue, SamplerAggregator.MAX_DOCS_PER_VALUE_FIELD);
5352
PARSER.declareString(DiversifiedAggregationBuilder::executionHint, SamplerAggregator.EXECUTION_HINT_FIELD);
5453
}
5554

56-
public static AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
57-
return PARSER.parse(parser, new DiversifiedAggregationBuilder(aggregationName), null);
58-
}
59-
6055
private int shardSize = SamplerAggregationBuilder.DEFAULT_SHARD_SAMPLE_SIZE;
6156
private int maxDocsPerValue = MAX_DOCS_PER_VALUE_DEFAULT;
6257
private String executionHint = null;

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/RareTermsAggregationBuilder.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.elasticsearch.common.io.stream.StreamOutput;
2525
import org.elasticsearch.common.xcontent.ObjectParser;
2626
import org.elasticsearch.common.xcontent.XContentBuilder;
27-
import org.elasticsearch.common.xcontent.XContentParser;
2827
import org.elasticsearch.index.query.QueryShardContext;
2928
import org.elasticsearch.search.aggregations.AggregationBuilder;
3029
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
@@ -48,9 +47,9 @@ public class RareTermsAggregationBuilder extends ValuesSourceAggregationBuilder<
4847
private static final ParseField PRECISION = new ParseField("precision");
4948

5049
private static final int MAX_MAX_DOC_COUNT = 100;
51-
private static final ObjectParser<RareTermsAggregationBuilder, Void> PARSER;
50+
public static final ObjectParser<RareTermsAggregationBuilder, String> PARSER =
51+
ObjectParser.fromBuilder(NAME, name -> new RareTermsAggregationBuilder(name, null));
5252
static {
53-
PARSER = new ObjectParser<>(RareTermsAggregationBuilder.NAME);
5453
ValuesSourceParserHelper.declareAnyFields(PARSER, true, true);
5554
PARSER.declareLong(RareTermsAggregationBuilder::maxDocCount, MAX_DOC_COUNT_FIELD_NAME);
5655

@@ -63,10 +62,6 @@ public class RareTermsAggregationBuilder extends ValuesSourceAggregationBuilder<
6362
PARSER.declareDouble(RareTermsAggregationBuilder::setPrecision, PRECISION);
6463
}
6564

66-
public static AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
67-
return PARSER.parse(parser, new RareTermsAggregationBuilder(aggregationName, null), null);
68-
}
69-
7065
private IncludeExclude includeExclude = null;
7166
private int maxDocCount = 1;
7267
private double precision = 0.001;

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregationBuilder.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
2626
import org.elasticsearch.common.xcontent.ObjectParser;
2727
import org.elasticsearch.common.xcontent.XContentBuilder;
28-
import org.elasticsearch.common.xcontent.XContentParser;
2928
import org.elasticsearch.index.query.QueryRewriteContext;
3029
import org.elasticsearch.index.query.QueryShardContext;
3130
import org.elasticsearch.search.aggregations.AggregationBuilder;
@@ -65,9 +64,9 @@ public class TermsAggregationBuilder extends ValuesSourceAggregationBuilder<Valu
6564
public static final ParseField SHOW_TERM_DOC_COUNT_ERROR = new ParseField("show_term_doc_count_error");
6665
public static final ParseField ORDER_FIELD = new ParseField("order");
6766

68-
private static final ObjectParser<TermsAggregationBuilder, Void> PARSER;
67+
public static final ObjectParser<TermsAggregationBuilder, String> PARSER =
68+
ObjectParser.fromBuilder(NAME, name -> new TermsAggregationBuilder(name, null));
6969
static {
70-
PARSER = new ObjectParser<>(TermsAggregationBuilder.NAME);
7170
ValuesSourceParserHelper.declareAnyFields(PARSER, true, true);
7271

7372
PARSER.declareBoolean(TermsAggregationBuilder::showTermDocCountError,
@@ -97,10 +96,6 @@ public class TermsAggregationBuilder extends ValuesSourceAggregationBuilder<Valu
9796
IncludeExclude::parseExclude, IncludeExclude.EXCLUDE_FIELD, ObjectParser.ValueType.STRING_ARRAY);
9897
}
9998

100-
public static AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
101-
return PARSER.parse(parser, new TermsAggregationBuilder(aggregationName, null), null);
102-
}
103-
10499
private BucketOrder order = BucketOrder.compound(BucketOrder.count(false)); // automatically adds tie-breaker key asc order
105100
private IncludeExclude includeExclude = null;
106101
private String executionHint = null;

‎server/src/main/java/org/elasticsearch/search/aggregations/metrics/CardinalityAggregationBuilder.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.elasticsearch.common.io.stream.StreamOutput;
2626
import org.elasticsearch.common.xcontent.ObjectParser;
2727
import org.elasticsearch.common.xcontent.XContentBuilder;
28-
import org.elasticsearch.common.xcontent.XContentParser;
2928
import org.elasticsearch.index.query.QueryShardContext;
3029
import org.elasticsearch.search.aggregations.AggregationBuilder;
3130
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
@@ -49,18 +48,14 @@ public final class CardinalityAggregationBuilder
4948
private static final ParseField REHASH = new ParseField("rehash").withAllDeprecated("no replacement - values will always be rehashed");
5049
public static final ParseField PRECISION_THRESHOLD_FIELD = new ParseField("precision_threshold");
5150

52-
private static final ObjectParser<CardinalityAggregationBuilder, Void> PARSER;
51+
public static final ObjectParser<CardinalityAggregationBuilder, String> PARSER =
52+
ObjectParser.fromBuilder(NAME, name -> new CardinalityAggregationBuilder(name, null));
5353
static {
54-
PARSER = new ObjectParser<>(CardinalityAggregationBuilder.NAME);
5554
ValuesSourceParserHelper.declareAnyFields(PARSER, true, false);
5655
PARSER.declareLong(CardinalityAggregationBuilder::precisionThreshold, CardinalityAggregationBuilder.PRECISION_THRESHOLD_FIELD);
5756
PARSER.declareLong((b, v) -> {/*ignore*/}, REHASH);
5857
}
5958

60-
public static AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
61-
return PARSER.parse(parser, new CardinalityAggregationBuilder(aggregationName, null), null);
62-
}
63-
6459
private Long precisionThreshold = null;
6560

6661
public CardinalityAggregationBuilder(String name, ValueType targetValueType) {

‎server/src/main/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregationBuilder.java

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.elasticsearch.common.io.stream.StreamOutput;
2525
import org.elasticsearch.common.xcontent.ObjectParser;
2626
import org.elasticsearch.common.xcontent.XContentBuilder;
27-
import org.elasticsearch.common.xcontent.XContentParser;
2827
import org.elasticsearch.index.query.QueryShardContext;
2928
import org.elasticsearch.search.aggregations.AggregationBuilder;
3029
import org.elasticsearch.search.aggregations.AggregatorFactories;
@@ -47,18 +46,13 @@ public class MedianAbsoluteDeviationAggregationBuilder extends LeafOnly<ValuesSo
4746

4847
private static final ParseField COMPRESSION_FIELD = new ParseField("compression");
4948

50-
private static final ObjectParser<MedianAbsoluteDeviationAggregationBuilder, Void> PARSER;
51-
49+
public static final ObjectParser<MedianAbsoluteDeviationAggregationBuilder, String> PARSER =
50+
ObjectParser.fromBuilder(NAME, MedianAbsoluteDeviationAggregationBuilder::new);
5251
static {
53-
PARSER = new ObjectParser<>(NAME);
5452
ValuesSourceParserHelper.declareNumericFields(PARSER, true, true, false);
5553
PARSER.declareDouble(MedianAbsoluteDeviationAggregationBuilder::compression, COMPRESSION_FIELD);
5654
}
5755

58-
public static MedianAbsoluteDeviationAggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
59-
return PARSER.parse(parser, new MedianAbsoluteDeviationAggregationBuilder(aggregationName), null);
60-
}
61-
6256
private double compression = 1000d;
6357

6458
public MedianAbsoluteDeviationAggregationBuilder(String name) {

‎server/src/test/java/org/elasticsearch/search/SearchModuleTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ public List<SearchPlugin.QuerySpec<?>> getQueries() {
154154
@Override
155155
public List<AggregationSpec> getAggregations() {
156156
return singletonList(new AggregationSpec(TermsAggregationBuilder.NAME, TermsAggregationBuilder::new,
157-
TermsAggregationBuilder::parse));
157+
TermsAggregationBuilder.PARSER));
158158
}
159159
};
160160
expectThrows(IllegalArgumentException.class, registryForPlugin(registersDupeAggregation));

0 commit comments

Comments
 (0)