Refactored GhostMailer into a class

no-issue

This breaks down the send method into distinct components that are
easier to reason about

core/server/services/mail/GhostMailer.js

@@ -8,15 +8,9 @@ const common = require('../../lib/common');
 const settingsCache = require('../settings/cache');
 const urlUtils = require('../../lib/url-utils');
 
-function GhostMailer() {
+const helpMessage = common.i18n.t('errors.api.authentication.checkEmailConfigInstructions', {url: 'https://ghost.org/docs/concepts/config/#mail'});
-    var nodemailer = require('nodemailer'),
+const defaultErrorMessage = common.i18n.t('errors.mail.failedSendingEmail.error');
-        transport = config.get('mail') && config.get('mail').transport || 'direct',
-        options = config.get('mail') && _.clone(config.get('mail').options) || {};
 
-    this.state = {};
-    this.transport = nodemailer.createTransport(transport, options);
-    this.state.usingDirect = transport === 'direct';
-}
 function getDomain() {
     const domain = urlUtils.urlFor('home', true).match(new RegExp('^https?://([^/:?#]+)(?:[/:?#]|$)', 'i'));
     return domain && domain[1];
@@ -41,75 +35,93 @@ function getFromAddress() {
     return address;
 }
 
-// Sends an email message enforcing `to` (blog owner) and `from` fields
+function createMessage(message) {
-// This assumes that api.settings.read('email') was already done on the API level
+    return Object.assign({}, message, {
-GhostMailer.prototype.send = function (message) {
+        from: getFromAddress(),
-    var self = this,
-        to,
-        help = common.i18n.t('errors.api.authentication.checkEmailConfigInstructions', {url: 'https://ghost.org/docs/concepts/config/#mail'}),
-        errorMessage = common.i18n.t('errors.mail.failedSendingEmail.error');
-
-    // important to clone message as we modify it
-    message = _.clone(message) || {};
-    to = message.to || false;
-
-    if (!(message && message.subject && message.html && message.to)) {
-        return Promise.reject(new common.errors.EmailError({
-            message: common.i18n.t('errors.mail.incompleteMessageData.error'),
-            help: help
-        }));
-    }
-
-    message = _.extend(message, {
-        from: self.from(),
-        to: to,
         generateTextFromHTML: true,
         encoding: 'base64'
     });
+}
 
-    return new Promise(function (resolve, reject) {
+function createMailError({message, err, ignoreDefaultMessage} = {message: ''}) {
-        self.transport.sendMail(message, function (err, response) {
+    const fullErrorMessage = defaultErrorMessage + message;
-            if (err) {
+    return new common.errors.EmailError({
-                errorMessage += common.i18n.t('errors.mail.reason', {reason: err.message || err});
+        message: ignoreDefaultMessage ? message : fullErrorMessage,
-
-                return reject(new common.errors.EmailError({
-                    message: errorMessage,
         err: err,
-                    help: help
+        help: helpMessage
+    });
+}
+
+module.exports = class GhostMailer {
+    constructor() {
+        const nodemailer = require('nodemailer');
+        const transport = config.get('mail') && config.get('mail').transport || 'direct';
+        // nodemailer mutates the options passed to createTransport
+        const options = config.get('mail') && _.clone(config.get('mail').options) || {};
+
+        this.state = {
+            usingDirect: transport === 'direct'
+        };
+        this.transport = nodemailer.createTransport(transport, options);
+    }
+
+    send(message) {
+        if (!(message && message.subject && message.html && message.to)) {
+            return Promise.reject(createMailError({
+                message: common.i18n.t('errors.mail.incompleteMessageData.error'),
+                ignoreDefaultMessage: true
             }));
         }
 
-            if (self.transport.transportType !== 'DIRECT') {
+        const messageToSend = createMessage(message);
-                return resolve(response);
+
+        return this.sendMail(messageToSend).then((response) => {
+            if (this.transport.transportType === 'DIRECT') {
+                return this.handleDirectTransportResponse(response);
+            }
+            return response;
+        });
     }
 
+    sendMail(message) {
+        return new Promise((resolve, reject) => {
+            this.transport.sendMail(message, (err, response) => {
+                if (err) {
+                    reject(createMailError({
+                        message: common.i18n.t('errors.mail.reason', {reason: err.message || err}),
+                        err
+                    }));
+                }
+                resolve(response);
+            });
+        });
+    }
+
+    handleDirectTransportResponse(response) {
+        return new Promise((resolve, reject) => {
             response.statusHandler.once('failed', function (data) {
                 if (data.error && data.error.errno === 'ENOTFOUND') {
-                    errorMessage += common.i18n.t('errors.mail.noMailServerAtAddress.error', {domain: data.domain});
+                    reject(createMailError({
+                        message: common.i18n.t('errors.mail.noMailServerAtAddress.error', {domain: data.domain})
+                    }));
                 }
 
-                return reject(new common.errors.EmailError({
+                reject(createMailError());
-                    message: errorMessage,
-                    help: help
-                }));
             });
 
             response.statusHandler.once('requeue', function (data) {
                 if (data.error && data.error.message) {
-                    errorMessage += common.i18n.t('errors.mail.reason', {reason: data.error.message});
+                    reject(createMailError({
+                        message: common.i18n.t('errors.mail.reason', {reason: data.error.message})
+                    }));
                 }
 
-                return reject(new common.errors.EmailError({
+                reject(createMailError());
-                    message: errorMessage,
-                    help: help
-                }));
             });
 
             response.statusHandler.once('sent', function () {
-                return resolve(common.i18n.t('notices.mail.messageSent'));
+                resolve(common.i18n.t('notices.mail.messageSent'));
-            });
             });
         });
+    }
 };
-
-module.exports = GhostMailer;

core/test/unit/services/mail/GhostMailer_spec.js

@@ -167,7 +167,7 @@ describe('Mail: Ghostmailer', function () {
     });
 
     describe('From address', function () {
-        it('should use the config', function () {
+        it('should use the config', async function () {
             configUtils.set({
                 mail: {
                     from: '"Blog Title" <static@example.com>'
@@ -176,89 +176,175 @@ describe('Mail: Ghostmailer', function () {
 
             mailer = new mail.GhostMailer();
 
-            mailer.from().should.equal('"Blog Title" <static@example.com>');
+            sandbox.stub(mailer, 'sendMail').resolves();
+            mailer.transport.transportType = 'NOT DIRECT';
+
+            await mailer.send({
+                to: 'user@example.com',
+                subject: 'subject',
+                html: 'content'
+            });
+
+            mailer.sendMail.firstCall.args[0].from.should.equal('"Blog Title" <static@example.com>');
         });
 
         describe('should fall back to [blog.title] <noreply@[blog.url]>', function () {
             let mailer;
 
-            beforeEach(function () {
+            beforeEach(async function () {
                 mailer = new mail.GhostMailer();
+                sandbox.stub(mailer, 'sendMail').resolves();
+                mailer.transport.transportType = 'NOT DIRECT';
                 sandbox.stub(settingsCache, 'get').returns('Test');
             });
 
-            it('standard domain', function () {
+            it('standard domain', async function () {
                 sandbox.stub(urlUtils, 'urlFor').returns('http://default.com');
                 configUtils.set({mail: {from: null}});
 
-                mailer.from().should.equal('"Test" <noreply@default.com>');
+                await mailer.send({
+                    to: 'user@example.com',
+                    subject: 'subject',
+                    html: 'content'
                 });
 
-            it('trailing slash', function () {
+                mailer.sendMail.firstCall.args[0].from.should.equal('"Test" <noreply@default.com>');
+            });
+
+            it('trailing slash', async function () {
                 sandbox.stub(urlUtils, 'urlFor').returns('http://default.com/');
                 configUtils.set({mail: {from: null}});
 
-                mailer.from().should.equal('"Test" <noreply@default.com>');
+                await mailer.send({
+                    to: 'user@example.com',
+                    subject: 'subject',
+                    html: 'content'
                 });
 
-            it('strip port', function () {
+                mailer.sendMail.firstCall.args[0].from.should.equal('"Test" <noreply@default.com>');
+            });
+
+            it('strip port', async function () {
                 sandbox.stub(urlUtils, 'urlFor').returns('http://default.com:2368/');
                 configUtils.set({mail: {from: null}});
-                mailer.from().should.equal('"Test" <noreply@default.com>');
+
+                await mailer.send({
+                    to: 'user@example.com',
+                    subject: 'subject',
+                    html: 'content'
                 });
 
-            it('Escape title', function () {
+                mailer.sendMail.firstCall.args[0].from.should.equal('"Test" <noreply@default.com>');
+            });
+
+            it('Escape title', async function () {
                 settingsCache.get.restore();
                 sandbox.stub(settingsCache, 'get').returns('Test"');
 
                 sandbox.stub(urlUtils, 'urlFor').returns('http://default.com:2368/');
                 configUtils.set({mail: {from: null}});
-                mailer.from().should.equal('"Test\\"" <noreply@default.com>');
+
+                await mailer.send({
+                    to: 'user@example.com',
+                    subject: 'subject',
+                    html: 'content'
+                });
+
+                mailer.sendMail.firstCall.args[0].from.should.equal('"Test\\"" <noreply@default.com>');
             });
         });
 
-        it('should use mail.from', function () {
+        it('should use mail.from', async function () {
             // Standard domain
             configUtils.set({mail: {from: '"bar" <from@default.com>'}});
 
             mailer = new mail.GhostMailer();
 
-            mailer.from().should.equal('"bar" <from@default.com>');
+            sandbox.stub(mailer, 'sendMail').resolves();
+            mailer.transport.transportType = 'NOT DIRECT';
+
+            await mailer.send({
+                to: 'user@example.com',
+                subject: 'subject',
+                html: 'content'
             });
 
-        it('should attach blog title', function () {
+            mailer.sendMail.firstCall.args[0].from.should.equal('"bar" <from@default.com>');
+        });
+
+        it('should attach blog title', async function () {
             sandbox.stub(settingsCache, 'get').returns('Test');
 
             configUtils.set({mail: {from: 'from@default.com'}});
 
             mailer = new mail.GhostMailer();
 
-            mailer.from().should.equal('"Test" <from@default.com>');
+            sandbox.stub(mailer, 'sendMail').resolves();
+            mailer.transport.transportType = 'NOT DIRECT';
+
+            await mailer.send({
+                to: 'user@example.com',
+                subject: 'subject',
+                html: 'content'
+            });
+
+            mailer.sendMail.firstCall.args[0].from.should.equal('"Test" <from@default.com>');
 
             // only from set
             configUtils.set({mail: {from: 'from@default.com'}});
-            mailer.from().should.equal('"Test" <from@default.com>');
+
+            await mailer.send({
+                to: 'user@example.com',
+                subject: 'subject',
+                html: 'content'
             });
 
-        it('should ignore theme title if from address is Title <email@address.com> format', function () {
+            mailer.sendMail.firstCall.args[0].from.should.equal('"Test" <from@default.com>');
+        });
+
+        it('should ignore theme title if from address is Title <email@address.com> format', async function () {
             configUtils.set({mail: {from: '"R2D2" <from@default.com>'}});
 
             mailer = new mail.GhostMailer();
 
-            mailer.from().should.equal('"R2D2" <from@default.com>');
+            sandbox.stub(mailer, 'sendMail').resolves();
+            mailer.transport.transportType = 'NOT DIRECT';
+
+            await mailer.send({
+                to: 'user@example.com',
+                subject: 'subject',
+                html: 'content'
+            });
+
+            mailer.sendMail.firstCall.args[0].from.should.equal('"R2D2" <from@default.com>');
 
             // only from set
             configUtils.set({mail: {from: '"R2D2" <from@default.com>'}});
-            mailer.from().should.equal('"R2D2" <from@default.com>');
+            await mailer.send({
+                to: 'user@example.com',
+                subject: 'subject',
+                html: 'content'
             });
 
-        it('should use default title if not theme title is provided', function () {
+            mailer.sendMail.firstCall.args[0].from.should.equal('"R2D2" <from@default.com>');
+        });
+
+        it('should use default title if not theme title is provided', async function () {
             configUtils.set({mail: {from: null}});
             sandbox.stub(urlUtils, 'urlFor').returns('http://default.com:2368/');
 
             mailer = new mail.GhostMailer();
 
-            mailer.from().should.equal('"Ghost at default.com" <noreply@default.com>');
+            sandbox.stub(mailer, 'sendMail').resolves();
+            mailer.transport.transportType = 'NOT DIRECT';
+
+            await mailer.send({
+                to: 'user@example.com',
+                subject: 'subject',
+                html: 'content'
+            });
+
+            mailer.sendMail.firstCall.args[0].from.should.equal('"Ghost at default.com" <noreply@default.com>');
         });
     });
 });