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@82843ce56c

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<Boolean> ENCRYPT_SENSITIVE_DATA_SETTING =
             Setting.boolSetting("xpack.watcher.encrypt_sensitive_data", false, Setting.Property.NodeScope);
+    public static final Setting<TimeValue> 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.<Class<? extends LifecycleComponent>>asList(
+        return Collections.singletonList(WatcherLicensee.class);
-            WatcherLicensee.class,
-            WatcherSettingsValidation.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));

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();
     }
 }

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);
+    public static final Setting<TimeValue> DEFAULT_THROTTLE_PERIOD_SETTING =
-    private static final String DEFAULT_MAX_STOP_TIMEOUT_SETTING = "xpack.watcher.stop.timeout";
+        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,
+                            WatchStore watchStore, WatchLockService watchLockService, Clock clock) {
-                            WatcherSettingsValidation settingsValidation) {
         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));
+        this.defaultThrottlePeriod = DEFAULT_THROTTLE_PERIOD_SETTING.get(settings);
-        maxStopTimeout = settings.getAsTime(DEFAULT_MAX_STOP_TIMEOUT_SETTING, DEFAULT_MAX_STOP_TIMEOUT);
+        this.maxStopTimeout = Watcher.MAX_STOP_TIMEOUT_SETTING.get(settings);
-        if (ExecutionService.this.defaultThrottlePeriod.millis() < 0) {
-            settingsValidation.addError("xpack.watcher.execution.default_throttle_period", "time value cannot be negative");
-        }
     }
 
     public void start(ClusterState state) throws Exception {

elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/validation/WatcherSettingsValidation.java

@@ -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<String> 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());
-    }
-}

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<Runnable>(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<TriggeredWatch>());