0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-04-01 02:41:39 -05:00

Used in memory cache adapter in settigs cache manager

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

- Settings Manager used to store all of it's settings values in a hash - an in memory cache in disguise. Having a hidden cache made it hard to reason about it's impact of memory usage and did not allow to swap it out for an alternative storage metchanism easily. Having a cache storage abstraction in Settings Manager allows to get rid of long lasting memory problems + decouples storage mechanism from the logic around transforming stored values.
This commit is contained in:
Naz 2022-08-04 13:06:33 +01:00 committed by naz
parent ed79d3e9b3
commit 2de7688eaf
5 changed files with 42 additions and 19 deletions

View file

@ -15,11 +15,12 @@ const adapterManager = new AdapterManager({
adapterManager.registerAdapter('storage', require('ghost-storage-base'));
adapterManager.registerAdapter('scheduling', require('../../adapters/scheduling/SchedulingBase'));
adapterManager.registerAdapter('sso', require('../../adapters/sso/Base'));
adapterManager.registerAdapter('cache', require('../../adapters/cache/Base'));
module.exports = {
/**
*
* @param {String} name - one of 'storage', 'scheduling', 'sso' etc. Or can contain a "resource" extension like "storage:image"
* @param {String} name - one of 'storage', 'scheduling', 'sso', 'cache' etc. Or can contain a "resource" extension like "storage:image"
* @returns {Object} instance of an adapter
*/
getAdapter(name) {

View file

@ -9,6 +9,7 @@ const events = require('../../lib/common/events');
const models = require('../../models');
const labs = require('../../../shared/labs');
const config = require('../../../shared/config');
const adapterManager = require('../adapter-manager');
const SettingsCache = require('../../../shared/settings-cache');
const SettingsBREADService = require('./settings-bread-service');
const {obfuscatedSetting, isSecretSetting, hideValueIfSecret} = require('./settings-utils');
@ -66,8 +67,9 @@ module.exports = {
* Initialize the cache, used in boot and in testing
*/
async init() {
const cacheStore = adapterManager.getAdapter('cache');
const settingsCollection = await models.Settings.populateDefaults();
SettingsCache.init(events, settingsCollection, this.getCalculatedFields(), {});
SettingsCache.init(events, settingsCollection, this.getCalculatedFields(), cacheStore);
},
/**

View file

@ -23,6 +23,9 @@
"adapters": {
"sso": {
"active": "Default"
},
"cache": {
"active": "Memory"
}
},
"storage": {

View file

@ -50,30 +50,30 @@ class CacheManager {
// NOTE: "!this.settingsCache" is for when setting's cache is used
// before it had a chance to initialize. Should be fixed when
// it is decoupled from the model layer
if (!this.settingsCache || !this.settingsCache[key]) {
if (!this.settingsCache || !this.settingsCache.get(key)) {
return;
}
// Don't try to resolve to the value of the setting
if (options && options.resolve === false) {
return this.settingsCache[key];
return this.settingsCache.get(key);
}
// Default behavior is to try to resolve the value and return that
try {
// CASE: handle literal false
if (this.settingsCache[key].value === false || this.settingsCache[key].value === 'false') {
if (this.settingsCache.get(key).value === false || this.settingsCache.get(key).value === 'false') {
return false;
}
// CASE: if a string contains a number e.g. "1", JSON.parse will auto convert into integer
if (!isNaN(Number(this.settingsCache[key].value))) {
return this.settingsCache[key].value || null;
if (!isNaN(Number(this.settingsCache.get(key).value))) {
return this.settingsCache.get(key).value || null;
}
return JSON.parse(this.settingsCache[key].value) || null;
return JSON.parse(this.settingsCache.get(key).value) || null;
} catch (err) {
return this.settingsCache[key].value || null;
return this.settingsCache.get(key).value || null;
}
}
@ -111,16 +111,27 @@ class CacheManager {
* @param {object} value json version of settings model
*/
set(key, value) {
this.settingsCache[key] = _.cloneDeep(value);
this.settingsCache.set(key, _.cloneDeep(value));
}
/**
* Get the entire cache object
* Uses clone to prevent modifications from being reflected
* This method is dangerous in case the cache is "lazily" initialized
* could result in returning only a partially filled cache
* @return {object} cache
* @deprecated this method is not "cache-friendly" and should be avoided from furhter usage
* instead using multiple "get" calls
*/
getAll() {
return _.cloneDeep(this.settingsCache);
const keys = this.settingsCache.keys();
const all = {};
keys.forEach((key) => {
all[key] = _.cloneDeep(this.settingsCache.get(key));
});
return all;
}
/**
@ -139,15 +150,15 @@ class CacheManager {
}
/**
* Initialise the cache
* Initialize the cache
*
* Optionally takes a collection of settings & can populate the cache with these.
*
* @param {EventEmitter} events
* @param {Bookshelf.Collection<Settings>} settingsCollection
* @param {Array} calculatedFields
* @param {Object} cacheStore - cache storage instance
* @return {object}
* @param {Object} cacheStore - cache storage instance base on Cache Base Adapter
* @return {Object} - filled out instance for Cache Base Adapter
*/
init(events, settingsCollection, calculatedFields, cacheStore) {
this.settingsCache = cacheStore;
@ -182,7 +193,9 @@ class CacheManager {
* @param {EventEmitter} events
*/
reset(events) {
this.settingsCache = {};
if (this.settingsCache) {
this.settingsCache.reset();
}
events.removeListener('settings.edited', this._updateSettingFromModel);
events.removeListener('settings.added', this._updateSettingFromModel);

View file

@ -6,6 +6,7 @@ const events = require('../../../core/server/lib/common/events');
// Testing the Private API
let CacheManager = require('../../../core/shared/settings-cache/cache');
const publicSettings = require('../../../core/shared/settings-cache/public');
const InMemoryCache = require('../../../core/server/adapters/cache/Memory');
should.equal(true, true);
@ -13,10 +14,11 @@ describe('UNIT: settings cache', function () {
let cache;
beforeEach(function () {
let cacheStore = new InMemoryCache();
cache = new CacheManager({
publicSettings
});
cache.init(events, {}, [], {});
cache.init(events, {}, [], cacheStore);
});
afterEach(function () {
@ -28,7 +30,7 @@ describe('UNIT: settings cache', function () {
(typeof cache.get('key1')).should.eql('string');
});
it('.get() oes not auto convert string into number: float', function () {
it('.get() does not auto convert string into number: float', function () {
cache.set('key1', {value: '1.4'});
(typeof cache.get('key1')).should.eql('string');
});
@ -128,7 +130,8 @@ describe('UNIT: settings cache', function () {
}]
};
cache.init(events, settingsCollection, []);
let cacheStore = new InMemoryCache();
cache.init(events, settingsCollection, [], cacheStore);
cache.get('key1').should.equal('init value');
// check handler only called once on settings.edit
@ -141,7 +144,8 @@ describe('UNIT: settings cache', function () {
cache.get('key1').should.equal('first edit');
// init does a reset by default
cache.init(events, settingsCollection, []);
let cacheStoreForReset = new InMemoryCache();
cache.init(events, settingsCollection, [], cacheStoreForReset);
setSpy.callCount.should.equal(3);
cache.get('key1').should.equal('init value');