From fd4a01199538cb1ec2b79adbd3d5149f44dfa899 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Wed, 1 Jul 2020 17:16:57 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20settings=20cache=20being?= =?UTF-8?q?=20out=20of=20sync=20after=20migrations=20(#11987)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs https://github.com/TryGhost/Ghost/issues/10318 - re-initialize settings cache after migrations by shutting down to clean up event listeners then and calling `init` again - important to ensure `db.ready` event is not emitted until settings have finished re-initializing to avoid problems with background processes using the db connection which is disconnected/re-connected or being kicked off with out-of-date settings --- core/server/index.js | 6 ++-- core/server/services/settings/index.js | 5 +++ test/unit/services/settings/cache_spec.js | 43 +++++++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/core/server/index.js b/core/server/index.js index a5a2b004d0..c4349b9ed9 100644 --- a/core/server/index.js +++ b/core/server/index.js @@ -130,8 +130,10 @@ const minimalRequiredSetupToStartGhost = (dbState) => { migrator.migrate() .then(() => { - events.emit('db.ready'); - return initialiseServices(); + return settings.reinit().then(() => { + events.emit('db.ready'); + return initialiseServices(); + }); }) .then(() => { config.set('maintenance:enabled', false); diff --git a/core/server/services/settings/index.js b/core/server/services/settings/index.js index e1650cb588..3c4991fbe4 100644 --- a/core/server/services/settings/index.js +++ b/core/server/services/settings/index.js @@ -14,5 +14,10 @@ module.exports = { // This will bind to events for further updates SettingsCache.init(settingsCollection); }); + }, + + reinit: function reinit() { + SettingsCache.shutdown(); + return this.init(); } }; diff --git a/test/unit/services/settings/cache_spec.js b/test/unit/services/settings/cache_spec.js index a69ee7ac28..9352c47c0c 100644 --- a/test/unit/services/settings/cache_spec.js +++ b/test/unit/services/settings/cache_spec.js @@ -1,6 +1,8 @@ const rewire = require('rewire'); const should = require('should'); +const sinon = require('sinon'); const _ = require('lodash'); +const {events} = require('../../../../core/server/lib/common'); const publicSettings = require('../../../../core/server/services/settings/public'); let cache = rewire('../../../../core/server/services/settings/cache'); @@ -11,6 +13,10 @@ describe('UNIT: settings cache', function () { cache = rewire('../../../../core/server/services/settings/cache'); }); + afterEach(function () { + sinon.restore(); + }); + it('does not auto convert string into number', function () { cache.set('key1', {value: '1'}); (typeof cache.get('key1')).should.eql('string'); @@ -53,4 +59,41 @@ describe('UNIT: settings cache', function () { cache.getPublic().should.eql(values); }); + + it('can shutdown and init without double handling of events', function () { + const setSpy = sinon.spy(cache, 'set'); + + const settingsCollection = { + models: [{ + get: () => 'key1', + toJSON: () => ({value: 'init value'}) + }] + }; + + cache.init(settingsCollection); + cache.get('key1').should.equal('init value'); + + // check handler only called once on settings.edit + setSpy.callCount.should.equal(1); + events.emit('settings.edited', { + get: () => 'key1', + toJSON: () => ({value: 'first edit'}) + }); + setSpy.callCount.should.equal(2); + cache.get('key1').should.equal('first edit'); + + // shutdown + init + cache.shutdown(); + cache.init(settingsCollection); + setSpy.callCount.should.equal(3); + cache.get('key1').should.equal('init value'); + + // edit again, check event only fired once + events.emit('settings.edited', { + get: () => 'key1', + toJSON: () => ({value: 'second edit'}) + }); + setSpy.callCount.should.equal(4); + cache.get('key1').should.equal('second edit'); + }); });