[ML] Remove forecast end param (elastic/x-pack-elasticsearch#3121)

The forecast API provides a `duration` parameters
which is the most convenient way of specifying
the span of the forecast. End time is now unnecessary
and possibly confusing.

Relates elastic/machine-learning-cpp#443

Original commit: elastic/x-pack-elasticsearch@04eb0408e7

docs/en/rest-api/ml/forecast.asciidoc

@@ -42,21 +42,15 @@ forecast. For more information about this property, see <<ml-job-resource>>.
 
 `duration`::
   (time units) A period of time that indicates how far into the future to
-  forecast. For example, `2w` corresponds to 2 weeks. The forecast starts at the
+  forecast. For example, `30d` corresponds to 30 days. The forecast starts at the
   last record that was processed. For more information about time units, see
   <<time-units>>.
 
-////
+`expires_in`::
-//Not a supported feature:
+  (time units) The period of time that forecast results are retained.
-`end`::
+  After a forecast expires, the results are deleted. The default value is 14 days.
-  (string) The time that the forecast should end. The string can contain
+  If set to a value of `0`, the forecast is never automatically deleted.
-  formatted dates, a number representing milliseconds since the epoch, or a
+  For more information about time units, see <<time-units>>.
-  number representing seconds since the epoch. It can also contain
-  <<date-math,date math expressions>>. The default value is one day after
-  the last record that was processed.
-
-NOTE: You can specify either the `duration` or `end` parameter; if you specify
-both, an error occurs.
 
 ////
 

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

@@ -34,8 +34,6 @@ import org.elasticsearch.xpack.ml.job.results.Forecast;
 import java.io.IOException;
 import java.util.Objects;
 
-import static org.elasticsearch.xpack.ml.action.ForecastJobAction.Request.END_TIME;
-
 public class ForecastJobAction extends Action<ForecastJobAction.Request, ForecastJobAction.Response, ForecastJobAction.RequestBuilder> {
 
     public static final ForecastJobAction INSTANCE = new ForecastJobAction();
@@ -57,7 +55,6 @@ public class ForecastJobAction extends Action<ForecastJobAction.Request, Forecas
 
     public static class Request extends TransportJobTaskAction.JobTaskRequest<Request> implements ToXContentObject {
 
-        public static final ParseField END_TIME = new ParseField("end");
         public static final ParseField DURATION = new ParseField("duration");
         public static final ParseField EXPIRES_IN = new ParseField("expires_in");
 
@@ -65,7 +62,6 @@ public class ForecastJobAction extends Action<ForecastJobAction.Request, Forecas
 
         static {
             PARSER.declareString((request, jobId) -> request.jobId = jobId, Job.ID);
-            PARSER.declareString(Request::setEndTime, END_TIME);
             PARSER.declareString(Request::setDuration, DURATION);
             PARSER.declareString(Request::setExpiresIn, EXPIRES_IN);
         }
@@ -78,7 +74,6 @@ public class ForecastJobAction extends Action<ForecastJobAction.Request, Forecas
             return request;
         }
 
-        private String endTime;
         private TimeValue duration;
         private TimeValue expiresIn;
 
@@ -89,20 +84,16 @@ public class ForecastJobAction extends Action<ForecastJobAction.Request, Forecas
             super(jobId);
         }
 
-        public String getEndTime() {
-            return endTime;
-        }
-
-        public void setEndTime(String endTime) {
-            this.endTime = endTime;
-        }
-
         public TimeValue getDuration() {
             return duration;
         }
 
         public void setDuration(String duration) {
-            this.duration = TimeValue.parseTimeValue(duration, DURATION.getPreferredName());
+            setDuration(TimeValue.parseTimeValue(duration, DURATION.getPreferredName()));
+        }
+
+        public void setDuration(TimeValue duration) {
+            this.duration = duration;
         }
 
         public TimeValue getExpiresIn() {
@@ -110,13 +101,16 @@ public class ForecastJobAction extends Action<ForecastJobAction.Request, Forecas
         }
 
         public void setExpiresIn(String expiration) {
-            this.expiresIn = TimeValue.parseTimeValue(expiration, EXPIRES_IN.getPreferredName());
+            setExpiresIn(TimeValue.parseTimeValue(expiration, EXPIRES_IN.getPreferredName()));
+        }
+
+        public void setExpiresIn(TimeValue expiresIn) {
+            this.expiresIn = expiresIn;
         }
 
         @Override
         public void readFrom(StreamInput in) throws IOException {
             super.readFrom(in);
-            this.endTime = in.readOptionalString();
             this.duration = in.readOptionalWriteable(TimeValue::new);
             this.expiresIn = in.readOptionalWriteable(TimeValue::new);
         }
@@ -124,14 +118,13 @@ public class ForecastJobAction extends Action<ForecastJobAction.Request, Forecas
         @Override
         public void writeTo(StreamOutput out) throws IOException {
             super.writeTo(out);
-            out.writeOptionalString(endTime);
             out.writeOptionalWriteable(duration);
             out.writeOptionalWriteable(expiresIn);
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(jobId, endTime, duration, expiresIn);
+            return Objects.hash(jobId, duration, expiresIn);
         }
 
         @Override
@@ -143,17 +136,15 @@ public class ForecastJobAction extends Action<ForecastJobAction.Request, Forecas
                 return false;
             }
             Request other = (Request) obj;
-            return Objects.equals(jobId, other.jobId) && Objects.equals(endTime, other.endTime) &&
+            return Objects.equals(jobId, other.jobId)
-                   Objects.equals(duration, other.duration) && Objects.equals(expiresIn, other.expiresIn);
+                    && Objects.equals(duration, other.duration)
+                    && Objects.equals(expiresIn, other.expiresIn);
         }
 
         @Override
         public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
             builder.startObject();
             builder.field(Job.ID.getPreferredName(), jobId);
-            if (endTime != null) {
-                builder.field(END_TIME.getPreferredName(), endTime);
-            }
             if (duration != null) {
                 builder.field(DURATION.getPreferredName(), duration.getStringRep());
             }
@@ -258,9 +249,6 @@ public class ForecastJobAction extends Action<ForecastJobAction.Request, Forecas
         @Override
         protected void taskOperation(Request request, OpenJobAction.JobTask task, ActionListener<Response> listener) {
             ForecastParams.Builder paramsBuilder = ForecastParams.builder();
-            if (request.getEndTime() != null) {
-                paramsBuilder.endTime(request.getEndTime(), END_TIME);
-            }
             if (request.getDuration() != null) {
                 paramsBuilder.duration(request.getDuration());
             }

plugin/src/main/java/org/elasticsearch/xpack/ml/job/messages/Messages.java

@@ -164,7 +164,6 @@ public final class Messages {
             "Model snapshot ''{0}'' is the active snapshot for job ''{1}'', so cannot be deleted";
     public static final String REST_INVALID_DATETIME_PARAMS =
             "Query param [{0}] with value [{1}] cannot be parsed as a date or converted to a number (epoch).";
-    public static final String REST_INVALID_DURATION_AND_ENDTIME = "Specify either duration or end time";
     public static final String REST_INVALID_FLUSH_PARAMS_MISSING = "Invalid flush parameters: ''{0}'' has not been specified.";
     public static final String REST_INVALID_FLUSH_PARAMS_UNEXPECTED = "Invalid flush parameters: unexpected ''{0}''.";
     public static final String REST_JOB_NOT_CLOSED_REVERT = "Can only revert to a model snapshot when the job is closed.";

plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/params/ForecastParams.java

@@ -5,13 +5,8 @@
  */
 package org.elasticsearch.xpack.ml.job.process.autodetect.params;
 
-import org.elasticsearch.ElasticsearchParseException;
-import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.UUIDs;
-import org.elasticsearch.common.joda.DateMathParser;
 import org.elasticsearch.common.unit.TimeValue;
-import org.elasticsearch.index.mapper.DateFieldMapper;
-import org.elasticsearch.xpack.ml.job.messages.Messages;
 
 import java.util.Objects;
 
@@ -19,14 +14,12 @@ public class ForecastParams {
 
     private final String forecastId;
     private final long createTime;
-    private final long endTime;
     private final long duration;
     private final long expiresIn;
 
-    private ForecastParams(String forecastId, long createTime, long endTime, long duration, long expiresIn) {
+    private ForecastParams(String forecastId, long createTime, long duration, long expiresIn) {
         this.forecastId = forecastId;
         this.createTime = createTime;
-        this.endTime = endTime;
         this.duration = duration;
         this.expiresIn = expiresIn;
     }
@@ -43,14 +36,6 @@ public class ForecastParams {
         return createTime;
     }
 
-    /**
-     * The forecast end time in seconds from the epoch
-     * @return The end time in seconds from the epoch
-     */
-    public long getEndTime() {
-        return endTime;
-    }
-
     /**
      * The forecast duration in seconds
      * @return The duration in seconds
@@ -69,7 +54,7 @@ public class ForecastParams {
 
     @Override
     public int hashCode() {
-        return Objects.hash(forecastId, createTime, endTime, duration, expiresIn);
+        return Objects.hash(forecastId, createTime, duration, expiresIn);
     }
 
     @Override
@@ -83,7 +68,6 @@ public class ForecastParams {
         ForecastParams other = (ForecastParams) obj;
         return Objects.equals(forecastId, other.forecastId)
                 && Objects.equals(createTime, other.createTime)
-                && Objects.equals(endTime, other.endTime)
                 && Objects.equals(duration, other.duration)
                 && Objects.equals(expiresIn, other.expiresIn);
     }
@@ -95,33 +79,18 @@ public class ForecastParams {
     public static class Builder {
         private final String forecastId;
         private final long createTimeEpochSecs;
-        private long endTimeEpochSecs;
         private long durationSecs;
         private long expiresInSecs;
 
         private Builder() {
             forecastId = UUIDs.base64UUID();
             createTimeEpochSecs = System.currentTimeMillis() / 1000;
-            endTimeEpochSecs = 0;
             durationSecs = 0;
 
             // because 0 means never expire, the default is -1
             expiresInSecs = -1;
         }
 
-        public Builder endTime(String endTime, ParseField paramName) {
-            DateMathParser dateMathParser = new DateMathParser(DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER);
-
-            try {
-                endTimeEpochSecs = dateMathParser.parse(endTime, System::currentTimeMillis) / 1000;
-            } catch (Exception e) {
-                String msg = Messages.getMessage(Messages.REST_INVALID_DATETIME_PARAMS, paramName.getPreferredName(), endTime);
-                throw new ElasticsearchParseException(msg, e);
-            }
-
-            return this;
-        }
-
         public Builder duration(TimeValue duration) {
             durationSecs = duration.seconds();
             return this;
@@ -133,11 +102,7 @@ public class ForecastParams {
         }
 
         public ForecastParams build() {
-            if (endTimeEpochSecs != 0 && durationSecs != 0) {
+            return new ForecastParams(forecastId, createTimeEpochSecs, durationSecs, expiresInSecs);
-                throw new ElasticsearchParseException(Messages.getMessage(Messages.REST_INVALID_DURATION_AND_ENDTIME));
-            }
-
-            return new ForecastParams(forecastId, createTimeEpochSecs, endTimeEpochSecs, durationSecs, expiresInSecs);
         }
     }
 }

plugin/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ControlMsgToProcessWriter.java

@@ -155,9 +155,6 @@ public class ControlMsgToProcessWriter {
         builder.field("forecast_id", params.getForecastId());
         builder.field("create_time", params.getCreateTime());
 
-        if (params.getEndTime() != 0) {
-            builder.field("end_time", params.getEndTime());
-        }
         if (params.getDuration() != 0) {
             builder.field("duration", params.getDuration());
         }

plugin/src/main/java/org/elasticsearch/xpack/ml/rest/job/RestForecastJobAction.java

@@ -40,9 +40,6 @@ public class RestForecastJobAction extends BaseRestHandler {
             request = ForecastJobAction.Request.parseRequest(jobId, parser);
         } else {
             request = new ForecastJobAction.Request(restRequest.param(Job.ID.getPreferredName()));
-            if (restRequest.hasParam(ForecastJobAction.Request.END_TIME.getPreferredName())) {
-                request.setEndTime(restRequest.param(ForecastJobAction.Request.END_TIME.getPreferredName()));
-            }
             if (restRequest.hasParam(ForecastJobAction.Request.DURATION.getPreferredName())) {
                 request.setDuration(restRequest.param(ForecastJobAction.Request.DURATION.getPreferredName()));
             }

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

@@ -10,8 +10,6 @@ import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.test.AbstractStreamableXContentTestCase;
 import org.elasticsearch.xpack.ml.action.ForecastJobAction.Request;
 
-import java.time.Instant;
-
 public class ForecastJobActionRequestTests extends AbstractStreamableXContentTestCase<Request> {
 
     @Override
@@ -27,9 +25,6 @@ public class ForecastJobActionRequestTests extends AbstractStreamableXContentTes
     @Override
     protected Request createTestInstance() {
         Request request = new Request(randomAlphaOfLengthBetween(1, 20));
-        if (randomBoolean()) {
-            request.setEndTime(Instant.ofEpochMilli(randomNonNegativeLong()).toString());
-        }
         if (randomBoolean()) {
             request.setDuration(TimeValue.timeValueSeconds(randomIntBetween(1, 1_000_000)).getStringRep());
         }

plugin/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/params/ForecastParamsTests.java

@@ -5,18 +5,14 @@
  */
 package org.elasticsearch.xpack.ml.job.process.autodetect.params;
 
-import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.ml.job.messages.Messages;
 
 import java.util.HashSet;
 import java.util.Set;
 
 import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.greaterThanOrEqualTo;
-import static org.hamcrest.Matchers.lessThanOrEqualTo;
 
 public class ForecastParamsTests extends ESTestCase {
 
@@ -31,32 +27,10 @@ public class ForecastParamsTests extends ESTestCase {
         assertThat(ids.size(), equalTo(10));
     }
 
-    public void test_UnparseableEndTimeThrows() {
-        ElasticsearchParseException e =
-                ESTestCase.expectThrows(ElasticsearchParseException.class, () -> ForecastParams.builder().endTime("bad", END).build());
-        assertEquals(Messages.getMessage(Messages.REST_INVALID_DATETIME_PARAMS, "end", "bad"), e.getMessage());
-    }
-
-    public void testFormats() {
-        assertEquals(10L, ForecastParams.builder().endTime("10000", END).build().getEndTime());
-        assertEquals(1462096800L, ForecastParams.builder().endTime("2016-05-01T10:00:00Z", END).build().getEndTime());
-
-        long nowSecs = System.currentTimeMillis() / 1000;
-        long end = ForecastParams.builder().endTime("now+2H", END).build().getEndTime();
-        assertThat(end, greaterThanOrEqualTo(nowSecs + 7200));
-        assertThat(end, lessThanOrEqualTo(nowSecs + 7200 +1));
-    }
-
     public void testDurationFormats() {
         assertEquals(34678L,
                 ForecastParams.builder().duration(TimeValue.parseTimeValue("34678s", DURATION.getPreferredName())).build().getDuration());
         assertEquals(172800L,
                 ForecastParams.builder().duration(TimeValue.parseTimeValue("2d", DURATION.getPreferredName())).build().getDuration());
     }
-
-    public void testDurationEndTimeThrows() {
-        ElasticsearchParseException e = ESTestCase.expectThrows(ElasticsearchParseException.class, () -> ForecastParams.builder()
-                .endTime("2016-05-01T10:00:00Z", END).duration(TimeValue.parseTimeValue("33d", DURATION.getPreferredName())).build());
-        assertEquals(Messages.getMessage(Messages.REST_INVALID_DURATION_AND_ENDTIME), e.getMessage());
-    }
 }

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

@@ -12,10 +12,15 @@
         }
       },
       "params": {
-        "end": {
+        "duration": {
-          "type": "string",
+          "type": "time",
           "required": false,
-          "description": "The end time of the forecast"
+          "description": "The duration of the forecast"
+        },
+        "expires_in": {
+          "type": "time",
+          "required": false,
+          "description": "The time interval after which the forecast expires. Expired forecasts will be deleted at the first opportunity."
         }
       }
     },

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

@@ -26,16 +26,3 @@ setup:
         catch: /status_exception/
         xpack.ml.forecast:
           job_id: "forecast-job"
-
----
-"Test bad end param errors":
-
-    - do:
-        xpack.ml.open_job:
-          job_id: "forecast-job"
-
-    - do:
-        catch: /parse_exception/
-        xpack.ml.forecast:
-          job_id: "forecast-job"
-          end: "tomorrow"