Skip to content

Commit dde2382

Browse files
nik9000nik9000
authored
Move scripted metric to ObjectParser (#50708)
This replaces the hand rolled parsing code for scripted metric with `ObjectParser` which is simpler to work with because it is declarative.
1 parent 979a28d commit dde2382

File tree

3 files changed

+32
-64
lines changed

3 files changed

+32
-64
lines changed

‎server/src/main/java/org/elasticsearch/script/Script.java

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,6 @@
5050
import java.util.Objects;
5151
import java.util.function.BiConsumer;
5252

53-
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
54-
5553
/**
5654
* {@link Script} represents used-defined input that can be used to
5755
* compile and execute a script from the {@link ScriptService}
@@ -274,13 +272,24 @@ private Script build(String defaultLang) {
274272
}
275273

276274
/**
277-
* Declare a script field on an {@link ObjectParser}.
275+
* Declare a script field on an {@link ObjectParser} with the standard name ({@code script}).
278276
* @param <T> Whatever type the {@linkplain ObjectParser} is parsing.
279277
* @param parser the parser itself
280278
* @param consumer the consumer for the script
281279
*/
282280
public static <T> void declareScript(AbstractObjectParser<T, ?> parser, BiConsumer<T, Script> consumer) {
283-
parser.declareField(constructorArg(), (p, c) -> Script.parse(p), Script.SCRIPT_PARSE_FIELD, ValueType.OBJECT_OR_STRING);
281+
declareScript(parser, consumer, Script.SCRIPT_PARSE_FIELD);
282+
}
283+
284+
/**
285+
* Declare a script field on an {@link ObjectParser}.
286+
* @param <T> Whatever type the {@linkplain ObjectParser} is parsing.
287+
* @param parser the parser itself
288+
* @param consumer the consumer for the script
289+
* @param parseField the field name
290+
*/
291+
public static <T> void declareScript(AbstractObjectParser<T, ?> parser, BiConsumer<T, Script> consumer, ParseField parseField) {
292+
parser.declareField(consumer, (p, c) -> Script.parse(p), parseField, ValueType.OBJECT_OR_STRING);
284293
}
285294

286295
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ private void registerAggregations(List<SearchPlugin> plugins) {
422422
registerAggregation(new AggregationSpec(GeoCentroidAggregationBuilder.NAME, GeoCentroidAggregationBuilder::new,
423423
GeoCentroidAggregationBuilder::parse).addResultReader(InternalGeoCentroid::new));
424424
registerAggregation(new AggregationSpec(ScriptedMetricAggregationBuilder.NAME, ScriptedMetricAggregationBuilder::new,
425-
ScriptedMetricAggregationBuilder::parse).addResultReader(InternalScriptedMetric::new));
425+
(name, p) -> ScriptedMetricAggregationBuilder.PARSER.parse(p, name)).addResultReader(InternalScriptedMetric::new));
426426
registerAggregation((new AggregationSpec(CompositeAggregationBuilder.NAME, CompositeAggregationBuilder::new,
427427
CompositeAggregationBuilder::parse).addResultReader(InternalComposite::new)));
428428
registerFromPlugin(plugins, SearchPlugin::getAggregations, this::registerAggregation);

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

Lines changed: 18 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,13 @@
2020
package org.elasticsearch.search.aggregations.metrics;
2121

2222
import org.elasticsearch.common.ParseField;
23-
import org.elasticsearch.common.ParsingException;
2423
import org.elasticsearch.common.io.stream.StreamInput;
2524
import org.elasticsearch.common.io.stream.StreamOutput;
25+
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;
29-
import org.elasticsearch.script.ScriptedMetricAggContexts;
3028
import org.elasticsearch.script.Script;
29+
import org.elasticsearch.script.ScriptedMetricAggContexts;
3130
import org.elasticsearch.search.aggregations.AbstractAggregationBuilder;
3231
import org.elasticsearch.search.aggregations.AggregationBuilder;
3332
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
@@ -38,6 +37,8 @@
3837
import java.util.Map;
3938
import java.util.Objects;
4039

40+
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
41+
4142
public class ScriptedMetricAggregationBuilder extends AbstractAggregationBuilder<ScriptedMetricAggregationBuilder> {
4243
public static final String NAME = "scripted_metric";
4344

@@ -47,6 +48,20 @@ public class ScriptedMetricAggregationBuilder extends AbstractAggregationBuilder
4748
private static final ParseField REDUCE_SCRIPT_FIELD = new ParseField("reduce_script");
4849
private static final ParseField PARAMS_FIELD = new ParseField("params");
4950

51+
public static final ConstructingObjectParser<ScriptedMetricAggregationBuilder, String> PARSER =
52+
new ConstructingObjectParser<>(NAME, false, (args, name) -> {
53+
ScriptedMetricAggregationBuilder builder = new ScriptedMetricAggregationBuilder(name);
54+
builder.mapScript((Script) args[0]);
55+
return builder;
56+
});
57+
static {
58+
Script.declareScript(PARSER, ScriptedMetricAggregationBuilder::initScript, INIT_SCRIPT_FIELD);
59+
Script.declareScript(PARSER, constructorArg(), MAP_SCRIPT_FIELD);
60+
Script.declareScript(PARSER, ScriptedMetricAggregationBuilder::combineScript, COMBINE_SCRIPT_FIELD);
61+
Script.declareScript(PARSER, ScriptedMetricAggregationBuilder::reduceScript, REDUCE_SCRIPT_FIELD);
62+
PARSER.declareObject(ScriptedMetricAggregationBuilder::params, (p, name) -> p.map(), PARAMS_FIELD);
63+
}
64+
5065
private Script initScript;
5166
private Script mapScript;
5267
private Script combineScript;
@@ -260,62 +275,6 @@ protected XContentBuilder internalXContent(XContentBuilder builder, Params build
260275
return builder;
261276
}
262277

263-
public static ScriptedMetricAggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException {
264-
Script initScript = null;
265-
Script mapScript = null;
266-
Script combineScript = null;
267-
Script reduceScript = null;
268-
Map<String, Object> params = null;
269-
XContentParser.Token token;
270-
String currentFieldName = null;
271-
272-
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
273-
if (token == XContentParser.Token.FIELD_NAME) {
274-
currentFieldName = parser.currentName();
275-
} else if (token == XContentParser.Token.START_OBJECT || token == XContentParser.Token.VALUE_STRING) {
276-
if (INIT_SCRIPT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
277-
initScript = Script.parse(parser);
278-
} else if (MAP_SCRIPT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
279-
mapScript = Script.parse(parser);
280-
} else if (COMBINE_SCRIPT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
281-
combineScript = Script.parse(parser);
282-
} else if (REDUCE_SCRIPT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
283-
reduceScript = Script.parse(parser);
284-
} else if (token == XContentParser.Token.START_OBJECT &&
285-
PARAMS_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
286-
params = parser.map();
287-
} else {
288-
throw new ParsingException(parser.getTokenLocation(),
289-
"Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "].");
290-
}
291-
} else {
292-
throw new ParsingException(parser.getTokenLocation(), "Unexpected token " + token + " in [" + aggregationName + "].");
293-
}
294-
}
295-
296-
if (mapScript == null) {
297-
throw new ParsingException(parser.getTokenLocation(), "map_script field is required in [" + aggregationName + "].");
298-
}
299-
300-
ScriptedMetricAggregationBuilder factory = new ScriptedMetricAggregationBuilder(aggregationName);
301-
if (initScript != null) {
302-
factory.initScript(initScript);
303-
}
304-
if (mapScript != null) {
305-
factory.mapScript(mapScript);
306-
}
307-
if (combineScript != null) {
308-
factory.combineScript(combineScript);
309-
}
310-
if (reduceScript != null) {
311-
factory.reduceScript(reduceScript);
312-
}
313-
if (params != null) {
314-
factory.params(params);
315-
}
316-
return factory;
317-
}
318-
319278
@Override
320279
public String getType() {
321280
return NAME;

0 commit comments

Comments
 (0)