From 35a44428c5a729a3d6e6c5acb2decf05784f792b Mon Sep 17 00:00:00 2001 From: Sam Bosley Date: Wed, 6 Feb 2013 11:18:01 -0800 Subject: [PATCH] Don't use a flag for indicating ongoing sync migration. Reading it from a preference fixes an issue where the sync thread could run before the migration had started --- .../todoroo/astrid/actfm/sync/ActFmSyncThread.java | 12 +++++++++--- .../astrid/actfm/sync/AstridNewSyncMigrator.java | 14 +++++++++++--- .../com/todoroo/astrid/service/StartupService.java | 3 +++ .../com/todoroo/astrid/service/UpgradeService.java | 2 +- astrid/src/com/todoroo/astrid/utility/Flags.java | 6 ------ 5 files changed, 24 insertions(+), 13 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 340b95f5a..896f3f248 100644 --- a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncThread.java +++ b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/ActFmSyncThread.java @@ -19,6 +19,7 @@ import com.todoroo.andlib.service.ContextManager; import com.todoroo.andlib.service.DependencyInjectionService; import com.todoroo.andlib.sql.Query; import com.todoroo.andlib.utility.AndroidUtilities; +import com.todoroo.andlib.utility.Preferences; import com.todoroo.astrid.actfm.sync.messages.BriefMe; import com.todoroo.astrid.actfm.sync.messages.ChangesHappened; import com.todoroo.astrid.actfm.sync.messages.ClientToServerMessage; @@ -43,7 +44,6 @@ import com.todoroo.astrid.data.Task; import com.todoroo.astrid.data.TaskOutstanding; import com.todoroo.astrid.data.UserActivity; import com.todoroo.astrid.data.UserActivityOutstanding; -import com.todoroo.astrid.utility.Flags; public class ActFmSyncThread { @@ -80,6 +80,8 @@ public class ActFmSyncThread { private String token; + private boolean syncMigration = false; + public static enum ModelType { TYPE_TASK, TYPE_TAG, @@ -124,6 +126,7 @@ public class ActFmSyncThread { this.pendingMessages = messageQueue; this.pendingCallbacks = Collections.synchronizedMap(new HashMap, Runnable>()); this.monitor = syncMonitor; + this.syncMigration = Preferences.getBoolean(AstridNewSyncMigrator.PREF_SYNC_MIGRATION, false); } public synchronized void startSyncThread() { @@ -157,10 +160,13 @@ public class ActFmSyncThread { List> messageBatch = new LinkedList>(); while(true) { synchronized(monitor) { - while ((pendingMessages.isEmpty() && !timeForBackgroundSync()) || !actFmPreferenceService.isLoggedIn()) { + while ((pendingMessages.isEmpty() && !timeForBackgroundSync()) || !actFmPreferenceService.isLoggedIn() || !syncMigration) { try { monitor.wait(); AndroidUtilities.sleepDeep(500L); // Wait briefly for large database operations to finish (e.g. adding a task with several tags may trigger a message before all saves are done--fix this?) + + if (!syncMigration) + syncMigration = Preferences.getBoolean(AstridNewSyncMigrator.PREF_SYNC_MIGRATION, false); } catch (InterruptedException e) { // Ignored } @@ -257,7 +263,7 @@ public class ActFmSyncThread { } private boolean timeForBackgroundSync() { - return !Flags.check(Flags.SYNC_MIGRATION_ONGOING) && false; // TODO: replace && false with a real background sync condition + return false; // TODO: replace false with a real background sync condition } private void repopulateQueueFromOutstandingTables() { diff --git a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/AstridNewSyncMigrator.java b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/AstridNewSyncMigrator.java index d7a25e16d..a1a72ebd2 100644 --- a/astrid/plugin-src/com/todoroo/astrid/actfm/sync/AstridNewSyncMigrator.java +++ b/astrid/plugin-src/com/todoroo/astrid/actfm/sync/AstridNewSyncMigrator.java @@ -11,6 +11,7 @@ import com.todoroo.andlib.service.DependencyInjectionService; import com.todoroo.andlib.sql.Criterion; import com.todoroo.andlib.sql.Query; import com.todoroo.andlib.utility.DateUtilities; +import com.todoroo.andlib.utility.Preferences; import com.todoroo.astrid.actfm.sync.messages.NameMaps; import com.todoroo.astrid.dao.MetadataDao.MetadataCriteria; import com.todoroo.astrid.dao.OutstandingEntryDao; @@ -36,7 +37,6 @@ import com.todoroo.astrid.helper.UUIDHelper; import com.todoroo.astrid.service.MetadataService; import com.todoroo.astrid.service.TagDataService; import com.todoroo.astrid.tags.TaskToTagMetadata; -import com.todoroo.astrid.utility.Flags; @SuppressWarnings("nls") public class AstridNewSyncMigrator { @@ -54,13 +54,17 @@ public class AstridNewSyncMigrator { private static final String LOG_TAG = "sync-migrate"; + public static final String PREF_SYNC_MIGRATION = "p_sync_migration"; + public AstridNewSyncMigrator() { DependencyInjectionService.getInstance().inject(this); } @SuppressWarnings("deprecation") public void performMigration() { - Flags.set(Flags.SYNC_MIGRATION_ONGOING); + if (Preferences.getBoolean(PREF_SYNC_MIGRATION, false)) + return; + // -------------- // First ensure that a TagData object exists for each tag metadata // -------------- @@ -225,7 +229,11 @@ public class AstridNewSyncMigrator { } catch (Exception e) { Log.e(LOG_TAG, "Error validating task to tag metadata", e); } - Flags.checkAndClear(Flags.SYNC_MIGRATION_ONGOING); + Preferences.setBoolean(PREF_SYNC_MIGRATION, true); + ActFmSyncMonitor monitor = ActFmSyncMonitor.getInstance(); + synchronized (monitor) { + monitor.notifyAll(); + } } private interface UUIDAssertionExtras { diff --git a/astrid/src/com/todoroo/astrid/service/StartupService.java b/astrid/src/com/todoroo/astrid/service/StartupService.java index b48c155f5..877ed1406 100644 --- a/astrid/src/com/todoroo/astrid/service/StartupService.java +++ b/astrid/src/com/todoroo/astrid/service/StartupService.java @@ -43,6 +43,7 @@ import com.todoroo.andlib.utility.DialogUtilities; import com.todoroo.andlib.utility.Preferences; import com.todoroo.astrid.actfm.sync.ActFmPreferenceService; import com.todoroo.astrid.actfm.sync.ActFmSyncThread; +import com.todoroo.astrid.actfm.sync.AstridNewSyncMigrator; import com.todoroo.astrid.activity.BeastModePreferences; import com.todoroo.astrid.backup.BackupConstants; import com.todoroo.astrid.backup.BackupService; @@ -202,6 +203,8 @@ public class StartupService { if(justUpgraded && version > 0) { if(latestSetVersion > 0) { upgradeService.performUpgrade(context, latestSetVersion); + } else { + Preferences.setBoolean(AstridNewSyncMigrator.PREF_SYNC_MIGRATION, true); // New installs should have this flag set up front } AstridPreferences.setCurrentVersion(version); AstridPreferences.setCurrentVersionName(versionName); diff --git a/astrid/src/com/todoroo/astrid/service/UpgradeService.java b/astrid/src/com/todoroo/astrid/service/UpgradeService.java index 3912b8045..9dc1d4fe3 100644 --- a/astrid/src/com/todoroo/astrid/service/UpgradeService.java +++ b/astrid/src/com/todoroo/astrid/service/UpgradeService.java @@ -236,8 +236,8 @@ public final class UpgradeService { new SubtasksMetadataMigration().performMigration(); if (from < V4_6_0) { - new GCMIntentService.GCMMigration().performMigration(UpgradeActivity.this); new AstridNewSyncMigrator().performMigration(); + new GCMIntentService.GCMMigration().performMigration(UpgradeActivity.this); } } finally { diff --git a/astrid/src/com/todoroo/astrid/utility/Flags.java b/astrid/src/com/todoroo/astrid/utility/Flags.java index e164f1b22..1d3a0d0e4 100644 --- a/astrid/src/com/todoroo/astrid/utility/Flags.java +++ b/astrid/src/com/todoroo/astrid/utility/Flags.java @@ -36,12 +36,6 @@ public class Flags { */ public static final int TLFP_NO_INTERCEPT_TOUCH = 1 << 7; - /** - * If set, indicates that the new sync migration is ongoing and the sync thread should block until it is finished - * Only set once. - */ - public static final int SYNC_MIGRATION_ONGOING = 1 << 8; - public static boolean checkAndClear(int flag) { boolean set = (state & flag) > 0; state &= ~flag;