From 42ac8c41e403bbb4cd6ceaff34658461e5935929 Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Fri, 11 Mar 2022 09:17:58 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20uppercase=20file=20exten?= =?UTF-8?q?sions=20ignored=20in=20content=20import=20(#14268)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs https://github.com/TryGhost/Team/issues/1363 - When uploading a zip of images in Settings > Labs > [Import], it will skip images that have an uppercase extension, citing an 'unsupported file type' error. - Cause: Glob ignored those files when matching extensions in ImportManager - Fix: Added nocase option where needed - Extended tests to also test the processZip method of ImportManager with getFilesFromZip - Added isValidZip for zip with uppercase image - Cleaned up JSDoc in ImportManager, and replaced some older JS syntax Fixed zipContainsMultipleDataFormats error never thrown: When a zip combines two data formats, no error was thrown. - The promise error was only returned in an _.each loop, but never thrown - Previously when combining multiple data types in a zip file, no error got thrown - Added a test for this error - Also added a test for noContentToImport error Other errors and fixes: - Added missing length in getBaseDirectory check - getContentTypes fixed (returned duplicate values). Type error came up after adding all JSDocs - updated tests to match real types from JSDoc and pass type validations - Rewrote some methods in the async await syntax - Added tests for ImportManager clean up --- core/server/data/importer/import-manager.js | 262 +++++++++++------- test/unit/server/data/importer/index.test.js | 161 +++++++++-- .../zips/zip-multiple-data-formats/test.json | 0 .../zips/zip-multiple-data-formats/test.md | 0 .../zips/zip-uppercase-extensions/image.JPG | 0 5 files changed, 299 insertions(+), 124 deletions(-) create mode 100644 test/utils/fixtures/import/zips/zip-multiple-data-formats/test.json create mode 100644 test/utils/fixtures/import/zips/zip-multiple-data-formats/test.md create mode 100644 test/utils/fixtures/import/zips/zip-uppercase-extensions/image.JPG diff --git a/core/server/data/importer/import-manager.js b/core/server/data/importer/import-manager.js index 48e54961ce..084ae4fdda 100644 --- a/core/server/data/importer/import-manager.js +++ b/core/server/data/importer/import-manager.js @@ -1,12 +1,10 @@ const _ = require('lodash'); -const Promise = require('bluebird'); const fs = require('fs-extra'); const path = require('path'); const os = require('os'); const glob = require('glob'); const uuid = require('uuid'); const {extract} = require('@tryghost/zip'); -const {pipeline, sequence} = require('@tryghost/promise'); const tpl = require('@tryghost/tpl'); const logging = require('@tryghost/logging'); const errors = require('@tryghost/errors'); @@ -40,10 +38,20 @@ let defaults = { class ImportManager { constructor() { + /** + * @type {Importer[]} importers + */ this.importers = [ImageImporter, DataImporter]; + + /** + * @type {Handler[]} + */ this.handlers = [ImageHandler, JSONHandler, MarkdownHandler]; // Keep track of file to cleanup at the end + /** + * @type {?string} + */ this.fileToDelete = null; } @@ -52,7 +60,7 @@ class ImportManager { * @returns {string[]} */ getExtensions() { - return _.flatten(_.union(_.map(this.handlers, 'extensions'), defaults.extensions)); + return _.union(_.flatMap(this.handlers, 'extensions'), defaults.extensions); } /** @@ -60,7 +68,7 @@ class ImportManager { * @returns {string[]} */ getContentTypes() { - return _.flatten(_.union(_.map(this.handlers, 'contentTypes'), defaults.contentTypes)); + return _.union(_.flatMap(this.handlers, 'contentTypes'), defaults.contentTypes); } /** @@ -68,7 +76,7 @@ class ImportManager { * @returns {string[]} */ getDirectories() { - return _.flatten(_.union(_.map(this.handlers, 'directories'), defaults.directories)); + return _.union(_.flatMap(this.handlers, 'directories'), defaults.directories); } /** @@ -84,7 +92,7 @@ class ImportManager { /** * @param {String[]} extensions - * @param {Number} level + * @param {Number} [level] * @returns {String} */ getExtensionGlob(extensions, level) { @@ -97,7 +105,7 @@ class ImportManager { /** * * @param {String[]} directories - * @param {Number} level + * @param {Number} [level] * @returns {String} */ getDirectoryGlob(directories, level) { @@ -109,26 +117,24 @@ class ImportManager { /** * Remove files after we're done (abstracted into a function for easier testing) - * @returns {Function} + * @returns {Promise} */ - cleanUp() { - const self = this; - - if (self.fileToDelete === null) { + async cleanUp() { + if (this.fileToDelete === null) { return; } - fs.remove(self.fileToDelete, function (err) { - if (err) { - logging.error(new errors.InternalServerError({ - err: err, - context: tpl(messages.couldNotCleanUpFile.error), - help: tpl(messages.couldNotCleanUpFile.context) - })); - } + try { + await fs.remove(this.fileToDelete); + } catch (err) { + logging.error(new errors.InternalServerError({ + err: err, + context: tpl(messages.couldNotCleanUpFile.error), + help: tpl(messages.couldNotCleanUpFile.context) + })); + } - self.fileToDelete = null; - }); + this.fileToDelete = null; } /** @@ -145,14 +151,14 @@ class ImportManager { * Importable content must be found either in the root, or inside one base directory * * @param {String} directory - * @returns {Promise} + * @returns {boolean} */ isValidZip(directory) { // Globs match content in the root or inside a single directory - const extMatchesBase = glob.sync(this.getExtensionGlob(this.getExtensions(), ROOT_OR_SINGLE_DIR), {cwd: directory}); + const extMatchesBase = glob.sync(this.getExtensionGlob(this.getExtensions(), ROOT_OR_SINGLE_DIR), {cwd: directory, nocase: true}); const extMatchesAll = glob.sync( - this.getExtensionGlob(this.getExtensions(), ALL_DIRS), {cwd: directory} + this.getExtensionGlob(this.getExtensions(), ALL_DIRS), {cwd: directory, nocase: true} ); const dirMatches = glob.sync( @@ -173,8 +179,8 @@ class ImportManager { /** * Use the extract module to extract the given zip file to a temp directory & return the temp directory path - * @param {String} filePath - * @returns {Promise[]} Files + * @param {string} filePath + * @returns {Promise} full path to the extracted folder */ extractZip(filePath) { const tmpDir = path.join(os.tmpdir(), uuid.v4()); @@ -190,11 +196,11 @@ class ImportManager { * are relevant to the given handler, and return them as a name and path combo * @param {Object} handler * @param {String} directory - * @returns [] Files + * @returns {File[]} Files */ getFilesFromZip(handler, directory) { const globPattern = this.getExtensionGlob(handler.extensions, ALL_DIRS); - return _.map(glob.sync(globPattern, {cwd: directory}), function (file) { + return _.map(glob.sync(globPattern, {cwd: directory, nocase: true}), function (file) { return {name: file, path: path.join(directory, file)}; }); } @@ -206,9 +212,9 @@ class ImportManager { */ getBaseDirectory(directory) { // Globs match root level only - const extMatches = glob.sync(this.getExtensionGlob(this.getExtensions(), ROOT_ONLY), {cwd: directory}); + const extMatches = glob.sync(this.getExtensionGlob(this.getExtensions(), ROOT_ONLY), {cwd: directory, nocase: true}); - const dirMatches = glob.sync(this.getDirectoryGlob(this.getDirectories(), ROOT_ONLY), {cwd: directory}); + const dirMatches = glob.sync(this.getDirectoryGlob(this.getDirectories(), ROOT_ONLY), {cwd: directory, nocase: true}); let extMatchesAll; // There is no base directory @@ -217,9 +223,9 @@ class ImportManager { } // There is a base directory, grab it from any ext match extMatchesAll = glob.sync( - this.getExtensionGlob(this.getExtensions(), ALL_DIRS), {cwd: directory} + this.getExtensionGlob(this.getExtensions(), ALL_DIRS), {cwd: directory, nocase: true} ); - if (extMatchesAll.length < 1 || extMatchesAll[0].split('/') < 1) { + if (extMatchesAll.length < 1 || extMatchesAll[0].split('/').length < 1) { throw new errors.ValidationError({message: tpl(messages.invalidZipFileBaseDirectory)}); } @@ -233,48 +239,42 @@ class ImportManager { * The data key contains JSON representing any data that should be imported * The image key contains references to images that will be stored (and where they will be stored) * @param {File} file - * @returns {Promise(ImportData)} + * @returns {Promise} */ - processZip(file) { - const self = this; + async processZip(file) { + const zipDirectory = await this.extractZip(file.path); - return this.extractZip(file.path).then(function (zipDirectory) { - const ops = []; - const importData = {}; - let baseDir; + /** + * @type {ImportData} + */ + const importData = {}; - self.isValidZip(zipDirectory); - baseDir = self.getBaseDirectory(zipDirectory); + this.isValidZip(zipDirectory); + const baseDir = this.getBaseDirectory(zipDirectory); - _.each(self.handlers, function (handler) { + for (const handler of this.handlers) { + const files = this.getFilesFromZip(handler, zipDirectory); + + if (files.length > 0) { if (Object.prototype.hasOwnProperty.call(importData, handler.type)) { // This limitation is here to reduce the complexity of the importer for now - return Promise.reject(new errors.UnsupportedMediaTypeError({ + throw new errors.UnsupportedMediaTypeError({ message: tpl(messages.zipContainsMultipleDataFormats) - })); - } - - const files = self.getFilesFromZip(handler, zipDirectory); - - if (files.length > 0) { - ops.push(function () { - return handler.loadFile(files, baseDir).then(function (data) { - importData[handler.type] = data; - }); }); } - }); - if (ops.length === 0) { - return Promise.reject(new errors.UnsupportedMediaTypeError({ - message: tpl(messages.noContentToImport) - })); + const data = await handler.loadFile(files, baseDir); + importData[handler.type] = data; } + } - return sequence(ops).then(function () { - return importData; + if (Object.keys(importData).length === 0) { + throw new errors.UnsupportedMediaTypeError({ + message: tpl(messages.noContentToImport) }); - }); + } + + return importData; } /** @@ -284,7 +284,7 @@ class ImportManager { * The data key contains JSON representing any data that should be imported * The image key contains references to images that will be stored (and where they will be stored) * @param {File} file - * @returns {Promise(ImportData)} + * @returns {Promise} */ processFile(file, ext) { const fileHandler = _.find(this.handlers, function (handler) { @@ -304,7 +304,7 @@ class ImportManager { * Load the given file into usable importData in the format: {data: {}, images: []}, regardless of * whether the file is a single importable file like a JSON file, or a zip file containing loads of files. * @param {File} file - * @returns {Promise} + * @returns {Promise} */ loadFile(file) { const self = this; @@ -317,17 +317,14 @@ class ImportManager { * Pass the prepared importData through the preProcess function of the various importers, so that the importers can * make any adjustments to the data based on relationships between it * @param {ImportData} importData - * @returns {Promise(ImportData)} + * @returns {Promise} */ - preProcess(importData) { - const ops = []; - _.each(this.importers, function (importer) { - ops.push(function () { - return importer.preProcess(importData); - }); - }); + async preProcess(importData) { + for (const importer of this.importers) { + importData = importer.preProcess(importData); + } - return pipeline(ops); + return Promise.resolve(importData); } /** @@ -335,65 +332,116 @@ class ImportManager { * Each importer gets passed the data from importData which has the key matching its type - i.e. it only gets the * data that it should import. Each importer then handles actually importing that data into Ghost * @param {ImportData} importData - * @param {Object} importOptions to allow override of certain import features such as locking a user - * @returns {Promise} + * @param {ImportOptions} [importOptions] to allow override of certain import features such as locking a user + * @returns {Promise} importResults */ - doImport(importData, importOptions) { + async doImport(importData, importOptions) { importOptions = importOptions || {}; - const ops = []; - _.each(this.importers, function (importer) { - if (Object.prototype.hasOwnProperty.call(importData, importer.type)) { - ops.push(function () { - return importer.doImport(importData[importer.type], importOptions); - }); - } - }); + const importResults = []; - return sequence(ops).then(function (importResult) { - return importResult; - }); + for (const importer of this.importers) { + if (Object.prototype.hasOwnProperty.call(importData, importer.type)) { + importResults.push(await importer.doImport(importData[importer.type], importOptions)); + } + } + + return importResults; } /** * Import Step 4: * Report on what was imported, currently a no-op - * @param {ImportData} importData - * @returns {Promise} + * @param {ImportResult[]} importResults + * @returns {Promise} importResults */ - generateReport(importData) { - return Promise.resolve(importData); + async generateReport(importResults) { + return Promise.resolve(importResults); } /** * Import From File * The main method of the ImportManager, call this to kick everything off! * @param {File} file - * @param {Object} importOptions to allow override of certain import features such as locking a user - * @returns {Promise} + * @param {ImportOptions} importOptions to allow override of certain import features such as locking a user + * @returns {Promise} */ - importFromFile(file, importOptions = {}) { - const self = this; + async importFromFile(file, importOptions = {}) { + try { + // Step 1: Handle converting the file to usable data + let importData = await this.loadFile(file); - // Step 1: Handle converting the file to usable data - return this.loadFile(file).then(function (importData) { // Step 2: Let the importers pre-process the data - return self.preProcess(importData); - }).then(function (importData) { + importData = await this.preProcess(importData); + // Step 3: Actually do the import // @TODO: It would be cool to have some sort of dry run flag here - return self.doImport(importData, importOptions); - }).then(function (importData) { + let importResult = await this.doImport(importData, importOptions); + // Step 4: Report on the import - return self.generateReport(importData); - }).finally(() => self.cleanUp()); // Step 5: Cleanup any files + return await this.generateReport(importResult); + } finally { + // Step 5: Cleanup any files + this.cleanUp(); + } } } /** - * A number, or a string containing a number. - * @typedef {Object} ImportData - * @property [Object] data - * @property [Array] images + * @typedef {object} ImportOptions + * @property {boolean} [returnImportedData] + * @property {boolean} [importPersistUser] */ +/** + * @typedef {object} Importer + * @property {"images"|"data"} type + * @property {PreProcessMethod} preProcess + * @property {DoImportMethod} doImport + */ + +/** + * @callback PreProcessMethod + * @param {ImportData} importData + * @returns {ImportData} + */ + +/** + * @callback DoImportMethod + * @param {object|object[]} importData + * @param {ImportOptions} importOptions + * @returns {Promise} import result + */ + +/** + * @typedef {object} Handler + * @property {"images"|"data"} type + * @property {string[]} extensions + * @property {string[]} contentTypes + * @property {string[]} directories + * @property {LoadFileMethod} loadFile + */ + +/** + * @callback LoadFileMethod + * @param {File[]} files + * @param {string} [baseDir] + * @returns {Promise} data + */ + +/** + * File object + * @typedef {Object} File + * @property {string} name + * @property {string} path + */ + +/** + * @typedef {Object} ImportData + * @property {Object} [data] + * @property {Array} [images] + */ + +/** + * @typedef {Object} ImportResult + */ module.exports = new ImportManager(); diff --git a/test/unit/server/data/importer/index.test.js b/test/unit/server/data/importer/index.test.js index db4884920f..c706e9bd27 100644 --- a/test/unit/server/data/importer/index.test.js +++ b/test/unit/server/data/importer/index.test.js @@ -2,11 +2,11 @@ const errors = require('@tryghost/errors'); const should = require('should'); const sinon = require('sinon'); const rewire = require('rewire'); -const Promise = require('bluebird'); const _ = require('lodash'); const testUtils = require('../../../../utils'); const moment = require('moment'); const path = require('path'); +const fs = require('fs-extra'); // Stuff we are testing const ImportManager = require('../../../../../core/server/data/importer'); @@ -46,13 +46,22 @@ describe('Importer', function () { }); it('gets the correct types', function () { - ImportManager.getContentTypes().should.be.instanceof(Array).and.have.lengthOf(13); + ImportManager.getContentTypes().should.be.instanceof(Array).and.have.lengthOf(12); + ImportManager.getContentTypes().should.containEql('image/jpeg'); + ImportManager.getContentTypes().should.containEql('image/png'); + ImportManager.getContentTypes().should.containEql('image/gif'); + ImportManager.getContentTypes().should.containEql('image/svg+xml'); + ImportManager.getContentTypes().should.containEql('image/x-icon'); + ImportManager.getContentTypes().should.containEql('image/vnd.microsoft.icon'); + ImportManager.getContentTypes().should.containEql('image/webp'); + ImportManager.getContentTypes().should.containEql('application/octet-stream'); ImportManager.getContentTypes().should.containEql('application/json'); + + ImportManager.getContentTypes().should.containEql('text/plain'); + ImportManager.getContentTypes().should.containEql('application/zip'); ImportManager.getContentTypes().should.containEql('application/x-zip-compressed'); - ImportManager.getContentTypes().should.containEql('text/plain'); - ImportManager.getContentTypes().should.containEql('image/webp'); }); it('gets the correct directories', function () { @@ -88,12 +97,40 @@ describe('Importer', function () { .should.equal('**/+(images|content)'); }); + it('cleans up', async function () { + const file = path.resolve('test/utils/fixtures/import/zips/zip-with-base-dir'); + ImportManager.fileToDelete = file; + const removeStub = sinon.stub(fs, 'remove').withArgs(file).returns(Promise.resolve()); + + await ImportManager.cleanUp(); + removeStub.calledOnce.should.be.true(); + should(ImportManager.fileToDelete).be.null(); + }); + + it('doesn\'t clean up', async function () { + ImportManager.fileToDelete = null; + const removeStub = sinon.stub(fs, 'remove').returns(Promise.resolve()); + + await ImportManager.cleanUp(); + removeStub.called.should.be.false(); + }); + + it('silently ignores clean up errors', async function () { + const file = path.resolve('test/utils/fixtures/import/zips/zip-with-base-dir'); + ImportManager.fileToDelete = file; + const removeStub = sinon.stub(fs, 'remove').withArgs(file).returns(Promise.reject(new Error('Unknown file'))); + + await ImportManager.cleanUp(); + removeStub.calledOnce.should.be.true(); + should(ImportManager.fileToDelete).be.null(); + }); + // Step 1 of importing is loadFile describe('loadFile', function () { it('knows when to process a file', function (done) { const testFile = {name: 'myFile.json', path: '/my/path/myFile.json'}; - const zipSpy = sinon.stub(ImportManager, 'processZip').returns(Promise.resolve()); - const fileSpy = sinon.stub(ImportManager, 'processFile').returns(Promise.resolve()); + const zipSpy = sinon.stub(ImportManager, 'processZip').returns(Promise.resolve({})); + const fileSpy = sinon.stub(ImportManager, 'processFile').returns(Promise.resolve({})); ImportManager.loadFile(testFile).then(function () { zipSpy.calledOnce.should.be.false(); @@ -105,8 +142,8 @@ describe('Importer', function () { // We need to make sure we don't actually extract a zip and leave temporary files everywhere! it('knows when to process a zip', function (done) { const testZip = {name: 'myFile.zip', path: '/my/path/myFile.zip'}; - const zipSpy = sinon.stub(ImportManager, 'processZip').returns(Promise.resolve()); - const fileSpy = sinon.stub(ImportManager, 'processFile').returns(Promise.resolve()); + const zipSpy = sinon.stub(ImportManager, 'processZip').returns(Promise.resolve({})); + const fileSpy = sinon.stub(ImportManager, 'processFile').returns(Promise.resolve({})); ImportManager.loadFile(testZip).then(function () { zipSpy.calledOnce.should.be.true(); @@ -123,14 +160,14 @@ describe('Importer', function () { const extractSpy = sinon.stub(ImportManager, 'extractZip').returns(Promise.resolve('/tmp/dir/')); const validSpy = sinon.stub(ImportManager, 'isValidZip').returns(true); - const baseDirSpy = sinon.stub(ImportManager, 'getBaseDirectory').returns(); + const baseDirSpy = sinon.stub(ImportManager, 'getBaseDirectory').returns(''); const getFileSpy = sinon.stub(ImportManager, 'getFilesFromZip'); const jsonSpy = sinon.stub(JSONHandler, 'loadFile').returns(Promise.resolve({posts: []})); const imageSpy = sinon.stub(ImageHandler, 'loadFile'); const mdSpy = sinon.stub(MarkdownHandler, 'loadFile'); getFileSpy.returns([]); - getFileSpy.withArgs(JSONHandler).returns(['/tmp/dir/myFile.json']); + getFileSpy.withArgs(JSONHandler, sinon.match.string).returns([{path: '/tmp/dir/myFile.json', name: 'myFile.json'}]); ImportManager.processZip(testZip).then(function (zipResult) { extractSpy.calledOnce.should.be.true(); @@ -172,6 +209,12 @@ describe('Importer', function () { ImportManager.isValidZip(testDir).should.be.ok(); }); + it('accepts a zip with uppercase image extensions', function () { + const testDir = path.resolve('test/utils/fixtures/import/zips/zip-uppercase-extensions'); + + ImportManager.isValidZip(testDir).should.be.ok(); + }); + it('fails a zip with two base directories', function () { const testDir = path.resolve('test/utils/fixtures/import/zips/zip-with-double-base-dir'); @@ -185,6 +228,72 @@ describe('Importer', function () { }); }); + describe('Process Zip', function () { + const testZip = {name: 'myFile.zip', path: '/my/path/myFile.zip'}; + + this.beforeEach(() => { + sinon.stub(JSONHandler, 'loadFile').returns(Promise.resolve({posts: []})); + sinon.stub(ImageHandler, 'loadFile'); + sinon.stub(MarkdownHandler, 'loadFile'); + }); + + it('accepts a zip with a base directory', async function () { + const testDir = path.resolve('test/utils/fixtures/import/zips/zip-with-base-dir'); + const extractSpy = sinon.stub(ImportManager, 'extractZip').returns(Promise.resolve(testDir)); + + const zipResult = await ImportManager.processZip(testZip); + zipResult.data.should.not.be.undefined(); + should(zipResult.images).be.undefined(); + extractSpy.calledOnce.should.be.true(); + }); + + it('accepts a zip without a base directory', async function () { + const testDir = path.resolve('test/utils/fixtures/import/zips/zip-without-base-dir'); + const extractSpy = sinon.stub(ImportManager, 'extractZip').returns(Promise.resolve(testDir)); + + const zipResult = await ImportManager.processZip(testZip); + zipResult.data.should.not.be.undefined(); + should(zipResult.images).be.undefined(); + extractSpy.calledOnce.should.be.true(); + }); + + it('accepts a zip with an image directory', async function () { + const testDir = path.resolve('test/utils/fixtures/import/zips/zip-image-dir'); + const extractSpy = sinon.stub(ImportManager, 'extractZip').returns(Promise.resolve(testDir)); + + const zipResult = await ImportManager.processZip(testZip); + zipResult.images.length.should.eql(1); + should(zipResult.data).be.undefined(); + extractSpy.calledOnce.should.be.true(); + }); + + it('accepts a zip with uppercase image extensions', async function () { + const testDir = path.resolve('test/utils/fixtures/import/zips/zip-uppercase-extensions'); + const extractSpy = sinon.stub(ImportManager, 'extractZip').returns(Promise.resolve(testDir)); + + const zipResult = await ImportManager.processZip(testZip); + zipResult.images.length.should.eql(1); + should(zipResult.data).be.undefined(); + extractSpy.calledOnce.should.be.true(); + }); + + it('throws zipContainsMultipleDataFormats', async function () { + const testDir = path.resolve('test/utils/fixtures/import/zips/zip-multiple-data-formats'); + const extractSpy = sinon.stub(ImportManager, 'extractZip').returns(Promise.resolve(testDir)); + + await should(ImportManager.processZip(testZip)).rejectedWith(/multiple data formats/); + extractSpy.calledOnce.should.be.true(); + }); + + it('throws noContentToImport', async function () { + const testDir = path.resolve('test/utils/fixtures/import/zips/zip-empty'); + const extractSpy = sinon.stub(ImportManager, 'extractZip').returns(Promise.resolve(testDir)); + + await should(ImportManager.processZip(testZip)).rejectedWith(/not include any content/); + extractSpy.calledOnce.should.be.true(); + }); + }); + describe('Get Base Dir', function () { it('returns string for base directory', function () { const testDir = path.resolve('test/utils/fixtures/import/zips/zip-with-base-dir'); @@ -192,11 +301,29 @@ describe('Importer', function () { ImportManager.getBaseDirectory(testDir).should.equal('basedir'); }); + it('returns string for double base directory', function () { + const testDir = path.resolve('test/utils/fixtures/import/zips/zip-with-double-base-dir'); + + ImportManager.getBaseDirectory(testDir).should.equal('basedir'); + }); + it('returns empty for no base directory', function () { const testDir = path.resolve('test/utils/fixtures/import/zips/zip-without-base-dir'); should.not.exist(ImportManager.getBaseDirectory(testDir)); }); + + it('returns empty for content handler directories', function () { + const testDir = path.resolve('test/utils/fixtures/import/zips/zip-image-dir'); + + should.not.exist(ImportManager.getBaseDirectory(testDir)); + }); + + it('throws invalidZipFileBaseDirectory', function () { + const testDir = path.resolve('test/utils/fixtures/import/zips/zip-empty'); + + should(() => ImportManager.getBaseDirectory(testDir)).throwError(/invalid zip file/i); + }); }); describe('Zip behaviour', function () { @@ -285,7 +412,7 @@ describe('Importer', function () { // generateReport is intended to create a message to show to the user about what has been imported // it is currently a noop it('is currently a noop', function (done) { - const input = {data: {}, images: []}; + const input = [{data: {}, images: []}]; ImportManager.generateReport(input).then(function (output) { output.should.equal(input); done(); @@ -295,13 +422,13 @@ describe('Importer', function () { describe('importFromFile', function () { it('does the import steps in order', function (done) { - const loadFileSpy = sinon.stub(ImportManager, 'loadFile').returns(Promise.resolve()); - const preProcessSpy = sinon.stub(ImportManager, 'preProcess').returns(Promise.resolve()); - const doImportSpy = sinon.stub(ImportManager, 'doImport').returns(Promise.resolve()); - const generateReportSpy = sinon.stub(ImportManager, 'generateReport').returns(Promise.resolve()); - const cleanupSpy = sinon.stub(ImportManager, 'cleanUp').returns({}); + const loadFileSpy = sinon.stub(ImportManager, 'loadFile').returns(Promise.resolve({})); + const preProcessSpy = sinon.stub(ImportManager, 'preProcess').returns(Promise.resolve({})); + const doImportSpy = sinon.stub(ImportManager, 'doImport').returns(Promise.resolve([])); + const generateReportSpy = sinon.spy(ImportManager, 'generateReport'); + const cleanupSpy = sinon.stub(ImportManager, 'cleanUp').returns(Promise.resolve()); - ImportManager.importFromFile({}).then(function () { + ImportManager.importFromFile({name: 'test.json', path: '/test.json'}).then(function () { loadFileSpy.calledOnce.should.be.true(); preProcessSpy.calledOnce.should.be.true(); doImportSpy.calledOnce.should.be.true(); diff --git a/test/utils/fixtures/import/zips/zip-multiple-data-formats/test.json b/test/utils/fixtures/import/zips/zip-multiple-data-formats/test.json new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/utils/fixtures/import/zips/zip-multiple-data-formats/test.md b/test/utils/fixtures/import/zips/zip-multiple-data-formats/test.md new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/utils/fixtures/import/zips/zip-uppercase-extensions/image.JPG b/test/utils/fixtures/import/zips/zip-uppercase-extensions/image.JPG new file mode 100644 index 0000000000..e69de29bb2