Refactor mail service

Closes #5350
- No longer necessary to initialize via async init().
- Adds a startup-check for mail configuration.
- Creates a notification in the admin client if
  mail transport is "direct" and sending a message fails.

core/server/api/mail.js

@@ -5,9 +5,10 @@ var _            = require('lodash').runInContext(),
     pipeline      = require('../utils/pipeline'),
     config        = require('../config'),
     errors        = require('../errors'),
-    mailer       = require('../mail'),
+    GhostMail     = require('../mail'),
     Models        = require('../models'),
     utils         = require('./utils'),
+    notifications = require('./notifications'),
     path          = require('path'),
     fs            = require('fs'),
     templatesDir  = path.resolve(__dirname, '..', 'mail', 'templates'),
@@ -15,6 +16,9 @@ var _            = require('lodash').runInContext(),
     readFile      = Promise.promisify(fs.readFile),
     docName       = 'mail',
     i18n          = require('../i18n'),
+    mode          = process.env.NODE_ENV,
+    testing       = mode !== 'production' && mode !== 'development',
+    mailer,
     mail;
 
 _.templateSettings.interpolate = /{{([\s\S]+?)}}/g;
@@ -24,10 +28,23 @@ _.templateSettings.interpolate = /{{([\s\S]+?)}}/g;
  */
 
 function sendMail(object) {
-    return mailer.send(object.mail[0].message).catch(function (err) {
+    if (!(mailer instanceof GhostMail) || testing) {
-        err = new errors.EmailError(err.message);
+        mailer = new GhostMail();
+    }
 
-        return Promise.reject(err);
+    return mailer.send(object.mail[0].message).catch(function (err) {
+        if (mailer.state.usingDirect) {
+            notifications.add({notifications: [{
+                type: 'warn',
+                message: [
+                    i18n.t('warnings.index.unableToSendEmail'),
+                    i18n.t('common.seeLinkForInstructions',
+                    {link: '<a href=\'http://support.ghost.org/mail\' target=\'_blank\'>http://support.ghost.org/mail</a>'})
+                ].join(' ')
+            }]}, {context: {internal: true}});
+        }
+
+        return Promise.reject(new errors.EmailError(err.message));
     });
 }
 

core/server/index.js

@@ -14,7 +14,6 @@ var express     = require('express'),
     config      = require('./config'),
     errors      = require('./errors'),
     helpers     = require('./helpers'),
-    mailer      = require('./mail'),
     middleware  = require('./middleware'),
     migrations  = require('./data/migration'),
     models      = require('./models'),
@@ -91,34 +90,6 @@ function builtFilesExist() {
     return Promise.all(deferreds);
 }
 
-// This is run after every initialization is done, right before starting server.
-// Its main purpose is to move adding notifications here, so none of the submodules
-// should need to include api, which previously resulted in circular dependencies.
-// This is also a "one central repository" of adding startup notifications in case
-// in the future apps will want to hook into here
-function initNotifications() {
-    if (mailer.state && mailer.state.usingDirect) {
-        api.notifications.add({notifications: [{
-            type: 'info',
-            message: [
-                i18n.t('warnings.index.usingDirectMethodToSendEmail'),
-                i18n.t('common.seeLinkForInstructions',
-                       {link: '<a href=\'http://support.ghost.org/mail\' target=\'_blank\'>http://support.ghost.org/mail</a>'})
-            ].join(' ')
-        }]}, {context: {internal: true}});
-    }
-    if (mailer.state && mailer.state.emailDisabled) {
-        api.notifications.add({notifications: [{
-            type: 'warn',
-            message: [
-                i18n.t('warnings.index.unableToSendEmail'),
-                i18n.t('common.seeLinkForInstructions',
-                       {link: '<a href=\'http://support.ghost.org/mail\' target=\'_blank\'>http://support.ghost.org/mail</a>'})
-            ].join(' ')
-        }]}, {context: {internal: true}});
-    }
-}
-
 // ## Initialise Ghost
 // Sets up the express server instances, runs init on a bunch of stuff, configures views, helpers, routes and more
 // Finally it returns an instance of GhostServer
@@ -161,8 +132,6 @@ function init(options) {
         return Promise.join(
             // Check for or initialise a dbHash.
             initDbHashAndFirstRun(),
-            // Initialize mail
-            mailer.init(),
             // Initialize apps
             apps.init(),
             // Initialize sitemaps
@@ -173,8 +142,6 @@ function init(options) {
     }).then(function () {
         var adminHbs = hbs.create();
 
-        // Output necessary notifications on init
-        initNotifications();
         // ##Configuration
 
         // return the correct mime type for woff files

core/server/mail/index.js

@@ -7,31 +7,17 @@ var _          = require('lodash'),
     config     = require('../config'),
     i18n       = require('../i18n');
 
-function GhostMailer(opts) {
+function GhostMailer() {
-    opts = opts || {};
+    var transport = config.mail && config.mail.transport || 'direct',
-    this.transport = opts.transport || null;
+        options = config.mail && _.clone(config.mail.options) || {};
+
+    this.state = {};
+
+    this.transport = nodemailer.createTransport(transport, options);
+
+    this.state.usingDirect = transport === 'direct';
 }
 
-// ## Email transport setup
-// *This promise should always resolve to avoid halting Ghost::init*.
-GhostMailer.prototype.init = function () {
-    var self = this;
-    self.state = {};
-    if (config.mail && config.mail.transport) {
-        this.createTransport();
-        return Promise.resolve();
-    }
-
-    self.transport = nodemailer.createTransport('direct');
-    self.state.usingDirect = true;
-
-    return Promise.resolve();
-};
-
-GhostMailer.prototype.createTransport = function () {
-    this.transport = nodemailer.createTransport(config.mail.transport, _.clone(config.mail.options) || {});
-};
-
 GhostMailer.prototype.from = function () {
     var from = config.mail && (config.mail.from || config.mail.fromaddress);
 
@@ -116,4 +102,4 @@ GhostMailer.prototype.send = function (message) {
     });
 };
 
-module.exports = new GhostMailer();
+module.exports = GhostMailer;

core/server/utils/startup-check.js

@@ -21,6 +21,7 @@ checks = {
         this.nodeEnv();
         this.packages();
         this.contentPath();
+        this.mail();
         this.sqlite();
     },
 
@@ -209,6 +210,23 @@ checks = {
 
             process.exit(exitCodes.SQLITE_DB_NOT_WRITABLE);
         }
+    },
+
+    mail: function checkMail() {
+        var configFile,
+            config;
+
+        try {
+            configFile = require(configFilePath);
+            config = configFile[mode];
+        } catch (e) {
+            configFilePath = path.join(appRoot, 'config.example.js');
+        }
+
+        if (!config.mail || !config.mail.transport) {
+            console.error('\x1B[31mWARNING: Ghost is attempting to use a direct method to send email. \nIt is recommended that you explicitly configure an email service.\033[0m');
+            console.error('\x1B[32mHelp and documentation can be found at http://support.ghost.org/mail.\033[0m\n');
+        }
     }
 };
 

core/test/integration/api/api_mail_spec.js

@@ -3,12 +3,8 @@
 var testUtils       = require('../../utils'),
     should          = require('should'),
     config          = require('../../../server/config'),
-    mailer          = require('../../../server/mail'),
     i18n            = require('../../../../core/server/i18n'),
 
-    // Stuff we are testing
-    MailAPI         = require('../../../server/api/mail'),
-
     // test data
     mailData = {
         mail: [{
@@ -27,15 +23,12 @@ describe('Mail API', function () {
     afterEach(testUtils.teardown);
     beforeEach(testUtils.setup('perms:mail', 'perms:init'));
 
-    should.exist(MailAPI);
-
     it('returns a success', function (done) {
         config.set({mail: {transport: 'stub'}});
 
-        mailer.init().then(function () {
+        var MailAPI = require('../../../server/api/mail');
-            mailer.transport.transportType.should.eql('STUB');
+
-            return MailAPI.send(mailData, testUtils.context.internal);
+        MailAPI.send(mailData, testUtils.context.internal).then(function (response) {
-        }).then(function (response) {
             should.exist(response.mail);
             should.exist(response.mail[0].message);
             should.exist(response.mail[0].status);
@@ -48,10 +41,9 @@ describe('Mail API', function () {
     it('returns a boo boo', function (done) {
         config.set({mail: {transport: 'stub', options: {error: 'Stub made a boo boo :('}}});
 
-        mailer.init().then(function () {
+        var MailAPI = require('../../../server/api/mail');
-            mailer.transport.transportType.should.eql('STUB');
+
-            return MailAPI.send(mailData, testUtils.context.internal);
+        MailAPI.send(mailData, testUtils.context.internal).then(function () {
-        }).then(function () {
             done(new Error('Stub did not error'));
         }).catch(function (error) {
             error.message.should.startWith('Error: Stub made a boo boo :(');

core/test/unit/mail_spec.js

@@ -4,9 +4,10 @@ var should          = require('should'),
     Promise         = require('bluebird'),
 
     // Stuff we are testing
-    mailer          = require('../../server/mail'),
+    GhostMail       = require('../../server/mail'),
     configUtils     = require('../utils/configUtils'),
     i18n            = require('../../server/i18n'),
+    mailer,
 
     // Mock SMTP config
     SMTP = {
@@ -40,45 +41,48 @@ i18n.init();
 
 describe('Mail', function () {
     afterEach(function () {
+        mailer = null;
+
         configUtils.restore();
     });
 
     it('should attach mail provider to ghost instance', function () {
+        mailer = new GhostMail();
+
         should.exist(mailer);
-        mailer.should.have.property('init');
-        mailer.should.have.property('transport');
         mailer.should.have.property('send').and.be.a.Function();
     });
 
-    it('should setup SMTP transport on initialization', function (done) {
+    it('should setup SMTP transport on initialization', function () {
         configUtils.set({mail: SMTP});
-        mailer.init().then(function () {
+        mailer = new GhostMail();
+
         mailer.should.have.property('transport');
         mailer.transport.transportType.should.eql('SMTP');
         mailer.transport.sendMail.should.be.a.Function();
-            done();
-        }).catch(done);
     });
 
-    it('should fallback to direct if config is empty', function (done) {
+    it('should fallback to direct if config is empty', function () {
         configUtils.set({mail: {}});
-        mailer.init().then(function () {
+
+        mailer = new GhostMail();
+
         mailer.should.have.property('transport');
         mailer.transport.transportType.should.eql('DIRECT');
-            done();
-        }).catch(done);
     });
 
     it('sends valid message successfully ', function (done) {
         configUtils.set({mail: {transport: 'stub'}});
 
-        mailer.init().then(function () {
+        mailer = new GhostMail();
+
         mailer.transport.transportType.should.eql('STUB');
-            return mailer.send(mailDataNoServer);
+
-        }).then(function (response) {
+        mailer.send(mailDataNoServer).then(function (response) {
             should.exist(response.message);
             should.exist(response.envelope);
             response.envelope.to.should.containEql('joe@example.com');
+
             done();
         }).catch(done);
     });
@@ -86,10 +90,11 @@ describe('Mail', function () {
     it('handles failure', function (done) {
         configUtils.set({mail: {transport: 'stub', options: {error: 'Stub made a boo boo :('}}});
 
-        mailer.init().then(function () {
+        mailer = new GhostMail();
+
         mailer.transport.transportType.should.eql('STUB');
-            return mailer.send(mailDataNoServer);
+
-        }).then(function () {
+        mailer.send(mailDataNoServer).then(function () {
             done(new Error('Stub did not error'));
         }).catch(function (error) {
             error.message.should.eql('Error: Stub made a boo boo :(');
@@ -98,6 +103,8 @@ describe('Mail', function () {
     });
 
     it('should fail to send messages when given insufficient data', function (done) {
+        mailer = new GhostMail();
+
         Promise.settle([
             mailer.send(),
             mailer.send({}),
@@ -114,12 +121,14 @@ describe('Mail', function () {
     });
 
     describe('Direct', function () {
-        beforeEach(function (done) {
+        beforeEach(function () {
             configUtils.set({mail: {}});
 
-            mailer.init().then(function () {
+            mailer = new GhostMail();
-                done();
         });
+
+        afterEach(function () {
+            mailer = null;
         });
 
         it('return correct failure message for domain doesn\'t exist', function (done) {
@@ -163,12 +172,18 @@ describe('Mail', function () {
                     from: '"Blog Title" <static@example.com>'
                 }
             });
+
+            mailer = new GhostMail();
+
             mailer.from().should.equal('"Blog Title" <static@example.com>');
         });
 
         it('should fall back to [blog.title] <ghost@[blog.url]>', function () {
             // Standard domain
             configUtils.set({url: 'http://default.com', mail: {from: null}, theme: {title: 'Test'}});
+
+            mailer = new GhostMail();
+
             mailer.from().should.equal('"Test" <ghost@default.com>');
 
             // Trailing slash
@@ -183,12 +198,18 @@ describe('Mail', function () {
         it('should use mail.from if both from and fromaddress are present', function () {
             // Standard domain
             configUtils.set({mail: {from: '"bar" <from@default.com>', fromaddress: '"Qux" <fa@default.com>'}});
+
+            mailer = new GhostMail();
+
             mailer.from().should.equal('"bar" <from@default.com>');
         });
 
         it('should attach blog title if from or fromaddress are only email addresses', function () {
             // from and fromaddress are both set
             configUtils.set({mail: {from: 'from@default.com', fromaddress: 'fa@default.com'}, theme: {title: 'Test'}});
+
+            mailer = new GhostMail();
+
             mailer.from().should.equal('"Test" <from@default.com>');
 
             // only from set
@@ -203,6 +224,9 @@ describe('Mail', function () {
         it('should ignore theme title if from address is Title <email@address.com> format', function () {
             // from and fromaddress are both set
             configUtils.set({mail: {from: '"R2D2" <from@default.com>', fromaddress: '"C3PO" <fa@default.com>'}, theme: {title: 'Test'}});
+
+            mailer = new GhostMail();
+
             mailer.from().should.equal('"R2D2" <from@default.com>');
 
             // only from set
@@ -216,6 +240,9 @@ describe('Mail', function () {
 
         it('should use default title if not theme title is provided', function () {
             configUtils.set({url: 'http://default.com:2368/', mail: {from: null}, theme: {title: null}});
+
+            mailer = new GhostMail();
+
             mailer.from().should.equal('"Ghost at default.com" <ghost@default.com>');
         });
     });