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

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<ClientToServerMessage<?>, 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<ClientToServerMessage<?>> messageBatch = new LinkedList<ClientToServerMessage<?>>();
             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() {

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<TYPE extends RemoteModel> {

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);

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 {

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;