From 6cf371395a5c43112e080bdd3e3468ca1418ed0e Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Fri, 3 Oct 2014 11:10:52 +0100 Subject: [PATCH] Aggregations: makes script params consistent with other APIs in scripted_metric This change removes the script_type parameter form the Scripted Metric Aggregation and adds support for _file and _id suffixes to the init_script, map_script, combine_script and reduce_script parameters to make defining the source of the script consistent with the other APIs which use the ScriptService --- .../scripted-metric-aggregation.asciidoc | 9 +- .../scripted/ScriptedMetricAggregator.java | 36 +++--- .../scripted/ScriptedMetricBuilder.java | 114 ++++++++++++++++-- .../scripted/ScriptedMetricParser.java | 84 ++++++++++++- .../metrics/ScriptedMetricTests.java | 10 +- 5 files changed, 213 insertions(+), 40 deletions(-) diff --git a/docs/reference/search/aggregations/metrics/scripted-metric-aggregation.asciidoc b/docs/reference/search/aggregations/metrics/scripted-metric-aggregation.asciidoc index 6930a8855ff8f..f99486a80fbc1 100644 --- a/docs/reference/search/aggregations/metrics/scripted-metric-aggregation.asciidoc +++ b/docs/reference/search/aggregations/metrics/scripted-metric-aggregation.asciidoc @@ -227,5 +227,12 @@ params:: Optional. An object whose contents will be passed as variable reduce_params:: Optional. An object whose contents will be passed as variables to the `reduce_script`. This can be useful to allow the user to control the behavior of the reduce phase. If this is not specified the variable will be undefined in the reduce_script execution. lang:: Optional. The script language used for the scripts. If this is not specified the default scripting language is used. -script_type:: Optional. The type of script provided. This can be `inline`, `file` or `indexed`. The default is `inline`. +init_script_file:: Optional. Can be used in place of the `init_script` parameter to provide the script using in a file. +init_script_id:: Optional. Can be used in place of the `init_script` parameter to provide the script using an indexed script. +map_script_file:: Optional. Can be used in place of the `map_script` parameter to provide the script using in a file. +map_script_id:: Optional. Can be used in place of the `map_script` parameter to provide the script using an indexed script. +combine_script_file:: Optional. Can be used in place of the `combine_script` parameter to provide the script using in a file. +combine_script_id:: Optional. Can be used in place of the `combine_script` parameter to provide the script using an indexed script. +reduce_script_file:: Optional. Can be used in place of the `reduce_script` parameter to provide the script using in a file. +reduce_script_id:: Optional. Can be used in place of the `reduce_script` parameter to provide the script using an indexed script. diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java index 7641ea5630000..6a21f49d01ba4 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java @@ -46,15 +46,15 @@ public class ScriptedMetricAggregator extends MetricsAggregator { // initial parameters for {reduce} private final Map reduceParams; private ScriptService scriptService; - private ScriptType scriptType; + private ScriptType reduceScriptType; - protected ScriptedMetricAggregator(String name, String scriptLang, ScriptType scriptType, String initScript, String mapScript, - String combineScript, String reduceScript, Map params, Map reduceParams, - AggregationContext context, Aggregator parent) { + protected ScriptedMetricAggregator(String name, String scriptLang, ScriptType initScriptType, String initScript, + ScriptType mapScriptType, String mapScript, ScriptType combineScriptType, String combineScript, ScriptType reduceScriptType, + String reduceScript, Map params, Map reduceParams, AggregationContext context, Aggregator parent) { super(name, 1, context, parent); this.scriptService = context.searchContext().scriptService(); this.scriptLang = scriptLang; - this.scriptType = scriptType; + this.reduceScriptType = reduceScriptType; if (params == null) { this.params = new HashMap<>(); this.params.put("_agg", new HashMap<>()); @@ -67,11 +67,11 @@ protected ScriptedMetricAggregator(String name, String scriptLang, ScriptType sc this.reduceParams = reduceParams; } if (initScript != null) { - scriptService.executable(scriptLang, initScript, scriptType, this.params).run(); + scriptService.executable(scriptLang, initScript, initScriptType, this.params).run(); } - this.mapScript = scriptService.search(context.searchContext().lookup(), scriptLang, mapScript, scriptType, this.params); + this.mapScript = scriptService.search(context.searchContext().lookup(), scriptLang, mapScript, mapScriptType, this.params); if (combineScript != null) { - this.combineScript = scriptService.executable(scriptLang, combineScript, scriptType, this.params); + this.combineScript = scriptService.executable(scriptLang, combineScript, combineScriptType, this.params); } else { this.combineScript = null; } @@ -102,18 +102,21 @@ public InternalAggregation buildAggregation(long owningBucketOrdinal) { } else { aggregation = params.get("_agg"); } - return new InternalScriptedMetric(name, aggregation, scriptLang, scriptType, reduceScript, reduceParams); + return new InternalScriptedMetric(name, aggregation, scriptLang, reduceScriptType, reduceScript, reduceParams); } @Override public InternalAggregation buildEmptyAggregation() { - return new InternalScriptedMetric(name, null, scriptLang, scriptType, reduceScript, reduceParams); + return new InternalScriptedMetric(name, null, scriptLang, reduceScriptType, reduceScript, reduceParams); } public static class Factory extends AggregatorFactory { private String scriptLang; - private ScriptType scriptType; + private ScriptType initScriptType; + private ScriptType mapScriptType; + private ScriptType combineScriptType; + private ScriptType reduceScriptType; private String initScript; private String mapScript; private String combineScript; @@ -121,11 +124,14 @@ public static class Factory extends AggregatorFactory { private Map params; private Map reduceParams; - public Factory(String name, String scriptLang, ScriptType scriptType, String initScript, String mapScript, String combineScript, String reduceScript, + public Factory(String name, String scriptLang, ScriptType initScriptType, String initScript, ScriptType mapScriptType, String mapScript, ScriptType combineScriptType, String combineScript, ScriptType reduceScriptType, String reduceScript, Map params, Map reduceParams) { super(name, InternalScriptedMetric.TYPE.name()); this.scriptLang = scriptLang; - this.scriptType = scriptType; + this.initScriptType = initScriptType; + this.mapScriptType = mapScriptType; + this.combineScriptType = combineScriptType; + this.reduceScriptType = reduceScriptType; this.initScript = initScript; this.mapScript = mapScript; this.combineScript = combineScript; @@ -136,8 +142,8 @@ public Factory(String name, String scriptLang, ScriptType scriptType, String ini @Override public Aggregator create(AggregationContext context, Aggregator parent, long expectedBucketsCount) { - return new ScriptedMetricAggregator(name, scriptLang, scriptType, initScript, mapScript, combineScript, reduceScript, params, - reduceParams, context, parent); + return new ScriptedMetricAggregator(name, scriptLang, initScriptType, initScript, mapScriptType, mapScript, combineScriptType, + combineScript, reduceScriptType, reduceScript, params, reduceParams, context, parent); } } diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricBuilder.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricBuilder.java index 8552c0d5cabb7..e0d5e4464214a 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricBuilder.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricBuilder.java @@ -20,7 +20,6 @@ package org.elasticsearch.search.aggregations.metrics.scripted; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.script.ScriptService.ScriptType; import org.elasticsearch.search.aggregations.metrics.MetricsAggregationBuilder; import java.io.IOException; @@ -33,11 +32,18 @@ public class ScriptedMetricBuilder extends MetricsAggregationBuilder { private Map params = null; private Map reduceParams = null; - private ScriptType scriptType = null; private String initScript = null; private String mapScript = null; private String combineScript = null; private String reduceScript = null; + private String initScriptFile = null; + private String mapScriptFile = null; + private String combineScriptFile = null; + private String reduceScriptFile = null; + private String initScriptId = null; + private String mapScriptId = null; + private String combineScriptId = null; + private String reduceScriptId = null; private String lang = null; /** @@ -97,18 +103,74 @@ public ScriptedMetricBuilder reduceScript(String reduceScript) { } /** - * Set the script language. + * Set the init script file. */ - public ScriptedMetricBuilder lang(String lang) { - this.lang = lang; + public ScriptedMetricBuilder initScriptFile(String initScriptFile) { + this.initScriptFile = initScriptFile; + return this; + } + + /** + * Set the map script file. + */ + public ScriptedMetricBuilder mapScriptFile(String mapScriptFile) { + this.mapScriptFile = mapScriptFile; + return this; + } + + /** + * Set the combine script file. + */ + public ScriptedMetricBuilder combineScriptFile(String combineScriptFile) { + this.combineScriptFile = combineScriptFile; + return this; + } + + /** + * Set the reduce script file. + */ + public ScriptedMetricBuilder reduceScriptFile(String reduceScriptFile) { + this.reduceScriptFile = reduceScriptFile; + return this; + } + + /** + * Set the indexed init script id. + */ + public ScriptedMetricBuilder initScriptId(String initScriptId) { + this.initScriptId = initScriptId; return this; } /** - * Set the script type. + * Set the indexed map script id. */ - public ScriptedMetricBuilder scriptType(ScriptType scriptType) { - this.scriptType = scriptType; + public ScriptedMetricBuilder mapScriptId(String mapScriptId) { + this.mapScriptId = mapScriptId; + return this; + } + + /** + * Set the indexed combine script id. + */ + public ScriptedMetricBuilder combineScriptId(String combineScriptId) { + this.combineScriptId = combineScriptId; + return this; + } + + /** + * Set the indexed reduce script id. + */ + public ScriptedMetricBuilder reduceScriptId(String reduceScriptId) { + this.reduceScriptId = reduceScriptId; + return this; + } + + /** + * Set the script language. + */ + public ScriptedMetricBuilder lang(String lang) { + this.lang = lang; return this; } @@ -140,12 +202,40 @@ protected void internalXContent(XContentBuilder builder, Params builderParams) t builder.field(ScriptedMetricParser.REDUCE_SCRIPT_FIELD.getPreferredName(), reduceScript); } - if (lang != null) { - builder.field(ScriptedMetricParser.LANG_FIELD.getPreferredName(), lang); + if (initScriptFile != null) { + builder.field(ScriptedMetricParser.INIT_SCRIPT_FILE_FIELD.getPreferredName(), initScriptFile); + } + + if (mapScriptFile != null) { + builder.field(ScriptedMetricParser.MAP_SCRIPT_FILE_FIELD.getPreferredName(), mapScriptFile); } - if (scriptType != null) { - builder.field(ScriptedMetricParser.SCRIPT_TYPE_FIELD.getPreferredName(), scriptType.name()); + if (combineScriptFile != null) { + builder.field(ScriptedMetricParser.COMBINE_SCRIPT_FILE_FIELD.getPreferredName(), combineScriptFile); + } + + if (reduceScriptFile != null) { + builder.field(ScriptedMetricParser.REDUCE_SCRIPT_FILE_FIELD.getPreferredName(), reduceScriptFile); + } + + if (initScriptId != null) { + builder.field(ScriptedMetricParser.INIT_SCRIPT_ID_FIELD.getPreferredName(), initScriptId); + } + + if (mapScriptId != null) { + builder.field(ScriptedMetricParser.MAP_SCRIPT_ID_FIELD.getPreferredName(), mapScriptId); + } + + if (combineScriptId != null) { + builder.field(ScriptedMetricParser.COMBINE_SCRIPT_ID_FIELD.getPreferredName(), combineScriptId); + } + + if (reduceScriptId != null) { + builder.field(ScriptedMetricParser.REDUCE_SCRIPT_ID_FIELD.getPreferredName(), reduceScriptId); + } + + if (lang != null) { + builder.field(ScriptedMetricParser.LANG_FIELD.getPreferredName(), lang); } } diff --git a/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricParser.java b/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricParser.java index e21e0e873cb29..136adfbecc803 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricParser.java +++ b/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricParser.java @@ -28,7 +28,6 @@ import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; -import java.util.Locale; import java.util.Map; public class ScriptedMetricParser implements Aggregator.Parser { @@ -39,8 +38,15 @@ public class ScriptedMetricParser implements Aggregator.Parser { public static final ParseField MAP_SCRIPT_FIELD = new ParseField("map_script"); public static final ParseField COMBINE_SCRIPT_FIELD = new ParseField("combine_script"); public static final ParseField REDUCE_SCRIPT_FIELD = new ParseField("reduce_script"); + public static final ParseField INIT_SCRIPT_FILE_FIELD = new ParseField("init_script_file"); + public static final ParseField MAP_SCRIPT_FILE_FIELD = new ParseField("map_script_file"); + public static final ParseField COMBINE_SCRIPT_FILE_FIELD = new ParseField("combine_script_file"); + public static final ParseField REDUCE_SCRIPT_FILE_FIELD = new ParseField("reduce_script_file"); + public static final ParseField INIT_SCRIPT_ID_FIELD = new ParseField("init_script_id"); + public static final ParseField MAP_SCRIPT_ID_FIELD = new ParseField("map_script_id"); + public static final ParseField COMBINE_SCRIPT_ID_FIELD = new ParseField("combine_script_id"); + public static final ParseField REDUCE_SCRIPT_ID_FIELD = new ParseField("reduce_script_id"); public static final ParseField LANG_FIELD = new ParseField("lang"); - public static final ParseField SCRIPT_TYPE_FIELD = new ParseField("script_type"); @Override public String type() { @@ -54,7 +60,10 @@ public AggregatorFactory parse(String aggregationName, XContentParser parser, Se String combineScript = null; String reduceScript = null; String scriptLang = null; - ScriptType scriptType = ScriptType.INLINE; + ScriptType initScriptType = ScriptType.INLINE; + ScriptType mapScriptType = ScriptType.INLINE; + ScriptType combineScriptType = ScriptType.INLINE; + ScriptType reduceScriptType = ScriptType.INLINE; Map params = null; Map reduceParams = null; XContentParser.Token token; @@ -73,17 +82,79 @@ public AggregatorFactory parse(String aggregationName, XContentParser parser, Se } } else if (token.isValue()) { if (INIT_SCRIPT_FIELD.match(currentFieldName)) { + if (initScript != null) { + throw new SearchParseException(context, "Only one of [init_script, init_script_file, init_script_id] is allowed in [" + aggregationName + "]."); + } initScript = parser.text(); + initScriptType = ScriptType.INLINE; } else if (MAP_SCRIPT_FIELD.match(currentFieldName)) { + if (mapScript != null) { + throw new SearchParseException(context, "Only one of [map_script, map_script_file, map_script_id] is allowed in [" + aggregationName + "]."); + } mapScript = parser.text(); + mapScriptType = ScriptType.INLINE; } else if (COMBINE_SCRIPT_FIELD.match(currentFieldName)) { + if (combineScript != null) { + throw new SearchParseException(context, "Only one of [combine_script, combine_script_file, combine_script_id] is allowed in [" + aggregationName + "]."); + } combineScript = parser.text(); + combineScriptType = ScriptType.INLINE; } else if (REDUCE_SCRIPT_FIELD.match(currentFieldName)) { + if (reduceScript != null) { + throw new SearchParseException(context, "Only one of [reduce_script, reduce_script_file, reduce_script_id] is allowed in [" + aggregationName + "]."); + } reduceScript = parser.text(); + reduceScriptType = ScriptType.INLINE; + } else if (INIT_SCRIPT_FILE_FIELD.match(currentFieldName)) { + if (initScript != null) { + throw new SearchParseException(context, "Only one of [init_script, init_script_file, init_script_id] is allowed in [" + aggregationName + "]."); + } + initScript = parser.text(); + initScriptType = ScriptType.FILE; + } else if (MAP_SCRIPT_FILE_FIELD.match(currentFieldName)) { + if (mapScript != null) { + throw new SearchParseException(context, "Only one of [map_script, map_script_file, map_script_id] is allowed in [" + aggregationName + "]."); + } + mapScript = parser.text(); + mapScriptType = ScriptType.FILE; + } else if (COMBINE_SCRIPT_FILE_FIELD.match(currentFieldName)) { + if (combineScript != null) { + throw new SearchParseException(context, "Only one of [combine_script, combine_script_file, combine_script_id] is allowed in [" + aggregationName + "]."); + } + combineScript = parser.text(); + combineScriptType = ScriptType.FILE; + } else if (REDUCE_SCRIPT_FILE_FIELD.match(currentFieldName)) { + if (reduceScript != null) { + throw new SearchParseException(context, "Only one of [reduce_script, reduce_script_file, reduce_script_id] is allowed in [" + aggregationName + "]."); + } + reduceScript = parser.text(); + reduceScriptType = ScriptType.FILE; + } else if (INIT_SCRIPT_ID_FIELD.match(currentFieldName)) { + if (initScript != null) { + throw new SearchParseException(context, "Only one of [init_script, init_script_file, init_script_id] is allowed in [" + aggregationName + "]."); + } + initScript = parser.text(); + initScriptType = ScriptType.INDEXED; + } else if (MAP_SCRIPT_ID_FIELD.match(currentFieldName)) { + if (mapScript != null) { + throw new SearchParseException(context, "Only one of [map_script, map_script_file, map_script_id] is allowed in [" + aggregationName + "]."); + } + mapScript = parser.text(); + mapScriptType = ScriptType.INDEXED; + } else if (COMBINE_SCRIPT_ID_FIELD.match(currentFieldName)) { + if (combineScript != null) { + throw new SearchParseException(context, "Only one of [combine_script, combine_script_file, combine_script_id] is allowed in [" + aggregationName + "]."); + } + combineScript = parser.text(); + combineScriptType = ScriptType.INDEXED; + } else if (REDUCE_SCRIPT_ID_FIELD.match(currentFieldName)) { + if (reduceScript != null) { + throw new SearchParseException(context, "Only one of [reduce_script, reduce_script_file, reduce_script_id] is allowed in [" + aggregationName + "]."); + } + reduceScript = parser.text(); + reduceScriptType = ScriptType.INDEXED; } else if (LANG_FIELD.match(currentFieldName)) { scriptLang = parser.text(); - } else if (SCRIPT_TYPE_FIELD.match(currentFieldName)) { - scriptType = ScriptType.valueOf(parser.text().toUpperCase(Locale.getDefault())); } else { throw new SearchParseException(context, "Unknown key for a " + token + " in [" + aggregationName + "]: [" + currentFieldName + "]."); } @@ -94,7 +165,8 @@ public AggregatorFactory parse(String aggregationName, XContentParser parser, Se if (mapScript == null) { throw new SearchParseException(context, "map_script field is required in [" + aggregationName + "]."); } - return new ScriptedMetricAggregator.Factory(aggregationName, scriptLang, scriptType, initScript, mapScript, combineScript, reduceScript, params, reduceParams); + return new ScriptedMetricAggregator.Factory(aggregationName, scriptLang, initScriptType, initScript, mapScriptType, mapScript, + combineScriptType, combineScript, reduceScriptType, reduceScript, params, reduceParams); } } diff --git a/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricTests.java b/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricTests.java index e9f0790cbf847..6e564cd4bd104 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricTests.java @@ -24,7 +24,6 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.script.ScriptService.ScriptType; import org.elasticsearch.search.aggregations.Aggregation; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.metrics.scripted.ScriptedMetric; @@ -508,8 +507,8 @@ public void testInitMapCombineReduce_withParams_Indexed() { .prepareSearch("idx") .setQuery(matchAllQuery()) .addAggregation( - scriptedMetric("scripted").params(params).scriptType(ScriptType.INDEXED).initScript("initScript_indexed") - .mapScript("mapScript_indexed").combineScript("combineScript_indexed").reduceScript("reduceScript_indexed")) + scriptedMetric("scripted").params(params).initScriptId("initScript_indexed") + .mapScriptId("mapScript_indexed").combineScriptId("combineScript_indexed").reduceScriptId("reduceScript_indexed")) .execute().actionGet(); assertSearchResponse(response); assertThat(response.getHits().getTotalHits(), equalTo(numDocs)); @@ -542,9 +541,8 @@ public void testInitMapCombineReduce_withParams_File() { .prepareSearch("idx") .setQuery(matchAllQuery()) .addAggregation( - scriptedMetric("scripted").params(params).scriptType(ScriptType.FILE).initScript("init_script") - .mapScript("map_script").combineScript("combine_script").reduceScript("reduce_script")) - .execute().actionGet(); + scriptedMetric("scripted").params(params).initScriptFile("init_script").mapScriptFile("map_script") + .combineScriptFile("combine_script").reduceScriptFile("reduce_script")).execute().actionGet(); assertSearchResponse(response); assertThat(response.getHits().getTotalHits(), equalTo(numDocs));