From 76234eaa9b93e4436fcfeb9258c47f3cc4dddc41 Mon Sep 17 00:00:00 2001
From: Juan Tejada <juans.tejada@gmail.com>
Date: Tue, 28 Mar 2017 12:32:47 -0700
Subject: [PATCH] [client-app] Correctly listen for new mail in between sync
 loops

Summary:
When using the IMAPConnectionPool in the sync-worker, we requested 2
connections from the pool: 1 to listen for new mail, and 1 to actually
operate on the mailbox. These 2 connections would be closed and returned
to the pool at the end of each sync loop iteration.

However, we never want to close the connection to listen for new mail.
Closing this connection caused us to not listen for new mail in between
sync loops, which was especially noticeable when initial sync was done
because the delay between sync loops is 5 minutes.

This commit makes it so we request the 2 connections separately from the
connection pool, and keep the listener connection always open until we
dispose of the sync-worker.

Test Plan:
manually make sure that I got new mail event listeners in between
sync loops :(

Reviewers: evan, spang, halla, mark

Reviewed By: mark

Differential Revision: https://phab.nylas.com/D4276
---
 .../src/local-sync-worker/sync-worker.es6     | 67 ++++++++++++-------
 1 file changed, 42 insertions(+), 25 deletions(-)

diff --git a/packages/client-sync/src/local-sync-worker/sync-worker.es6 b/packages/client-sync/src/local-sync-worker/sync-worker.es6
index bbdfc6d31..5f9f62683 100644
--- a/packages/client-sync/src/local-sync-worker/sync-worker.es6
+++ b/packages/client-sync/src/local-sync-worker/sync-worker.es6
@@ -51,6 +51,7 @@ class SyncWorker {
     this._requireTokenRefresh = false
     this._batchProcessedUids = new Map();
     this._latestOpenTimesByFolder = new Map();
+    this._disposeMailListenerConnection = null
 
     this._retryScheduler = new ExponentialBackoffScheduler({
       baseDelay: 15 * 1000,
@@ -244,28 +245,44 @@ class SyncWorker {
     this._conn._db = this._db;
   }
 
-  async _ensureIMAPMailListenerConnection(conn) {
-    if (this._mailListenerConn === conn) {
+  async _ensureIMAPMailListenerConnection() {
+    if (this._mailListenerConn) {
       return;
     }
 
-    this._mailListenerConn = conn;
-    this._mailListenerConn._db = this._db;
+    await IMAPConnectionPool.withConnectionsForAccount(this._account, {
+      desiredCount: 1,
+      logger: this._logger,
+      socketTimeout: this._retryScheduler.currentDelay(),
+      onConnected: async ([listenerConn], done) => {
+        this._mailListenerConn = listenerConn;
+        this._mailListenerConn._db = this._db;
 
-    conn.on('mail', () => {
-      this._onInboxUpdates(`You've got mail`);
-    })
-    conn.on('update', () => {
-      // `update` events happen when messages receive flag updates on the inbox
-      // (e.g. marking as unread or starred). We need to listen to that event for
-      // when those updates are performed from another mail client, but ignore
-      // them when they are caused from within N1.
-      if (this._shouldIgnoreInboxFlagUpdates) { return; }
-      this._onInboxUpdates(`There are flag updates on the inbox`);
+        this._mailListenerConn.on('mail', () => {
+          this._onInboxUpdates(`You've got mail`);
+        })
+        this._mailListenerConn.on('update', () => {
+          // `update` events happen when messages receive flag updates on the inbox
+          // (e.g. marking as unread or starred). We need to listen to that event for
+          // when those updates are performed from another mail client, but ignore
+          // them when they are caused from within N1.
+          if (this._shouldIgnoreInboxFlagUpdates) { return; }
+          this._onInboxUpdates(`There are flag updates on the inbox`);
+        })
+
+        this._disposeMailListenerConnection = done
+        // Return true to keep connection open
+        return true
+      },
     })
   }
 
+  _onInboxUpdates = _.debounce((reason) => {
+    this.syncNow({reason, interrupt: true});
+  }, 100)
+
   async _listenForNewMail() {
+    this._logger.log('🔃  Listening for new mail...')
     // Open the inbox folder on our dedicated mail listener connection to listen
     // to new mail events
     const inbox = await this._getInboxFolder();
@@ -274,13 +291,13 @@ class SyncWorker {
     }
   }
 
-  _onInboxUpdates = _.debounce((reason) => {
-    this.syncNow({reason, interrupt: true});
-  }, 100)
-
-  _clearConnections() {
+  _disposeConnections() {
     this._conn = null;
     this._mailListenerConn = null;
+    if (this._disposeMailListenerConnection) {
+      this._disposeMailListenerConnection()
+      this._disposeMailListenerConnection = null
+    }
   }
 
   async _getFoldersToSync() {
@@ -301,7 +318,7 @@ class SyncWorker {
 
   async _onSyncError(error) {
     try {
-      this._clearConnections();
+      this._disposeConnections();
       this._logger.error(`🔃  SyncWorker: Errored while syncing account`, error)
 
       // Check if we encountered an expired token error.
@@ -552,13 +569,13 @@ class SyncWorker {
     let error;
     try {
       await this._ensureSMTPConnection();
+      await this._ensureIMAPMailListenerConnection();
       await IMAPConnectionPool.withConnectionsForAccount(this._account, {
-        desiredCount: 2,
+        desiredCount: 1,
         logger: this._logger,
         socketTimeout: this._retryScheduler.currentDelay(),
-        onConnected: async ([mainConn, listenerConn]) => {
+        onConnected: async ([mainConn]) => {
           await this._ensureIMAPConnection(mainConn);
-          await this._ensureIMAPMailListenerConnection(listenerConn);
           await this._interruptible.run(this._performSync, this)
         },
       });
@@ -573,7 +590,7 @@ class SyncWorker {
     } finally {
       this._lastSyncTime = Date.now()
       this._syncInProgress = false
-      this._clearConnections();
+      this._conn = null;
       await this._scheduleNextSync(error)
     }
   }
@@ -601,7 +618,7 @@ class SyncWorker {
   async cleanup() {
     await this.stopSync()
     this._destroyed = true;
-    this._clearConnections()
+    this._disposeConnections()
   }
 }