diff --git a/core/server/api/themes.js b/core/server/api/themes.js index 1695f855ee..58636a1a73 100644 --- a/core/server/api/themes.js +++ b/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'), fs = require('fs-extra'), config = require('../config'), errors = require('../errors'), @@ -55,7 +56,10 @@ themes = { return settingsModel.edit(newSettings, options); }) .then(function hasEditedSetting() { - // @TODO actually do things to activate the theme, other than just the setting? + // Activate! (sort of) + debug('Activating theme (method B on API "activate")', themeName); + themeUtils.activate(loadedTheme, checkedTheme); + return themeUtils.toJSON(themeName, checkedTheme); }); }, @@ -107,7 +111,15 @@ themes = { // Sets the theme on the themeList return themeUtils.loadOne(zip.shortName); }) - .then(function () { + .then(function (loadedTheme) { + // If this is the active theme, we are overriding + // This is a special case of activation + if (zip.shortName === settingsCache.get('activeTheme')) { + // Activate! (sort of) + debug('Activating theme (method C, on API "override")', zip.shortName); + themeUtils.activate(loadedTheme, checkedTheme); + } + // @TODO: unify the name across gscan and Ghost! return themeUtils.toJSON(zip.shortName, checkedTheme); }) diff --git a/core/server/middleware/static-theme.js b/core/server/middleware/static-theme.js index 9edbbb0c06..2f96af2b9d 100644 --- a/core/server/middleware/static-theme.js +++ b/core/server/middleware/static-theme.js @@ -2,6 +2,7 @@ var _ = require('lodash'), express = require('express'), path = require('path'), config = require('../config'), + themeUtils = require('../themes'), utils = require('../utils'); function isBlackListedFileType(file) { @@ -17,12 +18,12 @@ function isWhiteListedFile(file) { } function forwardToExpressStatic(req, res, next) { - if (!req.app.get('activeTheme')) { + if (!themeUtils.getActive()) { next(); } else { var configMaxAge = config.get('caching:theme:maxAge'); - express.static(path.join(config.getContentPath('themes'), req.app.get('activeTheme')), + express.static(themeUtils.getActive().path, {maxAge: (configMaxAge || configMaxAge === 0) ? configMaxAge : utils.ONE_YEAR_MS} )(req, res, next); } diff --git a/core/server/middleware/theme-handler.js b/core/server/middleware/theme-handler.js index 0b296b0c35..d15004dc67 100644 --- a/core/server/middleware/theme-handler.js +++ b/core/server/middleware/theme-handler.js @@ -1,13 +1,11 @@ var _ = require('lodash'), - fs = require('fs'), - path = require('path'), hbs = require('express-hbs'), config = require('../config'), utils = require('../utils'), errors = require('../errors'), i18n = require('../i18n'), settingsCache = require('../settings/cache'), - themeList = require('../themes').list, + themeUtils = require('../themes'), themeHandler; themeHandler = { @@ -52,33 +50,29 @@ themeHandler = { // ### Activate Theme // Helper for updateActiveTheme - activateTheme: function activateTheme(blogApp, activeThemeName) { - var themePartialsPath = path.join(config.getContentPath('themes'), activeThemeName, 'partials'), - hbsOptions = { + activateTheme: function activateTheme(blogApp) { + var hbsOptions = { partialsDir: [config.get('paths').helperTemplates], onCompile: function onCompile(exhbs, source) { return exhbs.handlebars.compile(source, {preventIndent: true}); } }; - fs.stat(themePartialsPath, function stat(err, stats) { - // Check that the theme has a partials directory before trying to use it - if (!err && stats && stats.isDirectory()) { - hbsOptions.partialsDir.push(themePartialsPath); - } - }); + if (themeUtils.getActive().hasPartials()) { + hbsOptions.partialsDir.push(themeUtils.getActive().partialsPath); + } // reset the asset hash config.set('assetHash', null); // clear the view cache blogApp.cache = {}; // Set the views and engine - blogApp.set('views', path.join(config.getContentPath('themes'), activeThemeName)); + blogApp.set('views', themeUtils.getActive().path); blogApp.engine('hbs', hbs.express3(hbsOptions)); // Set active theme variable on the express server // Note: this is effectively the "mounted" theme, which has been loaded into the express app - blogApp.set('activeTheme', activeThemeName); + blogApp.set('activeTheme', themeUtils.getActive().name); }, // ### updateActiveTheme @@ -87,11 +81,13 @@ themeHandler = { // is not yet activated. updateActiveTheme: function updateActiveTheme(req, res, next) { var blogApp = req.app, + // We use the settingsCache here, because the setting will be set, even if the theme itself is + // not usable because it is invalid or missing. activeThemeName = settingsCache.get('activeTheme'), mountedThemeName = blogApp.get('activeTheme'); // This means that the theme hasn't been loaded yet i.e. there is no active theme - if (!themeList.get(activeThemeName)) { + if (!themeUtils.getActive()) { // This is the one place we ACTUALLY throw an error for a missing theme // As it's a request we cannot serve return next(new errors.InternalServerError({ @@ -101,7 +97,7 @@ themeHandler = { // If there is an active theme AND it has changed, call activate } else if (activeThemeName !== mountedThemeName) { // This is effectively "mounting" a theme into express, the theme is already "active" - themeHandler.activateTheme(blogApp, activeThemeName); + themeHandler.activateTheme(blogApp); } next(); diff --git a/core/server/themes/active.js b/core/server/themes/active.js new file mode 100644 index 0000000000..3526ffb239 --- /dev/null +++ b/core/server/themes/active.js @@ -0,0 +1,88 @@ +'use strict'; + +/** + * # Active Theme + * + * This file defines a class of active theme, and also controls the getting and setting a single instance, as there + * can only ever be one active theme. Unlike a singleton, the active theme can change, however only in a controlled way. + * + * I've made use of the new class & constructor syntax here, as we are now only supporting Node v4 and above, which has + * full support for these features. + * + * There are several different patterns available for keeping data private. Elsewhere in Ghost we use the + * naming convention of the _ prefix. Even though this has the downside of not being truly private, it is still one + * of the preferred options for keeping data private with the new class syntax, therefore I have kept it. + * + * No properties marked with an _ should be used directly. + * + */ +var _ = require('lodash'), + join = require('path').join, + // Current instance of ActiveTheme + currentActiveTheme; + +class ActiveTheme { + /** + * @TODO this API needs to be simpler, but for now should work! + * @param {object} loadedTheme - the loaded theme object from the theme list + * @param {object} checkedTheme - the result of gscan.format for the theme we're activating + */ + constructor(loadedTheme, checkedTheme) { + // Assign some data, mark it all as pseudo-private + this._name = loadedTheme.name; + this._path = loadedTheme.path; + + // @TODO: get gscan to return validated, useful package.json fields for us! + this._packageInfo = loadedTheme['package.json']; + this._partials = checkedTheme.partials; + // @TODO: get gscan to return a template collection for us + this._templates = _.reduce(checkedTheme.files, function (templates, entry) { + var tplMatch = entry.file.match(/(^[^\/]+).hbs$/); + if (tplMatch) { + templates.push(tplMatch[1]); + } + return templates; + }, []); + } + + get name() { + return this._name; + } + + get path() { + return this._path; + } + + get partialsPath() { + return join(this.path, 'partials'); + } + + hasPartials() { + return this._partials.length > 0; + } + + hasTemplate(templateName) { + return this._templates.indexOf(templateName) > -1; + } +} + +module.exports = { + get() { + return currentActiveTheme; + }, + /** + * Set theme + * + * At this point we trust that the theme has been validated. + * Any handling for invalid themes should happen before we get here + * + * @TODO this API needs to be simpler, but for now should work! + * @param {object} loadedTheme - the loaded theme object from the theme list + * @param {object} checkedTheme - the result of gscan.format for the theme we're activating + * @return {ActiveTheme} + */ + set(loadedTheme, checkedTheme) { + currentActiveTheme = new ActiveTheme(loadedTheme, checkedTheme); + return currentActiveTheme; + } +}; diff --git a/core/server/themes/index.js b/core/server/themes/index.js index 2ff98497fa..de55de5ec5 100644 --- a/core/server/themes/index.js +++ b/core/server/themes/index.js @@ -1,8 +1,12 @@ var debug = require('debug')('ghost:themes'), + _ = require('lodash'), events = require('../events'), + errors = require('../errors'), logging = require('../logging'), i18n = require('../i18n'), themeLoader = require('./loader'), + active = require('./active'), + validate = require('./validate'), settingsCache = require('../settings/cache'); // @TODO: reduce the amount of things we expose to the outside world @@ -11,7 +15,9 @@ module.exports = { // Init themes module // TODO: move this once we're clear what needs to happen here init: function initThemes() { - var activeThemeName = settingsCache.get('activeTheme'); + var activeThemeName = settingsCache.get('activeTheme'), + self = this; + debug('init themes', activeThemeName); // Register a listener for server-start to load all themes @@ -22,6 +28,20 @@ module.exports = { // Just read the active theme for now return themeLoader .loadOneTheme(activeThemeName) + .then(function activeThemeHasLoaded(theme) { + // Validate + return validate + .check(theme) + .then(function resultHandler(checkedTheme) { + // Activate! (sort of) + debug('Activating theme (method A on boot)', activeThemeName); + self.activate(theme, checkedTheme); + }) + .catch(function () { + // Active theme is not valid, we don't want to exit because the admin panel will still work + logging.warn(i18n.t('errors.middleware.themehandler.invalidTheme', {theme: activeThemeName})); + }); + }) .catch(function () { // Active theme is missing, we don't want to exit because the admin panel will still work logging.warn(i18n.t('errors.middleware.themehandler.missingTheme', {theme: activeThemeName})); @@ -31,6 +51,17 @@ module.exports = { loadAll: themeLoader.loadAllThemes, loadOne: themeLoader.loadOneTheme, list: require('./list'), - validate: require('./validate'), - toJSON: require('./to-json') + validate: validate, + toJSON: require('./to-json'), + getActive: active.get, + activate: function activate(loadedTheme, checkedTheme) { + if (!_.has(checkedTheme, 'results.score.level') || checkedTheme.results.score.level !== 'passing') { + throw new errors.InternalServerError({ + message: i18n.t('errors.middleware.themehandler.invalidTheme', {theme: loadedTheme.name}) + }); + } + + // Use the two theme objects to set the current active theme + active.set(loadedTheme, checkedTheme); + } }; diff --git a/core/test/unit/middleware/static-theme_spec.js b/core/test/unit/middleware/static-theme_spec.js index 9ac2a65c6d..8f8df5e50d 100644 --- a/core/test/unit/middleware/static-theme_spec.js +++ b/core/test/unit/middleware/static-theme_spec.js @@ -1,104 +1,124 @@ -var sinon = require('sinon'), - should = require('should'), +var sinon = require('sinon'), + should = require('should'), - express = require('express'), - staticTheme = require('../../../server/middleware/static-theme'); + express = require('express'), + themeUtils = require('../../../server/themes'), + staticTheme = require('../../../server/middleware/static-theme'), + + sandbox = sinon.sandbox.create(); describe('staticTheme', function () { - var next; + var expressStaticStub, activeThemeStub, req, res; beforeEach(function () { - next = sinon.spy(); + req = {}; + res = {}; + + activeThemeStub = sandbox.stub(themeUtils, 'getActive').returns({ + path: 'my/fake/path' + }); + + expressStaticStub = sandbox.spy(express, 'static'); }); - it('should call next if hbs file type', function () { - var req = { - path: 'mytemplate.hbs' - }; - - staticTheme(null)(req, null, next); - next.called.should.be.true(); + afterEach(function () { + sandbox.restore(); }); - it('should call next if md file type', function () { - var req = { - path: 'README.md' - }; + it('should skip for .hbs file', function (done) { + req.path = 'mytemplate.hbs'; - staticTheme(null)(req, null, next); - next.called.should.be.true(); + staticTheme()(req, res, function next() { + activeThemeStub.called.should.be.false(); + expressStaticStub.called.should.be.false(); + + done(); + }); }); - it('should call next if json file type', function () { - var req = { - path: 'sample.json' - }; + it('should skip for .md file', function (done) { + req.path = 'README.md'; - staticTheme(null)(req, null, next); - next.called.should.be.true(); + staticTheme()(req, res, function next() { + activeThemeStub.called.should.be.false(); + expressStaticStub.called.should.be.false(); + + done(); + }); }); - it('should call express.static if valid file type', function (done) { - var req = { - path: 'myvalidfile.css', - app: { - get: function () { return 'casper'; } - } - }, - activeThemeStub, - sandbox = sinon.sandbox.create(), - expressStatic = sinon.spy(express, 'static'); + it('should skip for .json file', function (done) { + req.path = 'sample.json'; - activeThemeStub = sandbox.spy(req.app, 'get'); + staticTheme()(req, res, function next() { + activeThemeStub.called.should.be.false(); + expressStaticStub.called.should.be.false(); + + done(); + }); + }); + + it('should call express.static for .css file', function (done) { + req.path = 'myvalidfile.css'; + + staticTheme()(req, res, function next() { + // Specifically gets called twice + activeThemeStub.calledTwice.should.be.true(); + expressStaticStub.called.should.be.true(); + + // Check that express static gets called with the theme path + maxAge + should.exist(expressStaticStub.firstCall.args); + expressStaticStub.firstCall.args[0].should.eql('my/fake/path'); + expressStaticStub.firstCall.args[1].should.be.an.Object().with.property('maxAge'); + + done(); + }); + }); + + it('should call express.static for .js file', function (done) { + req.path = 'myvalidfile.js'; + + staticTheme()(req, res, function next() { + // Specifically gets called twice + activeThemeStub.calledTwice.should.be.true(); + expressStaticStub.called.should.be.true(); + + // Check that express static gets called with the theme path + maxAge + should.exist(expressStaticStub.firstCall.args); + expressStaticStub.firstCall.args[0].should.eql('my/fake/path'); + expressStaticStub.firstCall.args[1].should.be.an.Object().with.property('maxAge'); - staticTheme(null)(req, null, function (reqArg, res, next2) { - /*jshint unused:false */ - sandbox.restore(); - next.called.should.be.false(); - activeThemeStub.called.should.be.true(); - expressStatic.called.should.be.true(); - should.exist(expressStatic.args[0][1].maxAge); done(); }); }); it('should not error if active theme is missing', function (done) { - var req = { - path: 'myvalidfile.css', - app: { - get: function () { return undefined; } - } - }, - activeThemeStub, - sandbox = sinon.sandbox.create(); + req.path = 'myvalidfile.css'; - activeThemeStub = sandbox.spy(req.app, 'get'); + // make the active theme not exist + activeThemeStub.returns(undefined); + + staticTheme()(req, res, function next() { + activeThemeStub.calledOnce.should.be.true(); + expressStaticStub.called.should.be.false(); - staticTheme(null)(req, null, function (reqArg, res, next2) { - /*jshint unused:false */ - sandbox.restore(); - next.called.should.be.false(); done(); }); }); - it('should not call next if file is on whitelist', function (done) { - var req = { - path: 'manifest.json', - app: { - get: function () { return 'casper'; } - } - }, - activeThemeStub, - sandbox = sinon.sandbox.create(); + it('should NOT skip if file is on whitelist', function (done) { + req.path = 'manifest.json'; - activeThemeStub = sandbox.spy(req.app, 'get'); + staticTheme()(req, res, function next() { + // Specifically gets called twice + activeThemeStub.calledTwice.should.be.true(); + expressStaticStub.called.should.be.true(); + + // Check that express static gets called with the theme path + maxAge + should.exist(expressStaticStub.firstCall.args); + expressStaticStub.firstCall.args[0].should.eql('my/fake/path'); + expressStaticStub.firstCall.args[1].should.be.an.Object().with.property('maxAge'); - staticTheme(null)(req, null, function (reqArg, res, next2) { - /*jshint unused:false */ - sandbox.restore(); - next.called.should.be.false(); - activeThemeStub.called.should.be.true(); done(); }); }); diff --git a/core/test/unit/middleware/theme-handler_spec.js b/core/test/unit/middleware/theme-handler_spec.js index 92d8b1ef51..e6fa1beb35 100644 --- a/core/test/unit/middleware/theme-handler_spec.js +++ b/core/test/unit/middleware/theme-handler_spec.js @@ -1,23 +1,25 @@ var sinon = require('sinon'), should = require('should'), express = require('express'), - fs = require('fs'), hbs = require('express-hbs'), - themeList = require('../../../server/themes').list, + configUtils = require('../../utils/configUtils'), + themeUtils = require('../../../server/themes'), + themeList = themeUtils.list, themeHandler = require('../../../server/middleware/theme-handler'), settingsCache = require('../../../server/settings/cache'), sandbox = sinon.sandbox.create(); describe('Theme Handler', function () { - var req, res, next, blogApp; + var req, res, blogApp, getActiveThemeStub; beforeEach(function () { req = sinon.spy(); res = sinon.spy(); - next = sinon.spy(); blogApp = express(); req.app = blogApp; + + getActiveThemeStub = sandbox.stub(themeUtils, 'getActive').returns({}); }); afterEach(function () { @@ -26,106 +28,143 @@ describe('Theme Handler', function () { }); describe('activateTheme', function () { + var hbsStub; + + beforeEach(function () { + hbsStub = sandbox.spy(hbs, 'express3'); + }); + it('should activate new theme with partials', function () { - var fsStub = sandbox.stub(fs, 'stat', function (path, cb) { - cb(null, {isDirectory: function () { return true; }}); - }), - hbsStub = sandbox.spy(hbs, 'express3'); + getActiveThemeStub.returns({ + name: 'casper', + path: 'my/fake/path', + partialsPath: 'my/fake/path/partials', + hasPartials: function () {return true;} + }); - themeHandler.activateTheme(blogApp, 'casper'); + themeHandler.activateTheme(blogApp); - fsStub.calledOnce.should.be.true(); + // hasPartials, partialsPath, path & name + getActiveThemeStub.callCount.should.be.eql(4); hbsStub.calledOnce.should.be.true(); hbsStub.firstCall.args[0].should.be.an.Object().and.have.property('partialsDir'); - hbsStub.firstCall.args[0].partialsDir.should.have.lengthOf(2); + hbsStub.firstCall.args[0].partialsDir.should.be.an.Array().with.lengthOf(2); + hbsStub.firstCall.args[0].partialsDir[1].should.eql('my/fake/path/partials'); + + // Check the asset hash gets reset + should(configUtils.config.get('assetHash')).eql(null); + blogApp.get('activeTheme').should.equal('casper'); + blogApp.get('views').should.eql('my/fake/path'); }); it('should activate new theme without partials', function () { - var fsStub = sandbox.stub(fs, 'stat', function (path, cb) { - cb(null, null); - }), - hbsStub = sandbox.spy(hbs, 'express3'); + getActiveThemeStub.returns({ + name: 'casper', + path: 'my/fake/path', + hasPartials: function () {return false;} + }); - themeHandler.activateTheme(blogApp, 'casper'); + themeHandler.activateTheme(blogApp); - fsStub.calledOnce.should.be.true(); + // hasPartials, path & name + getActiveThemeStub.callCount.should.eql(3); hbsStub.calledOnce.should.be.true(); hbsStub.firstCall.args[0].should.be.an.Object().and.have.property('partialsDir'); hbsStub.firstCall.args[0].partialsDir.should.have.lengthOf(1); + + // Check the asset hash gets reset + should(configUtils.config.get('assetHash')).eql(null); + blogApp.get('activeTheme').should.equal('casper'); + blogApp.get('views').should.eql('my/fake/path'); }); }); + // NOTE: These tests are totally dependent on the previous tests + // @TODO: properly fix these tests once theme refactor is finished describe('configHbsForContext', function () { - it('handles non secure context', function () { - res.locals = {}; - themeHandler.configHbsForContext(req, res, next); + var updateOptionsSpy; - should.not.exist(res.locals.secure); - next.called.should.be.true(); + beforeEach(function () { + updateOptionsSpy = sandbox.spy(hbs, 'updateTemplateOptions'); }); - it('sets view path', function () { - req.secure = true; + it('handles non secure context', function (done) { res.locals = {}; - blogApp.set('activeTheme', 'casper'); + themeHandler.configHbsForContext(req, res, function next() { + updateOptionsSpy.calledOnce.should.be.true(); + should.not.exist(res.locals.secure); - themeHandler.configHbsForContext(req, res, next); - - blogApp.get('views').should.not.be.undefined(); + done(); + }); }); - it('sets view path', function () { + it('handles secure context', function (done) { req.secure = true; res.locals = {}; - blogApp.set('activeTheme', 'casper'); + themeHandler.configHbsForContext(req, res, function next() { + updateOptionsSpy.calledOnce.should.be.true(); + should.exist(res.locals.secure); + res.locals.secure.should.be.true(); - themeHandler.configHbsForContext(req, res, next); - - blogApp.get('views').should.not.be.undefined(); + done(); + }); }); }); describe('updateActiveTheme', function () { + var activateThemeStub, + settingsCacheStub; + beforeEach(function () { - themeList.init({casper: {}}); + activateThemeStub = sandbox.stub(themeHandler, 'activateTheme'); + settingsCacheStub = sandbox.stub(settingsCache, 'get').withArgs('activeTheme').returns('casper'); }); it('updates the active theme if changed', function (done) { - var activateThemeSpy = sandbox.spy(themeHandler, 'activateTheme'); - - sandbox.stub(settingsCache, 'get').withArgs('activeTheme').returns('casper'); blogApp.set('activeTheme', 'not-casper'); - themeHandler.updateActiveTheme(req, res, function () { - activateThemeSpy.called.should.be.true(); + themeHandler.updateActiveTheme(req, res, function next(err) { + // Did not throw an error + should.not.exist(err); + + settingsCacheStub.calledWith('activeTheme').should.be.true(); + getActiveThemeStub.called.should.be.true(); + activateThemeStub.called.should.be.true(); + activateThemeStub.calledWith(blogApp).should.be.true(); + done(); }); }); it('does not update the active theme if not changed', function (done) { - var activateThemeSpy = sandbox.spy(themeHandler, 'activateTheme'); - - sandbox.stub(settingsCache, 'get').withArgs('activeTheme').returns('casper'); blogApp.set('activeTheme', 'casper'); - themeHandler.updateActiveTheme(req, res, function () { - activateThemeSpy.called.should.be.false(); + themeHandler.updateActiveTheme(req, res, function next(err) { + // Did not throw an error + should.not.exist(err); + + settingsCacheStub.calledWith('activeTheme').should.be.true(); + getActiveThemeStub.called.should.be.true(); + activateThemeStub.called.should.be.false(); + done(); }); }); it('throws error if theme is missing', function (done) { - var activateThemeSpy = sandbox.spy(themeHandler, 'activateTheme'); + getActiveThemeStub.returns(undefined); - sandbox.stub(settingsCache, 'get').withArgs('activeTheme').returns('rasper'); - blogApp.set('activeTheme', 'not-casper'); - - themeHandler.updateActiveTheme(req, res, function (err) { + themeHandler.updateActiveTheme(req, res, function next(err) { + // Did throw an error should.exist(err); - activateThemeSpy.called.should.be.false(); - err.message.should.eql('The currently active theme "rasper" is missing.'); + err.message.should.eql('The currently active theme "casper" is missing.'); + + settingsCacheStub.calledWith('activeTheme').should.be.true(); + getActiveThemeStub.called.should.be.true(); + activateThemeStub.called.should.be.false(); + done(); }); });