From f42e977fa776cd591022e398a3bdf2ec9ec97777 Mon Sep 17 00:00:00 2001 From: jamesbloomer Date: Fri, 1 Nov 2013 13:05:20 +0000 Subject: [PATCH] Tidy up the local file storage for images --- core/server/controllers/admin.js | 8 ++--- .../controllers/storage/localfilesystem.js | 9 ++--- core/test/unit/admin_spec.js | 27 ++++++++++----- .../test/unit/storage_localfilesystem_spec.js | 33 +++++++++---------- 4 files changed, 41 insertions(+), 36 deletions(-) diff --git a/core/server/controllers/admin.js b/core/server/controllers/admin.js index 3c65fe1a5d..65af7a77d3 100644 --- a/core/server/controllers/admin.js +++ b/core/server/controllers/admin.js @@ -45,15 +45,15 @@ function setSelected(list, name) { adminControllers = { 'get_storage': function () { - // TODO get storage choice from config + // TODO this is where the check for storage plugins should go + // Local file system is the default var storageChoice = 'localfilesystem.js'; return require('./storage/' + storageChoice); }, 'uploader': function (req, res) { var type = req.files.uploadimage.type, ext = path.extname(req.files.uploadimage.name).toLowerCase(), - storage = adminControllers.get_storage(), - rootToUrl = '/'; // TODO for local storage this works, for external storage not + storage = adminControllers.get_storage(); if ((type !== 'image/jpeg' && type !== 'image/png' && type !== 'image/gif') || (ext !== '.jpg' && ext !== '.jpeg' && ext !== '.png' && ext !== '.gif')) { @@ -61,7 +61,7 @@ adminControllers = { } storage - .save(new Date().getTime(), req.files.uploadimage, rootToUrl) + .save(new Date().getTime(), req.files.uploadimage) .then(function (url) { // delete the temporary file diff --git a/core/server/controllers/storage/localfilesystem.js b/core/server/controllers/storage/localfilesystem.js index dcdf1fe855..ee89a8fb58 100644 --- a/core/server/controllers/storage/localfilesystem.js +++ b/core/server/controllers/storage/localfilesystem.js @@ -34,17 +34,13 @@ function getUniqueFileName(dir, name, ext, i, done) { // ## 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) { + 'save': function (date, image) { // QUESTION is it okay for this module to know about content/images? var saved = when.defer(), @@ -62,7 +58,8 @@ localfilesystem = { 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'), '/'); + // For local file system storage can use relative path so add a slash + var fullUrl = ('/' + filename).replace(new RegExp('\\' + path.sep, 'g'), '/'); return saved.resolve(fullUrl); }).otherwise(function (e) { errors.logError(e); diff --git a/core/test/unit/admin_spec.js b/core/test/unit/admin_spec.js index 74c7ae75e4..45ebaf3cd7 100644 --- a/core/test/unit/admin_spec.js +++ b/core/test/unit/admin_spec.js @@ -1,6 +1,5 @@ /*globals describe, beforeEach, it*/ -var testUtils = require('./testUtils'), - fs = require('fs-extra'), +var fs = require('fs-extra'), should = require('should'), sinon = require('sinon'), when = require('when'), @@ -11,8 +10,7 @@ var testUtils = require('./testUtils'), describe('Admin Controller', function() { describe('uploader', function() { - var req; - var res; + var req, res, storage; beforeEach(function() { req = { @@ -27,7 +25,13 @@ describe('Admin Controller', function() { send: function(){} }; - // localfilesystem.save = sinon.stub().returns(when('URL')); + storage = sinon.stub(); + storage.save = sinon.stub().returns(when('URL')); + sinon.stub(admin, 'get_storage').returns(storage); + }); + + afterEach(function () { + admin.get_storage.restore(); }); describe('can not upload invalid file', function() { @@ -60,14 +64,10 @@ describe('Admin Controller', function() { req.files.uploadimage.name = 'IMAGE.jpg'; req.files.uploadimage.type = 'image/jpeg'; sinon.stub(fs, 'unlink').yields(); - var storage = sinon.stub(); - storage.save = sinon.stub().returns(when('URL')); - sinon.stub(admin, 'get_storage').returns(storage); }); afterEach(function() { fs.unlink.restore(); - admin.get_storage.restore(); }); it('can upload jpg', function(done) { @@ -120,6 +120,15 @@ describe('Admin Controller', function() { admin.uploader(req, res); }); + + it('should send correct url', function(done) { + sinon.stub(res, 'send', function(data) { + data.should.equal('URL'); + return done(); + }); + + admin.uploader(req, res); + }); }); }); }); diff --git a/core/test/unit/storage_localfilesystem_spec.js b/core/test/unit/storage_localfilesystem_spec.js index a52f747a44..b3b7852902 100644 --- a/core/test/unit/storage_localfilesystem_spec.js +++ b/core/test/unit/storage_localfilesystem_spec.js @@ -30,8 +30,8 @@ describe('Local File System Storage', function() { 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'); + localfilesystem.save(date, image).then(function (url) { + url.should.equal('/content/images/2013/Sep/IMAGE.jpg'); return done(); }); }); @@ -39,8 +39,8 @@ describe('Local File System Storage', function() { 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'); + localfilesystem.save(date, image).then(function (url) { + url.should.equal('/content/images/2013/Sep/AN_IMAGE.jpg'); return done(); }); }); @@ -48,8 +48,8 @@ describe('Local File System Storage', function() { 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'); + localfilesystem.save(date, image).then(function (url) { + url.should.equal('/content/images/2014/Jan/IMAGE.jpg'); return done(); }); }); @@ -57,7 +57,7 @@ describe('Local File System Storage', function() { 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) { + localfilesystem.save(date, image).then(function (url) { fs.mkdirs.calledOnce.should.be.true; fs.mkdirs.args[0][0].should.equal(path.join('content/images/2013/Sep')); done(); @@ -67,7 +67,7 @@ describe('Local File System Storage', function() { 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) { + localfilesystem.save(date, image).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')); @@ -87,8 +87,8 @@ describe('Local File System Storage', function() { 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'); + localfilesystem.save(date, image).then(function (url) { + url.should.equal('/content/images/2013/Sep/IMAGE-1.jpg'); return done(); }); }); @@ -110,8 +110,8 @@ describe('Local File System Storage', function() { 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'); + localfilesystem.save(date, image).then(function (url) { + url.should.equal('/content/images/2013/Sep/IMAGE-4.jpg'); return done(); }); }); @@ -133,13 +133,12 @@ describe('Local File System Storage', function() { 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'); + path.join.returns('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'); + localfilesystem.save(date, image).then(function (url) { + url.should.equal('/content/images/2013/Sep/IMAGE.jpg'); return done(); }); }); }); -}); \ No newline at end of file +});