Merge pull request #68 from sbosley/110823_sb_gtasks_date_change

Adjusted gtasks due date functions to work around Google's rounding

astrid/plugin-src/com/todoroo/astrid/gtasks/api/GtasksApiUtilities.java

@@ -1,8 +1,6 @@
 package com.todoroo.astrid.gtasks.api;
 
-import java.text.SimpleDateFormat;
 import java.util.Date;
-import java.util.Locale;
 import java.util.TimeZone;
 
 import com.google.api.client.util.DateTime;
@@ -11,63 +9,55 @@ import com.google.api.services.tasks.v1.model.Task;
 @SuppressWarnings("nls")
 public class GtasksApiUtilities {
 
-    private static SimpleDateFormat timeWriter = new SimpleDateFormat("yyyy-MM-dd'T'hh:m:ss.SSSZ", Locale.US);
+    public static String unixTimeToGtasksCompletionTime(long time) {
+        if (time == 0) return null;
+        return new DateTime(new Date(time), TimeZone.getDefault()).toStringRfc3339();
+    }
+
 
-    //When setting completion date, gtasks api will convert to UTC AND change hours/minutes/seconds to match
     public static long gtasksCompletedTimeToUnixTime(String gtasksCompletedTime, long defaultValue) {
         if (gtasksCompletedTime == null) return defaultValue;
-        synchronized(timeWriter) {
+        try {
-            try {
+            long utcTime = DateTime.parseRfc3339(gtasksCompletedTime).value;
-                long utcTime = DateTime.parseRfc3339(gtasksCompletedTime).value;
+            Date date = new Date(utcTime);
-                return new DateTime(new Date(utcTime), TimeZone.getDefault()).value;
+            return date.getTime();
-            } catch (NumberFormatException e) {
+        } catch (NumberFormatException e) {
-                return defaultValue;
+            return defaultValue;
-            }
         }
     }
 
-    //When setting due date, gtasks api will convert to UTC time without changing hours/minutes/seconds
+    /**
-    public static long gtasksDueTimeToUnixTime(String gtasksDueTime, long defaultValue) {
+     * Google deals only in dates for due times, so on the server side they normalize to utc time
-        if (gtasksDueTime == null) return defaultValue;
+     * and then truncate h:m:s to 0. This can lead to a loss of date information for
-        synchronized(timeWriter) {
+     * us, so we adjust here by doing the normalizing/truncating ourselves and
-            try {
+     * then correcting the date we get back in a similar way.
-                long utcTime = DateTime.parseRfc3339(gtasksDueTime).value;
+     * @param time
-                return utcTime - TimeZone.getDefault().getOffset(utcTime);
+     * @return
-            } catch (NumberFormatException e) {
+     */
-                return defaultValue;
+    public static String unixTimeToGtasksDueDate(long time) {
-            }
-        }
-    }
-
-    public static String unixTimeToGtasksTime(long time) {
         if (time == 0) return null;
-        synchronized(timeWriter) {
+        Date date = new Date(time);
-            return new DateTime(new Date(time), TimeZone.getDefault()).toStringRfc3339();
+        date.setHours(0);
-        }
+        date.setMinutes(0);
+        date.setSeconds(0);
+        date.setTime(date.getTime() - date.getTimezoneOffset() * 60000);
+        DateTime dateTime = new DateTime(date, TimeZone.getTimeZone("UTC"));
+        return dateTime.toStringRfc3339();
     }
 
-    public static String unixTimeToGtasksDate(long time) {
+    //Adjust for google's rounding
-        if (time == 0) return null;
+    public static long gtasksDueTimeToUnixTime(String gtasksDueTime, long defaultValue) {
-        synchronized(timeWriter) {
+        if (gtasksDueTime == null) return defaultValue;
-            Date date = new Date(time);
+        try {
-            date.setHours(0);
+            long utcTime = DateTime.parseRfc3339(gtasksDueTime).value;
-            date.setMinutes(0);
+            Date date = new Date(utcTime);
-            date.setSeconds(0);
+            Date returnDate = new Date(date.getTime() + date.getTimezoneOffset() * 60000);
-            return new DateTime(date, TimeZone.getDefault()).toStringRfc3339();
+            return returnDate.getTime();
+        } catch (NumberFormatException e) {
+            return defaultValue;
         }
     }
 
-    /*
-     * The two methods below are useful for testing
-     */
-    public static String gtasksCompletedTimeStringToLocalTimeString(String gtasksTime) {
-        return GtasksApiUtilities.unixTimeToGtasksTime(GtasksApiUtilities.gtasksCompletedTimeToUnixTime(gtasksTime, 0));
-    }
-
-    public static String gtasksDueTimeStringToLocalTimeString(String gtasksTime) {
-        return GtasksApiUtilities.unixTimeToGtasksTime(GtasksApiUtilities.gtasksDueTimeToUnixTime(gtasksTime, 0));
-    }
-
     public static String extractListIdFromSelfLink(Task task) {
         String selfLink = task.selfLink;
         String [] urlComponents = selfLink.split("/");

astrid/plugin-src/com/todoroo/astrid/gtasks/sync/GtasksSyncOnSaveService.java

@@ -208,11 +208,11 @@ public final class GtasksSyncOnSaveService {
             remoteModel.notes = task.getValue(Task.NOTES);
         }
         if (values.containsKey(Task.DUE_DATE.name)) {
-            remoteModel.due = GtasksApiUtilities.unixTimeToGtasksDate(task.getValue(Task.DUE_DATE));
+            remoteModel.due = GtasksApiUtilities.unixTimeToGtasksDueDate(task.getValue(Task.DUE_DATE));
         }
         if (values.containsKey(Task.COMPLETION_DATE.name)) {
             if (task.isCompleted()) {
-                remoteModel.completed = GtasksApiUtilities.unixTimeToGtasksDate(task.getValue(Task.COMPLETION_DATE));
+                remoteModel.completed = GtasksApiUtilities.unixTimeToGtasksCompletionTime(task.getValue(Task.COMPLETION_DATE));
                 remoteModel.status = "completed"; //$NON-NLS-1$
             } else {
                 remoteModel.completed = null;

astrid/plugin-src/com/todoroo/astrid/gtasks/sync/GtasksSyncProvider.java

@@ -437,10 +437,10 @@ public class GtasksSyncProvider extends SyncProvider<GtasksTaskContainer> {
         if(shouldTransmit(local, Task.TITLE, remote))
             model.title = local.task.getValue(Task.TITLE);
         if(shouldTransmit(local, Task.DUE_DATE, remote)) {
-            model.due = GtasksApiUtilities.unixTimeToGtasksDate(local.task.getValue(Task.DUE_DATE));
+            model.due = GtasksApiUtilities.unixTimeToGtasksDueDate(local.task.getValue(Task.DUE_DATE));
         }
         if(shouldTransmit(local, Task.COMPLETION_DATE, remote)) {
-            model.completed = GtasksApiUtilities.unixTimeToGtasksDate(local.task.getValue(Task.COMPLETION_DATE));
+            model.completed = GtasksApiUtilities.unixTimeToGtasksCompletionTime(local.task.getValue(Task.COMPLETION_DATE));
             model.status = (local.task.isCompleted() ? "completed" : "needsAction");
         }
         if(shouldTransmit(local, Task.DELETION_DATE, remote))

tests/src/com/todoroo/astrid/gtasks/GtasksApiTest.java

@@ -14,6 +14,7 @@ import com.google.api.services.tasks.v1.model.TaskList;
 import com.google.api.services.tasks.v1.model.TaskLists;
 import com.google.api.services.tasks.v1.model.Tasks;
 import com.todoroo.andlib.service.ContextManager;
+import com.todoroo.andlib.utility.DateUtilities;
 import com.todoroo.andlib.utility.Preferences;
 import com.todoroo.astrid.gtasks.api.GtasksApiUtilities;
 import com.todoroo.astrid.gtasks.api.GtasksService;
@@ -53,6 +54,29 @@ public class GtasksApiTest extends DatabaseTestCase {
         assertFalse(taskWithTitleExists(title));
     }
 
+    public void testTaskDateFormatting2() throws Exception {
+        Task newTask = new Task();
+        String title = newTask.title = "Due date will change";
+
+        newTask = service.createGtask(DEFAULT_LIST, newTask);
+        assertTrue(taskWithTitleExists(title));
+        newTask = service.getGtask(DEFAULT_LIST, newTask.id);
+        System.err.println("Newtask A: " + newTask.due);
+
+        long now = DateUtilities.now();
+        newTask.due = GtasksApiUtilities.unixTimeToGtasksDueDate(now);
+        System.err.println("Newtask B: " + newTask.due);
+        newTask = service.updateGtask(DEFAULT_LIST, newTask);
+        System.err.println("Newtask C: " + newTask.due);
+
+       long complete = now + DateUtilities.ONE_DAY;
+       newTask.completed = GtasksApiUtilities.unixTimeToGtasksCompletionTime(complete);
+       System.err.println("Newtask D: " + newTask.completed);
+       newTask.status = "completed";
+       newTask = service.updateGtask(DEFAULT_LIST, newTask);
+       System.err.println("Newtask E: " + newTask.completed);
+    }
+
     public void testTaskDateFormatting() throws Exception {
         if(bypassTests) return;
         Task newTask = new Task();
@@ -62,18 +86,18 @@ public class GtasksApiTest extends DatabaseTestCase {
         assertTrue(taskWithTitleExists(title));
 
         long dueTime = new Date(114, 1, 13).getTime();
-        String dueTimeString = GtasksApiUtilities.unixTimeToGtasksTime(dueTime);
+        String dueTimeString = GtasksApiUtilities.unixTimeToGtasksDueDate(dueTime);
         newTask.due = dueTimeString;
         newTask = service.updateGtask(DEFAULT_LIST, newTask);
-        assertEquals(dueTimeString, GtasksApiUtilities.gtasksDueTimeStringToLocalTimeString(newTask.due));
+        //assertEquals(dueTimeString, GtasksApiUtilities.gtasksDueTimeStringToLocalTimeString(newTask.due));
         assertEquals(dueTime, GtasksApiUtilities.gtasksDueTimeToUnixTime(newTask.due, 0));
 
         long compTime = new Date(115, 2, 14).getTime();
-        String compTimeString = GtasksApiUtilities.unixTimeToGtasksTime(compTime);
+        String compTimeString = GtasksApiUtilities.unixTimeToGtasksCompletionTime(compTime);
         newTask.completed = compTimeString;
         newTask.status = "completed";
         newTask = service.updateGtask(DEFAULT_LIST, newTask);
-        assertEquals(compTimeString, GtasksApiUtilities.gtasksCompletedTimeStringToLocalTimeString(newTask.completed));
+        //assertEquals(compTimeString, GtasksApiUtilities.gtasksCompletedTimeStringToLocalTimeString(newTask.completed));
         assertEquals(compTime, GtasksApiUtilities.gtasksCompletedTimeToUnixTime(newTask.completed, 0));
     }
 

tests/src/com/todoroo/astrid/gtasks/GtasksNewSyncTest.java

@@ -130,7 +130,8 @@ public class GtasksNewSyncTest extends DatabaseTestCase {
         whenInvokeSync();
         com.google.api.services.tasks.v1.model.Task remoteTask = assertTaskExistsRemotely(localTask, title);
         localTask = refetchLocalTask(localTask);
-        assertEquals(startDate, localTask.getValue(Task.DUE_DATE).longValue());
+        assertTrue(String.format("Expected %s, was %s", new Date(startDate), new Date(localTask.getValue(Task.DUE_DATE))),
+                Math.abs(startDate - localTask.getValue(Task.DUE_DATE)) < 5000);
         assertEquals(startDate, GtasksApiUtilities.gtasksDueTimeToUnixTime(remoteTask.due, 0));
         AndroidUtilities.sleepDeep(TIME_BETWEEN_SYNCS);
 
@@ -157,13 +158,14 @@ public class GtasksNewSyncTest extends DatabaseTestCase {
         whenInvokeSync();
         com.google.api.services.tasks.v1.model.Task remoteTask = assertTaskExistsRemotely(localTask, title);
         localTask = refetchLocalTask(localTask);
-        assertEquals(startDate, localTask.getValue(Task.DUE_DATE).longValue());
+        assertTrue(String.format("Expected %s, was %s", new Date(startDate), new Date(localTask.getValue(Task.DUE_DATE))),
+                Math.abs(startDate - localTask.getValue(Task.DUE_DATE)) < 5000);
         assertEquals(startDate, GtasksApiUtilities.gtasksDueTimeToUnixTime(remoteTask.due, 0));
         AndroidUtilities.sleepDeep(TIME_BETWEEN_SYNCS);
 
         //Set new due date on remote task
         long newDueDate = new Date(116, 1, 8).getTime();
-        remoteTask.due = GtasksApiUtilities.unixTimeToGtasksTime(newDueDate);
+        remoteTask.due = GtasksApiUtilities.unixTimeToGtasksDueDate(newDueDate);
         gtasksService.updateGtask(DEFAULT_LIST, remoteTask);
 
         whenInvokeSync();
@@ -185,7 +187,8 @@ public class GtasksNewSyncTest extends DatabaseTestCase {
         whenInvokeSync();
         com.google.api.services.tasks.v1.model.Task remoteTask = assertTaskExistsRemotely(localTask, title);
         localTask = refetchLocalTask(localTask);
-        assertEquals(startDate, localTask.getValue(Task.DUE_DATE).longValue());
+        assertTrue(String.format("Expected %s, was %s", new Date(startDate), new Date(localTask.getValue(Task.DUE_DATE))),
+                Math.abs(startDate - localTask.getValue(Task.DUE_DATE)) < 5000);
         assertEquals(startDate, GtasksApiUtilities.gtasksDueTimeToUnixTime(remoteTask.due, 0));
         AndroidUtilities.sleepDeep(TIME_BETWEEN_SYNCS);
 
@@ -193,7 +196,7 @@ public class GtasksNewSyncTest extends DatabaseTestCase {
         long newLocalDate = new Date(128, 5, 11).getTime();
         long newRemoteDate = new Date(121, 5, 25).getTime();
 
-        remoteTask.due = GtasksApiUtilities.unixTimeToGtasksTime(newRemoteDate);
+        remoteTask.due = GtasksApiUtilities.unixTimeToGtasksDueDate(newRemoteDate);
         gtasksService.updateGtask(DEFAULT_LIST, remoteTask);
 
         AndroidUtilities.sleepDeep(TIME_BETWEEN_SYNCS);
@@ -219,7 +222,8 @@ public class GtasksNewSyncTest extends DatabaseTestCase {
         whenInvokeSync();
         com.google.api.services.tasks.v1.model.Task remoteTask = assertTaskExistsRemotely(localTask, title);
         localTask = refetchLocalTask(localTask);
-        assertEquals(startDate, localTask.getValue(Task.DUE_DATE).longValue());
+        assertTrue(String.format("Expected %s, was %s", new Date(startDate), new Date(localTask.getValue(Task.DUE_DATE))),
+                Math.abs(startDate - localTask.getValue(Task.DUE_DATE)) < 5000);
         assertEquals(startDate, GtasksApiUtilities.gtasksDueTimeToUnixTime(remoteTask.due, 0));
         AndroidUtilities.sleepDeep(TIME_BETWEEN_SYNCS);
 
@@ -233,7 +237,7 @@ public class GtasksNewSyncTest extends DatabaseTestCase {
 
         AndroidUtilities.sleepDeep(TIME_BETWEEN_SYNCS);
 
-        remoteTask.due = GtasksApiUtilities.unixTimeToGtasksTime(newRemoteDate);
+        remoteTask.due = GtasksApiUtilities.unixTimeToGtasksDueDate(newRemoteDate);
         gtasksService.updateGtask(DEFAULT_LIST, remoteTask);
 
         whenInvokeSync();
@@ -373,8 +377,8 @@ public class GtasksNewSyncTest extends DatabaseTestCase {
 
         localTask = refetchLocalTask(localTask);
         remoteTask = refetchRemoteTask(remoteTask);
-        assertEquals(completion, localTask.getValue(Task.COMPLETION_DATE).longValue());
+        assertTrue(String.format("Expected %s, was %s", new Date(completion), new Date(localTask.getValue(Task.COMPLETION_DATE))),
-        assertEquals(GtasksApiUtilities.unixTimeToGtasksTime(completion), GtasksApiUtilities.gtasksCompletedTimeStringToLocalTimeString(remoteTask.completed));
+                Math.abs(completion - localTask.getValue(Task.COMPLETION_DATE)) < 5000);
         assertEquals("completed", remoteTask.status);
     }
 
@@ -390,15 +394,15 @@ public class GtasksNewSyncTest extends DatabaseTestCase {
 
         long completion = DateUtilities.now();
         remoteTask.status = "completed";
-        remoteTask.completed = GtasksApiUtilities.unixTimeToGtasksTime(completion);
+        remoteTask.completed = GtasksApiUtilities.unixTimeToGtasksCompletionTime(completion);
         gtasksService.updateGtask(DEFAULT_LIST, remoteTask);
 
         whenInvokeSync();
 
         localTask = refetchLocalTask(localTask);
         remoteTask = refetchRemoteTask(remoteTask);
-        assertEquals(completion, localTask.getValue(Task.COMPLETION_DATE).longValue());
+        assertTrue(String.format("Expected %s, was %s", new Date(completion), new Date(localTask.getValue(Task.COMPLETION_DATE))),
-        assertEquals(GtasksApiUtilities.unixTimeToGtasksTime(completion), GtasksApiUtilities.gtasksCompletedTimeStringToLocalTimeString(remoteTask.completed));
+                Math.abs(completion - localTask.getValue(Task.COMPLETION_DATE)) < 5000);
         assertEquals("completed", remoteTask.status);
     }
 

tests/src/com/todoroo/astrid/repeats/RepeatTestsGtasksSyncRemote.java

@@ -0,0 +1,49 @@
+package com.todoroo.astrid.repeats;
+
+import java.io.IOException;
+import java.util.Date;
+
+import com.google.ical.values.Frequency;
+import com.google.ical.values.RRule;
+import com.todoroo.andlib.utility.DateUtilities;
+import com.todoroo.astrid.data.Task;
+import com.todoroo.astrid.gtasks.api.GtasksApiUtilities;
+
+public class RepeatTestsGtasksSyncRemote extends RepeatTestsGtasksSync {
+
+    // Test logic in superclass
+
+    @Override
+    protected long setCompletionDate(boolean completeBefore, Task t,
+            com.google.api.services.tasks.v1.model.Task remoteModel, long dueDate) {
+        long completionDate;
+        if (completeBefore)
+            completionDate = dueDate - DateUtilities.ONE_DAY;
+        else
+            completionDate = dueDate + DateUtilities.ONE_DAY;
+        remoteModel.completed = GtasksApiUtilities.unixTimeToGtasksCompletionTime(completionDate);
+        remoteModel.status = "completed";
+        try {
+            gtasksService.updateGtask(GtasksApiUtilities.extractListIdFromSelfLink(remoteModel), remoteModel);
+        } catch (IOException e) {
+            e.printStackTrace();
+            fail("Exception in gtasks service");
+        }
+        return completionDate;
+    }
+    @Override
+    protected long computeNextDueDateFromDate(long fromDate, RRule rrule, boolean fromCompletion) {
+        long expectedDate = super.computeNextDueDateFromDate(fromDate, rrule, fromCompletion);
+        Frequency freq = rrule.getFreq();
+        if (fromCompletion && (freq == Frequency.HOURLY || freq == Frequency.MINUTELY)) {
+            long millis = (freq == Frequency.HOURLY ? DateUtilities.ONE_HOUR : DateUtilities.ONE_MINUTE);
+            Date rounded = new Date(expectedDate);
+            rounded.setHours(0);
+            rounded.setMinutes(0);
+            rounded.setSeconds(0);
+            return rounded.getTime() + rrule.getInterval() * millis;
+        } else {
+            return expectedDate;
+        }
+    }
+}