🔥✨ No more availableThemes (#8085)

no issue

🎨 Switch themes API to use config.availableThemes
- this gets rid of the only places where settings.availableThemes are used

🔥 Get rid of settings.availableThemes
- this is no longer used anywhere
- also get rid of every related call to updateSettingsCache

🔥 Replace config.availableThemes with theme cache
- Creates a tailor-made in-memory cache for themes inside the theme module
- Add methods for getting & setting items on the cache
- Move all references to config.availableThemes to use the new cache
- This can be abstracted later to support other kinds of caches?

🎨 Start improving theme lib's API
Still TODO: simplifying/clarifying:
- what is the structure of the internal list
- what is the difference between a package list, and a theme list?
- what is the difference between reading a theme and loading it?
- how do we update the theme list (add/remove)
- how do we refresh the theme list? (hot reload?!)
- how do we get from an internal list, to one that is sent as part of the API?
- how are we going to handle theme storage: read/write, such that the path is configurable

🎨 Use themeList consistently
🎨 Update list after storage

core/server/api/settings.js

@@ -3,12 +3,10 @@
 var _            = require('lodash'),
     dataProvider = require('../models'),
     Promise      = require('bluebird'),
-    config       = require('../config'),
     canThis      = require('../permissions').canThis,
     errors       = require('../errors'),
     utils        = require('./utils'),
     i18n         = require('../i18n'),
-    filterPackages = require('../utils/packages').filterPackages,
 
     docName      = 'settings',
     settings,
@@ -68,6 +66,17 @@ settingsFilter = function (settings, filter) {
 
 /**
  * ### Read Settings Result
+ *
+ * Converts the models to keyed JSON
+ * E.g.
+ * dbHash: {
+ *   id: '123abc',
+ *   key: 'dbash',
+ *   value: 'xxxx',
+ *   type: 'core',
+ *   timestamps
+ *  }
+ *
  * @private
  * @param {Array} settingsModels
  * @returns {Settings}
@@ -79,20 +88,7 @@ readSettingsResult = function (settingsModels) {
             }
 
             return memo;
-        }, {}),
-        themes = config.get('paths').availableThemes,
-        res;
-
-    // @TODO: remove availableThemes from settings cache and create an endpoint to fetch themes
-    if (settings.activeTheme && themes) {
-        res = filterPackages(themes, settings.activeTheme.value);
-
-        settings.availableThemes = {
-            key: 'availableThemes',
-            value: res,
-            type: 'theme'
-        };
-    }
+        }, {});
 
     return settings;
 };
@@ -255,7 +251,7 @@ settings = {
         }
 
         object.settings = _.reject(object.settings, function (setting) {
-            return setting.key === 'type' || setting.key === 'availableThemes';
+            return setting.key === 'type';
         });
 
         return canEditAllSettings(object.settings, options).then(function () {

core/server/api/themes.js

@@ -1,6 +1,7 @@
 // # Themes API
 // RESTful API for Themes
-var Promise = require('bluebird'),
+var debug = require('debug')('ghost:api:themes'),
+    Promise = require('bluebird'),
     _ = require('lodash'),
     gscan = require('gscan'),
     fs = require('fs-extra'),
@@ -9,12 +10,12 @@ var Promise = require('bluebird'),
     events = require('../events'),
     logging = require('../logging'),
     storage = require('../storage'),
-    settings = require('./settings'),
-    settingsCache = require('../settings/cache'),
     apiUtils = require('./utils'),
     utils = require('./../utils'),
     i18n = require('../i18n'),
     themeUtils = require('../themes'),
+    themeList = themeUtils.list,
+    packageUtils = require('../utils/packages'),
     themes;
 
 /**
@@ -24,7 +25,10 @@ var Promise = require('bluebird'),
  */
 themes = {
     browse: function browse() {
-        return Promise.resolve({themes: settingsCache.get('availableThemes')});
+        debug('browsing');
+        var result = packageUtils.filterPackages(themeList.getAll());
+        debug('got result');
+        return Promise.resolve({themes: result});
     },
 
     upload: function upload(options) {
@@ -80,23 +84,18 @@ themes = {
                 }, config.getContentPath('themes'));
             })
             .then(function () {
-                // force reload of availableThemes
-                // right now the logic is in the ConfigManager
-                // if we create a theme collection, we don't have to read them from disk
-                return themeUtils.load();
+                return themeUtils.loadOne(zip.shortName);
             })
-            .then(function () {
-                // the settings endpoint is used to fetch the availableThemes
-                // so we have to force updating the in process cache
-                return settings.updateSettingsCache();
-            })
-            .then(function (settings) {
+            .then(function (themeObject) {
+                // @TODO fix this craziness
+                var toFilter = {};
+                toFilter[zip.shortName] = themeObject;
+                themeObject = packageUtils.filterPackages(toFilter);
                 // gscan theme structure !== ghost theme structure
-                var themeObject = _.find(settings.availableThemes.value, {name: zip.shortName}) || {};
                 if (theme.results.warning.length > 0) {
                     themeObject.warnings = _.cloneDeep(theme.results.warning);
                 }
-                return {themes: [themeObject]};
+                return {themes: themeObject};
             })
             .finally(function () {
                 // remove zip upload from multer
@@ -119,7 +118,7 @@ themes = {
 
     download: function download(options) {
         var themeName = options.name,
-            theme = config.get('paths').availableThemes[themeName],
+            theme = themeList.get(themeName),
             storageAdapter = storage.getStorage('themes');
 
         if (!theme) {
@@ -148,20 +147,17 @@ themes = {
                     throw new errors.ValidationError({message: i18n.t('errors.api.themes.destroyCasper')});
                 }
 
-                theme = config.get('paths').availableThemes[name];
+                theme = themeList.get(name);
 
                 if (!theme) {
                     throw new errors.NotFoundError({message: i18n.t('errors.api.themes.themeDoesNotExist')});
                 }
 
-                events.emit('theme.deleted', name);
                 return storageAdapter.delete(name, config.getContentPath('themes'));
             })
             .then(function () {
-                return themeUtils.load();
-            })
-            .then(function () {
-                return settings.updateSettingsCache();
+                themeList.del(name);
+                events.emit('theme.deleted', name);
             });
     }
 };

core/server/apps/amp/tests/router_spec.js

@@ -7,6 +7,7 @@ var rewire            = require('rewire'),
     errors            = require('../../../errors'),
     should            = require('should'),
     configUtils       = require('../../../../test/utils/configUtils'),
+    themeList        = require('../../../themes').list,
     sandbox           = sinon.sandbox.create();
 
 // Helper function to prevent unit tests
@@ -53,10 +54,11 @@ describe('AMP Controller', function () {
     afterEach(function () {
         sandbox.restore();
         configUtils.restore();
+        themeList.init();
     });
 
     it('should render default amp page when theme has no amp template', function (done) {
-        configUtils.set({paths: {availableThemes: {casper: {}}}});
+        themeList.init({casper: {}});
 
         setResponseContextStub = sandbox.stub();
         ampController.__set__('setResponseContext', setResponseContextStub);
@@ -70,9 +72,9 @@ describe('AMP Controller', function () {
     });
 
     it('should render theme amp page when theme has amp template', function (done) {
-        configUtils.set({paths: {availableThemes: {casper: {
+        themeList.init({casper: {
             'amp.hbs': '/content/themes/casper/amp.hbs'
-        }}}});
+        }});
 
         setResponseContextStub = sandbox.stub();
         ampController.__set__('setResponseContext', setResponseContextStub);
@@ -86,7 +88,7 @@ describe('AMP Controller', function () {
     });
 
     it('should render with error when error is passed in', function (done) {
-        configUtils.set({paths: {availableThemes: {casper: {}}}});
+        themeList.init({casper: {}});
         res.error = 'Test Error';
 
         setResponseContextStub = sandbox.stub();
@@ -103,7 +105,7 @@ describe('AMP Controller', function () {
 
     it('does not render amp page when amp context is missing', function (done) {
         var renderSpy;
-        configUtils.set({paths: {availableThemes: {casper: {}}}});
+        themeList.init({casper: {}});
 
         setResponseContextStub = sandbox.stub();
         ampController.__set__('setResponseContext', setResponseContextStub);
@@ -121,7 +123,7 @@ describe('AMP Controller', function () {
 
     it('does not render amp page when context is other than amp and post', function (done) {
         var renderSpy;
-        configUtils.set({paths: {availableThemes: {casper: {}}}});
+        themeList.init({casper: {}});
 
         setResponseContextStub = sandbox.stub();
         ampController.__set__('setResponseContext', setResponseContextStub);

core/server/apps/private-blogging/tests/controller_spec.js

@@ -3,6 +3,7 @@ var privateController = require('../lib/router').controller,
     path              = require('path'),
     sinon             = require('sinon'),
     configUtils       = require('../../../../test/utils/configUtils'),
+    themeList        = require('../../../themes').list,
     sandbox = sinon.sandbox.create();
 
 describe('Private Controller', function () {
@@ -42,10 +43,11 @@ describe('Private Controller', function () {
     afterEach(function () {
         sandbox.restore();
         configUtils.restore();
+        themeList.init();
     });
 
     it('Should render default password page when theme has no password template', function (done) {
-        configUtils.set({paths: {availableThemes: {casper: {}}}});
+        themeList.init({casper: {}});
 
         res.render = function (view) {
             view.should.eql(defaultPath);
@@ -56,9 +58,9 @@ describe('Private Controller', function () {
     });
 
     it('Should render theme password page when it exists', function (done) {
-        configUtils.set({paths: {availableThemes: {casper: {
+        themeList.init({casper: {
             'private.hbs': '/content/themes/casper/private.hbs'
-        }}}});
+        }});
 
         res.render = function (view) {
             view.should.eql('private');
@@ -69,7 +71,7 @@ describe('Private Controller', function () {
     });
 
     it('Should render with error when error is passed in', function (done) {
-        configUtils.set({paths: {availableThemes: {casper: {}}}});
+        themeList.init({casper: {}});
         res.error = 'Test Error';
 
         res.render = function (view, context) {

core/server/controllers/frontend/templates.js

@@ -2,11 +2,11 @@
 //
 // Figure out which template should be used to render a request
 // based on the templates which are allowed, and what is available in the theme
-var _      = require('lodash'),
-    config = require('../../config');
+var _ = require('lodash'),
+    themeList = require('../../themes').list;
 
 function getActiveThemePaths(activeTheme) {
-    return config.get('paths').availableThemes[activeTheme];
+    return themeList.get(activeTheme);
 }
 
 /**

core/server/middleware/theme-handler.js

@@ -9,6 +9,7 @@ var _      = require('lodash'),
     logging = require('../logging'),
     errors = require('../errors'),
     i18n = require('../i18n'),
+    themeList = require('../themes').list,
     themeHandler;
 
 themeHandler = {
@@ -95,7 +96,7 @@ themeHandler = {
             // Check if the theme changed
             if (activeTheme.value !== blogApp.get('activeTheme')) {
                 // Change theme
-                if (!config.get('paths').availableThemes.hasOwnProperty(activeTheme.value)) {
+                if (!themeList.get(activeTheme.value)) {
                     if (!res.isAdmin) {
                         return next(new errors.NotFoundError({
                             message: i18n.t('errors.middleware.themehandler.missingTheme', {theme: activeTheme.value})

core/server/themes/index.js

@@ -1,7 +1,11 @@
 var themeLoader = require('./loader');
 
+// @TODO: reduce the amount of things we expose to the outside world
+// Make this a nice clean sensible API we can all understand!
 module.exports = {
     init: themeLoader.init,
-    load: themeLoader.load,
+    loadAll: themeLoader.loadAllThemes,
+    loadOne: themeLoader.loadOneTheme,
+    list: require('./list'),
     validate: require('./validate')
 };

core/server/themes/list.js

@@ -0,0 +1,36 @@
+/**
+ * Store themes after loading them from the file system
+ */
+var _ = require('lodash'),
+    themeListCache = {};
+
+module.exports = {
+    get: function get(key) {
+        return themeListCache[key];
+    },
+
+    getAll: function getAll() {
+        return themeListCache;
+    },
+
+    set: function set(key, theme) {
+        themeListCache[key] = _.cloneDeep(theme);
+        return themeListCache[key];
+    },
+
+    del: function del(key) {
+        delete themeListCache[key];
+    },
+
+    init: function init(themes) {
+        var self = this;
+        // First, reset the cache
+        themeListCache = {};
+        // For each theme, call set. Allows us to do processing on set later.
+        _.each(themes, function (theme, key) {
+            self.set(key, theme);
+        });
+
+        return themeListCache;
+    }
+};