diff --git a/app.js b/app.js index 25573573..2dabb3c9 100644 --- a/app.js +++ b/app.js @@ -1,11 +1,13 @@ var app = require('express')(); -var peliasConfig = require( 'pelias-config' ).generate().api; +var peliasConfig = require( 'pelias-config' ).generate(); +// validate the configuration before attempting to load the app +require('./src/configValidation').validate(peliasConfig); -if( peliasConfig.accessLog ){ - app.use( require( './middleware/access_log' ).createAccessLogger( peliasConfig.accessLog ) ); +if( peliasConfig.api.accessLog ){ + app.use( require( './middleware/access_log' ).createAccessLogger( peliasConfig.api.accessLog ) ); } /** ----------------------- pre-processing-middleware ----------------------- **/ @@ -18,10 +20,10 @@ app.use( require('./middleware/jsonp') ); /** ----------------------- routes ----------------------- **/ var legacy = require('./routes/legacy'); -legacy.addRoutes(app, peliasConfig); +legacy.addRoutes(app, peliasConfig.api); var v1 = require('./routes/v1'); -v1.addRoutes(app, peliasConfig); +v1.addRoutes(app, peliasConfig.api); /** ----------------------- error middleware ----------------------- **/ diff --git a/package.json b/package.json index 9059f0a9..398ce8c8 100644 --- a/package.json +++ b/package.json @@ -48,6 +48,7 @@ "geolib": "^2.0.18", "geopipes-elasticsearch-backend": "^0.2.0", "iso3166-1": "^0.2.3", + "joi": "^10.1.0", "lodash": "^4.5.0", "markdown": "0.5.0", "morgan": "1.7.0", @@ -56,7 +57,7 @@ "pelias-labels": "1.5.1", "pelias-logger": "0.1.0", "pelias-model": "4.4.0", - "pelias-query": "8.11.0", + "pelias-query": "8.12.0", "pelias-text-analyzer": "1.7.0", "stats-lite": "2.0.3", "through2": "^2.0.3" diff --git a/query/autocomplete_defaults.js b/query/autocomplete_defaults.js index 2800141b..34f396ae 100644 --- a/query/autocomplete_defaults.js +++ b/query/autocomplete_defaults.js @@ -31,7 +31,7 @@ module.exports = _.merge({}, peliasQuery.defaults, { 'focus:offset': '0km', 'focus:scale': '250km', 'focus:decay': 0.5, - 'focus:weight': 40, + 'focus:weight': 15, 'function_score:score_mode': 'avg', 'function_score:boost_mode': 'replace', diff --git a/sanitizer/_synthesize_analysis.js b/sanitizer/_synthesize_analysis.js index ab18b581..f26dfc6a 100644 --- a/sanitizer/_synthesize_analysis.js +++ b/sanitizer/_synthesize_analysis.js @@ -2,6 +2,7 @@ const _ = require('lodash'); const text_analyzer = require('pelias-text-analyzer'); const fields = { + 'venue': 'query', 'address': 'address', 'neighbourhood': 'neighbourhood', 'borough': 'borough', diff --git a/src/configValidation.js b/src/configValidation.js new file mode 100644 index 00000000..621f883b --- /dev/null +++ b/src/configValidation.js @@ -0,0 +1,44 @@ +'use strict'; + +const Joi = require('joi'); + +// Schema Configuration +// required: +// * api.version (string) +// * api.indexName (string) +// * api.host (string) +// * esclient (object - positive integer requestTimeout) +// +// optional: +// * api.accessLog (string) +// * api.relativeScores (boolean) +// * api.legacyUrl (string) +// * api.localization (flipNumberAndStreetCountries is array of 3 character strings) +const schema = Joi.object().keys({ + api: Joi.object().keys({ + version: Joi.string(), + indexName: Joi.string(), + host: Joi.string(), + legacyUrl: Joi.string(), + accessLog: Joi.string(), + relativeScores: Joi.boolean(), + localization: Joi.object().keys({ + flipNumberAndStreetCountries: Joi.array().items(Joi.string().regex(/^[A-Z]{3}$/)) + }).unknown(false) + + }).requiredKeys('version', 'indexName', 'host').unknown(true), + esclient: Joi.object().keys({ + requestTimeout: Joi.number().integer().min(0) + }).unknown(true) +}).requiredKeys('api', 'esclient').unknown(true); + +module.exports = { + validate: function validate(config) { + Joi.validate(config, schema, (err) => { + if (err) { + throw new Error(err.details[0].message); + } + }); + } + +}; diff --git a/test/unit/app.js b/test/unit/app.js new file mode 100644 index 00000000..58ac8cdb --- /dev/null +++ b/test/unit/app.js @@ -0,0 +1,35 @@ +'use strict'; + +const proxyquire = require('proxyquire').noCallThru(); + +module.exports.tests = {}; + +module.exports.tests.invalid_configuration = function(test, common) { + test('configuration validation throwing error should rethrow', function(t) { + t.throws(function() { + proxyquire('../../app', { + './src/configValidation': { + validate: () => { + throw Error('config is not valid'); + } + } + }); + + }, /config is not valid/); + + t.end(); + + }); + +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('app: ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/fixture/autocomplete_linguistic_focus.js b/test/unit/fixture/autocomplete_linguistic_focus.js index 07c741d7..d6aa0aa7 100644 --- a/test/unit/fixture/autocomplete_linguistic_focus.js +++ b/test/unit/fixture/autocomplete_linguistic_focus.js @@ -44,7 +44,7 @@ module.exports = { 'decay': 0.5 } }, - 'weight': 40 + 'weight': 15 }], 'score_mode': 'avg', 'boost_mode': 'replace', diff --git a/test/unit/fixture/autocomplete_linguistic_focus_null_island.js b/test/unit/fixture/autocomplete_linguistic_focus_null_island.js index b0761f40..8dd3f1e6 100644 --- a/test/unit/fixture/autocomplete_linguistic_focus_null_island.js +++ b/test/unit/fixture/autocomplete_linguistic_focus_null_island.js @@ -44,7 +44,7 @@ module.exports = { 'decay': 0.5 } }, - 'weight': 40 + 'weight': 15 }], 'score_mode': 'avg', 'boost_mode': 'replace', diff --git a/test/unit/fixture/structured_geocoding/fallback.json b/test/unit/fixture/structured_geocoding/fallback.json index a957b72c..cd12a864 100644 --- a/test/unit/fixture/structured_geocoding/fallback.json +++ b/test/unit/fixture/structured_geocoding/fallback.json @@ -6,6 +6,96 @@ "query": { "bool": { "should": [ + { + "bool": { + "_name": "fallback.venue", + "must": [ + { + "multi_match": { + "query": "query value", + "type": "phrase", + "fields": [ + "phrase.default", + "category" + ] + } + }, + { + "multi_match": { + "query": "neighbourhood value", + "type": "phrase", + "fields": [ + "parent.neighbourhood", + "parent.neighbourhood_a" + ] + } + }, + { + "multi_match": { + "query": "borough value", + "type": "phrase", + "fields": [ + "parent.borough", + "parent.borough_a" + ] + } + }, + { + "multi_match": { + "query": "city value", + "type": "phrase", + "fields": [ + "parent.locality", + "parent.locality_a", + "parent.localadmin", + "parent.localadmin_a" + ] + } + }, + { + "multi_match": { + "query": "county value", + "type": "phrase", + "fields": [ + "parent.county", + "parent.county_a", + "parent.macrocounty", + "parent.macrocounty_a" + ] + } + }, + { + "multi_match": { + "query": "state value", + "type": "phrase", + "fields": [ + "parent.region", + "parent.region_a", + "parent.macroregion", + "parent.macroregion_a" + ] + } + }, + { + "multi_match": { + "query": "country value", + "type": "phrase", + "fields": [ + "parent.country", + "parent.country_a", + "parent.dependency", + "parent.dependency_a" + ] + } + } + ], + "filter": { + "term": { + "layer": "venue" + } + } + } + }, { "bool": { "_name": "fallback.address", diff --git a/test/unit/run.js b/test/unit/run.js index d21890e7..017492b5 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -9,6 +9,7 @@ var common = { }; var tests = [ + require('./app'), require('./controller/index'), require('./controller/place'), require('./controller/search'), @@ -61,6 +62,7 @@ var tests = [ require('./sanitizer/_categories'), require('./sanitizer/nearby'), require('./src/backend'), + require('./src/configValidation'), require('./sanitizer/autocomplete'), require('./sanitizer/structured_geocoding'), require('./sanitizer/place'), diff --git a/test/unit/sanitizer/_synthesize_analysis.js b/test/unit/sanitizer/_synthesize_analysis.js index dabacef8..97d49b2a 100644 --- a/test/unit/sanitizer/_synthesize_analysis.js +++ b/test/unit/sanitizer/_synthesize_analysis.js @@ -12,7 +12,7 @@ module.exports.tests.text_parser = function(test, common) { }}); const raw = { - query: ' \t query \t value \t ', + venue: ' \t venue \t value \t ', neighbourhood: ' \t neighbourhood \t value \t ', borough: ' \t borough \t value \t ', locality: ' \t locality \t value \t ', @@ -26,6 +26,7 @@ module.exports.tests.text_parser = function(test, common) { const expected_clean = { parsed_text: { + query: 'venue value', neighbourhood: 'neighbourhood value', borough: 'borough value', city: 'locality value', @@ -58,6 +59,7 @@ module.exports.tests.text_parser = function(test, common) { } const raw = { + venue: getInvalidValue(), address: getInvalidValue(), neighbourhood: getInvalidValue(), borough: getInvalidValue(), @@ -78,7 +80,7 @@ module.exports.tests.text_parser = function(test, common) { t.deepEquals(clean, expected_clean); t.deepEquals(messages.errors, ['at least one of the following fields is required: ' + - 'address, neighbourhood, borough, locality, county, region, postalcode, country'], 'no errors'); + 'venue, address, neighbourhood, borough, locality, county, region, postalcode, country'], 'no errors'); t.deepEquals(messages.warnings, [], 'no warnings'); t.end(); @@ -101,7 +103,7 @@ module.exports.tests.text_parser = function(test, common) { t.deepEquals(clean, expected_clean); t.deepEquals(messages.errors, ['at least one of the following fields is required: ' + - 'address, neighbourhood, borough, locality, county, region, postalcode, country'], 'no errors'); + 'venue, address, neighbourhood, borough, locality, county, region, postalcode, country'], 'no errors'); t.deepEquals(messages.warnings, [], 'no warnings'); t.end(); diff --git a/test/unit/src/configValidation.js b/test/unit/src/configValidation.js new file mode 100644 index 00000000..91b34312 --- /dev/null +++ b/test/unit/src/configValidation.js @@ -0,0 +1,434 @@ +'use strict'; + +const configValidation = require('../../../src/configValidation'); + +module.exports.tests = {}; + +module.exports.tests.completely_valid = function(test, common) { + test('all valid configuration elements should not throw error', function(t) { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + legacyUrl: 'legacyUrl value', + accessLog: 'accessLog value', + relativeScores: true, + localization: { + flipNumberAndStreetCountries: ['ABC', 'DEF'] + } + }, + esclient: { + requestTimeout: 17 + } + }; + + t.doesNotThrow(function() { + configValidation.validate(config); + }); + + t.end(); + + }); + + test('basic valid configurtaion should not throw error', function(t) { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value' + }, + esclient: { + requestTimeout: 17 + } + }; + + t.doesNotThrow(function() { + configValidation.validate(config); + }); + + t.end(); + + }); + +}; + +module.exports.tests.api_validation = function(test, common) { + test('config without api should throw error', function(t) { + var config = { + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"api" is required/, 'api should exist'); + t.end(); + + }); + + test('config without unknown field in api should not throw error', function(t) { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + unknown: 'unknown value' + }, + esclient: {} + }; + + t.doesNotThrow(function() { + configValidation.validate(config); + }, 'unknown properties should be allowed'); + t.end(); + + }); + + test('non-string api.version should throw error', function(t) { + [null, 17, {}, [], true].forEach((value) => { + var config = { + api: { + version: value, + indexName: 'index name value', + host: 'host value' + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"version" must be a string/); + }); + + t.end(); + + }); + + test('non-string api.indexName should throw error', function(t) { + [null, 17, {}, [], true].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: value, + host: 'host value' + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"indexName" must be a string/); + }); + + t.end(); + + }); + + test('non-string api.host should throw error', function(t) { + [null, 17, {}, [], true].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: value + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"host" must be a string/); + }); + + t.end(); + + }); + + test('non-string api.legacyUrl should throw error', function(t) { + [null, 17, {}, [], true].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + legacyUrl: value + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"legacyUrl" must be a string/); + }); + + t.end(); + + }); + + test('non-string api.accessLog should throw error', function(t) { + [null, 17, {}, [], true].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + accessLog: value + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"accessLog" must be a string/); + }); + + t.end(); + + }); + + test('non-boolean api.relativeScores should throw error', function(t) { + [null, 17, {}, [], 'string'].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + relativeScores: value + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"relativeScores" must be a boolean/); + }); + + t.end(); + + }); + + test('non-object api.localization should throw error', function(t) { + [null, 17, false, [], 'string'].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + localization: value + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"localization" must be an object/); + }); + + t.end(); + + }); + + test('unknown properties in api.localization should throw error', function(t) { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + localization: { + unknown_property: 'value' + } + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"unknown_property" is not allowed/); + + t.end(); + + }); + + test('non-array api.localization.flipNumberAndStreetCountries should throw error', function(t) { + [null, 17, {}, false, 'string'].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + localization: { + flipNumberAndStreetCountries: value + } + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"flipNumberAndStreetCountries" must be an array/); + }); + + t.end(); + + }); + + test('non-string api.localization.flipNumberAndStreetCountries elements should throw error', function(t) { + [null, 17, {}, false, []].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + localization: { + flipNumberAndStreetCountries: [value] + } + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"0" must be a string/); + }); + + t.end(); + + }); + + test('non-3-char api.localization.flipNumberAndStreetCountries elements should throw error', function(t) { + ['AB', 'ABCD'].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + localization: { + flipNumberAndStreetCountries: [value] + } + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /fails to match the required pattern/); + }); + + t.end(); + + }); + +}; + +module.exports.tests.esclient_validation = function(test, common) { + test('config without esclient should throw error', function(t) { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value' + } + }; + + t.throws(function() { + configValidation.validate(config); + }, /"esclient" is required/, 'esclient should exist'); + t.end(); + + }); + + test('config with non-object esclient should throw error', function(t) { + [null, 17, [], 'string', true].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value' + }, + esclient: value + }; + + t.throws(function() { + configValidation.validate(config); + }, /"esclient" must be an object/, 'esclient should be an object'); + + }); + + t.end(); + + }); + + test('config with non-integer esclient.requestTimeout should throw error', function(t) { + [null, 'string', {}, [], false].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value' + }, + esclient: { + requestTimeout: value + } + }; + + t.throws(function() { + configValidation.validate(config); + }, /"requestTimeout" must be a number/, 'esclient.requestTimeout should be a number'); + }); + + t.end(); + + }); + + test('config with non-integer esclient.requestTimeout should throw error', function(t) { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value' + }, + esclient: { + requestTimeout: 17.3 + } + }; + + t.throws(function() { + configValidation.validate(config); + }, /"requestTimeout" must be an integer/, 'esclient.requestTimeout should be an integer'); + + t.end(); + + }); + + test('config with negative esclient.requestTimeout should throw error', function(t) { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value' + }, + esclient: { + requestTimeout: -1 + } + }; + + t.throws(function() { + configValidation.validate(config); + }, /"requestTimeout" must be larger than or equal to 0/, 'esclient.requestTimeout must be positive'); + + t.end(); + + }); + +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('configValidation: ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +};