From c3c315d64862b0695d6f90c9e9e2372a0e20b3ce Mon Sep 17 00:00:00 2001 From: indexzero Date: Sun, 20 Nov 2011 16:21:09 -0500 Subject: [PATCH] [refactor] Refactor to make using nconf more fluent. --- lib/nconf/provider.js | 157 ++++++--------------- lib/nconf/stores/argv.js | 59 ++++++++ lib/nconf/stores/env.js | 51 +++++++ lib/nconf/stores/literal.js | 18 +++ lib/nconf/stores/system.js | 118 ---------------- package.json | 2 +- test/fixtures/scripts/nconf-argv.js | 3 +- test/fixtures/scripts/nconf-change-argv.js | 6 +- test/fixtures/scripts/nconf-env.js | 3 +- test/fixtures/scripts/provider-argv.js | 2 +- test/fixtures/scripts/provider-env.js | 2 +- test/helpers.js | 4 +- test/nconf-test.js | 2 +- test/provider-test.js | 32 +---- test/stores/argv-test.js | 22 +++ test/stores/env-test.js | 23 +++ test/stores/literal-test.js | 30 ++++ test/stores/system-test.js | 26 ---- 18 files changed, 267 insertions(+), 293 deletions(-) create mode 100644 lib/nconf/stores/argv.js create mode 100644 lib/nconf/stores/env.js create mode 100644 lib/nconf/stores/literal.js delete mode 100644 lib/nconf/stores/system.js create mode 100644 test/stores/argv-test.js create mode 100644 test/stores/env-test.js create mode 100644 test/stores/literal-test.js delete mode 100644 test/stores/system-test.js diff --git a/lib/nconf/provider.js b/lib/nconf/provider.js index 10e31ca8..425bc048 100644 --- a/lib/nconf/provider.js +++ b/lib/nconf/provider.js @@ -24,21 +24,9 @@ var Provider = exports.Provider = function (options) { // Setup default options for working with `stores`, // `overrides`, `process.env` and `process.argv`. // - options = options || {}; - this._overrides = options.overrides || null; - this._argv = options.argv || false; - this._env  = options.env || false; - this._reserved = Object.keys(Provider.prototype); - this._stores = []; - this.sources = []; - - // - // Add the default `system` store for working with - // `overrides`, `process.env`, `process.argv` and - // a simple in-memory objects. - // - this.add('system', options); - + options = options || {}; + this.sources = []; + // // Add any stores passed in through the options // to this instance. @@ -55,21 +43,28 @@ var Provider = exports.Provider = function (options) { self.add(store.name || name || store.type, store); }); } - - // - // Add any read-only sources to this instance - // - if (options.source) { - this.sources.push(this.create(options.source.type || options.source.name, options.source)); - } - else if (options.sources) { - Object.keys(options.sources).forEach(function (name) { - var source = options.sources[name]; - self.sources.push(self.create(source.type || source.name || name, source)); - }); - } }; +// +// Define wrapper functions for using basic stores +// in this instance +// +['argv', 'env', 'file'].forEach(function (type) { + Provider.prototype[type] = function (options) { + return this.add(type, options); + }; +}); + +// +// Define wrapper functions for using +// overrides and defaults +// +['defaults', 'overrides'].forEach(function (type) { + Provider.prototype[type] = function (options) { + return this.add('literal', options); + } +}); + // // ### function use (name, options) // #### @type {string} Type of the nconf store to use. @@ -82,13 +77,6 @@ var Provider = exports.Provider = function (options) { // provider.use('file', { type: 'file', filename: '/path/to/userconf' }) // Provider.prototype.use = function (name, options) { - if (name === 'system') { - return; - } - else if (this._reserved.indexOf(name) !== -1) { - throw new Error('Cannot use reserved name: ' + name); - } - options = options || {}; var type = options.type || name; @@ -98,7 +86,7 @@ Provider.prototype.use = function (name, options) { }); } - var store = this[name], + var store = this.sources[name], update = store && !sameOptions(store); if (!store || update) { @@ -123,10 +111,6 @@ Provider.prototype.use = function (name, options) { // provider.add('userconf', { type: 'file', filename: '/path/to/userconf' }) // Provider.prototype.add = function (name, options) { - if (this._reserved.indexOf(name) !== -1) { - throw new Error('Cannot use reserved name: ' + name); - } - options = options || {}; var type = options.type || name; @@ -134,11 +118,10 @@ Provider.prototype.add = function (name, options) { throw new Error('Cannot add store with unknown type: ' + type); } - this[name] = this.create(type, options); - this._stores.push(name); + this.sources[name] = this.create(type, options); - if (this[name].loadSync) { - this[name].loadSync(); + if (this.sources[name].loadSync) { + this.sources[name].loadSync(); } return this; @@ -152,15 +135,7 @@ Provider.prototype.add = function (name, options) { // this was used in the call to `.add()`. // Provider.prototype.remove = function (name) { - if (this._reserved.indexOf(name) !== -1) { - throw new Error('Cannot use reserved name: ' + name); - } - else if (!this[name]) { - throw new Error('Cannot remove store that does not exist: ' + name); - } - - delete this[name]; - this._stores.splice(this._stores.indexOf(name), 1); + delete this.sources[name]; return this; }; @@ -196,13 +171,14 @@ Provider.prototype.get = function (key, callback) { // the entire set of stores, but up until there is a defined value. // var current = 0, + names = Object.keys(this.sources), self = this, response; async.whilst(function () { - return typeof response === 'undefined' && current < self._stores.length; + return typeof response === 'undefined' && current < names.length; }, function (next) { - var store = self[self._stores[current]]; + var store = self.sources[names[current]]; current++; if (store.get.length >= 2) { @@ -292,7 +268,7 @@ Provider.prototype.merge = function () { // Provider.prototype.load = function (callback) { var self = this, - stores = this._stores.map(function (name) { return self[name] }); + stores = Object.keys(this.sources).map(function (name) { return self.sources[name] }); function loadStoreSync(store) { if (!store.loadSync) { @@ -373,10 +349,11 @@ Provider.prototype.save = function (value, callback) { value = null; } - var self = this; + var self = this, + names = Object.keys(this.sources); function saveStoreSync(name) { - var store = self[name]; + var store = self.sources[name]; if (!store.saveSync) { throw new Error('nconf store ' + store.type + ' has no saveSync() method'); @@ -386,7 +363,7 @@ Provider.prototype.save = function (value, callback) { } function saveStore(name, next) { - var store = self[name]; + var store = self.sources[name]; if (!store.save && !store.saveSync) { return next(new Error('nconf store ' + store.type + ' has no save() method')); @@ -403,10 +380,10 @@ Provider.prototype.save = function (value, callback) { // then do so. // if (!callback) { - return common.merge(this._stores.map(saveStoreSync)); + return common.merge(names.map(saveStoreSync)); } - async.map(this._stores, saveStore, function (err, objs) { + async.map(names, saveStore, function (err, objs) { return err ? callback(err) : callback(); }); }; @@ -426,53 +403,30 @@ Provider.prototype._execute = function (action, syncLength /* [arguments] */) { response; function runAction (name, next) { - var store = self[name] - + var store = self.sources[name]; + return store[action].length > syncLength ? store[action].apply(store, args.concat(next)) : next(null, store[action].apply(store, args)); } if (callback) { - return async.forEach(self._stores, runAction, function (err) { + return async.forEach(Object.keys(self.sources), runAction, function (err) { return err ? callback(err) : callback(); }); } - this._stores.forEach(function (name) { - var store = self[name]; - response = store[action].apply(store, args); + + Object.keys(this.sources).forEach(function (name) { + if (typeof response === 'undefined') { + var store = self.sources[name]; + response = store[action].apply(store, args); + } }); return response; } -// -// ### @argv {boolean} -// Gets or sets a property representing overrides which supercede all -// other values for this instance. -// -Provider.prototype.__defineSetter__('overrides', function (val) { updateSystem.call(this, 'overrides', val) }); -Provider.prototype.__defineGetter__('overrides', function () { return this._argv }); - -// -// ### @argv {boolean} -// Gets or sets a property indicating if we should wrap calls to `.get` -// by checking `optimist.argv`. Can be a boolean or the pass-thru -// options for `optimist`. -// -Provider.prototype.__defineSetter__('argv', function (val) { updateSystem.call(this, 'argv', val) }); -Provider.prototype.__defineGetter__('argv', function () { return this._argv }); - -// -// ### @env {boolean} -// Gets or sets a property indicating if we should wrap calls to `.get` -// by checking `process.env`. Can be a boolean or an Array of -// environment variables to extract. -// -Provider.prototype.__defineSetter__('env', function (val) { updateSystem.call(this, 'env', val) }); -Provider.prototype.__defineGetter__('env', function () { return this._env }); - // // Throw the `err` if a callback is not supplied // @@ -482,21 +436,4 @@ function onError(err, callback) { } throw err; -} - -// -// Helper function for working with the -// default `system` store for providers. -// -function updateSystem(prop, value) { - var system = this['system']; - - if (system[prop] === value) { - return; - } - - value = value || false; - this['_' + prop] = value; - system[prop] = value; - system.loadSync(); } \ No newline at end of file diff --git a/lib/nconf/stores/argv.js b/lib/nconf/stores/argv.js new file mode 100644 index 00000000..62c83811 --- /dev/null +++ b/lib/nconf/stores/argv.js @@ -0,0 +1,59 @@ +/* + * argv.js: Simple memory-based store for command-line arguments. + * + * (C) 2011, Charlie Robbins + * + */ + +var util = require('util'), + optimist = require('optimist'), + Memory = require('./memory').Memory; + +// +// ### function Argv (options) +// #### @options {Object} Options for this instance. +// Constructor function for the Argv nconf store, a simple abstraction +// around the Memory store that can read command-line arguments. +// +var Argv = exports.Argv = function (options) { + Memory.call(this, options); + + this.type = 'argv'; + this.options = options || false; +}; + +// Inherit from the Memory store +util.inherits(Argv, Memory); + +// +// ### function loadSync () +// Loads the data passed in from `process.argv` into this instance. +// +Argv.prototype.loadSync = function () { + this.loadArgv(); + return this.store; +}; + +// +// ### function loadArgv () +// Loads the data passed in from the command-line arguments +// into this instance. +// +Argv.prototype.loadArgv = function () { + var self = this, + argv; + + argv = typeof this.options === 'object' + ? optimist(process.argv.slice(2)).options(this.options).argv + : optimist(process.argv.slice(2)).argv; + + if (!argv) { + return; + } + + Object.keys(argv).forEach(function (key) { + self.set(key, argv[key]); + }); + + return this.store; +}; \ No newline at end of file diff --git a/lib/nconf/stores/env.js b/lib/nconf/stores/env.js new file mode 100644 index 00000000..fa3ce42a --- /dev/null +++ b/lib/nconf/stores/env.js @@ -0,0 +1,51 @@ +/* + * env.js: Simple memory-based store for environment variables + * + * (C) 2011, Charlie Robbins + * + */ + +var util = require('util'), + Memory = require('./memory').Memory; + +// +// ### function Env (options) +// #### @options {Object} Options for this instance. +// Constructor function for the Env nconf store, a simple abstraction +// around the Memory store that can read process environment variables. +// +var Env = exports.Env = function (options) { + Memory.call(this, options); + + this.type = 'env'; + this.options = options || []; +}; + +// Inherit from the Memory store +util.inherits(Env, Memory); + +// +// ### function loadSync () +// Loads the data passed in from `process.env` into this instance. +// +Env.prototype.loadSync = function () { + this.loadEnv(); + return this.store; +}; + +// +// ### function loadEnv () +// Loads the data passed in from `process.env` into this instance. +// +Env.prototype.loadEnv = function () { + var self = this; + + Object.keys(process.env).filter(function (key) { + return !self.options.length || self.options.indexOf(key) !== -1; + }).forEach(function (key) { + self.set(key, process.env[key]); + }); + + return this.store; +}; + diff --git a/lib/nconf/stores/literal.js b/lib/nconf/stores/literal.js new file mode 100644 index 00000000..ff69f63f --- /dev/null +++ b/lib/nconf/stores/literal.js @@ -0,0 +1,18 @@ +/* + * literal.js: Simple literal Object store for nconf. + * + * (C) 2011, Charlie Robbins + * + */ + +var util = require('util'), + Memory = require('./memory').Memory + +var Literal = exports.Literal = function Literal (store) { + Memory.call(this); + this.type = 'literal' + this.store = store || {}; +}; + +// Inherit from Memory store. +util.inherits(Literal, Memory); \ No newline at end of file diff --git a/lib/nconf/stores/system.js b/lib/nconf/stores/system.js deleted file mode 100644 index c55c1281..00000000 --- a/lib/nconf/stores/system.js +++ /dev/null @@ -1,118 +0,0 @@ -/* - * system.js: Simple memory-based store for process environment variables and - * command-line arguments. - * - * (C) 2011, Charlie Robbins - * - */ - -var util = require('util'), - Memory = require('./memory').Memory; - -// -// ### function System (options) -// #### @options {Object} Options for this instance. -// Constructor function for the System nconf store, a simple abstraction -// around the Memory store that can read process environment variables -// and command-line arguments. -// -var System = exports.System = function (options) { - options = options || {}; - Memory.call(this, options); - - this.type = 'system'; - this.overrides = options.overrides || null; - this.env = options.env || false; - this.argv = options.argv || false; -}; - -// Inherit from the Memory store -util.inherits(System, Memory); - -// -// ### function loadSync () -// Loads the data passed in from `process.env` into this instance. -// -System.prototype.loadSync = function () { - if (this.env) { - this.loadEnv(); - } - - if (this.argv) { - this.loadArgv(); - } - - if (this.overrides) { - this.loadOverrides(); - } - - return this.store; -}; - -// -// ### function loadOverrides () -// Loads any overrides set on this instance into -// the underlying managed `Memory` store. -// -System.prototype.loadOverrides = function () { - if (!this.overrides) { - return; - } - - var self = this, - keys = Object.keys(this.overrides); - - keys.forEach(function (key) { - self.set(key, self.overrides[key]); - }); - - return this.store; -}; - -// -// ### function loadArgv () -// Loads the data passed in from the command-line arguments -// into this instance. -// -System.prototype.loadArgv = function () { - var self = this, - argv; - - if (typeof this.argv === 'object') { - argv = require('optimist')(process.argv.slice(2)).options(this.argv).argv; - } - else if (this.argv) { - argv = require('optimist')(process.argv.slice(2)).argv; - } - - if (!argv) { - return; - } - - Object.keys(argv).forEach(function (key) { - self.set(key, argv[key]); - }); - - return this.store; -}; - -// -// ### function loadEnv () -// Loads the data passed in from `process.env` into this instance. -// -System.prototype.loadEnv = function () { - var self = this; - - if (!this.env) { - return; - } - - Object.keys(process.env).filter(function (key) { - return !self.env.length || self.env.indexOf(key) !== -1; - }).forEach(function (key) { - self.set(key, process.env[key]); - }); - - return this.store; -}; - diff --git a/package.json b/package.json index 930ca285..5ffe4a3f 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,6 @@ "vows": "0.5.x >=0.5.11" }, "main": "./lib/nconf", - "scripts": { "test": "vows test/*-test.js --spec" }, + "scripts": { "test": "vows test/*-test.js test/**/*-test.js --spec" }, "engines": { "node": ">= 0.4.0" } } diff --git a/test/fixtures/scripts/nconf-argv.js b/test/fixtures/scripts/nconf-argv.js index 5ffbf334..898da29d 100644 --- a/test/fixtures/scripts/nconf-argv.js +++ b/test/fixtures/scripts/nconf-argv.js @@ -5,7 +5,6 @@ * */ -var nconf = require('../../../lib/nconf'); +var nconf = require('../../../lib/nconf').argv().env(); -nconf.argv = true; process.stdout.write(nconf.get('something')); \ No newline at end of file diff --git a/test/fixtures/scripts/nconf-change-argv.js b/test/fixtures/scripts/nconf-change-argv.js index 8dd4fd38..1648da2b 100644 --- a/test/fixtures/scripts/nconf-change-argv.js +++ b/test/fixtures/scripts/nconf-change-argv.js @@ -5,14 +5,12 @@ * */ -var nconf = require('../../../lib/nconf'); - -nconf.argv = true; +var nconf = require('../../../lib/nconf').argv(); // // Remove 'badValue', 'evenWorse' and 'OHNOEZ' // process.argv.splice(3, 3); -nconf.system.loadArgv(); +nconf.sources['argv'].loadArgv(); process.stdout.write(nconf.get('something')); diff --git a/test/fixtures/scripts/nconf-env.js b/test/fixtures/scripts/nconf-env.js index c145b842..321e32e7 100644 --- a/test/fixtures/scripts/nconf-env.js +++ b/test/fixtures/scripts/nconf-env.js @@ -5,7 +5,6 @@ * */ -var nconf = require('../../../lib/nconf'); +var nconf = require('../../../lib/nconf').env(); -nconf.env = true; process.stdout.write(nconf.get('SOMETHING')); \ No newline at end of file diff --git a/test/fixtures/scripts/provider-argv.js b/test/fixtures/scripts/provider-argv.js index 11d40052..48cb2b12 100644 --- a/test/fixtures/scripts/provider-argv.js +++ b/test/fixtures/scripts/provider-argv.js @@ -7,6 +7,6 @@ var nconf = require('../../../lib/nconf'); -var provider = new (nconf.Provider)({ argv: true }); +var provider = new (nconf.Provider)().argv(); process.stdout.write(provider.get('something')); \ No newline at end of file diff --git a/test/fixtures/scripts/provider-env.js b/test/fixtures/scripts/provider-env.js index a46ed9e2..1f3032c5 100644 --- a/test/fixtures/scripts/provider-env.js +++ b/test/fixtures/scripts/provider-env.js @@ -7,6 +7,6 @@ var nconf = require('../../../lib/nconf'); -var provider = new (nconf.Provider)({ env: true }); +var provider = new (nconf.Provider)().env(); process.stdout.write(provider.get('SOMETHING')); \ No newline at end of file diff --git a/test/helpers.js b/test/helpers.js index b09e7b6e..ce7aff5a 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -43,8 +43,8 @@ exports.assertSystemConf = function (options) { }); } - spawn(process.argv[0], [options.script].concat(options.argv), { env: env }) - .stdout.once('data', this.callback.bind(this, null)); + var child = spawn('node', [options.script].concat(options.argv), { env: env }); + child.stdout.once('data', this.callback.bind(this, null)); }, "should respond with the value passed into the script": function (_, data) { assert.equal(data.toString(), 'foobar'); diff --git a/test/nconf-test.js b/test/nconf-test.js index 3fff6c2c..482a543c 100644 --- a/test/nconf-test.js +++ b/test/nconf-test.js @@ -28,7 +28,7 @@ vows.describe('nconf').addBatch({ "the use() method": { "should instaniate the correct store": function () { nconf.use('memory'); - assert.instanceOf(nconf.memory, nconf.stores.Memory); + assert.instanceOf(nconf.sources['memory'], nconf.stores.Memory); } }, "it should": { diff --git a/test/provider-test.js b/test/provider-test.js index f6e899e1..fee27baf 100644 --- a/test/provider-test.js +++ b/test/provider-test.js @@ -24,13 +24,13 @@ vows.describe('nconf/provider').addBatch({ "calling the use() method with the same store type and different options": { topic: new nconf.Provider().use('file', { file: files[0] }), "should use a new instance of the store type": function (provider) { - var old = provider.file; + var old = provider.sources['file']; - assert.equal(provider.file.file, files[0]); + assert.equal(provider.sources.file.file, files[0]); provider.use('file', { file: files[1] }); - assert.notStrictEqual(old, provider.file); - assert.equal(provider.file.file, files[1]); + assert.notStrictEqual(old, provider.sources.file); + assert.equal(provider.sources.file.file, files[1]); } }, "when 'argv' is true": helpers.assertSystemConf({ @@ -40,7 +40,7 @@ vows.describe('nconf/provider').addBatch({ "when 'env' is true": helpers.assertSystemConf({ script: path.join(fixturesDir, 'scripts', 'provider-env.js'), env: { SOMETHING: 'foobar' } - }), + }) }, "the default nconf provider": { "when 'argv' is set to true": helpers.assertSystemConf({ @@ -66,27 +66,9 @@ vows.describe('nconf/provider').addBatch({ "should have the result merged in": function (provider) { provider.load(); provider.merge(override); - helpers.assertMerged(null, provider.file.store); - } - }, - "when sources are passed in": { - topic: new nconf.Provider({ - sources: { - user: { - type: 'file', - file: files[0] - }, - global: { - type: 'file', - file: files[1] - } - } - }), - "should have the result merged in": function (provider) { - helpers.assertMerged(null, provider.load()); + helpers.assertMerged(null, provider.sources.file.store); } } } } -}).export(module); - +}).export(module); \ No newline at end of file diff --git a/test/stores/argv-test.js b/test/stores/argv-test.js new file mode 100644 index 00000000..33014d1a --- /dev/null +++ b/test/stores/argv-test.js @@ -0,0 +1,22 @@ +/* + * argv-test.js: Tests for the nconf argv store. + * + * (C) 2011, Charlie Robbins + * + */ + +var vows = require('vows'), + assert = require('assert'), + helpers = require('../helpers'), + nconf = require('../../lib/nconf'); + +vows.describe('nconf/stores/argv').addBatch({ + "An instance of nconf.stores.Argv": { + topic: new nconf.stores.Argv(), + "should have the correct methods defined": function (argv) { + assert.isFunction(argv.loadSync); + assert.isFunction(argv.loadArgv); + assert.isFalse(argv.options); + } + } +}).export(module); \ No newline at end of file diff --git a/test/stores/env-test.js b/test/stores/env-test.js new file mode 100644 index 00000000..f9ce5d84 --- /dev/null +++ b/test/stores/env-test.js @@ -0,0 +1,23 @@ +/* + * env-test.js: Tests for the nconf env store. + * + * (C) 2011, Charlie Robbins + * + */ + +var vows = require('vows'), + assert = require('assert'), + helpers = require('../helpers'), + nconf = require('../../lib/nconf'); + +vows.describe('nconf/stores/env').addBatch({ + "An instance of nconf.stores.Env": { + topic: new nconf.stores.Env(), + "should have the correct methods defined": function (env) { + assert.isFunction(env.loadSync); + assert.isFunction(env.loadEnv); + assert.isArray(env.options); + assert.lengthOf(env.options, 0); + } + } +}).export(module); \ No newline at end of file diff --git a/test/stores/literal-test.js b/test/stores/literal-test.js new file mode 100644 index 00000000..3944bab2 --- /dev/null +++ b/test/stores/literal-test.js @@ -0,0 +1,30 @@ +/* + * literal-test.js: Tests for the nconf literal store. + * + * (C) 2011, Charlie Robbins + * + */ + +var vows = require('vows'), + assert = require('assert'), + helpers = require('../helpers'), + nconf = require('../../lib/nconf'); + +vows.describe('nconf/stores/literal').addBatch({ + "An instance of nconf.stores.Literal": { + topic: new nconf.stores.Literal({ + foo: 'bar', + one: 2 + }), + "should have the correct methods defined": function (literal) { + assert.equal(literal.type, 'literal'); + assert.isFunction(literal.get); + assert.isFunction(literal.set); + assert.isFunction(literal.merge); + }, + "should have the correct values in the store": function (literal) { + assert.equal(literal.store.foo, 'bar'); + assert.equal(literal.store.one, 2); + } + } +}).export(module); \ No newline at end of file diff --git a/test/stores/system-test.js b/test/stores/system-test.js deleted file mode 100644 index 8b3226ee..00000000 --- a/test/stores/system-test.js +++ /dev/null @@ -1,26 +0,0 @@ -/* - * system-test.js: Tests for the nconf system store. - * - * (C) 2011, Charlie Robbins - * - */ - -var vows = require('vows'), - assert = require('assert'), - helpers = require('../helpers'), - nconf = require('../../lib/nconf'); - -vows.describe('nconf/stores/system').addBatch({ - "An instance of nconf.stores.System": { - topic: new nconf.stores.System(), - "should have the correct methods defined": function (system) { - assert.isFunction(system.loadSync); - assert.isFunction(system.loadOverrides); - assert.isFunction(system.loadArgv); - assert.isFunction(system.loadEnv); - assert.isFalse(system.argv); - assert.isFalse(system.env); - assert.isNull(system.overrides); - } - } -}).export(module); \ No newline at end of file