Skip to content

Commit 191a711

Browse files
nik9000nik9000
authored
Drop "funny" functions building parsers (#50715)
Replaces the "funny" `Function<String, ConstructingObjectParser<T, Void>>` with a much simpler `ConstructingObjectParser<T, String>`. This makes pretty much all of our object parsers static.
1 parent dde2382 commit 191a711

File tree

12 files changed

+88
-123
lines changed

12 files changed

+88
-123
lines changed

‎client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupCapsResponse.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public static GetRollupCapsResponse fromXContent(final XContentParser parser) th
4646
if (token.equals(XContentParser.Token.FIELD_NAME)) {
4747
String pattern = parser.currentName();
4848

49-
RollableIndexCaps cap = RollableIndexCaps.PARSER.apply(pattern).apply(parser, null);
49+
RollableIndexCaps cap = RollableIndexCaps.PARSER.parse(parser, pattern);
5050
jobs.put(pattern, cap);
5151
}
5252
}

‎client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupIndexCapsResponse.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public static GetRollupIndexCapsResponse fromXContent(final XContentParser parse
4646
if (token.equals(XContentParser.Token.FIELD_NAME)) {
4747
String pattern = parser.currentName();
4848

49-
RollableIndexCaps cap = RollableIndexCaps.PARSER.apply(pattern).apply(parser, null);
49+
RollableIndexCaps cap = RollableIndexCaps.PARSER.apply(parser, pattern);
5050
jobs.put(pattern, cap);
5151
}
5252
}

‎client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/RollableIndexCaps.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,10 @@
2828
import java.util.Comparator;
2929
import java.util.List;
3030
import java.util.Objects;
31-
import java.util.function.Function;
3231
import java.util.stream.Collectors;
3332

33+
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
34+
3435
/**
3536
* Represents the rollup capabilities of a non-rollup index. E.g. what values/aggregations
3637
* were rolled up for this index, in what rollup jobs that data is stored and where those
@@ -41,16 +42,15 @@
4142
public class RollableIndexCaps implements ToXContentFragment {
4243
private static final ParseField ROLLUP_JOBS = new ParseField("rollup_jobs");
4344

44-
public static final Function<String, ConstructingObjectParser<RollableIndexCaps, Void>> PARSER = indexName -> {
45-
@SuppressWarnings("unchecked")
46-
ConstructingObjectParser<RollableIndexCaps, Void> p
47-
= new ConstructingObjectParser<>(indexName, true,
48-
a -> new RollableIndexCaps(indexName, (List<RollupJobCaps>) a[0]));
49-
50-
p.declareObjectArray(ConstructingObjectParser.constructorArg(), RollupJobCaps.PARSER::apply,
51-
ROLLUP_JOBS);
52-
return p;
53-
};
45+
public static final ConstructingObjectParser<RollableIndexCaps, String> PARSER = new ConstructingObjectParser<>(
46+
ROLLUP_JOBS.getPreferredName(), true, (Object[] args, String indexName) -> {
47+
@SuppressWarnings("unchecked")
48+
var caps = (List<RollupJobCaps>) args[0];
49+
return new RollableIndexCaps(indexName, caps);
50+
});
51+
static {
52+
PARSER.declareObjectArray(constructorArg(), (p, name) -> RollupJobCaps.PARSER.parse(p, null), ROLLUP_JOBS);
53+
}
5454

5555
private final String indexName;
5656
private final List<RollupJobCaps> jobCaps;

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ private void registerAggregations(List<SearchPlugin> plugins) {
424424
registerAggregation(new AggregationSpec(ScriptedMetricAggregationBuilder.NAME, ScriptedMetricAggregationBuilder::new,
425425
(name, p) -> ScriptedMetricAggregationBuilder.PARSER.parse(p, name)).addResultReader(InternalScriptedMetric::new));
426426
registerAggregation((new AggregationSpec(CompositeAggregationBuilder.NAME, CompositeAggregationBuilder::new,
427-
CompositeAggregationBuilder::parse).addResultReader(InternalComposite::new)));
427+
(name, p) -> CompositeAggregationBuilder.PARSER.parse(p, name)).addResultReader(InternalComposite::new)));
428428
registerFromPlugin(plugins, SearchPlugin::getAggregations, this::registerAggregation);
429429
}
430430

@@ -502,7 +502,7 @@ private void registerPipelineAggregations(List<SearchPlugin> plugins) {
502502
BucketScriptPipelineAggregationBuilder.NAME,
503503
BucketScriptPipelineAggregationBuilder::new,
504504
BucketScriptPipelineAggregator::new,
505-
BucketScriptPipelineAggregationBuilder::parse));
505+
(name, p) -> BucketScriptPipelineAggregationBuilder.PARSER.parse(p, name)));
506506
registerPipelineAggregation(new PipelineAggregationSpec(
507507
BucketSelectorPipelineAggregationBuilder.NAME,
508508
BucketSelectorPipelineAggregationBuilder::new,
@@ -519,10 +519,10 @@ private void registerPipelineAggregations(List<SearchPlugin> plugins) {
519519
SerialDiffPipelineAggregator::new,
520520
SerialDiffPipelineAggregationBuilder::parse));
521521
registerPipelineAggregation(new PipelineAggregationSpec(
522-
MovFnPipelineAggregationBuilder.NAME,
523-
MovFnPipelineAggregationBuilder::new,
524-
MovFnPipelineAggregator::new,
525-
MovFnPipelineAggregationBuilder::parse));
522+
MovFnPipelineAggregationBuilder.NAME,
523+
MovFnPipelineAggregationBuilder::new,
524+
MovFnPipelineAggregator::new,
525+
(name, p) -> MovFnPipelineAggregationBuilder.PARSER.parse(p, name)));
526526

527527
registerFromPlugin(plugins, SearchPlugin::getPipelineAggregations, this::registerPipelineAggregation);
528528
}

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilder.java

Lines changed: 12 additions & 22 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.ConstructingObjectParser;
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.AbstractAggregationBuilder;
3029
import org.elasticsearch.search.aggregations.AggregationBuilder;
@@ -39,7 +38,8 @@
3938
import java.util.Map;
4039
import java.util.Objects;
4140
import java.util.Set;
42-
import java.util.function.Function;
41+
42+
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
4343

4444
public class CompositeAggregationBuilder extends AbstractAggregationBuilder<CompositeAggregationBuilder> {
4545
public static final String NAME = "composite";
@@ -48,27 +48,17 @@ public class CompositeAggregationBuilder extends AbstractAggregationBuilder<Comp
4848
public static final ParseField SIZE_FIELD_NAME = new ParseField("size");
4949
public static final ParseField SOURCES_FIELD_NAME = new ParseField("sources");
5050

51-
private static final Function<String, ConstructingObjectParser<CompositeAggregationBuilder, Void>> PARSER = name -> {
52-
@SuppressWarnings("unchecked")
53-
ConstructingObjectParser<CompositeAggregationBuilder, Void> parser = new ConstructingObjectParser<>(NAME, a -> {
54-
CompositeAggregationBuilder builder = new CompositeAggregationBuilder(name, (List<CompositeValuesSourceBuilder<?>>)a[0]);
55-
if (a[1] != null) {
56-
builder.size((Integer)a[1]);
57-
}
58-
if (a[2] != null) {
59-
builder.aggregateAfter((Map<String, Object>)a[2]);
60-
}
61-
return builder;
62-
});
63-
parser.declareObjectArray(ConstructingObjectParser.constructorArg(),
51+
public static final ConstructingObjectParser<CompositeAggregationBuilder, String> PARSER = new ConstructingObjectParser<>(
52+
NAME, false, (args, name) -> {
53+
@SuppressWarnings("unchecked")
54+
List<CompositeValuesSourceBuilder<?>> sources = (List<CompositeValuesSourceBuilder<?>>) args[0];
55+
return new CompositeAggregationBuilder(name, sources);
56+
});
57+
static {
58+
PARSER.declareObjectArray(constructorArg(),
6459
(p, c) -> CompositeValuesSourceParserHelper.fromXContent(p), SOURCES_FIELD_NAME);
65-
parser.declareInt(ConstructingObjectParser.optionalConstructorArg(), SIZE_FIELD_NAME);
66-
parser.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, context) -> p.map(), AFTER_FIELD_NAME);
67-
return parser;
68-
};
69-
70-
public static CompositeAggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
71-
return PARSER.apply(aggregationName).parse(parser, null);
60+
PARSER.declareInt(CompositeAggregationBuilder::size, SIZE_FIELD_NAME);
61+
PARSER.declareObject(CompositeAggregationBuilder::aggregateAfter, (p, context) -> p.map(), AFTER_FIELD_NAME);
7262
}
7363

7464
private List<CompositeValuesSourceBuilder<?>> sources;

‎server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketScriptPipelineAggregationBuilder.java

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@
3737
import java.util.Map.Entry;
3838
import java.util.Objects;
3939
import java.util.TreeMap;
40-
import java.util.function.Function;
4140

41+
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
4242
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.BUCKETS_PATH;
4343
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.FORMAT;
4444
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.GAP_POLICY;
@@ -51,31 +51,24 @@ public class BucketScriptPipelineAggregationBuilder extends AbstractPipelineAggr
5151
private String format = null;
5252
private GapPolicy gapPolicy = GapPolicy.SKIP;
5353

54-
private static final Function<String, ConstructingObjectParser<BucketScriptPipelineAggregationBuilder, Void>> PARSER
55-
= name -> {
56-
57-
@SuppressWarnings("unchecked")
58-
ConstructingObjectParser<BucketScriptPipelineAggregationBuilder, Void> parser = new ConstructingObjectParser<>(
59-
BucketScriptPipelineAggregationBuilder.NAME,
60-
false,
61-
o -> new BucketScriptPipelineAggregationBuilder(name, (Map<String, String>) o[0], (Script) o[1]));
62-
63-
parser.declareField(ConstructingObjectParser.constructorArg()
64-
, BucketScriptPipelineAggregationBuilder::extractBucketPath
65-
, BUCKETS_PATH_FIELD
66-
, ObjectParser.ValueType.OBJECT_ARRAY_OR_STRING);
67-
parser.declareField(ConstructingObjectParser.constructorArg(),
68-
(p, c) -> Script.parse(p), Script.SCRIPT_PARSE_FIELD, ObjectParser.ValueType.OBJECT_OR_STRING);
69-
70-
parser.declareString(BucketScriptPipelineAggregationBuilder::format, FORMAT);
71-
parser.declareField(BucketScriptPipelineAggregationBuilder::gapPolicy, p -> {
54+
public static final ConstructingObjectParser<BucketScriptPipelineAggregationBuilder, String> PARSER = new ConstructingObjectParser<>(
55+
NAME, false, (args, name) -> {
56+
@SuppressWarnings("unchecked")
57+
var bucketsPathsMap = (Map<String, String>) args[0];
58+
return new BucketScriptPipelineAggregationBuilder(name, bucketsPathsMap, (Script) args[1]);
59+
});
60+
static {
61+
PARSER.declareField(constructorArg(), BucketScriptPipelineAggregationBuilder::extractBucketPath,
62+
BUCKETS_PATH_FIELD, ObjectParser.ValueType.OBJECT_ARRAY_OR_STRING);
63+
Script.declareScript(PARSER, constructorArg());
64+
65+
PARSER.declareString(BucketScriptPipelineAggregationBuilder::format, FORMAT);
66+
PARSER.declareField(BucketScriptPipelineAggregationBuilder::gapPolicy, p -> {
7267
if (p.currentToken() == XContentParser.Token.VALUE_STRING) {
7368
return GapPolicy.parse(p.text().toLowerCase(Locale.ROOT), p.getTokenLocation());
7469
}
7570
throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]");
7671
}, GAP_POLICY, ObjectParser.ValueType.STRING);
77-
78-
return parser;
7972
};
8073

8174

@@ -205,10 +198,6 @@ protected XContentBuilder internalXContent(XContentBuilder builder, Params param
205198
return builder;
206199
}
207200

208-
public static BucketScriptPipelineAggregationBuilder parse(String aggName, XContentParser parser) {
209-
return PARSER.apply(aggName).apply(parser, null);
210-
}
211-
212201
@Override
213202
protected boolean overrideBucketsPath() {
214203
return true;

‎server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MovFnPipelineAggregationBuilder.java

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@
4040
import java.util.Locale;
4141
import java.util.Map;
4242
import java.util.Objects;
43-
import java.util.function.Function;
4443

44+
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
4545
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.BUCKETS_PATH;
4646
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.FORMAT;
4747
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.Parser.GAP_POLICY;
@@ -58,29 +58,23 @@ public class MovFnPipelineAggregationBuilder extends AbstractPipelineAggregation
5858
private int window;
5959
private int shift;
6060

61-
private static final Function<String, ConstructingObjectParser<MovFnPipelineAggregationBuilder, Void>> PARSER
62-
= name -> {
63-
64-
ConstructingObjectParser<MovFnPipelineAggregationBuilder, Void> parser = new ConstructingObjectParser<>(
65-
MovFnPipelineAggregationBuilder.NAME,
66-
false,
67-
o -> new MovFnPipelineAggregationBuilder(name, (String) o[0], (Script) o[1], (int)o[2]));
68-
69-
parser.declareString(ConstructingObjectParser.constructorArg(), BUCKETS_PATH_FIELD);
70-
parser.declareField(ConstructingObjectParser.constructorArg(),
61+
public static final ConstructingObjectParser<MovFnPipelineAggregationBuilder, String> PARSER = new ConstructingObjectParser<>(
62+
NAME, false,
63+
(args, name) -> new MovFnPipelineAggregationBuilder(name, (String) args[0], (Script) args[1], (int)args[2]));
64+
static {
65+
PARSER.declareString(constructorArg(), BUCKETS_PATH_FIELD);
66+
PARSER.declareField(constructorArg(),
7167
(p, c) -> Script.parse(p), Script.SCRIPT_PARSE_FIELD, ObjectParser.ValueType.OBJECT_OR_STRING);
72-
parser.declareInt(ConstructingObjectParser.constructorArg(), WINDOW);
68+
PARSER.declareInt(constructorArg(), WINDOW);
7369

74-
parser.declareInt(MovFnPipelineAggregationBuilder::setShift, SHIFT);
75-
parser.declareString(MovFnPipelineAggregationBuilder::format, FORMAT);
76-
parser.declareField(MovFnPipelineAggregationBuilder::gapPolicy, p -> {
70+
PARSER.declareInt(MovFnPipelineAggregationBuilder::setShift, SHIFT);
71+
PARSER.declareString(MovFnPipelineAggregationBuilder::format, FORMAT);
72+
PARSER.declareField(MovFnPipelineAggregationBuilder::gapPolicy, p -> {
7773
if (p.currentToken() == XContentParser.Token.VALUE_STRING) {
7874
return GapPolicy.parse(p.text().toLowerCase(Locale.ROOT), p.getTokenLocation());
7975
}
8076
throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]");
8177
}, GAP_POLICY, ObjectParser.ValueType.STRING);
82-
83-
return parser;
8478
};
8579

8680

@@ -212,10 +206,6 @@ protected XContentBuilder internalXContent(XContentBuilder builder, Params param
212206
return builder;
213207
}
214208

215-
public static MovFnPipelineAggregationBuilder parse(String aggName, XContentParser parser) {
216-
return PARSER.apply(aggName).apply(parser, null);
217-
}
218-
219209
/**
220210
* Used for serialization testing, since pipeline aggs serialize themselves as a named object but are parsed
221211
* as a regular object with the name passed in.
@@ -228,7 +218,7 @@ static MovFnPipelineAggregationBuilder parse(XContentParser parser) throws IOExc
228218
String aggName = parser.currentName();
229219
parser.nextToken(); // "moving_fn"
230220
parser.nextToken(); // start_object
231-
return PARSER.apply(aggName).apply(parser, null);
221+
return PARSER.apply(parser, aggName);
232222
}
233223
}
234224

‎server/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketScriptIT.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -645,7 +645,7 @@ public void testSingleBucketPathAgg() throws Exception {
645645
.endObject()
646646
.endObject();
647647
BucketScriptPipelineAggregationBuilder bucketScriptAgg =
648-
BucketScriptPipelineAggregationBuilder.parse("seriesArithmetic", createParser(content));
648+
BucketScriptPipelineAggregationBuilder.PARSER.parse(createParser(content), "seriesArithmetic");
649649

650650
SearchResponse response = client()
651651
.prepareSearch("idx", "idx_unmapped")
@@ -690,7 +690,7 @@ public void testArrayBucketPathAgg() throws Exception {
690690
.endObject()
691691
.endObject();
692692
BucketScriptPipelineAggregationBuilder bucketScriptAgg =
693-
BucketScriptPipelineAggregationBuilder.parse("seriesArithmetic", createParser(content));
693+
BucketScriptPipelineAggregationBuilder.PARSER.parse(createParser(content), "seriesArithmetic");
694694

695695
SearchResponse response = client()
696696
.prepareSearch("idx", "idx_unmapped")
@@ -747,7 +747,7 @@ public void testObjectBucketPathAgg() throws Exception {
747747
.endObject()
748748
.endObject();
749749
BucketScriptPipelineAggregationBuilder bucketScriptAgg =
750-
BucketScriptPipelineAggregationBuilder.parse("seriesArithmetic", createParser(content));
750+
BucketScriptPipelineAggregationBuilder.PARSER.parse(createParser(content), "seriesArithmetic");
751751

752752
SearchResponse response = client()
753753
.prepareSearch("idx", "idx_unmapped")

‎server/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketScriptTests.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,8 @@ public void testParseBucketPath() throws IOException {
7171
.field("lang", "expression")
7272
.endObject()
7373
.endObject();
74-
BucketScriptPipelineAggregationBuilder builder1 = BucketScriptPipelineAggregationBuilder.parse("count", createParser(content));
74+
BucketScriptPipelineAggregationBuilder builder1 = BucketScriptPipelineAggregationBuilder.PARSER.parse(
75+
createParser(content), "count");
7576
assertEquals(builder1.getBucketsPaths().length , 1);
7677
assertEquals(builder1.getBucketsPaths()[0], "_count");
7778

@@ -86,7 +87,8 @@ public void testParseBucketPath() throws IOException {
8687
.field("lang", "expression")
8788
.endObject()
8889
.endObject();
89-
BucketScriptPipelineAggregationBuilder builder2 = BucketScriptPipelineAggregationBuilder.parse("count", createParser(content));
90+
BucketScriptPipelineAggregationBuilder builder2 = BucketScriptPipelineAggregationBuilder.PARSER.parse(
91+
createParser(content), "count");
9092
assertEquals(builder2.getBucketsPaths().length , 2);
9193
assertEquals(builder2.getBucketsPaths()[0], "_count1");
9294
assertEquals(builder2.getBucketsPaths()[1], "_count2");
@@ -99,7 +101,8 @@ public void testParseBucketPath() throws IOException {
99101
.field("lang", "expression")
100102
.endObject()
101103
.endObject();
102-
BucketScriptPipelineAggregationBuilder builder3 = BucketScriptPipelineAggregationBuilder.parse("count", createParser(content));
104+
BucketScriptPipelineAggregationBuilder builder3 = BucketScriptPipelineAggregationBuilder.PARSER.parse(
105+
createParser(content), "count");
103106
assertEquals(builder3.getBucketsPaths().length , 2);
104107
assertEquals(builder3.getBucketsPaths()[0], "_count1");
105108
assertEquals(builder3.getBucketsPaths()[1], "_count2");

‎x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsPlugin.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public List<PipelineAggregationSpec> getPipelineAggregations() {
5050
CumulativeCardinalityPipelineAggregationBuilder.NAME,
5151
CumulativeCardinalityPipelineAggregationBuilder::new,
5252
CumulativeCardinalityPipelineAggregator::new,
53-
CumulativeCardinalityPipelineAggregationBuilder::parse)
53+
(name, p) -> CumulativeCardinalityPipelineAggregationBuilder.PARSER.parse(p, name))
5454
);
5555
}
5656

0 commit comments

Comments
 (0)