Refactor outbox

src/js/email/outbox.js

@@ -56,8 +56,9 @@ Outbox.prototype.stopChecking = function() {
  * Put a email dto in the outbox for sending when ready
  * @param  {Object}   mail     The Email DTO
  * @param  {Function} callback Invoked when the object was encrypted and persisted to disk
+ * @returns {Promise}
  */
-Outbox.prototype.put = function(mail, callback) {
+Outbox.prototype.put = function(mail) {
     var self = this,
         allReaders = mail.from.concat(mail.to.concat(mail.cc.concat(mail.bcc))); // all the users that should be able to read the mail
 
@@ -66,67 +67,46 @@ Outbox.prototype.put = function(mail, callback) {
 
     // do not encrypt mails with a bcc recipient, due to a possible privacy leak
     if (mail.bcc.length > 0) {
-        storeAndForward(mail);
+        return storeAndForward(mail);
-        return;
     }
 
-    checkRecipients(allReaders);
+    return checkRecipients(allReaders).then(checkEncrypt);
 
     // check if there are unregistered recipients
     function checkRecipients(recipients) {
-        var after = _.after(recipients.length, function() {
+        var pubkeyJobs = [];
-            checkEncrypt();
-        });
-
-        // find out if there are unregistered users
         recipients.forEach(function(recipient) {
-            self._keychain.getReceiverPublicKey(recipient.address, function(err, key) {
+            var promise = self._keychain.getReceiverPublicKey(recipient.address).then(function(key) {
-                if (err) {
-                    callback(err);
-                    return;
-                }
-
                 // if a public key is available, add the recipient's key to the armored public keys,
                 // otherwise remember the recipient as unregistered for later sending
                 if (key) {
                     mail.publicKeysArmored.push(key.publicKey);
                 }
+            });
+            pubkeyJobs.push(promise);
+        });
 
-                after();
+        return Promise.all(pubkeyJobs);
-            });
-        });
     }
 
     function checkEncrypt() {
         // only encrypt if all recipients have public keys
         if (mail.publicKeysArmored.length < allReaders.length) {
-            storeAndForward(mail);
+            return storeAndForward(mail);
-            return;
         }
 
         // encrypts the body and attachments and persists the mail object
-        self._emailDao.encrypt({
+        return self._emailDao.encrypt({
             mail: mail,
             publicKeysArmored: mail.publicKeysArmored
-        }, function(err) {
+        }).then(function() {
-            if (err) {
+            return storeAndForward(mail);
-                callback(err);
-                return;
-            }
-
-            storeAndForward(mail);
         });
     }
 
     function storeAndForward(mail) {
         // store in outbox
-        self._devicestorage.storeList([mail], outboxDb, function(err) {
+        return self._devicestorage.storeList([mail], outboxDb).then(function() {
-            if (err) {
-                callback(err);
-                return;
-            }
-
-            callback();
             // don't wait for next round
             self._processOutbox(self._onUpdate);
         });
@@ -149,83 +129,63 @@ Outbox.prototype._processOutbox = function(callback) {
     self._outboxBusy = true;
 
     // get pending mails from the outbox
-    self._devicestorage.listItems(outboxDb, 0, null, function(err, pendingMails) {
+    self._devicestorage.listItems(outboxDb, 0, null).then(function(pendingMails) {
-        // error, we're done here
+        // if we're not online, don't even bother sending mails.
-        if (err) {
+        if (!self._emailDao._account.online || _.isEmpty(pendingMails)) {
-            self._outboxBusy = false;
+            unsentMails = pendingMails.length;
-            callback(err);
             return;
         }
 
-        // if we're not online, don't even bother sending mails.
+        var sendJobs = [];
-        if (!self._emailDao._account.online || _.isEmpty(pendingMails)) {
+        // send pending mails if possible
-            self._outboxBusy = false;
+        pendingMails.forEach(function(mail) {
-            callback(null, pendingMails.length);
+            sendJobs.push(send(mail));
-            return;
+        });
-        }
 
         // we're done after all the mails have been handled
         // update the outbox count...
-        var after = _.after(pendingMails.length, function() {
+        return Promise.all(sendJobs);
+
+    }).then(function() {
         self._outboxBusy = false;
         callback(null, unsentMails);
-        });
 
-        // send pending mails if possible
+    }).catch(function(err) {
-        pendingMails.forEach(function(mail) {
+        self._outboxBusy = false;
-            send(mail, after);
+        callback(err);
-        });
     });
 
     // send the message
-    function send(mail, done) {
+    function send(mail) {
-
         // check is email is to be sent encrypted or as plaintex
         if (mail.encrypted === true) {
             // email was already encrypted before persisting in outbox, tell pgpmailer to send encrypted and not encrypt again
-            self._emailDao.sendEncrypted({
+            return self._emailDao.sendEncrypted({
                 email: mail
-            }, onSend);
+            }).then(onSend).catch(sendFailed);
         } else {
             // send email as plaintext
-            self._emailDao.sendPlaintext({
+            return self._emailDao.sendPlaintext({
                 email: mail
-            }, onSend);
+            }).then(onSend).catch(sendFailed);
         }
 
-        function onSend(err) {
+        function onSend() {
-            if (err) {
-                self._outboxBusy = false;
-                if (err.code === 42) {
-                    // offline try again later
-                    done();
-                } else {
-                    self._outboxBusy = false;
-                    callback(err);
-                }
-                return;
-            }
-
-            // remove the pending mail from the storage
-            removeFromStorage(mail, done);
-
             // fire sent notification
             if (typeof self.onSent === 'function') {
                 self.onSent(mail);
             }
-        }
-    }
 
             // removes the mail object from disk after successfully sending it
-    function removeFromStorage(mail, done) {
+            return self._devicestorage.removeList(outboxDb + '_' + mail.uid);
-        self._devicestorage.removeList(outboxDb + '_' + mail.uid, function(err) {
-            if (err) {
-                self._outboxBusy = false;
-                callback(err);
-                return;
         }
 
-            done();
+        function sendFailed(err) {
-        });
+            if (err.code === 42) {
+                // offline. resolve promise and try again later
+                return;
+            }
+            throw err;
+        }
     }
 };

test/unit/email/outbox-bo-test.js

@@ -5,7 +5,7 @@ var OutboxBO = require('../../../src/js/email/outbox'),
     EmailDAO = require('../../../src/js/email/email'),
     DeviceStorageDAO = require('../../../src/js/service/devicestorage');
 
-describe('Outbox Business Object unit test', function() {
+describe('Outbox unit test', function() {
     var outbox, emailDaoStub, devicestorageStub, keychainStub,
         dummyUser = 'spiderpig@springfield.com';
 
@@ -42,6 +42,13 @@ describe('Outbox Business Object unit test', function() {
     });
 
     describe('put', function() {
+        beforeEach(function() {
+            sinon.stub(outbox, '_processOutbox');
+        });
+        afterEach(function() {
+            outbox._processOutbox.restore();
+        });
+
         it('should not encrypt and store a mail', function(done) {
             var mail, senderKey, receiverKey;
 
@@ -67,15 +74,13 @@ describe('Outbox Business Object unit test', function() {
                 bcc: []
             };
 
-            keychainStub.getReceiverPublicKey.withArgs(mail.from[0].address).yieldsAsync(null, senderKey);
+            keychainStub.getReceiverPublicKey.withArgs(mail.from[0].address).returns(resolves(senderKey));
-            keychainStub.getReceiverPublicKey.withArgs(mail.to[0].address).yieldsAsync(null, receiverKey);
+            keychainStub.getReceiverPublicKey.withArgs(mail.to[0].address).returns(resolves(receiverKey));
-            keychainStub.getReceiverPublicKey.withArgs(mail.to[1].address).yieldsAsync();
+            keychainStub.getReceiverPublicKey.withArgs(mail.to[1].address).returns(resolves());
 
-            devicestorageStub.storeList.withArgs([mail]).yieldsAsync();
+            devicestorageStub.storeList.withArgs([mail]).returns(resolves());
-
-            outbox.put(mail, function(error) {
-                expect(error).to.not.exist;
 
+            outbox.put(mail).then(function() {
                 expect(mail.publicKeysArmored.length).to.equal(2);
                 expect(emailDaoStub.encrypt.called).to.be.false;
                 expect(devicestorageStub.storeList.calledOnce).to.be.true;
@@ -103,11 +108,9 @@ describe('Outbox Business Object unit test', function() {
                 }]
             };
 
-            devicestorageStub.storeList.withArgs([mail]).yieldsAsync();
+            devicestorageStub.storeList.withArgs([mail]).returns(resolves());
-
-            outbox.put(mail, function(error) {
-                expect(error).to.not.exist;
 
+            outbox.put(mail).then(function() {
                 expect(mail.publicKeysArmored.length).to.equal(0);
                 expect(keychainStub.getReceiverPublicKey.called).to.be.false;
                 expect(emailDaoStub.encrypt.called).to.be.false;
@@ -142,20 +145,18 @@ describe('Outbox Business Object unit test', function() {
                 bcc: []
             };
 
-            keychainStub.getReceiverPublicKey.withArgs(mail.from[0].address).yieldsAsync(null, senderKey);
+            keychainStub.getReceiverPublicKey.withArgs(mail.from[0].address).returns(resolves(senderKey));
-            keychainStub.getReceiverPublicKey.withArgs(mail.to[0].address).yieldsAsync(null, receiverKey);
+            keychainStub.getReceiverPublicKey.withArgs(mail.to[0].address).returns(resolves(receiverKey));
-            keychainStub.getReceiverPublicKey.withArgs(mail.to[1].address).yieldsAsync(null, receiverKey);
+            keychainStub.getReceiverPublicKey.withArgs(mail.to[1].address).returns(resolves(receiverKey));
 
             emailDaoStub.encrypt.withArgs({
                 mail: mail,
                 publicKeysArmored: [senderKey.publicKey, receiverKey.publicKey, receiverKey.publicKey]
-            }).yieldsAsync();
+            }).returns(resolves());
 
-            devicestorageStub.storeList.withArgs([mail]).yieldsAsync();
+            devicestorageStub.storeList.withArgs([mail]).returns(resolves());
-
-            outbox.put(mail, function(error) {
-                expect(error).to.not.exist;
 
+            outbox.put(mail).then(function() {
                 expect(mail.publicKeysArmored.length).to.equal(3);
                 expect(emailDaoStub.encrypt.calledOnce).to.be.true;
                 expect(devicestorageStub.storeList.calledOnce).to.be.true;
@@ -230,19 +231,19 @@ describe('Outbox Business Object unit test', function() {
 
             dummyMails = [member, invited, notinvited, newlyjoined];
 
-            devicestorageStub.listItems.yieldsAsync(null, dummyMails);
+            devicestorageStub.listItems.returns(resolves(dummyMails));
 
-            emailDaoStub.sendPlaintext.yieldsAsync();
+            emailDaoStub.sendPlaintext.returns(resolves());
 
             emailDaoStub.sendEncrypted.withArgs({
                 email: newlyjoined
-            }).yieldsAsync();
+            }).returns(resolves());
 
             emailDaoStub.sendEncrypted.withArgs({
                 email: member
-            }).yieldsAsync();
+            }).returns(resolves());
 
-            devicestorageStub.removeList.yieldsAsync();
+            devicestorageStub.removeList.returns(resolves());
 
             function onOutboxUpdate(err, count) {
                 expect(err).to.not.exist;
@@ -263,7 +264,7 @@ describe('Outbox Business Object unit test', function() {
 
         it('should not process outbox in offline mode', function(done) {
             emailDaoStub._account.online = false;
-            devicestorageStub.listItems.yieldsAsync(null, [{}]);
+            devicestorageStub.listItems.returns(resolves([{}]));
 
             outbox._processOutbox(function(err, count) {
                 expect(err).to.not.exist;