Refactored settings loader to class

refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings

- It's a step to making the module follow class+DI pattern before fully extracting it into an external libarary
- Reminder, doing in Ghost repo instead of substituting big chunks all at once to have clear history of how the service evolved prior to the extraction into external lib!

core/server/services/route-settings/index.js

@@ -1,5 +1,5 @@
 const routeSettings = require('./route-settings');
-const SettingsLoader = require('./loader');
+const SettingsLoader = require('./settings-loader');
 const config = require('../../../shared/config');
 const DefaultSettingsManager = require('./default-settings-manager');
 
@@ -10,13 +10,15 @@ const defaultSettingsManager = new DefaultSettingsManager({
     sourceFolderPath: config.get('paths').defaultSettings
 });
 
+const settingsLoader = new SettingsLoader();
+
 module.exports = {
     init: async () => {
         return await defaultSettingsManager.ensureSettingsFileExists();
     },
 
-    loadRouteSettingsSync: SettingsLoader.loadSettingsSync,
+    loadRouteSettingsSync: settingsLoader.loadSettingsSync.bind(settingsLoader),
-    loadRouteSettings: SettingsLoader.loadSettings,
+    loadRouteSettings: settingsLoader.loadSettings.bind(settingsLoader),
     getDefaultHash: routeSettings.getDefaultHash,
     /**
      * Methods used in the API

core/server/services/route-settings/route-settings.js

@@ -10,7 +10,9 @@ const errors = require('@tryghost/errors');
 const tpl = require('@tryghost/tpl');
 const config = require('../../../shared/config');
 const bridge = require('../../../bridge');
-const SettingsLoader = require('./loader');
+const SettingsLoader = require('./settings-loader');
+
+const settingsLoader = new SettingsLoader();
 
 const messages = {
     loadError: 'Could not load {filename} file.'
@@ -148,7 +150,7 @@ const getDefaultHash = () => {
 };
 
 const getCurrentHash = async () => {
-    const data = await SettingsLoader.loadSettings();
+    const data = await settingsLoader.loadSettings();
 
     return calculateHash(JSON.stringify(data));
 };

core/server/services/route-settings/settings-loader.js

@@ -11,81 +11,92 @@ const messages = {
     settingsLoaderError: `Error trying to load YAML setting for {setting} from '{path}'.`
 };
 
-const getSettingFilePath = (setting) => {
+class SettingsLoader {
-    // we only support the `yaml` file extension. `yml` will be ignored.
+    constructor() {
-    const fileName = `${setting}.yaml`;
-    const contentPath = config.getContentPath('settings');
-    const filePath = path.join(contentPath, fileName);
 
-    return {
+    }
-        fileName,
+
-        contentPath,
+    /**
-        filePath
+     * NOTE: this method will have to go to an external module to reuse in redirects settings
+     * @param {String} setting type of the settings to load, e.g:'routes' or 'redirects'
+     * @returns {Object}
+     */
+    getSettingFilePath(setting) {
+        // we only support the `yaml` file extension. `yml` will be ignored.
+        const fileName = `${setting}.yaml`;
+        const contentPath = config.getContentPath('settings');
+        const filePath = path.join(contentPath, fileName);
+
+        return {
+            fileName,
+            contentPath,
+            filePath
+        };
     };
-};
 
-/**
+    /**
- * Functionally same as loadSettingsSync with exception of loading
+     * Functionally same as loadSettingsSync with exception of loading
- * settings asynchronously. This method is used at new places to read settings
+     * settings asynchronously. This method is used at new places to read settings
- * to prevent blocking the eventloop
+     * to prevent blocking the eventloop
- * @returns {Promise<Object>} settingsFile
+     * @returns {Promise<Object>} settingsFile
- */
+     */
-const loadSettings = async () => {
+    async loadSettings() {
-    const setting = 'routes';
+        const setting = 'routes';
-    const {fileName, contentPath, filePath} = getSettingFilePath(setting);
+        const {fileName, contentPath, filePath} = this.getSettingFilePath(setting);
 
-    try {
+        try {
-        const file = await fs.readFile(filePath, 'utf8');
+            const file = await fs.readFile(filePath, 'utf8');
-        debug('settings file found for', setting);
+            debug('settings file found for', setting);
 
-        const object = yamlParser(file, fileName);
+            const object = yamlParser(file, fileName);
-        return validate(object);
+            return validate(object);
-    } catch (err) {
+        } catch (err) {
-        if (errors.utils.isIgnitionError(err)) {
+            if (errors.utils.isIgnitionError(err)) {
-            throw err;
+                throw err;
+            }
+
+            throw new errors.GhostError({
+                message: tpl(messages.settingsLoaderError, {
+                    setting: setting,
+                    path: contentPath
+                }),
+                context: filePath,
+                err: err
+            });
         }
+    };
 
-        throw new errors.GhostError({
+    /**
-            message: tpl(messages.settingsLoaderError, {
+     * Reads the routes.yaml settings file and passes the
-                setting: setting,
+     * file to the YAML parser which then returns a JSON object.
-                path: contentPath
+     *
-            }),
+     * @returns {Object} settingsFile in following format: {routes: {}, collections: {}, resources: {}}
-            context: filePath,
+     */
-            err: err
+    loadSettingsSync() {
-        });
+        const setting = 'routes';
-    }
+        const {fileName, contentPath, filePath} = this.getSettingFilePath(setting);
-};
 
-/**
+        try {
- * Reads the routes.yaml settings file and passes the
+            const file = fs.readFileSync(filePath, 'utf8');
- * file to the YAML parser which then returns a JSON object.
+            debug('settings file found for', setting);
- *
- * @returns {Object} settingsFile in following format: {routes: {}, collections: {}, resources: {}}
- */
-module.exports.loadSettingsSync = function loadSettingsSync() {
-    const setting = 'routes';
-    const {fileName, contentPath, filePath} = getSettingFilePath(setting);
 
-    try {
+            const object = yamlParser(file, fileName);
-        const file = fs.readFileSync(filePath, 'utf8');
+            return validate(object);
-        debug('settings file found for', setting);
+        } catch (err) {
+            if (errors.utils.isIgnitionError(err)) {
+                throw err;
+            }
 
-        const object = yamlParser(file, fileName);
+            throw new errors.GhostError({
-        return validate(object);
+                message: tpl(messages.settingsLoaderError, {
-    } catch (err) {
+                    setting: setting,
-        if (errors.utils.isIgnitionError(err)) {
+                    path: contentPath
-            throw err;
+                }),
+                context: filePath,
+                err: err
+            });
         }
-
-        throw new errors.GhostError({
-            message: tpl(messages.settingsLoaderError, {
-                setting: setting,
-                path: contentPath
-            }),
-            context: filePath,
-            err: err
-        });
     }
-};
+}
 
-module.exports.loadSettings = loadSettings;
+module.exports = SettingsLoader;

test/unit/services/settings/settings-loader.test.js

@@ -5,9 +5,9 @@ const fs = require('fs-extra');
 const path = require('path');
 const configUtils = require('../../../utils/configUtils');
 const errors = require('@tryghost/errors');
-const loadSettings = rewire('../../../../core/server/services/route-settings/loader');
+const SettingsLoader = rewire('../../../../core/server/services/route-settings/settings-loader');
 
-describe('UNIT > Settings Service loader:', function () {
+describe('UNIT > SettingsLoader:', function () {
     beforeEach(function () {
         configUtils.set('paths:contentPath', path.join(__dirname, '../../../utils/fixtures/'));
     });
@@ -38,6 +38,7 @@ describe('UNIT > Settings Service loader:', function () {
         });
 
         it('reads a settings object for routes.yaml file', function () {
+            const settingsLoader = new SettingsLoader();
             const settingsStubFile = {
                 routes: null,
                 collections: {
@@ -53,7 +54,7 @@ describe('UNIT > Settings Service loader:', function () {
             };
             const fsReadFileStub = sinon.stub(fs, 'readFileSync').returns(settingsStubFile);
 
-            const result = loadSettings.loadSettingsSync();
+            const result = settingsLoader.loadSettingsSync();
             should.exist(result);
             result.should.be.an.Object().with.properties('routes', 'collections', 'taxonomies');
             fsReadFileStub.calledOnce.should.be.true();
@@ -66,10 +67,11 @@ describe('UNIT > Settings Service loader:', function () {
             yamlParserStub.returns(yamlStubFile);
             validateStub.returns({routes: {}, collections: {}, taxonomies: {}});
 
-            loadSettings.__set__('yamlParser', yamlParserStub);
+            SettingsLoader.__set__('yamlParser', yamlParserStub);
-            loadSettings.__set__('validate', validateStub);
+            SettingsLoader.__set__('validate', validateStub);
 
-            const setting = loadSettings.loadSettingsSync();
+            const settingsLoader = new SettingsLoader();
+            const setting = settingsLoader.loadSettingsSync();
             should.exist(setting);
             setting.should.be.an.Object().with.properties('routes', 'collections', 'taxonomies');
 
@@ -79,15 +81,15 @@ describe('UNIT > Settings Service loader:', 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'
             }));
 
-            loadSettings.__set__('yamlParser', yamlParserStub);
+            const settingsLoader = new SettingsLoader();
-
             try {
-                loadSettings.loadSettingsSync();
+                settingsLoader.loadSettingsSync();
                 done(new Error('Loader should fail'));
             } catch (err) {
                 should.exist(err);
@@ -112,11 +114,13 @@ describe('UNIT > Settings Service loader:', function () {
                 return originalFn(filePath, options);
             });
 
+            SettingsLoader.__set__('yamlParser', yamlParserStub);
             yamlParserStub = sinon.spy();
-            loadSettings.__set__('yamlParser', yamlParserStub);
+
+            const settingsLoader = new SettingsLoader();
 
             try {
-                loadSettings.loadSettingsSync();
+                settingsLoader.loadSettingsSync();
                 done(new Error('Loader should fail'));
             } catch (err) {
                 err.message.should.match(/Error trying to load YAML setting for routes from/);