From b97ae2b41ec98b2de7eda888668ba080d7d10aec Mon Sep 17 00:00:00 2001 From: Otacilio Lacerda Date: Sun, 19 Jul 2020 23:11:27 +0200 Subject: [PATCH] fix: init function now works properly with default params (#7) --- README.md | 2 +- lib/autoConfig.js | 76 +++++++---------------------------------- lib/utils.js | 57 +++++++++++++++++++++++++++++++ test/autoConfig.test.js | 72 ++++++++++++++++++++++++++++++++++++-- 4 files changed, 140 insertions(+), 67 deletions(-) diff --git a/README.md b/README.md index 4cecac8..5e45528 100644 --- a/README.md +++ b/README.md @@ -66,7 +66,7 @@ YAML is a superset of JSON and, as such, is a convenient format for specifying h config["my-app-name"] ``` -⚠️ Although Javascript and YAML are case sensitive and accept properties with same name but different case in the same object (e.g. `variable`/`VARiable`, this lib will through an error in such cases. This constraint has the goal to improve the configuration readability and to make it possible the override of such properties by system variables. +⚠️ Although Javascript and YAML are case sensitive and accept properties with same name but different case in the same object (e.g. `variable`/`VARiable`, this lib will throw an error in such cases. This constraint has the goal to improve the configuration readability and to make it possible the override of such properties by system variables. #### Name Convention diff --git a/lib/autoConfig.js b/lib/autoConfig.js index dc244e6..acab254 100644 --- a/lib/autoConfig.js +++ b/lib/autoConfig.js @@ -1,79 +1,29 @@ -const fs = require('fs'); -const path = require('path'); -const yaml = require('js-yaml'); - const { - isObject, + loadConfiguration, overrideConfigValuesFromSystemVariables, } = require('./utils'); // Global config let config; -/** - * Do a deep merge of two objects. This function will lead to infinite recursion on circular references - * @param target - * @param source - */ -function mergeDeep(target, source) { - const output = { ...target }; - if (isObject(target) && isObject(source)) { - Object.keys(source).forEach(key => { - if (isObject(source[key])) { - if (!(key in target)) Object.assign(output, { [key]: source[key] }); - else output[key] = mergeDeep(target[key], source[key]); - } else { - Object.assign(output, { [key]: source[key] }); - } - }); - } - return output; -} - -/** - * Load a configuration file recursively. The nested config overrides the previous. - * This function will lead to infinite recursion on circular references - * @param confObj - */ -function loadConfig(confObj, filePath, configDirectory) { - let resultConfig = {}; - if (confObj.include) { - if (Array.isArray(confObj)) - throw new Error(`${filePath}: include field must be an array!`); - - confObj.include.forEach(confName => { - const subPath = path.join(configDirectory, `app.${confName}.config.yaml`); - const includeConf = yaml.safeLoad(fs.readFileSync(subPath, 'utf8')); - resultConfig = mergeDeep( - resultConfig, - loadConfig(includeConf, subPath, configDirectory) - ); - }); - } - return mergeDeep(resultConfig, confObj); -} - -function init({ profile = process.env.NODE_ENV, configDirectory = './' }) { - if (!profile) throw new Error('NODE_ENV not set.'); +function init({ + profile = process.env.NODE_ENV, + configDirectory = process.cwd(), +} = {}) { + if (!profile) + throw new Error( + 'No profile was given: set NODE_ENV or pass it as a parameter' + ); if (config) { console.warn( - 'autoConfig.init was called again. Config will be re-initialized.' + 'Config will be re-initialized: autoConfig.init was called again' ); } - try { - // Load config - const filePath = path.join(configDirectory, `app.${profile}.config.yaml`); - - const confObj = yaml.safeLoad(fs.readFileSync(filePath, 'utf8')); - config = loadConfig(confObj, filePath, configDirectory); - - overrideConfigValuesFromSystemVariables(config); - delete config.include; - } catch (e) { - throw new Error(`Auto configuration failed. ${e}`); - } + config = loadConfiguration(configDirectory, profile); + overrideConfigValuesFromSystemVariables(config); + delete config.include; } module.exports = { init, getConfig: () => config }; diff --git a/lib/utils.js b/lib/utils.js index 0b3a54c..1fcc969 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -1,3 +1,7 @@ +const fs = require('fs'); +const path = require('path'); +const yaml = require('js-yaml'); + function isObject(item) { return item && typeof item === 'object' && !Array.isArray(item); } @@ -85,10 +89,63 @@ function overrideConfigValuesFromSystemVariables( }); } +// This function will lead to infinite recursion on circular references +function mergeDeep(target, source) { + const output = { ...target }; + if (isObject(target) && isObject(source)) { + Object.keys(source).forEach(key => { + if (isObject(source[key])) { + if (!(key in target)) Object.assign(output, { [key]: source[key] }); + else output[key] = mergeDeep(target[key], source[key]); + } else { + Object.assign(output, { [key]: source[key] }); + } + }); + } + return output; +} + +function loadYamlFile(configDirectory, profile) { + const filePath = path.join(configDirectory, `app.${profile}.config.yaml`); + return yaml.safeLoad(fs.readFileSync(filePath, 'utf8')); +} + +// This function will lead to an infinite loop on circular references +function loadConfiguration(configDirectory, profile) { + const queue = []; + + const rootConfigObject = loadYamlFile(configDirectory, profile); + + let resultConfig = {}; + let currentConfig = rootConfigObject; + let currentProfile = profile; + do { + if (currentConfig.include) { + if (!Array.isArray(currentConfig.include)) { + throw new Error( + `Include field must be an array in profile: ${profile}!` + ); + } + currentConfig.include.forEach(p => queue.unshift(p)); + } + + currentProfile = queue.shift(); + if (currentProfile) { + currentConfig = loadYamlFile(configDirectory, currentProfile); + resultConfig = mergeDeep(resultConfig, currentConfig); + } + } while (queue.length); + + return mergeDeep(resultConfig, rootConfigObject); +} + module.exports = { isObject, hasValue, getPropertyCaseInsensitive, setPropertyCaseInsensitive, overrideConfigValuesFromSystemVariables, + mergeDeep, + loadYamlFile, + loadConfiguration, }; diff --git a/test/autoConfig.test.js b/test/autoConfig.test.js index 17556f7..97b3867 100644 --- a/test/autoConfig.test.js +++ b/test/autoConfig.test.js @@ -1,5 +1,71 @@ -describe('Test autoConfig', () => { - it('dummy test to setup jest', () => { - expect(true).toBeTruthy(); +const utils = require('../lib/utils'); + +jest.mock('../lib/utils', () => ({ + loadConfiguration: jest.fn(() => ({ + include: [], + })), + overrideConfigValuesFromSystemVariables: jest.fn(), +})); + +describe('test autoConfig', () => { + let autoConfig; + + beforeEach(() => { + jest.isolateModules(() => { + autoConfig = require('../lib/autoConfig'); + }); + }); + + it('should throw error if profile is not defined', () => { + delete process.env.NODE_ENV; + expect(() => autoConfig.init()).toThrowError( + /No profile was given: set NODE_ENV or pass it as a parameter/ + ); + process.env.NODE_ENV = 'test'; + }); + + it('should warn if init is called more than once', () => { + const spy = jest.spyOn(console, 'warn'); + spy.mockImplementation(() => {}); + + autoConfig.init(); + autoConfig.init(); + + expect(console.warn).toHaveBeenCalledWith( + 'Config will be re-initialized: autoConfig.init was called again' + ); + spy.mockRestore(); + }); + + it('should not contain keyword include in the config object', () => { + autoConfig.init(); + expect(autoConfig.getConfig().include).toBeUndefined(); + }); + + it('should configure profile using optional object parameter', () => { + autoConfig.init({ profile: 'mockProfile' }); + expect(utils.loadConfiguration).toHaveBeenCalledWith( + expect.anything(), + 'mockProfile' + ); + }); + + it('should configure config directory using option object parameter', () => { + autoConfig.init({ profile: 'mockProfile', configDirectory: '/mock/test' }); + expect(utils.loadConfiguration).toHaveBeenCalledWith( + '/mock/test', + 'mockProfile' + ); + }); + + it('should be able to get the configuration files', () => { + const mockConfig = { + test: 1, + mock: 'mock', + }; + utils.loadConfiguration.mockImplementationOnce(() => mockConfig); + expect(autoConfig.getConfig()).toBeUndefined(); + autoConfig.init(); + expect(autoConfig.getConfig()).toEqual(mockConfig); }); });