From 70e2e896024d7d5a7601b08185008f2ff5d1202d Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 29 Jan 2024 13:02:43 +0100 Subject: [PATCH] Explicit documentation note on cron-vs-quartz parsing convention Closes gh-32128 --- .../scheduling/support/BitsCronField.java | 23 +++--- .../scheduling/support/CronExpression.java | 11 ++- .../scheduling/support/CronField.java | 19 +++-- .../scheduling/support/CronTrigger.java | 10 ++- .../scheduling/support/QuartzCronField.java | 78 ++++++++++--------- .../support/BitsCronFieldTests.java | 22 ++++-- .../support/QuartzCronFieldTests.java | 43 +++++++++- 7 files changed, 138 insertions(+), 68 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/scheduling/support/BitsCronField.java b/spring-context/src/main/java/org/springframework/scheduling/support/BitsCronField.java index 673aaaf6a11..7437575c7b4 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/support/BitsCronField.java +++ b/spring-context/src/main/java/org/springframework/scheduling/support/BitsCronField.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -247,24 +247,25 @@ final class BitsCronField extends CronField { } private void clearBit(int index) { - this.bits &= ~(1L << index); + this.bits &= ~(1L << index); } - @Override - public int hashCode() { - return Long.hashCode(this.bits); - } @Override - public boolean equals(Object o) { - if (this == o) { + public boolean equals(Object other) { + if (this == other) { return true; } - if (!(o instanceof BitsCronField)) { + if (!(other instanceof BitsCronField)) { return false; } - BitsCronField other = (BitsCronField) o; - return type() == other.type() && this.bits == other.bits; + BitsCronField otherField = (BitsCronField) other; + return (type() == otherField.type() && this.bits == otherField.bits); + } + + @Override + public int hashCode() { + return Long.hashCode(this.bits); } @Override 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 2549faa3efe..7edabd8fa84 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 @@ -29,9 +29,14 @@ import org.springframework.util.StringUtils; * crontab expression * that can calculate the next time it matches. * - *

{@code CronExpression} instances are created through - * {@link #parse(String)}; the next match is determined with - * {@link #next(Temporal)}. + *

{@code CronExpression} instances are created through {@link #parse(String)}; + * the next match is determined with {@link #next(Temporal)}. + * + *

Supports a Quartz day-of-month/week field with an L/# expression. Follows + * common cron conventions in every other respect, including 0-6 for SUN-SAT + * (plus 7 for SUN as well). Note that Quartz deviates from the day-of-week + * convention in cron through 1-7 for SUN-SAT whereas Spring strictly follows + * cron even in combination with the optional Quartz-specific L/# expressions. * * @author Arjen Poutsma * @since 5.3 diff --git a/spring-context/src/main/java/org/springframework/scheduling/support/CronField.java b/spring-context/src/main/java/org/springframework/scheduling/support/CronField.java index 99d940613e5..3369f62bafc 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/support/CronField.java +++ b/spring-context/src/main/java/org/springframework/scheduling/support/CronField.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,15 +31,22 @@ import org.springframework.util.StringUtils; * Single field in a cron pattern. Created using the {@code parse*} methods, * main and only entry point is {@link #nextOrSame(Temporal)}. * + *

Supports a Quartz day-of-month/week field with an L/# expression. Follows + * common cron conventions in every other respect, including 0-6 for SUN-SAT + * (plus 7 for SUN as well). Note that Quartz deviates from the day-of-week + * convention in cron through 1-7 for SUN-SAT whereas Spring strictly follows + * cron even in combination with the optional Quartz-specific L/# expressions. + * * @author Arjen Poutsma * @since 5.3 */ abstract class CronField { - private static final String[] MONTHS = new String[]{"JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG", "SEP", - "OCT", "NOV", "DEC"}; + private static final String[] MONTHS = new String[] + {"JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG", "SEP", "OCT", "NOV", "DEC"}; - private static final String[] DAYS = new String[]{"MON", "TUE", "WED", "THU", "FRI", "SAT", "SUN"}; + private static final String[] DAYS = new String[] + {"MON", "TUE", "WED", "THU", "FRI", "SAT", "SUN"}; private final Type type; @@ -48,6 +55,7 @@ abstract class CronField { this.type = type; } + /** * Return a {@code CronField} enabled for 0 nanoseconds. */ @@ -169,6 +177,7 @@ abstract class CronField { * day-of-month, month, day-of-week. */ protected enum Type { + NANO(ChronoField.NANO_OF_SECOND, ChronoUnit.SECONDS), SECOND(ChronoField.SECOND_OF_MINUTE, ChronoUnit.MINUTES, ChronoField.NANO_OF_SECOND), MINUTE(ChronoField.MINUTE_OF_HOUR, ChronoUnit.HOURS, ChronoField.SECOND_OF_MINUTE, ChronoField.NANO_OF_SECOND), @@ -184,14 +193,12 @@ abstract class CronField { private final ChronoField[] lowerOrders; - Type(ChronoField field, ChronoUnit higherOrder, ChronoField... lowerOrders) { this.field = field; this.higherOrder = higherOrder; this.lowerOrders = lowerOrders; } - /** * Return the value of this type for the given temporal. * @return the value of this type diff --git a/spring-context/src/main/java/org/springframework/scheduling/support/CronTrigger.java b/spring-context/src/main/java/org/springframework/scheduling/support/CronTrigger.java index 215ef40bbb1..7496d18f9d8 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/support/CronTrigger.java +++ b/spring-context/src/main/java/org/springframework/scheduling/support/CronTrigger.java @@ -27,8 +27,14 @@ import org.springframework.scheduling.TriggerContext; import org.springframework.util.Assert; /** - * {@link Trigger} implementation for cron expressions. - * Wraps a {@link CronExpression}. + * {@link Trigger} implementation for cron expressions. Wraps a + * {@link CronExpression} which parses according to common crontab conventions. + * + *

Supports a Quartz day-of-month/week field with an L/# expression. Follows + * common cron conventions in every other respect, including 0-6 for SUN-SAT + * (plus 7 for SUN as well). Note that Quartz deviates from the day-of-week + * convention in cron through 1-7 for SUN-SAT whereas Spring strictly follows + * cron even in combination with the optional Quartz-specific L/# expressions. * * @author Juergen Hoeller * @author Arjen Poutsma 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 36c70c08831..2963817743a 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,10 +29,16 @@ import org.springframework.util.Assert; /** * Extension of {@link CronField} for - * Quartz-specific fields. * Created using the {@code parse*} methods, uses a {@link TemporalAdjuster} * internally. * + *

Supports a Quartz day-of-month/week field with an L/# expression. Follows + * common cron conventions in every other respect, including 0-6 for SUN-SAT + * (plus 7 for SUN as well). Note that Quartz deviates from the day-of-week + * convention in cron through 1-7 for SUN-SAT whereas Spring strictly follows + * cron even in combination with the optional Quartz-specific L/# expressions. + * * @author Arjen Poutsma * @since 5.3 */ @@ -60,8 +66,9 @@ final class QuartzCronField extends CronField { this.rollForwardType = rollForwardType; } + /** - * Returns whether the given value is a Quartz day-of-month field. + * Determine whether the given value is a Quartz day-of-month field. */ public static boolean isQuartzDaysOfMonthField(String value) { return value.contains("L") || value.contains("W"); @@ -78,14 +85,14 @@ final class QuartzCronField extends CronField { if (idx != 0) { throw new IllegalArgumentException("Unrecognized characters before 'L' in '" + value + "'"); } - else if (value.length() == 2 && value.charAt(1) == 'W') { // "LW" + else if (value.length() == 2 && value.charAt(1) == 'W') { // "LW" adjuster = lastWeekdayOfMonth(); } else { - if (value.length() == 1) { // "L" + if (value.length() == 1) { // "L" adjuster = lastDayOfMonth(); } - else { // "L-[0-9]+" + else { // "L-[0-9]+" int offset = Integer.parseInt(value.substring(idx + 1)); if (offset >= 0) { throw new IllegalArgumentException("Offset '" + offset + " should be < 0 '" + value + "'"); @@ -103,7 +110,7 @@ final class QuartzCronField extends CronField { else if (idx != value.length() - 1) { throw new IllegalArgumentException("Unrecognized characters after 'W' in '" + value + "'"); } - else { // "[0-9]+W" + else { // "[0-9]+W" int dayOfMonth = Integer.parseInt(value.substring(0, idx)); dayOfMonth = Type.DAY_OF_MONTH.checkValidValue(dayOfMonth); TemporalAdjuster adjuster = weekdayNearestTo(dayOfMonth); @@ -114,15 +121,16 @@ final class QuartzCronField extends CronField { } /** - * Returns whether the given value is a Quartz day-of-week field. + * Determine whether the given value is a Quartz day-of-week field. */ public static boolean isQuartzDaysOfWeekField(String value) { return value.contains("L") || value.contains("#"); } /** - * Parse the given value into a days of week {@code QuartzCronField}, the sixth entry of a cron expression. - * Expects a "L" or "#" in the given value. + * Parse the given value into a days of week {@code QuartzCronField}, + * the sixth entry of a cron expression. + *

Expects a "L" or "#" in the given value. */ public static QuartzCronField parseDaysOfWeek(String value) { int idx = value.lastIndexOf('L'); @@ -135,7 +143,7 @@ final class QuartzCronField extends CronField { if (idx == 0) { throw new IllegalArgumentException("No day-of-week before 'L' in '" + value + "'"); } - else { // "[0-7]L" + else { // "[0-7]L" DayOfWeek dayOfWeek = parseDayOfWeek(value.substring(0, idx)); adjuster = lastInMonth(dayOfWeek); } @@ -157,7 +165,6 @@ final class QuartzCronField extends CronField { throw new IllegalArgumentException("Ordinal '" + ordinal + "' in '" + value + "' must be positive number "); } - TemporalAdjuster adjuster = dayOfWeekInMonth(ordinal, dayOfWeek); return new QuartzCronField(Type.DAY_OF_WEEK, Type.DAY_OF_MONTH, adjuster, value); } @@ -167,14 +174,13 @@ final class QuartzCronField extends CronField { private static DayOfWeek parseDayOfWeek(String value) { int dayOfWeek = Integer.parseInt(value); if (dayOfWeek == 0) { - dayOfWeek = 7; // cron is 0 based; java.time 1 based + dayOfWeek = 7; // cron is 0 based; java.time 1 based } try { return DayOfWeek.of(dayOfWeek); } catch (DateTimeException ex) { - String msg = ex.getMessage() + " '" + value + "'"; - throw new IllegalArgumentException(msg, ex); + throw new IllegalArgumentException(ex.getMessage() + " '" + value + "'", ex); } } @@ -213,10 +219,10 @@ final class QuartzCronField extends CronField { Temporal lastDom = adjuster.adjustInto(temporal); Temporal result; int dow = lastDom.get(ChronoField.DAY_OF_WEEK); - if (dow == 6) { // Saturday + if (dow == 6) { // Saturday result = lastDom.minus(1, ChronoUnit.DAYS); } - else if (dow == 7) { // Sunday + else if (dow == 7) { // Sunday result = lastDom.minus(2, ChronoUnit.DAYS); } else { @@ -227,7 +233,7 @@ final class QuartzCronField extends CronField { } /** - * Return a temporal adjuster that finds the nth-to-last day of the month. + * Returns a temporal adjuster that finds the nth-to-last day of the month. * @param offset the negative offset, i.e. -3 means third-to-last * @return a nth-to-last day-of-month adjuster */ @@ -241,7 +247,7 @@ final class QuartzCronField extends CronField { } /** - * Return a temporal adjuster that finds the weekday nearest to the given + * Returns a temporal adjuster that finds the weekday nearest to the given * day-of-month. If {@code dayOfMonth} falls on a Saturday, the date is * moved back to Friday; if it falls on a Sunday (or if {@code dayOfMonth} * is 1 and it falls on a Saturday), it is moved forward to Monday. @@ -253,10 +259,10 @@ final class QuartzCronField extends CronField { int current = Type.DAY_OF_MONTH.get(temporal); DayOfWeek dayOfWeek = DayOfWeek.from(temporal); - if ((current == dayOfMonth && isWeekday(dayOfWeek)) || // dayOfMonth is a weekday - (dayOfWeek == DayOfWeek.FRIDAY && current == dayOfMonth - 1) || // dayOfMonth is a Saturday, so Friday before - (dayOfWeek == DayOfWeek.MONDAY && current == dayOfMonth + 1) || // dayOfMonth is a Sunday, so Monday after - (dayOfWeek == DayOfWeek.MONDAY && dayOfMonth == 1 && current == 3)) { // dayOfMonth is Saturday 1st, so Monday 3rd + if ((current == dayOfMonth && isWeekday(dayOfWeek)) || // dayOfMonth is a weekday + (dayOfWeek == DayOfWeek.FRIDAY && current == dayOfMonth - 1) || // dayOfMonth is a Saturday, so Friday before + (dayOfWeek == DayOfWeek.MONDAY && current == dayOfMonth + 1) || // dayOfMonth is a Sunday, so Monday after + (dayOfWeek == DayOfWeek.MONDAY && dayOfMonth == 1 && current == 3)) { // dayOfMonth is Saturday 1st, so Monday 3rd return temporal; } int count = 0; @@ -292,7 +298,7 @@ final class QuartzCronField extends CronField { } /** - * Return a temporal adjuster that finds the last of the given doy-of-week + * Returns a temporal adjuster that finds the last of the given day-of-week * in a month. */ private static TemporalAdjuster lastInMonth(DayOfWeek dayOfWeek) { @@ -329,6 +335,7 @@ final class QuartzCronField extends CronField { } } + @Override public > T nextOrSame(T temporal) { T result = adjust(temporal); @@ -345,7 +352,6 @@ final class QuartzCronField extends CronField { return result; } - @Nullable @SuppressWarnings("unchecked") private > T adjust(T temporal) { @@ -354,27 +360,25 @@ final class QuartzCronField extends CronField { @Override - public int hashCode() { - return this.value.hashCode(); - } - - @Override - public boolean equals(Object o) { - if (this == o) { + public boolean equals(@Nullable Object other) { + if (this == other) { return true; } - if (!(o instanceof QuartzCronField)) { + if (!(other instanceof QuartzCronField)) { return false; } - QuartzCronField other = (QuartzCronField) o; - return type() == other.type() && - this.value.equals(other.value); + QuartzCronField otherField = (QuartzCronField) other; + return (type() == otherField.type() && this.value.equals(otherField.value)); + } + + @Override + public int hashCode() { + return this.value.hashCode(); } @Override public String toString() { return type() + " '" + this.value + "'"; - } } diff --git a/spring-context/src/test/java/org/springframework/scheduling/support/BitsCronFieldTests.java b/spring-context/src/test/java/org/springframework/scheduling/support/BitsCronFieldTests.java index 37d6b30d4fd..b69afdbb3d6 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/support/BitsCronFieldTests.java +++ b/spring-context/src/test/java/org/springframework/scheduling/support/BitsCronFieldTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,27 +35,32 @@ class BitsCronFieldTests { @Test void parse() { assertThat(BitsCronField.parseSeconds("42")).has(clearRange(0, 41)).has(set(42)).has(clearRange(43, 59)); - assertThat(BitsCronField.parseSeconds("0-4,8-12")).has(setRange(0, 4)).has(clearRange(5,7)).has(setRange(8, 12)).has(clearRange(13,59)); - assertThat(BitsCronField.parseSeconds("57/2")).has(clearRange(0, 56)).has(set(57)).has(clear(58)).has(set(59)); + assertThat(BitsCronField.parseSeconds("0-4,8-12")).has(setRange(0, 4)).has(clearRange(5,7)) + .has(setRange(8, 12)).has(clearRange(13,59)); + assertThat(BitsCronField.parseSeconds("57/2")).has(clearRange(0, 56)).has(set(57)) + .has(clear(58)).has(set(59)); assertThat(BitsCronField.parseMinutes("30")).has(set(30)).has(clearRange(1, 29)).has(clearRange(31, 59)); assertThat(BitsCronField.parseHours("23")).has(set(23)).has(clearRange(0, 23)); - assertThat(BitsCronField.parseHours("0-23/2")).has(set(0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22)).has(clear(1,3,5,7,9,11,13,15,17,19,21,23)); + assertThat(BitsCronField.parseHours("0-23/2")).has(set(0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22)) + .has(clear(1,3,5,7,9,11,13,15,17,19,21,23)); assertThat(BitsCronField.parseDaysOfMonth("1")).has(set(1)).has(clearRange(2, 31)); assertThat(BitsCronField.parseMonth("1")).has(set(1)).has(clearRange(2, 12)); assertThat(BitsCronField.parseDaysOfWeek("0")).has(set(7, 7)).has(clearRange(0, 6)); - - assertThat(BitsCronField.parseDaysOfWeek("7-5")).has(clear(0)).has(setRange(1, 5)).has(clear(6)).has(set(7)); + assertThat(BitsCronField.parseDaysOfWeek("7-5")).has(clear(0)).has(setRange(1, 5)) + .has(clear(6)).has(set(7)); } @Test void parseLists() { - assertThat(BitsCronField.parseSeconds("15,30")).has(set(15, 30)).has(clearRange(1, 15)).has(clearRange(31, 59)); - assertThat(BitsCronField.parseMinutes("1,2,5,9")).has(set(1, 2, 5, 9)).has(clear(0)).has(clearRange(3, 4)).has(clearRange(6, 8)).has(clearRange(10, 59)); + assertThat(BitsCronField.parseSeconds("15,30")).has(set(15, 30)).has(clearRange(1, 15)) + .has(clearRange(31, 59)); + assertThat(BitsCronField.parseMinutes("1,2,5,9")).has(set(1, 2, 5, 9)).has(clear(0)) + .has(clearRange(3, 4)).has(clearRange(6, 8)).has(clearRange(10, 59)); assertThat(BitsCronField.parseHours("1,2,3")).has(set(1, 2, 3)).has(clearRange(4, 23)); assertThat(BitsCronField.parseDaysOfMonth("1,2,3")).has(set(1, 2, 3)).has(clearRange(4, 31)); assertThat(BitsCronField.parseMonth("1,2,3")).has(set(1, 2, 3)).has(clearRange(4, 12)); @@ -107,6 +112,7 @@ class BitsCronFieldTests { .has(clear(0)).has(setRange(1, 7)); } + private static Condition set(int... indices) { return new Condition(String.format("set bits %s", Arrays.toString(indices))) { @Override 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 ec56f366b5b..14c5907af48 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,6 +28,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException * Unit tests for {@link QuartzCronField}. * * @author Arjen Poutsma + * @author Juergen Hoeller */ class QuartzCronFieldTests { @@ -71,6 +72,46 @@ class QuartzCronFieldTests { assertThat(field.nextOrSame(last)).isEqualTo(expected); } + @Test + void dayOfWeek_0(){ + // third Sunday (0) of the month + QuartzCronField field = QuartzCronField.parseDaysOfWeek("0#3"); + + LocalDate last = LocalDate.of(2024, 1, 1); + LocalDate expected = LocalDate.of(2024, 1, 21); + assertThat(field.nextOrSame(last)).isEqualTo(expected); + } + + @Test + void dayOfWeek_1(){ + // third Monday (1) of the month + QuartzCronField field = QuartzCronField.parseDaysOfWeek("1#3"); + + LocalDate last = LocalDate.of(2024, 1, 1); + LocalDate expected = LocalDate.of(2024, 1, 15); + assertThat(field.nextOrSame(last)).isEqualTo(expected); + } + + @Test + void dayOfWeek_2(){ + // third Tuesday (2) of the month + QuartzCronField field = QuartzCronField.parseDaysOfWeek("2#3"); + + LocalDate last = LocalDate.of(2024, 1, 1); + LocalDate expected = LocalDate.of(2024, 1, 16); + assertThat(field.nextOrSame(last)).isEqualTo(expected); + } + + @Test + void dayOfWeek_7() { + // third Sunday (7 as alternative to 0) of the month + QuartzCronField field = QuartzCronField.parseDaysOfWeek("7#3"); + + LocalDate last = LocalDate.of(2024, 1, 1); + LocalDate expected = LocalDate.of(2024, 1, 21); + assertThat(field.nextOrSame(last)).isEqualTo(expected); + } + @Test void invalidValues() { assertThatIllegalArgumentException().isThrownBy(() -> QuartzCronField.parseDaysOfMonth(""));