From ea8f3b2d3d2aec1c5bc10df32b070f9b5cb120be Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:51:18 +0700 Subject: [PATCH] fix: wrap month start/interval across year (#692) EveryFieldValueGenerator treated month "10/3" as only month 10 (from=start, to=12), so ExecutionTime skipped Jan/Apr/Jul. For MONTH + On/period, match months with year wrap via floorMod so 10/3 yields Oct, Jan, Apr, Jul. Non-month fields unchanged. Fixes #692 --- .../generator/EveryFieldValueGenerator.java | 79 ++++++++++++++-- src/test/java/com/cronutils/Issue692Test.java | 94 +++++++++++++++++++ .../EveryFieldValueGeneratorTest.java | 58 ++++++++++++ 3 files changed, 225 insertions(+), 6 deletions(-) create mode 100644 src/test/java/com/cronutils/Issue692Test.java diff --git a/src/main/java/com/cronutils/model/time/generator/EveryFieldValueGenerator.java b/src/main/java/com/cronutils/model/time/generator/EveryFieldValueGenerator.java index aed19c73..dabfecff 100644 --- a/src/main/java/com/cronutils/model/time/generator/EveryFieldValueGenerator.java +++ b/src/main/java/com/cronutils/model/time/generator/EveryFieldValueGenerator.java @@ -14,6 +14,7 @@ package com.cronutils.model.time.generator; import com.cronutils.model.field.CronField; +import com.cronutils.model.field.CronFieldName; import com.cronutils.model.field.expression.Between; import com.cronutils.model.field.expression.Every; import com.cronutils.model.field.expression.FieldExpression; @@ -27,6 +28,12 @@ class EveryFieldValueGenerator extends FieldValueGenerator { protected final int from; protected final int to; + /** + * When true, {@code start/period} on the month field wraps within the year + * (e.g. {@code 10/3} → Oct, Jan, Apr, Jul). Matches continuous "every N months + * starting at month M" semantics; see issue #692 and {@link Every} javadoc. + */ + private final boolean wrapMonths; public EveryFieldValueGenerator(final CronField cronField) { super(cronField); @@ -38,20 +45,32 @@ public EveryFieldValueGenerator(final CronField cronField) { from = Math.max(cronField.getConstraints().getStartRange(), BetweenFieldValueGenerator.map(between.getFrom())); to = Math.min(cronField.getConstraints().getEndRange(), BetweenFieldValueGenerator.map(between.getTo())); - } else if(everyExpression instanceof On){ - + wrapMonths = false; + } else if (everyExpression instanceof On) { final On on = (On) everyExpression; - - from = on.getTime().getValue(); - to = cronField.getConstraints().getEndRange(); + // Month intervals wrap the calendar year so "10/3" is every 3 months + // from October (1,4,7,10), not only month 10. + if (CronFieldName.MONTH.equals(cronField.getField())) { + from = cronField.getConstraints().getStartRange(); + to = cronField.getConstraints().getEndRange(); + wrapMonths = true; + } else { + from = on.getTime().getValue(); + to = cronField.getConstraints().getEndRange(); + wrapMonths = false; + } } else { from = cronField.getConstraints().getStartRange(); to = cronField.getConstraints().getEndRange(); + wrapMonths = false; } } @Override public int generateNextValue(final int reference) throws NoSuchValueException { + if (wrapMonths) { + return generateNextValueWrapping(reference); + } //intuition: for valid values, we have: offset+period*i if (reference >= to) { throw new NoSuchValueException(); @@ -69,6 +88,17 @@ public int generateNextValue(final int reference) throws NoSuchValueException { return next; } + private int generateNextValueWrapping(final int reference) throws NoSuchValueException { + final int period = ((Every) cronField.getExpression()).getPeriod().getValue(); + final int offset = offset(); + for (int candidate = reference + 1; candidate <= to; candidate++) { + if (matchesWrapped(candidate, offset, period)) { + return candidate; + } + } + throw new NoSuchValueException(); + } + private int getNext(int reference, Every every) { final int offset = offset(); @@ -85,6 +115,9 @@ private int getNext(int reference, Every every) { @Override public int generatePreviousValue(final int reference) throws NoSuchValueException { + if (wrapMonths) { + return generatePreviousValueWrapping(reference); + } final Every every = (Every) cronField.getExpression(); if (reference < from) { throw new NoSuchValueException(); @@ -101,10 +134,29 @@ public int generatePreviousValue(final int reference) throws NoSuchValueExceptio } } + private int generatePreviousValueWrapping(final int reference) throws NoSuchValueException { + final int period = ((Every) cronField.getExpression()).getPeriod().getValue(); + final int offset = offset(); + for (int candidate = reference - 1; candidate >= from; candidate--) { + if (matchesWrapped(candidate, offset, period)) { + return candidate; + } + } + throw new NoSuchValueException(); + } + @Override protected List generateCandidatesNotIncludingIntervalExtremes(final int start, final int end) { final List values = new ArrayList<>(); try { + if (wrapMonths) { + int reference = generateNextValue(start); + while (reference < end) { + values.add(reference); + reference = generateNextValue(reference); + } + return values; + } final int offset = offset(); if (start < offset && offset < end) { values.add(offset); @@ -125,8 +177,23 @@ protected List generateCandidatesNotIncludingIntervalExtremes(final int @Override public boolean isMatch(final int value) { final Every every = (Every) cronField.getExpression(); + final int period = every.getPeriod().getValue(); + if (wrapMonths) { + return value >= from && value <= to && matchesWrapped(value, offset(), period); + } final int start = offset(); - return value >= start && ((value - start) % every.getPeriod().getValue()) == 0 && value >= from && value <= to; + return value >= start && ((value - start) % period) == 0 && value >= from && value <= to; + } + + /** + * Month M matches start S with period P when M is on the arithmetic progression + * S, S±P, S±2P, … projected onto the calendar year (1–12). + */ + private static boolean matchesWrapped(final int value, final int offset, final int period) { + if (period <= 0) { + return false; + } + return Math.floorMod(value - offset, period) == 0; } @Override diff --git a/src/test/java/com/cronutils/Issue692Test.java b/src/test/java/com/cronutils/Issue692Test.java new file mode 100644 index 00000000..944cc8fc --- /dev/null +++ b/src/test/java/com/cronutils/Issue692Test.java @@ -0,0 +1,94 @@ +package com.cronutils; + +import com.cronutils.model.CronType; +import com.cronutils.model.definition.CronDefinitionBuilder; +import com.cronutils.model.time.ExecutionTime; +import com.cronutils.parser.CronParser; +import org.junit.jupiter.api.Test; + +import java.time.LocalDate; +import java.time.ZonedDateTime; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Issue #692: ExecutionTime incorrectly skips month intervals (e.g. "0 0 0 15 10/3 ? *"). + * "start/interval" on months must include year-wrapped months (Oct + every 3 months → Oct, Jan, Apr, Jul). + */ +public class Issue692Test { + + private final CronParser cronParser = + new CronParser(CronDefinitionBuilder.instanceDefinitionFor(CronType.QUARTZ)); + + @Test + public void nextExecutionForMonthInterval() { + // every 3 months starting in October + final ExecutionTime executionTime = ExecutionTime.forCron(cronParser.parse("0 0 0 * 10/3 ? *")); + final ZonedDateTime referenceDate = ZonedDateTime.parse("2026-02-01T00:00:00Z"); + final Optional next = executionTime.nextExecution(referenceDate); + assertTrue(next.isPresent()); + assertEquals(LocalDate.of(2026, 4, 1), next.get().toLocalDate()); + } + + @Test + public void nextExecutionForMonthIntervalWithSpecificDayOfMonth() { + final ExecutionTime executionTime = ExecutionTime.forCron(cronParser.parse("0 0 0 15 10/3 ? *")); + final ZonedDateTime referenceDate = ZonedDateTime.parse("2026-01-01T00:00:00Z"); + final Optional next = executionTime.nextExecution(referenceDate); + assertTrue(next.isPresent()); + assertEquals(LocalDate.of(2026, 1, 15), next.get().toLocalDate()); + } + + @Test + public void executionDatesForMonthInterval() { + final ExecutionTime executionTime = ExecutionTime.forCron(cronParser.parse("0 0 0 15 10/3 ? *")); + final ZonedDateTime startDate = ZonedDateTime.parse("2025-10-01T00:00:00Z"); + final ZonedDateTime endDate = ZonedDateTime.parse("2026-10-31T00:00:00Z"); + final List expected = Arrays.asList( + LocalDate.of(2025, 10, 15), + LocalDate.of(2026, 1, 15), + LocalDate.of(2026, 4, 15), + LocalDate.of(2026, 7, 15), + LocalDate.of(2026, 10, 15) + ); + final List actual = executionTime.getExecutionDates(startDate, endDate).stream() + .map(ZonedDateTime::toLocalDate) + .collect(Collectors.toList()); + assertEquals(expected, actual); + } + + @Test + public void previousExecutionForMonthInterval() { + final ExecutionTime executionTime = ExecutionTime.forCron(cronParser.parse("0 0 0 15 10/3 ? *")); + final Optional previous = + executionTime.lastExecution(ZonedDateTime.parse("2026-02-01T00:00:00Z")); + assertTrue(previous.isPresent()); + assertEquals(LocalDate.of(2026, 1, 15), previous.get().toLocalDate()); + } + + @Test + public void nextExecutionSequenceEveryThreeMonthsFromOctober() { + final ExecutionTime executionTime = ExecutionTime.forCron(cronParser.parse("0 0 0 1 10/3 ? *")); + ZonedDateTime current = ZonedDateTime.parse("2025-09-01T00:00:00Z"); + final List actual = new ArrayList<>(); + for (int i = 0; i < 5; i++) { + final Optional next = executionTime.nextExecution(current); + assertTrue(next.isPresent(), "expected next after " + current); + actual.add(next.get().toLocalDate().toString()); + current = next.get(); + } + assertEquals(Arrays.asList( + "2025-10-01", + "2026-01-01", + "2026-04-01", + "2026-07-01", + "2026-10-01" + ), actual); + } +} diff --git a/src/test/java/com/cronutils/model/time/generator/EveryFieldValueGeneratorTest.java b/src/test/java/com/cronutils/model/time/generator/EveryFieldValueGeneratorTest.java index 2180352b..862f4978 100755 --- a/src/test/java/com/cronutils/model/time/generator/EveryFieldValueGeneratorTest.java +++ b/src/test/java/com/cronutils/model/time/generator/EveryFieldValueGeneratorTest.java @@ -19,10 +19,13 @@ import com.cronutils.model.field.constraint.FieldConstraintsBuilder; import com.cronutils.model.field.expression.Every; import com.cronutils.model.field.expression.FieldExpression; +import com.cronutils.model.field.expression.On; import com.cronutils.model.field.value.IntegerFieldValue; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Random; @@ -82,4 +85,59 @@ public void testMatchesFieldExpressionClass() { public void testConstructorNotMatchesEvery() { assertThrows(IllegalArgumentException.class, () -> new EveryFieldValueGenerator(new CronField(CronFieldName.HOUR, mock(FieldExpression.class), constraints))); } + + @Test + public void testMonthIntervalWrapsYearForStartSlashPeriod() { + // 10/3 → months Oct, Jan, Apr, Jul (issue #692) + final FieldConstraints monthConstraints = FieldConstraintsBuilder.instance() + .forField(CronFieldName.MONTH) + .createConstraintsInstance(); + final Every every = new Every(new On(new IntegerFieldValue(10)), new IntegerFieldValue(3)); + final EveryFieldValueGenerator months = new EveryFieldValueGenerator( + new CronField(CronFieldName.MONTH, every, monthConstraints)); + + assertEquals(Arrays.asList(1, 4, 7, 10), months.generateCandidates(1, 12)); + assertTrue(months.isMatch(1)); + assertTrue(months.isMatch(4)); + assertTrue(months.isMatch(7)); + assertTrue(months.isMatch(10)); + assertFalse(months.isMatch(2)); + assertFalse(months.isMatch(11)); + } + + @Test + public void testMonthIntervalNextAndPreviousWrap() throws NoSuchValueException { + final FieldConstraints monthConstraints = FieldConstraintsBuilder.instance() + .forField(CronFieldName.MONTH) + .createConstraintsInstance(); + final Every every = new Every(new On(new IntegerFieldValue(10)), new IntegerFieldValue(3)); + final EveryFieldValueGenerator months = new EveryFieldValueGenerator( + new CronField(CronFieldName.MONTH, every, monthConstraints)); + + assertEquals(1, months.generateNextValue(0)); + assertEquals(4, months.generateNextValue(1)); + assertEquals(4, months.generateNextValue(2)); + assertEquals(10, months.generateNextValue(7)); + assertThrows(NoSuchValueException.class, () -> months.generateNextValue(10)); + + assertEquals(10, months.generatePreviousValue(11)); + assertEquals(7, months.generatePreviousValue(10)); + assertEquals(1, months.generatePreviousValue(4)); + assertThrows(NoSuchValueException.class, () -> months.generatePreviousValue(1)); + } + + @Test + public void testNonMonthOnIntervalDoesNotWrap() { + // hours 22/3 must remain 22 only (no wrap into early hours) + final FieldConstraints hourConstraints = FieldConstraintsBuilder.instance() + .forField(CronFieldName.HOUR) + .createConstraintsInstance(); + final Every every = new Every(new On(new IntegerFieldValue(22)), new IntegerFieldValue(3)); + final EveryFieldValueGenerator hours = new EveryFieldValueGenerator( + new CronField(CronFieldName.HOUR, every, hourConstraints)); + + assertEquals(Collections.singletonList(22), hours.generateCandidates(0, 23)); + assertTrue(hours.isMatch(22)); + assertFalse(hours.isMatch(1)); + } }