From 9e581813981433fc4b7f5ae27524885d2ec63671 Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Tue, 28 Feb 2023 08:39:28 -0600 Subject: [PATCH] combined fetch for metadata and verification (#16330) refs TryGhost/Team#2582 -removed duplicative fetch request -added mention as type in oembed service --- .../services/mentions/WebmentionMetadata.js | 5 ++- .../services/mentions/WebmentionRequest.js | 20 --------- .../core/server/services/mentions/service.js | 5 +-- ghost/oembed-service/lib/oembed-service.js | 8 +++- ghost/webmentions/lib/MentionsAPI.js | 15 ++----- ghost/webmentions/test/MentionsAPI.test.js | 44 ++++++------------- 6 files changed, 27 insertions(+), 70 deletions(-) delete mode 100644 ghost/core/core/server/services/mentions/WebmentionRequest.js diff --git a/ghost/core/core/server/services/mentions/WebmentionMetadata.js b/ghost/core/core/server/services/mentions/WebmentionMetadata.js index cd61ea5ddb..62417e0221 100644 --- a/ghost/core/core/server/services/mentions/WebmentionMetadata.js +++ b/ghost/core/core/server/services/mentions/WebmentionMetadata.js @@ -6,14 +6,15 @@ module.exports = class WebmentionMetadata { * @returns {Promise} */ async fetch(url) { - const data = await oembedService.fetchOembedDataFromUrl(url.href, 'bookmark'); + const data = await oembedService.fetchOembedDataFromUrl(url.href, 'mention'); const result = { siteTitle: data.metadata.publisher, title: data.metadata.title, excerpt: data.metadata.description, author: data.metadata.author, image: data.metadata.thumbnail ? new URL(data.metadata.thumbnail) : null, - favicon: data.metadata.icon ? new URL(data.metadata.icon) : null + favicon: data.metadata.icon ? new URL(data.metadata.icon) : null, + body: data.body }; return result; } diff --git a/ghost/core/core/server/services/mentions/WebmentionRequest.js b/ghost/core/core/server/services/mentions/WebmentionRequest.js deleted file mode 100644 index 1d50a3b349..0000000000 --- a/ghost/core/core/server/services/mentions/WebmentionRequest.js +++ /dev/null @@ -1,20 +0,0 @@ -const logging = require('@tryghost/logging'); -const oembedService = require('../oembed'); - -module.exports = class WebmentionRequest { - /** - * @param {URL} url - * @returns {Promise<{html: string}>} - */ - async fetch(url) { - try { - const data = await oembedService.fetchPageHtml(url.href); - return { - html: data.body - }; - } catch (err) { - logging.warn(err); - return null; - } - } -}; diff --git a/ghost/core/core/server/services/mentions/service.js b/ghost/core/core/server/services/mentions/service.js index 2ffef7fc59..703b1be896 100644 --- a/ghost/core/core/server/services/mentions/service.js +++ b/ghost/core/core/server/services/mentions/service.js @@ -1,6 +1,5 @@ const MentionController = require('./MentionController'); const WebmentionMetadata = require('./WebmentionMetadata'); -const WebmentionRequest = require('./WebmentionRequest'); const { MentionsAPI, MentionSendingService, @@ -33,7 +32,6 @@ module.exports = { DomainEvents }); const webmentionMetadata = new WebmentionMetadata(); - const webmentionRequest = new WebmentionRequest(); const discoveryService = new MentionDiscoveryService({externalRequest}); const resourceService = new ResourceService({ urlUtils, @@ -49,7 +47,6 @@ module.exports = { const api = new MentionsAPI({ repository, webmentionMetadata, - webmentionRequest, resourceService, routingService }); @@ -100,6 +97,6 @@ module.exports = { } } }); - sendingService.listen(events); + sendingService.listen(events); } }; diff --git a/ghost/oembed-service/lib/oembed-service.js b/ghost/oembed-service/lib/oembed-service.js index 373592cc0b..ae819a42d8 100644 --- a/ghost/oembed-service/lib/oembed-service.js +++ b/ghost/oembed-service/lib/oembed-service.js @@ -319,7 +319,7 @@ class OEmbed { } } - if (type !== 'bookmark') { + if (type !== 'bookmark' && type !== 'mention') { // if not a bookmark request, first // check against known oembed list const {url: providerUrl, provider} = findUrlWithProvider(url); @@ -336,6 +336,12 @@ class OEmbed { return this.fetchBookmarkData(url, body); } + // mentions need to return bookmark data (metadata) and body (html) for link verification + if (type === 'mention') { + const bookmark = await this.fetchBookmarkData(url, body); + return {...bookmark, body}; + } + // attempt to fetch oembed // In case response was a redirect, see if we were diff --git a/ghost/webmentions/lib/MentionsAPI.js b/ghost/webmentions/lib/MentionsAPI.js index cfe514a535..32a5b8f5a4 100644 --- a/ghost/webmentions/lib/MentionsAPI.js +++ b/ghost/webmentions/lib/MentionsAPI.js @@ -66,6 +66,7 @@ const Mention = require('./Mention'); * @prop {string} author * @prop {URL} image * @prop {URL} favicon + * @prop {string} body */ /** @@ -73,11 +74,6 @@ const Mention = require('./Mention'); * @prop {(url: URL) => Promise} fetch */ -/** - * @typedef {object} IWebmentionRequest - * @prop {(url: URL) => Promise<{html: string}>} fetch - */ - module.exports = class MentionsAPI { /** @type {IMentionRepository} */ #repository; @@ -87,8 +83,6 @@ module.exports = class MentionsAPI { #routingService; /** @type {IWebmentionMetadata} */ #webmentionMetadata; - /** @type {IWebmentionRequest} */ - #webmentionRequest; /** * @param {object} deps @@ -96,14 +90,12 @@ module.exports = class MentionsAPI { * @param {IResourceService} deps.resourceService * @param {IRoutingService} deps.routingService * @param {IWebmentionMetadata} deps.webmentionMetadata - * @param {IWebmentionRequest} deps.webmentionRequest */ constructor(deps) { this.#repository = deps.repository; this.#resourceService = deps.resourceService; this.#routingService = deps.routingService; this.#webmentionMetadata = deps.webmentionMetadata; - this.#webmentionRequest = deps.webmentionRequest; } /** @@ -198,10 +190,9 @@ module.exports = class MentionsAPI { }); } - const responseBody = await this.#webmentionRequest.fetch(webmention.source); - if (responseBody?.html) { + if (metadata?.body) { try { - mention.verify(responseBody.html); + mention.verify(metadata.body); } catch (e) { logging.error(e); } diff --git a/ghost/webmentions/test/MentionsAPI.test.js b/ghost/webmentions/test/MentionsAPI.test.js index 8e44eb241a..71e1b21686 100644 --- a/ghost/webmentions/test/MentionsAPI.test.js +++ b/ghost/webmentions/test/MentionsAPI.test.js @@ -27,15 +27,8 @@ const mockWebmentionMetadata = { excerpt: 'How many times have you woken up and almost cancelled your church plans? Well this breakfast is about to change everything, a hearty, faith restoring egg dish that will get your tastebuds in a twist.', author: 'Dr Egg Man', image: new URL('https://unsplash.com/photos/QAND9huzD04'), - favicon: new URL('https://ghost.org/favicon.ico') - }; - } -}; - -const mockWebmentionRequest = { - async fetch() { - return { - html: `

Some HTML and a mentioned url

` + favicon: new URL('https://ghost.org/favicon.ico'), + body: `

Some HTML and a mentioned url

` }; } }; @@ -57,8 +50,7 @@ describe('MentionsAPI', function () { repository, routingService: mockRoutingService, resourceService: mockResourceService, - webmentionMetadata: mockWebmentionMetadata, - webmentionRequest: mockWebmentionRequest + webmentionMetadata: mockWebmentionMetadata }); const mention = await api.processWebmention({ @@ -83,8 +75,7 @@ describe('MentionsAPI', function () { repository, routingService: mockRoutingService, resourceService: mockResourceService, - webmentionMetadata: mockWebmentionMetadata, - webmentionRequest: mockWebmentionRequest + webmentionMetadata: mockWebmentionMetadata }); const mention = await api.processWebmention({ @@ -108,8 +99,7 @@ describe('MentionsAPI', function () { repository, routingService: mockRoutingService, resourceService: mockResourceService, - webmentionMetadata: mockWebmentionMetadata, - webmentionRequest: mockWebmentionRequest + webmentionMetadata: mockWebmentionMetadata }); const mentionOne = await api.processWebmention({ @@ -141,8 +131,7 @@ describe('MentionsAPI', function () { repository, routingService: mockRoutingService, resourceService: mockResourceService, - webmentionMetadata: mockWebmentionMetadata, - webmentionRequest: mockWebmentionRequest + webmentionMetadata: mockWebmentionMetadata }); const mentionOne = await api.processWebmention({ @@ -178,8 +167,7 @@ describe('MentionsAPI', function () { repository, routingService: mockRoutingService, resourceService: mockResourceService, - webmentionMetadata: mockWebmentionMetadata, - webmentionRequest: mockWebmentionRequest + webmentionMetadata: mockWebmentionMetadata }); const mentionOne = await api.processWebmention({ @@ -215,8 +203,7 @@ describe('MentionsAPI', function () { repository, routingService: mockRoutingService, resourceService: mockResourceService, - webmentionMetadata: mockWebmentionMetadata, - webmentionRequest: mockWebmentionRequest + webmentionMetadata: mockWebmentionMetadata }); const mentionOne = await api.processWebmention({ @@ -253,8 +240,7 @@ describe('MentionsAPI', function () { } }, resourceService: mockResourceService, - webmentionMetadata: mockWebmentionMetadata, - webmentionRequest: mockWebmentionRequest + webmentionMetadata: mockWebmentionMetadata }); let errored = false; @@ -283,8 +269,7 @@ describe('MentionsAPI', function () { } }, resourceService: mockResourceService, - webmentionMetadata: mockWebmentionMetadata, - webmentionRequest: mockWebmentionRequest + webmentionMetadata: mockWebmentionMetadata }); await api.processWebmention({ @@ -307,8 +292,7 @@ describe('MentionsAPI', function () { }; } }, - webmentionMetadata: mockWebmentionMetadata, - webmentionRequest: mockWebmentionRequest + webmentionMetadata: mockWebmentionMetadata }); const mention = await api.processWebmention({ @@ -341,8 +325,7 @@ describe('MentionsAPI', function () { }; } }, - webmentionMetadata: mockWebmentionMetadata, - webmentionRequest: mockWebmentionRequest + webmentionMetadata: mockWebmentionMetadata }); checkFirstMention: { @@ -393,8 +376,7 @@ describe('MentionsAPI', function () { fetch: sinon.stub() .onFirstCall().resolves(mockWebmentionMetadata.fetch()) .onSecondCall().rejects() - }, - webmentionRequest: mockWebmentionRequest + } }); checkFirstMention: {