Scripts: Remove unnecessary executable shortcut (#24264)

ScriptService has two executable methods, one which takes a
CompiledScript, which is similar to search, and one that takes a raw
Script and both compiles and returns an ExecutableScript for it. The
latter is not needed, and the call sites which used one or the other
were mixed. This commit removes the extra executable method in favor of
callers first calling compile, then executable.

core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java

@@ -43,6 +43,7 @@ import org.elasticsearch.index.mapper.ParentFieldMapper;
 import org.elasticsearch.index.mapper.RoutingFieldMapper;
 import org.elasticsearch.index.shard.IndexShard;
 import org.elasticsearch.index.shard.ShardId;
+import org.elasticsearch.script.CompiledScript;
 import org.elasticsearch.script.ExecutableScript;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptContext;
@@ -213,7 +214,8 @@ public class UpdateHelper extends AbstractComponent {
     private Map<String, Object> executeScript(Script script, Map<String, Object> ctx) {
         try {
             if (scriptService != null) {
-                ExecutableScript executableScript = scriptService.executable(script, ScriptContext.Standard.UPDATE);
+                CompiledScript compiledScript = scriptService.compile(script, ScriptContext.Standard.UPDATE);
+                ExecutableScript executableScript = scriptService.executable(compiledScript, script.getParams());
                 executableScript.setNextVar("ctx", ctx);
                 executableScript.run();
             }

core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java

@@ -25,6 +25,7 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.mapper.MapperService;
+import org.elasticsearch.script.CompiledScript;
 import org.elasticsearch.script.ExecutableScript;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptContext;
@@ -104,7 +105,8 @@ public class QueryRewriteContext {
     }
 
     public BytesReference getTemplateBytes(Script template) {
-        ExecutableScript executable = scriptService.executable(template, ScriptContext.Standard.SEARCH);
+        CompiledScript compiledTemplate = scriptService.compile(template, ScriptContext.Standard.SEARCH);
+        ExecutableScript executable = scriptService.executable(compiledTemplate, template.getParams());
         return (BytesReference) executable.run();
     }
 }

core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java

@@ -351,7 +351,8 @@ public class QueryShardContext extends QueryRewriteContext {
      */
     public final ExecutableScript getExecutableScript(Script script, ScriptContext context) {
         failIfFrozen();
-        return scriptService.executable(script, context);
+        CompiledScript compiledScript = scriptService.compile(script, context);
+        return scriptService.executable(compiledScript, script.getParams());
     }
 
     /**

core/src/main/java/org/elasticsearch/script/ScriptService.java

@@ -469,13 +469,6 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
         }
     }
 
-    /**
-     * Compiles (or retrieves from cache) and executes the provided script
-     */
-    public ExecutableScript executable(Script script, ScriptContext scriptContext) {
-        return executable(compile(script, scriptContext), script.getParams());
-    }
-
     /**
      * Executes a previously compiled script provided as an argument
      */

core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java

@@ -28,6 +28,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.query.QueryParseContext;
 import org.elasticsearch.index.query.QueryShardException;
+import org.elasticsearch.script.CompiledScript;
 import org.elasticsearch.script.ExecutableScript;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptContext;
@@ -92,7 +93,8 @@ public class ScriptHeuristic extends SignificanceHeuristic {
 
     @Override
     public SignificanceHeuristic rewrite(InternalAggregation.ReduceContext context) {
-        return new ExecutableScriptHeuristic(script, context.scriptService().executable(script, ScriptContext.Standard.AGGS));
+        CompiledScript compiledScript = context.scriptService().compile(script, ScriptContext.Standard.AGGS);
+        return new ExecutableScriptHeuristic(script, context.scriptService().executable(compiledScript, script.getParams()));
     }
 
     @Override

core/src/test/java/org/elasticsearch/script/NativeScriptTests.java

@@ -52,8 +52,9 @@ public class NativeScriptTests extends ESTestCase {
         List<Setting<?>> scriptSettings = scriptModule.getSettings();
         scriptSettings.add(InternalSettingsPlugin.VERSION_CREATED);
 
-        ExecutableScript executable = scriptModule.getScriptService().executable(
+        Script script = new Script(ScriptType.INLINE, NativeScriptEngineService.NAME, "my", Collections.emptyMap());
-                new Script(ScriptType.INLINE, NativeScriptEngineService.NAME, "my", Collections.emptyMap()), ScriptContext.Standard.SEARCH);
+        CompiledScript compiledScript = scriptModule.getScriptService().compile(script, ScriptContext.Standard.SEARCH);
+        ExecutableScript executable = scriptModule.getScriptService().executable(compiledScript, script.getParams());
         assertThat(executable.run().toString(), equalTo("test"));
     }
 

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

@@ -346,7 +346,9 @@ public class ScriptServiceTests extends ESTestCase {
 
     public void testExecutableCountedInCompilationStats() throws IOException {
         buildScriptService(Settings.EMPTY);
-        scriptService.executable(new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()), randomFrom(scriptContexts));
+        Script script = new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap());
+        CompiledScript compiledScript = scriptService.compile(script, randomFrom(scriptContexts));
+        scriptService.executable(compiledScript, script.getParams());
         assertEquals(1L, scriptService.stats().getCompilations());
     }
 
@@ -371,8 +373,9 @@ public class ScriptServiceTests extends ESTestCase {
         builder.put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getKey(), 1);
         builder.put("script.inline", "true");
         buildScriptService(builder.build());
-        scriptService.executable(new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()), randomFrom(scriptContexts));
+        Script script = new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap());
-        scriptService.executable(new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()), randomFrom(scriptContexts));
+        scriptService.compile(script, randomFrom(scriptContexts));
+        scriptService.compile(script, randomFrom(scriptContexts));
         assertEquals(1L, scriptService.stats().getCompilations());
     }
 
@@ -394,8 +397,8 @@ public class ScriptServiceTests extends ESTestCase {
         builder.put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getKey(), 1);
         builder.put("script.inline", "true");
         buildScriptService(builder.build());
-        scriptService.executable(new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()), randomFrom(scriptContexts));
+        scriptService.compile(new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()), randomFrom(scriptContexts));
-        scriptService.executable(new Script(ScriptType.INLINE, "test", "2+2", Collections.emptyMap()), randomFrom(scriptContexts));
+        scriptService.compile(new Script(ScriptType.INLINE, "test", "2+2", Collections.emptyMap()), randomFrom(scriptContexts));
         assertEquals(2L, scriptService.stats().getCompilations());
         assertEquals(1L, scriptService.stats().getCacheEvictions());
     }

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

@@ -23,6 +23,7 @@ import org.elasticsearch.common.Strings;
 import org.elasticsearch.ingest.AbstractProcessor;
 import org.elasticsearch.ingest.IngestDocument;
 import org.elasticsearch.ingest.Processor;
+import org.elasticsearch.script.CompiledScript;
 import org.elasticsearch.script.ExecutableScript;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptContext;
@@ -70,7 +71,8 @@ public final class ScriptProcessor extends AbstractProcessor {
      */
     @Override
     public void execute(IngestDocument document) {
-        ExecutableScript executableScript = scriptService.executable(script, ScriptContext.Standard.INGEST);
+        CompiledScript compiledScript = scriptService.compile(script, ScriptContext.Standard.INGEST);
+        ExecutableScript executableScript = scriptService.executable(compiledScript, script.getParams());
         executableScript.setNextVar("ctx",  document.getSourceAndMetadata());
         executableScript.run();
     }

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

@@ -24,6 +24,7 @@ import java.util.Map;
 
 import org.elasticsearch.ingest.IngestDocument;
 import org.elasticsearch.ingest.RandomDocumentPicks;
+import org.elasticsearch.script.CompiledScript;
 import org.elasticsearch.script.ExecutableScript;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptService;
@@ -46,7 +47,7 @@ public class ScriptProcessorTests extends ESTestCase {
         ScriptService scriptService = mock(ScriptService.class);
         Script script = new Script("_script");
         ExecutableScript executableScript = mock(ExecutableScript.class);
-        when(scriptService.executable(any(Script.class), any())).thenReturn(executableScript);
+        when(scriptService.executable(any(CompiledScript.class), any())).thenReturn(executableScript);
 
         Map<String, Object> document = new HashMap<>();
         document.put("bytes_in", randomInt());

modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java

@@ -34,6 +34,7 @@ import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.query.QueryParseContext;
+import org.elasticsearch.script.CompiledScript;
 import org.elasticsearch.script.ExecutableScript;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptService;
@@ -71,7 +72,8 @@ public class TransportSearchTemplateAction extends HandledTransportAction<Search
         try {
             Script script = new Script(request.getScriptType(), TEMPLATE_LANG, request.getScript(),
                 request.getScriptParams() == null ? Collections.emptyMap() : request.getScriptParams());
-            ExecutableScript executable = scriptService.executable(script, SEARCH);
+            CompiledScript compiledScript = scriptService.compile(script, SEARCH);
+            ExecutableScript executable = scriptService.executable(compiledScript, script.getParams());
 
             BytesReference source = (BytesReference) executable.run();
             response.setSource(source);