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/ExpressionExecutableScript.java

@@ -1,88 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.script.expression;
-
-import org.apache.lucene.expressions.Expression;
-import org.elasticsearch.script.ExecutableScript;
-import org.elasticsearch.script.GeneralScriptException;
-
-import java.util.HashMap;
-import java.util.Map;
-
-/**
- * A bridge to evaluate an {@link Expression} against a map of variables in the context
- * of an {@link ExecutableScript}.
- */
-public class ExpressionExecutableScript implements ExecutableScript {
-    public final Expression expression;
-    public final Map<String, ReplaceableConstDoubleValues> functionValuesMap;
-    public final ReplaceableConstDoubleValues[] functionValuesArray;
-
-    public ExpressionExecutableScript(Expression expression, Map<String, Object> vars) {
-        this.expression = expression;
-        int functionValuesLength = expression.variables.length;
-
-        if (vars.size() != functionValuesLength) {
-            throw new GeneralScriptException("Error using " + expression + ". " +
-                    "The number of variables in an executable expression script [" +
-                    functionValuesLength + "] must match the number of variables in the variable map" +
-                    " [" + vars.size() + "].");
-        }
-
-        functionValuesArray = new ReplaceableConstDoubleValues[functionValuesLength];
-        functionValuesMap = new HashMap<>();
-
-        for (int functionValuesIndex = 0; functionValuesIndex < functionValuesLength; ++functionValuesIndex) {
-            String variableName = expression.variables[functionValuesIndex];
-            functionValuesArray[functionValuesIndex] = new ReplaceableConstDoubleValues();
-            functionValuesMap.put(variableName, functionValuesArray[functionValuesIndex]);
-        }
-
-        for (String varsName : vars.keySet()) {
-            setNextVar(varsName, vars.get(varsName));
-        }
-    }
-
-    @Override
-    public void setNextVar(String name, Object value) {
-        if (functionValuesMap.containsKey(name)) {
-            if (value instanceof Number) {
-                double doubleValue = ((Number)value).doubleValue();
-                functionValuesMap.get(name).setValue(doubleValue);
-            } else {
-                throw new GeneralScriptException("Error using " + expression + ". " +
-                        "Executable expressions scripts can only process numbers." +
-                        "  The variable [" + name + "] is not a number.");
-            }
-        } else {
-            throw new GeneralScriptException("Error using " + expression + ". " +
-                    "The variable [" + name + "] does not exist in the executable expressions script.");
-        }
-    }
-
-    @Override
-    public Object run() {
-        try {
-            return expression.evaluate(functionValuesArray);
-        } catch (Exception exception) {
-            throw new GeneralScriptException("Error evaluating " + expression, exception);
-        }
-    }
-}

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

@@ -41,7 +41,6 @@ import org.elasticsearch.index.mapper.MapperService;
 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 @@ public class ExpressionScriptEngine extends AbstractComponent implements ScriptE
         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.QueryBuilders;
 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 class MoreExpressionTests extends ESIntegTestCase {
                 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.common.settings.Settings;
 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 final class PainlessScriptEngine extends AbstractComponent implements Scr
 
         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 final class PainlessScriptEngine extends AbstractComponent implements Scr
                     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.DoubleSupplier;
 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 class BasicAPITests extends ScriptTestCase {
         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 class BasicStatementTests extends ScriptTestCase {
                               "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)";
+        String script = "testAddWithState(4, 5, test, 0.0)";
-        Map<String, Object> params = new HashMap<>();
+        BindingsTestScript.Factory factory = scriptEngine.compile(null, script, BindingsTestScript.CONTEXT, Collections.emptyMap());
-        ExecutableScript.Factory factory = scriptEngine.compile(null, script, ExecutableScript.CONTEXT, Collections.emptyMap());
+        BindingsTestScript executableScript = factory.newInstance();
-        ExecutableScript executableScript = factory.newInstance(params);
 
-        executableScript.setNextVar("test", 5);
+        assertEquals(14, executableScript.execute(5, 0));
-        assertEquals(14, executableScript.run());
+        assertEquals(13, executableScript.execute(4, 0));
-
+        assertEquals(16, executableScript.execute(7, 0));
-        executableScript.setNextVar("test", 4);
-        assertEquals(13, executableScript.run());
-
-        executableScript.setNextVar("test", 7);
-        assertEquals(16, executableScript.run());
     }
 
     public void testBoundBinding() {
-        String script = "testAddWithState(4, params.bound, params.test, 0.0)";
+        String script = "testAddWithState(4, bound, test, 0.0)";
-        Map<String, Object> params = new HashMap<>();
+        BindingsTestScript.Factory factory = scriptEngine.compile(null, script, BindingsTestScript.CONTEXT, Collections.emptyMap());
-        ExecutableScript.Factory factory = scriptEngine.compile(null, script, ExecutableScript.CONTEXT, Collections.emptyMap());
+        BindingsTestScript executableScript = factory.newInstance();
-        ExecutableScript executableScript = factory.newInstance(params);
 
-        executableScript.setNextVar("test", 5);
+        assertEquals(10, executableScript.execute(5, 1));
-        executableScript.setNextVar("bound", 1);
+        assertEquals(9, executableScript.execute(4, 2));
-        assertEquals(10, executableScript.run());
-
-        executableScript.setNextVar("test", 4);
-        executableScript.setNextVar("bound", 2);
-        assertEquals(9, executableScript.run());
     }
 }

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

@@ -23,7 +23,6 @@ import org.elasticsearch.common.settings.Settings;
 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 class NeedsScoreTests extends ESSingleNodeTestCase {
 
         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);

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

@@ -1,96 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.painless;
-
-import java.util.Collections;
-import java.util.HashMap;
-
-/** Tests for special reserved words such as _score */
-public class ReservedWordTests extends ScriptTestCase {
-
-    /** check that we can't declare a variable of _score, its really reserved! */
-    public void testScoreVar() {
-        IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, () -> {
-            exec("int _score = 5; return _score;");
-        });
-        assertTrue(expected.getMessage().contains("Variable [_score] is already defined"));
-    }
-
-    /** check that we can't write to _score, its read-only! */
-    public void testScoreStore() {
-        IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, () -> {
-            exec("_score = 5; return _score;");
-        });
-        assertTrue(expected.getMessage().contains("Variable [_score] is read-only"));
-    }
-
-    /** check that we can't declare a variable of doc, its really reserved! */
-    public void testDocVar() {
-        IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, () -> {
-            exec("int doc = 5; return doc;");
-        });
-        assertTrue(expected.getMessage().contains("Variable [doc] is already defined"));
-    }
-
-    /** check that we can't write to doc, its read-only! */
-    public void testDocStore() {
-        IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, () -> {
-            exec("doc = 5; return doc;");
-        });
-        assertTrue(expected.getMessage().contains("Variable [doc] is read-only"));
-    }
-
-    /** check that we can't declare a variable of ctx, its really reserved! */
-    public void testCtxVar() {
-        IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, () -> {
-            exec("int ctx = 5; return ctx;");
-        });
-        assertTrue(expected.getMessage().contains("Variable [ctx] is already defined"));
-    }
-
-    /** check that we can't write to ctx, its read-only! */
-    public void testCtxStore() {
-        IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, () -> {
-            exec("ctx = 5; return ctx;");
-        });
-        assertTrue(expected.getMessage().contains("Variable [ctx] is read-only"));
-    }
-
-    /** check that we can modify its contents though */
-    public void testCtxStoreMap() {
-        assertEquals(5, exec("ctx.foo = 5; return ctx.foo;", Collections.singletonMap("ctx", new HashMap<String,Object>()), true));
-    }
-
-    /** check that we can't declare a variable of _value, its really reserved! */
-    public void testAggregationValueVar() {
-        IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, () -> {
-            exec("int _value = 5; return _value;");
-        });
-        assertTrue(expected.getMessage().contains("Variable [_value] is already defined"));
-    }
-
-    /** check that we can't write to _value, its read-only! */
-    public void testAggregationValueStore() {
-        IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, () -> {
-            exec("_value = 5; return _value;");
-        });
-        assertTrue(expected.getMessage().contains("Variable [_value] is read-only"));
-    }
-}

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

@@ -1,72 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.painless;
-
-import org.apache.lucene.search.Scorable;
-
-import java.util.Collections;
-
-public class ScoreTests extends ScriptTestCase {
-
-    /** Most of a dummy scorer impl that requires overriding just score(). */
-    abstract class MockScorer extends Scorable {
-        @Override
-        public int docID() {
-            return 0;
-        }
-    }
-
-    public void testScoreWorks() {
-        assertEquals(2.5, exec("_score", Collections.emptyMap(), Collections.emptyMap(),
-            new MockScorer() {
-                @Override
-                public float score() {
-                    return 2.5f;
-                }
-            },
-            true));
-    }
-
-    public void testScoreNotUsed() {
-        assertEquals(3.5, exec("3.5", Collections.emptyMap(), Collections.emptyMap(),
-            new MockScorer() {
-                @Override
-                public float score() {
-                    throw new AssertionError("score() should not be called");
-                }
-            },
-            true));
-    }
-
-    public void testScoreCached() {
-        assertEquals(9.0, exec("_score + _score", Collections.emptyMap(), Collections.emptyMap(),
-            new MockScorer() {
-                private boolean used = false;
-                @Override
-                public float score() {
-                    if (used == false) {
-                        return 4.5f;
-                    }
-                    throw new AssertionError("score() should not be called twice");
-                }
-            },
-            true));
-    }
-}

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

@@ -19,10 +19,7 @@
 
 package org.elasticsearch.painless;
 
-import org.elasticsearch.script.ExecutableScript;
-
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -73,37 +70,4 @@ public class ScriptEngineTests extends ScriptTestCase {
 
         assertEquals("value1", exec("return params.l.3.prop1;", vars, true));
     }
-
-    public void testChangingVarsCrossExecution1() {
-        Map<String, Object> vars = new HashMap<>();
-        Map<String, Object> ctx = new HashMap<>();
-        vars.put("ctx", ctx);
-
-        ExecutableScript.Factory factory =
-            scriptEngine.compile(null, "return ctx.value;", ExecutableScript.CONTEXT, Collections.emptyMap());
-        ExecutableScript script = factory.newInstance(vars);
-
-        ctx.put("value", 1);
-        Object o = script.run();
-        assertEquals(1, ((Number) o).intValue());
-
-        ctx.put("value", 2);
-        o = script.run();
-        assertEquals(2, ((Number) o).intValue());
-    }
-
-    public void testChangingVarsCrossExecution2() {
-        Map<String, Object> vars = new HashMap<>();
-        ExecutableScript.Factory factory =
-            scriptEngine.compile(null, "return params['value'];", ExecutableScript.CONTEXT, Collections.emptyMap());
-        ExecutableScript script = factory.newInstance(vars);
-
-        script.setNextVar("value", 1);
-        Object value = script.run();
-        assertEquals(1, ((Number)value).intValue());
-
-        script.setNextVar("value", 2);
-        value = script.run();
-        assertEquals(2, ((Number)value).intValue());
-    }
 }

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

@@ -20,24 +20,23 @@
 package org.elasticsearch.painless;
 
 import junit.framework.AssertionFailedError;
-import org.apache.lucene.search.Scorable;
-import org.elasticsearch.common.lucene.ScorerAware;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.painless.antlr.Walker;
 import org.elasticsearch.painless.lookup.PainlessLookup;
 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.ScriptException;
 import org.elasticsearch.script.SearchScript;
 import org.elasticsearch.test.ESTestCase;
 import org.junit.Before;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import static org.elasticsearch.painless.PainlessExecuteAction.PainlessTestScript;
 import static org.elasticsearch.painless.node.SSource.MainMethodReserved;
 import static org.hamcrest.Matchers.hasSize;
 
@@ -69,7 +68,7 @@ public abstract class ScriptTestCase extends ESTestCase {
     protected Map<ScriptContext<?>, List<Whitelist>> scriptContexts() {
         Map<ScriptContext<?>, List<Whitelist>> contexts = new HashMap<>();
         contexts.put(SearchScript.CONTEXT, Whitelist.BASE_WHITELISTS);
-        contexts.put(ExecutableScript.CONTEXT, Whitelist.BASE_WHITELISTS);
+        contexts.put(PainlessTestScript.CONTEXT, Whitelist.BASE_WHITELISTS);
         return contexts;
     }
 
@@ -87,11 +86,11 @@ public abstract class ScriptTestCase extends ESTestCase {
     public Object exec(String script, Map<String, Object> vars, boolean picky) {
         Map<String,String> compilerSettings = new HashMap<>();
         compilerSettings.put(CompilerSettings.INITIAL_CALL_SITE_DEPTH, random().nextBoolean() ? "0" : "10");
-        return exec(script, vars, compilerSettings, null, picky);
+        return exec(script, vars, compilerSettings, picky);
     }
 
     /** Compiles and returns the result of {@code script} with access to {@code vars} and compile-time parameters */
-    public Object exec(String script, Map<String, Object> vars, Map<String,String> compileParams, Scorable scorer, boolean picky) {
+    public Object exec(String script, Map<String, Object> vars, Map<String,String> compileParams, boolean picky) {
         // test for ambiguity errors before running the actual script if picky is true
         if (picky) {
             ScriptClassInfo scriptClassInfo = new ScriptClassInfo(PAINLESS_LOOKUP, GenericElasticsearchScript.class);
@@ -99,15 +98,12 @@ public abstract class ScriptTestCase extends ESTestCase {
             pickySettings.setPicky(true);
             pickySettings.setRegexesEnabled(CompilerSettings.REGEX_ENABLED.get(scriptEngineSettings()));
             Walker.buildPainlessTree(
-                    scriptClassInfo, new MainMethodReserved(), getTestName(), script, pickySettings, PAINLESS_LOOKUP, null);
+                scriptClassInfo, new MainMethodReserved(), getTestName(), script, pickySettings, PAINLESS_LOOKUP, null);
         }
         // test actual script execution
-        ExecutableScript.Factory factory = scriptEngine.compile(null, script, ExecutableScript.CONTEXT, compileParams);
+        PainlessTestScript.Factory factory = scriptEngine.compile(null, script, PainlessTestScript.CONTEXT, compileParams);
-        ExecutableScript executableScript = factory.newInstance(vars);
+        PainlessTestScript testScript = factory.newInstance(vars == null ? Collections.emptyMap() : vars);
-        if (scorer != null) {
+        return testScript.execute();
-            ((ScorerAware)executableScript).setScorer(scorer);
-        }
-        return executableScript.run();
     }
 
     /**

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

@@ -233,7 +233,7 @@ public class StringTests extends ScriptTestCase {
         ctx.put("_id", "somerandomid");
         params.put("ctx", ctx);
 
-        assertEquals("somerandomid.somerandomid", exec("ctx._id += '.' + ctx._id", params, false));
+        assertEquals("somerandomid.somerandomid", exec("params.ctx._id += '.' + params.ctx._id", params, false));
         assertEquals("somerandomid.somerandomid", exec("String x = 'somerandomid'; x += '.' + x"));
         assertEquals("somerandomid.somerandomid", exec("def x = 'somerandomid'; x += '.' + x"));
     }

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

@@ -130,7 +130,7 @@ public class WhenThingsGoWrongTests extends ScriptTestCase {
 
     public void testBogusParameter() {
         IllegalArgumentException expected = expectThrows(IllegalArgumentException.class, () -> {
-            exec("return 5;", null, Collections.singletonMap("bogusParameterKey", "bogusParameterValue"), null, true);
+            exec("return 5;", null, Collections.singletonMap("bogusParameterKey", "bogusParameterValue"), true);
         });
         assertTrue(expected.getMessage().contains("Unrecognized compile-time parameter"));
     }
@@ -253,7 +253,7 @@ public class WhenThingsGoWrongTests extends ScriptTestCase {
     public void testRCurlyNotDelim() {
         IllegalArgumentException e = expectScriptThrows(IllegalArgumentException.class, () -> {
             // We don't want PICKY here so we get the normal error message
-            exec("def i = 1} return 1", emptyMap(), emptyMap(), null, false);
+            exec("def i = 1} return 1", emptyMap(), emptyMap(), false);
         });
         assertEquals("unexpected token ['}'] was expecting one of [{<EOF>, ';'}].", e.getMessage());
     }
@@ -285,7 +285,7 @@ public class WhenThingsGoWrongTests extends ScriptTestCase {
 
     public void testCanNotOverrideRegexEnabled() {
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
-                () -> exec("", null, singletonMap(CompilerSettings.REGEX_ENABLED.getKey(), "true"), null, false));
+                () -> exec("", null, singletonMap(CompilerSettings.REGEX_ENABLED.getKey(), "true"), false));
         assertEquals("[painless.regex.enabled] can only be set on node startup.", e.getMessage());
     }
 

modules/reindex/src/test/java/org/elasticsearch/index/reindex/AbstractAsyncBulkByScrollActionScriptTestCase.java

@@ -20,11 +20,10 @@
 package org.elasticsearch.index.reindex;
 
 import org.elasticsearch.action.ActionRequest;
-import org.elasticsearch.index.reindex.AbstractAsyncBulkByScrollAction.OpType;
-import org.elasticsearch.index.reindex.AbstractAsyncBulkByScrollAction.RequestWrapper;
 import org.elasticsearch.action.delete.DeleteRequest;
 import org.elasticsearch.action.index.IndexRequest;
-import org.elasticsearch.script.ExecutableScript;
+import org.elasticsearch.index.reindex.AbstractAsyncBulkByScrollAction.OpType;
+import org.elasticsearch.index.reindex.AbstractAsyncBulkByScrollAction.RequestWrapper;
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.script.UpdateScript;
 import org.junit.Before;
@@ -62,8 +61,6 @@ public abstract class AbstractAsyncBulkByScrollActionScriptTestCase<
                 scriptBody.accept(ctx);
             }
         };;
-        ExecutableScript simpleExecutableScript = new SimpleExecutableScript(scriptBody);
-        when(scriptService.compile(any(), eq(ExecutableScript.CONTEXT))).thenReturn(params -> simpleExecutableScript);
         when(scriptService.compile(any(), eq(UpdateScript.CONTEXT))).thenReturn(factory);
         AbstractAsyncBulkByScrollAction<Request> action = action(scriptService, request().setScript(mockScript("")));
         RequestWrapper<?> result = action.buildScriptApplier().apply(AbstractAsyncBulkByScrollAction.wrap(index), doc);

modules/reindex/src/test/java/org/elasticsearch/index/reindex/SimpleExecutableScript.java

@@ -1,51 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.index.reindex;
-
-import org.elasticsearch.script.ExecutableScript;
-
-import java.util.Map;
-import java.util.function.Consumer;
-
-
-public class SimpleExecutableScript implements ExecutableScript {
-    private final Consumer<Map<String, Object>> script;
-    private Map<String, Object> ctx;
-
-    public SimpleExecutableScript(Consumer<Map<String, Object>> script) {
-        this.script = script;
-    }
-
-    @Override
-    public Object run() {
-        script.accept(ctx);
-        return null;
-    }
-
-    @Override
-    @SuppressWarnings("unchecked")
-    public void setNextVar(String name, Object value) {
-        if ("ctx".equals(name)) {
-            ctx = (Map<String, Object>) value;
-        } else {
-            throw new IllegalArgumentException("Unsupported var [" + name + "]");
-        }
-    }
-}

server/src/main/java/org/elasticsearch/script/ExecutableScript.java

@@ -1,49 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.script;
-
-import java.util.Map;
-
-/**
- * An executable script, can't be used concurrently.
- */
-public interface ExecutableScript {
-
-    /**
-     * Sets a runtime script parameter.
-     * <p>
-     * Note that this method may be slow, involving put() and get() calls
-     * to a hashmap or similar.
-     * @param name parameter name
-     * @param value parameter value
-     */
-    void setNextVar(String name, Object value);
-
-    /**
-     * Executes the script.
-     */
-    Object run();
-
-    interface Factory {
-        ExecutableScript newInstance(Map<String, Object> params);
-    }
-
-    ScriptContext<Factory> CONTEXT = new ScriptContext<>("executable", Factory.class);
-}

server/src/main/java/org/elasticsearch/script/ScriptModule.java

@@ -45,7 +45,6 @@ public class ScriptModule {
             ScoreScript.CONTEXT,
             SearchScript.SCRIPT_SORT_CONTEXT,
             TermsSetQueryScript.CONTEXT,
-            ExecutableScript.CONTEXT,
             UpdateScript.CONTEXT,
             BucketAggregationScript.CONTEXT,
             BucketAggregationSelectorScript.CONTEXT,

server/src/main/java/org/elasticsearch/script/SearchScript.java

@@ -41,7 +41,7 @@ import java.util.Map;
  *     <li>Call one of the {@code run} methods: {@link #run()}, {@link #runAsDouble()}, or {@link #runAsLong()}</li>
  * </ol>
  */
-public abstract class SearchScript implements ScorerAware, ExecutableScript {
+public abstract class SearchScript implements ScorerAware {
 
     /** The generic runtime parameters for the script. */
     private final Map<String, Object> params;
@@ -112,7 +112,6 @@ public abstract class SearchScript implements ScorerAware, ExecutableScript {
         setNextVar("_value", value);
     }
 
-    @Override
     public void setNextVar(String field, Object value) {}
 
     /** Return the result as a long. This is used by aggregation scripts over long fields. */
@@ -120,7 +119,6 @@ public abstract class SearchScript implements ScorerAware, ExecutableScript {
         throw new UnsupportedOperationException("runAsLong is not implemented");
     }
 
-    @Override
     public Object run() {
         return runAsDouble();
     }

test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java

@@ -105,16 +105,14 @@ public class MockScriptEngine implements ScriptEngine {
                 }
             };
             return context.factoryClazz.cast(factory);
-        } else if (context.instanceClazz.equals(ExecutableScript.class)) {
-            ExecutableScript.Factory factory = mockCompiled::createExecutableScript;
-            return context.factoryClazz.cast(factory);
         } else if (context.instanceClazz.equals(IngestScript.class)) {
-            IngestScript.Factory factory = parameters -> new IngestScript(parameters) {
+            IngestScript.Factory factory = vars ->
-                @Override
+                new IngestScript(vars) {
-                public void execute(Map<String, Object> ctx) {
+                    @Override
-                    script.apply(ctx);
+                    public void execute(Map<String, Object> ctx) {
-                }
+                        script.apply(ctx);
-            };
+                    }
+                };
             return context.factoryClazz.cast(factory);
         } else if (context.instanceClazz.equals(IngestConditionalScript.class)) {
             IngestConditionalScript.Factory factory = parameters -> new IngestConditionalScript(parameters) {
@@ -167,16 +165,17 @@ public class MockScriptEngine implements ScriptEngine {
             return context.factoryClazz.cast(factory);
         } else if (context.instanceClazz.equals(TemplateScript.class)) {
             TemplateScript.Factory factory = vars -> {
-                // TODO: need a better way to implement all these new contexts
+                Map<String, Object> varsWithParams = new HashMap<>();
-                // this is just a shim to act as an executable script just as before
+                if (vars != null) {
-                ExecutableScript execScript = mockCompiled.createExecutableScript(vars);
+                    varsWithParams.put("params", vars);
-                    return new TemplateScript(vars) {
+                }
-                        @Override
+                return new TemplateScript(vars) {
-                        public String execute() {
+                    @Override
-                            return (String) execScript.run();
+                    public String execute() {
-                        }
+                        return (String) script.apply(varsWithParams);
-                    };
+                    }
                 };
+            };
             return context.factoryClazz.cast(factory);
         } else if (context.instanceClazz.equals(FilterScript.class)) {
             FilterScript.Factory factory = mockCompiled::createFilterScript;
@@ -231,19 +230,6 @@ public class MockScriptEngine implements ScriptEngine {
             return name;
         }
 
-        public ExecutableScript createExecutableScript(Map<String, Object> params) {
-            Map<String, Object> context = new HashMap<>();
-            if (options != null) {
-                context.putAll(options); // TODO: remove this once scripts know to look for options under options key
-                context.put("options", options);
-            }
-            if (params != null) {
-                context.putAll(params); // TODO: remove this once scripts know to look for params under params key
-                context.put("params", params);
-            }
-            return new MockExecutableScript(context, script != null ? script : ctx -> source);
-        }
-
         public SearchScript.LeafFactory createSearchScript(Map<String, Object> params, SearchLookup lookup) {
             Map<String, Object> context = new HashMap<>();
             if (options != null) {
@@ -294,27 +280,6 @@ public class MockScriptEngine implements ScriptEngine {
         }
     }
 
-    public class MockExecutableScript implements ExecutableScript {
-
-        private final Function<Map<String, Object>, Object> script;
-        private final Map<String, Object> vars;
-
-        public MockExecutableScript(Map<String, Object> vars, Function<Map<String, Object>, Object> script) {
-            this.vars = vars;
-            this.script = script;
-        }
-
-        @Override
-        public void setNextVar(String name, Object value) {
-            vars.put(name, value);
-        }
-
-        @Override
-        public Object run() {
-            return script.apply(vars);
-        }
-    }
-
     public class MockSearchScript implements SearchScript.LeafFactory {
 
         private final Function<Map<String, Object>, Object> script;

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/monitoring/test/MockPainlessScriptEngine.java

@@ -6,7 +6,6 @@
 package org.elasticsearch.xpack.core.monitoring.test;
 
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.script.ExecutableScript;
 import org.elasticsearch.script.MockScriptEngine;
 import org.elasticsearch.script.MockScriptPlugin;
 import org.elasticsearch.script.ScriptContext;
@@ -45,9 +44,7 @@ public class MockPainlessScriptEngine extends MockScriptEngine {
     @Override
     public <T> T compile(String name, String script, ScriptContext<T> context, Map<String, String> options) {
         MockCompiledScript compiledScript = new MockCompiledScript(name, options, script, p -> script);
-        if (context.instanceClazz.equals(ExecutableScript.class)) {
+        if (context.instanceClazz.equals(SearchScript.class)) {
-            return context.factoryClazz.cast((ExecutableScript.Factory) compiledScript::createExecutableScript);
-        } else if (context.instanceClazz.equals(SearchScript.class)) {
             return context.factoryClazz.cast((SearchScript.Factory) compiledScript::createSearchScript);
         }
         throw new IllegalArgumentException("mock painless does not know how to handle context [" + context.name + "]");