From af8c0dc7a5c82213b83c3704b2d62dbc919fb680 Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Mon, 12 Jun 2023 14:41:41 +0200 Subject: [PATCH] Prevented registering multiple Slack event listeners refs https://github.com/TryGhost/Toolbox/issues/592 - in tests, we boot Ghost over and over - this inits all the services each time - it turns out that the Slack event listener is registered 80+ times - to prevent this, we can check if it has already been registered, like we do with webhooks: https://github.com/TryGhost/Ghost/blob/4639396c3aa9325ab74ca733d037d299b0244271/ghost/core/core/server/services/webhooks/listen.js#L67-L69 - not sure about this pattern in general, but it's something we can review in coming weeks --- ghost/core/core/server/services/slack.js | 13 +++++++++---- ghost/core/test/unit/server/services/slack.test.js | 10 +++++----- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/ghost/core/core/server/services/slack.js b/ghost/core/core/server/services/slack.js index 7653d0c477..b2027ea85f 100644 --- a/ghost/core/core/server/services/slack.js +++ b/ghost/core/core/server/services/slack.js @@ -153,7 +153,7 @@ function ping(post) { } } -function listener(model, options) { +function slackListener(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) { @@ -163,15 +163,20 @@ function listener(model, options) { ping(model.toJSON()); } -function testPing() { +function slackTestPing() { ping({ message: 'Heya! This is a test notification from your Ghost blog :smile:. Seems to work fine!' }); } function listen() { - events.on('post.published', listener); - events.on('slack.test', testPing); + if (!events.hasRegisteredListener('post.published', 'slackListener')) { + events.on('post.published', slackListener); + } + + if (!events.hasRegisteredListener('slack.test', 'slackTestPing')) { + events.on('slack.test', slackTestPing); + } } // Public API diff --git a/ghost/core/test/unit/server/services/slack.test.js b/ghost/core/test/unit/server/services/slack.test.js index 0a8a537dd8..be3a019254 100644 --- a/ghost/core/test/unit/server/services/slack.test.js +++ b/ghost/core/test/unit/server/services/slack.test.js @@ -32,8 +32,8 @@ describe('Slack', function () { 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(); + eventStub.firstCall.calledWith('post.published', slack.__get__('slackListener')).should.be.true(); + eventStub.secondCall.calledWith('slack.test', slack.__get__('slackTestPing')).should.be.true(); }); it('listener() calls ping() with toJSONified model', function () { @@ -47,7 +47,7 @@ describe('Slack', function () { const pingStub = sinon.stub(); const resetSlack = slack.__set__('ping', pingStub); - const listener = slack.__get__('listener'); + const listener = slack.__get__('slackListener'); listener(testModel); @@ -69,7 +69,7 @@ describe('Slack', function () { const pingStub = sinon.stub(); const resetSlack = slack.__set__('ping', pingStub); - const listener = slack.__get__('listener'); + const listener = slack.__get__('slackListener'); listener(testModel, {importing: true}); @@ -82,7 +82,7 @@ describe('Slack', function () { it('testPing() calls ping() with default message', function () { const pingStub = sinon.stub(); const resetSlack = slack.__set__('ping', pingStub); - const testPing = slack.__get__('testPing'); + const testPing = slack.__get__('slackTestPing'); testPing();