From ecf6101798b053a910321ab90e4c45d0c5e87532 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 28 Jun 2016 13:20:00 -0700 Subject: [PATCH] Scripts: Remove ClusterState from compile api Stored scripts are pulled from the cluster state, and the current api requires passing the ClusterState on each call to compile. However, this means every user of the ScriptService needs to depend on the ClusterService. Instead, this change makes the ScriptService a ClusterStateListener. It also simplifies tests a lot, as they no longer need to create fake cluster states (except when testing stored scripts). --- .../action/update/UpdateHelper.java | 3 +- .../index/query/InnerHitBuilder.java | 2 +- .../index/query/ScriptQueryBuilder.java | 6 +-- .../index/query/TemplateQueryBuilder.java | 2 +- .../ScriptScoreFunctionBuilder.java | 2 +- .../ingest/InternalTemplateService.java | 3 +- .../java/org/elasticsearch/node/Node.java | 1 + .../elasticsearch/script/ScriptService.java | 31 +++++++---- .../elasticsearch/search/SearchService.java | 2 +- .../heuristics/ScriptHeuristic.java | 8 +-- .../scripted/InternalScriptedMetric.java | 2 +- .../scripted/ScriptedMetricAggregator.java | 6 +-- .../tophits/TopHitsAggregatorFactory.java | 2 +- .../BucketScriptPipelineAggregator.java | 2 +- .../BucketSelectorPipelineAggregator.java | 2 +- .../ValuesSourceAggregationBuilder.java | 3 +- .../search/sort/ScriptSortBuilder.java | 2 +- .../phrase/PhraseSuggestionBuilder.java | 2 +- .../elasticsearch/script/FileScriptTests.java | 4 +- .../script/NativeScriptTests.java | 21 ++++---- .../script/ScriptContextTests.java | 14 ++--- .../script/ScriptServiceTests.java | 53 ++++++++----------- .../search/sort/AbstractSortTestCase.java | 2 +- .../ingest/common/IngestCommonPlugin.java | 2 +- .../ingest/common/ScriptProcessor.java | 19 +++---- .../common/ScriptProcessorFactoryTests.java | 2 +- .../ingest/common/ScriptProcessorTests.java | 12 ++--- .../TransportSearchTemplateAction.java | 6 +-- .../AbstractAsyncBulkIndexByScrollAction.java | 6 +-- .../index/reindex/TransportReindexAction.java | 6 +-- .../reindex/TransportUpdateByQueryAction.java | 6 +-- 31 files changed, 108 insertions(+), 126 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java b/core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java index 0c9c1c67978..e2a035d3b5c 100644 --- a/core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java +++ b/core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java @@ -251,8 +251,7 @@ public class UpdateHelper extends AbstractComponent { private Map executeScript(Script script, Map ctx) { try { if (scriptService != null) { - ClusterState state = clusterService.state(); - ExecutableScript executableScript = scriptService.executable(script, ScriptContext.Standard.UPDATE, Collections.emptyMap(), state); + ExecutableScript executableScript = scriptService.executable(script, ScriptContext.Standard.UPDATE, Collections.emptyMap()); executableScript.setNextVar("ctx", ctx); executableScript.run(); // we need to unwrap the ctx... diff --git a/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java b/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java index a017159b451..b54d7713cd3 100644 --- a/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java @@ -504,7 +504,7 @@ public final class InnerHitBuilder extends ToXContentToBytes implements Writeabl if (scriptFields != null) { for (ScriptField field : scriptFields) { SearchScript searchScript = innerHitsContext.scriptService().search(innerHitsContext.lookup(), field.script(), - ScriptContext.Standard.SEARCH, Collections.emptyMap(), context.getClusterState()); + ScriptContext.Standard.SEARCH, Collections.emptyMap()); innerHitsContext.scriptFields().add(new org.elasticsearch.search.fetch.script.ScriptFieldsContext.ScriptField( field.fieldName(), searchScript, field.ignoreFailure())); } diff --git a/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java index 778501f0132..f0a50985b58 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java @@ -157,7 +157,7 @@ public class ScriptQueryBuilder extends AbstractQueryBuilder @Override protected Query doToQuery(QueryShardContext context) throws IOException { - return new ScriptQuery(script, context.getScriptService(), context.lookup(), context.getClusterState()); + return new ScriptQuery(script, context.getScriptService(), context.lookup()); } static class ScriptQuery extends Query { @@ -166,9 +166,9 @@ public class ScriptQueryBuilder extends AbstractQueryBuilder private final SearchScript searchScript; - public ScriptQuery(Script script, ScriptService scriptService, SearchLookup searchLookup, ClusterState state) { + public ScriptQuery(Script script, ScriptService scriptService, SearchLookup searchLookup) { this.script = script; - this.searchScript = scriptService.search(searchLookup, script, ScriptContext.Standard.SEARCH, Collections.emptyMap(), state); + this.searchScript = scriptService.search(searchLookup, script, ScriptContext.Standard.SEARCH, Collections.emptyMap()); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java index 3b41abbbe82..3b47378d1e8 100644 --- a/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java @@ -177,7 +177,7 @@ public class TemplateQueryBuilder extends AbstractQueryBuilder model) { diff --git a/core/src/main/java/org/elasticsearch/node/Node.java b/core/src/main/java/org/elasticsearch/node/Node.java index 675d29e9991..671164e5fff 100644 --- a/core/src/main/java/org/elasticsearch/node/Node.java +++ b/core/src/main/java/org/elasticsearch/node/Node.java @@ -246,6 +246,7 @@ public class Node implements Closeable { resourcesToClose.add(resourceWatcherService); final NetworkService networkService = new NetworkService(settings); final ClusterService clusterService = new ClusterService(settings, settingsModule.getClusterSettings(), threadPool); + clusterService.add(scriptModule.getScriptService()); resourcesToClose.add(clusterService); final TribeService tribeService = new TribeService(settings, clusterService); resourcesToClose.add(tribeService); diff --git a/core/src/main/java/org/elasticsearch/script/ScriptService.java b/core/src/main/java/org/elasticsearch/script/ScriptService.java index 57d46255e19..94b4680e4d5 100644 --- a/core/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/core/src/main/java/org/elasticsearch/script/ScriptService.java @@ -28,7 +28,9 @@ import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptReque import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest; import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptResponse; import org.elasticsearch.cluster.AckedClusterStateUpdateTask; +import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateListener; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.ParseField; @@ -75,7 +77,7 @@ import java.util.concurrent.ConcurrentMap; import static java.util.Collections.unmodifiableMap; -public class ScriptService extends AbstractComponent implements Closeable { +public class ScriptService extends AbstractComponent implements Closeable, ClusterStateListener { static final String DISABLE_DYNAMIC_SCRIPTING_SETTING = "script.disable_dynamic"; @@ -105,6 +107,8 @@ public class ScriptService extends AbstractComponent implements Closeable { private final ParseFieldMatcher parseFieldMatcher; private final ScriptMetrics scriptMetrics = new ScriptMetrics(); + private ClusterState clusterState; + /** * @deprecated Use {@link org.elasticsearch.script.Script.ScriptField} instead. This should be removed in * 2.0 @@ -217,7 +221,7 @@ public class ScriptService extends AbstractComponent implements Closeable { /** * Checks if a script can be executed and compiles it if needed, or returns the previously compiled and cached script. */ - public CompiledScript compile(Script script, ScriptContext scriptContext, Map params, ClusterState state) { + public CompiledScript compile(Script script, ScriptContext scriptContext, Map params) { if (script == null) { throw new IllegalArgumentException("The parameter script (Script) must not be null."); } @@ -245,14 +249,14 @@ public class ScriptService extends AbstractComponent implements Closeable { " operation [" + scriptContext.getKey() + "] and lang [" + lang + "] are not supported"); } - return compileInternal(script, params, state); + return compileInternal(script, params); } /** * Compiles a script straight-away, or returns the previously compiled and cached script, * without checking if it can be executed based on settings. */ - CompiledScript compileInternal(Script script, Map params, ClusterState state) { + CompiledScript compileInternal(Script script, Map params) { if (script == null) { throw new IllegalArgumentException("The parameter script (Script) must not be null."); } @@ -289,7 +293,7 @@ public class ScriptService extends AbstractComponent implements Closeable { //the script has been updated in the index since the last look up. final IndexedScript indexedScript = new IndexedScript(lang, name); name = indexedScript.id; - code = getScriptFromClusterState(state, indexedScript.lang, indexedScript.id); + code = getScriptFromClusterState(indexedScript.lang, indexedScript.id); } CacheKey cacheKey = new CacheKey(scriptEngineService, type == ScriptType.INLINE ? null : name, code, params); @@ -328,9 +332,9 @@ public class ScriptService extends AbstractComponent implements Closeable { return scriptLang; } - String getScriptFromClusterState(ClusterState state, String scriptLang, String id) { + String getScriptFromClusterState(String scriptLang, String id) { scriptLang = validateScriptLanguage(scriptLang); - ScriptMetaData scriptMetadata = state.metaData().custom(ScriptMetaData.TYPE); + ScriptMetaData scriptMetadata = clusterState.metaData().custom(ScriptMetaData.TYPE); if (scriptMetadata == null) { throw new ResourceNotFoundException("Unable to find script [" + scriptLang + "/" + id + "] in cluster state"); } @@ -443,8 +447,8 @@ public class ScriptService extends AbstractComponent implements Closeable { /** * Compiles (or retrieves from cache) and executes the provided script */ - public ExecutableScript executable(Script script, ScriptContext scriptContext, Map params, ClusterState state) { - return executable(compile(script, scriptContext, params, state), script.getParams()); + public ExecutableScript executable(Script script, ScriptContext scriptContext, Map params) { + return executable(compile(script, scriptContext, params), script.getParams()); } /** @@ -457,8 +461,8 @@ public class ScriptService extends AbstractComponent implements Closeable { /** * Compiles (or retrieves from cache) and executes the provided search script */ - public SearchScript search(SearchLookup lookup, Script script, ScriptContext scriptContext, Map params, ClusterState state) { - CompiledScript compiledScript = compile(script, scriptContext, params, state); + public SearchScript search(SearchLookup lookup, Script script, ScriptContext scriptContext, Map params) { + CompiledScript compiledScript = compile(script, scriptContext, params); return getScriptEngineServiceForLang(compiledScript.lang()).search(compiledScript, lookup, script.getParams()); } @@ -496,6 +500,11 @@ public class ScriptService extends AbstractComponent implements Closeable { } } + @Override + public void clusterChanged(ClusterChangedEvent event) { + clusterState = event.state(); + } + /** * A small listener for the script cache that calls each * {@code ScriptEngineService}'s {@code scriptRemoved} method when the diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index 96fee4b968f..3c17a506eb9 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -740,7 +740,7 @@ public class SearchService extends AbstractLifecycleComponent imp if (source.scriptFields() != null) { for (org.elasticsearch.search.builder.SearchSourceBuilder.ScriptField field : source.scriptFields()) { SearchScript searchScript = context.scriptService().search(context.lookup(), field.script(), ScriptContext.Standard.SEARCH, - Collections.emptyMap(), context.getQueryShardContext().getClusterState()); + Collections.emptyMap()); context.scriptFields().add(new ScriptField(field.fieldName(), searchScript, field.ignoreFailure())); } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java index 22fce0113e9..19673bf6e9d 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java @@ -79,16 +79,16 @@ public class ScriptHeuristic extends SignificanceHeuristic { @Override public void initialize(InternalAggregation.ReduceContext context) { - initialize(context.scriptService(), context.clusterState()); + initialize(context.scriptService()); } @Override public void initialize(SearchContext context) { - initialize(context.scriptService(), context.getQueryShardContext().getClusterState()); + initialize(context.scriptService()); } - public void initialize(ScriptService scriptService, ClusterState state) { - searchScript = scriptService.executable(script, ScriptContext.Standard.AGGS, Collections.emptyMap(), state); + public void initialize(ScriptService scriptService) { + searchScript = scriptService.executable(script, ScriptContext.Standard.AGGS, Collections.emptyMap()); searchScript.setNextVar("_subset_freq", subsetDfHolder); searchScript.setNextVar("_subset_size", subsetSizeHolder); searchScript.setNextVar("_superset_freq", supersetDfHolder); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetric.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetric.java index d6814871108..2c09d58afdb 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetric.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetric.java @@ -92,7 +92,7 @@ public class InternalScriptedMetric extends InternalMetricsAggregation implement vars.putAll(firstAggregation.reduceScript.getParams()); } CompiledScript compiledScript = reduceContext.scriptService().compile(firstAggregation.reduceScript, - ScriptContext.Standard.AGGS, Collections.emptyMap(), reduceContext.clusterState()); + ScriptContext.Standard.AGGS, Collections.emptyMap()); ExecutableScript script = reduceContext.scriptService().executable(compiledScript, vars); aggregation = script.run(); } else { diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java index 053b0e82d69..44708eba409 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java @@ -54,11 +54,11 @@ public class ScriptedMetricAggregator extends MetricsAggregator { ScriptService scriptService = context.searchContext().scriptService(); ClusterState state = context.searchContext().getQueryShardContext().getClusterState(); if (initScript != null) { - scriptService.executable(initScript, ScriptContext.Standard.AGGS, Collections.emptyMap(), state).run(); + scriptService.executable(initScript, ScriptContext.Standard.AGGS, Collections.emptyMap()).run(); } - this.mapScript = scriptService.search(context.searchContext().lookup(), mapScript, ScriptContext.Standard.AGGS, Collections.emptyMap(), state); + this.mapScript = scriptService.search(context.searchContext().lookup(), mapScript, ScriptContext.Standard.AGGS, Collections.emptyMap()); if (combineScript != null) { - this.combineScript = scriptService.executable(combineScript, ScriptContext.Standard.AGGS, Collections.emptyMap(), state); + this.combineScript = scriptService.executable(combineScript, ScriptContext.Standard.AGGS, Collections.emptyMap()); } else { this.combineScript = null; } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java index ac001222301..65a8c24eb08 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java @@ -106,7 +106,7 @@ public class TopHitsAggregatorFactory extends AggregatorFactory buckets = originalAgg.getBuckets(); CompiledScript compiledScript = reduceContext.scriptService().compile(script, ScriptContext.Standard.AGGS, - Collections.emptyMap(), reduceContext.clusterState()); + Collections.emptyMap()); List newBuckets = new ArrayList<>(); for (Bucket bucket : buckets) { Map vars = new HashMap<>(); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketselector/BucketSelectorPipelineAggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketselector/BucketSelectorPipelineAggregator.java index ed61b881d56..28fded6f559 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketselector/BucketSelectorPipelineAggregator.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketselector/BucketSelectorPipelineAggregator.java @@ -90,7 +90,7 @@ public class BucketSelectorPipelineAggregator extends PipelineAggregator { List buckets = originalAgg.getBuckets(); CompiledScript compiledScript = reduceContext.scriptService().compile(script, ScriptContext.Standard.AGGS, - Collections.emptyMap(), reduceContext.clusterState()); + Collections.emptyMap()); List newBuckets = new ArrayList<>(); for (Bucket bucket : buckets) { Map vars = new HashMap<>(); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceAggregationBuilder.java index cd03ceb9eca..a55b9035c03 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceAggregationBuilder.java @@ -377,8 +377,7 @@ public abstract class ValuesSourceAggregationBuilder { @Override public SortFieldAndFormat build(QueryShardContext context) throws IOException { final SearchScript searchScript = context.getScriptService().search( - context.lookup(), script, ScriptContext.Standard.SEARCH, Collections.emptyMap(), context.getClusterState()); + context.lookup(), script, ScriptContext.Standard.SEARCH, Collections.emptyMap()); MultiValueMode valueMode = null; if (sortMode != null) { diff --git a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java index 4ff74fb08df..d3ddbeb2f1a 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java @@ -631,7 +631,7 @@ public class PhraseSuggestionBuilder extends SuggestionBuilder> scriptSettings = scriptModule.getSettings(); scriptSettings.add(InternalSettingsPlugin.VERSION_CREATED); - ClusterState state = ClusterState.builder(new ClusterName("_name")).build(); ExecutableScript executable = scriptModule.getScriptService().executable( new Script("my", ScriptType.INLINE, NativeScriptEngineService.NAME, null), ScriptContext.Standard.SEARCH, - Collections.emptyMap(), state); + Collections.emptyMap()); assertThat(executable.run().toString(), equalTo("test")); } @@ -85,7 +82,7 @@ public class NativeScriptTests extends ESTestCase { for (ScriptContext scriptContext : scriptContextRegistry.scriptContexts()) { assertThat(scriptService.compile(new Script("my", ScriptType.INLINE, NativeScriptEngineService.NAME, null), scriptContext, - Collections.emptyMap(), null), notNullValue()); + Collections.emptyMap()), notNullValue()); } } diff --git a/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java b/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java index 57bdfff3f7f..c7ee421e7e0 100644 --- a/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java +++ b/core/src/test/java/org/elasticsearch/script/ScriptContextTests.java @@ -59,7 +59,7 @@ public class ScriptContextTests extends ESTestCase { for (ScriptService.ScriptType scriptType : ScriptService.ScriptType.values()) { try { Script script = new Script("1", scriptType, MockScriptEngine.NAME, null); - scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_globally_disabled_op"), Collections.emptyMap(), null); + scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_globally_disabled_op"), Collections.emptyMap()); fail("script compilation should have been rejected"); } catch (IllegalStateException e) { assertThat(e.getMessage(), containsString("scripts of type [" + scriptType + "], operation [" + PLUGIN_NAME + "_custom_globally_disabled_op] and lang [" + MockScriptEngine.NAME + "] are disabled")); @@ -71,16 +71,16 @@ public class ScriptContextTests extends ESTestCase { ScriptService scriptService = makeScriptService(); Script script = new Script("1", ScriptService.ScriptType.INLINE, MockScriptEngine.NAME, null); try { - scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_exp_disabled_op"), Collections.emptyMap(), null); + scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_exp_disabled_op"), Collections.emptyMap()); fail("script compilation should have been rejected"); } catch (IllegalStateException e) { assertTrue(e.getMessage(), e.getMessage().contains("scripts of type [inline], operation [" + PLUGIN_NAME + "_custom_exp_disabled_op] and lang [" + MockScriptEngine.NAME + "] are disabled")); } // still works for other script contexts - assertNotNull(scriptService.compile(script, ScriptContext.Standard.AGGS, Collections.emptyMap(), null)); - assertNotNull(scriptService.compile(script, ScriptContext.Standard.SEARCH, Collections.emptyMap(), null)); - assertNotNull(scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_op"), Collections.emptyMap(), null)); + assertNotNull(scriptService.compile(script, ScriptContext.Standard.AGGS, Collections.emptyMap())); + assertNotNull(scriptService.compile(script, ScriptContext.Standard.SEARCH, Collections.emptyMap())); + assertNotNull(scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "custom_op"), Collections.emptyMap())); } public void testUnknownPluginScriptContext() throws Exception { @@ -88,7 +88,7 @@ public class ScriptContextTests extends ESTestCase { for (ScriptService.ScriptType scriptType : ScriptService.ScriptType.values()) { try { Script script = new Script("1", scriptType, MockScriptEngine.NAME, null); - scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "unknown"), Collections.emptyMap(), null); + scriptService.compile(script, new ScriptContext.Plugin(PLUGIN_NAME, "unknown"), Collections.emptyMap()); fail("script compilation should have been rejected"); } catch (IllegalArgumentException e) { assertTrue(e.getMessage(), e.getMessage().contains("script context [" + PLUGIN_NAME + "_unknown] not supported")); @@ -107,7 +107,7 @@ public class ScriptContextTests extends ESTestCase { for (ScriptService.ScriptType scriptType : ScriptService.ScriptType.values()) { try { Script script = new Script("1", scriptType, MockScriptEngine.NAME, null); - scriptService.compile(script, context, Collections.emptyMap(), null); + scriptService.compile(script, context, Collections.emptyMap()); fail("script compilation should have been rejected"); } catch (IllegalArgumentException e) { assertTrue(e.getMessage(), e.getMessage().contains("script context [test] not supported")); diff --git a/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java index 597cb7e6032..43224488480 100644 --- a/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java +++ b/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java @@ -114,9 +114,10 @@ public class ScriptServiceTests extends ESTestCase { private void buildScriptService(Settings additionalSettings) throws IOException { Settings finalSettings = Settings.builder().put(baseSettings).put(additionalSettings).build(); Environment environment = new Environment(finalSettings); + // TODO: scriptService = new ScriptService(finalSettings, environment, resourceWatcherService, scriptEngineRegistry, scriptContextRegistry, scriptSettings) { @Override - String getScriptFromClusterState(ClusterState state, String scriptLang, String id) { + String getScriptFromClusterState(String scriptLang, String id) { //mock the script that gets retrieved from an index return "100"; } @@ -141,7 +142,7 @@ public class ScriptServiceTests extends ESTestCase { resourceWatcherService.notifyNow(); CompiledScript compiledScript = scriptService.compile(new Script("test_script", ScriptType.FILE, "test", null), - ScriptContext.Standard.SEARCH, Collections.emptyMap(), emptyClusterState()); + ScriptContext.Standard.SEARCH, Collections.emptyMap()); assertThat(compiledScript.compiled(), equalTo((Object) "compiled_test_file")); Files.delete(testFileNoExt); @@ -150,7 +151,7 @@ public class ScriptServiceTests extends ESTestCase { try { scriptService.compile(new Script("test_script", ScriptType.FILE, "test", null), ScriptContext.Standard.SEARCH, - Collections.emptyMap(), emptyClusterState()); + Collections.emptyMap()); fail("the script test_script should no longer exist"); } catch (IllegalArgumentException ex) { assertThat(ex.getMessage(), containsString("Unable to find on disk file script [test_script] using lang [test]")); @@ -168,7 +169,7 @@ public class ScriptServiceTests extends ESTestCase { resourceWatcherService.notifyNow(); CompiledScript compiledScript = scriptService.compile(new Script("file_script", ScriptType.FILE, "test", null), - ScriptContext.Standard.SEARCH, Collections.emptyMap(), emptyClusterState()); + ScriptContext.Standard.SEARCH, Collections.emptyMap()); assertThat(compiledScript.compiled(), equalTo((Object) "compiled_test_file_script")); Files.delete(testHiddenFile); @@ -179,9 +180,9 @@ public class ScriptServiceTests extends ESTestCase { public void testInlineScriptCompiledOnceCache() throws IOException { buildScriptService(Settings.EMPTY); CompiledScript compiledScript1 = scriptService.compile(new Script("1+1", ScriptType.INLINE, "test", null), - randomFrom(scriptContexts), Collections.emptyMap(), emptyClusterState()); + randomFrom(scriptContexts), Collections.emptyMap()); CompiledScript compiledScript2 = scriptService.compile(new Script("1+1", ScriptType.INLINE, "test", null), - randomFrom(scriptContexts), Collections.emptyMap(), emptyClusterState()); + randomFrom(scriptContexts), Collections.emptyMap()); assertThat(compiledScript1.compiled(), sameInstance(compiledScript2.compiled())); } @@ -304,7 +305,7 @@ public class ScriptServiceTests extends ESTestCase { String type = scriptEngineService.getType(); try { scriptService.compile(new Script("test", randomFrom(ScriptType.values()), type, null), new ScriptContext.Plugin( - pluginName, unknownContext), Collections.emptyMap(), emptyClusterState()); + pluginName, unknownContext), Collections.emptyMap()); fail("script compilation should have been rejected"); } catch(IllegalArgumentException e) { assertThat(e.getMessage(), containsString("script context [" + pluginName + "_" + unknownContext + "] not supported")); @@ -314,22 +315,20 @@ public class ScriptServiceTests extends ESTestCase { public void testCompileCountedInCompilationStats() throws IOException { buildScriptService(Settings.EMPTY); scriptService.compile(new Script("1+1", ScriptType.INLINE, "test", null), randomFrom(scriptContexts), - Collections.emptyMap(), emptyClusterState()); + Collections.emptyMap()); assertEquals(1L, scriptService.stats().getCompilations()); } public void testExecutableCountedInCompilationStats() throws IOException { buildScriptService(Settings.EMPTY); - ClusterState state = ClusterState.builder(new ClusterName("_name")).build(); - scriptService.executable(new Script("1+1", ScriptType.INLINE, "test", null), randomFrom(scriptContexts), Collections.emptyMap(), state); + scriptService.executable(new Script("1+1", ScriptType.INLINE, "test", null), randomFrom(scriptContexts), Collections.emptyMap()); assertEquals(1L, scriptService.stats().getCompilations()); } public void testSearchCountedInCompilationStats() throws IOException { buildScriptService(Settings.EMPTY); - ClusterState state = ClusterState.builder(new ClusterName("_name")).build(); scriptService.search(null, new Script("1+1", ScriptType.INLINE, "test", null), randomFrom(scriptContexts), - Collections.emptyMap(), state); + Collections.emptyMap()); assertEquals(1L, scriptService.stats().getCompilations()); } @@ -339,7 +338,7 @@ public class ScriptServiceTests extends ESTestCase { for (int i = 0; i < numberOfCompilations; i++) { scriptService .compile(new Script(i + " + " + i, ScriptType.INLINE, "test", null), randomFrom(scriptContexts), - Collections.emptyMap(), emptyClusterState()); + Collections.emptyMap()); } assertEquals(numberOfCompilations, scriptService.stats().getCompilations()); } @@ -349,9 +348,8 @@ public class ScriptServiceTests extends ESTestCase { builder.put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getKey(), 1); builder.put("script.inline", "true"); buildScriptService(builder.build()); - ClusterState state = ClusterState.builder(new ClusterName("_name")).build(); - scriptService.executable(new Script("1+1", ScriptType.INLINE, "test", null), randomFrom(scriptContexts), Collections.emptyMap(), state); - scriptService.executable(new Script("1+1", ScriptType.INLINE, "test", null), randomFrom(scriptContexts), Collections.emptyMap(), state); + scriptService.executable(new Script("1+1", ScriptType.INLINE, "test", null), randomFrom(scriptContexts), Collections.emptyMap()); + scriptService.executable(new Script("1+1", ScriptType.INLINE, "test", null), randomFrom(scriptContexts), Collections.emptyMap()); assertEquals(1L, scriptService.stats().getCompilations()); } @@ -359,14 +357,14 @@ public class ScriptServiceTests extends ESTestCase { buildScriptService(Settings.EMPTY); createFileScripts("test"); scriptService.compile(new Script("file_script", ScriptType.FILE, "test", null), randomFrom(scriptContexts), - Collections.emptyMap(), emptyClusterState()); + Collections.emptyMap()); assertEquals(1L, scriptService.stats().getCompilations()); } public void testIndexedScriptCountedInCompilationStats() throws IOException { buildScriptService(Settings.EMPTY); scriptService.compile(new Script("script", ScriptType.STORED, "test", null), randomFrom(scriptContexts), - Collections.emptyMap(), emptyClusterState()); + Collections.emptyMap()); assertEquals(1L, scriptService.stats().getCompilations()); } @@ -375,9 +373,8 @@ public class ScriptServiceTests extends ESTestCase { builder.put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getKey(), 1); builder.put("script.inline", "true"); buildScriptService(builder.build()); - ClusterState state = ClusterState.builder(new ClusterName("_name")).build(); - scriptService.executable(new Script("1+1", ScriptType.INLINE, "test", null), randomFrom(scriptContexts), Collections.emptyMap(), state); - scriptService.executable(new Script("2+2", ScriptType.INLINE, "test", null), randomFrom(scriptContexts), Collections.emptyMap(), state); + scriptService.executable(new Script("1+1", ScriptType.INLINE, "test", null), randomFrom(scriptContexts), Collections.emptyMap()); + scriptService.executable(new Script("2+2", ScriptType.INLINE, "test", null), randomFrom(scriptContexts), Collections.emptyMap()); assertEquals(2L, scriptService.stats().getCompilations()); assertEquals(1L, scriptService.stats().getCacheEvictions()); } @@ -388,7 +385,7 @@ public class ScriptServiceTests extends ESTestCase { builder.put("script.inline", "true"); buildScriptService(builder.build()); CompiledScript script = scriptService.compile(new Script("1 + 1", ScriptType.INLINE, null, null), - randomFrom(scriptContexts), Collections.emptyMap(), emptyClusterState()); + randomFrom(scriptContexts), Collections.emptyMap()); assertEquals(script.lang(), "test"); } @@ -469,7 +466,7 @@ public class ScriptServiceTests extends ESTestCase { private void assertCompileRejected(String lang, String script, ScriptType scriptType, ScriptContext scriptContext) { try { - scriptService.compile(new Script(script, scriptType, lang, null), scriptContext, Collections.emptyMap(), emptyClusterState()); + scriptService.compile(new Script(script, scriptType, lang, null), scriptContext, Collections.emptyMap()); fail("compile should have been rejected for lang [" + lang + "], script_type [" + scriptType + "], scripted_op [" + scriptContext + "]"); } catch(IllegalStateException e) { //all good @@ -477,9 +474,8 @@ public class ScriptServiceTests extends ESTestCase { } private void assertCompileAccepted(String lang, String script, ScriptType scriptType, ScriptContext scriptContext) { - ClusterState state = emptyClusterState(); assertThat( - scriptService.compile(new Script(script, scriptType, lang, null), scriptContext, Collections.emptyMap(), state), + scriptService.compile(new Script(script, scriptType, lang, null), scriptContext, Collections.emptyMap()), notNullValue() ); } @@ -528,8 +524,6 @@ public class ScriptServiceTests extends ESTestCase { public static final String NAME = "dtest"; - public static final List EXTENSIONS = Collections.unmodifiableList(Arrays.asList("dtest")); - @Override public String getType() { return NAME; @@ -559,9 +553,4 @@ public class ScriptServiceTests extends ESTestCase { public void close() { } } - - private static ClusterState emptyClusterState() { - return ClusterState.builder(new ClusterName("_name")).build(); - } - } diff --git a/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java b/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java index 2ac7fd68e2e..bdd5c76534c 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java +++ b/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java @@ -100,7 +100,7 @@ public abstract class AbstractSortTestCase> extends EST scriptService = new ScriptService(baseSettings, environment, new ResourceWatcherService(baseSettings, null), scriptEngineRegistry, scriptContextRegistry, scriptSettings) { @Override - public CompiledScript compile(Script script, ScriptContext scriptContext, Map params, ClusterState state) { + public CompiledScript compile(Script script, ScriptContext scriptContext, Map params) { return new CompiledScript(ScriptType.INLINE, "mockName", "test", script); } }; diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/IngestCommonPlugin.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/IngestCommonPlugin.java index 821e47b85bc..60bfdd37a9f 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/IngestCommonPlugin.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/IngestCommonPlugin.java @@ -60,7 +60,7 @@ public class IngestCommonPlugin extends Plugin { nodeModule.registerProcessor(SortProcessor.TYPE, (registry) -> new SortProcessor.Factory()); nodeModule.registerProcessor(GrokProcessor.TYPE, (registry) -> new GrokProcessor.Factory(builtinPatterns)); nodeModule.registerProcessor(ScriptProcessor.TYPE, (registry) -> - new ScriptProcessor.Factory(registry.getScriptService(), registry.getClusterService())); + new ScriptProcessor.Factory(registry.getScriptService())); } // Code for loading built-in grok patterns packaged with the jar file: diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java index 6ce85f8a6bf..e4881366165 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java @@ -19,7 +19,9 @@ package org.elasticsearch.ingest.common; -import org.elasticsearch.cluster.service.ClusterService; +import java.util.HashMap; +import java.util.Map; + import org.elasticsearch.common.Strings; import org.elasticsearch.ingest.AbstractProcessor; import org.elasticsearch.ingest.AbstractProcessorFactory; @@ -30,9 +32,6 @@ import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptService; -import java.util.HashMap; -import java.util.Map; - import static java.util.Collections.emptyMap; import static org.elasticsearch.common.Strings.hasLength; import static org.elasticsearch.ingest.ConfigurationUtils.newConfigurationException; @@ -52,14 +51,12 @@ public final class ScriptProcessor extends AbstractProcessor { private final Script script; private final ScriptService scriptService; - private final ClusterService clusterService; private final String field; - ScriptProcessor(String tag, Script script, ScriptService scriptService, ClusterService clusterService, String field) { + ScriptProcessor(String tag, Script script, ScriptService scriptService, String field) { super(tag); this.script = script; this.scriptService = scriptService; - this.clusterService = clusterService; this.field = field; } @@ -67,7 +64,7 @@ public final class ScriptProcessor extends AbstractProcessor { public void execute(IngestDocument document) { Map vars = new HashMap<>(); vars.put("ctx", document.getSourceAndMetadata()); - CompiledScript compiledScript = scriptService.compile(script, ScriptContext.Standard.INGEST, emptyMap(), clusterService.state()); + CompiledScript compiledScript = scriptService.compile(script, ScriptContext.Standard.INGEST, emptyMap()); ExecutableScript executableScript = scriptService.executable(compiledScript, vars); Object value = executableScript.run(); if (field != null) { @@ -83,11 +80,9 @@ public final class ScriptProcessor extends AbstractProcessor { public static final class Factory extends AbstractProcessorFactory { private final ScriptService scriptService; - private final ClusterService clusterService; - public Factory(ScriptService scriptService, ClusterService clusterService) { + public Factory(ScriptService scriptService) { this.scriptService = scriptService; - this.clusterService = clusterService; } @Override @@ -120,7 +115,7 @@ public final class ScriptProcessor extends AbstractProcessor { throw newConfigurationException(TYPE, processorTag, null, "Could not initialize script"); } - return new ScriptProcessor(processorTag, script, scriptService, clusterService, field); + return new ScriptProcessor(processorTag, script, scriptService, field); } } } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java index 59a5d7ceaa2..ed47894d4d9 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java @@ -37,7 +37,7 @@ public class ScriptProcessorFactoryTests extends ESTestCase { @Before public void init() { - factory = new ScriptProcessor.Factory(mock(ScriptService.class), mock(ClusterService.class)); + factory = new ScriptProcessor.Factory(mock(ScriptService.class)); } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java index 6da127a798b..5fe7db77dd8 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java @@ -19,7 +19,9 @@ package org.elasticsearch.ingest.common; -import org.elasticsearch.cluster.service.ClusterService; +import java.util.HashMap; +import java.util.Map; + import org.elasticsearch.ingest.IngestDocument; import org.elasticsearch.ingest.RandomDocumentPicks; import org.elasticsearch.script.CompiledScript; @@ -28,9 +30,6 @@ import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESTestCase; -import java.util.HashMap; -import java.util.Map; - import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.core.Is.is; import static org.mockito.Mockito.any; @@ -42,16 +41,15 @@ public class ScriptProcessorTests extends ESTestCase { public void testScripting() throws Exception { int randomInt = randomInt(); ScriptService scriptService = mock(ScriptService.class); - ClusterService clusterService = mock(ClusterService.class); CompiledScript compiledScript = mock(CompiledScript.class); Script script = mock(Script.class); - when(scriptService.compile(any(), any(), any(), any())).thenReturn(compiledScript); + when(scriptService.compile(any(), any(), any())).thenReturn(compiledScript); ExecutableScript executableScript = mock(ExecutableScript.class); when(scriptService.executable(any(), any())).thenReturn(executableScript); when(executableScript.run()).thenReturn(randomInt); ScriptProcessor processor = new ScriptProcessor(randomAsciiOfLength(10), script, - scriptService, clusterService, "bytes_total"); + scriptService, "bytes_total"); Map document = new HashMap<>(); document.put("bytes_in", 1234); diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/action/search/template/TransportSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/action/search/template/TransportSearchTemplateAction.java index 4c956bb209a..9e1dab3af00 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/action/search/template/TransportSearchTemplateAction.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/action/search/template/TransportSearchTemplateAction.java @@ -53,7 +53,6 @@ public class TransportSearchTemplateAction extends HandledTransportAction params; private ExecutableScript executable; private Map context; - public ScriptApplier(BulkByScrollTask task, ScriptService scriptService, Script script, ClusterState state, + public ScriptApplier(BulkByScrollTask task, ScriptService scriptService, Script script, Map params) { this.task = task; this.scriptService = scriptService; this.script = script; - this.state = state; this.params = params; } @@ -462,7 +460,7 @@ public abstract class AbstractAsyncBulkIndexByScrollAction, SearchHit, RequestWrapper> buildScriptApplier() { Script script = mainRequest.getScript(); if (script != null) { - return new ReindexScriptApplier(task, scriptService, script, clusterState, script.getParams()); + return new ReindexScriptApplier(task, scriptService, script, script.getParams()); } return super.buildScriptApplier(); } @@ -205,9 +205,9 @@ public class TransportReindexAction extends HandledTransportAction params) { - super(task, scriptService, script, state, params); + super(task, scriptService, script, params); } /* diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/TransportUpdateByQueryAction.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/TransportUpdateByQueryAction.java index 7459972ce64..e30f0d2b140 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/TransportUpdateByQueryAction.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/TransportUpdateByQueryAction.java @@ -92,7 +92,7 @@ public class TransportUpdateByQueryAction extends HandledTransportAction, SearchHit, RequestWrapper> buildScriptApplier() { Script script = mainRequest.getScript(); if (script != null) { - return new UpdateByQueryScriptApplier(task, scriptService, script, clusterState, script.getParams()); + return new UpdateByQueryScriptApplier(task, scriptService, script, script.getParams()); } return super.buildScriptApplier(); } @@ -112,9 +112,9 @@ public class TransportUpdateByQueryAction extends HandledTransportAction params) { - super(task, scriptService, script, state, params); + super(task, scriptService, script, params); } @Override