improvement: ensure custom storage adapter has required functions (#7234

)

refs #2852

- improvement: ensure custom storage adapter has required functions
- serve, save and exists are from now on required functions for a custom storage adapter
- add delete as required storage function

core/server/storage/base.js

@@ -2,6 +2,10 @@ var moment = require('moment'),
     path = require('path');
 
 function StorageBase() {
+    Object.defineProperty(this, 'requiredFns', {
+        value: ['exists', 'save', 'serve', 'delete'],
+        writable: false
+    });
 }
 
 StorageBase.prototype.getTargetDir = function (baseDir) {

core/server/storage/index.js

@@ -1,5 +1,7 @@
 var errors = require('../errors'),
     config = require('../config'),
+    Base = require('./base'),
+    _ = require('lodash'),
     storage = {};
 
 /**
@@ -33,10 +35,20 @@ function getStorage(type) {
         }
     }
 
-    // TODO: determine if storage has all the necessary methods.
-    // Instantiate and cache the storage module instance.
     storage[storageChoice] = new storage[storageChoice](storageConfig);
 
+    if (!(storage[storageChoice] instanceof Base)) {
+        throw new errors.IncorrectUsage('Your storage adapter does not inherit from the Storage Base.');
+    }
+
+    if (!storage[storageChoice].requiredFns) {
+        throw new errors.IncorrectUsage('Your storage adapter does not provide the minimum required functions.');
+    }
+
+    if (_.xor(storage[storageChoice].requiredFns, Object.keys(_.pick(Object.getPrototypeOf(storage[storageChoice]), storage[storageChoice].requiredFns))).length) {
+        throw new errors.IncorrectUsage('Your storage adapter does not provide the minimum required functions.');
+    }
+
     return storage[storageChoice];
 }
 

core/server/storage/local-file-store.js

@@ -9,11 +9,12 @@ var serveStatic = require('express').static,
     errors      = require('../errors'),
     config      = require('../config'),
     utils       = require('../utils'),
-    baseStore   = require('./base');
+    BaseStore   = require('./base');
 
 function LocalFileStore() {
+    BaseStore.call(this);
 }
-util.inherits(LocalFileStore, baseStore);
+util.inherits(LocalFileStore, BaseStore);
 
 // ### Save
 // Saves the image to storage (the file system)
@@ -56,4 +57,8 @@ LocalFileStore.prototype.serve = function () {
     return serveStatic(config.paths.imagesPath, {maxAge: utils.ONE_YEAR_MS, fallthrough: false});
 };
 
+LocalFileStore.prototype.delete = function () {
+    return Promise.reject('not implemented');
+};
+
 module.exports = LocalFileStore;

core/test/unit/storage/index_spec.js

@@ -9,6 +9,23 @@ var fs = require('fs-extra'),
 should.equal(true, true);
 
 describe('storage: index_spec', function () {
+    var scope = {adapter: null};
+
+    before(function () {
+        if (!fs.existsSync(configUtils.config.paths.storagePath.custom)) {
+            fs.mkdirSync(configUtils.config.paths.storagePath.custom);
+        }
+    });
+
+    afterEach(function () {
+        if (scope.adapter) {
+            fs.unlinkSync(scope.adapter);
+            scope.adapter = null;
+        }
+
+        configUtils.restore();
+    });
+
     describe('default ghost storage config', function () {
         it('load without a type', function () {
             var chosenStorage = storage.getStorage();
@@ -31,6 +48,8 @@ describe('storage: index_spec', function () {
 
     describe('custom ghost storage config', function () {
         it('images storage adapter is custom, themes is default', function () {
+            scope.adapter = configUtils.config.paths.storagePath.custom + 'custom-adapter.js';
+
             configUtils.set({
                 storage: {
                     active: {
@@ -46,21 +65,81 @@ describe('storage: index_spec', function () {
                 'util.inherits(AnotherAdapter, StorageBase);' +
                 'AnotherAdapter.prototype.exists = function (){};' +
                 'AnotherAdapter.prototype.save = function (){};' +
+                'AnotherAdapter.prototype.serve = function (){};' +
+                'AnotherAdapter.prototype.delete = function (){};' +
                 'module.exports = AnotherAdapter', chosenStorage;
 
-            if (!fs.existsSync(configUtils.config.paths.storagePath.custom)) {
-                fs.mkdirSync(configUtils.config.paths.storagePath.custom);
-            }
-
-            fs.writeFileSync(configUtils.config.paths.storagePath.custom + 'custom-adapter.js', jsFile);
+            fs.writeFileSync(scope.adapter, jsFile);
 
             chosenStorage = storage.getStorage('themes');
             (chosenStorage instanceof localFileStorage).should.eql(true);
 
             chosenStorage = storage.getStorage('images');
             (chosenStorage instanceof localFileStorage).should.eql(false);
+        });
+    });
+
+    describe('adapter validation', function () {
+        it('create good adapter', function () {
+            scope.adapter = configUtils.config.paths.storagePath.custom + 'another-storage.js';
+
+            configUtils.set({
+                storage: {
+                    active: 'another-storage'
+                },
+                paths: {
+                    storage: __dirname + '/another-storage.js'
+                }
+            });
+
+            var jsFile = '' +
+                'var util = require(\'util\');' +
+                'var StorageBase = require(__dirname + \'/../../core/server/storage/base\');' +
+                'var AnotherAdapter = function (){ StorageBase.call(this); };' +
+                'util.inherits(AnotherAdapter, StorageBase);' +
+                'AnotherAdapter.prototype.exists = function (){};' +
+                'AnotherAdapter.prototype.save = function (){};' +
+                'AnotherAdapter.prototype.serve = function (){};' +
+                'AnotherAdapter.prototype.delete = function (){};' +
+                'module.exports = AnotherAdapter', adapter;
+
+            fs.writeFileSync(scope.adapter, jsFile);
+
+            adapter = storage.getStorage();
+            should.exist(adapter);
+            (adapter instanceof localFileStorage).should.eql(false);
+        });
+
+        it('create bad adapter: exists fn is missing', function () {
+            scope.adapter = configUtils.config.paths.storagePath.custom + 'broken-storage.js';
 
-            fs.unlinkSync(configUtils.config.paths.storagePath.custom + 'custom-adapter.js');
+            configUtils.set({
+                storage: {
+                    active: 'broken-storage'
+                },
+                paths: {
+                    storage: __dirname + '/broken-storage.js'
+                }
+            });
+
+            var jsFile = '' +
+                'var util = require(\'util\');' +
+                'var StorageBase = require(__dirname + \'/../../core/server/storage/base\');' +
+                'var AnotherAdapter = function (){ StorageBase.call(this); };' +
+                'util.inherits(AnotherAdapter, StorageBase);' +
+                'AnotherAdapter.prototype.save = function (){};' +
+                'AnotherAdapter.prototype.serve = function (){};' +
+                'AnotherAdapter.prototype.delete = function (){};' +
+                'module.exports = AnotherAdapter';
+
+            fs.writeFileSync(scope.adapter, jsFile);
+
+            try {
+                storage.getStorage();
+            } catch (err) {
+                should.exist(err);
+                (err instanceof errors.IncorrectUsage).should.eql(true);
+            }
         });
     });
 });

core/test/unit/storage_local-file-store_spec.js → core/test/unit/storage/local-file-store_spec.js

@@ -4,10 +4,10 @@ var fs              = require('fs-extra'),
     should          = require('should'),
     sinon           = require('sinon'),
     _               = require('lodash'),
-    LocalFileStore  = require('../../server/storage/local-file-store'),
+    LocalFileStore  = require('../../../server/storage/local-file-store'),
     localFileStore,
 
-    configUtils     = require('../utils/configUtils');
+    configUtils     = require('../../utils/configUtils');
 
 // To stop jshint complaining
 should.equal(true, true);