[local-sync] Ensure order correct order when running syncback requests

When running syncback requests, if a task is meant to change the UID of
a message (e.g. move it to a new folder), that task should be run after
other tasks that don't affect the UID. Otherwise, when trying to run the
other tasks, they would reference a UID that is no longer valid.

This commit will make sure that we run any tasks that will change message uids last,
/and/ make sure that we don't run more than 1 task that will affect the uids of
the same messages in a row (i.e. without running a sync loop in between)

packages/local-sync/src/local-sync-worker/sync-worker.js

@@ -115,14 +115,62 @@ class SyncWorker {
     return await this._conn.connect();
   }
 
-  async syncbackMessageActions() {
-    const {SyncbackRequest} = this._db;
-    const where = {where: {status: "NEW"}, limit: 100};
+  async runNewSyncbackRequests() {
+    const {SyncbackRequest, Message} = this._db;
+    const where = {
+      limit: 100,
+      where: {status: "NEW"},
+      order: [['createdAt', 'ASC']],
+    };
 
+    // Make sure we run the tasks that affect IMAP uids last, and that we don't
+    // run 2 tasks that will affect the same set of UIDS together (i.e. without
+    // running a sync loop in between)
     const tasks = await SyncbackRequest.findAll(where)
-    .map((req) => SyncbackTaskFactory.create(this._account, req));
+    .map((req) => SyncbackTaskFactory.create(this._account, req))
 
-    return PromiseUtils.each(tasks, this.runSyncbackTask.bind(this));
+    if (tasks.length === 0) { return Promise.resolve() }
+
+    const tasksToProcess = tasks.filter(t => !t.affectsImapMessageUIDs())
+    const tasksAffectingUIDs = tasks.filter(t => t.affectsImapMessageUIDs())
+
+    const changeFolderTasks = tasksAffectingUIDs.filter(t =>
+      t.description() === 'RenameFolder' || t.description() === 'DeleteFolder'
+    )
+    if (changeFolderTasks.length > 0) {
+      // If we are renaming or deleting folders, those are the only tasks we
+      // want to process before executing any other tasks that may change UIDs
+      const affectedFolderIds = new Set()
+      changeFolderTasks.forEach((task) => {
+        const {props: {folderId}} = task.syncbackRequestObject()
+        if (folderId && !affectedFolderIds.has(folderId)) {
+          tasksToProcess.push(task)
+          affectedFolderIds.add(folderId)
+        }
+      })
+      return PromiseUtils.each(tasks, (task) => this.runSyncbackTask(task));
+    }
+
+    // Otherwise, make sure that we don't process more than 1 task that will affect
+    // the UID of the same message
+    const affectedMessageIds = new Set()
+    for (const task of tasksAffectingUIDs) {
+      const {props: {messageId, threadId}} = task.syncbackRequestObject()
+      if (messageId) {
+        if (!affectedMessageIds.has(messageId)) {
+          tasksToProcess.push(task)
+          affectedMessageIds.add(messageId)
+        }
+      } else if (threadId) {
+        const messageIds = await Message.findAll({where: {threadId}}).map(m => m.id)
+        const shouldIncludeTask = messageIds.every(id => !affectedMessageIds.has(id))
+        if (shouldIncludeTask) {
+          tasksToProcess.push(task)
+          messageIds.forEach(id => affectedMessageIds.add(id))
+        }
+      }
+    }
+    return PromiseUtils.each(tasks, (task) => this.runSyncbackTask(task));
   }
 
   async runSyncbackTask(task) {
@@ -176,7 +224,7 @@ class SyncWorker {
     try {
       await this._account.update({syncError: null});
       await this.ensureConnection();
-      await this.syncbackMessageActions();
+      await this.runNewSyncbackRequests();
       await this._conn.runOperation(new FetchFolderList(this._account.provider, this._logger));
       await this.syncMessagesInAllFolders();
       await this.onSyncDidComplete();

packages/local-sync/src/local-sync-worker/syncback-task-factory.js

@@ -5,14 +5,6 @@
  */
 class SyncbackTaskFactory {
 
-  static TaskTypesAffectingMessageFolderUIDs = [
-    'MoveThreadToFolder',
-    'MoveMessageToFolder',
-    'SetThreadFolderAndLabels',
-    'RenameFolder',
-    'DeleteFolder',
-  ]
-
   static create(account, syncbackRequest) {
     let Task = null;
     switch (syncbackRequest.type) {

packages/local-sync/src/local-sync-worker/syncback_tasks/create-category.imap.js

@@ -5,6 +5,10 @@ class CreateCategoryIMAP extends SyncbackTask {
     return `CreateCategory`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   async run(db, imap) {
     const syncbackRequestObject = this.syncbackRequestObject()
     const displayName = syncbackRequestObject.props.displayName

packages/local-sync/src/local-sync-worker/syncback_tasks/delete-folder.imap.js

@@ -5,6 +5,10 @@ class DeleteFolderIMAP extends SyncbackTask {
     return `DeleteFolder`;
   }
 
+  affectsImapMessageUIDs() {
+    return true
+  }
+
   async run(db, imap) {
     const folderId = this.syncbackRequestObject().props.folderId
     const folder = await db.Folder.findById(folderId)

packages/local-sync/src/local-sync-worker/syncback_tasks/delete-label.imap.js

@@ -5,6 +5,10 @@ class DeleteLabelIMAP extends SyncbackTask {
     return `DeleteLabel`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   async run(db, imap) {
     const labelId = this.syncbackRequestObject().props.labelId
     const label = await db.Label.findById(labelId)

packages/local-sync/src/local-sync-worker/syncback_tasks/delete-message.imap.js

@@ -6,6 +6,10 @@ class DeleteMessageIMAP extends SyncbackTask {
     return `DeleteMessage`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   async run(db, imap) {
     const messageId = this.syncbackRequestObject().props.messageId
     const {box, message} = await TaskHelpers.openMessageBox({messageId, db, imap})

packages/local-sync/src/local-sync-worker/syncback_tasks/mark-message-as-read.imap.js

@@ -6,6 +6,10 @@ class MarkMessageAsReadIMAP extends SyncbackTask {
     return `MarkMessageAsRead`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   run(db, imap) {
     const messageId = this.syncbackRequestObject().props.messageId
 

packages/local-sync/src/local-sync-worker/syncback_tasks/mark-message-as-unread.imap.js

@@ -6,6 +6,10 @@ class MarkMessageAsUnreadIMAP extends SyncbackTask {
     return `MarkMessageAsUnread`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   run(db, imap) {
     const messageId = this.syncbackRequestObject().props.messageId
 

packages/local-sync/src/local-sync-worker/syncback_tasks/mark-thread-as-read.imap.js

@@ -6,6 +6,10 @@ class MarkThreadAsRead extends SyncbackTask {
     return `MarkThreadAsRead`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   run(db, imap) {
     const threadId = this.syncbackRequestObject().props.threadId
 

packages/local-sync/src/local-sync-worker/syncback_tasks/mark-thread-as-unread.imap.js

@@ -6,6 +6,10 @@ class MarkThreadAsUnread extends SyncbackTask {
     return `MarkThreadAsUnread`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   run(db, imap) {
     const threadId = this.syncbackRequestObject().props.threadId
 

packages/local-sync/src/local-sync-worker/syncback_tasks/move-message-to-folder.imap.js

@@ -6,6 +6,10 @@ class MoveMessageToFolderIMAP extends SyncbackTask {
     return `MoveMessageToFolder`;
   }
 
+  affectsImapMessageUIDs() {
+    return true
+  }
+
   async run(db, imap) {
     const messageId = this.syncbackRequestObject().props.messageId
     const targetFolderId = this.syncbackRequestObject().props.folderId

packages/local-sync/src/local-sync-worker/syncback_tasks/move-thread-to-folder.imap.js

@@ -6,6 +6,10 @@ class MoveThreadToFolderIMAP extends SyncbackTask {
     return `MoveThreadToFolder`;
   }
 
+  affectsImapMessageUIDs() {
+    return true
+  }
+
   async run(db, imap) {
     const threadId = this.syncbackRequestObject().props.threadId
     const targetFolderId = this.syncbackRequestObject().props.folderId

packages/local-sync/src/local-sync-worker/syncback_tasks/rename-folder.imap.js

@@ -5,6 +5,10 @@ class RenameFolderIMAP extends SyncbackTask {
     return `RenameFolder`;
   }
 
+  affectsImapMessageUIDs() {
+    return true
+  }
+
   async run(db, imap) {
     const folderId = this.syncbackRequestObject().props.folderId
     const newFolderName = this.syncbackRequestObject().props.displayName

packages/local-sync/src/local-sync-worker/syncback_tasks/rename-label.imap.js

@@ -5,6 +5,10 @@ class RenameLabelIMAP extends SyncbackTask {
     return `RenameLabel`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   async run(db, imap) {
     const labelId = this.syncbackRequestObject().props.labelId
     const newLabelName = this.syncbackRequestObject().props.displayName

packages/local-sync/src/local-sync-worker/syncback_tasks/save-sent-message.imap.js

@@ -5,6 +5,10 @@ class SaveSentMessageIMAP extends SyncbackTask {
     return `SaveSentMessage`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   async run(db, imap) {
     const {rawMime, messageId} = this.syncbackRequestObject().props;
 

packages/local-sync/src/local-sync-worker/syncback_tasks/set-message-labels.imap.js

@@ -6,6 +6,10 @@ class SetMessageLabelsIMAP extends SyncbackTask {
     return `SetMessageLabels`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   async run(db, imap) {
     const messageId = this.syncbackRequestObject().props.messageId
     const labelIds = this.syncbackRequestObject().props.labelIds

packages/local-sync/src/local-sync-worker/syncback_tasks/set-thread-folder-and-labels.imap.js

@@ -6,6 +6,10 @@ class SetThreadFolderAndLabelsIMAP extends SyncbackTask {
     return `SetThreadFolderAndLabels`;
   }
 
+  affectsImapMessageUIDs() {
+    return true
+  }
+
   async run(db, imap) {
     const threadId = this.syncbackRequestObject().props.threadId
     const labelIds = this.syncbackRequestObject().props.labelIds

packages/local-sync/src/local-sync-worker/syncback_tasks/set-thread-labels.imap.js

@@ -6,6 +6,10 @@ class SetThreadLabelsIMAP extends SyncbackTask {
     return `SetThreadLabels`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   async run(db, imap) {
     const threadId = this.syncbackRequestObject().props.threadId
     const labelIds = this.syncbackRequestObject().props.labelIds

packages/local-sync/src/local-sync-worker/syncback_tasks/star-message.imap.js

@@ -6,6 +6,10 @@ class StarMessageIMAP extends SyncbackTask {
     return `StarMessage`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   run(db, imap) {
     const messageId = this.syncbackRequestObject().props.messageId
 

packages/local-sync/src/local-sync-worker/syncback_tasks/star-thread.imap.js

@@ -6,6 +6,10 @@ class StarThread extends SyncbackTask {
     return `StarThread`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   run(db, imap) {
     const threadId = this.syncbackRequestObject().props.threadId
 

packages/local-sync/src/local-sync-worker/syncback_tasks/syncback-task.js

@@ -12,6 +12,10 @@ class SyncbackTask {
     throw new Error("Must return a description")
   }
 
+  affectsImapMessageUIDs() {
+    throw new Error("Must implement `affectsImapMessageUIDs`")
+  }
+
   run() {
     throw new Error("Must implement a run method")
   }

packages/local-sync/src/local-sync-worker/syncback_tasks/unstar-message.imap.js

@@ -6,6 +6,10 @@ class UnstarMessageIMAP extends SyncbackTask {
     return `UnstarMessage`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   run(db, imap) {
     const messageId = this.syncbackRequestObject().props.messageId
 

packages/local-sync/src/local-sync-worker/syncback_tasks/unstar-thread.imap.js

@@ -6,6 +6,10 @@ class UnstarThread extends SyncbackTask {
     return `UnstarThread`;
   }
 
+  affectsImapMessageUIDs() {
+    return false
+  }
+
   run(db, imap) {
     const threadId = this.syncbackRequestObject().props.threadId