Moves storage module to use prototypes for inheritance and structure.

addresses #2852

- Moves storage modules to use prototypes and to create prototypes
that inherit from the base storage ctor.

- Makes storage/base conform to an all Promise interface.

core/server/api/upload.js

@@ -29,7 +29,7 @@ upload = {
      * @returns {Promise} Success
      */
     add: function (options) {
-        var store = storage.get_storage(),
+        var store = storage.getStorage(),
             type,
             ext,
             filepath;

core/server/middleware/index.js

@@ -278,7 +278,7 @@ setupMiddleware = function (server) {
 
     // Static assets
     expressServer.use('/shared', express['static'](path.join(corePath, '/shared'), {maxAge: utils.ONE_HOUR_MS}));
-    expressServer.use('/content/images', storage.get_storage().serve());
+    expressServer.use('/content/images', storage.getStorage().serve());
     expressServer.use('/ghost/scripts', express['static'](path.join(corePath, '/built/scripts'), {maxAge: utils.ONE_YEAR_MS}));
     expressServer.use('/public', express['static'](path.join(corePath, '/built/public'), {maxAge: utils.ONE_YEAR_MS}));
 

core/server/storage/base.js

@@ -1,11 +1,10 @@
 var moment  = require('moment'),
-    path    = require('path'),
-    Promise = require('bluebird'),
-    baseStore;
+    path    = require('path');
 
-// TODO: would probably be better to put these on the prototype and have proper constructors etc
-baseStore = {
-    'getTargetDir': function (baseDir) {
+function StorageBase() {
+}
+
+StorageBase.prototype.getTargetDir = function (baseDir) {
     var m = moment(new Date().getTime()),
         month = m.format('MMM'),
         year =  m.format('YYYY');
@@ -15,8 +14,9 @@ baseStore = {
     }
 
     return path.join(year, month);
-    },
-    'generateUnique': function (store, dir, name, ext, i, done) {
+};
+
+StorageBase.prototype.generateUnique = function (store, dir, name, ext, i) {
     var self = this,
         filename,
         append = '';
@@ -27,26 +27,22 @@ baseStore = {
 
     filename = path.join(dir, name + append + ext);
 
-        store.exists(filename).then(function (exists) {
+    return store.exists(filename).then(function (exists) {
         if (exists) {
-                setImmediate(function () {
             i = i + 1;
-                    self.generateUnique(store, dir, name, ext, i, done);
-                });
+            return self.generateUnique(store, dir, name, ext, i);
         } else {
-                done(filename);
+            return filename;
         }
     });
-    },
-    'getUniqueFileName': function (store, image, targetDir) {
+};
+
+StorageBase.prototype.getUniqueFileName = function (store, image, targetDir) {
     var ext = path.extname(image.name),
         name = path.basename(image.name, ext).replace(/[\W]/gi, '-'),
         self = this;
 
-        return new Promise(function (resolve) {
-            self.generateUnique(store, targetDir, name, ext, 0, resolve);
-        });
-    }
+    return self.generateUnique(store, targetDir, name, ext, 0);
 };
 
-module.exports = baseStore;
+module.exports = StorageBase;

core/server/storage/index.js

@@ -1,22 +1,26 @@
 var errors  = require('../errors'),
-    storage;
+    storage = {};
 
-function get_storage() {
+function getStorage(storageChoice) {
     // TODO: this is where the check for storage apps should go
-    // Local file system is the default
-    var storageChoice = 'localfilesystem';
+    // Local file system is the default.  Fow now that is all we support.
+    storageChoice = 'localfilesystem';
 
-    if (storage) {
-        return storage;
+    if (storage[storageChoice]) {
+        return storage[storageChoice];
     }
 
     try {
-        // TODO: determine if storage has all the necessary methods
-        storage = require('./' + storageChoice);
+        // TODO: determine if storage has all the necessary methods.
+        storage[storageChoice] = require('./' + storageChoice);
     } catch (e) {
         errors.logError(e);
     }
-    return storage;
+
+    // Instantiate and cache the storage module instance.
+    storage[storageChoice] = new storage[storageChoice]();
+
+    return storage[storageChoice];
 }
 
-module.exports.get_storage = get_storage;
+module.exports.getStorage = getStorage;

core/server/storage/localfilesystem.js

@@ -1,24 +1,25 @@
 // # Local File System Image Storage module
 // The (default) module for storing images, using the local file system
 
-var _         = require('lodash'),
-    express   = require('express'),
+var express   = require('express'),
     fs        = require('fs-extra'),
     path      = require('path'),
+    util      = require('util'),
     Promise   = require('bluebird'),
     errors    = require('../errors'),
     config    = require('../config'),
     utils     = require('../utils'),
-    baseStore = require('./base'),
+    baseStore = require('./base');
 
-    localFileStore;
+function LocalFileStore() {
+}
+util.inherits(LocalFileStore, baseStore);
 
-localFileStore = _.extend(baseStore, {
 // ### Save
 // Saves the image to storage (the file system)
 // - image is the express image object
 // - returns a promise which ultimately returns the full url to the uploaded image
-    'save': function (image) {
+LocalFileStore.prototype.save = function (image) {
     var targetDir = this.getTargetDir(config.paths.imagesPath),
         targetFilename;
 
@@ -30,27 +31,27 @@ localFileStore = _.extend(baseStore, {
     }).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
-            var fullUrl = (config.paths.subdir + '/' + config.paths.imagesRelPath + '/' + path.relative(config.paths.imagesPath, targetFilename)).replace(new RegExp('\\' + path.sep, 'g'), '/');
+        var fullUrl = (config.paths.subdir + '/' + config.paths.imagesRelPath + '/' +
+            path.relative(config.paths.imagesPath, targetFilename)).replace(new RegExp('\\' + path.sep, 'g'), '/');
         return fullUrl;
     }).catch(function (e) {
         errors.logError(e);
         return Promise.reject(e);
     });
-    },
+};
 
-    'exists': function (filename) {
+LocalFileStore.prototype.exists = function (filename) {
     return new Promise(function (resolve) {
         fs.exists(filename, function (exists) {
             resolve(exists);
         });
     });
-    },
+};
 
 // middleware for serving the files
-    'serve': function () {
+LocalFileStore.prototype.serve = function () {
     // For some reason send divides the max age number by 1000
     return express['static'](config.paths.imagesPath, {maxAge: utils.ONE_YEAR_MS});
-    }
-});
+};
 
-module.exports = localFileStore;
+module.exports = LocalFileStore;

core/test/integration/api/api_upload_spec.js

@@ -17,7 +17,7 @@ describe('Upload API', function () {
     // Doesn't test the DB
 
     afterEach(function () {
-        storage.get_storage.restore();
+        storage.getStorage.restore();
         fs.unlink.restore();
     });
 
@@ -26,7 +26,7 @@ describe('Upload API', function () {
         store.save = sinon.stub().returns(Promise.resolve('URL'));
         store.exists = sinon.stub().returns(Promise.resolve(true));
         store.destroy = sinon.stub().returns(Promise.resolve());
-        sinon.stub(storage, 'get_storage').returns(store);
+        sinon.stub(storage, 'getStorage').returns(store);
         sinon.stub(fs, 'unlink').yields();
     });
 

core/test/unit/storage_localfilesystem_spec.js

@@ -7,7 +7,8 @@ var fs              = require('fs-extra'),
     rewire          = require('rewire'),
     _               = require('lodash'),
     config          = rewire('../../server/config'),
-    localfilesystem = rewire('../../server/storage/localfilesystem');
+    LocalFileStore  = rewire('../../server/storage/localfilesystem'),
+    localfilesystem;
 
 // To stop jshint complaining
 should.equal(true, true);
@@ -16,10 +17,10 @@ describe('Local File System Storage', function () {
 
     var image,
         overrideConfig = function (newConfig) {
-            var existingConfig = localfilesystem.__get__('config'),
+            var existingConfig = LocalFileStore.__get__('config'),
                 updatedConfig = _.extend(existingConfig, newConfig);
             config.set(updatedConfig);
-            localfilesystem.__set__('config', updatedConfig);
+            LocalFileStore.__set__('config', updatedConfig);
         };
 
     beforeEach(function () {
@@ -38,6 +39,8 @@ describe('Local File System Storage', function () {
 
         // Sat Sep 07 2013 21:24
         this.clock = sinon.useFakeTimers(new Date(2013, 8, 7, 21, 24).getTime());
+
+        localfilesystem = new LocalFileStore();
     });
 
     afterEach(function () {