From 6ee94f66b4dd46530b230b3f44e87dcb614be2fd Mon Sep 17 00:00:00 2001 From: Naz Date: Tue, 23 Nov 2021 20:17:38 +0400 Subject: [PATCH] Fixed invalid settings file path configuration refs https://linear.app/tryghost/issue/CORE-35/refactor-route-and-redirect-settings refs https://github.com/TryGhost/Utils/commit/e457fd5fe0507c10b0a6a8dc5c193009f0e8e10e#diff-b292e8480eee007786cc602f55ed05006a06b8da9fe6934d51fbef8328013278R36 - There were two separate instances of the SettingsPathManager in route-settings and settings-loader causing the configured paths missmatching on test environment. Because of this missmatch, uploading and resetting the routes.yaml file didn't work! --- core/server/services/route-settings/index.js | 15 +++---- .../services/route-settings/route-settings.js | 42 ++++++++----------- .../route-settings/settings-loader.js | 12 ++---- .../services/settings/settings-loader.test.js | 8 ++-- 4 files changed, 32 insertions(+), 45 deletions(-) diff --git a/core/server/services/route-settings/index.js b/core/server/services/route-settings/index.js index 70cbaf67d8..9d6b7106e9 100644 --- a/core/server/services/route-settings/index.js +++ b/core/server/services/route-settings/index.js @@ -1,5 +1,6 @@ const config = require('../../../shared/config'); const parseYaml = require('./yaml-parser'); +const SettingsPathManager = require('@tryghost/settings-path-manager'); let settingsLoader; let routeSettings; @@ -10,8 +11,13 @@ module.exports = { const SettingsLoader = require('./settings-loader'); const DefaultSettingsManager = require('./default-settings-manager'); - routeSettings = new RouteSettings(); - + const settingsPathManager = new SettingsPathManager({type: 'routes', paths: [config.getContentPath('settings')]}); + settingsLoader = new SettingsLoader({parseYaml, settingFilePath: settingsPathManager.getDefaultFilePath()}); + routeSettings = new RouteSettings({ + settingsLoader, + settingsPath: settingsPathManager.getDefaultFilePath(), + backupPath: settingsPathManager.getBackupFilePath() + }); const defaultSettingsManager = new DefaultSettingsManager({ type: 'routes', extension: '.yaml', @@ -19,11 +25,6 @@ module.exports = { sourceFolderPath: config.get('paths').defaultSettings }); - settingsLoader = new SettingsLoader({ - parseYaml, - storageFolderPath: config.getContentPath('settings') - }); - return await defaultSettingsManager.ensureSettingsFileExists(); }, diff --git a/core/server/services/route-settings/route-settings.js b/core/server/services/route-settings/route-settings.js index d7c0a501a3..cc4dbef5b0 100644 --- a/core/server/services/route-settings/route-settings.js +++ b/core/server/services/route-settings/route-settings.js @@ -6,21 +6,7 @@ const urlService = require('../url'); const debug = require('@tryghost/debug')('services:route-settings'); const errors = require('@tryghost/errors'); 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 SettingsPathManager = require('@tryghost/settings-path-manager'); - -const settingsPathManager = new SettingsPathManager({ - type: 'routes', - paths: [config.getContentPath('settings')] -}); - -const settingsLoader = new SettingsLoader({ - parseYaml, - storageFolderPath: config.getContentPath('settings') -}); const messages = { loadError: 'Could not load {filename} file.' @@ -39,7 +25,14 @@ const messages = { */ class RouteSettings { - constructor() { + /** + * + * @param {Object} options + * @param {Object} options.settingsLoader + * @param {String} options.settingsPath + * @param {String} options.backupPath + */ + constructor({settingsLoader, settingsPath, backupPath}) { /** * md5 hashes of default routes settings * @private @@ -53,6 +46,10 @@ class RouteSettings { * @private */ this.ext = 'yaml'; + + this.settingsLoader = settingsLoader; + this.settingsPath = settingsPath; + this.backupPath = backupPath; } /** @@ -104,18 +101,15 @@ class RouteSettings { } async setFromFilePath(filePath) { - const settingsPath = settingsPathManager.getDefaultFilePath(); - const backupPath = settingsPathManager.getBackupFilePath(); - - await this.createBackupFile(settingsPath, backupPath); - await this.saveFile(filePath, settingsPath); + await this.createBackupFile(this.settingsPath, this.backupPath); + await this.saveFile(filePath, this.settingsPath); urlService.resetGenerators({releaseResourcesOnly: true}); const bringBackValidRoutes = async () => { urlService.resetGenerators({releaseResourcesOnly: true}); - await this.restoreBackupFile(settingsPath, backupPath); + await this.restoreBackupFile(this.settingsPath, this.backupPath); return bridge.reloadFrontend(); }; @@ -160,9 +154,7 @@ class RouteSettings { } async get() { - const settingsFilePath = settingsPathManager.getDefaultFilePath(); - - return this.readFile(settingsFilePath); + return this.readFile(this.settingsPath); } calculateHash(data) { @@ -176,7 +168,7 @@ class RouteSettings { } async getCurrentHash() { - const data = await settingsLoader.loadSettings(); + const data = await this.settingsLoader.loadSettings(); return this.calculateHash(JSON.stringify(data)); } diff --git a/core/server/services/route-settings/settings-loader.js b/core/server/services/route-settings/settings-loader.js index c105897724..f2799fc0fb 100644 --- a/core/server/services/route-settings/settings-loader.js +++ b/core/server/services/route-settings/settings-loader.js @@ -3,7 +3,6 @@ const debug = require('@tryghost/debug')('frontend:services:settings:settings-lo const tpl = require('@tryghost/tpl'); const errors = require('@tryghost/errors'); const validate = require('./validate'); -const SettingsPathManager = require('@tryghost/settings-path-manager'); const messages = { settingsLoaderError: `Error trying to load YAML setting for {setting} from '{path}'.` @@ -13,17 +12,12 @@ class SettingsLoader { /** * @param {Object} options * @param {Function} options.parseYaml yaml parser - * @param {String} options.storageFolderPath routes settings file path + * @param {String} options.settingFilePath routes settings file path */ - constructor({parseYaml, storageFolderPath}) { + constructor({parseYaml, settingFilePath}) { this.parseYaml = parseYaml; - const settingsPathManager = new SettingsPathManager({ - type: 'routes', - paths: [storageFolderPath] - }); - - this.settingFilePath = settingsPathManager.getDefaultFilePath(); + this.settingFilePath = settingFilePath; } /** diff --git a/test/unit/server/services/settings/settings-loader.test.js b/test/unit/server/services/settings/settings-loader.test.js index c8719e8c43..cca5b3f84a 100644 --- a/test/unit/server/services/settings/settings-loader.test.js +++ b/test/unit/server/services/settings/settings-loader.test.js @@ -34,7 +34,7 @@ describe('UNIT > SettingsLoader:', function () { it('reads a settings object for routes.yaml file', function () { const settingsLoader = new SettingsLoader({ parseYaml: yamlParserStub, - storageFolderPath: '/content/data' + settingFilePath: '/content/data/routes.yaml' }); const settingsStubFile = { routes: null, @@ -69,7 +69,7 @@ describe('UNIT > SettingsLoader:', function () { const settingsLoader = new SettingsLoader({ parseYaml: yamlParserStub, - storageFolderPath: storageFolderPath + settingFilePath: expectedSettingsFile }); const setting = settingsLoader.loadSettingsSync(); should.exist(setting); @@ -89,7 +89,7 @@ describe('UNIT > SettingsLoader:', function () { const settingsLoader = new SettingsLoader({ parseYaml: yamlParserStub, - storageFolderPath: storageFolderPath + settingFilePath: path.join(storageFolderPath, 'routes.yaml') }); try { settingsLoader.loadSettingsSync(); @@ -122,7 +122,7 @@ describe('UNIT > SettingsLoader:', function () { const settingsLoader = new SettingsLoader({ parseYaml: yamlParserStub, - storageFolderPath: storageFolderPath + settingFilePath: expectedSettingsFile }); try {