Scripting: Remove ExecutableScript (#34154)

This commit removes the legacy ExecutableScript, which was no longer
used except in tests. All uses have previously been converted to script
contexts.

modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngine.java

@@ -41,7 +41,6 @@
 import org.elasticsearch.script.BucketAggregationScript;
 import org.elasticsearch.script.BucketAggregationSelectorScript;
 import org.elasticsearch.script.ClassPermission;
-import org.elasticsearch.script.ExecutableScript;
 import org.elasticsearch.script.FilterScript;
 import org.elasticsearch.script.ScoreScript;
 import org.elasticsearch.script.ScriptContext;
@@ -112,9 +111,6 @@ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundE
         if (context.instanceClazz.equals(SearchScript.class)) {
             SearchScript.Factory factory = (p, lookup) -> newSearchScript(expr, lookup, p);
             return context.factoryClazz.cast(factory);
-        } else if (context.instanceClazz.equals(ExecutableScript.class)) {
-            ExecutableScript.Factory factory = (p) -> new ExpressionExecutableScript(expr, p);
-            return context.factoryClazz.cast(factory);
         } else if (context.instanceClazz.equals(BucketAggregationScript.class)) {
             return context.factoryClazz.cast(newBucketAggregationScriptFactory(expr));
         } else if (context.instanceClazz.equals(BucketAggregationSelectorScript.class)) {

modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java

@@ -19,8 +19,6 @@
 
 package org.elasticsearch.script.expression;
 
-import org.apache.lucene.expressions.Expression;
-import org.apache.lucene.expressions.js.JavascriptCompiler;
 import org.elasticsearch.action.search.SearchPhaseExecutionException;
 import org.elasticsearch.action.search.SearchRequestBuilder;
 import org.elasticsearch.action.search.SearchResponse;
@@ -33,7 +31,6 @@
 import org.elasticsearch.index.query.functionscore.ScoreFunctionBuilder;
 import org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders;
 import org.elasticsearch.plugins.Plugin;
-import org.elasticsearch.script.GeneralScriptException;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.search.SearchHits;
@@ -504,68 +501,6 @@ public void testStringSpecialValueVariable() throws Exception {
                 message.contains("text variable"), equalTo(true));
     }
 
-    // series of unit test for using expressions as executable scripts
-    public void testExecutableScripts() throws Exception {
-        assumeTrue("test creates classes directly, cannot run with security manager", System.getSecurityManager() == null);
-        Map<String, Object> vars = new HashMap<>();
-        vars.put("a", 2.5);
-        vars.put("b", 3);
-        vars.put("xyz", -1);
-
-        Expression expr = JavascriptCompiler.compile("a+b+xyz");
-
-        ExpressionExecutableScript ees = new ExpressionExecutableScript(expr, vars);
-        assertEquals((Double) ees.run(), 4.5, 0.001);
-
-        ees.setNextVar("b", -2.5);
-        assertEquals((Double) ees.run(), -1, 0.001);
-
-        ees.setNextVar("a", -2.5);
-        ees.setNextVar("b", -2.5);
-        ees.setNextVar("xyz", -2.5);
-        assertEquals((Double) ees.run(), -7.5, 0.001);
-
-        String message;
-
-        try {
-            vars = new HashMap<>();
-            vars.put("a", 1);
-            ees = new ExpressionExecutableScript(expr, vars);
-            ees.run();
-            fail("An incorrect number of variables were allowed to be used in an expression.");
-        } catch (GeneralScriptException se) {
-            message = se.getMessage();
-            assertThat(message + " should have contained number of variables", message.contains("number of variables"), equalTo(true));
-        }
-
-        try {
-            vars = new HashMap<>();
-            vars.put("a", 1);
-            vars.put("b", 3);
-            vars.put("c", -1);
-            ees = new ExpressionExecutableScript(expr, vars);
-            ees.run();
-            fail("A variable was allowed to be set that does not exist in the expression.");
-        } catch (GeneralScriptException se) {
-            message = se.getMessage();
-            assertThat(message + " should have contained does not exist in", message.contains("does not exist in"), equalTo(true));
-        }
-
-        try {
-            vars = new HashMap<>();
-            vars.put("a", 1);
-            vars.put("b", 3);
-            vars.put("xyz", "hello");
-            ees = new ExpressionExecutableScript(expr, vars);
-            ees.run();
-            fail("A non-number was allowed to be use in the expression.");
-        } catch (GeneralScriptException se) {
-            message = se.getMessage();
-            assertThat(message + " should have contained process numbers", message.contains("process numbers"), equalTo(true));
-        }
-
-    }
-
     // test to make sure expressions are not allowed to be used as update scripts
     public void testInvalidUpdateScript() throws Exception {
         try {

modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java

@@ -26,7 +26,6 @@
 import org.elasticsearch.painless.Compiler.Loader;
 import org.elasticsearch.painless.lookup.PainlessLookupBuilder;
 import org.elasticsearch.painless.spi.Whitelist;
-import org.elasticsearch.script.ExecutableScript;
 import org.elasticsearch.script.ScriptContext;
 import org.elasticsearch.script.ScriptEngine;
 import org.elasticsearch.script.ScriptException;
@@ -102,7 +101,7 @@ public PainlessScriptEngine(Settings settings, Map<ScriptContext<?>, List<Whitel
 
         for (Map.Entry<ScriptContext<?>, List<Whitelist>> entry : contexts.entrySet()) {
             ScriptContext<?> context = entry.getKey();
-            if (context.instanceClazz.equals(SearchScript.class) || context.instanceClazz.equals(ExecutableScript.class)) {
+            if (context.instanceClazz.equals(SearchScript.class)) {
                 contextsToCompilers.put(context, new Compiler(GenericElasticsearchScript.class, null, null,
                         PainlessLookupBuilder.buildFromWhitelists(entry.getValue())));
             } else {
@@ -154,23 +153,6 @@ public boolean needs_score() {
                     return needsScore;
                 }
             };
-            return context.factoryClazz.cast(factory);
-        } else if (context.instanceClazz.equals(ExecutableScript.class)) {
-            Constructor<?> constructor = compile(compiler, scriptName, scriptSource, params);
-
-            ExecutableScript.Factory factory = new ExecutableScript.Factory() {
-                @Override
-                public ExecutableScript newInstance(Map<String, Object> params) {
-                    try {
-                        // a new instance is required for the class bindings model to work correctly
-                        GenericElasticsearchScript newInstance = (GenericElasticsearchScript)constructor.newInstance();
-                        return new ScriptImpl(newInstance, params, null, null);
-                    } catch (InstantiationException | IllegalAccessException | InvocationTargetException e) {
-                        throw new IllegalArgumentException("internal error");
-                    }
-                }
-            };
-
             return context.factoryClazz.cast(factory);
         } else {
             // Check we ourselves are not being called by unprivileged code.

modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptImpl.java

@@ -20,7 +20,6 @@
 package org.elasticsearch.painless;
 
 import org.apache.lucene.index.LeafReaderContext;
-import org.elasticsearch.script.ExecutableScript;
 import org.elasticsearch.script.SearchScript;
 import org.elasticsearch.search.lookup.LeafSearchLookup;
 import org.elasticsearch.search.lookup.SearchLookup;
@@ -31,7 +30,7 @@
 import java.util.function.Function;
 
 /**
- * ScriptImpl can be used as either an {@link ExecutableScript} or a {@link SearchScript}
+ * ScriptImpl can be used as a {@link SearchScript}
  * to run a previously compiled Painless script.
  */
 final class ScriptImpl extends SearchScript {

modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicAPITests.java

@@ -67,7 +67,8 @@ public void testUpdateMapLoadStore() {
         ctx.put("_source", _source);
         params.put("ctx", ctx);
 
-        assertEquals("testvalue", exec("ctx._source['load'].5 = ctx._source['load'].remove('load5')", params, true));
+        assertEquals("testvalue", exec("params.ctx._source['load'].5 = params.ctx._source['load'].remove('load5')",
+            params, true));
     }
 
     /** Test loads and stores with a list */

modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java

@@ -262,19 +262,19 @@ public void testArrayLoopWithoutCounter() {
                               "for (int i = 0; i < array.length; i++) { sum += array[i] } return sum",
                               Collections.emptyMap(),
                               Collections.singletonMap(CompilerSettings.MAX_LOOP_COUNTER, "0"),
-                              null, true
+                              true
        ));
        assertEquals(6L, exec("long sum = 0; long[] array = new long[] { 1, 2, 3 };" +
                              "int i = 0; while (i < array.length) { sum += array[i++] } return sum",
                              Collections.emptyMap(),
                              Collections.singletonMap(CompilerSettings.MAX_LOOP_COUNTER, "0"),
-                             null, true
+                             true
        ));
        assertEquals(6L, exec("long sum = 0; long[] array = new long[] { 1, 2, 3 };" +
                              "int i = 0; do { sum += array[i++] } while (i < array.length); return sum",
                              Collections.emptyMap(),
                              Collections.singletonMap(CompilerSettings.MAX_LOOP_COUNTER, "0"),
-                             null, true
+                             true
        ));
     }
 

modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java

@@ -19,46 +19,51 @@
 
 package org.elasticsearch.painless;
 
-import org.elasticsearch.script.ExecutableScript;
+import org.elasticsearch.painless.spi.Whitelist;
+import org.elasticsearch.script.ScriptContext;
 
 import java.util.Collections;
-import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 public class BindingsTests extends ScriptTestCase {
 
+    public abstract static class BindingsTestScript {
+        public static final String[] PARAMETERS = { "test", "bound" };
+        public abstract int execute(int test, int bound);
+        public interface Factory {
+            BindingsTestScript newInstance();
+        }
+        public static final ScriptContext<Factory> CONTEXT = new ScriptContext<>("bindings_test", Factory.class);
+    }
+
+    @Override
+    protected Map<ScriptContext<?>, List<Whitelist>> scriptContexts() {
+        Map<ScriptContext<?>, List<Whitelist>> contexts = super.scriptContexts();
+        contexts.put(BindingsTestScript.CONTEXT, Whitelist.BASE_WHITELISTS);
+        return contexts;
+    }
+
     public void testBasicBinding() {
         assertEquals(15, exec("testAddWithState(4, 5, 6, 0.0)"));
     }
 
     public void testRepeatedBinding() {
-        String script = "testAddWithState(4, 5, params.test, 0.0)";
-        Map<String, Object> params = new HashMap<>();
-        ExecutableScript.Factory factory = scriptEngine.compile(null, script, ExecutableScript.CONTEXT, Collections.emptyMap());
-        ExecutableScript executableScript = factory.newInstance(params);
+        String script = "testAddWithState(4, 5, test, 0.0)";
+        BindingsTestScript.Factory factory = scriptEngine.compile(null, script, BindingsTestScript.CONTEXT, Collections.emptyMap());
+        BindingsTestScript executableScript = factory.newInstance();
 
-        executableScript.setNextVar("test", 5);
-        assertEquals(14, executableScript.run());
-
-        executableScript.setNextVar("test", 4);
-        assertEquals(13, executableScript.run());
-
-        executableScript.setNextVar("test", 7);
-        assertEquals(16, executableScript.run());
+        assertEquals(14, executableScript.execute(5, 0));
+        assertEquals(13, executableScript.execute(4, 0));
+        assertEquals(16, executableScript.execute(7, 0));
     }
 
     public void testBoundBinding() {
-        String script = "testAddWithState(4, params.bound, params.test, 0.0)";
-        Map<String, Object> params = new HashMap<>();
-        ExecutableScript.Factory factory = scriptEngine.compile(null, script, ExecutableScript.CONTEXT, Collections.emptyMap());
-        ExecutableScript executableScript = factory.newInstance(params);
-
-        executableScript.setNextVar("test", 5);
-        executableScript.setNextVar("bound", 1);
-        assertEquals(10, executableScript.run());
+        String script = "testAddWithState(4, bound, test, 0.0)";
+        BindingsTestScript.Factory factory = scriptEngine.compile(null, script, BindingsTestScript.CONTEXT, Collections.emptyMap());
+        BindingsTestScript executableScript = factory.newInstance();
 
-        executableScript.setNextVar("test", 4);
-        executableScript.setNextVar("bound", 2);
-        assertEquals(9, executableScript.run());
+        assertEquals(10, executableScript.execute(5, 1));
+        assertEquals(9, executableScript.execute(4, 2));
     }
 }

modules/lang-painless/src/test/java/org/elasticsearch/painless/NeedsScoreTests.java

@@ -23,7 +23,6 @@
 import org.elasticsearch.index.IndexService;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.painless.spi.Whitelist;
-import org.elasticsearch.script.ExecutableScript;
 import org.elasticsearch.script.ScriptContext;
 import org.elasticsearch.script.SearchScript;
 import org.elasticsearch.search.lookup.SearchLookup;
@@ -45,7 +44,6 @@ public void testNeedsScores() {
 
         Map<ScriptContext<?>, List<Whitelist>> contexts = new HashMap<>();
         contexts.put(SearchScript.CONTEXT, Whitelist.BASE_WHITELISTS);
-        contexts.put(ExecutableScript.CONTEXT, Whitelist.BASE_WHITELISTS);
         PainlessScriptEngine service = new PainlessScriptEngine(Settings.EMPTY, contexts);
 
         QueryShardContext shardContext = index.newQueryShardContext(0, null, () -> 0, null);