From 1d0097b5e83090072de5f8b62e36c2789d167722 Mon Sep 17 00:00:00 2001 From: Mehran Koushkebaghi Date: Wed, 27 Feb 2019 09:18:03 +0000 Subject: [PATCH] [ML] Refactoring scheduled event to store instant instead of zoned time zone (#39380) The ScheduledEvent class has never preserved the time zone so it makes more sense for it to store the start and end time using Instant rather than ZonedDateTime. Closes #38620 --- .../core/ml/calendars/ScheduledEvent.java | 48 +++++++++---------- .../ml/calendars/ScheduledEventTests.java | 17 ++----- .../ml/integration/ScheduledEventsIT.java | 26 +++++----- .../ml/integration/JobResultsProviderIT.java | 9 +++- .../AutodetectControlMsgWriterTests.java | 8 ++-- .../writer/FieldConfigWriterTests.java | 10 ++-- .../writer/ScheduledEventsWriterTests.java | 12 ++--- 7 files changed, 60 insertions(+), 70 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEvent.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEvent.java index 042775c8024..afd10e0c17b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEvent.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEvent.java @@ -27,8 +27,6 @@ import org.elasticsearch.xpack.core.ml.utils.time.TimeUtils; import java.io.IOException; import java.time.Instant; -import java.time.ZoneOffset; -import java.time.ZonedDateTime; import java.util.ArrayList; import java.util.List; import java.util.Objects; @@ -55,18 +53,18 @@ public class ScheduledEvent implements ToXContentObject, Writeable { parser.declareString(ScheduledEvent.Builder::description, DESCRIPTION); parser.declareField(ScheduledEvent.Builder::startTime, p -> { if (p.currentToken() == XContentParser.Token.VALUE_NUMBER) { - return ZonedDateTime.ofInstant(Instant.ofEpochMilli(p.longValue()), ZoneOffset.UTC); + return Instant.ofEpochMilli(p.longValue()); } else if (p.currentToken() == XContentParser.Token.VALUE_STRING) { - return ZonedDateTime.ofInstant(Instant.ofEpochMilli(TimeUtils.dateStringToEpoch(p.text())), ZoneOffset.UTC); + return Instant.ofEpochMilli(TimeUtils.dateStringToEpoch(p.text())); } throw new IllegalArgumentException( "unexpected token [" + p.currentToken() + "] for [" + START_TIME.getPreferredName() + "]"); }, START_TIME, ObjectParser.ValueType.VALUE); parser.declareField(ScheduledEvent.Builder::endTime, p -> { if (p.currentToken() == XContentParser.Token.VALUE_NUMBER) { - return ZonedDateTime.ofInstant(Instant.ofEpochMilli(p.longValue()), ZoneOffset.UTC); + return Instant.ofEpochMilli(p.longValue()); } else if (p.currentToken() == XContentParser.Token.VALUE_STRING) { - return ZonedDateTime.ofInstant(Instant.ofEpochMilli(TimeUtils.dateStringToEpoch(p.text())), ZoneOffset.UTC); + return Instant.ofEpochMilli(TimeUtils.dateStringToEpoch(p.text())); } throw new IllegalArgumentException( "unexpected token [" + p.currentToken() + "] for [" + END_TIME.getPreferredName() + "]"); @@ -83,12 +81,12 @@ public class ScheduledEvent implements ToXContentObject, Writeable { } private final String description; - private final ZonedDateTime startTime; - private final ZonedDateTime endTime; + private final Instant startTime; + private final Instant endTime; private final String calendarId; private final String eventId; - ScheduledEvent(String description, ZonedDateTime startTime, ZonedDateTime endTime, String calendarId, @Nullable String eventId) { + ScheduledEvent(String description, Instant startTime, Instant endTime, String calendarId, @Nullable String eventId) { this.description = Objects.requireNonNull(description); this.startTime = Objects.requireNonNull(startTime); this.endTime = Objects.requireNonNull(endTime); @@ -98,8 +96,8 @@ public class ScheduledEvent implements ToXContentObject, Writeable { public ScheduledEvent(StreamInput in) throws IOException { description = in.readString(); - startTime = ZonedDateTime.ofInstant(Instant.ofEpochMilli(in.readVLong()), ZoneOffset.UTC); - endTime = ZonedDateTime.ofInstant(Instant.ofEpochMilli(in.readVLong()), ZoneOffset.UTC); + startTime = Instant.ofEpochMilli(in.readVLong()); + endTime = Instant.ofEpochMilli(in.readVLong()); calendarId = in.readString(); eventId = in.readOptionalString(); } @@ -108,11 +106,11 @@ public class ScheduledEvent implements ToXContentObject, Writeable { return description; } - public ZonedDateTime getStartTime() { + public Instant getStartTime() { return startTime; } - public ZonedDateTime getEndTime() { + public Instant getEndTime() { return endTime; } @@ -141,9 +139,9 @@ public class ScheduledEvent implements ToXContentObject, Writeable { long bucketSpanSecs = bucketSpan.getSeconds(); - long bucketStartTime = Intervals.alignToFloor(getStartTime().toEpochSecond(), bucketSpanSecs); + long bucketStartTime = Intervals.alignToFloor(getStartTime().getEpochSecond(), bucketSpanSecs); conditions.add(RuleCondition.createTime(Operator.GTE, bucketStartTime)); - long bucketEndTime = Intervals.alignToCeil(getEndTime().toEpochSecond(), bucketSpanSecs); + long bucketEndTime = Intervals.alignToCeil(getEndTime().getEpochSecond(), bucketSpanSecs); conditions.add(RuleCondition.createTime(Operator.LT, bucketEndTime)); DetectionRule.Builder builder = new DetectionRule.Builder(conditions); @@ -154,8 +152,8 @@ public class ScheduledEvent implements ToXContentObject, Writeable { @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(description); - out.writeVLong(startTime.toInstant().toEpochMilli()); - out.writeVLong(endTime.toInstant().toEpochMilli()); + out.writeVLong(startTime.toEpochMilli()); + out.writeVLong(endTime.toEpochMilli()); out.writeString(calendarId); out.writeOptionalString(eventId); } @@ -164,8 +162,8 @@ public class ScheduledEvent implements ToXContentObject, Writeable { public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); builder.field(DESCRIPTION.getPreferredName(), description); - builder.timeField(START_TIME.getPreferredName(), START_TIME.getPreferredName() + "_string", startTime.toInstant().toEpochMilli()); - builder.timeField(END_TIME.getPreferredName(), END_TIME.getPreferredName() + "_string", endTime.toInstant().toEpochMilli()); + builder.timeField(START_TIME.getPreferredName(), START_TIME.getPreferredName() + "_string", startTime.toEpochMilli()); + builder.timeField(END_TIME.getPreferredName(), END_TIME.getPreferredName() + "_string", endTime.toEpochMilli()); builder.field(Calendar.ID.getPreferredName(), calendarId); if (eventId != null) { builder.field(EVENT_ID.getPreferredName(), eventId); @@ -197,8 +195,8 @@ public class ScheduledEvent implements ToXContentObject, Writeable { // Note ZonedDataTime.equals() fails because the time zone and date-time must be the same // which isn't the case in tests where the time zone is randomised. return description.equals(other.description) - && Objects.equals(startTime.toInstant().getEpochSecond(), other.startTime.toInstant().getEpochSecond()) - && Objects.equals(endTime.toInstant().getEpochSecond(), other.endTime.toInstant().getEpochSecond()) + && Objects.equals(startTime.getEpochSecond(), other.startTime.getEpochSecond()) + && Objects.equals(endTime.getEpochSecond(), other.endTime.getEpochSecond()) && calendarId.equals(other.calendarId); } @@ -209,8 +207,8 @@ public class ScheduledEvent implements ToXContentObject, Writeable { public static class Builder { private String description; - private ZonedDateTime startTime; - private ZonedDateTime endTime; + private Instant startTime; + private Instant endTime; private String calendarId; private String eventId; @@ -219,12 +217,12 @@ public class ScheduledEvent implements ToXContentObject, Writeable { return this; } - public Builder startTime(ZonedDateTime startTime) { + public Builder startTime(Instant startTime) { this.startTime = startTime; return this; } - public Builder endTime(ZonedDateTime endTime) { + public Builder endTime(Instant endTime) { this.endTime = endTime; return this; } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEventTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEventTests.java index 6508ee5cb20..cd14d4b35bd 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEventTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEventTests.java @@ -17,10 +17,7 @@ import org.elasticsearch.xpack.core.ml.job.config.RuleAction; import org.elasticsearch.xpack.core.ml.job.config.RuleCondition; import java.io.IOException; -import java.time.Clock; import java.time.Instant; -import java.time.ZoneOffset; -import java.time.ZonedDateTime; import java.util.EnumSet; import java.util.List; @@ -29,7 +26,7 @@ import static org.hamcrest.Matchers.containsString; public class ScheduledEventTests extends AbstractSerializingTestCase { public static ScheduledEvent createScheduledEvent(String calendarId) { - ZonedDateTime start = nowWithMillisResolution(); + Instant start = Instant.ofEpochMilli(Instant.now().toEpochMilli()); return new ScheduledEvent(randomAlphaOfLength(10), start, start.plusSeconds(randomIntBetween(1, 10000)), calendarId, null); } @@ -72,7 +69,7 @@ public class ScheduledEventTests extends AbstractSerializingTestCase events = new ArrayList<>(); events.add(new ScheduledEvent.Builder().description("Black Friday") - .startTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(1511395200000L), ZoneOffset.UTC)) - .endTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(1515369600000L), ZoneOffset.UTC)) + .startTime(Instant.ofEpochMilli(1511395200000L)) + .endTime(Instant.ofEpochMilli(1515369600000L)) .calendarId("calendar_id").build()); events.add(new ScheduledEvent.Builder().description("Blue Monday") - .startTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(1519603200000L), ZoneOffset.UTC)) - .endTime(ZonedDateTime.ofInstant(Instant.ofEpochMilli(1519862400000L), ZoneOffset.UTC)) + .startTime(Instant.ofEpochMilli(1519603200000L)) + .endTime(Instant.ofEpochMilli(1519862400000L)) .calendarId("calendar_id").build()); StringBuilder buffer = new StringBuilder(); @@ -54,4 +52,4 @@ public class ScheduledEventsWriterTests extends ESTestCase { "\n"; assertThat(buffer.toString(), equalTo(expectedString)); } -} \ No newline at end of file +}