Browse Source

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
Colin Goodheart-Smithe 11 years ago
parent
commit
6cf371395a

+ 8 - 1
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.
 

+ 21 - 15
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<String, Object> 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<String, Object> params, Map<String, Object> 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<String, Object> params, Map<String, Object> 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 @@ public class ScriptedMetricAggregator extends MetricsAggregator {
             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 class ScriptedMetricAggregator extends MetricsAggregator {
         } 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 class ScriptedMetricAggregator extends MetricsAggregator {
         private Map<String, Object> params;
         private Map<String, Object> 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<String, Object> params, Map<String, Object> 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 class ScriptedMetricAggregator extends MetricsAggregator {
 
         @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);
         }
 
     }

+ 102 - 12
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<String, Object> params = null;
     private Map<String, Object> 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 class ScriptedMetricBuilder extends MetricsAggregationBuilder {
     }
 
     /**
-     * Set the script language.
+     * Set the <tt>init</tt> script file.
      */
-    public ScriptedMetricBuilder lang(String lang) {
-        this.lang = lang;
+    public ScriptedMetricBuilder initScriptFile(String initScriptFile) {
+        this.initScriptFile = initScriptFile;
+        return this;
+    }
+
+    /**
+     * Set the <tt>map</tt> script file.
+     */
+    public ScriptedMetricBuilder mapScriptFile(String mapScriptFile) {
+        this.mapScriptFile = mapScriptFile;
+        return this;
+    }
+
+    /**
+     * Set the <tt>combine</tt> script file.
+     */
+    public ScriptedMetricBuilder combineScriptFile(String combineScriptFile) {
+        this.combineScriptFile = combineScriptFile;
+        return this;
+    }
+
+    /**
+     * Set the <tt>reduce</tt> script file.
+     */
+    public ScriptedMetricBuilder reduceScriptFile(String reduceScriptFile) {
+        this.reduceScriptFile = reduceScriptFile;
+        return this;
+    }
+
+    /**
+     * Set the indexed <tt>init</tt> script id.
+     */
+    public ScriptedMetricBuilder initScriptId(String initScriptId) {
+        this.initScriptId = initScriptId;
         return this;
     }
 
     /**
-     * Set the script type.
+     * Set the indexed <tt>map</tt> script id.
      */
-    public ScriptedMetricBuilder scriptType(ScriptType scriptType) {
-        this.scriptType = scriptType;
+    public ScriptedMetricBuilder mapScriptId(String mapScriptId) {
+        this.mapScriptId = mapScriptId;
+        return this;
+    }
+
+    /**
+     * Set the indexed <tt>combine</tt> script id.
+     */
+    public ScriptedMetricBuilder combineScriptId(String combineScriptId) {
+        this.combineScriptId = combineScriptId;
+        return this;
+    }
+
+    /**
+     * Set the indexed <tt>reduce</tt> 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 @@ public class ScriptedMetricBuilder extends MetricsAggregationBuilder {
             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);
         }
     }
 

+ 78 - 6
src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricParser.java

@@ -28,7 +28,6 @@ import org.elasticsearch.search.aggregations.AggregatorFactory;
 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 class ScriptedMetricParser implements Aggregator.Parser {
         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<String, Object> params = null;
         Map<String, Object> reduceParams = null;
         XContentParser.Token token;
@@ -73,17 +82,79 @@ public class ScriptedMetricParser implements Aggregator.Parser {
                 }
             } 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 class ScriptedMetricParser implements Aggregator.Parser {
         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);
     }
 
 }

+ 4 - 6
src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricTests.java

@@ -24,7 +24,6 @@ import org.elasticsearch.action.indexedscripts.put.PutIndexedScriptResponse;
 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 class ScriptedMetricTests extends ElasticsearchIntegrationTest {
                 .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 class ScriptedMetricTests extends ElasticsearchIntegrationTest {
                 .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));