From c1137b3b78c50e6d148718da5ea488561d3aa483 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Fri, 10 Jul 2015 10:48:19 -0700 Subject: [PATCH] Add script type and script name to error messages Modified ScriptEngineService to pass in a CompiledScript object with newly added name and type member variables. This can in turn be used to give better scripting error messages with the type of script used and the name of the script. Required slight modifications to the caching mechanism. Note that this does not enforce good behavior in that plugins will have to write exceptions that also output the name of the script in order to be effective. There was no way to wrap the script methods in a try/catch block properly further up the chain because many have script-like objects passed back that can be run at a later time. closes #6653 closes #11449 --- .../elasticsearch/script/CompiledScript.java | 36 ++++- .../script/NativeScriptEngineService.java | 10 +- .../script/ScriptEngineService.java | 6 +- .../elasticsearch/script/ScriptService.java | 83 ++++++----- .../ExpressionExecutableScript.java | 23 ++- .../ExpressionScriptEngineService.java | 137 +++++++++--------- .../expression/ExpressionSearchScript.java | 20 ++- .../groovy/GroovyScriptEngineService.java | 32 ++-- .../mustache/MustacheScriptEngineService.java | 55 +++---- .../script/ScriptModesTests.java | 6 +- .../script/ScriptServiceTests.java | 14 +- .../expression/ExpressionScriptTests.java | 11 +- .../mustache/MustacheScriptEngineTest.java | 6 +- .../JavaScriptScriptEngineService.java | 12 +- .../JavaScriptScriptEngineTests.java | 37 +++-- .../JavaScriptScriptMultiThreadedTest.java | 8 +- .../script/javascript/SimpleBench.java | 11 +- .../python/PythonScriptEngineService.java | 12 +- .../python/PythonScriptEngineTests.java | 21 +-- .../python/PythonScriptMultiThreadedTest.java | 8 +- .../script/python/SimpleBench.java | 12 +- 21 files changed, 324 insertions(+), 236 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/script/CompiledScript.java b/core/src/main/java/org/elasticsearch/script/CompiledScript.java index 9e3bfaf3f4c..aa34678c041 100644 --- a/core/src/main/java/org/elasticsearch/script/CompiledScript.java +++ b/core/src/main/java/org/elasticsearch/script/CompiledScript.java @@ -24,17 +24,39 @@ package org.elasticsearch.script; */ public class CompiledScript { + private final ScriptService.ScriptType type; + private final String name; private final String lang; private final Object compiled; /** * Constructor for CompiledScript. + * @param type The type of script to be executed. + * @param name The name of the script to be executed. * @param lang The language of the script to be executed. * @param compiled The compiled script Object that is executable. */ - public CompiledScript(String lang, Object compiled) { - this.lang = lang; - this.compiled = compiled; + public CompiledScript(ScriptService.ScriptType type, String name, String lang, Object compiled) { + this.type = type; + this.name = name; + this.lang = lang; + this.compiled = compiled; + } + + /** + * Method to get the type of language. + * @return The type of language the script was compiled in. + */ + public ScriptService.ScriptType type() { + return type; + } + + /** + * Method to get the name of the script. + * @return The name of the script to be executed. + */ + public String name() { + return name; } /** @@ -52,4 +74,12 @@ public class CompiledScript { public Object compiled() { return compiled; } + + /** + * @return A string composed of type, lang, and name to describe the CompiledScript. + */ + @Override + public String toString() { + return type + " script [" + name + "] using lang [" + lang + "]"; + } } diff --git a/core/src/main/java/org/elasticsearch/script/NativeScriptEngineService.java b/core/src/main/java/org/elasticsearch/script/NativeScriptEngineService.java index 70bf27b82e4..b46bc7328d0 100644 --- a/core/src/main/java/org/elasticsearch/script/NativeScriptEngineService.java +++ b/core/src/main/java/org/elasticsearch/script/NativeScriptEngineService.java @@ -71,14 +71,14 @@ public class NativeScriptEngineService extends AbstractComponent implements Scri } @Override - public ExecutableScript executable(Object compiledScript, @Nullable Map vars) { - NativeScriptFactory scriptFactory = (NativeScriptFactory) compiledScript; + public ExecutableScript executable(CompiledScript compiledScript, @Nullable Map vars) { + NativeScriptFactory scriptFactory = (NativeScriptFactory) compiledScript.compiled(); return scriptFactory.newScript(vars); } @Override - public SearchScript search(Object compiledScript, final SearchLookup lookup, @Nullable final Map vars) { - final NativeScriptFactory scriptFactory = (NativeScriptFactory) compiledScript; + public SearchScript search(CompiledScript compiledScript, final SearchLookup lookup, @Nullable final Map vars) { + final NativeScriptFactory scriptFactory = (NativeScriptFactory) compiledScript.compiled(); return new SearchScript() { @Override public LeafSearchScript getLeafSearchScript(LeafReaderContext context) throws IOException { @@ -90,7 +90,7 @@ public class NativeScriptEngineService extends AbstractComponent implements Scri } @Override - public Object execute(Object compiledScript, Map vars) { + public Object execute(CompiledScript compiledScript, Map vars) { return executable(compiledScript, vars).run(); } diff --git a/core/src/main/java/org/elasticsearch/script/ScriptEngineService.java b/core/src/main/java/org/elasticsearch/script/ScriptEngineService.java index 7b78427ebc3..966085754c0 100644 --- a/core/src/main/java/org/elasticsearch/script/ScriptEngineService.java +++ b/core/src/main/java/org/elasticsearch/script/ScriptEngineService.java @@ -38,11 +38,11 @@ public interface ScriptEngineService extends Closeable { Object compile(String script); - ExecutableScript executable(Object compiledScript, @Nullable Map vars); + ExecutableScript executable(CompiledScript compiledScript, @Nullable Map vars); - SearchScript search(Object compiledScript, SearchLookup lookup, @Nullable Map vars); + SearchScript search(CompiledScript compiledScript, SearchLookup lookup, @Nullable Map vars); - Object execute(Object compiledScript, Map vars); + Object execute(CompiledScript compiledScript, Map vars); Object unwrap(Object value); diff --git a/core/src/main/java/org/elasticsearch/script/ScriptService.java b/core/src/main/java/org/elasticsearch/script/ScriptService.java index b41be3250c4..e683e5d66fd 100644 --- a/core/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/core/src/main/java/org/elasticsearch/script/ScriptService.java @@ -249,50 +249,67 @@ public class ScriptService extends AbstractComponent implements Closeable { } /** - * Compiles a script straight-away, or returns the previously compiled and cached script, without checking if it can be executed based on settings. + * Compiles a script straight-away, or returns the previously compiled and cached script, + * without checking if it can be executed based on settings. */ public CompiledScript compileInternal(Script script) { if (script == null) { throw new IllegalArgumentException("The parameter script (Script) must not be null."); } - String lang = script.getLang(); + String lang = script.getLang() == null ? defaultLang : script.getLang(); + ScriptType type = script.getType(); + //script.getScript() 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.getScript(); - if (lang == null) { - lang = defaultLang; - } if (logger.isTraceEnabled()) { - logger.trace("Compiling lang: [{}] type: [{}] script: {}", lang, script.getType(), script.getScript()); + logger.trace("Compiling lang: [{}] type: [{}] script: {}", lang, type, name); } ScriptEngineService scriptEngineService = getScriptEngineServiceForLang(lang); - String cacheKey = getCacheKey(scriptEngineService, script.getScript()); - if (script.getType() == ScriptType.FILE) { - CompiledScript compiled = staticCache.get(cacheKey); //On disk scripts will be loaded into the staticCache by the listener - if (compiled == null) { - throw new IllegalArgumentException("Unable to find on disk script " + script.getScript()); + if (type == ScriptType.FILE) { + String cacheKey = getCacheKey(scriptEngineService, name, null); + //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 compiled; + + return compiledScript; } + //script.getScript() will be code if the script type is inline String code = script.getScript(); - if (script.getType() == ScriptType.INDEXED) { - final IndexedScript indexedScript = new IndexedScript(lang, script.getScript()); + if (type == ScriptType.INDEXED) { + //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 = getScriptFromIndex(indexedScript.lang, indexedScript.id); - cacheKey = getCacheKey(scriptEngineService, code); } - CompiledScript compiled = cache.getIfPresent(cacheKey); - if (compiled == null) { - //Either an un-cached inline script or an indexed script - compiled = new CompiledScript(lang, scriptEngineService.compile(code)); + String cacheKey = getCacheKey(scriptEngineService, type == ScriptType.INLINE ? null : name, code); + CompiledScript compiledScript = cache.getIfPresent(cacheKey); + + if (compiledScript == null) { + //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 + try { + compiledScript = new CompiledScript(type, name, lang, scriptEngineService.compile(code)); + } catch (Exception exception) { + throw new ScriptException("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. - cache.put(cacheKey, compiled); + cache.put(cacheKey, compiledScript); } - return compiled; + + return compiledScript; } public void queryScriptIndex(GetIndexedScriptRequest request, final ActionListener listener) { @@ -334,13 +351,13 @@ public class ScriptService extends AbstractComponent implements Closeable { Template template = TemplateQueryParser.parse(scriptLang, parser, parseFieldMatcher, "params", "script", "template"); if (Strings.hasLength(template.getScript())) { //Just try and compile it - //This will have the benefit of also adding the script to the cache if it compiles 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 and cache it. - if (isAnyScriptContextEnabled(scriptLang, getScriptEngineServiceForLang(scriptLang), ScriptType.INDEXED)) { - CompiledScript compiledScript = compileInternal(template); - if (compiledScript == null) { + //indexed scripts are disabled, it makes no sense to even compile it. + if (isAnyScriptContextEnabled(scriptLang, scriptEngineService, ScriptType.INDEXED)) { + Object compiled = scriptEngineService.compile(template.getScript()); + if (compiled == null) { throw new IllegalArgumentException("Unable to parse [" + template.getScript() + "] lang [" + scriptLang + "] (ScriptService.compile returned null)"); } @@ -419,7 +436,7 @@ public class ScriptService extends AbstractComponent implements Closeable { * Executes a previously compiled script provided as an argument */ public ExecutableScript executable(CompiledScript compiledScript, Map vars) { - return getScriptEngineServiceForLang(compiledScript.lang()).executable(compiledScript.compiled(), vars); + return getScriptEngineServiceForLang(compiledScript.lang()).executable(compiledScript, vars); } /** @@ -427,7 +444,7 @@ public class ScriptService extends AbstractComponent implements Closeable { */ public SearchScript search(SearchLookup lookup, Script script, ScriptContext scriptContext) { CompiledScript compiledScript = compile(script, scriptContext); - return getScriptEngineServiceForLang(compiledScript.lang()).search(compiledScript.compiled(), lookup, script.getParams()); + return getScriptEngineServiceForLang(compiledScript.lang()).search(compiledScript, lookup, script.getParams()); } private boolean isAnyScriptContextEnabled(String lang, ScriptEngineService scriptEngineService, ScriptType scriptType) { @@ -513,8 +530,8 @@ public class ScriptService extends AbstractComponent implements Closeable { logger.info("compiling script file [{}]", file.toAbsolutePath()); try(InputStreamReader reader = new InputStreamReader(Files.newInputStream(file), Charsets.UTF_8)) { String script = Streams.copyToString(reader); - String cacheKey = getCacheKey(engineService, scriptNameExt.v1()); - staticCache.put(cacheKey, new CompiledScript(engineService.types()[0], engineService.compile(script))); + String cacheKey = getCacheKey(engineService, scriptNameExt.v1(), null); + staticCache.put(cacheKey, new CompiledScript(ScriptType.FILE, scriptNameExt.v1(), engineService.types()[0], engineService.compile(script))); } } else { logger.warn("skipping compile of script file [{}] as all scripted operations are disabled for file scripts", file.toAbsolutePath()); @@ -538,7 +555,7 @@ public class ScriptService extends AbstractComponent implements Closeable { ScriptEngineService engineService = getScriptEngineServiceForFileExt(scriptNameExt.v2()); assert engineService != null; logger.info("removing script file [{}]", file.toAbsolutePath()); - staticCache.remove(getCacheKey(engineService, scriptNameExt.v1())); + staticCache.remove(getCacheKey(engineService, scriptNameExt.v1(), null)); } } @@ -598,9 +615,9 @@ public class ScriptService extends AbstractComponent implements Closeable { } } - private static String getCacheKey(ScriptEngineService scriptEngineService, String script) { + private static String getCacheKey(ScriptEngineService scriptEngineService, String name, String code) { String lang = scriptEngineService.types()[0]; - return lang + ":" + script; + return lang + ":" + (name != null ? ":" + name : "") + (code != null ? ":" + code : ""); } private static class IndexedScript { diff --git a/core/src/main/java/org/elasticsearch/script/expression/ExpressionExecutableScript.java b/core/src/main/java/org/elasticsearch/script/expression/ExpressionExecutableScript.java index ff41fb8fd78..d4ae9e14140 100644 --- a/core/src/main/java/org/elasticsearch/script/expression/ExpressionExecutableScript.java +++ b/core/src/main/java/org/elasticsearch/script/expression/ExpressionExecutableScript.java @@ -20,6 +20,7 @@ package org.elasticsearch.script.expression; import org.apache.lucene.expressions.Expression; +import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.ScriptException; @@ -34,16 +35,18 @@ public class ExpressionExecutableScript implements ExecutableScript { private final int NO_DOCUMENT = -1; - public final Expression expression; + public final CompiledScript compiledScript; public final Map functionValuesMap; public final ReplaceableConstFunctionValues[] functionValuesArray; - public ExpressionExecutableScript(Object compiledScript, Map vars) { - expression = (Expression)compiledScript; + public ExpressionExecutableScript(CompiledScript compiledScript, Map vars) { + this.compiledScript = compiledScript; + Expression expression = (Expression)this.compiledScript.compiled(); int functionValuesLength = expression.variables.length; if (vars.size() != functionValuesLength) { - throw new ScriptException("The number of variables in an executable expression script [" + + throw new ScriptException("Error using " + compiledScript + ". " + + "The number of variables in an executable expression script [" + functionValuesLength + "] must match the number of variables in the variable map" + " [" + vars.size() + "]."); } @@ -69,17 +72,23 @@ public class ExpressionExecutableScript implements ExecutableScript { double doubleValue = ((Number)value).doubleValue(); functionValuesMap.get(name).setValue(doubleValue); } else { - throw new ScriptException("Executable expressions scripts can only process numbers." + + throw new ScriptException("Error using " + compiledScript + ". " + + "Executable expressions scripts can only process numbers." + " The variable [" + name + "] is not a number."); } } else { - throw new ScriptException("The variable [" + name + "] does not exist in the executable expressions script."); + throw new ScriptException("Error using " + compiledScript + ". " + + "The variable [" + name + "] does not exist in the executable expressions script."); } } @Override public Object run() { - return expression.evaluate(NO_DOCUMENT, functionValuesArray); + try { + return ((Expression) compiledScript.compiled()).evaluate(NO_DOCUMENT, functionValuesArray); + } catch (Exception exception) { + throw new ScriptException("Error evaluating " + compiledScript, exception); + } } @Override diff --git a/core/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngineService.java b/core/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngineService.java index 2c4c8f4a25b..d9dd34c896f 100644 --- a/core/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngineService.java +++ b/core/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngineService.java @@ -38,6 +38,7 @@ import org.elasticsearch.index.mapper.core.NumberFieldMapper; import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.ScriptEngineService; +import org.elasticsearch.script.ScriptException; import org.elasticsearch.script.SearchScript; import org.elasticsearch.search.MultiValueMode; import org.elasticsearch.search.lookup.SearchLookup; @@ -99,79 +100,83 @@ public class ExpressionScriptEngineService extends AbstractComponent implements } @Override - public SearchScript search(Object compiledScript, SearchLookup lookup, @Nullable Map vars) { - Expression expr = (Expression)compiledScript; - MapperService mapper = lookup.doc().mapperService(); - // NOTE: if we need to do anything complicated with bindings in the future, we can just extend Bindings, - // instead of complicating SimpleBindings (which should stay simple) - SimpleBindings bindings = new SimpleBindings(); - ReplaceableConstValueSource specialValue = null; + public SearchScript search(CompiledScript compiledScript, SearchLookup lookup, @Nullable Map vars) { + try { + Expression expr = (Expression)compiledScript.compiled(); + MapperService mapper = lookup.doc().mapperService(); + // NOTE: if we need to do anything complicated with bindings in the future, we can just extend Bindings, + // instead of complicating SimpleBindings (which should stay simple) + SimpleBindings bindings = new SimpleBindings(); + ReplaceableConstValueSource specialValue = null; - for (String variable : expr.variables) { - if (variable.equals("_score")) { - bindings.add(new SortField("_score", SortField.Type.SCORE)); + for (String variable : expr.variables) { + if (variable.equals("_score")) { + bindings.add(new SortField("_score", SortField.Type.SCORE)); - } else if (variable.equals("_value")) { - specialValue = new ReplaceableConstValueSource(); - bindings.add("_value", specialValue); - // noop: _value is special for aggregations, and is handled in ExpressionScriptBindings - // TODO: if some uses it in a scoring expression, they will get a nasty failure when evaluating...need a - // way to know this is for aggregations and so _value is ok to have... + } else if (variable.equals("_value")) { + specialValue = new ReplaceableConstValueSource(); + bindings.add("_value", specialValue); + // noop: _value is special for aggregations, and is handled in ExpressionScriptBindings + // TODO: if some uses it in a scoring expression, they will get a nasty failure when evaluating...need a + // way to know this is for aggregations and so _value is ok to have... + + } else if (vars != null && vars.containsKey(variable)) { + // TODO: document and/or error if vars contains _score? + // NOTE: by checking for the variable in vars first, it allows masking document fields with a global constant, + // but if we were to reverse it, we could provide a way to supply dynamic defaults for documents missing the field? + Object value = vars.get(variable); + if (value instanceof Number) { + bindings.add(variable, new DoubleConstValueSource(((Number) value).doubleValue())); + } else { + throw new ExpressionScriptCompilationException("Parameter [" + variable + "] must be a numeric type"); + } - } else if (vars != null && vars.containsKey(variable)) { - // TODO: document and/or error if vars contains _score? - // NOTE: by checking for the variable in vars first, it allows masking document fields with a global constant, - // but if we were to reverse it, we could provide a way to supply dynamic defaults for documents missing the field? - Object value = vars.get(variable); - if (value instanceof Number) { - bindings.add(variable, new DoubleConstValueSource(((Number)value).doubleValue())); } else { - throw new ExpressionScriptCompilationException("Parameter [" + variable + "] must be a numeric type"); - } + String fieldname = null; + String methodname = null; + VariableContext[] parts = VariableContext.parse(variable); + if (parts[0].text.equals("doc") == false) { + throw new ExpressionScriptCompilationException("Unknown variable [" + parts[0].text + "] in expression"); + } + if (parts.length < 2 || parts[1].type != VariableContext.Type.STR_INDEX) { + throw new ExpressionScriptCompilationException("Variable 'doc' in expression must be used with a specific field like: doc['myfield']"); + } else { + fieldname = parts[1].text; + } + if (parts.length == 3) { + if (parts[2].type == VariableContext.Type.METHOD) { + methodname = parts[2].text; + } else if (parts[2].type != VariableContext.Type.MEMBER || !"value".equals(parts[2].text)) { + throw new ExpressionScriptCompilationException("Only the member variable [value] or member methods may be accessed on a field when not accessing the field directly"); + } + } + if (parts.length > 3) { + throw new ExpressionScriptCompilationException("Variable [" + variable + "] does not follow an allowed format of either doc['field'] or doc['field'].method()"); + } - } else { - String fieldname = null; - String methodname = null; - VariableContext[] parts = VariableContext.parse(variable); - if (parts[0].text.equals("doc") == false) { - throw new ExpressionScriptCompilationException("Unknown variable [" + parts[0].text + "] in expression"); - } - if (parts.length < 2 || parts[1].type != VariableContext.Type.STR_INDEX) { - throw new ExpressionScriptCompilationException("Variable 'doc' in expression must be used with a specific field like: doc['myfield']"); - } else { - fieldname = parts[1].text; - } - if (parts.length == 3) { - if (parts[2].type == VariableContext.Type.METHOD) { - methodname = parts[2].text; - } else if (parts[2].type != VariableContext.Type.MEMBER || !"value".equals(parts[2].text)) { - throw new ExpressionScriptCompilationException("Only the member variable [value] or member methods may be accessed on a field when not accessing the field directly"); + MappedFieldType fieldType = mapper.smartNameFieldType(fieldname); + + if (fieldType == null) { + throw new ExpressionScriptCompilationException("Field [" + fieldname + "] used in expression does not exist in mappings"); + } + if (fieldType.isNumeric() == false) { + // TODO: more context (which expression?) + throw new ExpressionScriptCompilationException("Field [" + fieldname + "] used in expression must be numeric"); + } + + IndexFieldData fieldData = lookup.doc().fieldDataService().getForField((NumberFieldMapper.NumberFieldType) fieldType); + if (methodname == null) { + bindings.add(variable, new FieldDataValueSource(fieldData, MultiValueMode.MIN)); + } else { + bindings.add(variable, getMethodValueSource(fieldType, fieldData, fieldname, methodname)); } } - if (parts.length > 3) { - throw new ExpressionScriptCompilationException("Variable [" + variable + "] does not follow an allowed format of either doc['field'] or doc['field'].method()"); - } - - MappedFieldType fieldType = mapper.smartNameFieldType(fieldname); - - if (fieldType == null) { - throw new ExpressionScriptCompilationException("Field [" + fieldname + "] used in expression does not exist in mappings"); - } - if (fieldType.isNumeric() == false) { - // TODO: more context (which expression?) - throw new ExpressionScriptCompilationException("Field [" + fieldname + "] used in expression must be numeric"); - } - - IndexFieldData fieldData = lookup.doc().fieldDataService().getForField((NumberFieldMapper.NumberFieldType)fieldType); - if (methodname == null) { - bindings.add(variable, new FieldDataValueSource(fieldData, MultiValueMode.MIN)); - } else { - bindings.add(variable, getMethodValueSource(fieldType, fieldData, fieldname, methodname)); - } } - } - return new ExpressionSearchScript((Expression)compiledScript, bindings, specialValue); + return new ExpressionSearchScript(compiledScript, bindings, specialValue); + } catch (Exception exception) { + throw new ScriptException("Error during search with " + compiledScript, exception); + } } protected ValueSource getMethodValueSource(MappedFieldType fieldType, IndexFieldData fieldData, String fieldName, String methodName) { @@ -214,12 +219,12 @@ public class ExpressionScriptEngineService extends AbstractComponent implements } @Override - public ExecutableScript executable(Object compiledScript, Map vars) { + public ExecutableScript executable(CompiledScript compiledScript, Map vars) { return new ExpressionExecutableScript(compiledScript, vars); } @Override - public Object execute(Object compiledScript, Map vars) { + public Object execute(CompiledScript compiledScript, Map vars) { ExpressionExecutableScript expressionExecutableScript = new ExpressionExecutableScript(compiledScript, vars); return expressionExecutableScript.run(); } diff --git a/core/src/main/java/org/elasticsearch/script/expression/ExpressionSearchScript.java b/core/src/main/java/org/elasticsearch/script/expression/ExpressionSearchScript.java index 59a73a3ff6d..948b8d3365f 100644 --- a/core/src/main/java/org/elasticsearch/script/expression/ExpressionSearchScript.java +++ b/core/src/main/java/org/elasticsearch/script/expression/ExpressionSearchScript.java @@ -27,7 +27,9 @@ import org.apache.lucene.queries.function.FunctionValues; import org.apache.lucene.queries.function.ValueSource; import org.apache.lucene.search.Scorer; import org.elasticsearch.common.lucene.Lucene; +import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.LeafSearchScript; +import org.elasticsearch.script.ScriptException; import org.elasticsearch.script.SearchScript; import java.io.IOException; @@ -40,17 +42,17 @@ import java.util.Map; */ class ExpressionSearchScript implements SearchScript { - final Expression expression; + final CompiledScript compiledScript; final SimpleBindings bindings; final ValueSource source; final ReplaceableConstValueSource specialValue; // _value Scorer scorer; int docid; - ExpressionSearchScript(Expression e, SimpleBindings b, ReplaceableConstValueSource v) { - expression = e; + ExpressionSearchScript(CompiledScript c, SimpleBindings b, ReplaceableConstValueSource v) { + compiledScript = c; bindings = b; - source = expression.getValueSource(bindings); + source = ((Expression)compiledScript.compiled()).getValueSource(bindings); specialValue = v; } @@ -61,7 +63,11 @@ class ExpressionSearchScript implements SearchScript { FunctionValues values = source.getValues(Collections.singletonMap("scorer", Lucene.illegalScorer("Scores are not available in the current context")), leaf); double evaluate() { - return values.doubleVal(docid); + try { + return values.doubleVal(docid); + } catch (Exception exception) { + throw new ScriptException("Error evaluating " + compiledScript, exception); + } } @Override @@ -91,7 +97,7 @@ class ExpressionSearchScript implements SearchScript { // We have a new binding for the scorer so we need to reset the values values = source.getValues(Collections.singletonMap("scorer", scorer), leaf); } catch (IOException e) { - throw new IllegalStateException("Can't get values", e); + throw new IllegalStateException("Can't get values using " + compiledScript, e); } } @@ -109,7 +115,7 @@ class ExpressionSearchScript implements SearchScript { if (value instanceof Number) { specialValue.setValue(((Number)value).doubleValue()); } else { - throw new ExpressionScriptExecutionException("Cannot use expression with text variable"); + throw new ExpressionScriptExecutionException("Cannot use expression with text variable using " + compiledScript); } } }; diff --git a/core/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java b/core/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java index 96d63f13cc8..683ffc58f0f 100644 --- a/core/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java +++ b/core/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java @@ -135,21 +135,21 @@ public class GroovyScriptEngineService extends AbstractComponent implements Scri @SuppressWarnings({"unchecked"}) @Override - public ExecutableScript executable(Object compiledScript, Map vars) { + public ExecutableScript executable(CompiledScript compiledScript, Map vars) { try { Map allVars = new HashMap<>(); if (vars != null) { allVars.putAll(vars); } - return new GroovyScript(createScript(compiledScript, allVars), this.logger); + return new GroovyScript(compiledScript, createScript(compiledScript.compiled(), allVars), this.logger); } catch (Exception e) { - throw new ScriptException("failed to build executable script", e); + throw new ScriptException("failed to build executable " + compiledScript, e); } } @SuppressWarnings({"unchecked"}) @Override - public SearchScript search(final Object compiledScript, final SearchLookup lookup, @Nullable final Map vars) { + public SearchScript search(final CompiledScript compiledScript, final SearchLookup lookup, @Nullable final Map vars) { return new SearchScript() { @Override @@ -162,26 +162,26 @@ public class GroovyScriptEngineService extends AbstractComponent implements Scri } Script scriptObject; try { - scriptObject = createScript(compiledScript, allVars); + scriptObject = createScript(compiledScript.compiled(), allVars); } catch (InstantiationException | IllegalAccessException e) { - throw new ScriptException("failed to build search script", e); + throw new ScriptException("failed to build search " + compiledScript, e); } - return new GroovyScript(scriptObject, leafLookup, logger); + return new GroovyScript(compiledScript, scriptObject, leafLookup, logger); } }; } @Override - public Object execute(Object compiledScript, Map vars) { + public Object execute(CompiledScript compiledScript, Map vars) { try { Map allVars = new HashMap<>(); if (vars != null) { allVars.putAll(vars); } - Script scriptObject = createScript(compiledScript, allVars); + Script scriptObject = createScript(compiledScript.compiled(), allVars); return scriptObject.run(); } catch (Exception e) { - throw new ScriptException("failed to execute script", e); + throw new ScriptException("failed to execute " + compiledScript, e); } } @@ -196,17 +196,19 @@ public class GroovyScriptEngineService extends AbstractComponent implements Scri public static final class GroovyScript implements ExecutableScript, LeafSearchScript { + private final CompiledScript compiledScript; private final Script script; private final LeafSearchLookup lookup; private final Map variables; private final ESLogger logger; - public GroovyScript(Script script, ESLogger logger) { - this(script, null, logger); + public GroovyScript(CompiledScript compiledScript, Script script, ESLogger logger) { + this(compiledScript, script, null, logger); } @SuppressWarnings("unchecked") - public GroovyScript(Script script, @Nullable LeafSearchLookup lookup, ESLogger logger) { + public GroovyScript(CompiledScript compiledScript, Script script, @Nullable LeafSearchLookup lookup, ESLogger logger) { + this.compiledScript = compiledScript; this.script = script; this.lookup = lookup; this.logger = logger; @@ -244,9 +246,9 @@ public class GroovyScriptEngineService extends AbstractComponent implements Scri return script.run(); } catch (Throwable e) { if (logger.isTraceEnabled()) { - logger.trace("exception running Groovy script", e); + logger.trace("failed to run " + compiledScript, e); } - throw new GroovyScriptExecutionException(ExceptionsHelper.detailedMessage(e)); + throw new GroovyScriptExecutionException("failed to run " + compiledScript + ": " + ExceptionsHelper.detailedMessage(e)); } } diff --git a/core/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngineService.java b/core/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngineService.java index c40523750bb..b5d4e967af3 100644 --- a/core/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngineService.java +++ b/core/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngineService.java @@ -19,6 +19,7 @@ package org.elasticsearch.script.mustache; import com.github.mustachejava.Mustache; +import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; @@ -29,6 +30,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ExecutableScript; import org.elasticsearch.script.ScriptEngineService; +import org.elasticsearch.script.ScriptException; import org.elasticsearch.script.SearchScript; import org.elasticsearch.search.lookup.SearchLookup; @@ -98,20 +100,13 @@ public class MustacheScriptEngineService extends AbstractComponent implements Sc * @return the processed string with all given variables substitued. * */ @Override - public Object execute(Object template, Map vars) { + public Object execute(CompiledScript template, Map vars) { BytesStreamOutput result = new BytesStreamOutput(); - UTF8StreamWriter writer = utf8StreamWriter().setOutput(result); - ((Mustache) template).execute(writer, vars); - try { - writer.flush(); - } catch (IOException e) { - logger.error("Could not execute query template (failed to flush writer): ", e); - } finally { - try { - writer.close(); - } catch (IOException e) { - logger.error("Could not execute query template (failed to close writer): ", e); - } + try (UTF8StreamWriter writer = utf8StreamWriter().setOutput(result)) { + ((Mustache) template.compiled()).execute(writer, vars); + } catch (Exception e) { + logger.error("Error executing " + template, e); + throw new ScriptException("Error executing " + template, e); } return result.bytes(); } @@ -132,13 +127,13 @@ public class MustacheScriptEngineService extends AbstractComponent implements Sc } @Override - public ExecutableScript executable(Object mustache, + public ExecutableScript executable(CompiledScript compiledScript, @Nullable Map vars) { - return new MustacheExecutableScript((Mustache) mustache, vars); + return new MustacheExecutableScript(compiledScript, vars); } @Override - public SearchScript search(Object compiledScript, SearchLookup lookup, + public SearchScript search(CompiledScript compiledScript, SearchLookup lookup, @Nullable Map vars) { throw new UnsupportedOperationException(); } @@ -162,18 +157,17 @@ public class MustacheScriptEngineService extends AbstractComponent implements Sc * Used at query execution time by script service in order to execute a query template. * */ private class MustacheExecutableScript implements ExecutableScript { - /** Compiled template object. */ - private Mustache mustache; + /** Compiled template object wrapper. */ + private CompiledScript template; /** Parameters to fill above object with. */ private Map vars; /** - * @param mustache the compiled template object + * @param template the compiled template object wrapper * @param vars the parameters to fill above object with **/ - public MustacheExecutableScript(Mustache mustache, - Map vars) { - this.mustache = mustache; + public MustacheExecutableScript(CompiledScript template, Map vars) { + this.template = template; this.vars = vars == null ? Collections.emptyMap() : vars; } @@ -185,18 +179,11 @@ public class MustacheScriptEngineService extends AbstractComponent implements Sc @Override public Object run() { BytesStreamOutput result = new BytesStreamOutput(); - UTF8StreamWriter writer = utf8StreamWriter().setOutput(result); - mustache.execute(writer, vars); - try { - writer.flush(); - } catch (IOException e) { - logger.error("Could not execute query template (failed to flush writer): ", e); - } finally { - try { - writer.close(); - } catch (IOException e) { - logger.error("Could not execute query template (failed to close writer): ", e); - } + try (UTF8StreamWriter writer = utf8StreamWriter().setOutput(result)) { + ((Mustache) template.compiled()).execute(writer, vars); + } catch (Exception e) { + logger.error("Error running " + template, e); + throw new ScriptException("Error running " + template, e); } return result.bytes(); } diff --git a/core/src/test/java/org/elasticsearch/script/ScriptModesTests.java b/core/src/test/java/org/elasticsearch/script/ScriptModesTests.java index 1ea2145e03b..188cd8d7650 100644 --- a/core/src/test/java/org/elasticsearch/script/ScriptModesTests.java +++ b/core/src/test/java/org/elasticsearch/script/ScriptModesTests.java @@ -287,17 +287,17 @@ public class ScriptModesTests extends ElasticsearchTestCase { } @Override - public ExecutableScript executable(Object compiledScript, @Nullable Map vars) { + public ExecutableScript executable(CompiledScript compiledScript, @Nullable Map vars) { return null; } @Override - public SearchScript search(Object compiledScript, SearchLookup lookup, @Nullable Map vars) { + public SearchScript search(CompiledScript compiledScript, SearchLookup lookup, @Nullable Map vars) { return null; } @Override - public Object execute(Object compiledScript, Map vars) { + public Object execute(CompiledScript compiledScript, Map vars) { return null; } diff --git a/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java index 19ada40353a..ca7401e2f3f 100644 --- a/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java +++ b/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java @@ -148,7 +148,7 @@ public class ScriptServiceTests extends ElasticsearchTestCase { scriptService.compile(new Script("test_script", ScriptType.FILE, "test", null), ScriptContext.Standard.SEARCH); fail("the script test_script should no longer exist"); } catch (IllegalArgumentException ex) { - assertThat(ex.getMessage(), containsString("Unable to find on disk script test_script")); + assertThat(ex.getMessage(), containsString("Unable to find on disk file script [test_script] using lang [test]")); } } @@ -171,7 +171,7 @@ public class ScriptServiceTests extends ElasticsearchTestCase { randomFrom(scriptContexts)); CompiledScript compiledScript2 = scriptService.compile(new Script("1+1", ScriptType.INLINE, "test", null), randomFrom(scriptContexts)); - assertThat(compiledScript1, sameInstance(compiledScript2)); + assertThat(compiledScript1.compiled(), sameInstance(compiledScript2.compiled())); } @Test @@ -181,7 +181,7 @@ public class ScriptServiceTests extends ElasticsearchTestCase { randomFrom(scriptContexts)); CompiledScript compiledScript2 = scriptService.compile(new Script("script", ScriptType.INLINE, "test2", null), randomFrom(scriptContexts)); - assertThat(compiledScript1, sameInstance(compiledScript2)); + assertThat(compiledScript1.compiled(), sameInstance(compiledScript2.compiled())); } @Test @@ -192,7 +192,7 @@ public class ScriptServiceTests extends ElasticsearchTestCase { randomFrom(scriptContexts)); CompiledScript compiledScript2 = scriptService.compile(new Script("file_script", ScriptType.FILE, "test2", null), randomFrom(scriptContexts)); - assertThat(compiledScript1, sameInstance(compiledScript2)); + assertThat(compiledScript1.compiled(), sameInstance(compiledScript2.compiled())); } @Test @@ -431,17 +431,17 @@ public class ScriptServiceTests extends ElasticsearchTestCase { } @Override - public ExecutableScript executable(final Object compiledScript, @Nullable Map vars) { + public ExecutableScript executable(final CompiledScript compiledScript, @Nullable Map vars) { return null; } @Override - public SearchScript search(Object compiledScript, SearchLookup lookup, @Nullable Map vars) { + public SearchScript search(CompiledScript compiledScript, SearchLookup lookup, @Nullable Map vars) { return null; } @Override - public Object execute(Object compiledScript, Map vars) { + public Object execute(CompiledScript compiledScript, Map vars) { return null; } diff --git a/core/src/test/java/org/elasticsearch/script/expression/ExpressionScriptTests.java b/core/src/test/java/org/elasticsearch/script/expression/ExpressionScriptTests.java index 0156c727899..1dedc6ef350 100644 --- a/core/src/test/java/org/elasticsearch/script/expression/ExpressionScriptTests.java +++ b/core/src/test/java/org/elasticsearch/script/expression/ExpressionScriptTests.java @@ -29,9 +29,11 @@ import org.elasticsearch.action.search.SearchType; import org.elasticsearch.action.update.UpdateRequestBuilder; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.functionscore.ScoreFunctionBuilder; import org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders; +import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptException; import org.elasticsearch.script.ScriptService.ScriptType; @@ -414,8 +416,9 @@ public class ExpressionScriptTests extends ElasticsearchIntegrationTest { vars.put("xyz", -1); Expression expr = JavascriptCompiler.compile("a+b+xyz"); + CompiledScript compiledScript = new CompiledScript(ScriptType.INLINE, "", "expression", expr); - ExpressionExecutableScript ees = new ExpressionExecutableScript(expr, vars); + ExpressionExecutableScript ees = new ExpressionExecutableScript(compiledScript, vars); assertEquals((Double) ees.run(), 4.5, 0.001); ees.setNextVar("b", -2.5); @@ -431,7 +434,7 @@ public class ExpressionScriptTests extends ElasticsearchIntegrationTest { try { vars = new HashMap<>(); vars.put("a", 1); - ees = new ExpressionExecutableScript(expr, vars); + ees = new ExpressionExecutableScript(compiledScript, vars); ees.run(); fail("An incorrect number of variables were allowed to be used in an expression."); } catch (ScriptException se) { @@ -444,7 +447,7 @@ public class ExpressionScriptTests extends ElasticsearchIntegrationTest { vars.put("a", 1); vars.put("b", 3); vars.put("c", -1); - ees = new ExpressionExecutableScript(expr, vars); + ees = new ExpressionExecutableScript(compiledScript, vars); ees.run(); fail("A variable was allowed to be set that does not exist in the expression."); } catch (ScriptException se) { @@ -457,7 +460,7 @@ public class ExpressionScriptTests extends ElasticsearchIntegrationTest { vars.put("a", 1); vars.put("b", 3); vars.put("xyz", "hello"); - ees = new ExpressionExecutableScript(expr, vars); + ees = new ExpressionExecutableScript(compiledScript, vars); ees.run(); fail("A non-number was allowed to be use in the expression."); } catch (ScriptException se) { diff --git a/core/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTest.java b/core/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTest.java index ed7de33cde1..beea87ec603 100644 --- a/core/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTest.java +++ b/core/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTest.java @@ -20,6 +20,8 @@ package org.elasticsearch.script.mustache; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.script.CompiledScript; +import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ElasticsearchTestCase; import org.junit.Before; import org.junit.Test; @@ -52,7 +54,7 @@ public class MustacheScriptEngineTest extends ElasticsearchTestCase { + "\"negative\": {\"term\": {\"body\": {\"value\": \"solr\"}" + "}}, \"negative_boost\": {{boost_val}} } }}"; Map vars = new HashMap<>(); vars.put("boost_val", "0.3"); - BytesReference o = (BytesReference) qe.execute(qe.compile(template), vars); + BytesReference o = (BytesReference) qe.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "", "mustache", qe.compile(template)), vars); assertEquals("GET _search {\"query\": {\"boosting\": {\"positive\": {\"match\": {\"body\": \"gift\"}}," + "\"negative\": {\"term\": {\"body\": {\"value\": \"solr\"}}}, \"negative_boost\": 0.3 } }}", new String(o.toBytes(), Charset.forName("UTF-8"))); @@ -63,7 +65,7 @@ public class MustacheScriptEngineTest extends ElasticsearchTestCase { Map vars = new HashMap<>(); vars.put("boost_val", "0.3"); vars.put("body_val", "\"quick brown\""); - BytesReference o = (BytesReference) qe.execute(qe.compile(template), vars); + BytesReference o = (BytesReference) qe.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "", "mustache", qe.compile(template)), vars); assertEquals("GET _search {\"query\": {\"boosting\": {\"positive\": {\"match\": {\"body\": \"gift\"}}," + "\"negative\": {\"term\": {\"body\": {\"value\": \"\\\"quick brown\\\"\"}}}, \"negative_boost\": 0.3 } }}", new String(o.toBytes(), Charset.forName("UTF-8"))); diff --git a/plugins/lang-javascript/src/main/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineService.java b/plugins/lang-javascript/src/main/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineService.java index 7aa8b2bbf3f..71d519717c9 100644 --- a/plugins/lang-javascript/src/main/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineService.java +++ b/plugins/lang-javascript/src/main/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineService.java @@ -105,7 +105,7 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements } @Override - public ExecutableScript executable(Object compiledScript, Map vars) { + public ExecutableScript executable(CompiledScript compiledScript, Map vars) { Context ctx = Context.enter(); try { ctx.setWrapFactory(wrapFactory); @@ -117,14 +117,14 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements ScriptableObject.putProperty(scope, entry.getKey(), entry.getValue()); } - return new JavaScriptExecutableScript((Script) compiledScript, scope); + return new JavaScriptExecutableScript((Script) compiledScript.compiled(), scope); } finally { Context.exit(); } } @Override - public SearchScript search(final Object compiledScript, final SearchLookup lookup, @Nullable final Map vars) { + public SearchScript search(final CompiledScript compiledScript, final SearchLookup lookup, @Nullable final Map vars) { Context ctx = Context.enter(); try { ctx.setWrapFactory(wrapFactory); @@ -148,7 +148,7 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements } } - return new JavaScriptSearchScript((Script) compiledScript, scope, leafLookup); + return new JavaScriptSearchScript((Script) compiledScript.compiled(), scope, leafLookup); } }; } finally { @@ -157,11 +157,11 @@ public class JavaScriptScriptEngineService extends AbstractComponent implements } @Override - public Object execute(Object compiledScript, Map vars) { + public Object execute(CompiledScript compiledScript, Map vars) { Context ctx = Context.enter(); ctx.setWrapFactory(wrapFactory); try { - Script script = (Script) compiledScript; + Script script = (Script) compiledScript.compiled(); Scriptable scope = ctx.newObject(globalScope); scope.setPrototype(globalScope); scope.setParentScope(null); diff --git a/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineTests.java b/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineTests.java index 9bb3543b012..e596bf8b3ae 100644 --- a/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineTests.java +++ b/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptScriptEngineTests.java @@ -21,7 +21,9 @@ package org.elasticsearch.script.javascript; import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ExecutableScript; +import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ElasticsearchTestCase; import org.junit.After; import org.junit.Before; @@ -55,7 +57,7 @@ public class JavaScriptScriptEngineTests extends ElasticsearchTestCase { @Test public void testSimpleEquation() { Map vars = new HashMap(); - Object o = se.execute(se.compile("1 + 2"), vars); + Object o = se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testSimpleEquation", "js", se.compile("1 + 2")), vars); assertThat(((Number) o).intValue(), equalTo(3)); } @@ -66,20 +68,21 @@ public class JavaScriptScriptEngineTests extends ElasticsearchTestCase { Map obj2 = MapBuilder.newMapBuilder().put("prop2", "value2").map(); Map obj1 = MapBuilder.newMapBuilder().put("prop1", "value1").put("obj2", obj2).put("l", Arrays.asList("2", "1")).map(); vars.put("obj1", obj1); - Object o = se.execute(se.compile("obj1"), vars); + Object o = se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testMapAccess", "js", se.compile("obj1")), vars); assertThat(o, instanceOf(Map.class)); obj1 = (Map) o; assertThat((String) obj1.get("prop1"), equalTo("value1")); assertThat((String) ((Map) obj1.get("obj2")).get("prop2"), equalTo("value2")); - o = se.execute(se.compile("obj1.l[0]"), vars); + o = se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testMapAccess", "js", se.compile("obj1.l[0]")), vars); assertThat(((String) o), equalTo("2")); } @Test public void testJavaScriptObjectToMap() { Map vars = new HashMap(); - Object o = se.execute(se.compile("var obj1 = {}; obj1.prop1 = 'value1'; obj1.obj2 = {}; obj1.obj2.prop2 = 'value2'; obj1"), vars); + Object o = se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testJavaScriptObjectToMap", "js", + se.compile("var obj1 = {}; obj1.prop1 = 'value1'; obj1.obj2 = {}; obj1.obj2.prop2 = 'value2'; obj1")), vars); Map obj1 = (Map) o; assertThat((String) obj1.get("prop1"), equalTo("value1")); assertThat((String) ((Map) obj1.get("obj2")).get("prop2"), equalTo("value2")); @@ -94,7 +97,8 @@ public class JavaScriptScriptEngineTests extends ElasticsearchTestCase { ctx.put("obj1", obj1); vars.put("ctx", ctx); - se.execute(se.compile("ctx.obj2 = {}; ctx.obj2.prop2 = 'value2'; ctx.obj1.prop1 = 'uvalue1'"), vars); + se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testJavaScriptObjectMapInter", "js", + se.compile("ctx.obj2 = {}; ctx.obj2.prop2 = 'value2'; ctx.obj1.prop1 = 'uvalue1'")), vars); ctx = (Map) se.unwrap(vars.get("ctx")); assertThat(ctx.containsKey("obj1"), equalTo(true)); assertThat((String) ((Map) ctx.get("obj1")).get("prop1"), equalTo("uvalue1")); @@ -108,8 +112,9 @@ public class JavaScriptScriptEngineTests extends ElasticsearchTestCase { Map doc = new HashMap(); ctx.put("doc", doc); - Object complied = se.compile("ctx.doc.field1 = ['value1', 'value2']"); - ExecutableScript script = se.executable(complied, new HashMap()); + Object compiled = se.compile("ctx.doc.field1 = ['value1', 'value2']"); + ExecutableScript script = se.executable(new CompiledScript(ScriptService.ScriptType.INLINE, "testJavaScriptInnerArrayCreation", "js", + compiled), new HashMap()); script.setNextVar("ctx", ctx); script.run(); @@ -125,18 +130,22 @@ public class JavaScriptScriptEngineTests extends ElasticsearchTestCase { Map obj1 = MapBuilder.newMapBuilder().put("prop1", "value1").put("obj2", obj2).map(); vars.put("l", Arrays.asList("1", "2", "3", obj1)); - Object o = se.execute(se.compile("l.length"), vars); + Object o = se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testAccessInScript", "js", + se.compile("l.length")), vars); assertThat(((Number) o).intValue(), equalTo(4)); - o = se.execute(se.compile("l[0]"), vars); + o = se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testAccessInScript", "js", + se.compile("l[0]")), vars); assertThat(((String) o), equalTo("1")); - o = se.execute(se.compile("l[3]"), vars); + o = se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testAccessInScript", "js", + se.compile("l[3]")), vars); obj1 = (Map) o; assertThat((String) obj1.get("prop1"), equalTo("value1")); assertThat((String) ((Map) obj1.get("obj2")).get("prop2"), equalTo("value2")); - o = se.execute(se.compile("l[3].prop1"), vars); + o = se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testAccessInScript", "js", + se.compile("l[3].prop1")), vars); assertThat(((String) o), equalTo("value1")); } @@ -147,7 +156,8 @@ public class JavaScriptScriptEngineTests extends ElasticsearchTestCase { vars.put("ctx", ctx); Object compiledScript = se.compile("ctx.value"); - ExecutableScript script = se.executable(compiledScript, vars); + ExecutableScript script = se.executable(new CompiledScript(ScriptService.ScriptType.INLINE, "testChangingVarsCrossExecution1", "js", + compiledScript), vars); ctx.put("value", 1); Object o = script.run(); assertThat(((Number) o).intValue(), equalTo(1)); @@ -162,7 +172,8 @@ public class JavaScriptScriptEngineTests extends ElasticsearchTestCase { Map vars = new HashMap(); Object compiledScript = se.compile("value"); - ExecutableScript script = se.executable(compiledScript, vars); + ExecutableScript script = se.executable(new CompiledScript(ScriptService.ScriptType.INLINE, "testChangingVarsCrossExecution2", "js", + compiledScript), vars); script.setNextVar("value", 1); Object o = script.run(); assertThat(((Number) o).intValue(), equalTo(1)); diff --git a/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptScriptMultiThreadedTest.java b/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptScriptMultiThreadedTest.java index c235128e83f..baf10625cdf 100644 --- a/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptScriptMultiThreadedTest.java +++ b/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptScriptMultiThreadedTest.java @@ -20,7 +20,9 @@ package org.elasticsearch.script.javascript; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ExecutableScript; +import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ElasticsearchTestCase; import org.junit.Test; @@ -59,7 +61,7 @@ public class JavaScriptScriptMultiThreadedTest extends ElasticsearchTestCase { Map vars = new HashMap(); vars.put("x", x); vars.put("y", y); - ExecutableScript script = se.executable(compiled, vars); + ExecutableScript script = se.executable(new CompiledScript(ScriptService.ScriptType.INLINE, "testExecutableNoRuntimeParams", "js", compiled), vars); for (int i = 0; i < 100000; i++) { long result = ((Number) script.run()).longValue(); assertThat(result, equalTo(addition)); @@ -100,7 +102,7 @@ public class JavaScriptScriptMultiThreadedTest extends ElasticsearchTestCase { long x = ThreadLocalRandom.current().nextInt(); Map vars = new HashMap(); vars.put("x", x); - ExecutableScript script = se.executable(compiled, vars); + ExecutableScript script = se.executable(new CompiledScript(ScriptService.ScriptType.INLINE, "testExecutableNoRuntimeParams", "js", compiled), vars); for (int i = 0; i < 100000; i++) { long y = ThreadLocalRandom.current().nextInt(); long addition = x + y; @@ -147,7 +149,7 @@ public class JavaScriptScriptMultiThreadedTest extends ElasticsearchTestCase { long addition = x + y; runtimeVars.put("x", x); runtimeVars.put("y", y); - long result = ((Number) se.execute(compiled, runtimeVars)).longValue(); + long result = ((Number) se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testExecutableNoRuntimeParams", "js", compiled), runtimeVars)).longValue(); assertThat(result, equalTo(addition)); } } catch (Throwable t) { diff --git a/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/SimpleBench.java b/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/SimpleBench.java index e0b47c8a919..9cb44ef43e4 100644 --- a/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/SimpleBench.java +++ b/plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/SimpleBench.java @@ -21,7 +21,9 @@ package org.elasticsearch.script.javascript; import org.elasticsearch.common.StopWatch; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ExecutableScript; +import org.elasticsearch.script.ScriptService; import java.util.HashMap; import java.util.Map; @@ -34,32 +36,33 @@ public class SimpleBench { public static void main(String[] args) { JavaScriptScriptEngineService se = new JavaScriptScriptEngineService(Settings.Builder.EMPTY_SETTINGS); Object compiled = se.compile("x + y"); + CompiledScript compiledScript = new CompiledScript(ScriptService.ScriptType.INLINE, "testExecutableNoRuntimeParams", "js", compiled); Map vars = new HashMap(); // warm up for (int i = 0; i < 1000; i++) { vars.put("x", i); vars.put("y", i + 1); - se.execute(compiled, vars); + se.execute(compiledScript, vars); } final long ITER = 100000; StopWatch stopWatch = new StopWatch().start(); for (long i = 0; i < ITER; i++) { - se.execute(compiled, vars); + se.execute(compiledScript, vars); } System.out.println("Execute Took: " + stopWatch.stop().lastTaskTime()); stopWatch = new StopWatch().start(); - ExecutableScript executableScript = se.executable(compiled, vars); + ExecutableScript executableScript = se.executable(compiledScript, vars); for (long i = 0; i < ITER; i++) { executableScript.run(); } System.out.println("Executable Took: " + stopWatch.stop().lastTaskTime()); stopWatch = new StopWatch().start(); - executableScript = se.executable(compiled, vars); + executableScript = se.executable(compiledScript, vars); for (long i = 0; i < ITER; i++) { for (Map.Entry entry : vars.entrySet()) { executableScript.setNextVar(entry.getKey(), entry.getValue()); diff --git a/plugins/lang-python/src/main/java/org/elasticsearch/script/python/PythonScriptEngineService.java b/plugins/lang-python/src/main/java/org/elasticsearch/script/python/PythonScriptEngineService.java index 6138453925e..3e3fb7f542e 100644 --- a/plugins/lang-python/src/main/java/org/elasticsearch/script/python/PythonScriptEngineService.java +++ b/plugins/lang-python/src/main/java/org/elasticsearch/script/python/PythonScriptEngineService.java @@ -78,26 +78,26 @@ public class PythonScriptEngineService extends AbstractComponent implements Scri } @Override - public ExecutableScript executable(Object compiledScript, Map vars) { - return new PythonExecutableScript((PyCode) compiledScript, vars); + public ExecutableScript executable(CompiledScript compiledScript, Map vars) { + return new PythonExecutableScript((PyCode) compiledScript.compiled(), vars); } @Override - public SearchScript search(final Object compiledScript, final SearchLookup lookup, @Nullable final Map vars) { + public SearchScript search(final CompiledScript compiledScript, final SearchLookup lookup, @Nullable final Map vars) { return new SearchScript() { @Override public LeafSearchScript getLeafSearchScript(LeafReaderContext context) throws IOException { final LeafSearchLookup leafLookup = lookup.getLeafSearchLookup(context); - return new PythonSearchScript((PyCode) compiledScript, vars, leafLookup); + return new PythonSearchScript((PyCode) compiledScript.compiled(), vars, leafLookup); } }; } @Override - public Object execute(Object compiledScript, Map vars) { + public Object execute(CompiledScript compiledScript, Map vars) { PyObject pyVars = Py.java2py(vars); interp.setLocals(pyVars); - PyObject ret = interp.eval((PyCode) compiledScript); + PyObject ret = interp.eval((PyCode) compiledScript.compiled()); if (ret == null) { return null; } diff --git a/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonScriptEngineTests.java b/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonScriptEngineTests.java index 1621d22ac01..94528f2b0e1 100644 --- a/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonScriptEngineTests.java +++ b/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonScriptEngineTests.java @@ -21,7 +21,9 @@ package org.elasticsearch.script.python; import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ExecutableScript; +import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ElasticsearchTestCase; import org.junit.After; import org.junit.Before; @@ -57,7 +59,7 @@ public class PythonScriptEngineTests extends ElasticsearchTestCase { @Test public void testSimpleEquation() { Map vars = new HashMap(); - Object o = se.execute(se.compile("1 + 2"), vars); + Object o = se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testSimpleEquation", "python", se.compile("1 + 2")), vars); assertThat(((Number) o).intValue(), equalTo(3)); } @@ -68,13 +70,13 @@ public class PythonScriptEngineTests extends ElasticsearchTestCase { Map obj2 = MapBuilder.newMapBuilder().put("prop2", "value2").map(); Map obj1 = MapBuilder.newMapBuilder().put("prop1", "value1").put("obj2", obj2).put("l", Arrays.asList("2", "1")).map(); vars.put("obj1", obj1); - Object o = se.execute(se.compile("obj1"), vars); + Object o = se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testMapAccess", "python", se.compile("obj1")), vars); assertThat(o, instanceOf(Map.class)); obj1 = (Map) o; assertThat((String) obj1.get("prop1"), equalTo("value1")); assertThat((String) ((Map) obj1.get("obj2")).get("prop2"), equalTo("value2")); - o = se.execute(se.compile("obj1['l'][0]"), vars); + o = se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testMapAccess", "python", se.compile("obj1['l'][0]")), vars); assertThat(((String) o), equalTo("2")); } @@ -87,7 +89,8 @@ public class PythonScriptEngineTests extends ElasticsearchTestCase { ctx.put("obj1", obj1); vars.put("ctx", ctx); - se.execute(se.compile("ctx['obj2'] = { 'prop2' : 'value2' }; ctx['obj1']['prop1'] = 'uvalue1'"), vars); + se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testObjectInterMap", "python", + se.compile("ctx['obj2'] = { 'prop2' : 'value2' }; ctx['obj1']['prop1'] = 'uvalue1'")), vars); ctx = (Map) se.unwrap(vars.get("ctx")); assertThat(ctx.containsKey("obj1"), equalTo(true)); assertThat((String) ((Map) ctx.get("obj1")).get("prop1"), equalTo("uvalue1")); @@ -106,15 +109,15 @@ public class PythonScriptEngineTests extends ElasticsearchTestCase { // Object o = se.execute(se.compile("l.length"), vars); // assertThat(((Number) o).intValue(), equalTo(4)); - Object o = se.execute(se.compile("l[0]"), vars); + Object o = se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testAccessListInScript", "python", se.compile("l[0]")), vars); assertThat(((String) o), equalTo("1")); - o = se.execute(se.compile("l[3]"), vars); + o = se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testAccessListInScript", "python", se.compile("l[3]")), vars); obj1 = (Map) o; assertThat((String) obj1.get("prop1"), equalTo("value1")); assertThat((String) ((Map) obj1.get("obj2")).get("prop2"), equalTo("value2")); - o = se.execute(se.compile("l[3]['prop1']"), vars); + o = se.execute(new CompiledScript(ScriptService.ScriptType.INLINE, "testAccessListInScript", "python", se.compile("l[3]['prop1']")), vars); assertThat(((String) o), equalTo("value1")); } @@ -125,7 +128,7 @@ public class PythonScriptEngineTests extends ElasticsearchTestCase { vars.put("ctx", ctx); Object compiledScript = se.compile("ctx['value']"); - ExecutableScript script = se.executable(compiledScript, vars); + ExecutableScript script = se.executable(new CompiledScript(ScriptService.ScriptType.INLINE, "testChangingVarsCrossExecution1", "python", compiledScript), vars); ctx.put("value", 1); Object o = script.run(); assertThat(((Number) o).intValue(), equalTo(1)); @@ -141,7 +144,7 @@ public class PythonScriptEngineTests extends ElasticsearchTestCase { Map ctx = new HashMap(); Object compiledScript = se.compile("value"); - ExecutableScript script = se.executable(compiledScript, vars); + ExecutableScript script = se.executable(new CompiledScript(ScriptService.ScriptType.INLINE, "testChangingVarsCrossExecution2", "python", compiledScript), vars); script.setNextVar("value", 1); Object o = script.run(); assertThat(((Number) o).intValue(), equalTo(1)); diff --git a/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonScriptMultiThreadedTest.java b/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonScriptMultiThreadedTest.java index 9d53507388b..13d1d2b7a05 100644 --- a/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonScriptMultiThreadedTest.java +++ b/plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonScriptMultiThreadedTest.java @@ -20,7 +20,9 @@ package org.elasticsearch.script.python; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ExecutableScript; +import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ElasticsearchTestCase; import org.junit.After; import org.junit.Test; @@ -50,6 +52,7 @@ public class PythonScriptMultiThreadedTest extends ElasticsearchTestCase { public void testExecutableNoRuntimeParams() throws Exception { final PythonScriptEngineService se = new PythonScriptEngineService(Settings.Builder.EMPTY_SETTINGS); final Object compiled = se.compile("x + y"); + final CompiledScript compiledScript = new CompiledScript(ScriptService.ScriptType.INLINE, "testExecutableNoRuntimeParams", "python", compiled); final AtomicBoolean failed = new AtomicBoolean(); Thread[] threads = new Thread[4]; @@ -67,7 +70,7 @@ public class PythonScriptMultiThreadedTest extends ElasticsearchTestCase { Map vars = new HashMap(); vars.put("x", x); vars.put("y", y); - ExecutableScript script = se.executable(compiled, vars); + ExecutableScript script = se.executable(compiledScript, vars); for (int i = 0; i < 10000; i++) { long result = ((Number) script.run()).longValue(); assertThat(result, equalTo(addition)); @@ -136,6 +139,7 @@ public class PythonScriptMultiThreadedTest extends ElasticsearchTestCase { public void testExecute() throws Exception { final PythonScriptEngineService se = new PythonScriptEngineService(Settings.Builder.EMPTY_SETTINGS); final Object compiled = se.compile("x + y"); + final CompiledScript compiledScript = new CompiledScript(ScriptService.ScriptType.INLINE, "testExecute", "python", compiled); final AtomicBoolean failed = new AtomicBoolean(); Thread[] threads = new Thread[4]; @@ -154,7 +158,7 @@ public class PythonScriptMultiThreadedTest extends ElasticsearchTestCase { long addition = x + y; runtimeVars.put("x", x); runtimeVars.put("y", y); - long result = ((Number) se.execute(compiled, runtimeVars)).longValue(); + long result = ((Number) se.execute(compiledScript, runtimeVars)).longValue(); assertThat(result, equalTo(addition)); } } catch (Throwable t) { diff --git a/plugins/lang-python/src/test/java/org/elasticsearch/script/python/SimpleBench.java b/plugins/lang-python/src/test/java/org/elasticsearch/script/python/SimpleBench.java index 583bab163fa..4fab7dd8fb9 100644 --- a/plugins/lang-python/src/test/java/org/elasticsearch/script/python/SimpleBench.java +++ b/plugins/lang-python/src/test/java/org/elasticsearch/script/python/SimpleBench.java @@ -21,7 +21,9 @@ package org.elasticsearch.script.python; import org.elasticsearch.common.StopWatch; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.script.CompiledScript; import org.elasticsearch.script.ExecutableScript; +import org.elasticsearch.script.ScriptService; import java.util.HashMap; import java.util.Map; @@ -34,32 +36,34 @@ public class SimpleBench { public static void main(String[] args) { PythonScriptEngineService se = new PythonScriptEngineService(Settings.Builder.EMPTY_SETTINGS); Object compiled = se.compile("x + y"); + CompiledScript compiledScript = new CompiledScript(ScriptService.ScriptType.INLINE, "SimpleBench", "python", compiled); + Map vars = new HashMap(); // warm up for (int i = 0; i < 1000; i++) { vars.put("x", i); vars.put("y", i + 1); - se.execute(compiled, vars); + se.execute(compiledScript, vars); } final long ITER = 100000; StopWatch stopWatch = new StopWatch().start(); for (long i = 0; i < ITER; i++) { - se.execute(compiled, vars); + se.execute(compiledScript, vars); } System.out.println("Execute Took: " + stopWatch.stop().lastTaskTime()); stopWatch = new StopWatch().start(); - ExecutableScript executableScript = se.executable(compiled, vars); + ExecutableScript executableScript = se.executable(compiledScript, vars); for (long i = 0; i < ITER; i++) { executableScript.run(); } System.out.println("Executable Took: " + stopWatch.stop().lastTaskTime()); stopWatch = new StopWatch().start(); - executableScript = se.executable(compiled, vars); + executableScript = se.executable(compiledScript, vars); for (long i = 0; i < ITER; i++) { for (Map.Entry entry : vars.entrySet()) { executableScript.setNextVar(entry.getKey(), entry.getValue());