Skip to content

Commit 4e91c9d

Browse files
lukewhitinglukewhiting
committed
Remove redundant conversions from ZoneId to TimeZone following move to LocalDateTime
1 parent 8851cdf commit 4e91c9d

File tree

9 files changed

+53
-65
lines changed

9 files changed

+53
-65
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/scheduler/Cron.java‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import java.io.IOException;
1616
import java.time.LocalDateTime;
17+
import java.time.ZoneId;
1718
import java.time.ZoneOffset;
1819
import java.time.temporal.ChronoField;
1920
import java.util.Calendar;
@@ -235,7 +236,7 @@ public class Cron implements ToXContentFragment {
235236

236237
private final String expression;
237238

238-
private TimeZone timeZone;
239+
private ZoneId timeZone;
239240

240241
private transient TreeSet<Integer> seconds;
241242
private transient TreeSet<Integer> minutes;
@@ -255,7 +256,7 @@ public class Cron implements ToXContentFragment {
255256
// for the next ~60 years
256257
public static final int MAX_YEAR = Calendar.getInstance(UTC, Locale.ROOT).get(Calendar.YEAR) + 50;
257258

258-
public Cron(String expression, TimeZone timeZone) {
259+
public Cron(String expression, ZoneId timeZone) {
259260
this.timeZone = timeZone;
260261
assert expression != null : "cron expression cannot be null";
261262
this.expression = expression.toUpperCase(Locale.ROOT);
@@ -277,7 +278,7 @@ public Cron(String expression, TimeZone timeZone) {
277278
* <CODE>CronExpression</CODE>
278279
*/
279280
public Cron(String expression) {
280-
this(expression, UTC);
281+
this(expression, UTC.toZoneId());
281282
}
282283

283284
/**
@@ -290,7 +291,7 @@ public Cron(Cron cron) {
290291
this(cron.expression, cron.timeZone);
291292
}
292293

293-
public void setTimeZone(TimeZone timeZone) {
294+
public void setTimeZone(ZoneId timeZone) {
294295
this.timeZone = timeZone;
295296
}
296297

@@ -305,7 +306,7 @@ public void setTimeZone(TimeZone timeZone) {
305306
@SuppressForbidden(reason = "In this case, the DST ambiguity of the atZone method is desired, understood and tested")
306307
public long getNextValidTimeAfter(final long time) {
307308

308-
LocalDateTime afterTimeLdt = LocalDateTime.ofInstant(java.time.Instant.ofEpochMilli(time), timeZone.toZoneId()).plusSeconds(1);
309+
LocalDateTime afterTimeLdt = LocalDateTime.ofInstant(java.time.Instant.ofEpochMilli(time), timeZone).plusSeconds(1);
309310
LocalDateTimeLegacyWrapper cl = new LocalDateTimeLegacyWrapper(afterTimeLdt.with(ChronoField.MILLI_OF_SECOND, 0));
310311

311312
boolean gotOne = false;
@@ -407,7 +408,7 @@ public long getNextValidTimeAfter(final long time) {
407408
day = getLastDayOfMonth(mon, cl.getYear());
408409
day -= lastdayOffset;
409410

410-
LocalDateTimeLegacyWrapper tcal = new LocalDateTimeLegacyWrapper(LocalDateTime.now(timeZone.toZoneId()));
411+
LocalDateTimeLegacyWrapper tcal = new LocalDateTimeLegacyWrapper(LocalDateTime.now(timeZone));
411412
tcal.setSecond(0);
412413
tcal.setMinute(0);
413414
tcal.setHour(0);
@@ -442,7 +443,7 @@ public long getNextValidTimeAfter(final long time) {
442443
t = day;
443444
day = daysOfMonth.first();
444445

445-
LocalDateTimeLegacyWrapper tcal = new LocalDateTimeLegacyWrapper(LocalDateTime.now(timeZone.toZoneId()));
446+
LocalDateTimeLegacyWrapper tcal = new LocalDateTimeLegacyWrapper(LocalDateTime.now(timeZone));
446447
tcal.setSecond(0);
447448
tcal.setMinute(0);
448449
tcal.setHour(0);
@@ -696,7 +697,7 @@ public long getNextValidTimeAfter(final long time) {
696697

697698
LocalDateTime nextRuntime = cl.getLocalDateTime();
698699

699-
return nextRuntime.atZone(timeZone.toZoneId()).toInstant().toEpochMilli();
700+
return nextRuntime.atZone(timeZone).toInstant().toEpochMilli();
700701
}
701702

702703
public String expression() {

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/scheduler/LocalDateTimeLegacyWrapper.java‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -112,26 +112,6 @@ public void plusYears(long years) {
112112
ldt = ldt.plusYears(years);
113113
}
114114

115-
public void plusMonths(long months) {
116-
ldt = ldt.plusMonths(months);
117-
}
118-
119-
public void plusWeeks(long weeks) {
120-
ldt = ldt.plusWeeks(weeks);
121-
}
122-
123-
public void plusDays(long days) {
124-
ldt = ldt.plusDays(days);
125-
}
126-
127-
public void plusHours(long hours) {
128-
ldt = ldt.plusHours(hours);
129-
}
130-
131-
public void plusMinutes(long minutes) {
132-
ldt = ldt.plusMinutes(minutes);
133-
}
134-
135115
public void plusSeconds(long seconds) {
136116
ldt = ldt.plusSeconds(seconds);
137117
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/scheduler/CronTimezoneTests.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
public class CronTimezoneTests extends ESTestCase {
2828

2929
public void testForFixedOffsetCorrectlyCalculateNextRuntime() {
30-
Cron cron = new Cron("0 0 2 * * ?", getTimeZone(ZoneOffset.of("+1")));
30+
Cron cron = new Cron("0 0 2 * * ?", ZoneOffset.of("+1"));
3131
long midnightUTC = Instant.parse("2020-01-01T00:00:00Z").toEpochMilli();
3232
long nextValidTimeAfter = cron.getNextValidTimeAfter(midnightUTC);
3333
assertThat(Instant.ofEpochMilli(nextValidTimeAfter), equalTo(Instant.parse("2020-01-01T01:00:00Z")));
@@ -36,7 +36,7 @@ public void testForFixedOffsetCorrectlyCalculateNextRuntime() {
3636
public void testForLondonFixedDSTTransitionCheckCorrectSchedule() {
3737
ZoneId londonZone = getTimeZone("Europe/London").toZoneId();
3838

39-
Cron cron = new Cron("0 0 2 * * ?", getTimeZone(londonZone));
39+
Cron cron = new Cron("0 0 2 * * ?", londonZone);
4040
ZoneRules londonZoneRules = londonZone.getRules();
4141
Instant springMidnight = Instant.parse("2020-03-01T00:00:00Z");
4242
long timeBeforeDST = springMidnight.toEpochMilli();
@@ -68,7 +68,7 @@ public void testRandomDSTTransitionCalculateNextTimeCorrectlyRelativeToUTC() {
6868
long epochBefore = midnightBefore.toInstant().toEpochMilli();
6969
long epochAfter = midnightAfter.toInstant().toEpochMilli();
7070

71-
Cron cron = new Cron("0 0 2 * * ?", getTimeZone(timeZone));
71+
Cron cron = new Cron("0 0 2 * * ?", timeZone);
7272

7373
long nextScheduleBefore = cron.getNextValidTimeAfter(epochBefore);
7474
long nextScheduleAfter = cron.getNextValidTimeAfter(epochAfter);
@@ -118,7 +118,7 @@ private ZoneId generateRandomDSTZone() {
118118

119119
public void testForGMTGapTransitionTriggerTimeIsAsIfTransitionHasntHappenedYet() {
120120
ZoneId london = ZoneId.of("Europe/London");
121-
Cron cron = new Cron("0 30 1 * * ?", getTimeZone(london)); // Every day at 1:30
121+
Cron cron = new Cron("0 30 1 * * ?", london); // Every day at 1:30
122122

123123
Instant beforeTransition = Instant.parse("2025-03-30T00:00:00Z");
124124
long beforeTransitionEpoch = beforeTransition.toEpochMilli();
@@ -129,7 +129,7 @@ public void testForGMTGapTransitionTriggerTimeIsAsIfTransitionHasntHappenedYet()
129129

130130
public void testForGMTOverlapTransitionTriggerSkipSecondExecution() {
131131
ZoneId london = ZoneId.of("Europe/London");
132-
Cron cron = new Cron("0 30 1 * * ?", getTimeZone(london)); // Every day at 01:30
132+
Cron cron = new Cron("0 30 1 * * ?", london); // Every day at 01:30
133133

134134
Instant beforeTransition = Instant.parse("2024-10-27T00:00:00Z");
135135
long beforeTransitionEpoch = beforeTransition.toEpochMilli();
@@ -145,7 +145,7 @@ public void testForGMTOverlapTransitionTriggerSkipSecondExecution() {
145145
public void testDiscontinuityResolutionForNonHourCronInRandomTimezone() {
146146
var timezone = generateRandomDSTZone();
147147

148-
var cron = new Cron("0 * * * * ?", getTimeZone(timezone)); // Once per minute
148+
var cron = new Cron("0 * * * * ?", timezone); // Once per minute
149149

150150
Instant referenceTime = randomInstantBetween(Instant.now(), Instant.now().plus(1826, ChronoUnit.DAYS)); // ~5 years
151151
ZoneOffsetTransition transition1 = timezone.getRules().nextTransition(referenceTime);
@@ -203,14 +203,14 @@ private static void testHourCronTransition(ZoneOffsetTransition transition, Zone
203203
if (transition.isGap()) {
204204
LocalDateTime targetTime = transition.getDateTimeBefore().plusMinutes(10);
205205

206-
var cron = new Cron("0 " + targetTime.getMinute() + " " + targetTime.getHour() + " * * ?", getTimeZone(timezone));
206+
var cron = new Cron("0 " + targetTime.getMinute() + " " + targetTime.getHour() + " * * ?", timezone);
207207

208208
long nextTrigger = cron.getNextValidTimeAfter(transition.getInstant().minus(10, ChronoUnit.MINUTES).toEpochMilli());
209209

210210
assertThat(ofEpochMilli(nextTrigger), equalTo(transition.getInstant().plus(10, ChronoUnit.MINUTES)));
211211
} else {
212212
LocalDateTime targetTime = transition.getDateTimeAfter().plusMinutes(10);
213-
var cron = new Cron("0 " + targetTime.getMinute() + " " + targetTime.getHour() + " * * ?", getTimeZone(timezone));
213+
var cron = new Cron("0 " + targetTime.getMinute() + " " + targetTime.getHour() + " * * ?", timezone);
214214

215215
long transitionLength = Math.abs(transition.getDuration().toSeconds());
216216
long firstTrigger = cron.getNextValidTimeAfter(

‎x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/CronnableSchedule.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,17 @@
88

99
import org.elasticsearch.xpack.core.scheduler.Cron;
1010

11+
import java.time.ZoneId;
1112
import java.util.Arrays;
1213
import java.util.Comparator;
1314
import java.util.Objects;
14-
import java.util.TimeZone;
1515

1616
public abstract class CronnableSchedule implements Schedule {
1717

1818
private static final Comparator<Cron> CRON_COMPARATOR = Comparator.comparing(Cron::expression);
1919

2020
protected final Cron[] crons;
21-
private TimeZone timeZone;
21+
private ZoneId timeZone;
2222

2323
CronnableSchedule(String... expressions) {
2424
this(crons(expressions));
@@ -30,14 +30,14 @@ private CronnableSchedule(Cron... crons) {
3030
Arrays.sort(crons, CRON_COMPARATOR);
3131
}
3232

33-
protected void setTimeZone(TimeZone timeZone) {
33+
protected void setTimeZone(ZoneId timeZone) {
3434
this.timeZone = timeZone;
3535
for (Cron cron : crons) {
3636
cron.setTimeZone(timeZone);
3737
}
3838
}
3939

40-
public TimeZone getTimeZone() {
40+
public ZoneId getTimeZone() {
4141
return timeZone;
4242
}
4343

‎x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleRegistry.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@
1212

1313
import java.io.IOException;
1414
import java.time.DateTimeException;
15+
import java.time.ZoneId;
1516
import java.util.HashMap;
1617
import java.util.Map;
1718
import java.util.Set;
18-
import java.util.TimeZone;
1919

2020
public class ScheduleRegistry {
2121
private final Map<String, Schedule.Parser<? extends Schedule>> parsers = new HashMap<>();
@@ -32,7 +32,7 @@ public Schedule parse(String context, XContentParser parser) throws IOException
3232
String type = null;
3333
XContentParser.Token token;
3434
Schedule schedule = null;
35-
TimeZone timeZone = null; // Default to UTC
35+
ZoneId timeZone = null; // Default to UTC
3636
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
3737
if (token == XContentParser.Token.FIELD_NAME) {
3838
var fieldName = parser.currentName();
@@ -66,8 +66,8 @@ public Schedule parse(String context, XContentParser parser) throws IOException
6666
return schedule;
6767
}
6868

69-
private static TimeZone parseTimezone(XContentParser parser) throws IOException {
70-
TimeZone timeZone;
69+
private static ZoneId parseTimezone(XContentParser parser) throws IOException {
70+
ZoneId timeZone;
7171
XContentParser.Token token = parser.nextToken();
7272
if (token == XContentParser.Token.VALUE_STRING) {
7373
String text = parser.text();

‎x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleTrigger.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public int hashCode() {
5252
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
5353
builder.startObject();
5454
if (schedule instanceof CronnableSchedule cronnableSchedule && cronnableSchedule.getTimeZone() != null) {
55-
builder.field(TIMEZONE_FIELD, cronnableSchedule.getTimeZone().getID());
55+
builder.field(TIMEZONE_FIELD, cronnableSchedule.getTimeZone().getId());
5656
}
5757

5858
builder.field(schedule.type(), schedule, params);

‎x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/trigger/schedule/support/TimezoneUtils.java‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import java.time.ZoneId;
1212
import java.util.Locale;
1313
import java.util.Map;
14-
import java.util.TimeZone;
1514

1615
import static java.util.stream.Collectors.toMap;
1716

@@ -20,32 +19,32 @@
2019
*/
2120
public class TimezoneUtils {
2221

23-
private static final Map<String, TimeZone> caseInsensitiveTZLookup;
22+
private static final Map<String, ZoneId> caseInsensitiveTZLookup;
2423

2524
static {
2625
caseInsensitiveTZLookup = ZoneId.getAvailableZoneIds()
2726
.stream()
28-
.collect(toMap(zoneId -> zoneId.toLowerCase(Locale.ROOT), TimeZone::getTimeZone));
27+
.collect(toMap(zoneId -> zoneId.toLowerCase(Locale.ROOT), ZoneId::of));
2928
}
3029

3130
/**
32-
* Parses a timezone string into a {@link TimeZone} object. The timezone string can be a valid timezone ID, or a
31+
* Parses a timezone string into a {@link ZoneId} object. The timezone string can be a valid timezone ID, or a
3332
* timezone offset string and is case-insensitive.
3433
*
3534
* @param timezoneString The timezone string to parse
36-
* @return The parsed {@link TimeZone} object
35+
* @return The parsed {@link ZoneId} object
3736
* @throws DateTimeException If the timezone string is not a valid timezone ID or offset
3837
*/
39-
public static TimeZone parse(String timezoneString) throws DateTimeException {
38+
public static ZoneId parse(String timezoneString) throws DateTimeException {
4039
try {
41-
return TimeZone.getTimeZone(ZoneId.of(timezoneString));
40+
return ZoneId.of(timezoneString);
4241
} catch (DateTimeException e) {
43-
TimeZone timeZone = caseInsensitiveTZLookup.get(timezoneString.toLowerCase(Locale.ROOT));
42+
ZoneId timeZone = caseInsensitiveTZLookup.get(timezoneString.toLowerCase(Locale.ROOT));
4443
if (timeZone != null) {
4544
return timeZone;
4645
}
4746
try {
48-
return TimeZone.getTimeZone(ZoneId.of(timezoneString.toUpperCase(Locale.ROOT)));
47+
return ZoneId.of(timezoneString.toUpperCase(Locale.ROOT));
4948
} catch (DateTimeException ignored) {
5049
// ignore
5150
}

‎x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/ScheduleRegistryTests.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@
1212
import org.elasticsearch.xcontent.json.JsonXContent;
1313
import org.junit.Before;
1414

15+
import java.time.ZoneId;
1516
import java.util.HashSet;
1617
import java.util.Set;
17-
import java.util.TimeZone;
1818

1919
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
2020
import static org.hamcrest.Matchers.equalTo;
@@ -51,12 +51,12 @@ public void testParserInterval() throws Exception {
5151

5252
public void testParseCron() throws Exception {
5353
var cron = randomBoolean() ? Schedules.cron("* 0/5 * * * ?") : Schedules.cron("* 0/2 * * * ?", "* 0/3 * * * ?", "* 0/5 * * * ?");
54-
TimeZone timeZone = null;
54+
ZoneId timeZone = null;
5555
XContentBuilder builder = jsonBuilder().startObject().field(CronSchedule.TYPE, cron);
5656
if (randomBoolean()) {
57-
timeZone = randomTimeZone();
57+
timeZone = randomTimeZone().toZoneId();
5858
cron.setTimeZone(timeZone);
59-
builder.field(ScheduleTrigger.TIMEZONE_FIELD, timeZone.getID());
59+
builder.field(ScheduleTrigger.TIMEZONE_FIELD, timeZone.getId());
6060
}
6161
builder.endObject();
6262
BytesReference bytes = BytesReference.bytes(builder);

‎x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/trigger/schedule/support/TimezoneUtilsTests.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,25 +9,33 @@
99

1010
import org.elasticsearch.test.ESTestCase;
1111

12+
import java.time.ZoneId;
1213
import java.util.Locale;
13-
import java.util.TimeZone;
1414

1515
import static org.hamcrest.Matchers.equalTo;
1616

1717
public class TimezoneUtilsTests extends ESTestCase {
1818

19+
public void testExpectedFormatParsing() {
20+
assertThat(TimezoneUtils.parse("Europe/London").getId(),equalTo("Europe/London"));
21+
assertThat(TimezoneUtils.parse("+1").getId(),equalTo("+01:00"));
22+
assertThat(TimezoneUtils.parse("GMT+01:00").getId(),equalTo("GMT+01:00"));
23+
}
24+
25+
1926
public void testParsingIsCaseInsensitive() {
20-
TimeZone timeZone = randomTimeZone();
21-
assertThat(TimezoneUtils.parse(timeZone.getID()), equalTo(timeZone));
22-
assertThat(TimezoneUtils.parse(timeZone.getID().toLowerCase(Locale.ROOT)), equalTo(timeZone));
23-
assertThat(TimezoneUtils.parse(timeZone.getID().toUpperCase(Locale.ROOT)), equalTo(timeZone));
27+
ZoneId timeZone = randomTimeZone().toZoneId();
28+
assertThat(TimezoneUtils.parse(timeZone.getId()), equalTo(timeZone));
29+
assertThat(TimezoneUtils.parse(timeZone.getId().toLowerCase(Locale.ROOT)), equalTo(timeZone));
30+
assertThat(TimezoneUtils.parse(timeZone.getId().toUpperCase(Locale.ROOT)), equalTo(timeZone));
2431
}
2532

2633
public void testParsingOffsets() {
27-
TimeZone timeZone = TimeZone.getTimeZone("GMT+01:00");
34+
ZoneId timeZone = ZoneId.of("GMT+01:00");
2835
assertThat(TimezoneUtils.parse("GMT+01:00"), equalTo(timeZone));
2936
assertThat(TimezoneUtils.parse("gmt+01:00"), equalTo(timeZone));
3037
assertThat(TimezoneUtils.parse("GMT+1"), equalTo(timeZone));
31-
assertThat(TimezoneUtils.parse("+1"), equalTo(timeZone));
38+
39+
assertThat(TimezoneUtils.parse("+1"), equalTo(ZoneId.of("+01:00")));
3240
}
3341
}

0 commit comments

Comments
 (0)