From 29d50022a57f30d4e0bbf889d52d97a41ab41bf3 Mon Sep 17 00:00:00 2001 From: Sam Bosley Date: Fri, 16 Nov 2012 12:25:40 -0800 Subject: [PATCH] Refactored message instantiation, simplified enqueueing messages --- .../astrid/actfm/sync/ActFmSyncThread.java | 53 +++++++++---------- .../actfm/sync/SyncDatabaseListener.java | 9 ++-- .../astrid/actfm/sync/messages/BriefMe.java | 6 +++ .../actfm/sync/messages/ChangesHappened.java | 21 +++++++- .../sync/messages/ClientToServerMessage.java | 20 ------- 5 files changed, 55 insertions(+), 54 deletions(-) diff --git a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncThread.java b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncThread.java index 36258ff52..3854264fe 100644 --- a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncThread.java +++ b/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> changesQueue; + private final List> pendingMessages; private final Object monitor; private Thread thread; @@ -45,7 +42,7 @@ public class ActFmSyncThread { } public static ActFmSyncThread initializeSyncComponents(TaskDao taskDao, TagDataDao tagDataDao) { - List> syncQueue = Collections.synchronizedList(new LinkedList>()); + List> syncQueue = Collections.synchronizedList(new LinkedList>()); ActFmSyncMonitor monitor = ActFmSyncMonitor.getInstance(); taskDao.addListener(new SyncDatabaseListener(syncQueue, monitor, ModelType.TYPE_TASK)); @@ -56,9 +53,9 @@ public class ActFmSyncThread { return thread; } - public ActFmSyncThread(List> queue, Object syncMonitor) { + private ActFmSyncThread(List> 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> messages = new LinkedList>(); + List> messageBatch = new LinkedList>(); 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()) { - Pair tuple = changesQueue.remove(0); - if (tuple != null) { - ChangesHappened changes = ClientToServerMessage.instantiateChangesHappened(tuple.getLeft(), tuple.getRight()); - if (changes != null) - messages.add(changes); - } + while (messageBatch.size() < batchSize && !pendingMessages.isEmpty()) { + ClientToServerMessage message = pendingMessages.remove(0); + if (message != null) + messageBatch.add(message); } - if (messages.isEmpty() && timeForBackgroundSync()) { - messages.add(instantiateBriefMe(Task.class)); - messages.add(instantiateBriefMe(TagData.class)); + if (messageBatch.isEmpty() && timeForBackgroundSync()) { + messageBatch.add(BriefMe.instantiateBriefMeForClass(Task.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>(); + messageBatch = new LinkedList>(); } } } catch (Exception e) { @@ -147,12 +148,6 @@ public class ActFmSyncThread { } - private BriefMe instantiateBriefMe(Class cls) { - // TODO: compute last pushed at value for model class - long pushedAt = 0; - return new BriefMe(cls, null, pushedAt); - } - private boolean timeForBackgroundSync() { return true; } diff --git a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/SyncDatabaseListener.java b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/SyncDatabaseListener.java index 136824fda..4ecaa9a31 100644 --- a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/SyncDatabaseListener.java +++ b/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 implements ModelUpdateListener { - private final List> queue; + private final List> queue; private final Object monitor; private final ModelType modelType; - public SyncDatabaseListener(List> queue, Object syncMonitor, ModelType modelType) { + public SyncDatabaseListener(List> queue, Object syncMonitor, ModelType modelType) { this.queue = queue; this.monitor = syncMonitor; this.modelType = modelType; @@ -21,7 +22,7 @@ public class SyncDatabaseListener 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(); } diff --git a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/messages/BriefMe.java b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/messages/BriefMe.java index 99979cf8f..dd29b0d9a 100644 --- a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/messages/BriefMe.java +++ b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/messages/BriefMe.java @@ -8,6 +8,12 @@ import com.todoroo.astrid.data.RemoteModel; public class BriefMe extends ClientToServerMessage { + public static BriefMe instantiateBriefMeForClass(Class cls) { + // TODO: compute last pushed at value for model class + long pushedAt = 0; + return new BriefMe(cls, null, pushedAt); + } + public BriefMe(long id, Class modelClass, RemoteModelDao modelDao) { super(id, modelClass, modelDao); } diff --git a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/messages/ChangesHappened.java b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/messages/ChangesHappened.java index e18518099..89dd4054b 100644 --- a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/messages/ChangesHappened.java +++ b/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> extends ClientToServerMessage { @@ -30,7 +36,20 @@ public class ChangesHappened modelClass, RemoteModelDao modelDao, + public static ChangesHappened instantiateChangesHappened(Long id, ModelType modelType) { + switch(modelType) { + case TYPE_TASK: + return new ChangesHappened(id, Task.class, + PluginServices.getTaskDao(), PluginServices.getTaskOutstandingDao()); + case TYPE_TAG: + return new ChangesHappened(id, TagData.class, + PluginServices.getTagDataDao(), PluginServices.getTagOutstandingDao()); + default: + return null; + } + } + + private ChangesHappened(long id, Class modelClass, RemoteModelDao modelDao, OutstandingEntryDao outstandingDao) { super(id, modelClass, modelDao); diff --git a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/messages/ClientToServerMessage.java b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/messages/ClientToServerMessage.java index 58d3b5a06..5694178ad 100644 --- a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/messages/ClientToServerMessage.java +++ b/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 { @@ -82,18 +76,4 @@ public abstract class ClientToServerMessage { 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(id, Task.class, - PluginServices.getTaskDao(), PluginServices.getTaskOutstandingDao()); - case TYPE_TAG: - return new ChangesHappened(id, TagData.class, - PluginServices.getTagDataDao(), PluginServices.getTagOutstandingDao()); - default: - return null; - } - } - }