From ecf6101798b053a910321ab90e4c45d0c5e87532 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 28 Jun 2016 13:20:00 -0700 Subject: [PATCH 1/5] 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 From 00356edd338613423ef497751b413f1dc06cce43 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 29 Jun 2016 17:02:15 -0400 Subject: [PATCH 2/5] Clarify time units usage in docs This commit clarifies the distinction between supported time units for durations and supported time units for durations in the docs. Relates #19159 --- .../bucket/datehistogram-aggregation.asciidoc | 9 +++-- .../diversified-sampler-aggregation.asciidoc | 6 ++-- docs/reference/api-conventions.asciidoc | 36 +++++++++++-------- .../migrate_5_0/aggregations.asciidoc | 5 +++ .../migration/migrate_5_0/settings.asciidoc | 5 ++- 5 files changed, 36 insertions(+), 25 deletions(-) diff --git a/docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc b/docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc index 29ba5e49626..4d9147067ec 100644 --- a/docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc +++ b/docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc @@ -26,8 +26,9 @@ Requesting bucket intervals of a month. Available expressions for interval: `year`, `quarter`, `month`, `week`, `day`, `hour`, `minute`, `second` - -Fractional values are allowed for seconds, minutes, hours, days and weeks. For example 1.5 hours: +Time values can also be specified via abbreviations supported by <> parsing. +Note that fractional time values are not supported, but you can address this by shifting to another +time unit (e.g., `1.5h` could instead be specified as `90m`). [source,js] -------------------------------------------------- @@ -36,15 +37,13 @@ Fractional values are allowed for seconds, minutes, hours, days and weeks. For e "articles_over_time" : { "date_histogram" : { "field" : "date", - "interval" : "1.5h" + "interval" : "90m" } } } } -------------------------------------------------- -See <> for accepted abbreviations. - ==== Keys Internally, a date is represented as a 64 bit number representing a timestamp diff --git a/docs/reference/aggregations/bucket/diversified-sampler-aggregation.asciidoc b/docs/reference/aggregations/bucket/diversified-sampler-aggregation.asciidoc index 0307840d685..70412d2680a 100644 --- a/docs/reference/aggregations/bucket/diversified-sampler-aggregation.asciidoc +++ b/docs/reference/aggregations/bucket/diversified-sampler-aggregation.asciidoc @@ -153,5 +153,7 @@ In this situation an error will be thrown. The de-duplication logic in the diversify settings applies only at a shard level so will not apply across shards. ===== No specialized syntax for geo/date fields -Currently the syntax for defining the diversifying values is defined by a choice of `field` or `script` - there is no added syntactical sugar for expressing geo or date units such as "1w" (1 week). -This support may be added in a later release and users will currently have to create these sorts of values using a script. +Currently the syntax for defining the diversifying values is defined by a choice of `field` or +`script` - there is no added syntactical sugar for expressing geo or date units such as "7d" (7 +days). This support may be added in a later release and users will currently have to create these +sorts of values using a script. diff --git a/docs/reference/api-conventions.asciidoc b/docs/reference/api-conventions.asciidoc index 4e487325829..26c1a7476ed 100644 --- a/docs/reference/api-conventions.asciidoc +++ b/docs/reference/api-conventions.asciidoc @@ -171,8 +171,18 @@ one or more maths expressions: * `-1d` - subtract one day * `/d` - round down to the nearest day -The supported <> are: `y` (year), `M` (month), `w` (week), -`d` (day), `h` (hour), `m` (minute), and `s` (second). +The supported time units differ than those supported by <> for durations. +The supported units are: + +[horizontal] +`y`:: years +`M`:: months +`w`:: weeks +`d`:: days +`h`:: hours +`H`:: hours +`m`:: minutes +`s`:: seconds Some examples are: @@ -348,21 +358,17 @@ of supporting the native JSON number types. [float] === Time units -Whenever durations need to be specified, eg for a `timeout` parameter, the -duration must specify the unit, like `2d` for 2 days. The supported units -are: +Whenever durations need to be specified, e.g. for a `timeout` parameter, the duration must specify +the unit, like `2d` for 2 days. The supported units are: [horizontal] -`y`:: Year -`M`:: Month -`w`:: Week -`d`:: Day -`h`:: Hour -`m`:: Minute -`s`:: Second -`ms`:: Milli-second -`micros`:: Micro-second -`nanos`:: Nano-second +`d`:: days +`h`:: hours +`m`:: minutes +`s`:: seconds +`ms`:: milliseconds +`micros`:: microseconds +`nanos`:: nanoseconds [[byte-units]] [float] diff --git a/docs/reference/migration/migrate_5_0/aggregations.asciidoc b/docs/reference/migration/migrate_5_0/aggregations.asciidoc index d9227e91385..287da1efb99 100644 --- a/docs/reference/migration/migrate_5_0/aggregations.asciidoc +++ b/docs/reference/migration/migrate_5_0/aggregations.asciidoc @@ -26,3 +26,8 @@ for `from` and `to` anymore. `size: 0` is no longer valid for the terms, significant terms and geohash grid aggregations. Instead a size should be explicitly specified with a number greater than zero. + +==== Fractional time values + +Fractional time values (e.g., 0.5s) are no longer supported. For example, this means when setting +date histogram intervals "1.5h" will be rejected and should instead be input as "90m". diff --git a/docs/reference/migration/migrate_5_0/settings.asciidoc b/docs/reference/migration/migrate_5_0/settings.asciidoc index 8a7c386acdb..ffe69aa3cfb 100644 --- a/docs/reference/migration/migrate_5_0/settings.asciidoc +++ b/docs/reference/migration/migrate_5_0/settings.asciidoc @@ -291,6 +291,5 @@ still defaults to `true` in that case. The unit 'w' representing weeks is no longer supported. -Fractional time values (e.g., 0.5s) are no longer supported. For -example, this means when setting timeouts "0.5s" will be rejected and -should instead be input as "500ms". +Fractional time values (e.g., 0.5s) are no longer supported. For example, this means when setting +timeouts "0.5s" will be rejected and should instead be input as "500ms". From 8db43c01079aaec4f5ca5c9adb89364e63eb9bbe Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 29 Jun 2016 14:55:31 -0400 Subject: [PATCH 3/5] Move RestHandler registration to ActionModule and ActionPlugin `RestHandler`s are highly tied to actions so registering them in the same place makes sense. Removes the need to for plugins to check if they are in transport client mode before registering a RestHandler - `getRestHandlers` isn't called at all in transport client mode. This caused guice to throw a massive fit about the circular dependency between NodeClient and the allocation deciders. I broke the circular dependency by registering the actions map with the node client after instantiation. --- .../elasticsearch/action/ActionModule.java | 279 +++++++++++++++++- .../elasticsearch/client/node/NodeClient.java | 16 +- .../metadata/IndexNameExpressionResolver.java | 2 - .../elasticsearch/common/NamedRegistry.java | 2 +- .../common/network/NetworkModule.java | 266 ----------------- .../java/org/elasticsearch/node/Node.java | 11 +- .../elasticsearch/plugins/ActionPlugin.java | 34 +++ .../elasticsearch/rest/RestController.java | 2 - .../action/ActionModuleTests.java | 129 ++++++++ .../client/node/NodeClientHeadersTests.java | 4 +- .../common/network/NetworkModuleTests.java | 29 -- .../TestResponseHeaderPlugin.java | 13 +- .../migration/migrate_5_0/plugins.asciidoc | 3 + .../script/mustache/MustachePlugin.java | 15 +- .../percolator/PercolatorPlugin.java | 12 +- .../index/reindex/ReindexPlugin.java | 11 +- 16 files changed, 488 insertions(+), 340 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/action/ActionModuleTests.java diff --git a/core/src/main/java/org/elasticsearch/action/ActionModule.java b/core/src/main/java/org/elasticsearch/action/ActionModule.java index f5c83e272ce..5ce7aaa4d95 100644 --- a/core/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/core/src/main/java/org/elasticsearch/action/ActionModule.java @@ -194,14 +194,120 @@ import org.elasticsearch.common.NamedRegistry; import org.elasticsearch.common.inject.AbstractModule; import org.elasticsearch.common.inject.multibindings.MapBinder; import org.elasticsearch.common.inject.multibindings.Multibinder; +import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.ActionPlugin.ActionHandler; +import org.elasticsearch.rest.RestController; +import org.elasticsearch.rest.RestHandler; +import org.elasticsearch.rest.action.admin.cluster.allocation.RestClusterAllocationExplainAction; +import org.elasticsearch.rest.action.admin.cluster.health.RestClusterHealthAction; +import org.elasticsearch.rest.action.admin.cluster.node.hotthreads.RestNodesHotThreadsAction; +import org.elasticsearch.rest.action.admin.cluster.node.info.RestNodesInfoAction; +import org.elasticsearch.rest.action.admin.cluster.node.stats.RestNodesStatsAction; +import org.elasticsearch.rest.action.admin.cluster.node.tasks.RestCancelTasksAction; +import org.elasticsearch.rest.action.admin.cluster.node.tasks.RestGetTaskAction; +import org.elasticsearch.rest.action.admin.cluster.node.tasks.RestListTasksAction; +import org.elasticsearch.rest.action.admin.cluster.repositories.delete.RestDeleteRepositoryAction; +import org.elasticsearch.rest.action.admin.cluster.repositories.get.RestGetRepositoriesAction; +import org.elasticsearch.rest.action.admin.cluster.repositories.put.RestPutRepositoryAction; +import org.elasticsearch.rest.action.admin.cluster.repositories.verify.RestVerifyRepositoryAction; +import org.elasticsearch.rest.action.admin.cluster.reroute.RestClusterRerouteAction; +import org.elasticsearch.rest.action.admin.cluster.settings.RestClusterGetSettingsAction; +import org.elasticsearch.rest.action.admin.cluster.settings.RestClusterUpdateSettingsAction; +import org.elasticsearch.rest.action.admin.cluster.shards.RestClusterSearchShardsAction; +import org.elasticsearch.rest.action.admin.cluster.snapshots.create.RestCreateSnapshotAction; +import org.elasticsearch.rest.action.admin.cluster.snapshots.delete.RestDeleteSnapshotAction; +import org.elasticsearch.rest.action.admin.cluster.snapshots.get.RestGetSnapshotsAction; +import org.elasticsearch.rest.action.admin.cluster.snapshots.restore.RestRestoreSnapshotAction; +import org.elasticsearch.rest.action.admin.cluster.snapshots.status.RestSnapshotsStatusAction; +import org.elasticsearch.rest.action.admin.cluster.state.RestClusterStateAction; +import org.elasticsearch.rest.action.admin.cluster.stats.RestClusterStatsAction; +import org.elasticsearch.rest.action.admin.cluster.storedscripts.RestDeleteStoredScriptAction; +import org.elasticsearch.rest.action.admin.cluster.storedscripts.RestGetStoredScriptAction; +import org.elasticsearch.rest.action.admin.cluster.storedscripts.RestPutStoredScriptAction; +import org.elasticsearch.rest.action.admin.cluster.tasks.RestPendingClusterTasksAction; +import org.elasticsearch.rest.action.admin.indices.RestRolloverIndexAction; +import org.elasticsearch.rest.action.admin.indices.RestShrinkIndexAction; +import org.elasticsearch.rest.action.admin.indices.alias.RestIndicesAliasesAction; +import org.elasticsearch.rest.action.admin.indices.alias.delete.RestIndexDeleteAliasesAction; +import org.elasticsearch.rest.action.admin.indices.alias.get.RestGetAliasesAction; +import org.elasticsearch.rest.action.admin.indices.alias.head.RestAliasesExistAction; +import org.elasticsearch.rest.action.admin.indices.alias.put.RestIndexPutAliasAction; +import org.elasticsearch.rest.action.admin.indices.analyze.RestAnalyzeAction; +import org.elasticsearch.rest.action.admin.indices.cache.clear.RestClearIndicesCacheAction; +import org.elasticsearch.rest.action.admin.indices.close.RestCloseIndexAction; +import org.elasticsearch.rest.action.admin.indices.create.RestCreateIndexAction; +import org.elasticsearch.rest.action.admin.indices.delete.RestDeleteIndexAction; +import org.elasticsearch.rest.action.admin.indices.exists.indices.RestIndicesExistsAction; +import org.elasticsearch.rest.action.admin.indices.exists.types.RestTypesExistsAction; +import org.elasticsearch.rest.action.admin.indices.flush.RestFlushAction; +import org.elasticsearch.rest.action.admin.indices.flush.RestSyncedFlushAction; +import org.elasticsearch.rest.action.admin.indices.forcemerge.RestForceMergeAction; +import org.elasticsearch.rest.action.admin.indices.get.RestGetIndicesAction; +import org.elasticsearch.rest.action.admin.indices.mapping.get.RestGetFieldMappingAction; +import org.elasticsearch.rest.action.admin.indices.mapping.get.RestGetMappingAction; +import org.elasticsearch.rest.action.admin.indices.mapping.put.RestPutMappingAction; +import org.elasticsearch.rest.action.admin.indices.open.RestOpenIndexAction; +import org.elasticsearch.rest.action.admin.indices.recovery.RestRecoveryAction; +import org.elasticsearch.rest.action.admin.indices.refresh.RestRefreshAction; +import org.elasticsearch.rest.action.admin.indices.segments.RestIndicesSegmentsAction; +import org.elasticsearch.rest.action.admin.indices.settings.RestGetSettingsAction; +import org.elasticsearch.rest.action.admin.indices.settings.RestUpdateSettingsAction; +import org.elasticsearch.rest.action.admin.indices.shards.RestIndicesShardStoresAction; +import org.elasticsearch.rest.action.admin.indices.stats.RestIndicesStatsAction; +import org.elasticsearch.rest.action.admin.indices.template.delete.RestDeleteIndexTemplateAction; +import org.elasticsearch.rest.action.admin.indices.template.get.RestGetIndexTemplateAction; +import org.elasticsearch.rest.action.admin.indices.template.head.RestHeadIndexTemplateAction; +import org.elasticsearch.rest.action.admin.indices.template.put.RestPutIndexTemplateAction; +import org.elasticsearch.rest.action.admin.indices.upgrade.RestUpgradeAction; +import org.elasticsearch.rest.action.admin.indices.validate.query.RestValidateQueryAction; +import org.elasticsearch.rest.action.bulk.RestBulkAction; +import org.elasticsearch.rest.action.cat.AbstractCatAction; +import org.elasticsearch.rest.action.cat.RestAliasAction; +import org.elasticsearch.rest.action.cat.RestAllocationAction; +import org.elasticsearch.rest.action.cat.RestCatAction; +import org.elasticsearch.rest.action.cat.RestFielddataAction; +import org.elasticsearch.rest.action.cat.RestHealthAction; +import org.elasticsearch.rest.action.cat.RestIndicesAction; +import org.elasticsearch.rest.action.cat.RestMasterAction; +import org.elasticsearch.rest.action.cat.RestNodeAttrsAction; +import org.elasticsearch.rest.action.cat.RestNodesAction; +import org.elasticsearch.rest.action.cat.RestPluginsAction; +import org.elasticsearch.rest.action.cat.RestRepositoriesAction; +import org.elasticsearch.rest.action.cat.RestSegmentsAction; +import org.elasticsearch.rest.action.cat.RestShardsAction; +import org.elasticsearch.rest.action.cat.RestSnapshotAction; +import org.elasticsearch.rest.action.cat.RestTasksAction; +import org.elasticsearch.rest.action.cat.RestThreadPoolAction; +import org.elasticsearch.rest.action.delete.RestDeleteAction; +import org.elasticsearch.rest.action.explain.RestExplainAction; +import org.elasticsearch.rest.action.fieldstats.RestFieldStatsAction; +import org.elasticsearch.rest.action.get.RestGetAction; +import org.elasticsearch.rest.action.get.RestGetSourceAction; +import org.elasticsearch.rest.action.get.RestHeadAction; +import org.elasticsearch.rest.action.get.RestMultiGetAction; +import org.elasticsearch.rest.action.index.RestIndexAction; +import org.elasticsearch.rest.action.ingest.RestDeletePipelineAction; +import org.elasticsearch.rest.action.ingest.RestGetPipelineAction; +import org.elasticsearch.rest.action.ingest.RestPutPipelineAction; +import org.elasticsearch.rest.action.ingest.RestSimulatePipelineAction; +import org.elasticsearch.rest.action.main.RestMainAction; +import org.elasticsearch.rest.action.search.RestClearScrollAction; +import org.elasticsearch.rest.action.search.RestMultiSearchAction; +import org.elasticsearch.rest.action.search.RestSearchAction; +import org.elasticsearch.rest.action.search.RestSearchScrollAction; +import org.elasticsearch.rest.action.suggest.RestSuggestAction; +import org.elasticsearch.rest.action.termvectors.RestMultiTermVectorsAction; +import org.elasticsearch.rest.action.termvectors.RestTermVectorsAction; +import org.elasticsearch.rest.action.update.RestUpdateAction; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import static java.util.Collections.unmodifiableList; import static java.util.Collections.unmodifiableMap; @@ -212,21 +318,27 @@ import static java.util.Collections.unmodifiableMap; public class ActionModule extends AbstractModule { private final boolean transportClient; + private final Settings settings; + private final List actionPlugins; private final Map> actions; private final List> actionFilters; private final AutoCreateIndex autoCreateIndex; private final DestructiveOperations destructiveOperations; + private final RestController restController; public ActionModule(boolean ingestEnabled, boolean transportClient, Settings settings, IndexNameExpressionResolver resolver, ClusterSettings clusterSettings, List actionPlugins) { this.transportClient = transportClient; + this.settings = settings; + this.actionPlugins = actionPlugins; actions = setupActions(actionPlugins); actionFilters = setupActionFilters(actionPlugins, ingestEnabled); autoCreateIndex = transportClient ? null : new AutoCreateIndex(settings, resolver); destructiveOperations = new DestructiveOperations(settings, clusterSettings); + restController = new RestController(settings); } - private Map> setupActions(List actionPlugins) { + static Map> setupActions(List actionPlugins) { // Subclass NamedRegistry for easy registration class ActionRegistry extends NamedRegistry> { public ActionRegistry() { @@ -357,6 +469,149 @@ public class ActionModule extends AbstractModule { return unmodifiableList(filters); } + static Set> setupRestHandlers(List actionPlugins) { + Set> handlers = new HashSet<>(); + registerRestHandler(handlers, RestMainAction.class); + registerRestHandler(handlers, RestNodesInfoAction.class); + registerRestHandler(handlers, RestNodesStatsAction.class); + registerRestHandler(handlers, RestNodesHotThreadsAction.class); + registerRestHandler(handlers, RestClusterAllocationExplainAction.class); + registerRestHandler(handlers, RestClusterStatsAction.class); + registerRestHandler(handlers, RestClusterStateAction.class); + registerRestHandler(handlers, RestClusterHealthAction.class); + registerRestHandler(handlers, RestClusterUpdateSettingsAction.class); + registerRestHandler(handlers, RestClusterGetSettingsAction.class); + registerRestHandler(handlers, RestClusterRerouteAction.class); + registerRestHandler(handlers, RestClusterSearchShardsAction.class); + registerRestHandler(handlers, RestPendingClusterTasksAction.class); + registerRestHandler(handlers, RestPutRepositoryAction.class); + registerRestHandler(handlers, RestGetRepositoriesAction.class); + registerRestHandler(handlers, RestDeleteRepositoryAction.class); + registerRestHandler(handlers, RestVerifyRepositoryAction.class); + registerRestHandler(handlers, RestGetSnapshotsAction.class); + registerRestHandler(handlers, RestCreateSnapshotAction.class); + registerRestHandler(handlers, RestRestoreSnapshotAction.class); + registerRestHandler(handlers, RestDeleteSnapshotAction.class); + registerRestHandler(handlers, RestSnapshotsStatusAction.class); + + registerRestHandler(handlers, RestIndicesExistsAction.class); + registerRestHandler(handlers, RestTypesExistsAction.class); + registerRestHandler(handlers, RestGetIndicesAction.class); + registerRestHandler(handlers, RestIndicesStatsAction.class); + registerRestHandler(handlers, RestIndicesSegmentsAction.class); + registerRestHandler(handlers, RestIndicesShardStoresAction.class); + registerRestHandler(handlers, RestGetAliasesAction.class); + registerRestHandler(handlers, RestAliasesExistAction.class); + registerRestHandler(handlers, RestIndexDeleteAliasesAction.class); + registerRestHandler(handlers, RestIndexPutAliasAction.class); + registerRestHandler(handlers, RestIndicesAliasesAction.class); + registerRestHandler(handlers, RestCreateIndexAction.class); + registerRestHandler(handlers, RestShrinkIndexAction.class); + registerRestHandler(handlers, RestRolloverIndexAction.class); + registerRestHandler(handlers, RestDeleteIndexAction.class); + registerRestHandler(handlers, RestCloseIndexAction.class); + registerRestHandler(handlers, RestOpenIndexAction.class); + + registerRestHandler(handlers, RestUpdateSettingsAction.class); + registerRestHandler(handlers, RestGetSettingsAction.class); + + registerRestHandler(handlers, RestAnalyzeAction.class); + registerRestHandler(handlers, RestGetIndexTemplateAction.class); + registerRestHandler(handlers, RestPutIndexTemplateAction.class); + registerRestHandler(handlers, RestDeleteIndexTemplateAction.class); + registerRestHandler(handlers, RestHeadIndexTemplateAction.class); + + registerRestHandler(handlers, RestPutMappingAction.class); + registerRestHandler(handlers, RestGetMappingAction.class); + registerRestHandler(handlers, RestGetFieldMappingAction.class); + + registerRestHandler(handlers, RestRefreshAction.class); + registerRestHandler(handlers, RestFlushAction.class); + registerRestHandler(handlers, RestSyncedFlushAction.class); + registerRestHandler(handlers, RestForceMergeAction.class); + registerRestHandler(handlers, RestUpgradeAction.class); + registerRestHandler(handlers, RestClearIndicesCacheAction.class); + + registerRestHandler(handlers, RestIndexAction.class); + registerRestHandler(handlers, RestGetAction.class); + registerRestHandler(handlers, RestGetSourceAction.class); + registerRestHandler(handlers, RestHeadAction.Document.class); + registerRestHandler(handlers, RestHeadAction.Source.class); + registerRestHandler(handlers, RestMultiGetAction.class); + registerRestHandler(handlers, RestDeleteAction.class); + registerRestHandler(handlers, org.elasticsearch.rest.action.count.RestCountAction.class); + registerRestHandler(handlers, RestSuggestAction.class); + registerRestHandler(handlers, RestTermVectorsAction.class); + registerRestHandler(handlers, RestMultiTermVectorsAction.class); + registerRestHandler(handlers, RestBulkAction.class); + registerRestHandler(handlers, RestUpdateAction.class); + + registerRestHandler(handlers, RestSearchAction.class); + registerRestHandler(handlers, RestSearchScrollAction.class); + registerRestHandler(handlers, RestClearScrollAction.class); + registerRestHandler(handlers, RestMultiSearchAction.class); + + registerRestHandler(handlers, RestValidateQueryAction.class); + + registerRestHandler(handlers, RestExplainAction.class); + + registerRestHandler(handlers, RestRecoveryAction.class); + + // Scripts API + registerRestHandler(handlers, RestGetStoredScriptAction.class); + registerRestHandler(handlers, RestPutStoredScriptAction.class); + registerRestHandler(handlers, RestDeleteStoredScriptAction.class); + + registerRestHandler(handlers, RestFieldStatsAction.class); + + // Tasks API + registerRestHandler(handlers, RestListTasksAction.class); + registerRestHandler(handlers, RestGetTaskAction.class); + registerRestHandler(handlers, RestCancelTasksAction.class); + + // Ingest API + registerRestHandler(handlers, RestPutPipelineAction.class); + registerRestHandler(handlers, RestGetPipelineAction.class); + registerRestHandler(handlers, RestDeletePipelineAction.class); + registerRestHandler(handlers, RestSimulatePipelineAction.class); + + // CAT API + registerRestHandler(handlers, RestCatAction.class); + registerRestHandler(handlers, RestAllocationAction.class); + registerRestHandler(handlers, RestShardsAction.class); + registerRestHandler(handlers, RestMasterAction.class); + registerRestHandler(handlers, RestNodesAction.class); + registerRestHandler(handlers, RestTasksAction.class); + registerRestHandler(handlers, RestIndicesAction.class); + registerRestHandler(handlers, RestSegmentsAction.class); + // Fully qualified to prevent interference with rest.action.count.RestCountAction + registerRestHandler(handlers, org.elasticsearch.rest.action.cat.RestCountAction.class); + // Fully qualified to prevent interference with rest.action.indices.RestRecoveryAction + registerRestHandler(handlers, org.elasticsearch.rest.action.cat.RestRecoveryAction.class); + registerRestHandler(handlers, RestHealthAction.class); + registerRestHandler(handlers, org.elasticsearch.rest.action.cat.RestPendingClusterTasksAction.class); + registerRestHandler(handlers, RestAliasAction.class); + registerRestHandler(handlers, RestThreadPoolAction.class); + registerRestHandler(handlers, RestPluginsAction.class); + registerRestHandler(handlers, RestFielddataAction.class); + registerRestHandler(handlers, RestNodeAttrsAction.class); + registerRestHandler(handlers, RestRepositoriesAction.class); + registerRestHandler(handlers, RestSnapshotAction.class); + for (ActionPlugin plugin : actionPlugins) { + for (Class handler : plugin.getRestHandlers()) { + registerRestHandler(handlers, handler); + } + } + return handlers; + } + + private static void registerRestHandler(Set> handlers, Class handler) { + if (handlers.contains(handler)) { + throw new IllegalArgumentException("can't register the same [rest_handler] more than once for [" + handler.getName() + "]"); + } + handlers.add(handler); + } + @Override protected void configure() { Multibinder actionFilterMultibinder = Multibinder.newSetBinder(binder(), ActionFilter.class); @@ -374,11 +629,12 @@ public class ActionModule extends AbstractModule { for (Map.Entry> entry : actions.entrySet()) { actionsBinder.addBinding(entry.getKey()).toInstance(entry.getValue().getAction()); } - // register GenericAction -> transportAction Map that can be injected to instances. - // also register any supporting classes if (false == transportClient) { + // Supporting classes only used when not a transport client bind(AutoCreateIndex.class).toInstance(autoCreateIndex); bind(TransportLivenessAction.class).asEagerSingleton(); + + // register GenericAction -> transportAction Map used by NodeClient @SuppressWarnings("rawtypes") MapBinder transportActionsBinder = MapBinder.newMapBinder(binder(), GenericAction.class, TransportAction.class); @@ -390,6 +646,23 @@ public class ActionModule extends AbstractModule { bind(supportAction).asEagerSingleton(); } } + + // Bind the RestController which is required (by Node) even if rest isn't enabled. + bind(RestController.class).toInstance(restController); + + // Setup the RestHandlers + if (NetworkModule.HTTP_ENABLED.get(settings)) { + Multibinder restHandlers = Multibinder.newSetBinder(binder(), RestHandler.class); + Multibinder catHandlers = Multibinder.newSetBinder(binder(), AbstractCatAction.class); + for (Class handler : setupRestHandlers(actionPlugins)) { + bind(handler).asEagerSingleton(); + if (AbstractCatAction.class.isAssignableFrom(handler)) { + catHandlers.addBinding().to(handler.asSubclass(AbstractCatAction.class)); + } else { + restHandlers.addBinding().to(handler); + } + } + } } } } diff --git a/core/src/main/java/org/elasticsearch/client/node/NodeClient.java b/core/src/main/java/org/elasticsearch/client/node/NodeClient.java index 3e9bed9e25d..59909df428b 100644 --- a/core/src/main/java/org/elasticsearch/client/node/NodeClient.java +++ b/core/src/main/java/org/elasticsearch/client/node/NodeClient.java @@ -27,25 +27,24 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.GenericAction; import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.client.support.AbstractClient; -import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.threadpool.ThreadPool; import java.util.Map; -import static java.util.Collections.unmodifiableMap; - /** * */ public class NodeClient extends AbstractClient { - private final Map actions; + private Map actions; - @Inject - public NodeClient(Settings settings, ThreadPool threadPool, Map actions) { + public NodeClient(Settings settings, ThreadPool threadPool) { super(settings, threadPool); - this.actions = unmodifiableMap(actions); + } + + public void intialize(Map actions) { + this.actions = actions; } @Override @@ -57,6 +56,9 @@ public class NodeClient extends AbstractClient { @Override public , Response extends ActionResponse, RequestBuilder extends ActionRequestBuilder> void doExecute( Action action, Request request, ActionListener listener) { + if (actions == null) { + throw new IllegalStateException("NodeClient has not been initialized"); + } TransportAction transportAction = actions.get(action); if (transportAction == null) { throw new IllegalStateException("failed to find action [" + action + "] to execute"); diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 2abbea04d51..4eefac0f17f 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -27,7 +27,6 @@ import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.component.AbstractComponent; -import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.joda.DateMathParser; import org.elasticsearch.common.joda.FormatDateTimeFormatter; import org.elasticsearch.common.regex.Regex; @@ -57,7 +56,6 @@ public class IndexNameExpressionResolver extends AbstractComponent { private final List expressionResolvers; private final DateMathExpressionResolver dateMathExpressionResolver; - @Inject public IndexNameExpressionResolver(Settings settings) { super(settings); expressionResolvers = Arrays.asList( diff --git a/core/src/main/java/org/elasticsearch/common/NamedRegistry.java b/core/src/main/java/org/elasticsearch/common/NamedRegistry.java index 7573b5268fd..c326da7a495 100644 --- a/core/src/main/java/org/elasticsearch/common/NamedRegistry.java +++ b/core/src/main/java/org/elasticsearch/common/NamedRegistry.java @@ -45,7 +45,7 @@ public class NamedRegistry { requireNonNull(name, "name is required"); requireNonNull(t, targetName + " is required"); if (registry.putIfAbsent(name, t) != null) { - throw new IllegalArgumentException(targetName + " for name " + name + " already registered"); + throw new IllegalArgumentException(targetName + " for name [" + name + "] already registered"); } } diff --git a/core/src/main/java/org/elasticsearch/common/network/NetworkModule.java b/core/src/main/java/org/elasticsearch/common/network/NetworkModule.java index b0287ad153a..2b94f224b9c 100644 --- a/core/src/main/java/org/elasticsearch/common/network/NetworkModule.java +++ b/core/src/main/java/org/elasticsearch/common/network/NetworkModule.java @@ -41,109 +41,6 @@ import org.elasticsearch.common.util.ExtensionPoint; import org.elasticsearch.http.HttpServer; import org.elasticsearch.http.HttpServerTransport; import org.elasticsearch.http.netty.NettyHttpServerTransport; -import org.elasticsearch.rest.RestController; -import org.elasticsearch.rest.RestHandler; -import org.elasticsearch.rest.action.admin.cluster.allocation.RestClusterAllocationExplainAction; -import org.elasticsearch.rest.action.admin.cluster.health.RestClusterHealthAction; -import org.elasticsearch.rest.action.admin.cluster.node.hotthreads.RestNodesHotThreadsAction; -import org.elasticsearch.rest.action.admin.cluster.node.info.RestNodesInfoAction; -import org.elasticsearch.rest.action.admin.cluster.node.stats.RestNodesStatsAction; -import org.elasticsearch.rest.action.admin.cluster.node.tasks.RestCancelTasksAction; -import org.elasticsearch.rest.action.admin.cluster.node.tasks.RestGetTaskAction; -import org.elasticsearch.rest.action.admin.cluster.node.tasks.RestListTasksAction; -import org.elasticsearch.rest.action.admin.cluster.repositories.delete.RestDeleteRepositoryAction; -import org.elasticsearch.rest.action.admin.cluster.repositories.get.RestGetRepositoriesAction; -import org.elasticsearch.rest.action.admin.cluster.repositories.put.RestPutRepositoryAction; -import org.elasticsearch.rest.action.admin.cluster.repositories.verify.RestVerifyRepositoryAction; -import org.elasticsearch.rest.action.admin.cluster.reroute.RestClusterRerouteAction; -import org.elasticsearch.rest.action.admin.cluster.settings.RestClusterGetSettingsAction; -import org.elasticsearch.rest.action.admin.cluster.settings.RestClusterUpdateSettingsAction; -import org.elasticsearch.rest.action.admin.cluster.shards.RestClusterSearchShardsAction; -import org.elasticsearch.rest.action.admin.cluster.snapshots.create.RestCreateSnapshotAction; -import org.elasticsearch.rest.action.admin.cluster.snapshots.delete.RestDeleteSnapshotAction; -import org.elasticsearch.rest.action.admin.cluster.snapshots.get.RestGetSnapshotsAction; -import org.elasticsearch.rest.action.admin.cluster.snapshots.restore.RestRestoreSnapshotAction; -import org.elasticsearch.rest.action.admin.cluster.snapshots.status.RestSnapshotsStatusAction; -import org.elasticsearch.rest.action.admin.cluster.state.RestClusterStateAction; -import org.elasticsearch.rest.action.admin.cluster.stats.RestClusterStatsAction; -import org.elasticsearch.rest.action.admin.cluster.storedscripts.RestDeleteStoredScriptAction; -import org.elasticsearch.rest.action.admin.cluster.storedscripts.RestGetStoredScriptAction; -import org.elasticsearch.rest.action.admin.cluster.storedscripts.RestPutStoredScriptAction; -import org.elasticsearch.rest.action.admin.cluster.tasks.RestPendingClusterTasksAction; -import org.elasticsearch.rest.action.admin.indices.RestRolloverIndexAction; -import org.elasticsearch.rest.action.admin.indices.RestShrinkIndexAction; -import org.elasticsearch.rest.action.admin.indices.alias.RestIndicesAliasesAction; -import org.elasticsearch.rest.action.admin.indices.alias.delete.RestIndexDeleteAliasesAction; -import org.elasticsearch.rest.action.admin.indices.alias.get.RestGetAliasesAction; -import org.elasticsearch.rest.action.admin.indices.alias.head.RestAliasesExistAction; -import org.elasticsearch.rest.action.admin.indices.alias.put.RestIndexPutAliasAction; -import org.elasticsearch.rest.action.admin.indices.analyze.RestAnalyzeAction; -import org.elasticsearch.rest.action.admin.indices.cache.clear.RestClearIndicesCacheAction; -import org.elasticsearch.rest.action.admin.indices.close.RestCloseIndexAction; -import org.elasticsearch.rest.action.admin.indices.create.RestCreateIndexAction; -import org.elasticsearch.rest.action.admin.indices.delete.RestDeleteIndexAction; -import org.elasticsearch.rest.action.admin.indices.exists.indices.RestIndicesExistsAction; -import org.elasticsearch.rest.action.admin.indices.exists.types.RestTypesExistsAction; -import org.elasticsearch.rest.action.admin.indices.flush.RestFlushAction; -import org.elasticsearch.rest.action.admin.indices.flush.RestSyncedFlushAction; -import org.elasticsearch.rest.action.admin.indices.forcemerge.RestForceMergeAction; -import org.elasticsearch.rest.action.admin.indices.get.RestGetIndicesAction; -import org.elasticsearch.rest.action.admin.indices.mapping.get.RestGetFieldMappingAction; -import org.elasticsearch.rest.action.admin.indices.mapping.get.RestGetMappingAction; -import org.elasticsearch.rest.action.admin.indices.mapping.put.RestPutMappingAction; -import org.elasticsearch.rest.action.admin.indices.open.RestOpenIndexAction; -import org.elasticsearch.rest.action.admin.indices.recovery.RestRecoveryAction; -import org.elasticsearch.rest.action.admin.indices.refresh.RestRefreshAction; -import org.elasticsearch.rest.action.admin.indices.segments.RestIndicesSegmentsAction; -import org.elasticsearch.rest.action.admin.indices.settings.RestGetSettingsAction; -import org.elasticsearch.rest.action.admin.indices.settings.RestUpdateSettingsAction; -import org.elasticsearch.rest.action.admin.indices.shards.RestIndicesShardStoresAction; -import org.elasticsearch.rest.action.admin.indices.stats.RestIndicesStatsAction; -import org.elasticsearch.rest.action.admin.indices.template.delete.RestDeleteIndexTemplateAction; -import org.elasticsearch.rest.action.admin.indices.template.get.RestGetIndexTemplateAction; -import org.elasticsearch.rest.action.admin.indices.template.head.RestHeadIndexTemplateAction; -import org.elasticsearch.rest.action.admin.indices.template.put.RestPutIndexTemplateAction; -import org.elasticsearch.rest.action.admin.indices.upgrade.RestUpgradeAction; -import org.elasticsearch.rest.action.admin.indices.validate.query.RestValidateQueryAction; -import org.elasticsearch.rest.action.bulk.RestBulkAction; -import org.elasticsearch.rest.action.cat.AbstractCatAction; -import org.elasticsearch.rest.action.cat.RestAliasAction; -import org.elasticsearch.rest.action.cat.RestAllocationAction; -import org.elasticsearch.rest.action.cat.RestCatAction; -import org.elasticsearch.rest.action.cat.RestFielddataAction; -import org.elasticsearch.rest.action.cat.RestHealthAction; -import org.elasticsearch.rest.action.cat.RestIndicesAction; -import org.elasticsearch.rest.action.cat.RestMasterAction; -import org.elasticsearch.rest.action.cat.RestNodeAttrsAction; -import org.elasticsearch.rest.action.cat.RestNodesAction; -import org.elasticsearch.rest.action.cat.RestPluginsAction; -import org.elasticsearch.rest.action.cat.RestRepositoriesAction; -import org.elasticsearch.rest.action.cat.RestSegmentsAction; -import org.elasticsearch.rest.action.cat.RestShardsAction; -import org.elasticsearch.rest.action.cat.RestSnapshotAction; -import org.elasticsearch.rest.action.cat.RestTasksAction; -import org.elasticsearch.rest.action.cat.RestThreadPoolAction; -import org.elasticsearch.rest.action.delete.RestDeleteAction; -import org.elasticsearch.rest.action.explain.RestExplainAction; -import org.elasticsearch.rest.action.fieldstats.RestFieldStatsAction; -import org.elasticsearch.rest.action.get.RestGetAction; -import org.elasticsearch.rest.action.get.RestGetSourceAction; -import org.elasticsearch.rest.action.get.RestHeadAction; -import org.elasticsearch.rest.action.get.RestMultiGetAction; -import org.elasticsearch.rest.action.index.RestIndexAction; -import org.elasticsearch.rest.action.ingest.RestDeletePipelineAction; -import org.elasticsearch.rest.action.ingest.RestGetPipelineAction; -import org.elasticsearch.rest.action.ingest.RestPutPipelineAction; -import org.elasticsearch.rest.action.ingest.RestSimulatePipelineAction; -import org.elasticsearch.rest.action.main.RestMainAction; -import org.elasticsearch.rest.action.search.RestClearScrollAction; -import org.elasticsearch.rest.action.search.RestMultiSearchAction; -import org.elasticsearch.rest.action.search.RestSearchAction; -import org.elasticsearch.rest.action.search.RestSearchScrollAction; -import org.elasticsearch.rest.action.suggest.RestSuggestAction; -import org.elasticsearch.rest.action.termvectors.RestMultiTermVectorsAction; -import org.elasticsearch.rest.action.termvectors.RestTermVectorsAction; -import org.elasticsearch.rest.action.update.RestUpdateAction; import org.elasticsearch.tasks.RawTaskStatus; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.Transport; @@ -151,9 +48,6 @@ import org.elasticsearch.transport.TransportService; import org.elasticsearch.transport.local.LocalTransport; import org.elasticsearch.transport.netty.NettyTransport; -import java.util.Arrays; -import java.util.List; - /** * A module to handle registering and binding all network related classes. */ @@ -171,140 +65,6 @@ public class NetworkModule extends AbstractModule { Setting.simpleString(TRANSPORT_SERVICE_TYPE_KEY, Property.NodeScope); public static final Setting TRANSPORT_TYPE_SETTING = Setting.simpleString(TRANSPORT_TYPE_KEY, Property.NodeScope); - - - private static final List> builtinRestHandlers = Arrays.asList( - RestMainAction.class, - RestNodesInfoAction.class, - RestNodesStatsAction.class, - RestNodesHotThreadsAction.class, - RestClusterAllocationExplainAction.class, - RestClusterStatsAction.class, - RestClusterStateAction.class, - RestClusterHealthAction.class, - RestClusterUpdateSettingsAction.class, - RestClusterGetSettingsAction.class, - RestClusterRerouteAction.class, - RestClusterSearchShardsAction.class, - RestPendingClusterTasksAction.class, - RestPutRepositoryAction.class, - RestGetRepositoriesAction.class, - RestDeleteRepositoryAction.class, - RestVerifyRepositoryAction.class, - RestGetSnapshotsAction.class, - RestCreateSnapshotAction.class, - RestRestoreSnapshotAction.class, - RestDeleteSnapshotAction.class, - RestSnapshotsStatusAction.class, - - RestIndicesExistsAction.class, - RestTypesExistsAction.class, - RestGetIndicesAction.class, - RestIndicesStatsAction.class, - RestIndicesSegmentsAction.class, - RestIndicesShardStoresAction.class, - RestGetAliasesAction.class, - RestAliasesExistAction.class, - RestIndexDeleteAliasesAction.class, - RestIndexPutAliasAction.class, - RestIndicesAliasesAction.class, - RestCreateIndexAction.class, - RestShrinkIndexAction.class, - RestRolloverIndexAction.class, - RestDeleteIndexAction.class, - RestCloseIndexAction.class, - RestOpenIndexAction.class, - - RestUpdateSettingsAction.class, - RestGetSettingsAction.class, - - RestAnalyzeAction.class, - RestGetIndexTemplateAction.class, - RestPutIndexTemplateAction.class, - RestDeleteIndexTemplateAction.class, - RestHeadIndexTemplateAction.class, - - RestPutMappingAction.class, - RestGetMappingAction.class, - RestGetFieldMappingAction.class, - - RestRefreshAction.class, - RestFlushAction.class, - RestSyncedFlushAction.class, - RestForceMergeAction.class, - RestUpgradeAction.class, - RestClearIndicesCacheAction.class, - - RestIndexAction.class, - RestGetAction.class, - RestGetSourceAction.class, - RestHeadAction.Document.class, - RestHeadAction.Source.class, - RestMultiGetAction.class, - RestDeleteAction.class, - org.elasticsearch.rest.action.count.RestCountAction.class, - RestSuggestAction.class, - RestTermVectorsAction.class, - RestMultiTermVectorsAction.class, - RestBulkAction.class, - RestUpdateAction.class, - - RestSearchAction.class, - RestSearchScrollAction.class, - RestClearScrollAction.class, - RestMultiSearchAction.class, - - RestValidateQueryAction.class, - - RestExplainAction.class, - - RestRecoveryAction.class, - - // Scripts API - RestGetStoredScriptAction.class, - RestPutStoredScriptAction.class, - RestDeleteStoredScriptAction.class, - - RestFieldStatsAction.class, - - // no abstract cat action - RestCatAction.class, - - // Tasks API - RestListTasksAction.class, - RestGetTaskAction.class, - RestCancelTasksAction.class, - - // Ingest API - RestPutPipelineAction.class, - RestGetPipelineAction.class, - RestDeletePipelineAction.class, - RestSimulatePipelineAction.class - ); - - private static final List> builtinCatHandlers = Arrays.asList( - RestAllocationAction.class, - RestShardsAction.class, - RestMasterAction.class, - RestNodesAction.class, - RestTasksAction.class, - RestIndicesAction.class, - RestSegmentsAction.class, - // Fully qualified to prevent interference with rest.action.count.RestCountAction - org.elasticsearch.rest.action.cat.RestCountAction.class, - // Fully qualified to prevent interference with rest.action.indices.RestRecoveryAction - org.elasticsearch.rest.action.cat.RestRecoveryAction.class, - RestHealthAction.class, - org.elasticsearch.rest.action.cat.RestPendingClusterTasksAction.class, - RestAliasAction.class, - RestThreadPoolAction.class, - RestPluginsAction.class, - RestFielddataAction.class, - RestNodeAttrsAction.class, - RestRepositoriesAction.class, - RestSnapshotAction.class - ); - private final NetworkService networkService; private final Settings settings; private final boolean transportClient; @@ -313,9 +73,6 @@ public class NetworkModule extends AbstractModule { private final ExtensionPoint.SelectedType transportServiceTypes = new ExtensionPoint.SelectedType<>("transport_service", TransportService.class); private final ExtensionPoint.SelectedType transportTypes = new ExtensionPoint.SelectedType<>("transport", Transport.class); private final ExtensionPoint.SelectedType httpTransportTypes = new ExtensionPoint.SelectedType<>("http_transport", HttpServerTransport.class); - private final ExtensionPoint.ClassSet restHandlers = new ExtensionPoint.ClassSet<>("rest_handler", RestHandler.class); - // we must separate the cat rest handlers so RestCatAction can collect them... - private final ExtensionPoint.ClassSet catHandlers = new ExtensionPoint.ClassSet<>("cat_handler", AbstractCatAction.class); private final NamedWriteableRegistry namedWriteableRegistry; /** @@ -340,13 +97,6 @@ public class NetworkModule extends AbstractModule { if (transportClient == false) { registerHttpTransport(NETTY_TRANSPORT, NettyHttpServerTransport.class); - - for (Class catAction : builtinCatHandlers) { - catHandlers.registerExtension(catAction); - } - for (Class restAction : builtinRestHandlers) { - restHandlers.registerExtension(restAction); - } } } @@ -373,19 +123,6 @@ public class NetworkModule extends AbstractModule { httpTransportTypes.registerExtension(name, clazz); } - /** Adds an additional rest action. */ - // TODO: change this further to eliminate the middle man, ie RestController, and just register method and path here - public void registerRestHandler(Class clazz) { - if (transportClient) { - throw new IllegalArgumentException("Cannot register rest handler " + clazz.getName() + " for transport client"); - } - if (AbstractCatAction.class.isAssignableFrom(clazz)) { - catHandlers.registerExtension(clazz.asSubclass(AbstractCatAction.class)); - } else { - restHandlers.registerExtension(clazz); - } - } - public void registerTaskStatus(String name, Writeable.Reader reader) { namedWriteableRegistry.register(Task.Status.class, name, reader); } @@ -430,9 +167,6 @@ public class NetworkModule extends AbstractModule { bind(HttpServer.class).asEagerSingleton(); httpTransportTypes.bindType(binder(), settings, HTTP_TYPE_SETTING.getKey(), NETTY_TRANSPORT); } - bind(RestController.class).asEagerSingleton(); - catHandlers.bind(binder()); - restHandlers.bind(binder()); // Bind the AllocationCommandRegistry so RestClusterRerouteAction can get it. bind(AllocationCommandRegistry.class).toInstance(allocationCommandRegistry); } diff --git a/core/src/main/java/org/elasticsearch/node/Node.java b/core/src/main/java/org/elasticsearch/node/Node.java index 211ff9ffe65..1d18f696163 100644 --- a/core/src/main/java/org/elasticsearch/node/Node.java +++ b/core/src/main/java/org/elasticsearch/node/Node.java @@ -26,6 +26,8 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchTimeoutException; import org.elasticsearch.Version; import org.elasticsearch.action.ActionModule; +import org.elasticsearch.action.GenericAction; +import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.client.Client; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.ClusterModule; @@ -42,6 +44,7 @@ import org.elasticsearch.common.StopWatch; import org.elasticsearch.common.component.Lifecycle; import org.elasticsearch.common.component.LifecycleComponent; import org.elasticsearch.common.inject.Injector; +import org.elasticsearch.common.inject.Key; import org.elasticsearch.common.inject.Module; import org.elasticsearch.common.inject.ModulesBuilder; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; @@ -122,6 +125,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.function.Function; @@ -165,7 +169,7 @@ public class Node implements Closeable { private final Environment environment; private final NodeEnvironment nodeEnvironment; private final PluginsService pluginsService; - private final Client client; + private final NodeClient client; /** * Constructs a node with the given settings. @@ -275,9 +279,10 @@ public class Node implements Closeable { BigArrays bigArrays = createBigArrays(settings, circuitBreakerService); resourcesToClose.add(bigArrays); modules.add(settingsModule); + client = new NodeClient(settings, threadPool); modules.add(b -> { b.bind(PluginsService.class).toInstance(pluginsService); - b.bind(Client.class).to(NodeClient.class).asEagerSingleton(); + b.bind(Client.class).toInstance(client); b.bind(Environment.class).toInstance(environment); b.bind(ThreadPool.class).toInstance(threadPool); b.bind(NodeEnvironment.class).toInstance(nodeEnvironment); @@ -290,7 +295,7 @@ public class Node implements Closeable { } ); injector = modules.createInjector(); - client = injector.getInstance(Client.class); + client.intialize(injector.getInstance(new Key>() {})); success = true; } catch (IOException ex) { throw new ElasticsearchException("failed to bind service", ex); diff --git a/core/src/main/java/org/elasticsearch/plugins/ActionPlugin.java b/core/src/main/java/org/elasticsearch/plugins/ActionPlugin.java index 0d4ee16ab89..f05c6654393 100644 --- a/core/src/main/java/org/elasticsearch/plugins/ActionPlugin.java +++ b/core/src/main/java/org/elasticsearch/plugins/ActionPlugin.java @@ -25,8 +25,11 @@ import org.elasticsearch.action.GenericAction; import org.elasticsearch.action.support.ActionFilter; import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.action.support.TransportActions; +import org.elasticsearch.common.Strings; +import org.elasticsearch.rest.RestHandler; import java.util.List; +import java.util.Objects; import static java.util.Collections.emptyList; @@ -55,6 +58,12 @@ public interface ActionPlugin { default List> getActionFilters() { return emptyList(); } + /** + * Rest handlers added by this plugin. + */ + default List> getRestHandlers() { + return emptyList(); + } public static final class ActionHandler, Response extends ActionResponse> { private final GenericAction action; @@ -83,5 +92,30 @@ public interface ActionPlugin { public Class[] getSupportTransportActions() { return supportTransportActions; } + + @Override + public String toString() { + StringBuilder b = new StringBuilder().append(action.name()).append(" is handled by ").append(transportAction.getName()); + if (supportTransportActions.length > 0) { + b.append('[').append(Strings.arrayToCommaDelimitedString(supportTransportActions)).append(']'); + } + return b.toString(); + } + + @Override + public boolean equals(Object obj) { + if (obj == null || obj.getClass() != ActionHandler.class) { + return false; + } + ActionHandler other = (ActionHandler) obj; + return Objects.equals(action, other.action) + && Objects.equals(transportAction, other.transportAction) + && Objects.deepEquals(supportTransportActions, other.supportTransportActions); + } + + @Override + public int hashCode() { + return Objects.hash(action, transportAction, supportTransportActions); + } } } diff --git a/core/src/main/java/org/elasticsearch/rest/RestController.java b/core/src/main/java/org/elasticsearch/rest/RestController.java index be72ed19c40..e8212ff09c6 100644 --- a/core/src/main/java/org/elasticsearch/rest/RestController.java +++ b/core/src/main/java/org/elasticsearch/rest/RestController.java @@ -22,7 +22,6 @@ package org.elasticsearch.rest; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.component.AbstractLifecycleComponent; -import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.path.PathTrie; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; @@ -59,7 +58,6 @@ public class RestController extends AbstractLifecycleComponent { // non volatile since the assumption is that pre processors are registered on startup private RestFilter[] filters = new RestFilter[0]; - @Inject public RestController(Settings settings) { super(settings); } diff --git a/core/src/test/java/org/elasticsearch/action/ActionModuleTests.java b/core/src/test/java/org/elasticsearch/action/ActionModuleTests.java new file mode 100644 index 00000000000..4b9a833e8c4 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/action/ActionModuleTests.java @@ -0,0 +1,129 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.action; + +import org.elasticsearch.action.main.MainAction; +import org.elasticsearch.action.main.TransportMainAction; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.TransportAction; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.plugins.ActionPlugin; +import org.elasticsearch.plugins.ActionPlugin.ActionHandler; +import org.elasticsearch.rest.RestChannel; +import org.elasticsearch.rest.RestHandler; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.action.main.RestMainAction; +import org.elasticsearch.tasks.TaskManager; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.ThreadPool; + +import java.util.List; + +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; +import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.hasItem; + +public class ActionModuleTests extends ESTestCase { + public void testSetupActionsContainsKnownBuiltin() { + assertThat(ActionModule.setupActions(emptyList()), + hasEntry(MainAction.INSTANCE.name(), new ActionHandler<>(MainAction.INSTANCE, TransportMainAction.class))); + } + + public void testPluginCantOverwriteBuiltinAction() { + ActionPlugin dupsMainAction = new ActionPlugin() { + @Override + public List, ? extends ActionResponse>> getActions() { + return singletonList(new ActionHandler<>(MainAction.INSTANCE, TransportMainAction.class)); + } + }; + Exception e = expectThrows(IllegalArgumentException.class, () -> ActionModule.setupActions(singletonList(dupsMainAction))); + assertEquals("action for name [" + MainAction.NAME + "] already registered", e.getMessage()); + } + + public void testPluginCanRegisterAction() { + class FakeRequest extends ActionRequest { + @Override + public ActionRequestValidationException validate() { + return null; + } + } + class FakeTransportAction extends TransportAction { + protected FakeTransportAction(Settings settings, String actionName, ThreadPool threadPool, ActionFilters actionFilters, + IndexNameExpressionResolver indexNameExpressionResolver, TaskManager taskManager) { + super(settings, actionName, threadPool, actionFilters, indexNameExpressionResolver, taskManager); + } + + @Override + protected void doExecute(FakeRequest request, ActionListener listener) { + } + } + class FakeAction extends GenericAction { + protected FakeAction() { + super("fake"); + } + + @Override + public ActionResponse newResponse() { + return null; + } + } + FakeAction action = new FakeAction(); + ActionPlugin registersFakeAction = new ActionPlugin() { + @Override + public List, ? extends ActionResponse>> getActions() { + return singletonList(new ActionHandler<>(action, FakeTransportAction.class)); + } + }; + assertThat(ActionModule.setupActions(singletonList(registersFakeAction)), + hasEntry("fake", new ActionHandler<>(action, FakeTransportAction.class))); + } + + public void testSetupRestHandlerContainsKnownBuiltin() { + assertThat(ActionModule.setupRestHandlers(emptyList()), hasItem(RestMainAction.class)); + } + + public void testPluginCantOverwriteBuiltinRestHandler() { + ActionPlugin dupsMainAction = new ActionPlugin() { + @Override + public List> getRestHandlers() { + return singletonList(RestMainAction.class); + } + }; + Exception e = expectThrows(IllegalArgumentException.class, () -> ActionModule.setupRestHandlers(singletonList(dupsMainAction))); + assertEquals("can't register the same [rest_handler] more than once for [" + RestMainAction.class.getName() + "]", e.getMessage()); + } + + public void testPluginCanRegisterRestHandler() { + class FakeHandler implements RestHandler { + @Override + public void handleRequest(RestRequest request, RestChannel channel) throws Exception { + } + } + ActionPlugin registersFakeHandler = new ActionPlugin() { + @Override + public List> getRestHandlers() { + return singletonList(FakeHandler.class); + } + }; + assertThat(ActionModule.setupRestHandlers(singletonList(registersFakeHandler)), hasItem(FakeHandler.class)); + } +} diff --git a/core/src/test/java/org/elasticsearch/client/node/NodeClientHeadersTests.java b/core/src/test/java/org/elasticsearch/client/node/NodeClientHeadersTests.java index f69c8f2da0b..04f7b73b1f2 100644 --- a/core/src/test/java/org/elasticsearch/client/node/NodeClientHeadersTests.java +++ b/core/src/test/java/org/elasticsearch/client/node/NodeClientHeadersTests.java @@ -46,7 +46,9 @@ public class NodeClientHeadersTests extends AbstractClientHeadersTestCase { protected Client buildClient(Settings headersSettings, GenericAction[] testedActions) { Settings settings = HEADER_SETTINGS; Actions actions = new Actions(settings, threadPool, testedActions); - return new NodeClient(settings, threadPool, actions); + NodeClient client = new NodeClient(settings, threadPool); + client.intialize(actions); + return client; } private static class Actions extends HashMap { diff --git a/core/src/test/java/org/elasticsearch/common/network/NetworkModuleTests.java b/core/src/test/java/org/elasticsearch/common/network/NetworkModuleTests.java index d54edbcaa9d..749ffa3c9d9 100644 --- a/core/src/test/java/org/elasticsearch/common/network/NetworkModuleTests.java +++ b/core/src/test/java/org/elasticsearch/common/network/NetworkModuleTests.java @@ -36,11 +36,8 @@ import org.elasticsearch.http.HttpServerTransport; import org.elasticsearch.http.HttpStats; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestChannel; -import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.action.cat.AbstractCatAction; -import org.elasticsearch.rest.action.cat.RestNodesAction; -import org.elasticsearch.rest.action.main.RestMainAction; import org.elasticsearch.tasks.Task; import org.elasticsearch.test.transport.AssertingLocalTransport; import org.elasticsearch.transport.Transport; @@ -163,32 +160,6 @@ public class NetworkModuleTests extends ModuleTestCase { assertFalse(module.isTransportClient()); } - public void testRegisterRestHandler() { - Settings settings = Settings.EMPTY; - NetworkModule module = new NetworkModule(new NetworkService(settings), settings, false, new NamedWriteableRegistry()); - module.registerRestHandler(FakeRestHandler.class); - // also check a builtin is bound - assertSetMultiBinding(module, RestHandler.class, FakeRestHandler.class, RestMainAction.class); - - // check registration not allowed for transport only - module = new NetworkModule(new NetworkService(settings), settings, true, new NamedWriteableRegistry()); - try { - module.registerRestHandler(FakeRestHandler.class); - fail(); - } catch (IllegalArgumentException e) { - assertTrue(e.getMessage().contains("Cannot register rest handler")); - assertTrue(e.getMessage().contains("for transport client")); - } - } - - public void testRegisterCatRestHandler() { - Settings settings = Settings.EMPTY; - NetworkModule module = new NetworkModule(new NetworkService(settings), settings, false, new NamedWriteableRegistry()); - module.registerRestHandler(FakeCatRestHandler.class); - // also check a builtin is bound - assertSetMultiBinding(module, AbstractCatAction.class, FakeCatRestHandler.class, RestNodesAction.class); - } - public void testRegisterTaskStatus() { NamedWriteableRegistry registry = new NamedWriteableRegistry(); Settings settings = Settings.EMPTY; diff --git a/core/src/test/java/org/elasticsearch/plugins/responseheader/TestResponseHeaderPlugin.java b/core/src/test/java/org/elasticsearch/plugins/responseheader/TestResponseHeaderPlugin.java index 701d1154587..9dfd5b6a93a 100644 --- a/core/src/test/java/org/elasticsearch/plugins/responseheader/TestResponseHeaderPlugin.java +++ b/core/src/test/java/org/elasticsearch/plugins/responseheader/TestResponseHeaderPlugin.java @@ -19,12 +19,17 @@ package org.elasticsearch.plugins.responseheader; -import org.elasticsearch.common.network.NetworkModule; +import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.rest.RestHandler; -public class TestResponseHeaderPlugin extends Plugin { +import java.util.List; - public void onModule(NetworkModule module) { - module.registerRestHandler(TestResponseHeaderRestAction.class); +import static java.util.Collections.singletonList; + +public class TestResponseHeaderPlugin extends Plugin implements ActionPlugin { + @Override + public List> getRestHandlers() { + return singletonList(TestResponseHeaderRestAction.class); } } diff --git a/docs/reference/migration/migrate_5_0/plugins.asciidoc b/docs/reference/migration/migrate_5_0/plugins.asciidoc index e1e8e6c614b..2826c822d15 100644 --- a/docs/reference/migration/migrate_5_0/plugins.asciidoc +++ b/docs/reference/migration/migrate_5_0/plugins.asciidoc @@ -137,3 +137,6 @@ Plugins that register custom mappers should implement Plugins that register custom actions should implement `ActionPlugin` and remove their `onModule(ActionModule)` implementation. + +Plugins that register custom `RestHandler`s should implement `ActionPlugin` and +remove their `onModule(NetworkModule)` implemnetation. diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustachePlugin.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustachePlugin.java index 50bae57887c..d3ffa13cd54 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustachePlugin.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustachePlugin.java @@ -25,11 +25,11 @@ import org.elasticsearch.action.search.template.MultiSearchTemplateAction; import org.elasticsearch.action.search.template.SearchTemplateAction; import org.elasticsearch.action.search.template.TransportMultiSearchTemplateAction; import org.elasticsearch.action.search.template.TransportSearchTemplateAction; -import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.ScriptPlugin; +import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.action.search.template.RestDeleteSearchTemplateAction; import org.elasticsearch.rest.action.search.template.RestGetSearchTemplateAction; import org.elasticsearch.rest.action.search.template.RestMultiSearchTemplateAction; @@ -54,14 +54,9 @@ public class MustachePlugin extends Plugin implements ScriptPlugin, ActionPlugin new ActionHandler<>(MultiSearchTemplateAction.INSTANCE, TransportMultiSearchTemplateAction.class)); } - public void onModule(NetworkModule module) { - if (module.isTransportClient() == false) { - module.registerRestHandler(RestSearchTemplateAction.class); - module.registerRestHandler(RestMultiSearchTemplateAction.class); - module.registerRestHandler(RestGetSearchTemplateAction.class); - module.registerRestHandler(RestPutSearchTemplateAction.class); - module.registerRestHandler(RestDeleteSearchTemplateAction.class); - module.registerRestHandler(RestRenderSearchTemplateAction.class); - } + @Override + public List> getRestHandlers() { + return Arrays.asList(RestSearchTemplateAction.class, RestMultiSearchTemplateAction.class, RestGetSearchTemplateAction.class, + RestPutSearchTemplateAction.class, RestDeleteSearchTemplateAction.class, RestRenderSearchTemplateAction.class); } } diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorPlugin.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorPlugin.java index 87fc6726c56..a15acf67c02 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorPlugin.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorPlugin.java @@ -23,13 +23,13 @@ import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.client.Client; import org.elasticsearch.client.transport.TransportClient; -import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.mapper.Mapper; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.MapperPlugin; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.rest.RestHandler; import org.elasticsearch.search.SearchModule; import java.util.Arrays; @@ -41,11 +41,9 @@ public class PercolatorPlugin extends Plugin implements MapperPlugin, ActionPlug public static final String NAME = "percolator"; - private final boolean transportClientMode; private final Settings settings; public PercolatorPlugin(Settings settings) { - this.transportClientMode = transportClientMode(settings); this.settings = settings; } @@ -55,11 +53,9 @@ public class PercolatorPlugin extends Plugin implements MapperPlugin, ActionPlug new ActionHandler<>(MultiPercolateAction.INSTANCE, TransportMultiPercolateAction.class)); } - public void onModule(NetworkModule module) { - if (transportClientMode == false) { - module.registerRestHandler(RestPercolateAction.class); - module.registerRestHandler(RestMultiPercolateAction.class); - } + @Override + public List> getRestHandlers() { + return Arrays.asList(RestPercolateAction.class, RestMultiPercolateAction.class); } public void onModule(SearchModule module) { diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/ReindexPlugin.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/ReindexPlugin.java index 89e6c1cc753..5a7b81a7124 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/ReindexPlugin.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/ReindexPlugin.java @@ -24,6 +24,7 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.rest.RestHandler; import java.util.Arrays; import java.util.List; @@ -39,11 +40,13 @@ public class ReindexPlugin extends Plugin implements ActionPlugin { new ActionHandler<>(RethrottleAction.INSTANCE, TransportRethrottleAction.class)); } + @Override + public List> getRestHandlers() { + return Arrays.asList(RestReindexAction.class, RestUpdateByQueryAction.class, RestDeleteByQueryAction.class, + RestRethrottleAction.class); + } + public void onModule(NetworkModule networkModule) { - networkModule.registerRestHandler(RestReindexAction.class); - networkModule.registerRestHandler(RestUpdateByQueryAction.class); - networkModule.registerRestHandler(RestDeleteByQueryAction.class); - networkModule.registerRestHandler(RestRethrottleAction.class); networkModule.registerTaskStatus(BulkByScrollTask.Status.NAME, BulkByScrollTask.Status::new); } } From 172ced3e2df5c960c591dee04ab2bdfee8f24f1d Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 29 Jun 2016 15:56:36 -0700 Subject: [PATCH 4/5] Fix test bug in plugin cli progress tests --- .../elasticsearch/plugins/ProgressInputStreamTests.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/test/java/org/elasticsearch/plugins/ProgressInputStreamTests.java b/core/src/test/java/org/elasticsearch/plugins/ProgressInputStreamTests.java index 81e937d26a9..813921963c0 100644 --- a/core/src/test/java/org/elasticsearch/plugins/ProgressInputStreamTests.java +++ b/core/src/test/java/org/elasticsearch/plugins/ProgressInputStreamTests.java @@ -69,19 +69,19 @@ public class ProgressInputStreamTests extends ESTestCase { } public void testOddBytes() throws Exception { - int odd = (randomIntBetween(100, 200) / 2) + 1; + int odd = randomIntBetween(10, 100) * 2 + 1; ProgressInputStream is = newProgressInputStream(odd); for (int i = 0; i < odd; i++) { is.checkProgress(1); } is.checkProgress(-1); - assertThat(progresses, hasSize(odd+1)); + assertThat(progresses, hasSize(Math.min(odd + 1, 100))); assertThat(progresses, hasItem(100)); } public void testEvenBytes() throws Exception { - int even = (randomIntBetween(100, 200) / 2); + int even = randomIntBetween(10, 100) * 2; ProgressInputStream is = newProgressInputStream(even); for (int i = 0; i < even; i++) { @@ -89,7 +89,7 @@ public class ProgressInputStreamTests extends ESTestCase { } is.checkProgress(-1); - assertThat(progresses, hasSize(even+1)); + assertThat(progresses, hasSize(Math.min(even + 1, 100))); assertThat(progresses, hasItem(100)); } From 7c50de182e9c5cc0a63f4d4e5138dec8d619d3ba Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 29 Jun 2016 16:23:16 -0700 Subject: [PATCH 5/5] Remove test for closing ingest processors, this is now handled at the plugin level --- .../elasticsearch/ingest/IngestCloseIT.java | 73 ------------------- 1 file changed, 73 deletions(-) delete mode 100644 core/src/test/java/org/elasticsearch/ingest/IngestCloseIT.java diff --git a/core/src/test/java/org/elasticsearch/ingest/IngestCloseIT.java b/core/src/test/java/org/elasticsearch/ingest/IngestCloseIT.java deleted file mode 100644 index ed0a302cf73..00000000000 --- a/core/src/test/java/org/elasticsearch/ingest/IngestCloseIT.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.ingest; - -import org.elasticsearch.node.NodeModule; -import org.elasticsearch.plugins.Plugin; -import org.elasticsearch.test.ESSingleNodeTestCase; - -import java.io.Closeable; -import java.io.IOException; -import java.util.Collection; -import java.util.Map; -import java.util.concurrent.atomic.AtomicBoolean; - -import static org.hamcrest.Matchers.is; - -public class IngestCloseIT extends ESSingleNodeTestCase { - - @Override - protected Collection> getPlugins() { - return pluginList(IngestPlugin.class); - } - - private static AtomicBoolean called = new AtomicBoolean(false); - - public void testCloseNode() throws Exception { - // We manually stop the node and check we called - stopNode(); - - assertThat(called.get(), is(true)); - - // We need to restart the node for the next tests (and because tearDown() expects a Node) - startNode(); - } - - public static class IngestPlugin extends Plugin { - public void onModule(NodeModule nodeModule) { - nodeModule.registerProcessor("test", (registry) -> new Factory()); - } - } - - public static final class Factory extends AbstractProcessorFactory implements Closeable { - @Override - protected TestProcessor doCreate(String tag, Map config) throws Exception { - return new TestProcessor("id", "test", ingestDocument -> { - throw new UnsupportedOperationException("this code is actually never called from the test"); - }); - } - - @Override - public void close() throws IOException { - called.set(true); - } - } - -}