Replace Ingest ScriptContext with Custom Interface (#32003) (#32060)

* Replace Ingest ScriptContext with Custom Interface
* Make org.elasticsearch.ingest.common.ScriptProcessorTests#testScripting more precise
* Don't mock script factory in ScriptProcessorTests
* Adjust mock script plugin in IT for new API

modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java

@@ -26,7 +26,7 @@
 import org.elasticsearch.ingest.AbstractProcessor;
 import org.elasticsearch.ingest.IngestDocument;
 import org.elasticsearch.ingest.Processor;
-import org.elasticsearch.script.ExecutableScript;
+import org.elasticsearch.script.IngestScript;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptException;
 import org.elasticsearch.script.ScriptService;
@@ -71,10 +71,8 @@ public final class ScriptProcessor extends AbstractProcessor {
      */
     @Override
     public void execute(IngestDocument document) {
-        ExecutableScript.Factory factory = scriptService.compile(script, ExecutableScript.INGEST_CONTEXT);
-        ExecutableScript executableScript = factory.newInstance(script.getParams());
-        executableScript.setNextVar("ctx",  document.getSourceAndMetadata());
-        executableScript.run();
+        IngestScript.Factory factory = scriptService.compile(script, IngestScript.CONTEXT);
+        factory.newInstance(script.getParams()).execute(document.getSourceAndMetadata());
     }
 
     @Override
@@ -144,7 +142,7 @@ public ScriptProcessor create(Map<String, Processor.Factory> registry, String pr
 
             // verify script is able to be compiled before successfully creating processor.
             try {
-                scriptService.compile(script, ExecutableScript.INGEST_CONTEXT);
+                scriptService.compile(script, IngestScript.CONTEXT);
             } catch (ScriptException e) {
                 throw newConfigurationException(TYPE, processorTag, scriptPropertyUsed, e);
             }

modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/IngestRestartIT.java

@@ -58,9 +58,7 @@ protected boolean ignoreExternalCluster() {
     public static class CustomScriptPlugin extends MockScriptPlugin {
         @Override
         protected Map<String, Function<Map<String, Object>, Object>> pluginScripts() {
-            return Collections.singletonMap("my_script", script -> {
-                @SuppressWarnings("unchecked")
-                Map<String, Object> ctx = (Map) script.get("ctx");
+            return Collections.singletonMap("my_script", ctx -> {
                 ctx.put("z", 0);
                 return null;
             });

modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java

@@ -19,47 +19,51 @@
 
 package org.elasticsearch.ingest.common;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.ingest.IngestDocument;
 import org.elasticsearch.ingest.RandomDocumentPicks;
-import org.elasticsearch.script.ExecutableScript;
+import org.elasticsearch.script.MockScriptEngine;
 import org.elasticsearch.script.Script;
+import org.elasticsearch.script.ScriptModule;
 import org.elasticsearch.script.ScriptService;
+import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.test.ESTestCase;
 
 import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.core.Is.is;
-import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.doAnswer;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
 
 public class ScriptProcessorTests extends ESTestCase {
 
     public void testScripting() throws Exception {
         int randomBytesIn = randomInt();
         int randomBytesOut = randomInt();
         int randomBytesTotal = randomBytesIn + randomBytesOut;
-
-        ScriptService scriptService = mock(ScriptService.class);
-        Script script = mockScript("_script");
-        ExecutableScript.Factory factory = mock(ExecutableScript.Factory.class);
-        ExecutableScript executableScript = mock(ExecutableScript.class);
-        when(scriptService.compile(script, ExecutableScript.INGEST_CONTEXT)).thenReturn(factory);
-        when(factory.newInstance(any())).thenReturn(executableScript);
+        String scriptName = "script";
+        ScriptService scriptService = new ScriptService(Settings.builder().build(),
+            Collections.singletonMap(
+                Script.DEFAULT_SCRIPT_LANG, new MockScriptEngine(
+                    Script.DEFAULT_SCRIPT_LANG,
+                    Collections.singletonMap(
+                        scriptName, ctx -> {
+                            ctx.put("bytes_total", randomBytesTotal);
+                            return null;
+                        }
+                    )
+                )
+            ),
+            new HashMap<>(ScriptModule.CORE_CONTEXTS)
+        );
+        Script script = new Script(ScriptType.INLINE, Script.DEFAULT_SCRIPT_LANG, scriptName, Collections.emptyMap());
 
         Map<String, Object> document = new HashMap<>();
         document.put("bytes_in", randomInt());
         document.put("bytes_out", randomInt());
         IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
 
-        doAnswer(invocationOnMock ->  {
-            ingestDocument.setFieldValue("bytes_total", randomBytesTotal);
-            return null;
-        }).when(executableScript).run();
-
         ScriptProcessor processor = new ScriptProcessor(randomAlphaOfLength(10), script, scriptService);
 
         processor.execute(ingestDocument);

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

@@ -50,5 +50,4 @@ interface Factory {
     // TODO: remove these once each has its own script interface
     ScriptContext<Factory> AGGS_CONTEXT = new ScriptContext<>("aggs_executable", Factory.class);
     ScriptContext<Factory> UPDATE_CONTEXT = new ScriptContext<>("update", Factory.class);
-    ScriptContext<Factory> INGEST_CONTEXT = new ScriptContext<>("ingest", Factory.class);
 }

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

@@ -0,0 +1,52 @@
+
+/*
+ * 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;
+
+/**
+ * A script used by the Ingest Script Processor.
+ */
+public abstract class IngestScript {
+
+    public static final String[] PARAMETERS = { "ctx" };
+
+    /** The context used to compile {@link IngestScript} factories. */
+    public static final ScriptContext<Factory> CONTEXT = new ScriptContext<>("ingest", Factory.class);
+
+    /** The generic runtime parameters for the script. */
+    private final Map<String, Object> params;
+
+    public IngestScript(Map<String, Object> params) {
+        this.params = params;
+    }
+
+    /** Return the parameters for this script. */
+    public Map<String, Object> getParams() {
+        return params;
+    }
+
+    public abstract void execute(Map<String, Object> ctx);
+
+    public interface Factory {
+        IngestScript newInstance(Map<String, Object> params);
+    }
+}

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

@@ -48,7 +48,7 @@ public class ScriptModule {
             ExecutableScript.CONTEXT,
             ExecutableScript.AGGS_CONTEXT,
             ExecutableScript.UPDATE_CONTEXT,
-            ExecutableScript.INGEST_CONTEXT,
+            IngestScript.CONTEXT,
             FilterScript.CONTEXT,
             SimilarityScript.CONTEXT,
             SimilarityWeightScript.CONTEXT,

server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java

@@ -168,7 +168,7 @@ public void testAllowAllScriptContextSettings() throws IOException {
         assertCompileAccepted("painless", "script", ScriptType.INLINE, SearchScript.CONTEXT);
         assertCompileAccepted("painless", "script", ScriptType.INLINE, SearchScript.AGGS_CONTEXT);
         assertCompileAccepted("painless", "script", ScriptType.INLINE, ExecutableScript.UPDATE_CONTEXT);
-        assertCompileAccepted("painless", "script", ScriptType.INLINE, ExecutableScript.INGEST_CONTEXT);
+        assertCompileAccepted("painless", "script", ScriptType.INLINE, IngestScript.CONTEXT);
     }
 
     public void testAllowSomeScriptTypeSettings() throws IOException {
@@ -209,13 +209,13 @@ public void testAllowNoScriptContextSettings() throws IOException {
     }
 
     public void testCompileNonRegisteredContext() throws IOException {
-        contexts.remove(ExecutableScript.INGEST_CONTEXT.name);
+        contexts.remove(IngestScript.CONTEXT.name);
         buildScriptService(Settings.EMPTY);
 
         String type = scriptEngine.getType();
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
-            scriptService.compile(new Script(ScriptType.INLINE, type, "test", Collections.emptyMap()), ExecutableScript.INGEST_CONTEXT));
-        assertThat(e.getMessage(), containsString("script context [" + ExecutableScript.INGEST_CONTEXT.name + "] not supported"));
+            scriptService.compile(new Script(ScriptType.INLINE, type, "test", Collections.emptyMap()), IngestScript.CONTEXT));
+        assertThat(e.getMessage(), containsString("script context [" + IngestScript.CONTEXT.name + "] not supported"));
     }
 
     public void testCompileCountedInCompilationStats() throws IOException {

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

@@ -88,6 +88,14 @@ public <T> T compile(String name, String source, ScriptContext<T> context, Map<S
         } 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) {
+                @Override
+                public void execute(Map<String, Object> ctx) {
+                    script.apply(ctx);
+                }
+            };
+            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