From 37dd187fe6c77a42ac00b2e6918d189cf87c3cbd Mon Sep 17 00:00:00 2001 From: Naz Date: Tue, 6 Sep 2022 12:35:07 +0800 Subject: [PATCH] Added adapter caching based on features refs https://github.com/TryGhost/Toolbox/issues/384 - Adapter cache was not able to store multiple object instances derived from same Base class. This created a need to create boilerplate "shell" classes inheriting from the Base class, e.g.: ImageSizeCacheSyncInMemory etc. - Having feature-based adapter instance caching in the adapter manager allows to simplify configuration and reuse the "base class" instead of creating artificial "shell" classes. - For example with this change both image sizes and settings caches will create separate cache instances deriving from default "Memory" class. Less code, less configuration! --- ghost/adapter-manager/lib/AdapterManager.js | 8 ++-- .../test/AdapterManager.test.js | 44 +++++++++++++++++++ .../cache/ImageSizesCacheSyncInMemory.js | 7 --- .../cache/SettingsCacheSyncInMemory.js | 7 --- .../server/services/adapter-manager/index.js | 4 +- ghost/core/core/shared/config/defaults.json | 6 +-- 6 files changed, 53 insertions(+), 23 deletions(-) delete mode 100644 ghost/core/core/server/adapters/cache/ImageSizesCacheSyncInMemory.js delete mode 100644 ghost/core/core/server/adapters/cache/SettingsCacheSyncInMemory.js diff --git a/ghost/adapter-manager/lib/AdapterManager.js b/ghost/adapter-manager/lib/AdapterManager.js index d61591cf74..9c866687d0 100644 --- a/ghost/adapter-manager/lib/AdapterManager.js +++ b/ghost/adapter-manager/lib/AdapterManager.js @@ -84,8 +84,10 @@ module.exports = class AdapterManager { }); } - if (adapterCache[adapterName]) { - return adapterCache[adapterName]; + // @NOTE: example cache key value 'email:newsletters:custom-newsletter-adapter' + const adapterCacheKey = `${adapterName}:${adapterClassName}`; + if (adapterCache[adapterCacheKey]) { + return adapterCache[adapterCacheKey]; } /** @type AdapterConstructor */ @@ -143,7 +145,7 @@ module.exports = class AdapterManager { } } - adapterCache[adapterName] = adapter; + adapterCache[adapterCacheKey] = adapter; return adapter; } diff --git a/ghost/adapter-manager/test/AdapterManager.test.js b/ghost/adapter-manager/test/AdapterManager.test.js index 284e12b2a9..2fb65b8ae4 100644 --- a/ghost/adapter-manager/test/AdapterManager.test.js +++ b/ghost/adapter-manager/test/AdapterManager.test.js @@ -129,4 +129,48 @@ describe('AdapterManager', function () { should.fail(err, null, 'Should not have errored'); } }); + + it('Creates separate class instances for adapters requested for different features', function () { + const pathsToAdapters = [ + '/path' + ]; + + const loadAdapterFromPath = sinon.stub(); + + loadAdapterFromPath.withArgs('/path/mail/custom') + .returns(CustomMailAdapter); + loadAdapterFromPath.withArgs('/path/mail/default') + .returns(DefaultMailAdapter); + + const adapterManager = new AdapterManager({ + loadAdapterFromPath, + pathsToAdapters + }); + adapterManager.registerAdapter('mail', BaseMailAdapter); + + let mailNewslettersAdapter; + try { + mailNewslettersAdapter = adapterManager.getAdapter('mail:newsletters', 'custom'); + + should.ok(mailNewslettersAdapter instanceof BaseMailAdapter); + should.ok(mailNewslettersAdapter instanceof CustomMailAdapter); + } catch (err) { + should.fail(err, null, 'Should not have errored'); + } + + let mailNotificationsAdapter; + try { + mailNotificationsAdapter = adapterManager.getAdapter('mail:notifications', 'custom'); + + should.ok(mailNotificationsAdapter instanceof BaseMailAdapter); + should.ok(mailNotificationsAdapter instanceof CustomMailAdapter); + } catch (err) { + should.fail(err, null, 'Should not have errored'); + } + + should.notEqual(mailNewslettersAdapter, mailNotificationsAdapter); + + const secondMailNewslettersAdapter = adapterManager.getAdapter('mail:newsletters', 'custom'); + should.equal(mailNewslettersAdapter, secondMailNewslettersAdapter); + }); }); diff --git a/ghost/core/core/server/adapters/cache/ImageSizesCacheSyncInMemory.js b/ghost/core/core/server/adapters/cache/ImageSizesCacheSyncInMemory.js deleted file mode 100644 index e31ca80000..0000000000 --- a/ghost/core/core/server/adapters/cache/ImageSizesCacheSyncInMemory.js +++ /dev/null @@ -1,7 +0,0 @@ -const Memory = require('./Memory'); - -class ImageSizesCacheSyncInMemory extends Memory { - -} - -module.exports = ImageSizesCacheSyncInMemory; diff --git a/ghost/core/core/server/adapters/cache/SettingsCacheSyncInMemory.js b/ghost/core/core/server/adapters/cache/SettingsCacheSyncInMemory.js deleted file mode 100644 index 9d2a1cfd48..0000000000 --- a/ghost/core/core/server/adapters/cache/SettingsCacheSyncInMemory.js +++ /dev/null @@ -1,7 +0,0 @@ -const Memory = require('./Memory'); - -class SettingsCacheSyncInMemory extends Memory { - -} - -module.exports = SettingsCacheSyncInMemory; diff --git a/ghost/core/core/server/services/adapter-manager/index.js b/ghost/core/core/server/services/adapter-manager/index.js index 90d82c5789..291f763172 100644 --- a/ghost/core/core/server/services/adapter-manager/index.js +++ b/ghost/core/core/server/services/adapter-manager/index.js @@ -26,8 +26,8 @@ module.exports = { getAdapter(name) { const adapterServiceConfig = getAdapterServiceConfig(config); - const {adapterType, adapterName, adapterConfig} = resolveAdapterOptions(name, adapterServiceConfig); + const {adapterName, adapterConfig} = resolveAdapterOptions(name, adapterServiceConfig); - return adapterManager.getAdapter(adapterType, adapterName, adapterConfig); + return adapterManager.getAdapter(name, adapterName, adapterConfig); } }; diff --git a/ghost/core/core/shared/config/defaults.json b/ghost/core/core/shared/config/defaults.json index e2ea525d2c..0a7f070c70 100644 --- a/ghost/core/core/shared/config/defaults.json +++ b/ghost/core/core/shared/config/defaults.json @@ -26,10 +26,8 @@ }, "cache": { "active": "Memory", - "settings": "SettingsCacheSyncInMemory", - "SettingsCacheSyncInMemory": {}, - "imageSizes": "ImageSizesCacheSyncInMemory", - "ImageSizesCacheSyncInMemory": {} + "settings": {}, + "imageSizes": {} } }, "storage": {