Remove unneeded promises and fix tests

This commit is contained in:
Jason Williams 2015-01-03 21:11:40 +00:00 committed by Hannah Wolfe
parent add4c6b078
commit 05877124ae
2 changed files with 54 additions and 83 deletions

View File

@ -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:

View File

@ -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);
});
});