diff --git a/core/server/services/webhooks/listen.js b/core/server/services/webhooks/listen.js index 5fa5e19e05..743876cf41 100644 --- a/core/server/services/webhooks/listen.js +++ b/core/server/services/webhooks/listen.js @@ -1,7 +1,9 @@ const _ = require('lodash'); const limitService = require('../../services/limits'); const logging = require('@tryghost/logging'); -const trigger = require('./trigger'); +const WebhookTrigger = require('./trigger'); +const models = require('../../models'); +const payload = require('./payload'); // The webhook system is fundamentally built on top of our model event system const events = require('../../lib/common/events'); @@ -55,6 +57,7 @@ const listen = async () => { } } + const webhookTrigger = new WebhookTrigger({models, payload}); _.each(WEBHOOKS, (event) => { events.on(event, (model, options) => { // CASE: avoid triggering webhooks when importing @@ -62,7 +65,7 @@ const listen = async () => { return; } - trigger(event, model); + webhookTrigger.trigger(event, model); }); }); }; diff --git a/core/server/services/webhooks/trigger.js b/core/server/services/webhooks/trigger.js index f107891436..4dfbce1db4 100644 --- a/core/server/services/webhooks/trigger.js +++ b/core/server/services/webhooks/trigger.js @@ -1,19 +1,22 @@ -const _ = require('lodash'); const debug = require('@tryghost/debug')('services:webhooks:trigger'); const logging = require('@tryghost/logging'); const request = require('@tryghost/request'); -const models = require('../../models'); -const payload = require('./payload'); +const ghostVersion = require('@tryghost/version'); + +class WebhookTrigger { + constructor({models, payload}){ + this.models = models; + this.payload = payload; + } -const webhooks = { getAll(event) { - return models + return this.models .Webhook .findAllByEvent(event, {context: {internal: true}}); - }, + } update(webhook, data) { - models + this.models .Webhook .edit({ last_triggered_at: Date.now(), @@ -23,36 +26,34 @@ const webhooks = { .catch(() => { logging.warn(`Unable to update "last_triggered" for webhook: ${webhook.id}`); }); - }, + } destroy(webhook) { - return models + return this.models .Webhook .destroy({id: webhook.id}, {context: {internal: true}}) .catch(() => { logging.warn(`Unable to destroy webhook ${webhook.id}.`); }); } -}; -const response = { onSuccess(webhook) { return (res) => { - webhooks.update(webhook, { + this.update(webhook, { statusCode: res.statusCode }); }; - }, + } onError(webhook) { return (err) => { if (err.statusCode === 410) { logging.info(`Webhook destroyed (410 response) for "${webhook.get('event')}" with url "${webhook.get('target_url')}".`); - return webhooks.destroy(webhook); + return this.destroy(webhook); } - webhooks.update(webhook, { + this.update(webhook, { statusCode: err.statusCode, error: `Request failed: ${err.code || 'unknown'}` }); @@ -60,34 +61,39 @@ const response = { logging.warn(`Request to ${webhook.get('target_url') || null} failed because of: ${err.code || ''}.`); }; } -}; -module.exports = (event, model) => { - webhooks.getAll(event) - .then((hooks) => { - debug(`${hooks.models.length} webhooks found for ${event}.`); + async trigger(event, model) { + const response = { + onSuccess: this.onSuccess.bind(this), + onError: this.onError.bind(this) + }; - _.each(hooks.models, (webhook) => { - payload(webhook.get('event'), model) - .then((hookPayload) => { - const reqPayload = JSON.stringify(hookPayload); - const url = webhook.get('target_url'); - const opts = { - body: reqPayload, - headers: { - 'Content-Length': Buffer.byteLength(reqPayload), - 'Content-Type': 'application/json' - }, - timeout: 2 * 1000, - retry: 5 - }; + const hooks = await this.getAll(event); - logging.info(`Triggering webhook for "${webhook.get('event')}" with url "${url}"`); + debug(`${hooks.models.length} webhooks found for ${event}.`); - request(url, opts) - .then(response.onSuccess(webhook)) - .catch(response.onError(webhook)); - }); - }); + hooks.models.forEach(async (webhook) => { + const hookPayload = await this.payload(webhook.get('event'), model); + + const reqPayload = JSON.stringify(hookPayload); + const url = webhook.get('target_url'); + const opts = { + body: reqPayload, + headers: { + 'Content-Length': Buffer.byteLength(reqPayload), + 'Content-Type': 'application/json' + }, + timeout: 2 * 1000, + retry: 5 + }; + + logging.info(`Triggering webhook for "${webhook.get('event')}" with url "${url}"`); + + request(url, opts) + .then(response.onSuccess(webhook)) + .catch(response.onError(webhook)); }); -}; + } +} + +module.exports = WebhookTrigger; diff --git a/test/e2e-api/shared/version.test.js b/test/e2e-api/shared/version.test.js index 23b15dc955..c16ae3372d 100644 --- a/test/e2e-api/shared/version.test.js +++ b/test/e2e-api/shared/version.test.js @@ -1,7 +1,7 @@ const {agentProvider, fixtureManager, matchers, mockManager} = require('../../utils/e2e-framework'); const {anyErrorId, stringMatching, anyObjectId, anyLocationFor, anyISODateTime, anyEtag} = matchers; -describe.only('API Versioning', function () { +describe('API Versioning', function () { describe('Admin API', function () { let agentAdminAPI; diff --git a/test/unit/server/services/webhooks/trigger.test.js b/test/unit/server/services/webhooks/trigger.test.js new file mode 100644 index 0000000000..fbb022d67a --- /dev/null +++ b/test/unit/server/services/webhooks/trigger.test.js @@ -0,0 +1,40 @@ +const should = require('should'); +const sinon = require('sinon'); + +const WebhookTrigger = require('../../../../../core/server/services/webhooks/trigger'); + +describe('Webhook Service', function () { + const models = { + Webhook: { + edit: () => sinon.stub().resolves(null), + destroy: () => sinon.stub().resolves(null), + findAllByEvent: () => sinon.stub().resolves(null), + getByEventAndTarget: () => sinon.stub().resolves(null), + add: () => sinon.stub().resolves(null) + } + }; + + const payload = sinon.stub().resolves(null); + + afterEach(function () { + sinon.restore(); + }); + + describe('trigger', function () { + it('Does not trigger payload handler when event and model that has no hooks registered', async function () { + sinon.stub(models.Webhook, 'findAllByEvent') + .withArgs('post.added', {context: {internal: true}}) + .resolves({models: []}); + + const webhookTrigger = new WebhookTrigger({ + models, + payload + }); + + await webhookTrigger.trigger('post.added'); + + should.equal(models.Webhook.findAllByEvent.called, true); + should.equal(payload.called, false); + }); + }); +});