Refactored message instantiation, simplified enqueueing messages

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

@@ -12,14 +12,11 @@ import android.util.Log;
 
 import com.todoroo.andlib.service.Autowired;
 import com.todoroo.andlib.service.DependencyInjectionService;
-import com.todoroo.andlib.utility.Pair;
 import com.todoroo.astrid.actfm.sync.messages.BriefMe;
-import com.todoroo.astrid.actfm.sync.messages.ChangesHappened;
 import com.todoroo.astrid.actfm.sync.messages.ClientToServerMessage;
 import com.todoroo.astrid.actfm.sync.messages.ServerToClientMessage;
 import com.todoroo.astrid.dao.TagDataDao;
 import com.todoroo.astrid.dao.TaskDao;
-import com.todoroo.astrid.data.RemoteModel;
 import com.todoroo.astrid.data.TagData;
 import com.todoroo.astrid.data.Task;
 
@@ -27,7 +24,7 @@ public class ActFmSyncThread {
 
     private static final String ERROR_TAG = "actfm-sync-thread"; //$NON-NLS-1$
 
-    private final List<Pair<Long, ModelType>> changesQueue;
+    private final List<ClientToServerMessage<?>> pendingMessages;
     private final Object monitor;
     private Thread thread;
 
@@ -45,7 +42,7 @@ public class ActFmSyncThread {
     }
 
     public static ActFmSyncThread initializeSyncComponents(TaskDao taskDao, TagDataDao tagDataDao) {
-        List<Pair<Long, ModelType>> syncQueue = Collections.synchronizedList(new LinkedList<Pair<Long, ModelType>>());
+        List<ClientToServerMessage<?>> syncQueue = Collections.synchronizedList(new LinkedList<ClientToServerMessage<?>>());
         ActFmSyncMonitor monitor = ActFmSyncMonitor.getInstance();
 
         taskDao.addListener(new SyncDatabaseListener<Task>(syncQueue, monitor, ModelType.TYPE_TASK));
@@ -56,9 +53,9 @@ public class ActFmSyncThread {
         return thread;
     }
 
-    public ActFmSyncThread(List<Pair<Long, ModelType>> queue, Object syncMonitor) {
+    private ActFmSyncThread(List<ClientToServerMessage<?>> messageQueue, Object syncMonitor) {
         DependencyInjectionService.getInstance().inject(this);
-        this.changesQueue = queue;
+        this.pendingMessages = messageQueue;
         this.monitor = syncMonitor;
     }
 
@@ -74,14 +71,21 @@ public class ActFmSyncThread {
         }
     }
 
+    public void enqueueMessage(ClientToServerMessage<?> message) {
+        pendingMessages.add(message);
+        synchronized(monitor) {
+            monitor.notifyAll();
+        }
+    }
+
     @SuppressWarnings("nls")
     private void sync() {
         try {
             int batchSize = 1;
-            List<ClientToServerMessage<?>> messages = new LinkedList<ClientToServerMessage<?>>();
+            List<ClientToServerMessage<?>> messageBatch = new LinkedList<ClientToServerMessage<?>>();
             while(true) {
                 synchronized(monitor) {
-                    while (changesQueue.isEmpty() && !timeForBackgroundSync()) {
+                    while (pendingMessages.isEmpty() && !timeForBackgroundSync()) {
                         try {
                             monitor.wait();
                         } catch (InterruptedException e) {
@@ -91,23 +95,20 @@ public class ActFmSyncThread {
                 }
 
                 // Stuff in the document
-                while (messages.size() < batchSize && !changesQueue.isEmpty()) {
+                while (messageBatch.size() < batchSize && !pendingMessages.isEmpty()) {
-                    Pair<Long, ModelType> tuple = changesQueue.remove(0);
+                    ClientToServerMessage<?> message = pendingMessages.remove(0);
-                    if (tuple != null) {
+                    if (message != null)
-                        ChangesHappened<?, ?> changes = ClientToServerMessage.instantiateChangesHappened(tuple.getLeft(), tuple.getRight());
+                        messageBatch.add(message);
-                        if (changes != null)
-                            messages.add(changes);
-                    }
                 }
 
-                if (messages.isEmpty() && timeForBackgroundSync()) {
+                if (messageBatch.isEmpty() && timeForBackgroundSync()) {
-                    messages.add(instantiateBriefMe(Task.class));
+                    messageBatch.add(BriefMe.instantiateBriefMeForClass(Task.class));
-                    messages.add(instantiateBriefMe(TagData.class));
+                    messageBatch.add(BriefMe.instantiateBriefMeForClass(TagData.class));
                 }
 
-                if (!messages.isEmpty() && checkForToken()) {
+                if (!messageBatch.isEmpty() && checkForToken()) {
                     JSONArray payload = new JSONArray();
-                    for (ClientToServerMessage<?> message : messages) {
+                    for (ClientToServerMessage<?> message : messageBatch) {
                         JSONObject serialized = message.serializeToJSON();
                         if (serialized != null)
                             payload.put(serialized);
@@ -131,11 +132,11 @@ public class ActFmSyncThread {
                             }
                         }
 
-                        batchSize = Math.min(batchSize, messages.size()) * 2;
+                        batchSize = Math.min(batchSize, messageBatch.size()) * 2;
                     } catch (IOException e) {
                         batchSize = Math.max(batchSize / 2, 1);
                     }
-                    messages = new LinkedList<ClientToServerMessage<?>>();
+                    messageBatch = new LinkedList<ClientToServerMessage<?>>();
                 }
             }
         } catch (Exception e) {
@@ -147,12 +148,6 @@ public class ActFmSyncThread {
 
     }
 
-    private <TYPE extends RemoteModel> BriefMe<TYPE> instantiateBriefMe(Class<TYPE> cls) {
-        // TODO: compute last pushed at value for model class
-        long pushedAt = 0;
-        return new BriefMe<TYPE>(cls, null, pushedAt);
-    }
-
     private boolean timeForBackgroundSync() {
         return true;
     }

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

@@ -3,17 +3,18 @@ package com.todoroo.astrid.actfm.sync;
 import java.util.List;
 
 import com.todoroo.andlib.data.DatabaseDao.ModelUpdateListener;
-import com.todoroo.andlib.utility.Pair;
 import com.todoroo.astrid.actfm.sync.ActFmSyncThread.ModelType;
+import com.todoroo.astrid.actfm.sync.messages.ChangesHappened;
+import com.todoroo.astrid.actfm.sync.messages.ClientToServerMessage;
 import com.todoroo.astrid.data.RemoteModel;
 
 public class SyncDatabaseListener<MTYPE extends RemoteModel> implements ModelUpdateListener<MTYPE> {
 
-    private final List<Pair<Long, ModelType>> queue;
+    private final List<ClientToServerMessage<?>> queue;
     private final Object monitor;
     private final ModelType modelType;
 
-    public SyncDatabaseListener(List<Pair<Long, ModelType>> queue, Object syncMonitor, ModelType modelType) {
+    public SyncDatabaseListener(List<ClientToServerMessage<?>> queue, Object syncMonitor, ModelType modelType) {
         this.queue = queue;
         this.monitor = syncMonitor;
         this.modelType = modelType;
@@ -21,7 +22,7 @@ public class SyncDatabaseListener<MTYPE extends RemoteModel> implements ModelUpd
 
     @Override
     public void onModelUpdated(MTYPE model) {
-        queue.add(Pair.create(model.getId(), modelType));
+        queue.add(ChangesHappened.instantiateChangesHappened(model.getId(), modelType));
         synchronized(monitor) {
             monitor.notifyAll();
         }

astrid/plugin-src/com/todoroo/astrid/actfm/sync/messages/BriefMe.java

@@ -8,6 +8,12 @@ import com.todoroo.astrid.data.RemoteModel;
 
 public class BriefMe<TYPE extends RemoteModel> extends ClientToServerMessage<TYPE> {
 
+    public static <TYPE extends RemoteModel> BriefMe<TYPE> instantiateBriefMeForClass(Class<TYPE> cls) {
+        // TODO: compute last pushed at value for model class
+        long pushedAt = 0;
+        return new BriefMe<TYPE>(cls, null, pushedAt);
+    }
+
     public BriefMe(long id, Class<TYPE> modelClass, RemoteModelDao<TYPE> modelDao) {
         super(id, modelClass, modelDao);
     }

astrid/plugin-src/com/todoroo/astrid/actfm/sync/messages/ChangesHappened.java

@@ -14,11 +14,17 @@ import com.todoroo.andlib.data.Property;
 import com.todoroo.andlib.data.TodorooCursor;
 import com.todoroo.andlib.sql.Order;
 import com.todoroo.andlib.sql.Query;
+import com.todoroo.astrid.actfm.sync.ActFmSyncThread.ModelType;
+import com.todoroo.astrid.core.PluginServices;
 import com.todoroo.astrid.dao.DaoReflectionHelpers;
 import com.todoroo.astrid.dao.OutstandingEntryDao;
 import com.todoroo.astrid.dao.RemoteModelDao;
 import com.todoroo.astrid.data.OutstandingEntry;
 import com.todoroo.astrid.data.RemoteModel;
+import com.todoroo.astrid.data.TagData;
+import com.todoroo.astrid.data.TagOutstanding;
+import com.todoroo.astrid.data.Task;
+import com.todoroo.astrid.data.TaskOutstanding;
 
 @SuppressWarnings("nls")
 public class ChangesHappened<TYPE extends RemoteModel, OE extends OutstandingEntry<TYPE>> extends ClientToServerMessage<TYPE> {
@@ -30,7 +36,20 @@ public class ChangesHappened<TYPE extends RemoteModel, OE extends OutstandingEnt
 
     public static final String CHANGES_KEY = "changes";
 
-    public ChangesHappened(long id, Class<TYPE> modelClass, RemoteModelDao<TYPE> modelDao,
+    public static ChangesHappened<?, ?> instantiateChangesHappened(Long id, ModelType modelType) {
+        switch(modelType) {
+        case TYPE_TASK:
+            return new ChangesHappened<Task, TaskOutstanding>(id, Task.class,
+                    PluginServices.getTaskDao(), PluginServices.getTaskOutstandingDao());
+        case TYPE_TAG:
+            return new ChangesHappened<TagData, TagOutstanding>(id, TagData.class,
+                    PluginServices.getTagDataDao(), PluginServices.getTagOutstandingDao());
+        default:
+            return null;
+        }
+    }
+
+    private ChangesHappened(long id, Class<TYPE> modelClass, RemoteModelDao<TYPE> modelDao,
             OutstandingEntryDao<OE> outstandingDao) {
         super(id, modelClass, modelDao);
 

astrid/plugin-src/com/todoroo/astrid/actfm/sync/messages/ClientToServerMessage.java

@@ -5,15 +5,9 @@ import org.json.JSONObject;
 
 import com.todoroo.andlib.data.AbstractModel;
 import com.todoroo.andlib.data.Table;
-import com.todoroo.astrid.actfm.sync.ActFmSyncThread.ModelType;
-import com.todoroo.astrid.core.PluginServices;
 import com.todoroo.astrid.dao.DaoReflectionHelpers;
 import com.todoroo.astrid.dao.RemoteModelDao;
 import com.todoroo.astrid.data.RemoteModel;
-import com.todoroo.astrid.data.TagData;
-import com.todoroo.astrid.data.TagOutstanding;
-import com.todoroo.astrid.data.Task;
-import com.todoroo.astrid.data.TaskOutstanding;
 
 @SuppressWarnings("nls")
 public abstract class ClientToServerMessage<TYPE extends RemoteModel> {
@@ -82,18 +76,4 @@ public abstract class ClientToServerMessage<TYPE extends RemoteModel> {
 
     protected abstract void serializeToJSONImpl(JSONObject serializeTo) throws JSONException;
     protected abstract String getTypeString();
-
-    public static ChangesHappened<?, ?> instantiateChangesHappened(Long id, ModelType modelType) {
-        switch(modelType) {
-        case TYPE_TASK:
-            return new ChangesHappened<Task, TaskOutstanding>(id, Task.class,
-                    PluginServices.getTaskDao(), PluginServices.getTaskOutstandingDao());
-        case TYPE_TAG:
-            return new ChangesHappened<TagData, TagOutstanding>(id, TagData.class,
-                    PluginServices.getTagDataDao(), PluginServices.getTagOutstandingDao());
-        default:
-            return null;
-        }
-    }
-
 }