Merge pull request #166 from sbosley/120319_sb_sync_conflict_merge

Resolve sync conflicts by choosing side with more recently modified

astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncService.java

@@ -428,11 +428,12 @@ public final class ActFmSyncService {
         } catch (JSONException e) {
             handleException("task-save-json", e);
         } catch (IOException e) {
-            if (notPermanentError(e))
+            if (notPermanentError(e)) {
                 addFailedPush(new FailedPush(PUSH_TYPE_TASK, task.getId()));
-            else
+            } else {
-            handleException("task-save-io", e);
+                handleException("task-save-io", e);
-            task.setValue(Task.LAST_SYNC, DateUtilities.now() + 1000L);
+                task.setValue(Task.LAST_SYNC, DateUtilities.now() + 1000L);
+            }
         }
 
         task.putTransitory(SyncFlags.ACTFM_SUPPRESS_SYNC, true);
@@ -445,7 +446,7 @@ public final class ActFmSyncService {
      */
     public void pushTask(long taskId) {
         Task task = taskService.fetchById(taskId, Task.PROPERTIES);
-        if (task != null)
+        if (task != null && task.getValue(Task.MODIFICATION_DATE) > task.getValue(Task.LAST_SYNC))
             pushTaskOnSave(task, task.getMergedValues());
     }
 
@@ -608,7 +609,7 @@ public final class ActFmSyncService {
     public void fetchTagDataDashboard(boolean manual, final Runnable done) {
         invokeFetchList("goal", manual, null, new ListItemProcessor<TagData>() {
             @Override
-            protected void mergeAndSave(JSONArray list, HashMap<Long,Long> locals) throws JSONException {
+            protected void mergeAndSave(JSONArray list, HashMap<Long,Long> locals, long serverTime) throws JSONException {
                 TagData remote = new TagData();
                 for(int i = 0; i < list.length(); i++) {
                     JSONObject item = list.getJSONObject(i);
@@ -716,57 +717,7 @@ public final class ActFmSyncService {
      * @param done
      */
     public void fetchActiveTasks(final boolean manual, SyncExceptionHandler handler, Runnable done) {
-        invokeFetchList("task", manual, handler, new ListItemProcessor<Task>() {
+        invokeFetchList("task", manual, handler, new TaskListItemProcessor(manual), done, "active_tasks");
-            @Override
-            protected void mergeAndSave(JSONArray list, HashMap<Long,Long> locals) throws JSONException {
-                Task remote = new Task();
-
-                ArrayList<Metadata> metadata = new ArrayList<Metadata>();
-                HashSet<Long> ids = new HashSet<Long>(list.length());
-                for(int i = 0; i < list.length(); i++) {
-                    JSONObject item = list.getJSONObject(i);
-                    readIds(locals, item, remote);
-                    JsonHelper.taskFromJson(item, remote, metadata);
-
-                    if(remote.getValue(Task.USER_ID) == 0) {
-                        if(!remote.isSaved())
-                            StatisticsService.reportEvent(StatisticsConstants.ACTFM_TASK_CREATED);
-                        else if(remote.isCompleted())
-                            StatisticsService.reportEvent(StatisticsConstants.ACTFM_TASK_COMPLETED);
-                    }
-
-                    if(!remote.isSaved() && remote.hasDueDate() &&
-                            remote.getValue(Task.DUE_DATE) < DateUtilities.now())
-                        remote.setFlag(Task.REMINDER_FLAGS, Task.NOTIFY_AFTER_DEADLINE, false);
-
-                    remote.putTransitory(SyncFlags.ACTFM_SUPPRESS_SYNC, true);
-                    taskService.save(remote);
-                    ids.add(remote.getId());
-                    metadataService.synchronizeMetadata(remote.getId(), metadata, MetadataCriteria.withKey(TagService.KEY));
-                    remote.clear();
-                }
-
-                if(manual) {
-                    Long[] localIds = ids.toArray(new Long[ids.size()]);
-                    taskService.deleteWhere(Criterion.and(TaskCriteria.activeAndVisible(),
-                            Task.REMOTE_ID.isNotNull(),
-                            Criterion.not(Task.ID.in(localIds))));
-                }
-            }
-
-            @Override
-            protected HashMap<Long, Long> getLocalModels() {
-                TodorooCursor<Task> cursor = taskService.query(Query.select(Task.ID,
-                        Task.REMOTE_ID).where(Task.REMOTE_ID.in(remoteIds)).orderBy(
-                                Order.asc(Task.REMOTE_ID)));
-                return cursorToMap(cursor, taskDao, Task.REMOTE_ID, Task.ID);
-            }
-
-            @Override
-            protected Class<Task> typeClass() {
-                return Task.class;
-            }
-        }, done, "active_tasks");
     }
 
     /**
@@ -776,54 +727,14 @@ public final class ActFmSyncService {
      * @param done
      */
     public void fetchTasksForTag(final TagData tagData, final boolean manual, Runnable done) {
-        invokeFetchList("task", manual, null, new ListItemProcessor<Task>() {
+        invokeFetchList("task", manual, null, new TaskListItemProcessor(manual) {
-            @Override
-            protected void mergeAndSave(JSONArray list, HashMap<Long,Long> locals) throws JSONException {
-                Task remote = new Task();
-
-                ArrayList<Metadata> metadata = new ArrayList<Metadata>();
-                HashSet<Long> ids = new HashSet<Long>(list.length());
-                for(int i = 0; i < list.length(); i++) {
-
-                    JSONObject item = list.getJSONObject(i);
-                    readIds(locals, item, remote);
-                    JsonHelper.taskFromJson(item, remote, metadata);
-
-                    if(remote.getValue(Task.USER_ID) == 0) {
-                        if(!remote.isSaved())
-                            StatisticsService.reportEvent(StatisticsConstants.ACTFM_TASK_CREATED);
-                        else if(remote.isCompleted())
-                            StatisticsService.reportEvent(StatisticsConstants.ACTFM_TASK_COMPLETED);
-                    }
-
-                    remote.putTransitory(SyncFlags.ACTFM_SUPPRESS_SYNC, true);
-                    taskService.save(remote);
-                    ids.add(remote.getId());
-                    metadataService.synchronizeMetadata(remote.getId(), metadata, MetadataCriteria.withKey(TagService.KEY));
-                    remote.clear();
-                }
-
-                if(manual) {
-                    Long[] localIds = ids.toArray(new Long[ids.size()]);
-                    taskService.deleteWhere(Criterion.and(
-                            TagService.memberOfTagData(tagData.getValue(TagData.REMOTE_ID)),
-                            TaskCriteria.activeAndVisible(),
-                            Task.REMOTE_ID.isNotNull(),
-                            Criterion.not(Task.ID.in(localIds))));
-                }
-            }
-
-            @Override
-            protected HashMap<Long, Long> getLocalModels() {
-                TodorooCursor<Task> cursor = taskService.query(Query.select(Task.ID,
-                        Task.REMOTE_ID).where(Task.REMOTE_ID.in(remoteIds)).orderBy(
-                                Order.asc(Task.REMOTE_ID)));
-                return cursorToMap(cursor, taskDao, Task.REMOTE_ID, Task.ID);
-            }
-
             @Override
-            protected Class<Task> typeClass() {
+            protected void deleteExtras(Long[] localIds) {
-                return Task.class;
+                taskService.deleteWhere(Criterion.and(
+                        TagService.memberOfTagData(tagData.getValue(TagData.REMOTE_ID)),
+                        TaskCriteria.activeAndVisible(),
+                        Task.REMOTE_ID.isNotNull(),
+                        Criterion.not(Task.ID.in(localIds))));
             }
         }, done, "tasks:" + tagData.getId(), "tag_id", tagData.getValue(TagData.REMOTE_ID));
     }
@@ -932,7 +843,7 @@ public final class ActFmSyncService {
 
     private class UpdateListItemProcessor extends ListItemProcessor<Update> {
         @Override
-        protected void mergeAndSave(JSONArray list, HashMap<Long,Long> locals) throws JSONException {
+        protected void mergeAndSave(JSONArray list, HashMap<Long,Long> locals, long serverTime) throws JSONException {
             Update remote = new Update();
             for(int i = 0; i < list.length(); i++) {
                 JSONObject item = list.getJSONObject(i);
@@ -1016,13 +927,13 @@ public final class ActFmSyncService {
         abstract protected Class<TYPE> typeClass();
 
         abstract protected void mergeAndSave(JSONArray list,
-                HashMap<Long,Long> locals) throws JSONException;
+                HashMap<Long,Long> locals, long serverTime) throws JSONException;
 
-        public void process(JSONArray list) throws JSONException {
+        public void process(JSONArray list, long serverTime) throws JSONException {
             readRemoteIds(list);
             synchronized (typeClass()) {
                 HashMap<Long, Long> locals = getLocalModels();
-                mergeAndSave(list, locals);
+                mergeAndSave(list, locals, serverTime);
             }
         }
 
@@ -1062,6 +973,89 @@ public final class ActFmSyncService {
 
     }
 
+    private class TaskListItemProcessor extends ListItemProcessor<Task> {
+
+        private final boolean deleteExtras;
+        private final HashMap<Long, Long> modificationDates;
+
+        public TaskListItemProcessor(boolean deleteExtras) {
+            this.deleteExtras = deleteExtras;
+            this.modificationDates = new HashMap<Long, Long>();
+        }
+
+        @Override
+        protected void mergeAndSave(JSONArray list, HashMap<Long,Long> locals, long serverTime) throws JSONException {
+            Task remote = new Task();
+
+            ArrayList<Metadata> metadata = new ArrayList<Metadata>();
+            HashSet<Long> ids = new HashSet<Long>(list.length());
+
+            long timeDelta = serverTime == 0 ? 0 : DateUtilities.now() - serverTime * 1000;
+
+            for(int i = 0; i < list.length(); i++) {
+                JSONObject item = list.getJSONObject(i);
+                readIds(locals, item, remote);
+
+                long serverModificationDate = item.optLong("updated_at") * 1000;
+                if (serverModificationDate > 0 && modificationDates.containsKey(remote.getId())
+                        && serverModificationDate < (modificationDates.get(remote.getId()) - timeDelta)) {
+                    ids.add(remote.getId());
+                    continue; // Modified locally more recently than remotely -- don't overwrite changes
+                }
+
+                JsonHelper.taskFromJson(item, remote, metadata);
+
+                if(remote.getValue(Task.USER_ID) == 0) {
+                    if(!remote.isSaved())
+                        StatisticsService.reportEvent(StatisticsConstants.ACTFM_TASK_CREATED);
+                    else if(remote.isCompleted())
+                        StatisticsService.reportEvent(StatisticsConstants.ACTFM_TASK_COMPLETED);
+                }
+
+                if(!remote.isSaved() && remote.hasDueDate() &&
+                        remote.getValue(Task.DUE_DATE) < DateUtilities.now())
+                    remote.setFlag(Task.REMINDER_FLAGS, Task.NOTIFY_AFTER_DEADLINE, false);
+
+                remote.putTransitory(SyncFlags.ACTFM_SUPPRESS_SYNC, true);
+                taskService.save(remote);
+                ids.add(remote.getId());
+                metadataService.synchronizeMetadata(remote.getId(), metadata, MetadataCriteria.withKey(TagService.KEY));
+                remote.clear();
+            }
+
+            if(deleteExtras) {
+                Long[] localIds = ids.toArray(new Long[ids.size()]);
+                deleteExtras(localIds);
+            }
+        }
+
+        protected void deleteExtras(Long[] localIds) {
+            taskService.deleteWhere(Criterion.and(TaskCriteria.activeAndVisible(),
+                    Task.REMOTE_ID.isNotNull(),
+                    Criterion.not(Task.ID.in(localIds))));
+        }
+
+        @Override
+        protected HashMap<Long, Long> getLocalModels() {
+            TodorooCursor<Task> cursor = taskService.query(Query.select(Task.ID, Task.MODIFICATION_DATE,
+                    Task.REMOTE_ID).where(Task.REMOTE_ID.in(remoteIds)).orderBy(
+                            Order.asc(Task.REMOTE_ID)));
+            Task task = new Task();
+            for (cursor.moveToFirst(); !cursor.isAfterLast(); cursor.moveToNext()) {
+                task.readFromCursor(cursor);
+                modificationDates.put(task.getId(), task.getValue(Task.MODIFICATION_DATE));
+            }
+
+            return cursorToMap(cursor, taskDao, Task.REMOTE_ID, Task.ID);
+        }
+
+        @Override
+        protected Class<Task> typeClass() {
+            return Task.class;
+        }
+
+    }
+
     /** Call sync method */
     private void invokeFetchList(final String model, final boolean manual, final SyncExceptionHandler handler,
             final ListItemProcessor<?> processor, final Runnable done, final String lastSyncKey,
@@ -1079,9 +1073,10 @@ public final class ActFmSyncService {
                 JSONObject result = null;
                 try {
                     result = actFmInvoker.invoke(model + "_list", getParams);
+                    long serverTime = result.optLong("time", 0);
                     JSONArray list = result.getJSONArray("list");
-                    processor.process(list);
+                    processor.process(list, serverTime);
-                    Preferences.setLong("actfm_time_" + lastSyncKey, result.optLong("time", 0));
+                    Preferences.setLong("actfm_time_" + lastSyncKey, serverTime);
                     Preferences.setLong("actfm_last_" + lastSyncKey, DateUtilities.now());
 
                     if(done != null)

astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncV2Provider.java

@@ -17,9 +17,11 @@ import com.todoroo.andlib.data.TodorooCursor;
 import com.todoroo.andlib.service.Autowired;
 import com.todoroo.andlib.service.ContextManager;
 import com.todoroo.andlib.sql.Criterion;
+import com.todoroo.andlib.sql.Join;
 import com.todoroo.andlib.sql.Query;
 import com.todoroo.andlib.utility.Preferences;
 import com.todoroo.astrid.dao.TaskDao.TaskCriteria;
+import com.todoroo.astrid.data.Metadata;
 import com.todoroo.astrid.data.RemoteModel;
 import com.todoroo.astrid.data.TagData;
 import com.todoroo.astrid.data.Task;
@@ -306,6 +308,8 @@ public class ActFmSyncV2Provider extends SyncV2Provider {
         actFmSyncService.fetchTasksForTag(tagData, manual, new Runnable() {
             @Override
             public void run() {
+                pushQueuedTasksByTag(tagData, callback, finisher);
+
                 callback.incrementProgress(30);
                 if(finisher.decrementAndGet() == 0)
                     callback.finished();
@@ -313,4 +317,15 @@ public class ActFmSyncV2Provider extends SyncV2Provider {
         });
     }
 
+    private void pushQueuedTasksByTag(TagData tagData, SyncResultCallback callback, AtomicInteger finisher) {
+        TodorooCursor<Task> taskCursor = taskService.query(Query.select(Task.PROPERTIES)
+                .join(Join.inner(Metadata.TABLE, Criterion.and(Metadata.KEY.eq(TagService.KEY), Metadata.TASK.eq(Task.ID), TagService.TAG.eq(tagData.getId()))))
+                .where(Criterion.or(
+                        Criterion.and(TaskCriteria.isActive(),
+                                Task.REMOTE_ID.isNull()),
+                        Criterion.and(Task.REMOTE_ID.isNotNull(),
+                                Task.MODIFICATION_DATE.gt(Task.LAST_SYNC)))));
+        pushQueued(callback, finisher, taskCursor, false, taskPusher);
+    }
+
 }