Repository: jmrozanec/cron-utils
Pull request #356 (2018-10-28 16:08:11)

#336 - Fix Every Day Of Week handling
Fixes the day of week handling for the "every" expression.

The testCorrectNextExecutionDoW test in ExecutionTimeUnixIntegrationTest was also expecting the wrong next execution time value so I fixed that test.


Patch:
diff --git a/src/main/java/com/cronutils/model/time/generator/EveryDayOfWeekValueGenerator.java b/src/main/java/com/cronutils/model/time/generator/EveryDayOfWeekValueGenerator.java
new file mode 100644
index 0000000..8fa05db
--- /dev/null
+++ b/src/main/java/com/cronutils/model/time/generator/EveryDayOfWeekValueGenerator.java
@@ -0,0 +1,117 @@
+/*
+ * Copyright 2015 jmrozanec
+ * 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
+ * http://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 com.cronutils.model.time.generator;
+
+import java.time.DayOfWeek;
+import java.time.LocalDate;
+import java.util.ArrayList;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Set;
+
+import com.cronutils.mapper.ConstantsMapper;
+import com.cronutils.mapper.WeekDay;
+import com.cronutils.model.field.CronField;
+import com.cronutils.model.field.CronFieldName;
+import com.cronutils.model.field.expression.Every;
+import com.cronutils.utils.Preconditions;
+
+/**
+ * Generates matching days for a given year and month for a given day of week cron field with an every expression.
+ */
+class EveryDayOfWeekValueGenerator extends EveryFieldValueGenerator {
+    private final int lastDayOfMonth;
+    private int year;
+    private int month;
+    private final Set<DayOfWeek> dowValidValues;
+
+    EveryDayOfWeekValueGenerator(final CronField cronField, final int year, final int month, WeekDay mondayDoWValue) {
+        super(cronField);
+        Preconditions.checkArgument(CronFieldName.DAY_OF_WEEK.equals(cronField.getField()), "CronField does not belong to day of week");
+        this.year = year;
+        this.month = month;
+        final LocalDate date = LocalDate.of(year, month, 1);
+        lastDayOfMonth = date.lengthOfMonth();
+
+        // from is set by EveryFieldValueGenerator to be the first day of the week to start counting from
+        // and to is set by EveryFieldValueGenerator to be the last day of the week
+        // in the case of from-to/period (ex. MON-FRI/2)
+        final Every every = (Every) cronField.getExpression();
+        int period = every.getPeriod().getValue();
+        Preconditions.checkArgument(period > 0 && period < 8, "Cron Expression for day of week has an invalid period.");
+        dowValidValues = getValidDays(mondayDoWValue, period, from, to);
+    }
+
+    private static Set<DayOfWeek> getValidDays(WeekDay mondayDoWValue, int period, int from, int to) {
+        List<DayOfWeek> validDays = new ArrayList<>(7);
+        for (int day = from; day <= to; day += period) {
+            // Convert from cron day of the week to Java DayOfWeek
+            int javaDay = ConstantsMapper.weekDayMapping(mondayDoWValue, ConstantsMapper.JAVA8, day);
+            validDays.add(DayOfWeek.of(javaDay));
+        }
+        return EnumSet.copyOf(validDays);
+    }
+
+    @Override
+    public int generateNextValue(final int reference) throws NoSuchValueException {
+        int day = reference;
+        do {
+            day++;
+        } while (!isMatch(day) && day <= lastDayOfMonth);
+        if (day > lastDayOfMonth) {
+            throw new NoSuchValueException();
+        }
+        return day;
+    }
+
+    @Override
+    public int generatePreviousValue(final int reference) throws NoSuchValueException {
+        int day = reference;
+        do {
+            day--;
+        } while (!isMatch(day) && day > 0);
+        if (day <= 0) {
+            throw new NoSuchValueException();
+        }
+        return day;
+    }
+
+    @Override
+    protected List<Integer> generateCandidatesNotIncludingIntervalExtremes(int start, int end) {
+        // start is the day of month to start from
+        List<Integer> candidates = new ArrayList<>();
+        int reference = start;
+        try {
+            while (reference < end) {
+                reference = generateNextValue(reference);
+                if (reference < end) {
+                    candidates.add(reference);
+                }
+            }
+        } catch (NoSuchValueException ignored) {
+            // next generated value would be beyond the end of the month, so just ignore it and finish
+        }
+        return candidates;
+    }
+
+    @Override
+    public boolean isMatch(final int value) {
+        // value is the day of the month
+        if (value > lastDayOfMonth || value < 1) {
+            return false;
+        }
+        DayOfWeek dayOfWeek = LocalDate.of(year, month, value).getDayOfWeek();
+        return dowValidValues.contains(dayOfWeek);
+    }
+}
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 b160824..8678f94 100644
--- a/src/main/java/com/cronutils/model/time/generator/EveryFieldValueGenerator.java
+++ b/src/main/java/com/cronutils/model/time/generator/EveryFieldValueGenerator.java
@@ -25,8 +25,8 @@ import com.cronutils.utils.VisibleForTesting;
 
 class EveryFieldValueGenerator extends FieldValueGenerator {
 
-    private final int from;
-    private final int to;
+    protected final int from;
+    protected final int to;
 
     public EveryFieldValueGenerator(final CronField cronField) {
         super(cronField);
diff --git a/src/main/java/com/cronutils/model/time/generator/FieldValueGeneratorFactory.java b/src/main/java/com/cronutils/model/time/generator/FieldValueGeneratorFactory.java
index 060eeaa..4dc5e72 100644
--- a/src/main/java/com/cronutils/model/time/generator/FieldValueGeneratorFactory.java
+++ b/src/main/java/com/cronutils/model/time/generator/FieldValueGeneratorFactory.java
@@ -79,6 +79,9 @@ public class FieldValueGeneratorFactory {
         if (fieldExpression instanceof And) {
             return new AndDayOfWeekValueGenerator(cronField, year, month, mondayDoWValue);
         }
+        if (fieldExpression instanceof Every) {
+            return new EveryDayOfWeekValueGenerator(cronField, year, month, mondayDoWValue);
+        }
         return forCronField(cronField);
     }
 


Test patch:
diff --git a/src/test/java/com/cronutils/model/time/generator/EveryDayOfWeekValueGeneratorTest.java b/src/test/java/com/cronutils/model/time/generator/EveryDayOfWeekValueGeneratorTest.java
new file mode 100644
index 0000000..17711c4
--- /dev/null
+++ b/src/test/java/com/cronutils/model/time/generator/EveryDayOfWeekValueGeneratorTest.java
@@ -0,0 +1,115 @@
+/*
+ * Copyright 2015 jmrozanec
+ * 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
+ * http://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 com.cronutils.model.time.generator;
+
+import java.time.DayOfWeek;
+import java.time.LocalDate;
+import java.time.Year;
+import java.util.Arrays;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import com.cronutils.mapper.WeekDay;
+import com.cronutils.model.field.CronField;
+import com.cronutils.model.field.CronFieldName;
+import com.cronutils.model.field.constraint.FieldConstraints;
+import com.cronutils.model.field.constraint.FieldConstraintsBuilder;
+import com.cronutils.model.field.expression.Between;
+import com.cronutils.model.field.expression.Every;
+import com.cronutils.model.field.expression.FieldExpression;
+import com.cronutils.model.field.value.IntegerFieldValue;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+
+public class EveryDayOfWeekValueGeneratorTest {
+    private EveryFieldValueGenerator fieldValueGenerator;
+    private final int year = 2018;
+    private int month = 10;
+    private Set<DayOfWeek> validDow;
+
+    @Before
+    public void setUp() {
+        FieldConstraints constraints = FieldConstraintsBuilder.instance().createConstraintsInstance();
+        // every 2 days between 1-5
+        final CronField cronField = new CronField(CronFieldName.DAY_OF_WEEK,
+                new Every(new Between(new IntegerFieldValue(1), new IntegerFieldValue(5)),
+                        new IntegerFieldValue(2)),
+                constraints);
+        // Using monday = 1
+        WeekDay mondayDoWValue = new WeekDay(1, false);
+        // so MON-FRI/2, which should translate to MON,WED,FRI
+        validDow = EnumSet.of(DayOfWeek.MONDAY, DayOfWeek.WEDNESDAY, DayOfWeek.FRIDAY);
+
+        fieldValueGenerator = new EveryDayOfWeekValueGenerator(cronField, year, month, mondayDoWValue);
+    }
+
+    @Test
+    public void testGenerateNextValue() throws NoSuchValueException {
+        // Oct 1, 2018 is a Monday. generateNextValue given 1 should return 3 (Wednesday Oct. 3, 2018)
+        assertEquals(3, fieldValueGenerator.generateNextValue(1));
+        // Next should be Friday (Oct. 5, 2018)
+        assertEquals(5, fieldValueGenerator.generateNextValue(3));
+        // Next should be Monday (Oct. 8, 2018) because of the restriction 1-5
+        assertEquals(8, fieldValueGenerator.generateNextValue(5));
+    }
+
+    @Test
+    public void testGeneratePreviousValue() throws NoSuchValueException {
+        // Oct. 31, 2018 is a Wednesday, generatePreviousValue given 31 should give Monday 29, 2018
+        assertEquals(29, fieldValueGenerator.generatePreviousValue(31));
+        // Previous week's Friday is Oct. 26, 2018
+        assertEquals(26, fieldValueGenerator.generatePreviousValue(29));
+        assertEquals(24, fieldValueGenerator.generatePreviousValue(26));
+        assertEquals(22, fieldValueGenerator.generatePreviousValue(24));
+    }
+
+    @Test
+    public void testGenerateCandidatesNotIncludingIntervalExtremes() {
+        int lengthOfMonth = LocalDate.of(year, month, 1).getMonth().length(Year.isLeap(year));
+        List<Integer> results = fieldValueGenerator.generateCandidatesNotIncludingIntervalExtremes(1, lengthOfMonth);
+        for (Integer day : results) {
+            assertTrue(validDow.contains(LocalDate.of(year, month, day).getDayOfWeek()));
+        }
+        // should not contain the extremes, 1 and 31
+        assertFalse(results.contains(1));
+        assertFalse(results.contains(lengthOfMonth));
+        assertTrue(results.containsAll(
+                Arrays.asList(
+                        3, 5,
+                        8, 10, 12,
+                        15, 17, 19,
+                        22, 24, 26,
+                        29)));
+    }
+
+    @Test
+    public void testIsMatch() {
+        assertFalse(fieldValueGenerator.isMatch(16));
+        assertTrue(fieldValueGenerator.isMatch(17));
+        assertFalse(fieldValueGenerator.isMatch(18));
+    }
+
+    @Test
+    public void testMatchesFieldExpressionClass() {
+        assertTrue(fieldValueGenerator.matchesFieldExpressionClass(mock(Every.class)));
+        assertFalse(fieldValueGenerator.matchesFieldExpressionClass(mock(FieldExpression.class)));
+    }
+}
diff --git a/src/test/java/com/cronutils/model/time/generator/ExecutionTimeUnixIntegrationTest.java b/src/test/java/com/cronutils/model/time/generator/ExecutionTimeUnixIntegrationTest.java
index 2d68543..85691b8 100755
--- a/src/test/java/com/cronutils/model/time/generator/ExecutionTimeUnixIntegrationTest.java
+++ b/src/test/java/com/cronutils/model/time/generator/ExecutionTimeUnixIntegrationTest.java
@@ -272,8 +272,14 @@ public class ExecutionTimeUnixIntegrationTest {
     @Test
     public void testCorrectNextExecutionDoW() {
         //DoW: 0-6 -> 0, 4 (sunday, thursday)
-        final Cron cron = getUnixCron("* * * * */4");
-        final Optional<ZonedDateTime> nextExecution = getNextExecutionFor(cron, ZonedDateTime.parse("2016-01-28T16:32:56.586-08:00"));
+        final Cron cron = getUnixCron("0 0 * * */4");
+        Optional<ZonedDateTime> nextExecution = getNextExecutionFor(cron, ZonedDateTime.parse("2016-01-28T16:32:56.586-08:00"));
+        if (nextExecution.isPresent()) {
+            assertEquals(ZonedDateTime.parse("2016-01-31T00:00:00.000-08:00"), nextExecution.get());
+        } else {
+            fail(NEXT_EXECUTION_NOT_PRESENT_ERROR);
+        }
+        nextExecution = getNextExecutionFor(cron, nextExecution.get());
         if (nextExecution.isPresent()) {
             assertEquals(ZonedDateTime.parse("2016-02-04T00:00:00.000-08:00"), nextExecution.get());
         } else {
@@ -565,4 +571,20 @@ public class ExecutionTimeUnixIntegrationTest {
         final Optional<ZonedDateTime> nextExecution = ExecutionTime.forCron(myCron).nextExecution(time);
         assertFalse(nextExecution.isPresent());
     }
+
+    /**
+     * https://github.com/jmrozanec/cron-utils/issues/336
+     */
+    @Test
+    public void testEveryDayPerWeek() {
+        CronParser cronParser = new CronParser(CronDefinitionBuilder.instanceDefinitionFor(CronType.UNIX));
+        // every 3 days - Sun (1), Wed (4), Sat (7)
+        String cronString = "0 0 * * */3";
+        Cron cron = cronParser.parse(cronString);
+        ExecutionTime executionTime = ExecutionTime.forCron(cron);
+        Optional<ZonedDateTime> nextExecution = executionTime
+                .nextExecution(ZonedDateTime.of(2018, 2, 8, 0, 0, 0, 0, ZoneId.of("UTC")));
+        assertTrue(nextExecution.isPresent());
+        assertEquals(ZonedDateTime.of(2018, 2, 10, 0, 0, 0, 0, ZoneId.of("UTC")), nextExecution.get());
+    }
 }