feat: Merge pull request #182 from pelias/add-joi-config-validation

added schema validation and updated lint configuration

.jshintrc

@@ -1,10 +1,22 @@
 {
   "node": true,
-
   "curly": true,
-  "latedef": true,
-  "quotmark": true,
+  "eqeqeq": true,
+  "esversion": 6,
+  "freeze": true,
+  "immed": true,
+  "indent": 2,
+  "latedef": false,
+  "newcap": true,
+  "noarg": true,
+  "noempty": true,
+  "nonbsp": true,
+  "nonew": true,
+  "plusplus": false,
+  "quotmark": "single",
   "undef": true,
-  "unused": true,
-  "trailing": true
+  "unused": false,
+  "maxparams": 4,
+  "maxdepth": 4,
+  "maxlen": 140
 }

download_data.js

@@ -6,6 +6,9 @@ var os = require('os');
 var bundles = require('./src/bundleList');
 var config = require('pelias-config').generate();
 
+// validate the WOF importer configuration before continuing
+require('./src/configValidation').validate(config.imports.whosonfirst);
+
 //ensure required directory structure exists
 fs.ensureDirSync(config.imports.whosonfirst.datapath + '/meta');
 
@@ -17,12 +20,16 @@ var simultaneousDownloads = Math.max(4, Math.min(1, os.cpus().length / 2));
 
 /*
  * generate a shell command that does the following:
- * 1.) use curl to download the bundle, piping directly to tar (this avoids the need for intermediate storage of the archive file)
- * 2.) extract the archive so that the data directory goes in the right place and the README file is ignored (it just would get overridden by subsequent bundles)
+ * 1.) use curl to download the bundle, piping directly to tar (this avoids the
+       need for intermediate storage of the archive file)
+ * 2.) extract the archive so that the data directory goes in the right place and
+       the README file is ignored (it just would get overridden by subsequent bundles)
  * 3.) move the meta file to the meta files directory
  */
 function generateCommand(type, directory) {
-  return 'curl https://whosonfirst.mapzen.com/bundles/wof-' + type + '-latest-bundle.tar.bz2 | tar -xj --strip-components=1 --exclude=README.txt -C ' + directory + ' && mv ' + directory + '/wof-' + type  + '-latest.csv ' + directory + '/meta/';
+  return 'curl https://whosonfirst.mapzen.com/bundles/wof-' + type +
+          '-latest-bundle.tar.bz2 | tar -xj --strip-components=1 --exclude=README.txt -C ' +
+          directory + ' && mv ' + directory + '/wof-' + type  + '-latest.csv ' + directory + '/meta/';
 }
 
 var bundlesToDownload = bundles.hierarchyBundles;
@@ -33,7 +40,7 @@ if (config.imports.whosonfirst.importVenues) {
 
 // this should override the config setting since the hierarchy bundles are useful
 // on their own to allow other importers to start when using admin lookup
-if (process.argv[2] == '--admin-only') {
+if (process.argv[2] === '--admin-only') {
   bundlesToDownload = bundles.hierarchyBundles;
 }
 

import.js

@@ -10,15 +10,8 @@ var bundles = require('./src/bundleList');
 // print a warning if an unsupported Node.JS version is used
 version_checker();
 
-function hasDataDirectory() {
-  return peliasConfig.imports.hasOwnProperty('whosonfirst') &&
-          peliasConfig.imports.whosonfirst.hasOwnProperty('datapath');
-}
-
-if (!hasDataDirectory()) {
-  console.error('Could not find whosonfirst data directory in configuration');
-  process.exit( 2 );
-}
+// validate the WOF importer configuration before continuing
+require('./src/configValidation').validate(peliasConfig.imports.whosonfirst);
 
 var directory = peliasConfig.imports.whosonfirst.datapath;
 

package.json

@@ -31,6 +31,7 @@
     "csv-parse": "^1.0.0",
     "fs-extra": "^1.0.0",
     "iso3166-1": "^0.2.5",
+    "joi": "^10.0.6",
     "lodash": "^4.5.1",
     "node-version-checker": "^2.0.0",
     "pelias-config": "2.4.0",

src/configValidation.js

@@ -0,0 +1,20 @@
+'use strict';
+
+const Joi = require('joi');
+
+// who's on first importer requires just `datapath` and defaults `importVenues` to false
+const schema = Joi.object().keys({
+  datapath: Joi.string(),
+  importVenues: Joi.boolean().default(false).truthy('yes').falsy('no').insensitive(true)
+}).requiredKeys('datapath').unknown(false);
+
+module.exports = {
+  validate: function validate(config) {
+    Joi.validate(config, schema, (err, value) => {
+      if (err) {
+        throw new Error(err.details[0].message);
+      }
+    });
+  }
+
+};

test/configValidationTest.js

@@ -0,0 +1,113 @@
+'use strict';
+
+var tape = require('tape');
+
+var configValidation = require('../src/configValidation');
+
+tape('tests for looking up hierarchies', function(test) {
+  test.test('config lacking datapath should throw error', function(t) {
+    var config = {
+      importVenues: true
+    };
+
+    t.throws(function() {
+      configValidation.validate(config);
+    }, /"datapath" is required/);
+    t.end();
+
+  });
+
+  test.test('config lacking importVenues should not throw error', function(t) {
+    var config = {
+      datapath: '/path/to/data'
+    };
+
+    t.doesNotThrow(function() {
+      configValidation.validate(config);
+    });
+    t.end();
+
+  });
+
+  test.test('config with spurious keys should throw error', function(t) {
+    var config = {
+      datapath: '/path/to/data',
+      importVenues: true,
+      spurious_key: 'value'
+    };
+
+    t.throws(function() {
+      configValidation.validate(config);
+    }, /"spurious_key" is not allowed/);
+    t.end();
+
+  });
+
+  test.test('non-string datapath should throw error', function(t) {
+    [null, 17, {}, [], true].forEach((value) => {
+      var config = {
+        datapath: value
+      };
+
+      t.throws(function() {
+        configValidation.validate(config);
+      }, /"datapath" must be a string/);
+
+    });
+
+    t.end();
+
+  });
+
+  test.test('non-boolean importVenues should throw error', function(t) {
+    [null, 17, {}, [], 'string'].forEach((value) => {
+      var config = {
+        datapath: '/path/to/data',
+        importVenues: value
+      };
+
+      t.throws(function() {
+        configValidation.validate(config);
+      }, /"importVenues" must be a boolean/);
+
+    });
+
+    t.end();
+
+  });
+
+  test.test('case-insensitive \'yes\' and true should be valid importVenues values', function(t) {
+    [true, 'YeS', 'yEs'].forEach((value) => {
+      var config = {
+        datapath: '/path/to/data',
+        importVenues: value
+      };
+
+      t.doesNotThrow(function() {
+        configValidation.validate(config);
+      });
+
+    });
+
+    t.end();
+
+  });
+
+  test.test('case-insensitive \'no\' and false should be valid importVenues values', function(t) {
+    [false, 'nO', 'No'].forEach((value) => {
+      var config = {
+        datapath: '/path/to/data',
+        importVenues: value
+      };
+
+      t.doesNotThrow(function() {
+        configValidation.validate(config);
+      });
+
+    });
+
+    t.end();
+
+  });
+
+});

test/peliasDocGeneratorsTest.js

@@ -12,7 +12,8 @@ function test_stream(input, testedStream, callback) {
 
 tape('create', function(test) {
   test.test('non-country place_types should be returned as Document with that place_type', function(t) {
-    var place_types = ['neighbourhood', 'locality', 'borough', 'localadmin', 'county', 'macrocounty', 'region', 'macroregion', 'dependency'];
+    var place_types = ['neighbourhood', 'locality', 'borough', 'localadmin',
+                        'county', 'macrocounty', 'region', 'macroregion', 'dependency'];
 
     place_types.forEach(function(place_type) {
       var wofRecords = {

test/test.js

@@ -9,3 +9,4 @@ require ('./importStreamTest.js');
 require ('./peliasDocGeneratorsTest.js');
 require ('./readStreamTest.js');
 require ('./wofRecordStreamTest.js');
+require ('./configValidationTest.js');