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.

10 files changed, 30 insertions, 21 deletions

diff --git a/core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java b/core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java
index b1621bc118..09fc0bccbf 100644
--- a/core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java
+++ b/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();
             }
diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java
index 80726496a7..183ab690ce 100644
--- a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java
+++ b/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();
     }
 }
diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java
index 2b5e69947f..a6a7108f7a 100644
--- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java
+++ b/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());
     }
 
     /**
diff --git a/core/src/main/java/org/elasticsearch/script/ScriptService.java b/core/src/main/java/org/elasticsearch/script/ScriptService.java
index 692e081a7b..e0c7b3c63d 100644
--- a/core/src/main/java/org/elasticsearch/script/ScriptService.java
+++ b/core/src/main/java/org/elasticsearch/script/ScriptService.java
@@ -470,13 +470,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
      */
     public ExecutableScript executable(CompiledScript compiledScript, Map<String, Object> params) {
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java
index b73f2f0987..7618839d49 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java
+++ b/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
diff --git a/core/src/test/java/org/elasticsearch/script/NativeScriptTests.java b/core/src/test/java/org/elasticsearch/script/NativeScriptTests.java
index bf5c7e0daa..aa2e260c7c 100644
--- a/core/src/test/java/org/elasticsearch/script/NativeScriptTests.java
+++ b/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(
-                new Script(ScriptType.INLINE, NativeScriptEngineService.NAME, "my", Collections.emptyMap()), ScriptContext.Standard.SEARCH);
+        Script script = new Script(ScriptType.INLINE, NativeScriptEngineService.NAME, "my", Collections.emptyMap());
+        CompiledScript compiledScript = scriptModule.getScriptService().compile(script, ScriptContext.Standard.SEARCH);
+        ExecutableScript executable = scriptModule.getScriptService().executable(compiledScript, script.getParams());
         assertThat(executable.run().toString(), equalTo("test"));
     }
 
diff --git a/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java
index da205a9292..6c2bd6f1be 100644
--- a/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java
+++ b/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));
-        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.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.executable(new Script(ScriptType.INLINE, "test", "2+2", Collections.emptyMap()), randomFrom(scriptContexts));
+        scriptService.compile(new Script(ScriptType.INLINE, "test", "1+1", 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());
     }
diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java
index 33c06a3804..a5bc803402 100644
--- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java
+++ b/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();
     }
diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java
index 94430622d1..e76f3016dd 100644
--- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java
+++ b/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());
diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java
index d7b0406238..61f099f6c2 100644
--- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java
+++ b/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);