From 1ac0ba07de03728315cb32b4c527796ebf8d0b6b Mon Sep 17 00:00:00 2001 From: Naz Date: Thu, 30 Sep 2021 14:25:36 +0200 Subject: [PATCH] Extracted yarml persed dep out of settings loader refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings - Moving internal dependencies to be injected through constructor DI for better testability. This is first step of few more to follow. Not doing it all at once as there's too many thing failing when doing a bulk refactor --- .../services/route-settings/route-settings.js | 3 ++- .../route-settings/settings-loader.js | 15 +++++++++------ .../services/settings/settings-loader.test.js | 19 ++++++++++++------- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/core/server/services/route-settings/route-settings.js b/core/server/services/route-settings/route-settings.js index 47b125fd53..55c37475d7 100644 --- a/core/server/services/route-settings/route-settings.js +++ b/core/server/services/route-settings/route-settings.js @@ -11,8 +11,9 @@ const tpl = require('@tryghost/tpl'); const config = require('../../../shared/config'); const bridge = require('../../../bridge'); const SettingsLoader = require('./settings-loader'); +const parseYaml = require('./yaml-parser'); -const settingsLoader = new SettingsLoader(); +const settingsLoader = new SettingsLoader({parseYaml}); const messages = { loadError: 'Could not load {filename} file.' diff --git a/core/server/services/route-settings/settings-loader.js b/core/server/services/route-settings/settings-loader.js index 0ef1cc598b..82b133ac6e 100644 --- a/core/server/services/route-settings/settings-loader.js +++ b/core/server/services/route-settings/settings-loader.js @@ -4,7 +4,6 @@ const debug = require('@tryghost/debug')('frontend:services:settings:settings-lo const tpl = require('@tryghost/tpl'); const errors = require('@tryghost/errors'); const config = require('../../../shared/config'); -const yamlParser = require('./yaml-parser'); const validate = require('./validate'); const messages = { @@ -12,8 +11,12 @@ const messages = { }; class SettingsLoader { - constructor() { - + /** + * @param {Object} options + * @param {Function} options.parseYaml yaml parser + */ + constructor({parseYaml}) { + this.parseYaml = parseYaml; } /** @@ -48,7 +51,7 @@ class SettingsLoader { const file = await fs.readFile(filePath, 'utf8'); debug('settings file found for', setting); - const object = yamlParser(file, fileName); + const object = this.parseYaml(file, fileName); return validate(object); } catch (err) { if (errors.utils.isIgnitionError(err)) { @@ -64,7 +67,7 @@ class SettingsLoader { err: err }); } - }; + } /** * Reads the routes.yaml settings file and passes the @@ -80,7 +83,7 @@ class SettingsLoader { const file = fs.readFileSync(filePath, 'utf8'); debug('settings file found for', setting); - const object = yamlParser(file, fileName); + const object = this.parseYaml(file, fileName); return validate(object); } catch (err) { if (errors.utils.isIgnitionError(err)) { diff --git a/test/unit/services/settings/settings-loader.test.js b/test/unit/services/settings/settings-loader.test.js index 27bb63b0ac..88a83b8bd0 100644 --- a/test/unit/services/settings/settings-loader.test.js +++ b/test/unit/services/settings/settings-loader.test.js @@ -38,7 +38,9 @@ describe('UNIT > SettingsLoader:', function () { }); it('reads a settings object for routes.yaml file', function () { - const settingsLoader = new SettingsLoader(); + const settingsLoader = new SettingsLoader({ + parseYaml: yamlParserStub + }); const settingsStubFile = { routes: null, collections: { @@ -67,10 +69,11 @@ describe('UNIT > SettingsLoader:', function () { yamlParserStub.returns(yamlStubFile); validateStub.returns({routes: {}, collections: {}, taxonomies: {}}); - SettingsLoader.__set__('yamlParser', yamlParserStub); SettingsLoader.__set__('validate', validateStub); - const settingsLoader = new SettingsLoader(); + const settingsLoader = new SettingsLoader({ + parseYaml: yamlParserStub + }); const setting = settingsLoader.loadSettingsSync(); should.exist(setting); setting.should.be.an.Object().with.properties('routes', 'collections', 'taxonomies'); @@ -81,13 +84,14 @@ describe('UNIT > SettingsLoader:', function () { }); it('can handle errors from YAML parser', function (done) { - SettingsLoader.__set__('yamlParser', yamlParserStub); yamlParserStub.throws(new errors.GhostError({ message: 'could not parse yaml file', context: 'bad indentation of a mapping entry at line 5, column 10' })); - const settingsLoader = new SettingsLoader(); + const settingsLoader = new SettingsLoader({ + parseYaml: yamlParserStub + }); try { settingsLoader.loadSettingsSync(); done(new Error('Loader should fail')); @@ -114,10 +118,11 @@ describe('UNIT > SettingsLoader:', function () { return originalFn(filePath, options); }); - SettingsLoader.__set__('yamlParser', yamlParserStub); yamlParserStub = sinon.spy(); - const settingsLoader = new SettingsLoader(); + const settingsLoader = new SettingsLoader({ + parseYaml: yamlParserStub + }); try { settingsLoader.loadSettingsSync();