Removed method complexity in settings API controller

refs https://github.com/TryGhost/Team/issues/694
refs https://linear.app/tryghost/issue/CORE-13

- The controller code is not meant to contain complex business logic.
Reduced complexity in the settings.read method
- Broke the usual "xxxService" naming pattern here in favor of "xxxBREADService" pattern that members package has been experimenting with recently (0469707f2e/packages/members-api/lib/services/member-bread.js (L25)). This naming choice was made because we already had a "SettingsService" and it would've become quite convoluted distinguishing the naming or doing renames for the sake of having a new temporary location for read/edit/add methods
- Also duplicated `hideValueIfSecret` method code with an intention to move it fully into the BREAD service once the refactoring is completed

core/server/api/canary/settings.js

@@ -10,6 +10,8 @@ const settingsService = require('../../services/settings');
 const settingsCache = require('../../../shared/settings-cache');
 const membersService = require('../../services/members');
 
+const settingsBREADService = settingsService.getSettingsBREADServiceInstance();
+
 module.exports = {
     docName: 'settings',
 
@@ -55,41 +57,7 @@ module.exports = {
             }
         },
         query(frame) {
-            let setting;
-            if (frame.options.key === 'slack') {
-                const slackURL = settingsCache.get('slack_url', {resolve: false});
-                const slackUsername = settingsCache.get('slack_username', {resolve: false});
-
-                setting = slackURL || slackUsername;
-                setting.key = 'slack';
-                setting.value = [{
-                    url: slackURL && slackURL.value,
-                    username: slackUsername && slackUsername.value
-                }];
-            } else {
-                setting = settingsCache.get(frame.options.key, {resolve: false});
-            }
-
-            if (!setting) {
-                return Promise.reject(new NotFoundError({
-                    message: i18n.t('errors.api.settings.problemFindingSetting', {
-                        key: frame.options.key
-                    })
-                }));
-            }
-
-            // @TODO: handle in settings model permissible fn
-            if (setting.group === 'core' && !(frame.options.context && frame.options.context.internal)) {
-                return Promise.reject(new NoPermissionError({
-                    message: i18n.t('errors.api.settings.accessCoreSettingFromExtReq')
-                }));
-            }
-
-            setting = settingsService.hideValueIfSecret(setting);
-
-            return {
-                [frame.options.key]: setting
-            };
+            return settingsBREADService.read(frame.options.key, frame.options.context);
         }
     },
 

core/server/services/settings/index.js

@@ -5,6 +5,7 @@
 const events = require('../../lib/common/events');
 const models = require('../../models');
 const SettingsCache = require('../../../shared/settings-cache');
+const SettingsBREADService = require('./settings-bread-service');
 
 // The string returned when a setting is set as write-only
 const obfuscatedSetting = '••••••••';
@@ -15,6 +16,7 @@ function isSecretSetting(setting) {
 }
 
 // The function that obfuscates a write-only setting
+// NOTE: move this method into SettingsBREADService once once refactoring is finished
 function hideValueIfSecret(setting) {
     if (setting.value && isSecretSetting(setting)) {
         return {...setting, value: obfuscatedSetting};
@@ -22,6 +24,15 @@ function hideValueIfSecret(setting) {
     return setting;
 }
 
+/**
+ * @returns {SettingsBREADService} instance of the PostsService
+ */
+const getSettingsBREADServiceInstance = () => {
+    return new SettingsBREADService({
+        settingsCache: SettingsCache
+    });
+};
+
 module.exports = {
     /**
      * Initialize the cache, used in boot and in testing
@@ -74,5 +85,6 @@ module.exports = {
 
     obfuscatedSetting,
     isSecretSetting,
-    hideValueIfSecret
+    hideValueIfSecret,
+    getSettingsBREADServiceInstance
 };

core/server/services/settings/settings-bread-service.js

@@ -0,0 +1,81 @@
+const tpl = require('@tryghost/tpl');
+const {NotFoundError, NoPermissionError} = require('@tryghost/errors');
+
+const messages = {
+    problemFindingSetting: 'Problem finding setting: {key}',
+    accessCoreSettingFromExtReq: 'Attempted to access core setting from external request'
+};
+
+// The string returned when a setting is set as write-only
+const obfuscatedSetting = '••••••••';
+
+// The function used to decide whether a setting is write-only
+function isSecretSetting(setting) {
+    return /secret/.test(setting.key);
+}
+
+// The function that obfuscates a write-only setting
+function hideValueIfSecret(setting) {
+    if (setting.value && isSecretSetting(setting)) {
+        return {...setting, value: obfuscatedSetting};
+    }
+    return setting;
+}
+
+class SettingsBREADService {
+    /**
+     *
+     * @param {Object} options
+     * @param {Object} options.settingsCache - SettingsCache instance
+     */
+    constructor({settingsCache}) {
+        this.settingsCache = settingsCache;
+    }
+
+    /**
+     *
+     * @param {String} key setting key
+     * @param {Object} [context] API context instance
+     * @returns {Object} an object with a filled out key that comes in a parameter
+     */
+    read(key, context) {
+        let setting;
+
+        if (key === 'slack') {
+            const slackURL = this.settingsCache.get('slack_url', {resolve: false});
+            const slackUsername = this.settingsCache.get('slack_username', {resolve: false});
+
+            setting = slackURL || slackUsername;
+            setting.key = 'slack';
+            setting.value = [{
+                url: slackURL && slackURL.value,
+                username: slackUsername && slackUsername.value
+            }];
+        } else {
+            setting = this.settingsCache.get(key, {resolve: false});
+        }
+
+        if (!setting) {
+            return Promise.reject(new NotFoundError({
+                message: tpl(messages.problemFindingSetting, {
+                    key: key
+                })
+            }));
+        }
+
+        // @TODO: handle in settings model permissible fn
+        if (setting.group === 'core' && !(context && context.internal)) {
+            return Promise.reject(new NoPermissionError({
+                message: tpl(messages.accessCoreSettingFromExtReq)
+            }));
+        }
+
+        setting = hideValueIfSecret(setting);
+
+        return {
+            [key]: setting
+        };
+    }
+}
+
+module.exports = SettingsBREADService;