Scripting: fill in get contexts REST API

Updates response for `GET /_script_context`, returning a `contexts` object with a list of context description objects. The description includes the context name and a list of methods available. The methods list has the signature for the `execute` mathod and any getters. eg. ``` { "contexts": [ { "name" : "moving-function", "methods" : [ { "name" : "execute", "return_type" : "double", "params" : [ { "type" : "java.util.Map", "name" : "params" }, { "type" : "double[]", "name" : "values" } ] } ] }, { "name" : "number_sort", "methods" : [ { "name" : "execute", "return_type" : "double", "params" : [ ] }, { "name" : "getDoc", "return_type" : "java.util.Map", "params" : [ ] }, { "name" : "getParams", "return_type" : "java.util.Map", "params" : [ ] }, { "name" : "get_score", "return_type" : "double", "params" : [ ] } ] }, ... ] } ``` fixes: elastic#47411
stu-elastic · Oct 21, 2019 · 990e65e · 990e65e
1 parent 95bcb6f
commit 990e65e
Show file tree

Hide file tree

Showing 8 changed files with 332 additions and 203 deletions.
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/scripts/20_get_script_context.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/scripts/20_get_script_context.yml
@@ -4,24 +4,68 @@
       reason: "get_all_contexts introduced in 7.6.0"
   - do:
       get_script_context: {}
-  - match: { contexts.aggregation_selector: {} }
-  - match: { contexts.aggs: {} }
-  - match: { contexts.aggs_combine: {} }
-  - match: { contexts.aggs_init: {} }
-  - match: { contexts.aggs_map: {} }
-  - match: { contexts.aggs_reduce: {} }
-  - match: { contexts.bucket_aggregation: {} }
-  - match: { contexts.field: {} }
-  - match: { contexts.filter: {} }
-  - match: { contexts.ingest: {} }
-  - match: { contexts.interval: {} }
-  - match: { contexts.number_sort: {} }
-  - match: { contexts.processor_conditional: {} }
-  - match: { contexts.score: {} }
-  - match: { contexts.script_heuristic: {} }
-  - match: { contexts.similarity: {} }
-  - match: { contexts.similarity_weight: {} }
-  - match: { contexts.string_sort: {} }
-  - match: { contexts.template: {} }
-  - match: { contexts.terms_set: {} }
-  - match: { contexts.update: {} }
+  - match: { contexts.0.name: "aggregation_selector" }
+  - match: { contexts.0.methods: [{"name": "execute", "return_type": "boolean", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}] }
+
+  - match: { contexts.1.name: "aggs" }
+  - match: { contexts.1.methods: [{"name": "execute", "return_type": "java.lang.Object", "params": []}, {"name": "getDoc", "return_type": "java.util.Map", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}, {"name": "get_score", "return_type": "java.lang.Number", "params": []}, {"name": "get_value", "return_type": "java.lang.Object", "params": []}] }
+
+  - match: { contexts.2.name: "aggs_combine" }
+  - match: { contexts.2.methods: [{"name": "execute", "return_type": "java.lang.Object", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}, {"name": "getState", "return_type": "java.util.Map", "params": []}] }
+
+  - match: { contexts.3.name: "aggs_init" }
+  - match: { contexts.3.methods: [{"name": "execute", "return_type": "void", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}, {"name": "getState", "return_type": "java.lang.Object", "params": []}] }
+
+  - match: { contexts.4.name: "aggs_map" }
+  - match: { contexts.4.methods: [{"name": "execute", "return_type": "void", "params": []}, {"name": "getDoc", "return_type": "java.util.Map", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}, {"name": "getState", "return_type": "java.util.Map", "params": []}, {"name": "get_score", "return_type": "double", "params": []}] }
+
+  - match: { contexts.5.name: "aggs_reduce" }
+  - match: { contexts.5.methods: [{"name": "execute", "return_type": "java.lang.Object", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}, {"name": "getStates", "return_type": "java.util.List", "params": []}] }
+
+  - match: { contexts.6.name: "bucket_aggregation" }
+  - match: { contexts.6.methods: [{"name": "execute", "return_type": "java.lang.Number", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}] }
+
+  - match: { contexts.7.name: "field" }
+  - match: { contexts.7.methods: [{"name": "execute", "return_type": "java.lang.Object", "params": []}, {"name": "getDoc", "return_type": "java.util.Map", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}] }
+
+  - match: { contexts.8.name: "filter" }
+  - match: { contexts.8.methods: [{"name": "execute", "return_type": "boolean", "params": []}, {"name": "getDoc", "return_type": "java.util.Map", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}] }
+
+  - match: { contexts.9.name: "ingest" }
+  - match: { contexts.9.methods: [{"name": "execute", "return_type": "void", "params": [{"type": "java.util.Map", "name": "ctx"}]}, {"name": "getParams", "return_type": "java.util.Map", "params": []}] }
+
+  - match: { contexts.10.name: "interval" }
+  - match: { contexts.10.methods: [{"name": "execute", "return_type": "boolean", "params": [{"type": "org.elasticsearch.index.query.IntervalFilterScript$Interval", "name": "interval"}]}] }
+
+  - match: { contexts.11.name: "moving-function" }
+  - match: { contexts.11.methods: [{"name": "execute", "return_type": "double", "params": [{"type": "java.util.Map", "name": "params"}, {"type": "double[]", "name": "values"}]}] }
+
+  - match: { contexts.12.name: "number_sort" }
+  - match: { contexts.12.methods: [{"name": "execute", "return_type": "double", "params": []}, {"name": "getDoc", "return_type": "java.util.Map", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}, {"name": "get_score", "return_type": "double", "params": []}] }
+
+  - match: { contexts.13.name: "processor_conditional" }
+  - match: { contexts.13.methods: [{"name": "execute", "return_type": "boolean", "params": [{"type": "java.util.Map", "name": "ctx"}]}, {"name": "getParams", "return_type": "java.util.Map", "params": []}] }
+
+  - match: { contexts.14.name: "score" }
+  - match: { contexts.14.methods: [{"name": "execute", "return_type": "double", "params": [{"type": "org.elasticsearch.script.ScoreScript$ExplanationHolder", "name": "explanation"}]}, {"name": "getDoc", "return_type": "java.util.Map", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}, {"name": "get_score", "return_type": "double", "params": []}] }
+
+  - match: { contexts.15.name: "script_heuristic" }
+  - match: { contexts.15.methods: [{"name": "execute", "return_type": "double", "params": [{"type": "java.util.Map", "name": "params"}]}] }
+
+  - match: { contexts.16.name: "similarity" }
+  - match: { contexts.16.methods: [{"name": "execute", "return_type": "double", "params": [{"type": "double", "name": "weight"}, {"type": "org.elasticsearch.index.similarity.ScriptedSimilarity$Query", "name": "query"}, {"type": "org.elasticsearch.index.similarity.ScriptedSimilarity$Field", "name": "field"}, {"type": "org.elasticsearch.index.similarity.ScriptedSimilarity$Term", "name": "term"}, {"type": "org.elasticsearch.index.similarity.ScriptedSimilarity$Doc", "name": "doc"}]}] }
+
+  - match: { contexts.17.name: "similarity_weight" }
+  - match: { contexts.17.methods: [{"name": "execute", "return_type": "double", "params": [{"type": "org.elasticsearch.index.similarity.ScriptedSimilarity$Query", "name": "query"}, {"type": "org.elasticsearch.index.similarity.ScriptedSimilarity$Field", "name": "field"}, {"type": "org.elasticsearch.index.similarity.ScriptedSimilarity$Term", "name": "term"}]}] }
+
+  - match: { contexts.18.name: "string_sort" }
+  - match: { contexts.18.methods: [{"name": "execute", "return_type": "java.lang.String", "params": []}, {"name": "getDoc", "return_type": "java.util.Map", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}, {"name": "get_score", "return_type": "double", "params": []}] }
+
+  - match: { contexts.19.name: "template" }
+  - match: { contexts.19.methods: [{"name": "execute", "return_type": "java.lang.String", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}] }
+
+  - match: { contexts.20.name: "terms_set" }
+  - match: { contexts.20.methods: [{"name": "execute", "return_type": "java.lang.Number", "params": []}, {"name": "getDoc", "return_type": "java.util.Map", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}] }
+
+  - match: { contexts.21.name: "update" }
+  - match: { contexts.21.methods: [{"name": "execute", "return_type": "void", "params": []}, {"name": "getCtx", "return_type": "java.util.Map", "params": []}, {"name": "getParams", "return_type": "java.util.Map", "params": []}] }
diff --git a/...n/java/org/elasticsearch/action/admin/cluster/storedscripts/GetScriptContextResponse.java b/...n/java/org/elasticsearch/action/admin/cluster/storedscripts/GetScriptContextResponse.java
@@ -31,8 +31,10 @@
 import org.elasticsearch.script.ScriptContextInfo;
 
 import java.io.IOException;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.List;
 import java.util.Objects;
 import java.util.HashSet;
 import java.util.Set;
@@ -41,18 +43,19 @@
 public class GetScriptContextResponse extends ActionResponse implements StatusToXContentObject {
 
     private static final ParseField CONTEXTS = new ParseField("contexts");
-    private final Set<ScriptContextInfo> contexts;
+    final Set<ScriptContextInfo> contexts;
 
     @SuppressWarnings("unchecked")
     public static final ConstructingObjectParser<GetScriptContextResponse,Void> PARSER =
         new ConstructingObjectParser<>("get_script_context", true,
             (a) -> {
-                    return new GetScriptContextResponse((Set<ScriptContextInfo>)a[0]);
+                    return new GetScriptContextResponse((List<ScriptContextInfo>)a[0]);
             }
         );
 
     static {
-        PARSER.declareNamedObjects(ConstructingObjectParser.constructorArg(), (p, c, n) -> ScriptContextInfo.fromXContent(p), CONTEXTS);
+        PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(),
+            (parser, ctx) -> ScriptContextInfo.PARSER.apply(parser, ctx), CONTEXTS);
     }
 
     GetScriptContextResponse(StreamInput in) throws IOException {
@@ -65,8 +68,8 @@ public class GetScriptContextResponse extends ActionResponse implements StatusTo
         this.contexts = Collections.unmodifiableSet(contexts);
     }
 
-    GetScriptContextResponse(Set<ScriptContextInfo> contexts) {
-        this.contexts = Collections.unmodifiableSet(contexts);
+    GetScriptContextResponse(Collection<ScriptContextInfo> contexts) {
+        this.contexts = Set.copyOf(contexts);
     }
 
     @Override

diff --git a/server/src/main/java/org/elasticsearch/script/ScriptContextInfo.java b/server/src/main/java/org/elasticsearch/script/ScriptContextInfo.java
@@ -35,34 +35,38 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
 
 public class ScriptContextInfo implements ToXContentObject, Writeable {
     public final String name;
     public final ScriptMethodInfo execute;
-    public final List<ScriptMethodInfo> getters;
+    public final Set<ScriptMethodInfo> getters;
 
     private static String NAME_FIELD = "name";
     private static String METHODS_FIELD = "methods";
 
-    public ScriptContextInfo(String name, ScriptMethodInfo execute,  List<ScriptMethodInfo> getters) {
-        this.name = Objects.requireNonNull(name);
-        this.execute = Objects.requireNonNull(execute);
-        this.getters = Objects.requireNonNull(getters);
+    // ScriptService constructor
+    ScriptContextInfo(String name, Class<?> clazz) {
+        this.name = name;
+        this.execute = ScriptMethodInfo.executeFromContext(clazz);
+        this.getters = Collections.unmodifiableSet(ScriptMethodInfo.gettersFromContext(clazz));
     }
 
-    // TODO(stu): test specifically
-    private ScriptContextInfo(String name, List<ScriptMethodInfo> methods) {
+    // Deserialization constructor
+    ScriptContextInfo(String name, List<ScriptMethodInfo> methods) {
         this.name = Objects.requireNonNull(name);
         Objects.requireNonNull(methods);
 
         String executeName = "execute";
         String getName = "get";
+        // ignored instead of error, so future implementations can add methods.  Same as ScriptContextInfo(String, Class).
         String otherName = "other";
         Map<String, List<ScriptMethodInfo>> methodTypes = methods.stream().collect(Collectors.groupingBy(
             m -> {
@@ -75,34 +79,39 @@ private ScriptContextInfo(String name, List<ScriptMethodInfo> methods) {
             }
         ));
 
-        if (methodTypes.containsKey(otherName)) {
-            throw new IllegalArgumentException("unexpected methods, expected 'execute' and getters, not " +
-                methodTypes.get(otherName).stream().map(m -> m.name).collect(Collectors.joining(", ", "[", "]")));
-        }
-
         if (!methodTypes.containsKey(executeName)) {
-            throw new IllegalArgumentException("execute method required");
+            throw new IllegalArgumentException("Could not find required method [" + executeName + "] in [" + name + "], found " +
+                methods.stream().map(m -> m.name).sorted().collect(Collectors.joining(", ", "[", "]")));
         } else if (!(methodTypes.get(executeName).size() == 1)) {
-            throw new IllegalArgumentException("Only one execute method expected");
+            throw new IllegalArgumentException("Cannot have multiple [execute] methods in [" + name + "], found [" +
+                methodTypes.get(executeName).size() + "]"
+            );
         }
         this.execute = methodTypes.get(executeName).get(0);
 
         if (methodTypes.containsKey(getName)) {
-            this.getters = Collections.unmodifiableList(methodTypes.get(getName));
+            this.getters = Set.copyOf(methodTypes.get(getName));
         } else {
-            this.getters = Collections.unmodifiableList(new ArrayList<>());
+            this.getters = Collections.emptySet();
         }
     }
 
+    // Test constructor
+    public ScriptContextInfo(String name, ScriptMethodInfo execute,  Set<ScriptMethodInfo> getters) {
+        this.name = Objects.requireNonNull(name);
+        this.execute = Objects.requireNonNull(execute);
+        this.getters = Objects.requireNonNull(getters);
+    }
+
     public ScriptContextInfo(StreamInput in) throws IOException {
         this.name = in.readString();
         this.execute = new ScriptMethodInfo(in);
         int numGetters = in.readInt();
-        List<ScriptMethodInfo> getters = new ArrayList<>(numGetters);
+        Set<ScriptMethodInfo> getters = new HashSet<>(numGetters);
         for (int i = 0; i < numGetters; i++) {
             getters.add(new ScriptMethodInfo(in));
         }
-        this.getters = Collections.unmodifiableList(getters);
+        this.getters = Collections.unmodifiableSet(getters);
     }
 
     public void writeTo(StreamOutput out) throws IOException {
@@ -121,14 +130,8 @@ public List<ScriptMethodInfo> methods() {
         return Collections.unmodifiableList(methods);
     }
 
-    ScriptContextInfo(String name, Class<?> clazz) {
-        this.name = name;
-        this.execute = ScriptMethodInfo.executeFromContext(clazz);
-        this.getters = Collections.unmodifiableList(ScriptMethodInfo.gettersFromContext(clazz));
-    }
-
     @SuppressWarnings("unchecked")
-    private static ConstructingObjectParser<ScriptContextInfo,Void> PARSER =
+    public static ConstructingObjectParser<ScriptContextInfo,Void> PARSER =
         new ConstructingObjectParser<>("script_context_info", true,
             (m, name) -> new ScriptContextInfo((String) m[0], (List<ScriptMethodInfo>) m[1])
         );
@@ -313,7 +316,7 @@ static ScriptMethodInfo executeFromContext(Class<?> clazz) {
                 }
             }
             if (execute == null) {
-                throw new IllegalArgumentException("Could not find method [" + name + "] on class [" + clazz.getName() + "]");
+                throw new IllegalArgumentException("Could not find required method [" + name + "] on class [" + clazz.getName() + "]");
             }
 
             Class<?> returnTypeClazz = execute.getReturnType();
@@ -359,9 +362,9 @@ static ScriptMethodInfo executeFromContext(Class<?> clazz) {
             return new ScriptMethodInfo(name, returnType, parameters);
         }
 
-        static List<ScriptMethodInfo> gettersFromContext(Class<?> clazz) {
+        static Set<ScriptMethodInfo> gettersFromContext(Class<?> clazz) {
             // See ScriptClassInfo(PainlessLookup painlessLookup, Class<?> baseClass)
-            ArrayList<ScriptMethodInfo> getters = new ArrayList<>();
+            HashSet<ScriptMethodInfo> getters = new HashSet<>();
             for (java.lang.reflect.Method m : clazz.getMethods()) {
                 if (!m.isDefault() &&
                     m.getName().startsWith("get") &&

diff --git a/...a/org/elasticsearch/action/admin/cluster/storedscripts/GetScriptContextResponseTests.java b/...a/org/elasticsearch/action/admin/cluster/storedscripts/GetScriptContextResponseTests.java
@@ -20,39 +20,19 @@
 
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.common.bytes.BytesArray;
-import org.elasticsearch.common.io.stream.Writeable;
-import org.elasticsearch.common.xcontent.DeprecationHandler;
-import org.elasticsearch.common.xcontent.NamedXContentRegistry;
-import org.elasticsearch.common.xcontent.XContentBuilder;
-import org.elasticsearch.common.xcontent.XContentFactory;
-import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.common.xcontent.XContentType;
-import org.elasticsearch.script.ScriptContextInfo;
 import org.elasticsearch.test.AbstractSerializingTestCase;
 
 import java.io.IOException;
 import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-
-import static com.carrotsearch.randomizedtesting.RandomizedTest.randomAsciiLettersOfLengthBetween;
 
 public class GetScriptContextResponseTests extends AbstractSerializingTestCase<GetScriptContextResponse> {
 
     @Override
     protected GetScriptContextResponse createTestInstance() {
-        /*
         if (randomBoolean()) {
-            return new GetScriptContextResponse(Collections.emptyMap());
+            return new GetScriptContextResponse(Collections.emptySet());
         }
-        Map<String,Object> items = new HashMap<>();
-        for (int i = randomIntBetween(1, 10); i > 0; i--) {
-            items.put(randomAsciiLettersOfLengthBetween(1, 16), new Object());
-        }
-        return new GetScriptContextResponse(items);
-        */
-        return null;
+        return new GetScriptContextResponse(ScriptContextInfoSerializingTests.randomInstances());
     }
 
     @Override
@@ -67,14 +47,6 @@ protected GetScriptContextResponse doParseInstance(XContentParser parser) throws
 
     @Override
     protected GetScriptContextResponse mutateInstance(GetScriptContextResponse instance) throws IOException {
-        // TODO(stu): flaky make sure we don't generate the same thing
-        /*
-        Map<String,Object> items = new HashMap<>();
-        for (int i = randomIntBetween(1, 10); i > 0; i--) {
-            items.put(randomAsciiLettersOfLengthBetween(1, 16), new Object());
-        }
-        return new GetScriptContextResponse(items);
-         */
-        return null;
+        return new GetScriptContextResponse(ScriptContextInfoSerializingTests.mutateOne(instance.contexts));
     }
 }