Validate configuration (issue #8)

[danilo-valente]

Added Joi schema to validate configuration objects. Solves #8.

[futurechan]

Thanks for the PR and your hard work.

A few things:

• there are merge conflicts, which need to be resolved before I can merge to master. If you need help, I can assist you. If you want to move on to something else, that's cool too.
• I decoupled the parser from the processor. The validation needs to happen before parsing. We could probably check the validation in index.js before calling the parser/processor.
• let's pull the schema into it's own file for unit testing (preferably in a /schemas folder)
• let's add unit tests to test the schema for things like mutual exclusivity
• we need to sure up the way errors are evented from within the task.

By the way, what do you think of joi?

[danilo-valente]

Yeah, I just realized that I forgot to add unit tests. About throwing a gutil.PluginError, I think it works, but I'll check to be sure. I loved Joi, it's pretty simple and easy to use.

[danilo-valente]

I've found this article, written by Gulp's staff. It says errors might be thrown, but only from within plugin level functions, which means I was doing wrong. But now, since the validation will be moved to index.js, I think it's a good idea to validate the configuration object before creating the stream object (through.obj), and then throw a PluginError in case of bad configuration.

[danilo-valente]

I have a question: In the old code, I had to validate a block configuration, which means this object:

{
    tasks:[less(), minifyCss(), 'concat']
}

But now, I have this object:

{
     id1: {
         tasks:[less(), minifyCss(), 'concat']
    },
    id2: {
        tasks:[uglify(), 'concat']
    }
}

So, what's the best way to validate each block config? Does Joi have some flag to validate each property with given schema? Should I iterate over this object and joi.validate each property?

[futurechan]

You can compose schemas from schemas. Try this: https://github.com/hapijs/joi#objectkeysschema

[danilo-valente]

I know, but the user can define any id, so I need a schema that applies to any property of the config object. Something like:

var configSchema = joi.object().keys({
    id1: blockConfigSchema,
    id2: blockConfigSchema
}).unknown();

But I can't explicitly assign to id1 and id2 because they are user-defined.

[futurechan]

The walmart guys are pretty quick with responses, especially in the irc. I'd recommend raising the issue there as I'd think this would be a very common use case.

For now, we could try something like this:

function validate(config, cb){  
    try{
        Object.keys(config).forEach(function(key){
            var transConfig = config[key];

            joi.assert(transConfig, transConfigSchema, 'Invalid schema in pipeline: ' + key + '\n');
        })      
    }catch(ex){
        cb(ex);
    }

    cb(null)
}

[danilo-valente]

Yeah, I did something like this too. I'll post there and see if they have a solution.

[futurechan]

Yeah, it's like they need an joi.object().values(...);

[danilo-valente]

Done. I've resolved the conflicts. Now:

• Validation happens before parsing
• Each schema has its own file
• A bunch of unit tests were added to test validation flow

I kept throwing the PluginError instead of emitting it because the validation occurs before the creation of the stream object.

[futurechan]

Excellent work!