Refactored ensure settings module into a class with DI

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

- Ensure settings had only one method but would benefit from class+DI pattern before extracting it into an outside module.
- The logic is now also less coupled with "routes" and single source/destination paths. It's all configureable instead and might be reused if similar pattern is needed for example with redirect settings defaults.

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

@@ -0,0 +1,62 @@
+const fs = require('fs-extra');
+const Promise = require('bluebird');
+const path = require('path');
+const debug = require('@tryghost/debug')('frontend:services:settings:ensure-settings');
+const tpl = require('@tryghost/tpl');
+const errors = require('@tryghost/errors');
+
+const messages = {
+    ensureSettings: 'Error trying to access settings files in {path}.'
+};
+
+class DefaultSettingsManager {
+    /**
+     *
+     * @param {Object} options
+     * @param {String} options.type - name of the setting file
+     * @param {String} options.extension - settings file extension
+     * @param {String} options.destinationFolderPath - path to store the default setting config
+     * @param {String} options.sourceFolderPath - path where the default config can be seeded from
+     */
+    constructor({type, extension, destinationFolderPath, sourceFolderPath}) {
+        this.type = type;
+        this.extension = extension;
+        this.destinationFolderPath = destinationFolderPath;
+        this.sourceFolderPath = sourceFolderPath;
+    }
+
+    /**
+     *
+     * Makes sure the destination folder either contains a file or copies over a default file.
+     * @returns {Promise<any>}
+     */
+    async ensureSettingsFileExists() {
+        const fileName = this.type + this.extension;
+        const defaultFileName = `default-${fileName}`;
+
+        const destinationFilePath = path.join(this.destinationFolderPath, fileName);
+        const defaultFilePath = path.join(this.sourceFolderPath, defaultFileName);
+
+        return Promise.resolve(fs.readFile(destinationFilePath, 'utf8'))
+            .catch({code: 'ENOENT'}, () => {
+                // CASE: file doesn't exist, copy it from our defaults
+                return fs.copy(
+                    defaultFilePath,
+                    destinationFilePath
+                ).then(() => {
+                    debug(`'${defaultFileName}' copied to ${this.destinationFolderPath}.`);
+                });
+            }).catch((error) => {
+                // CASE: we might have a permission error, as we can't access the directory
+                throw new errors.GhostError({
+                    message: tpl(messages.ensureSettings, {
+                        path: this.destinationFolderPath
+                    }),
+                    err: error,
+                    context: error.path
+                });
+            });
+    }
+}
+
+module.exports = DefaultSettingsManager;

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

@@ -1,43 +0,0 @@
-const fs = require('fs-extra');
-const Promise = require('bluebird');
-const path = require('path');
-const debug = require('@tryghost/debug')('frontend:services:settings:ensure-settings');
-const tpl = require('@tryghost/tpl');
-const errors = require('@tryghost/errors');
-const config = require('../../../shared/config');
-
-const messages = {
-    ensureSettings: 'Error trying to access settings files in {path}.'
-};
-
-/**
-* Makes sure file is in the `/content/settings` directory. If not, copy the default over.
- * @param {String} fileName - name of the setting file
- * @returns {Promise<any>}
- */
-module.exports = function ensureSettingsFile(fileName) {
-    const contentPath = config.getContentPath('settings');
-    const defaultSettingsPath = config.get('paths').defaultSettings;
-
-    const defaultFileName = `default-${fileName}`;
-    const filePath = path.join(contentPath, fileName);
-
-    return Promise.resolve(fs.readFile(filePath, 'utf8'))
-        .catch({code: 'ENOENT'}, () => {
-            const defaultFilePath = path.join(defaultSettingsPath, defaultFileName);
-            // CASE: file doesn't exist, copy it from our defaults
-            return fs.copy(
-                defaultFilePath,
-                filePath
-            ).then(() => {
-                debug(`'${defaultFileName}' copied to ${contentPath}.`);
-            });
-        }).catch((error) => {
-            // CASE: we might have a permission error, as we can't access the directory
-            throw new errors.GhostError({
-                message: tpl(messages.ensureSettings, {path: contentPath}),
-                err: error,
-                context: error.path
-            });
-        });
-};

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

@@ -1,12 +1,18 @@
 const routeSettings = require('./route-settings');
 const SettingsLoader = require('./loader');
-const ensureSettingsFile = require('./ensure-settings');
+const config = require('../../../shared/config');
+const DefaultSettingsManager = require('./default-settings-manager');
+
+const defaultSettingsManager = new DefaultSettingsManager({
+    type: 'routes',
+    extension: '.yaml',
+    destinationFolderPath: config.getContentPath('settings'),
+    sourceFolderPath: config.get('paths').defaultSettings
+});
 
 module.exports = {
     init: async () => {
-        // Make sure that supported settings files are available
+        return await defaultSettingsManager.ensureSettingsFileExists();
-        // inside of the `content/setting` directory
-        return ensureSettingsFile('routes.yaml');
     },
 
     loadRouteSettingsSync: SettingsLoader.loadSettingsSync,

test/unit/services/settings/default-settings-manager.test.js

@@ -0,0 +1,84 @@
+const sinon = require('sinon');
+const should = require('should');
+const fs = require('fs-extra');
+const path = require('path');
+const DefaultSettingsManager = require('../../../../core/server/services/route-settings/default-settings-manager');
+
+describe('UNIT > Settings Service DefaultSettingsManager:', function () {
+    beforeEach(function () {
+        sinon.stub(fs, 'readFile');
+        sinon.stub(fs, 'copy');
+    });
+
+    afterEach(function () {
+        sinon.restore();
+    });
+
+    describe('Ensure settings files', function () {
+        it('returns yaml file from settings folder if it exists', async function () {
+            fs.readFile.withArgs(path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml'), 'utf8').resolves('content');
+
+            const defaultSettingsManager = new DefaultSettingsManager({
+                type: 'routes',
+                extension: '.yaml',
+                destinationFolderPath: path.join(__dirname, '../../../utils/fixtures/settings/'),
+                sourceFolderPath: ''
+            });
+
+            await defaultSettingsManager.ensureSettingsFileExists();
+
+            // Assert did not attempt to copy the default config
+            fs.copy.called.should.be.false();
+        });
+
+        it('copies default settings file if no file found', async function () {
+            const destinationFolderPath = path.join(__dirname, '../../../utils/fixtures/settings/');
+            const sourceFolderPath = path.join(__dirname, '../../../../core/server/services/route-settings/');
+
+            const defaultSettingsManager = new DefaultSettingsManager({
+                type: 'routes',
+                extension: '.yaml',
+                destinationFolderPath: destinationFolderPath,
+                sourceFolderPath: sourceFolderPath
+            });
+
+            const fsError = new Error('not found');
+            fsError.code = 'ENOENT';
+
+            const settingsDestinationPath = path.join(destinationFolderPath, 'routes.yaml');
+            fs.readFile.withArgs(settingsDestinationPath, 'utf8').rejects(fsError);
+            fs.copy.withArgs(path.join(sourceFolderPath, 'default-routes.yaml'), settingsDestinationPath).resolves();
+
+            await defaultSettingsManager.ensureSettingsFileExists();
+
+            // Assert attempt to copy the default config
+            fs.copy.calledOnce.should.be.true();
+        });
+
+        it('rejects, if error is not a not found error', async function () {
+            const destinationFolderPath = path.join(__dirname, '../../../utils/fixtures/settings/');
+
+            const defaultSettingsManager = new DefaultSettingsManager({
+                type: 'routes',
+                extension: '.yaml',
+                destinationFolderPath: destinationFolderPath,
+                sourceFolderPath: ''
+            });
+
+            const fsError = new Error('no permission');
+            fsError.code = 'EPERM';
+
+            fs.readFile.withArgs(path.join(destinationFolderPath, 'routes.yaml'), 'utf8').rejects(fsError);
+
+            try {
+                await defaultSettingsManager.ensureSettingsFileExists('routes.yaml');
+                throw new Error('Expected test to fail');
+            } catch (error) {
+                should.exist(error);
+                error.message.should.be.eql(`Error trying to access settings files in ${destinationFolderPath}.`);
+                fs.readFile.calledOnce.should.be.true();
+                fs.copy.called.should.be.false();
+            }
+        });
+    });
+});

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

@@ -1,64 +0,0 @@
-const sinon = require('sinon');
-const should = require('should');
-const fs = require('fs-extra');
-const path = require('path');
-const configUtils = require('../../../utils/configUtils');
-const ensureSettings = require('../../../../core/server/services/route-settings/ensure-settings');
-
-describe('UNIT > Settings Service ensure settings:', function () {
-    beforeEach(function () {
-        configUtils.set('paths:contentPath', path.join(__dirname, '../../../utils/fixtures/'));
-        sinon.stub(fs, 'readFile');
-        sinon.stub(fs, 'copy');
-    });
-
-    afterEach(function () {
-        sinon.restore();
-        configUtils.restore();
-    });
-
-    describe('Ensure settings files', function () {
-        it('returns yaml file from settings folder if it exists', function () {
-            fs.readFile.withArgs(path.join(__dirname, '../../../utils/fixtures/settings/goodroutes.yaml'), 'utf8').resolves('content');
-
-            return ensureSettings('goodroutes.yaml').then(() => {
-                fs.readFile.callCount.should.be.eql(1);
-                fs.copy.called.should.be.false();
-            });
-        });
-
-        it('copies default settings file if no file found', function () {
-            const expectedDefaultSettingsPath = path.join(__dirname, '../../../../core/server/services/route-settings/default-routes.yaml');
-            const expectedContentPath = path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml');
-            const fsError = new Error('not found');
-            fsError.code = 'ENOENT';
-
-            fs.readFile.withArgs(path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml'), 'utf8').rejects(fsError);
-            fs.copy.withArgs(expectedDefaultSettingsPath, expectedContentPath).resolves();
-
-            return ensureSettings('routes.yaml').then(() => {
-                fs.readFile.calledOnce.should.be.true();
-                fs.copy.calledOnce.should.be.true();
-            });
-        });
-
-        it('rejects, if error is not a not found error', function () {
-            const expectedContentPath = path.join(__dirname, '../../../utils/fixtures/settings/');
-            const fsError = new Error('no permission');
-            fsError.code = 'EPERM';
-
-            fs.readFile.withArgs(path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml'), 'utf8').rejects(fsError);
-
-            return ensureSettings('routes.yaml')
-                .then(() => {
-                    throw new Error('Expected test to fail');
-                })
-                .catch((error) => {
-                    should.exist(error);
-                    error.message.should.be.eql(`Error trying to access settings files in ${expectedContentPath}.`);
-                    fs.readFile.calledOnce.should.be.true();
-                    fs.copy.called.should.be.false();
-                });
-        });
-    });
-});