From b0b80d672d7e6605d355ca243db2040b83161c51 Mon Sep 17 00:00:00 2001 From: Naz Date: Thu, 2 Mar 2023 15:32:36 +0800 Subject: [PATCH] Refactored content file handler constructor refs https://github.com/TryGhost/Toolbox/issues/523 - To support generic "file" import handling the content file handler needs more configuration options for properties like type, extensions, content path etc. This refactor makes the handler configurable and reusable for any type of file import --- .../server/data/importer/import-manager.js | 13 ++++-- .../lib/ImporterContentFileHandler.js | 46 +++++++++++-------- .../test/ImporterContentFileHandler.test.js | 30 ++---------- 3 files changed, 43 insertions(+), 46 deletions(-) diff --git a/ghost/core/core/server/data/importer/import-manager.js b/ghost/core/core/server/data/importer/import-manager.js index 11ba7c1734..4bc90de62e 100644 --- a/ghost/core/core/server/data/importer/import-manager.js +++ b/ghost/core/core/server/data/importer/import-manager.js @@ -11,7 +11,7 @@ const debug = require('@tryghost/debug')('import-manager'); const logging = require('@tryghost/logging'); const errors = require('@tryghost/errors'); const ImageHandler = require('./handlers/image'); -const MediaHandler = require('@tryghost/importer-handler-content-files'); +const ImporterContentFileHandler = require('@tryghost/importer-handler-content-files'); const RevueHandler = require('./handlers/revue'); const JSONHandler = require('./handlers/json'); const MarkdownHandler = require('./handlers/markdown'); @@ -54,8 +54,15 @@ let defaults = { class ImportManager { constructor() { - const mediaHandler = new MediaHandler({ - config: config, + const mediaHandler = new ImporterContentFileHandler({ + type: 'media', + // @NOTE: making the second parameter strict folder "content/media" broke the glob pattern + // in the importer, so we need to keep it as general "content" unless + // it becomes a strict requirement + directories: ['media', 'content'], + extensions: config.get('uploads').media.extensions, + contentTypes: config.get('uploads').media.contentTypes, + contentPath: config.getContentPath('media'), urlUtils: urlUtils, storage: mediaStorage }); diff --git a/ghost/importer-handler-content-files/lib/ImporterContentFileHandler.js b/ghost/importer-handler-content-files/lib/ImporterContentFileHandler.js index e925773a9f..8258605b35 100644 --- a/ghost/importer-handler-content-files/lib/ImporterContentFileHandler.js +++ b/ghost/importer-handler-content-files/lib/ImporterContentFileHandler.js @@ -2,34 +2,44 @@ const _ = require('lodash'); const path = require('path'); class ImporterContentFileHandler { + /** @property {'media' | 'files' | 'images'} */ + type; + + /** @property {string[]} */ + directories; + + /** @property {string[]} */ + extensions; + + /** @property {string[]} */ + contentTypes; + + /** + * Holds path to the destination content directory + * @property {string} */ + #contentPath; + /** * * @param {Object} deps dependencies + * @param {'media' | 'files' | 'images'} deps.type type of content file + * @param {string[]} deps.extensions file extensions to search for + * @param {string[]} deps.contentTypes content types to search for + * @param {string[]} deps.directories directories to search for content files + * @param {string} deps.contentPath path to the destination content directory * @param {Object} deps.storage storage adapter instance - * @param {Object} deps.config config instance * @param {object} deps.urlUtils urlUtils instance */ constructor(deps) { + this.type = deps.type; + this.directories = deps.directories; + this.extensions = deps.extensions; + this.contentTypes = deps.contentTypes; this.storage = deps.storage; - this.config = deps.config; + this.#contentPath = deps.contentPath; this.urlUtils = deps.urlUtils; } - type = 'media'; - - get extensions() { - return this.config.get('uploads').media.extensions; - } - - get contentTypes() { - return this.config.get('uploads').media.contentTypes; - } - - // @NOTE: making the second parameter strict folder "content/media" broke the glob pattern - // in the importer, so we need to keep it as general "content" unless - // it becomes a strict requirement - directories = ['media', 'content']; - async loadFile(files, baseDir) { const baseDirRegex = baseDir ? new RegExp('^' + baseDir + '/') : new RegExp(''); @@ -38,7 +48,7 @@ class ImporterContentFileHandler { }); // normalize the directory structure - const mediaContentPath = this.config.getContentPath('media'); + const mediaContentPath = this.#contentPath; files = _.map(files, function (file) { const noBaseDir = file.name.replace(baseDirRegex, ''); let noGhostDirs = noBaseDir; diff --git a/ghost/importer-handler-content-files/test/ImporterContentFileHandler.test.js b/ghost/importer-handler-content-files/test/ImporterContentFileHandler.test.js index 7d82388743..aefbe872de 100644 --- a/ghost/importer-handler-content-files/test/ImporterContentFileHandler.test.js +++ b/ghost/importer-handler-content-files/test/ImporterContentFileHandler.test.js @@ -1,10 +1,10 @@ const assert = require('assert'); -const sinon = require('sinon'); const ImporterContentFileHandler = require('../index'); describe('ImporterContentFileHandler', function () { it('creates an instance', function () { const contentFileImporter = new ImporterContentFileHandler({ + type: 'media', storage: {}, config: {}, urlUtils: {} @@ -17,13 +17,7 @@ describe('ImporterContentFileHandler', function () { it('returns configured extensions', function () { const contentFileImporter = new ImporterContentFileHandler({ storage: {}, - config: { - get: () => ({ - media: { - extensions: ['mp4'] - } - }) - }, + extensions: ['mp4'], urlUtils: {} }); @@ -33,13 +27,7 @@ describe('ImporterContentFileHandler', function () { it('returns configured contentTypes', function () { const contentFileImporter = new ImporterContentFileHandler({ storage: {}, - config: { - get: () => ({ - media: { - contentTypes: ['video/mp4'] - } - }) - }, + contentTypes: ['video/mp4'], urlUtils: {} }); @@ -56,11 +44,7 @@ describe('ImporterContentFileHandler', function () { staticFileURLPrefix: 'content/media', getUniqueFileName: (file, targetDir) => Promise.resolve(targetDir + '/' + file.name) }, - config: { - getContentPath: sinon.stub() - .withArgs('media') - .returns('/var/www/ghost/content/media') - }, + contentPath: '/var/www/ghost/content/media', urlUtils: { getSubdir: () => 'blog', urlJoin: (...args) => args.join('/') @@ -86,11 +70,7 @@ describe('ImporterContentFileHandler', function () { staticFileURLPrefix: 'content/media', getUniqueFileName: (file, targetDir) => Promise.resolve(targetDir + '/' + file.name) }, - config: { - getContentPath: sinon.stub() - .withArgs('media') - .returns('/var/www/ghost/content/media') - }, + contentPath: '/var/www/ghost/content/media', urlUtils: { getSubdir: () => 'blog', urlJoin: (...args) => args.join('/')