🔥 do not store settings in config (#7924)

* 🎨  🔥  do not store settings in config and make settings cache easier available

- remove remembering settings value in theme config
- if we need a cache value, we are asking the settings cache directly
- instead of settings.getSettingSync we use settings.cache.get

- added TODO:
  - think about moving the settings cache out of api/settings
  - we could create a folder named cache cache/settings
  - this settings cache listens on model changes for settings
  - decoupling

* 🔥  remove timezone from config

- no need to store in overrides config and in defaults settings

* 🎨  context object helper

- replace config.get('theme') by settings cache

* 🎨  replace config.get('theme') by settings.cache.get

* 🎨  adapt tests

* fixes from comments

core/server/api/configuration.js

@@ -2,6 +2,7 @@
 // RESTful API for browsing the configuration
 var _                  = require('lodash'),
     config             = require('../config'),
+    settingsCache      = require('../api/settings').cache,
     ghostVersion       = require('../utils/ghost-version'),
     models             = require('../models'),
     Promise            = require('bluebird'),
@@ -29,7 +30,7 @@ function getBaseConfig() {
         useGravatar:    !config.isPrivacyDisabled('useGravatar'),
         publicAPI:      config.get('publicAPI') === true,
         blogUrl:        utils.url.urlFor('home', true),
-        blogTitle:      config.get('theme').title,
+        blogTitle:      settingsCache.get('title'),
         routeKeywords:  config.get('routeKeywords')
     };
 }

core/server/api/settings.js

@@ -6,15 +6,12 @@ var _            = require('lodash'),
     config       = require('../config'),
     canThis      = require('../permissions').canThis,
     errors       = require('../errors'),
-    logging      = require('../logging'),
     utils        = require('./utils'),
     i18n         = require('../i18n'),
-    globalUtils  = require('../utils'),
 
     docName      = 'settings',
     settings,
 
-    updateConfigCache,
     updateSettingsCache,
     settingsFilter,
     filterPaths,
@@ -31,49 +28,6 @@ var _            = require('lodash'),
      */
     settingsCache = {};
 
-/**
-* ### Updates Config Theme Settings
-* Maintains the cache of theme specific variables that are reliant on settings.
-* @private
-*/
-updateConfigCache = function () {
-    var labsValue = {};
-
-    if (settingsCache.labs && settingsCache.labs.value) {
-        try {
-            labsValue = JSON.parse(settingsCache.labs.value);
-        } catch (err) {
-            logging.error(new errors.GhostError({
-                err: err,
-                message: i18n.t('errors.api.settings.invalidJsonInLabs'),
-                context: i18n.t('errors.api.settings.labsColumnCouldNotBeParsed'),
-                help: i18n.t('errors.api.settings.tryUpdatingLabs')
-            }));
-        }
-    }
-
-    // @TODO: why are we putting the settings cache values into config?we could access the cache directly
-    // @TODO: plus: why do we assign the values to the prefix "theme"?
-    // @TODO: might be related to https://github.com/TryGhost/Ghost/issues/7488
-    config.set('theme:title', (settingsCache.title && settingsCache.title.value) || '');
-    config.set('theme:description', (settingsCache.description && settingsCache.description.value) || '');
-    config.set('theme:logo', (settingsCache.logo && settingsCache.logo.value) || '');
-    config.set('theme:cover', (settingsCache.cover && settingsCache.cover.value) || '');
-    config.set('theme:navigation', (settingsCache.navigation && JSON.parse(settingsCache.navigation.value)) || []);
-    config.set('theme:postsPerPage', (settingsCache.postsPerPage && settingsCache.postsPerPage.value) || config.get('theme').postsPerPage);
-    config.set('theme:permalinks', (settingsCache.permalinks && settingsCache.permalinks.value) || config.get('theme').permalinks);
-    config.set('theme:twitter', (settingsCache.twitter && settingsCache.twitter.value) || '');
-    config.set('theme:facebook', (settingsCache.facebook && settingsCache.facebook.value) || '');
-    config.set('theme:timezone', (settingsCache.activeTimezone && settingsCache.activeTimezone.value) || config.get('theme').timezone);
-    config.set('theme:url', globalUtils.url.urlFor('home', true));
-    config.set('theme:amp', (settingsCache.amp && settingsCache.amp.value === 'true'));
-    config.set('theme:icon', settingsCache.icon && settingsCache.icon.value);
-
-    _.each(labsValue, function (value, key) {
-        config.set('labs:' + key, value);
-    });
-};
-
 /**
  * ### Update Settings Cache
  * Maintain the internal cache of the settings object
@@ -90,16 +44,14 @@ updateSettingsCache = function (settings, options) {
             settingsCache[key] = setting;
         });
 
-        updateConfigCache();
-
         return Promise.resolve(settingsCache);
     }
 
     return dataProvider.Settings.findAll(options)
         .then(function (result) {
             // keep reference and update all keys
             _.extend(settingsCache, readSettingsResult(result.models));
-            updateConfigCache();
+
             return settingsCache;
         });
 };
@@ -187,6 +139,7 @@ readSettingsResult = function (settingsModels) {
         apps = config.get('paths').availableApps,
         res;
 
+    // @TODO: remove availableThemes from settings cache and create an endpoint to fetch themes
     if (settings.activeTheme && themes) {
         res = filterPaths(themes, settings.activeTheme.value);
 
@@ -255,7 +208,7 @@ populateDefaultSetting = function (key) {
             return Promise.reject(err);
         }
 
-        // TODO: Different kind of error?
+        // TODO: different kind of error?
         return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.api.settings.problemFindingSetting', {key: key})}));
     });
 };
@@ -434,19 +387,36 @@ settings = {
 module.exports = settings;
 
 /**
- * synchronous function to get cached settings value
- * returns the value of the settings entry
+ * @TODO:
+ * - move settings cache somewhere else e.q. listen on model changes
+ *
+ * IMPORTANT:
+ * We store settings with a type and a key in the database.
+ *
+ * {
+ *   type: core
+ *   key: dbHash
+ *   value: ...
+ * }
+ *
+ * But the settings cache does not allow requesting a value by type, only by key.
+ * e.g. settings.cache.get('dbHash')
  */
-module.exports.getSettingSync = function getSettingSync(key) {
-    return settingsCache[key] && settingsCache[key].value;
-};
+module.exports.cache = {
+    get: function get(key) {
+        if (!settingsCache[key]) {
+            return;
+        }
 
-/**
- * synchronous function to get all cached settings values
- * returns everything for now
- */
-module.exports.getSettingsSync = function getSettingsSync() {
-    return settingsCache;
+        try {
+            return JSON.parse(settingsCache[key].value);
+        } catch (err) {
+            return settingsCache[key].value;
+        }
+    },
+    getAll: function getAll() {
+        return settingsCache;
+    }
 };
 
 module.exports.updateSettingsCache = updateSettingsCache;

core/server/apps/amp/lib/router.js

@@ -5,8 +5,8 @@ var path                = require('path'),
     i18n                = require('../../../i18n'),
 
     // Dirty requires
-    config              = require('../../../config'),
     errors              = require('../../../errors'),
+    settingsCache       = require('../../../api/settings').cache,
     templates           = require('../../../controllers/frontend/templates'),
     postLookup          = require('../../../controllers/frontend/post-lookup'),
     setResponseContext  = require('../../../controllers/frontend/context');
@@ -36,6 +36,7 @@ function controller(req, res, next) {
 
 function getPostData(req, res, next) {
     req.body = req.body || {};
+
     postLookup(res.locals.relativeUrl)
         .then(function (result) {
             if (result && result.post) {
@@ -50,7 +51,7 @@ function getPostData(req, res, next) {
 }
 
 function checkIfAMPIsEnabled(req, res, next) {
-    var ampIsEnabled = config.get('theme:amp');
+    var ampIsEnabled = settingsCache.get('amp');
 
     if (ampIsEnabled) {
         return next();

core/server/config/overrides.json

@@ -63,9 +63,6 @@
         "cannotScheduleAPostBeforeInMinutes": 2,
         "publishAPostBySchedulerToleranceInMinutes": 2
     },
-    "theme": {
-        "timezone": "Etc/UTC"
-    },
     "maintenance": {
         "enabled": false
     }

core/server/controllers/frontend/fetch-data.js

@@ -4,8 +4,8 @@
  */
 var api = require('../../api'),
     _   = require('lodash'),
-    config = require('../../config'),
     Promise = require('bluebird'),
+    settingsCache = api.settings.cache,
     queryDefaults,
     defaultPostQuery = {};
 
@@ -33,7 +33,7 @@ _.extend(defaultPostQuery, queryDefaults, {
 function fetchPostsPerPage(options) {
     options = options || {};
 
-    var postsPerPage = parseInt(config.get('theme').postsPerPage);
+    var postsPerPage = parseInt(settingsCache.get('postsPerPage'));
 
     // No negative posts per page, must be number
     if (!isNaN(postsPerPage) && postsPerPage > 0) {

core/server/controllers/frontend/post-lookup.js

@@ -3,8 +3,7 @@ var _          = require('lodash'),
     url        = require('url'),
     routeMatch = require('path-match')(),
     api        = require('../../api'),
-    config     = require('../../config'),
-
+    settingsCache = api.settings.cache,
     optionsFormat = '/:options?';
 
 function getOptionsFormat(linkStructure) {
@@ -13,7 +12,7 @@ function getOptionsFormat(linkStructure) {
 
 function postLookup(postUrl) {
     var postPath = url.parse(postUrl).path,
-        postPermalink = config.get('theme').permalinks,
+        postPermalink = settingsCache.get('permalinks'),
         pagePermalink = '/:slug/',
         isEditURL = false,
         matchFuncPost,

core/server/data/meta/asset_url.js

@@ -1,4 +1,5 @@
 var config = require('../../config'),
+    settingsCache = require('../../api/settings').cache,
     utils = require('../../utils');
 
 function getAssetUrl(path, isAdmin, minify) {
@@ -19,7 +20,11 @@ function getAssetUrl(path, isAdmin, minify) {
         if (isAdmin) {
             output = utils.url.urlJoin(utils.url.getSubdir(), '/favicon.ico');
         } else {
-            output = config.get('theme:icon') ? utils.url.urlJoin(utils.url.getSubdir(), utils.url.urlFor('image', {image: config.get('theme:icon')})) : utils.url.urlJoin(utils.url.getSubdir(), '/favicon.ico');
+            if (settingsCache.get('icon')) {
+                output = utils.url.urlJoin(utils.url.getSubdir(), utils.url.urlFor('image', {image: settingsCache.get('icon')}));
+            } else {
+                output = utils.url.urlJoin(utils.url.getSubdir(), '/favicon.ico');
+            }
         }
     }
     // Get rid of any leading slash on the path

core/server/data/meta/context_object.js

@@ -1,8 +1,15 @@
-var config = require('../../config'),
-    _      = require('lodash');
+var settingsCache = require('../../api/settings').cache,
+    _ = require('lodash');
 
 function getContextObject(data, context) {
-    var blog = config.get('theme'),
+    /**
+     * If the data object does not contain the requested context, we return the fallback object.
+     */
+    var blog = {
+            cover: settingsCache.get('cover'),
+            twitter: settingsCache.get('twitter'),
+            facebook: settingsCache.get('facebook')
+        },
         contextObject;
 
     context = _.includes(context, 'page') || _.includes(context, 'amp') ? 'post' : context;

core/server/data/meta/description.js

@@ -1,16 +1,17 @@
 var _ = require('lodash'),
-    config = require('../../config');
+    settingsCache = require('../../api/settings').cache;
 
 function getDescription(data, root) {
     var description = '',
-        context = root ? root.context : null;
+        context = root ? root.context : null,
+        blogDescription = settingsCache.get('description');
 
     if (data.meta_description) {
         description = data.meta_description;
     } else if (_.includes(context, 'paged')) {
         description = '';
     } else if (_.includes(context, 'home')) {
-        description = config.get('theme').description;
+        description = blogDescription;
     } else if (_.includes(context, 'author') && data.author) {
         description = data.author.meta_description || data.author.bio;
     } else if (_.includes(context, 'tag') && data.tag) {

core/server/data/meta/index.js

@@ -1,6 +1,5 @@
-var _ = require('lodash'),
-    Promise = require('bluebird'),
-    config = require('../../config'),
+var Promise = require('bluebird'),
+    settingsCache = require('../../api/settings').cache,
     utils = require('../../utils'),
     getUrl = require('./url'),
     getImageDimensions = require('./image-dimensions'),
@@ -46,12 +45,30 @@ function getMetaData(data, root) {
         publishedDate: getPublishedDate(data),
         modifiedDate: getModifiedDate(data),
         ogType: getOgType(data),
-        blog: _.cloneDeep(config.get('theme'))
+        // @TODO: pass into each meta helper - wrap each helper
+        blog: {
+            title: settingsCache.get('title'),
+            description: settingsCache.get('description'),
+            url: utils.url.urlFor('home', true),
+            facebook: settingsCache.get('facebook'),
+            twitter: settingsCache.get('twitter'),
+            timezone: settingsCache.get('activeTimezone'),
+            navigation: settingsCache.get('navigation'),
+            posts_per_page: settingsCache.get('postsPerPage'),
+            icon: settingsCache.get('icon'),
+            cover: settingsCache.get('cover'),
+            logo: settingsCache.get('logo'),
+            amp: settingsCache.get('amp')
+        }
     };
 
     metaData.blog.logo = {};
-    metaData.blog.logo.url = config.get('theme').logo ?
-        utils.url.urlFor('image', {image: config.get('theme').logo}, true) : utils.url.urlJoin(utils.url.urlFor('admin'), 'img/ghosticon.jpg');
+
+    if (settingsCache.get('logo')) {
+        metaData.blog.logo.url = utils.url.urlFor('image', {image: settingsCache.get('logo')}, true);
+    } else {
+        metaData.blog.logo.url = utils.url.urlJoin(utils.url.urlFor('admin'), 'img/ghosticon.jpg');
+    }
 
     // TODO: cleanup these if statements
     if (data.post && data.post.html) {

core/server/data/meta/title.js

@@ -1,28 +1,29 @@
 var _ = require('lodash'),
-    config = require('../../config');
+    settingsCache = require('../../api/settings').cache;
 
 function getTitle(data, root) {
     var title = '',
         context = root ? root.context : null,
-        blog = config.get('theme'),
+        blogTitle = settingsCache.get('title'),
         pagination = root ? root.pagination : null,
         pageString = '';
 
     if (pagination && pagination.total > 1) {
         pageString = ' - Page ' + pagination.page;
     }
+
     if (data.meta_title) {
         title = data.meta_title;
     } else if (_.includes(context, 'home')) {
-        title = blog.title;
+        title = blogTitle;
     } else if (_.includes(context, 'author') && data.author) {
-        title = data.author.name + pageString + ' - ' + blog.title;
+        title = data.author.name + pageString + ' - ' + blogTitle;
     } else if (_.includes(context, 'tag') && data.tag) {
-        title = data.tag.meta_title || data.tag.name + pageString + ' - ' + blog.title;
+        title = data.tag.meta_title || data.tag.name + pageString + ' - ' + blogTitle;
     } else if ((_.includes(context, 'post') || _.includes(context, 'page')) && data.post) {
         title = data.post.meta_title || data.post.title;
     } else {
-        title = blog.title + pageString;
+        title = blogTitle + pageString;
     }
 
     return (title || '').trim();

core/server/data/xml/rss/index.js

@@ -7,6 +7,7 @@ var crypto      = require('crypto'),
     i18n        = require('../../../i18n'),
     filters     = require('../../../filters'),
     processUrls = require('../../../utils/make-absolute-urls'),
+    settingsCache = require('../../../api/settings').cache,
 
     // Really ugly temporary hack for location of things
     fetchData   = require('../../../controllers/frontend/fetch-data'),
@@ -41,8 +42,8 @@ function getData(channelOpts, slugParam) {
         if (result.data && result.data.tag) { titleStart = result.data.tag[0].name + ' - ' || ''; }
         if (result.data && result.data.author) { titleStart = result.data.author[0].name + ' - ' || ''; }
 
-        response.title = titleStart + config.get('theme').title;
-        response.description = config.get('theme').description;
+        response.title = titleStart + settingsCache.get('title');
+        response.description = settingsCache.get('description');
         response.results = {
             posts: result.posts,
             meta: result.meta