From 3d2626c4c694db0abd1ad15badecd75a06818f40 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Tue, 31 Jan 2017 13:27:02 -0800 Subject: [PATCH] Change Namespace for Stored Script to Only Use Id (#22206) Currently, stored scripts use a namespace of (lang, id) to be put, get, deleted, and executed. This is not necessary since the lang is stored with the stored script. A user should only have to specify an id to use a stored script. This change makes that possible while keeping backwards compatibility with the previous namespace of (lang, id). Anywhere the previous namespace is used will log deprecation warnings. The new behavior is the following: When a user specifies a stored script, that script will be stored under both the new namespace and old namespace. Take for example script 'A' with lang 'L0' and data 'D0'. If we add script 'A' to the empty set, the scripts map will be ["A" -- D0, "A#L0" -- D0]. If a script 'A' with lang 'L1' and data 'D1' is then added, the scripts map will be ["A" -- D1, "A#L1" -- D1, "A#L0" -- D0]. When a user deletes a stored script, that script will be deleted from both the new namespace (if it exists) and the old namespace. Take for example a scripts map with {"A" -- D1, "A#L1" -- D1, "A#L0" -- D0}. If a script is removed specified by an id 'A' and lang null then the scripts map will be {"A#L0" -- D0}. To remove the final script, the deprecated namespace must be used, so an id 'A' and lang 'L0' would need to be specified. When a user gets/executes a stored script, if the new namespace is used then the script will be retrieved/executed using only 'id', and if the old namespace is used then the script will be retrieved/executed using 'id' and 'lang' --- .../resources/checkstyle_suppressions.xml | 2 +- .../DeleteStoredScriptRequest.java | 57 +- .../DeleteStoredScriptRequestBuilder.java | 6 +- .../storedscripts/GetStoredScriptRequest.java | 62 +- .../GetStoredScriptResponse.java | 41 +- .../storedscripts/PutStoredScriptRequest.java | 87 +-- .../PutStoredScriptRequestBuilder.java | 16 +- .../TransportPutStoredScriptAction.java | 2 +- .../AbstractAsyncBulkByScrollAction.java | 2 +- .../client/support/AbstractClient.java | 2 +- .../index/query/QueryShardContext.java | 4 +- .../ingest/InternalTemplateService.java | 5 +- .../cluster/RestDeleteStoredScriptAction.java | 36 +- .../cluster/RestGetStoredScriptAction.java | 90 ++- .../cluster/RestPutStoredScriptAction.java | 42 +- .../java/org/elasticsearch/script/Script.java | 320 +++++++--- .../elasticsearch/script/ScriptMetaData.java | 589 +++++++++++------- .../elasticsearch/script/ScriptService.java | 412 ++++++------ .../script/StoredScriptSource.java | 485 ++++++++++++++ .../scripted/InternalScriptedMetric.java | 4 +- .../BucketScriptPipelineAggregator.java | 3 +- .../BucketSelectorPipelineAggregator.java | 3 +- .../index/query/InnerHitBuilderTests.java | 3 +- .../elasticsearch/script/FileScriptTests.java | 4 +- .../script/NativeScriptTests.java | 2 +- .../script/ScriptContextTests.java | 28 +- .../script/ScriptMetaDataTests.java | 141 ++--- .../script/ScriptServiceTests.java | 101 +-- .../org/elasticsearch/script/ScriptTests.java | 5 +- .../script/StoredScriptTests.java | 364 +++++++++++ .../elasticsearch/script/StoredScriptsIT.java | 27 +- .../metrics/ScriptedMetricIT.java | 16 +- .../metrics/ScriptedMetricTests.java | 3 +- .../aggregations/pipeline/BucketScriptIT.java | 4 +- .../pipeline/BucketScriptTests.java | 4 +- .../pipeline/BucketSelectorIT.java | 4 +- .../pipeline/BucketSelectorTests.java | 4 +- .../search/sort/AbstractSortTestCase.java | 2 +- .../SharedClusterSnapshotRestoreIT.java | 8 +- .../modules/scripting/using.asciidoc | 54 +- .../ingest/common/ScriptProcessor.java | 7 +- .../common/ScriptProcessorFactoryTests.java | 2 +- ...nTests.java => StoredExpressionTests.java} | 8 +- .../RestDeleteSearchTemplateAction.java | 21 +- .../mustache/RestGetSearchTemplateAction.java | 55 +- .../mustache/RestPutSearchTemplateAction.java | 23 +- .../script/mustache/TemplateQueryBuilder.java | 12 +- .../script/mustache/SearchTemplateIT.java | 92 +-- .../test/lang_mustache/10_basic.yaml | 2 +- .../20_render_search_template.yaml | 2 +- .../test/lang_mustache/40_template_query.yaml | 2 +- .../test/painless/16_update2.yaml | 35 +- .../test/mixed_cluster/10_basic.yaml | 94 +++ .../test/old_cluster/10_basic.yaml | 120 ++++ .../test/upgraded_cluster/10_basic.yaml | 424 +++++++++++++ .../50_script_processor_using_painless.yaml | 11 +- .../rest-api-spec/api/delete_script.json | 4 +- .../rest-api-spec/api/get_script.json | 4 +- .../rest-api-spec/api/put_script.json | 4 +- 59 files changed, 2963 insertions(+), 1003 deletions(-) create mode 100644 core/src/main/java/org/elasticsearch/script/StoredScriptSource.java create mode 100644 core/src/test/java/org/elasticsearch/script/StoredScriptTests.java rename modules/lang-expression/src/test/java/org/elasticsearch/script/expression/{IndexedExpressionTests.java => StoredExpressionTests.java} (94%) diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml index c8f7682388c..a800411cc63 100644 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ b/buildSrc/src/main/resources/checkstyle_suppressions.xml @@ -886,7 +886,7 @@ - + diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/DeleteStoredScriptRequest.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/DeleteStoredScriptRequest.java index d01c128cf36..c30eae12a82 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/DeleteStoredScriptRequest.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/DeleteStoredScriptRequest.java @@ -31,66 +31,79 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; public class DeleteStoredScriptRequest extends AcknowledgedRequest { private String id; - private String scriptLang; + private String lang; DeleteStoredScriptRequest() { + super(); } - public DeleteStoredScriptRequest(String scriptLang, String id) { - this.scriptLang = scriptLang; + public DeleteStoredScriptRequest(String id, String lang) { + super(); + this.id = id; + this.lang = lang; } @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = null; - if (id == null) { - validationException = addValidationError("id is missing", validationException); + + if (id == null || id.isEmpty()) { + validationException = addValidationError("must specify id for stored script", validationException); } else if (id.contains("#")) { - validationException = addValidationError("id can't contain: '#'", validationException); + validationException = addValidationError("id cannot contain '#' for stored script", validationException); } - if (scriptLang == null) { - validationException = addValidationError("lang is missing", validationException); - } else if (scriptLang.contains("#")) { - validationException = addValidationError("lang can't contain: '#'", validationException); + + if (lang != null && lang.contains("#")) { + validationException = addValidationError("lang cannot contain '#' for stored script", validationException); } + return validationException; } - public String scriptLang() { - return scriptLang; - } - - public DeleteStoredScriptRequest scriptLang(String type) { - this.scriptLang = type; - return this; - } - public String id() { return id; } public DeleteStoredScriptRequest id(String id) { this.id = id; + + return this; + } + + public String lang() { + return lang; + } + + public DeleteStoredScriptRequest lang(String lang) { + this.lang = lang; + return this; } @Override public void readFrom(StreamInput in) throws IOException { super.readFrom(in); - scriptLang = in.readString(); + + lang = in.readString(); + + if (lang.isEmpty()) { + lang = null; + } + id = in.readString(); } @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - out.writeString(scriptLang); + + out.writeString(lang == null ? "" : lang); out.writeString(id); } @Override public String toString() { - return "delete script {[" + scriptLang + "][" + id + "]}"; + return "delete stored script {id [" + id + "]" + (lang != null ? ", lang [" + lang + "]" : "") + "}"; } } diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/DeleteStoredScriptRequestBuilder.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/DeleteStoredScriptRequestBuilder.java index caf55a03f18..8a65506dabd 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/DeleteStoredScriptRequestBuilder.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/DeleteStoredScriptRequestBuilder.java @@ -29,13 +29,15 @@ public class DeleteStoredScriptRequestBuilder extends AcknowledgedRequestBuilder super(client, action, new DeleteStoredScriptRequest()); } - public DeleteStoredScriptRequestBuilder setScriptLang(String scriptLang) { - request.scriptLang(scriptLang); + public DeleteStoredScriptRequestBuilder setLang(String lang) { + request.lang(lang); + return this; } public DeleteStoredScriptRequestBuilder setId(String id) { request.id(id); + return this; } diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/GetStoredScriptRequest.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/GetStoredScriptRequest.java index bb7a9effd32..2bfd547362c 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/GetStoredScriptRequest.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/GetStoredScriptRequest.java @@ -28,61 +28,79 @@ import org.elasticsearch.common.io.stream.StreamOutput; import java.io.IOException; +import static org.elasticsearch.action.ValidateActions.addValidationError; + public class GetStoredScriptRequest extends MasterNodeReadRequest { protected String id; protected String lang; GetStoredScriptRequest() { + super(); } - public GetStoredScriptRequest(String lang, String id) { - this.lang = lang; + public GetStoredScriptRequest(String id, String lang) { + super(); + this.id = id; + this.lang = lang; } @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = null; - if (lang == null) { - validationException = ValidateActions.addValidationError("lang is missing", validationException); + + if (id == null || id.isEmpty()) { + validationException = addValidationError("must specify id for stored script", validationException); + } else if (id.contains("#")) { + validationException = addValidationError("id cannot contain '#' for stored script", validationException); } - if (id == null) { - validationException = ValidateActions.addValidationError("id is missing", validationException); + + if (lang != null && lang.contains("#")) { + validationException = addValidationError("lang cannot contain '#' for stored script", validationException); } + return validationException; } - public GetStoredScriptRequest lang(@Nullable String type) { - this.lang = type; - return this; - } - - public GetStoredScriptRequest id(String id) { - this.id = id; - return this; - } - - - public String lang() { - return lang; - } - public String id() { return id; } + public GetStoredScriptRequest id(String id) { + this.id = id; + + return this; + } + + public String lang() { + return lang; + } + + public GetStoredScriptRequest lang(String lang) { + this.lang = lang; + + return this; + } + @Override public void readFrom(StreamInput in) throws IOException { super.readFrom(in); + lang = in.readString(); + + if (lang.isEmpty()) { + lang = null; + } + id = in.readString(); } @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - out.writeString(lang); + + out.writeString(lang == null ? "" : lang); out.writeString(id); } diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/GetStoredScriptResponse.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/GetStoredScriptResponse.java index 36dd9beb38a..b8302a03c28 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/GetStoredScriptResponse.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/GetStoredScriptResponse.java @@ -19,49 +19,70 @@ package org.elasticsearch.action.admin.cluster.storedscripts; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionResponse; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.script.Script; +import org.elasticsearch.script.StoredScriptSource; import java.io.IOException; public class GetStoredScriptResponse extends ActionResponse implements ToXContent { - private String storedScript; + private StoredScriptSource source; GetStoredScriptResponse() { } - GetStoredScriptResponse(String storedScript) { - this.storedScript = storedScript; + GetStoredScriptResponse(StoredScriptSource source) { + this.source = source; } /** * @return if a stored script and if not found null */ - public String getStoredScript() { - return storedScript; + public StoredScriptSource getSource() { + return source; } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.value(storedScript); + source.toXContent(builder, params); + return builder; } @Override public void readFrom(StreamInput in) throws IOException { super.readFrom(in); - storedScript = in.readOptionalString(); + + if (in.readBoolean()) { + if (in.getVersion().onOrAfter(Version.V_5_3_0_UNRELEASED)) { + source = new StoredScriptSource(in); + } else { + source = new StoredScriptSource(in.readString()); + } + } else { + source = null; + } } @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - out.writeOptionalString(storedScript); + + if (source == null) { + out.writeBoolean(false); + } else { + out.writeBoolean(true); + + if (out.getVersion().onOrAfter(Version.V_5_3_0_UNRELEASED)) { + source.writeTo(out); + } else { + out.writeString(source.getCode()); + } + } } } diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptRequest.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptRequest.java index cfe153d7d96..0dcfe225142 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptRequest.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptRequest.java @@ -33,94 +33,105 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; public class PutStoredScriptRequest extends AcknowledgedRequest { private String id; - private String scriptLang; - private BytesReference script; + private String lang; + private BytesReference content; public PutStoredScriptRequest() { super(); } - public PutStoredScriptRequest(String scriptLang) { + public PutStoredScriptRequest(String id, String lang, BytesReference content) { super(); - this.scriptLang = scriptLang; - } - public PutStoredScriptRequest(String scriptLang, String id) { - super(); - this.scriptLang = scriptLang; this.id = id; + this.lang = lang; + this.content = content; } @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = null; - if (id == null) { - validationException = addValidationError("id is missing", validationException); + + if (id == null || id.isEmpty()) { + validationException = addValidationError("must specify id for stored script", validationException); } else if (id.contains("#")) { - validationException = addValidationError("id can't contain: '#'", validationException); + validationException = addValidationError("id cannot contain '#' for stored script", validationException); } - if (scriptLang == null) { - validationException = addValidationError("lang is missing", validationException); - } else if (scriptLang.contains("#")) { - validationException = addValidationError("lang can't contain: '#'", validationException); + + if (lang != null && lang.contains("#")) { + validationException = addValidationError("lang cannot contain '#' for stored script", validationException); } - if (script == null) { - validationException = addValidationError("script is missing", validationException); + + if (content == null) { + validationException = addValidationError("must specify code for stored script", validationException); } + return validationException; } - public String scriptLang() { - return scriptLang; - } - - public PutStoredScriptRequest scriptLang(String scriptLang) { - this.scriptLang = scriptLang; - return this; - } - public String id() { return id; } public PutStoredScriptRequest id(String id) { this.id = id; + return this; } - public BytesReference script() { - return script; + public String lang() { + return lang; } - public PutStoredScriptRequest script(BytesReference source) { - this.script = source; + public PutStoredScriptRequest lang(String lang) { + this.lang = lang; + + return this; + } + + public BytesReference content() { + return content; + } + + public PutStoredScriptRequest content(BytesReference content) { + this.content = content; + return this; } @Override public void readFrom(StreamInput in) throws IOException { super.readFrom(in); - scriptLang = in.readString(); + + lang = in.readString(); + + if (lang.isEmpty()) { + lang = null; + } + id = in.readOptionalString(); - script = in.readBytesReference(); + content = in.readBytesReference(); } @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - out.writeString(scriptLang); + + out.writeString(lang == null ? "" : lang); out.writeOptionalString(id); - out.writeBytesReference(script); + out.writeBytesReference(content); } @Override public String toString() { - String sSource = "_na_"; + String source = "_na_"; + try { - sSource = XContentHelper.convertToJson(script, false); - } catch (Exception e) { + source = XContentHelper.convertToJson(content, false); + } catch (Exception exception) { // ignore } - return "put script {[" + id + "][" + scriptLang + "], script[" + sSource + "]}"; + + return "put stored script {id [" + id + "]" + (lang != null ? ", lang [" + lang + "]" : "") + ", content [" + source + "]}"; } } diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptRequestBuilder.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptRequestBuilder.java index 15c51c2ccd7..b701745e47a 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptRequestBuilder.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptRequestBuilder.java @@ -30,19 +30,21 @@ public class PutStoredScriptRequestBuilder extends AcknowledgedRequestBuilder listener) throws Exception { - scriptService.storeScript(clusterService, request, listener); + scriptService.putStoredScript(clusterService, request, listener); } @Override diff --git a/core/src/main/java/org/elasticsearch/action/bulk/byscroll/AbstractAsyncBulkByScrollAction.java b/core/src/main/java/org/elasticsearch/action/bulk/byscroll/AbstractAsyncBulkByScrollAction.java index 2336e422ac2..834321f1798 100644 --- a/core/src/main/java/org/elasticsearch/action/bulk/byscroll/AbstractAsyncBulkByScrollAction.java +++ b/core/src/main/java/org/elasticsearch/action/bulk/byscroll/AbstractAsyncBulkByScrollAction.java @@ -761,7 +761,7 @@ public abstract class AbstractAsyncBulkByScrollAction, SearchScript> getLazySearchScript(Script script, ScriptContext context) { failIfFrozen(); - CompiledScript compile = scriptService.compile(script, context, script.getOptions()); + CompiledScript compile = scriptService.compile(script, context); return (p) -> scriptService.search(lookup(), compile, p); } @@ -360,7 +360,7 @@ public class QueryShardContext extends QueryRewriteContext { */ public final Function, ExecutableScript> getLazyExecutableScript(Script script, ScriptContext context) { failIfFrozen(); - CompiledScript executable = scriptService.compile(script, context, script.getOptions()); + CompiledScript executable = scriptService.compile(script, context); return (p) -> scriptService.executable(executable, p); } diff --git a/core/src/main/java/org/elasticsearch/ingest/InternalTemplateService.java b/core/src/main/java/org/elasticsearch/ingest/InternalTemplateService.java index 6bb410c78ea..aedbe70e434 100644 --- a/core/src/main/java/org/elasticsearch/ingest/InternalTemplateService.java +++ b/core/src/main/java/org/elasticsearch/ingest/InternalTemplateService.java @@ -44,10 +44,7 @@ public class InternalTemplateService implements TemplateService { int mustacheEnd = template.indexOf("}}"); if (mustacheStart != -1 && mustacheEnd != -1 && mustacheStart < mustacheEnd) { Script script = new Script(ScriptType.INLINE, "mustache", template, Collections.emptyMap()); - CompiledScript compiledScript = scriptService.compile( - script, - ScriptContext.Standard.INGEST, - Collections.emptyMap()); + CompiledScript compiledScript = scriptService.compile(script, ScriptContext.Standard.INGEST); return new Template() { @Override public String execute(Map model) { diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestDeleteStoredScriptAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestDeleteStoredScriptAction.java index d9552477b64..102ae6bd571 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestDeleteStoredScriptAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestDeleteStoredScriptAction.java @@ -31,25 +31,35 @@ import java.io.IOException; import static org.elasticsearch.rest.RestRequest.Method.DELETE; public class RestDeleteStoredScriptAction extends BaseRestHandler { + public RestDeleteStoredScriptAction(Settings settings, RestController controller) { - this(settings, controller, true); - } - - protected RestDeleteStoredScriptAction(Settings settings, RestController controller, boolean registerDefaultHandlers) { super(settings); - if (registerDefaultHandlers) { - controller.registerHandler(DELETE, "/_scripts/{lang}/{id}", this); - } - } - protected String getScriptLang(RestRequest request) { - return request.param("lang"); + // Note {lang} is actually {id} in the first handler. It appears + // parameters as part of the path must be of the same ordering relative + // to name or they will not work as expected. + controller.registerHandler(DELETE, "/_scripts/{lang}", this); + controller.registerHandler(DELETE, "/_scripts/{lang}/{id}", this); } @Override - public RestChannelConsumer prepareRequest(final RestRequest request, NodeClient client) throws IOException { - DeleteStoredScriptRequest deleteStoredScriptRequest = new DeleteStoredScriptRequest(getScriptLang(request), request.param("id")); + public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + String id = request.param("id"); + String lang = request.param("lang"); + + // In the case where only {lang} is not null, we make it {id} because of + // name ordering issues in the handlers' paths. + if (id == null) { + id = lang; + lang = null; + } + + if (lang != null) { + deprecationLogger.deprecated( + "specifying lang [" + lang + "] as part of the url path is deprecated"); + } + + DeleteStoredScriptRequest deleteStoredScriptRequest = new DeleteStoredScriptRequest(id, lang); return channel -> client.admin().cluster().deleteStoredScript(deleteStoredScriptRequest, new AcknowledgedRestListener<>(channel)); } - } diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetStoredScriptAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetStoredScriptAction.java index 2725699d9fa..0c3857ffb58 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetStoredScriptAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetStoredScriptAction.java @@ -21,6 +21,7 @@ package org.elasticsearch.rest.action.admin.cluster; import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptRequest; import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptResponse; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.ParseField; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.rest.BaseRestHandler; @@ -30,57 +31,84 @@ import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestResponse; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.rest.action.RestBuilderListener; +import org.elasticsearch.script.StoredScriptSource; import java.io.IOException; import static org.elasticsearch.rest.RestRequest.Method.GET; public class RestGetStoredScriptAction extends BaseRestHandler { + + public static final ParseField _ID_PARSE_FIELD = new ParseField("_id"); + + public static final ParseField FOUND_PARSE_FIELD = new ParseField("found"); + public RestGetStoredScriptAction(Settings settings, RestController controller) { - this(settings, controller, true); - } - - protected RestGetStoredScriptAction(Settings settings, RestController controller, boolean registerDefaultHandlers) { super(settings); - if (registerDefaultHandlers) { - controller.registerHandler(GET, "/_scripts/{lang}/{id}", this); - } - } - protected String getScriptFieldName() { - return Fields.SCRIPT; - } - - protected String getScriptLang(RestRequest request) { - return request.param("lang"); + // Note {lang} is actually {id} in the first handler. It appears + // parameters as part of the path must be of the same ordering relative + // to name or they will not work as expected. + controller.registerHandler(GET, "/_scripts/{lang}", this); + controller.registerHandler(GET, "/_scripts/{lang}/{id}", this); } @Override public RestChannelConsumer prepareRequest(final RestRequest request, NodeClient client) throws IOException { - final GetStoredScriptRequest getRequest = new GetStoredScriptRequest(getScriptLang(request), request.param("id")); + String id; + String lang; + + // In the case where only {lang} is not null, we make it {id} because of + // name ordering issues in the handlers' paths. + if (request.param("id") == null) { + id = request.param("lang");; + lang = null; + } else { + id = request.param("id"); + lang = request.param("lang"); + } + + if (lang != null) { + deprecationLogger.deprecated( + "specifying lang [" + lang + "] as part of the url path is deprecated"); + } + + GetStoredScriptRequest getRequest = new GetStoredScriptRequest(id, lang); + return channel -> client.admin().cluster().getStoredScript(getRequest, new RestBuilderListener(channel) { @Override public RestResponse buildResponse(GetStoredScriptResponse response, XContentBuilder builder) throws Exception { builder.startObject(); - builder.field(Fields.LANG, getRequest.lang()); - builder.field(Fields._ID, getRequest.id()); - boolean found = response.getStoredScript() != null; - builder.field(Fields.FOUND, found); - RestStatus status = RestStatus.NOT_FOUND; - if (found) { - builder.field(getScriptFieldName(), response.getStoredScript()); - status = RestStatus.OK; + builder.field(_ID_PARSE_FIELD.getPreferredName(), id); + + if (lang != null) { + builder.field(StoredScriptSource.LANG_PARSE_FIELD.getPreferredName(), lang); } + + StoredScriptSource source = response.getSource(); + boolean found = source != null; + builder.field(FOUND_PARSE_FIELD.getPreferredName(), found); + + if (found) { + if (lang == null) { + builder.startObject(StoredScriptSource.SCRIPT_PARSE_FIELD.getPreferredName()); + builder.field(StoredScriptSource.LANG_PARSE_FIELD.getPreferredName(), source.getLang()); + builder.field(StoredScriptSource.CODE_PARSE_FIELD.getPreferredName(), source.getCode()); + + if (source.getOptions().isEmpty() == false) { + builder.field(StoredScriptSource.OPTIONS_PARSE_FIELD.getPreferredName(), source.getOptions()); + } + + builder.endObject(); + } else { + builder.field(StoredScriptSource.SCRIPT_PARSE_FIELD.getPreferredName(), source.getCode()); + } + } + builder.endObject(); - return new BytesRestResponse(status, builder); + + return new BytesRestResponse(found ? RestStatus.OK : RestStatus.NOT_FOUND, builder); } }); } - - private static final class Fields { - private static final String SCRIPT = "script"; - private static final String LANG = "lang"; - private static final String _ID = "_id"; - private static final String FOUND = "found"; - } } diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestPutStoredScriptAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestPutStoredScriptAction.java index 78c4d5ed16d..cee46eba04a 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestPutStoredScriptAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestPutStoredScriptAction.java @@ -20,6 +20,7 @@ package org.elasticsearch.rest.action.admin.cluster; import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestController; @@ -32,26 +33,39 @@ import static org.elasticsearch.rest.RestRequest.Method.POST; import static org.elasticsearch.rest.RestRequest.Method.PUT; public class RestPutStoredScriptAction extends BaseRestHandler { + public RestPutStoredScriptAction(Settings settings, RestController controller) { - this(settings, controller, true); - } - - protected RestPutStoredScriptAction(Settings settings, RestController controller, boolean registerDefaultHandlers) { super(settings); - if (registerDefaultHandlers) { - controller.registerHandler(POST, "/_scripts/{lang}/{id}", this); - controller.registerHandler(PUT, "/_scripts/{lang}/{id}", this); - } - } - protected String getScriptLang(RestRequest request) { - return request.param("lang"); + // Note {lang} is actually {id} in the first two handlers. It appears + // parameters as part of the path must be of the same ordering relative + // to name or they will not work as expected. + controller.registerHandler(POST, "/_scripts/{lang}", this); + controller.registerHandler(PUT, "/_scripts/{lang}", this); + controller.registerHandler(POST, "/_scripts/{lang}/{id}", this); + controller.registerHandler(PUT, "/_scripts/{lang}/{id}", this); } @Override - public RestChannelConsumer prepareRequest(final RestRequest request, NodeClient client) throws IOException { - PutStoredScriptRequest putRequest = new PutStoredScriptRequest(getScriptLang(request), request.param("id")); - putRequest.script(request.content()); + public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + String id = request.param("id"); + String lang = request.param("lang"); + + // In the case where only {lang} is not null, we make it {id} because of + // name ordering issues in the handlers' paths. + if (id == null) { + id = lang; + lang = null; + } + + BytesReference content = request.content(); + + if (lang != null) { + deprecationLogger.deprecated( + "specifying lang [" + lang + "] as part of the url path is deprecated, use request content instead"); + } + + PutStoredScriptRequest putRequest = new PutStoredScriptRequest(id, lang, content); return channel -> client.admin().cluster().putStoredScript(putRequest, new AcknowledgedRestListener<>(channel)); } } diff --git a/core/src/main/java/org/elasticsearch/script/Script.java b/core/src/main/java/org/elasticsearch/script/Script.java index 5c7f0ec62eb..67010770d83 100644 --- a/core/src/main/java/org/elasticsearch/script/Script.java +++ b/core/src/main/java/org/elasticsearch/script/Script.java @@ -19,12 +19,15 @@ package org.elasticsearch.script; +import org.apache.logging.log4j.Logger; import org.elasticsearch.Version; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.ObjectParser.ValueType; import org.elasticsearch.common.xcontent.ToXContentObject; @@ -42,12 +45,58 @@ import java.util.Map; import java.util.Objects; /** - * Script represents used-defined input that can be used to + * {@link Script} represents used-defined input that can be used to * compile and execute a script from the {@link ScriptService} * based on the {@link ScriptType}. + * + * There are three types of scripts specified by {@link ScriptType}. + * + * The following describes the expected parameters for each type of script: + * + *
    + *
  • {@link ScriptType#INLINE} + *
      + *
    • {@link Script#lang} - specifies the language, defaults to {@link Script#DEFAULT_SCRIPT_LANG} + *
    • {@link Script#idOrCode} - specifies the code to be compiled, must not be {@code null} + *
    • {@link Script#options} - specifies the compiler options for this script; must not be {@code null}, + * use an empty {@link Map} to specify no options + *
    • {@link Script#params} - {@link Map} of user-defined parameters; must not be {@code null}, + * use an empty {@link Map} to specify no params + *
    + *
  • {@link ScriptType#STORED} + *
      + *
    • {@link Script#lang} - the language will be specified when storing the script, so this should + * be {@code null}; however, this can be specified to look up a stored + * script as part of the deprecated API + *
    • {@link Script#idOrCode} - specifies the id of the stored script to be looked up, must not be {@code null} + *
    • {@link Script#options} - compiler options will be specified when a stored script is stored, + * so they have no meaning here and must be {@code null} + *
    • {@link Script#params} - {@link Map} of user-defined parameters; must not be {@code null}, + * use an empty {@link Map} to specify no params + *
    + *
  • {@link ScriptType#FILE} + *
      + *
    • {@link Script#lang} - specifies the language for look up, defaults to {@link Script#DEFAULT_SCRIPT_LANG} + *
    • {@link Script#idOrCode} - specifies the id of the file script to be looked up, must not be {@code null} + *
    • {@link Script#options} - compiler options will be specified when a file script is loaded, + * so they have no meaning here and must be {@code null} + *
    • {@link Script#params} - {@link Map} of user-defined parameters; must not be {@code null}, + * use an empty {@link Map} to specify no params + *
    + *
*/ public final class Script implements ToXContentObject, Writeable { + /** + * Standard logger necessary for allocation of the deprecation logger. + */ + private static final Logger LOGGER = ESLoggerFactory.getLogger(ScriptMetaData.class); + + /** + * Deprecation logger necessary for namespace changes related to stored scripts. + */ + private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(LOGGER); + /** * The name of the of the default scripting language. */ @@ -196,15 +245,62 @@ public final class Script implements ToXContentObject, Writeable { "or [" + ScriptType.FILE.getParseField().getPreferredName() + "] script"); } - if (idOrCode == null) { - throw new IllegalArgumentException("must specify an id or code for a script"); + if (type == ScriptType.INLINE) { + if (lang == null) { + lang = defaultLang; + } + + if (idOrCode == null) { + throw new IllegalArgumentException( + "must specify for an [" + ScriptType.INLINE.getParseField().getPreferredName() + "] script"); + } + + if (options.size() > 1 || options.size() == 1 && options.get(CONTENT_TYPE_OPTION) == null) { + options.remove(CONTENT_TYPE_OPTION); + + throw new IllegalArgumentException("illegal compiler options [" + options + "] specified"); + } + } else if (type == ScriptType.STORED) { + // Only issue this deprecation warning if we aren't using a template. Templates during + // this deprecation phase must always specify the default template language or they would + // possibly pick up a script in a different language as defined by the user under the new + // namespace unintentionally. + if (lang != null && lang.equals(DEFAULT_TEMPLATE_LANG) == false) { + DEPRECATION_LOGGER.deprecated("specifying the field [" + LANG_PARSE_FIELD.getPreferredName() + "] " + + "for executing " + ScriptType.STORED + " scripts is deprecated; use only the field " + + "[" + ScriptType.STORED.getParseField().getPreferredName() + "] to specify an "); + } + + if (idOrCode == null) { + throw new IllegalArgumentException( + "must specify for an [" + ScriptType.STORED.getParseField().getPreferredName() + "] script"); + } + + if (options.isEmpty()) { + options = null; + } else { + throw new IllegalArgumentException("field [" + OPTIONS_PARSE_FIELD.getPreferredName() + "] " + + "cannot be specified using a [" + ScriptType.STORED.getParseField().getPreferredName() + "] script"); + } + } else if (type == ScriptType.FILE) { + if (lang == null) { + lang = defaultLang; + } + + if (idOrCode == null) { + throw new IllegalArgumentException( + "must specify for an [" + ScriptType.FILE.getParseField().getPreferredName() + "] script"); + } + + if (options.isEmpty()) { + options = null; + } else { + throw new IllegalArgumentException("field [" + OPTIONS_PARSE_FIELD.getPreferredName() + "] " + + "cannot be specified using a [" + ScriptType.FILE.getParseField().getPreferredName() + "] script"); + } } - if (options.size() > 1 || options.size() == 1 && options.get(CONTENT_TYPE_OPTION) == null) { - throw new IllegalArgumentException("illegal compiler options [" + options + "] specified"); - } - - return new Script(type, this.lang == null ? defaultLang : this.lang, idOrCode, options, params); + return new Script(type, lang, idOrCode, options, params); } } @@ -292,6 +388,7 @@ public final class Script implements ToXContentObject, Writeable { * @param defaultLang The default language to use if no language is specified. The default language isn't necessarily * the one defined by {@link Script#DEFAULT_SCRIPT_LANG} due to backwards compatiblity requirements * related to stored queries using previously default languauges. + * * @return The parsed {@link Script}. */ public static Script parse(XContentParser parser, String defaultLang) throws IOException { @@ -327,34 +424,57 @@ public final class Script implements ToXContentObject, Writeable { /** * Constructor for a script that does not need to use compiler options. * @param type The {@link ScriptType}. - * @param lang The lang for this {@link Script}. + * @param lang The language for this {@link Script} if the {@link ScriptType} is {@link ScriptType#INLINE} or + * {@link ScriptType#FILE}. For {@link ScriptType#STORED} scripts this should be null, but can + * be specified to access scripts stored as part of the stored scripts deprecated API. * @param idOrCode The id for this {@link Script} if the {@link ScriptType} is {@link ScriptType#FILE} or {@link ScriptType#STORED}. * The code for this {@link Script} if the {@link ScriptType} is {@link ScriptType#INLINE}. * @param params The user-defined params to be bound for script execution. */ public Script(ScriptType type, String lang, String idOrCode, Map params) { - this(type, lang, idOrCode, Collections.emptyMap(), params); + this(type, lang, idOrCode, type == ScriptType.INLINE ? Collections.emptyMap() : null, params); } /** * Constructor for a script that requires the use of compiler options. * @param type The {@link ScriptType}. - * @param lang The lang for this {@link Script}. + * @param lang The language for this {@link Script} if the {@link ScriptType} is {@link ScriptType#INLINE} or + * {@link ScriptType#FILE}. For {@link ScriptType#STORED} scripts this should be null, but can + * be specified to access scripts stored as part of the stored scripts deprecated API. * @param idOrCode The id for this {@link Script} if the {@link ScriptType} is {@link ScriptType#FILE} or {@link ScriptType#STORED}. * The code for this {@link Script} if the {@link ScriptType} is {@link ScriptType#INLINE}. - * @param options The options to be passed to the compiler for use at compile-time. + * @param options The map of compiler options for this {@link Script} if the {@link ScriptType} + * is {@link ScriptType#INLINE}, {@code null} otherwise. * @param params The user-defined params to be bound for script execution. */ public Script(ScriptType type, String lang, String idOrCode, Map options, Map params) { - this.idOrCode = Objects.requireNonNull(idOrCode); this.type = Objects.requireNonNull(type); - this.lang = Objects.requireNonNull(lang); - this.options = Collections.unmodifiableMap(Objects.requireNonNull(options)); + this.idOrCode = Objects.requireNonNull(idOrCode); this.params = Collections.unmodifiableMap(Objects.requireNonNull(params)); - if (type != ScriptType.INLINE && !options.isEmpty()) { - throw new IllegalArgumentException( - "Compiler options [" + options + "] cannot be specified at runtime for [" + type + "] scripts."); + if (type == ScriptType.INLINE) { + this.lang = Objects.requireNonNull(lang); + this.options = Collections.unmodifiableMap(Objects.requireNonNull(options)); + } else if (type == ScriptType.STORED) { + this.lang = lang; + + if (options != null) { + throw new IllegalStateException( + "options must be null for [" + ScriptType.STORED.getParseField().getPreferredName() + "] scripts"); + } + + this.options = null; + } else if (type == ScriptType.FILE) { + this.lang = Objects.requireNonNull(lang); + + if (options != null) { + throw new IllegalStateException( + "options must be null for [" + ScriptType.FILE.getParseField().getPreferredName() + "] scripts"); + } + + this.options = null; + } else { + throw new IllegalStateException("unknown script type [" + type.getName() + "]"); } } @@ -362,82 +482,125 @@ public final class Script implements ToXContentObject, Writeable { * Creates a {@link Script} read from an input stream. */ public Script(StreamInput in) throws IOException { - // Version 5.1+ requires all Script members to be non-null and supports the potential - // for more options than just XContentType. Reorders the read in contents to be in - // same order as the constructor. - if (in.getVersion().onOrAfter(Version.V_5_1_1_UNRELEASED)) { + // Version 5.3 allows lang to be an optional parameter for stored scripts and expects + // options to be null for stored and file scripts. + if (in.getVersion().onOrAfter(Version.V_5_3_0_UNRELEASED)) { this.type = ScriptType.readFrom(in); - this.lang = in.readString(); + this.lang = in.readOptionalString(); this.idOrCode = in.readString(); @SuppressWarnings("unchecked") Map options = (Map)(Map)in.readMap(); this.options = options; this.params = in.readMap(); - // Prior to version 5.1 the script members are read in certain cases as optional and given - // default values when necessary. Also the only option supported is for XContentType. + // Version 5.1 to 5.3 (exclusive) requires all Script members to be non-null and supports the potential + // for more options than just XContentType. Reorders the read in contents to be in + // same order as the constructor. + } else if (in.getVersion().onOrAfter(Version.V_5_1_1_UNRELEASED)) { + this.type = ScriptType.readFrom(in); + this.lang = in.readString(); + + this.idOrCode = in.readString(); + @SuppressWarnings("unchecked") + Map options = (Map)(Map)in.readMap(); + + if (this.type != ScriptType.INLINE && options.isEmpty()) { + this.options = null; + } else { + this.options = options; + } + + this.params = in.readMap(); + // Prior to version 5.1 the script members are read in certain cases as optional and given + // default values when necessary. Also the only option supported is for XContentType. } else { - String idOrCode = in.readString(); - ScriptType type; + this.idOrCode = in.readString(); if (in.readBoolean()) { - type = ScriptType.readFrom(in); + this.type = ScriptType.readFrom(in); } else { - type = DEFAULT_SCRIPT_TYPE; + this.type = DEFAULT_SCRIPT_TYPE; } String lang = in.readOptionalString(); if (lang == null) { - lang = DEFAULT_SCRIPT_LANG; + this.lang = DEFAULT_SCRIPT_LANG; + } else { + this.lang = lang; } Map params = in.readMap(); if (params == null) { - params = new HashMap<>(); + this.params = new HashMap<>(); + } else { + this.params = params; } - Map options = new HashMap<>(); - if (in.readBoolean()) { + this.options = new HashMap<>(); XContentType contentType = XContentType.readFrom(in); - options.put(CONTENT_TYPE_OPTION, contentType.mediaType()); + this.options.put(CONTENT_TYPE_OPTION, contentType.mediaType()); + } else if (type == ScriptType.INLINE) { + options = new HashMap<>(); + } else { + this.options = null; } - - this.type = type; - this.lang = lang; - this.idOrCode = idOrCode; - this.options = options; - this.params = params; } } @Override public void writeTo(StreamOutput out) throws IOException { - // Version 5.1+ requires all Script members to be non-null and supports the potential - // for more options than just XContentType. Reorders the written out contents to be in - // same order as the constructor. - if (out.getVersion().onOrAfter(Version.V_5_1_1_UNRELEASED)) { + // Version 5.3+ allows lang to be an optional parameter for stored scripts and expects + // options to be null for stored and file scripts. + if (out.getVersion().onOrAfter(Version.V_5_3_0_UNRELEASED)) { type.writeTo(out); - out.writeString(lang); + out.writeOptionalString(lang); out.writeString(idOrCode); @SuppressWarnings("unchecked") Map options = (Map)(Map)this.options; out.writeMap(options); out.writeMap(params); - // Prior to version 5.1 the Script members were possibly written as optional or null, though this is no longer - // necessary since Script members cannot be null anymore, and there is no case where a null value wasn't equivalent - // to it's default value when actually compiling/executing a script. Meaning, there are no backwards compatibility issues, - // and now there's enforced consistency. Also the only supported compiler option was XContentType. + // Version 5.1 to 5.3 (exclusive) requires all Script members to be non-null and supports the potential + // for more options than just XContentType. Reorders the written out contents to be in + // same order as the constructor. + } else if (out.getVersion().onOrAfter(Version.V_5_1_1_UNRELEASED)) { + type.writeTo(out); + + if (lang == null) { + out.writeString(""); + } else { + out.writeString(lang); + } + + out.writeString(idOrCode); + @SuppressWarnings("unchecked") + Map options = (Map)(Map)this.options; + + if (options == null) { + out.writeMap(new HashMap<>()); + } else { + out.writeMap(options); + } + + out.writeMap(params); + // Prior to version 5.1 the Script members were possibly written as optional or null, though there is no case where a null + // value wasn't equivalent to it's default value when actually compiling/executing a script. Meaning, there are no + // backwards compatibility issues, and now there's enforced consistency. Also the only supported compiler + // option was XContentType. } else { out.writeString(idOrCode); out.writeBoolean(true); type.writeTo(out); - out.writeBoolean(true); - out.writeString(lang); - out.writeMap(params.isEmpty() ? null : params); + out.writeOptionalString(lang); - if (options.containsKey(CONTENT_TYPE_OPTION)) { + if (params.isEmpty()) { + out.writeMap(null); + } else { + out.writeMap(params); + } + + if (options != null && options.containsKey(CONTENT_TYPE_OPTION)) { XContentType contentType = XContentType.fromMediaTypeOrFormat(options.get(CONTENT_TYPE_OPTION)); out.writeBoolean(true); contentType.writeTo(out); @@ -478,7 +641,7 @@ public final class Script implements ToXContentObject, Writeable { * } * } * - * Note that options and params will only be included if there have been any specified. + * Note that lang, options, and params will only be included if there have been any specified. * * This also handles templates in a special way. If the {@link Script#CONTENT_TYPE_OPTION} option * is provided and the {@link ScriptType#INLINE} is specified then the template will be preserved as a raw field. @@ -504,7 +667,7 @@ public final class Script implements ToXContentObject, Writeable { public XContentBuilder toXContent(XContentBuilder builder, Params builderParams) throws IOException { builder.startObject(); - String contentType = options.get(CONTENT_TYPE_OPTION); + String contentType = options == null ? null : options.get(CONTENT_TYPE_OPTION); if (type == ScriptType.INLINE && contentType != null && builder.contentType().mediaType().equals(contentType)) { builder.rawField(type.getParseField().getPreferredName(), new BytesArray(idOrCode)); @@ -512,9 +675,11 @@ public final class Script implements ToXContentObject, Writeable { builder.field(type.getParseField().getPreferredName(), idOrCode); } - builder.field(LANG_PARSE_FIELD.getPreferredName(), lang); + if (lang != null) { + builder.field(LANG_PARSE_FIELD.getPreferredName(), lang); + } - if (!options.isEmpty()) { + if (options != null && !options.isEmpty()) { builder.field(OPTIONS_PARSE_FIELD.getPreferredName(), options); } @@ -527,6 +692,22 @@ public final class Script implements ToXContentObject, Writeable { return builder; } + /** + * @return The {@link ScriptType} for this {@link Script}. + */ + public ScriptType getType() { + return type; + } + + /** + * @return The language for this {@link Script} if the {@link ScriptType} is {@link ScriptType#INLINE} or + * {@link ScriptType#FILE}. For {@link ScriptType#STORED} scripts this should be null, but can + * be specified to access scripts stored as part of the stored scripts deprecated API. + */ + public String getLang() { + return lang; + } + /** * @return The id for this {@link Script} if the {@link ScriptType} is {@link ScriptType#FILE} or {@link ScriptType#STORED}. * The code for this {@link Script} if the {@link ScriptType} is {@link ScriptType#INLINE}. @@ -536,21 +717,8 @@ public final class Script implements ToXContentObject, Writeable { } /** - * @return The {@link ScriptType} for this {@link Script}. - */ - public ScriptType getType() { - return type; - } - - /** - * @return The language for this {@link Script}. - */ - public String getLang() { - return lang; - } - - /** - * @return The map of compiler options for this {@link Script}. + * @return The map of compiler options for this {@link Script} if the {@link ScriptType} + * is {@link ScriptType#INLINE}, {@code null} otherwise. */ public Map getOptions() { return options; @@ -571,9 +739,9 @@ public final class Script implements ToXContentObject, Writeable { Script script = (Script)o; if (type != script.type) return false; - if (!lang.equals(script.lang)) return false; + if (lang != null ? !lang.equals(script.lang) : script.lang != null) return false; if (!idOrCode.equals(script.idOrCode)) return false; - if (!options.equals(script.options)) return false; + if (options != null ? !options.equals(script.options) : script.options != null) return false; return params.equals(script.params); } @@ -581,9 +749,9 @@ public final class Script implements ToXContentObject, Writeable { @Override public int hashCode() { int result = type.hashCode(); - result = 31 * result + lang.hashCode(); + result = 31 * result + (lang != null ? lang.hashCode() : 0); result = 31 * result + idOrCode.hashCode(); - result = 31 * result + options.hashCode(); + result = 31 * result + (options != null ? options.hashCode() : 0); result = 31 * result + params.hashCode(); return result; } diff --git a/core/src/main/java/org/elasticsearch/script/ScriptMetaData.java b/core/src/main/java/org/elasticsearch/script/ScriptMetaData.java index 7c99f3a80dd..e9fb6e126fc 100644 --- a/core/src/main/java/org/elasticsearch/script/ScriptMetaData.java +++ b/core/src/main/java/org/elasticsearch/script/ScriptMetaData.java @@ -18,24 +18,24 @@ */ package org.elasticsearch.script; +import org.apache.logging.log4j.Logger; import org.elasticsearch.ResourceNotFoundException; -import org.elasticsearch.cluster.AbstractDiffable; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.Diff; import org.elasticsearch.cluster.DiffableUtils; import org.elasticsearch.cluster.NamedDiff; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.common.ParsingException; -import org.elasticsearch.common.bytes.BytesArray; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.logging.ESLoggerFactory; +import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser.Token; -import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; import java.util.Collections; @@ -43,68 +43,372 @@ import java.util.EnumSet; import java.util.HashMap; import java.util.Map; -public final class ScriptMetaData implements MetaData.Custom { +/** + * {@link ScriptMetaData} is used to store user-defined scripts + * as part of the {@link ClusterState}. Currently scripts can + * be stored as part of the new namespace for a stored script where + * only an id is used or as part of the deprecated namespace where + * both a language and an id are used. + */ +public final class ScriptMetaData implements MetaData.Custom, Writeable, ToXContent { + /** + * A builder used to modify the currently stored scripts data held within + * the {@link ClusterState}. Scripts can be added or deleted, then built + * to generate a new {@link Map} of scripts that will be used to update + * the current {@link ClusterState}. + */ + public static final class Builder { + + private final Map scripts; + + /** + * @param previous The current {@link ScriptMetaData} or {@code null} if there + * is no existing {@link ScriptMetaData}. + */ + public Builder(ScriptMetaData previous) { + this.scripts = previous == null ? new HashMap<>() :new HashMap<>(previous.scripts); + } + + /** + * Add a new script to the existing stored scripts. The script will be added under + * both the new namespace and the deprecated namespace, so that look ups under + * the deprecated namespace will continue to work. Should a script already exist under + * the new namespace using a different language, it will be replaced and a deprecation + * warning will be issued. The replaced script will still exist under the deprecated + * namespace and can continue to be looked up this way until it is deleted. + *

+ * Take for example script 'A' with lang 'L0' and data 'D0'. If we add script 'A' to the + * empty set, the scripts {@link Map} will be ["A" -- D0, "A#L0" -- D0]. If a script + * 'A' with lang 'L1' and data 'D1' is then added, the scripts {@link Map} will be + * ["A" -- D1, "A#L1" -- D1, "A#L0" -- D0]. + * @param id The user-specified id to use for the look up. + * @param source The user-specified stored script data held in {@link StoredScriptSource}. + */ + public Builder storeScript(String id, StoredScriptSource source) { + StoredScriptSource previous = scripts.put(id, source); + scripts.put(source.getLang() + "#" + id, source); + + if (previous != null && previous.getLang().equals(source.getLang()) == false) { + DEPRECATION_LOGGER.deprecated("stored script [" + id + "] already exists using a different lang " + + "[" + previous.getLang() + "], the new namespace for stored scripts will only use (id) instead of (lang, id)"); + } + + return this; + } + + /** + * Delete a script from the existing stored scripts. The script will be removed from the + * new namespace if the script language matches the current script under the same id or + * if the script language is {@code null}. The script will be removed from the deprecated + * namespace on any delete either using using the specified lang parameter or the language + * found from looking up the script in the new namespace. + *

+ * Take for example a scripts {@link Map} with {"A" -- D1, "A#L1" -- D1, "A#L0" -- D0}. + * If a script is removed specified by an id 'A' and lang {@code null} then the scripts + * {@link Map} will be {"A#L0" -- D0}. To remove the final script, the deprecated + * namespace must be used, so an id 'A' and lang 'L0' would need to be specified. + * @param id The user-specified id to use for the look up. + * @param lang The user-specified language to use for the look up if using the deprecated + * namespace, otherwise {@code null}. + */ + public Builder deleteScript(String id, String lang) { + StoredScriptSource source = scripts.get(id); + + if (lang == null) { + if (source == null) { + throw new ResourceNotFoundException("stored script [" + id + "] does not exist and cannot be deleted"); + } + + lang = source.getLang(); + } + + if (source != null) { + if (lang.equals(source.getLang())) { + scripts.remove(id); + } + } + + source = scripts.get(lang + "#" + id); + + if (source == null) { + throw new ResourceNotFoundException( + "stored script [" + id + "] using lang [" + lang + "] does not exist and cannot be deleted"); + } + + scripts.remove(lang + "#" + id); + + return this; + } + + /** + * @return A {@link ScriptMetaData} with the updated {@link Map} of scripts. + */ + public ScriptMetaData build() { + return new ScriptMetaData(scripts); + } + } + + static final class ScriptMetadataDiff implements NamedDiff { + + final Diff> pipelines; + + ScriptMetadataDiff(ScriptMetaData before, ScriptMetaData after) { + this.pipelines = DiffableUtils.diff(before.scripts, after.scripts, DiffableUtils.getStringKeySerializer()); + } + + ScriptMetadataDiff(StreamInput in) throws IOException { + pipelines = DiffableUtils.readJdkMapDiff(in, DiffableUtils.getStringKeySerializer(), + StoredScriptSource::new, StoredScriptSource::readDiffFrom); + } + + @Override + public String getWriteableName() { + return TYPE; + } + + @Override + public MetaData.Custom apply(MetaData.Custom part) { + return new ScriptMetaData(pipelines.apply(((ScriptMetaData) part).scripts)); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + pipelines.writeTo(out); + } + } + + /** + * Convenience method to build and return a new + * {@link ScriptMetaData} adding the specified stored script. + */ + static ScriptMetaData putStoredScript(ScriptMetaData previous, String id, StoredScriptSource source) { + Builder builder = new Builder(previous); + builder.storeScript(id, source); + + return builder.build(); + } + + /** + * Convenience method to build and return a new + * {@link ScriptMetaData} deleting the specified stored script. + */ + static ScriptMetaData deleteStoredScript(ScriptMetaData previous, String id, String lang) { + Builder builder = new ScriptMetaData.Builder(previous); + builder.deleteScript(id, lang); + + return builder.build(); + } + + /** + * Standard logger necessary for allocation of the deprecation logger. + */ + private static final Logger LOGGER = ESLoggerFactory.getLogger(ScriptMetaData.class); + + /** + * Deprecation logger necessary for namespace changes related to stored scripts. + */ + private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(LOGGER); + + /** + * The type of {@link ClusterState} data. + */ public static final String TYPE = "stored_scripts"; - private final Map scripts; + /** + * This will parse XContent into {@link ScriptMetaData}. + * + * The following format will be parsed for the new namespace: + * + * {@code + * { + * "" : "<{@link StoredScriptSource#fromXContent(XContentParser)}>", + * "" : "<{@link StoredScriptSource#fromXContent(XContentParser)}>", + * ... + * } + * } + * + * The following format will be parsed for the deprecated namespace: + * + * {@code + * { + * "" : "", + * "" : "", + * ... + * } + * } + * + * Note when using the deprecated namespace, the language will be pulled from + * the id and options will be set to an empty {@link Map}. + */ + public static ScriptMetaData fromXContent(XContentParser parser) throws IOException { + Map scripts = new HashMap<>(); + String id = null; + StoredScriptSource source; - ScriptMetaData(Map scripts) { - this.scripts = scripts; - } + Token token = parser.currentToken(); - public BytesReference getScriptAsBytes(String language, String id) { - ScriptAsBytes scriptAsBytes = scripts.get(toKey(language, id)); - if (scriptAsBytes != null) { - return scriptAsBytes.script; - } else { - return null; + if (token == null) { + token = parser.nextToken(); } - } - public String getScript(String language, String id) { - BytesReference scriptAsBytes = getScriptAsBytes(language, id); - if (scriptAsBytes == null) { - return null; + if (token != Token.START_OBJECT) { + throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + "], expected [{]"); } - return scriptAsBytes.utf8ToString(); - } - public static String parseStoredScript(BytesReference scriptAsBytes) { - // Scripts can be stored via API in several ways: - // 1) wrapped into a 'script' json object or field - // 2) wrapped into a 'template' json object or field - // 3) just as is - // In order to fetch the actual script in consistent manner this parsing logic is needed: - // EMPTY is ok here because we never call namedObject, we're just copying structure. - try (XContentParser parser = XContentHelper.createParser(NamedXContentRegistry.EMPTY, scriptAsBytes); - XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { - parser.nextToken(); - parser.nextToken(); - if (parser.currentToken() == Token.END_OBJECT) { - throw new IllegalArgumentException("Empty script"); - } - switch (parser.currentName()) { - case "script": - case "template": - if (parser.nextToken() == Token.VALUE_STRING) { - return parser.text(); - } else { - builder.copyCurrentStructure(parser); + token = parser.nextToken(); + + while (token != Token.END_OBJECT) { + switch (token) { + case FIELD_NAME: + id = parser.currentName(); + break; + case VALUE_STRING: + if (id == null) { + throw new ParsingException(parser.getTokenLocation(), + "unexpected token [" + token + "], expected [, , {]"); } + + int split = id.indexOf('#'); + + if (split == -1) { + throw new IllegalArgumentException("illegal stored script id [" + id + "], does not contain lang"); + } else { + source = new StoredScriptSource(id.substring(0, split), parser.text(), Collections.emptyMap()); + } + + scripts.put(id, source); + + id = null; + + break; + case START_OBJECT: + if (id == null) { + throw new ParsingException(parser.getTokenLocation(), + "unexpected token [" + token + "], expected [, , {]"); + } + + source = StoredScriptSource.fromXContent(parser); + scripts.put(id, source); + break; default: - // There is no enclosing 'script' or 'template' object so we just need to return the script as is... - // because the parsers current location is already beyond the beginning we need to add a START_OBJECT: - builder.startObject(); - builder.copyCurrentStructure(parser); - builder.endObject(); - break; + throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + "], expected [, , {]"); } - return builder.string(); - } catch (IOException e) { - throw new RuntimeException(e); + + token = parser.nextToken(); } + + return new ScriptMetaData(scripts); + } + + public static NamedDiff readDiffFrom(StreamInput in) throws IOException { + return new ScriptMetadataDiff(in); + } + + private final Map scripts; + + /** + * Standard constructor to create metadata to store scripts. + * @param scripts The currently stored scripts. Must not be {@code null}, + * use and empty {@link Map} to specify there were no + * previously stored scripts. + */ + ScriptMetaData(Map scripts) { + this.scripts = Collections.unmodifiableMap(scripts); + } + + public ScriptMetaData(StreamInput in) throws IOException { + Map scripts = new HashMap<>(); + StoredScriptSource source; + int size = in.readVInt(); + + for (int i = 0; i < size; i++) { + String id = in.readString(); + + // Prior to version 5.3 all scripts were stored using the deprecated namespace. + // Split the id to find the language then use StoredScriptSource to parse the + // expected BytesReference after which a new StoredScriptSource is created + // with the appropriate language and options. + if (in.getVersion().before(Version.V_5_3_0_UNRELEASED)) { + int split = id.indexOf('#'); + + if (split == -1) { + throw new IllegalArgumentException("illegal stored script id [" + id + "], does not contain lang"); + } else { + source = new StoredScriptSource(in); + source = new StoredScriptSource(id.substring(0, split), source.getCode(), Collections.emptyMap()); + } + // Version 5.3+ can just be parsed normally using StoredScriptSource. + } else { + source = new StoredScriptSource(in); + } + + scripts.put(id, source); + } + + this.scripts = Collections.unmodifiableMap(scripts); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + // Version 5.3+ will output the contents of the scripts' Map using + // StoredScriptSource to stored the language, code, and options. + if (out.getVersion().onOrAfter(Version.V_5_3_0_UNRELEASED)) { + out.writeVInt(scripts.size()); + + for (Map.Entry entry : scripts.entrySet()) { + out.writeString(entry.getKey()); + entry.getValue().writeTo(out); + } + // Prior to Version 5.3, stored scripts can only be read using the deprecated + // namespace. Scripts using the deprecated namespace are first isolated in a + // temporary Map, then written out. Since all scripts will be stored using the + // deprecated namespace, no scripts will be lost. + } else { + Map filtered = new HashMap<>(); + + for (Map.Entry entry : scripts.entrySet()) { + if (entry.getKey().contains("#")) { + filtered.put(entry.getKey(), entry.getValue()); + } + } + + out.writeVInt(filtered.size()); + + for (Map.Entry entry : filtered.entrySet()) { + out.writeString(entry.getKey()); + entry.getValue().writeTo(out); + } + } + } + + + + /** + * This will write XContent from {@link ScriptMetaData}. The following format will be written: + * + * {@code + * { + * "" : "<{@link StoredScriptSource#toXContent(XContentBuilder, Params)}>", + * "" : "<{@link StoredScriptSource#toXContent(XContentBuilder, Params)}>", + * ... + * } + * } + */ + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + for (Map.Entry entry : scripts.entrySet()) { + builder.field(entry.getKey()); + entry.getValue().toXContent(builder, params); + } + + return builder; + } + + @Override + public Diff diff(MetaData.Custom before) { + return new ScriptMetadataDiff((ScriptMetaData)before, this); } @Override @@ -112,72 +416,33 @@ public final class ScriptMetaData implements MetaData.Custom { return TYPE; } - public static ScriptMetaData fromXContent(XContentParser parser) throws IOException { - Map scripts = new HashMap<>(); - String key = null; - for (Token token = parser.nextToken(); token != Token.END_OBJECT; token = parser.nextToken()) { - switch (token) { - case FIELD_NAME: - key = parser.currentName(); - break; - case VALUE_STRING: - scripts.put(key, new ScriptAsBytes(new BytesArray(parser.text()))); - break; - default: - throw new ParsingException(parser.getTokenLocation(), "Unexpected token [" + token + "]"); - } - } - return new ScriptMetaData(scripts); - } - @Override public EnumSet context() { return MetaData.ALL_CONTEXTS; } - public ScriptMetaData(StreamInput in) throws IOException { - int size = in.readVInt(); - this.scripts = new HashMap<>(); - for (int i = 0; i < size; i++) { - String languageAndId = in.readString(); - BytesReference script = in.readBytesReference(); - scripts.put(languageAndId, new ScriptAsBytes(script)); + /** + * Retrieves a stored script from the new namespace if lang is {@code null}. + * Otherwise, returns a stored script from the deprecated namespace. Either + * way an id is required. + */ + StoredScriptSource getStoredScript(String id, String lang) { + if (lang == null) { + return scripts.get(id); + } else { + return scripts.get(lang + "#" + id); } } - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - for (Map.Entry entry : scripts.entrySet()) { - builder.field(entry.getKey(), entry.getValue().script.utf8ToString()); - } - return builder; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeVInt(scripts.size()); - for (Map.Entry entry : scripts.entrySet()) { - out.writeString(entry.getKey()); - entry.getValue().writeTo(out); - } - } - - @Override - public Diff diff(MetaData.Custom before) { - return new ScriptMetadataDiff((ScriptMetaData) before, this); - } - - public static NamedDiff readDiffFrom(StreamInput in) throws IOException { - return new ScriptMetadataDiff(in); - } - @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - ScriptMetaData other = (ScriptMetaData) o; - return scripts.equals(other.scripts); + ScriptMetaData that = (ScriptMetaData)o; + + return scripts.equals(that.scripts); + } @Override @@ -191,112 +456,4 @@ public final class ScriptMetaData implements MetaData.Custom { "scripts=" + scripts + '}'; } - - static String toKey(String language, String id) { - if (id.contains("#")) { - throw new IllegalArgumentException("stored script id can't contain: '#'"); - } - if (language.contains("#")) { - throw new IllegalArgumentException("stored script language can't contain: '#'"); - } - - return language + "#" + id; - } - - public static final class Builder { - - private Map scripts; - - public Builder(ScriptMetaData previous) { - if (previous != null) { - this.scripts = new HashMap<>(previous.scripts); - } else { - this.scripts = new HashMap<>(); - } - } - - public Builder storeScript(String lang, String id, BytesReference script) { - BytesReference scriptBytest = new BytesArray(parseStoredScript(script)); - scripts.put(toKey(lang, id), new ScriptAsBytes(scriptBytest)); - return this; - } - - public Builder deleteScript(String lang, String id) { - if (scripts.remove(toKey(lang, id)) == null) { - throw new ResourceNotFoundException("Stored script with id [{}] for language [{}] does not exist", id, lang); - } - return this; - } - - public ScriptMetaData build() { - return new ScriptMetaData(Collections.unmodifiableMap(scripts)); - } - } - - static final class ScriptMetadataDiff implements NamedDiff { - - final Diff> pipelines; - - ScriptMetadataDiff(ScriptMetaData before, ScriptMetaData after) { - this.pipelines = DiffableUtils.diff(before.scripts, after.scripts, DiffableUtils.getStringKeySerializer()); - } - - public ScriptMetadataDiff(StreamInput in) throws IOException { - pipelines = DiffableUtils.readJdkMapDiff(in, DiffableUtils.getStringKeySerializer(), ScriptAsBytes::new, - ScriptAsBytes::readDiffFrom); - } - - @Override - public MetaData.Custom apply(MetaData.Custom part) { - return new ScriptMetaData(pipelines.apply(((ScriptMetaData) part).scripts)); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - pipelines.writeTo(out); - } - - @Override - public String getWriteableName() { - return TYPE; - } - } - - static final class ScriptAsBytes extends AbstractDiffable { - - public ScriptAsBytes(BytesReference script) { - this.script = script; - } - - private final BytesReference script; - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeBytesReference(script); - } - - public ScriptAsBytes(StreamInput in) throws IOException { - this(in.readBytesReference()); - } - - public static Diff readDiffFrom(StreamInput in) throws IOException { - return readDiffFrom(ScriptAsBytes::new, in); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - ScriptAsBytes that = (ScriptAsBytes) o; - - return script.equals(that.script); - - } - - @Override - public int hashCode() { - return script.hashCode(); - } - } } diff --git a/core/src/main/java/org/elasticsearch/script/ScriptService.java b/core/src/main/java/org/elasticsearch/script/ScriptService.java index 60cdb03e479..f9bcc77ab47 100644 --- a/core/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/core/src/main/java/org/elasticsearch/script/ScriptService.java @@ -38,7 +38,6 @@ import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Strings; import org.elasticsearch.common.breaker.CircuitBreakingException; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.cache.Cache; import org.elasticsearch.common.cache.CacheBuilder; import org.elasticsearch.common.cache.RemovalListener; @@ -46,7 +45,6 @@ import org.elasticsearch.common.cache.RemovalNotification; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.io.Streams; -import org.elasticsearch.common.logging.LoggerMessageFormat; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; @@ -209,18 +207,44 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust /** * Checks if a script can be executed and compiles it if needed, or returns the previously compiled and cached script. */ - public CompiledScript compile(Script script, ScriptContext scriptContext, Map params) { - if (script == null) { - throw new IllegalArgumentException("The parameter script (Script) must not be null."); - } - if (scriptContext == null) { - throw new IllegalArgumentException("The parameter scriptContext (ScriptContext) must not be null."); - } + public CompiledScript compile(Script script, ScriptContext scriptContext) { + Objects.requireNonNull(script); + Objects.requireNonNull(scriptContext); + ScriptType type = script.getType(); String lang = script.getLang(); - ScriptEngineService scriptEngineService = getScriptEngineServiceForLang(lang); - if (canExecuteScript(lang, script.getType(), scriptContext) == false) { - throw new IllegalStateException("scripts of type [" + script.getType() + "], operation [" + scriptContext.getKey() + "] and lang [" + lang + "] are disabled"); + String idOrCode = script.getIdOrCode(); + Map options = script.getOptions(); + + String id = idOrCode; + + // lang may be null when looking up a stored script, so we must get the + // source to retrieve the lang before checking if the context is supported + if (type == ScriptType.STORED) { + // search template requests can possibly pass in the entire path instead + // of just an id for looking up a stored script, so we parse the path and + // check for appropriate errors + String[] path = id.split("/"); + + if (path.length == 3) { + if (lang != null && lang.equals(path[1]) == false) { + throw new IllegalStateException("conflicting script languages, found [" + path[1] + "] but expected [" + lang + "]"); + } + + id = path[2]; + + deprecationLogger.deprecated("use of [" + idOrCode + "] for looking up" + + " stored scripts/templates has been deprecated, use only [" + id + "] instead"); + } else if (path.length != 1) { + throw new IllegalArgumentException("illegal stored script format [" + id + "] use only "); + } + + // a stored script must be pulled from the cluster state every time in case + // the script has been updated since the last compilation + StoredScriptSource source = getScriptFromClusterState(id, lang); + lang = source.getLang(); + idOrCode = source.getCode(); + options = source.getOptions(); } // TODO: fix this through some API or something, that's wrong @@ -229,10 +253,71 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust boolean notSupported = scriptContext.getKey().equals(ScriptContext.Standard.UPDATE.getKey()); if (expression && notSupported) { throw new UnsupportedOperationException("scripts of type [" + script.getType() + "]," + - " operation [" + scriptContext.getKey() + "] and lang [" + lang + "] are not supported"); + " operation [" + scriptContext.getKey() + "] and lang [" + lang + "] are not supported"); } - return compileInternal(script, params); + ScriptEngineService scriptEngineService = getScriptEngineServiceForLang(lang); + + if (canExecuteScript(lang, type, scriptContext) == false) { + throw new IllegalStateException("scripts of type [" + script.getType() + "]," + + " operation [" + scriptContext.getKey() + "] and lang [" + lang + "] are disabled"); + } + + if (logger.isTraceEnabled()) { + logger.trace("compiling lang: [{}] type: [{}] script: {}", lang, type, idOrCode); + } + + if (type == ScriptType.FILE) { + CacheKey cacheKey = new CacheKey(lang, idOrCode, options); + CompiledScript compiledScript = staticCache.get(cacheKey); + + if (compiledScript == null) { + throw new IllegalArgumentException("unable to find file script [" + idOrCode + "] using lang [" + lang + "]"); + } + + return compiledScript; + } + + CacheKey cacheKey = new CacheKey(lang, idOrCode, options); + CompiledScript compiledScript = cache.get(cacheKey); + + if (compiledScript != null) { + return compiledScript; + } + + // Synchronize so we don't compile scripts many times during multiple shards all compiling a script + synchronized (this) { + // Retrieve it again in case it has been put by a different thread + compiledScript = cache.get(cacheKey); + + if (compiledScript == null) { + try { + // Either an un-cached inline script or indexed script + // If the script type is inline the name will be the same as the code for identification in exceptions + + // but give the script engine the chance to be better, give it separate name + source code + // for the inline case, then its anonymous: null. + if (logger.isTraceEnabled()) { + logger.trace("compiling script, type: [{}], lang: [{}], options: [{}]", type, lang, options); + } + // Check whether too many compilations have happened + checkCompilationLimit(); + compiledScript = new CompiledScript(type, id, lang, scriptEngineService.compile(id, idOrCode, options)); + } catch (ScriptException good) { + // TODO: remove this try-catch completely, when all script engines have good exceptions! + throw good; // its already good + } catch (Exception exception) { + throw new GeneralScriptException("Failed to compile " + type + " script [" + id + "] using lang [" + lang + "]", exception); + } + + // Since the cache key is the script content itself we don't need to + // invalidate/check the cache if an indexed script changes. + scriptMetrics.onCompilation(); + cache.put(cacheKey, compiledScript); + } + + return compiledScript; + } } /** @@ -267,152 +352,71 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust } } - /** - * Compiles a script straight-away, or returns the previously compiled and cached script, - * without checking if it can be executed based on settings. - */ - CompiledScript compileInternal(Script script, Map params) { - if (script == null) { - throw new IllegalArgumentException("The parameter script (Script) must not be null."); - } + public boolean isLangSupported(String lang) { + Objects.requireNonNull(lang); - String lang = script.getLang(); - ScriptType type = script.getType(); - //script.getIdOrCode() could return either a name or code for a script, - //but we check for a file script name first and an indexed script name second - String name = script.getIdOrCode(); - - if (logger.isTraceEnabled()) { - logger.trace("Compiling lang: [{}] type: [{}] script: {}", lang, type, name); - } - - ScriptEngineService scriptEngineService = getScriptEngineServiceForLang(lang); - - if (type == ScriptType.FILE) { - CacheKey cacheKey = new CacheKey(scriptEngineService, name, null, params); - //On disk scripts will be loaded into the staticCache by the listener - CompiledScript compiledScript = staticCache.get(cacheKey); - - if (compiledScript == null) { - throw new IllegalArgumentException("Unable to find on disk file script [" + name + "] using lang [" + lang + "]"); - } - - return compiledScript; - } - - //script.getIdOrCode() will be code if the script type is inline - String code = script.getIdOrCode(); - - if (type == ScriptType.STORED) { - //The look up for an indexed script must be done every time in case - //the script has been updated in the index since the last look up. - final IndexedScript indexedScript = new IndexedScript(lang, name); - name = indexedScript.id; - code = getScriptFromClusterState(indexedScript.lang, indexedScript.id); - } - - CacheKey cacheKey = new CacheKey(scriptEngineService, type == ScriptType.INLINE ? null : name, code, params); - CompiledScript compiledScript = cache.get(cacheKey); - - if (compiledScript != null) { - return compiledScript; - } - - // Synchronize so we don't compile scripts many times during multiple shards all compiling a script - synchronized (this) { - // Retrieve it again in case it has been put by a different thread - compiledScript = cache.get(cacheKey); - - if (compiledScript == null) { - try { - // Either an un-cached inline script or indexed script - // If the script type is inline the name will be the same as the code for identification in exceptions - - // but give the script engine the chance to be better, give it separate name + source code - // for the inline case, then its anonymous: null. - String actualName = (type == ScriptType.INLINE) ? null : name; - if (logger.isTraceEnabled()) { - logger.trace("compiling script, type: [{}], lang: [{}], params: [{}]", type, lang, params); - } - // Check whether too many compilations have happened - checkCompilationLimit(); - compiledScript = new CompiledScript(type, name, lang, scriptEngineService.compile(actualName, code, params)); - } catch (ScriptException good) { - // TODO: remove this try-catch completely, when all script engines have good exceptions! - throw good; // its already good - } catch (Exception exception) { - throw new GeneralScriptException("Failed to compile " + type + " script [" + name + "] using lang [" + lang + "]", exception); - } - - // Since the cache key is the script content itself we don't need to - // invalidate/check the cache if an indexed script changes. - scriptMetrics.onCompilation(); - cache.put(cacheKey, compiledScript); - } - - return compiledScript; - } + return scriptEnginesByLang.containsKey(lang); } - private String validateScriptLanguage(String scriptLang) { - Objects.requireNonNull(scriptLang); - if (scriptEnginesByLang.containsKey(scriptLang) == false) { - throw new IllegalArgumentException("script_lang not supported [" + scriptLang + "]"); + StoredScriptSource getScriptFromClusterState(String id, String lang) { + if (lang != null && isLangSupported(lang) == false) { + throw new IllegalArgumentException("unable to get stored script with unsupported lang [" + lang + "]"); } - return scriptLang; - } - String getScriptFromClusterState(String scriptLang, String id) { - scriptLang = validateScriptLanguage(scriptLang); ScriptMetaData scriptMetadata = clusterState.metaData().custom(ScriptMetaData.TYPE); + if (scriptMetadata == null) { - throw new ResourceNotFoundException("Unable to find script [" + scriptLang + "/" + id + "] in cluster state"); + throw new ResourceNotFoundException("unable to find script [" + id + "]" + + (lang == null ? "" : " using lang [" + lang + "]") + " in cluster state"); } - String script = scriptMetadata.getScript(scriptLang, id); - if (script == null) { - throw new ResourceNotFoundException("Unable to find script [" + scriptLang + "/" + id + "] in cluster state"); + StoredScriptSource source = scriptMetadata.getStoredScript(id, lang); + + if (source == null) { + throw new ResourceNotFoundException("unable to find script [" + id + "]" + + (lang == null ? "" : " using lang [" + lang + "]") + " in cluster state"); } - return script; + + return source; } - void validateStoredScript(String id, String scriptLang, BytesReference scriptBytes) { - validateScriptSize(id, scriptBytes.length()); - String script = ScriptMetaData.parseStoredScript(scriptBytes); - if (Strings.hasLength(scriptBytes)) { - //Just try and compile it - try { - ScriptEngineService scriptEngineService = getScriptEngineServiceForLang(scriptLang); - //we don't know yet what the script will be used for, but if all of the operations for this lang with - //indexed scripts are disabled, it makes no sense to even compile it. - if (isAnyScriptContextEnabled(scriptLang, ScriptType.STORED)) { - Object compiled = scriptEngineService.compile(id, script, Collections.emptyMap()); - if (compiled == null) { - throw new IllegalArgumentException("Unable to parse [" + script + "] lang [" + scriptLang + - "] (ScriptService.compile returned null)"); - } - } else { - logger.warn( - "skipping compile of script [{}], lang [{}] as all scripted operations are disabled for indexed scripts", - script, scriptLang); + public void putStoredScript(ClusterService clusterService, PutStoredScriptRequest request, + ActionListener listener) { + int max = SCRIPT_MAX_SIZE_IN_BYTES.get(settings); + + if (request.content().length() > max) { + throw new IllegalArgumentException("exceeded max allowed stored script size in bytes [" + max + "] with size [" + + request.content().length() + "] for script [" + request.id() + "]"); + } + + StoredScriptSource source = StoredScriptSource.parse(request.lang(), request.content()); + + if (isLangSupported(source.getLang()) == false) { + throw new IllegalArgumentException("unable to put stored script with unsupported lang [" + source.getLang() + "]"); + } + + try { + ScriptEngineService scriptEngineService = getScriptEngineServiceForLang(source.getLang()); + + if (isAnyScriptContextEnabled(source.getLang(), ScriptType.STORED)) { + Object compiled = scriptEngineService.compile(request.id(), source.getCode(), Collections.emptyMap()); + + if (compiled == null) { + throw new IllegalArgumentException("failed to parse/compile stored script [" + request.id() + "]" + + (source.getCode() == null ? "" : " using code [" + source.getCode() + "]")); } - } catch (ScriptException good) { - // TODO: remove this when all script engines have good exceptions! - throw good; // its already good! - } catch (Exception e) { - throw new IllegalArgumentException("Unable to parse [" + script + - "] lang [" + scriptLang + "]", e); + } else { + throw new IllegalArgumentException( + "cannot put stored script [" + request.id() + "], stored scripts cannot be run under any context"); } - } else { - throw new IllegalArgumentException("Unable to find script in : " + scriptBytes.utf8ToString()); + } catch (ScriptException good) { + throw good; + } catch (Exception exception) { + throw new IllegalArgumentException("failed to parse/compile stored script [" + request.id() + "]", exception); } - } - public void storeScript(ClusterService clusterService, PutStoredScriptRequest request, ActionListener listener) { - String scriptLang = validateScriptLanguage(request.scriptLang()); - //verify that the script compiles - validateStoredScript(request.id(), scriptLang, request.script()); - clusterService.submitStateUpdateTask("put-script-" + request.id(), new AckedClusterStateUpdateTask(request, listener) { + clusterService.submitStateUpdateTask("put-script-" + request.id(), + new AckedClusterStateUpdateTask(request, listener) { @Override protected PutStoredScriptResponse newResponse(boolean acknowledged) { @@ -421,23 +425,23 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust @Override public ClusterState execute(ClusterState currentState) throws Exception { - return innerStoreScript(currentState, scriptLang, request); + ScriptMetaData smd = currentState.metaData().custom(ScriptMetaData.TYPE); + smd = ScriptMetaData.putStoredScript(smd, request.id(), source); + MetaData.Builder mdb = MetaData.builder(currentState.getMetaData()).putCustom(ScriptMetaData.TYPE, smd); + + return ClusterState.builder(currentState).metaData(mdb).build(); } }); } - static ClusterState innerStoreScript(ClusterState currentState, String validatedScriptLang, PutStoredScriptRequest request) { - ScriptMetaData scriptMetadata = currentState.metaData().custom(ScriptMetaData.TYPE); - ScriptMetaData.Builder scriptMetadataBuilder = new ScriptMetaData.Builder(scriptMetadata); - scriptMetadataBuilder.storeScript(validatedScriptLang, request.id(), request.script()); - MetaData.Builder metaDataBuilder = MetaData.builder(currentState.getMetaData()) - .putCustom(ScriptMetaData.TYPE, scriptMetadataBuilder.build()); - return ClusterState.builder(currentState).metaData(metaDataBuilder).build(); - } + public void deleteStoredScript(ClusterService clusterService, DeleteStoredScriptRequest request, + ActionListener listener) { + if (request.lang() != null && isLangSupported(request.lang()) == false) { + throw new IllegalArgumentException("unable to delete stored script with unsupported lang [" + request.lang() +"]"); + } - public void deleteStoredScript(ClusterService clusterService, DeleteStoredScriptRequest request, ActionListener listener) { - String scriptLang = validateScriptLanguage(request.scriptLang()); - clusterService.submitStateUpdateTask("delete-script-" + request.id(), new AckedClusterStateUpdateTask(request, listener) { + clusterService.submitStateUpdateTask("delete-script-" + request.id(), + new AckedClusterStateUpdateTask(request, listener) { @Override protected DeleteStoredScriptResponse newResponse(boolean acknowledged) { @@ -446,24 +450,20 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust @Override public ClusterState execute(ClusterState currentState) throws Exception { - return innerDeleteScript(currentState, scriptLang, request); + ScriptMetaData smd = currentState.metaData().custom(ScriptMetaData.TYPE); + smd = ScriptMetaData.deleteStoredScript(smd, request.id(), request.lang()); + MetaData.Builder mdb = MetaData.builder(currentState.getMetaData()).putCustom(ScriptMetaData.TYPE, smd); + + return ClusterState.builder(currentState).metaData(mdb).build(); } }); } - static ClusterState innerDeleteScript(ClusterState currentState, String validatedLang, DeleteStoredScriptRequest request) { - ScriptMetaData scriptMetadata = currentState.metaData().custom(ScriptMetaData.TYPE); - ScriptMetaData.Builder scriptMetadataBuilder = new ScriptMetaData.Builder(scriptMetadata); - scriptMetadataBuilder.deleteScript(validatedLang, request.id()); - MetaData.Builder metaDataBuilder = MetaData.builder(currentState.getMetaData()) - .putCustom(ScriptMetaData.TYPE, scriptMetadataBuilder.build()); - return ClusterState.builder(currentState).metaData(metaDataBuilder).build(); - } - - public String getStoredScript(ClusterState state, GetStoredScriptRequest request) { + public StoredScriptSource getStoredScript(ClusterState state, GetStoredScriptRequest request) { ScriptMetaData scriptMetadata = state.metaData().custom(ScriptMetaData.TYPE); + if (scriptMetadata != null) { - return scriptMetadata.getScript(request.lang(), request.id()); + return scriptMetadata.getStoredScript(request.id(), request.lang()); } else { return null; } @@ -473,7 +473,7 @@ 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.getOptions()), script.getParams()); + return executable(compile(script, scriptContext), script.getParams()); } /** @@ -487,7 +487,7 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust * Compiles (or retrieves from cache) and executes the provided search script */ public SearchScript search(SearchLookup lookup, Script script, ScriptContext scriptContext) { - CompiledScript compiledScript = compile(script, scriptContext, script.getOptions()); + CompiledScript compiledScript = compile(script, scriptContext); return search(lookup, compiledScript, script.getParams()); } @@ -520,18 +520,6 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust return scriptMetrics.stats(); } - private void validateScriptSize(String identifier, int scriptSizeInBytes) { - int allowedScriptSizeInBytes = SCRIPT_MAX_SIZE_IN_BYTES.get(settings); - if (scriptSizeInBytes > allowedScriptSizeInBytes) { - String message = LoggerMessageFormat.format( - "Limit of script size in bytes [{}] has been exceeded for script [{}] with size [{}]", - allowedScriptSizeInBytes, - identifier, - scriptSizeInBytes); - throw new IllegalArgumentException(message); - } - } - @Override public void clusterChanged(ClusterChangedEvent event) { clusterState = event.state(); @@ -592,11 +580,11 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust logger.info("compiling script file [{}]", file.toAbsolutePath()); try (InputStreamReader reader = new InputStreamReader(Files.newInputStream(file), StandardCharsets.UTF_8)) { String script = Streams.copyToString(reader); - String name = scriptNameExt.v1(); - CacheKey cacheKey = new CacheKey(engineService, name, null, Collections.emptyMap()); + String id = scriptNameExt.v1(); + CacheKey cacheKey = new CacheKey(engineService.getType(), id, null); // pass the actual file name to the compiler (for script engines that care about this) Object executable = engineService.compile(file.getFileName().toString(), script, Collections.emptyMap()); - CompiledScript compiledScript = new CompiledScript(ScriptType.FILE, name, engineService.getType(), executable); + CompiledScript compiledScript = new CompiledScript(ScriptType.FILE, id, engineService.getType(), executable); staticCache.put(cacheKey, compiledScript); scriptMetrics.onCompilation(); } @@ -637,7 +625,7 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust ScriptEngineService engineService = getScriptEngineServiceForFileExt(scriptNameExt.v2()); assert engineService != null; logger.info("removing script file [{}]", file.toAbsolutePath()); - staticCache.remove(new CacheKey(engineService, scriptNameExt.v1(), null, Collections.emptyMap())); + staticCache.remove(new CacheKey(engineService.getType(), scriptNameExt.v1(), null)); } } @@ -650,15 +638,13 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust private static final class CacheKey { final String lang; - final String name; - final String code; - final Map params; + final String idOrCode; + final Map options; - private CacheKey(final ScriptEngineService service, final String name, final String code, final Map params) { - this.lang = service.getType(); - this.name = name; - this.code = code; - this.params = params; + private CacheKey(String lang, String idOrCode, Map options) { + this.lang = lang; + this.idOrCode = idOrCode; + this.options = options; } @Override @@ -668,44 +654,18 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust CacheKey cacheKey = (CacheKey)o; - if (!lang.equals(cacheKey.lang)) return false; - if (name != null ? !name.equals(cacheKey.name) : cacheKey.name != null) return false; - if (code != null ? !code.equals(cacheKey.code) : cacheKey.code != null) return false; - return params.equals(cacheKey.params); + if (lang != null ? !lang.equals(cacheKey.lang) : cacheKey.lang != null) return false; + if (!idOrCode.equals(cacheKey.idOrCode)) return false; + return options != null ? options.equals(cacheKey.options) : cacheKey.options == null; } @Override public int hashCode() { - int result = lang.hashCode(); - result = 31 * result + (name != null ? name.hashCode() : 0); - result = 31 * result + (code != null ? code.hashCode() : 0); - result = 31 * result + params.hashCode(); + int result = lang != null ? lang.hashCode() : 0; + result = 31 * result + idOrCode.hashCode(); + result = 31 * result + (options != null ? options.hashCode() : 0); return result; } } - - - private static class IndexedScript { - private final String lang; - private final String id; - - IndexedScript(String lang, String script) { - this.lang = lang; - final String[] parts = script.split("/"); - if (parts.length == 1) { - this.id = script; - } else { - if (parts.length != 3) { - throw new IllegalArgumentException("Illegal index script format [" + script + "]" + - " should be /lang/id"); - } else { - if (!parts[1].equals(this.lang)) { - throw new IllegalStateException("Conflicting script language, found [" + parts[1] + "] expected + ["+ this.lang + "]"); - } - this.id = parts[2]; - } - } - } - } } diff --git a/core/src/main/java/org/elasticsearch/script/StoredScriptSource.java b/core/src/main/java/org/elasticsearch/script/StoredScriptSource.java new file mode 100644 index 00000000000..acf7135424f --- /dev/null +++ b/core/src/main/java/org/elasticsearch/script/StoredScriptSource.java @@ -0,0 +1,485 @@ +/* + * 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 org.elasticsearch.Version; +import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptResponse; +import org.elasticsearch.cluster.AbstractDiffable; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.Diff; +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.ObjectParser; +import org.elasticsearch.common.xcontent.ObjectParser.ValueType; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentParser.Token; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +/** + * {@link StoredScriptSource} represents user-defined parameters for a script + * saved in the {@link ClusterState}. + */ +public class StoredScriptSource extends AbstractDiffable implements Writeable, ToXContent { + + /** + * Standard {@link ParseField} for outer level of stored script source. + */ + public static final ParseField SCRIPT_PARSE_FIELD = new ParseField("script"); + + /** + * Standard {@link ParseField} for outer level of stored script source. + */ + public static final ParseField TEMPLATE_PARSE_FIELD = new ParseField("template"); + + /** + * Standard {@link ParseField} for lang on the inner level. + */ + public static final ParseField LANG_PARSE_FIELD = new ParseField("lang"); + + /** + * Standard {@link ParseField} for code on the inner level. + */ + public static final ParseField CODE_PARSE_FIELD = new ParseField("code"); + + /** + * Standard {@link ParseField} for options on the inner level. + */ + public static final ParseField OPTIONS_PARSE_FIELD = new ParseField("options"); + + /** + * Helper class used by {@link ObjectParser} to store mutable {@link StoredScriptSource} variables and then + * construct an immutable {@link StoredScriptSource} object based on parsed XContent. + */ + private static final class Builder { + private String lang; + private String code; + private Map options; + + private Builder() { + // This cannot default to an empty map because options are potentially added at multiple points. + this.options = new HashMap<>(); + } + + private void setLang(String lang) { + this.lang = lang; + } + + /** + * Since stored scripts can accept templates rather than just scripts, they must also be able + * to handle template parsing, hence the need for custom parsing code. Templates can + * consist of either an {@link String} or a JSON object. If a JSON object is discovered + * then the content type option must also be saved as a compiler option. + */ + private void setCode(XContentParser parser) { + try { + if (parser.currentToken() == Token.START_OBJECT) { + XContentBuilder builder = XContentFactory.contentBuilder(parser.contentType()); + code = builder.copyCurrentStructure(parser).bytes().utf8ToString(); + options.put(Script.CONTENT_TYPE_OPTION, parser.contentType().mediaType()); + } else { + code = parser.text(); + } + } catch (IOException exception) { + throw new UncheckedIOException(exception); + } + } + + /** + * Options may have already been added if a template was specified. + * Appends the user-defined compiler options with the internal compiler options. + */ + private void setOptions(Map options) { + if (options.containsKey(Script.CONTENT_TYPE_OPTION)) { + throw new IllegalArgumentException(Script.CONTENT_TYPE_OPTION + " cannot be user-specified"); + } + + this.options.putAll(options); + } + + /** + * Validates the parameters and creates an {@link StoredScriptSource}. + */ + private StoredScriptSource build() { + if (lang == null) { + throw new IllegalArgumentException("must specify lang for stored script"); + } else if (lang.isEmpty()) { + throw new IllegalArgumentException("lang cannot be empty"); + } + + if (code == null) { + throw new IllegalArgumentException("must specify code for stored script"); + } else if (code.isEmpty()) { + throw new IllegalArgumentException("code cannot be empty"); + } + + if (options.size() > 1 || options.size() == 1 && options.get(Script.CONTENT_TYPE_OPTION) == null) { + throw new IllegalArgumentException("illegal compiler options [" + options + "] specified"); + } + + return new StoredScriptSource(lang, code, options); + } + } + + private static final ObjectParser PARSER = new ObjectParser<>("stored script source", Builder::new); + + static { + // Defines the fields necessary to parse a Script as XContent using an ObjectParser. + PARSER.declareString(Builder::setLang, LANG_PARSE_FIELD); + PARSER.declareField(Builder::setCode, parser -> parser, CODE_PARSE_FIELD, ValueType.OBJECT_OR_STRING); + PARSER.declareField(Builder::setOptions, XContentParser::mapStrings, OPTIONS_PARSE_FIELD, ValueType.OBJECT); + } + + /** + * This will parse XContent into a {@link StoredScriptSource}. The following formats can be parsed: + * + * The simple script format with no compiler options or user-defined params: + * + * Example: + * {@code + * {"script": "return Math.log(doc.popularity) * 100;"} + * } + * + * The above format requires the lang to be specified using the deprecated stored script namespace + * (as a url parameter during a put request). See {@link ScriptMetaData} for more information about + * the stored script namespaces. + * + * The complex script format using the new stored script namespace + * where lang and code are required but options is optional: + * + * {@code + * { + * "script" : { + * "lang" : "", + * "code" : "", + * "options" : { + * "option0" : "", + * "option1" : "", + * ... + * } + * } + * } + * } + * + * Example: + * {@code + * { + * "script": { + * "lang" : "painless", + * "code" : "return Math.log(doc.popularity) * params.multiplier" + * } + * } + * } + * + * The simple template format: + * + * {@code + * { + * "query" : ... + * } + * } + * + * The complex template format: + * + * {@code + * { + * "template": { + * "query" : ... + * } + * } + * } + * + * Note that templates can be handled as both strings and complex JSON objects. + * Also templates may be part of the 'code' parameter in a script. The Parser + * can handle this case as well. + * + * @param lang An optional parameter to allow for use of the deprecated stored + * script namespace. This will be used to specify the language + * coming in as a url parameter from a request or for stored templates. + * @param content The content from the request to be parsed as described above. + * @return The parsed {@link StoredScriptSource}. + */ + public static StoredScriptSource parse(String lang, BytesReference content) { + try (XContentParser parser = XContentHelper.createParser(NamedXContentRegistry.EMPTY, content)) { + Token token = parser.nextToken(); + + if (token != Token.START_OBJECT) { + throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + "], expected [{]"); + } + + token = parser.nextToken(); + + if (token != Token.FIELD_NAME) { + throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + ", expected [" + + SCRIPT_PARSE_FIELD.getPreferredName() + ", " + TEMPLATE_PARSE_FIELD.getPreferredName()); + } + + String name = parser.currentName(); + + if (SCRIPT_PARSE_FIELD.getPreferredName().equals(name)) { + token = parser.nextToken(); + + if (token == Token.VALUE_STRING) { + if (lang == null) { + throw new IllegalArgumentException( + "must specify lang as a url parameter when using the deprecated stored script namespace"); + } + + return new StoredScriptSource(lang, parser.text(), Collections.emptyMap()); + } else if (token == Token.START_OBJECT) { + if (lang == null) { + return PARSER.apply(parser, null).build(); + } else { + try (XContentBuilder builder = XContentFactory.contentBuilder(parser.contentType())) { + builder.copyCurrentStructure(parser); + + return new StoredScriptSource(lang, builder.string(), + Collections.singletonMap(Script.CONTENT_TYPE_OPTION, parser.contentType().mediaType())); + } + } + + } else { + throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + "], expected [{, ]"); + } + } else { + if (lang == null) { + throw new IllegalArgumentException("unexpected stored script format"); + } + + if (TEMPLATE_PARSE_FIELD.getPreferredName().equals(name)) { + token = parser.nextToken(); + + if (token == Token.VALUE_STRING) { + return new StoredScriptSource(lang, parser.text(), + Collections.singletonMap(Script.CONTENT_TYPE_OPTION, parser.contentType().mediaType())); + } + } + + try (XContentBuilder builder = XContentFactory.contentBuilder(parser.contentType())) { + if (token != Token.START_OBJECT) { + builder.startObject(); + builder.copyCurrentStructure(parser); + builder.endObject(); + } else { + builder.copyCurrentStructure(parser); + } + + return new StoredScriptSource(lang, builder.string(), + Collections.singletonMap(Script.CONTENT_TYPE_OPTION, parser.contentType().mediaType())); + } + } + } catch (IOException ioe) { + throw new UncheckedIOException(ioe); + } + } + + /** + * This will parse XContent into a {@link StoredScriptSource}. The following format is what will be parsed: + * + * {@code + * { + * "script" : { + * "lang" : "", + * "code" : "", + * "options" : { + * "option0" : "", + * "option1" : "", + * ... + * } + * } + * } + * } + * + * Note that the "code" parameter can also handle template parsing including from + * a complex JSON object. + */ + public static StoredScriptSource fromXContent(XContentParser parser) throws IOException { + return PARSER.apply(parser, null).build(); + } + + /** + * Required for {@link ScriptMetaData.ScriptMetadataDiff}. Uses + * the {@link StoredScriptSource#StoredScriptSource(StreamInput)} + * constructor. + */ + public static Diff readDiffFrom(StreamInput in) throws IOException { + return readDiffFrom(StoredScriptSource::new, in); + } + + private final String lang; + private final String code; + private final Map options; + + /** + * Constructor for use with {@link GetStoredScriptResponse} + * to support the deprecated stored script namespace. + */ + public StoredScriptSource(String code) { + this.lang = null; + this.code = Objects.requireNonNull(code); + this.options = null; + } + + /** + * Standard StoredScriptSource constructor. + * @param lang The language to compile the script with. Must not be {@code null}. + * @param code The source code to compile with. Must not be {@code null}. + * @param options Compiler options to be compiled with. Must not be {@code null}, + * use an empty {@link Map} to represent no options. + */ + public StoredScriptSource(String lang, String code, Map options) { + this.lang = Objects.requireNonNull(lang); + this.code = Objects.requireNonNull(code); + this.options = Collections.unmodifiableMap(Objects.requireNonNull(options)); + } + + /** + * Reads a {@link StoredScriptSource} from a stream. Version 5.3+ will read + * all of the lang, code, and options parameters. For versions prior to 5.3, + * only the code parameter will be read in as a bytes reference. + */ + public StoredScriptSource(StreamInput in) throws IOException { + if (in.getVersion().onOrAfter(Version.V_5_3_0_UNRELEASED)) { + this.lang = in.readString(); + this.code = in.readString(); + @SuppressWarnings("unchecked") + Map options = (Map)(Map)in.readMap(); + this.options = options; + } else { + this.lang = null; + this.code = in.readBytesReference().utf8ToString(); + this.options = null; + } + } + + /** + * Writes a {@link StoredScriptSource} to a stream. Version 5.3+ will write + * all of the lang, code, and options parameters. For versions prior to 5.3, + * only the code parameter will be read in as a bytes reference. + */ + @Override + public void writeTo(StreamOutput out) throws IOException { + if (out.getVersion().onOrAfter(Version.V_5_3_0_UNRELEASED)) { + out.writeString(lang); + out.writeString(code); + @SuppressWarnings("unchecked") + Map options = (Map)(Map)this.options; + out.writeMap(options); + } else { + out.writeBytesReference(new BytesArray(code)); + } + } + + /** + * This will write XContent from a {@link StoredScriptSource}. The following format will be written: + * + * {@code + * { + * "script" : { + * "lang" : "", + * "code" : "", + * "options" : { + * "option0" : "", + * "option1" : "", + * ... + * } + * } + * } + * } + * + * Note that the 'code' parameter can also handle templates written as complex JSON. + */ + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.field(LANG_PARSE_FIELD.getPreferredName(), lang); + builder.field(CODE_PARSE_FIELD.getPreferredName(), code); + builder.field(OPTIONS_PARSE_FIELD.getPreferredName(), options); + builder.endObject(); + + return builder; + } + + /** + * @return The language used for compiling this script. + */ + public String getLang() { + return lang; + } + + /** + * @return The code used for compiling this script. + */ + public String getCode() { + return code; + } + + /** + * @return The compiler options used for this script. + */ + public Map getOptions() { + return options; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + StoredScriptSource that = (StoredScriptSource)o; + + if (lang != null ? !lang.equals(that.lang) : that.lang != null) return false; + if (code != null ? !code.equals(that.code) : that.code != null) return false; + return options != null ? options.equals(that.options) : that.options == null; + + } + + @Override + public int hashCode() { + int result = lang != null ? lang.hashCode() : 0; + result = 31 * result + (code != null ? code.hashCode() : 0); + result = 31 * result + (options != null ? options.hashCode() : 0); + return result; + } + + @Override + public String toString() { + return "StoredScriptSource{" + + "lang='" + lang + '\'' + + ", code='" + code + '\'' + + ", options=" + options + + '}'; + } +} diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetric.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetric.java index c5704d9f2b6..6cb3b626f91 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetric.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetric.java @@ -88,8 +88,8 @@ public class InternalScriptedMetric extends InternalMetricsAggregation implement if (firstAggregation.reduceScript.getParams() != null) { vars.putAll(firstAggregation.reduceScript.getParams()); } - CompiledScript compiledScript = reduceContext.scriptService().compile(firstAggregation.reduceScript, - ScriptContext.Standard.AGGS, Collections.emptyMap()); + CompiledScript compiledScript = reduceContext.scriptService().compile( + firstAggregation.reduceScript, ScriptContext.Standard.AGGS); ExecutableScript script = reduceContext.scriptService().executable(compiledScript, vars); aggregation = script.run(); } else { diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketscript/BucketScriptPipelineAggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketscript/BucketScriptPipelineAggregator.java index 008b0217c65..059d2fec037 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketscript/BucketScriptPipelineAggregator.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketscript/BucketScriptPipelineAggregator.java @@ -92,8 +92,7 @@ public class BucketScriptPipelineAggregator extends PipelineAggregator { InternalMultiBucketAggregation originalAgg = (InternalMultiBucketAggregation) aggregation; List buckets = originalAgg.getBuckets(); - CompiledScript compiledScript = reduceContext.scriptService().compile(script, ScriptContext.Standard.AGGS, - Collections.emptyMap()); + CompiledScript compiledScript = reduceContext.scriptService().compile(script, ScriptContext.Standard.AGGS); List newBuckets = new ArrayList<>(); for (Bucket bucket : buckets) { Map vars = new HashMap<>(); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketselector/BucketSelectorPipelineAggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketselector/BucketSelectorPipelineAggregator.java index eabbad7213a..f45ebd1c5a3 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketselector/BucketSelectorPipelineAggregator.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketselector/BucketSelectorPipelineAggregator.java @@ -86,8 +86,7 @@ public class BucketSelectorPipelineAggregator extends PipelineAggregator { (InternalMultiBucketAggregation) aggregation; List buckets = originalAgg.getBuckets(); - CompiledScript compiledScript = reduceContext.scriptService().compile(script, ScriptContext.Standard.AGGS, - Collections.emptyMap()); + CompiledScript compiledScript = reduceContext.scriptService().compile(script, ScriptContext.Standard.AGGS); List newBuckets = new ArrayList<>(); for (Bucket bucket : buckets) { Map vars = new HashMap<>(); diff --git a/core/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java index 3f480262f49..9774af189da 100644 --- a/core/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java @@ -415,7 +415,8 @@ public class InnerHitBuilderTests extends ESTestCase { randomMap.put(String.valueOf(i), randomAsciiOfLength(16)); } } - Script script = new Script(randomScriptType, randomAsciiOfLengthBetween(1, 4), randomAsciiOfLength(128), randomMap); + Script script = new Script(randomScriptType, randomScriptType == ScriptType.STORED ? null : randomAsciiOfLengthBetween(1, 4), + randomAsciiOfLength(128), randomMap); return new SearchSourceBuilder.ScriptField(randomAsciiOfLengthBetween(1, 32), script, randomBoolean()); } diff --git a/core/src/test/java/org/elasticsearch/script/FileScriptTests.java b/core/src/test/java/org/elasticsearch/script/FileScriptTests.java index f5ae13cf3a0..92e659ac755 100644 --- a/core/src/test/java/org/elasticsearch/script/FileScriptTests.java +++ b/core/src/test/java/org/elasticsearch/script/FileScriptTests.java @@ -55,7 +55,7 @@ public class FileScriptTests extends ESTestCase { .put("script.engine." + MockScriptEngine.NAME + ".file.aggs", "false").build(); ScriptService scriptService = makeScriptService(settings); Script script = new Script(ScriptType.FILE, MockScriptEngine.NAME, "script1", Collections.emptyMap()); - CompiledScript compiledScript = scriptService.compile(script, ScriptContext.Standard.SEARCH, Collections.emptyMap()); + CompiledScript compiledScript = scriptService.compile(script, ScriptContext.Standard.SEARCH); assertNotNull(compiledScript); MockCompiledScript executable = (MockCompiledScript) compiledScript.compiled(); assertEquals("script1.mockscript", executable.getName()); @@ -72,7 +72,7 @@ public class FileScriptTests extends ESTestCase { Script script = new Script(ScriptType.FILE, MockScriptEngine.NAME, "script1", Collections.emptyMap()); for (ScriptContext context : ScriptContext.Standard.values()) { try { - scriptService.compile(script, context, Collections.emptyMap()); + scriptService.compile(script, context); fail(context.getKey() + " script should have been rejected"); } catch(Exception e) { assertTrue(e.getMessage(), e.getMessage().contains("scripts of type [file], operation [" + context.getKey() + "] and lang [" + MockScriptEngine.NAME + "] are disabled")); diff --git a/core/src/test/java/org/elasticsearch/script/NativeScriptTests.java b/core/src/test/java/org/elasticsearch/script/NativeScriptTests.java index 5423103f92d..bf5c7e0daa7 100644 --- a/core/src/test/java/org/elasticsearch/script/NativeScriptTests.java +++ b/core/src/test/java/org/elasticsearch/script/NativeScriptTests.java @@ -80,7 +80,7 @@ public class NativeScriptTests extends ESTestCase { for (ScriptContext scriptContext : scriptContextRegistry.scriptContexts()) { assertThat(scriptService.compile(new Script(ScriptType.INLINE, NativeScriptEngineService.NAME, "my", Collections.emptyMap()), - scriptContext, Collections.emptyMap()), notNullValue()); + scriptContext), notNullValue()); } } diff --git a/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java b/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java index d961fd677aa..e25335e5d68 100644 --- a/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java +++ b/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java @@ -19,6 +19,10 @@ package org.elasticsearch.script; +import org.elasticsearch.cluster.ClusterChangedEvent; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.test.ESTestCase; @@ -50,8 +54,16 @@ public class ScriptContextTests extends ESTestCase { new ScriptContext.Plugin(PLUGIN_NAME, "custom_globally_disabled_op")); ScriptContextRegistry scriptContextRegistry = new ScriptContextRegistry(customContexts); ScriptSettings scriptSettings = new ScriptSettings(scriptEngineRegistry, scriptContextRegistry); + ScriptService scriptService = new ScriptService(settings, new Environment(settings), null, scriptEngineRegistry, scriptContextRegistry, scriptSettings); - return new ScriptService(settings, new Environment(settings), null, scriptEngineRegistry, scriptContextRegistry, scriptSettings); + ClusterState empty = ClusterState.builder(new ClusterName("_name")).build(); + ScriptMetaData smd = empty.metaData().custom(ScriptMetaData.TYPE); + smd = ScriptMetaData.putStoredScript(smd, "1", new StoredScriptSource(MockScriptEngine.NAME, "1", Collections.emptyMap())); + MetaData.Builder mdb = MetaData.builder(empty.getMetaData()).putCustom(ScriptMetaData.TYPE, smd); + ClusterState stored = ClusterState.builder(empty).metaData(mdb).build(); + scriptService.clusterChanged(new ClusterChangedEvent("test", stored, empty)); + + return scriptService; } public void testCustomGlobalScriptContextSettings() throws Exception { @@ -59,7 +71,7 @@ public class ScriptContextTests extends ESTestCase { for (ScriptType scriptType : ScriptType.values()) { try { Script script = new Script(scriptType, MockScriptEngine.NAME, "1", Collections.emptyMap()); - scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_globally_disabled_op"), Collections.emptyMap()); + scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_globally_disabled_op")); fail("script compilation should have been rejected"); } catch (IllegalStateException e) { assertThat(e.getMessage(), containsString("scripts of type [" + scriptType + "], operation [" + PLUGIN_NAME + "_custom_globally_disabled_op] and lang [" + MockScriptEngine.NAME + "] are disabled")); @@ -71,16 +83,16 @@ public class ScriptContextTests extends ESTestCase { ScriptService scriptService = makeScriptService(); Script script = new Script(ScriptType.INLINE, MockScriptEngine.NAME, "1", Collections.emptyMap()); try { - scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_exp_disabled_op"), Collections.emptyMap()); + scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_exp_disabled_op")); fail("script compilation should have been rejected"); } catch (IllegalStateException e) { assertTrue(e.getMessage(), e.getMessage().contains("scripts of type [inline], operation [" + PLUGIN_NAME + "_custom_exp_disabled_op] and lang [" + MockScriptEngine.NAME + "] are disabled")); } // still works for other script contexts - assertNotNull(scriptService.compile(script, ScriptContext.Standard.AGGS, Collections.emptyMap())); - assertNotNull(scriptService.compile(script, ScriptContext.Standard.SEARCH, Collections.emptyMap())); - assertNotNull(scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_op"), Collections.emptyMap())); + assertNotNull(scriptService.compile(script, ScriptContext.Standard.AGGS)); + assertNotNull(scriptService.compile(script, ScriptContext.Standard.SEARCH)); + assertNotNull(scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_op"))); } public void testUnknownPluginScriptContext() throws Exception { @@ -88,7 +100,7 @@ public class ScriptContextTests extends ESTestCase { for (ScriptType scriptType : ScriptType.values()) { try { Script script = new Script(scriptType, MockScriptEngine.NAME, "1", Collections.emptyMap()); - scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "unknown"), Collections.emptyMap()); + scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "unknown")); fail("script compilation should have been rejected"); } catch (IllegalArgumentException e) { assertTrue(e.getMessage(), e.getMessage().contains("script context [" + PLUGIN_NAME + "_unknown] not supported")); @@ -107,7 +119,7 @@ public class ScriptContextTests extends ESTestCase { for (ScriptType scriptType : ScriptType.values()) { try { Script script = new Script(scriptType, MockScriptEngine.NAME, "1", Collections.emptyMap()); - scriptService.compile(script, context, Collections.emptyMap()); + scriptService.compile(script, context); fail("script compilation should have been rejected"); } catch (IllegalArgumentException e) { assertTrue(e.getMessage(), e.getMessage().contains("script context [test] not supported")); diff --git a/core/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java b/core/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java index 4b2b9a49dc0..5365d75bdf9 100644 --- a/core/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java +++ b/core/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java @@ -20,124 +20,83 @@ package org.elasticsearch.script; import org.elasticsearch.cluster.DiffableUtils; import org.elasticsearch.common.bytes.BytesArray; -import org.elasticsearch.common.io.stream.InputStreamStreamInput; -import org.elasticsearch.common.io.stream.OutputStreamStreamOutput; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.AbstractSerializingTestCase; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.util.Collections; +import java.io.UncheckedIOException; - -public class ScriptMetaDataTests extends ESTestCase { +public class ScriptMetaDataTests extends AbstractSerializingTestCase { public void testGetScript() throws Exception { ScriptMetaData.Builder builder = new ScriptMetaData.Builder(null); XContentBuilder sourceBuilder = XContentFactory.jsonBuilder(); sourceBuilder.startObject().startObject("template").field("field", "value").endObject().endObject(); - builder.storeScript("lang", "template", sourceBuilder.bytes()); + builder.storeScript("template", StoredScriptSource.parse("lang", sourceBuilder.bytes())); sourceBuilder = XContentFactory.jsonBuilder(); sourceBuilder.startObject().field("template", "value").endObject(); - builder.storeScript("lang", "template_field", sourceBuilder.bytes()); + builder.storeScript("template_field", StoredScriptSource.parse("lang", sourceBuilder.bytes())); sourceBuilder = XContentFactory.jsonBuilder(); sourceBuilder.startObject().startObject("script").field("field", "value").endObject().endObject(); - builder.storeScript("lang", "script", sourceBuilder.bytes()); + builder.storeScript("script", StoredScriptSource.parse("lang", sourceBuilder.bytes())); sourceBuilder = XContentFactory.jsonBuilder(); sourceBuilder.startObject().field("script", "value").endObject(); - builder.storeScript("lang", "script_field", sourceBuilder.bytes()); + builder.storeScript("script_field", StoredScriptSource.parse("lang", sourceBuilder.bytes())); sourceBuilder = XContentFactory.jsonBuilder(); sourceBuilder.startObject().field("field", "value").endObject(); - builder.storeScript("lang", "any", sourceBuilder.bytes()); + builder.storeScript("any", StoredScriptSource.parse("lang", sourceBuilder.bytes())); ScriptMetaData scriptMetaData = builder.build(); - assertEquals("{\"field\":\"value\"}", scriptMetaData.getScript("lang", "template")); - assertEquals("value", scriptMetaData.getScript("lang", "template_field")); - assertEquals("{\"field\":\"value\"}", scriptMetaData.getScript("lang", "script")); - assertEquals("value", scriptMetaData.getScript("lang", "script_field")); - assertEquals("{\"field\":\"value\"}", scriptMetaData.getScript("lang", "any")); - } - - public void testToAndFromXContent() throws IOException { - XContentType contentType = randomFrom(XContentType.values()); - XContentBuilder xContentBuilder = XContentBuilder.builder(contentType.xContent()); - ScriptMetaData expected = randomScriptMetaData(contentType); - - xContentBuilder.startObject(); - expected.toXContent(xContentBuilder, new ToXContent.MapParams(Collections.emptyMap())); - xContentBuilder.endObject(); - xContentBuilder = shuffleXContent(xContentBuilder); - - XContentParser parser = createParser(xContentBuilder); - parser.nextToken(); - ScriptMetaData result = ScriptMetaData.fromXContent(parser); - assertEquals(expected, result); - assertEquals(expected.hashCode(), result.hashCode()); - } - - public void testReadFromWriteTo() throws IOException { - ScriptMetaData expected = randomScriptMetaData(randomFrom(XContentType.values())); - ByteArrayOutputStream out = new ByteArrayOutputStream(); - expected.writeTo(new OutputStreamStreamOutput(out)); - - ScriptMetaData result = new ScriptMetaData(new InputStreamStreamInput(new ByteArrayInputStream(out.toByteArray()))); - assertEquals(expected, result); - assertEquals(expected.hashCode(), result.hashCode()); + assertEquals("{\"field\":\"value\"}", scriptMetaData.getStoredScript("template", "lang").getCode()); + assertEquals("value", scriptMetaData.getStoredScript("template_field", "lang").getCode()); + assertEquals("{\"field\":\"value\"}", scriptMetaData.getStoredScript("script", "lang").getCode()); + assertEquals("value", scriptMetaData.getStoredScript("script_field", "lang").getCode()); + assertEquals("{\"field\":\"value\"}", scriptMetaData.getStoredScript("any", "lang").getCode()); } public void testDiff() throws Exception { ScriptMetaData.Builder builder = new ScriptMetaData.Builder(null); - builder.storeScript("lang", "1", new BytesArray("{\"foo\":\"abc\"}")); - builder.storeScript("lang", "2", new BytesArray("{\"foo\":\"def\"}")); - builder.storeScript("lang", "3", new BytesArray("{\"foo\":\"ghi\"}")); + builder.storeScript("1", StoredScriptSource.parse("lang", new BytesArray("{\"foo\":\"abc\"}"))); + builder.storeScript("2", StoredScriptSource.parse("lang", new BytesArray("{\"foo\":\"def\"}"))); + builder.storeScript("3", StoredScriptSource.parse("lang", new BytesArray("{\"foo\":\"ghi\"}"))); ScriptMetaData scriptMetaData1 = builder.build(); builder = new ScriptMetaData.Builder(scriptMetaData1); - builder.storeScript("lang", "2", new BytesArray("{\"foo\":\"changed\"}")); - builder.deleteScript("lang", "3"); - builder.storeScript("lang", "4", new BytesArray("{\"foo\":\"jkl\"}")); + builder.storeScript("2", StoredScriptSource.parse("lang", new BytesArray("{\"foo\":\"changed\"}"))); + builder.deleteScript("3", "lang"); + builder.storeScript("4", StoredScriptSource.parse("lang", new BytesArray("{\"foo\":\"jkl\"}"))); ScriptMetaData scriptMetaData2 = builder.build(); ScriptMetaData.ScriptMetadataDiff diff = (ScriptMetaData.ScriptMetadataDiff) scriptMetaData2.diff(scriptMetaData1); - assertEquals(1, ((DiffableUtils.MapDiff) diff.pipelines).getDeletes().size()); - assertEquals("lang#3", ((DiffableUtils.MapDiff) diff.pipelines).getDeletes().get(0)); - assertEquals(1, ((DiffableUtils.MapDiff) diff.pipelines).getDiffs().size()); - assertNotNull(((DiffableUtils.MapDiff) diff.pipelines).getDiffs().get("lang#2")); - assertEquals(1, ((DiffableUtils.MapDiff) diff.pipelines).getUpserts().size()); - assertNotNull(((DiffableUtils.MapDiff) diff.pipelines).getUpserts().get("lang#4")); + assertEquals(2, ((DiffableUtils.MapDiff) diff.pipelines).getDeletes().size()); + assertEquals("3", ((DiffableUtils.MapDiff) diff.pipelines).getDeletes().get(0)); + assertEquals(2, ((DiffableUtils.MapDiff) diff.pipelines).getDiffs().size()); + assertNotNull(((DiffableUtils.MapDiff) diff.pipelines).getDiffs().get("2")); + assertEquals(2, ((DiffableUtils.MapDiff) diff.pipelines).getUpserts().size()); + assertNotNull(((DiffableUtils.MapDiff) diff.pipelines).getUpserts().get("4")); ScriptMetaData result = (ScriptMetaData) diff.apply(scriptMetaData1); - assertEquals(new BytesArray("{\"foo\":\"abc\"}"), result.getScriptAsBytes("lang", "1")); - assertEquals(new BytesArray("{\"foo\":\"changed\"}"), result.getScriptAsBytes("lang", "2")); - assertEquals(new BytesArray("{\"foo\":\"jkl\"}"), result.getScriptAsBytes("lang", "4")); + assertEquals("{\"foo\":\"abc\"}", result.getStoredScript("1", "lang").getCode()); + assertEquals("{\"foo\":\"changed\"}", result.getStoredScript("2", "lang").getCode()); + assertEquals("{\"foo\":\"jkl\"}", result.getStoredScript("4", "lang").getCode()); } public void testBuilder() { ScriptMetaData.Builder builder = new ScriptMetaData.Builder(null); - builder.storeScript("_lang", "_id", new BytesArray("{\"script\":\"1 + 1\"}")); - - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> builder.storeScript("_lang#", "_id", new BytesArray("{\"foo\": \"bar\"}"))); - assertEquals("stored script language can't contain: '#'", e.getMessage()); - e = expectThrows(IllegalArgumentException.class, () -> builder.storeScript("_lang", "_id#", new BytesArray("{\"foo\": \"bar\"}"))); - assertEquals("stored script id can't contain: '#'", e.getMessage()); - e = expectThrows(IllegalArgumentException.class, () -> builder.deleteScript("_lang#", "_id")); - assertEquals("stored script language can't contain: '#'", e.getMessage()); - e = expectThrows(IllegalArgumentException.class, () -> builder.deleteScript("_lang", "_id#")); - assertEquals("stored script id can't contain: '#'", e.getMessage()); + builder.storeScript("_id", StoredScriptSource.parse("_lang", new BytesArray("{\"script\":\"1 + 1\"}"))); ScriptMetaData result = builder.build(); - assertEquals("1 + 1", result.getScript("_lang", "_id")); + assertEquals("1 + 1", result.getStoredScript("_id", "_lang").getCode()); } private ScriptMetaData randomScriptMetaData(XContentType sourceContentType) throws IOException { @@ -146,10 +105,44 @@ public class ScriptMetaDataTests extends ESTestCase { for (int i = 0; i < numScripts; i++) { String lang = randomAsciiOfLength(4); XContentBuilder sourceBuilder = XContentBuilder.builder(sourceContentType.xContent()); - sourceBuilder.startObject().field(randomAsciiOfLength(4), randomAsciiOfLength(4)).endObject(); - builder.storeScript(lang, randomAsciiOfLength(i + 1), sourceBuilder.bytes()); + sourceBuilder.startObject().field("script", randomAsciiOfLength(4)).endObject(); + builder.storeScript(randomAsciiOfLength(i + 1), StoredScriptSource.parse(lang, sourceBuilder.bytes())); } return builder.build(); } + @Override + protected ScriptMetaData createTestInstance() { + try { + return randomScriptMetaData(randomFrom(XContentType.values())); + } catch (IOException ioe) { + throw new UncheckedIOException(ioe); + } + } + + @Override + protected Writeable.Reader instanceReader() { + return ScriptMetaData::new; + } + + @Override + protected XContentBuilder toXContent(ScriptMetaData instance, XContentType contentType) throws IOException { + XContentBuilder builder = XContentFactory.contentBuilder(contentType); + if (randomBoolean()) { + builder.prettyPrint(); + } + builder.startObject(); + instance.toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + return builder; + } + + @Override + protected ScriptMetaData doParseInstance(XContentParser parser) { + try { + return ScriptMetaData.fromXContent(parser); + } catch (IOException ioe) { + throw new UncheckedIOException(ioe); + } + } } diff --git a/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java index 9d0c5b1867f..71fdea972f5 100644 --- a/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java +++ b/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java @@ -19,9 +19,7 @@ package org.elasticsearch.script; import org.elasticsearch.ResourceNotFoundException; -import org.elasticsearch.action.admin.cluster.storedscripts.DeleteStoredScriptRequest; import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptRequest; -import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.MetaData; @@ -121,9 +119,9 @@ public class ScriptServiceTests extends ESTestCase { // TODO: scriptService = new ScriptService(finalSettings, environment, resourceWatcherService, scriptEngineRegistry, scriptContextRegistry, scriptSettings) { @Override - String getScriptFromClusterState(String scriptLang, String id) { + StoredScriptSource getScriptFromClusterState(String id, String lang) { //mock the script that gets retrieved from an index - return "100"; + return new StoredScriptSource(lang, "100", Collections.emptyMap()); } }; } @@ -170,7 +168,7 @@ public class ScriptServiceTests extends ESTestCase { resourceWatcherService.notifyNow(); CompiledScript compiledScript = scriptService.compile(new Script(ScriptType.FILE, "test", "test_script", Collections.emptyMap()), - ScriptContext.Standard.SEARCH, Collections.emptyMap()); + ScriptContext.Standard.SEARCH); assertThat(compiledScript.compiled(), equalTo((Object) "compiled_test_file")); Files.delete(testFileNoExt); @@ -178,11 +176,10 @@ public class ScriptServiceTests extends ESTestCase { resourceWatcherService.notifyNow(); try { - scriptService.compile(new Script(ScriptType.FILE, "test", "test_script", Collections.emptyMap()), ScriptContext.Standard.SEARCH, - Collections.emptyMap()); + scriptService.compile(new Script(ScriptType.FILE, "test", "test_script", Collections.emptyMap()), ScriptContext.Standard.SEARCH); fail("the script test_script should no longer exist"); } catch (IllegalArgumentException ex) { - assertThat(ex.getMessage(), containsString("Unable to find on disk file script [test_script] using lang [test]")); + assertThat(ex.getMessage(), containsString("unable to find file script [test_script] using lang [test]")); } } @@ -197,7 +194,7 @@ public class ScriptServiceTests extends ESTestCase { resourceWatcherService.notifyNow(); CompiledScript compiledScript = scriptService.compile(new Script(ScriptType.FILE, "test", "file_script", Collections.emptyMap()), - ScriptContext.Standard.SEARCH, Collections.emptyMap()); + ScriptContext.Standard.SEARCH); assertThat(compiledScript.compiled(), equalTo((Object) "compiled_test_file_script")); Files.delete(testHiddenFile); @@ -208,9 +205,9 @@ public class ScriptServiceTests extends ESTestCase { public void testInlineScriptCompiledOnceCache() throws IOException { buildScriptService(Settings.EMPTY); CompiledScript compiledScript1 = scriptService.compile(new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()), - randomFrom(scriptContexts), Collections.emptyMap()); + randomFrom(scriptContexts)); CompiledScript compiledScript2 = scriptService.compile(new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()), - randomFrom(scriptContexts), Collections.emptyMap()); + randomFrom(scriptContexts)); assertThat(compiledScript1.compiled(), sameInstance(compiledScript2.compiled())); } @@ -333,7 +330,7 @@ public class ScriptServiceTests extends ESTestCase { String type = scriptEngineService.getType(); try { scriptService.compile(new Script(randomFrom(ScriptType.values()), type, "test", Collections.emptyMap()), - new ScriptContext.Plugin(pluginName, unknownContext), Collections.emptyMap()); + new ScriptContext.Plugin(pluginName, unknownContext)); fail("script compilation should have been rejected"); } catch(IllegalArgumentException e) { assertThat(e.getMessage(), containsString("script context [" + pluginName + "_" + unknownContext + "] not supported")); @@ -342,8 +339,7 @@ public class ScriptServiceTests extends ESTestCase { public void testCompileCountedInCompilationStats() throws IOException { buildScriptService(Settings.EMPTY); - scriptService.compile(new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()), randomFrom(scriptContexts), - Collections.emptyMap()); + scriptService.compile(new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()), randomFrom(scriptContexts)); assertEquals(1L, scriptService.stats().getCompilations()); } @@ -364,8 +360,7 @@ public class ScriptServiceTests extends ESTestCase { int numberOfCompilations = randomIntBetween(1, 1024); for (int i = 0; i < numberOfCompilations; i++) { scriptService - .compile(new Script(ScriptType.INLINE, "test", i + " + " + i, Collections.emptyMap()), randomFrom(scriptContexts), - Collections.emptyMap()); + .compile(new Script(ScriptType.INLINE, "test", i + " + " + i, Collections.emptyMap()), randomFrom(scriptContexts)); } assertEquals(numberOfCompilations, scriptService.stats().getCompilations()); } @@ -383,15 +378,13 @@ public class ScriptServiceTests extends ESTestCase { public void testFileScriptCountedInCompilationStats() throws IOException { buildScriptService(Settings.EMPTY); createFileScripts("test"); - scriptService.compile(new Script(ScriptType.FILE, "test", "file_script", Collections.emptyMap()), randomFrom(scriptContexts), - Collections.emptyMap()); + scriptService.compile(new Script(ScriptType.FILE, "test", "file_script", Collections.emptyMap()), randomFrom(scriptContexts)); assertEquals(1L, scriptService.stats().getCompilations()); } public void testIndexedScriptCountedInCompilationStats() throws IOException { buildScriptService(Settings.EMPTY); - scriptService.compile(new Script(ScriptType.STORED, "test", "script", Collections.emptyMap()), randomFrom(scriptContexts), - Collections.emptyMap()); + scriptService.compile(new Script(ScriptType.STORED, "test", "script", Collections.emptyMap()), randomFrom(scriptContexts)); assertEquals(1L, scriptService.stats().getCompilations()); } @@ -411,8 +404,7 @@ public class ScriptServiceTests extends ESTestCase { builder.put("script.inline", "true"); buildScriptService(builder.build()); CompiledScript script = scriptService.compile( - new Script(ScriptType.INLINE, Script.DEFAULT_SCRIPT_LANG, "1 + 1", Collections.emptyMap()), - randomFrom(scriptContexts), Collections.emptyMap()); + new Script(ScriptType.INLINE, Script.DEFAULT_SCRIPT_LANG, "1 + 1", Collections.emptyMap()), randomFrom(scriptContexts)); assertEquals(script.lang(), Script.DEFAULT_SCRIPT_LANG); } @@ -421,68 +413,41 @@ public class ScriptServiceTests extends ESTestCase { .field("script", "abc") .endObject().bytes(); - ClusterState empty = ClusterState.builder(new ClusterName("_name")).build(); - PutStoredScriptRequest request = new PutStoredScriptRequest("_lang", "_id") - .script(script); - ClusterState result = ScriptService.innerStoreScript(empty, "_lang", request); - ScriptMetaData scriptMetaData = result.getMetaData().custom(ScriptMetaData.TYPE); + ScriptMetaData scriptMetaData = ScriptMetaData.putStoredScript(null, "_id", StoredScriptSource.parse("_lang", script)); assertNotNull(scriptMetaData); - assertEquals("abc", scriptMetaData.getScript("_lang", "_id")); + assertEquals("abc", scriptMetaData.getStoredScript("_id", "_lang").getCode()); } public void testDeleteScript() throws Exception { - ClusterState cs = ClusterState.builder(new ClusterName("_name")) - .metaData(MetaData.builder() - .putCustom(ScriptMetaData.TYPE, - new ScriptMetaData.Builder(null).storeScript("_lang", "_id", - new BytesArray("{\"script\":\"abc\"}")).build())) - .build(); - - DeleteStoredScriptRequest request = new DeleteStoredScriptRequest("_lang", "_id"); - ClusterState result = ScriptService.innerDeleteScript(cs, "_lang", request); - ScriptMetaData scriptMetaData = result.getMetaData().custom(ScriptMetaData.TYPE); + ScriptMetaData scriptMetaData = ScriptMetaData.putStoredScript(null, "_id", + StoredScriptSource.parse("_lang", new BytesArray("{\"script\":\"abc\"}"))); + scriptMetaData = ScriptMetaData.deleteStoredScript(scriptMetaData, "_id", "_lang"); assertNotNull(scriptMetaData); - assertNull(scriptMetaData.getScript("_lang", "_id")); - assertNull(scriptMetaData.getScriptAsBytes("_lang", "_id")); + assertNull(scriptMetaData.getStoredScript("_id", "_lang")); + ScriptMetaData errorMetaData = scriptMetaData; ResourceNotFoundException e = expectThrows(ResourceNotFoundException.class, () -> { - ScriptService.innerDeleteScript(cs, "_lang", new DeleteStoredScriptRequest("_lang", "_non_existing_id")); + ScriptMetaData.deleteStoredScript(errorMetaData, "_id", "_lang"); }); - assertEquals("Stored script with id [_non_existing_id] for language [_lang] does not exist", e.getMessage()); + assertEquals("stored script [_id] using lang [_lang] does not exist and cannot be deleted", e.getMessage()); } public void testGetStoredScript() throws Exception { buildScriptService(Settings.EMPTY); ClusterState cs = ClusterState.builder(new ClusterName("_name")) - .metaData(MetaData.builder() - .putCustom(ScriptMetaData.TYPE, - new ScriptMetaData.Builder(null).storeScript("_lang", "_id", - new BytesArray("{\"script\":\"abc\"}")).build())) - .build(); + .metaData(MetaData.builder() + .putCustom(ScriptMetaData.TYPE, + new ScriptMetaData.Builder(null).storeScript("_id", + StoredScriptSource.parse("_lang", new BytesArray("{\"script\":\"abc\"}"))).build())) + .build(); - assertEquals("abc", scriptService.getStoredScript(cs, new GetStoredScriptRequest("_lang", "_id"))); - assertNull(scriptService.getStoredScript(cs, new GetStoredScriptRequest("_lang", "_id2"))); + assertEquals("abc", scriptService.getStoredScript(cs, new GetStoredScriptRequest("_id", "_lang")).getCode()); + assertNull(scriptService.getStoredScript(cs, new GetStoredScriptRequest("_id2", "_lang"))); cs = ClusterState.builder(new ClusterName("_name")).build(); - assertNull(scriptService.getStoredScript(cs, new GetStoredScriptRequest("_lang", "_id"))); + assertNull(scriptService.getStoredScript(cs, new GetStoredScriptRequest("_id", "_lang"))); } - public void testValidateScriptSize() throws Exception { - int maxSize = 0xFFFF; - buildScriptService(Settings.EMPTY); - // allowed - scriptService.validateStoredScript("_id", "test", new BytesArray("{\"script\":\"" + randomAsciiOfLength(maxSize - 13) + "\"}")); - - // disallowed - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> { - scriptService.validateStoredScript("_id", "test", new BytesArray("{\"script\":\"" + randomAsciiOfLength(maxSize - 12) + "\"}")); - }); - assertThat(e.getMessage(), equalTo( - "Limit of script size in bytes [" + maxSize+ "] has been exceeded for script [_id] with size [" + (maxSize + 1) + "]")); - } - - private void createFileScripts(String... langs) throws IOException { for (String lang : langs) { Path scriptPath = scriptsFilePath.resolve("file_script." + lang); @@ -493,7 +458,7 @@ public class ScriptServiceTests extends ESTestCase { private void assertCompileRejected(String lang, String script, ScriptType scriptType, ScriptContext scriptContext) { try { - scriptService.compile(new Script(scriptType, lang, script, Collections.emptyMap()), scriptContext, Collections.emptyMap()); + scriptService.compile(new Script(scriptType, lang, script, Collections.emptyMap()), scriptContext); fail("compile should have been rejected for lang [" + lang + "], script_type [" + scriptType + "], scripted_op [" + scriptContext + "]"); } catch(IllegalStateException e) { //all good @@ -502,7 +467,7 @@ public class ScriptServiceTests extends ESTestCase { private void assertCompileAccepted(String lang, String script, ScriptType scriptType, ScriptContext scriptContext) { assertThat( - scriptService.compile(new Script(scriptType, lang, script, Collections.emptyMap()), scriptContext, Collections.emptyMap()), + scriptService.compile(new Script(scriptType, lang, script, Collections.emptyMap()), scriptContext), notNullValue() ); } diff --git a/core/src/test/java/org/elasticsearch/script/ScriptTests.java b/core/src/test/java/org/elasticsearch/script/ScriptTests.java index 19c8467486a..fc841bd1648 100644 --- a/core/src/test/java/org/elasticsearch/script/ScriptTests.java +++ b/core/src/test/java/org/elasticsearch/script/ScriptTests.java @@ -78,10 +78,9 @@ public class ScriptTests extends ESTestCase { } return new Script( scriptType, - randomFrom("_lang1", "_lang2", "_lang3"), + scriptType == ScriptType.STORED ? null : randomFrom("_lang1", "_lang2", "_lang3"), script, - scriptType == ScriptType.INLINE ? - Collections.singletonMap(Script.CONTENT_TYPE_OPTION, xContent.type().mediaType()) : Collections.emptyMap(), + scriptType == ScriptType.INLINE ? Collections.singletonMap(Script.CONTENT_TYPE_OPTION, xContent.type().mediaType()) : null, params ); } diff --git a/core/src/test/java/org/elasticsearch/script/StoredScriptTests.java b/core/src/test/java/org/elasticsearch/script/StoredScriptTests.java new file mode 100644 index 00000000000..8a6d54c0b9c --- /dev/null +++ b/core/src/test/java/org/elasticsearch/script/StoredScriptTests.java @@ -0,0 +1,364 @@ +/* + * 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 org.elasticsearch.ResourceNotFoundException; +import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.test.AbstractSerializingTestCase; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.Collections; + +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonMap; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; + +public class StoredScriptTests extends AbstractSerializingTestCase { + + public void testBasicAddDelete() { + StoredScriptSource source = new StoredScriptSource("lang", "code", emptyMap()); + ScriptMetaData smd = ScriptMetaData.putStoredScript(null, "test", source); + + assertThat(smd.getStoredScript("test", null), equalTo(source)); + assertThat(smd.getStoredScript("test", "lang"), equalTo(source)); + + smd = ScriptMetaData.deleteStoredScript(smd, "test", null); + + assertThat(smd.getStoredScript("test", null), nullValue()); + assertThat(smd.getStoredScript("test", "lang"), nullValue()); + } + + public void testDifferentMultiAddDelete() { + StoredScriptSource source0 = new StoredScriptSource("lang0", "code0", emptyMap()); + StoredScriptSource source1 = new StoredScriptSource("lang0", "code1", emptyMap()); + StoredScriptSource source2 = new StoredScriptSource("lang1", "code2", + singletonMap(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType())); + + ScriptMetaData smd = ScriptMetaData.putStoredScript(null, "test0", source0); + smd = ScriptMetaData.putStoredScript(smd, "test1", source1); + smd = ScriptMetaData.putStoredScript(smd, "test2", source2); + + assertThat(smd.getStoredScript("test0", null), equalTo(source0)); + assertThat(smd.getStoredScript("test0", "lang0"), equalTo(source0)); + assertThat(smd.getStoredScript("test1", null), equalTo(source1)); + assertThat(smd.getStoredScript("test1", "lang0"), equalTo(source1)); + assertThat(smd.getStoredScript("test2", null), equalTo(source2)); + assertThat(smd.getStoredScript("test2", "lang1"), equalTo(source2)); + + assertThat(smd.getStoredScript("test3", null), nullValue()); + assertThat(smd.getStoredScript("test0", "lang1"), nullValue()); + assertThat(smd.getStoredScript("test2", "lang0"), nullValue()); + + smd = ScriptMetaData.deleteStoredScript(smd, "test0", null); + + assertThat(smd.getStoredScript("test0", null), nullValue()); + assertThat(smd.getStoredScript("test0", "lang0"), nullValue()); + assertThat(smd.getStoredScript("test1", null), equalTo(source1)); + assertThat(smd.getStoredScript("test1", "lang0"), equalTo(source1)); + assertThat(smd.getStoredScript("test2", null), equalTo(source2)); + assertThat(smd.getStoredScript("test2", "lang1"), equalTo(source2)); + + assertThat(smd.getStoredScript("test3", null), nullValue()); + assertThat(smd.getStoredScript("test0", "lang1"), nullValue()); + assertThat(smd.getStoredScript("test2", "lang0"), nullValue()); + + smd = ScriptMetaData.deleteStoredScript(smd, "test2", "lang1"); + + assertThat(smd.getStoredScript("test0", null), nullValue()); + assertThat(smd.getStoredScript("test0", "lang0"), nullValue()); + assertThat(smd.getStoredScript("test1", null), equalTo(source1)); + assertThat(smd.getStoredScript("test1", "lang0"), equalTo(source1)); + assertThat(smd.getStoredScript("test2", null), nullValue()); + assertThat(smd.getStoredScript("test2", "lang1"), nullValue()); + + assertThat(smd.getStoredScript("test3", null), nullValue()); + assertThat(smd.getStoredScript("test0", "lang1"), nullValue()); + assertThat(smd.getStoredScript("test2", "lang0"), nullValue()); + + smd = ScriptMetaData.deleteStoredScript(smd, "test1", "lang0"); + + assertThat(smd.getStoredScript("test0", null), nullValue()); + assertThat(smd.getStoredScript("test0", "lang0"), nullValue()); + assertThat(smd.getStoredScript("test1", null), nullValue()); + assertThat(smd.getStoredScript("test1", "lang0"), nullValue()); + assertThat(smd.getStoredScript("test2", null), nullValue()); + assertThat(smd.getStoredScript("test2", "lang1"), nullValue()); + + assertThat(smd.getStoredScript("test3", null), nullValue()); + assertThat(smd.getStoredScript("test0", "lang1"), nullValue()); + assertThat(smd.getStoredScript("test2", "lang0"), nullValue()); + } + + public void testSameMultiAddDelete() { + StoredScriptSource source0 = new StoredScriptSource("lang0", "code0", emptyMap()); + StoredScriptSource source1 = new StoredScriptSource("lang1", "code1", emptyMap()); + StoredScriptSource source2 = new StoredScriptSource("lang2", "code1", emptyMap()); + StoredScriptSource source3 = new StoredScriptSource("lang1", "code2", + singletonMap(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType())); + + ScriptMetaData smd = ScriptMetaData.putStoredScript(null, "test0", source0); + smd = ScriptMetaData.putStoredScript(smd, "test0", source1); + assertWarnings("stored script [test0] already exists using a different lang [lang0]," + + " the new namespace for stored scripts will only use (id) instead of (lang, id)"); + smd = ScriptMetaData.putStoredScript(smd, "test3", source3); + smd = ScriptMetaData.putStoredScript(smd, "test0", source2); + assertWarnings("stored script [test0] already exists using a different lang [lang1]," + + " the new namespace for stored scripts will only use (id) instead of (lang, id)"); + + assertThat(smd.getStoredScript("test0", null), equalTo(source2)); + assertThat(smd.getStoredScript("test0", "lang0"), equalTo(source0)); + assertThat(smd.getStoredScript("test0", "lang1"), equalTo(source1)); + assertThat(smd.getStoredScript("test0", "lang2"), equalTo(source2)); + assertThat(smd.getStoredScript("test3", null), equalTo(source3)); + assertThat(smd.getStoredScript("test3", "lang1"), equalTo(source3)); + + smd = ScriptMetaData.deleteStoredScript(smd, "test0", "lang1"); + + assertThat(smd.getStoredScript("test0", null), equalTo(source2)); + assertThat(smd.getStoredScript("test0", "lang0"), equalTo(source0)); + assertThat(smd.getStoredScript("test0", "lang1"), nullValue()); + assertThat(smd.getStoredScript("test0", "lang2"), equalTo(source2)); + assertThat(smd.getStoredScript("test3", null), equalTo(source3)); + assertThat(smd.getStoredScript("test3", "lang1"), equalTo(source3)); + + smd = ScriptMetaData.deleteStoredScript(smd, "test0", null); + + assertThat(smd.getStoredScript("test0", null), nullValue()); + assertThat(smd.getStoredScript("test0", "lang0"), equalTo(source0)); + assertThat(smd.getStoredScript("test0", "lang1"), nullValue()); + assertThat(smd.getStoredScript("test0", "lang2"), nullValue()); + assertThat(smd.getStoredScript("test3", null), equalTo(source3)); + assertThat(smd.getStoredScript("test3", "lang1"), equalTo(source3)); + + smd = ScriptMetaData.deleteStoredScript(smd, "test3", "lang1"); + + assertThat(smd.getStoredScript("test0", null), nullValue()); + assertThat(smd.getStoredScript("test0", "lang0"), equalTo(source0)); + assertThat(smd.getStoredScript("test0", "lang1"), nullValue()); + assertThat(smd.getStoredScript("test0", "lang2"), nullValue()); + assertThat(smd.getStoredScript("test3", null), nullValue()); + assertThat(smd.getStoredScript("test3", "lang1"), nullValue()); + + smd = ScriptMetaData.deleteStoredScript(smd, "test0", "lang0"); + + assertThat(smd.getStoredScript("test0", null), nullValue()); + assertThat(smd.getStoredScript("test0", "lang0"), nullValue()); + assertThat(smd.getStoredScript("test0", "lang1"), nullValue()); + assertThat(smd.getStoredScript("test0", "lang2"), nullValue()); + assertThat(smd.getStoredScript("test3", null), nullValue()); + assertThat(smd.getStoredScript("test3", "lang1"), nullValue()); + } + + public void testInvalidDelete() { + ResourceNotFoundException rnfe = + expectThrows(ResourceNotFoundException.class, () -> ScriptMetaData.deleteStoredScript(null, "test", "lang")); + assertThat(rnfe.getMessage(), equalTo("stored script [test] using lang [lang] does not exist and cannot be deleted")); + + rnfe = expectThrows(ResourceNotFoundException.class, () -> ScriptMetaData.deleteStoredScript(null, "test", null)); + assertThat(rnfe.getMessage(), equalTo("stored script [test] does not exist and cannot be deleted")); + } + + public void testSourceParsing() throws Exception { + // simple script value string + try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { + builder.startObject().field("script", "code").endObject(); + + StoredScriptSource parsed = StoredScriptSource.parse("lang", builder.bytes()); + StoredScriptSource source = new StoredScriptSource("lang", "code", Collections.emptyMap()); + + assertThat(parsed, equalTo(source)); + } + + // simple template value string + try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { + builder.startObject().field("template", "code").endObject(); + + StoredScriptSource parsed = StoredScriptSource.parse("lang", builder.bytes()); + StoredScriptSource source = new StoredScriptSource("lang", "code", + Collections.singletonMap(Script.CONTENT_TYPE_OPTION, builder.contentType().mediaType())); + + assertThat(parsed, equalTo(source)); + } + + // complex template with wrapper template object + try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { + builder.startObject().field("template").startObject().field("query", "code").endObject().endObject(); + String code; + + try (XContentBuilder cb = XContentFactory.contentBuilder(builder.contentType())) { + code = cb.startObject().field("query", "code").endObject().string(); + } + + StoredScriptSource parsed = StoredScriptSource.parse("lang", builder.bytes()); + StoredScriptSource source = new StoredScriptSource("lang", code, + Collections.singletonMap(Script.CONTENT_TYPE_OPTION, builder.contentType().mediaType())); + + assertThat(parsed, equalTo(source)); + } + + // complex template with no wrapper object + try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { + builder.startObject().field("query", "code").endObject(); + String code; + + try (XContentBuilder cb = XContentFactory.contentBuilder(builder.contentType())) { + code = cb.startObject().field("query", "code").endObject().string(); + } + + StoredScriptSource parsed = StoredScriptSource.parse("lang", builder.bytes()); + StoredScriptSource source = new StoredScriptSource("lang", code, + Collections.singletonMap(Script.CONTENT_TYPE_OPTION, builder.contentType().mediaType())); + + assertThat(parsed, equalTo(source)); + } + + // complex template using script as the field name + try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { + builder.startObject().field("script").startObject().field("query", "code").endObject().endObject(); + String code; + + try (XContentBuilder cb = XContentFactory.contentBuilder(builder.contentType())) { + code = cb.startObject().field("query", "code").endObject().string(); + } + + StoredScriptSource parsed = StoredScriptSource.parse("lang", builder.bytes()); + StoredScriptSource source = new StoredScriptSource("lang", code, + Collections.singletonMap(Script.CONTENT_TYPE_OPTION, builder.contentType().mediaType())); + + assertThat(parsed, equalTo(source)); + } + + // complex script with script object + try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { + builder.startObject().field("script").startObject().field("lang", "lang").field("code", "code").endObject().endObject(); + + StoredScriptSource parsed = StoredScriptSource.parse(null, builder.bytes()); + StoredScriptSource source = new StoredScriptSource("lang", "code", Collections.emptyMap()); + + assertThat(parsed, equalTo(source)); + } + + // complex script with script object and empty options + try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { + builder.startObject().field("script").startObject().field("lang", "lang").field("code", "code") + .field("options").startObject().endObject().endObject().endObject(); + + StoredScriptSource parsed = StoredScriptSource.parse(null, builder.bytes()); + StoredScriptSource source = new StoredScriptSource("lang", "code", Collections.emptyMap()); + + assertThat(parsed, equalTo(source)); + } + + // complex script with embedded template + try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { + builder.startObject().field("script").startObject().field("lang", "lang").startObject("code").field("query", "code") + .endObject().startObject("options").endObject().endObject().endObject().string(); + String code; + + try (XContentBuilder cb = XContentFactory.contentBuilder(builder.contentType())) { + code = cb.startObject().field("query", "code").endObject().string(); + } + + StoredScriptSource parsed = StoredScriptSource.parse(null, builder.bytes()); + StoredScriptSource source = new StoredScriptSource("lang", code, + Collections.singletonMap(Script.CONTENT_TYPE_OPTION, builder.contentType().mediaType())); + + assertThat(parsed, equalTo(source)); + } + } + + public void testSourceParsingErrors() throws Exception { + // check for missing lang parameter when parsing a template + try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { + builder.startObject().field("template", "code").endObject(); + + IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> + StoredScriptSource.parse(null, builder.bytes())); + assertThat(iae.getMessage(), equalTo("unexpected stored script format")); + } + + // check for missing lang parameter when parsing a script + try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { + builder.startObject().field("script").startObject().field("code", "code").endObject().endObject(); + + IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> + StoredScriptSource.parse(null, builder.bytes())); + assertThat(iae.getMessage(), equalTo("must specify lang for stored script")); + } + + // check for missing code parameter when parsing a script + try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { + builder.startObject().field("script").startObject().field("lang", "lang").endObject().endObject(); + + IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> + StoredScriptSource.parse(null, builder.bytes())); + assertThat(iae.getMessage(), equalTo("must specify code for stored script")); + } + + // check for illegal options parameter when parsing a script + try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { + builder.startObject().field("script").startObject().field("lang", "lang").field("code", "code") + .startObject("options").field("option", "option").endObject().endObject().endObject(); + + IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> + StoredScriptSource.parse(null, builder.bytes())); + assertThat(iae.getMessage(), equalTo("illegal compiler options [{option=option}] specified")); + } + + // check for illegal use of content type option + try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { + builder.startObject().field("script").startObject().field("lang", "lang").field("code", "code") + .startObject("options").field("content_type", "option").endObject().endObject().endObject(); + + ParsingException pe = expectThrows(ParsingException.class, () -> + StoredScriptSource.parse(null, builder.bytes())); + assertThat(pe.getRootCause().getMessage(), equalTo("content_type cannot be user-specified")); + } + } + + @Override + protected StoredScriptSource createTestInstance() { + return new StoredScriptSource( + randomAsciiOfLength(randomIntBetween(4, 32)), + randomAsciiOfLength(randomIntBetween(4, 16383)), + Collections.emptyMap()); + } + + @Override + protected Writeable.Reader instanceReader() { + return StoredScriptSource::new; + } + + @Override + protected StoredScriptSource doParseInstance(XContentParser parser) { + try { + return StoredScriptSource.fromXContent(parser); + } catch (IOException ioe) { + throw new UncheckedIOException(ioe); + } + } +} diff --git a/core/src/test/java/org/elasticsearch/script/StoredScriptsIT.java b/core/src/test/java/org/elasticsearch/script/StoredScriptsIT.java index 2bc58bb0d22..106cb82526f 100644 --- a/core/src/test/java/org/elasticsearch/script/StoredScriptsIT.java +++ b/core/src/test/java/org/elasticsearch/script/StoredScriptsIT.java @@ -50,37 +50,38 @@ public class StoredScriptsIT extends ESIntegTestCase { public void testBasics() { assertAcked(client().admin().cluster().preparePutStoredScript() - .setScriptLang(LANG) + .setLang(LANG) .setId("foobar") - .setSource(new BytesArray("{\"script\":\"1\"}"))); + .setContent(new BytesArray("{\"script\":\"1\"}"))); String script = client().admin().cluster().prepareGetStoredScript(LANG, "foobar") - .get().getStoredScript(); + .get().getSource().getCode(); assertNotNull(script); assertEquals("1", script); assertAcked(client().admin().cluster().prepareDeleteStoredScript() .setId("foobar") - .setScriptLang(LANG)); - script = client().admin().cluster().prepareGetStoredScript(LANG, "foobar") - .get().getStoredScript(); - assertNull(script); + .setLang(LANG)); + StoredScriptSource source = client().admin().cluster().prepareGetStoredScript(LANG, "foobar") + .get().getSource(); + assertNull(source); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client().admin().cluster().preparePutStoredScript() - .setScriptLang("lang#") + .setLang("lang#") .setId("id#") - .setSource(new BytesArray("{}")) + .setContent(new BytesArray("{}")) .get()); - assertEquals("Validation Failed: 1: id can't contain: '#';2: lang can't contain: '#';", e.getMessage()); + assertEquals("Validation Failed: 1: id cannot contain '#' for stored script;" + + "2: lang cannot contain '#' for stored script;", e.getMessage()); } public void testMaxScriptSize() { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client().admin().cluster().preparePutStoredScript() - .setScriptLang(LANG) + .setLang(LANG) .setId("foobar") - .setSource(new BytesArray(randomAsciiOfLength(SCRIPT_MAX_SIZE_IN_BYTES + 1))) + .setContent(new BytesArray(randomAsciiOfLength(SCRIPT_MAX_SIZE_IN_BYTES + 1))) .get() ); - assertEquals("Limit of script size in bytes [64] has been exceeded for script [foobar] with size [65]", e.getMessage()); + assertEquals("exceeded max allowed stored script size in bytes [64] with size [65] for script [foobar]", e.getMessage()); } public static class CustomScriptPlugin extends MockScriptPlugin { diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java index 545c10bcb03..ca88b56cd42 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java @@ -228,24 +228,24 @@ public class ScriptedMetricIT extends ESIntegTestCase { // the id of the stored script is used in test method while the source of the stored script // must match a predefined script from CustomScriptPlugin.pluginScripts() method assertAcked(client().admin().cluster().preparePutStoredScript() - .setScriptLang(CustomScriptPlugin.NAME) + .setLang(CustomScriptPlugin.NAME) .setId("initScript_stored") - .setSource(new BytesArray("{\"script\":\"vars.multiplier = 3\"}"))); + .setContent(new BytesArray("{\"script\":\"vars.multiplier = 3\"}"))); assertAcked(client().admin().cluster().preparePutStoredScript() - .setScriptLang(CustomScriptPlugin.NAME) + .setLang(CustomScriptPlugin.NAME) .setId("mapScript_stored") - .setSource(new BytesArray("{\"script\":\"_agg.add(vars.multiplier)\"}"))); + .setContent(new BytesArray("{\"script\":\"_agg.add(vars.multiplier)\"}"))); assertAcked(client().admin().cluster().preparePutStoredScript() - .setScriptLang(CustomScriptPlugin.NAME) + .setLang(CustomScriptPlugin.NAME) .setId("combineScript_stored") - .setSource(new BytesArray("{\"script\":\"sum agg values as a new aggregation\"}"))); + .setContent(new BytesArray("{\"script\":\"sum agg values as a new aggregation\"}"))); assertAcked(client().admin().cluster().preparePutStoredScript() - .setScriptLang(CustomScriptPlugin.NAME) + .setLang(CustomScriptPlugin.NAME) .setId("reduceScript_stored") - .setSource(new BytesArray("{\"script\":\"sum aggs of agg values as a new aggregation\"}"))); + .setContent(new BytesArray("{\"script\":\"sum aggs of agg values as a new aggregation\"}"))); indexRandom(true, builders); ensureSearchable(); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricTests.java index 11c6ed7f6a2..8e09447fef0 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricTests.java @@ -55,8 +55,9 @@ public class ScriptedMetricTests extends BaseAggregationTestCase 100)\" }"))); + .setContent(new BytesArray("{ \"script\": \"Double.isNaN(_value0) ? false : (_value0 + _value1 > 100)\" }"))); Script script = new Script(ScriptType.STORED, CustomScriptPlugin.NAME, "my_script", Collections.emptyMap()); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorTests.java index 563894906ed..0311781455a 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketSelectorTests.java @@ -46,7 +46,9 @@ public class BucketSelectorTests extends BasePipelineAggregationTestCase> extends EST scriptService = new ScriptService(baseSettings, environment, new ResourceWatcherService(baseSettings, null), scriptEngineRegistry, scriptContextRegistry, scriptSettings) { @Override - public CompiledScript compile(Script script, ScriptContext scriptContext, Map params) { + public CompiledScript compile(Script script, ScriptContext scriptContext) { return new CompiledScript(ScriptType.INLINE, "mockName", "test", script); } }; diff --git a/core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java b/core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java index 2957a98f72a..0000e73e27d 100644 --- a/core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java +++ b/core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java @@ -539,9 +539,9 @@ public class SharedClusterSnapshotRestoreIT extends AbstractSnapshotIntegTestCas if(testScript) { logger.info("--> creating test script"); assertAcked(client().admin().cluster().preparePutStoredScript() - .setScriptLang(MockScriptEngine.NAME) + .setLang(MockScriptEngine.NAME) .setId("foobar") - .setSource(new BytesArray("{\"script\":\"1\"}"))); + .setContent(new BytesArray("{\"script\":\"1\"}"))); } logger.info("--> snapshot without global state"); @@ -600,7 +600,7 @@ public class SharedClusterSnapshotRestoreIT extends AbstractSnapshotIntegTestCas if (testScript) { logger.info("--> check that script is restored"); GetStoredScriptResponse getStoredScriptResponse = client().admin().cluster().prepareGetStoredScript(MockScriptEngine.NAME, "foobar").get(); - assertNotNull(getStoredScriptResponse.getStoredScript()); + assertNotNull(getStoredScriptResponse.getSource()); } createIndex("test-idx"); @@ -644,7 +644,7 @@ public class SharedClusterSnapshotRestoreIT extends AbstractSnapshotIntegTestCas getIndexTemplatesResponse = client().admin().indices().prepareGetTemplates().get(); assertIndexTemplateMissing(getIndexTemplatesResponse, "test-template"); assertFalse(client().admin().cluster().prepareGetPipeline("barbaz").get().isFound()); - assertNull(client().admin().cluster().prepareGetStoredScript(MockScriptEngine.NAME, "foobar").get().getStoredScript()); + assertNull(client().admin().cluster().prepareGetStoredScript(MockScriptEngine.NAME, "foobar").get().getSource()); assertThat(client.prepareSearch("test-idx").setSize(0).get().getHits().totalHits(), equalTo(100L)); } diff --git a/docs/reference/modules/scripting/using.asciidoc b/docs/reference/modules/scripting/using.asciidoc index 5f2132d142d..7c89201c7fc 100644 --- a/docs/reference/modules/scripting/using.asciidoc +++ b/docs/reference/modules/scripting/using.asciidoc @@ -54,11 +54,11 @@ GET my_index/_search setting `script.default_lang` to the appropriate language. -`inline`, `id`, `file`:: +`inline`, `stored`, `file`:: Specifies the source of the script. An `inline` script is specified - `inline` as in the example above, a stored script with the specified `id` - is retrieved from the cluster state (see <>), + `inline` as in the example above, a `stored` script is specified `stored` + and is retrieved from the cluster state (see <>), and a `file` script is retrieved from a file in the `config/scripts` directory (see <>). + @@ -164,7 +164,6 @@ hierarchy of directories is flattened and concatenated with underscores. A script in `group1/group2/my_script.painless` should use `group1_group2_myscript` as the `file` name. - [[reload-scripts]] [float] ==== Automatic script reloading @@ -181,23 +180,51 @@ Script reloading can be completely disabled by setting === Stored Scripts Scripts may be stored in and retrieved from the cluster state using the -`_scripts` end-point: +`_scripts` end-point. + +==== Deprecated Namespace + +The namespace for stored scripts using both `lang` and `id` as a unique +identifier has been deprecated. The new namespace for stored scripts will +only use `id`. Stored scripts with the same `id`, but different `lang`'s +will no longer be allowed in 6.0. To comply with the new namespace for +stored scripts, existing stored scripts should be deleted and put again. +Any scripts that share an `id` but have different `lang`s will need to +be re-named. For example, take the following: + +"id": "example", "lang": "painless" +"id": "example", "lang": "expressions" + +The above scripts will conflict under the new namespace since the id's are +the same. At least one will have to be re-named to comply with the new +namespace of only `id`. + +As a final caveat, stored search templates and stored scripts share +the same namespace, so if a search template has the same `id` as a +stored script, one of the two will have to be re-named as well using +delete and put requests. + +==== Request Examples + +The following are examples of stored script requests: [source,js] ----------------------------------- -/_scripts/{lang}/{id} <1> <2> +/_scripts/{id} <1> ----------------------------------- -<1> The `lang` represents the script language. -<2> The `id` is a unique identifier or script name. +<1> The `id` is a unique identifier for the stored script. This example stores a Painless script called `calculate-score` in the cluster state: [source,js] ----------------------------------- -POST _scripts/painless/calculate-score +POST _scripts/calculate-score { - "script": "Math.log(_score * 2) + params.my_modifier" + "script": { + "lang": "painless", + "code": "Math.log(_score * 2) + params.my_modifier" + } } ----------------------------------- // CONSOLE @@ -206,12 +233,12 @@ This same script can be retrieved with: [source,js] ----------------------------------- -GET _scripts/painless/calculate-score +GET _scripts/calculate-score ----------------------------------- // CONSOLE // TEST[continued] -Stored scripts can be used by specifying the `lang` and `stored` parameters as follows: +Stored scripts can be used by specifying the `stored` parameters as follows: [source,js] -------------------------------------------------- @@ -220,7 +247,6 @@ GET _search "query": { "script": { "script": { - "lang": "painless", "stored": "calculate-score", "params": { "my_modifier": 2 @@ -237,7 +263,7 @@ And deleted with: [source,js] ----------------------------------- -DELETE _scripts/painless/calculate-score +DELETE _scripts/calculate-score ----------------------------------- // CONSOLE // TEST[continued] 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 4480ca1da58..33c06a3804a 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 @@ -19,8 +19,6 @@ package org.elasticsearch.ingest.common; -import java.util.Map; - import org.elasticsearch.common.Strings; import org.elasticsearch.ingest.AbstractProcessor; import org.elasticsearch.ingest.IngestDocument; @@ -30,7 +28,8 @@ import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptException; import org.elasticsearch.script.ScriptService; -import org.elasticsearch.script.ScriptType; + +import java.util.Map; import static java.util.Collections.emptyMap; import static org.elasticsearch.common.Strings.hasLength; @@ -139,7 +138,7 @@ public final class ScriptProcessor extends AbstractProcessor { // verify script is able to be compiled before successfully creating processor. try { - scriptService.compile(script, ScriptContext.Standard.INGEST, script.getOptions()); + scriptService.compile(script, ScriptContext.Standard.INGEST); } catch (ScriptException e) { throw newConfigurationException(TYPE, processorTag, scriptPropertyUsed, e); } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java index 73b971ee297..60b42979466 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java @@ -107,7 +107,7 @@ public class ScriptProcessorFactoryTests extends ESTestCase { ScriptService mockedScriptService = mock(ScriptService.class); ScriptException thrownException = new ScriptException("compile-time exception", new RuntimeException(), Collections.emptyList(), "script", "mockscript"); - when(mockedScriptService.compile(any(), any(), any())).thenThrow(thrownException); + when(mockedScriptService.compile(any(), any())).thenThrow(thrownException); factory = new ScriptProcessor.Factory(mockedScriptService); Map configMap = new HashMap<>(); diff --git a/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/IndexedExpressionTests.java b/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/StoredExpressionTests.java similarity index 94% rename from modules/lang-expression/src/test/java/org/elasticsearch/script/expression/IndexedExpressionTests.java rename to modules/lang-expression/src/test/java/org/elasticsearch/script/expression/StoredExpressionTests.java index 71c87937618..e2e7b819333 100644 --- a/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/IndexedExpressionTests.java +++ b/modules/lang-expression/src/test/java/org/elasticsearch/script/expression/StoredExpressionTests.java @@ -35,7 +35,7 @@ import java.util.Collections; import static org.hamcrest.Matchers.containsString; //TODO: please convert to unit tests! -public class IndexedExpressionTests extends ESIntegTestCase { +public class StoredExpressionTests extends ESIntegTestCase { @Override protected Settings nodeSettings(int nodeOrdinal) { Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal)); @@ -51,9 +51,9 @@ public class IndexedExpressionTests extends ESIntegTestCase { public void testAllOpsDisabledIndexedScripts() throws IOException { client().admin().cluster().preparePutStoredScript() - .setScriptLang(ExpressionScriptEngineService.NAME) + .setLang(ExpressionScriptEngineService.NAME) .setId("script1") - .setSource(new BytesArray("{\"script\":\"2\"}")) + .setContent(new BytesArray("{\"script\":\"2\"}")) .get(); client().prepareIndex("test", "scriptTest", "1").setSource("{\"theField\":\"foo\"}").get(); try { @@ -62,7 +62,7 @@ public class IndexedExpressionTests extends ESIntegTestCase { fail("update script should have been rejected"); } catch(Exception e) { assertThat(e.getMessage(), containsString("failed to execute script")); - assertThat(e.getCause().getMessage(), containsString("scripts of type [stored], operation [update] and lang [expression] are disabled")); + assertThat(e.getCause().getMessage(), containsString("scripts of type [stored], operation [update] and lang [expression] are not supported")); } try { client().prepareSearch() diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestDeleteSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestDeleteSearchTemplateAction.java index 027235496b8..61a394462f9 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestDeleteSearchTemplateAction.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestDeleteSearchTemplateAction.java @@ -18,21 +18,32 @@ */ package org.elasticsearch.script.mustache; +import org.elasticsearch.action.admin.cluster.storedscripts.DeleteStoredScriptRequest; +import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.action.admin.cluster.RestDeleteStoredScriptAction; +import org.elasticsearch.rest.action.AcknowledgedRestListener; +import org.elasticsearch.script.Script; + +import java.io.IOException; import static org.elasticsearch.rest.RestRequest.Method.DELETE; -public class RestDeleteSearchTemplateAction extends RestDeleteStoredScriptAction { +public class RestDeleteSearchTemplateAction extends BaseRestHandler { + public RestDeleteSearchTemplateAction(Settings settings, RestController controller) { - super(settings, controller, false); + super(settings); + controller.registerHandler(DELETE, "/_search/template/{id}", this); } @Override - protected String getScriptLang(RestRequest request) { - return "mustache"; + public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + String id = request.param("id"); + + DeleteStoredScriptRequest deleteStoredScriptRequest = new DeleteStoredScriptRequest(id, Script.DEFAULT_TEMPLATE_LANG); + return channel -> client.admin().cluster().deleteStoredScript(deleteStoredScriptRequest, new AcknowledgedRestListener<>(channel)); } } diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestGetSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestGetSearchTemplateAction.java index c02c01273af..7d1ed4b57a4 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestGetSearchTemplateAction.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestGetSearchTemplateAction.java @@ -18,29 +18,64 @@ */ package org.elasticsearch.script.mustache; +import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptRequest; +import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptResponse; +import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.ParseField; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.rest.BaseRestHandler; +import org.elasticsearch.rest.BytesRestResponse; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.action.admin.cluster.RestGetStoredScriptAction; +import org.elasticsearch.rest.RestResponse; +import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.rest.action.RestBuilderListener; +import org.elasticsearch.script.Script; +import org.elasticsearch.script.StoredScriptSource; + +import java.io.IOException; import static org.elasticsearch.rest.RestRequest.Method.GET; -public class RestGetSearchTemplateAction extends RestGetStoredScriptAction { +public class RestGetSearchTemplateAction extends BaseRestHandler { - private static final String TEMPLATE = "template"; + public static final ParseField _ID_PARSE_FIELD = new ParseField("_id"); + + public static final ParseField FOUND_PARSE_FIELD = new ParseField("found"); public RestGetSearchTemplateAction(Settings settings, RestController controller) { - super(settings, controller, false); + super(settings); + controller.registerHandler(GET, "/_search/template/{id}", this); } @Override - protected String getScriptLang(RestRequest request) { - return "mustache"; - } + public RestChannelConsumer prepareRequest(final RestRequest request, NodeClient client) throws IOException { + String id = request.param("id"); - @Override - protected String getScriptFieldName() { - return TEMPLATE; + GetStoredScriptRequest getRequest = new GetStoredScriptRequest(id, Script.DEFAULT_TEMPLATE_LANG); + + return channel -> client.admin().cluster().getStoredScript(getRequest, new RestBuilderListener(channel) { + @Override + public RestResponse buildResponse(GetStoredScriptResponse response, XContentBuilder builder) throws Exception { + builder.startObject(); + builder.field(_ID_PARSE_FIELD.getPreferredName(), id); + + builder.field(StoredScriptSource.LANG_PARSE_FIELD.getPreferredName(), Script.DEFAULT_TEMPLATE_LANG); + + StoredScriptSource source = response.getSource(); + boolean found = source != null; + builder.field(FOUND_PARSE_FIELD.getPreferredName(), found); + + if (found) { + builder.field(StoredScriptSource.TEMPLATE_PARSE_FIELD.getPreferredName(), source.getCode()); + } + + builder.endObject(); + + return new BytesRestResponse(found ? RestStatus.OK : RestStatus.NOT_FOUND, builder); + } + }); } } diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestPutSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestPutSearchTemplateAction.java index 2426a09ec8e..6c22b0160b6 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestPutSearchTemplateAction.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestPutSearchTemplateAction.java @@ -18,23 +18,36 @@ */ package org.elasticsearch.script.mustache; +import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest; +import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.action.admin.cluster.RestPutStoredScriptAction; +import org.elasticsearch.rest.action.AcknowledgedRestListener; +import org.elasticsearch.script.Script; + +import java.io.IOException; import static org.elasticsearch.rest.RestRequest.Method.POST; import static org.elasticsearch.rest.RestRequest.Method.PUT; -public class RestPutSearchTemplateAction extends RestPutStoredScriptAction { +public class RestPutSearchTemplateAction extends BaseRestHandler { + public RestPutSearchTemplateAction(Settings settings, RestController controller) { - super(settings, controller, false); + super(settings); + controller.registerHandler(POST, "/_search/template/{id}", this); controller.registerHandler(PUT, "/_search/template/{id}", this); } @Override - protected String getScriptLang(RestRequest request) { - return "mustache"; + public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + String id = request.param("id"); + BytesReference content = request.content(); + + PutStoredScriptRequest put = new PutStoredScriptRequest(id, Script.DEFAULT_TEMPLATE_LANG, content); + return channel -> client.admin().cluster().putStoredScript(put, new AcknowledgedRestListener<>(channel)); } } diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TemplateQueryBuilder.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TemplateQueryBuilder.java index 8bbcf09990a..66a5c09977b 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TemplateQueryBuilder.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TemplateQueryBuilder.java @@ -60,8 +60,9 @@ public class TemplateQueryBuilder extends AbstractQueryBuilder params, XContentType ct) { - this(new Script(scriptType, "mustache", template, - ct == null ? Collections.emptyMap() : Collections.singletonMap(Script.CONTENT_TYPE_OPTION, ct.mediaType()), params)); + this(new Script(scriptType, "mustache", template, scriptType == ScriptType.INLINE ? + (ct == null ? Collections.emptyMap() : Collections.singletonMap(Script.CONTENT_TYPE_OPTION, ct.mediaType())) + : null, params)); } TemplateQueryBuilder(Script template) { @@ -138,6 +139,13 @@ public class TemplateQueryBuilder extends AbstractQueryBuilder new SearchTemplateRequestBuilder(client()) .setRequest(new SearchRequest("test").types("type")) .setScript("/template_index/mustache/1000").setScriptType(ScriptType.STORED).setScriptParams(templateParams) .get()); - assertThat(e.getMessage(), containsString("Illegal index script format [/template_index/mustache/1000] should be /lang/id")); + assertThat(e.getMessage(), containsString("illegal stored script format [/template_index/mustache/1000] use only ")); } public void testIndexedTemplate() throws Exception { assertAcked(client().admin().cluster().preparePutStoredScript() - .setScriptLang(MustacheScriptEngineService.NAME) + .setLang(MustacheScriptEngineService.NAME) .setId("1a") - .setSource(new BytesArray("{" + + .setContent(new BytesArray("{" + "\"template\":{" + " \"query\":{" + " \"match\":{" + @@ -224,9 +224,9 @@ public class SearchTemplateIT extends ESSingleNodeTestCase { )) ); assertAcked(client().admin().cluster().preparePutStoredScript() - .setScriptLang(MustacheScriptEngineService.NAME) + .setLang(MustacheScriptEngineService.NAME) .setId("2") - .setSource(new BytesArray("{" + + .setContent(new BytesArray("{" + "\"template\":{" + " \"query\":{" + " \"match\":{" + @@ -236,9 +236,9 @@ public class SearchTemplateIT extends ESSingleNodeTestCase { "}")) ); assertAcked(client().admin().cluster().preparePutStoredScript() - .setScriptLang(MustacheScriptEngineService.NAME) + .setLang(MustacheScriptEngineService.NAME) .setId("3") - .setSource(new BytesArray("{" + + .setContent(new BytesArray("{" + "\"template\":{" + " \"match\":{" + " \"theField\" : \"{{fieldParam}}\"}" + @@ -260,7 +260,7 @@ public class SearchTemplateIT extends ESSingleNodeTestCase { SearchTemplateResponse searchResponse = new SearchTemplateRequestBuilder(client()) .setRequest(new SearchRequest().indices("test").types("type")) - .setScript("/mustache/1a") + .setScript("1a") .setScriptType(ScriptType.STORED) .setScriptParams(templateParams) .get(); @@ -286,6 +286,8 @@ public class SearchTemplateIT extends ESSingleNodeTestCase { .setScript("/mustache/2").setScriptType(ScriptType.STORED).setScriptParams(templateParams) .get(); assertHitCount(searchResponse.getResponse(), 1); + assertWarnings("use of [/mustache/2] for looking up" + + " stored scripts/templates has been deprecated, use only [2] instead"); Map vars = new HashMap<>(); vars.put("fieldParam", "bar"); @@ -307,45 +309,51 @@ public class SearchTemplateIT extends ESSingleNodeTestCase { .get(); client().admin().indices().prepareRefresh().get(); - assertAcked(client().admin().cluster().preparePutStoredScript() - .setScriptLang(MustacheScriptEngineService.NAME) - .setId("git01") - .setSource(new BytesArray("{\"template\":{\"query\": {\"match_phrase_prefix\": " + - "{\"searchtext\": {\"query\": \"{{P_Keyword1}}\"," + - "\"unsupported\": \"unsupported\"}}}}}"))); + int iterations = randomIntBetween(2, 11); + for (int i = 1; i < iterations; i++) { + assertAcked(client().admin().cluster().preparePutStoredScript() + .setLang(MustacheScriptEngineService.NAME) + .setId("git01") + .setContent(new BytesArray("{\"template\":{\"query\": {\"match\": {\"searchtext\": {\"query\": \"{{P_Keyword1}}\"," + + "\"type\": \"ooophrase_prefix\"}}}}}"))); - GetStoredScriptResponse getResponse = client().admin().cluster() - .prepareGetStoredScript(MustacheScriptEngineService.NAME, "git01").get(); - assertNotNull(getResponse.getStoredScript()); + GetStoredScriptResponse getResponse = client().admin().cluster() + .prepareGetStoredScript(MustacheScriptEngineService.NAME, "git01").get(); + assertNotNull(getResponse.getSource()); - Map templateParams = new HashMap<>(); - templateParams.put("P_Keyword1", "dev"); + Map templateParams = new HashMap<>(); + templateParams.put("P_Keyword1", "dev"); - ParsingException e = expectThrows(ParsingException.class, () -> new SearchTemplateRequestBuilder(client()) - .setRequest(new SearchRequest("testindex").types("test")) - .setScript("git01").setScriptType(ScriptType.STORED).setScriptParams(templateParams) - .get()); - assertThat(e.getMessage(), containsString("[match_phrase_prefix] query does not support [unsupported]")); + ParsingException e = expectThrows(ParsingException.class, () -> new SearchTemplateRequestBuilder(client()) + .setRequest(new SearchRequest("testindex").types("test")) + .setScript("git01").setScriptType(ScriptType.STORED).setScriptParams(templateParams) + .get()); + assertThat(e.getMessage(), containsString("[match] query does not support type ooophrase_prefix")); + assertWarnings("Deprecated field [type] used, replaced by [match_phrase and match_phrase_prefix query]"); - assertAcked(client().admin().cluster().preparePutStoredScript() - .setScriptLang(MustacheScriptEngineService.NAME) - .setId("git01") - .setSource(new BytesArray("{\"query\": {\"match_phrase_prefix\": {\"searchtext\": {\"query\": \"{{P_Keyword1}}\"}}}}"))); + assertAcked(client().admin().cluster().preparePutStoredScript() + .setLang(MustacheScriptEngineService.NAME) + .setId("git01") + .setContent(new BytesArray("{\"query\": {\"match\": {\"searchtext\": {\"query\": \"{{P_Keyword1}}\"," + + "\"type\": \"phrase_prefix\"}}}}"))); - SearchTemplateResponse searchResponse = new SearchTemplateRequestBuilder(client()) - .setRequest(new SearchRequest("testindex").types("test")) - .setScript("git01").setScriptType(ScriptType.STORED).setScriptParams(templateParams) - .get(); - assertHitCount(searchResponse.getResponse(), 1); + SearchTemplateResponse searchResponse = new SearchTemplateRequestBuilder(client()) + .setRequest(new SearchRequest("testindex").types("test")) + .setScript("git01").setScriptType(ScriptType.STORED).setScriptParams(templateParams) + .get(); + assertHitCount(searchResponse.getResponse(), 1); + assertWarnings("Deprecated field [type] used, replaced by [match_phrase and match_phrase_prefix query]"); + + } } public void testIndexedTemplateWithArray() throws Exception { String multiQuery = "{\"query\":{\"terms\":{\"theField\":[\"{{#fieldParam}}\",\"{{.}}\",\"{{/fieldParam}}\"]}}}"; assertAcked( client().admin().cluster().preparePutStoredScript() - .setScriptLang(MustacheScriptEngineService.NAME) + .setLang(MustacheScriptEngineService.NAME) .setId("4") - .setSource(jsonBuilder().startObject().field("template", multiQuery).endObject().bytes()) + .setContent(jsonBuilder().startObject().field("template", multiQuery).endObject().bytes()) ); BulkRequestBuilder bulkRequestBuilder = client().prepareBulk(); bulkRequestBuilder.add(client().prepareIndex("test", "type", "1").setSource("{\"theField\":\"foo\"}")); @@ -362,7 +370,7 @@ public class SearchTemplateIT extends ESSingleNodeTestCase { SearchTemplateResponse searchResponse = new SearchTemplateRequestBuilder(client()) .setRequest(new SearchRequest("test").types("type")) - .setScript("/mustache/4").setScriptType(ScriptType.STORED).setScriptParams(arrayTemplateParams) + .setScript("4").setScriptType(ScriptType.STORED).setScriptParams(arrayTemplateParams) .get(); assertHitCount(searchResponse.getResponse(), 5); } diff --git a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/10_basic.yaml b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/10_basic.yaml index 9533cde81ac..644319d50ec 100644 --- a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/10_basic.yaml +++ b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/10_basic.yaml @@ -54,7 +54,7 @@ body: { "template": { "query": { "match{{}}_all": {}}, "size": "{{my_size}}" } } - do: - catch: /Unable\sto\sparse.*/ + catch: /failed\sto\sparse.*/ put_template: id: "1" body: { "template": { "query": { "match{{}}_all": {}}, "size": "{{my_size}}" } } diff --git a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/20_render_search_template.yaml b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/20_render_search_template.yaml index 9a9a1b0c017..f6fc458a08e 100644 --- a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/20_render_search_template.yaml +++ b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/20_render_search_template.yaml @@ -150,7 +150,7 @@ - match: { hits.total: 1 } - do: - catch: /Unable.to.find.on.disk.file.script.\[simple1\].using.lang.\[mustache\]/ + catch: /unable.to.find.file.script.\[simple1\].using.lang.\[mustache\]/ search_template: body: { "file" : "simple1"} diff --git a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/40_template_query.yaml b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/40_template_query.yaml index cfa97b8bc9f..a21184650f5 100644 --- a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/40_template_query.yaml +++ b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/40_template_query.yaml @@ -52,7 +52,7 @@ warnings: - '[template] query is deprecated, use search template api instead' search: - body: { "query": { "template": { "stored": "/mustache/1", "params": { "my_value": "value1" } } } } + body: { "query": { "template": { "stored": "1", "params": { "my_value": "value1" } } } } - match: { hits.total: 1 } diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/16_update2.yaml b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/16_update2.yaml index 524f203b2fc..eb7cb01da33 100644 --- a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/16_update2.yaml +++ b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/16_update2.yaml @@ -1,54 +1,47 @@ --- -"Indexed script": +"Stored script": + - skip: + features: warnings - do: put_script: - id: "1" - lang: "painless" - body: { "script": "_score * doc['myParent.weight'].value" } + lang: "1" + body: { "script": {"lang": "painless", "code": "_score * doc['myParent.weight'].value" } } - match: { acknowledged: true } - do: get_script: - id: "1" - lang: "painless" + lang: "1" - match: { found: true } - - match: { lang: painless } - match: { _id: "1" } - - match: { "script": "_score * doc['myParent.weight'].value" } + - match: { "script": {"lang": "painless", "code": "_score * doc['myParent.weight'].value"} } - do: catch: missing get_script: - id: "2" - lang: "painless" + lang: "2" - match: { found: false } - - match: { lang: painless } - match: { _id: "2" } - is_false: script - do: delete_script: - id: "1" - lang: "painless" + lang: "1" - match: { acknowledged: true } - do: catch: missing delete_script: - id: "non_existing" - lang: "painless" + lang: "non_existing" - do: catch: request put_script: - id: "1" - lang: "painless" - body: { "script": "_score * foo bar + doc['myParent.weight'].value" } + lang: "1" + body: { "script": {"lang": "painless", "code": "_score * foo bar + doc['myParent.weight'].value"} } - do: catch: /compile error/ put_script: - id: "1" - lang: "painless" - body: { "script": "_score * foo bar + doc['myParent.weight'].value" } + lang: "1" + body: { "script": {"lang": "painless", "code": "_score * foo bar + doc['myParent.weight'].value"} } diff --git a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yaml b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yaml index f9475057bc4..36083de1de2 100644 --- a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yaml +++ b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/10_basic.yaml @@ -70,3 +70,97 @@ ingest.get_pipeline: id: "my_pipeline" - match: { my_pipeline.description: "_description" } + +--- +"Test rolling upgrade for stored scripts between the old namespace and the new namespace": + - skip: + features: warnings + + - do: + cluster.health: + wait_for_status: green + wait_for_nodes: 2 + + - do: + search: + index: stored_index + body: { + "query": { + "match_all": { + } + } + } + - match: { hits.total: 3 } + + - do: + warnings: + - 'specifying lang [painless] as part of the url path is deprecated' + get_script: + id: "greater" + lang: "painless" + - match: { "found": true } + - match: { "_id": "greater" } + - match: { "lang": "painless"} + - match: { "script": "doc['num'].value > 1.0" } + + - do: + warnings: + - 'specifying lang [painless] as part of the url path is deprecated' + get_script: + id: "value" + lang: "painless" + - match: { "found": true } + - match: { "_id": "value" } + - match: { "lang": "painless"} + - match: { "script": "doc['num'].value" } + + - do: + warnings: + - 'specifying lang [expression] as part of the url path is deprecated' + get_script: + id: "value" + lang: "expression" + - match: { "found": true } + - match: { "_id": "value" } + - match: { "lang": "expression"} + - match: { "script": "doc['num'].value" } + + - do: + warnings: + - 'specifying the field [lang] for executing stored scripts is deprecated; use only the field [stored] to specify an ' + search: + index: stored_index + body: { + "query": { + "script": { + "script": { + "stored": "greater", + "lang": "painless" + } + } + }, + "script_fields": { + "script_painless": { + "script": { + "stored": "value", + "lang": "painless" + } + }, + "script_expressions": { + "script": { + "stored": "value", + "lang": "expression" + } + } + }, + "sort": { + "num": { + "order": "asc" + } + } + } + - match: { hits.total: 2 } + - match: { hits.hits.0.fields.script_painless.0: 2.0 } + - match: { hits.hits.1.fields.script_painless.0: 3.0 } + - match: { hits.hits.0.fields.script_expressions.0: 2.0 } + - match: { hits.hits.1.fields.script_expressions.0: 3.0 } diff --git a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yaml b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yaml index 98627e03419..1f014f22329 100644 --- a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yaml +++ b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/10_basic.yaml @@ -54,3 +54,123 @@ ] } - match: { "acknowledged": true } + +--- +"Test rolling upgrade for stored scripts between the old namespace and the new namespace": + - skip: + features: warnings + + - do: + indices.create: + index: stored_index + body: + settings: + index: + number_of_replicas: 0 + + - do: + bulk: + refresh: true + body: + - '{"index": {"_index": "stored_index", "_type": "test"}}' + - '{"value": "value1", "num": 1.0}' + - '{"index": {"_index": "stored_index", "_type": "test"}}' + - '{"value": "value2", "num": 2.0}' + - '{"index": {"_index": "stored_index", "_type": "test"}}' + - '{"value": "value3", "num": 3.0}' + + - do: + indices.flush: + index: stored_index + + - do: + put_script: + id: "greater" + lang: "painless" + body: { + "script": "doc['num'].value > 1.0" + } + - match: { acknowledged: true } + + - do: + put_script: + id: "value" + lang: "painless" + body: { + "script": "doc['num'].value" + } + - match: { acknowledged: true } + + - do: + put_script: + id: "value" + lang: "expression" + body: { + "script": "doc['num'].value" + } + - match: { acknowledged: true } + + - do: + get_script: + id: "greater" + lang: "painless" + - match: { "found": true } + - match: { "_id": "greater" } + - match: { "lang": "painless"} + - match: { "script": "doc['num'].value > 1.0" } + + - do: + get_script: + id: "value" + lang: "painless" + - match: { "found": true } + - match: { "_id": "value" } + - match: { "lang": "painless"} + - match: { "script": "doc['num'].value" } + + - do: + get_script: + id: "value" + lang: "expression" + - match: { "found": true } + - match: { "_id": "value" } + - match: { "lang": "expression"} + - match: { "script": "doc['num'].value" } + + - do: + search: + index: stored_index + body: { + "query": { + "script": { + "script": { + "stored": "greater", + "lang": "painless" + } + } + }, + "script_fields": { + "script_painless": { + "script": { + "stored": "value", + "lang": "painless" + } + }, + "script_expressions": { + "script": { + "stored": "value", + "lang": "expression" + } + } + }, + "sort": { + "num": { + "order": "asc" + } + } + } + - match: { hits.total: 2 } + - match: { hits.hits.0.fields.script_painless.0: 2.0 } + - match: { hits.hits.1.fields.script_painless.0: 3.0 } + - match: { hits.hits.0.fields.script_expressions.0: 2.0 } + - match: { hits.hits.1.fields.script_expressions.0: 3.0 } diff --git a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yaml b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yaml index dc1f9fc1bbf..c673ca39805 100644 --- a/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yaml +++ b/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/10_basic.yaml @@ -47,3 +47,427 @@ ingest.get_pipeline: id: "my_pipeline" - match: { my_pipeline.description: "_description" } + +--- +"Test rolling upgrade for stored scripts between the old namespace and the new namespace": + - skip: + features: warnings + + - do: + cluster.health: + wait_for_status: green + wait_for_nodes: 2 + + - do: + search: + index: stored_index + body: { + "query": { + "match_all": { + } + } + } + - match: { hits.total: 3 } + + - do: + warnings: + - 'specifying lang [painless] as part of the url path is deprecated' + get_script: + id: "greater" + lang: "painless" + - match: { "found": true } + - match: { "_id": "greater" } + - match: { "lang": "painless"} + - match: { "script": "doc['num'].value > 1.0" } + + - do: + warnings: + - 'specifying lang [painless] as part of the url path is deprecated' + get_script: + id: "value" + lang: "painless" + - match: { "found": true } + - match: { "_id": "value" } + - match: { "lang": "painless"} + - match: { "script": "doc['num'].value" } + + - do: + warnings: + - 'specifying lang [expression] as part of the url path is deprecated' + get_script: + id: "value" + lang: "expression" + - match: { "found": true } + - match: { "_id": "value" } + - match: { "lang": "expression"} + - match: { "script": "doc['num'].value" } + + - do: + warnings: + - 'specifying the field [lang] for executing stored scripts is deprecated; use only the field [stored] to specify an ' + search: + index: stored_index + body: { + "query": { + "script": { + "script": { + "stored": "greater", + "lang": "painless" + } + } + }, + "script_fields": { + "script_painless": { + "script": { + "stored": "value", + "lang": "painless" + } + }, + "script_expressions": { + "script": { + "stored": "value", + "lang": "expression" + } + } + }, + "sort": { + "num": { + "order": "asc" + } + } + } + - match: { hits.total: 2 } + - match: { hits.hits.0.fields.script_painless.0: 2.0 } + - match: { hits.hits.1.fields.script_painless.0: 3.0 } + - match: { hits.hits.0.fields.script_expressions.0: 2.0 } + - match: { hits.hits.1.fields.script_expressions.0: 3.0 } + + - do: + warnings: + - 'specifying lang [painless] as part of the url path is deprecated, use request content instead' + put_script: + id: "greater" + lang: "painless" + body: { + "script": "doc['num'].value > 1.0" + } + - match: { acknowledged: true } + + - do: + warnings: + - 'specifying lang [painless] as part of the url path is deprecated, use request content instead' + put_script: + id: "value" + lang: "painless" + body: { + "script": "doc['num'].value" + } + - match: { acknowledged: true } + + - do: + warnings: + - 'specifying lang [expression] as part of the url path is deprecated, use request content instead' + - 'stored script [value] already exists using a different lang [painless], the new namespace for stored scripts will only use (id) instead of (lang, id)' + put_script: + id: "value" + lang: "expression" + body: { + "script": "doc['num'].value" + } + - match: { acknowledged: true } + + - do: + warnings: + - 'specifying lang [painless] as part of the url path is deprecated' + get_script: + id: "greater" + lang: "painless" + - match: { "found": true } + - match: { "_id": "greater" } + - match: { "lang": "painless"} + - match: { "script": "doc['num'].value > 1.0" } + + - do: + warnings: + - 'specifying lang [painless] as part of the url path is deprecated' + get_script: + id: "value" + lang: "painless" + - match: { "found": true } + - match: { "_id": "value" } + - match: { "lang": "painless"} + - match: { "script": "doc['num'].value" } + + - do: + warnings: + - 'specifying lang [expression] as part of the url path is deprecated' + get_script: + id: "value" + lang: "expression" + - match: { "found": true } + - match: { "_id": "value" } + - match: { "lang": "expression"} + - match: { "script": "doc['num'].value" } + + - do: + warnings: + - 'specifying the field [lang] for executing stored scripts is deprecated; use only the field [stored] to specify an ' + search: + index: stored_index + body: { + "query": { + "script": { + "script": { + "stored": "greater", + "lang": "painless" + } + } + }, + "script_fields": { + "script_painless": { + "script": { + "stored": "value" + } + }, + "script_expressions": { + "script": { + "stored": "value", + "lang": "expression" + } + } + }, + "sort": { + "num": { + "order": "asc" + } + } + } + - match: { hits.total: 2 } + - match: { hits.hits.0.fields.script_painless.0: 2.0 } + - match: { hits.hits.1.fields.script_painless.0: 3.0 } + - match: { hits.hits.0.fields.script_expressions.0: 2.0 } + - match: { hits.hits.1.fields.script_expressions.0: 3.0 } + + - do: + warnings: + - 'specifying lang [painless] as part of the url path is deprecated' + delete_script: + id: "value" + lang: "painless" + - match: { acknowledged: true } + + - do: + get_script: + lang: "value" + - match: { found: true } + - match: { _id: "value" } + - match: { script.lang: "expression"} + - match: { script.code: "doc['num'].value" } + + - do: + warnings: + - 'specifying lang [expression] as part of the url path is deprecated' + get_script: + id: "value" + lang: "expression" + - match: { found: true } + - match: { _id: "value" } + - match: { lang: "expression"} + - match: { script: "doc['num'].value" } + + - do: + warnings: + - 'specifying lang [painless] as part of the url path is deprecated' + catch: missing + get_script: + id: "value" + lang: "painless" + - match: { found: false } + - match: { _id: "value" } + + - do: + warnings: + - 'stored script [value] already exists using a different lang [expression], the new namespace for stored scripts will only use (id) instead of (lang, id)' + put_script: + lang: "value" + body: { + "script": { + "code": "doc['num'].value", + "lang": "painless" + } + } + - match: { acknowledged: true } + + - do: + get_script: + lang: "value" + - match: { found: true } + - match: { _id: "value" } + - match: { script.lang: "painless"} + - match: { script.code: "doc['num'].value" } + + - do: + warnings: + - 'specifying lang [expression] as part of the url path is deprecated' + get_script: + id: "value" + lang: "expression" + - match: { found: true } + - match: { _id: "value" } + - match: { lang: "expression"} + - match: { script: "doc['num'].value" } + + - do: + warnings: + - 'specifying lang [painless] as part of the url path is deprecated' + get_script: + id: "value" + lang: "painless" + - match: { found: true } + - match: { _id: "value" } + - match: { lang: "painless"} + - match: { script: "doc['num'].value" } + + - do: + warnings: + - 'stored script [value] already exists using a different lang [painless], the new namespace for stored scripts will only use (id) instead of (lang, id)' + put_script: + lang: "value" + body: { + "script": { + "code": "doc['num'].value", + "lang": "expression" + } + } + - match: { acknowledged: true } + + - do: + get_script: + lang: "value" + - match: { found: true } + - match: { _id: "value" } + - match: { script.lang: "expression"} + - match: { script.code: "doc['num'].value" } + + - do: + warnings: + - 'specifying lang [expression] as part of the url path is deprecated' + get_script: + id: "value" + lang: "expression" + - match: { found: true } + - match: { _id: "value" } + - match: { lang: "expression"} + - match: { script: "doc['num'].value" } + + - do: + warnings: + - 'specifying lang [painless] as part of the url path is deprecated' + get_script: + id: "value" + lang: "painless" + - match: { found: true } + - match: { _id: "value" } + - match: { lang: "painless"} + - match: { script: "doc['num'].value" } + + - do: + delete_script: + lang: "value" + - match: { acknowledged: true } + + - do: + catch: missing + get_script: + lang: "value" + - match: { found: false } + - match: { _id: "value" } + + - do: + warnings: + - 'specifying lang [expression] as part of the url path is deprecated' + catch: missing + get_script: + id: "value" + lang: "expression" + - match: { found: false } + - match: { _id: "value" } + + - do: + warnings: + - 'specifying lang [painless] as part of the url path is deprecated' + get_script: + id: "value" + lang: "painless" + - match: { found: true } + - match: { _id: "value" } + - match: { lang: "painless"} + - match: { script: "doc['num'].value" } + + - do: + put_script: + lang: "value" + body: { + "script": { + "code": "doc['num'].value", + "lang": "painless" + } + } + - match: { acknowledged: true } + + - do: + get_script: + lang: "value" + - match: { found: true } + - match: { _id: "value" } + - match: { script.lang: "painless"} + - match: { script.code: "doc['num'].value" } + + - do: + warnings: + - 'specifying lang [expression] as part of the url path is deprecated' + catch: missing + get_script: + id: "value" + lang: "expression" + - match: { found: false } + - match: { _id: "value" } + + - do: + warnings: + - 'specifying lang [painless] as part of the url path is deprecated' + get_script: + id: "value" + lang: "painless" + - match: { found: true } + - match: { _id: "value" } + - match: { lang: "painless"} + - match: { script: "doc['num'].value" } + + - do: + search: + index: stored_index + body: { + "query": { + "script": { + "script": { + "stored": "greater" + } + } + }, + "script_fields": { + "script_painless": { + "script": { + "stored": "value" + } + } + }, + "sort": { + "num": { + "order": "asc" + } + } + } + - match: { hits.total: 2 } + - match: { hits.hits.0.fields.script_painless.0: 2.0 } + - match: { hits.hits.1.fields.script_painless.0: 3.0 } diff --git a/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/50_script_processor_using_painless.yaml b/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/50_script_processor_using_painless.yaml index c87441472f2..729347ee197 100644 --- a/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/50_script_processor_using_painless.yaml +++ b/qa/smoke-test-ingest-with-all-dependencies/src/test/resources/rest-api-spec/test/ingest/50_script_processor_using_painless.yaml @@ -73,13 +73,18 @@ --- "Test script processor with stored script": + - skip: + features: warnings + - do: put_script: - id: "sum_bytes" - lang: "painless" + lang: "sum_bytes" body: > { - "script" : "ctx.bytes_total = ctx.bytes_in + ctx.bytes_out" + "script": { + "lang": "painless", + "code" : "ctx.bytes_total = ctx.bytes_in + ctx.bytes_out" + } } - match: { acknowledged: true } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/delete_script.json b/rest-api-spec/src/main/resources/rest-api-spec/api/delete_script.json index 2c9742057fe..7be743e8e78 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/delete_script.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/delete_script.json @@ -3,8 +3,8 @@ "documentation": "http://www.elastic.co/guide/en/elasticsearch/reference/master/modules-scripting.html", "methods": ["DELETE"], "url": { - "path": "/_scripts/{lang}/{id}", - "paths": [ "/_scripts/{lang}/{id}" ], + "path": "/_scripts/{lang}", + "paths": [ "/_scripts/{lang}", "/_scripts/{lang}/{id}" ], "parts": { "id": { "type" : "string", diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/get_script.json b/rest-api-spec/src/main/resources/rest-api-spec/api/get_script.json index 163f147aa42..c253c6e1fdb 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/get_script.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/get_script.json @@ -3,8 +3,8 @@ "documentation": "http://www.elastic.co/guide/en/elasticsearch/reference/master/modules-scripting.html", "methods": ["GET"], "url": { - "path": "/_scripts/{lang}/{id}", - "paths": [ "/_scripts/{lang}/{id}" ], + "path": "/_scripts/{lang}", + "paths": [ "/_scripts/{lang}", "/_scripts/{lang}/{id}" ], "parts": { "id": { "type" : "string", diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/put_script.json b/rest-api-spec/src/main/resources/rest-api-spec/api/put_script.json index c5676392625..93fef4c030b 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/put_script.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/put_script.json @@ -3,8 +3,8 @@ "documentation": "http://www.elastic.co/guide/en/elasticsearch/reference/master/modules-scripting.html", "methods": ["PUT", "POST"], "url": { - "path": "/_scripts/{lang}/{id}", - "paths": [ "/_scripts/{lang}/{id}" ], + "path": "/_scripts/{lang}", + "paths": [ "/_scripts/{lang}", "/_scripts/{lang}/{id}" ], "parts": { "id": { "type" : "string",