Merge pull request #3947 from hswolff/goodbye-config-theme

Removes config.theme and restructures how theme variables are cached

core/server/api/settings.js

@@ -11,6 +11,7 @@ var _            = require('lodash'),
     docName      = 'settings',
     settings,
 
+    updateConfigTheme,
     updateSettingsCache,
     settingsFilter,
     filterPaths,
@@ -29,6 +30,22 @@ var _            = require('lodash'),
     settingsCache = {};
 
 
+/**
+* ### Updates Config Theme Settings
+* Maintains the cache of theme specific variables that are reliant on settings.
+* @private
+*/
+updateConfigTheme = function () {
+    config.set({
+        theme: {
+            title: settingsCache.title.value || '',
+            description: settingsCache.description.value || '',
+            logo: settingsCache.logo.value || '',
+            cover: settingsCache.cover.value || ''
+        }
+    });
+};
+
 /**
  * ### Update Settings Cache
  * Maintain the internal cache of the settings object
@@ -44,6 +61,8 @@ updateSettingsCache = function (settings) {
             settingsCache[key] = setting;
         });
 
+        updateConfigTheme();
+
         return Promise.resolve(settingsCache);
     }
 
@@ -51,6 +70,8 @@ updateSettingsCache = function (settings) {
         .then(function (result) {
             settingsCache = readSettingsResult(result.models);
 
+            updateConfigTheme();
+
             return settingsCache;
         });
 };
@@ -200,10 +221,6 @@ populateDefaultSetting = function (key) {
 
         // Add to the settings cache
         return updateSettingsCache(readResult).then(function () {
-            // Try to update theme with the new settings
-            // if we're in the middle of populating, this might not work
-            return config.theme.update(settings, config.url).then(function () { return; }, function () { return; });
-        }).then(function () {
             // Get the result from the cache with permission checks
         });
     }).catch(function (err) {
@@ -382,8 +399,6 @@ settings = {
                 var readResult = readSettingsResult(result);
 
                 return updateSettingsCache(readResult).then(function () {
-                    return config.theme.update(settings, config.url);
-                }).then(function () {
                     return settingsResult(readResult, type);
                 });
             });

core/server/config/index.js

@@ -12,7 +12,6 @@ var path          = require('path'),
     validator     = require('validator'),
     requireTree   = require('../require-tree').readAll,
     errors        = require('../errors'),
-    theme         = require('./theme'),
     configUrl     = require('./url'),
     appRoot       = path.resolve(__dirname, '../../../'),
     corePath      = path.resolve(appRoot, 'core/'),
@@ -30,7 +29,6 @@ function ConfigManager(config) {
     this._config = {};
 
     // Allow other modules to be externally accessible.
-    this.theme = theme;
     this.urlFor = configUrl.urlFor;
     this.urlForPost = configUrl.urlForPost;
 
@@ -131,6 +129,10 @@ ConfigManager.prototype.set = function (config) {
             'availableThemes':  this._config.paths.availableThemes || {},
             'availableApps':    this._config.paths.availableApps || {},
             'builtScriptPath':  path.join(corePath, 'built/scripts/')
+        },
+        theme: {
+            // normalise the URL by removing any trailing slash
+            url: this._config.url ? this._config.url.replace(/\/$/, '') : ''
         }
     });
 

core/server/config/theme.js

@@ -1,37 +0,0 @@
-// Holds all theme configuration information
-// that as mostly used by templates and handlebar helpers.
-
-var Promise = require('bluebird'),
-
-// Variables
-    themeConfig = {};
-
-
-function theme() {
-    return themeConfig;
-}
-
-// We must pass the api.settings object
-// into this method due to circular dependencies.
-// If we were to require the api module here
-// there would be a race condition where the ./models/base
-// tries to access the config() object before it is created.
-function update(settings, configUrl) {
-    // TODO: Pass the context into this method instead of hard coding internal: true?
-    return Promise.all([
-        settings.read('title'),
-        settings.read('description'),
-        settings.read('logo'),
-        settings.read('cover')
-    ]).then(function (globals) {
-        // normalise the URL by removing any trailing slash
-        themeConfig.url = configUrl.replace(/\/$/, '');
-        themeConfig.title = globals[0].settings[0].value;
-        themeConfig.description = globals[1].settings[0].value;
-        themeConfig.logo = globals[2].settings[0] ? globals[2].settings[0].value : '';
-        themeConfig.cover = globals[3].settings[0] ? globals[3].settings[0].value : '';
-    });
-}
-
-module.exports = theme;
-module.exports.update = update;

core/server/helpers/index.js

@@ -389,7 +389,7 @@ coreHelpers.apps = function (context, options) {
 // Returns the config value for url.
 coreHelpers.blog_url = function (context, options) {
     /*jshint unused:false*/
-    return config.theme().url.toString();
+    return config.theme.url.toString();
 };
 
 coreHelpers.ghost_script_tags = function () {
@@ -501,7 +501,7 @@ coreHelpers.post_class = function (options) {
 coreHelpers.ghost_head = function (options) {
     /*jshint unused:false*/
     var self = this,
-        blog = config.theme(),
+        blog = config.theme,
         head = [],
         majorMinor = /^(\d+\.)?(\d+)/,
         trimmedVersion = this.version,
@@ -563,7 +563,7 @@ coreHelpers.meta_title = function (options) {
         pageString = '';
 
     if (_.isString(this.relativeUrl)) {
-        blog = config.theme();
+        blog = config.theme;
 
         page = this.relativeUrl.match(/\/page\/(\d+)/);
 
@@ -595,7 +595,7 @@ coreHelpers.meta_description = function (options) {
         blog;
 
     if (_.isString(this.relativeUrl)) {
-        blog = config.theme();
+        blog = config.theme;
         if (!this.relativeUrl || this.relativeUrl === '/' || this.relativeUrl === '') {
             description = blog.description;
         } else if (this.author) {

core/server/index.js

@@ -160,12 +160,8 @@ function init(options) {
         return api.init();
     }).then(function () {
         // Initialize the permissions actions and objects
-        // NOTE: Must be done before the config.theme.update and initDbHashAndFirstRun calls
+        // NOTE: Must be done before initDbHashAndFirstRun calls
         return permissions.init();
-    }).then(function () {
-        // We must pass the api.settings object
-        // into this method due to circular dependencies.
-        return config.theme.update(api.settings, config.url);
     }).then(function () {
         return Promise.join(
             // Check for or initialise a dbHash.

core/server/middleware/index.js

@@ -44,7 +44,7 @@ function ghostLocals(req, res, next) {
 }
 
 function initThemeData(secure) {
-    var themeConfig = config.theme();
+    var themeConfig = config.theme;
     if (secure && config.urlSSL) {
         // For secure requests override .url property with the SSL version
         themeConfig = _.clone(themeConfig);

core/test/unit/config_spec.js

@@ -12,7 +12,6 @@ var should         = require('should'),
 
     // Thing we are testing
     defaultConfig  = require('../../../config.example')[process.env.NODE_ENV],
-    theme          = rewire('../../server/config/theme'),
     config         = rewire('../../server/config');
 
 // To stop jshint complaining
@@ -22,41 +21,31 @@ describe('Config', function () {
 
     describe('Theme', function () {
 
-        var sandbox,
+        beforeEach(function () {
-            settings,
+            config.set({
-            settingsStub;
+                url: 'http://my-ghost-blog.com',
-
+                theme: {
-        beforeEach(function (done) {
+                    title: 'casper',
-            sandbox = sinon.sandbox.create();
+                    description: 'casper',
-
+                    logo: 'casper',
-            settings = {'read': function read() {}};
+                    cover: 'casper'
-
+                }
-            settingsStub = sandbox.stub(settings, 'read', function () {
-                return Promise.resolve({ settings: [{value: 'casper'}] });
             });
-
-            theme.update(settings, 'http://my-ghost-blog.com')
-                .then(done)
-                .catch(done);
         });
 
-        afterEach(function (done) {
+        afterEach(function () {
-            theme.update(settings, defaultConfig.url)
+            config.set(_.merge({}, defaultConfig));
-                .then(done)
-                .catch(done);
-
-            sandbox.restore();
         });
 
         it('should have exactly the right keys', function () {
-            var themeConfig = theme();
+            var themeConfig = config.theme;
 
             // This will fail if there are any extra keys
             themeConfig.should.have.keys('url', 'title', 'description', 'logo', 'cover');
         });
 
         it('should have the correct values for each key', function () {
-            var themeConfig = theme();
+            var themeConfig = config.theme;
 
             // Check values are as we expect
             themeConfig.should.have.property('url', 'http://my-ghost-blog.com');
@@ -64,9 +53,6 @@ describe('Config', function () {
             themeConfig.should.have.property('description', 'casper');
             themeConfig.should.have.property('logo', 'casper');
             themeConfig.should.have.property('cover', 'casper');
-
-            // Check settings.read gets called exactly 4 times
-            settingsStub.callCount.should.equal(4);
         });
     });
 

core/test/unit/server_helpers_index_spec.js

@@ -47,15 +47,14 @@ describe('Core Helpers', function () {
                         'post.hbs': '/content/themes/casper/post.hbs'
                     }
                 }
+            },
+            theme: {
+                title: 'Ghost',
+                description: 'Just a blogging platform.',
+                url: 'http://testurl.com'
             }
         });
 
-        existingConfig.theme = sandbox.stub().returns({
-            title: 'Ghost',
-            description: 'Just a blogging platform.',
-            url: 'http://testurl.com'
-        });
-
         helpers.loadCoreHelpers(adminHbs);
         // Load template helpers in handlebars
         hbs.express3({ partialsDir: [config.paths.helperTemplates] });
@@ -564,7 +563,12 @@ describe('Core Helpers', function () {
         var configUrl = config.url;
 
         afterEach(function () {
-            config.set({url: configUrl});
+            overrideConfig({
+                url: configUrl,
+                theme: {
+                    title: 'Ghost'
+                }
+            });
         });
 
         it('has loaded ghost_head helper', function () {