[ML] Validate duration and expires_in params in forecast API (elastic/x-pack-elasticsearch#3139)

Relates elastic/machine-learning-cpp#443

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

plugin/src/main/java/org/elasticsearch/xpack/ml/action/ForecastJobAction.java

@@ -58,6 +58,9 @@ public class ForecastJobAction extends Action<ForecastJobAction.Request, Forecas
         public static final ParseField DURATION = new ParseField("duration");
         public static final ParseField EXPIRES_IN = new ParseField("expires_in");
 
+        // Max allowed duration: 8 weeks
+        private static final TimeValue MAX_DURATION = TimeValue.parseTimeValue("56d", "");
+
         private static final ObjectParser<Request, Void> PARSER = new ObjectParser<>(NAME, Request::new);
 
         static {
@@ -94,6 +97,14 @@ public class ForecastJobAction extends Action<ForecastJobAction.Request, Forecas
 
         public void setDuration(TimeValue duration) {
             this.duration = duration;
+            if (this.duration.compareTo(TimeValue.ZERO) <= 0) {
+                throw new IllegalArgumentException("[" + DURATION.getPreferredName() + "] must be positive: ["
+                        + duration.getStringRep() + "]");
+            }
+            if (this.duration.compareTo(MAX_DURATION) > 0) {
+                throw new IllegalArgumentException("[" + DURATION.getPreferredName() + "] must be " + MAX_DURATION.getStringRep()
+                        + " or less: [" + duration.getStringRep() + "]");
+            }
         }
 
         public TimeValue getExpiresIn() {
@@ -106,6 +117,10 @@ public class ForecastJobAction extends Action<ForecastJobAction.Request, Forecas
 
         public void setExpiresIn(TimeValue expiresIn) {
             this.expiresIn = expiresIn;
+            if (this.expiresIn.compareTo(TimeValue.ZERO) < 0) {
+                throw new IllegalArgumentException("[" + EXPIRES_IN.getPreferredName() + "] must be non-negative: ["
+                        + expiresIn.getStringRep() + "]");
+            }
         }
 
         @Override

plugin/src/test/java/org/elasticsearch/xpack/ml/action/ForecastJobActionRequestTests.java

@@ -10,6 +10,8 @@ import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.test.AbstractStreamableXContentTestCase;
 import org.elasticsearch.xpack.ml.action.ForecastJobAction.Request;
 
+import static org.hamcrest.Matchers.equalTo;
+
 public class ForecastJobActionRequestTests extends AbstractStreamableXContentTestCase<Request> {
 
     @Override
@@ -28,7 +30,9 @@ public class ForecastJobActionRequestTests extends AbstractStreamableXContentTes
         if (randomBoolean()) {
             request.setDuration(TimeValue.timeValueSeconds(randomIntBetween(1, 1_000_000)).getStringRep());
         }
-
+        if (randomBoolean()) {
+            request.setExpiresIn(TimeValue.timeValueSeconds(randomIntBetween(0, 1_000_000)).getStringRep());
+        }
         return request;
     }
 
@@ -36,4 +40,25 @@ public class ForecastJobActionRequestTests extends AbstractStreamableXContentTes
     protected Request createBlankInstance() {
         return new Request();
     }
+
+    public void testSetDuration_GivenZero() {
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new Request().setDuration("0"));
+        assertThat(e.getMessage(), equalTo("[duration] must be positive: [0ms]"));
+    }
+
+    public void testSetDuration_GivenNegative() {
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new Request().setDuration("-1s"));
+        assertThat(e.getMessage(), equalTo("[duration] must be positive: [-1]"));
+    }
+
+    public void testSetExpiresIn_GivenZero() {
+        Request request = new Request();
+        request.setExpiresIn("0");
+        assertThat(request.getExpiresIn(), equalTo(TimeValue.ZERO));
+    }
+
+    public void testSetExpiresIn_GivenNegative() {
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new Request().setExpiresIn("-1s"));
+        assertThat(e.getMessage(), equalTo("[expires_in] must be non-negative: [-1]"));
+    }
 }

plugin/src/test/resources/rest-api-spec/test/ml/forecast.yml

@@ -15,14 +15,38 @@ setup:
 
 ---
 "Test forecast unknown job":
-    - do:
-        catch: missing
-        xpack.ml.forecast:
-          job_id: "non-existing-job"
+  - do:
+      catch: missing
+      xpack.ml.forecast:
+        job_id: "non-existing-job"
 
 ---
 "Test forecast on closed job":
-    - do:
-        catch: /status_exception/
-        xpack.ml.forecast:
-          job_id: "forecast-job"
+  - do:
+      catch: /status_exception/
+      xpack.ml.forecast:
+        job_id: "forecast-job"
+
+---
+"Test forecast given duration is zero":
+  - do:
+      catch: /\[duration\] must be positive[:] \[0s\]/
+      xpack.ml.forecast:
+        job_id: "forecast-job"
+        duration: "0s"
+
+---
+"Test forecast given duration is negative":
+  - do:
+      catch: /\[duration\] must be positive[:] \[-1\]/
+      xpack.ml.forecast:
+        job_id: "forecast-job"
+        duration: "-1s"
+
+---
+"Test forecast given expires_in is negative":
+  - do:
+      catch: /\[expires_in\] must be non-negative[:] \[-1\]/
+      xpack.ml.forecast:
+        job_id: "forecast-job"
+        expires_in: "-1s"