From 0f86a05ed4ab1f8dd4b9a4bbebac63ead2728d94 Mon Sep 17 00:00:00 2001 From: Naz Date: Tue, 6 Sep 2022 12:05:59 +0800 Subject: [PATCH] Added ":" syntax to adapter manager refs https://github.com/TryGhost/Toolbox/issues/384 - The syntax using a colon ":" separator has been successfully used to enable multiple adapters. The adapter manager can benefit from same convention to enable more elastic adapter cache - have multiple instances of adapters from same base class --- ghost/adapter-manager/lib/AdapterManager.js | 27 ++++++---- .../test/AdapterManager.test.js | 52 +++++++++++++++++++ 2 files changed, 69 insertions(+), 10 deletions(-) diff --git a/ghost/adapter-manager/lib/AdapterManager.js b/ghost/adapter-manager/lib/AdapterManager.js index 2db835cb58..d61591cf74 100644 --- a/ghost/adapter-manager/lib/AdapterManager.js +++ b/ghost/adapter-manager/lib/AdapterManager.js @@ -56,19 +56,26 @@ module.exports = class AdapterManager { /** * getAdapter * - * @param {string} adapterType The type of adapter, e.g. "storage" or "scheduling" - * @param {string} adapterName The active adapter, e.g. "LocalFileStorage" + * @param {string} adapterName The name of the type of adapter, e.g. "storage" or "scheduling", optionally including the feature, e.g. "storage:files" + * @param {string} adapterClassName The active adapter instance class name e.g. "LocalFileStorage" * @param {object} [config] The config the adapter could be instantiated with * * @returns {Adapter} The resolved and instantiated adapter */ - getAdapter(adapterType, adapterName, config) { - if (!adapterType || !adapterName) { + getAdapter(adapterName, adapterClassName, config) { + if (!adapterName || !adapterClassName) { throw new errors.IncorrectUsageError({ - message: 'getAdapter must be called with a adapterType and a name.' + message: 'getAdapter must be called with a adapterName and a adapterClassName.' }); } + let adapterType; + if (adapterName.includes(':')) { + [adapterType] = adapterName.split(':'); + } else { + adapterType = adapterName; + } + const adapterCache = this.instanceCache[adapterType]; if (!adapterCache) { @@ -84,7 +91,7 @@ module.exports = class AdapterManager { /** @type AdapterConstructor */ let Adapter; for (const pathToAdapters of this.pathsToAdapters) { - const pathToAdapter = path.join(pathToAdapters, adapterType, adapterName); + const pathToAdapter = path.join(pathToAdapters, adapterType, adapterClassName); try { Adapter = this.loadAdapterFromPath(pathToAdapter); if (Adapter) { @@ -108,7 +115,7 @@ module.exports = class AdapterManager { if (!Adapter) { throw new errors.IncorrectUsageError({ - message: `Unable to find ${adapterType} adapter ${adapterName} in ${this.pathsToAdapters}.` + message: `Unable to find ${adapterType} adapter ${adapterClassName} in ${this.pathsToAdapters}.` }); } @@ -117,21 +124,21 @@ module.exports = class AdapterManager { if (!(adapter instanceof this.baseClasses[adapterType])) { if (Object.getPrototypeOf(Adapter).name !== this.baseClasses[adapterType].name) { throw new errors.IncorrectUsageError({ - message: `${adapterType} adapter ${adapterName} does not inherit from the base class.` + message: `${adapterType} adapter ${adapterClassName} does not inherit from the base class.` }); } } if (!adapter.requiredFns) { throw new errors.IncorrectUsageError({ - message: `${adapterType} adapter ${adapterName} does not have the requiredFns.` + message: `${adapterType} adapter ${adapterClassName} does not have the requiredFns.` }); } for (const requiredFn of adapter.requiredFns) { if (typeof adapter[requiredFn] !== 'function') { throw new errors.IncorrectUsageError({ - message: `${adapterType} adapter ${adapterName} is missing the ${requiredFn} method.` + message: `${adapterType} adapter ${adapterClassName} is missing the ${requiredFn} method.` }); } } diff --git a/ghost/adapter-manager/test/AdapterManager.test.js b/ghost/adapter-manager/test/AdapterManager.test.js index 2d1d644bc9..284e12b2a9 100644 --- a/ghost/adapter-manager/test/AdapterManager.test.js +++ b/ghost/adapter-manager/test/AdapterManager.test.js @@ -19,6 +19,29 @@ class DefaultMailAdapter extends BaseMailAdapter { } describe('AdapterManager', function () { + it('getAdapter throws if called without correct parameters', function () { + const pathsToAdapters = [ + 'first/path' + ]; + + const loadAdapterFromPath = sinon.stub(); + const adapterManager = new AdapterManager({ + loadAdapterFromPath, + pathsToAdapters + }); + + adapterManager.registerAdapter('mail', BaseMailAdapter); + + try { + adapterManager.getAdapter('mail'); + should.fail(null, null, 'Should not have created'); + } catch (err) { + should.exist(err); + should.equal(err.errorType, 'IncorrectUsageError'); + should.equal(err.message, 'getAdapter must be called with a adapterName and a adapterClassName.'); + } + }); + it('Loads registered adapters in the order defined by the paths', function () { const pathsToAdapters = [ 'first/path', @@ -58,6 +81,7 @@ describe('AdapterManager', function () { } catch (err) { should.exist(err); should.equal(err.errorType, 'IncorrectUsageError'); + should.equal(err.message, 'mail adapter incomplete is missing the someMethod method.'); } try { @@ -77,4 +101,32 @@ describe('AdapterManager', function () { should.equal(err.errorType, 'IncorrectUsageError'); } }); + + it('Reads adapter type from the adapter name divided with a colon (adapter:feature syntax)', 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); + + try { + const customAdapter = adapterManager.getAdapter('mail:newsletters', 'custom'); + + should.ok(customAdapter instanceof BaseMailAdapter); + should.ok(customAdapter instanceof CustomMailAdapter); + } catch (err) { + should.fail(err, null, 'Should not have errored'); + } + }); });