Explicit documentation note on cron-vs-quartz parsing convention

Closes gh-32128

(cherry picked from commit a738e4d)

Author: jhoeller

Diff for: spring-context/src/main/java/org/springframework/scheduling/support/CronExpression.java

@@ -29,9 +29,14 @@
  * <a href="https://www.manpagez.com/man/5/crontab/">crontab expression</a>
  * that can calculate the next time it matches.
  *
- * <p>{@code CronExpression} instances are created through
- * {@link #parse(String)}; the next match is determined with
- * {@link #next(Temporal)}.
+ * <p>{@code CronExpression} instances are created through {@link #parse(String)};
+ * the next match is determined with {@link #next(Temporal)}.
+ *
+ * <p>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 for: 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 @@
  * Single field in a cron pattern. Created using the {@code parse*} methods,
  * main and only entry point is {@link #nextOrSame(Temporal)}.
  *
+ * <p>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 @@ protected CronField(Type type) {
 		this.type = type;
 	}
 
+
 	/**
 	 * Return a {@code CronField} enabled for 0 nanoseconds.
 	 */
@@ -169,6 +177,7 @@ protected static <T extends Temporal & Comparable<? super T>> T cast(Temporal te
 	 * 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 @@ protected enum Type {
 
 		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 for: spring-context/src/main/java/org/springframework/scheduling/support/CronTrigger.java

@@ -27,8 +27,14 @@
 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.
+ *
+ * <p>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 for: spring-context/src/main/java/org/springframework/scheduling/support/QuartzCronField.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 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.
@@ -30,10 +30,15 @@
 /**
  * Extension of {@link CronField} for
  * <a href="https://www.quartz-scheduler.org">Quartz</a>-specific fields.
- *
- * <p>Created using the {@code parse*} methods, uses a {@link TemporalAdjuster}
+ * Created using the {@code parse*} methods, uses a {@link TemporalAdjuster}
  * internally.
  *
+ * <p>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
  */
@@ -61,8 +66,9 @@ private QuartzCronField(Type type, Type rollForwardType, TemporalAdjuster adjust
 		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");
@@ -80,14 +86,14 @@ public static QuartzCronField parseDaysOfMonth(String value) {
 			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, idx + 1, value.length(), 10);
 					if (offset >= 0) {
 						throw new IllegalArgumentException("Offset '" + offset + " should be < 0 '" + value + "'");
@@ -105,7 +111,7 @@ else if (value.length() == 2 && value.charAt(1) == 'W') { // "LW"
 			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, 0, idx, 10);
 				dayOfMonth = Type.DAY_OF_MONTH.checkValidValue(dayOfMonth);
 				TemporalAdjuster adjuster = weekdayNearestTo(dayOfMonth);
@@ -116,7 +122,7 @@ else if (idx != value.length() - 1) {
 	}
 
 	/**
-	 * 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("#");
@@ -138,7 +144,7 @@ public static QuartzCronField parseDaysOfWeek(String value) {
 				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);
 				}
@@ -160,7 +166,6 @@ else if (idx == value.length() - 1) {
 				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);
 		}
@@ -170,14 +175,13 @@ else if (idx == value.length() - 1) {
 	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);
 		}
 	}
 
@@ -216,10 +220,10 @@ private static TemporalAdjuster lastWeekdayOfMonth() {
 			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 {
@@ -256,10 +260,10 @@ private static TemporalAdjuster weekdayNearestTo(int dayOfMonth) {
 			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;
@@ -357,26 +361,19 @@ private <T extends Temporal & Comparable<? super T>> T adjust(T temporal) {
 
 
 	@Override
-	public int hashCode() {
-		return this.value.hashCode();
+	public boolean equals(@Nullable Object other) {
+		return (this == other || (other instanceof QuartzCronField that &&
+				type() == that.type() && this.value.equals(that.value)));
 	}
 
 	@Override
-	public boolean equals(@Nullable Object o) {
-		if (this == o) {
-			return true;
-		}
-		if (!(o instanceof QuartzCronField other)) {
-			return false;
-		}
-		return type() == other.type() &&
-				this.value.equals(other.value);
+	public int hashCode() {
+		return this.value.hashCode();
 	}
 
 	@Override
 	public String toString() {
 		return type() + " '" + this.value + "'";
-
 	}
 
 }

Diff for: spring-context/src/test/java/org/springframework/scheduling/support/BitsCronFieldTests.java

@@ -35,27 +35,33 @@ 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 +113,7 @@ void names() {
 				.has(clear(0)).has(setRange(1, 7));
 	}
 
+
 	private static Condition<BitsCronField> set(int... indices) {
 		return new Condition<>(String.format("set bits %s", Arrays.toString(indices))) {
 			@Override

Diff for: spring-context/src/test/java/org/springframework/scheduling/support/QuartzCronFieldTests.java

@@ -28,6 +28,7 @@
  * Unit tests for {@link QuartzCronField}.
  *
  * @author Arjen Poutsma
+ * @author Juergen Hoeller
  */
 class QuartzCronFieldTests {
 
@@ -71,6 +72,42 @@ void lastDayOfWeekOffset() {
 		assertThat(field.nextOrSame(last)).isEqualTo(expected);
 	}
 
+	@Test
+	void dayOfWeek_0(){
+		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(){
+		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(){
+		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() {
+		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(""));