From 5810bb6b79bf07d7b8f3e96159cebc1b978ea697 Mon Sep 17 00:00:00 2001
From: Juan Tejada <juans.tejada@gmail.com>
Date: Mon, 19 Dec 2016 08:08:41 -0800
Subject: [PATCH] [local-sync] Fix issue with imap connection mail event

Summary:
See https://github.com/mscdex/node-imap/issues/585 for details.

This issue was causing us constantly run the sync loop without pauses,
i.e. every next sync loop was scheduled immediately.

Currently, when we receive a new `'mail'`event, we trigger a new sync loop. Previously, when this happened while a sync loop was already in progress we would just ignore the event. However, my recent patch keeps track of how many times we tried to start a sync loop while one was already in progress. If the number of times this happens is > 0, it will schedule the next sync loop immediately (as opposed to waiting a constant amount seconds before the next loop).

The problem is that this new logic is making the sync worker always schedule the next sync loop immediately (without pausing for a few seconds). This is due to the following chain of events (assume we are just syncing `all` and `trash` folders):

This commit is a temporary workaround to this problem.

Test Plan: manual

Reviewers: evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3537
---
 packages/isomorphic-core/src/imap-connection.js     | 12 +++++++++---
 .../local-sync/src/local-sync-worker/sync-worker.js | 13 ++++++-------
 packages/local-sync/src/models/folder.js            | 10 +++++++++-
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/packages/isomorphic-core/src/imap-connection.js b/packages/isomorphic-core/src/imap-connection.js
index 1130fe571..ffedc349d 100644
--- a/packages/isomorphic-core/src/imap-connection.js
+++ b/packages/isomorphic-core/src/imap-connection.js
@@ -46,6 +46,7 @@ class IMAPConnection extends EventEmitter {
     this._settings = settings;
     this._imap = null;
     this._connectPromise = null;
+    this._isOpeningBox = false;
   }
 
   static generateXOAuth2Token(username, accessToken) {
@@ -123,6 +124,9 @@ class IMAPConnection extends EventEmitter {
       // Fix https://github.com/mscdex/node-imap/issues/445
       let lastMailEventBox = null;
       this._imap.on('mail', () => {
+        // This check is for working around
+        // https://github.com/mscdex/node-imap/issues/585
+        if (this._isOpeningBox) { return }
         if (lastMailEventBox === this._imap._box.name) {
           this.emit('mail');
         }
@@ -182,9 +186,11 @@ class IMAPConnection extends EventEmitter {
     if (!this._imap) {
       throw new IMAPConnectionNotReadyError(`IMAPConnection::openBox`)
     }
-    return this._imap.openBoxAsync(folderName, readOnly).then((box) =>
-      new IMAPBox(this, box)
-    )
+    this._isOpeningBox = true
+    return this._imap.openBoxAsync(folderName, readOnly).then((box) => {
+      this._isOpeningBox = false
+      return new IMAPBox(this, box)
+    })
   }
 
   getBoxes() {
diff --git a/packages/local-sync/src/local-sync-worker/sync-worker.js b/packages/local-sync/src/local-sync-worker/sync-worker.js
index 6fd7328fa..b26a79381 100644
--- a/packages/local-sync/src/local-sync-worker/sync-worker.js
+++ b/packages/local-sync/src/local-sync-worker/sync-worker.js
@@ -370,10 +370,10 @@ class SyncWorker {
     const {Folder} = this._db;
 
     const folders = await Folder.findAll();
-    const moreToSync = folders.some((f) =>
-      f.syncState.fetchedmax < f.syncState.uidnext || f.syncState.fetchedmin > 1
-    )
+    const moreToSync = folders.some((f) => !f.isSyncComplete())
 
+    // Continue syncing if initial sync isn't done, or if the loop was
+    // interrupted or a sync was requested
     const shouldSyncImmediately = (
       moreToSync ||
       this._interrupted ||
@@ -381,20 +381,19 @@ class SyncWorker {
     )
     const reason = this._interrupted ? 'Sync interrupted for restart' : 'Scheduled'
     const interval = shouldSyncImmediately ? 1 : intervals.active;
-    const nextSyncTime = this._lastSyncTime + interval
-    const nextSyncStart = Math.max(1, nextSyncTime - Date.now())
+    const nextSyncIn = Math.max(1, this._lastSyncTime + interval - Date.now())
 
     this._logger.info({
       moreToSync,
       shouldSyncImmediately,
       interrupted: this._interrupted,
-      nextSyncStartingIn: `${nextSyncStart}ms`,
+      nextSyncStartingIn: `${nextSyncIn}ms`,
       syncAttemptsWhileInProgress: this._syncAttemptsWhileInProgress,
     }, `SyncWorker: Scheduling next sync iteration`)
 
     this._syncTimer = setTimeout(() => {
       this.syncNow({reason});
-    }, nextSyncStart);
+    }, nextSyncIn);
   }
 }
 
diff --git a/packages/local-sync/src/models/folder.js b/packages/local-sync/src/models/folder.js
index 4e103a457..1dc0f11f3 100644
--- a/packages/local-sync/src/models/folder.js
+++ b/packages/local-sync/src/models/folder.js
@@ -55,7 +55,15 @@ module.exports = (sequelize, Sequelize) => {
       },
     },
     instanceMethods: {
-      toJSON: function toJSON() {
+      isSyncComplete() {
+        if (!this.syncState) { return true }
+        return (
+          this.syncState.fetchedmin <= 1 &&
+          this.syncState.fetchedmax >= this.syncState.uidnext
+        )
+      },
+
+      toJSON() {
         return {
           id: `${this.id}`,
           account_id: this.accountId,