Theme loading part 1 (#7989)

no issue

* ✨ Add new server start & stop events
* 🔥 Get rid of unused availableApps concept
- when we need an API endpoint for a list of apps, we'll build one 😝
* ✨ Move theme loading into a module
- move loading from API method to a module method and use as needed
- wire up read one vs read all as per LTS
- read one (the active theme) on boot, and read the rest after
- fudge validation - this isn't all that helpful
* Settings API tests need to preload themes
- this used to automatically happen as part of loading settings
- now we need to trigger this to happen specifically for this test

core/server/api/settings.js

@@ -136,7 +136,6 @@ readSettingsResult = function (settingsModels) {
             return memo;
         }, {}),
         themes = config.get('paths').availableThemes,
-        apps = config.get('paths').availableApps,
         res;
 
     // @TODO: remove availableThemes from settings cache and create an endpoint to fetch themes
@@ -150,16 +149,6 @@ readSettingsResult = function (settingsModels) {
         };
     }
 
-    if (settings.activeApps && apps) {
-        res = filterPaths(apps, JSON.parse(settings.activeApps.value));
-
-        settings.availableApps = {
-            key: 'availableApps',
-            value: res,
-            type: 'app'
-        };
-    }
-
     return settings;
 };
 
@@ -366,7 +355,7 @@ settings = {
         }
 
         object.settings = _.reject(object.settings, function (setting) {
-            return setting.key === 'type' || setting.key === 'availableThemes' || setting.key === 'availableApps';
+            return setting.key === 'type' || setting.key === 'availableThemes';
         });
 
         return canEditAllSettings(object.settings, options).then(function () {

core/server/api/themes.js

@@ -13,6 +13,7 @@ var Promise = require('bluebird'),
     apiUtils = require('./utils'),
     utils = require('./../utils'),
     i18n = require('../i18n'),
+    themeUtils = require('../themes'),
     themes;
 
 /**
@@ -88,7 +89,7 @@ themes = {
                 // 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 themes.loadThemes();
+                return themeUtils.load();
             })
             .then(function () {
                 // the settings endpoint is used to fetch the availableThemes
@@ -163,7 +164,7 @@ themes = {
                 return storageAdapter.delete(name, config.getContentPath('themes'));
             })
             .then(function () {
-                return themes.loadThemes();
+                return themeUtils.load();
             })
             .then(function () {
                 return settings.updateSettingsCache();

core/server/data/validation/index.js

@@ -6,15 +6,12 @@ var schema    = require('../schema').tables,
     Promise   = require('bluebird'),
     errors    = require('../../errors'),
     config    = require('../../config'),
-    readThemes  = require('../../utils/read-themes'),
     i18n        = require('../../i18n'),
 
     validateSchema,
     validateSettings,
     validateActiveTheme,
-    validate,
-
-    availableThemes;
+    validate;
 
 function assertString(input) {
     assert(typeof input === 'string', 'Validator js validates strings only');
@@ -131,24 +128,17 @@ validateSettings = function validateSettings(defaultSettings, model) {
 };
 
 validateActiveTheme = function validateActiveTheme(themeName) {
-    // If Ghost is running and its availableThemes collection exists
-    // give it priority.
-    if (config.get('paths').availableThemes && Object.keys(config.get('paths').availableThemes).length > 0) {
-        availableThemes = Promise.resolve(config.get('paths').availableThemes);
+    // @TODO come up with something way better here - we should probably attempt to read the theme from the
+    // File system at this point and validate the theme using gscan rather than just checking if it's in a cache object
+    if (!config.get('paths').availableThemes || Object.keys(config.get('paths').availableThemes).length === 0) {
+        // We haven't yet loaded all themes, this is probably being called early?
+        return Promise.resolve();
     }
 
-    if (!availableThemes) {
-        // 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.getContentPath('themes'));
+    // Else, if we have a list, check if the theme is in it
+    if (!config.get('paths').availableThemes.hasOwnProperty(themeName)) {
+        return Promise.reject(new errors.ValidationError({message: i18n.t('notices.data.validation.index.themeCannotBeActivated', {themeName: themeName}), context: 'activeTheme'}));
     }
-
-    return availableThemes.then(function then(themes) {
-        if (!themes.hasOwnProperty(themeName)) {
-            return Promise.reject(new errors.ValidationError({message: i18n.t('notices.data.validation.index.themeCannotBeActivated', {themeName: themeName}), context: 'activeTheme'}));
-        }
-    });
 };
 
 // Validate default settings using the validator module.

core/server/ghost-server.js

@@ -7,6 +7,7 @@ var debug = require('debug')('ghost:server'),
     path = require('path'),
     _ = require('lodash'),
     errors = require('./errors'),
+    events = require('./events'),
     config = require('./config'),
     utils = require('./utils'),
     i18n   = require('./i18n'),
@@ -96,6 +97,7 @@ GhostServer.prototype.start = function (externalApp) {
         self.httpServer.on('connection', self.connection.bind(self));
         self.httpServer.on('listening', function () {
             debug('...Started');
+            events.emit('server:start');
             self.logStartMessages();
             resolve(self);
         });
@@ -116,6 +118,7 @@ GhostServer.prototype.stop = function () {
             resolve(self);
         } else {
             self.httpServer.close(function () {
+                events.emit('server:stop');
                 self.httpServer = null;
                 self.logShutdownMessages();
                 resolve(self);

core/server/index.js

@@ -29,7 +29,7 @@ var debug = require('debug')('ghost:boot:init'),
     slack = require('./data/slack'),
     GhostServer = require('./ghost-server'),
     scheduling = require('./scheduling'),
-    readDirectory = require('./utils/read-directory'),
+    themes = require('./themes'),
     utils = require('./utils');
 
 // ## Initialise Ghost
@@ -44,13 +44,7 @@ function init() {
     models.init();
     debug('models done');
 
-    return readDirectory(config.getContentPath('apps')).then(function loadThemes(result) {
-        config.set('paths:availableApps', result);
-        return api.themes.loadThemes();
-    }).then(function () {
-        debug('Themes & apps done');
-        return dbHealth.check();
-    }).then(function () {
+    return dbHealth.check().then(function () {
         debug('DB health check done');
         // Populate any missing default settings
         return models.Settings.populateDefaults();
@@ -65,6 +59,7 @@ function init() {
     }).then(function () {
         debug('Permissions done');
         return Promise.join(
+            themes.init(),
             // Initialize apps
             apps.init(),
             // Initialize xmrpc ping

core/server/themes/index.js

@@ -0,0 +1,6 @@
+var themeLoader = require('./loader');
+
+module.exports = {
+    init: themeLoader.init,
+    load: themeLoader.load
+};

core/server/themes/loader.js

@@ -0,0 +1,39 @@
+var debug = require('debug')('ghost:themes:loader'),
+    config = require('../config'),
+    events = require('../events'),
+    read = require('./read'),
+    settingsApi = require('../api/settings'),
+    updateConfigAndCache,
+    loadThemes,
+    initThemes;
+
+updateConfigAndCache = function updateConfigAndCache(themes) {
+    debug('loading themes', themes);
+    config.set('paths:availableThemes', themes);
+    settingsApi.updateSettingsCache();
+};
+
+loadThemes = function loadThemes() {
+    return read
+        .all(config.getContentPath('themes'))
+        .then(updateConfigAndCache);
+};
+
+initThemes = function initThemes() {
+    debug('init themes', settingsApi.cache.get('activeTheme'));
+
+    // Register a listener for server-start to load all themes
+    events.on('server:start', function readAllThemesOnServerStart() {
+        loadThemes();
+    });
+
+    // Just read the active theme for now
+    return read
+        .one(config.getContentPath('themes'), settingsApi.cache.get('activeTheme'))
+        .then(updateConfigAndCache);
+};
+
+module.exports = {
+    init: initThemes,
+    load: loadThemes
+};

core/server/themes/read.js

@@ -2,18 +2,31 @@
  * Dependencies
  */
 
-var readDirectory = require('./read-directory'),
+var readDirectory = require('../utils').readDirectory,
     Promise = require('bluebird'),
+    _ = require('lodash'),
     join = require('path').join,
     fs = require('fs'),
 
-    statFile = Promise.promisify(fs.stat);
+    statFile = Promise.promisify(fs.stat),
+    readOneTheme,
+    readAllThemes;
 
-/**
- * Read themes
- */
+readOneTheme = function readOneTheme(dir, name) {
+    var toRead = join(dir, name),
+        themes = {};
 
-function readThemes(dir) {
+    return readDirectory(toRead)
+        .then(function (tree) {
+            if (!_.isEmpty(tree)) {
+                themes[name] = tree;
+            }
+
+            return themes;
+        });
+};
+
+readAllThemes = function readAllThemes(dir) {
     var originalTree;
 
     return readDirectory(dir)
@@ -37,10 +50,11 @@ function readThemes(dir) {
 
             return themes;
         });
-}
+};
 
 /**
- * Expose `read-themes`
+ * Expose public API
  */
 
-module.exports = readThemes;
+module.exports.all = readAllThemes;
+module.exports.one = readOneTheme;

core/server/utils/index.js

@@ -104,9 +104,9 @@ utils = {
     },
 
     readCSV: require('./read-csv'),
+    readDirectory: require('./read-directory'),
     removeOpenRedirectFromUrl: require('./remove-open-redirect-from-url'),
     zipFolder: require('./zip-folder'),
-    readThemes: require('./read-themes'),
     generateAssetHash: require('./asset-hash'),
     url: require('./url'),
     tokens: require('./tokens'),

core/test/functional/routes/api/themes_spec.js

@@ -108,7 +108,7 @@ describe('Themes API', function () {
                 });
         });
 
-        it('get all available themes', function (done) {
+        it('get all available themes + new theme', function (done) {
             request.get(testUtils.API.getApiQuery('settings/'))
                 .set('Authorization', 'Bearer ' + scope.ownerAccessToken)
                 .end(function (err, res) {

core/test/integration/api/api_settings_spec.js

@@ -164,18 +164,6 @@ describe('Settings API', function () {
         });
     });
 
-    it('does not allow an active theme which is not installed', function () {
-        return callApiWithContext(defaultContext, 'edit', 'activeTheme', {
-            settings: [{key: 'activeTheme', value: 'rasper'}]
-        }).then(function () {
-            throw new Error('Allowed to set an active theme which is not installed');
-        }).catch(function (err) {
-            should.exist(err);
-
-            err.errorType.should.eql('ValidationError');
-        });
-    });
-
     it('set activeTimezone: unknown timezone', function () {
         return callApiWithContext(defaultContext, 'edit', {settings: [{key: 'activeTimezone', value: 'MFG'}]}, {})
             .then(function () {
@@ -190,4 +178,20 @@ describe('Settings API', function () {
     it('set activeTimezone: known timezone', function () {
         return callApiWithContext(defaultContext, 'edit', {settings: [{key: 'activeTimezone', value: 'Etc/UTC'}]}, {});
     });
+
+    describe('Themes (to be removed from settings)', function () {
+        beforeEach(testUtils.setup('themes'));
+
+        it('does not allow an active theme which is not installed', function () {
+            return callApiWithContext(defaultContext, 'edit', 'activeTheme', {
+                settings: [{key: 'activeTheme', value: 'rasper'}]
+            }).then(function () {
+                throw new Error('Allowed to set an active theme which is not installed');
+            }).catch(function (err) {
+                should.exist(err);
+
+                err.errorType.should.eql('ValidationError');
+            });
+        });
+    });
 });

core/test/unit/server_utils_spec.js

@@ -7,7 +7,6 @@ var should          = require('should'),
     configUtils     = require('../utils/configUtils'),
     parsePackageJson = require('../../server/utils/parse-package-json'),
     readDirectory   = require('../../server/utils/read-directory'),
-    readThemes      = require('../../server/utils/read-themes'),
     gravatar        = require('../../server/utils/gravatar'),
     utils           = require('../../server/utils');
 
@@ -335,38 +334,6 @@ describe('Server Utilities', function () {
         });
     });
 
-    describe('read-themes', function () {
-        it('should read directory and include only folders', function (done) {
-            var themesPath = tmp.dirSync({unsafeCleanup: true});
-
-            // create trash
-            fs.writeFileSync(join(themesPath.name, 'casper.zip'));
-            fs.writeFileSync(join(themesPath.name, '.DS_Store'));
-
-            // create actual theme
-            fs.mkdirSync(join(themesPath.name, 'casper'));
-            fs.mkdirSync(join(themesPath.name, 'casper', 'partials'));
-            fs.writeFileSync(join(themesPath.name, 'casper', 'index.hbs'));
-            fs.writeFileSync(join(themesPath.name, 'casper', 'partials', 'navigation.hbs'));
-
-            readThemes(themesPath.name)
-                .then(function (tree) {
-                    tree.should.eql({
-                        casper: {
-                            partials: {
-                                'navigation.hbs': join(themesPath.name, 'casper', 'partials', 'navigation.hbs')
-                            },
-                            'index.hbs': join(themesPath.name, 'casper', 'index.hbs')
-                        }
-                    });
-
-                    done();
-                })
-                .catch(done)
-                .finally(themesPath.removeCallback);
-        });
-    });
-
     describe('gravatar-lookup', function () {
         beforeEach(function () {
             configUtils.set('privacy:useGravatar', true);

core/test/unit/themes_spec.js

@@ -0,0 +1,42 @@
+var should          = require('should'),
+    fs              = require('fs'),
+    tmp             = require('tmp'),
+    join            = require('path').join,
+    readThemes      = require('../../server/themes/read');
+
+// To stop jshint complaining
+should.equal(true, true);
+
+describe('Themes', function () {
+    describe('Read', function () {
+        it('should read directory and include only folders', function (done) {
+            var themesPath = tmp.dirSync({unsafeCleanup: true});
+
+            // create trash
+            fs.writeFileSync(join(themesPath.name, 'casper.zip'));
+            fs.writeFileSync(join(themesPath.name, '.DS_Store'));
+
+            // create actual theme
+            fs.mkdirSync(join(themesPath.name, 'casper'));
+            fs.mkdirSync(join(themesPath.name, 'casper', 'partials'));
+            fs.writeFileSync(join(themesPath.name, 'casper', 'index.hbs'));
+            fs.writeFileSync(join(themesPath.name, 'casper', 'partials', 'navigation.hbs'));
+
+            readThemes.all(themesPath.name)
+                .then(function (tree) {
+                    tree.should.eql({
+                        casper: {
+                            partials: {
+                                'navigation.hbs': join(themesPath.name, 'casper', 'partials', 'navigation.hbs')
+                            },
+                            'index.hbs': join(themesPath.name, 'casper', 'index.hbs')
+                        }
+                    });
+
+                    done();
+                })
+                .catch(done)
+                .finally(themesPath.removeCallback);
+        });
+    });
+});

core/test/utils/index.js

@@ -15,6 +15,7 @@ var Promise       = require('bluebird'),
     SettingsAPI   = require('../../server/api/settings'),
     permissions   = require('../../server/permissions'),
     sequence      = require('../../server/utils/sequence'),
+    themes        = require('../../server/themes'),
     DataGenerator = require('./fixtures/data-generator'),
     filterData    = require('./fixtures/filter-param'),
     API           = require('./api'),
@@ -449,7 +450,8 @@ toDoList = {
     },
     clients: function insertClients() { return fixtures.insertClients(); },
     filter: function createFilterParamFixtures() { return filterData(DataGenerator); },
-    invites: function insertInvites() { return fixtures.insertInvites(); }
+    invites: function insertInvites() { return fixtures.insertInvites(); },
+    themes: function loadThemes() { return themes.load(); }
 };
 
 /**