Scripting: Use ParameterMap for deprecated ctx var in update scripts (elastic#34065)

This commit removes the sysprop controlling whether ctx is in params for
update scripts and replaces it with use of the new ParameterMap, which
outputs a deprecation warning whenever params.ctx is used.

Author: rjernst

modules/reindex/src/main/java/org/elasticsearch/index/reindex/AbstractAsyncBulkByScrollAction.java

@@ -763,9 +763,6 @@ public abstract static class ScriptApplier implements BiFunction<RequestWrapper<
         private final Script script;
         private final Map<String, Object> params;
 
-        private UpdateScript executable;
-        private Map<String, Object> context;
-
         public ScriptApplier(WorkerBulkByScrollTaskState taskWorker,
                              ScriptService scriptService,
                              Script script,
@@ -782,16 +779,8 @@ public RequestWrapper<?> apply(RequestWrapper<?> request, ScrollableHitSource.Hi
             if (script == null) {
                 return request;
             }
-            if (executable == null) {
-                UpdateScript.Factory factory = scriptService.compile(script, UpdateScript.CONTEXT);
-                executable = factory.newInstance(params);
-            }
-            if (context == null) {
-                context = new HashMap<>();
-            } else {
-                context.clear();
-            }
 
+            Map<String, Object> context = new HashMap<>();
             context.put(IndexFieldMapper.NAME, doc.getIndex());
             context.put(TypeFieldMapper.NAME, doc.getType());
             context.put(IdFieldMapper.NAME, doc.getId());
@@ -806,7 +795,9 @@ public RequestWrapper<?> apply(RequestWrapper<?> request, ScrollableHitSource.Hi
             OpType oldOpType = OpType.INDEX;
             context.put("op", oldOpType.toString());
 
-            executable.execute(context);
+            UpdateScript.Factory factory = scriptService.compile(script, UpdateScript.CONTEXT);
+            UpdateScript updateScript = factory.newInstance(params, context);
+            updateScript.execute();
 
             String newOp = (String) context.remove("op");
             if (newOp == null) {

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

@@ -56,13 +56,12 @@ public void setupScriptService() {
     protected <T extends ActionRequest> T applyScript(Consumer<Map<String, Object>> scriptBody) {
         IndexRequest index = new IndexRequest("index", "type", "1").source(singletonMap("foo", "bar"));
         ScrollableHitSource.Hit doc = new ScrollableHitSource.BasicHit("test", "type", "id", 0);
-        UpdateScript updateScript = new UpdateScript(Collections.emptyMap()) {
+        UpdateScript.Factory factory = (params, ctx) -> new UpdateScript(Collections.emptyMap(), ctx) {
             @Override
-            public void execute(Map<String, Object> ctx) {
+            public void execute() {
                 scriptBody.accept(ctx);
             }
-        };
-        UpdateScript.Factory factory = params -> updateScript;
+        };;
         ExecutableScript simpleExecutableScript = new SimpleExecutableScript(scriptBody);
         when(scriptService.compile(any(), eq(ExecutableScript.CONTEXT))).thenReturn(params -> simpleExecutableScript);
         when(scriptService.compile(any(), eq(UpdateScript.CONTEXT))).thenReturn(factory);

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

@@ -19,10 +19,6 @@
 
 package org.elasticsearch.action.update;
 
-import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.function.LongSupplier;
 import org.apache.logging.log4j.Logger;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.DocWriteResponse;
@@ -53,18 +49,17 @@
 import org.elasticsearch.script.UpdateScript;
 import org.elasticsearch.search.lookup.SourceLookup;
 
+import java.io.IOException;
 import java.util.ArrayList;
-import static org.elasticsearch.common.Booleans.parseBoolean;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.LongSupplier;
 
 /**
  * Helper for translating an update request to an index, delete request or update response.
  */
 public class UpdateHelper extends AbstractComponent {
 
-    /** Whether scripts should add the ctx variable to the params map. */
-    private static final boolean CTX_IN_PARAMS =
-        parseBoolean(System.getProperty("es.scripting.update.ctx_in_params"), true);
-
     private final ScriptService scriptService;
 
     public UpdateHelper(Settings settings, ScriptService scriptService) {
@@ -304,17 +299,8 @@ private Map<String, Object> executeScript(Script script, Map<String, Object> ctx
         try {
             if (scriptService != null) {
                 UpdateScript.Factory factory = scriptService.compile(script, UpdateScript.CONTEXT);
-                final Map<String, Object> params;
-                if (CTX_IN_PARAMS) {
-                    params = new HashMap<>(script.getParams());
-                    params.put(ContextFields.CTX, ctx);
-                    deprecationLogger.deprecated("Using `ctx` via `params.ctx` is deprecated. " +
-                        "Use -Des.scripting.update.ctx_in_params=false to enforce non-deprecated usage.");
-                } else {
-                    params = script.getParams();
-                }
-                UpdateScript executableScript = factory.newInstance(params);
-                executableScript.execute(ctx);
+                UpdateScript executableScript = factory.newInstance(script.getParams(), ctx);
+                executableScript.execute();
             }
         } catch (Exception e) {
             throw new IllegalArgumentException("failed to execute script", e);

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

@@ -20,33 +20,57 @@
 
 package org.elasticsearch.script;
 
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
 
 /**
  * An update script.
  */
 public abstract class UpdateScript {
 
-    public static final String[] PARAMETERS = { "ctx" };
+    public static final String[] PARAMETERS = { };
+
+    private static final Map<String, String> DEPRECATIONS;
+    static {
+        Map<String, String> deprecations = new HashMap<>();
+        deprecations.put(
+            "ctx",
+            "Accessing variable [ctx] via [params.ctx] from within a update script " +
+                "is deprecated in favor of directly accessing [ctx]."
+        );
+        DEPRECATIONS = Collections.unmodifiableMap(deprecations);
+    }
 
     /** The context used to compile {@link UpdateScript} factories. */
     public static final ScriptContext<Factory> CONTEXT = new ScriptContext<>("update", Factory.class);
 
     /** The generic runtime parameters for the script. */
     private final Map<String, Object> params;
 
-    public UpdateScript(Map<String, Object> params) {
-        this.params = params;
+    /** The update context for the script. */
+    private final Map<String, Object> ctx;
+
+    public UpdateScript(Map<String, Object> params, Map<String, Object> ctx) {
+        Map<String, Object> paramsWithCtx = new HashMap<>(params);
+        paramsWithCtx.put("ctx", ctx);
+        this.params = new ParameterMap(paramsWithCtx, DEPRECATIONS);
+        this.ctx = ctx;
     }
 
     /** Return the parameters for this script. */
     public Map<String, Object> getParams() {
         return params;
     }
 
-    public abstract void execute(Map<String, Object> ctx);
+    /** Return the update context for this script. */
+    public Map<String, Object> getCtx() {
+        return ctx;
+    }
+
+    public abstract void execute();
 
     public interface Factory {
-        UpdateScript newInstance(Map<String, Object> params);
+        UpdateScript newInstance(Map<String, Object> params, Map<String, Object> ctx);
     }
 }

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

@@ -117,9 +117,9 @@ public boolean execute(Map<String, Object> ctx) {
             };
             return context.factoryClazz.cast(factory);
         } else if (context.instanceClazz.equals(UpdateScript.class)) {
-            UpdateScript.Factory factory = parameters -> new UpdateScript(parameters) {
+            UpdateScript.Factory factory = (parameters, ctx) -> new UpdateScript(parameters, ctx) {
                 @Override
-                public void execute(Map<String, Object> ctx) {
+                public void execute() {
                     final Map<String, Object> vars = new HashMap<>();
                     vars.put("ctx", ctx);
                     vars.put("params", parameters);