From 68dea047601ecdb72d81c0858fd0b578ae336d5d Mon Sep 17 00:00:00 2001 From: austek <13117393+austek@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:28:05 +0100 Subject: [PATCH] Name the position of an nth day of week "0 59 10 ? 1/2 MON#1 *" read "at 10:59 every 2 months from month 1 Monday 1 of every month". The nth day of week printed its position as a bare number, and claimed "of every month" right after the month field had restricted it to every second month. Give the # and L forms their own phrasing, so they read "on the first Monday of the month" and "on the last Friday of the month" and no longer contradict a restricted month. Positions past the fifth, outside the Quartz range but reachable through the generic descriptor, fall back to the plain number. Field order is untouched, so the day of week still follows the month rather than leading the sentence as the report suggests. Fixes #126 --- .../DescriptionStrategyFactory.java | 14 ++++- .../com/cronutils/CronUtilsI18N.properties | 7 +++ .../utils/descriptor/CronDescriptorTest.java | 4 +- .../utils/descriptor/Issue126Test.java | 56 +++++++++++++++++++ 4 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 src/test/java/com/cronutils/utils/descriptor/Issue126Test.java diff --git a/src/main/java/com/cronutils/descriptor/DescriptionStrategyFactory.java b/src/main/java/com/cronutils/descriptor/DescriptionStrategyFactory.java index 61920609..3b31d7d8 100755 --- a/src/main/java/com/cronutils/descriptor/DescriptionStrategyFactory.java +++ b/src/main/java/com/cronutils/descriptor/DescriptionStrategyFactory.java @@ -31,6 +31,15 @@ class DescriptionStrategyFactory { private DescriptionStrategyFactory() { } + /** + * Names the position an nth day of week occupies, falling back to the plain number for + * positions no bundle spells out. + */ + private static String ordinal(final int nth, final ResourceBundle bundle) { + final String key = "nth_" + nth; + return bundle.containsKey(key) ? bundle.getString(key) : String.valueOf(nth); + } + /** * Creates description strategy for days of week. * @@ -54,9 +63,10 @@ public static DescriptionStrategy daysOfWeekInstance(final ResourceBundle bundle final On on = (On) fieldExpression; switch (on.getSpecialChar().getValue()) { case HASH: - return String.format("%s %s %s ", nominal.apply(on.getTime().getValue()), on.getNth(), bundle.getString("of_every_month")); + return MessageFormat.format(bundle.getString("on_nth_day_of_week_x"), + ordinal(on.getNth().getValue(), bundle), nominal.apply(on.getTime().getValue())); case L: - return String.format("%s %s %s ", bundle.getString("last"), nominal.apply(on.getTime().getValue()), bundle.getString("of_every_month")); + return MessageFormat.format(bundle.getString("on_last_day_of_week_x"), nominal.apply(on.getTime().getValue())); default: return ""; } diff --git a/src/main/resources/com/cronutils/CronUtilsI18N.properties b/src/main/resources/com/cronutils/CronUtilsI18N.properties index 027f0089..9c349351 100644 --- a/src/main/resources/com/cronutils/CronUtilsI18N.properties +++ b/src/main/resources/com/cronutils/CronUtilsI18N.properties @@ -16,6 +16,13 @@ months=months year=year years=years between=between +on_nth_day_of_week_x=on the {0} {1} of the month +on_last_day_of_week_x=on the last {0} of the month +nth_1=first +nth_2=second +nth_3=third +nth_4=fourth +nth_5=fifth of_every_month=of every month of_the_month=of the month last=last diff --git a/src/test/java/com/cronutils/utils/descriptor/CronDescriptorTest.java b/src/test/java/com/cronutils/utils/descriptor/CronDescriptorTest.java index 4cc60f79..571a6b79 100755 --- a/src/test/java/com/cronutils/utils/descriptor/CronDescriptorTest.java +++ b/src/test/java/com/cronutils/utils/descriptor/CronDescriptorTest.java @@ -148,7 +148,7 @@ public void testLastDayOfWeekInMonth() { results.add(new CronField(CronFieldName.MINUTE, new On(new IntegerFieldValue(minute)), nullFieldConstraints)); results.add(new CronField(CronFieldName.DAY_OF_WEEK, new On(new IntegerFieldValue(dayOfWeek), new SpecialCharFieldValue(SpecialChar.L)), nullFieldConstraints)); - assertEquals(String.format("at %s:%s last Tuesday of every month", hour, minute), descriptor.describe(new SingleCron(mockDefinition, results))); + assertEquals(String.format("at %s:%s on the last Tuesday of the month", hour, minute), descriptor.describe(new SingleCron(mockDefinition, results))); } @Test @@ -161,7 +161,7 @@ public void testNthDayOfWeekInMonth() { results.add(new CronField(CronFieldName.MINUTE, new On(new IntegerFieldValue(minute)), nullFieldConstraints)); results.add(new CronField(CronFieldName.DAY_OF_WEEK, new On(new IntegerFieldValue(dayOfWeek), new SpecialCharFieldValue(SpecialChar.HASH), new IntegerFieldValue(dayOfWeek)), nullFieldConstraints)); - assertEquals(String.format("at %s:%s Tuesday %s of every month", hour, minute, dayOfWeek), descriptor.describe(new SingleCron(mockDefinition, results))); + assertEquals(String.format("at %s:%s on the second Tuesday of the month", hour, minute), descriptor.describe(new SingleCron(mockDefinition, results))); } @Test diff --git a/src/test/java/com/cronutils/utils/descriptor/Issue126Test.java b/src/test/java/com/cronutils/utils/descriptor/Issue126Test.java new file mode 100644 index 00000000..6c9aea3c --- /dev/null +++ b/src/test/java/com/cronutils/utils/descriptor/Issue126Test.java @@ -0,0 +1,56 @@ +package com.cronutils.utils.descriptor; + +import com.cronutils.descriptor.CronDescriptor; +import com.cronutils.model.CronType; +import com.cronutils.model.definition.CronDefinitionBuilder; +import com.cronutils.parser.CronParser; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +import java.util.Locale; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Issue 126 - "0 59 10 ? 1/2 MON#1 *" read as "at 10:59 every February months Monday 1 of every + * month". The nth day of week was spelled out as a bare number and claimed to happen "of every + * month", contradicting the month field that had just restricted it. + */ +public class Issue126Test { + + private final CronParser parser = new CronParser(CronDefinitionBuilder.instanceDefinitionFor(CronType.QUARTZ)); + + @ParameterizedTest + @CsvSource({ + "'0 59 10 ? * MON#1 *', 'at 10:59 on the first Monday of the month'", + "'0 0 0 ? * TUE#2', 'at 00:00 on the second Tuesday of the month'", + "'0 0 0 ? * WED#3', 'at 00:00 on the third Wednesday of the month'", + "'0 0 0 ? * THU#4', 'at 00:00 on the fourth Thursday of the month'", + "'0 0 0 ? * FRI#5', 'at 00:00 on the fifth Friday of the month'" + }) + public void nthDayOfWeekNamesItsPosition(String expression, String expected) { + assertEquals(expected, CronDescriptor.instance(Locale.ENGLISH).describe(parser.parse(expression))); + } + + @ParameterizedTest + @CsvSource({ + "'0 0 0 ? * MONL', 'at 00:00 on the last Monday of the month'", + "'0 0 0 ? * 6L', 'at 00:00 on the last Friday of the month'" + }) + public void lastDayOfWeekReadsTheSameWay(String expression, String expected) { + assertEquals(expected, CronDescriptor.instance(Locale.ENGLISH).describe(parser.parse(expression))); + } + + /** + * The expression from the report: a restricted month must not be followed by a day of week + * claiming every month. + */ + @ParameterizedTest + @CsvSource({ + "'0 59 10 ? 1/2 MON#1 *', 'at 10:59 every 2 months from month 1 on the first Monday of the month'", + "'0 59 10 ? 3 MON#1 *', 'at 10:59 at March month on the first Monday of the month'" + }) + public void aRestrictedMonthIsNotContradicted(String expression, String expected) { + assertEquals(expected, CronDescriptor.instance(Locale.ENGLISH).describe(parser.parse(expression))); + } +}