From 99d8a5777fdd9326edada4f4b9be152bce772dd5 Mon Sep 17 00:00:00 2001 From: Sag Date: Tue, 17 Sep 2024 20:55:03 +0200 Subject: [PATCH] Fixed "Unsaved post" modal shown after publishing (#21028) ref https://linear.app/tryghost/issue/ENG-661 ref INC-109 - this PR reverts two commits: 5903dd7 and 426b1d4 --- .../core/server/services/oembed/service.js | 3 +- ghost/core/test/e2e-api/admin/oembed.test.js | 65 ++----------- ghost/oembed-service/lib/OEmbedService.js | 96 +++---------------- 3 files changed, 18 insertions(+), 146 deletions(-) diff --git a/ghost/core/core/server/services/oembed/service.js b/ghost/core/core/server/services/oembed/service.js index 25f55e88b1..77b7cd72d2 100644 --- a/ghost/core/core/server/services/oembed/service.js +++ b/ghost/core/core/server/services/oembed/service.js @@ -1,9 +1,8 @@ const config = require('../../../shared/config'); -const storage = require('../../adapters/storage'); const externalRequest = require('../../lib/request-external'); const OEmbed = require('@tryghost/oembed-service'); -const oembed = new OEmbed({config, externalRequest, storage}); +const oembed = new OEmbed({config, externalRequest}); const NFT = require('./NFTOEmbedProvider'); const nft = new NFT({ diff --git a/ghost/core/test/e2e-api/admin/oembed.test.js b/ghost/core/test/e2e-api/admin/oembed.test.js index 7cfad57810..39a6184baf 100644 --- a/ghost/core/test/e2e-api/admin/oembed.test.js +++ b/ghost/core/test/e2e-api/admin/oembed.test.js @@ -6,7 +6,6 @@ const testUtils = require('../../utils/index'); const config = require('../../../core/shared/config/index'); const localUtils = require('./utils'); const {mockManager} = require('../../utils/e2e-framework'); -const oembed = require('../../../../core/core/server/services/oembed'); // for sinon stubs const dnsPromises = require('dns').promises; @@ -20,18 +19,9 @@ describe('Oembed API', function () { await localUtils.doAuth(request); }); - let processImageFromUrlStub; - beforeEach(function () { // ensure sure we're not network dependent mockManager.disableNetwork(); - processImageFromUrlStub = sinon.stub(oembed, 'processImageFromUrl'); - processImageFromUrlStub.callsFake(async function (imageUrl) { - if (imageUrl === 'http://example.com/bad-image') { - throw new Error('Failed to process image'); - } - return '/content/images/image-01.png'; - }); }); afterEach(function () { @@ -238,10 +228,15 @@ describe('Oembed API', function () { .get('/page-with-icon') .reply( 200, - 'TESTING', + 'TESTING', {'content-type': 'text/html'} ); + // Mock the icon URL to return 404 + nock('http://example.com/') + .head('/icon.svg') + .reply(404); + const url = encodeURIComponent(' http://example.com/page-with-icon\t '); // Whitespaces are to make sure urls are trimmed const res = await request.get(localUtils.API.getApiQuery(`oembed/?url=${url}&type=bookmark`)) .set('Origin', config.get('url')) @@ -257,54 +252,6 @@ describe('Oembed API', function () { }); }); - it('should fetch and store icons', async function () { - // Mock the page to contain a readable icon URL - const pageMock = nock('http://example.com') - .get('/page-with-icon') - .reply( - 200, - 'TESTING', - {'content-type': 'text/html'} - ); - - const url = encodeURIComponent(' http://example.com/page-with-icon\t '); // Whitespaces are to make sure urls are trimmed - const res = await request.get(localUtils.API.getApiQuery(`oembed/?url=${url}&type=bookmark`)) - .set('Origin', config.get('url')) - .expect('Content-Type', /json/) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(200); - - // Check that the icon URL mock was loaded - pageMock.isDone().should.be.true(); - - // Check that the substitute icon URL is returned in place of the original - res.body.metadata.icon.should.eql('/content/images/image-01.png'); - }); - - it('should fetch and store thumbnails', async function () { - // Mock the page to contain a readable icon URL - const pageMock = nock('http://example.com') - .get('/page-with-thumbnail') - .reply( - 200, - 'TESTING', - {'content-type': 'text/html'} - ); - - const url = encodeURIComponent(' http://example.com/page-with-thumbnail\t '); // Whitespaces are to make sure urls are trimmed - const res = await request.get(localUtils.API.getApiQuery(`oembed/?url=${url}&type=bookmark`)) - .set('Origin', config.get('url')) - .expect('Content-Type', /json/) - .expect('Cache-Control', testUtils.cacheRules.private) - .expect(200); - - // Check that the thumbnail URL mock was loaded - pageMock.isDone().should.be.true(); - - // Check that the substitute thumbnail URL is returned in place of the original - res.body.metadata.thumbnail.should.eql('/content/images/image-01.png'); - }); - describe('with unknown provider', function () { it('fetches url and follows redirects', async function () { const redirectMock = nock('http://test.com/') diff --git a/ghost/oembed-service/lib/OEmbedService.js b/ghost/oembed-service/lib/OEmbedService.js index 0fcba74835..85e1330b71 100644 --- a/ghost/oembed-service/lib/OEmbedService.js +++ b/ghost/oembed-service/lib/OEmbedService.js @@ -4,7 +4,6 @@ const logging = require('@tryghost/logging'); const _ = require('lodash'); const charset = require('charset'); const iconv = require('iconv-lite'); -const path = require('path'); // Some sites block non-standard user agents so we need to mimic a typical browser const USER_AGENT = 'Mozilla/5.0 (compatible; Ghost/5.0; +https://ghost.org/)'; @@ -52,11 +51,6 @@ const findUrlWithProvider = (url) => { * @prop {(key: string) => string} get */ -/** - * @typedef {Object} IStorage - * @prop {(feature: string) => Object} getStorage - */ - /** * @typedef {(url: string, config: Object) => Promise} IExternalRequest */ @@ -72,12 +66,10 @@ class OEmbedService { * * @param {Object} dependencies * @param {IConfig} dependencies.config - * @param {IStorage} dependencies.storage * @param {IExternalRequest} dependencies.externalRequest */ - constructor({config, externalRequest, storage}) { + constructor({config, externalRequest}) { this.config = config; - this.storage = storage; /** @type {IExternalRequest} */ this.externalRequest = externalRequest; @@ -126,55 +118,6 @@ class OEmbedService { } } - /** - * Fetches the image buffer from a URL using fetch - * @param {String} imageUrl - URL of the image to fetch - * @returns {Promise} - Promise resolving to the image buffer - */ - async fetchImageBuffer(imageUrl) { - const response = await fetch(imageUrl); - - if (!response.ok) { - throw Error(`Failed to fetch image: ${response.statusText}`); - } - - const arrayBuffer = await response.arrayBuffer(); - - const buffer = Buffer.from(arrayBuffer); - return buffer; - } - - /** - * Process and store image from a URL - * @param {String} imageUrl - URL of the image to process - * @param {String} imageType - What is the image used for. Example - icon, thumbnail - * @returns {Promise} - URL where the image is stored - */ - async processImageFromUrl(imageUrl, imageType) { - // Fetch image buffer from the URL - const imageBuffer = await this.fetchImageBuffer(imageUrl); - const store = this.storage.getStorage('images'); - - // Extract file name from URL - const fileName = path.basename(new URL(imageUrl).pathname); - let ext = path.extname(fileName); - let name; - - if (ext) { - name = store.getSanitizedFileName(path.basename(fileName, ext)); - } else { - name = store.getSanitizedFileName(path.basename(fileName)); - } - - let targetDir = path.join(this.config.getContentPath('images'), imageType); - const uniqueFilePath = await store.generateUnique(targetDir, name, ext, 0); - const targetPath = path.join(imageType, path.basename(uniqueFilePath)); - - const imageStoredUrl = await store.saveRaw(imageBuffer, targetPath); - - return imageStoredUrl; - } - /** * @param {string} url * @param {Object} options @@ -266,7 +209,7 @@ class OEmbedService { * * @returns {Promise} */ - async fetchBookmarkData(url, html, type) { + async fetchBookmarkData(url, html) { const gotOpts = { headers: { 'User-Agent': USER_AGENT @@ -328,29 +271,12 @@ class OEmbedService { }); } - if (type === 'bookmark') { - await this.processImageFromUrl(metadata.icon, 'icon') - .then((processedImageUrl) => { - metadata.icon = processedImageUrl; - }).catch((err) => { - metadata.icon = 'https://static.ghost.org/v5.0.0/images/link-icon.svg'; - logging.error(err); - }); - - await this.processImageFromUrl(metadata.thumbnail, 'thumbnail') - .then((processedImageUrl) => { - metadata.thumbnail = processedImageUrl; - }).catch((err) => { - logging.error(err); - }); - } else { - if (metadata.icon) { - try { - await this.externalRequest.head(metadata.icon); - } catch (err) { - metadata.icon = 'https://static.ghost.org/v5.0.0/images/link-icon.svg'; - logging.error(err); - } + if (metadata.icon) { + try { + await this.externalRequest.head(metadata.icon); + } catch (err) { + metadata.icon = 'https://static.ghost.org/v5.0.0/images/link-icon.svg'; + logging.error(err); } } @@ -486,7 +412,7 @@ class OEmbedService { // fetch only bookmark when explicitly requested if (type === 'bookmark') { - return this.fetchBookmarkData(url, body, type); + return this.fetchBookmarkData(url, body); } // mentions need to return bookmark data (metadata) and body (html) for link verification @@ -509,7 +435,7 @@ class OEmbedService { }; return {...bookmark, body}; } - const bookmark = await this.fetchBookmarkData(url, body, type); + const bookmark = await this.fetchBookmarkData(url, body); return {...bookmark, body, contentType}; } @@ -528,7 +454,7 @@ class OEmbedService { // fallback to bookmark when we can't get oembed if (!data && !type) { - data = await this.fetchBookmarkData(url, body, type); + data = await this.fetchBookmarkData(url, body); } // couldn't get anything, throw a validation error