Config Validation

[wesleytodd]

I would like a way to register config validation functions. In the past I have wrapped nconf with a custom method like this:

// Requirements
var nconf = require('nconf'),
    util = require('util'),
    async = require('async');

// config constructor is a light wrapper around nconf
var Config = module.exports = function() {
    // Extends from nconf
    nconf.Provider.call(this);

    // Keep the validation rules
    this._rules = [];
};
util.inherits(Config, nconf.Provider);

// Add validation rules, or do validation
Config.prototype.validate = function(path, fnc, ctx) {

    // Add a validation rule?
    if (typeof path !== 'function') {
        return this._rules.push({
            path: path,
            fnc: fnc,
            ctx: ctx
        });
    }

    // Re-assign
    fnc = path;

    // Otherwise run the validation
    async.each(this._rules, function(rule, done) {
        rule.fnc.call(rule.ctx || this, this.get(rule.path), done);
    }.bind(this), fnc);

};

So you can do this:

// Adds a validation rule to the hostname key
conf.validate('hostname', function(val, done) {
    if (typeof val !== 'string') {
        var err =  'Hostname is required.';
    }
    // Call back with an error if hostname was not a string
    done(err);
});

conf.validate(function(err) {
    if (err) {
        // Exit on startup if configs do not validate
        console.error(err);
        process.exit(1);
    }
});

I almost feel like the above code could be used verbatim. What do you all think of this?

[cpsubrian]

Hrmm .. I'm on board with the general idea. Though I get the idea of 'registering' validation handlers for specific properties, that might be overkill.

etc objects have a hooks property that is an instance of node-stact-hooks. In theory this provides you with some basic structure to just do validations in your app. Something like:

// Grab a 'stact'.
var validators = conf.hooks('validate');

// Add a hostname validator.
validators.add(function (conf, cb) {
  if (typeof conf.get('hostname') !== 'string') {
    return cb(new Error('Hostname is required'));
  }
  cb();
});

// Run the validations.
validators.run(conf, function (err) {
  if (err) {
    console.error(err);
    process.exit(1);
  }
});

Having said that ... I'd listen to more arguments why etc should have a more explicit validation engine. Maybe if you described a specific use case that the more property-level validators help solve.

[wesleytodd]

This solution works just fine for my use case. I didn't know there was a hook system already included, so that is great.

One of the features that I use heavily in my above example is the ability to specify a context for the validation function. It doesn't look like the stact module supports that. Maybe we could add that as well?

[cpsubrian]

You're right that there's not an easy way to pass alternative context in stact. I would typically use scope or currying to handle that situation. A case could probably be made for a more api-centric way to handle it though.

Scope:

var ctx = {my: 'context'};
validatiors.add(function (conf, cb) {
  // do something with ctx.
  cb();
});

Currying:

validators.add((function (ctx, conf, cb) {
  // do something with ctx.
  cb();
}).bind(null, ctx));

[cpsubrian]

I didn't know there was a hook system already included

Yeah, I added it in anticipation for supporting asynchronous conf loading (like from Redis). I never actually had a use case for that though, so its remains unimplemented.

[wesleytodd]

Ok, those are also both good simple solutions. Previously I had my validation rules defined in a separate file where I did not have access to the context, but I can get on board with the currying method.

The more I think about the original method signature compared to this, I think it is still worth consideration. Generally it is good practice to keep the api as small as possible, and wrapping a whole second library's api into this one seems to me to create an unnecessarily complicated API for this. Having a single facade method that does both adding validation functions and run's the validation keeps it as small as possible. If you totally disagree with the dual purpose validate method then I think an api like this might work:

conf.addValidator(function(conf, done) {
    // Do validation
    done(err);
});

conf.runValidation(function(err) {
    // Handle errors
});

This could just be a simple wrapper around the functionality you outlined above. This keeps the cognitive overhead for users low as well as keeping the api explicit and simple.

[cpsubrian]

@wesleytodd I definitely don't 'totally disagree' .. just trying to offer insight into how I might 'work around' the simplistic API. I agree that a standardized API can help developers accomplish their goal without having to learn stact (or worry if stact might be removed from etc). I will give some consideration to your API suggestions and decide what I like best.

Thanks for bearing with me on this exploration stuff, I appreciate that we might not be moving forward as fast as you would like :smile:

[wesleytodd]

No worries about moving forward, that is the joy of open source, github and npm, no need to wait for you. Just fork and link, and even if you never decide to use my changes, doesn't mean I can't. I prefer to contribute if possible, but if it doesn't work out then no worries either.

[cpsubrian]

Hey @wesleytodd, just checking back here so you don't think I've abandoned it. After talking with some colleagues I was all ready to suggest you make a standalone node module called etc-validate that could hook in just like etc-yaml .. that way you could do whatever you wanted (and I'd be happy to chime-in, review your approach over there). Then, we just hit a use case where we are doing some conf validation :smile:

So now, I'm waffling a little on whether I want this in core or still as a plugin.