From 03cbf821012c894a618f4f97e126c5192766ada7 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sat, 2 Jul 2016 14:56:46 -0700 Subject: [PATCH] Remove WatcherSettingsValidation service This looks like it predates settings validation in core, and only had a single use inside the watcher ExecutionService. This change moves the settings inside ExecutionService to be validated settings, and removes the watcher specific validation. Original commit: elastic/x-pack-elasticsearch@82843ce56c4a0634fb6a36997e144e31c5bdd75c --- .../elasticsearch/xpack/watcher/Watcher.java | 12 ++-- .../xpack/watcher/WatcherModule.java | 2 - .../watcher/execution/ExecutionService.java | 21 +++---- .../validation/WatcherSettingsValidation.java | 57 ------------------- .../execution/ExecutionServiceTests.java | 4 +- 5 files changed, 16 insertions(+), 80 deletions(-) delete mode 100644 elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/validation/WatcherSettingsValidation.java diff --git a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index 3ec90aedeef..30e9654f19b 100644 --- a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -19,6 +19,7 @@ import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.rest.RestHandler; @@ -29,6 +30,7 @@ import org.elasticsearch.xpack.watcher.actions.WatcherActionModule; import org.elasticsearch.xpack.watcher.client.WatcherClientModule; import org.elasticsearch.xpack.watcher.condition.ConditionModule; import org.elasticsearch.xpack.watcher.execution.ExecutionModule; +import org.elasticsearch.xpack.watcher.execution.ExecutionService; import org.elasticsearch.xpack.watcher.execution.InternalWatchExecutor; import org.elasticsearch.xpack.watcher.history.HistoryModule; import org.elasticsearch.xpack.watcher.history.HistoryStore; @@ -44,7 +46,6 @@ import org.elasticsearch.xpack.watcher.rest.action.RestWatchServiceAction; import org.elasticsearch.xpack.watcher.rest.action.RestWatcherStatsAction; import org.elasticsearch.xpack.watcher.support.WatcherIndexTemplateRegistry; import org.elasticsearch.xpack.watcher.support.WatcherIndexTemplateRegistry.TemplateConfig; -import org.elasticsearch.xpack.watcher.support.validation.WatcherSettingsValidation; import org.elasticsearch.xpack.watcher.transform.TransformModule; import org.elasticsearch.xpack.watcher.transport.actions.ack.AckWatchAction; import org.elasticsearch.xpack.watcher.transport.actions.ack.TransportAckWatchAction; @@ -87,6 +88,8 @@ public class Watcher implements ActionPlugin { new Setting<>("index.xpack.watcher.template.version", "", Function.identity(), Setting.Property.IndexScope); public static final Setting ENCRYPT_SENSITIVE_DATA_SETTING = Setting.boolSetting("xpack.watcher.encrypt_sensitive_data", false, Setting.Property.NodeScope); + public static final Setting MAX_STOP_TIMEOUT_SETTING = + Setting.timeSetting("xpack.watcher.stop.timeout", TimeValue.timeValueSeconds(30), Setting.Property.NodeScope); private static final ESLogger logger = Loggers.getLogger(XPackPlugin.class); @@ -127,9 +130,7 @@ public class Watcher implements ActionPlugin { if (enabled == false|| transportClient) { return Collections.emptyList(); } - return Arrays.>asList( - WatcherLicensee.class, - WatcherSettingsValidation.class); + return Collections.singletonList(WatcherLicensee.class); } public Settings additionalSettings() { @@ -144,6 +145,8 @@ public class Watcher implements ActionPlugin { } settings.add(INDEX_WATCHER_VERSION_SETTING); settings.add(INDEX_WATCHER_TEMPLATE_VERSION_SETTING); + settings.add(MAX_STOP_TIMEOUT_SETTING); + settings.add(ExecutionService.DEFAULT_THROTTLE_PERIOD_SETTING); settings.add(Setting.intSetting("xpack.watcher.execution.scroll.size", 0, Setting.Property.NodeScope)); settings.add(Setting.intSetting("xpack.watcher.watch.scroll.size", 0, Setting.Property.NodeScope)); settings.add(Setting.boolSetting(XPackPlugin.featureEnabledSetting(Watcher.NAME), true, Setting.Property.NodeScope)); @@ -152,7 +155,6 @@ public class Watcher implements ActionPlugin { settings.add(Setting.simpleString("xpack.watcher.internal.ops.search.default_timeout", Setting.Property.NodeScope)); settings.add(Setting.simpleString("xpack.watcher.internal.ops.bulk.default_timeout", Setting.Property.NodeScope)); settings.add(Setting.simpleString("xpack.watcher.internal.ops.index.default_timeout", Setting.Property.NodeScope)); - settings.add(Setting.simpleString("xpack.watcher.execution.default_throttle_period", Setting.Property.NodeScope)); settings.add(Setting.simpleString("xpack.watcher.actions.index.default_timeout", Setting.Property.NodeScope)); settings.add(Setting.simpleString("xpack.watcher.index.rest.direct_access", Setting.Property.NodeScope)); settings.add(Setting.simpleString("xpack.watcher.trigger.schedule.engine", Setting.Property.NodeScope)); diff --git a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherModule.java b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherModule.java index a51e4a7bffe..01aa4a8ad62 100644 --- a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherModule.java +++ b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherModule.java @@ -10,7 +10,6 @@ import org.elasticsearch.common.inject.AbstractModule; import org.elasticsearch.common.inject.util.Providers; import org.elasticsearch.xpack.XPackPlugin; import org.elasticsearch.xpack.watcher.support.WatcherIndexTemplateRegistry; -import org.elasticsearch.xpack.watcher.support.validation.WatcherSettingsValidation; public class WatcherModule extends AbstractModule { @@ -34,7 +33,6 @@ public class WatcherModule extends AbstractModule { bind(WatcherLicensee.class).asEagerSingleton(); bind(WatcherLifeCycleService.class).asEagerSingleton(); - bind(WatcherSettingsValidation.class).asEagerSingleton(); bind(WatcherIndexTemplateRegistry.class).asEagerSingleton(); } } diff --git a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/ExecutionService.java b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/ExecutionService.java index 219f7941586..b70198d35eb 100644 --- a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/ExecutionService.java +++ b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/ExecutionService.java @@ -10,16 +10,17 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException; +import org.elasticsearch.xpack.watcher.Watcher; import org.elasticsearch.xpack.watcher.actions.ActionWrapper; import org.elasticsearch.xpack.watcher.condition.Condition; import org.elasticsearch.xpack.watcher.history.HistoryStore; import org.elasticsearch.xpack.watcher.history.WatchRecord; import org.elasticsearch.xpack.watcher.input.Input; import org.elasticsearch.xpack.support.clock.Clock; -import org.elasticsearch.xpack.watcher.support.validation.WatcherSettingsValidation; import org.elasticsearch.xpack.watcher.transform.Transform; import org.elasticsearch.xpack.watcher.trigger.TriggerEvent; import org.elasticsearch.xpack.watcher.watch.Watch; @@ -35,17 +36,15 @@ import java.util.Collections; import java.util.Comparator; import java.util.LinkedList; import java.util.List; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import static org.elasticsearch.common.logging.LoggerMessageFormat.format; -/** - */ public class ExecutionService extends AbstractComponent { - private static final TimeValue DEFAULT_MAX_STOP_TIMEOUT = new TimeValue(30, TimeUnit.SECONDS); - private static final String DEFAULT_MAX_STOP_TIMEOUT_SETTING = "xpack.watcher.stop.timeout"; + public static final Setting DEFAULT_THROTTLE_PERIOD_SETTING = + Setting.positiveTimeSetting("xpack.watcher.execution.default_throttle_period", + TimeValue.timeValueSeconds(5), Setting.Property.NodeScope); private final HistoryStore historyStore; private final TriggeredWatchStore triggeredWatchStore; @@ -61,8 +60,7 @@ public class ExecutionService extends AbstractComponent { @Inject public ExecutionService(Settings settings, HistoryStore historyStore, TriggeredWatchStore triggeredWatchStore, WatchExecutor executor, - WatchStore watchStore, WatchLockService watchLockService, Clock clock, - WatcherSettingsValidation settingsValidation) { + WatchStore watchStore, WatchLockService watchLockService, Clock clock) { super(settings); this.historyStore = historyStore; this.triggeredWatchStore = triggeredWatchStore; @@ -70,11 +68,8 @@ public class ExecutionService extends AbstractComponent { this.watchStore = watchStore; this.watchLockService = watchLockService; this.clock = clock; - this.defaultThrottlePeriod = settings.getAsTime("xpack.watcher.execution.default_throttle_period", TimeValue.timeValueSeconds(5)); - maxStopTimeout = settings.getAsTime(DEFAULT_MAX_STOP_TIMEOUT_SETTING, DEFAULT_MAX_STOP_TIMEOUT); - if (ExecutionService.this.defaultThrottlePeriod.millis() < 0) { - settingsValidation.addError("xpack.watcher.execution.default_throttle_period", "time value cannot be negative"); - } + this.defaultThrottlePeriod = DEFAULT_THROTTLE_PERIOD_SETTING.get(settings); + this.maxStopTimeout = Watcher.MAX_STOP_TIMEOUT_SETTING.get(settings); } public void start(ClusterState state) throws Exception { diff --git a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/validation/WatcherSettingsValidation.java b/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/validation/WatcherSettingsValidation.java deleted file mode 100644 index f320d151963..00000000000 --- a/elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/validation/WatcherSettingsValidation.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.watcher.support.validation; - -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.common.component.AbstractLifecycleComponent; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.logging.LoggerMessageFormat; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.xpack.watcher.support.Exceptions; - -import java.util.ArrayList; -import java.util.List; - -/** - * - */ -public class WatcherSettingsValidation extends AbstractLifecycleComponent { - - private List errors = new ArrayList<>(); - - @Inject - public WatcherSettingsValidation(Settings settings) { - super(settings); - } - - @Override - protected void doStart() throws ElasticsearchException { - validate(); - } - - @Override - protected void doStop() throws ElasticsearchException { - } - - @Override - protected void doClose() throws ElasticsearchException { - } - - public void addError(String setting, String reason) { - errors.add(LoggerMessageFormat.format("", "invalid [{}] setting value [{}]. {}", setting, settings.get(setting), reason)); - } - - private void validate() throws ElasticsearchException { - if (errors.isEmpty()) { - return; - } - StringBuilder sb = new StringBuilder("encountered invalid watcher settings:\n"); - for (String error : errors) { - sb.append("- ").append(error).append("\n"); - } - throw Exceptions.invalidSettings(sb.toString()); - } -} diff --git a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/execution/ExecutionServiceTests.java b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/execution/ExecutionServiceTests.java index 8395da0acda..348eae3a904 100644 --- a/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/execution/ExecutionServiceTests.java +++ b/elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/execution/ExecutionServiceTests.java @@ -25,7 +25,6 @@ import org.elasticsearch.xpack.watcher.input.ExecutableInput; import org.elasticsearch.xpack.watcher.input.Input; import org.elasticsearch.xpack.support.clock.Clock; import org.elasticsearch.xpack.support.clock.ClockMock; -import org.elasticsearch.xpack.watcher.support.validation.WatcherSettingsValidation; import org.elasticsearch.xpack.watcher.transform.ExecutableTransform; import org.elasticsearch.xpack.watcher.transform.Transform; import org.elasticsearch.xpack.watcher.trigger.schedule.ScheduleTriggerEvent; @@ -89,10 +88,9 @@ public class ExecutionServiceTests extends ESTestCase { when(executor.queue()).thenReturn(new ArrayBlockingQueue(1)); watchLockService = mock(WatchLockService.class); - WatcherSettingsValidation settingsValidator = mock(WatcherSettingsValidation.class); clock = new ClockMock(); executionService = new ExecutionService(Settings.EMPTY, historyStore, triggeredWatchStore, executor, watchStore, - watchLockService, clock, settingsValidator); + watchLockService, clock); ClusterState clusterState = mock(ClusterState.class); when(triggeredWatchStore.loadTriggeredWatches(clusterState)).thenReturn(new ArrayList());