Scripting: Replace Update Context (#32096)

* SCRIPTING: Move Update Scripts to their own context
* Added system property for backwards compatibility of change to `ctx.params`
This commit is contained in:
Armin Braun 2018-08-09 14:32:36 +02:00 committed by GitHub
parent 823d40e19b
commit 79375d35bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 116 additions and 28 deletions

View File

@ -782,9 +782,12 @@ class BuildPlugin implements Plugin<Project> {
}
}
// 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'

View File

@ -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

View File

@ -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 {

View File

@ -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" }

View File

@ -48,9 +48,9 @@ import org.elasticsearch.index.mapper.SourceFieldMapper;
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 class AbstractAsyncBulkByScrollAction<Request extends AbstractBu
private final Script script;
private final Map<String, Object> params;
private ExecutableScript executable;
private UpdateScript executable;
private Map<String, Object> context;
public ScriptApplier(WorkerBulkByScrollTaskState taskWorker,
@ -766,7 +766,7 @@ public abstract class AbstractAsyncBulkByScrollAction<Request extends AbstractBu
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 abstract class AbstractAsyncBulkByScrollAction<Request extends AbstractBu
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) {

View File

@ -26,8 +26,10 @@ import org.elasticsearch.action.delete.DeleteRequest;
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 abstract class AbstractAsyncBulkByScrollActionScriptTestCase<
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);
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<String, Object> 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<Request> action = action(scriptService, request().setScript(mockScript("")));
RequestWrapper<?> result = action.buildScriptApplier().apply(AbstractAsyncBulkByScrollAction.wrap(index), doc);
return (result != null) ? (T) result.self() : null;

View File

@ -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.get.GetResult;
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 @@ public class UpdateHelper extends AbstractComponent {
private Map<String, Object> executeScript(Script script, Map<String, Object> 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<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);
}
} catch (Exception e) {
throw new IllegalArgumentException("failed to execute script", e);

View File

@ -46,7 +46,4 @@ public interface ExecutableScript {
}
ScriptContext<Factory> CONTEXT = new ScriptContext<>("executable", Factory.class);
// TODO: remove these once each has its own script interface
ScriptContext<Factory> UPDATE_CONTEXT = new ScriptContext<>("update", Factory.class);
}

View File

@ -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,

View File

@ -285,7 +285,7 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
// TODO: fix this through some API or something, that's wrong
// special exception to prevent expressions from compiling as update or mapping scripts
boolean expression = "expression".equals(lang);
boolean notSupported = context.name.equals(ExecutableScript.UPDATE_CONTEXT.name);
boolean notSupported = context.name.equals(UpdateScript.CONTEXT.name);
if (expression && notSupported) {
throw new UnsupportedOperationException("scripts of type [" + script.getType() + "]," +
" operation [" + context.name + "] and lang [" + lang + "] are not supported");

View File

@ -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;
/**
* An update script.
*/
public abstract class UpdateScript {
public static final String[] PARAMETERS = { "ctx" };
/** 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;
}
/** Return the parameters for this script. */
public Map<String, Object> getParams() {
return params;
}
public abstract void execute(Map<String, Object> ctx);
public interface Factory {
UpdateScript newInstance(Map<String, Object> params);
}
}

View File

@ -167,7 +167,7 @@ public class ScriptServiceTests extends ESTestCase {
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 class ScriptServiceTests extends ESTestCase {
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 {

View File

@ -93,6 +93,7 @@ public class UpdateIT extends ESIntegTestCase {
}
Map<String, Object> source = (Map<String, Object>) ctx.get("_source");
params.remove("ctx");
source.putAll(params);
return ctx;

View File

@ -96,6 +96,18 @@ public class MockScriptEngine implements ScriptEngine {
}
};
return context.factoryClazz.cast(factory);
} else if (context.instanceClazz.equals(UpdateScript.class)) {
UpdateScript.Factory factory = parameters -> new UpdateScript(parameters) {
@Override
public void execute(Map<String, Object> ctx) {
final Map<String, Object> 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