Tidy up the local file storage for images

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(),
+            storage = adminControllers.get_storage();
-            rootToUrl = '/'; // TODO for local storage this works, for external storage not
 
         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

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

core/test/unit/admin_spec.js

@@ -1,6 +1,5 @@
 /*globals describe, beforeEach, it*/
-var testUtils = require('./testUtils'),
+var fs = require('fs-extra'),
-    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 req, res, storage;
-        var res;
 
         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);
+            });
         });
     });
 });

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) {
+        localfilesystem.save(date, image).then(function (url) {
-            url.should.equal('GHOSTURL/content/images/2013/Sep/IMAGE.jpg');
+            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) {
+        localfilesystem.save(date, image).then(function (url) {
-            url.should.equal('GHOSTURL/content/images/2013/Sep/AN_IMAGE.jpg');
+            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) {
+        localfilesystem.save(date, image).then(function (url) {
-            url.should.equal('GHOSTURL/content/images/2014/Jan/IMAGE.jpg');
+            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) {
+        localfilesystem.save(date, image).then(function (url) {
-            url.should.equal('GHOSTURL/content/images/2013/Sep/IMAGE-1.jpg');
+            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) {
+        localfilesystem.save(date, image).then(function (url) {
-            url.should.equal('GHOSTURL/content/images/2013/Sep/IMAGE-4.jpg');
+            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.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) {
+            localfilesystem.save(date, image).then(function (url) {
-                url.should.equal('GHOSTURL/content/images/2013/Sep/IMAGE.jpg');
+                url.should.equal('/content/images/2013/Sep/IMAGE.jpg');
                 return done();
             });
         });
     });
-});
+});