From fe90cf2be249096fa7ccca5e539073b1a4af1fed Mon Sep 17 00:00:00 2001
From: Hannah Wolfe <github.erisds@gmail.com>
Date: Tue, 21 Feb 2017 23:26:19 +0000
Subject: [PATCH] Theme loading part 1 (#7989)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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 | 13 +-----
core/server/api/themes.js | 5 ++-
core/server/data/validation/index.js | 28 ++++---------
core/server/ghost-server.js | 3 ++
core/server/index.js | 11 ++---
core/server/themes/index.js | 6 +++
core/server/themes/loader.js | 39 +++++++++++++++++
.../{utils/read-themes.js => themes/read.js} | 32 ++++++++++----
core/server/utils/index.js | 2 +-
.../test/functional/routes/api/themes_spec.js | 2 +-
.../test/integration/api/api_settings_spec.js | 28 +++++++------
core/test/unit/server_utils_spec.js | 33 ---------------
core/test/unit/themes_spec.js | 42 +++++++++++++++++++
core/test/utils/index.js | 4 +-
14 files changed, 150 insertions(+), 98 deletions(-)
create mode 100644 core/server/themes/loader.js
rename core/server/{utils/read-themes.js => themes/read.js} (54%)
create mode 100644 core/test/unit/themes_spec.js
diff --git a/core/server/api/settings.js b/core/server/api/settings.js
index 28532f39ad..22636d207a 100644
--- a/core/server/api/settings.js
+++ b/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 () {
diff --git a/core/server/api/themes.js b/core/server/api/themes.js
index 2bafb7aa55..88a3e28ee5 100644
--- a/core/server/api/themes.js
+++ b/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();
diff --git a/core/server/data/validation/index.js b/core/server/data/validation/index.js
index 2ef0fc3110..78a3e57bf5 100644
--- a/core/server/data/validation/index.js
+++ b/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.
diff --git a/core/server/ghost-server.js b/core/server/ghost-server.js
index 6e55e95c9f..65e665b010 100644
--- a/core/server/ghost-server.js
+++ b/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);
diff --git a/core/server/index.js b/core/server/index.js
index aa17f91a93..f866f25c41 100644
--- a/core/server/index.js
+++ b/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
diff --git a/core/server/themes/index.js b/core/server/themes/index.js
index e69de29bb2..43a9e87e10 100644
--- a/core/server/themes/index.js
+++ b/core/server/themes/index.js
@@ -0,0 +1,6 @@
+var themeLoader = require('./loader');
+
+module.exports = {
+ init: themeLoader.init,
+ load: themeLoader.load
+};
diff --git a/core/server/themes/loader.js b/core/server/themes/loader.js
new file mode 100644
index 0000000000..38ff7f67e3
--- /dev/null
+++ b/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
+};
diff --git a/core/server/utils/read-themes.js b/core/server/themes/read.js
similarity index 54%
rename from core/server/utils/read-themes.js
rename to core/server/themes/read.js
index 295d16ba88..881be08f14 100644
--- a/core/server/utils/read-themes.js
+++ b/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;
diff --git a/core/server/utils/index.js b/core/server/utils/index.js
index 9bcf9dd3d9..0d83df01c9 100644
--- a/core/server/utils/index.js
+++ b/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'),
diff --git a/core/test/functional/routes/api/themes_spec.js b/core/test/functional/routes/api/themes_spec.js
index 5c501fc7a5..8061e82daf 100644
--- a/core/test/functional/routes/api/themes_spec.js
+++ b/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) {
diff --git a/core/test/integration/api/api_settings_spec.js b/core/test/integration/api/api_settings_spec.js
index 327bb1e423..0ddd2d7a15 100644
--- a/core/test/integration/api/api_settings_spec.js
+++ b/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');
+ });
+ });
+ });
});
diff --git a/core/test/unit/server_utils_spec.js b/core/test/unit/server_utils_spec.js
index 11fae514a6..0a28bc1fff 100644
--- a/core/test/unit/server_utils_spec.js
+++ b/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);
diff --git a/core/test/unit/themes_spec.js b/core/test/unit/themes_spec.js
new file mode 100644
index 0000000000..4444ebf61b
--- /dev/null
+++ b/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);
+ });
+ });
+});
diff --git a/core/test/utils/index.js b/core/test/utils/index.js
index c94581aca4..f4f904d1c0 100644
--- a/core/test/utils/index.js
+++ b/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(); }
};
/**