Skip to content

Commit

Permalink
Remove some ceremony in agg parsing (elastic#53078)
Browse files Browse the repository at this point in the history 
With elastic#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`.
  • Loading branch information
@nik9000
nik9000 committed Mar 4, 2020
1 parent 892f0d5 commit 87163a2
Show file tree
Hide file tree
Showing 12 changed files with 31 additions and 63 deletions.
12 changes: 6 additions & 6 deletions server/src/main/java/org/elasticsearch/search/SearchModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,14 +386,14 @@ private void registerAggregations(List<SearchPlugin> plugins) {
.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,
Expand All @@ -405,16 +405,16 @@ private void registerAggregations(List<SearchPlugin> plugins) {
.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));
Expand Down
9 changes: 4 additions & 5 deletions ...elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregationBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,15 @@ public class AdjacencyMatrixAggregationBuilder extends AbstractAggregationBuilde
private List<KeyedFilter> filters;
private String separator = DEFAULT_SEPARATOR;

private static final ObjectParser<AdjacencyMatrixAggregationBuilder, Void> PARSER = new ObjectParser<>(
AdjacencyMatrixAggregationBuilder.NAME);
private static final ObjectParser<AdjacencyMatrixAggregationBuilder, String> 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;
}
Expand All @@ -76,7 +76,6 @@ protected void checkConsistency() {
}
}


protected void setFiltersAsMap(Map<String, QueryBuilder> filters) {
// Convert uniquely named objects into internal KeyedFilters
this.filters = new ArrayList<>(filters.size());
Expand Down
4 changes: 2 additions & 2 deletions ...ava/org/elasticsearch/search/aggregations/bucket/adjacency/AdjacencyMatrixAggregator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ protected static class KeyedFilter implements Writeable, ToXContentFragment {
private final String key;
private final QueryBuilder filter;

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

public KeyedFilter(String key, QueryBuilder filter) {
Expand Down
2 changes: 1 addition & 1 deletion ...in/java/org/elasticsearch/search/aggregations/bucket/filter/FilterAggregationBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ protected XContentBuilder internalXContent(XContentBuilder builder, Params param
return builder;
}

public static FilterAggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
public static FilterAggregationBuilder parse(XContentParser parser, String aggregationName) throws IOException {
QueryBuilder filter = parseInnerQueryBuilder(parser);
return new FilterAggregationBuilder(aggregationName, filter);
}
Expand Down
10 changes: 5 additions & 5 deletions ...in/java/org/elasticsearch/search/aggregations/bucket/global/GlobalAggregationBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@
import java.util.Map;

public class GlobalAggregationBuilder extends AbstractAggregationBuilder<GlobalAggregationBuilder> {
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) {
Expand Down Expand Up @@ -73,11 +78,6 @@ protected XContentBuilder internalXContent(XContentBuilder builder, Params param
return builder;
}

public static GlobalAggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
parser.nextToken();
return new GlobalAggregationBuilder(aggregationName);
}

@Override
public String getType() {
return NAME;
Expand Down
9 changes: 2 additions & 7 deletions .../java/org/elasticsearch/search/aggregations/bucket/missing/MissingAggregationBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
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;
Expand All @@ -43,16 +42,12 @@
public class MissingAggregationBuilder extends ValuesSourceAggregationBuilder<ValuesSource, MissingAggregationBuilder> {
public static final String NAME = "missing";

private static final ObjectParser<MissingAggregationBuilder, Void> PARSER;
public static final ObjectParser<MissingAggregationBuilder, String> 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);
}
Expand Down
9 changes: 2 additions & 7 deletions ...a/org/elasticsearch/search/aggregations/bucket/sampler/DiversifiedAggregationBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
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;
Expand All @@ -44,19 +43,15 @@ public class DiversifiedAggregationBuilder extends ValuesSourceAggregationBuilde

public static final int MAX_DOCS_PER_VALUE_DEFAULT = 1;

private static final ObjectParser<DiversifiedAggregationBuilder, Void> PARSER;
public static final ObjectParser<DiversifiedAggregationBuilder, String> 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;
Expand Down
9 changes: 2 additions & 7 deletions .../java/org/elasticsearch/search/aggregations/bucket/terms/RareTermsAggregationBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
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;
Expand All @@ -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<RareTermsAggregationBuilder, Void> PARSER;
public static final ObjectParser<RareTermsAggregationBuilder, String> 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);

Expand All @@ -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;
Expand Down
9 changes: 2 additions & 7 deletions ...main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregationBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
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;
Expand Down Expand Up @@ -65,9 +64,9 @@ public class TermsAggregationBuilder extends ValuesSourceAggregationBuilder<Valu
public static final ParseField SHOW_TERM_DOC_COUNT_ERROR = new ParseField("show_term_doc_count_error");
public static final ParseField ORDER_FIELD = new ParseField("order");

private static final ObjectParser<TermsAggregationBuilder, Void> PARSER;
public static final ObjectParser<TermsAggregationBuilder, String> PARSER =
ObjectParser.fromBuilder(NAME, name -> new TermsAggregationBuilder(name, null));
static {
PARSER = new ObjectParser<>(TermsAggregationBuilder.NAME);
ValuesSourceParserHelper.declareAnyFields(PARSER, true, true);

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

public static AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
return PARSER.parse(parser, new TermsAggregationBuilder(aggregationName, null), null);
}

private BucketOrder order = BucketOrder.compound(BucketOrder.count(false)); // automatically adds tie-breaker key asc order
private IncludeExclude includeExclude = null;
private String executionHint = null;
Expand Down
9 changes: 2 additions & 7 deletions ...ain/java/org/elasticsearch/search/aggregations/metrics/CardinalityAggregationBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
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;
Expand All @@ -49,18 +48,14 @@ public final class CardinalityAggregationBuilder
private static final ParseField REHASH = new ParseField("rehash").withAllDeprecated("no replacement - values will always be rehashed");
public static final ParseField PRECISION_THRESHOLD_FIELD = new ParseField("precision_threshold");

private static final ObjectParser<CardinalityAggregationBuilder, Void> PARSER;
public static final ObjectParser<CardinalityAggregationBuilder, String> 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) {
Expand Down
10 changes: 2 additions & 8 deletions .../elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregationBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
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;
Expand All @@ -47,18 +46,13 @@ public class MedianAbsoluteDeviationAggregationBuilder extends LeafOnly<ValuesSo

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

private static final ObjectParser<MedianAbsoluteDeviationAggregationBuilder, Void> PARSER;

public static final ObjectParser<MedianAbsoluteDeviationAggregationBuilder, String> 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) {
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/search/SearchModuleTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public List<SearchPlugin.QuerySpec<?>> getQueries() {
@Override
public List<AggregationSpec> getAggregations() {
return singletonList(new AggregationSpec(TermsAggregationBuilder.NAME, TermsAggregationBuilder::new,
TermsAggregationBuilder::parse));
TermsAggregationBuilder.PARSER));
}
};
expectThrows(IllegalArgumentException.class, registryForPlugin(registersDupeAggregation));
Expand Down

0 comments on commit 87163a2

Please sign in to comment.