🎨 🔦 refactor content paths (images, apps, themes, storage, scheduling)

refs #6982 - create config util fn: getContentPath - we can later let the user change the folder names in contentPath - get rid of custom/default storage paths [ci skip]
TryGhost · Sep 20, 2016 · 6a97873 · 6a97873
1 parent 0487ac5
commit 6a97873
Show file tree

Hide file tree

Showing 19 changed files with 103 additions and 79 deletions.
diff --git a/core/server/api/themes.js b/core/server/api/themes.js
@@ -21,7 +21,7 @@ var Promise = require('bluebird'),
  */
 themes = {
     loadThemes: function () {
-        return utils.readThemes(config.get('paths').themePath)
+        return utils.readThemes(config.getContentPath('themes'))
             .then(function (result) {
                 config.set('paths:availableThemes', result);
             });
@@ -63,12 +63,12 @@ themes = {
                 );
             })
             .then(function () {
-                return storageAdapter.exists(config.get('paths').themePath + '/' + zip.shortName);
+                return storageAdapter.exists(config.getContentPath('themes') + '/' + zip.shortName);
             })
             .then(function (themeExists) {
                 // delete existing theme
                 if (themeExists) {
-                    return storageAdapter.delete(zip.shortName, config.get('paths').themePath);
+                    return storageAdapter.delete(zip.shortName, config.getContentPath('themes'));
                 }
             })
             .then(function () {
@@ -77,7 +77,7 @@ themes = {
                 return storageAdapter.save({
                     name: zip.shortName,
                     path: theme.path
-                }, config.get('paths').themePath);
+                }, config.getContentPath('themes'));
             })
             .then(function () {
                 // force reload of availableThemes
@@ -155,7 +155,7 @@ themes = {
                 }
 
                 events.emit('theme.deleted', name);
-                return storageAdapter.delete(name, config.get('paths').themePath);
+                return storageAdapter.delete(name, config.getContentPath('themes'));
             })
             .then(function () {
                 return themes.loadThemes();

diff --git a/core/server/apps/loader.js b/core/server/apps/loader.js
@@ -20,7 +20,7 @@ function getAppAbsolutePath(name) {
         return path.join(config.get('paths').internalAppPath, name);
     }
 
-    return path.join(config.get('paths').appPath, name);
+    return path.join(config.getContentPath('apps'), name);
 }
 
 // Get a relative path to the given apps root, defaults

diff --git a/core/server/config/defaults.json b/core/server/config/defaults.json
@@ -15,14 +15,7 @@
     },
     "privacy": false,
     "paths": {
-        "contentPath": "content/",
-        "themePath": "content/themes/",
-        "imagesPath": "content/images/",
-        "appPath": "content/apps/",
-        "storagePath": {
-            "custom": "content/storage/"
-        },
-        "schedulingPath": "core/server/scheduling/"
+        "contentPath": "content/"
     },
     "storage": {
         "active": {

diff --git a/core/server/config/index.js b/core/server/config/index.js
@@ -37,3 +37,4 @@ nconf.set('ghostVersion', packageInfo.version);
 
 module.exports = nconf;
 module.exports.isPrivacyDisabled = localUtils.isPrivacyDisabled.bind(nconf);
+module.exports.getContentPath = localUtils.getContentPath.bind(nconf);
diff --git a/core/server/config/overrides.json b/core/server/config/overrides.json
@@ -2,14 +2,13 @@
     "paths": {
         "appRoot": ".",
         "corePath": "core/",
-        "internalAppPath": "core/server/apps/",
-        "storagePath": {
-            "default": "core/server/storage/"
-        },
         "clientAssets": "core/built/assets",
         "imagesRelPath": "content/images",
         "helperTemplates": "core/server/helpers/tpl/",
-        "adminViews": "core/server/views/"
+        "adminViews": "core/server/views/",
+        "internalAppPath": "core/server/apps/",
+        "internalStoragePath": "core/server/storage/",
+        "internalSchedulingPath": "core/server/scheduling/"
     },
     "internalApps": [
         "private-blogging",

diff --git a/core/server/config/utils.js b/core/server/config/utils.js
@@ -35,3 +35,23 @@ exports.makePathsAbsolute = function makePathsAbsolute(paths, parent) {
         }
     });
 };
+
+/**
+ * we can later support setting folder names via custom config values
+ */
+exports.getContentPath = function getContentPath(type) {
+    switch (type) {
+        case 'storage':
+            return path.join(this.get('paths:contentPath'), 'storage/');
+        case 'images':
+            return path.join(this.get('paths:contentPath'), 'images/');
+        case 'apps':
+            return path.join(this.get('paths:contentPath'), 'apps/');
+        case 'themes':
+            return path.join(this.get('paths:contentPath'), 'themes/');
+        case 'scheduling':
+            return path.join(this.get('paths:contentPath'), 'scheduling/');
+        default:
+            throw new Error('getContentPath was called with: ' + type);
+    }
+};
diff --git a/core/server/data/importer/handlers/image.js b/core/server/data/importer/handlers/image.js
@@ -31,14 +31,14 @@ ImageHandler = {
 
             file.originalPath = noBaseDir;
             file.name = noGhostDirs;
-            file.targetDir = path.join(config.get('paths').imagesPath, path.dirname(noGhostDirs));
+            file.targetDir = path.join(config.getContentPath('images'), path.dirname(noGhostDirs));
             return file;
         });
 
         return Promise.map(files, function (image) {
             return store.getUniqueFileName(store, image, image.targetDir).then(function (targetFilename) {
                 image.newPath = (utils.url.getSubdir() + '/' +
-                    config.get('paths').imagesRelPath + '/' + path.relative(config.get('paths').imagesPath, targetFilename))
+                    config.get('paths').imagesRelPath + '/' + path.relative(config.getContentPath('images'), targetFilename))
                         .replace(new RegExp('\\' + path.sep, 'g'), '/');
 
                 return image;

diff --git a/core/server/data/validation/index.js b/core/server/data/validation/index.js
@@ -141,7 +141,7 @@ validateActiveTheme = function validateActiveTheme(themeName) {
         // A Promise that will resolve to an object with a property for each installed theme.
         // This is necessary because certain configuration data is only available while Ghost
         // is running and at times the validations are used when it's not (e.g. tests)
-        availableThemes = readThemes(config.get('paths').themePath);
+        availableThemes = readThemes(config.getContentPath('themes'));
     }
 
     return availableThemes.then(function then(themes) {

diff --git a/core/server/index.js b/core/server/index.js
@@ -72,7 +72,7 @@ function init(options) {
     // Initialize Internationalization
     i18n.init();
 
-    return readDirectory(config.get('paths').appPath).then(function loadThemes(result) {
+    return readDirectory(config.getContentPath('apps')).then(function loadThemes(result) {
         config.set('paths:availableApps', result);
         return api.themes.loadThemes();
     }).then(function () {
@@ -143,7 +143,7 @@ function init(options) {
         middleware(parentApp);
 
         // Log all theme errors and warnings
-        validateThemes(config.get('paths').themePath)
+        validateThemes(config.getContentPath('themes'))
             .catch(function (result) {
                 // TODO: change `result` to something better
                 result.errors.forEach(function (err) {
@@ -163,8 +163,9 @@ function init(options) {
         // scheduling module can create x schedulers with different adapters
         return scheduling.init({
             active: config.get('scheduling').active,
-            path: config.get('paths').schedulingPath,
-            apiUrl: utils.url.apiUrl()
+            apiUrl: utils.url.apiUrl(),
+            internalPath: config.get('paths').internalSchedulingPath,
+            contentPath: config.getContentPath('scheduling')
         });
     }).then(function () {
         return ghostServer;

diff --git a/core/server/middleware/static-theme.js b/core/server/middleware/static-theme.js
@@ -21,7 +21,7 @@ function forwardToExpressStatic(req, res, next) {
         next();
     } else {
         express.static(
-            path.join(config.get('paths').themePath, req.app.get('activeTheme')),
+            path.join(config.getContentPath('themes'), req.app.get('activeTheme')),
             {maxAge: utils.ONE_YEAR_MS}
         )(req, res, next);
     }

diff --git a/core/server/middleware/theme-handler.js b/core/server/middleware/theme-handler.js
@@ -41,8 +41,8 @@ themeHandler = {
 
         hbs.updateTemplateOptions({data: {blog: themeData, labs: labsData}});
 
-        if (config.get('paths').themePath && blogApp.get('activeTheme')) {
-            blogApp.set('views', path.join(config.get('paths').themePath, blogApp.get('activeTheme')));
+        if (config.getContentPath('themes') && blogApp.get('activeTheme')) {
+            blogApp.set('views', path.join(config.getContentPath('themes'), blogApp.get('activeTheme')));
         }
 
         // Pass 'secure' flag to the view engine
@@ -56,7 +56,7 @@ themeHandler = {
     // Helper for updateActiveTheme
     activateTheme: function activateTheme(blogApp, activeTheme) {
         var hbsOptions,
-            themePartials = path.join(config.get('paths').themePath, activeTheme, 'partials');
+            themePartials = path.join(config.getContentPath('themes'), activeTheme, 'partials');
 
         // clear the view cache
         blogApp.cache = {};

diff --git a/core/server/scheduling/utils.js b/core/server/scheduling/utils.js
@@ -8,7 +8,8 @@ exports.createAdapter = function (options) {
 
     var adapter = null,
         activeAdapter = options.active,
-        path = options.path;
+        internalPath = options.internalPath,
+        contentPath = options.contentPath;
 
     if (!activeAdapter) {
         return Promise.reject(new errors.IncorrectUsage('Please provide an active adapter.'));
@@ -29,10 +30,27 @@ exports.createAdapter = function (options) {
      * CASE: active adapter is located in specific ghost path
      */
     try {
-        adapter = adapter || new (require(path + activeAdapter))(options);
+        adapter = adapter || new (require(contentPath + activeAdapter))(options);
     } catch (err) {
+        // CASE: only throw error if module does exist
+        if (err.code !== 'MODULE_NOT_FOUND') {
+            return Promise.reject(new errors.IncorrectUsage(err.message));
+        }
+        // CASE: if module not found it can be an error within the adapter (cannot find bluebird for example)
+        else if (err.code === 'MODULE_NOT_FOUND' && err.message.indexOf(contentPath + activeAdapter) === -1) {
+            return Promise.reject(new errors.IncorrectUsage(err.message));
+        }
+    }
+
+    /**
+     * CASE: active adapter is located in internal ghost path
+     */
+    try {
+        adapter = adapter || new (require(internalPath + activeAdapter))(options);
+    } catch (err) {
+        // CASE: only throw error if module does exist
         if (err.code === 'MODULE_NOT_FOUND') {
-            return Promise.reject(new errors.IncorrectUsage('MODULE_NOT_FOUND', activeAdapter));
+            return Promise.reject(new errors.IncorrectUsage('We cannot find your adapter in: ' + contentPath + ' or: ' + internalPath));
         }
 
         return Promise.reject(new errors.IncorrectUsage(err.message));

diff --git a/core/server/storage/index.js b/core/server/storage/index.js
@@ -33,24 +33,24 @@ function getStorage(type) {
 
     // CASE: load adapter from custom path  (.../content/storage)
     try {
-        storage[storageChoice] = require(config.get('paths').storagePath.custom + storageChoice);
+        storage[storageChoice] = require(config.getContentPath('storage') + storageChoice);
     } catch (err) {
         // CASE: only throw error if module does exist
         if (err.code !== 'MODULE_NOT_FOUND') {
             throw new errors.IncorrectUsage(err.message);
         }
         // CASE: if module not found it can be an error within the adapter (cannot find bluebird for example)
-        else if (err.code === 'MODULE_NOT_FOUND' && err.message.indexOf(config.get('paths').storagePath.custom + storageChoice) === -1) {
+        else if (err.code === 'MODULE_NOT_FOUND' && err.message.indexOf(config.getContentPath('storage') + storageChoice) === -1) {
             throw new errors.IncorrectUsage(err.message);
         }
     }
 
     // CASE: either storage[storageChoice] is already set or why check for in the default storage path
     try {
-        storage[storageChoice] = storage[storageChoice] || require(config.get('paths').storagePath.default + storageChoice);
+        storage[storageChoice] = storage[storageChoice] || require(config.get('paths').internalStoragePath + storageChoice);
     } catch (err) {
         if (err.code === 'MODULE_NOT_FOUND') {
-            throw new errors.IncorrectUsage('We cannot find your adpter in: ' + config.get('paths').storagePath.custom + ' or: ' + config.get('paths').storagePath.default);
+            throw new errors.IncorrectUsage('We cannot find your adapter in: ' + config.getContentPath('storage') + ' or: ' + config.get('paths').internalStoragePath);
         } else {
             throw new errors.IncorrectUsage(err.message);
         }

diff --git a/core/server/storage/local-file-store.js b/core/server/storage/local-file-store.js
@@ -24,7 +24,7 @@ util.inherits(LocalFileStore, BaseStore);
 // - image is the express image object
 // - returns a promise which ultimately returns the full url to the uploaded image
 LocalFileStore.prototype.save = function (image, targetDir) {
-    targetDir = targetDir || this.getTargetDir(config.get('paths').imagesPath);
+    targetDir = targetDir || this.getTargetDir(config.getContentPath('images'));
     var targetFilename;
 
     return this.getUniqueFileName(this, image, targetDir).then(function (filename) {
@@ -35,8 +35,13 @@ LocalFileStore.prototype.save = function (image, targetDir) {
     }).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 = (utils.url.getSubdir() + '/' + config.get('paths').imagesRelPath + '/' +
-        path.relative(config.get('paths').imagesPath, targetFilename)).replace(new RegExp('\\' + path.sep, 'g'), '/');
+        var fullUrl = (
+            utils.url.getSubdir() + '/' +
+            config.get('paths').imagesRelPath +
+            '/' +
+            path.relative(config.getContentPath('images'), targetFilename)
+        ).replace(new RegExp('\\' + path.sep, 'g'), '/');
+
         return fullUrl;
     }).catch(function (e) {
         errors.logError(e);
@@ -63,7 +68,7 @@ LocalFileStore.prototype.serve = function (options) {
     if (options.isTheme) {
         return function downloadTheme(req, res, next) {
             var themeName = options.name,
-                themePath = path.join(config.get('paths').themePath, themeName),
+                themePath = path.join(config.getContentPath('themes'), themeName),
                 zipName = themeName + '.zip',
                 // store this in a unique temporary folder
                 zipBasePath = path.join(os.tmpdir(), utils.uid(10)),
@@ -95,12 +100,12 @@ LocalFileStore.prototype.serve = function (options) {
         // CASE: serve images
         // For some reason send divides the max age number by 1000
         // Fallthrough: false ensures that if an image isn't found, it automatically 404s
-        return serveStatic(config.get('paths').imagesPath, {maxAge: utils.ONE_YEAR_MS, fallthrough: false});
+        return serveStatic(config.getContentPath('images'), {maxAge: utils.ONE_YEAR_MS, fallthrough: false});
     }
 };
 
 LocalFileStore.prototype.delete = function (fileName, targetDir) {
-    targetDir = targetDir || this.getTargetDir(config.get('paths').imagesPath);
+    targetDir = targetDir || this.getTargetDir(config.getContentPath('images'));
 
     var pathToDelete = path.join(targetDir, fileName);
     return remove(pathToDelete);

diff --git a/core/test/functional/routes/api/themes_spec.js b/core/test/functional/routes/api/themes_spec.js
@@ -42,8 +42,8 @@ describe('Themes API', function () {
 
     after(function (done) {
         // clean successful uploaded themes
-        fs.removeSync(config.get('paths').themePath + '/valid');
-        fs.removeSync(config.get('paths').themePath + '/casper.zip');
+        fs.removeSync(config.getContentPath('themes') + '/valid');
+        fs.removeSync(config.getContentPath('themes') + '/casper.zip');
 
         // gscan creates /test/tmp in test mode
         fs.removeSync(config.get('paths').appRoot + '/test');
@@ -92,7 +92,7 @@ describe('Themes API', function () {
                             }
 
                             // ensure contains two files (zip and extracted theme)
-                            fs.readdirSync(config.get('paths').themePath).join().match(/valid/gi).length.should.eql(1);
+                            fs.readdirSync(config.getContentPath('themes')).join().match(/valid/gi).length.should.eql(1);
                             done();
                         });
                 });
@@ -139,8 +139,8 @@ describe('Themes API', function () {
                         return done(err);
                     }
 
-                    fs.existsSync(config.get('paths').themePath + '/valid').should.eql(false);
-                    fs.existsSync(config.get('paths').themePath + '/valid.zip').should.eql(false);
+                    fs.existsSync(config.getContentPath('themes') + '/valid').should.eql(false);
+                    fs.existsSync(config.getContentPath('themes') + '/valid.zip').should.eql(false);
                     done();
                 });
         });

diff --git a/core/test/unit/config_spec.js b/core/test/unit/config_spec.js
@@ -94,13 +94,11 @@ describe('Config', function () {
             // This will fail if there are any extra keys
             pathConfig.should.have.keys(
                 'appRoot',
-                'storagePath',
+                'internalStoragePath',
+                'internalSchedulingPath',
                 'contentPath',
                 'corePath',
-                'themePath',
-                'appPath',
                 'internalAppPath',
-                'imagesPath',
                 'imagesRelPath',
                 'adminViews',
                 'helperTemplates',
@@ -118,25 +116,15 @@ describe('Config', function () {
         it('should allow specific properties to be user defined', function () {
             var contentPath = path.join(config.get('paths').appRoot, 'otherContent', '/');
 
-            configUtils.set({
-                paths: {
-                    contentPath: contentPath
-                }
-            });
-
+            configUtils.set('paths:contentPath', contentPath);
             config.get('paths').should.have.property('contentPath', contentPath);
-            config.get('paths').should.have.property('themePath', contentPath + 'themes');
-            config.get('paths').should.have.property('appPath', contentPath + 'apps');
-            config.get('paths').should.have.property('imagesPath', contentPath + 'images');
+            config.getContentPath('images').should.eql(contentPath + 'images/');
         });
     });
 
     describe('Storage', function () {
         it('should default to local-file-store', function () {
-            config.get('paths').should.have.property('storagePath', {
-                default: path.join(config.get('paths').corePath, '/server/storage/'),
-                custom:  path.join(config.get('paths').contentPath, 'storage/')
-            });
+            config.get('paths').should.have.property('internalStoragePath', path.join(config.get('paths').corePath, '/server/storage/'));
 
             config.get('storage').should.have.property('active', {
                 images: 'local-file-store',