Merge pull request #344 from ohtake/fix-crontab

Fixes for crontab and proposals to hashed crontab

core/src/main/grammar/crontab.g

@@ -67,7 +67,7 @@ throws ANTLRException
       }
       | "midnight"
       {
-        table.set("H H * * *",hash);
+        table.set("H H(0-2) * * *",hash);
       }
       | "hourly"
       {

core/src/main/java/hudson/scheduler/BaseParser.java

@@ -36,8 +36,9 @@ import antlr.TokenStreamException;
  * @author Kohsuke Kawaguchi
  */
 abstract class BaseParser extends LLkParser {
-    private static final int[] LOWER_BOUNDS = new int[] {0,0,1,1,0};
-    private static final int[] UPPER_BOUNDS = new int[] {59,23,31,12,7};
+    // lower/uppser bounds of fields (inclusive)
+    static final int[] LOWER_BOUNDS = new int[] {0,0,1,1,0};
+    static final int[] UPPER_BOUNDS = new int[] {59,23,31,12,7};
 
     /**
      * Used to pick a value from within the range
@@ -90,6 +91,7 @@ abstract class BaseParser extends LLkParser {
     protected long doHash( int field ) {
         int u = UPPER_BOUNDS[field];
         if (field==2) u = 28;   // day of month can vary depending on month, so to make life simpler, just use [1,28] that's always safe
+        if (field==4) u = 6;   // Both 0 and 7 of day of week are Sunday. For better distribution, limit upper bound to 6
         int h = hash.next(u+1 - LOWER_BOUNDS[field]); // upper bound is inclusive
         return 1L << (h+LOWER_BOUNDS[field]);
     }

core/src/main/java/hudson/scheduler/CronTab.java

@@ -89,8 +89,10 @@ public final class CronTab {
         spec = format;
 
         parser.startRule(this);
-        if((dayOfWeek&(1<<7))!=0)
+        if((dayOfWeek&(1<<7))!=0) {
             dayOfWeek |= 1; // copy bit 7 over to bit 0
+            dayOfWeek &= ~(1<<7); // clear bit 7 or CalendarField#ceil will return an invalid value 7
+        }
     }
 
 
@@ -413,7 +415,7 @@ public final class CronTab {
     public String checkSanity() {
         for( int i=0; i<5; i++ ) {
             long bitMask = (i<4)?bits[i]:(long)dayOfWeek;
-            for( int j=LOWER_BOUNDS[i]; j<=UPPER_BOUNDS[i]; j++ ) {
+            for( int j=BaseParser.LOWER_BOUNDS[i]; j<=BaseParser.UPPER_BOUNDS[i]; j++ ) {
                 if(!checkBits(bitMask,j)) {
                     // this rank has a sparse entry.
                     // if we have a sparse rank, one of them better be the left-most.
@@ -428,8 +430,4 @@ public final class CronTab {
 
         return null;
     }
-
-    // lower/uppser bounds of fields
-    private static final int[] LOWER_BOUNDS = new int[] {0,0,1,0,0};
-    private static final int[] UPPER_BOUNDS = new int[] {59,23,31,12,7};
 }

core/src/test/java/hudson/scheduler/CronTabDayOfWeekLocaleTest.java

@@ -13,6 +13,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 import org.junit.runners.Parameterized.Parameters;
+import org.jvnet.hudson.test.Bug;
 import org.jvnet.hudson.test.For;
 import org.jvnet.hudson.test.Url;
 
@@ -260,7 +261,7 @@ public class CronTabDayOfWeekLocaleTest {
     }
     
     @Test
-    public void isSundayAndNextRunIsPreviousSunday() throws Exception {
+    public void isSundayAndPreviousRunIsPreviousSunday() throws Exception {
         final Calendar cal = Calendar.getInstance(locale);
         cal.set(2011, 0, 16, 0, 0, 0); // Sunday, Jan 16th 2011, 00:00
         final String cronStr = "0 1 * * 0"; // Sundays @01:00
@@ -273,7 +274,68 @@ public class CronTabDayOfWeekLocaleTest {
         expected.set(2011, 0, 9, 1, 0, 0);
         compare(expected, actual);
     }
-    
+
+    @Test
+    @Bug(12357)
+    public void isSundayAndNextRunIsNextSunday7() throws Exception {
+        final Calendar cal = Calendar.getInstance(locale);
+        cal.set(2011, 0, 16, 1, 0, 0); // Sunday, Jan 16th 2011, 01:00
+        final String cronStr = "0 0 * * 7"; // Sundays(7 not 0) @00:00
+
+        final CronTab cron = new CronTab(cronStr);
+        final Calendar actual = cron.ceil(cal);
+
+        final Calendar expected = Calendar.getInstance();
+        // Expected next: Sunday, Jan 22th 2011, 00:00
+        expected.set(2011, 0, 23, 0, 0, 0);
+        compare(expected, actual);
+    }
+
+    @Test
+    public void isSundayAndPreviousRunIsPreviousSunday7() throws Exception {
+        final Calendar cal = Calendar.getInstance(locale);
+        cal.set(2011, 0, 16, 0, 0, 0); // Sunday, Jan 16th 2011, 00:00
+        final String cronStr = "0 1 * * 7"; // Sundays(7 not 0) @01:00
+
+        final CronTab cron = new CronTab(cronStr);
+        final Calendar actual = cron.floor(cal);
+
+        final Calendar expected = Calendar.getInstance();
+        // Expected next: Sunday, Jan 9th 2011, 01:00
+        expected.set(2011, 0, 9, 1, 0, 0);
+        compare(expected, actual);
+    }
+
+    @Test
+    public void isSaturdayAndNextRunIsSundayAsterisk() throws Exception {
+        final Calendar cal = Calendar.getInstance(locale);
+        cal.set(2011, 0, 15, 1, 0, 0); // Saturday, Jan 15th 2011, 01:00
+        final String cronStr = "0 0 * * *"; // Everyday @00:00
+
+        final CronTab cron = new CronTab(cronStr);
+        final Calendar actual = cron.ceil(cal);
+
+        final Calendar expected = Calendar.getInstance();
+        // Expected next: Sunday, Jan 16th 2011, 00:00
+        expected.set(2011, 0, 16, 0, 0, 0);
+        compare(expected, actual);
+    }
+
+    @Test
+    public void isSundayAndPreviousRunIsSaturdayAsterisk() throws Exception {
+        final Calendar cal = Calendar.getInstance(locale);
+        cal.set(2011, 0, 16, 0, 0, 0); // Sunday, Jan 16th 2011, 00:00
+        final String cronStr = "0 23 * * *"; // Everyday @23:00
+
+        final CronTab cron = new CronTab(cronStr);
+        final Calendar actual = cron.floor(cal);
+
+        final Calendar expected = Calendar.getInstance();
+        // Expected next: Saturday, Jan 15th 2011, 23:00
+        expected.set(2011, 0, 15, 23, 0, 0);
+        compare(expected, actual);
+    }
+
     private void compare(final Calendar expected, final Calendar actual) {
         final DateFormat f = DateFormat.getDateTimeInstance();
         final String msg = "Locale: " + locale + " FirstDayOfWeek: " + actual.getFirstDayOfWeek() + " Expected: "

core/src/test/java/hudson/scheduler/CronTabEventualityTest.java

+package hudson.scheduler;
+
+import antlr.ANTLRException;
+import static org.junit.Assert.*;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.jvnet.hudson.test.Bug;
+import org.jvnet.hudson.test.For;
+
+import java.text.DateFormat;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.Collection;
+import java.util.GregorianCalendar;
+
+@RunWith(Parameterized.class)
+@For({CronTab.class, Hash.class})
+public class CronTabEventualityTest {
+    @Parameterized.Parameters
+    public static Collection<Object[]> parameters() {
+        Collection<Object[]> parameters = new ArrayList<Object[]>();
+        parameters.add(new Object[]{"zero", Hash.zero()});
+        parameters.add(new Object[]{"seed1", Hash.from("seed1")});
+        parameters.add(new Object[]{"seed2", Hash.from("seed2")});
+        return parameters;
+    }
+    
+    private Calendar createLimit(Calendar start, int field, int amount){
+        Calendar limit = (Calendar)start.clone();
+        limit.add(field, amount);
+        return limit;
+    }
+
+    private String name;
+    private Hash hash;
+
+    public CronTabEventualityTest(String name, Hash hash) {
+        this.name = name;
+        this.hash = hash;
+    }
+
+    @Test(timeout = 1000)
+    @Bug(12388)
+    public void testYearlyWillBeEventuallyTriggeredWithinOneYear() throws ANTLRException {
+        Calendar start = new GregorianCalendar(2012, 0, 11, 22, 33); // Jan 11th 2012 22:33
+        Calendar limit = createLimit(start, Calendar.YEAR, 1);
+        checkEventuality(start, "@yearly", limit);
+    }
+
+    @Test(timeout = 1000)
+    @Bug(12388)
+    public void testAnnuallyWillBeEventuallyTriggeredWithinOneYear() throws ANTLRException {
+        Calendar start = new GregorianCalendar(2012, 0, 11, 22, 33); // Jan 11th 2012 22:33
+        Calendar limit = createLimit(start, Calendar.YEAR, 1);
+        checkEventuality(start, "@annually", limit);
+    }
+
+    @Test(timeout = 1000)
+    public void testMonthlyWillBeEventuallyTriggeredWithinOneMonth() throws ANTLRException {
+        Calendar start = new GregorianCalendar(2012, 0, 11, 22, 33); // Jan 11th 2012 22:33
+        Calendar limit = createLimit(start, Calendar.MONTH, 1);
+        checkEventuality(start, "@monthly", limit);
+    }
+
+    @Test(timeout = 1000)
+    public void testWeeklyWillBeEventuallyTriggeredWithinOneWeek() throws ANTLRException {
+        Calendar start = new GregorianCalendar(2012, 0, 11, 22, 33); // Jan 11th 2012 22:33
+        Calendar limit = createLimit(start, Calendar.WEEK_OF_YEAR, 1);
+        checkEventuality(start, "@weekly", limit);
+    }
+
+    @Test(timeout = 1000)
+    public void testDailyWillBeEventuallyTriggeredWithinOneDay() throws ANTLRException {
+        Calendar start = new GregorianCalendar(2012, 0, 11, 22, 33); // Jan 11th 2012 22:33
+        Calendar limit = createLimit(start, Calendar.DAY_OF_MONTH, 1);
+        checkEventuality(start, "@daily", limit);
+    }
+
+    @Test(timeout = 1000)
+    public void testMidnightWillBeEventuallyTriggeredWithinOneDay() throws ANTLRException {
+        Calendar start = new GregorianCalendar(2012, 0, 11, 22, 33); // Jan 11th 2012 22:33
+        Calendar limit = createLimit(start, Calendar.DAY_OF_MONTH, 1);
+        checkEventuality(start, "@midnight", limit);
+    }
+
+    @Test(timeout = 1000)
+    public void testHourlyWillBeEventuallyTriggeredWithinOneHour() throws ANTLRException {
+        Calendar start = new GregorianCalendar(2012, 0, 11, 22, 33); // Jan 11th 2012 22:33
+        Calendar limit = createLimit(start, Calendar.HOUR, 1);
+        checkEventuality(start, "@hourly", limit);
+    }
+
+    @Test(timeout = 1000)
+    public void testFirstDayOfMonthWillBeEventuallyTriggeredWithinOneMonth() throws ANTLRException {
+        Calendar start = new GregorianCalendar(2012, 0, 11, 22, 33); // Jan 11th 2012 22:33
+        Calendar limit = createLimit(start, Calendar.MONTH, 1);
+        checkEventuality(start, "H H 1 * *", limit);
+    }
+
+    @Test(timeout = 1000)
+    public void testFirstSundayOfMonthWillBeEventuallyTriggeredWithinOneMonthAndOneWeek() throws ANTLRException {
+        Calendar start = new GregorianCalendar(2012, 0, 11, 22, 33); // Jan 11th 2012 22:33
+        Calendar limit = createLimit(start, Calendar.DAY_OF_MONTH, 31+7);
+        // If both day of month and day of week are specified:
+        //     UNIX: triggered when either matches
+        //     Jenkins: triggered when both match
+        checkEventuality(start, "H H 1-7 * 0", limit);
+    }
+
+    private void checkEventuality(Calendar start, String crontabFormat, Calendar limit) throws ANTLRException {
+        CronTab cron = new CronTab(crontabFormat, hash);
+        Calendar next = cron.ceil(start);
+        if(next.after(limit)) {
+            DateFormat f = DateFormat.getDateTimeInstance();
+            String msg = "Name: " + name
+                    + " Limit: " + f.format(limit.getTime())
+                    + " Next: " + f.format(next.getTime());
+            fail(msg);
+        }
+    }
+}

core/src/test/java/hudson/scheduler/CronTabTest.java

@@ -29,7 +29,8 @@ import java.util.Calendar;
 import java.util.GregorianCalendar;
 import java.util.Locale;
 
-import junit.framework.TestCase;
+import static org.junit.Assert.*;
+import org.junit.Test;
 import org.jvnet.hudson.test.Bug;
 import org.jvnet.hudson.test.Url;
 
@@ -38,7 +39,7 @@ import static java.util.Calendar.MONDAY;
 /**
  * @author Kohsuke Kawaguchi
  */
-public class CronTabTest extends TestCase {
+public class CronTabTest {
     public void test1() throws ANTLRException {
         new CronTab("@yearly");
         new CronTab("@weekly");
@@ -72,6 +73,16 @@ public class CronTabTest extends TestCase {
         compare(new GregorianCalendar(2010,7,1,0,0),x.ceil(c));
     }
 
+    @Test(timeout = 1000)
+    @Bug(12357)
+    public void testCeil3_DoW7() throws Exception {
+        // similar to testCeil3, but DoW=7 may stuck in an infinite loop
+        CronTab x = new CronTab("0 0 1 * 7");
+        Calendar c = new GregorianCalendar(2010,0,1,15,55);
+        // the first such day in 2010 is Aug 1st
+        compare(new GregorianCalendar(2010, 7, 1, 0, 0), x.ceil(c));
+    }
+
     /**
      * Verifies that HUDSON-8656 never crops up again.
      */