From 0bf1542bc688b8f6ac5c3069c868e52527175fca Mon Sep 17 00:00:00 2001 From: Naz Gargol Date: Tue, 25 Jun 2019 18:33:56 +0200 Subject: [PATCH] Extracted settings service part manipulating routes.yaml (#10800) refs #10790 refs #9528 - The settings service was designed to handle more settings then just routing, but till this day there wasn't anything else added. As routes.yaml is only being used by frontend router so conceptually it fits better to have this code in frontend, so that it doesn't have to reach out to server - The code left in server settings is the one that interacts with the database `settings` table and only partially provides information to frontend. That part is known as 'settings cache' and will be accessed through API controllers. --- core/frontend/services/routing/bootstrap.js | 2 +- .../services/settings/default-routes.yaml | 0 .../services/settings/ensure-settings.js | 4 +- core/frontend/services/settings/index.js | 80 ++++++++++++++++++ .../services/settings/loader.js | 4 +- .../services/settings/validate.js | 4 +- .../services/settings/yaml-parser.js | 2 +- core/server/config/overrides.json | 2 +- core/server/index.js | 6 ++ core/server/services/settings/index.js | 84 +------------------ .../services/settings/ensure-settings_spec.js | 8 +- .../unit/services/settings/loader_spec.js | 4 +- .../unit/services/settings/settings_spec.js | 2 +- .../unit/services/settings/validate_spec.js | 2 +- .../services/settings/yaml-parser_spec.js | 4 +- core/test/utils/index.js | 6 +- 16 files changed, 114 insertions(+), 100 deletions(-) rename core/{server => frontend}/services/settings/default-routes.yaml (100%) rename core/{server => frontend}/services/settings/ensure-settings.js (94%) create mode 100644 core/frontend/services/settings/index.js rename core/{server => frontend}/services/settings/loader.js (92%) rename core/{server => frontend}/services/settings/validate.js (99%) rename core/{server => frontend}/services/settings/yaml-parser.js (95%) diff --git a/core/frontend/services/routing/bootstrap.js b/core/frontend/services/routing/bootstrap.js index 53ec567b35..4ca6e3de92 100644 --- a/core/frontend/services/routing/bootstrap.js +++ b/core/frontend/services/routing/bootstrap.js @@ -1,7 +1,7 @@ const debug = require('ghost-ignition').debug('services:routing:bootstrap'); const _ = require('lodash'); const common = require('../../../server/lib/common'); -const settingsService = require('../../../server/services/settings'); +const settingsService = require('../settings'); const themeService = require('../themes'); const StaticRoutesRouter = require('./StaticRoutesRouter'); const StaticPagesRouter = require('./StaticPagesRouter'); diff --git a/core/server/services/settings/default-routes.yaml b/core/frontend/services/settings/default-routes.yaml similarity index 100% rename from core/server/services/settings/default-routes.yaml rename to core/frontend/services/settings/default-routes.yaml diff --git a/core/server/services/settings/ensure-settings.js b/core/frontend/services/settings/ensure-settings.js similarity index 94% rename from core/server/services/settings/ensure-settings.js rename to core/frontend/services/settings/ensure-settings.js index b685f1e0f9..4861d2ade5 100644 --- a/core/server/services/settings/ensure-settings.js +++ b/core/frontend/services/settings/ensure-settings.js @@ -2,8 +2,8 @@ const fs = require('fs-extra'), Promise = require('bluebird'), path = require('path'), debug = require('ghost-ignition').debug('services:settings:ensure-settings'), - common = require('../../lib/common'), - config = require('../../config'); + common = require('../../../server/lib/common'), + config = require('../../../server/config'); /** * Makes sure that all supported settings files are in the diff --git a/core/frontend/services/settings/index.js b/core/frontend/services/settings/index.js new file mode 100644 index 0000000000..589c1cda08 --- /dev/null +++ b/core/frontend/services/settings/index.js @@ -0,0 +1,80 @@ +const _ = require('lodash'); +const debug = require('ghost-ignition').debug('frontend:services:settings:index'); +const SettingsLoader = require('./loader'); +const ensureSettingsFiles = require('./ensure-settings'); + +const common = require('../../../server/lib/common'); + +module.exports = { + init: function () { + const knownSettings = this.knownSettings(); + + debug('init settings service for:', knownSettings); + + // Make sure that supported settings files are available + // inside of the `content/setting` directory + return ensureSettingsFiles(knownSettings); + }, + + /** + * Global place to switch on more available settings. + */ + knownSettings: function knownSettings() { + return ['routes']; + }, + + /** + * Getter for YAML settings. + * Example: `settings.get('routes').then(...)` + * will return an Object like this: + * {routes: {}, collections: {}, resources: {}} + * @param {String} setting type of supported setting. + * @returns {Object} settingsFile + * @description Returns settings object as defined per YAML files in + * `/content/settings` directory. + */ + get: function get(setting) { + const knownSettings = this.knownSettings(); + + // CASE: this should be an edge case and only if internal usage of the + // getter is incorrect. + if (!setting || _.indexOf(knownSettings, setting) < 0) { + throw new common.errors.IncorrectUsageError({ + message: `Requested setting is not supported: '${setting}'.`, + help: `Please use only the supported settings: ${knownSettings}.` + }); + } + + return SettingsLoader(setting); + }, + + /** + * Getter for all YAML settings. + * Example: `settings.getAll().then(...)` + * will return an Object like this (assuming we're supporting `routes` + * and `globals`): + * { + * routes: { + * routes: null, + * collections: { '/': [Object] }, + * resources: { tag: '/tag/{slug}/', author: '/author/{slug}/' } + * }, + * globals: { + * config: { url: 'testblog.com' } + * } + * } + * @returns {Object} settingsObject + * @description Returns all settings object as defined per YAML files in + * `/content/settings` directory. + */ + getAll: function getAll() { + const knownSettings = this.knownSettings(), + settingsToReturn = {}; + + _.each(knownSettings, function (setting) { + settingsToReturn[setting] = SettingsLoader(setting); + }); + + return settingsToReturn; + } +}; diff --git a/core/server/services/settings/loader.js b/core/frontend/services/settings/loader.js similarity index 92% rename from core/server/services/settings/loader.js rename to core/frontend/services/settings/loader.js index 40c64f6925..14e89dec98 100644 --- a/core/server/services/settings/loader.js +++ b/core/frontend/services/settings/loader.js @@ -1,8 +1,8 @@ const fs = require('fs-extra'), path = require('path'), debug = require('ghost-ignition').debug('services:settings:settings-loader'), - common = require('../../lib/common'), - config = require('../../config'), + common = require('../../../server/lib/common'), + config = require('../../../server/config'), yamlParser = require('./yaml-parser'), validate = require('./validate'); diff --git a/core/server/services/settings/validate.js b/core/frontend/services/settings/validate.js similarity index 99% rename from core/server/services/settings/validate.js rename to core/frontend/services/settings/validate.js index 2ea1f0fecf..fd7e7f292c 100644 --- a/core/server/services/settings/validate.js +++ b/core/frontend/services/settings/validate.js @@ -1,7 +1,7 @@ const _ = require('lodash'); const debug = require('ghost-ignition').debug('services:settings:validate'); -const common = require('../../lib/common'); -const themeService = require('../../../frontend/services/themes'); +const common = require('../../../server/lib/common'); +const themeService = require('../themes'); const _private = {}; let RESOURCE_CONFIG; diff --git a/core/server/services/settings/yaml-parser.js b/core/frontend/services/settings/yaml-parser.js similarity index 95% rename from core/server/services/settings/yaml-parser.js rename to core/frontend/services/settings/yaml-parser.js index 6b969f297d..59b32b1bdb 100644 --- a/core/server/services/settings/yaml-parser.js +++ b/core/frontend/services/settings/yaml-parser.js @@ -1,6 +1,6 @@ const yaml = require('js-yaml'), debug = require('ghost-ignition').debug('services:settings:yaml-parser'), - common = require('../../lib/common'); + common = require('../../../server/lib/common'); /** * Takes a YAML file, parses it and returns a JSON Object diff --git a/core/server/config/overrides.json b/core/server/config/overrides.json index cf915e1af4..4f7f54c9d9 100644 --- a/core/server/config/overrides.json +++ b/core/server/config/overrides.json @@ -6,7 +6,7 @@ "helperTemplates": "core/frontend/helpers/tpl/", "adminViews": "core/server/web/admin/views/", "defaultViews": "core/server/views/", - "defaultSettings": "core/server/services/settings/", + "defaultSettings": "core/frontend/services/settings/", "internalAppPath": "core/frontend/apps/", "internalStoragePath": "core/server/adapters/storage/", "internalSchedulingPath": "core/server/adapters/scheduling/", diff --git a/core/server/index.js b/core/server/index.js index 325c125e2d..93693125a1 100644 --- a/core/server/index.js +++ b/core/server/index.js @@ -77,6 +77,7 @@ function initialiseServices() { const minimalRequiredSetupToStartGhost = (dbState) => { const settings = require('./services/settings'); const models = require('./models'); + const frontendSettings = require('../frontend/services/settings'); const themes = require('../frontend/services/themes'); const GhostServer = require('./ghost-server'); @@ -92,6 +93,11 @@ const minimalRequiredSetupToStartGhost = (dbState) => { return settings.init() .then(() => { debug('Settings done'); + + return frontendSettings.init(); + }) + .then(() => { + debug('Frontend settings done'); return themes.init(); }) .then(() => { diff --git a/core/server/services/settings/index.js b/core/server/services/settings/index.js index 5d304fedaf..e1650cb588 100644 --- a/core/server/services/settings/index.js +++ b/core/server/services/settings/index.js @@ -2,93 +2,17 @@ * Settings Lib * A collection of utilities for handling settings including a cache */ -const _ = require('lodash'), - debug = require('ghost-ignition').debug('services:settings:index'), - common = require('../../lib/common'), - models = require('../../models'), - SettingsCache = require('./cache'), - SettingsLoader = require('./loader'), - ensureSettingsFiles = require('./ensure-settings'); +const models = require('../../models'); +const SettingsCache = require('./cache'); module.exports = { init: function init() { - const knownSettings = this.knownSettings(); - - debug('init settings service for:', knownSettings); - - // Make sure that supported settings files are available - // inside of the `content/setting` directory - return ensureSettingsFiles(knownSettings) - .then(() => { - // Update the defaults - return models.Settings.populateDefaults(); - }) + // Update the defaults + return models.Settings.populateDefaults() .then((settingsCollection) => { // Initialise the cache with the result // This will bind to events for further updates SettingsCache.init(settingsCollection); }); - }, - - /** - * Global place to switch on more available settings. - */ - knownSettings: function knownSettings() { - return ['routes']; - }, - - /** - * Getter for YAML settings. - * Example: `settings.get('routes').then(...)` - * will return an Object like this: - * {routes: {}, collections: {}, resources: {}} - * @param {String} setting type of supported setting. - * @returns {Object} settingsFile - * @description Returns settings object as defined per YAML files in - * `/content/settings` directory. - */ - get: function get(setting) { - const knownSettings = this.knownSettings(); - - // CASE: this should be an edge case and only if internal usage of the - // getter is incorrect. - if (!setting || _.indexOf(knownSettings, setting) < 0) { - throw new common.errors.IncorrectUsageError({ - message: `Requested setting is not supported: '${setting}'.`, - help: `Please use only the supported settings: ${knownSettings}.` - }); - } - - return SettingsLoader(setting); - }, - - /** - * Getter for all YAML settings. - * Example: `settings.getAll().then(...)` - * will return an Object like this (assuming we're supporting `routes` - * and `globals`): - * { - * routes: { - * routes: null, - * collections: { '/': [Object] }, - * resources: { tag: '/tag/{slug}/', author: '/author/{slug}/' } - * }, - * globals: { - * config: { url: 'testblog.com' } - * } - * } - * @returns {Object} settingsObject - * @description Returns all settings object as defined per YAML files in - * `/content/settings` directory. - */ - getAll: function getAll() { - const knownSettings = this.knownSettings(), - settingsToReturn = {}; - - _.each(knownSettings, function (setting) { - settingsToReturn[setting] = SettingsLoader(setting); - }); - - return settingsToReturn; } }; diff --git a/core/test/unit/services/settings/ensure-settings_spec.js b/core/test/unit/services/settings/ensure-settings_spec.js index 4385aa3967..220d679e7a 100644 --- a/core/test/unit/services/settings/ensure-settings_spec.js +++ b/core/test/unit/services/settings/ensure-settings_spec.js @@ -6,9 +6,9 @@ const sinon = require('sinon'), configUtils = require('../../../utils/configUtils'), common = require('../../../../server/lib/common'), - ensureSettings = require('../../../../server/services/settings/ensure-settings'); + ensureSettings = require('../../../../frontend/services/settings/ensure-settings'); -describe('UNIT > Settings Service:', function () { +describe('UNIT > Settings Service ensure settings:', function () { beforeEach(function () { configUtils.set('paths:contentPath', path.join(__dirname, '../../../utils/fixtures/')); sinon.stub(fs, 'readFile'); @@ -32,7 +32,7 @@ describe('UNIT > Settings Service:', function () { }); it('copies default settings file if not found but does not overwrite existing files', function () { - const expectedDefaultSettingsPath = path.join(__dirname, '../../../../server/services/settings/default-globals.yaml'); + const expectedDefaultSettingsPath = path.join(__dirname, '../../../../frontend/services/settings/default-globals.yaml'); const expectedContentPath = path.join(__dirname, '../../../utils/fixtures/settings/globals.yaml'); const fsError = new Error('not found'); fsError.code = 'ENOENT'; @@ -49,7 +49,7 @@ describe('UNIT > Settings Service:', function () { }); it('copies default settings file if no file found', function () { - const expectedDefaultSettingsPath = path.join(__dirname, '../../../../server/services/settings/default-routes.yaml'); + const expectedDefaultSettingsPath = path.join(__dirname, '../../../../frontend/services/settings/default-routes.yaml'); const expectedContentPath = path.join(__dirname, '../../../utils/fixtures/settings/routes.yaml'); const fsError = new Error('not found'); fsError.code = 'ENOENT'; diff --git a/core/test/unit/services/settings/loader_spec.js b/core/test/unit/services/settings/loader_spec.js index fa06f4911d..8319838878 100644 --- a/core/test/unit/services/settings/loader_spec.js +++ b/core/test/unit/services/settings/loader_spec.js @@ -5,9 +5,9 @@ const sinon = require('sinon'), path = require('path'), configUtils = require('../../../utils/configUtils'), common = require('../../../../server/lib/common'), - loadSettings = rewire('../../../../server/services/settings/loader'); + loadSettings = rewire('../../../../frontend/services/settings/loader'); -describe('UNIT > Settings Service:', function () { +describe('UNIT > Settings Service loader:', function () { beforeEach(function () { configUtils.set('paths:contentPath', path.join(__dirname, '../../../utils/fixtures/')); }); diff --git a/core/test/unit/services/settings/settings_spec.js b/core/test/unit/services/settings/settings_spec.js index 37d5e2e671..0751483af1 100644 --- a/core/test/unit/services/settings/settings_spec.js +++ b/core/test/unit/services/settings/settings_spec.js @@ -2,7 +2,7 @@ const sinon = require('sinon'), should = require('should'), rewire = require('rewire'), common = require('../../../../server/lib/common'), - settings = rewire('../../../../server/services/settings'); + settings = rewire('../../../../frontend/services/settings'); describe('UNIT > Settings Service:', function () { afterEach(function () { diff --git a/core/test/unit/services/settings/validate_spec.js b/core/test/unit/services/settings/validate_spec.js index 115ef1c10e..c03e09eba8 100644 --- a/core/test/unit/services/settings/validate_spec.js +++ b/core/test/unit/services/settings/validate_spec.js @@ -2,7 +2,7 @@ const should = require('should'); const sinon = require('sinon'); const common = require('../../../../server/lib/common'); const themesService = require('../../../../frontend/services/themes'); -const validate = require('../../../../server/services/settings/validate'); +const validate = require('../../../../frontend/services/settings/validate'); should.equal(true, true); diff --git a/core/test/unit/services/settings/yaml-parser_spec.js b/core/test/unit/services/settings/yaml-parser_spec.js index f048c94909..fbfea628dd 100644 --- a/core/test/unit/services/settings/yaml-parser_spec.js +++ b/core/test/unit/services/settings/yaml-parser_spec.js @@ -4,9 +4,9 @@ const sinon = require('sinon'), yaml = require('js-yaml'), path = require('path'), - yamlParser = require('../../../../server/services/settings/yaml-parser'); + yamlParser = require('../../../../frontend/services/settings/yaml-parser'); -describe('UNIT > Settings Service:', function () { +describe('UNIT > Settings Service yaml parser:', function () { let yamlSpy; beforeEach(function () { diff --git a/core/test/utils/index.js b/core/test/utils/index.js index 293610268d..693de22493 100644 --- a/core/test/utils/index.js +++ b/core/test/utils/index.js @@ -22,6 +22,7 @@ var Promise = require('bluebird'), urlService = require('../../frontend/services/url'), routingService = require('../../frontend/services/routing'), settingsService = require('../../server/services/settings'), + frontendSettingsService = require('../../frontend/services/settings'), settingsCache = require('../../server/services/settings/cache'), imageLib = require('../../server/lib/image'), web = require('../../server/web'), @@ -895,6 +896,9 @@ startGhost = function startGhost(options) { settingsCache.shutdown(); return settingsService.init(); }) + .then(function () { + return frontendSettingsService.init(); + }) .then(function () { return themes.init(); }) @@ -1109,7 +1113,7 @@ module.exports = { }, init: function () { - const routes = settingsService.get('routes'); + const routes = frontendSettingsService.get('routes'); const collectionRouter = new routingService.CollectionRouter('/', routes.collections['/']); const tagRouter = new routingService.TaxonomyRouter('tag', routes.taxonomies.tag);