Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-03 23:00:14 -05:00
Code Issues Activity

Refactored settings cache to use class/DI pattern

Browse source 
refs https://github.com/TryGhost/Toolbox/issues/364

- This is a groundwork which moves the "cache" property in settings cache to be injectable parameter, so we can swap it out with different implementations.
- The module will be broken downn into two concepts - an injectable cache  and a cache manager (the update system)
This commit is contained in:
Naz 2022-08-02 16:30:52 +01:00
parent e112f1cd40
commit 492960b9a8
3 changed files with 124 additions and 107 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

133
ghost/core/core/shared/settings-cache/cache.js
View file

@ -4,61 +4,71 @@
const debug = require('@tryghost/debug')('settings:cache'); const debug = require('@tryghost/debug')('settings:cache');
const _ = require('lodash'); const _ = require('lodash');
const publicSettings = require('./public'); /**
* Why hasn't this been moved to @tryghost/settings-cache yet?
*
* - It currently still couples the frontend and server together in a weird way via the event system
* - See the notes in core/server/lib/common/events
* - There's also a plan to introduce a proper caching layer, and rewrite this on top of that
*/
class CacheManager {
/**
* @prop {Object} options
* @prop {{}} options.cache - object of objects. Holds cached settings, keyed by setting.key, contains the JSON version of the model
* @prop {Object} options.publicSettings - key/value pairs of settings which are publicly accessible
*/
constructor({cache, publicSettings}) {
this.settingsCache = cache;
this.publicSettings = publicSettings;
this.calculatedFields = [];
// Local function, only ever used for initializing this._updateSettingFromModel = this._updateSettingFromModel.bind(this);
// We deliberately call "set" on each model so that set is a consistent interface this._updateCalculatedField = this._updateCalculatedField.bind(this);
const updateSettingFromModel = function updateSettingFromModel(settingModel) { }
// Local function, only ever used for initializing
// We deliberately call "set" on each model so that set is a consistent interface
_updateSettingFromModel(settingModel) {
debug('Auto updating', settingModel.get('key')); debug('Auto updating', settingModel.get('key'));
module.exports.set(settingModel.get('key'), settingModel.toJSON()); this.set(settingModel.get('key'), settingModel.toJSON());
}; }
const updateCalculatedField = function updateCalculatedField(field) { _updateCalculatedField(field) {
return () => { return () => {
debug('Auto updating', field.key); debug('Auto updating', field.key);
module.exports.set(field.key, field.getSetting()); this.set(field.key, field.getSetting());
}; };
}; }
/** _doGet(key, options) {
* ## Cache if (!this.settingsCache[key]) {
* Holds cached settings
* Keyed by setting.key
* Contains the JSON version of the model
* @type {{}} - object of objects
*/
let settingsCache = {};
let _calculatedFields = [];
const doGet = (key, options) => {
if (!settingsCache[key]) {
return; return;
} }
// Don't try to resolve to the value of the setting // Don't try to resolve to the value of the setting
if (options && options.resolve === false) { if (options && options.resolve === false) {
return settingsCache[key]; return this.settingsCache[key];
} }
// Default behaviour is to try to resolve the value and return that // Default behaviour is to try to resolve the value and return that
try { try {
// CASE: handle literal false // CASE: handle literal false
if (settingsCache[key].value === false || settingsCache[key].value === 'false') { if (this.settingsCache[key].value === false || this.settingsCache[key].value === 'false') {
return false; return false;
} }
// CASE: if a string contains a number e.g. "1", JSON.parse will auto convert into integer // CASE: if a string contains a number e.g. "1", JSON.parse will auto convert into integer
if (!isNaN(Number(settingsCache[key].value))) { if (!isNaN(Number(this.settingsCache[key].value))) {
return settingsCache[key].value || null; return this.settingsCache[key].value || null;
} }
return JSON.parse(settingsCache[key].value) || null; return JSON.parse(this.settingsCache[key].value) || null;
} catch (err) { } catch (err) {
return settingsCache[key].value || null; return this.settingsCache[key].value || null;
}
} }
};
/** /**
* *
* IMPORTANT: * IMPORTANT:
* We store settings with a type and a key in the database. * We store settings with a type and a key in the database.
@ -71,10 +81,8 @@ const doGet = (key, options) => {
* *
* But the settings cache does not allow requesting a value by type, only by key. * But the settings cache does not allow requesting a value by type, only by key.
* e.g. settingsCache.get('db_hash') * e.g. settingsCache.get('db_hash')
*/ *
module.exports = { * Get a key from the this.settingsCache
/**
* Get a key from the settingsCache
* Will resolve to the value, including parsing JSON, unless {resolve: false} is passed in as an option * Will resolve to the value, including parsing JSON, unless {resolve: false} is passed in as an option
* In which case the full JSON version of the model will be resolved * In which case the full JSON version of the model will be resolved
* *
@ -83,8 +91,9 @@ module.exports = {
* @return {*} * @return {*}
*/ */
get(key, options) { get(key, options) {
return doGet(key, options); return this._doGet(key, options);
}, }
/** /**
* Set a key on the cache * Set a key on the cache
* The only way to get an object into the cache * The only way to get an object into the cache
@ -93,16 +102,17 @@ module.exports = {
* @param {object} value json version of settings model * @param {object} value json version of settings model
*/ */
set(key, value) { set(key, value) {
settingsCache[key] = _.cloneDeep(value); this.settingsCache[key] = _.cloneDeep(value);
}, }
/** /**
* Get the entire cache object * Get the entire cache object
* Uses clone to prevent modifications from being reflected * Uses clone to prevent modifications from being reflected
* @return {object} cache * @return {object} cache
*/ */
getAll() { getAll() {
return _.cloneDeep(settingsCache); return _.cloneDeep(this.settingsCache);
}, }
/** /**
* Get all the publically accessible cache entries with their correct names * Get all the publically accessible cache entries with their correct names
@ -112,12 +122,13 @@ module.exports = {
getPublic() { getPublic() {
let settings = {}; let settings = {};
_.each(publicSettings, (key, newKey) => { _.each(this.publicSettings, (key, newKey) => {
settings[newKey] = doGet(key) ?? null; settings[newKey] = this._doGet(key) ?? null;
}); });
return settings; return settings;
}, }
/** /**
* Initialise the cache * Initialise the cache
* *
@ -134,45 +145,47 @@ module.exports = {
// // if we have been passed a collection of settings, use this to populate the cache // // if we have been passed a collection of settings, use this to populate the cache
if (settingsCollection && settingsCollection.models) { if (settingsCollection && settingsCollection.models) {
_.each(settingsCollection.models, updateSettingFromModel); _.each(settingsCollection.models, this._updateSettingFromModel);
} }
_calculatedFields = Array.isArray(calculatedFields) ? calculatedFields : []; this.calculatedFields = Array.isArray(calculatedFields) ? calculatedFields : [];
// Bind to events to automatically keep up-to-date // Bind to events to automatically keep up-to-date
events.on('settings.edited', updateSettingFromModel); events.on('settings.edited', this._updateSettingFromModel);
events.on('settings.added', updateSettingFromModel); events.on('settings.added', this._updateSettingFromModel);
events.on('settings.deleted', updateSettingFromModel); events.on('settings.deleted', this._updateSettingFromModel);
// set and bind calculated fields // set and bind calculated fields
_calculatedFields.forEach((field) => { this.calculatedFields.forEach((field) => {
updateCalculatedField(field)(); this._updateCalculatedField(field)();
field.dependents.forEach((dependent) => { field.dependents.forEach((dependent) => {
events.on(`settings.${dependent}.edited`, updateCalculatedField(field)); events.on(`settings.${dependent}.edited`, this._updateCalculatedField(field));
}); });
}); });
return settingsCache; return this.settingsCache;
}, }
/** /**
* Reset both the cache and the listeners, must be called during init * Reset both the cache and the listeners, must be called during init
* @param {EventEmitter} events * @param {EventEmitter} events
*/ */
reset(events) { reset(events) {
settingsCache = {}; this.settingsCache = {};
events.removeListener('settings.edited', updateSettingFromModel); events.removeListener('settings.edited', this._updateSettingFromModel);
events.removeListener('settings.added', updateSettingFromModel); events.removeListener('settings.added', this._updateSettingFromModel);
events.removeListener('settings.deleted', updateSettingFromModel); events.removeListener('settings.deleted', this._updateSettingFromModel);
//unbind calculated fields //unbind calculated fields
_calculatedFields.forEach((field) => { this.calculatedFields.forEach((field) => {
field.dependents.forEach((dependent) => { field.dependents.forEach((dependent) => {
events.removeListener(`settings.${dependent}.edited`, updateCalculatedField(field)); events.removeListener(`settings.${dependent}.edited`, this._updateCalculatedField(field));
}); });
}); });
_calculatedFields = []; this.calculatedFields = [];
} }
}; }
module.exports = CacheManager;

16
ghost/core/core/shared/settings-cache/index.js
View file

@ -1,9 +1,7 @@
/** const CacheManager = require('./cache');
* Why hasn't this been moved to @tryghost/settings-cache yet? const publicSettings = require('./public');
* const cache = {};
* - It currently still couples the frontend and server together in a weird way via the event system
* - See the notes in core/server/lib/common/events const cacheManager = new CacheManager({cache, publicSettings});
* - There's also a plan to introduce a proper caching layer, and rewrite this on top of that
* - Finally, I'm not sure if this shouldn't be two things - a cache, and a cache manager (the update system) module.exports = cacheManager;
*/
module.exports = require('./cache');

12
ghost/core/test/unit/shared/settings-cache.test.js
View file

@ -4,13 +4,19 @@ const _ = require('lodash');
const events = require('../../../core/server/lib/common/events'); const events = require('../../../core/server/lib/common/events');
// Testing the Private API // Testing the Private API
let cache = require('../../../core/shared/settings-cache/cache'); let CacheManager = require('../../../core/shared/settings-cache/cache');
const publicSettings = require('../../../core/shared/settings-cache/public'); const publicSettings = require('../../../core/shared/settings-cache/public');
should.equal(true, true); should.equal(true, true);
describe('UNIT: settings cache', function () { describe('UNIT: settings cache', function () {
let cache;
beforeEach(function () { beforeEach(function () {
cache = new CacheManager({
cache: {},
publicSettings
});
cache.init(events, {}, []); cache.init(events, {}, []);
}); });
@ -123,7 +129,7 @@ describe('UNIT: settings cache', function () {
}] }]
}; };
cache.init(events, settingsCollection); cache.init(events, settingsCollection, []);
cache.get('key1').should.equal('init value'); cache.get('key1').should.equal('init value');
// check handler only called once on settings.edit // check handler only called once on settings.edit
@ -136,7 +142,7 @@ describe('UNIT: settings cache', function () {
cache.get('key1').should.equal('first edit'); cache.get('key1').should.equal('first edit');
// init does a reset by default // init does a reset by default
cache.init(events, settingsCollection); cache.init(events, settingsCollection, []);
setSpy.callCount.should.equal(3); setSpy.callCount.should.equal(3);
cache.get('key1').should.equal('init value'); cache.get('key1').should.equal('init value');