Merge branch 'image-refactor'

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 type = req.files.uploadimage.type,
-        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),
             ext = path.extname(req.files.uploadimage.name).toLowerCase(),
-            type = req.files.uploadimage.type || req.files.uploadimage.headers['content-type'],
+            storage = adminControllers.get_storage(),
-            basename = path.basename(req.files.uploadimage.name, ext).replace(/[\W]/gi, '_');
+            rootToUrl = '/'; // TODO for local storage this works, for external storage not
 
-        function renameFile(target_path) {
+        if ((type !== 'image/jpeg' && type !== 'image/png' && type !== 'image/gif')
-            // adds directories recursively
+                || (ext !== '.jpg' && ext !== '.jpeg' && ext !== '.png' && ext !== '.gif')) {
-            fs.mkdirs(dir, function (err) {
+            return res.send(415, 'Unsupported Media Type');
-                if (err) {
-                    return errors.logError(err);
         }
 
-                fs.copy(tmp_path, target_path, function (err) {
+        storage
-                    if (err) {
+            .save(new Date().getTime(), req.files.uploadimage, rootToUrl)
-                        return errors.logError(err);
+            .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) {
+                    return res.send(url);
-                        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);
                 });
+            })
+            .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', {

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;

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();
+                admin.get_storage.restore();
-                clock.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');

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();
+            });
+        });
+    });
+});