Merge pull request #6929 from ErisDS/event-test-fix

Add removal for event listeners, fixes test error

core/server/data/slack/index.js

@@ -9,7 +9,6 @@ var https           = require('https'),
     schema          = require('../schema').checks,
     options,
     req,
-    slack = {},
     slackData = {};
 
 function getSlackSettings() {
@@ -17,7 +16,7 @@ function getSlackSettings() {
         var slackSetting = response.settings[0].value;
 
         try {
-            slackSetting = JSON.parse(slackSetting) || slackSetting;
+            slackSetting = JSON.parse(slackSetting);
         } catch (e) {
             return Promise.reject(e);
         }
@@ -83,24 +82,29 @@ function ping(post) {
             options.headers = {'Content-type': 'application/json'};
 
             // with all the data we have, we're doing the request now
-            slack._makeRequest(options, slackData);
+            makeRequest(options, slackData);
         } else {
             return;
         }
     });
 }
 
-function init() {
-    events.on('post.published', function (model) {
-        slack._ping(model.toJSON());
-    });
-    events.on('slack.test', function () {
-        slack._ping({
-            message: 'Heya! This is a test notification from your Ghost blog :simple_smile:. Seems to work fine!'
-        });
+function listener(model) {
+    ping(model.toJSON());
+}
+
+function testPing() {
+    ping({
+        message: 'Heya! This is a test notification from your Ghost blog :simple_smile:. Seems to work fine!'
     });
 }
-slack.init = init;
-slack._ping = ping;
-slack._makeRequest = makeRequest;
-module.exports = slack;
+
+function listen() {
+    events.on('post.published', listener);
+    events.on('slack.test', testPing);
+}
+
+// Public API
+module.exports = {
+    listen: listen
+};

core/server/data/xml/xmlrpc.js

@@ -66,23 +66,26 @@ function ping(post) {
 
         req = http.request(options);
         req.write(pingXML);
-        req.on('error', function (error) {
-            errors.logError(
-                error,
-                i18n.t('errors.data.xml.xmlrpc.pingUpdateFailed.error'),
-                i18n.t('errors.data.xml.xmlrpc.pingUpdateFailed.help', {url: 'http://support.ghost.org'})
-            );
-        });
+        req.on('error', function handleError(error) {
+                errors.logError(
+                    error,
+                    i18n.t('errors.data.xml.xmlrpc.pingUpdateFailed.error'),
+                    i18n.t('errors.data.xml.xmlrpc.pingUpdateFailed.help', {url: 'http://support.ghost.org'})
+                );
+            }
+        );
         req.end();
     });
 }
 
-function init() {
-    events.on('post.published', function (model) {
-        ping(model.toJSON());
-    });
+function listener(model) {
+    ping(model.toJSON());
+}
+
+function listen() {
+    events.on('post.published', listener);
 }
 
 module.exports = {
-    init: init
+    listen: listen
 };

core/server/index.js

@@ -83,9 +83,9 @@ function init(options) {
             // Initialize sitemaps
             sitemap.init(),
             // Initialize xmrpc ping
-            xmlrpc.init(),
+            xmlrpc.listen(),
             // Initialize slack ping
-            slack.init()
+            slack.listen()
         );
     }).then(function () {
         // Get reference to an express app instance.

core/test/unit/slack_spec.js

@@ -1,17 +1,20 @@
 /*globals describe, beforeEach, afterEach, it*/
-var nock            = require('nock'),
+var _               = require('lodash'),
+    nock            = require('nock'),
     should          = require('should'),
     sinon           = require('sinon'),
+    rewire          = require('rewire'),
     Promise         = require('bluebird'),
     testUtils       = require('../utils'),
     url             = require('url'),
 
 // Stuff we test
-    slack          = require('../../server/data/slack'),
+    slack          = rewire('../../server/data/slack'),
     events         = require('../../server/events'),
     api            = require('../../server/api/settings'),
     config         = require('../../server/config'),
     schema         = require('../../server/data/schema').checks,
+
     sandbox        = sinon.sandbox.create(),
 // Test data
     slackObjNoUrl =
@@ -43,121 +46,198 @@ var nock            = require('nock'),
 should.equal(true, true);
 
 describe('Slack', function () {
-    var testPost;
+    var eventStub;
+
+    beforeEach(function () {
+        eventStub = sandbox.stub(events, 'on');
+    });
 
     afterEach(function () {
         sandbox.restore();
     });
 
-    it('should call ping if post is published', function () {
-        // set up
-        var ping = sandbox.stub(slack, '_ping');
-        testPost = {
-                toJSON: function () {
-                    return testUtils.DataGenerator.Content.posts[2];
-                }
-            };
-        // execute code
-        slack.init();
-        events.emit('post.published', testPost);
-
-        // assertions
-        ping.calledOnce.should.be.true();
+    it('listen() should initialise event correctly', function () {
+        slack.listen();
+        eventStub.calledTwice.should.be.true();
+        eventStub.firstCall.calledWith('post.published', slack.__get__('listener')).should.be.true();
+        eventStub.secondCall.calledWith('slack.test', slack.__get__('testPing')).should.be.true();
     });
 
-    it('should make request to slack correctly', function () {
-        // set up
-        var reqOptions,
-            pingSlack;
+    it('listener() calls ping() with toJSONified model', function () {
+        var testPost = _.clone(testUtils.DataGenerator.Content.posts[2]),
+            testModel = {toJSON: function () {return testPost; }},
+            pingStub = sandbox.stub(),
+            resetSlack = slack.__set__('ping', pingStub),
+            listener = slack.__get__('listener');
 
-        // fill the options for https request
-        reqOptions = url.parse('https://hooks.slack.com/services/a-b-c-d');
-        reqOptions.method = 'POST';
-        reqOptions.headers = {'Content-type': 'application/json'};
+        listener(testModel);
 
-        pingSlack = nock('https://hooks.slack.com/')
-                        .post('/services/a-b-c-d', {text:'http://myblog.com/mypost', icon_url: 'http://myblog.com/someImageurl.jpg', username: 'Ghost'})
-                        .reply(200);
-        // execute code
-        slack._makeRequest(reqOptions, {text:'http://myblog.com/mypost', icon_url: 'http://myblog.com/someImageurl.jpg', username: 'Ghost'});
+        pingStub.calledOnce.should.be.true();
+        pingStub.calledWith(testPost).should.be.true();
 
-        // assertions
-        pingSlack.isDone().should.be.true();
+        // Reset slack ping method
+        resetSlack();
     });
 
-    it('can handle an error response correctly', function () {
-        // set up
-        var reqOptions,
-            pingSlack;
+    it('testPing() calls ping() with default message', function () {
+        var pingStub = sandbox.stub(),
+            resetSlack = slack.__set__('ping', pingStub),
+            testPing = slack.__get__('testPing');
 
-        // fill the options for https request
-        reqOptions = url.parse('https://hooks.slack.com/services/a-b-c-d');
-        reqOptions.method = 'POST';
-        reqOptions.headers = {'Content-type': 'application/json'};
+        testPing();
 
-        pingSlack = nock('https://hooks.slack.com/')
-                        .post('/services/a-b-c-d', {text:'http://myblog.com/mypost', icon_url: 'http://myblog.com/someImageurl.jpg', username: 'Ghost'})
-                        .replyWithError(404);
-        // execute code
-        slack._makeRequest(reqOptions, {text:'http://myblog.com/mypost', icon_url: 'http://myblog.com/someImageurl.jpg', username: 'Ghost'});
+        pingStub.calledOnce.should.be.true();
+        pingStub.calledWith(sinon.match.has('message')).should.be.true();
 
-        // assertions
-        pingSlack.isDone().should.be.true();
+        // Reset slack ping method
+        resetSlack();
     });
 
-    describe('Ping', function () {
+    describe('makeRequest()', function () {
+        var makeRequest = slack.__get__('makeRequest');
+
+        it('should make request to slack correctly', function () {
+            // set up
+            var reqOptions,
+                pingSlack;
+
+            // fill the options for https request
+            reqOptions = url.parse('https://hooks.slack.com/services/a-b-c-d');
+            reqOptions.method = 'POST';
+            reqOptions.headers = {'Content-type': 'application/json'};
+
+            pingSlack = nock('https://hooks.slack.com/')
+                .post('/services/a-b-c-d', {text:'http://myblog.com/mypost', icon_url: 'http://myblog.com/someImageurl.jpg', username: 'Ghost'})
+                .reply(200);
+
+            // execute code
+            makeRequest(reqOptions, {text:'http://myblog.com/mypost', icon_url: 'http://myblog.com/someImageurl.jpg', username: 'Ghost'});
+
+            // assertions
+            pingSlack.isDone().should.be.true();
+        });
+
+        it('can handle an error response correctly', function () {
+            // set up
+            var reqOptions,
+                pingSlack;
+
+            // fill the options for https request
+            reqOptions = url.parse('https://hooks.slack.com/services/a-b-c-d');
+            reqOptions.method = 'POST';
+            reqOptions.headers = {'Content-type': 'application/json'};
+
+            pingSlack = nock('https://hooks.slack.com/')
+                .post('/services/a-b-c-d', {text:'http://myblog.com/mypost', icon_url: 'http://myblog.com/someImageurl.jpg', username: 'Ghost'})
+                .replyWithError(404);
+
+            // execute code
+            makeRequest(reqOptions, {text:'http://myblog.com/mypost', icon_url: 'http://myblog.com/someImageurl.jpg', username: 'Ghost'});
+
+            // assertions
+            pingSlack.isDone().should.be.true();
+        });
+    });
+
+    describe('ping()', function () {
         var makeRequestAssertions,
-            schemaStub,
+            isPostStub,
             urlForStub,
             settingsAPIStub,
             settingsObj,
-            makeRequestStub;
+            slackReset,
+            makeRequestMock,
+            makeRequestSpy,
+            ping =  slack.__get__('ping');
 
         beforeEach(function () {
-            schemaStub = sandbox.stub(schema, 'isPost');
+            isPostStub = sandbox.stub(schema, 'isPost');
             urlForStub = sandbox.stub(config, 'urlFor');
-            urlForStub.onFirstCall().returns('http://myblog.com/post');
-            urlForStub.onSecondCall().returns('http://myblog.com/someImageurl.jpg');
+            urlForStub.withArgs('post').returns('http://myblog.com/post');
+            urlForStub.returns('http://myblog.com/someImageurl.jpg');
             settingsObj = {settings: [], meta: {}};
             settingsAPIStub = sandbox.stub(api, 'read').returns(Promise.resolve(settingsObj));
-            makeRequestStub = sandbox.stub(slack, '_makeRequest', function () {
+
+            makeRequestMock = function () {
                 makeRequestAssertions.apply(this, arguments);
-            });
+            };
+            makeRequestSpy = sandbox.spy(makeRequestMock);
+            slackReset = slack.__set__('makeRequest', makeRequestMock);
         });
 
-        it('makes a request if url is provided', function (done) {
+        afterEach(function () {
+            slackReset();
+        });
+
+        it('makes a request for a post if url is provided', function (done) {
             // set up
-            schemaStub.returns('true');
+            isPostStub.returns(true);
             settingsObj.settings[0] = slackObjWithUrl;
 
             // assertions
             makeRequestAssertions = function (requestOptions, requestData) {
-                    schemaStub.calledOnce.should.be.true();
-                    urlForStub.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/post');
-                    requestData.should.have.property('icon_url').and.be.equal('http://myblog.com/someImageurl.jpg');
-                    requestData.should.have.property('username').and.be.equal('Ghost');
-                    done();
-                };
+                isPostStub.calledOnce.should.be.true();
+                urlForStub.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/post');
+                requestData.should.have.property('icon_url').and.be.equal('http://myblog.com/someImageurl.jpg');
+                requestData.should.have.property('username').and.be.equal('Ghost');
+                done();
+            };
 
             // execute code
-            slack._ping({}).catch(done);
+            ping({}).catch(done);
+        });
+
+        it('makes a request for a message if url is provided', function (done) {
+            isPostStub.returns(false);
+            settingsObj.settings[0] = slackObjWithUrl;
+
+            // assertions
+            makeRequestAssertions = function (requestOptions, requestData) {
+                isPostStub.calledOnce.should.be.true();
+                urlForStub.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('http://myblog.com/someImageurl.jpg');
+                requestData.should.have.property('username').and.be.equal('Ghost');
+                done();
+            };
+
+            ping({message: 'Hi!'}).catch(done);
+        });
+
+        it('does not make a request if post is a page', function (done) {
+            // set up
+            isPostStub.returns(true);
+            settingsObj.settings[0] = slackObjWithUrl;
+
+            // execute code
+            ping({page: true}).then(function (result) {
+                // assertions
+                isPostStub.calledOnce.should.be.true();
+                urlForStub.calledOnce.should.be.true();
+                settingsAPIStub.calledOnce.should.be.true();
+                makeRequestSpy.called.should.be.false();
+                should.not.exist(result);
+                done();
+            }).catch(done);
         });
 
         it('does not make a request if no url is provided', function (done) {
             // set up
-            schemaStub.returns('true');
+            isPostStub.returns(true);
             settingsObj.settings[0] = slackObjNoUrl;
 
             // execute code
-            slack._ping({}).then(function (result) {
+            ping({}).then(function (result) {
                 // assertions
-                schemaStub.calledOnce.should.be.true();
+                isPostStub.calledOnce.should.be.true();
                 urlForStub.calledOnce.should.be.true();
                 settingsAPIStub.calledOnce.should.be.true();
-                makeRequestStub.called.should.be.false();
+                makeRequestSpy.called.should.be.false();
                 should.not.exist(result);
                 done();
             }).catch(done);
@@ -165,20 +245,33 @@ describe('Slack', function () {
 
         it('does not send webhook for \'welcome-to-ghost\' post', function (done) {
             // set up
-            schemaStub.returns('true');
+            isPostStub.returns(true);
             settingsObj.settings[0] = slackObjWithUrl;
 
-            makeRequestAssertions = function () {};
             // execute code
-            slack._ping({slug: 'welcome-to-ghost'}).then(function (result) {
+            ping({slug: 'welcome-to-ghost'}).then(function (result) {
                 // assertions
-                schemaStub.calledOnce.should.be.true();
+                isPostStub.calledOnce.should.be.true();
                 urlForStub.calledOnce.should.be.true();
                 settingsAPIStub.calledOnce.should.be.true();
-                makeRequestStub.called.should.be.false();
+                makeRequestSpy.called.should.be.false();
                 should.not.exist(result);
                 done();
             }).catch(done);
         });
+
+        it('handles broken slack settings', function (done) {
+            settingsObj.settings[0] = '';
+
+            ping({}).then(function () {
+                done('This should not get called');
+            }).catch(function () {
+                isPostStub.calledOnce.should.be.true();
+                urlForStub.calledOnce.should.be.false();
+                settingsAPIStub.calledOnce.should.be.true();
+                makeRequestSpy.called.should.be.false();
+                done();
+            });
+        });
     });
 });

core/test/unit/xmlrpc_spec.js

@@ -1,9 +1,12 @@
 /*globals describe, beforeEach, afterEach, it*/
-var nock            = require('nock'),
+var _               = require('lodash'),
+    nock            = require('nock'),
     should          = require('should'),
     sinon           = require('sinon'),
+    rewire          = require('rewire'),
     testUtils       = require('../utils'),
-    xmlrpc          = require('../../server/data/xml/xmlrpc'),
+    configUtils     = require('../utils/configUtils'),
+    xmlrpc          = rewire('../../server/data/xml/xmlrpc'),
     events          = require('../../server/events'),
     // storing current environment
     currentEnv      = process.env.NODE_ENV;
@@ -12,32 +15,119 @@ var nock            = require('nock'),
 should.equal(true, true);
 
 describe('XMLRPC', function () {
-    var sandbox;
+    var sandbox, eventStub;
 
     beforeEach(function () {
         sandbox = sinon.sandbox.create();
+        eventStub = sandbox.stub(events, 'on');
         // give environment a value that will ping
         process.env.NODE_ENV = 'production';
     });
 
     afterEach(function () {
         sandbox.restore();
+        configUtils.restore();
+        nock.cleanAll();
         // reset the environment
         process.env.NODE_ENV = currentEnv;
     });
 
-    it('should execute two pings', function () {
-        var ping1 = nock('http://blogsearch.google.com').post('/ping/RPC2').reply(200),
-            ping2 = nock('http://rpc.pingomatic.com').post('/').reply(200),
-            testPost = {
-                toJSON: function () {
-                    return testUtils.DataGenerator.Content.posts[2];
+    it('listen() should initialise event correctly', function () {
+        xmlrpc.listen();
+        eventStub.calledOnce.should.be.true();
+        eventStub.calledWith('post.published', xmlrpc.__get__('listener')).should.be.true();
+    });
+
+    it('listener() calls ping() with toJSONified model', function () {
+        var testPost = _.clone(testUtils.DataGenerator.Content.posts[2]),
+            testModel = {toJSON: function () {return testPost; }},
+            pingStub = sandbox.stub(),
+            resetXmlRpc = xmlrpc.__set__('ping', pingStub),
+            listener = xmlrpc.__get__('listener');
+
+        listener(testModel);
+
+        pingStub.calledOnce.should.be.true();
+        pingStub.calledWith(testPost).should.be.true();
+
+        // Reset xmlrpc ping method
+        resetXmlRpc();
+    });
+
+    describe('ping()', function () {
+        var ping = xmlrpc.__get__('ping');
+
+        it('with a post should execute two pings', function () {
+            var ping1 = nock('http://blogsearch.google.com').post('/ping/RPC2').reply(200),
+                ping2 = nock('http://rpc.pingomatic.com').post('/').reply(200),
+                testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
+
+            ping(testPost);
+
+            ping1.isDone().should.be.true();
+            ping2.isDone().should.be.true();
+        });
+
+        it('with default post should not execute pings', function () {
+            var ping1 = nock('http://blogsearch.google.com').post('/ping/RPC2').reply(200),
+                ping2 = nock('http://rpc.pingomatic.com').post('/').reply(200),
+                testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
+
+            testPost.slug = 'welcome-to-ghost';
+
+            ping(testPost);
+
+            ping1.isDone().should.be.false();
+            ping2.isDone().should.be.false();
+        });
+
+        it('with a page should not execute pings', function () {
+            var ping1 = nock('http://blogsearch.google.com').post('/ping/RPC2').reply(200),
+                ping2 = nock('http://rpc.pingomatic.com').post('/').reply(200),
+                testPage = _.clone(testUtils.DataGenerator.Content.posts[5]);
+
+            ping(testPage);
+
+            ping1.isDone().should.be.false();
+            ping2.isDone().should.be.false();
+        });
+
+        it('when privacy.useRpcPing is false should not execute pings', function () {
+            var ping1 = nock('http://blogsearch.google.com').post('/ping/RPC2').reply(200),
+                ping2 = nock('http://rpc.pingomatic.com').post('/').reply(200),
+                testPost = _.clone(testUtils.DataGenerator.Content.posts[2]);
+
+            configUtils.set({privacy: {useRpcPing: false}});
+
+            ping(testPost);
+
+            ping1.isDone().should.be.false();
+            ping2.isDone().should.be.false();
+        });
+
+        it('captures errors from requests', function (done) {
+            var ping1 = nock('http://blogsearch.google.com').post('/ping/RPC2').reply(200),
+                ping2 = nock('http://rpc.pingomatic.com').post('/').replyWithError('ping site is down'),
+                testPost = _.clone(testUtils.DataGenerator.Content.posts[2]),
+                errorMock, resetXmlRpc;
+
+            errorMock = {
+                logError: function logError(error) {
+                    should.exist(error);
+                    error.message.should.eql('ping site is down');
+
+                    // Reset xmlrpc handleError method and exit test
+                    resetXmlRpc();
+                    done();
                 }
             };
 
-        xmlrpc.init();
-        events.emit('post.published', testPost);
-        ping1.isDone().should.be.true();
-        ping2.isDone().should.be.true();
+            resetXmlRpc = xmlrpc.__set__('errors', errorMock);
+
+            ping(testPost);
+
+            ping1.isDone().should.be.true();
+            ping2.isDone().should.be.true();
+        });
     });
 });