Refactor connection doctor

src/js/util/connection-doctor.js

@@ -15,7 +15,8 @@ var TCPSocket = require('tcp-socket'),
  *
  * @constructor
  */
-function ConnectionDoctor(appConfig) {
+function ConnectionDoctor($q, appConfig) {
+    this._q = $q;
     this._appConfig = appConfig;
     this._workerPath = appConfig.config.workerPath + '/tcp-socket-tls-worker.min.js';
 }
@@ -82,40 +83,25 @@ ConnectionDoctor.prototype.configure = function(credentials) {
  * 3) Login to the server
  * 4) Perform some basic commands (e.g. list folders)
  * 5) Exposes error codes
- *
- * @param {Function} callback(error) Invoked when the test suite passed, or with an error object if something went wrong
  */
-ConnectionDoctor.prototype.check = function(callback) {
+ConnectionDoctor.prototype.check = function() {
     var self = this;
-
+    return self._q(function(resolve) {
-    if (!self.credentials) {
+        if (!self.credentials) {
-        return callback(new Error('You need to configure() the connection doctor first!'));
+            throw new Error('You need to configure() the connection doctor first!');
-    }
+        } else {
-
+            resolve();
-    self._checkOnline(function(error) {
-        if (error) {
-            return callback(error);
         }
-
+    }).then(function() {
-        self._checkReachable(self.credentials.imap, function(error) {
+        return self._checkOnline();
-            if (error) {
+    }).then(function() {
-                return callback(error);
+        return self._checkReachable(self.credentials.imap);
-            }
+    }).then(function() {
-
+        return self._checkReachable(self.credentials.smtp);
-            self._checkReachable(self.credentials.smtp, function(error) {
+    }).then(function() {
-                if (error) {
+        return self._checkImap();
-                    return callback(error);
+    }).then(function() {
-                }
+        return self._checkSmtp();
-
-                self._checkImap(function(error) {
-                    if (error) {
-                        return callback(error);
-                    }
-
-                    self._checkSmtp(callback);
-                });
-            });
-        });
     });
 };
 
@@ -126,15 +112,16 @@ ConnectionDoctor.prototype.check = function(callback) {
 
 /**
  * Checks if the browser is online
- *
- * @param {Function} callback(error) Invoked when the test suite passed, or with an error object if browser is offline
  */
-ConnectionDoctor.prototype._checkOnline = function(callback) {
+ConnectionDoctor.prototype._checkOnline = function() {
-    if (navigator.onLine) {
+    var self = this;
-        callback();
+    return self._q(function(resolve) {
-    } else {
+        if (navigator.onLine) {
-        callback(createError(OFFLINE, this._appConfig.string.connDocOffline));
+            resolve();
-    }
+        } else {
+            throw createError(OFFLINE, self._appConfig.string.connDocOffline);
+        }
+    });
 };
 
 /**
@@ -143,105 +130,113 @@ ConnectionDoctor.prototype._checkOnline = function(callback) {
  * @param {String} options.host
  * @param {Number} options.port
  * @param {Boolean} options.secure
- * @param {Function} callback(error) Invoked when the test suite passed, or with an error object if something went wrong
  */
-ConnectionDoctor.prototype._checkReachable = function(options, callback) {
+ConnectionDoctor.prototype._checkReachable = function(options) {
-    var socket,
+    var self = this;
-        error, // remember the error message
+    return self._q(function(resolve, reject) {
-        timeout, // remember the timeout object
+        var socket,
-        host = options.host + ':' + options.port,
+            error, // remember the error message
-        hasTimedOut = false, // prevents multiple callbacks
+            timeout, // remember the timeout object
-        cfg = this._appConfig.config,
+            host = options.host + ':' + options.port,
-        str = this._appConfig.string;
+            hasTimedOut = false, // prevents multiple callbacks
+            cfg = self._appConfig.config,
+            str = self._appConfig.string;
 
-    timeout = setTimeout(function() {
+        timeout = setTimeout(function() {
-        hasTimedOut = true;
+            hasTimedOut = true;
-        callback(createError(HOST_TIMEOUT, str.connDocHostTimeout.replace('{0}', host).replace('{1}', cfg.connDocTimeout)));
+            reject(createError(HOST_TIMEOUT, str.connDocHostTimeout.replace('{0}', host).replace('{1}', cfg.connDocTimeout)));
-    }, cfg.connDocTimeout);
+        }, cfg.connDocTimeout);
 
-    socket = TCPSocket.open(options.host, options.port, {
+        socket = TCPSocket.open(options.host, options.port, {
-        binaryType: 'arraybuffer',
+            binaryType: 'arraybuffer',
-        useSecureTransport: options.secure,
+            useSecureTransport: options.secure,
-        ca: options.ca,
+            ca: options.ca,
-        tlsWorkerPath: this._workerPath
+            tlsWorkerPath: self._workerPath
-    });
+        });
 
-    socket.ondata = function() {}; // we don't actually care about the data
+        socket.ondata = function() {}; // we don't actually care about the data
 
-    // [WO-625] Mozilla forbids extensions to the TCPSocket object,
+        // [WO-625] Mozilla forbids extensions to the TCPSocket object,
-    // throws an exception when assigned unexpected callback functions.
+        // throws an exception when assigned unexpected callback functions.
-    // The exception can be safely ignored since we need the callback
+        // The exception can be safely ignored since we need the callback
-    // for the other shims
+        // for the other shims
-    try {
+        try {
-        socket.oncert = function() {
+            socket.oncert = function() {
-            if (options.ca) {
+                if (options.ca) {
-                // the certificate we already have is outdated
+                    // the certificate we already have is outdated
-                error = createError(TLS_WRONG_CERT, str.connDocTlsWrongCert.replace('{0}', host));
+                    error = createError(TLS_WRONG_CERT, str.connDocTlsWrongCert.replace('{0}', host));
+                }
+            };
+        } catch (e) {}
+
+        socket.onerror = function(e) {
+            if (!error) {
+                error = createError(HOST_UNREACHABLE, str.connDocHostUnreachable.replace('{0}', host), e.data);
             }
         };
-    } catch (e) {}
 
-    socket.onerror = function(e) {
+        socket.onopen = function() {
-        if (!error) {
+            socket.close();
-            error = createError(HOST_UNREACHABLE, str.connDocHostUnreachable.replace('{0}', host), e.data);
+        };
-        }
-    };
 
-    socket.onopen = function() {
+        socket.onclose = function() {
-        socket.close();
+            if (!hasTimedOut) {
-    };
+                clearTimeout(timeout);
-
+                if (error) {
-    socket.onclose = function() {
+                    reject(error);
-        if (!hasTimedOut) {
+                } else {
-            clearTimeout(timeout);
+                    resolve();
-            callback(error);
+                }
-        }
+            }
-    };
+        };
+    });
 };
 
 /**
  * Checks if an IMAP server is reachable, accepts the credentials, can list folders and has an inbox and logs out.
  * Adds the certificate to the IMAP settings if not provided.
- *
- * @param {Function} callback(error) Invoked when the test suite passed, or with an error object if something went wrong
  */
-ConnectionDoctor.prototype._checkImap = function(callback) {
+ConnectionDoctor.prototype._checkImap = function() {
-    var self = this,
+    var self = this;
-        loggedIn = false,
+    return self._q(function(resolve, reject) {
-        host = self.credentials.imap.host + ':' + self.credentials.imap.port,
+        var loggedIn = false,
-        str = this._appConfig.string;
+            host = self.credentials.imap.host + ':' + self.credentials.imap.port,
+            str = self._appConfig.string;
 
-    self._imap.onCert = function(pemEncodedCert) {
+        self._imap.onCert = function(pemEncodedCert) {
-        if (!self.credentials.imap.ca) {
+            if (!self.credentials.imap.ca) {
-            self.credentials.imap.ca = pemEncodedCert;
+                self.credentials.imap.ca = pemEncodedCert;
-        }
-    };
-
-    // login and logout do not use error objects in the callback, but rather invoke
-    // the global onError handler, so we need to track if login was successful
-    self._imap.onError = function(error) {
-        if (!loggedIn) {
-            callback(createError(AUTH_REJECTED, str.connDocAuthRejected.replace('{0}', host), error));
-        } else {
-            callback(createError(GENERIC_ERROR, str.connDocGenericError.replace('{0}', host).replace('{1}', error.message), error));
-        }
-    };
-
-    self._imap.login(function() {
-        loggedIn = true;
-
-        self._imap.listWellKnownFolders(function(error, wellKnownFolders) {
-            if (error) {
-                return callback(createError(GENERIC_ERROR, str.connDocGenericError.replace('{0}', host).replace('{1}', error.message), error));
             }
+        };
 
-            if (wellKnownFolders.Inbox.length === 0) {
+        // login and logout do not use error objects in the callback, but rather invoke
-                // the client needs at least an inbox folder to work properly
+        // the global onError handler, so we need to track if login was successful
-                return callback(createError(NO_INBOX, str.connDocNoInbox.replace('{0}', host)));
+        self._imap.onError = function(error) {
+            if (!loggedIn) {
+                reject(createError(AUTH_REJECTED, str.connDocAuthRejected.replace('{0}', host), error));
+            } else {
+                reject(createError(GENERIC_ERROR, str.connDocGenericError.replace('{0}', host).replace('{1}', error.message), error));
             }
+        };
 
-            self._imap.logout(function() {
+        self._imap.login(function() {
-                callback();
+            loggedIn = true;
+
+            self._imap.listWellKnownFolders(function(error, wellKnownFolders) {
+                if (error) {
+                    reject(createError(GENERIC_ERROR, str.connDocGenericError.replace('{0}', host).replace('{1}', error.message), error));
+                    return;
+                }
+
+                if (wellKnownFolders.Inbox.length === 0) {
+                    // the client needs at least an inbox folder to work properly
+                    reject(createError(NO_INBOX, str.connDocNoInbox.replace('{0}', host)));
+                    return;
+                }
+
+                self._imap.logout(function() {
+                    resolve();
+                });
             });
         });
     });
@@ -250,39 +245,39 @@ ConnectionDoctor.prototype._checkImap = function(callback) {
 /**
  * Checks if an SMTP server is reachable and accepts the credentials and logs out.
  * Adds the certificate to the SMTP settings if not provided.
- *
- * @param {Function} callback(error) Invoked when the test suite passed, or with an error object if something went wrong
  */
-ConnectionDoctor.prototype._checkSmtp = function(callback) {
+ConnectionDoctor.prototype._checkSmtp = function() {
-    var self = this,
+    var self = this;
-        host = self.credentials.smtp.host + ':' + self.credentials.smtp.port,
+    return self._q(function(resolve, reject) {
-        errored = false, // tracks if we need to invoke the callback at onclose or not
+        var host = self.credentials.smtp.host + ':' + self.credentials.smtp.port,
-        str = this._appConfig.string;
+            errored = false, // tracks if we need to invoke the callback at onclose or not
+            str = self._appConfig.string;
 
-    self._smtp.oncert = function(pemEncodedCert) {
+        self._smtp.oncert = function(pemEncodedCert) {
-        if (!self.credentials.smtp.ca) {
+            if (!self.credentials.smtp.ca) {
-            self.credentials.smtp.ca = pemEncodedCert;
+                self.credentials.smtp.ca = pemEncodedCert;
-        }
+            }
-    };
+        };
 
-    self._smtp.onerror = function(error) {
+        self._smtp.onerror = function(error) {
-        if (error) {
+            if (error) {
-            errored = true;
+                errored = true;
-            callback(createError(AUTH_REJECTED, str.connDocAuthRejected.replace('{0}', host), error));
+                reject(createError(AUTH_REJECTED, str.connDocAuthRejected.replace('{0}', host), error));
-        }
+            }
-    };
+        };
 
-    self._smtp.onidle = function() {
+        self._smtp.onidle = function() {
-        self._smtp.quit();
+            self._smtp.quit();
-    };
+        };
 
-    self._smtp.onclose = function() {
+        self._smtp.onclose = function() {
-        if (!errored) {
+            if (!errored) {
-            callback();
+                resolve();
-        }
+            }
-    };
+        };
 
-    self._smtp.connect();
+        self._smtp.connect();
+    });
 };
 
 

test/unit/util/connection-doctor-test.js

@@ -52,7 +52,7 @@ describe('Connection Doctor', function() {
         //
         // Setup SUT
         //
-        doctor = new ConnectionDoctor(appConfig);
+        doctor = new ConnectionDoctor(qMock, appConfig);
         doctor.configure(credentials);
         doctor._imap = imapStub;
         doctor._smtp = smtpStub;
@@ -64,15 +64,14 @@ describe('Connection Doctor', function() {
 
     describe('#_checkOnline', function() {
         it('should check if browser is online', function(done) {
-            doctor._checkOnline(function(error) {
+            if (navigator.onLine) {
-                if (navigator.onLine) {
+                doctor._checkOnline().then(done);
-                    expect(error).to.not.exist;
+            } else {
-                } else {
+                doctor._checkOnline().catch(function(err) {
-                    expect(error).to.exist;
+                    expect(err.code).to.equal(ConnectionDoctor.OFFLINE);
-                    expect(error.code).to.equal(ConnectionDoctor.OFFLINE);
+                    done();
-                }
+                });
-                done();
+            }
-            });
         });
     });
 
@@ -80,8 +79,7 @@ describe('Connection Doctor', function() {
         it('should be able to reach the host w/o cert', function(done) {
             credentials.imap.ca = undefined;
 
-            doctor._checkReachable(credentials.imap, function(error) {
+            doctor._checkReachable(credentials.imap).then(function() {
-                expect(error).to.not.exist;
                 expect(TCPSocket.open.calledOnce).to.be.true;
                 expect(TCPSocket.open.calledWith(credentials.imap.host, credentials.imap.port, {
                     binaryType: 'arraybuffer',
@@ -105,8 +103,7 @@ describe('Connection Doctor', function() {
                 }
             });
 
-            doctor._checkReachable(credentials.imap, function(error) {
+            doctor._checkReachable(credentials.imap).then(function() {
-                expect(error).to.not.exist;
                 expect(TCPSocket.open.calledOnce).to.be.true;
                 expect(TCPSocket.open.calledWith(credentials.imap.host, credentials.imap.port, {
                     binaryType: 'arraybuffer',
@@ -122,8 +119,7 @@ describe('Connection Doctor', function() {
         });
 
         it('should fail w/ wrong cert', function(done) {
-            doctor._checkReachable(credentials.imap, function(error) {
+            doctor._checkReachable(credentials.imap).catch(function(error) {
-                expect(error).to.exist;
                 expect(error.code).to.equal(ConnectionDoctor.TLS_WRONG_CERT);
                 expect(TCPSocket.open.calledOnce).to.be.true;
                 expect(TCPSocket.open.calledWith(credentials.imap.host, credentials.imap.port, {
@@ -142,8 +138,7 @@ describe('Connection Doctor', function() {
         });
 
         it('should fail w/ host unreachable', function(done) {
-            doctor._checkReachable(credentials.imap, function(error) {
+            doctor._checkReachable(credentials.imap).catch(function(error) {
-                expect(error).to.exist;
                 expect(error.code).to.equal(ConnectionDoctor.HOST_UNREACHABLE);
                 expect(TCPSocket.open.calledOnce).to.be.true;
 
@@ -160,8 +155,7 @@ describe('Connection Doctor', function() {
             var origTimeout = cfg.connDocTimeout; // remember timeout from the config to reset it on done
             cfg.connDocTimeout = 20; // set to 20ms for the test
 
-            doctor._checkReachable(credentials.imap, function(error) {
+            doctor._checkReachable(credentials.imap).catch(function(error) {
-                expect(error).to.exist;
                 expect(error.code).to.equal(ConnectionDoctor.HOST_TIMEOUT);
                 expect(TCPSocket.open.calledOnce).to.be.true;
                 cfg.connDocTimeout = origTimeout;
@@ -179,8 +173,7 @@ describe('Connection Doctor', function() {
             });
             imapStub.logout.yieldsAsync();
 
-            doctor._checkImap(function(error) {
+            doctor._checkImap().then(function() {
-                expect(error).to.not.exist;
                 expect(imapStub.login.calledOnce).to.be.true;
                 expect(imapStub.listWellKnownFolders.calledOnce).to.be.true;
                 expect(imapStub.logout.calledOnce).to.be.true;
@@ -195,8 +188,7 @@ describe('Connection Doctor', function() {
                 Inbox: [{}]
             });
 
-            doctor._checkImap(function(error) {
+            doctor._checkImap().catch(function(error) {
-                expect(error).to.exist;
                 expect(error.code).to.equal(ConnectionDoctor.GENERIC_ERROR);
                 expect(error.underlyingError).to.exist;
                 expect(imapStub.login.calledOnce).to.be.true;
@@ -218,8 +210,7 @@ describe('Connection Doctor', function() {
                 Inbox: []
             });
 
-            doctor._checkImap(function(error) {
+            doctor._checkImap().catch(function(error) {
-                expect(error).to.exist;
                 expect(error.code).to.equal(ConnectionDoctor.NO_INBOX);
                 expect(imapStub.login.calledOnce).to.be.true;
                 expect(imapStub.listWellKnownFolders.calledOnce).to.be.true;
@@ -233,8 +224,7 @@ describe('Connection Doctor', function() {
             imapStub.login.yieldsAsync();
             imapStub.listWellKnownFolders.yieldsAsync(new Error());
 
-            doctor._checkImap(function(error) {
+            doctor._checkImap().catch(function(error) {
-                expect(error).to.exist;
                 expect(error.code).to.equal(ConnectionDoctor.GENERIC_ERROR);
                 expect(error.underlyingError).to.exist;
                 expect(imapStub.login.calledOnce).to.be.true;
@@ -246,8 +236,7 @@ describe('Connection Doctor', function() {
         });
 
         it('should fail w/ auth rejected', function(done) {
-            doctor._checkImap(function(error) {
+            doctor._checkImap().catch(function(error) {
-                expect(error).to.exist;
                 expect(error.code).to.equal(ConnectionDoctor.AUTH_REJECTED);
                 expect(error.underlyingError).to.exist;
                 expect(imapStub.login.calledOnce).to.be.true;
@@ -266,8 +255,7 @@ describe('Connection Doctor', function() {
 
     describe('#_checkSmtp', function() {
         it('should perform SMTP login, logout', function(done) {
-            doctor._checkSmtp(function(error) {
+            doctor._checkSmtp().then(function() {
-                expect(error).to.not.exist;
                 expect(smtpStub.connect.calledOnce).to.be.true;
                 expect(smtpStub.quit.calledOnce).to.be.true;
 
@@ -279,8 +267,7 @@ describe('Connection Doctor', function() {
         });
 
         it('should fail w/ auth rejected', function(done) {
-            doctor._checkSmtp(function(error) {
+            doctor._checkSmtp().catch(function(error) {
-                expect(error).to.exist;
                 expect(error.code).to.equal(ConnectionDoctor.AUTH_REJECTED);
                 expect(error.underlyingError).to.exist;
                 expect(smtpStub.connect.calledOnce).to.be.true;
@@ -302,14 +289,13 @@ describe('Connection Doctor', function() {
         });
 
         it('should perform all tests', function(done) {
-            doctor._checkOnline.yieldsAsync();
+            doctor._checkOnline.returns(resolves());
-            doctor._checkReachable.withArgs(credentials.imap).yieldsAsync();
+            doctor._checkReachable.withArgs(credentials.imap).returns(resolves());
-            doctor._checkReachable.withArgs(credentials.smtp).yieldsAsync();
+            doctor._checkReachable.withArgs(credentials.smtp).returns(resolves());
-            doctor._checkImap.yieldsAsync();
+            doctor._checkImap.returns(resolves());
-            doctor._checkSmtp.yieldsAsync();
+            doctor._checkSmtp.returns(resolves());
 
-            doctor.check(function(err) {
+            doctor.check().then(function() {
-                expect(err).to.not.exist;
                 expect(doctor._checkOnline.calledOnce).to.be.true;
                 expect(doctor._checkReachable.calledTwice).to.be.true;
                 expect(doctor._checkImap.calledOnce).to.be.true;
@@ -320,13 +306,13 @@ describe('Connection Doctor', function() {
         });
 
         it('should fail for smtp', function(done) {
-            doctor._checkOnline.yieldsAsync();
+            doctor._checkOnline.returns(resolves());
-            doctor._checkReachable.withArgs(credentials.imap).yieldsAsync();
+            doctor._checkReachable.withArgs(credentials.imap).returns(resolves());
-            doctor._checkReachable.withArgs(credentials.smtp).yieldsAsync();
+            doctor._checkReachable.withArgs(credentials.smtp).returns(resolves());
-            doctor._checkImap.yieldsAsync();
+            doctor._checkImap.returns(resolves());
-            doctor._checkSmtp.yieldsAsync(new Error());
+            doctor._checkSmtp.returns(rejects(new Error()));
 
-            doctor.check(function(err) {
+            doctor.check().catch(function(err) {
                 expect(err).to.exist;
                 expect(doctor._checkOnline.calledOnce).to.be.true;
                 expect(doctor._checkReachable.calledTwice).to.be.true;
@@ -338,12 +324,12 @@ describe('Connection Doctor', function() {
         });
 
         it('should fail for imap', function(done) {
-            doctor._checkOnline.yieldsAsync();
+            doctor._checkOnline.returns(resolves());
-            doctor._checkReachable.withArgs(credentials.imap).yieldsAsync();
+            doctor._checkReachable.withArgs(credentials.imap).returns(resolves());
-            doctor._checkReachable.withArgs(credentials.smtp).yieldsAsync();
+            doctor._checkReachable.withArgs(credentials.smtp).returns(resolves());
-            doctor._checkImap.yieldsAsync(new Error());
+            doctor._checkImap.returns(rejects(new Error()));
 
-            doctor.check(function(err) {
+            doctor.check().catch(function(err) {
                 expect(err).to.exist;
                 expect(doctor._checkOnline.calledOnce).to.be.true;
                 expect(doctor._checkReachable.calledTwice).to.be.true;
@@ -355,11 +341,11 @@ describe('Connection Doctor', function() {
         });
 
         it('should fail for smtp reachability', function(done) {
-            doctor._checkOnline.yieldsAsync();
+            doctor._checkOnline.returns(resolves());
-            doctor._checkReachable.withArgs(credentials.imap).yieldsAsync();
+            doctor._checkReachable.withArgs(credentials.imap).returns(resolves());
-            doctor._checkReachable.withArgs(credentials.smtp).yieldsAsync(new Error());
+            doctor._checkReachable.withArgs(credentials.smtp).returns(rejects(new Error()));
 
-            doctor.check(function(err) {
+            doctor.check().catch(function(err) {
                 expect(err).to.exist;
                 expect(doctor._checkOnline.calledOnce).to.be.true;
                 expect(doctor._checkReachable.calledTwice).to.be.true;
@@ -371,10 +357,10 @@ describe('Connection Doctor', function() {
         });
 
         it('should fail for imap reachability', function(done) {
-            doctor._checkOnline.yieldsAsync();
+            doctor._checkOnline.returns(resolves());
-            doctor._checkReachable.withArgs(credentials.imap).yieldsAsync(new Error());
+            doctor._checkReachable.withArgs(credentials.imap).returns(rejects(new Error()));
 
-            doctor.check(function(err) {
+            doctor.check().catch(function(err) {
                 expect(err).to.exist;
                 expect(doctor._checkOnline.calledOnce).to.be.true;
                 expect(doctor._checkReachable.calledOnce).to.be.true;
@@ -386,9 +372,9 @@ describe('Connection Doctor', function() {
         });
 
         it('should fail for offline', function(done) {
-            doctor._checkOnline.yieldsAsync(new Error());
+            doctor._checkOnline.returns(rejects(new Error()));
 
-            doctor.check(function(err) {
+            doctor.check().catch(function(err) {
                 expect(err).to.exist;
                 expect(doctor._checkOnline.calledOnce).to.be.true;
                 expect(doctor._checkReachable.called).to.be.false;
@@ -402,7 +388,7 @@ describe('Connection Doctor', function() {
         it('should fail w/o config', function(done) {
             doctor.credentials = doctor._imap = doctor._smtp = undefined;
 
-            doctor.check(function(err) {
+            doctor.check().catch(function(err) {
                 expect(err).to.exist;
                 expect(doctor._checkOnline.called).to.be.false;
                 expect(doctor._checkReachable.called).to.be.false;