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))); + } +}