From 4d670ee25d8a1e615883dfb4468919c2b43c33b0 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Wed, 5 Aug 2020 09:42:18 +0200 Subject: [PATCH] Drop support for standalone "L" in CronExpression This commit removes support for a standalone "L" in the day-of-week of a cron expression, which used a locale-dependent API to determine what the last day of the week is (Saturday or Sunday ?). Alternatively, we could have implement this in the exact way as Quartz has done (i.e. treat the "L" like "SAT"), but we opted not to do that, as having an explicit SAT or SUN is much clearer. --- .../scheduling/support/CronExpression.java | 5 +- .../scheduling/support/QuartzCronField.java | 19 +------ .../support/CronExpressionTests.java | 56 ------------------- .../support/QuartzCronFieldTests.java | 46 +-------------- 4 files changed, 4 insertions(+), 122 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/scheduling/support/CronExpression.java b/spring-context/src/main/java/org/springframework/scheduling/support/CronExpression.java index e3bf57b27ee..08a6444a6b4 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/support/CronExpression.java +++ b/spring-context/src/main/java/org/springframework/scheduling/support/CronExpression.java @@ -121,9 +121,7 @@ public final class CronExpression { * *
  • * In the "day of week" field, {@code L} stands for "the last day of the - * week", and uses the - * {@linkplain java.util.Locale#getDefault() system default locale} - * to determine which day that is (i.e. Sunday or Saturday). + * week". * If prefixed by a number or three-letter name (i.e. {@code dL} or * {@code DDDL}), it means "the last day of week {@code d} (or {@code DDD}) * in the month". @@ -158,7 +156,6 @@ public final class CronExpression { *
  • {@code "0 0 0 L-3 * *"} = third-to-last day of the month at midnight
    • *
  • {@code "0 0 0 1W * *"} = first weekday of the month at midnight
    • *
  • {@code "0 0 0 LW * *"} = last weekday of the month at midnight
    • - *
  • {@code "0 0 0 * * L"} = last day of the week at midnight
    • *
  • {@code "0 0 0 * * 5L"} = last Friday of the month at midnight
    • *
  • {@code "0 0 0 * * THUL"} = last Thursday of the month at midnight
    • *
  • {@code "0 0 0 ? * 5#2"} = the second Friday in the month at midnight
    • diff --git a/spring-context/src/main/java/org/springframework/scheduling/support/QuartzCronField.java b/spring-context/src/main/java/org/springframework/scheduling/support/QuartzCronField.java index 5db9d31bf36..8492a590a18 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/support/QuartzCronField.java +++ b/spring-context/src/main/java/org/springframework/scheduling/support/QuartzCronField.java @@ -23,9 +23,6 @@ import java.time.temporal.ChronoUnit; import java.time.temporal.Temporal; import java.time.temporal.TemporalAdjuster; import java.time.temporal.TemporalAdjusters; -import java.time.temporal.TemporalField; -import java.time.temporal.WeekFields; -import java.util.Locale; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -140,8 +137,8 @@ final class QuartzCronField extends CronField { } else { TemporalAdjuster adjuster; - if (idx == 0) { // "L" - adjuster = lastDayOfWeek(Locale.getDefault()); + if (idx == 0) { + throw new IllegalArgumentException("No day-of-week before 'L' in '" + value + "'"); } else { // "[0-7]L" DayOfWeek dayOfWeek = parseDayOfWeek(value.substring(0, idx)); @@ -196,18 +193,6 @@ final class QuartzCronField extends CronField { }; } - /** - * Return a temporal adjuster that finds the last day-of-week, depending - * on the given locale. - * @param locale the locale to base the last day calculation on - * @return the last day-of-week adjuster - */ - private static TemporalAdjuster lastDayOfWeek(Locale locale) { - Assert.notNull(locale, "Locale must not be null"); - TemporalField dayOfWeek = WeekFields.of(locale).dayOfWeek(); - return temporal -> temporal.with(dayOfWeek, 7); - } - /** * Return a temporal adjuster that finds the weekday nearest to the given * day-of-month. If {@code dayOfMonth} falls on a Saturday, the date is diff --git a/spring-context/src/test/java/org/springframework/scheduling/support/CronExpressionTests.java b/spring-context/src/test/java/org/springframework/scheduling/support/CronExpressionTests.java index 93206f1d5f7..9d25010cd43 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/support/CronExpressionTests.java +++ b/spring-context/src/test/java/org/springframework/scheduling/support/CronExpressionTests.java @@ -24,14 +24,12 @@ import java.time.ZoneId; import java.time.ZonedDateTime; import java.time.temporal.ChronoField; import java.time.temporal.Temporal; -import java.util.Locale; import org.assertj.core.api.Condition; import org.junit.jupiter.api.Test; import static java.time.DayOfWeek.FRIDAY; import static java.time.DayOfWeek.MONDAY; -import static java.time.DayOfWeek.SATURDAY; import static java.time.DayOfWeek.SUNDAY; import static java.time.DayOfWeek.TUESDAY; import static java.time.DayOfWeek.WEDNESDAY; @@ -736,60 +734,6 @@ class CronExpressionTests { assertThat(actual).is(weekday); } - @Test - public void quartzLastDayOfWeekFirstDayMonday() { - Locale defaultLocale = Locale.getDefault(); - try { - Locale.setDefault(Locale.UK); - - CronExpression expression = CronExpression.parse("0 0 0 * * L"); - - LocalDateTime last = LocalDateTime.of(LocalDate.of(2008, 1, 4), LocalTime.now()); - LocalDateTime expected = LocalDateTime.of(2008, 1, 6, 0, 0); - LocalDateTime actual = expression.next(last); - assertThat(actual).isNotNull(); - assertThat(actual).isEqualTo(expected); - assertThat(actual.getDayOfWeek()).isEqualTo(SUNDAY); - - last = actual; - expected = expected.plusWeeks(1); - actual = expression.next(last); - assertThat(actual).isNotNull(); - assertThat(actual).isEqualTo(expected); - assertThat(actual.getDayOfWeek()).isEqualTo(SUNDAY); - } - finally { - Locale.setDefault(defaultLocale); - } - } - - @Test - public void quartzLastDayOfWeekFirstDaySunday() { - Locale defaultLocale = Locale.getDefault(); - try { - Locale.setDefault(Locale.US); - - CronExpression expression = CronExpression.parse("0 0 0 * * L"); - - LocalDateTime last = LocalDateTime.of(LocalDate.of(2008, 1, 4), LocalTime.now()); - LocalDateTime expected = LocalDateTime.of(2008, 1, 5, 0, 0); - LocalDateTime actual = expression.next(last); - assertThat(actual).isNotNull(); - assertThat(actual).isEqualTo(expected); - assertThat(actual.getDayOfWeek()).isEqualTo(SATURDAY); - - last = actual; - expected = expected.plusWeeks(1); - actual = expression.next(last); - assertThat(actual).isNotNull(); - assertThat(actual).isEqualTo(expected); - assertThat(actual.getDayOfWeek()).isEqualTo(SATURDAY); - } - finally { - Locale.setDefault(defaultLocale); - } - } - @Test public void quartzLastDayOfWeekOffset() { // last Friday (5) of the month diff --git a/spring-context/src/test/java/org/springframework/scheduling/support/QuartzCronFieldTests.java b/spring-context/src/test/java/org/springframework/scheduling/support/QuartzCronFieldTests.java index 3d3c79ada19..bd16243c1bd 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/support/QuartzCronFieldTests.java +++ b/spring-context/src/test/java/org/springframework/scheduling/support/QuartzCronFieldTests.java @@ -18,12 +18,9 @@ package org.springframework.scheduling.support; import java.time.DayOfWeek; import java.time.LocalDate; -import java.util.Locale; import org.junit.jupiter.api.Test; -import static java.time.DayOfWeek.SATURDAY; -import static java.time.DayOfWeek.SUNDAY; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -62,48 +59,6 @@ class QuartzCronFieldTests { assertThat(actual).isEqualTo(expected); } - @Test - public void lastDayOfWeekFirstDayMonday() { - Locale defaultLocale = Locale.getDefault(); - try { - Locale.setDefault(Locale.UK); - QuartzCronField field = QuartzCronField.parseDaysOfWeek("L"); - - LocalDate last = LocalDate.of(2020, 6, 16); - LocalDate expected = LocalDate.of(2020, 6, 21); - assertThat(field.nextOrSame(last)).isEqualTo(expected); - - LocalDate actual = field.nextOrSame(last); - assertThat(actual).isNotNull(); - assertThat(actual).isEqualTo(expected); - assertThat(actual.getDayOfWeek()).isEqualTo(SUNDAY); - } - finally { - Locale.setDefault(defaultLocale); - } - } - - @Test - public void lastDayOfWeekFirstDaySunday() { - Locale defaultLocale = Locale.getDefault(); - try { - Locale.setDefault(Locale.US); - QuartzCronField field = QuartzCronField.parseDaysOfWeek("L"); - - LocalDate last = LocalDate.of(2020, 6, 16); - LocalDate expected = LocalDate.of(2020, 6, 20); - assertThat(field.nextOrSame(last)).isEqualTo(expected); - - LocalDate actual = field.nextOrSame(last); - assertThat(actual).isNotNull(); - assertThat(actual).isEqualTo(expected); - assertThat(actual.getDayOfWeek()).isEqualTo(SATURDAY); - } - finally { - Locale.setDefault(defaultLocale); - } - } - @Test void lastDayOfWeekOffset() { // last Thursday (4) of the month @@ -129,6 +84,7 @@ class QuartzCronFieldTests { assertThatIllegalArgumentException().isThrownBy(() -> QuartzCronField.parseDaysOfWeek("")); assertThatIllegalArgumentException().isThrownBy(() -> QuartzCronField.parseDaysOfWeek("1")); + assertThatIllegalArgumentException().isThrownBy(() -> QuartzCronField.parseDaysOfWeek("L")); assertThatIllegalArgumentException().isThrownBy(() -> QuartzCronField.parseDaysOfWeek("L1")); assertThatIllegalArgumentException().isThrownBy(() -> QuartzCronField.parseDaysOfWeek("LL")); assertThatIllegalArgumentException().isThrownBy(() -> QuartzCronField.parseDaysOfWeek("-4L"));