Skip to content

Commit

Permalink
feat: Merge pull request #209 from pelias/add-config-validation
Browse files Browse the repository at this point in the history
added configuration validation with tests
  • Loading branch information
trescube authored Jan 4, 2017
2 parents 1fe9205 + e5e12f4 commit 6b8672b
Show file tree
Hide file tree
Showing 6 changed files with 184 additions and 0 deletions.
26 changes: 26 additions & 0 deletions configValidation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
'use strict';

const Joi = require('joi');

// Schema Configuration
// datapath: string (required)
// files: array of strings
// deduplicate: boolean
// adminLookup: boolean
const schema = Joi.object().keys({
files: Joi.array().items(Joi.string()),
datapath: Joi.string(),
deduplicate: Joi.boolean(),
adminLookup: Joi.boolean()
}).requiredKeys('datapath').unknown(false);

module.exports = {
validate: function validate(config) {
Joi.validate(config, schema, (err) => {
if (err) {
throw new Error(err.details[0].message);
}
});
}

};
3 changes: 3 additions & 0 deletions import.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
'use strict';

var peliasConfig = require( 'pelias-config' ).generate();

require('./configValidation').validate(peliasConfig);

var logger = require( 'pelias-logger' ).get( 'openaddresses' );

var parameters = require( './lib/parameters' );
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"combined-stream": "1.0.5",
"csv-parse": "^1.1.7",
"glob": "^7.0.0",
"joi": "^10.1.0",
"lodash": "^4.10.0",
"minimist": "1.2.0",
"pelias-address-deduplicator": "1.1.0",
Expand All @@ -28,6 +29,7 @@
"event-stream": "^3.3.2",
"jshint": "^2.9.4",
"precommit-hook": "3.0.0",
"proxyquire": "^1.7.10",
"tap-spec": "4.1.1",
"tape": "^4.5.0",
"temp": "^0.8.3",
Expand Down
130 changes: 130 additions & 0 deletions test/configValidation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
'use strict';

const tape = require( 'tape' );

const configValidation = require( '../configValidation' );

tape( 'missing datapath should throw error', function(test) {
const config = {};

test.throws(() => {
configValidation.validate(config);
}, /"datapath" is required/);

test.end();
});

tape( 'non-string datapath should throw error', function(test) {
[null, 17, {}, [], false].forEach((value) => {
const config = {
datapath: value
};

test.throws(() => {
configValidation.validate(config);
}, /"datapath" must be a string/);

});

test.end();
});

tape( 'non-array files should throw error', function(test) {
[null, 17, {}, 'string', false].forEach((value) => {
const config = {
datapath: 'this is the datapath',
files: value
};

test.throws(() => {
configValidation.validate(config);
}, /"files" must be an array/, 'datapath is required');
});

test.end();
});

tape( 'non-string elements in files array should throw error', function(test) {
[null, 17, {}, [], false].forEach((value) => {
const config = {
datapath: 'this is the datapath',
files: [value]
};

test.throws(() => {
configValidation.validate(config);
}, /"0" must be a string/, 'files elements must be strings');
});

test.end();
});

tape( 'non-boolean adminLookup should throw error', function(test) {
[null, 17, {}, [], 'string'].forEach((value) => {
const config = {
datapath: 'this is the datapath',
adminLookup: value
};

test.throws(() => {
configValidation.validate(config);
}, /"adminLookup" must be a boolean/);
});

test.end();
});

tape( 'non-boolean deduplicate should throw error', function(test) {
[null, 17, {}, [], 'string'].forEach((value) => {
const config = {
datapath: 'this is the datapath',
deduplicate: value
};

test.throws(() => {
configValidation.validate(config);
}, /"deduplicate" must be a boolean/);
});

test.end();
});

tape( 'unknown config fields should throw error', function(test) {
const config = {
datapath: 'this is the datapath',
unknown: 'value'
};

test.throws(() => {
configValidation.validate(config);
}, /"unknown" is not allowed/, 'unknown fields should be disallowed');
test.end();

});

tape( 'configuration with only datapath should not throw error', function(test) {
const config = {
datapath: 'this is the datapath'
};

test.doesNotThrow(() => {
configValidation.validate(config);
}, 'config should be valid');
test.end();

});

tape( 'valid configuration should not throw error', function(test) {
const config = {
datapath: 'this is the datapath',
deduplicate: false,
adminLookup: false,
files: ['file 1', 'file 2']
};

test.doesNotThrow(() => {
configValidation.validate(config);
}, 'config should be valid');
test.end();

});
21 changes: 21 additions & 0 deletions test/import.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';

const tape = require( 'tape' );

const proxyquire = require('proxyquire').noCallThru();

tape( 'configValidation throwing error should rethrow', function(test) {
test.throws(function() {
proxyquire('../import', {
'./configValidation': {
validate: () => {
throw Error('config is not valid');
}
}
})();

}, /config is not valid/);

test.end();

});
2 changes: 2 additions & 0 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

'use strict';

require( './configValidation' );
require( './isValidCsvRecord' );
require( './streams/adminLookupStream');
require( './import');
require( './importPipeline');
require( './parameters' );
require( './streams/cleanupStream' );
Expand Down

0 comments on commit 6b8672b

Please sign in to comment.