Image Importer Improvements

core/server/data/import/index.js

@@ -9,7 +9,8 @@ var Promise         = require('bluebird'),
     handleErrors,
     checkDuplicateAttributes,
     sanitize,
-    cleanError;
+    cleanError,
+    doImport;
 
 cleanError = function cleanError(error) {
     var temp,
@@ -184,7 +185,7 @@ validate = function validate(data) {
     });
 };
 
-module.exports = function (data) {
+doImport = function (data) {
     var sanitizeResults = sanitize(data);
 
     data = sanitizeResults.data;
@@ -197,3 +198,5 @@ module.exports = function (data) {
         return handleErrors(result);
     });
 };
+
+module.exports.doImport = doImport;

core/server/data/importer/handlers/image.js

@@ -10,24 +10,25 @@ ImageHandler = {
     type: 'images',
     extensions: config.uploads.extensions,
     types: config.uploads.contentTypes,
+    directories: ['images', 'content'],
 
-    loadFile: function (files, startDir) {
+    loadFile: function (files, baseDir) {
         var store = storage.getStorage(),
-            startDirRegex = startDir ? new RegExp('^' + startDir + '/') : new RegExp(''),
+            baseDirRegex = baseDir ? new RegExp('^' + baseDir + '/') : new RegExp(''),
             imageFolderRegexes = _.map(config.paths.imagesRelPath.split('/'), function (dir) {
                 return new RegExp('^' + dir + '/');
             });
 
         // normalize the directory structure
         files = _.map(files, function (file) {
-            var noStartDir = file.name.replace(startDirRegex, ''),
-                noGhostDirs = noStartDir;
+            var noBaseDir = file.name.replace(baseDirRegex, ''),
+                noGhostDirs = noBaseDir;
 
             _.each(imageFolderRegexes, function (regex) {
                 noGhostDirs = noGhostDirs.replace(regex, '');
             });
 
-            file.originalPath = noStartDir;
+            file.originalPath = noBaseDir;
             file.name = noGhostDirs;
             file.targetDir = path.join(config.paths.imagesPath, path.dirname(noGhostDirs));
             return file;

core/server/data/importer/handlers/json.js

@@ -8,6 +8,7 @@ JSONHandler = {
     type: 'data',
     extensions: ['.json'],
     types: ['application/octet-stream', 'application/json'],
+    directories: [],
 
     loadFile: function (files, startDir) {
         /*jshint unused:false */

core/server/data/importer/importers/data.js

@@ -8,7 +8,7 @@ DataImporter = {
         return importData;
     },
     doImport: function (importData) {
-        return importer(importData);
+        return importer.doImport(importData);
     }
 };
 

core/server/data/importer/index.js

@@ -14,16 +14,24 @@ var _            = require('lodash'),
     ImageImporter = require('./importers/image'),
     DataImporter  = require('./importers/data'),
 
+    // Glob levels
+    ROOT_ONLY = 0,
+    ROOT_OR_SINGLE_DIR = 1,
+    ALL_DIRS = 2,
+
     defaults;
 
 defaults = {
     extensions: ['.zip'],
-    types: ['application/zip', 'application/x-zip-compressed']
+    types: ['application/zip', 'application/x-zip-compressed'],
+    directories: []
 };
 
 function ImportManager() {
     this.importers = [ImageImporter, DataImporter];
     this.handlers = [ImageHandler, JSONHandler];
+    // Keep track of files to cleanup at the end
+    this.filesToDelete = [];
 }
 
 /**
@@ -36,40 +44,73 @@ function ImportManager() {
 _.extend(ImportManager.prototype, {
     /**
      * Get an array of all the file extensions for which we have handlers
-     * @returns []
+     * @returns {string[]}
      */
     getExtensions: function () {
         return _.flatten(_.union(_.pluck(this.handlers, 'extensions'), defaults.extensions));
     },
     /**
      * Get an array of all the mime types for which we have handlers
-     * @returns []
+     * @returns {string[]}
      */
     getTypes: function () {
         return _.flatten(_.union(_.pluck(this.handlers, 'types'), defaults.types));
     },
     /**
-     * Convert the extensions supported by a given handler into a glob string
-     * @returns String
+     * Get an array of directories for which we have handlers
+     * @returns {string[]}
      */
-    getGlobPattern: function (handler) {
-        return '**/*+(' + _.reduce(handler.extensions, function (memo, ext) {
+    getDirectories: function () {
+        return _.flatten(_.union(_.pluck(this.handlers, 'directories'), defaults.directories));
+    },
+    /**
+     * Convert items into a glob string
+     * @param {String[]} items
+     * @returns {String}
+     */
+    getGlobPattern: function (items) {
+        return '+(' + _.reduce(items, function (memo, ext) {
             return memo !== '' ? memo + '|'  + ext : ext;
         }, '') + ')';
     },
     /**
-     * Remove a file after we're done (abstracted into a function for easier testing)
-     * @param {File} file
+     * @param {String[]} extensions
+     * @param {Number} level
+     * @returns {String}
+     */
+    getExtensionGlob: function (extensions, level) {
+        var prefix = level === ALL_DIRS ? '**/*' :
+            (level === ROOT_OR_SINGLE_DIR ? '{*/*,*}' : '*');
+
+        return prefix + this.getGlobPattern(extensions);
+    },
+    /**
+     *
+     * @param {String[]} directories
+     * @param {Number} level
+     * @returns {String}
+     */
+    getDirectoryGlob: function (directories, level) {
+        var prefix = level === ALL_DIRS ? '**/' :
+            (level === ROOT_OR_SINGLE_DIR ? '{*/,}' : '');
+
+        return prefix + this.getGlobPattern(directories);
+    },
+    /**
+     * Remove files after we're done (abstracted into a function for easier testing)
      * @returns {Function}
      */
-    cleanUp: function (file) {
-        var fileToDelete = file;
+    cleanUp: function () {
+        var filesToDelete = this.filesToDelete;
         return function (result) {
-            try {
-                fs.remove(fileToDelete);
-            } catch (err) {
-                errors.logError(err, 'Import could not clean up file', 'You blog will continue to work as expected');
-            }
+            _.each(filesToDelete, function (fileToDelete) {
+                fs.remove(fileToDelete, function (err) {
+                    if (err) {
+                        errors.logError(err, 'Import could not clean up file ', 'Your blog will continue to work as expected');
+                    }
+                });
+            });
+
             return result;
         };
     },
@@ -80,13 +121,45 @@ _.extend(ImportManager.prototype, {
     isZip: function (ext) {
         return _.contains(defaults.extensions, ext);
     },
+    /**
+     * Checks the content of a zip folder to see if it is valid.
+     * Importable content includes any files or directories which the handlers can process
+     * Importable content must be found either in the root, or inside one base directory
+     *
+     * @param {String} directory
+     * @returns {Promise}
+     */
+    isValidZip: function (directory) {
+        // Globs match content in the root or inside a single directory
+        var extMatchesBase = glob.sync(
+                this.getExtensionGlob(this.getExtensions(), ROOT_OR_SINGLE_DIR), {cwd: directory}
+            ),
+            extMatchesAll = glob.sync(
+                this.getExtensionGlob(this.getExtensions(), ALL_DIRS), {cwd: directory}
+            ),
+            dirMatches = glob.sync(
+                this.getDirectoryGlob(this.getDirectories(), ROOT_OR_SINGLE_DIR), {cwd: directory}
+            );
+
+        // If this folder contains importable files or a content or images directory
+        if (extMatchesBase.length > 0 || (dirMatches.length > 0 && extMatchesAll.length > 0)) {
+            return true;
+        }
+
+        if (extMatchesAll.length < 1) {
+            throw new errors.UnsupportedMediaTypeError('Zip did not include any content to import.');
+        }
+
+        throw new errors.UnsupportedMediaTypeError('Invalid zip file structure.');
+    },
     /**
      * 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
      */
     extractZip: function (filePath) {
         var tmpDir = path.join(os.tmpdir(), uuid.v4());
+        this.filesToDelete.push(tmpDir);
         return Promise.promisify(extract)(filePath, {dir: tmpDir}).then(function () {
             return tmpDir;
         });
@@ -99,11 +172,36 @@ _.extend(ImportManager.prototype, {
      * @returns [] Files
      */
     getFilesFromZip: function (handler, directory) {
-        var globPattern = this.getGlobPattern(handler);
+        var globPattern = this.getExtensionGlob(handler.extensions, ALL_DIRS);
         return _.map(glob.sync(globPattern, {cwd: directory}), function (file) {
             return {name: file, path: path.join(directory, file)};
         });
     },
+    /**
+     * Get the name of the single base directory if there is one, else return an empty string
+     * @param {String} directory
+     * @returns {Promise (String)}
+     */
+    getBaseDirectory: function (directory) {
+        // Globs match root level only
+        var extMatches = glob.sync(this.getExtensionGlob(this.getExtensions(), ROOT_ONLY), {cwd: directory}),
+            dirMatches = glob.sync(this.getDirectoryGlob(this.getDirectories(), ROOT_ONLY), {cwd: directory}),
+            extMatchesAll;
+
+        // There is no base directory
+        if (extMatches.length > 0 || dirMatches.length > 0) {
+            return;
+        }
+        // There is a base directory, grab it from any ext match
+        extMatchesAll = glob.sync(
+            this.getExtensionGlob(this.getExtensions(), ALL_DIRS), {cwd: directory}
+        );
+        if (extMatchesAll.length < 1 || extMatchesAll[0].split('/') < 1) {
+            throw new errors.ValidationError('Invalid zip file: base directory read failed');
+        }
+
+        return extMatchesAll[0].split('/')[0];
+    },
     /**
      * Process Zip
      * Takes a reference to a zip file, extracts it, sends any relevant files from inside to the right handler, and
@@ -115,26 +213,28 @@ _.extend(ImportManager.prototype, {
      */
     processZip: function (file) {
         var self = this;
-        return this.extractZip(file.path).then(function (directory) {
+
+        return this.extractZip(file.path).then(function (zipDirectory) {
             var ops = [],
                 importData = {},
-                startDir = glob.sync(file.name.replace('.zip', ''), {cwd: directory});
+                baseDir;
 
-            startDir = startDir[0] || false;
+            self.isValidZip(zipDirectory);
+            baseDir = self.getBaseDirectory(zipDirectory);
 
             _.each(self.handlers, function (handler) {
                 if (importData.hasOwnProperty(handler.type)) {
                     // This limitation is here to reduce the complexity of the importer for now
                     return Promise.reject(new errors.UnsupportedMediaTypeError(
-                        'Zip file contains too many types of import data. Please split it up and import separately.'
+                        'Zip file contains multiple data formats. Please split up and import separately.'
                     ));
                 }
 
-                var files = self.getFilesFromZip(handler, directory);
+                var files = self.getFilesFromZip(handler, zipDirectory);
 
                 if (files.length > 0) {
                     ops.push(function () {
-                        return handler.loadFile(files, startDir).then(function (data) {
+                        return handler.loadFile(files, baseDir).then(function (data) {
                             importData[handler.type] = data;
                         });
                     });
@@ -149,7 +249,7 @@ _.extend(ImportManager.prototype, {
 
             return sequence(ops).then(function () {
                 return importData;
-            }).finally(self.cleanUp(directory));
+            });
         });
     },
     /**
@@ -184,15 +284,9 @@ _.extend(ImportManager.prototype, {
         var self = this,
             ext = path.extname(file.name).toLowerCase();
 
-        return Promise.resolve(this.isZip(ext)).then(function (isZip) {
-            if (isZip) {
-                // If it's a zip, process the zip file
-                return self.processZip(file);
-            } else {
-                // Else process the file
-                return self.processFile(file, ext);
-            }
-        }).finally(self.cleanUp(file.path));
+        this.filesToDelete.push(file.path);
+
+        return this.isZip(ext) ? self.processZip(file) : self.processFile(file, ext);
     },
     /**
      * Import Step 2:
@@ -245,7 +339,7 @@ _.extend(ImportManager.prototype, {
      * Import From File
      * The main method of the ImportManager, call this to kick everything off!
      * @param {File} file
-     * @returns {*}
+     * @returns {Promise}
      */
     importFromFile: function (file) {
         var self = this;
@@ -259,8 +353,10 @@ _.extend(ImportManager.prototype, {
             // @TODO: It would be cool to have some sort of dry run flag here
             return self.doImport(importData);
         }).then(function (importData) {
-            // Step 4: Finally, report on the import
-            return self.generateReport(importData);
+            // Step 4: Report on the import
+            return self.generateReport(importData)
+                // Step 5: Cleanup any files
+                .finally(self.cleanUp());
         });
     }
 });