From 74a1ab45250ed35baf45ce56cc0156835bb9ce5f Mon Sep 17 00:00:00 2001 From: Naz Date: Thu, 16 Mar 2023 22:48:57 +0100 Subject: [PATCH] Fixed root zip media/files copying during import refs https://github.com/TryGhost/Toolbox/issues/523 - During import process of content files the files from the root directory were also copied over. This is causing chaos in the root of content folder with files that only needed for data import. For example, the csv files needed for Revue import were also copied over by "file importer" even though those do not belong to any content. - The approach of filtering on a "handler" level was taken over filtering in the import manager due to how our globing patterns work. See a previous commit with reverted previous fix for more context. --- .../server/data/importer/import-manager.js | 2 ++ .../lib/ImporterContentFileHandler.js | 8 +++++ .../test/ImporterContentFileHandler.test.js | 36 +++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/ghost/core/core/server/data/importer/import-manager.js b/ghost/core/core/server/data/importer/import-manager.js index a7b0053564..d1d4891785 100644 --- a/ghost/core/core/server/data/importer/import-manager.js +++ b/ghost/core/core/server/data/importer/import-manager.js @@ -61,6 +61,7 @@ class ImportManager { // in the importer, so we need to keep it as general "content" unless // it becomes a strict requirement directories: ['media', 'content'], + ignoreRootFolderFiles: true, extensions: config.get('uploads').media.extensions, contentTypes: config.get('uploads').media.contentTypes, contentPath: config.getContentPath('media'), @@ -74,6 +75,7 @@ class ImportManager { // in the importer, so we need to keep it as general "content" unless // it becomes a strict requirement directories: ['files', 'content'], + ignoreRootFolderFiles: true, extensions: config.get('uploads').files.extensions, contentTypes: config.get('uploads').files.contentTypes, contentPath: config.getContentPath('files'), diff --git a/ghost/importer-handler-content-files/lib/ImporterContentFileHandler.js b/ghost/importer-handler-content-files/lib/ImporterContentFileHandler.js index a7bbafc032..ca44dab55c 100644 --- a/ghost/importer-handler-content-files/lib/ImporterContentFileHandler.js +++ b/ghost/importer-handler-content-files/lib/ImporterContentFileHandler.js @@ -24,6 +24,7 @@ class ImporterContentFileHandler { * @param {Object} deps dependencies * @param {'media' | 'files' | 'images'} deps.type type of content file * @param {string[]} deps.extensions file extensions to search for + * @param {boolean} [deps.ignoreRootFolderFiles] whether to ignore files in the root folder * @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 @@ -35,6 +36,7 @@ class ImporterContentFileHandler { this.directories = deps.directories; this.extensions = deps.extensions; this.contentTypes = deps.contentTypes; + this.ignoreRootFolderFiles = deps.ignoreRootFolderFiles; this.storage = deps.storage; this.#contentPath = deps.contentPath; this.urlUtils = deps.urlUtils; @@ -47,6 +49,12 @@ class ImporterContentFileHandler { return new RegExp('^' + dir + '/'); }); + if (this.ignoreRootFolderFiles) { + files = _.filter(files, function (file) { + return file.name.indexOf('/') !== -1; + }); + } + // normalize the directory structure const filesContentPath = this.#contentPath; files = _.map(files, function (file) { diff --git a/ghost/importer-handler-content-files/test/ImporterContentFileHandler.test.js b/ghost/importer-handler-content-files/test/ImporterContentFileHandler.test.js index aefbe872de..98373698e3 100644 --- a/ghost/importer-handler-content-files/test/ImporterContentFileHandler.test.js +++ b/ghost/importer-handler-content-files/test/ImporterContentFileHandler.test.js @@ -88,5 +88,41 @@ describe('ImporterContentFileHandler', function () { assert.equal(files[0].targetDir, '/var/www/ghost/content/media'); assert.equal(files[0].newPath, '//blog/content/media/1.mp4'); }); + + it('ignores files in root folder', async function () { + const contentFileImporter = new ImporterContentFileHandler({ + storage: { + staticFileURLPrefix: 'content/media', + getUniqueFileName: (file, targetDir) => Promise.resolve(targetDir + '/' + file.name) + }, + contentPath: '/var/www/ghost/content/media', + ignoreRootFolderFiles: true, + urlUtils: { + getSubdir: () => 'blog', + urlJoin: (...args) => args.join('/') + } + }); + + const files = [{ + name: 'root.mp4' + }, { + name: 'content/media/1.mp4' + }]; + + await contentFileImporter.loadFile(files); + + // @NOTE: the root file is ignored. It's a weird test because ideally the file + // should be removed completely from the list, but the importer works + // by modifying the list in place and it's not easy to remove the file + assert.equal(files[0].name, 'root.mp4'); + assert.equal(files[0].originalPath, undefined); + assert.equal(files[0].targetDir, undefined); + assert.equal(files[0].newPath, undefined); + + assert.equal(files[1].name, '1.mp4'); + assert.equal(files[1].originalPath, 'content/media/1.mp4'); + assert.equal(files[1].targetDir, '/var/www/ghost/content/media'); + assert.equal(files[1].newPath, '//blog/content/media/1.mp4'); + }); }); });