Revert "Fixed fetching and storing bookmark card icons and thumbnails (…

…#21036)" ref https://linear.app/tryghost/issue/ONC-433 ref https://linear.app/tryghost/issue/ENG-904 - the reverted commit (871d21a) caused a regression on recommendations: incoming recommendations were marked as deleted and did not render in Admin Settings anymore
TryGhost · Oct 16, 2024 · 27d2af3 · 27d2af3
1 parent d689484
commit 27d2af3
Show file tree

Hide file tree

Showing 6 changed files with 14 additions and 146 deletions.
diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/index.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/index.js
@@ -9,7 +9,6 @@ module.exports = {
     emailFailures: require('./email-failures'),
     images: require('./images'),
     integrations: require('./integrations'),
-    oembed: require('./oembed'),
     pages: require('./pages'),
     posts: require('./posts'),
     settings: require('./settings'),

diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/oembed.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/oembed.js
diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/oembed.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/oembed.js
@@ -1,15 +1,8 @@
 const debug = require('@tryghost/debug')('api:endpoints:utils:serializers:output:oembed');
-const mappers = require('./mappers');
 
 module.exports = {
     all(data, apiConfig, frame) {
         debug('all');
-        if (data?.metadata?.thumbnail) {
-            data.metadata.thumbnail = mappers.oembed(data.metadata.thumbnail);
-        }
-        if (data?.metadata?.icon) {
-            data.metadata.icon = mappers.oembed(data.metadata.icon);
-        }
         frame.response = data;
     }
 };
diff --git 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
@@ -6,8 +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');
-const urlUtils = require('../../../core/shared/url-utils');
 
 // for sinon stubs
 const dnsPromises = require('dns').promises;
@@ -21,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, imageType) {
-            if (imageUrl === 'http://example.com/bad-image') {
-                throw new Error('Failed to process image');
-            }
-            return `/content/images/${imageType}/image-01.png`;
-        });
     });
 
     afterEach(function () {
@@ -239,10 +228,15 @@ describe('Oembed API', function () {
                 .get('/page-with-icon')
                 .reply(
                     200,
-                    '<html><head><title>TESTING</title><link rel="icon" href="http://example.com/bad-image"></head><body></body></html>',
+                    '<html><head><title>TESTING</title><link rel="icon" href="http://example.com/icon.svg"></head><body></body></html>',
                     {'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'))
@@ -258,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,
-                '<html><head><title>TESTING</title><link rel="icon" href="http://example.com/icon.svg"></head><body></body></html>',
-                {'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(`${urlUtils.urlFor('home', true)}content/images/icon/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,
-                '<html><head><title>TESTING</title><link rel="thumbnail" href="http://example.com/thumbnail.svg"></head><body></body></html>',
-                {'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(`${urlUtils.urlFor('home', true)}content/images/thumbnail/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
@@ -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/)';
@@ -50,12 +49,6 @@ const findUrlWithProvider = (url) => {
 /**
  * @typedef {Object} IConfig
  * @prop {(key: string) => string} get
- * @prop {(key: string) => string} getContentPath
- */
-
-/**
- * @typedef {Object} IStorage
- * @prop {(feature: string) => Object} getStorage
  */
 
 /**
@@ -73,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;
@@ -127,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<Buffer>} - 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<String>} - 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
@@ -329,20 +271,14 @@ class OEmbedService {
             });
         }
 
-        await this.processImageFromUrl(metadata.icon, 'icon')
-            .then((processedImageUrl) => {
-                metadata.icon = processedImageUrl;
-            }).catch((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);
-            });
-
-        await this.processImageFromUrl(metadata.thumbnail, 'thumbnail')
-            .then((processedImageUrl) => {
-                metadata.thumbnail = processedImageUrl;
-            }).catch((err) => {
-                logging.error(err);
-            });
+            }
+        }
 
         return {
             version: '1.0',