From cd721b638658db2eada162444ece45b8eef773fc Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Thu, 19 Dec 2019 10:14:28 -0700 Subject: [PATCH] Scripting: ScriptFactory not required by compile (#50344) Avoid backwards incompatible changes for 8.x and 7.6 by removing type restriction on compile and Factory. Factories may optionally implement ScriptFactory. If so, then they can indicate determinism and thus cacheability. Relates: #49466 --- .../common/AnalysisPredicateScript.java | 3 +- .../PredicateTokenScriptFilterTests.java | 3 +- .../ScriptedConditionTokenFilterTests.java | 3 +- .../expression/ExpressionScriptEngine.java | 63 ++++++++++++++++--- .../script/mustache/MustacheScriptEngine.java | 6 +- .../painless/PainlessScriptEngine.java | 7 +-- .../action/PainlessExecuteAction.java | 3 +- .../painless/BaseClassTests.java | 43 +++++++------ .../painless/BasicStatementTests.java | 3 +- .../elasticsearch/painless/BindingsTests.java | 3 +- .../elasticsearch/painless/FactoryTests.java | 44 ++++++++++--- .../expertscript/ExpertScriptPlugin.java | 5 +- .../index/query/QueryShardContext.java | 6 +- .../BucketAggregationSelectorScript.java | 2 +- .../script/IngestConditionalScript.java | 2 +- .../elasticsearch/script/IngestScript.java | 2 +- .../elasticsearch/script/ScriptContext.java | 2 +- .../elasticsearch/script/ScriptEngine.java | 2 +- .../elasticsearch/script/ScriptService.java | 2 +- .../elasticsearch/script/TemplateScript.java | 2 +- .../elasticsearch/script/UpdateScript.java | 2 +- .../query/IntervalQueryBuilderTests.java | 3 +- .../script/ScriptContextTests.java | 12 ++-- .../script/ScriptLanguagesInfoTests.java | 2 +- .../metrics/ScriptedMetricIT.java | 1 - .../functionscore/ExplainableScriptIT.java | 3 +- .../search/suggest/SuggestSearchIT.java | 3 +- .../ingest/TestTemplateService.java | 3 +- .../script/MockScriptEngine.java | 19 +++--- .../script/MockMustacheScriptEngine.java | 2 +- .../test/MockPainlessScriptEngine.java | 3 +- .../condition/WatcherConditionScript.java | 3 +- .../script/WatcherTransformScript.java | 3 +- 33 files changed, 160 insertions(+), 105 deletions(-) diff --git a/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/AnalysisPredicateScript.java b/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/AnalysisPredicateScript.java index f14bd9ded9cc1..5d8c491efc585 100644 --- a/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/AnalysisPredicateScript.java +++ b/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/AnalysisPredicateScript.java @@ -27,7 +27,6 @@ import org.apache.lucene.analysis.tokenattributes.TypeAttribute; import org.apache.lucene.util.AttributeSource; import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.ScriptFactory; /** * A predicate based on the current token in a TokenStream @@ -108,7 +107,7 @@ public boolean isKeyword() { */ public abstract boolean execute(Token token); - public interface Factory extends ScriptFactory { + public interface Factory { AnalysisPredicateScript newInstance(); } diff --git a/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/PredicateTokenScriptFilterTests.java b/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/PredicateTokenScriptFilterTests.java index 9e61a59237348..84ba5e5d3373c 100644 --- a/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/PredicateTokenScriptFilterTests.java +++ b/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/PredicateTokenScriptFilterTests.java @@ -30,7 +30,6 @@ import org.elasticsearch.indices.analysis.AnalysisModule; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.ScriptFactory; import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESTokenStreamTestCase; import org.elasticsearch.test.IndexSettingsModule; @@ -64,7 +63,7 @@ public boolean execute(Token token) { @SuppressWarnings("unchecked") ScriptService scriptService = new ScriptService(indexSettings, Collections.emptyMap(), Collections.emptyMap()){ @Override - public FactoryType compile(Script script, ScriptContext context) { + public FactoryType compile(Script script, ScriptContext context) { assertEquals(context, AnalysisPredicateScript.CONTEXT); assertEquals(new Script("my_script"), script); return (FactoryType) factory; diff --git a/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/ScriptedConditionTokenFilterTests.java b/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/ScriptedConditionTokenFilterTests.java index d2bbc780c8747..58226ac169bc3 100644 --- a/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/ScriptedConditionTokenFilterTests.java +++ b/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/ScriptedConditionTokenFilterTests.java @@ -30,7 +30,6 @@ import org.elasticsearch.indices.analysis.AnalysisModule; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.ScriptFactory; import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESTokenStreamTestCase; import org.elasticsearch.test.IndexSettingsModule; @@ -64,7 +63,7 @@ public boolean execute(Token token) { @SuppressWarnings("unchecked") ScriptService scriptService = new ScriptService(indexSettings, Collections.emptyMap(), Collections.emptyMap()){ @Override - public FactoryType compile(Script script, ScriptContext context) { + public FactoryType compile(Script script, ScriptContext context) { assertEquals(context, AnalysisPredicateScript.CONTEXT); assertEquals(new Script("token.getPosition() > 1"), script); return (FactoryType) factory; diff --git a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngine.java b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngine.java index ead6d733b10de..b99730be826d5 100644 --- a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngine.java +++ b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngine.java @@ -44,7 +44,6 @@ import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptEngine; import org.elasticsearch.script.ScriptException; -import org.elasticsearch.script.ScriptFactory; import org.elasticsearch.script.TermsSetQueryScript; import org.elasticsearch.search.lookup.SearchLookup; @@ -85,22 +84,72 @@ public boolean execute() { }, FilterScript.CONTEXT, - (Expression expr) -> (FilterScript.Factory) (p, lookup) -> newFilterScript(expr, lookup, p), + (Expression expr) -> new FilterScript.Factory() { + @Override + public boolean isResultDeterministic() { + return true; + } + + @Override + public FilterScript.LeafFactory newFactory(Map params, SearchLookup lookup) { + return newFilterScript(expr, lookup, params); + } + }, ScoreScript.CONTEXT, - (Expression expr) -> (ScoreScript.Factory) (p, lookup) -> newScoreScript(expr, lookup, p), + (Expression expr) -> new ScoreScript.Factory() { + @Override + public ScoreScript.LeafFactory newFactory(Map params, SearchLookup lookup) { + return newScoreScript(expr, lookup, params); + } + + @Override + public boolean isResultDeterministic() { + return true; + } + }, TermsSetQueryScript.CONTEXT, (Expression expr) -> (TermsSetQueryScript.Factory) (p, lookup) -> newTermsSetQueryScript(expr, lookup, p), AggregationScript.CONTEXT, - (Expression expr) -> (AggregationScript.Factory) (p, lookup) -> newAggregationScript(expr, lookup, p), + (Expression expr) -> new AggregationScript.Factory() { + @Override + public AggregationScript.LeafFactory newFactory(Map params, SearchLookup lookup) { + return newAggregationScript(expr, lookup, params); + } + + @Override + public boolean isResultDeterministic() { + return true; + } + }, NumberSortScript.CONTEXT, - (Expression expr) -> (NumberSortScript.Factory) (p, lookup) -> newSortScript(expr, lookup, p), + (Expression expr) -> new NumberSortScript.Factory() { + @Override + public NumberSortScript.LeafFactory newFactory(Map params, SearchLookup lookup) { + return newSortScript(expr, lookup, params); + } + + @Override + public boolean isResultDeterministic() { + return true; + } + }, FieldScript.CONTEXT, - (Expression expr) -> (FieldScript.Factory) (p, lookup) -> newFieldScript(expr, lookup, p) + (Expression expr) -> new FieldScript.Factory() { + @Override + public FieldScript.LeafFactory newFactory(Map params, SearchLookup lookup) { + return newFieldScript(expr, lookup, params); + } + + @Override + public boolean isResultDeterministic() { + return true; + } + } ); @Override @@ -109,7 +158,7 @@ public String getType() { } @Override - public T compile( + public T compile( String scriptName, String scriptSource, ScriptContext context, diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java index f14dd35d339fd..391b6a5c2d20c 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java @@ -21,9 +21,8 @@ import com.github.mustachejava.Mustache; import com.github.mustachejava.MustacheException; import com.github.mustachejava.MustacheFactory; - -import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; import org.elasticsearch.SpecialPermission; @@ -32,7 +31,6 @@ import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptEngine; import org.elasticsearch.script.ScriptException; -import org.elasticsearch.script.ScriptFactory; import org.elasticsearch.script.TemplateScript; import java.io.Reader; @@ -65,7 +63,7 @@ public final class MustacheScriptEngine implements ScriptEngine { * @return a compiled template object for later execution. * */ @Override - public T compile( + public T compile( String templateName, String templateSource, ScriptContext context, diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java index 45db4428aeced..67aedf113c32c 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java @@ -28,7 +28,6 @@ import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptEngine; import org.elasticsearch.script.ScriptException; -import org.elasticsearch.script.ScriptFactory; import org.objectweb.asm.ClassWriter; import org.objectweb.asm.Opcodes; import org.objectweb.asm.Type; @@ -123,7 +122,7 @@ public String getType() { } @Override - public T compile( + public T compile( String scriptName, String scriptSource, ScriptContext context, @@ -169,7 +168,7 @@ public Set> getSupportedContexts() { * @param The factory class. * @return A factory class that will return script instances. */ - private Type generateStatefulFactory( + private Type generateStatefulFactory( Loader loader, ScriptContext context, Set extractedVariables @@ -275,7 +274,7 @@ private Type generateStatefulFactory( * @param The factory class. * @return A factory class that will return script instances. */ - private T generateFactory( + private T generateFactory( Loader loader, ScriptContext context, Set extractedVariables, diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java index e38d87e69ccb3..b6bb1a842640d 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java @@ -75,7 +75,6 @@ import org.elasticsearch.script.ScoreScript; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.ScriptFactory; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.ScriptType; import org.elasticsearch.threadpool.ThreadPool; @@ -416,7 +415,7 @@ public Map getParams() { public abstract Object execute(); - public interface Factory extends ScriptFactory { + public interface Factory { PainlessTestScript newInstance(Map params); diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BaseClassTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BaseClassTests.java index 96790301139f3..4c6aa890ba1f8 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BaseClassTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BaseClassTests.java @@ -21,7 +21,6 @@ import org.elasticsearch.painless.spi.Whitelist; import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.ScriptFactory; import java.util.Collections; import java.util.HashMap; @@ -66,7 +65,7 @@ protected Map, List> scriptContexts() { public abstract static class Gets { - public interface Factory extends ScriptFactory { + public interface Factory { Gets newInstance(String testString, int testInt, Map params); } @@ -112,7 +111,7 @@ public void testGets() throws Exception { } public abstract static class NoArgs { - public interface Factory extends ScriptFactory { + public interface Factory { NoArgs newInstance(); } @@ -138,7 +137,7 @@ public void testNoArgs() throws Exception { } public abstract static class OneArg { - public interface Factory extends ScriptFactory { + public interface Factory { OneArg newInstance(); } @@ -155,7 +154,7 @@ public void testOneArg() throws Exception { } public abstract static class ArrayArg { - public interface Factory extends ScriptFactory { + public interface Factory { ArrayArg newInstance(); } @@ -172,7 +171,7 @@ public void testArrayArg() throws Exception { } public abstract static class PrimitiveArrayArg { - public interface Factory extends ScriptFactory { + public interface Factory { PrimitiveArrayArg newInstance(); } @@ -189,7 +188,7 @@ public void testPrimitiveArrayArg() throws Exception { } public abstract static class DefArrayArg { - public interface Factory extends ScriptFactory { + public interface Factory { DefArrayArg newInstance(); } @@ -213,7 +212,7 @@ public void testDefArrayArg()throws Exception { } public abstract static class ManyArgs { - public interface Factory extends ScriptFactory { + public interface Factory { ManyArgs newInstance(); } @@ -252,7 +251,7 @@ public void testManyArgs() throws Exception { } public abstract static class VarArgs { - public interface Factory extends ScriptFactory { + public interface Factory { VarArgs newInstance(); } @@ -268,7 +267,7 @@ public void testVarArgs() throws Exception { } public abstract static class DefaultMethods { - public interface Factory extends ScriptFactory { + public interface Factory { DefaultMethods newInstance(); } @@ -302,7 +301,7 @@ public void testDefaultMethods() throws Exception { } public abstract static class ReturnsVoid { - public interface Factory extends ScriptFactory { + public interface Factory { ReturnsVoid newInstance(); } @@ -326,7 +325,7 @@ public void testReturnsVoid() throws Exception { } public abstract static class ReturnsPrimitiveBoolean { - public interface Factory extends ScriptFactory { + public interface Factory { ReturnsPrimitiveBoolean newInstance(); } @@ -392,7 +391,7 @@ public void testReturnsPrimitiveBoolean() throws Exception { } public abstract static class ReturnsPrimitiveInt { - public interface Factory extends ScriptFactory { + public interface Factory { ReturnsPrimitiveInt newInstance(); } @@ -456,7 +455,7 @@ public void testReturnsPrimitiveInt() throws Exception { } public abstract static class ReturnsPrimitiveFloat { - public interface Factory extends ScriptFactory { + public interface Factory { ReturnsPrimitiveFloat newInstance(); } @@ -505,7 +504,7 @@ public void testReturnsPrimitiveFloat() throws Exception { } public abstract static class ReturnsPrimitiveDouble { - public interface Factory extends ScriptFactory { + public interface Factory { ReturnsPrimitiveDouble newInstance(); } @@ -568,7 +567,7 @@ public void testReturnsPrimitiveDouble() throws Exception { } public abstract static class NoArgsConstant { - public interface Factory extends ScriptFactory { + public interface Factory { NoArgsConstant newInstance(); } @@ -585,7 +584,7 @@ public void testNoArgsConstant() { } public abstract static class WrongArgsConstant { - public interface Factory extends ScriptFactory { + public interface Factory { WrongArgsConstant newInstance(); } @@ -603,7 +602,7 @@ public void testWrongArgsConstant() { } public abstract static class WrongLengthOfArgConstant { - public interface Factory extends ScriptFactory { + public interface Factory { WrongLengthOfArgConstant newInstance(); } @@ -620,7 +619,7 @@ public void testWrongLengthOfArgConstant() { } public abstract static class UnknownArgType { - public interface Factory extends ScriptFactory { + public interface Factory { UnknownArgType newInstance(); } @@ -637,7 +636,7 @@ public void testUnknownArgType() { } public abstract static class UnknownReturnType { - public interface Factory extends ScriptFactory { + public interface Factory { UnknownReturnType newInstance(); } @@ -654,7 +653,7 @@ public void testUnknownReturnType() { } public abstract static class UnknownArgTypeInArray { - public interface Factory extends ScriptFactory { + public interface Factory { UnknownArgTypeInArray newInstance(); } @@ -671,7 +670,7 @@ public void testUnknownArgTypeInArray() { } public abstract static class TwoExecuteMethods { - public interface Factory extends ScriptFactory { + public interface Factory { TwoExecuteMethods newInstance(); } diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java index 72fc01dbf2ce6..e4d1db2243b82 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java @@ -2,7 +2,6 @@ import org.elasticsearch.painless.spi.Whitelist; import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.ScriptFactory; import java.util.ArrayList; import java.util.Collections; @@ -261,7 +260,7 @@ public void testReturnStatement() { } public abstract static class OneArg { - public interface Factory extends ScriptFactory { + public interface Factory { OneArg newInstance(); } diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java index d165e0ef76d81..171880abd7907 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java @@ -23,7 +23,6 @@ import org.elasticsearch.painless.spi.WhitelistInstanceBinding; import org.elasticsearch.painless.spi.WhitelistLoader; import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.ScriptFactory; import java.util.ArrayList; import java.util.Collections; @@ -90,7 +89,7 @@ public abstract static class BindingsTestScript { public static final String[] PARAMETERS = { "test", "bound" }; public int getTestValue() {return 7;} public abstract int execute(int test, int bound); - public interface Factory extends ScriptFactory { + public interface Factory { BindingsTestScript newInstance(); } public static final ScriptContext CONTEXT = new ScriptContext<>("bindings_test", Factory.class); diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/FactoryTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/FactoryTests.java index 33645e24c0b3c..ffd4df43c9070 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/FactoryTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/FactoryTests.java @@ -35,6 +35,7 @@ protected Map, List> scriptContexts() { Map, List> contexts = super.scriptContexts(); contexts.put(StatefulFactoryTestScript.CONTEXT, Whitelist.BASE_WHITELISTS); contexts.put(FactoryTestScript.CONTEXT, Whitelist.BASE_WHITELISTS); + contexts.put(DeterministicFactoryTestScript.CONTEXT, Whitelist.BASE_WHITELISTS); contexts.put(EmptyTestScript.CONTEXT, Whitelist.BASE_WHITELISTS); contexts.put(TemplateScript.CONTEXT, Whitelist.BASE_WHITELISTS); @@ -85,7 +86,7 @@ public interface StatefulFactory { boolean needsD(); } - public interface Factory extends ScriptFactory { + public interface Factory { StatefulFactory newFactory(int x, int y); boolean needsTest(); @@ -138,7 +139,7 @@ public Map getParams() { public static final String[] PARAMETERS = new String[] {"test"}; public abstract Object execute(int test); - public interface Factory extends ScriptFactory { + public interface Factory { FactoryTestScript newInstance(Map params); boolean needsTest(); @@ -149,6 +150,31 @@ public interface Factory extends ScriptFactory { new ScriptContext<>("test", FactoryTestScript.Factory.class); } + public abstract static class DeterministicFactoryTestScript { + private final Map params; + + public DeterministicFactoryTestScript(Map params) { + this.params = params; + } + + public Map getParams() { + return params; + } + + public static final String[] PARAMETERS = new String[] {"test"}; + public abstract Object execute(int test); + + public interface Factory extends ScriptFactory{ + FactoryTestScript newInstance(Map params); + + boolean needsTest(); + boolean needsNothing(); + } + + public static final ScriptContext CONTEXT = + new ScriptContext<>("test", DeterministicFactoryTestScript.Factory.class); + } + public void testFactory() { FactoryTestScript.Factory factory = scriptEngine.compile("factory_test", "test + params.get('test')", FactoryTestScript.CONTEXT, Collections.emptyMap()); @@ -163,26 +189,26 @@ public void testFactory() { } public void testDeterministic() { - FactoryTestScript.Factory factory = + DeterministicFactoryTestScript.Factory factory = scriptEngine.compile("deterministic_test", "Integer.parseInt('123')", - FactoryTestScript.CONTEXT, Collections.emptyMap()); + DeterministicFactoryTestScript.CONTEXT, Collections.emptyMap()); assertTrue(factory.isResultDeterministic()); assertEquals(123, factory.newInstance(Collections.emptyMap()).execute(0)); } public void testNotDeterministic() { - FactoryTestScript.Factory factory = + DeterministicFactoryTestScript.Factory factory = scriptEngine.compile("not_deterministic_test", "Math.random()", - FactoryTestScript.CONTEXT, Collections.emptyMap()); + DeterministicFactoryTestScript.CONTEXT, Collections.emptyMap()); assertFalse(factory.isResultDeterministic()); Double d = (Double)factory.newInstance(Collections.emptyMap()).execute(0); assertTrue(d >= 0.0 && d <= 1.0); } public void testMixedDeterministicIsNotDeterministic() { - FactoryTestScript.Factory factory = + DeterministicFactoryTestScript.Factory factory = scriptEngine.compile("not_deterministic_test", "Integer.parseInt('123') + Math.random()", - FactoryTestScript.CONTEXT, Collections.emptyMap()); + DeterministicFactoryTestScript.CONTEXT, Collections.emptyMap()); assertFalse(factory.isResultDeterministic()); Double d = (Double)factory.newInstance(Collections.emptyMap()).execute(0); assertTrue(d >= 123.0 && d <= 124.0); @@ -192,7 +218,7 @@ public abstract static class EmptyTestScript { public static final String[] PARAMETERS = {}; public abstract Object execute(); - public interface Factory extends ScriptFactory { + public interface Factory { EmptyTestScript newInstance(); } diff --git a/plugins/examples/script-expert-scoring/src/main/java/org/elasticsearch/example/expertscript/ExpertScriptPlugin.java b/plugins/examples/script-expert-scoring/src/main/java/org/elasticsearch/example/expertscript/ExpertScriptPlugin.java index 5846f000f5e2d..5b9a357cff2f0 100644 --- a/plugins/examples/script-expert-scoring/src/main/java/org/elasticsearch/example/expertscript/ExpertScriptPlugin.java +++ b/plugins/examples/script-expert-scoring/src/main/java/org/elasticsearch/example/expertscript/ExpertScriptPlugin.java @@ -62,7 +62,7 @@ public String getType() { } @Override - public T compile( + public T compile( String scriptName, String scriptSource, ScriptContext context, @@ -92,7 +92,8 @@ public Set> getSupportedContexts() { return Set.of(ScoreScript.CONTEXT); } - private static class PureDfFactory implements ScoreScript.Factory { + private static class PureDfFactory implements ScoreScript.Factory, + ScriptFactory { @Override public boolean isResultDeterministic() { // PureDfLeafFactory only uses deterministic APIs, this diff --git a/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 56161d8f2fa34..12b2da120f63e 100644 --- a/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -323,11 +323,9 @@ public Index index() { } /** Compile script using script service */ - public FactoryType compile(Script script, ScriptContext context) { + public FactoryType compile(Script script, ScriptContext context) { FactoryType factory = scriptService.compile(script, context); - // TODO(stu): can check `factory instanceof ScriptFactory && ((ScriptFactory) factory).isResultDeterministic() == false` - // to avoid being so intrusive - if (factory.isResultDeterministic() == false) { + if (factory instanceof ScriptFactory && ((ScriptFactory) factory).isResultDeterministic() == false) { failIfFrozen(); } return factory; diff --git a/server/src/main/java/org/elasticsearch/script/BucketAggregationSelectorScript.java b/server/src/main/java/org/elasticsearch/script/BucketAggregationSelectorScript.java index 3c765439223d2..a8e2fad7cdcda 100644 --- a/server/src/main/java/org/elasticsearch/script/BucketAggregationSelectorScript.java +++ b/server/src/main/java/org/elasticsearch/script/BucketAggregationSelectorScript.java @@ -48,7 +48,7 @@ public Map getParams() { public abstract boolean execute(); - public interface Factory extends ScriptFactory { + public interface Factory { BucketAggregationSelectorScript newInstance(Map params); } } diff --git a/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java b/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java index 44d87cfe6aba2..27ce29b95dc50 100644 --- a/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java +++ b/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java @@ -45,7 +45,7 @@ public Map getParams() { public abstract boolean execute(Map ctx); - public interface Factory extends ScriptFactory { + public interface Factory { IngestConditionalScript newInstance(Map params); } } diff --git a/server/src/main/java/org/elasticsearch/script/IngestScript.java b/server/src/main/java/org/elasticsearch/script/IngestScript.java index 7104ed7d9b0d2..f357394ed31f0 100644 --- a/server/src/main/java/org/elasticsearch/script/IngestScript.java +++ b/server/src/main/java/org/elasticsearch/script/IngestScript.java @@ -46,7 +46,7 @@ public Map getParams() { public abstract void execute(Map ctx); - public interface Factory extends ScriptFactory { + public interface Factory { IngestScript newInstance(Map params); } } diff --git a/server/src/main/java/org/elasticsearch/script/ScriptContext.java b/server/src/main/java/org/elasticsearch/script/ScriptContext.java index 4e927da09d118..081a26d1e511a 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptContext.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptContext.java @@ -54,7 +54,7 @@ * If the variable name starts with an underscore, for example, {@code _score}, the needs method would * be {@code boolean needs_score()}. */ -public final class ScriptContext { +public final class ScriptContext { /** A unique identifier for this context. */ public final String name; diff --git a/server/src/main/java/org/elasticsearch/script/ScriptEngine.java b/server/src/main/java/org/elasticsearch/script/ScriptEngine.java index 4c38ae5c6e19c..78a012700c2b2 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptEngine.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptEngine.java @@ -42,7 +42,7 @@ public interface ScriptEngine extends Closeable { * @param params compile-time parameters (such as flags to the compiler) * @return A compiled script of the FactoryType from {@link ScriptContext} */ - FactoryType compile( + FactoryType compile( String name, String code, ScriptContext context, diff --git a/server/src/main/java/org/elasticsearch/script/ScriptService.java b/server/src/main/java/org/elasticsearch/script/ScriptService.java index 77375838e7f16..a1788ee74a163 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptService.java @@ -284,7 +284,7 @@ void setMaxCompilationRate(Tuple newRate) { * * @return a compiled script which may be used to construct instances of a script for the given context */ - public FactoryType compile(Script script, ScriptContext context) { + public FactoryType compile(Script script, ScriptContext context) { Objects.requireNonNull(script); Objects.requireNonNull(context); diff --git a/server/src/main/java/org/elasticsearch/script/TemplateScript.java b/server/src/main/java/org/elasticsearch/script/TemplateScript.java index f7cf4590387d8..c053cf2b509d0 100644 --- a/server/src/main/java/org/elasticsearch/script/TemplateScript.java +++ b/server/src/main/java/org/elasticsearch/script/TemplateScript.java @@ -41,7 +41,7 @@ public Map getParams() { /** Run a template and return the resulting string, encoded in utf8 bytes. */ public abstract String execute(); - public interface Factory extends ScriptFactory { + public interface Factory { TemplateScript newInstance(Map params); } diff --git a/server/src/main/java/org/elasticsearch/script/UpdateScript.java b/server/src/main/java/org/elasticsearch/script/UpdateScript.java index 5dd08d5b602dd..ae0827ff83934 100644 --- a/server/src/main/java/org/elasticsearch/script/UpdateScript.java +++ b/server/src/main/java/org/elasticsearch/script/UpdateScript.java @@ -58,7 +58,7 @@ public Map getCtx() { public abstract void execute(); - public interface Factory extends ScriptFactory { + public interface Factory { UpdateScript newInstance(Map params, Map ctx); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/IntervalQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/IntervalQueryBuilderTests.java index cfcc1a9cd3eb0..763d10ddf30e8 100644 --- a/server/src/test/java/org/elasticsearch/index/query/IntervalQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/IntervalQueryBuilderTests.java @@ -35,7 +35,6 @@ import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.ScriptFactory; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.ScriptType; import org.elasticsearch.test.AbstractQueryTestCase; @@ -397,7 +396,7 @@ public boolean execute(Interval interval) { ScriptService scriptService = new ScriptService(Settings.EMPTY, Collections.emptyMap(), Collections.emptyMap()){ @Override @SuppressWarnings("unchecked") - public FactoryType compile(Script script, ScriptContext context) { + public FactoryType compile(Script script, ScriptContext context) { assertEquals(IntervalFilterScript.CONTEXT, context); assertEquals(new Script("interval.start > 3"), script); return (FactoryType) factory; diff --git a/server/src/test/java/org/elasticsearch/script/ScriptContextTests.java b/server/src/test/java/org/elasticsearch/script/ScriptContextTests.java index dc77fb0126262..157b0969ae813 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptContextTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptContextTests.java @@ -23,28 +23,28 @@ public class ScriptContextTests extends ESTestCase { - public interface TwoNewInstance extends ScriptFactory { + public interface TwoNewInstance { String newInstance(int foo, int bar); String newInstance(int foo); - interface StatefulFactory extends ScriptFactory { + interface StatefulFactory { TwoNewInstance newFactory(); } } - public interface TwoNewFactory extends ScriptFactory { + public interface TwoNewFactory { String newFactory(int foo, int bar); String newFactory(int foo); } - public interface MissingNewInstance extends ScriptFactory { + public interface MissingNewInstance { String typoNewInstanceMethod(int foo); } public interface DummyScript { int execute(int foo); - interface Factory extends ScriptFactory { + interface Factory { DummyScript newInstance(); } } @@ -54,7 +54,7 @@ public interface DummyStatefulScript { interface StatefulFactory { DummyStatefulScript newInstance(); } - interface Factory extends ScriptFactory { + interface Factory { StatefulFactory newFactory(); } } diff --git a/server/src/test/java/org/elasticsearch/script/ScriptLanguagesInfoTests.java b/server/src/test/java/org/elasticsearch/script/ScriptLanguagesInfoTests.java index 6e720b480934b..38139103ed2ab 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptLanguagesInfoTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptLanguagesInfoTests.java @@ -75,7 +75,7 @@ private ScriptService getMockScriptService(Settings settings) { } - public interface MiscContext extends ScriptFactory { + public interface MiscContext { void execute(); Object newInstance(); } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java index 406a96e3c34cd..6c2981c4ad79d 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricIT.java @@ -1037,7 +1037,6 @@ public void testEmptyAggregation() throws Exception { * Make sure that a request using a deterministic script gets cached and nondeterministic scripts do not get cached. */ public void testScriptCaching() throws Exception { - // TODO(stu): add non-determinism in init, agg, combine and reduce, ensure not cached Script mapScript = new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "state['count'] = 1", Collections.emptyMap()); Script combineScript = new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "no-op aggregation", Collections.emptyMap()); diff --git a/server/src/test/java/org/elasticsearch/search/functionscore/ExplainableScriptIT.java b/server/src/test/java/org/elasticsearch/search/functionscore/ExplainableScriptIT.java index 2e61882256b77..dda35ff803c1d 100644 --- a/server/src/test/java/org/elasticsearch/search/functionscore/ExplainableScriptIT.java +++ b/server/src/test/java/org/elasticsearch/search/functionscore/ExplainableScriptIT.java @@ -34,7 +34,6 @@ import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptEngine; -import org.elasticsearch.script.ScriptFactory; import org.elasticsearch.script.ScriptType; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.SearchHits; @@ -76,7 +75,7 @@ public String getType() { } @Override - public T compile( + public T compile( String scriptName, String scriptSource, ScriptContext context, diff --git a/server/src/test/java/org/elasticsearch/search/suggest/SuggestSearchIT.java b/server/src/test/java/org/elasticsearch/search/suggest/SuggestSearchIT.java index 5434f05c9aa00..fc2826258a47a 100644 --- a/server/src/test/java/org/elasticsearch/search/suggest/SuggestSearchIT.java +++ b/server/src/test/java/org/elasticsearch/search/suggest/SuggestSearchIT.java @@ -34,7 +34,6 @@ import org.elasticsearch.plugins.ScriptPlugin; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptEngine; -import org.elasticsearch.script.ScriptFactory; import org.elasticsearch.script.TemplateScript; import org.elasticsearch.search.suggest.phrase.DirectCandidateGeneratorBuilder; import org.elasticsearch.search.suggest.phrase.Laplace; @@ -1138,7 +1137,7 @@ public String getType() { } @Override - public T compile( + public T compile( String scriptName, String scriptSource, ScriptContext context, diff --git a/test/framework/src/main/java/org/elasticsearch/ingest/TestTemplateService.java b/test/framework/src/main/java/org/elasticsearch/ingest/TestTemplateService.java index b5fcb2a37d7e3..5bbf39d8fdc17 100644 --- a/test/framework/src/main/java/org/elasticsearch/ingest/TestTemplateService.java +++ b/test/framework/src/main/java/org/elasticsearch/ingest/TestTemplateService.java @@ -23,7 +23,6 @@ import org.elasticsearch.script.MockScriptEngine; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.ScriptFactory; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.TemplateScript; @@ -49,7 +48,7 @@ private TestTemplateService(boolean compilationException) { } @Override - public FactoryType compile(Script script, ScriptContext context) { + public FactoryType compile(Script script, ScriptContext context) { if (this.compilationException) { throw new RuntimeException("could not compile script"); } else { diff --git a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java index 3069efbebb451..ab34dcbf09bf5 100644 --- a/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java +++ b/test/framework/src/main/java/org/elasticsearch/script/MockScriptEngine.java @@ -93,7 +93,7 @@ public String getType() { } @Override - public T compile(String name, String source, ScriptContext context, Map params) { + public T compile(String name, String source, ScriptContext context, Map params) { // Scripts are always resolved using the script's source. For inline scripts, it's easy because they don't have names and the // source is always provided. For stored and file scripts, the source of the script must match the key of a predefined script. MockDeterministicScript script = scripts.get(source); @@ -252,7 +252,6 @@ public double execute(Map params1, double[] values) { @Override public Set> getSupportedContexts() { - // TODO(stu): make part of `compile()` return Set.of( FieldScript.CONTEXT, TermsSetQueryScript.CONTEXT, @@ -395,7 +394,7 @@ public double execute(Query query, Field field, Term term) { } } - public static class MockMetricAggInitScriptFactory implements ScriptedMetricAggContexts.InitScript.Factory, ScriptFactory { + public static class MockMetricAggInitScriptFactory implements ScriptedMetricAggContexts.InitScript.Factory { private final MockDeterministicScript script; MockMetricAggInitScriptFactory(MockDeterministicScript script) { this.script = script; } @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } @@ -428,7 +427,7 @@ public void execute() { } } - public static class MockMetricAggMapScriptFactory implements ScriptedMetricAggContexts.MapScript.Factory, ScriptFactory { + public static class MockMetricAggMapScriptFactory implements ScriptedMetricAggContexts.MapScript.Factory { private final MockDeterministicScript script; MockMetricAggMapScriptFactory(MockDeterministicScript script) { this.script = script; } @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } @@ -476,7 +475,7 @@ public void execute() { } } - public static class MockMetricAggCombineScriptFactory implements ScriptedMetricAggContexts.CombineScript.Factory, ScriptFactory { + public static class MockMetricAggCombineScriptFactory implements ScriptedMetricAggContexts.CombineScript.Factory { private final MockDeterministicScript script; MockMetricAggCombineScriptFactory(MockDeterministicScript script) { this.script = script; } @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } @@ -508,7 +507,7 @@ public Object execute() { } } - public static class MockMetricAggReduceScriptFactory implements ScriptedMetricAggContexts.ReduceScript.Factory, ScriptFactory { + public static class MockMetricAggReduceScriptFactory implements ScriptedMetricAggContexts.ReduceScript.Factory { private final MockDeterministicScript script; MockMetricAggReduceScriptFactory(MockDeterministicScript script) { this.script = script; } @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } @@ -584,7 +583,7 @@ public void setScorer(Scorable scorer) { } } - class MockAggregationScript implements AggregationScript.Factory, ScriptFactory { + class MockAggregationScript implements AggregationScript.Factory { private final MockDeterministicScript script; MockAggregationScript(MockDeterministicScript script) { this.script = script; } @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } @@ -615,7 +614,7 @@ public boolean needs_score() { } } - class MockSignificantTermsHeuristicScoreScript implements SignificantTermsHeuristicScoreScript.Factory, ScriptFactory { + class MockSignificantTermsHeuristicScoreScript implements SignificantTermsHeuristicScoreScript.Factory { private final MockDeterministicScript script; MockSignificantTermsHeuristicScoreScript(MockDeterministicScript script) { this.script = script; } @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } @@ -631,7 +630,7 @@ public double execute(Map vars) { } } - class MockFieldScriptFactory implements FieldScript.Factory, ScriptFactory { + class MockFieldScriptFactory implements FieldScript.Factory { private final MockDeterministicScript script; MockFieldScriptFactory(MockDeterministicScript script) { this.script = script; } @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } @@ -650,7 +649,7 @@ public Object execute() { } } - class MockStringSortScriptFactory implements StringSortScript.Factory, ScriptFactory { + class MockStringSortScriptFactory implements StringSortScript.Factory { private final MockDeterministicScript script; MockStringSortScriptFactory(MockDeterministicScript script) { this.script = script; } @Override public boolean isResultDeterministic() { return script.isResultDeterministic(); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/script/MockMustacheScriptEngine.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/script/MockMustacheScriptEngine.java index 16a95f0accdac..4f9b125a9fd6b 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/script/MockMustacheScriptEngine.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/script/MockMustacheScriptEngine.java @@ -39,7 +39,7 @@ public String getType() { } @Override - public T compile(String name, String script, ScriptContext context, Map params) { + public T compile(String name, String script, ScriptContext context, Map params) { if (script.contains("{{") && script.contains("}}")) { throw new IllegalArgumentException("Fix your test to not rely on mustache"); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/monitoring/test/MockPainlessScriptEngine.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/monitoring/test/MockPainlessScriptEngine.java index 254542f355e5d..2052cebe1d0e9 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/monitoring/test/MockPainlessScriptEngine.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/monitoring/test/MockPainlessScriptEngine.java @@ -11,7 +11,6 @@ import org.elasticsearch.script.ScoreScript; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptEngine; -import org.elasticsearch.script.ScriptFactory; import java.util.Collection; import java.util.Collections; @@ -43,7 +42,7 @@ public String getType() { } @Override - public T compile(String name, String script, ScriptContext context, Map options) { + public T compile(String name, String script, ScriptContext context, Map options) { if (context.instanceClazz.equals(ScoreScript.class)) { return context.factoryClazz.cast(new MockScoreScript(p -> 0.0)); } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/WatcherConditionScript.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/WatcherConditionScript.java index f5376aa424692..1a5c8718bbd45 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/WatcherConditionScript.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/WatcherConditionScript.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.watcher.condition; import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.ScriptFactory; import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext; import org.elasticsearch.xpack.watcher.support.Variables; @@ -38,7 +37,7 @@ public Map getCtx() { return ctx; } - public interface Factory extends ScriptFactory { + public interface Factory { WatcherConditionScript newInstance(Map params, WatchExecutionContext watcherContext); } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transform/script/WatcherTransformScript.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transform/script/WatcherTransformScript.java index 3ef1f87cd608c..57ee1e9f35c5d 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transform/script/WatcherTransformScript.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transform/script/WatcherTransformScript.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.watcher.transform.script; import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.script.ScriptFactory; import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext; import org.elasticsearch.xpack.core.watcher.watch.Payload; import org.elasticsearch.xpack.watcher.support.Variables; @@ -39,7 +38,7 @@ public Map getCtx() { return ctx; } - public interface Factory extends ScriptFactory { + public interface Factory { WatcherTransformScript newInstance(Map params, WatchExecutionContext watcherContext, Payload payload); }