From 31c2355742d6317a091ac5f47cfee0d17811fdeb Mon Sep 17 00:00:00 2001 From: Tim Su Date: Fri, 8 Jul 2011 16:11:44 -0700 Subject: [PATCH] Rewrote repeats completely to fulfil new requirements --- .../repeats/RepeatTaskCompleteListener.java | 171 ++++++++++-------- .../astrid/repeats/AdvancedRepeatTests.java | 162 +++++++++-------- 2 files changed, 179 insertions(+), 154 deletions(-) diff --git a/astrid/plugin-src/com/todoroo/astrid/repeats/RepeatTaskCompleteListener.java b/astrid/plugin-src/com/todoroo/astrid/repeats/RepeatTaskCompleteListener.java index 91642e11a..ae20655e2 100644 --- a/astrid/plugin-src/com/todoroo/astrid/repeats/RepeatTaskCompleteListener.java +++ b/astrid/plugin-src/com/todoroo/astrid/repeats/RepeatTaskCompleteListener.java @@ -54,7 +54,6 @@ public class RepeatTaskCompleteListener extends BroadcastReceiver { return; } - System.err.println("REPEATING, time to clonear"); StatisticsService.reportEvent("v2-task-repeat"); //$NON-NLS-1$ long hideUntil = task.getValue(Task.HIDE_UNTIL); @@ -87,96 +86,116 @@ public class RepeatTaskCompleteListener extends BroadcastReceiver { } } + /** Compute next due date */ public static long computeNextDueDate(Task task, String recurrence) throws ParseException { - DateValue repeatFrom; - Date repeatFromDate = new Date(); - - DateValue today = new DateValueImpl(repeatFromDate.getYear() + 1900, - repeatFromDate.getMonth() + 1, repeatFromDate.getDate()); boolean repeatAfterCompletion = task.getFlag(Task.FLAGS, Task.FLAG_REPEAT_AFTER_COMPLETION); - if(task.hasDueDate() && !repeatAfterCompletion) { - repeatFromDate = new Date(task.getValue(Task.DUE_DATE)); - if(task.hasDueTime()) { - repeatFrom = new DateTimeValueImpl(repeatFromDate.getYear() + 1900, - repeatFromDate.getMonth() + 1, repeatFromDate.getDate(), - repeatFromDate.getHours(), repeatFromDate.getMinutes(), repeatFromDate.getSeconds()); - } else { - repeatFrom = today; - } - } else if (task.hasDueDate() && repeatAfterCompletion) { - if(task.hasDueTime()) { - repeatFromDate = new Date(task.getValue(Task.DUE_DATE)); - repeatFrom = new DateTimeValueImpl(today.year(), - today.month(), today.day(), - repeatFromDate.getHours(), repeatFromDate.getMinutes(), repeatFromDate.getSeconds()); - } else { - repeatFrom = today; - } - repeatFromDate = new Date(); + RRule rrule = initRRule(recurrence); + + // initialize startDateAsDV + Date original = setUpStartDate(task, repeatAfterCompletion); + DateValue startDateAsDV = setUpStartDateAsDV(task, rrule, original, repeatAfterCompletion); + + if(rrule.getFreq() == Frequency.HOURLY) + return handleHourlyRepeat(original, rrule); + else + return invokeRecurrence(rrule, original, startDateAsDV); + } + + private static long invokeRecurrence(RRule rrule, Date original, + DateValue startDateAsDV) { + long newDueDate = -1; + RecurrenceIterator iterator = RecurrenceIteratorFactory.createRecurrenceIterator(rrule, + startDateAsDV, TimeZone.getDefault()); + DateValue nextDate = startDateAsDV; + + for(int i = 0; i < 10; i++) { // ten tries then we give up + if(!iterator.hasNext()) + return -1; + nextDate = iterator.next(); + + if(nextDate.compareTo(startDateAsDV) == 0) + continue; + + newDueDate = buildNewDueDate(original, nextDate); + + // detect if we finished + if(newDueDate > original.getTime()) + break; + } + return newDueDate; + } + + /** Compute long due date from DateValue */ + private static long buildNewDueDate(Date original, DateValue nextDate) { + long newDueDate; + if(nextDate instanceof DateTimeValueImpl) { + DateTimeValueImpl newDateTime = (DateTimeValueImpl)nextDate; + Date date = new Date(Date.UTC(newDateTime.year() - 1900, newDateTime.month() - 1, + newDateTime.day(), newDateTime.hour(), + newDateTime.minute(), newDateTime.second())); + // time may be inaccurate due to DST, force time to be same + date.setHours(original.getHours()); + date.setMinutes(original.getMinutes()); + newDueDate = Task.createDueDate(Task.URGENCY_SPECIFIC_DAY_TIME, + date.getTime()); } else { - repeatFrom = today; + newDueDate = Task.createDueDate(Task.URGENCY_SPECIFIC_DAY, + new Date(nextDate.year() - 1900, nextDate.month() - 1, + nextDate.day()).getTime()); } + return newDueDate; + } - // invoke the recurrence iterator - long newDueDate = -1; + /** Initialize RRule instance */ + private static RRule initRRule(String recurrence) throws ParseException { RRule rrule = new RRule(recurrence); // handle the iCalendar "byDay" field differently depending on if // we are weekly or otherwise - - if(rrule.getFreq() != Frequency.WEEKLY) { + if(rrule.getFreq() != Frequency.WEEKLY) rrule.setByDay(Collections.EMPTY_LIST); - } - if(rrule.getFreq() == Frequency.HOURLY) { - newDueDate = task.createDueDate(Task.URGENCY_SPECIFIC_DAY_TIME, - repeatFromDate.getTime() + DateUtilities.ONE_HOUR * rrule.getInterval()); - } else { - RecurrenceIterator iterator = RecurrenceIteratorFactory.createRecurrenceIterator(rrule, - repeatFrom, TimeZone.getDefault()); - DateValue nextDate = repeatFrom; - if(repeatFrom.compareTo(today) < 0) - iterator.advanceTo(today); - - for(int i = 0; i < 10; i++) { // ten tries then we give up - if(!iterator.hasNext()) - return -1; - nextDate = iterator.next(); - - if(nextDate.compareTo(repeatFrom) == 0) - continue; - - if(nextDate instanceof DateTimeValueImpl) { - DateTimeValueImpl newDateTime = (DateTimeValueImpl)nextDate; - Date date = new Date(Date.UTC(newDateTime.year() - 1900, newDateTime.month() - 1, - newDateTime.day(), newDateTime.hour(), - newDateTime.minute(), newDateTime.second())); - // time may be inaccurate due to DST, force time to be same - date.setHours(repeatFromDate.getHours()); - date.setMinutes(repeatFromDate.getMinutes()); - newDueDate = task.createDueDate(Task.URGENCY_SPECIFIC_DAY_TIME, - date.getTime()); - } else { - newDueDate = task.createDueDate(Task.URGENCY_SPECIFIC_DAY, - new Date(nextDate.year() - 1900, nextDate.month() - 1, - nextDate.day()).getTime()); - } - - // detect if we finished - if(newDueDate > DateUtilities.now()) { - // if byDay is set and interval > 1, we need to run again - if(rrule.getByDay().size() > 0 && rrule.getInterval() > 1) { - if(newDueDate - repeatFromDate.getTime() > DateUtilities.ONE_WEEK) - break; - } else if(newDueDate > repeatFromDate.getTime()) - break; - } + return rrule; + } + + /** Set up repeat start date */ + private static Date setUpStartDate(Task task, boolean repeatAfterCompletion) { + Date startDate = new Date(); + if(task.hasDueDate()) { + Date dueDate = new Date(task.getValue(Task.DUE_DATE)); + if(!repeatAfterCompletion) + startDate = dueDate; + else if(task.hasDueTime()) { + startDate.setHours(dueDate.getHours()); + startDate.setMinutes(dueDate.getMinutes()); } } + return startDate; + } + + private static DateValue setUpStartDateAsDV(Task task, RRule rrule, Date startDate, + boolean repeatAfterCompletion) { - if(newDueDate == -1) - return -1; + // if repeat after completion with weekdays, pre-compute + if(repeatAfterCompletion && rrule.getByDay().size() > 0) { + startDate = new Date(startDate.getTime() + DateUtilities.ONE_WEEK * rrule.getInterval() - + DateUtilities.ONE_DAY); + rrule.setInterval(1); + } + + if(task.hasDueTime()) + return new DateTimeValueImpl(startDate.getYear() + 1900, + startDate.getMonth() + 1, startDate.getDate(), + startDate.getHours(), startDate.getMinutes(), startDate.getSeconds()); + else + return new DateValueImpl(startDate.getYear() + 1900, + startDate.getMonth() + 1, startDate.getDate()); + } + private static long handleHourlyRepeat(Date startDate, RRule rrule) { + long newDueDate; + newDueDate = Task.createDueDate(Task.URGENCY_SPECIFIC_DAY_TIME, + startDate.getTime() + DateUtilities.ONE_HOUR * rrule.getInterval()); return newDueDate; } diff --git a/tests/src/com/todoroo/astrid/repeats/AdvancedRepeatTests.java b/tests/src/com/todoroo/astrid/repeats/AdvancedRepeatTests.java index 8ea3c8fea..157e50660 100644 --- a/tests/src/com/todoroo/astrid/repeats/AdvancedRepeatTests.java +++ b/tests/src/com/todoroo/astrid/repeats/AdvancedRepeatTests.java @@ -33,51 +33,42 @@ public class AdvancedRepeatTests extends TodorooTestCase { rrule = new RRule(); } - public static void assertDatesEqual(long date, long other) { - assertEquals("Expected: " + new Date(date) + ", Actual: " + new Date(other), - date, other); - } + // --- date with time tests - public void testDueDateInPast() throws ParseException { - rrule.setInterval(1); - rrule.setFreq(Frequency.DAILY); - - // repeat once => due date should become tomorrow - long past = Task.createDueDate(Task.URGENCY_SPECIFIC_DAY, new Date(110, 7, 1).getTime()); - task.setValue(Task.DUE_DATE, past); - long tomorrow = Task.createDueDate(Task.URGENCY_SPECIFIC_DAY, DateUtilities.now() + DateUtilities.ONE_DAY); - long nextDueDate = RepeatTaskCompleteListener.computeNextDueDate(task, rrule.toIcal()); - assertDatesEqual(tomorrow, nextDueDate); + public void testDueDateSpecificTime() throws ParseException { + task.setFlag(Task.FLAGS, Task.FLAG_REPEAT_AFTER_COMPLETION, false); + buildRRule(1, Frequency.DAILY); // test specific day & time - long pastWithTime = Task.createDueDate(Task.URGENCY_SPECIFIC_DAY_TIME, new Date(110, 7, 1, 10, 4).getTime()); - task.setValue(Task.DUE_DATE, pastWithTime); - Date date = new Date(DateUtilities.now() / 1000L * 1000L); - date.setHours(10); - date.setMinutes(4); - date.setSeconds(0); - long todayWithTime = Task.createDueDate(Task.URGENCY_SPECIFIC_DAY_TIME, date.getTime()) / 1000L * 1000L; - if(todayWithTime < DateUtilities.now()) - todayWithTime += DateUtilities.ONE_DAY; + long dayWithTime = Task.createDueDate(Task.URGENCY_SPECIFIC_DAY_TIME, new Date(110, 7, 1, 10, 4).getTime()); + task.setValue(Task.DUE_DATE, dayWithTime); + + long nextDayWithTime = dayWithTime + DateUtilities.ONE_DAY; nextDueDate = RepeatTaskCompleteListener.computeNextDueDate(task, rrule.toIcal()); - assertDatesEqual(todayWithTime, nextDueDate); + assertDateTimeEquals(nextDayWithTime, nextDueDate); } - public void testDueDateInPastRepeatMultiple() throws ParseException { - rrule.setInterval(1); - rrule.setFreq(Frequency.DAILY); + public void testCompletionDateSpecificTime() throws ParseException { + task.setFlag(Task.FLAGS, Task.FLAG_REPEAT_AFTER_COMPLETION, true); + buildRRule(1, Frequency.DAILY); + + // test specific day & time + long dayWithTime = Task.createDueDate(Task.URGENCY_SPECIFIC_DAY_TIME, new Date(110, 7, 1, 10, 4).getTime()); + task.setValue(Task.DUE_DATE, dayWithTime); + + Date todayWithTime = new Date(); + todayWithTime.setHours(10); + todayWithTime.setMinutes(4); + todayWithTime.setSeconds(0); + long nextDayWithTimeLong = todayWithTime.getTime(); + if(nextDayWithTimeLong < DateUtilities.now()) + nextDayWithTimeLong += DateUtilities.ONE_DAY; - // repeat once => due date should become tomorrow - long past = Task.createDueDate(Task.URGENCY_SPECIFIC_DAY_TIME, new Date(110, 7, 1, 0, 0, 0).getTime()); - task.setValue(Task.DUE_DATE, past); nextDueDate = RepeatTaskCompleteListener.computeNextDueDate(task, rrule.toIcal()); - assertTrue(nextDueDate > DateUtilities.now()); - task.setValue(Task.DUE_DATE, nextDueDate); - long evenMoreNextDueDate = RepeatTaskCompleteListener.computeNextDueDate(task, rrule.toIcal()); - assertNotSame(nextDueDate, evenMoreNextDueDate); + assertDateTimeEquals(nextDueDate, nextDueDate); } - // --- in-the-past tests + // --- due date tests /** test multiple days per week - DUE DATE */ public void testDueDateInPastSingleWeekMultiDay() throws Exception { @@ -98,10 +89,8 @@ public class AdvancedRepeatTests extends TodorooTestCase { assertDueDate(nextDueDate, THIS, Calendar.MONDAY); } - // --- single day tests - /** test single day repeats - DUE DATE */ - public void testSingleDay() throws Exception { + public void testDueDateSingleDay() throws Exception { task.setFlag(Task.FLAGS, Task.FLAG_REPEAT_AFTER_COMPLETION, false); buildRRule(1, Frequency.WEEKLY, Weekday.MO); @@ -131,8 +120,6 @@ public class AdvancedRepeatTests extends TodorooTestCase { assertDueDate(nextDueDate, NEXT, Calendar.MONDAY); } - // --- multi day tests - /** test multiple days per week - DUE DATE */ public void testDueDateSingleWeekMultiDay() throws Exception { task.setFlag(Task.FLAGS, Task.FLAG_REPEAT_AFTER_COMPLETION, false); @@ -160,7 +147,7 @@ public class AdvancedRepeatTests extends TodorooTestCase { setTaskDueDate(THIS, Calendar.SUNDAY); computeNextDueDate(); - assertDueDate(nextDueDate, THIS, Calendar.MONDAY); + assertDueDate(nextDueDate, NEXT, Calendar.MONDAY); setTaskDueDate(THIS, Calendar.MONDAY); computeNextDueDate(); @@ -171,42 +158,56 @@ public class AdvancedRepeatTests extends TodorooTestCase { assertDueDate(nextDueDate, NEXT, Calendar.MONDAY); } + // --- completion tests + /** test multiple days per week - COMPLETE DATE */ - public void testCompleteDateSingleWeekMultiDay() throws Exception { + public void testCompleteDateSingleWeek() throws Exception { task.setFlag(Task.FLAGS, Task.FLAG_REPEAT_AFTER_COMPLETION, true); - buildRRule(1, Frequency.WEEKLY, Weekday.MO, Weekday.WE, Weekday.FR); - - setTaskDueDate(THIS, Calendar.SUNDAY); - computeNextDueDate(); - assertDueDate(nextDueDate, NEXT, Calendar.MONDAY); - - setTaskDueDate(THIS, Calendar.MONDAY); - computeNextDueDate(); - assertDueDate(nextDueDate, NEXT, Calendar.WEDNESDAY); - - setTaskDueDate(THIS, Calendar.FRIDAY); - computeNextDueDate(); - assertDueDate(nextDueDate, NEXT, Calendar.MONDAY); + for(Weekday wday : Weekday.values()) { + buildRRule(1, Frequency.WEEKLY, wday); + computeNextDueDate(); + long expected = getDate(DateUtilities.now(), NEXT, wday.javaDayNum); + assertDateEquals(nextDueDate, expected); + } + + for(Weekday wday1 : Weekday.values()) { + for(Weekday wday2 : Weekday.values()) { + if(wday1 == wday2) + continue; + + buildRRule(1, Frequency.WEEKLY, wday1, wday2); + long nextOne = getDate(DateUtilities.now(), NEXT, wday1.javaDayNum); + long nextTwo = getDate(DateUtilities.now(), NEXT, wday2.javaDayNum); + computeNextDueDate(); + assertDateEquals(nextDueDate, Math.min(nextOne, nextTwo)); + } + } } /** test multiple days per week, multiple intervals - COMPLETE DATE */ - public void testCompleteDateMultiWeekMultiDay() throws Exception { + public void testCompleteDateMultiWeek() throws Exception { task.setFlag(Task.FLAGS, Task.FLAG_REPEAT_AFTER_COMPLETION, true); - buildRRule(2, Frequency.WEEKLY, Weekday.MO, Weekday.WE, Weekday.FR); - - setTaskDueDate(THIS, Calendar.SUNDAY); - computeNextDueDate(); - assertDueDate(nextDueDate, NEXT_NEXT, Calendar.MONDAY); - - setTaskDueDate(THIS, Calendar.MONDAY); - computeNextDueDate(); - assertDueDate(nextDueDate, NEXT_NEXT, Calendar.WEDNESDAY); - - setTaskDueDate(THIS, Calendar.FRIDAY); - computeNextDueDate(); - assertDueDate(nextDueDate, NEXT_NEXT, Calendar.MONDAY); + for(Weekday wday : Weekday.values()) { + buildRRule(2, Frequency.WEEKLY, wday); + computeNextDueDate(); + long expected = getDate(DateUtilities.now(), NEXT_NEXT, wday.javaDayNum); + assertDateEquals(nextDueDate, expected); + } + + for(Weekday wday1 : Weekday.values()) { + for(Weekday wday2 : Weekday.values()) { + if(wday1 == wday2) + continue; + + buildRRule(2, Frequency.WEEKLY, wday1, wday2); + long nextOne = getDate(DateUtilities.now(), NEXT_NEXT, wday1.javaDayNum); + long nextTwo = getDate(DateUtilities.now(), NEXT_NEXT, wday2.javaDayNum); + computeNextDueDate(); + assertDateEquals(nextDueDate, Math.min(nextOne, nextTwo)); + } + } } // --- helpers @@ -215,7 +216,7 @@ public class AdvancedRepeatTests extends TodorooTestCase { nextDueDate = RepeatTaskCompleteListener.computeNextDueDate(task, rrule.toIcal()); } - private void buildRRule(int interval, Frequency freq, Weekday mo, Weekday... weekdays) { + private void buildRRule(int interval, Frequency freq, Weekday... weekdays) { rrule.setInterval(interval); rrule.setFreq(freq); setRRuleDays(rrule, weekdays); @@ -223,7 +224,17 @@ public class AdvancedRepeatTests extends TodorooTestCase { private void assertDueDate(long actual, int expectedWhich, int expectedDayOfWeek) { long expected = getDate(task.getValue(Task.DUE_DATE), expectedWhich, expectedDayOfWeek); - assertEquals("Due Date is '" + new Date(actual) + "', expected '" + new Date(expected) + "'", + assertDateEquals(actual, expected); + } + + public static void assertDateTimeEquals(long date, long other) { + assertEquals("Expected: " + new Date(date) + ", Actual: " + new Date(other), + date, other); + } + + private void assertDateEquals(long actual, long expected) { + assertEquals("Due Date is '" + DateUtilities.getDateStringWithWeekday(getContext(), new Date(actual)) + + "', expected '" + DateUtilities.getDateStringWithWeekday(getContext(), new Date(expected)) + "'", expected, actual); } @@ -238,8 +249,7 @@ public class AdvancedRepeatTests extends TodorooTestCase { private void setTaskDueDate(int which, int day) { long time = getDate(DateUtilities.now(), which, day); - task.setValue(Task.DUE_DATE, Task.createDueDate(Task.URGENCY_SPECIFIC_DAY, - time)); + task.setValue(Task.DUE_DATE, time); } private long getDate(long start, int which, int dayOfWeek) { @@ -252,11 +262,7 @@ public class AdvancedRepeatTests extends TodorooTestCase { c.add(Calendar.DAY_OF_MONTH, direction * 7); while(c.get(Calendar.DAY_OF_WEEK) != dayOfWeek) c.add(Calendar.DAY_OF_MONTH, direction); - c.set(Calendar.HOUR, 0); - c.set(Calendar.MINUTE, 0); - c.set(Calendar.SECOND, 0); - c.set(Calendar.MILLISECOND, 0); - return c.getTimeInMillis(); + return Task.createDueDate(Task.URGENCY_SPECIFIC_DAY, c.getTimeInMillis()); } }