Skip to content

Commit

Permalink
fix(schema): Remove required keys
Browse files Browse the repository at this point in the history
The CSV importer is designed to be optional, and thus all config options
should be optional as well.
  • Loading branch information
orangejulius committed Jan 28, 2019
1 parent a669c22 commit 3268aa5
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 40 deletions.
2 changes: 1 addition & 1 deletion import.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function startTiming() {
var args = parameters.interpretUserArgs( process.argv.slice( 2 ) );

if( 'exitCode' in args ){
((args.exitCode > 0) ? console.error : console.info)( args.errMessage );
((args.exitCode > 0) ? logger.error : logger.info)( args.errMessage );
process.exit( args.exitCode );
} else {
startTiming();
Expand Down
8 changes: 5 additions & 3 deletions lib/parameters.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,13 @@ function interpretUserArgs( argv ){
'parallel-count': argv['parallel-count'],
'parallel-id': argv['parallel-id']
};
if (peliasConfig.imports.csv) {
if (peliasConfig.get('imports.csv.datapath')) {
opts.dirPath = peliasConfig.imports.csv.datapath;
} else {
//pick a somewhat reasonable data path
opts.dirPath = '/mnt/data/csv'
return {
errMessage: 'No datapath configured, nothing to do',
exitCode: 0
};
}

opts.dirPath = path.normalize(opts.dirPath);
Expand Down
2 changes: 1 addition & 1 deletion schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ module.exports = Joi.object().keys({
download: Joi.array(),
deduplicate: Joi.boolean(),
adminLookup: Joi.boolean()
}).requiredKeys('datapath').unknown(false)
})
}).requiredKeys('csv').unknown(true)
}).requiredKeys('imports').unknown(true);
62 changes: 29 additions & 33 deletions test/parameters.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
var tape = require( 'tape' );
var path = require( 'path' );
var fs = require('fs');
var proxyquire = require('proxyquire');
var proxyquire = require('proxyquire').noCallThru();

var temp = require( 'temp' ).track();

const config = require('pelias-config');

// fake FS module that always says a file is present
const fakeFsModule = {
statSync: function() {
Expand All @@ -19,57 +21,51 @@ const fakeFsModule = {
}
};

// fake pelias-config module that returns a blank config
const fakePeliasConfig = {
generate: function() {
return {
imports: {}
}
}
};

var parameters = proxyquire( '../lib/parameters', {
'pelias-config': {},
var parameters_default = proxyquire( '../lib/parameters', {
'pelias-config': {
generate: config.generateDefaults
},
fs: fakeFsModule
});


tape( 'interpretUserArgs() sets default parameter options', function ( test ){
tape( 'interpretUserArgs() allows missing datapath', function ( test ){
var parametersForThisTest = proxyquire( '../lib/parameters', {
fs: fakeFsModule,
'pelias-config': fakePeliasConfig
'pelias-config': {
generate: config.generateDefaults
}
});

const input = [ ]; // pass no input

const expectedParamOutput = {
dirPath: '/mnt/data/csv', //default path expected
'parallel-count': undefined,
'parallel-id': undefined
errMessage: 'No datapath configured, nothing to do',
exitCode: 0,
};


const actual = parametersForThisTest.interpretUserArgs(input);
test.deepEqual(actual, expectedParamOutput, 'parameters should have their default values');
test.deepEqual(actual, expectedParamOutput, 'Non-failure exit code and message set');
test.end();
});

tape('interpretUserArgs returns dir from pelias config if set', function(test) {
temp.mkdir('tmpdir2', function(err, temporary_dir) {
var peliasConfig = {
var customPeliasConfig = {
generate: function() {
return {
return config.generateCustom( {
imports: {
csv: {
datapath: temporary_dir
}
}
}
});
}
};

const parameters = proxyquire('../lib/parameters', {
'pelias-config': peliasConfig
'pelias-config': customPeliasConfig
});

var input = [];
Expand All @@ -83,20 +79,20 @@ tape('interpretUserArgs returns dir from pelias config if set', function(test) {
tape('interpretUserArgs returns normalized path from config', function(test) {
temp.mkdir('tmpdir2', function(err, temporary_dir) {
var input_dir = path.sep + '.' + temporary_dir;
var peliasConfig = {
var customPeliasConfig = {
generate: function() {
return {
return config.generateCustom({
imports: {
csv: {
datapath: input_dir
}
}
}
});
}
};

const parameters = proxyquire('../lib/parameters', {
'pelias-config': peliasConfig
'pelias-config': customPeliasConfig
});

var input = [];
Expand Down Expand Up @@ -133,7 +129,7 @@ tape('getFileList returns all .csv path names when config has empty files list',
dirPath: temp_dir
};

var actual = parameters.getFileList(peliasConfig, args);
var actual = parameters_default.getFileList(peliasConfig, args);

test.equal(actual.length, 3);
test.ok(actual.find((f) => f === path.join(temp_dir, 'dirA', 'fileA.csv')));
Expand Down Expand Up @@ -168,7 +164,7 @@ tape('getFileList returns all .csv path names when config doesn\'t have files pr
dirPath: temp_dir
};

var actual = parameters.getFileList(peliasConfig, args);
var actual = parameters_default.getFileList(peliasConfig, args);

test.equal(actual.length, 3);
test.ok(actual.find((f) => f === path.join(temp_dir, 'dirA', 'fileA.csv')));
Expand All @@ -194,7 +190,7 @@ tape('getFileList returns fully qualified path names when config has a files lis

var expected = [path.join(temporary_dir, 'filea.csv'), path.join(temporary_dir, 'fileb.csv')];

var actual = parameters.getFileList(peliasConfig, args);
var actual = parameters_default.getFileList(peliasConfig, args);

test.deepEqual(actual, expected, 'file names should be equal');
test.end();
Expand All @@ -220,7 +216,7 @@ tape('getFileList handles parallel builds', function(test) {

var expected = [path.join(temporary_dir, 'filea.csv')];

var actual = parameters.getFileList(peliasConfig, args);
var actual = parameters_default.getFileList(peliasConfig, args);

t.deepEqual(actual, expected, 'only first file is indexed');
t.end();
Expand All @@ -235,7 +231,7 @@ tape('getFileList handles parallel builds', function(test) {

var expected = [path.join(temporary_dir, 'fileb.csv')];

var actual = parameters.getFileList(peliasConfig, args);
var actual = parameters_default.getFileList(peliasConfig, args);

t.deepEqual(actual, expected, 'only second file indexed');
t.end();
Expand All @@ -250,7 +246,7 @@ tape('getFileList handles parallel builds', function(test) {

var expected = [path.join(temporary_dir, 'filec.csv')];

var actual = parameters.getFileList(peliasConfig, args);
var actual = parameters_default.getFileList(peliasConfig, args);

t.deepEqual(actual, expected, 'only third file indexed');
t.end();
Expand All @@ -265,7 +261,7 @@ tape('getFileList handles parallel builds', function(test) {

var expected = [];

var actual = parameters.getFileList(peliasConfig, args);
var actual = parameters_default.getFileList(peliasConfig, args);

t.deepEqual(actual, expected, 'file list is empty');
t.end();
Expand Down
4 changes: 2 additions & 2 deletions test/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ tape('non-object imports.csv should throw error', function(test) {

});

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

test.throws(validate.bind(null, config), /"datapath" is required/);
test.doesNotThrow(validate.bind(null, config), 'datapath is not required');
test.end();

});
Expand Down

0 comments on commit 3268aa5

Please sign in to comment.