Move xmlrpc & slack to services (#9179)

refs #9178

- Introduce the /services/ folder
- Move xmlrpc there
- Move slack there
- In slack: remove a usage of the settings API that should use settingsCache
- In slack: Simplify the tests 
- Various tiny changes to move towards code consistency

core/server/data/slack/index.js

@@ -1,126 +0,0 @@
-var https           = require('https'),
-    url             = require('url'),
-    Promise         = require('bluebird'),
-    errors          = require('../../errors'),
-    logging         = require('../../logging'),
-    utils           = require('../../utils'),
-    blogIconUtils   = require('../../utils/blog-icon'),
-    events          = require('../../events'),
-    api             = require('../../api/settings'),
-    i18n            = require('../../i18n'),
-    schema          = require('../schema').checks,
-    req,
-    slackData = {};
-
-function getSlackSettings() {
-    return api.read({context: {internal: true}, key: 'slack'}).then(function (response) {
-        var slackSetting = response.settings[0].value;
-
-        try {
-            slackSetting = JSON.parse(slackSetting);
-        } catch (e) {
-            return Promise.reject(e);
-        }
-
-        return slackSetting[0];
-    });
-}
-
-function makeRequest(reqOptions, reqPayload) {
-    req = https.request(reqOptions);
-
-    reqPayload = JSON.stringify(reqPayload);
-
-    req.write(reqPayload);
-    req.on('error', function (err) {
-        logging.error(new errors.GhostError({
-            err: err,
-            context: i18n.t('errors.data.xml.xmlrpc.pingUpdateFailed.error'),
-            help: i18n.t('errors.data.xml.xmlrpc.pingUpdateFailed.help', {url: 'http://docs.ghost.org'})
-        }));
-    });
-
-    req.end();
-}
-
-function ping(post) {
-    var message, reqOptions;
-
-    // If this is a post, we want to send the link of the post
-    if (schema.isPost(post)) {
-        message = utils.url.urlFor('post', {post: post}, true);
-    } else {
-        message = post.message;
-    }
-
-    return getSlackSettings().then(function (slackSettings) {
-        // Quit here if slack integration is not activated
-        var defaultPostSlugs = [
-            'welcome',
-            'the-editor',
-            'using-tags',
-            'managing-users',
-            'private-sites',
-            'advanced-markdown',
-            'themes'
-        ];
-
-        if (slackSettings.url && slackSettings.url !== '') {
-            // Only ping when not a page
-            if (post.page) {
-                return;
-            }
-
-            // Don't ping for the default posts.
-            // This also handles the case where during ghost's first run
-            // models.init() inserts this post but permissions.init() hasn't
-            // (can't) run yet.
-            if (defaultPostSlugs.indexOf(post.slug) > -1) {
-                return;
-            }
-
-            slackData = {
-                text: message,
-                unfurl_links: true,
-                icon_url: blogIconUtils.getIconUrl(true),
-                username: 'Ghost'
-            };
-
-            // fill the options for https request
-            reqOptions = url.parse(slackSettings.url);
-            reqOptions.method = 'POST';
-            reqOptions.headers = {'Content-type': 'application/json'};
-
-            // with all the data we have, we're doing the request now
-            makeRequest(reqOptions, slackData);
-        } else {
-            return;
-        }
-    });
-}
-
-function listener(model, options) {
-    // CASE: do not ping slack if we import a database
-    // TODO: refactor post.published events to never fire on importing
-    if (options && options.importing) {
-        return;
-    }
-
-    ping(model.toJSON());
-}
-
-function testPing() {
-    ping({
-        message: 'Heya! This is a test notification from your Ghost blog :simple_smile:. Seems to work fine!'
-    });
-}
-
-function listen() {
-    events.on('post.published', listener);
-    events.on('slack.test', testPing);
-}
-
-// Public API
-module.exports = {
-    listen: listen
-};

core/server/index.js

@@ -23,8 +23,8 @@ var debug = require('ghost-ignition').debug('boot:init'),
     apps = require('./apps'),
     auth = require('./auth'),
     dbHealth = require('./data/db/health'),
-    xmlrpc = require('./data/xml/xmlrpc'),
-    slack = require('./data/slack'),
+    xmlrpc = require('./services/xmlrpc'),
+    slack = require('./services/slack'),
     GhostServer = require('./ghost-server'),
     scheduling = require('./adapters/scheduling'),
     settings = require('./settings'),

core/server/services/slack.js

@@ -0,0 +1,115 @@
+var https = require('https'),
+    url = require('url'),
+    errors = require('../errors'),
+    logging = require('../logging'),
+    utils = require('../utils'),
+    blogIconUtils = require('../utils/blog-icon'),
+    events = require('../events'),
+    settingsCache = require('../settings/cache'),
+    i18n = require('../i18n'),
+    schema = require('../data/schema').checks,
+    defaultPostSlugs = [
+        'welcome',
+        'the-editor',
+        'using-tags',
+        'managing-users',
+        'private-sites',
+        'advanced-markdown',
+        'themes'
+    ];
+
+function getSlackSettings() {
+    var setting = settingsCache.get('slack');
+    // This might one day have multiple entries, for now its always a array
+    // and we return the first item or an empty object
+    return setting ? setting[0] : {};
+}
+
+function makeRequest(reqOptions, reqPayload) {
+    var req = https.request(reqOptions);
+
+    reqPayload = JSON.stringify(reqPayload);
+
+    req.write(reqPayload);
+    req.on('error', function (err) {
+        logging.error(new errors.GhostError({
+            err: err,
+            context: i18n.t('errors.services.ping.requestFailed.error', {service: 'slack'}),
+            help: i18n.t('errors.services.ping.requestFailed.help', {url: 'http://docs.ghost.org'})
+        }));
+    });
+
+    req.end();
+}
+
+function ping(post) {
+    var message,
+        reqOptions,
+        slackData = {},
+        slackSettings = getSlackSettings();
+
+    // If this is a post, we want to send the link of the post
+    if (schema.isPost(post)) {
+        message = utils.url.urlFor('post', {post: post}, true);
+    } else {
+        message = post.message;
+    }
+    // Quit here if slack integration is not activated
+    if (slackSettings.url && slackSettings.url !== '') {
+        // Only ping when not a page
+        if (post.page) {
+            return;
+        }
+
+        // Don't ping for the default posts.
+        // This also handles the case where during ghost's first run
+        // models.init() inserts this post but permissions.init() hasn't
+        // (can't) run yet.
+        if (defaultPostSlugs.indexOf(post.slug) > -1) {
+            return;
+        }
+
+        slackData = {
+            text: message,
+            unfurl_links: true,
+            icon_url: blogIconUtils.getIconUrl(true),
+            username: 'Ghost'
+        };
+
+        // fill the options for https request
+        reqOptions = url.parse(slackSettings.url);
+        reqOptions.method = 'POST';
+        reqOptions.headers = {'Content-type': 'application/json'};
+
+        // with all the data we have, we're doing the request now
+        makeRequest(reqOptions, slackData);
+    } else {
+        return;
+    }
+}
+
+function listener(model, options) {
+    // CASE: do not ping slack if we import a database
+    // TODO: refactor post.published events to never fire on importing
+    if (options && options.importing) {
+        return;
+    }
+
+    ping(model.toJSON());
+}
+
+function testPing() {
+    ping({
+        message: 'Heya! This is a test notification from your Ghost blog :simple_smile:. Seems to work fine!'
+    });
+}
+
+function listen() {
+    events.on('post.published', listener);
+    events.on('slack.test', testPing);
+}
+
+// Public API
+module.exports = {
+    listen: listen
+};

core/server/services/xmlrpc.js

@@ -1,37 +1,36 @@
 var _ = require('lodash'),
     http = require('http'),
     xml = require('xml'),
-    config = require('../../config'),
-    utils = require('../../utils'),
-    errors = require('../../errors'),
-    logging = require('../../logging'),
-    events = require('../../events'),
-    i18n = require('../../i18n'),
-    settingsCache = require('../../settings/cache'),
-    pingList;
+    config = require('../config'),
+    utils = require('../utils'),
+    errors = require('../errors'),
+    logging = require('../logging'),
+    events = require('../events'),
+    i18n = require('../i18n'),
+    settingsCache = require('../settings/cache'),
 
-// ToDo: Make this configurable
-pingList = [{
-    host: 'blogsearch.google.com',
-    path: '/ping/RPC2'
-}, {
-    host: 'rpc.pingomatic.com',
-    path: '/'
-}];
+    defaultPostSlugs = [
+        'welcome',
+        'the-editor',
+        'using-tags',
+        'managing-users',
+        'private-sites',
+        'advanced-markdown',
+        'themes'
+    ],
+    // ToDo: Make this configurable
+    pingList = [{
+        host: 'blogsearch.google.com',
+        path: '/ping/RPC2'
+    }, {
+        host: 'rpc.pingomatic.com',
+        path: '/'
+    }];
 
 function ping(post) {
     var pingXML,
         title = post.title,
-        url = utils.url.urlFor('post', {post: post}, true),
-        defaultPostSlugs = [
-            'welcome',
-            'the-editor',
-            'using-tags',
-            'managing-users',
-            'private-sites',
-            'advanced-markdown',
-            'themes'
-        ];
+        url = utils.url.urlFor('post', {post: post}, true);
 
     if (post.page || config.isPrivacyDisabled('useRpcPing') || settingsCache.get('is_private')) {
         return;
@@ -82,8 +81,8 @@ function ping(post) {
             logging.error(new errors.GhostError({
                 err: err,
                 message: err.message,
-                context: i18n.t('errors.data.xml.xmlrpc.pingUpdateFailed.error'),
-                help: i18n.t('errors.data.xml.xmlrpc.pingUpdateFailed.help', {url: 'http://docs.ghost.org'})
+                context: i18n.t('errors.services.ping.requestFailed.error', {service: 'slack'}),
+                help: i18n.t('errors.services.ping.requestFailed.help', {url: 'http://docs.ghost.org'})
             }));
         });
 

core/server/translations/en.json

@@ -453,13 +453,13 @@
                         "failedToParseImportJson": "Failed to parse the import JSON file."
                     }
                 }
-            },
-            "xml": {
-                "xmlrpc": {
-                    "pingUpdateFailed": {
-                        "error": "Pinging services for updates on your blog failed, your blog will continue to function.",
-                        "help": "If you get this error repeatedly, please seek help on {url}."
-                    }
+            }
+        },
+        "services": {
+            "ping": {
+                "requestFailed": {
+                    "error": "The {service} service was unable to send a ping request, your blog will continue to function.",
+                    "help": "If you get this error repeatedly, please seek help on {url}."
                 }
             }
         },

core/test/unit/services/slack_spec.js

@@ -3,45 +3,22 @@ var should = require('should'), // jshint ignore:line,
     _ = require('lodash'),
     nock = require('nock'),
     rewire = require('rewire'),
-    Promise = require('bluebird'),
-    testUtils = require('../utils'),
     url = require('url'),
+    testUtils = require('../../utils'),
+    configUtils = require('../../utils/configUtils'),
 
     // Stuff we test
-    configUtils = require('../utils/configUtils'),
-    slack = rewire('../../server/data/slack'),
-    events = require('../../server/events'),
-    api = require('../../server/api/settings'),
-    utils = require('../../server/utils'),
-    schema = require('../../server/data/schema').checks,
+    slack = rewire('../../../server/services/slack'),
+    events = require('../../../server/events'),
+    utils = require('../../../server/utils'),
+    schema = require('../../../server/data/schema').checks,
+    settingsCache = require('../../../server/settings/cache'),
 
     sandbox = sinon.sandbox.create(),
 
     // Test data
-    slackObjNoUrl =
-    {
-        id: 17,
-        uuid: '50f50671-6e7c-4636-85e0-2962967764fa',
-        key: 'slack',
-        value: '[{"url":""}]',
-        type: 'blog',
-        created_at: '2016-04-06T15:19:19.492Z',
-        created_by: 1,
-        updated_at: '2016-04-06T15:19:19.492Z',
-        updated_by: 1
-    },
-    slackObjWithUrl =
-    {
-        id: 17,
-        uuid: '50f50671-6e7c-4636-85e0-2962967764fa',
-        key: 'slack',
-        value: '[{"url":"https://hooks.slack.com/services/a-b-c-d"}]',
-        type: 'blog',
-        created_at: '2016-04-06T15:19:19.492Z',
-        created_by: 1,
-        updated_at: '2016-04-06T15:19:19.492Z',
-        updated_by: 1
-    };
+    slackObjNoUrl = [{url: ''}],
+    slackObjWithUrl = [{url: 'https://hooks.slack.com/services/a-b-c-d'}];
 
 describe('Slack', function () {
     var eventStub;
@@ -178,28 +155,22 @@ describe('Slack', function () {
     });
 
     describe('ping()', function () {
-        var makeRequestAssertions,
-            isPostStub,
+        var isPostStub,
             urlForSpy,
-            settingsAPIStub,
-            settingsObj,
+            settingsCacheStub,
+
             slackReset,
-            makeRequestMock,
-            makeRequestSpy,
+            makeRequestStub,
             ping = slack.__get__('ping');
 
         beforeEach(function () {
             isPostStub = sandbox.stub(schema, 'isPost');
             urlForSpy = sandbox.spy(utils.url, 'urlFor');
-            settingsObj = {settings: [], meta: {}};
-            settingsAPIStub = sandbox.stub(api, 'read').returns(Promise.resolve(settingsObj));
 
-            makeRequestMock = function () {
-                makeRequestAssertions.apply(this, arguments);
-            };
+            settingsCacheStub = sandbox.stub(settingsCache, 'get');
 
-            makeRequestSpy = sandbox.spy(makeRequestMock);
-            slackReset = slack.__set__('makeRequest', makeRequestMock);
+            makeRequestStub = sandbox.stub();
+            slackReset = slack.__set__('makeRequest', makeRequestStub);
 
             configUtils.set('url', 'http://myblog.com');
         });
@@ -208,110 +179,112 @@ describe('Slack', function () {
             slackReset();
         });
 
-        it('makes a request for a post if url is provided', function (done) {
-            isPostStub.returns(true);
-            settingsObj.settings[0] = slackObjWithUrl;
+        it('makes a request for a post if url is provided', function () {
+            var requestOptions, requestData;
 
-            // assertions
-            makeRequestAssertions = function (requestOptions, requestData) {
-                isPostStub.calledOnce.should.be.true();
-                urlForSpy.calledTwice.should.be.true();
-                settingsAPIStub.calledOnce.should.be.true();
-                requestOptions.should.have.property('href').and.be.equal('https://hooks.slack.com/services/a-b-c-d');
-                requestData.should.have.property('text').and.be.equal('http://myblog.com/');
-                requestData.should.have.property('icon_url').and.be.equal('http://myblog.com/favicon.ico');
-                requestData.should.have.property('username').and.be.equal('Ghost');
-                done();
-            };
+            isPostStub.returns(true);
+            settingsCacheStub.withArgs('slack').returns(slackObjWithUrl);
 
             // execute code
-            ping({}).catch(done);
+            ping({});
+
+            // assertions
+            makeRequestStub.calledOnce.should.be.true();
+            isPostStub.calledOnce.should.be.true();
+            urlForSpy.calledTwice.should.be.true();
+            settingsCacheStub.calledWith('slack').should.be.true();
+
+            requestOptions = makeRequestStub.firstCall.args[0];
+            requestData = makeRequestStub.firstCall.args[1];
+
+            requestOptions.should.have.property('href').and.be.equal('https://hooks.slack.com/services/a-b-c-d');
+            requestData.should.have.property('text').and.be.equal('http://myblog.com/');
+            requestData.should.have.property('icon_url').and.be.equal('http://myblog.com/favicon.ico');
+            requestData.should.have.property('username').and.be.equal('Ghost');
         });
 
-        it('makes a request for a message if url is provided', function (done) {
+        it('makes a request for a message if url is provided', function () {
+            var requestOptions, requestData;
+
             isPostStub.returns(false);
-            settingsObj.settings[0] = slackObjWithUrl;
+            settingsCacheStub.withArgs('slack').returns(slackObjWithUrl);
 
             configUtils.set('url', 'https://myblog.com');
 
+            // execute code
+            ping({message: 'Hi!'});
+
             // assertions
-            makeRequestAssertions = function (requestOptions, requestData) {
-                isPostStub.calledOnce.should.be.true();
-                urlForSpy.calledOnce.should.be.true();
-                settingsAPIStub.calledOnce.should.be.true();
-                requestOptions.should.have.property('href').and.be.equal('https://hooks.slack.com/services/a-b-c-d');
-                requestData.should.have.property('text').and.be.equal('Hi!');
-                requestData.should.have.property('icon_url').and.be.equal('https://myblog.com/favicon.ico');
-                requestData.should.have.property('username').and.be.equal('Ghost');
-                done();
-            };
+            makeRequestStub.calledOnce.should.be.true();
+            isPostStub.calledOnce.should.be.true();
+            urlForSpy.calledOnce.should.be.true();
+            settingsCacheStub.calledWith('slack').should.be.true();
 
-            ping({message: 'Hi!'}).catch(done);
+            requestOptions = makeRequestStub.firstCall.args[0];
+            requestData = makeRequestStub.firstCall.args[1];
+
+            requestOptions.should.have.property('href').and.be.equal('https://hooks.slack.com/services/a-b-c-d');
+            requestData.should.have.property('text').and.be.equal('Hi!');
+            requestData.should.have.property('icon_url').and.be.equal('https://myblog.com/favicon.ico');
+            requestData.should.have.property('username').and.be.equal('Ghost');
         });
 
-        it('does not make a request if post is a page', function (done) {
+        it('does not make a request if post is a page', function () {
             // set up
             isPostStub.returns(true);
-            settingsObj.settings[0] = slackObjWithUrl;
+            settingsCacheStub.withArgs('slack').returns(slackObjWithUrl);
 
             // execute code
-            ping({page: true}).then(function (result) {
-                // assertions
-                isPostStub.calledOnce.should.be.true();
-                urlForSpy.calledOnce.should.be.true();
-                settingsAPIStub.calledOnce.should.be.true();
-                makeRequestSpy.called.should.be.false();
-                should.not.exist(result);
-                done();
-            }).catch(done);
+            ping({page: true});
+
+            // assertions
+            makeRequestStub.calledOnce.should.be.false();
+            isPostStub.calledOnce.should.be.true();
+            urlForSpy.calledOnce.should.be.true();
+            settingsCacheStub.calledWith('slack').should.be.true();
         });
 
-        it('does not make a request if no url is provided', function (done) {
+        it('does not make a request if no url is provided', function () {
             // set up
             isPostStub.returns(true);
-            settingsObj.settings[0] = slackObjNoUrl;
+            settingsCacheStub.withArgs('slack').returns(slackObjNoUrl);
 
             // execute code
-            ping({}).then(function (result) {
-                // assertions
-                isPostStub.calledOnce.should.be.true();
-                urlForSpy.calledOnce.should.be.true();
-                settingsAPIStub.calledOnce.should.be.true();
-                makeRequestSpy.called.should.be.false();
-                should.not.exist(result);
-                done();
-            }).catch(done);
+            ping({});
+            // assertions
+            makeRequestStub.calledOnce.should.be.false();
+            isPostStub.calledOnce.should.be.true();
+            urlForSpy.calledOnce.should.be.true();
+            settingsCacheStub.calledWith('slack').should.be.true();
         });
 
-        it('does not send webhook for \'welcome\' post', function (done) {
+        it('does not send webhook for \'welcome\' post', function () {
             // set up
             isPostStub.returns(true);
-            settingsObj.settings[0] = slackObjWithUrl;
+            settingsCacheStub.withArgs('slack').returns(slackObjWithUrl);
 
             // execute code
-            ping({slug: 'welcome'}).then(function (result) {
-                // assertions
-                isPostStub.calledOnce.should.be.true();
-                urlForSpy.calledOnce.should.be.true();
-                settingsAPIStub.calledOnce.should.be.true();
-                makeRequestSpy.called.should.be.false();
-                should.not.exist(result);
-                done();
-            }).catch(done);
+            ping({slug: 'welcome'});
+
+            // assertions
+            makeRequestStub.calledOnce.should.be.false();
+            isPostStub.calledOnce.should.be.true();
+            urlForSpy.calledOnce.should.be.true();
+            settingsCacheStub.calledWith('slack').should.be.true();
         });
 
-        it('handles broken slack settings', function (done) {
-            settingsObj.settings[0] = '';
+        it('handles broken slack settings', function () {
+            // set up
+            settingsCacheStub.withArgs('slack').returns();
 
-            ping({}).then(function () {
-                done('This should not get called');
-            }).catch(function () {
-                isPostStub.calledOnce.should.be.true();
-                urlForSpy.calledOnce.should.be.false();
-                settingsAPIStub.calledOnce.should.be.true();
-                makeRequestSpy.called.should.be.false();
-                done();
-            });
+            // execute code
+            ping({});
+
+            // assertions
+            makeRequestStub.calledOnce.should.be.false();
+            isPostStub.calledOnce.should.be.true();
+            urlForSpy.calledOnce.should.be.false();
+            settingsCacheStub.calledWith('slack').should.be.true();
         });
     });
 });

core/test/unit/services/xmlrpc_spec.js

@@ -4,11 +4,11 @@ var should = require('should'), // jshint ignore:line
     nock = require('nock'),
     http = require('http'),
     rewire = require('rewire'),
-    testUtils = require('../utils'),
-    configUtils = require('../utils/configUtils'),
-    xmlrpc = rewire('../../server/data/xml/xmlrpc'),
-    events = require('../../server/events'),
-    logging = require('../../server/logging'),
+    testUtils = require('../../utils'),
+    configUtils = require('../../utils/configUtils'),
+    xmlrpc = rewire('../../../server/services/xmlrpc'),
+    events = require('../../../server/events'),
+    logging = require('../../../server/logging'),
 
     sandbox = sinon.sandbox.create();