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

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({
+        const mediaHandler = new ImporterContentFileHandler({
-            config: config,
+            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
         });

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;

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: {
+            extensions: ['mp4'],
-                get: () => ({
-                    media: {
-                        extensions: ['mp4']
-                    }
-                })
-            },
             urlUtils: {}
         });
 
@@ -33,13 +27,7 @@ describe('ImporterContentFileHandler', function () {
     it('returns configured contentTypes', function () {
         const contentFileImporter = new ImporterContentFileHandler({
             storage: {},
-            config: {
+            contentTypes: ['video/mp4'],
-                get: () => ({
-                    media: {
-                        contentTypes: ['video/mp4']
-                    }
-                })
-            },
             urlUtils: {}
         });
 
@@ -56,11 +44,7 @@ describe('ImporterContentFileHandler', function () {
                     staticFileURLPrefix: 'content/media',
                     getUniqueFileName: (file, targetDir) => Promise.resolve(targetDir + '/' + file.name)
                 },
-                config: {
+                contentPath: '/var/www/ghost/content/media',
-                    getContentPath: sinon.stub()
-                        .withArgs('media')
-                        .returns('/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: {
+                contentPath: '/var/www/ghost/content/media',
-                    getContentPath: sinon.stub()
-                        .withArgs('media')
-                        .returns('/var/www/ghost/content/media')
-                },
                 urlUtils: {
                     getSubdir: () => 'blog',
                     urlJoin: (...args) => args.join('/')