Allow quartz expression in cron expression lists

This commit introduces support for lists of quartz cron fields, such
as "1L, LW" or "TUE#1, TUE#3, TUE#5".

Closes gh-26289

spring-context/src/main/java/org/springframework/scheduling/support/BitsCronField.java

@@ -19,14 +19,13 @@ package org.springframework.scheduling.support;
 import java.time.DateTimeException;
 import java.time.temporal.Temporal;
 import java.time.temporal.ValueRange;
-import java.util.BitSet;
 
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 
 /**
- * Efficient {@link BitSet}-based extension of {@link CronField}.
+ * Efficient bitwise-operator extension of {@link CronField}.
  * Created using the {@code parse*} methods.
  *
  * @author Arjen Poutsma

spring-context/src/main/java/org/springframework/scheduling/support/CompositeCronField.java

@@ -0,0 +1,97 @@
+/*
+ * Copyright 2002-2021 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.scheduling.support;
+
+import java.time.temporal.Temporal;
+
+import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
+
+/**
+ * Extension of {@link CronField} that wraps an array of cron fields.
+ *
+ * @author Arjen Poutsma
+ * @since 5.3.3
+ */
+final class CompositeCronField extends CronField {
+
+	private final CronField[] fields;
+
+	private final String value;
+
+
+	private CompositeCronField(Type type, CronField[] fields, String value) {
+		super(type);
+		this.fields = fields;
+		this.value = value;
+	}
+
+	/**
+	 * Composes the given fields into a {@link CronField}.
+	 */
+	public static CronField compose(CronField[] fields, Type type, String value) {
+		Assert.notEmpty(fields, "Fields must not be empty");
+		Assert.hasLength(value, "Value must not be empty");
+
+		if (fields.length == 1) {
+			return fields[0];
+		}
+		else {
+			return new CompositeCronField(type, fields, value);
+		}
+	}
+
+
+	@Nullable
+	@Override
+	public <T extends Temporal & Comparable<? super T>> T nextOrSame(T temporal) {
+		T result = null;
+		for (CronField field : this.fields) {
+			T candidate = field.nextOrSame(temporal);
+			if (result == null ||
+					candidate != null && candidate.compareTo(result) < 0) {
+				result = candidate;
+			}
+		}
+		return result;
+	}
+
+
+	@Override
+	public int hashCode() {
+		return this.value.hashCode();
+	}
+
+	@Override
+	public boolean equals(Object o) {
+		if (this == o) {
+			return true;
+		}
+		if (!(o instanceof CompositeCronField)) {
+			return false;
+		}
+		CompositeCronField other = (CompositeCronField) o;
+		return type() == other.type() &&
+				this.value.equals(other.value);
+	}
+
+	@Override
+	public String toString() {
+		return type() + " '" + this.value + "'";
+
+	}
+}

spring-context/src/main/java/org/springframework/scheduling/support/CronField.java

@@ -20,8 +20,10 @@ import java.time.DateTimeException;
 import java.time.temporal.ChronoField;
 import java.time.temporal.Temporal;
 import java.time.temporal.ValueRange;
+import java.util.function.BiFunction;
 
 import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 
 /**
@@ -77,11 +79,18 @@ abstract class CronField {
 	 * Parse the given value into a days of months {@code CronField}, the fourth entry of a cron expression.
 	 */
 	public static CronField parseDaysOfMonth(String value) {
-		if (value.contains("L") || value.contains("W")) {
-			return QuartzCronField.parseDaysOfMonth(value);
+		if (!QuartzCronField.isQuartzDaysOfMonthField(value)) {
+			return BitsCronField.parseDaysOfMonth(value);
 		}
 		else {
-			return BitsCronField.parseDaysOfMonth(value);
+			return parseList(value, Type.DAY_OF_MONTH, (field, type) -> {
+				if (QuartzCronField.isQuartzDaysOfMonthField(field)) {
+					return QuartzCronField.parseDaysOfMonth(field);
+				}
+				else {
+					return BitsCronField.parseDaysOfMonth(field);
+				}
+			});
 		}
 	}
 
@@ -98,15 +107,32 @@ abstract class CronField {
 	 */
 	public static CronField parseDaysOfWeek(String value) {
 		value = replaceOrdinals(value, DAYS);
-		if (value.contains("L") || value.contains("#")) {
-			return QuartzCronField.parseDaysOfWeek(value);
+		if (!QuartzCronField.isQuartzDaysOfWeekField(value)) {
+			return BitsCronField.parseDaysOfWeek(value);
 		}
 		else {
-			return BitsCronField.parseDaysOfWeek(value);
+			return parseList(value, Type.DAY_OF_WEEK, (field, type) -> {
+				if (QuartzCronField.isQuartzDaysOfWeekField(field)) {
+					return QuartzCronField.parseDaysOfWeek(field);
+				}
+				else {
+					return BitsCronField.parseDaysOfWeek(field);
+				}
+			});
 		}
 	}
 
 
+	private static CronField parseList(String value, Type type, BiFunction<String, Type, CronField> parseFieldFunction) {
+		Assert.hasLength(value, "Value must not be empty");
+		String[] fields = StringUtils.delimitedListToStringArray(value, ",");
+		CronField[] cronFields = new CronField[fields.length];
+		for (int i = 0; i < fields.length; i++) {
+			cronFields[i] = parseFieldFunction.apply(fields[i], type);
+		}
+		return CompositeCronField.compose(cronFields, type, value);
+	}
+
 	private static String replaceOrdinals(String value, String[] list) {
 		value = value.toUpperCase();
 		for (int i = 0; i < list.length; i++) {

spring-context/src/main/java/org/springframework/scheduling/support/QuartzCronField.java

@@ -78,6 +78,12 @@ final class QuartzCronField extends CronField {
 		this.rollForwardType = rollForwardType;
 	}
 
+	/**
+	 * Returns whether the given value is a Quartz day-of-month field.
+	 */
+	public static boolean isQuartzDaysOfMonthField(String value) {
+		return value.contains("L") || value.contains("W");
+	}
 
 	/**
 	 * Parse the given value into a days of months {@code QuartzCronField}, the fourth entry of a cron expression.
@@ -125,6 +131,13 @@ final class QuartzCronField extends CronField {
 		throw new IllegalArgumentException("No 'L' or 'W' found in '" + value + "'");
 	}
 
+	/**
+	 * Returns 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.

spring-context/src/test/java/org/springframework/scheduling/support/CronExpressionTests.java

@@ -31,6 +31,7 @@ import org.junit.jupiter.api.Test;
 import static java.time.DayOfWeek.FRIDAY;
 import static java.time.DayOfWeek.MONDAY;
 import static java.time.DayOfWeek.SUNDAY;
+import static java.time.DayOfWeek.THURSDAY;
 import static java.time.DayOfWeek.TUESDAY;
 import static java.time.DayOfWeek.WEDNESDAY;
 import static java.time.temporal.TemporalAdjusters.next;
@@ -116,6 +117,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.now().withMinute(10);
 		LocalDateTime expected = last.plusMinutes(1).withSecond(0).withNano(0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -149,6 +151,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.of(year, 10, 30, 11, 1);
 		LocalDateTime expected = last.withHour(12).withMinute(0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -173,6 +176,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.now().withDayOfMonth(1);
 		LocalDateTime expected = last.plusDays(1).withHour(0).withMinute(0).withSecond(0).withNano(0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -207,6 +211,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.now().withMonth(9).withDayOfMonth(30);
 		LocalDateTime expected = LocalDateTime.of(last.getYear(), 10, 1, 0, 0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -222,6 +227,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.now().withMonth(8).withDayOfMonth(30);
 		LocalDateTime expected = last.plusDays(1).withHour(0).withMinute(0).withSecond(0).withNano(0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -237,6 +243,7 @@ class CronExpressionTests {
 		ZonedDateTime last = ZonedDateTime.now(ZoneId.of("CET")).withMonth(10).withDayOfMonth(30);
 		ZonedDateTime expected = last.withDayOfMonth(31).withHour(0).withMinute(0).withSecond(0).withNano(0);
 		ZonedDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -251,6 +258,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.now().withMonth(10).withDayOfMonth(30);
 		LocalDateTime expected = LocalDateTime.of(last.getYear(), 11, 1, 0, 0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -265,6 +273,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.now().withYear(2010).withMonth(12).withDayOfMonth(31);
 		LocalDateTime expected = LocalDateTime.of(2011, 1, 1, 0, 0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -297,6 +306,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.now().with(next(MONDAY));
 		LocalDateTime expected = last.plusDays(1).withHour(0).withMinute(0).withSecond(0).withNano(0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 		assertThat(actual.getDayOfWeek()).isEqualTo(TUESDAY);
 	}
@@ -308,6 +318,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.now().with(next(WEDNESDAY));
 		LocalDateTime expected = last.plusDays(6).withHour(0).withMinute(0).withSecond(0).withNano(0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 		assertThat(actual.getDayOfWeek()).isEqualTo(TUESDAY);
 	}
@@ -319,6 +330,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.now().withMinute(4).withSecond(54);
 		LocalDateTime expected = last.plusMinutes(1).withSecond(55).withNano(0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -333,6 +345,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.now().withHour(9).withSecond(54);
 		LocalDateTime expected = last.plusHours(1).withMinute(0).withSecond(55).withNano(0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -347,6 +360,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.now().withHour(9).withMinute(4);
 		LocalDateTime expected = last.plusHours(1).plusMinutes(1).withSecond(0).withNano(0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -362,6 +376,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.now().withDayOfMonth(2).withSecond(54);
 		LocalDateTime expected = last.plusDays(1).withHour(0).withMinute(0).withSecond(55).withNano(0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -376,6 +391,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.now().withMonth(10).withDayOfMonth(2);
 		LocalDateTime expected = LocalDateTime.of(last.getYear(), 11, 3, 0, 0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -398,6 +414,7 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.now().withYear(2007).withMonth(2).withDayOfMonth(10);
 		LocalDateTime expected = LocalDateTime.of(2008, 2, 29, 0, 0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -413,12 +430,14 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.of(LocalDate.of(2009, 9, 26), LocalTime.now());
 		LocalDateTime expected = last.plusDays(2).withHour(7).withMinute(0).withSecond(0).withNano(0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		// Next day is a week day so add one
 		last = actual;
 		expected = expected.plusDays(1);
 		actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -433,12 +452,14 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.of(LocalDate.of(2010, 12, 30), LocalTime.now());
 		LocalDateTime expected = last.plusMonths(1).withHour(23).withMinute(30).withSecond(0).withSecond(0).withNano(0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		// Next trigger is 3 months later
 		last = actual;
 		expected = expected.plusMonths(3);
 		actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -485,11 +506,13 @@ class CronExpressionTests {
 		LocalDateTime expected = LocalDateTime.of(last.getYear() + 1, 1, 1, 0, 0);
 
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
 		expected = expected.plusYears(1);
 		actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -513,11 +536,13 @@ class CronExpressionTests {
 		LocalDateTime expected = LocalDateTime.of(last.getYear(), 11, 1, 0, 0);
 
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
 		expected = expected.plusMonths(1);
 		actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -534,11 +559,13 @@ class CronExpressionTests {
 		LocalDateTime expected = last.with(next(SUNDAY)).withHour(0).withMinute(0).withSecond(0).withNano(0);
 
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
 		expected = expected.plusWeeks(1);
 		actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -555,11 +582,13 @@ class CronExpressionTests {
 		LocalDateTime expected = last.plusDays(1).withHour(0).withMinute(0).withSecond(0).withNano(0);
 
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
 		expected = expected.plusDays(1);
 		actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -583,11 +612,13 @@ class CronExpressionTests {
 		LocalDateTime expected = last.plusHours(1).withMinute(0).withSecond(0).withNano(0);
 
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
 		expected = expected.plusHours(1);
 		actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -603,16 +634,19 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.of(LocalDate.of(2008, 1, 4), LocalTime.now());
 		LocalDateTime expected = LocalDateTime.of(2008, 1, 31, 0, 0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
 		expected = LocalDateTime.of(2008, 2, 29, 0, 0);
 		actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
 		expected = LocalDateTime.of(2008, 3, 31, 0, 0);
 		actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -628,16 +662,19 @@ class CronExpressionTests {
 		LocalDateTime last = LocalDateTime.of(LocalDate.of(2008, 1, 4), LocalTime.now());
 		LocalDateTime expected = LocalDateTime.of(2008, 1, 28, 0, 0);
 		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
 		expected = LocalDateTime.of(2008, 2, 26, 0, 0);
 		actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
 		expected = LocalDateTime.of(2008, 3, 28, 0, 0);
 		actual = expression.next(last);
+		assertThat(actual).isNotNull();
 		assertThat(actual).isEqualTo(expected);
 
 		last = actual;
@@ -1042,4 +1079,62 @@ class CronExpressionTests {
 		assertThat(actual).isEqualTo(expected);
 		assertThat(actual.getDayOfWeek()).isEqualTo(WEDNESDAY);
 	}
+
+	@Test
+	void dayOfMonthListWithQuartz() {
+		CronExpression expression = CronExpression.parse("0 0 0 1W,15,LW * ?");
+
+		LocalDateTime last = LocalDateTime.of(2019, 12, 30, 0, 0);
+		LocalDateTime expected = LocalDateTime.of(2019, 12, 31, 0, 0);
+		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
+		assertThat(actual).isEqualTo(expected);
+		assertThat(actual).is(weekday);
+
+		last = actual;
+		expected = LocalDateTime.of(2020, 1, 1, 0, 0);
+		actual = expression.next(last);
+		assertThat(actual).isNotNull();
+		assertThat(actual).isEqualTo(expected);
+		assertThat(actual).is(weekday);
+
+		last = actual;
+		expected = LocalDateTime.of(2020, 1, 15, 0, 0);
+		actual = expression.next(last);
+		assertThat(actual).isNotNull();
+		assertThat(actual).isEqualTo(expected);
+
+		last = actual;
+		expected = LocalDateTime.of(2020, 1, 31, 0, 0);
+		actual = expression.next(last);
+		assertThat(actual).isNotNull();
+		assertThat(actual).isEqualTo(expected);
+		assertThat(actual).is(weekday);
+	}
+
+	@Test
+	void dayOfWeekListWithQuartz() {
+		CronExpression expression = CronExpression.parse("0 0 0 ? * THU#1,THU#3,THU#5");
+
+		LocalDateTime last = LocalDateTime.of(2019, 12, 31, 0, 0);
+		LocalDateTime expected = LocalDateTime.of(2020, 1, 2, 0, 0);
+		LocalDateTime actual = expression.next(last);
+		assertThat(actual).isNotNull();
+		assertThat(actual).isEqualTo(expected);
+		assertThat(actual.getDayOfWeek()).isEqualTo(THURSDAY);
+
+		last = actual;
+		expected = LocalDateTime.of(2020, 1, 16, 0, 0);
+		actual = expression.next(last);
+		assertThat(actual).isNotNull();
+		assertThat(actual).isEqualTo(expected);
+		assertThat(actual.getDayOfWeek()).isEqualTo(THURSDAY);
+
+		last = actual;
+		expected = LocalDateTime.of(2020, 1, 30, 0, 0);
+		actual = expression.next(last);
+		assertThat(actual).isNotNull();
+		assertThat(actual).isEqualTo(expected);
+		assertThat(actual.getDayOfWeek()).isEqualTo(THURSDAY);
+	}
 }