From 73bddef7c57d3ab478e36bc363d0a00d921aeeb6 Mon Sep 17 00:00:00 2001 From: Fabien 'egg' O'Carroll Date: Mon, 30 Jan 2023 18:11:30 +0700 Subject: [PATCH] Used job queue for processing incoming Webmentions refs https://github.com/TryGhost/Team/issues/2419 We use a job queue to ensure that webmentions can be processed outside of the request/response cycle, but still finish executing if the processed is closed. With this we're able to update the e2e tests to await the processing of the mention rather than sleepign for arbitrary lengths of time, and we've reintroduced the tests removed previously - https://github.com/TryGhost/Ghost/commit/aa14207b695caf6883895cb5180d2bab1221f93c - https://github.com/TryGhost/Ghost/commit/48e9393159998ff4c7a00ccc7fc21baf77ac7cb1 --- .../services/mentions/MentionController.js | 34 +++-- .../core/server/services/mentions/service.js | 14 ++- .../e2e-api/webmentions/webmentions.test.js | 117 +++++++++++++----- 3 files changed, 124 insertions(+), 41 deletions(-) diff --git a/ghost/core/core/server/services/mentions/MentionController.js b/ghost/core/core/server/services/mentions/MentionController.js index f07fde57a6..2fec519021 100644 --- a/ghost/core/core/server/services/mentions/MentionController.js +++ b/ghost/core/core/server/services/mentions/MentionController.js @@ -10,12 +10,26 @@ const logging = require('@tryghost/logging'); * @typedef {import('@tryghost/webmentions/lib/MentionsAPI').Page} Page */ +/** + * @typedef {object} IJobService + * @prop {(name: string, fn: Function) => void} addJob + */ + module.exports = class MentionController { /** @type {import('@tryghost/webmentions/lib/MentionsAPI')} */ #api; + /** @type {IJobService} */ + #jobService; + + /** + * @param {object} deps + * @param {import('@tryghost/webmentions/lib/MentionsAPI')} deps.api + * @param {IJobService} deps.jobService + */ async init(deps) { this.#api = deps.api; + this.#jobService = deps.jobService; } /** @@ -52,15 +66,17 @@ module.exports = class MentionController { */ async receive(frame) { logging.info('[Webmention] ' + JSON.stringify(frame.data)); - const {source, target, ...payload} = frame.data; - const result = this.#api.processWebmention({ - source: new URL(source), - target: new URL(target), - payload - }); - - result.catch(function rejected(err) { - logging.error(err); + this.#jobService.addJob('processWebmention', async () => { + const {source, target, ...payload} = frame.data; + try { + await this.#api.processWebmention({ + source: new URL(source), + target: new URL(target), + payload + }); + } catch (err) { + logging.error(err); + } }); } }; diff --git a/ghost/core/core/server/services/mentions/service.js b/ghost/core/core/server/services/mentions/service.js index f9a6b28193..6f61793eef 100644 --- a/ghost/core/core/server/services/mentions/service.js +++ b/ghost/core/core/server/services/mentions/service.js @@ -16,6 +16,7 @@ const outputSerializerUrlUtil = require('../../../server/api/endpoints/utils/ser const labs = require('../../../shared/labs'); const urlService = require('../url'); const DomainEvents = require('@tryghost/domain-events'); +const jobsService = require('../jobs'); function getPostUrl(post) { const jsonModel = {}; @@ -50,7 +51,18 @@ module.exports = { routingService }); - this.controller.init({api}); + this.controller.init({ + api, + jobService: { + async addJob(name, fn) { + jobsService.addJob({ + name, + job: fn, + offloaded: false + }); + } + } + }); const sendingService = new MentionSendingService({ discoveryService, diff --git a/ghost/core/test/e2e-api/webmentions/webmentions.test.js b/ghost/core/test/e2e-api/webmentions/webmentions.test.js index 5b4eabe7bd..95ebdfc30b 100644 --- a/ghost/core/test/e2e-api/webmentions/webmentions.test.js +++ b/ghost/core/test/e2e-api/webmentions/webmentions.test.js @@ -3,6 +3,7 @@ const models = require('../../../core/server/models'); const assert = require('assert'); const urlUtils = require('../../../core/shared/url-utils'); const nock = require('nock'); +const jobsService = require('../../../core/server/services/jobs'); describe('Webmentions (receiving)', function () { let agent; @@ -18,7 +19,7 @@ describe('Webmentions (receiving)', function () { nock.cleanAll(); nock.enableNetConnect(); }); - + beforeEach(function () { emailMockReceiver = mockManager.mockMail(); }); @@ -28,24 +29,29 @@ describe('Webmentions (receiving)', function () { }); it('can receive a webmention', async function () { - const url = new URL('http://testpage.com/external-article/'); + const processWebmentionJob = jobsService.awaitCompletion('processWebmention'); + const targetUrl = new URL('integrations/', urlUtils.getSiteUrl()); + const sourceUrl = new URL('http://testpage.com/external-article/'); const html = ` Test Page `; - nock(url.href) - .get('/') - .reply(200, html, {'content-type': 'text/html'}); + nock(targetUrl.origin) + .head(targetUrl.pathname) + .reply(200); + + nock(sourceUrl.origin) + .get(sourceUrl.pathname) + .reply(200, html, {'Content-Type': 'text/html'}); await agent.post('/receive') .body({ - source: 'http://testpage.com/external-article/', - target: urlUtils.getSiteUrl() + 'integrations/', + source: sourceUrl.href, + target: targetUrl.href, withExtension: true // test payload recorded }) .expectStatus(202); - // todo: remove sleep in future - await sleep(2000); + await processWebmentionJob; const mention = await models.Mention.findOne({source: 'http://testpage.com/external-article/'}); assert(mention); @@ -60,54 +66,103 @@ describe('Webmentions (receiving)', function () { })); }); - it('can send an email notification for a new webmention', async function () { - const url = new URL('http://testpage.com/external-article-123-email-test/'); + it('can receive a webmention to homepage', async function () { + const processWebmentionJob = jobsService.awaitCompletion('processWebmention'); + const targetUrl = new URL(urlUtils.getSiteUrl()); + const sourceUrl = new URL('http://testpage.com/external-article-2/'); const html = ` Test Page `; - nock(url.href) - .get('/') - .reply(200, html, {'content-type': 'text/html'}); + + nock(targetUrl.origin) + .head(targetUrl.pathname) + .reply(200); + + nock(sourceUrl.origin) + .get(sourceUrl.pathname) + .reply(200, html, {'Content-Type': 'text/html'}); + + await agent.post('/receive') + .body({ + source: sourceUrl.href, + target: targetUrl.href + }) + .expectStatus(202); + + await processWebmentionJob; + + const mention = await models.Mention.findOne({source: 'http://testpage.com/external-article-2/'}); + assert(mention); + assert.equal(mention.get('target'), urlUtils.getSiteUrl()); + assert.ok(!mention.get('resource_id')); + assert.equal(mention.get('resource_type'), null); + assert.equal(mention.get('source_title'), 'Test Page'); + assert.equal(mention.get('source_excerpt'), 'Test description'); + assert.equal(mention.get('source_author'), 'John Doe'); + assert.equal(mention.get('payload'), JSON.stringify({})); + }); + + it('can send an email notification for a new webmention', async function () { + const processWebmentionJob = jobsService.awaitCompletion('processWebmention'); + const targetUrl = new URL('integrations/', urlUtils.getSiteUrl()); + const sourceUrl = new URL('http://testpage.com/external-article-123-email-test/'); + const html = ` + Test Page + `; + + nock(targetUrl.origin) + .head(targetUrl.pathname) + .reply(200); + + nock(sourceUrl.origin) + .get(sourceUrl.pathname) + .reply(200, html, {'Content-Type': 'text/html'}); await agent.post('/receive/') .body({ - source: 'http://testpage.com/external-article-123-email-test/', - target: urlUtils.getSiteUrl() + 'integrations/', - withExtension: true // test payload recorded + source: sourceUrl.href, + target: targetUrl.href }) .expectStatus(202); - - await sleep(2000); + + await processWebmentionJob; const users = await models.User.findAll(); users.forEach(async (user) => { await mockManager.assert.sentEmail({ subject: 'You\'ve been mentioned!', to: user.toJSON().email - }); + }); }); emailMockReceiver.sentEmailCount(users.length); }); it('does not send notification with flag disabled', async function () { mockManager.mockLabsDisabled('webmentionEmail'); - const url = new URL('http://testpage.com/external-article-123-email-test/'); + const processWebmentionJob = jobsService.awaitCompletion('processWebmention'); + const targetUrl = new URL('integrations/', urlUtils.getSiteUrl()); + const sourceUrl = new URL('http://testpage.com/external-article-123-email-test/'); const html = ` - Test Page - `; - nock(url.href) - .get('/') - .reply(200, html, {'content-type': 'text/html'}); + Test Page + `; + + nock(targetUrl.origin) + .head(targetUrl.pathname) + .reply(200); + + nock(sourceUrl.origin) + .get(sourceUrl.pathname) + .reply(200, html, {'Content-Type': 'text/html'}); await agent.post('/receive/') .body({ - source: 'http://testpage.com/external-article-123-email-test/', - target: urlUtils.getSiteUrl() + 'integrations/', - withExtension: true // test payload recorded + source: sourceUrl.href, + target: targetUrl.href }) .expectStatus(202); - - await sleep(2000); + + await processWebmentionJob; + emailMockReceiver.sentEmailCount(0); }); });