diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index f6c6d5d7fd7c2..05fd4784863ad 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -782,9 +782,12 @@ class BuildPlugin implements Plugin { } } - // TODO: remove this once joda time is removed from scriptin in 7.0 + // TODO: remove this once joda time is removed from scripting in 7.0 systemProperty 'es.scripting.use_java_time', 'true' + // TODO: remove this once ctx isn't added to update script params in 7.0 + systemProperty 'es.scripting.update.ctx_in_params', 'false' + // Set the system keystore/truststore password if we're running tests in a FIPS-140 JVM if (project.inFipsJvm) { systemProperty 'javax.net.ssl.trustStorePassword', 'password' diff --git a/docs/build.gradle b/docs/build.gradle index 4c0502a0e06fc..029147bba2ff0 100644 --- a/docs/build.gradle +++ b/docs/build.gradle @@ -40,6 +40,7 @@ integTestCluster { // TODO: remove this for 7.0, this exists to allow the doc examples in 6.x to continue using the defaults systemProperty 'es.scripting.use_java_time', 'false' + systemProperty 'es.scripting.update.ctx_in_params', 'false' } // remove when https://github.com/elastic/elasticsearch/issues/31305 is fixed diff --git a/modules/lang-painless/build.gradle b/modules/lang-painless/build.gradle index e3a7ccecae2e5..ed4b1d631e064 100644 --- a/modules/lang-painless/build.gradle +++ b/modules/lang-painless/build.gradle @@ -25,6 +25,7 @@ esplugin { integTestCluster { module project.project(':modules:mapper-extras') systemProperty 'es.scripting.use_java_time', 'true' + systemProperty 'es.scripting.update.ctx_in_params', 'false' } dependencies { diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/15_update.yml b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/15_update.yml index 20047e7d4825d..f2e1cb616b980 100644 --- a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/15_update.yml +++ b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/15_update.yml @@ -132,7 +132,7 @@ body: script: lang: painless - source: "for (def key : params.keySet()) { ctx._source[key] = params[key]}" + source: "ctx._source.ctx = ctx" params: { bar: 'xxx' } - match: { error.root_cause.0.type: "remote_transport_exception" } diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AbstractAsyncBulkByScrollAction.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AbstractAsyncBulkByScrollAction.java index 5bd83b6c19a2e..731a27aa72c84 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AbstractAsyncBulkByScrollAction.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AbstractAsyncBulkByScrollAction.java @@ -48,9 +48,9 @@ import org.elasticsearch.index.mapper.TypeFieldMapper; import org.elasticsearch.index.mapper.VersionFieldMapper; import org.elasticsearch.index.reindex.ScrollableHitSource.SearchFailure; -import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.UpdateScript; import org.elasticsearch.search.sort.SortBuilder; import org.elasticsearch.threadpool.ThreadPool; @@ -746,7 +746,7 @@ public abstract static class ScriptApplier implements BiFunction params; - private ExecutableScript executable; + private UpdateScript executable; private Map context; public ScriptApplier(WorkerBulkByScrollTaskState taskWorker, @@ -766,7 +766,7 @@ public RequestWrapper apply(RequestWrapper request, ScrollableHitSource.Hi return request; } if (executable == null) { - ExecutableScript.Factory factory = scriptService.compile(script, ExecutableScript.UPDATE_CONTEXT); + UpdateScript.Factory factory = scriptService.compile(script, UpdateScript.CONTEXT); executable = factory.newInstance(params); } if (context == null) { @@ -787,8 +787,7 @@ public RequestWrapper apply(RequestWrapper request, ScrollableHitSource.Hi OpType oldOpType = OpType.INDEX; context.put("op", oldOpType.toString()); - executable.setNextVar("ctx", context); - executable.run(); + executable.execute(context); String newOp = (String) context.remove("op"); if (newOp == null) { diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/AbstractAsyncBulkByScrollActionScriptTestCase.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/AbstractAsyncBulkByScrollActionScriptTestCase.java index 955c99568c7d2..94f375e9333ed 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/AbstractAsyncBulkByScrollActionScriptTestCase.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/AbstractAsyncBulkByScrollActionScriptTestCase.java @@ -26,8 +26,10 @@ import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.UpdateScript; import org.junit.Before; +import java.util.Collections; import java.util.Map; import java.util.function.Consumer; @@ -54,10 +56,16 @@ public void setupScriptService() { protected T applyScript(Consumer> scriptBody) { IndexRequest index = new IndexRequest("index", "type", "1").source(singletonMap("foo", "bar")); ScrollableHitSource.Hit doc = new ScrollableHitSource.BasicHit("test", "type", "id", 0); - ExecutableScript executableScript = new SimpleExecutableScript(scriptBody); - ExecutableScript.Factory factory = params -> executableScript; - when(scriptService.compile(any(), eq(ExecutableScript.CONTEXT))).thenReturn(factory); - when(scriptService.compile(any(), eq(ExecutableScript.UPDATE_CONTEXT))).thenReturn(factory); + UpdateScript updateScript = new UpdateScript(Collections.emptyMap()) { + @Override + public void execute(Map ctx) { + 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); AbstractAsyncBulkByScrollAction action = action(scriptService, request().setScript(mockScript(""))); RequestWrapper result = action.buildScriptApplier().apply(AbstractAsyncBulkByScrollAction.wrap(index), doc); return (result != null) ? (T) result.self() : null; diff --git a/server/src/main/java/org/elasticsearch/action/update/UpdateHelper.java b/server/src/main/java/org/elasticsearch/action/update/UpdateHelper.java index 5212b1f35214c..77485f81e58c0 100644 --- a/server/src/main/java/org/elasticsearch/action/update/UpdateHelper.java +++ b/server/src/main/java/org/elasticsearch/action/update/UpdateHelper.java @@ -19,6 +19,11 @@ package org.elasticsearch.action.update; +import java.io.IOException; +import java.util.Collections; +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; @@ -42,21 +47,22 @@ import org.elasticsearch.index.mapper.RoutingFieldMapper; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.shard.ShardId; -import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.UpdateScript; import org.elasticsearch.search.lookup.SourceLookup; -import java.io.IOException; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; -import java.util.function.LongSupplier; +import static org.elasticsearch.common.Booleans.parseBoolean; /** * 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) { @@ -279,10 +285,18 @@ Result prepareUpdateScriptRequest(ShardId shardId, UpdateRequest request, GetRes private Map executeScript(Script script, Map ctx) { try { if (scriptService != null) { - ExecutableScript.Factory factory = scriptService.compile(script, ExecutableScript.UPDATE_CONTEXT); - ExecutableScript executableScript = factory.newInstance(script.getParams()); - executableScript.setNextVar(ContextFields.CTX, ctx); - executableScript.run(); + UpdateScript.Factory factory = scriptService.compile(script, UpdateScript.CONTEXT); + final Map 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); } } catch (Exception e) { throw new IllegalArgumentException("failed to execute script", e); diff --git a/server/src/main/java/org/elasticsearch/script/ExecutableScript.java b/server/src/main/java/org/elasticsearch/script/ExecutableScript.java index 1bd4c31ebf349..d0d8020371bd1 100644 --- a/server/src/main/java/org/elasticsearch/script/ExecutableScript.java +++ b/server/src/main/java/org/elasticsearch/script/ExecutableScript.java @@ -46,7 +46,4 @@ interface Factory { } ScriptContext CONTEXT = new ScriptContext<>("executable", Factory.class); - - // TODO: remove these once each has its own script interface - ScriptContext UPDATE_CONTEXT = new ScriptContext<>("update", Factory.class); } diff --git a/server/src/main/java/org/elasticsearch/script/ScriptModule.java b/server/src/main/java/org/elasticsearch/script/ScriptModule.java index 3eeb26317f9cc..f04e690fa425c 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptModule.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptModule.java @@ -46,10 +46,10 @@ public class ScriptModule { SearchScript.SCRIPT_SORT_CONTEXT, SearchScript.TERMS_SET_QUERY_CONTEXT, ExecutableScript.CONTEXT, + UpdateScript.CONTEXT, BucketAggregationScript.CONTEXT, BucketAggregationSelectorScript.CONTEXT, SignificantTermsHeuristicScoreScript.CONTEXT, - ExecutableScript.UPDATE_CONTEXT, IngestScript.CONTEXT, FilterScript.CONTEXT, SimilarityScript.CONTEXT, diff --git a/server/src/main/java/org/elasticsearch/script/ScriptService.java b/server/src/main/java/org/elasticsearch/script/ScriptService.java index ca79e3b80fc81..9768547b89819 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptService.java @@ -285,7 +285,7 @@ public FactoryType compile(Script script, ScriptContext CONTEXT = new ScriptContext<>("update", Factory.class); + + /** The generic runtime parameters for the script. */ + private final Map params; + + public UpdateScript(Map params) { + this.params = params; + } + + /** Return the parameters for this script. */ + public Map getParams() { + return params; + } + + public abstract void execute(Map ctx); + + public interface Factory { + UpdateScript newInstance(Map params); + } +} diff --git a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java index 585f860165160..ea8b6a9223412 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java @@ -167,7 +167,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, UpdateScript.CONTEXT); assertCompileAccepted("painless", "script", ScriptType.INLINE, IngestScript.CONTEXT); } @@ -187,7 +187,7 @@ public void testAllowSomeScriptContextSettings() throws IOException { assertCompileAccepted("painless", "script", ScriptType.INLINE, SearchScript.CONTEXT); assertCompileAccepted("painless", "script", ScriptType.INLINE, SearchScript.AGGS_CONTEXT); - assertCompileRejected("painless", "script", ScriptType.INLINE, ExecutableScript.UPDATE_CONTEXT); + assertCompileRejected("painless", "script", ScriptType.INLINE, UpdateScript.CONTEXT); } public void testAllowNoScriptTypeSettings() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/update/UpdateIT.java b/server/src/test/java/org/elasticsearch/update/UpdateIT.java index e4ea078b8f716..85ebf01ef28c0 100644 --- a/server/src/test/java/org/elasticsearch/update/UpdateIT.java +++ b/server/src/test/java/org/elasticsearch/update/UpdateIT.java @@ -93,6 +93,7 @@ protected Map, Object>> pluginScripts() { } Map source = (Map) ctx.get("_source"); + params.remove("ctx"); source.putAll(params); return ctx; diff --git a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java index 14dcac926f7b0..8083931e73d03 100644 --- a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java +++ b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java @@ -96,6 +96,18 @@ public void execute(Map ctx) { } }; return context.factoryClazz.cast(factory); + } else if (context.instanceClazz.equals(UpdateScript.class)) { + UpdateScript.Factory factory = parameters -> new UpdateScript(parameters) { + @Override + public void execute(Map ctx) { + final Map vars = new HashMap<>(); + vars.put("ctx", ctx); + vars.put("params", parameters); + vars.putAll(parameters); + script.apply(vars); + } + }; + return context.factoryClazz.cast(factory); } else if (context.instanceClazz.equals(BucketAggregationScript.class)) { BucketAggregationScript.Factory factory = parameters -> new BucketAggregationScript(parameters) { @Override