diff --git a/.eslintrc.yaml b/.eslintrc.yaml index bba83db..035a400 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -5,6 +5,3 @@ env: es2022: true extends: ['@haraka'] - -rules: - no-extra-semi: 1 diff --git a/CHANGELOG.md b/CHANGELOG.md index d047ff6..bc4582e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ ### [1.2.0] - 2024-04-14 +- feat: getDir can parse different types of files in a dir +- feat: all file readers now have load and loadPromise, so that + a feature like getDir can safely use the nicer promise API +- chore: require syntax, prefix node builtins with `node:` +- es6 + - add several uses of `...` (spread operator / param collection) + - replace `for i` with `for ... of` + - consolidate all cases of type detection into configfile.getType + - replace `new Promise` with async/await + - use shortened array function syntax - ci: update to shared haraka/.github - dep: eslint-plugin-haraka -> @haraka/eslint-config - lint: remove duplicate / stale rules from .eslintrc diff --git a/config.js b/config.js index b9109ed..19a8601 100644 --- a/config.js +++ b/config.js @@ -18,22 +18,23 @@ class Config { } } - get(name, type, cb, options) { - const a = this.arrange_args([name, type, cb, options]) - if (!a[1]) a[1] = 'value' + get(...args) { + /* eslint prefer-const: 0 */ + let [name, type, cb, options] = this.arrange_args(args) + if (!type) type = 'value' const full_path = path.isAbsolute(name) ? name - : path.resolve(this.root_path, a[0]) + : path.resolve(this.root_path, name) - let results = cfreader.read_config(full_path, a[1], a[2], a[3]) + let results = cfreader.read_config(full_path, type, cb, options) if (this.overrides_path) { - const overrides_path = path.resolve(this.overrides_path, a[0]) + const overrides_path = path.resolve(this.overrides_path, name) - const overrides = cfreader.read_config(overrides_path, a[1], a[2], a[3]) + const overrides = cfreader.read_config(overrides_path, type, cb, options) - results = merge_config(results, overrides, a[1]) + results = merge_config(results, overrides, type) } // Pass arrays by value to prevent config being modified accidentally. @@ -77,45 +78,27 @@ class Config { let cb let options - for (let i = 0; i < args.length; i++) { - if (args[i] === undefined) continue - switch ( - typeof args[i] // what is it? - ) { + for (const arg of args) { + if ([undefined, null].includes(arg)) continue + switch (typeof arg) { case 'function': - cb = args[i] - break + cb = arg + continue case 'object': - options = args[i] - break + options = arg + continue case 'string': - if (/^(ini|value|list|data|h?json|js|yaml|binary)$/.test(args[i])) { - fs_type = args[i] - break + if (/^(ini|value|list|data|h?json|js|yaml|binary)$/.test(arg)) { + fs_type = arg + continue } - console.log(`unknown string: ${args[i]}`) - break + console.log(`unknown string: ${arg}`) + continue } - // console.log(`unknown arg: ${args[i]}, typeof: ${typeof args[i]}`); + // console.log(`unknown arg: ${arg}, typeof: ${typeof arg}`); } - if (!fs_type) { - const fs_ext = path.extname(fs_name).substring(1) - - switch (fs_ext) { - case 'hjson': - case 'json': - case 'yaml': - case 'js': - case 'ini': - fs_type = fs_ext - break - - default: - fs_type = 'value' - break - } - } + if (!fs_type) fs_type = cfreader.getType(fs_name) return [fs_name, fs_type, cb, options] } diff --git a/configfile.js b/configfile.js index a74a61a..c52224b 100644 --- a/configfile.js +++ b/configfile.js @@ -1,7 +1,8 @@ 'use strict' -const fs = require('fs') -const path = require('path') +const fs = require('node:fs') +const fsp = require('node:fs/promises') +const path = require('node:path') let config_dir_candidates = [ // these work when this file is loaded as require('./config.js') @@ -71,6 +72,28 @@ class cfreader { } } + getType(fileName) { + const ext = path.extname(fileName).substring(1).toLowerCase() + switch (ext) { + case 'hjson': + case 'json': + case 'yaml': + case 'js': + case 'ini': + case 'list': + case 'data': + return ext + case 'yml': + return 'yaml' + case 'pem': + case 'bin': + case 'binary': + return 'binary' + default: + return 'value' + } + } + on_watch_event(name, type, options, cb) { return (fse) => { if (this._sedation_timers[name]) { @@ -220,27 +243,25 @@ class cfreader { return result } - read_dir(name, opts) { + read_dir(name, opts = {}) { return new Promise((resolve, reject) => { this._read_args[name] = { opts } - const type = opts.type || 'binary' - isDirectory(name) - .then(() => { - return fsReadDir(name) - }) - .then((fileList) => { - const reader = require(path.resolve(__dirname, 'readers', type)) - const promises = [] + fsp + .stat(name) + .then((stat) => stat.isDirectory()) + .then(() => fsp.readdir(name)) + .then(async (fileList) => { + const contents = [] for (const file of fileList) { - promises.push(reader.loadPromise(path.resolve(name, file))) + const type = opts.type ?? this.getType(file) + contents.push( + this.load_config(path.resolve(name, file), type, opts), + ) } - return Promise.all(promises) - }) - .then((fileList) => { - // console.log(fileList); - resolve(fileList) + return contents }) + .then(resolve) .catch(reject) if (opts.watchCb) this.fsWatchDir(name) @@ -283,9 +304,7 @@ class cfreader { load_config(name, type, options) { let result - if (!type) { - type = path.extname(name).toLowerCase().substring(1) - } + if (!type) type = this.getType(name) let cfrType = this.get_filetype_reader(type) @@ -380,21 +399,3 @@ class cfreader { } module.exports = new cfreader() - -function isDirectory(filepath) { - return new Promise((resolve, reject) => { - fs.stat(filepath, (err, stat) => { - if (err) return reject(err) - resolve(stat.isDirectory()) - }) - }) -} - -function fsReadDir(filepath) { - return new Promise((resolve, reject) => { - fs.readdir(filepath, (err, fileList) => { - if (err) return reject(err) - resolve(fileList) - }) - }) -} diff --git a/package.json b/package.json index 544fdac..749b373 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "hjson": "^3.2.2" }, "devDependencies": { - "@haraka/eslint-config": "^1.1.1" + "@haraka/eslint-config": "^1.1.3" }, "bugs": { "mail": "haraka.mail@gmail.com", diff --git a/readers/binary.js b/readers/binary.js index 6acc3bd..de22534 100644 --- a/readers/binary.js +++ b/readers/binary.js @@ -1,18 +1,14 @@ 'use strict' -const fs = require('fs') - exports.load = (name) => { - return fs.readFileSync(name) + return require('node:fs').readFileSync(name) } -exports.loadPromise = (name) => { - return new Promise((resolve, reject) => { - fs.readFile(name, (err, content) => { - if (err) return reject(err) - resolve({ path: name, data: content }) - }) - }) +exports.loadPromise = async (name) => { + return { + path: name, + data: await require('node:fs/promises').readFile(name), + } } exports.empty = () => { diff --git a/readers/flat.js b/readers/flat.js index 829eeda..d8c507f 100644 --- a/readers/flat.js +++ b/readers/flat.js @@ -1,11 +1,22 @@ 'use strict' -const fs = require('fs') +exports.load = (...args) => { + return this.parseValue( + ...args, + require('node:fs').readFileSync(args[0], 'UTF-8'), + ) +} + +exports.loadPromise = async (...args) => { + return this.parseValue( + ...args, + await require('node:fs/promises').readFile(args[0], 'UTF-8'), + ) +} -exports.load = (name, type, options, regex) => { +exports.parseValue = (name, type, options, regex, data) => { let result = [] - let data = fs.readFileSync(name, 'UTF-8') if (type === 'data') { while (data.length > 0) { const match = data.match(/^([^\r\n]*)\r?\n?/) @@ -15,15 +26,15 @@ exports.load = (name, type, options, regex) => { return result } - data.split(/\r\n|\r|\n/).forEach((line) => { - if (regex.comment.test(line)) return - if (regex.blank.test(line)) return + for (const line of data.split(/\r\n|\r|\n/)) { + if (regex.comment.test(line)) continue + if (regex.blank.test(line)) continue const line_data = regex.line.exec(line) - if (!line_data) return + if (!line_data) continue result.push(line_data[1].trim()) - }) + } if (result.length && type !== 'list' && type !== 'data') { result = result[0] @@ -63,7 +74,6 @@ exports.empty = (options, type) => { } function in_array(item, array) { - if (!array) return false if (!Array.isArray(array)) return false - return array.indexOf(item) !== -1 + return array.includes(item) } diff --git a/readers/hjson.js b/readers/hjson.js index b3d71f5..29e53ea 100644 --- a/readers/hjson.js +++ b/readers/hjson.js @@ -1,10 +1,13 @@ 'use strict' -const fs = require('fs') const hjson = require('hjson') exports.load = (name) => { - return hjson.parse(fs.readFileSync(name, 'utf8')) + return hjson.parse(require('node:fs').readFileSync(name, 'UTF-8')) +} + +exports.loadPromise = async (name) => { + return hjson.parse(await require('node:fs/promises').readFile(name, 'UTF-8')) } exports.empty = () => { diff --git a/readers/ini.js b/readers/ini.js index b5098f1..57e836d 100644 --- a/readers/ini.js +++ b/readers/ini.js @@ -1,81 +1,77 @@ 'use strict' -const fs = require('fs') +exports.load = (...args) => { + return this.parseIni( + ...args, + require('node:fs').readFileSync(args[0], 'UTF-8'), + ) +} + +exports.loadPromise = async (...args) => { + return this.parseIni( + ...args, + await require('node:fs/promises').readFile(args[0], 'UTF-8'), + ) +} -exports.load = (name, options, regex) => { +exports.parseIni = (name, options = {}, regex, data) => { let result = { main: {} } let current_sect = result.main let current_sect_name = 'main' this.bool_matches = [] - if (options && options.booleans) { + if (options?.booleans) { this.bool_matches = options.booleans.slice() } // Initialize any booleans result = this.init_booleans(options, result) - let match - let setter let pre = '' - fs.readFileSync(name, 'UTF-8') - .split(/\r\n|\r|\n/) - .forEach((line) => { - if (regex.comment.test(line)) return - if (regex.blank.test(line)) return - - match = regex.section.exec(line) - if (match) { - if (!result[match[1]]) result[match[1]] = {} - current_sect = result[match[1]] - current_sect_name = match[1] - return - } - - if (regex.continuation.test(line)) { - pre += line.replace(regex.continuation, '') - return - } - - line = `${pre}${line}` - pre = '' - - match = regex.param.exec(line) - if (!match) { - exports.logger(`Invalid line in config file '${name}': ${line}`) - return - } - - const is_array_match = regex.is_array.exec(match[1]) - if (is_array_match) { - setter = function (key, value) { - key = key.replace('[]', '') - if (!current_sect[key]) current_sect[key] = [] - current_sect[key].push(value) - } - } else { - setter = function (key, value) { - current_sect[key] = value - } - } - - if ( - options && - Array.isArray(options.booleans) && - (exports.bool_matches.indexOf(`${current_sect_name}.${match[1]}`) !== - -1 || - exports.bool_matches.indexOf(`*.${match[1]}`) !== -1) - ) { - current_sect[match[1]] = regex.is_truth.test(match[2]) - // exports.logger(`Using boolean ${current_sect[match[1]]} for ${current_sect_name}.${match[1]}=${match[2]}`, 'logdebug'); - } else if (regex.is_integer.test(match[2])) { - setter(match[1], parseInt(match[2], 10)) - } else if (regex.is_float.test(match[2])) { - setter(match[1], parseFloat(match[2])) - } else { - setter(match[1], match[2]) - } - }) + for (let line of data.split(/\r\n|\r|\n/)) { + if (regex.comment.test(line)) continue + if (regex.blank.test(line)) continue + + let match = regex.section.exec(line) + if (match) { + if (!result[match[1]]) result[match[1]] = {} + current_sect = result[match[1]] + current_sect_name = match[1] + continue + } + + if (regex.continuation.test(line)) { + pre += line.replace(regex.continuation, '') + continue + } + + line = `${pre}${line}` + pre = '' + + match = regex.param.exec(line) + if (!match) { + exports.logger(`Invalid line in config file '${name}': ${line}`) + continue + } + + const keyName = match[1] + const keyVal = match[2] + + const setter = this.getSetter(current_sect, regex.is_array.test(keyName)) + + if ( + exports.isDeclaredBoolean(`${current_sect_name}.${keyName}`) || + exports.isDeclaredBoolean(`*.${keyName}`) + ) { + current_sect[keyName] = regex.is_truth.test(keyVal) + } else if (regex.is_integer.test(keyVal)) { + setter(keyName, parseInt(keyVal, 10)) + } else if (regex.is_float.test(keyVal)) { + setter(keyName, parseFloat(keyVal)) + } else { + setter(keyName, keyVal) + } + } return result } @@ -85,6 +81,25 @@ exports.empty = (options) => { return this.init_booleans(options, { main: {} }) } +exports.getSetter = (current_sect, isArray) => { + if (isArray) { + return (key, value) => { + key = key.replace('[]', '') + if (!current_sect[key]) current_sect[key] = [] + current_sect[key].push(value) + } + } else { + return (key, value) => { + current_sect[key] = value + } + } +} + +exports.isDeclaredBoolean = (entry) => { + if (exports.bool_matches.includes(entry)) return true + return false +} + exports.init_booleans = (options, result) => { if (!options) return result if (!Array.isArray(options.booleans)) return result diff --git a/readers/js.js b/readers/js.js index af6f9c1..b6463fa 100644 --- a/readers/js.js +++ b/readers/js.js @@ -4,6 +4,10 @@ exports.load = (name) => { return require(name) } +exports.loadPromise = async (name) => { + return require(name) +} + exports.empty = () => { return {} } diff --git a/readers/json.js b/readers/json.js index daa1c18..64144fe 100644 --- a/readers/json.js +++ b/readers/json.js @@ -1,9 +1,11 @@ 'use strict' -const fs = require('fs') - exports.load = (name) => { - return JSON.parse(fs.readFileSync(name)) + return JSON.parse(require('node:fs').readFileSync(name)) +} + +exports.loadPromise = async (name) => { + return JSON.parse(await require('node:fs/promises').readFile(name)) } exports.empty = () => { diff --git a/readers/yaml.js b/readers/yaml.js index e4b7d94..35a88dd 100644 --- a/readers/yaml.js +++ b/readers/yaml.js @@ -1,10 +1,13 @@ 'use strict' -const fs = require('fs') const yaml = require('js-yaml') exports.load = (name) => { - return yaml.load(fs.readFileSync(name, 'utf8')) + return yaml.load(require('node:fs').readFileSync(name, 'UTF-8')) +} + +exports.loadPromise = async (name) => { + return yaml.load(await require('node:fs/promises').readFile(name, 'UTF-8')) } exports.empty = () => { diff --git a/test/.eslintrc.yaml b/test/.eslintrc.yaml deleted file mode 100644 index 1837116..0000000 --- a/test/.eslintrc.yaml +++ /dev/null @@ -1,2 +0,0 @@ -rules: - no-unused-vars: 1 diff --git a/test/config.js b/test/config.js index ac3929d..daaf9a5 100644 --- a/test/config.js +++ b/test/config.js @@ -1,8 +1,8 @@ -const assert = require('assert') +const assert = require('node:assert') // const { beforeEach, describe, it } = require('node:test') -const fs = require('fs') -const os = require('os') -const path = require('path') +const fs = require('node:fs') +const os = require('node:os') +const path = require('node:path') function cb() { return false @@ -10,9 +10,8 @@ function cb() { const opts = { booleans: ['arg1'] } function clearRequireCache() { - // node_unit runs all the tests in the same process, so the process.env - // changes affect other tests. Icky. Work around by invalidating - // the require cache, so config and configfile re-initialize + // the tests are run in the same process, so process.env changes affect + // other tests. Invalidate the require cache between tests delete require.cache[`${path.resolve(__dirname, '..', 'config')}.js`] delete require.cache[`${path.resolve(__dirname, '..', 'configfile')}.js`] } @@ -242,7 +241,6 @@ describe('get', function () { done() }) - // config.get('test.ini'); it('test.ini, no opts', function () { _test_get('test.ini', null, null, null, { main: { @@ -307,12 +305,10 @@ describe('get', function () { ) }) - // config.get('test.txt'); it('test.txt', function () { _test_get('test.txt', null, null, null, null) }) - // config.get('test.flat'); it('test.flat, type=', function () { _test_get('test.flat', null, null, null, 'line1') }) @@ -320,12 +316,10 @@ describe('get', function () { // NOTE: the test.flat file had to be duplicated for these tests, to avoid // the config cache from returning invalid results. - // config.get('test.flat', 'value'); it('test.flat, type=value', function () { _test_get('test.value', 'value', null, null, 'line1') }) - // config.get('test.flat', 'list'); it('test.flat, type=list', function () { _test_get('test.list', 'list', null, null, [ 'line1', @@ -335,7 +329,6 @@ describe('get', function () { ]) }) - // config.get('test.flat', 'data'); it('test.flat, type=data', function () { _test_get('test.data', 'data', null, null, [ 'line1', @@ -346,53 +339,47 @@ describe('get', function () { ]) }) - // config.get('test.hjson'); it('test.hjson, type=', function () { _test_get('test.hjson', null, null, null, hjsonRes) }) - // config.get('test.hjson', 'hjson'); it('test.hjson, type=hjson', function () { _test_get('test.hjson', 'hjson', null, null, hjsonRes) }) - // config.get('test.json'); it('test.json, type=', function () { _test_get('test.json', null, null, null, jsonRes) }) - // config.get('test.json', 'json'); it('test.json, type=json', function () { _test_get('test.json', 'json', null, null, jsonRes) }) - // config.get('test.yaml'); it('test.yaml, type=', function () { _test_get('test.yaml', null, null, null, yamlRes) }) - // config.get('test.yaml', 'yaml'); + it('test.yaml, type=yaml', function () { _test_get('test.yaml', 'yaml', null, null, yamlRes) }) - // config.get('missing2.hjson'); + it('missing2.yaml, asked for hjson', function () { _test_get('missing2.hjson', 'hjson', null, null, { matt: 'waz here - hjson type', }) }) - // config.get('missing.json'); + it('missing.yaml, asked for json', function () { _test_get('missing.json', 'json', null, null, { matt: 'waz here' }) }) - it('test.bin, type=binary', function (done) { + it('test.bin, type=binary', function () { const res = this.config.get('test.binary', 'binary') assert.equal(res.length, 120) assert.ok(Buffer.isBuffer(res)) - done() }) - it('fully qualified path: /etc/services', function (done) { + it('fully qualified path: /etc/services', function () { let res if (/^win/.test(process.platform)) { res = this.config.get('c:\\windows\\win.ini', 'list') @@ -400,23 +387,21 @@ describe('get', function () { res = this.config.get('/etc/services', 'list') } assert.ok(res.length) - done() }) }) describe('merged', function () { beforeEach(testSetup) - it('before_merge', function (done) { + it('before_merge', function () { const lc = this.config.module_config(path.join('test', 'default')) assert.deepEqual(lc.get('test.ini'), { main: {}, defaults: { one: 'one', two: 'two' }, }) - done() }) - it('after_merge', function (done) { + it('after_merge', function () { const lc = this.config.module_config( path.join('test', 'default'), path.join('test', 'override'), @@ -425,16 +410,14 @@ describe('merged', function () { main: {}, defaults: { one: 'three', two: 'four' }, }) - done() }) - it('flat overridden', function (done) { + it('flat overridden', function () { const lc = this.config.module_config( path.join('test', 'default'), path.join('test', 'override'), ) assert.equal(lc.get('test.flat'), 'flatoverrode') - done() }) }) @@ -442,18 +425,16 @@ describe('getInt', function () { beforeEach(testSetup) // config.get('name'); - it('empty filename is NaN', function (done) { + it('empty filename is NaN', function () { const result = this.config.getInt() assert.equal(typeof result, 'number') assert.ok(isNaN(result)) - done() }) - it('empty/missing file contents is NaN', function (done) { + it('empty/missing file contents is NaN', function () { const result = this.config.getInt('test-non-exist') assert.equal(typeof result, 'number') assert.ok(isNaN(result)) - done() }) it('non-existing file returns default', function () { @@ -479,18 +460,22 @@ function cleanup(done) { describe('getDir', function () { beforeEach(function (done) { + process.env.NODE_ENV = 'test' process.env.HARAKA = '' + process.env.WITHOUT_CONFIG_CACHE = '1' + clearRequireCache() this.config = require('../config') cleanup(done) }) it('loads all files in dir', function (done) { this.config.getDir('dir', { type: 'binary' }, (err, files) => { + if (err) console.error(err) assert.ifError(err) assert.equal(err, null) assert.equal(files.length, 3) - assert.equal(files[0].data, `contents1${os.EOL}`) - assert.equal(files[2].data, `contents3${os.EOL}`) + assert.equal(files[0], `contents1${os.EOL}`) + assert.equal(files[2], `contents3${os.EOL}`) done() }) }) @@ -509,6 +494,7 @@ describe('getDir', function () { done() return } + const self = this let callCount = 0 @@ -522,15 +508,16 @@ describe('getDir', function () { // console.log(files); assert.equal(err, null) assert.equal(files.length, 3) - assert.equal(files[0].data, `contents1${os.EOL}`) - assert.equal(files[2].data, `contents3${os.EOL}`) + assert.equal(files[0], `contents1${os.EOL}`) + assert.equal(files[2], `contents3${os.EOL}`) fs.writeFile(tmpFile, 'contents4\n', (err2) => { assert.equal(err2, null) // console.log('file touched, waiting for callback'); }) } if (callCount === 2) { - assert.equal(files[3].data, 'contents4\n') + assert.equal(files[3], 'contents4\n') + fs.unlink(tmpFile, () => {}) done() } }) @@ -548,7 +535,7 @@ describe('hjsonOverrides', function () { it('with smtpgreeting override', function () { process.env.WITHOUT_CONFIG_CACHE = '' - const main = this.config.get('main.hjson') + this.config.get('main.hjson') assert.deepEqual(this.config.get('smtpgreeting', 'list'), [ 'this is line one for hjson', 'this is line two for hjson', @@ -565,7 +552,7 @@ describe('jsonOverrides', function () { it('with smtpgreeting override', function () { process.env.WITHOUT_CONFIG_CACHE = '' - const main = this.config.get('main.json') + this.config.get('main.json') assert.deepEqual(this.config.get('smtpgreeting', 'list'), [ 'this is line one', 'this is line two', diff --git a/test/config/mixed/1.ini b/test/config/mixed/1.ini new file mode 100644 index 0000000..a71a2af --- /dev/null +++ b/test/config/mixed/1.ini @@ -0,0 +1,2 @@ +[sect] +one=true diff --git a/test/config/mixed/2.yml b/test/config/mixed/2.yml new file mode 100644 index 0000000..40272c9 --- /dev/null +++ b/test/config/mixed/2.yml @@ -0,0 +1,2 @@ +main: + two: false diff --git a/test/configfile.js b/test/configfile.js index 024cbeb..24b2751 100644 --- a/test/configfile.js +++ b/test/configfile.js @@ -1,16 +1,15 @@ 'use strict' -const assert = require('assert') - -function _setUp(done) { - process.env.NODE_ENV === 'test' - this.cfreader = require('../configfile') - this.opts = { booleans: ['main.bool_true', 'main.bool_false'] } - done() -} +const assert = require('node:assert') +const path = require('node:path') describe('configfile', function () { - beforeEach(_setUp) + beforeEach(function (done) { + process.env.NODE_ENV === 'test' + this.cfreader = require('../configfile') + this.opts = { booleans: ['main.bool_true', 'main.bool_false'] } + done() + }) describe('load_config', function () { describe('non-exist.ini', function () { @@ -169,6 +168,24 @@ describe('configfile', function () { }) }) + describe('read_dir', function () { + it.skip('returns dir contents', async function () { + // may have race collission with config.getDir test + const result = await this.cfreader.read_dir( + path.resolve('test/config/dir'), + ) + assert.deepEqual(result, ['contents1', 'contents2', 'contents3']) + }) + + it('returns dir with mixed types', async function () { + const result = await this.cfreader.read_dir('test/config/mixed') + assert.deepEqual(result, [ + { main: {}, sect: { one: 'true' } }, + { main: { two: false } }, + ]) + }) + }) + describe('get_filetype_reader', function () { it('binary', function () { const reader = this.cfreader.get_filetype_reader('binary')