Skip to content

Commit 6ea2c48

Browse files
ErisDSkirrg001
authored andcommitted
🎨 Optimise theme load & remove validation (#7970)
closes #7854 - Remove the concept of validating themes. This can now be done with gscan - Delay the loading of themes until after the server has started - Use events to trigger behaviour after the server has started - Load the active theme first - Don't load apps - this is totally unneeded - this is not used anywhere at present - the framework/lifecycle for apps works without it - this was intended for use in an app listing and isn't needed at all NOTE: this is pretty hacky, intended to slightly improve LTS boot times. A better solution should be added for 1.0
1 parent e11feea commit 6ea2c48

File tree

7 files changed

+47
-157
lines changed

7 files changed

+47
-157
lines changed

core/server/api/settings.js

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ var _ = require('lodash'),
66
config = require('../config'),
77
canThis = require('../permissions').canThis,
88
errors = require('../errors'),
9+
events = require('../events'),
910
utils = require('./utils'),
1011
i18n = require('../i18n'),
12+
readThemes = require('../utils/read-themes'),
1113

1214
docName = 'settings',
1315
settings,
@@ -30,6 +32,20 @@ var _ = require('lodash'),
3032
*/
3133
settingsCache = {};
3234

35+
// @TODO figure out a better way to do this in the alpha
36+
function initActiveTheme(theme) {
37+
return readThemes.active(config.paths.themePath, theme).then(function (result) {
38+
config.set({paths: {availableThemes: result}});
39+
});
40+
}
41+
42+
// @TODO figure out a better way to do this in the alpha
43+
events.on('server:start', function () {
44+
config.loadExtras().then(function () {
45+
updateSettingsCache();
46+
});
47+
});
48+
3349
/**
3450
* ### Updates Config Theme Settings
3551
* Maintains the cache of theme specific variables that are reliant on settings.
@@ -182,14 +198,16 @@ readSettingsResult = function (settingsModels) {
182198
apps = config.paths.availableApps,
183199
res;
184200

185-
if (settings.activeTheme && themes) {
201+
if (settings.activeTheme && !_.isEmpty(themes)) {
186202
res = filterPaths(themes, settings.activeTheme.value);
187203

188204
settings.availableThemes = {
189205
key: 'availableThemes',
190206
value: res,
191207
type: 'theme'
192208
};
209+
} else if (settings.activeTheme) {
210+
initActiveTheme(settings.activeTheme.value);
193211
}
194212

195213
if (settings.activeApps && apps) {

core/server/config/index.js

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ var path = require('path'),
99

1010
validator = require('validator'),
1111
generateAssetHash = require('../utils/asset-hash'),
12-
readDirectory = require('../utils/read-directory'),
1312
readThemes = require('../utils/read-themes'),
1413
errors = require('../errors'),
1514
configUrl = require('./url'),
@@ -78,10 +77,13 @@ ConfigManager.prototype.init = function (rawConfig) {
7877
// just the object appropriate for this NODE_ENV
7978
self.set(rawConfig);
8079

80+
return Promise.resolve(self._config);
81+
};
82+
83+
ConfigManager.prototype.loadExtras = function () {
84+
var self = this;
85+
8186
return self.loadThemes()
82-
.then(function () {
83-
return self.loadApps();
84-
})
8587
.then(function () {
8688
return self._config;
8789
});
@@ -92,16 +94,7 @@ ConfigManager.prototype.loadThemes = function () {
9294

9395
return readThemes(self._config.paths.themePath)
9496
.then(function (result) {
95-
self._config.paths.availableThemes = result;
96-
});
97-
};
98-
99-
ConfigManager.prototype.loadApps = function () {
100-
var self = this;
101-
102-
return readDirectory(self._config.paths.appPath)
103-
.then(function (result) {
104-
self._config.paths.availableApps = result;
97+
self.set({paths: {availableThemes: result}});
10598
});
10699
};
107100

core/server/ghost-server.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ var Promise = require('bluebird'),
44
chalk = require('chalk'),
55
fs = require('fs'),
66
errors = require('./errors'),
7+
events = require('./events'),
78
config = require('./config'),
89
i18n = require('./i18n'),
910
moment = require('moment');
@@ -76,6 +77,7 @@ GhostServer.prototype.start = function (externalApp) {
7677
});
7778
self.httpServer.on('connection', self.connection.bind(self));
7879
self.httpServer.on('listening', function () {
80+
events.emit('server:start');
7981
self.logStartMessages();
8082
resolve(self);
8183
});

core/server/index.js

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ var express = require('express'),
3131
slack = require('./data/slack'),
3232
GhostServer = require('./ghost-server'),
3333
scheduling = require('./scheduling'),
34-
validateThemes = require('./utils/validate-themes'),
3534
dbHash;
3635

3736
function initDbHashAndFirstRun() {
@@ -163,7 +162,9 @@ function init(options) {
163162
return Promise.reject(response.error);
164163
}
165164
}).then(function () {
166-
// Initialize the settings cache
165+
// Initialize the settings cache now,
166+
// This is an optimisation, so that further reads from settings are fast.
167+
// We do also do this after boot
167168
return api.init();
168169
}).then(function () {
169170
// Initialize the permissions actions and objects
@@ -187,19 +188,6 @@ function init(options) {
187188
// ## Middleware and Routing
188189
middleware(parentApp);
189190

190-
// Log all theme errors and warnings
191-
validateThemes(config.paths.themePath)
192-
.catch(function (result) {
193-
// TODO: change `result` to something better
194-
result.errors.forEach(function (err) {
195-
errors.logError(err.message, err.context, err.help);
196-
});
197-
198-
result.warnings.forEach(function (warn) {
199-
errors.logWarn(warn.message, warn.context, warn.help);
200-
});
201-
});
202-
203191
return new GhostServer(parentApp);
204192
}).then(function (_ghostServer) {
205193
ghostServer = _ghostServer;

core/server/utils/read-themes.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,27 @@
33
*/
44

55
var readDirectory = require('./read-directory'),
6+
_ = require('lodash'),
67
Promise = require('bluebird'),
78
join = require('path').join,
89
fs = require('fs'),
910

1011
statFile = Promise.promisify(fs.stat);
1112

13+
function readActiveTheme(dir, name) {
14+
var toRead = join(dir, name),
15+
themes = {};
16+
17+
return readDirectory(toRead)
18+
.then(function (tree) {
19+
if (!_.isEmpty(tree)) {
20+
themes[name] = tree;
21+
}
22+
23+
return themes;
24+
});
25+
}
26+
1227
/**
1328
* Read themes
1429
*/
@@ -44,3 +59,4 @@ function readThemes(dir) {
4459
*/
4560

4661
module.exports = readThemes;
62+
module.exports.active = readActiveTheme;

core/server/utils/validate-themes.js

Lines changed: 0 additions & 65 deletions
This file was deleted.

core/test/unit/server_utils_spec.js

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ var should = require('should'),
22
sinon = require('sinon'),
33
nock = require('nock'),
44
parsePackageJson = require('../../server/utils/parse-package-json'),
5-
validateThemes = require('../../server/utils/validate-themes'),
65
readDirectory = require('../../server/utils/read-directory'),
76
readThemes = require('../../server/utils/read-themes'),
87
gravatar = require('../../server/utils/gravatar'),
@@ -367,67 +366,6 @@ describe('Server Utilities', function () {
367366
});
368367
});
369368

370-
describe('validate-themes', function () {
371-
it('should return warnings for themes without package.json', function (done) {
372-
var themesPath, pkgJson;
373-
374-
themesPath = tmp.dirSync({unsafeCleanup: true});
375-
pkgJson = JSON.stringify({
376-
name: 'casper',
377-
version: '1.0.0'
378-
});
379-
380-
fs.mkdirSync(join(themesPath.name, 'casper'));
381-
fs.mkdirSync(join(themesPath.name, 'invalid-casper'));
382-
383-
fs.writeFileSync(join(themesPath.name, 'casper', 'package.json'), pkgJson);
384-
385-
validateThemes(themesPath.name)
386-
.then(function () {
387-
done(new Error('validateThemes succeeded, but should\'ve failed'));
388-
})
389-
.catch(function (result) {
390-
result.errors.length.should.equal(0);
391-
result.warnings.should.eql([{
392-
message: 'Found a theme with no package.json file',
393-
context: 'Theme name: invalid-casper',
394-
help: 'This will be required in future. Please see http://docs.ghost.org/themes/'
395-
}]);
396-
397-
done();
398-
})
399-
.catch(done)
400-
.finally(themesPath.removeCallback);
401-
});
402-
403-
it('should return warning for theme with invalid package.json', function (done) {
404-
var themesPath, pkgJson;
405-
406-
themesPath = tmp.dirSync({unsafeCleanup: true});
407-
pkgJson = '{"name":casper}';
408-
409-
fs.mkdirSync(join(themesPath.name, 'casper'));
410-
fs.writeFileSync(join(themesPath.name, 'casper', 'package.json'), pkgJson);
411-
412-
validateThemes(themesPath.name)
413-
.then(function () {
414-
done(new Error('validateThemes succeeded, but should\'ve failed'));
415-
})
416-
.catch(function (result) {
417-
result.errors.length.should.equal(0);
418-
result.warnings.should.eql([{
419-
message: 'Found a malformed package.json',
420-
context: 'Theme name: casper',
421-
help: 'Valid package.json will be required in future. Please see http://docs.ghost.org/themes/'
422-
}]);
423-
424-
done();
425-
})
426-
.catch(done)
427-
.finally(themesPath.removeCallback);
428-
});
429-
});
430-
431369
describe('gravatar-lookup', function () {
432370
var currentEnv = process.env.NODE_ENV;
433371

0 commit comments

Comments
 (0)