[local-sync] Correctly sync folders and labels

This commit will correctly keep track of folder and label ids when
creating them from N1.

Previously, when we sent the request to create a folder or label to our api,
we would immediately get back a serverId because it was created optimistically
in the back end— given that K2 is strictly non-optimistic, we won’t have a serverId
until some undetermined time in the future, and we need to somehow reference
the object that /was/ optimistically created in N1 to update the ui when
we do get the server id.

Since we can deterministically generate ids for folders and labels,
we "guess" what its going to be, and include it in the props of the syncback request
returned to N1. This is the simplest solution to get thing working
correctly right now, but we’ll need to revisit this in the future for
other types of objects (drafts, contacts, events), and revisit how we
will manage optimistic updates in N1 when we merge the 2 codebases
with K2 (given that K2 was designed to be non-optimisitc).

packages/local-sync/src/local-api/route-helpers.js

@@ -1,13 +1,14 @@
 const Serialization = require('./serialization');
+const SyncProcessManager = require('../local-sync-worker/sync-process-manager')
 
 module.exports = {
-  createSyncbackRequest: function createSyncbackRequest(request, reply, syncRequestArgs) {
-    request.getAccountDatabase().then((db) => {
-      const accountId = request.auth.credentials.id;
-      syncRequestArgs.accountId = accountId
-      db.SyncbackRequest.create(syncRequestArgs).then((syncbackRequest) => {
-        reply(Serialization.jsonStringify(syncbackRequest))
-      })
-    })
+  async createSyncbackRequest(request, reply, syncRequestArgs) {
+    const account = request.auth.credentials
+    syncRequestArgs.accountId = account.id
+
+    const db = await request.getAccountDatabase()
+    const syncbackRequest = await db.SyncbackRequest.create(syncRequestArgs)
+    SyncProcessManager.wakeWorkerForAccount(account)
+    reply(Serialization.jsonStringify(syncbackRequest))
   },
 }

packages/local-sync/src/local-api/routes/categories.js

@@ -25,16 +25,14 @@ module.exports = (server) => {
           ),
         },
       },
-      handler: (request, reply) => {
-        request.getAccountDatabase().then((db) => {
-          const Klass = db[klass];
-          Klass.findAll({
-            limit: request.query.limit,
-            offset: request.query.offset,
-          }).then((items) => {
-            reply(Serialization.jsonStringify(items));
-          })
+      async handler(request, reply) {
+        const db = await request.getAccountDatabase()
+        const Klass = db[klass];
+        const items = await Klass.findAll({
+          limit: request.query.limit,
+          offset: request.query.offset,
         })
+        reply(Serialization.jsonStringify(items));
       },
     });
 
@@ -47,17 +45,30 @@ module.exports = (server) => {
       config: {
         description: `Create ${term}`,
         tags: [term],
-        validate: {},
+        validate: {
+          params: {
+            payload: {
+              display_name: Joi.string().required(),
+            },
+          },
+        },
         response: {
           schema: Serialization.jsonSchema('SyncbackRequest'),
         },
       },
-      handler: (request, reply) => {
-        if (request.payload.display_name) {
+      async handler(request, reply) {
+        const {payload} = request
+        if (payload.display_name) {
+          const accountId = request.auth.credentials.id
+          const db = await request.getAccountDatabase()
+          const objectId = db[klass].hash({boxName: payload.display_name, accountId})
+
           createSyncbackRequest(request, reply, {
-            type: "CreateFolder",
+            type: "CreateCategory",
             props: {
-              displayName: request.payload.display_name,
+              objectId,
+              object: klass.toLowerCase(),
+              displayName: payload.display_name,
             },
           })
         }
@@ -72,22 +83,42 @@ module.exports = (server) => {
         tags: [term],
         validate: {
           params: {
-            id: Joi.string(),
+            id: Joi.string().required(),
+            payload: {
+              display_name: Joi.string().required(),
+            },
           },
         },
         response: {
           schema: Serialization.jsonSchema('SyncbackRequest'),
         },
       },
-      handler: (request, reply) => {
-        if (request.payload.display_name) {
-          createSyncbackRequest(request, reply, {
-            type: "RenameFolder",
-            props: {
-              displayName: request.payload.display_name,
-              id: request.params.id,
-            },
-          })
+      async handler(request, reply) {
+        const {payload} = request
+        if (payload.display_name) {
+          const accountId = request.auth.credentials.id
+          const db = await request.getAccountDatabase()
+          const objectId = db[klass].hash({boxName: payload.display_name, accountId})
+
+          if (klass === 'Label') {
+            createSyncbackRequest(request, reply, {
+              type: "RenameLabel",
+              props: {
+                objectId,
+                labelId: request.params.id,
+                displayName: payload.display_name,
+              },
+            })
+          } else {
+            createSyncbackRequest(request, reply, {
+              type: "RenameFolder",
+              props: {
+                objectId,
+                folderId: request.params.id,
+                displayName: payload.display_name,
+              },
+            })
+          }
         }
       },
     })
@@ -100,7 +131,7 @@ module.exports = (server) => {
         tags: [term],
         validate: {
           params: {
-            id: Joi.number().integer(),
+            id: Joi.string().required(),
           },
         },
         response: {
@@ -108,12 +139,21 @@ module.exports = (server) => {
         },
       },
       handler: (request, reply) => {
-        createSyncbackRequest(request, reply, {
-          type: "DeleteFolder",
-          props: {
-            id: request.params.id,
-          },
-        })
+        if (klass === 'Label') {
+          createSyncbackRequest(request, reply, {
+            type: "DeleteLabel",
+            props: {
+              labelId: request.params.id,
+            },
+          })
+        } else {
+          createSyncbackRequest(request, reply, {
+            type: "DeleteFolder",
+            props: {
+              folderId: request.params.id,
+            },
+          })
+        }
       },
     })
   });

packages/local-sync/src/local-sync-worker/imap/fetch-folder-list.js

@@ -1,4 +1,3 @@
-const crypto = require('crypto')
 const {Provider, PromiseUtils} = require('isomorphic-core');
 const {localizedCategoryNames} = require('../sync-utils')
 
@@ -110,7 +109,7 @@ class FetchFolderList {
         const {accountId} = this._db
         category = Klass.build({
           accountId,
-          id: crypto.createHash('sha256').update(`${accountId}${boxName}`, 'utf8').digest('hex'),
+          id: Klass.hash({boxName, accountId}),
           name: boxName,
           role: role,
         });

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

@@ -68,4 +68,4 @@ class SyncProcessManager {
   }
 }
 
-module.exports = new SyncProcessManager();
+module.exports = new SyncProcessManager()

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

@@ -4,10 +4,7 @@ const {
   PromiseUtils,
 } = require('isomorphic-core');
 const LocalDatabaseConnector = require('../shared/local-database-connector')
-const {
-  jsonError,
-} = require('./sync-utils')
-
+const {jsonError} = require('./sync-utils')
 const FetchFolderList = require('./imap/fetch-folder-list')
 const FetchMessagesInFolder = require('./imap/fetch-messages-in-folder')
 const SyncbackTaskFactory = require('./syncback-task-factory')
@@ -112,8 +109,7 @@ class SyncWorker {
     conn.on('update', () => {
       this._onConnectionIdleUpdate();
     })
-    conn.on('queue-empty', () => {
-    });
+    conn.on('queue-empty', () => {});
 
     this._conn = conn;
     return await this._conn.connect();
@@ -123,9 +119,8 @@ class SyncWorker {
     const {SyncbackRequest} = this._db;
     const where = {where: {status: "NEW"}, limit: 100};
 
-    const tasks = (await SyncbackRequest.findAll(where)).map((req) =>
-      SyncbackTaskFactory.create(this._account, req)
-    );
+    const tasks = await SyncbackRequest.findAll(where)
+    .map((req) => SyncbackTaskFactory.create(this._account, req));
 
     return PromiseUtils.each(tasks, this.runSyncbackTask.bind(this));
   }

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

@@ -4,6 +4,15 @@
  *
  */
 class SyncbackTaskFactory {
+
+  static TaskTypesAffectingMessageFolderUIDs = [
+    'MoveThreadToFolder',
+    'MoveMessageToFolder',
+    'SetThreadFolderAndLabels',
+    'RenameFolder',
+    'DeleteFolder',
+  ]
+
   static create(account, syncbackRequest) {
     let Task = null;
     switch (syncbackRequest.type) {
@@ -33,12 +42,16 @@ class SyncbackTaskFactory {
         Task = require('./syncback_tasks/star-message.imap'); break;
       case "UnstarMessage":
         Task = require('./syncback_tasks/unstar-message.imap'); break;
-      case "CreateFolder":
-        Task = require('./syncback_tasks/create-folder.imap'); break;
+      case "CreateCategory":
+        Task = require('./syncback_tasks/create-category.imap'); break;
       case "RenameFolder":
         Task = require('./syncback_tasks/rename-folder.imap'); break;
+      case "RenameLabel":
+        Task = require('./syncback_tasks/rename-label.imap'); break;
       case "DeleteFolder":
         Task = require('./syncback_tasks/delete-folder.imap'); break;
+      case "DeleteLabel":
+        Task = require('./syncback_tasks/delete-label.imap'); break;
       case "DeleteMessage":
         Task = require('./syncback_tasks/delete-message.imap'); break;
       case "SaveSentMessage":

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

@@ -0,0 +1,14 @@
+const SyncbackTask = require('./syncback-task')
+
+class CreateCategoryIMAP extends SyncbackTask {
+  description() {
+    return `CreateCategory`;
+  }
+
+  async run(db, imap) {
+    const syncbackRequestObject = this.syncbackRequestObject()
+    const displayName = syncbackRequestObject.props.displayName
+    await imap.addBox(displayName)
+  }
+}
+module.exports = CreateCategoryIMAP

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

@@ -1,13 +0,0 @@
-const SyncbackTask = require('./syncback-task')
-
-class CreateFolderIMAP extends SyncbackTask {
-  description() {
-    return `CreateFolder`;
-  }
-
-  run(db, imap) {
-    const folderName = this.syncbackRequestObject().props.displayName
-    return imap.addBox(folderName)
-  }
-}
-module.exports = CreateFolderIMAP

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

@@ -5,11 +5,10 @@ class DeleteFolderIMAP extends SyncbackTask {
     return `DeleteFolder`;
   }
 
-  run(db, imap) {
-    const folderId = this.syncbackRequestObject().props.id
-    return db.Folder.findById(folderId).then((folder) => {
-      return imap.delBox(folder.name);
-    })
+  async run(db, imap) {
+    const folderId = this.syncbackRequestObject().props.folderId
+    const folder = await db.Folder.findById(folderId)
+    return imap.delBox(folder.name);
   }
 }
 module.exports = DeleteFolderIMAP

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

@@ -0,0 +1,14 @@
+const SyncbackTask = require('./syncback-task')
+
+class DeleteLabelIMAP extends SyncbackTask {
+  description() {
+    return `DeleteLabel`;
+  }
+
+  async run(db, imap) {
+    const labelId = this.syncbackRequestObject().props.labelId
+    const label = await db.Label.findById(labelId)
+    return imap.delBox(label.name);
+  }
+}
+module.exports = DeleteLabelIMAP

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

@@ -5,12 +5,11 @@ class RenameFolderIMAP extends SyncbackTask {
     return `RenameFolder`;
   }
 
-  run(db, imap) {
-    const folderId = this.syncbackRequestObject().props.id
+  async run(db, imap) {
+    const folderId = this.syncbackRequestObject().props.folderId
     const newFolderName = this.syncbackRequestObject().props.displayName
-    return db.Folder.findById(folderId).then((folder) => {
-      return imap.renameBox(folder.name, newFolderName);
-    })
+    const folder = await db.Folder.findById(folderId)
+    return imap.renameBox(folder.name, newFolderName);
   }
 }
 module.exports = RenameFolderIMAP

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

@@ -0,0 +1,15 @@
+const SyncbackTask = require('./syncback-task')
+
+class RenameLabelIMAP extends SyncbackTask {
+  description() {
+    return `RenameLabel`;
+  }
+
+  async run(db, imap) {
+    const labelId = this.syncbackRequestObject().props.labelId
+    const newLabelName = this.syncbackRequestObject().props.displayName
+    const folder = await db.Label.findById(labelId)
+    return imap.renameBox(folder.name, newLabelName);
+  }
+}
+module.exports = RenameLabelIMAP

packages/local-sync/src/models/folder.js

@@ -1,3 +1,4 @@
+const crypto = require('crypto')
 const {DatabaseTypes: {buildJSONColumnOptions}} = require('isomorphic-core');
 const {formatImapPath} = require('../shared/imap-paths-utils');
 
@@ -21,10 +22,15 @@ module.exports = (sequelize, Sequelize) => {
       },
     ],
     classMethods: {
-      associate: ({Folder, Message, Thread}) => {
+      associate({Folder, Message, Thread}) {
         Folder.hasMany(Message)
         Folder.belongsToMany(Thread, {through: 'thread_folders'})
       },
+
+      hash({boxName, accountId}) {
+        const cleanName = formatImapPath(boxName)
+        return crypto.createHash('sha256').update(`${accountId}${cleanName}`, 'utf8').digest('hex')
+      },
     },
     instanceMethods: {
       toJSON: function toJSON() {

packages/local-sync/src/models/label.js

@@ -1,3 +1,4 @@
+const crypto = require('crypto')
 const {formatImapPath} = require('../shared/imap-paths-utils');
 
 module.exports = (sequelize, Sequelize) => {
@@ -40,6 +41,11 @@ module.exports = (sequelize, Sequelize) => {
           where: sequelize.or({name: labelNames}, {role: labelRoles}),
         })
       },
+
+      hash({boxName, accountId}) {
+        const cleanName = formatImapPath(boxName)
+        return crypto.createHash('sha256').update(`${accountId}${cleanName}`, 'utf8').digest('hex')
+      },
     },
     instanceMethods: {
       imapLabelIdentifier() {

packages/local-sync/src/models/syncbackRequest.js

@@ -13,14 +13,14 @@ module.exports = (sequelize, Sequelize) => {
     accountId: { type: Sequelize.STRING, allowNull: false },
   }, {
     instanceMethods: {
-      toJSON: function toJSON() {
+      toJSON() {
         return {
           id: `${this.id}`,
           type: this.type,
           error: JSON.stringify(this.error || {}),
           props: this.props,
           status: this.status,
-          object: 'providerSyncbackRequest',
+          object: 'syncbackRequest',
           account_id: this.accountId,
         }
       },