Move image upload to API

closes #3252
- added `/ghost/api/v0.1/uploads/` endpoint
- removed upload method from `controller/admin.js`
- moved removal of temporary files from storage to endpoint (needed to
account for failed uploads)
- changed and moved tests
- Oversight: I think that we use `.otherwise()` and `.catch()` a bit
too extensive and mask the real error objects. We probably need an
error handling strategy at some point in the future.
This commit is contained in:
Sebastian Gierlinger 2014-07-15 12:40:14 +02:00
parent 516fd2680d
commit 2957b0175e
11 changed files with 223 additions and 192 deletions

View File

@ -63,7 +63,7 @@ UploadUi = function ($dropzone, settings) {
var self = this;
$dropzone.find('.js-fileupload').fileupload().fileupload('option', {
url: Ghost.subdir + '/ghost/upload/',
url: Ghost.subdir + '/ghost/api/v0.1/uploads/',
add: function (e, data) {
/*jshint unused:false*/
$('.js-button-accept').prop('disabled', true);

View File

@ -7,6 +7,7 @@ var dataExport = require('../data/export'),
when = require('when'),
nodefn = require('when/node'),
_ = require('lodash'),
path = require('path'),
validation = require('../data/validation'),
errors = require('../../server/errors'),
canThis = require('../permissions').canThis,
@ -15,6 +16,13 @@ var dataExport = require('../data/export'),
api.settings = require('./settings');
function isValidFile(type, ext) {
if (type === 'application/json' && ext === '.json') {
return true;
}
return false;
}
/**
* ## DB API Methods
*
@ -53,31 +61,36 @@ db = {
*/
'importContent': function (options) {
options = options || {};
var databaseVersion;
var databaseVersion,
type,
ext,
filepath;
return canThis(options.context).importContent.db().then(function () {
if (!options.importfile || !options.importfile.path || options.importfile.name.indexOf('json') === -1) {
/**
* Notify of an error if it occurs
*
* - If there's no file (although if you don't select anything, the input is still submitted, so
* !req.files.importfile will always be false)
* - If there is no path
* - If the name doesn't have json in it
*/
return when.reject(new errors.UnsupportedMediaTypeError('Please select a .json file to import.'));
if (!options.importfile || !options.importfile.type || !options.importfile.path) {
return when.reject(new errors.NoPermissionError('Please select a file to import.'));
}
return api.settings.read({key: 'databaseVersion', context: { internal: true }}).then(function (response) {
var setting = response.settings[0];
type = options.importfile.type;
ext = path.extname(options.importfile.name).toLowerCase();
filepath = options.importfile.path;
return when(setting.value);
}, function () {
return when('002');
return when(isValidFile(options.importfile)).then(function (result) {
if (!result) {
return when.reject(new errors.UnsupportedMediaTypeError('Please select a .json file to import.'));
}
}).then(function () {
return api.settings.read({key: 'databaseVersion', context: { internal: true }}).then(function (response) {
var setting = response.settings[0];
return when(setting.value);
}, function () {
return when('002');
});
}).then(function (version) {
databaseVersion = version;
// Read the file contents
return nodefn.call(fs.readFile, options.importfile.path);
return nodefn.call(fs.readFile, filepath);
}).then(function (fileContents) {
var importData,
error = '';
@ -118,11 +131,9 @@ db = {
return api.settings.updateSettingsCache();
}).then(function () {
return when.resolve({ db: [] });
}).otherwise(function importFailure(error) {
return when.reject(new errors.InternalServerError(error.message || error));
}).finally(function () {
// Unlink the file after import
return nodefn.call(fs.unlink, options.importfile.path);
return nodefn.call(fs.unlink, filepath);
});
}, function () {
return when.reject(new errors.NoPermissionError('You do not have permission to import data. (no rights)'));

View File

@ -18,6 +18,7 @@ var _ = require('lodash'),
users = require('./users'),
slugs = require('./slugs'),
authentication = require('./authentication'),
uploads = require('./upload'),
http,
formatHttpErrors,
@ -255,7 +256,8 @@ module.exports = {
themes: themes,
users: users,
slugs: slugs,
authentication: authentication
authentication: authentication,
uploads: uploads
};
/**

64
core/server/api/upload.js Normal file
View File

@ -0,0 +1,64 @@
var when = require('when'),
path = require('path'),
nodefn = require('when/node'),
fs = require('fs-extra'),
storage = require('../storage'),
errors = require('../errors'),
upload;
function isImage(type, ext) {
if ((type === 'image/jpeg' || type === 'image/png' || type === 'image/gif' || type === 'image/svg+xml')
&& (ext === '.jpg' || ext === '.jpeg' || ext === '.png' || ext === '.gif' || ext === '.svg' || ext === '.svgz')) {
return true;
}
return false;
}
/**
* ## Upload API Methods
*
* **See:** [API Methods](index.js.html#api%20methods)
*/
upload = {
/**
* ### Add Image
*
* @public
* @param {{context}} options
* @returns {Promise} Success
*/
'add': function (options) {
var store = storage.get_storage(),
type,
ext,
filepath;
if (!options.uploadimage || !options.uploadimage.type || !options.uploadimage.path) {
return when.reject(new errors.NoPermissionError('Please select an image.'));
}
type = options.uploadimage.type;
ext = path.extname(options.uploadimage.name).toLowerCase();
filepath = options.uploadimage.path;
return when(isImage(type, ext)).then(function (result) {
if (!result) {
return when.reject(new errors.UnsupportedMediaTypeError('Please select a valid image.'));
}
}).then(function () {
return store.save(options.uploadimage);
}).then(function (url) {
return when.resolve(url);
}).finally(function () {
// Remove uploaded file from tmp location
return nodefn.call(fs.unlink, filepath);
});
}
};
module.exports = upload;

View File

@ -1,10 +1,8 @@
var config = require('../config'),
path = require('path'),
_ = require('lodash'),
when = require('when'),
api = require('../api'),
errors = require('../errors'),
storage = require('../storage'),
updateCheck = require('../update-check'),
adminControllers;
@ -51,29 +49,6 @@ adminControllers = {
}).finally(function () {
renderIndex();
}).catch(errors.logError);
},
// Route: upload
// Path: /ghost/upload/
// Method: POST
'upload': function (req, res) {
var type = req.files.uploadimage.type,
ext = path.extname(req.files.uploadimage.name).toLowerCase(),
store = storage.get_storage();
if ((type !== 'image/jpeg' && type !== 'image/png' && type !== 'image/gif' && type !== 'image/svg+xml')
|| (ext !== '.jpg' && ext !== '.jpeg' && ext !== '.png' && ext !== '.gif' && ext !== '.svg' && ext !== '.svgz')) {
return res.send(415, 'Unsupported Media Type');
}
store
.save(req.files.uploadimage)
.then(function (url) {
return res.send(url);
})
.otherwise(function (e) {
errors.logError(e);
return res.send(500, e.message);
});
}
};

View File

@ -33,8 +33,6 @@ adminRoutes = function (middleware) {
res.redirect(301, subdir + '/ghost/signup/');
});
router.post('/ghost/upload/', middleware.busboy, admin.upload);
// redirect to /ghost and let that do the authentication to prevent redirects to /ghost//admin etc.
router.get(/^\/((ghost-admin|admin|wp-admin|dashboard|signin)\/?)$/, function (req, res) {
/*jslint unparam:true*/

View File

@ -78,6 +78,8 @@ apiRoutes = function (middleware) {
middleware.generateAccessToken
);
// ## Uploads
router.post('/uploads', middleware.busboy, api.http(api.uploads.add));
return router;
};

View File

@ -28,8 +28,6 @@ localFileStore = _.extend(baseStore, {
return nodefn.call(fs.mkdirs, targetDir);
}).then(function () {
return nodefn.call(fs.copy, image.path, targetFilename);
}).then(function () {
return nodefn.call(fs.unlink, image.path).otherwise(errors.logError);
}).then(function () {
// The src for the image must be in URI format, not a file system path, which in Windows uses \
// For local file system storage can use relative path so add a slash

View File

@ -0,0 +1,122 @@
/*globals describe, beforeEach, afterEach, it*/
/*jshint expr:true*/
var fs = require('fs-extra'),
should = require('should'),
sinon = require('sinon'),
when = require('when'),
storage = require('../../../server/storage'),
// Stuff we are testing
UploadAPI = require('../../../server/api/upload');
// To stop jshint complaining
should.equal(true, true);
describe('Upload API', function () {
describe('upload', function () {
var req, res, store;
beforeEach(function () {
store = sinon.stub();
store.save = sinon.stub().returns(when('URL'));
store.exists = sinon.stub().returns(when(true));
store.destroy = sinon.stub().returns(when());
sinon.stub(storage, 'get_storage').returns(store);
sinon.stub(fs, 'unlink').yields();
});
afterEach(function () {
storage.get_storage.restore();
fs.unlink.restore();
});
describe('can not upload invalid file', function () {
it('should return 415 for invalid file type', function (done) {
var uploadimage = {
name: 'INVALID.FILE',
type: 'application/octet-stream',
path: '/tmp/TMPFILEID'
};
UploadAPI.add({uploadimage: uploadimage}).then(function () {
done(new Error('Upload suceeded with invalid file.'));
}, function (result) {
result.code.should.equal(415);
result.type.should.equal('UnsupportedMediaTypeError');
done();
});
});
});
describe('can not upload file with valid extension but invalid type', function () {
it('should return 415 for invalid file type', function (done) {
var uploadimage = {
name: 'INVALID.jpg',
type: 'application/octet-stream',
path: '/tmp/TMPFILEID'
};
UploadAPI.add({uploadimage: uploadimage}).then(function () {
done(new Error('Upload suceeded with invalid file.'));
}, function (result) {
result.code.should.equal(415);
result.type.should.equal('UnsupportedMediaTypeError');
done();
});
});
});
describe('valid file', function () {
it('can upload jpg', function (done) {
var uploadimage = {
name: 'INVALID.jpg',
type: 'image/jpeg',
path: '/tmp/TMPFILEID'
};
UploadAPI.add({uploadimage: uploadimage}).then(function (result) {
result.should.equal('URL');
done();
});
});
it('cannot upload jpg with incorrect extension', function (done) {
var uploadimage = {
name: 'INVALID.xjpg',
type: 'image/jpeg',
path: '/tmp/TMPFILEID'
};
UploadAPI.add({uploadimage: uploadimage}).then(function (result) {
done(new Error('Upload suceeded with invalid file.'));
}, function (result) {
result.code.should.equal(415);
result.type.should.equal('UnsupportedMediaTypeError');
done();
});
});
it('can upload png', function (done) {
var uploadimage = {
name: 'INVALID.png',
type: 'image/png',
path: '/tmp/TMPFILEID'
};
UploadAPI.add({uploadimage: uploadimage}).then(function (result) {
result.should.equal('URL');
done();
});
});
it('can upload gif', function (done) {
var uploadimage = {
name: 'INVALID.gif',
type: 'image/gif',
path: '/tmp/TMPFILEID'
};
UploadAPI.add({uploadimage: uploadimage}).then(function (result) {
result.should.equal('URL');
done();
});
});
});
});
});

View File

@ -1,132 +0,0 @@
/*globals describe, beforeEach, afterEach, it*/
/*jshint expr:true*/
var fs = require('fs-extra'),
should = require('should'),
sinon = require('sinon'),
when = require('when'),
storage = require('../../server/storage'),
// Stuff we are testing
admin = require('../../server/controllers/admin');
// To stop jshint complaining
should.equal(true, true);
describe('Admin Controller', function () {
describe('upload', function () {
var req, res, store;
beforeEach(function () {
req = {
files: {
uploadimage: {
path: '/tmp/TMPFILEID'
}
}
};
res = {
send: function () {
}
};
store = sinon.stub();
store.save = sinon.stub().returns(when('URL'));
store.exists = sinon.stub().returns(when(true));
store.destroy = sinon.stub().returns(when());
sinon.stub(storage, 'get_storage').returns(store);
});
afterEach(function () {
storage.get_storage.restore();
});
describe('can not upload invalid file', function () {
it('should return 415 for invalid file type', function () {
res.send = sinon.stub();
req.files.uploadimage.name = 'INVALID.FILE';
req.files.uploadimage.type = 'application/octet-stream';
admin.upload(req, res);
res.send.calledOnce.should.be.true;
res.send.args[0][0].should.equal(415);
res.send.args[0][1].should.equal('Unsupported Media Type');
});
});
describe('can not upload file with valid extension but invalid type', function () {
it('should return 415 for invalid file type', function () {
res.send = sinon.stub();
req.files.uploadimage.name = 'INVALID.jpg';
req.files.uploadimage.type = 'application/octet-stream';
admin.upload(req, res);
res.send.calledOnce.should.be.true;
res.send.args[0][0].should.equal(415);
res.send.args[0][1].should.equal('Unsupported Media Type');
});
});
describe('valid file', function () {
beforeEach(function () {
req.files.uploadimage.name = 'IMAGE.jpg';
req.files.uploadimage.type = 'image/jpeg';
sinon.stub(fs, 'unlink').yields();
});
afterEach(function () {
fs.unlink.restore();
});
it('can upload jpg', function (done) {
sinon.stub(res, 'send', function (data) {
data.should.not.equal(415);
return done();
});
admin.upload(req, res);
});
it('cannot upload jpg with incorrect extension', function (done) {
req.files.uploadimage.name = 'IMAGE.xjpg';
sinon.stub(res, 'send', function (data) {
data.should.equal(415);
return done();
});
admin.upload(req, res);
});
it('can upload png', function (done) {
req.files.uploadimage.name = 'IMAGE.png';
req.files.uploadimage.type = 'image/png';
sinon.stub(res, 'send', function (data) {
data.should.not.equal(415);
return done();
});
admin.upload(req, res);
});
it('can upload gif', function (done) {
req.files.uploadimage.name = 'IMAGE.gif';
req.files.uploadimage.type = 'image/gif';
sinon.stub(res, 'send', function (data) {
data.should.not.equal(415);
return done();
});
admin.upload(req, res);
});
it('should send correct url', function (done) {
sinon.stub(res, 'send', function (data) {
data.should.equal('URL');
return done();
});
admin.upload(req, res);
});
});
});
});

View File

@ -81,15 +81,6 @@ describe('Local File System Storage', function () {
}).catch(done);
});
it('should not leave temporary file when uploading', function (done) {
localfilesystem.save(image).then(function (url) {
/*jshint unused:false*/
fs.unlink.calledOnce.should.be.true;
fs.unlink.args[0][0].should.equal('tmp/123456.jpg');
done();
}).catch(done);
});
it('can upload two different images with the same name without overwriting the first', function (done) {
// Sun Sep 08 2013 10:57
this.clock = sinon.useFakeTimers(new Date(2013, 8, 8, 10, 57).getTime());