diff --git a/core/server/controllers/admin.js b/core/server/controllers/admin.js index 79b69b87cc..a1106fefe4 100644 --- a/core/server/controllers/admin.js +++ b/core/server/controllers/admin.js @@ -43,75 +43,40 @@ function setSelected(list, name) { return list; } -// TODO: this could be a separate module -function getUniqueFileName(dir, name, ext, i, done) { - var filename, - append = ''; - - if (i) { - append = '-' + i; - } - - filename = path.join(dir, name + append + ext); - fs.exists(filename, function (exists) { - if (exists) { - setImmediate(function () { - i = i + 1; - return getUniqueFileName(dir, name, ext, i, done); - }); - } else { - return done(filename); - } - }); -} - adminControllers = { + 'get_storage': function () { + // TODO get storage choice from config + var storageChoice = 'localfilesystem.js'; + return require('./storage/' + storageChoice); + }, 'uploader': function (req, res) { - - var currentDate = moment(), - month = currentDate.format('MMM'), - year = currentDate.format('YYYY'), - tmp_path = req.files.uploadimage.path, - imagespath = path.join(ghost.paths().appRoot, 'content/images'), - dir = path.join(imagespath, year, month), + var type = req.files.uploadimage.type, ext = path.extname(req.files.uploadimage.name).toLowerCase(), - type = req.files.uploadimage.type || req.files.uploadimage.headers['content-type'], - basename = path.basename(req.files.uploadimage.name, ext).replace(/[\W]/gi, '_'); + storage = adminControllers.get_storage(), + rootToUrl = '/'; // TODO for local storage this works, for external storage not - function renameFile(target_path) { - // adds directories recursively - fs.mkdirs(dir, function (err) { - if (err) { - return errors.logError(err); - } + if ((type !== 'image/jpeg' && type !== 'image/png' && type !== 'image/gif') + || (ext !== '.jpg' && ext !== '.jpeg' && ext !== '.png' && ext !== '.gif')) { + return res.send(415, 'Unsupported Media Type'); + } - fs.copy(tmp_path, target_path, function (err) { - if (err) { - return errors.logError(err); + storage + .save(new Date().getTime(), req.files.uploadimage, rootToUrl) + .then(function (url) { + + // delete the temporary file + // TODO convert to promise using nodefn + fs.unlink(req.files.uploadimage.path, function (e) { + if (e) { + return errors.logError(e); } - fs.unlink(tmp_path, function (e) { - if (err) { - return errors.logError(err); - } - - // the src for the image must be in URI format, not a file system path, which in Windows uses \ - var src = path.join('/', target_path.replace(ghost.paths().appRoot, "")).replace(new RegExp('\\' + path.sep, 'g'), '/'); - return res.send(src); - }); + return res.send(url); }); + }) + .otherwise(function (e) { + return errors.logError(e); }); - } - - //limit uploads to type && extension - if ((type === 'image/jpeg' || type === 'image/png' || type === 'image/gif') - && (ext === '.jpg' || ext === '.jpeg' || ext === '.png' || ext === '.gif')) { - getUniqueFileName(dir, basename, ext, null, function (filename) { - renameFile(filename); - }); - } else { - res.send(415, 'Unsupported Media Type'); - } }, 'login': function (req, res) { res.render('login', { diff --git a/core/server/controllers/storage/localfilesystem.js b/core/server/controllers/storage/localfilesystem.js new file mode 100644 index 0000000000..dcdf1fe855 --- /dev/null +++ b/core/server/controllers/storage/localfilesystem.js @@ -0,0 +1,77 @@ +// # Local File System Image Storage module +// The (default) module for storing images, using the local file system + +var errors = require('../../errorHandling'), + fs = require('fs-extra'), + moment = require('moment'), + nodefn = require('when/node/function'), + path = require('path'), + when = require('when'); + +var localfilesystem; + +// TODO: this could be a separate module +function getUniqueFileName(dir, name, ext, i, done) { + var filename, + append = ''; + + if (i) { + append = '-' + i; + } + + filename = path.join(dir, name + append + ext); + fs.exists(filename, function (exists) { + if (exists) { + setImmediate(function () { + i = i + 1; + return getUniqueFileName(dir, name, ext, i, done); + }); + } else { + return done(filename); + } + }); +} + +// ## Module interface +localfilesystem = { + // TODO use promises!! + // QUESTION pass date or month and year? And should the date be ticks or an object? Gone with ticks. + // QUESTION feels wrong to pass in the ghostUrl, the local file system needs it but something like S3 won't? + + // ### Save + // Saves the image to storage (the file system) + // - date is current date in ticks + // - image is the express image object + // - ghosturl is thr base url for the site + // - returns a promise which ultimately returns the full url to the uploaded image + 'save': function (date, image, ghostUrl) { + + // QUESTION is it okay for this module to know about content/images? + var saved = when.defer(), + m = moment(date), + month = m.format('MMM'), + year = m.format('YYYY'), + target_dir = path.join('content/images', year, month), + target_path = path.join(target_dir, image.name), + ext = path.extname(image.name), + basename = path.basename(image.name, ext).replace(/[\W]/gi, '_'); + + getUniqueFileName(target_dir, basename, ext, null, function (filename) { + + nodefn.call(fs.mkdirs, target_dir).then(function () { + return nodefn.call(fs.copy, image.path, target_path); + }).then(function () { + // The src for the image must be in URI format, not a file system path, which in Windows uses \ + var fullUrl = path.join(ghostUrl, filename).replace(new RegExp('\\' + path.sep, 'g'), '/'); + return saved.resolve(fullUrl); + }).otherwise(function (e) { + errors.logError(e); + return saved.reject(e); + }); + }); + + return saved.promise; + } +}; + +module.exports = localfilesystem; diff --git a/core/test/unit/admin_spec.js b/core/test/unit/admin_spec.js index 22b40d6684..74c7ae75e4 100644 --- a/core/test/unit/admin_spec.js +++ b/core/test/unit/admin_spec.js @@ -1,11 +1,9 @@ /*globals describe, beforeEach, it*/ var testUtils = require('./testUtils'), + fs = require('fs-extra'), should = require('should'), sinon = require('sinon'), when = require('when'), - fs = require('fs-extra'), - path = require('path'), - appRoot = path.resolve(__dirname, '../../../'), // Stuff we are testing admin = require('../../server/controllers/admin'); @@ -28,6 +26,8 @@ describe('Admin Controller', function() { res = { send: function(){} }; + + // localfilesystem.save = sinon.stub().returns(when('URL')); }); describe('can not upload invalid file', function() { @@ -56,40 +56,33 @@ describe('Admin Controller', function() { describe('valid file', function() { - var clock; - beforeEach(function() { req.files.uploadimage.name = 'IMAGE.jpg'; req.files.uploadimage.type = 'image/jpeg'; - sinon.stub(fs, 'mkdirs').yields(); - sinon.stub(fs, 'copy').yields(); sinon.stub(fs, 'unlink').yields(); - sinon.stub(fs, 'exists').yields(false); + var storage = sinon.stub(); + storage.save = sinon.stub().returns(when('URL')); + sinon.stub(admin, 'get_storage').returns(storage); }); afterEach(function() { - fs.mkdirs.restore(); - fs.copy.restore(); fs.unlink.restore(); - fs.exists.restore(); - clock.restore(); + admin.get_storage.restore(); }); it('can upload jpg', function(done) { - clock = sinon.useFakeTimers(42); sinon.stub(res, 'send', function(data) { - data.should.not.equal(404); + data.should.not.equal(415); return done(); }); admin.uploader(req, res); }); - it('can upload jpg with incorrect extension', function(done) { + it('cannot upload jpg with incorrect extension', function(done) { req.files.uploadimage.name = 'IMAGE.xjpg'; - clock = sinon.useFakeTimers(42); sinon.stub(res, 'send', function(data) { - data.should.not.equal(404); + data.should.equal(415); return done(); }); @@ -99,9 +92,8 @@ describe('Admin Controller', function() { it('can upload png', function(done) { req.files.uploadimage.name = 'IMAGE.png'; req.files.uploadimage.type = 'image/png'; - clock = sinon.useFakeTimers(42); sinon.stub(res, 'send', function(data) { - data.should.not.equal(404); + data.should.not.equal(415); return done(); }); @@ -111,83 +103,15 @@ describe('Admin Controller', function() { it('can upload gif', function(done) { req.files.uploadimage.name = 'IMAGE.gif'; req.files.uploadimage.type = 'image/gif'; - clock = sinon.useFakeTimers(42); sinon.stub(res, 'send', function(data) { - data.should.not.equal(404); + data.should.not.equal(415); return done(); }); admin.uploader(req, res); }); - it('should send correct path to image when today is in Sep 2013', function(done) { - // Sat Sep 07 2013 21:24 - clock = sinon.useFakeTimers(new Date(2013, 8, 7, 21, 24).getTime()); - sinon.stub(res, 'send', function(data) { - data.should.equal('/content/images/2013/Sep/IMAGE.jpg'); - return done(); - }); - - return admin.uploader(req, res); - }); - - it('should send correct path to image when today is in Jan 2014', function(done) { - // Jan 1 2014 12:00 - clock = sinon.useFakeTimers(new Date(2014, 0, 1, 12).getTime()); - sinon.stub(res, 'send', function(data) { - data.should.equal('/content/images/2014/Jan/IMAGE.jpg'); - return done(); - }); - - admin.uploader(req, res); - }); - - it('can upload two different images with the same name without overwriting the first', function(done) { - // Sun Sep 08 2013 10:57 - clock = sinon.useFakeTimers(new Date(2013, 8, 8, 10, 57).getTime()); - fs.exists.withArgs(path.join(appRoot, 'content/images/2013/Sep/IMAGE.jpg')).yields(true); - fs.exists.withArgs(path.join(appRoot, 'content/images/2013/Sep/IMAGE-1.jpg')).yields(false); - - // if on windows need to setup with back slashes - // doesn't hurt for the test to cope with both - fs.exists.withArgs(path.join(appRoot, 'content\\images\\2013\\Sep\\IMAGE.jpg')).yields(true); - fs.exists.withArgs(path.join(appRoot, 'content\\images\\2013\\Sep\\IMAGE-1.jpg')).yields(false); - - sinon.stub(res, 'send', function(data) { - data.should.equal('/content/images/2013/Sep/IMAGE-1.jpg'); - return done(); - }); - - return admin.uploader(req, res); - }); - - it('can upload five different images with the same name without overwriting the first', function(done) { - // Sun Sep 08 2013 10:57 - clock = sinon.useFakeTimers(new Date(2013, 8, 8, 10, 57).getTime()); - fs.exists.withArgs(path.join(appRoot, 'content/images/2013/Sep/IMAGE.jpg')).yields(true); - fs.exists.withArgs(path.join(appRoot, 'content/images/2013/Sep/IMAGE-1.jpg')).yields(true); - fs.exists.withArgs(path.join(appRoot, 'content/images/2013/Sep/IMAGE-2.jpg')).yields(true); - fs.exists.withArgs(path.join(appRoot, 'content/images/2013/Sep/IMAGE-3.jpg')).yields(true); - fs.exists.withArgs(path.join(appRoot, 'content/images/2013/Sep/IMAGE-4.jpg')).yields(false); - - // windows setup - fs.exists.withArgs(path.join(appRoot, 'content\\images\\2013\\Sep\\IMAGE.jpg')).yields(true); - fs.exists.withArgs(path.join(appRoot, 'content\\images\\2013\\Sep\\IMAGE-1.jpg')).yields(true); - fs.exists.withArgs(path.join(appRoot, 'content\\images\\2013\\Sep\\IMAGE-2.jpg')).yields(true); - fs.exists.withArgs(path.join(appRoot, 'content\\images\\2013\\Sep\\IMAGE-3.jpg')).yields(true); - fs.exists.withArgs(path.join(appRoot, 'content\\images\\2013\\Sep\\IMAGE-4.jpg')).yields(false); - - sinon.stub(res, 'send', function(data) { - data.should.equal('/content/images/2013/Sep/IMAGE-4.jpg'); - return done(); - }); - - return admin.uploader(req, res); - }); - it('should not leave temporary file when uploading', function(done) { - // Sun Sep 08 2013 10:57 - clock = sinon.useFakeTimers(new Date(2013, 8, 8, 10, 57).getTime()); sinon.stub(res, 'send', function(data) { fs.unlink.calledOnce.should.be.true; fs.unlink.args[0][0].should.equal('/tmp/TMPFILEID'); diff --git a/core/test/unit/storage_localfilesystem_spec.js b/core/test/unit/storage_localfilesystem_spec.js new file mode 100644 index 0000000000..a52f747a44 --- /dev/null +++ b/core/test/unit/storage_localfilesystem_spec.js @@ -0,0 +1,145 @@ +/*globals describe, beforeEach, afterEach, it*/ +var fs = require('fs-extra'), + path = require('path'), + should = require('should'), + sinon = require('sinon'), + when = require('when'), + localfilesystem = require('../../server/controllers/storage/localfilesystem'); + +describe('Local File System Storage', function() { + + var image; + + beforeEach(function () { + sinon.stub(fs, 'mkdirs').yields(); + sinon.stub(fs, 'copy').yields(); + sinon.stub(fs, 'exists').yields(false); + image = { + path: "tmp/123456.jpg", + name: "IMAGE.jpg", + type: "image/jpeg" + }; + }); + + afterEach(function () { + fs.mkdirs.restore(); + fs.copy.restore(); + fs.exists.restore(); + }); + + it('should send correct path to image when date is in Sep 2013', function (done) { + // Sat Sep 07 2013 21:24 + var date = new Date(2013, 8, 7, 21, 24).getTime(); + localfilesystem.save(date, image, 'GHOSTURL').then(function (url) { + url.should.equal('GHOSTURL/content/images/2013/Sep/IMAGE.jpg'); + return done(); + }); + }); + + it('should send correct path to image when original file has spaces', function (done) { + var date = new Date(2013, 8, 7, 21, 24).getTime(); + image.name = 'AN IMAGE.jpg'; + localfilesystem.save(date, image, 'GHOSTURL').then(function (url) { + url.should.equal('GHOSTURL/content/images/2013/Sep/AN_IMAGE.jpg'); + return done(); + }); + }); + + it('should send correct path to image when date is in Jan 2014', function (done) { + // Jan 1 2014 12:00 + var date = new Date(2014, 0, 1, 12).getTime(); + localfilesystem.save(date, image, 'GHOSTURL').then(function (url) { + url.should.equal('GHOSTURL/content/images/2014/Jan/IMAGE.jpg'); + return done(); + }); + }); + + it('should create month and year directory', function (done) { + // Sat Sep 07 2013 21:24 + var date = new Date(2013, 8, 7, 21, 24).getTime(); + localfilesystem.save(date, image, 'GHOSTURL').then(function (url) { + fs.mkdirs.calledOnce.should.be.true; + fs.mkdirs.args[0][0].should.equal(path.join('content/images/2013/Sep')); + done(); + }).then(null, done); + }); + + it('should copy temp file to new location', function (done) { + // Sat Sep 07 2013 21:24 + var date = new Date(2013, 8, 7, 21, 24).getTime(); + localfilesystem.save(date, image, 'GHOSTURL').then(function (url) { + fs.copy.calledOnce.should.be.true; + fs.copy.args[0][0].should.equal('tmp/123456.jpg'); + fs.copy.args[0][1].should.equal(path.join('content/images/2013/Sep/IMAGE.jpg')); + done(); + }).then(null, done); + }); + + it('can upload two different images with the same name without overwriting the first', function (done) { + // Sun Sep 08 2013 10:57 + var date = new Date(2013, 8, 8, 10, 57).getTime(); + clock = sinon.useFakeTimers(date); + fs.exists.withArgs('content/images/2013/Sep/IMAGE.jpg').yields(true); + fs.exists.withArgs('content/images/2013/Sep/IMAGE-1.jpg').yields(false); + + // if on windows need to setup with back slashes + // doesn't hurt for the test to cope with both + fs.exists.withArgs('content\\images\\2013\\Sep\\IMAGE.jpg').yields(true); + fs.exists.withArgs('content\\images\\2013\\Sep\\IMAGE-1.jpg').yields(false); + + localfilesystem.save(date, image, 'GHOSTURL').then(function (url) { + url.should.equal('GHOSTURL/content/images/2013/Sep/IMAGE-1.jpg'); + return done(); + }); + }); + + it('can upload five different images with the same name without overwriting the first', function (done) { + // Sun Sep 08 2013 10:57 + var date = new Date(2013, 8, 8, 10, 57).getTime(); + clock = sinon.useFakeTimers(date); + fs.exists.withArgs('content/images/2013/Sep/IMAGE.jpg').yields(true); + fs.exists.withArgs('content/images/2013/Sep/IMAGE-1.jpg').yields(true); + fs.exists.withArgs('content/images/2013/Sep/IMAGE-2.jpg').yields(true); + fs.exists.withArgs('content/images/2013/Sep/IMAGE-3.jpg').yields(true); + fs.exists.withArgs('content/images/2013/Sep/IMAGE-4.jpg').yields(false); + + // windows setup + fs.exists.withArgs('content\\images\\2013\\Sep\\IMAGE.jpg').yields(true); + fs.exists.withArgs('content\\images\\2013\\Sep\\IMAGE-1.jpg').yields(true); + fs.exists.withArgs('content\\images\\2013\\Sep\\IMAGE-2.jpg').yields(true); + fs.exists.withArgs('content\\images\\2013\\Sep\\IMAGE-3.jpg').yields(true); + fs.exists.withArgs('content\\images\\2013\\Sep\\IMAGE-4.jpg').yields(false); + + localfilesystem.save(date, image, 'GHOSTURL').then(function (url) { + url.should.equal('GHOSTURL/content/images/2013/Sep/IMAGE-4.jpg'); + return done(); + }); + }); + + + describe('on Windows', function () { + // TODO tests to check for working on windows + + var truePathSep = path.sep; + + beforeEach(function () { + sinon.stub(path, 'join'); + }); + + afterEach(function () { + path.join.restore(); + path.sep = truePathSep; + }); + + it('should return url in proper format for windows', function (done) { + path.sep = '\\'; + path.join.returns('/content/images/2013/Sep/IMAGE.jpg'); + path.join.withArgs('GHOSTURL', '/content/images/2013/Sep/IMAGE.jpg').returns('GHOSTURL\\content\\images\\2013\\Sep\\IMAGE.jpg'); + var date = new Date(2013, 8, 7, 21, 24).getTime(); + localfilesystem.save(date, image, 'GHOSTURL').then(function (url) { + url.should.equal('GHOSTURL/content/images/2013/Sep/IMAGE.jpg'); + return done(); + }); + }); + }); +}); \ No newline at end of file