schedules: Don't allow schedules with negative interval.

Closes elastic/elasticsearch#270

Original commit: elastic/x-pack-elasticsearch@a24941683b

src/main/java/org/elasticsearch/watcher/trigger/TriggerException.java

@@ -12,11 +12,11 @@ import org.elasticsearch.watcher.WatcherException;
  */
 public class TriggerException extends WatcherException {
 
-    public TriggerException(String msg) {
+    public TriggerException(String msg, Object... args) {
-        super(msg);
+        super(msg, args);
     }
 
-    public TriggerException(String msg, Throwable cause) {
+    public TriggerException(String msg, Throwable cause, Object... args) {
-        super(msg, cause);
+        super(msg, cause, args);
     }
 }

src/main/java/org/elasticsearch/watcher/trigger/schedule/IntervalSchedule.java

@@ -24,6 +24,9 @@ public class IntervalSchedule implements Schedule {
     private final Interval interval;
 
     public IntervalSchedule(Interval interval) {
+        if (interval.millis < 1000) {
+            throw new ScheduleTriggerException("interval can't be lower than 1000 ms, but [{}] was specified", interval.millis);
+        }
         this.interval = interval;
     }
 

src/main/java/org/elasticsearch/watcher/trigger/schedule/ScheduleTriggerException.java

@@ -0,0 +1,19 @@
+/*
+ * 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.watcher.trigger.schedule;
+
+import org.elasticsearch.watcher.trigger.TriggerException;
+
+public class ScheduleTriggerException extends TriggerException {
+
+    public ScheduleTriggerException(String msg, Object... args) {
+        super(msg, args);
+    }
+
+    public ScheduleTriggerException(String msg, Throwable cause, Object... args) {
+        super(msg, cause, args);
+    }
+}

src/test/java/org/elasticsearch/watcher/test/integration/BasicWatcherTests.java

@@ -15,6 +15,7 @@ import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.elasticsearch.test.junit.annotations.TestLogging;
 import org.elasticsearch.watcher.WatcherException;
+import org.elasticsearch.watcher.WatcherSettingsException;
 import org.elasticsearch.watcher.client.WatchSourceBuilder;
 import org.elasticsearch.watcher.client.WatcherClient;
 import org.elasticsearch.watcher.history.HistoryStore;
@@ -24,7 +25,10 @@ import org.elasticsearch.watcher.transport.actions.delete.DeleteWatchResponse;
 import org.elasticsearch.watcher.transport.actions.get.GetWatchResponse;
 import org.elasticsearch.watcher.transport.actions.put.PutWatchResponse;
 import org.elasticsearch.watcher.trigger.schedule.IntervalSchedule;
+import org.elasticsearch.watcher.trigger.schedule.ScheduleTriggerException;
 import org.elasticsearch.watcher.trigger.schedule.Schedules;
+import org.elasticsearch.watcher.trigger.schedule.support.MonthTimes;
+import org.elasticsearch.watcher.trigger.schedule.support.WeekTimes;
 import org.elasticsearch.watcher.watch.WatchStore;
 import org.junit.Test;
 
@@ -39,11 +43,13 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitC
 import static org.elasticsearch.watcher.actions.ActionBuilders.indexAction;
 import static org.elasticsearch.watcher.actions.ActionBuilders.loggingAction;
 import static org.elasticsearch.watcher.client.WatchSourceBuilders.watchBuilder;
+import static org.elasticsearch.watcher.condition.ConditionBuilders.alwaysCondition;
 import static org.elasticsearch.watcher.condition.ConditionBuilders.scriptCondition;
 import static org.elasticsearch.watcher.input.InputBuilders.searchInput;
+import static org.elasticsearch.watcher.input.InputBuilders.simpleInput;
 import static org.elasticsearch.watcher.test.WatcherTestUtils.newInputSearchRequest;
 import static org.elasticsearch.watcher.trigger.TriggerBuilders.schedule;
-import static org.elasticsearch.watcher.trigger.schedule.Schedules.interval;
+import static org.elasticsearch.watcher.trigger.schedule.Schedules.*;
 import static org.hamcrest.Matchers.*;
 
 /**
@@ -321,6 +327,74 @@ public class BasicWatcherTests extends AbstractWatcherIntegrationTests {
         assertThat((Integer) ((Map) payload.get("hits")).get("total"), equalTo(1));
     }
 
+    @Test
+    public void testPutWatchWithNegativeSchedule() throws Exception {
+        try {
+            watcherClient().preparePutWatch("_name")
+                    .setSource(watchBuilder()
+                            .trigger(schedule(interval(-5, IntervalSchedule.Interval.Unit.SECONDS)))
+                            .input(simpleInput("key", "value"))
+                            .condition(alwaysCondition())
+                            .addAction("_logger", loggingAction("executed!")))
+                    .get();
+            fail("put watch should have failed");
+        } catch (ScheduleTriggerException e) {
+            assertThat(e.getMessage(), equalTo("interval can't be lower than 1000 ms, but [-5000] was specified"));
+        }
+
+        try {
+            watcherClient().preparePutWatch("_name")
+                    .setSource(watchBuilder()
+                            .trigger(schedule(hourly().minutes(-10).build()))
+                            .input(simpleInput("key", "value"))
+                            .condition(alwaysCondition())
+                            .addAction("_logger", loggingAction("executed!")))
+                    .get();
+            fail("put watch should have failed");
+        } catch (WatcherSettingsException e) {
+            assertThat(e.getMessage(), equalTo("invalid hourly minute [-10]. minute must be between 0 and 59 incl."));
+        }
+
+        try {
+            watcherClient().preparePutWatch("_name")
+                    .setSource(watchBuilder()
+                            .trigger(schedule(daily().atRoundHour(-10).build()))
+                            .input(simpleInput("key", "value"))
+                            .condition(alwaysCondition())
+                            .addAction("_logger", loggingAction("executed!")))
+                    .get();
+            fail("put watch should have failed");
+        } catch (WatcherSettingsException e) {
+            assertThat(e.getMessage(), equalTo("invalid time [0-10:00]. invalid time hour value [-10]. time hours must be between 0 and 23 incl."));
+        }
+
+        try {
+            watcherClient().preparePutWatch("_name")
+                    .setSource(watchBuilder()
+                            .trigger(schedule(weekly().time(WeekTimes.builder().atRoundHour(-10).build()).build()))
+                                    .input(simpleInput("key", "value"))
+                                    .condition(alwaysCondition())
+                                    .addAction("_logger", loggingAction("executed!")))
+                            .get();
+            fail("put watch should have failed");
+        } catch (WatcherSettingsException e) {
+            assertThat(e.getMessage(), equalTo("invalid time [0-10:00]. invalid time hour value [-10]. time hours must be between 0 and 23 incl."));
+        }
+
+        try {
+            watcherClient().preparePutWatch("_name")
+                    .setSource(watchBuilder()
+                            .trigger(schedule(monthly().time(MonthTimes.builder().atRoundHour(-10).build()).build()))
+                            .input(simpleInput("key", "value"))
+                            .condition(alwaysCondition())
+                            .addAction("_logger", loggingAction("executed!")))
+                    .get();
+            fail("put watch should have failed");
+        } catch (WatcherSettingsException e) {
+            assertThat(e.getMessage(), equalTo("invalid time [0-10:00]. invalid time hour value [-10]. time hours must be between 0 and 23 incl."));
+        }
+    }
+
     private void testConditionSearch(SearchRequest request) throws Exception {
         if (timeWarped()) {
             // reset, so we don't miss event docs when we filter over the _timestamp field.

src/test/java/org/elasticsearch/watcher/trigger/schedule/IntervalScheduleTests.java

@@ -14,8 +14,7 @@ import org.elasticsearch.test.ElasticsearchTestCase;
 import org.junit.Test;
 
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
-import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.*;
-import static org.hamcrest.Matchers.notNullValue;
 
 /**
  *
@@ -24,7 +23,7 @@ public class IntervalScheduleTests extends ElasticsearchTestCase {
 
     @Test
     public void testParse_Number() throws Exception {
-        long value = (long) randomInt();
+        long value = (long) randomIntBetween(0, Integer.MAX_VALUE);
         XContentBuilder builder = jsonBuilder().value(value);
         BytesReference bytes = builder.bytes();
         XContentParser parser = JsonXContent.jsonXContent.createParser(bytes);
@@ -34,6 +33,21 @@ public class IntervalScheduleTests extends ElasticsearchTestCase {
         assertThat(schedule.interval().seconds(), is(value));
     }
 
+    @Test
+    public void testParse_NegativeNumber() throws Exception {
+        long value = (long) randomIntBetween(Integer.MIN_VALUE, 0);
+        XContentBuilder builder = jsonBuilder().value(value);
+        BytesReference bytes = builder.bytes();
+        XContentParser parser = JsonXContent.jsonXContent.createParser(bytes);
+        parser.nextToken(); // advancing to the start object
+        try {
+            new IntervalSchedule.Parser().parse(parser);
+            fail("exception expected, because interval is negative");
+        } catch (ScheduleTriggerException e) {
+            assertThat(e.getMessage(), containsString("interval can't be lower than 1000 ms, but"));
+        }
+    }
+
     @Test
     public void testParse_String() throws Exception {
         IntervalSchedule.Interval value = randomTimeValue();