From 05877124ae1b7660cf73db7edc5918266bed9ddb Mon Sep 17 00:00:00 2001 From: Jason Williams Date: Sat, 3 Jan 2015 21:11:40 +0000 Subject: [PATCH] Remove unneeded promises and fix tests --- core/server/data/importer/index.js | 42 +++++-------- core/test/unit/importer_spec.js | 95 ++++++++++++------------------ 2 files changed, 54 insertions(+), 83 deletions(-) diff --git a/core/server/data/importer/index.js b/core/server/data/importer/index.js index 8075522e76..02bbb4e685 100644 --- a/core/server/data/importer/index.js +++ b/core/server/data/importer/index.js @@ -143,16 +143,14 @@ _.extend(ImportManager.prototype, { // If this folder contains importable files or a content or images directory if (extMatchesBase.length > 0 || (dirMatches.length > 0 && extMatchesAll.length > 0)) { - return Promise.resolve(true); + return true; } if (extMatchesAll.length < 1) { - return Promise.reject(new errors.UnsupportedMediaTypeError( - 'Zip did not include any content to import.' - )); + throw new errors.UnsupportedMediaTypeError('Zip did not include any content to import.'); } - return Promise.reject(new errors.UnsupportedMediaTypeError('Invalid zip file structure.')); + 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 @@ -192,18 +190,17 @@ _.extend(ImportManager.prototype, { // There is no base directory if (extMatches.length > 0 || dirMatches.length > 0) { - return Promise.resolve(); + 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) { - return Promise.resolve(new errors.ValidationError('Invalid zip file: base directory read failed')); + throw new errors.ValidationError('Invalid zip file: base directory read failed'); } - return Promise.resolve(extMatchesAll[0].split('/')[0]); + return extMatchesAll[0].split('/')[0]; }, /** * Process Zip @@ -215,16 +212,15 @@ _.extend(ImportManager.prototype, { * @returns {Promise(ImportData)} */ processZip: function (file) { - var self = this, - directory; + var self = this; + return this.extractZip(file.path).then(function (zipDirectory) { - directory = zipDirectory; - return self.isValidZip(directory); - }).then(function () { - return self.getBaseDirectory(directory); - }).then(function (baseDir) { var ops = [], - importData = {}; + importData = {}, + baseDir; + + self.isValidZip(zipDirectory); + baseDir = self.getBaseDirectory(zipDirectory); _.each(self.handlers, function (handler) { if (importData.hasOwnProperty(handler.type)) { @@ -234,7 +230,7 @@ _.extend(ImportManager.prototype, { )); } - var files = self.getFilesFromZip(handler, directory); + var files = self.getFilesFromZip(handler, zipDirectory); if (files.length > 0) { ops.push(function () { @@ -290,15 +286,7 @@ _.extend(ImportManager.prototype, { this.filesToDelete.push(file.path); - 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); - } - }); + return this.isZip(ext) ? self.processZip(file) : self.processFile(file, ext); }, /** * Import Step 2: diff --git a/core/test/unit/importer_spec.js b/core/test/unit/importer_spec.js index bb703d7b06..d388cb480e 100644 --- a/core/test/unit/importer_spec.js +++ b/core/test/unit/importer_spec.js @@ -7,6 +7,7 @@ var should = require('should'), testUtils = require('../utils'), config = require('../../server/config'), path = require('path'), + errors = require('../../server/errors'), // Stuff we are testing ImportManager = require('../../server/data/importer'), @@ -83,7 +84,7 @@ describe('Importer', function () { zipSpy.calledOnce.should.be.false; fileSpy.calledOnce.should.be.true; done(); - }); + }).catch(done); }); // We need to make sure we don't actually extract a zip and leave temporary files everywhere! @@ -96,7 +97,7 @@ describe('Importer', function () { zipSpy.calledOnce.should.be.true; fileSpy.calledOnce.should.be.false; done(); - }); + }).catch(done); }); it('has same result for zips and files', function (done) { @@ -104,7 +105,8 @@ describe('Importer', function () { testZip = {name: 'myFile.zip', path: '/my/path/myFile.zip'}, // need to stub out the extract and glob function for zip extractSpy = sandbox.stub(ImportManager, 'extractZip').returns(Promise.resolve('/tmp/dir/')), - validSpy = sandbox.stub(ImportManager, 'isValidZip').returns(Promise.resolve()), + validSpy = sandbox.stub(ImportManager, 'isValidZip').returns(true), + baseDirSpy = sandbox.stub(ImportManager, 'getBaseDirectory').returns(), getFileSpy = sandbox.stub(ImportManager, 'getFilesFromZip'), jsonSpy = sandbox.stub(JSONHandler, 'loadFile').returns(Promise.resolve({posts: []})), imageSpy = sandbox.stub(ImageHandler, 'loadFile'); @@ -115,6 +117,7 @@ describe('Importer', function () { ImportManager.processZip(testZip).then(function (zipResult) { extractSpy.calledOnce.should.be.true; validSpy.calledOnce.should.be.true; + baseDirSpy.calledOnce.should.be.true; getFileSpy.calledTwice.should.be.true; jsonSpy.calledOnce.should.be.true; imageSpy.called.should.be.false; @@ -128,72 +131,52 @@ describe('Importer', function () { zipResult.should.eql(fileResult); done(); }); - }); + }).catch(done); }); describe('Validate Zip', function () { - it('accepts a zip with a base directory', function (done) { + it('accepts a zip with a base directory', function () { var testDir = path.resolve('core/test/utils/fixtures/import/zips/zip-with-base-dir'); - ImportManager.isValidZip(testDir).then(function (isValid) { - isValid.should.be.ok; - done(); - }); + + ImportManager.isValidZip(testDir).should.be.ok; }); - it('accepts a zip without a base directory', function (done) { + it('accepts a zip without a base directory', function () { var testDir = path.resolve('core/test/utils/fixtures/import/zips/zip-without-base-dir'); - ImportManager.isValidZip(testDir).then(function (isValid) { - isValid.should.be.ok; - done(); - }); + + ImportManager.isValidZip(testDir).should.be.ok; }); - it('accepts a zip with an image directory', function (done) { + it('accepts a zip with an image directory', function () { var testDir = path.resolve('core/test/utils/fixtures/import/zips/zip-image-dir'); - ImportManager.isValidZip(testDir).then(function (isValid) { - isValid.should.be.ok; - done(); - }); + + ImportManager.isValidZip(testDir).should.be.ok; }); - it('fails a zip with two base directories', function (done) { + it('fails a zip with two base directories', function () { var testDir = path.resolve('core/test/utils/fixtures/import/zips/zip-with-double-base-dir'); - ImportManager.isValidZip(testDir).then(function () { - done(new Error('Double base directory did not throw error')); - }).catch(function (err) { - err.message.should.equal('Invalid zip file structure.'); - err.type.should.equal('UnsupportedMediaTypeError'); - done(); - }); + + ImportManager.isValidZip.bind(ImportManager, testDir).should.throw(errors.UnsupportedMediaTypeError); }); - it('fails a zip with no content', function (done) { + it('fails a zip with no content', function () { var testDir = path.resolve('core/test/utils/fixtures/import/zips/zip-invalid'); - ImportManager.isValidZip(testDir).then(function () { - done(new Error('Double base directory did not throw error')); - }).catch(function (err) { - err.message.should.equal('Zip did not include any content to import.'); - err.type.should.equal('UnsupportedMediaTypeError'); - done(); - }); + + ImportManager.isValidZip.bind(ImportManager, testDir).should.throw(errors.UnsupportedMediaTypeError); }); }); describe('Get Base Dir', function () { - it('returns string for base directory', function (done) { + it('returns string for base directory', function () { var testDir = path.resolve('core/test/utils/fixtures/import/zips/zip-with-base-dir'); - ImportManager.getBaseDirectory(testDir).then(function (baseDir) { - baseDir.should.equal('basedir'); - done(); - }); + + ImportManager.getBaseDirectory(testDir).should.equal('basedir'); }); - it('returns empty for no base directory', function (done) { + it('returns empty for no base directory', function () { var testDir = path.resolve('core/test/utils/fixtures/import/zips/zip-without-base-dir'); - ImportManager.getBaseDirectory(testDir).then(function (baseDir) { - should.not.exist(baseDir); - done(); - }); + + should.not.exist(ImportManager.getBaseDirectory(testDir)); }); }); }); @@ -219,7 +202,7 @@ describe('Importer', function () { output.should.have.property('preProcessedByData', true); output.should.have.property('preProcessedByImage', true); done(); - }); + }).catch(done); }); }); @@ -253,7 +236,7 @@ describe('Importer', function () { // we stubbed this as a noop but ImportManager calls with sequence, so we should get an array output.should.eql([expectedImages, expectedData]); done(); - }); + }).catch(done); }); }); @@ -266,7 +249,7 @@ describe('Importer', function () { ImportManager.generateReport(input).then(function (output) { output.should.equal(input); done(); - }); + }).catch(done); }); }); @@ -287,7 +270,7 @@ describe('Importer', function () { sinon.assert.callOrder(loadFileSpy, preProcessSpy, doImportSpy, generateReportSpy, cleanupSpy); done(); - }); + }).catch(done); }); }); }); @@ -312,7 +295,7 @@ describe('Importer', function () { _.keys(result).should.containEql('meta'); _.keys(result).should.containEql('data'); done(); - }); + }).catch(done); }); it('correctly errors when given a bad db api wrapper', function (done) { @@ -326,7 +309,7 @@ describe('Importer', function () { }).catch(function (response) { response.type.should.equal('BadRequestError'); done(); - }); + }).catch(done); }); }); @@ -372,7 +355,7 @@ describe('Importer', function () { storeSpy.firstCall.args[1].newPath.should.eql('/content/images/test-image.jpeg'); done(); - }); + }).catch(done); }); it('can load a single file, maintaining structure', function (done) { @@ -392,7 +375,7 @@ describe('Importer', function () { storeSpy.firstCall.args[1].newPath.should.eql('/content/images/photos/my-cat.jpeg'); done(); - }); + }).catch(done); }); it('can load a single file, removing ghost dirs', function (done) { @@ -412,7 +395,7 @@ describe('Importer', function () { storeSpy.firstCall.args[1].newPath.should.eql('/content/images/my-cat.jpeg'); done(); - }); + }).catch(done); }); it('can load a file (subdirectory)', function (done) { @@ -434,7 +417,7 @@ describe('Importer', function () { storeSpy.firstCall.args[1].newPath.should.eql('/subdir/content/images/test-image.jpeg'); done(); - }); + }).catch(done); }); it('can load multiple files', function (done) { @@ -474,7 +457,7 @@ describe('Importer', function () { storeSpy.lastCall.args[1].newPath.should.eql('/content/images/puppy.jpg'); done(); - }); + }).catch(done); }); });