From 2bffce8b8c2ad730e75c23f28b3dac81cda466a0 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Sat, 4 Mar 2017 15:57:55 +0000 Subject: [PATCH 01/36] fix(package): update iso3166-1 to version 0.3.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index bd538bac..f732b4df 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "geojson": "^0.4.0", "geojson-extent": "^0.3.1", "geolib": "^2.0.18", - "iso3166-1": "^0.2.3", + "iso3166-1": "^0.3.0", "joi": "^10.1.0", "lodash": "^4.5.0", "markdown": "0.5.0", From 30405acd72a84771f61364cac369139f50d4a321 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Sun, 5 Mar 2017 05:02:45 +0000 Subject: [PATCH 02/36] fix(package): update stats-lite to version 2.0.4 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index bd538bac..23dcac9a 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,7 @@ "pelias-query": "8.12.0", "pelias-text-analyzer": "1.7.1", "retry": "^0.10.1", - "stats-lite": "2.0.3", + "stats-lite": "2.0.4", "superagent": "^3.2.1", "through2": "^2.0.3" }, From b47f77ead1b6c33ee14cc0a0b1fe9f8ffac57220 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 13 Mar 2017 12:08:28 -0400 Subject: [PATCH 03/36] Use carat version for stats-lite dependency --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 23dcac9a..f996ae79 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,7 @@ "pelias-query": "8.12.0", "pelias-text-analyzer": "1.7.1", "retry": "^0.10.1", - "stats-lite": "2.0.4", + "stats-lite": "^2.0.4", "superagent": "^3.2.1", "through2": "^2.0.3" }, From 52a6ec68ff6cab465d6f8ae47debd64a83d0a374 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 6 Mar 2017 14:57:20 -0500 Subject: [PATCH 04/36] added support for coarse reverse geocoding several things are in this commit: - coarse reverse controller / pip service - inject "conditional execute" predicate into search service to shortcut execution conditions - added coarse reverse controller to precede standard reverse controller - lots of tests! --- controller/coarse_reverse.js | 129 ++++ controller/predicates/has_data.js | 5 + controller/predicates/has_errors.js | 5 + controller/predicates/is_coarse_reverse.js | 7 + controller/search.js | 21 +- package.json | 3 + routes/v1.js | 29 +- service/pointinpolygon.js | 33 + test/unit/controller/coarse_reverse.js | 578 ++++++++++++++++++ test/unit/controller/place.js | 2 +- test/unit/controller/predicates/has_data.js | 60 ++ test/unit/controller/predicates/has_errors.js | 60 ++ .../predicates/is_coarse_reverse.js | 128 ++++ test/unit/controller/search.js | 58 +- test/unit/run.js | 7 +- test/unit/service/pointinpolygon.js | 140 +++++ 16 files changed, 1195 insertions(+), 70 deletions(-) create mode 100644 controller/coarse_reverse.js create mode 100644 controller/predicates/has_data.js create mode 100644 controller/predicates/has_errors.js create mode 100644 controller/predicates/is_coarse_reverse.js create mode 100644 service/pointinpolygon.js create mode 100644 test/unit/controller/coarse_reverse.js create mode 100644 test/unit/controller/predicates/has_data.js create mode 100644 test/unit/controller/predicates/has_errors.js create mode 100644 test/unit/controller/predicates/is_coarse_reverse.js create mode 100644 test/unit/service/pointinpolygon.js diff --git a/controller/coarse_reverse.js b/controller/coarse_reverse.js new file mode 100644 index 00000000..dc48d6db --- /dev/null +++ b/controller/coarse_reverse.js @@ -0,0 +1,129 @@ +const logger = require('pelias-logger').get('coarse_reverse'); +const _ = require('lodash'); +const Document = require('pelias-model').Document; + +const granularities = [ + 'neighbourhood', + 'borough', + 'locality', + 'localadmin', + 'county', + 'macrocounty', + 'region', + 'macroregion', + 'dependency', + 'country' +]; + +function getMostGranularLayer(results) { + return granularities.find((granularity) => { + return results.hasOwnProperty(granularity); + }); +} + +function hasResultsAtRequestedLayers(results, layers) { + return _.intersection(layers, Object.keys(results)).length > 0; +} + +function synthesizeDoc(results) { + // now create a model.Document from what's level, using the most granular + // result available as the starting point + // the immediately above cannot be re-used since county may be the most + // granular layer requested but the results may start at region (no county found) + const most_granular_layer = getMostGranularLayer(results); + const id = results[most_granular_layer][0].id; + + const doc = new Document('whosonfirst', most_granular_layer, id.toString()); + doc.setName('default', results[most_granular_layer][0].name); + + if (results[most_granular_layer][0].hasOwnProperty('centroid')) { + doc.setCentroid( results[most_granular_layer][0].centroid ); + } + + if (results[most_granular_layer][0].hasOwnProperty('bounding_box')) { + const parsedBoundingBox = results[most_granular_layer][0].bounding_box.split(',').map(parseFloat); + doc.setBoundingBox({ + upperLeft: { + lat: parsedBoundingBox[3], + lon: parsedBoundingBox[0] + }, + lowerRight: { + lat: parsedBoundingBox[1], + lon: parsedBoundingBox[2] + } + }); + + } + + if (_.has(results, 'country[0].abbr')) { + doc.setAlpha3(results.country[0].abbr); + } + + // assign the administrative hierarchy + Object.keys(results).forEach((layer) => { + if (results[layer][0].hasOwnProperty('abbr')) { + doc.addParent(layer, results[layer][0].name, results[layer][0].id.toString(), results[layer][0].abbr); + } else { + doc.addParent(layer, results[layer][0].name, results[layer][0].id.toString()); + } + + }); + + const esDoc = doc.toESDocument(); + esDoc.data._id = esDoc._id; + esDoc.data._type = esDoc._type; + return esDoc.data; + +} + +function setup(service, should_execute) { + function controller(req, res, next) { + // do not run controller when a request validation error has occurred + if (!should_execute(req, res)) { + return next(); + } + + const centroid = { + lat: req.clean['point.lat'], + lon: req.clean['point.lon'] + }; + + service(centroid, (err, results) => { + // if there's an error, log it and bail + if (err) { + logger.error(err); + return next(); + } + + // find the finest granularity requested + const finest_granularity_requested = granularities.findIndex((granularity) => { + return req.clean.layers.indexOf(granularity) !== -1; + }); + + // now remove everything from the response that is more granular than the + // most granular layer requested. that is, if req.clean.layers=['county'], + // remove neighbourhoods, localities, and localadmins + Object.keys(results).forEach((layer) => { + if (granularities.indexOf(layer) < finest_granularity_requested) { + delete results[layer]; + } + }); + + res.meta = {}; + res.data = []; + // synthesize a doc from results if there's a result at the request layer(s) + if (hasResultsAtRequestedLayers(results, req.clean.layers)) { + res.data.push(synthesizeDoc(results)); + } + + return next(); + + }); + + } + + return controller; + +} + +module.exports = setup; diff --git a/controller/predicates/has_data.js b/controller/predicates/has_data.js new file mode 100644 index 00000000..a61907d4 --- /dev/null +++ b/controller/predicates/has_data.js @@ -0,0 +1,5 @@ +const _ = require('lodash'); + +module.exports = (request, response) => { + return _.get(response, 'data', []).length > 0; +}; diff --git a/controller/predicates/has_errors.js b/controller/predicates/has_errors.js new file mode 100644 index 00000000..ccc13361 --- /dev/null +++ b/controller/predicates/has_errors.js @@ -0,0 +1,5 @@ +const _ = require('lodash'); + +module.exports = (request, response) => { + return _.get(request, 'errors', []).length > 0; +}; diff --git a/controller/predicates/is_coarse_reverse.js b/controller/predicates/is_coarse_reverse.js new file mode 100644 index 00000000..b2dd1a8d --- /dev/null +++ b/controller/predicates/is_coarse_reverse.js @@ -0,0 +1,7 @@ +const _ = require('lodash'); + +module.exports = (req, res) => { + // returns true if layers is undefined, empty, or contains 'address' or 'venue' + return !_.isEmpty(req.clean.layers) && + _.intersection(req.clean.layers, ['address', 'venue']).length === 0; +}; diff --git a/controller/search.js b/controller/search.js index 2813bf3c..989060a8 100644 --- a/controller/search.js +++ b/controller/search.js @@ -7,30 +7,13 @@ const logger = require('pelias-logger').get('api'); const logging = require( '../helper/logging' ); const retry = require('retry'); -function requestHasErrors(request) { - return _.get(request, 'errors', []).length > 0; -} - -function responseHasData(response) { - return _.get(response, 'data', []).length > 0; -} - function isRequestTimeout(err) { return _.get(err, 'status') === 408; } -function setup( apiConfig, esclient, query ){ +function setup( apiConfig, esclient, query, should_execute ){ function controller( req, res, next ){ - // do not run controller when a request - // validation error has occurred. - if (requestHasErrors(req)) { - return next(); - } - - // do not run controller if there are already results - // this was added during libpostal integration. if the libpostal parse/query - // doesn't return anything then fallback to old search-engine-y behavior - if (responseHasData(res)) { + if (!should_execute(req, res)) { return next(); } diff --git a/package.json b/package.json index 69089d4c..80b46808 100644 --- a/package.json +++ b/package.json @@ -56,9 +56,11 @@ "pelias-config": "2.8.0", "pelias-labels": "1.5.1", "pelias-logger": "0.1.0", + "pelias-mock-logger": "^1.0.1", "pelias-model": "4.5.1", "pelias-query": "8.13.0", "pelias-text-analyzer": "1.7.2", + "predicates": "^1.0.1", "retry": "^0.10.1", "stats-lite": "^2.0.4", "superagent": "^3.2.1", @@ -72,6 +74,7 @@ "nsp": "^2.2.0", "precommit-hook": "^3.0.0", "proxyquire": "^1.7.10", + "request": "^2.79.0", "semantic-release": "^6.3.2", "source-map": "^0.5.6", "tap-dot": "1.0.5", diff --git a/routes/v1.js b/routes/v1.js index 64f820db..ecb19a3c 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -1,6 +1,10 @@ var Router = require('express').Router; var elasticsearch = require('elasticsearch'); +const all = require('predicates').all; +const any = require('predicates').any; +const not = require('predicates').not; + /** ----------------------- sanitizers ----------------------- **/ var sanitizers = { autocomplete: require('../sanitizer/autocomplete'), @@ -20,6 +24,7 @@ var middleware = { /** ----------------------- controllers ----------------------- **/ var controllers = { + coarse_reverse: require('../controller/coarse_reverse'), mdToHTML: require('../controller/markdownToHtml'), place: require('../controller/place'), search: require('../controller/search'), @@ -55,6 +60,17 @@ var postProc = { assignLabels: require('../middleware/assignLabels') }; +// make configurable to return do-nothing service if PIP URL not set +const pipService = require('../service/pointinpolygon')('http://localhost:3102'); + +// predicates that drive whether controller/search runs +const hasData = require('../controller/predicates/has_data'); +const hasErrors = require('../controller/predicates/has_errors'); +const isCoarseReverse = require('../controller/predicates/is_coarse_reverse'); + +// shorthand for standard early-exit conditions +const hasDataOrErrors = any(hasData, hasErrors); + /** * Append routes to app * @@ -80,9 +96,9 @@ function addRoutes(app, peliasConfig) { middleware.calcSize(), // 3rd parameter is which query module to use, use fallback/geodisambiguation // first, then use original search strategy if first query didn't return anything - controllers.search(peliasConfig.api, esclient, queries.libpostal), + controllers.search(peliasConfig.api, esclient, queries.libpostal, not(hasDataOrErrors)), sanitizers.search_fallback.middleware, - controllers.search(peliasConfig.api, esclient, queries.fallback_to_old_prod), + controllers.search(peliasConfig.api, esclient, queries.fallback_to_old_prod, not(hasDataOrErrors)), postProc.trimByGranularity(), postProc.distances('focus.point.'), postProc.confidenceScores(peliasConfig.api), @@ -101,7 +117,7 @@ function addRoutes(app, peliasConfig) { structured: createRouter([ sanitizers.structured_geocoding.middleware, middleware.calcSize(), - controllers.search(peliasConfig.api, esclient, queries.structured_geocoding), + controllers.search(peliasConfig.api, esclient, queries.structured_geocoding, not(hasDataOrErrors)), postProc.trimByGranularityStructured(), postProc.distances('focus.point.'), postProc.confidenceScores(peliasConfig.api), @@ -119,7 +135,7 @@ function addRoutes(app, peliasConfig) { ]), autocomplete: createRouter([ sanitizers.autocomplete.middleware, - controllers.search(peliasConfig.api, esclient, queries.autocomplete), + controllers.search(peliasConfig.api, esclient, queries.autocomplete, not(hasDataOrErrors)), postProc.distances('focus.point.'), postProc.confidenceScores(peliasConfig.api), postProc.dedupe(), @@ -135,7 +151,8 @@ function addRoutes(app, peliasConfig) { reverse: createRouter([ sanitizers.reverse.middleware, middleware.calcSize(), - controllers.search(peliasConfig.api, esclient, queries.reverse), + controllers.coarse_reverse(pipService, all(isCoarseReverse, not(hasErrors))), + controllers.search(peliasConfig.api, esclient, queries.reverse, not(any(hasDataOrErrors, isCoarseReverse))), postProc.distances('point.'), // reverse confidence scoring depends on distance from origin // so it must be calculated first @@ -153,7 +170,7 @@ function addRoutes(app, peliasConfig) { nearby: createRouter([ sanitizers.nearby.middleware, middleware.calcSize(), - controllers.search(peliasConfig.api, esclient, queries.reverse), + controllers.search(peliasConfig.api, esclient, queries.reverse, not(hasDataOrErrors)), postProc.distances('point.'), // reverse confidence scoring depends on distance from origin // so it must be calculated first diff --git a/service/pointinpolygon.js b/service/pointinpolygon.js new file mode 100644 index 00000000..aaece195 --- /dev/null +++ b/service/pointinpolygon.js @@ -0,0 +1,33 @@ +const logger = require( 'pelias-logger' ).get( 'pointinpolygon' ); +const request = require('request'); + +module.exports = (url) => { + function service( centroid, callback ){ + const requestUrl = `${url}/${centroid.lon}/${centroid.lat}`; + + request.get(requestUrl, (err, response, body) => { + if (err) { + logger.error(JSON.stringify(err)); + callback(err); + } + else if (response.statusCode === 200) { + try { + const parsed = JSON.parse(body); + callback(err, parsed); + } + catch (err) { + logger.error(`${requestUrl}: could not parse response body: ${body}`); + callback(`${requestUrl} returned status 200 but with non-JSON response: ${body}`); + } + } + else { + logger.error(`${requestUrl} returned status ${response.statusCode}: ${body}`); + callback(`${requestUrl} returned status ${response.statusCode}: ${body}`); + } + }); + + } + + return service; + +}; diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js new file mode 100644 index 00000000..d8071e62 --- /dev/null +++ b/test/unit/controller/coarse_reverse.js @@ -0,0 +1,578 @@ +'use strict'; + +const setup = require('../../../controller/coarse_reverse'); +const proxyquire = require('proxyquire').noCallThru(); + +module.exports.tests = {}; + +module.exports.tests.interface = (test, common) => { + test('valid interface', (t) => { + t.equal(typeof setup, 'function', 'setup is a function'); + t.equal(typeof setup(), 'function', 'setup returns a controller'); + t.end(); + }); +}; + +module.exports.tests.early_exit_conditions = (test, common) => { + test('should_execute returning false should not call service', (t) => { + const service = () => { + throw Error('service should not have been called'); + }; + + const should_execute = () => { return false; }; + const controller = setup(service, should_execute); + + const req = { + clean: { + layers: ['locality'] + }, + errors: ['error'] + }; + + // verify that next was called + let next_was_called = false; + const next = () => { + next_was_called = true; + }; + + // passing res=undefined verifies that it wasn't interacted with + t.doesNotThrow(controller.bind(null, req, undefined, next)); + t.ok(next_was_called); + t.end(); + + }); + +}; + +module.exports.tests.error_conditions = (test, common) => { + test('service error should log and call next', (t) => { + const service = (point, callback) => { + callback('this is an error'); + }; + + const logger = require('pelias-mock-logger')(); + + const should_execute = () => { return true; }; + const controller = proxyquire('../../../controller/coarse_reverse', { + 'pelias-logger': logger + })(service, should_execute); + + const req = { + clean: { + layers: ['locality'], + point: { + lat: 12.121212, + lon: 21.212121 + } + } + }; + + // verify that next was called + let next_was_called = false; + const next = () => { + next_was_called = true; + }; + + // passing res=undefined verifies that it wasn't interacted with + controller(req, undefined, next); + + t.ok(logger.isErrorMessage('this is an error')); + t.ok(next_was_called); + t.end(); + + }); + +}; + +module.exports.tests.success_conditions = (test, common) => { + test('service returning results should use first entry for each layer', (t) => { + const service = (point, callback) => { + const results = { + neighbourhood: [ + { + id: 10, + name: 'neighbourhood name', + abbr: 'neighbourhood abbr', + centroid: { + lat: 12.121212, + lon: 21.212121 + }, + bounding_box: '-76.345902,40.006751,-76.254038,40.072939' + }, + { id: 11, name: 'neighbourhood name 2'} + ], + borough: [ + { id: 20, name: 'borough name', abbr: 'borough abbr'}, + { id: 21, name: 'borough name 2'} + ], + locality: [ + { id: 30, name: 'locality name', abbr: 'locality abbr'}, + { id: 31, name: 'locality name 2'} + ], + localadmin: [ + { id: 40, name: 'localadmin name', abbr: 'localadmin abbr'}, + { id: 41, name: 'localadmin name 2'} + ], + county: [ + { id: 50, name: 'county name', abbr: 'county abbr'}, + { id: 51, name: 'county name 2'} + ], + macrocounty: [ + { id: 60, name: 'macrocounty name', abbr: 'macrocounty abbr'}, + { id: 61, name: 'macrocounty name 2'} + ], + region: [ + { id: 70, name: 'region name', abbr: 'region abbr'}, + { id: 71, name: 'region name 2'} + ], + macroregion: [ + { id: 80, name: 'macroregion name', abbr: 'macroregion abbr'}, + { id: 81, name: 'macroregion name 2'} + ], + dependency: [ + { id: 90, name: 'dependency name', abbr: 'dependency abbr'}, + { id: 91, name: 'dependency name 2'} + ], + country: [ + { id: 100, name: 'country name', abbr: 'xyz'}, + { id: 101, name: 'country name 2'} + ] + }; + + callback(undefined, results); + }; + + const logger = require('pelias-mock-logger')(); + + const should_execute = () => { return true; }; + const controller = proxyquire('../../../controller/coarse_reverse', { + 'pelias-logger': logger + })(service, should_execute); + + const req = { + clean: { + layers: ['neighbourhood'], + point: { + lat: 12.121212, + lon: 21.212121 + } + } + }; + + const res = { }; + + // verify that next was called + let next_was_called = false; + const next = () => { + next_was_called = true; + }; + + controller(req, res, next); + + const expected = { + meta: {}, + data: [ + { + _id: '10', + _type: 'neighbourhood', + layer: 'neighbourhood', + source: 'whosonfirst', + source_id: '10', + name: { + 'default': 'neighbourhood name' + }, + phrase: { + 'default': 'neighbourhood name' + }, + parent: { + neighbourhood: ['neighbourhood name'], + neighbourhood_id: ['10'], + neighbourhood_a: ['neighbourhood abbr'], + borough: ['borough name'], + borough_id: ['20'], + borough_a: ['borough abbr'], + locality: ['locality name'], + locality_id: ['30'], + locality_a: ['locality abbr'], + localadmin: ['localadmin name'], + localadmin_id: ['40'], + localadmin_a: ['localadmin abbr'], + county: ['county name'], + county_id: ['50'], + county_a: ['county abbr'], + macrocounty: ['macrocounty name'], + macrocounty_id: ['60'], + macrocounty_a: ['macrocounty abbr'], + region: ['region name'], + region_id: ['70'], + region_a: ['region abbr'], + macroregion: ['macroregion name'], + macroregion_id: ['80'], + macroregion_a: ['macroregion abbr'], + dependency: ['dependency name'], + dependency_id: ['90'], + dependency_a: ['dependency abbr'], + country: ['country name'], + country_id: ['100'], + country_a: ['xyz'] + }, + alpha3: 'XYZ', + center_point: { + lat: 12.121212, + lon: 21.212121 + }, + bounding_box: '{"min_lat":40.006751,"max_lat":40.072939,"min_lon":-76.345902,"max_lon":-76.254038}' + } + ] + }; + + t.deepEquals(res, expected); + + t.notOk(logger.hasErrorMessages()); + t.ok(next_was_called); + t.end(); + + }); + + test('layers missing from results should be ignored', (t) => { + const service = (point, callback) => { + const results = { + neighbourhood: [ + { + id: 10, + name: 'neighbourhood name', + abbr: 'neighbourhood abbr', + centroid: { + lat: 12.121212, + lon: 21.212121 + }, + bounding_box: '-76.345902,40.006751,-76.254038,40.072939' + } + ] + }; + + callback(undefined, results); + }; + + const logger = require('pelias-mock-logger')(); + + const should_execute = () => { return true; }; + const controller = proxyquire('../../../controller/coarse_reverse', { + 'pelias-logger': logger + })(service, should_execute); + + const req = { + clean: { + layers: ['neighbourhood'], + point: { + lat: 12.121212, + lon: 21.212121 + } + } + }; + + const res = { }; + + // verify that next was called + let next_was_called = false; + const next = () => { + next_was_called = true; + }; + + controller(req, res, next); + + const expected = { + meta: {}, + data: [ + { + _id: '10', + _type: 'neighbourhood', + layer: 'neighbourhood', + source: 'whosonfirst', + source_id: '10', + name: { + 'default': 'neighbourhood name' + }, + phrase: { + 'default': 'neighbourhood name' + }, + parent: { + neighbourhood: ['neighbourhood name'], + neighbourhood_id: ['10'], + neighbourhood_a: ['neighbourhood abbr'] + }, + center_point: { + lat: 12.121212, + lon: 21.212121 + }, + bounding_box: '{"min_lat":40.006751,"max_lat":40.072939,"min_lon":-76.345902,"max_lon":-76.254038}' + } + ] + }; + + t.deepEquals(res, expected); + + t.notOk(logger.hasErrorMessages()); + t.ok(next_was_called); + t.end(); + + }); + + test('most granular layer missing centroid should not set', (t) => { + const service = (point, callback) => { + const results = { + neighbourhood: [ + { + id: 10, + name: 'neighbourhood name', + abbr: 'neighbourhood abbr', + bounding_box: '-76.345902,40.006751,-76.254038,40.072939' + } + ] + }; + + callback(undefined, results); + }; + + const logger = require('pelias-mock-logger')(); + + const should_execute = () => { return true; }; + const controller = proxyquire('../../../controller/coarse_reverse', { + 'pelias-logger': logger + })(service, should_execute); + + const req = { + clean: { + layers: ['neighbourhood'], + point: { + lat: 12.121212, + lon: 21.212121 + } + } + }; + + const res = { }; + + // verify that next was called + let next_was_called = false; + const next = () => { + next_was_called = true; + }; + + controller(req, res, next); + + const expected = { + meta: {}, + data: [ + { + _id: '10', + _type: 'neighbourhood', + layer: 'neighbourhood', + source: 'whosonfirst', + source_id: '10', + name: { + 'default': 'neighbourhood name' + }, + phrase: { + 'default': 'neighbourhood name' + }, + parent: { + neighbourhood: ['neighbourhood name'], + neighbourhood_id: ['10'], + neighbourhood_a: ['neighbourhood abbr'] + }, + bounding_box: '{"min_lat":40.006751,"max_lat":40.072939,"min_lon":-76.345902,"max_lon":-76.254038}' + } + ] + }; + + t.deepEquals(res, expected); + + t.notOk(logger.hasErrorMessages()); + t.ok(next_was_called); + t.end(); + + }); + + test('most granular layer missing bounding_box should not set', (t) => { + const service = (point, callback) => { + const results = { + neighbourhood: [ + { + id: 10, + name: 'neighbourhood name', + abbr: 'neighbourhood abbr', + centroid: { + lat: 12.121212, + lon: 21.212121 + } + } + ] + }; + + callback(undefined, results); + }; + + const logger = require('pelias-mock-logger')(); + + const should_execute = () => { return true; }; + const controller = proxyquire('../../../controller/coarse_reverse', { + 'pelias-logger': logger + })(service, should_execute); + + const req = { + clean: { + layers: ['neighbourhood'], + point: { + lat: 12.121212, + lon: 21.212121 + } + } + }; + + const res = { }; + + // verify that next was called + let next_was_called = false; + const next = () => { + next_was_called = true; + }; + + controller(req, res, next); + + const expected = { + meta: {}, + data: [ + { + _id: '10', + _type: 'neighbourhood', + layer: 'neighbourhood', + source: 'whosonfirst', + source_id: '10', + name: { + 'default': 'neighbourhood name' + }, + phrase: { + 'default': 'neighbourhood name' + }, + parent: { + neighbourhood: ['neighbourhood name'], + neighbourhood_id: ['10'], + neighbourhood_a: ['neighbourhood abbr'] + }, + center_point: { + lat: 12.121212, + lon: 21.212121 + } + } + ] + }; + + t.deepEquals(res, expected); + + t.notOk(logger.hasErrorMessages()); + t.ok(next_was_called); + t.end(); + + }); + +}; + +module.exports.tests.failure_conditions = (test, common) => { + test('service returning 0 results at the requested layer should return nothing', (t) => { + const service = (point, callback) => { + // response without neighbourhood results + const results = { + borough: [ + { id: 20, name: 'borough name', abbr: 'borough abbr'}, + { id: 21, name: 'borough name 2'} + ], + locality: [ + { id: 30, name: 'locality name', abbr: 'locality abbr'}, + { id: 31, name: 'locality name 2'} + ], + localadmin: [ + { id: 40, name: 'localadmin name', abbr: 'localadmin abbr'}, + { id: 41, name: 'localadmin name 2'} + ], + county: [ + { id: 50, name: 'county name', abbr: 'county abbr'}, + { id: 51, name: 'county name 2'} + ], + macrocounty: [ + { id: 60, name: 'macrocounty name', abbr: 'macrocounty abbr'}, + { id: 61, name: 'macrocounty name 2'} + ], + region: [ + { id: 70, name: 'region name', abbr: 'region abbr'}, + { id: 71, name: 'region name 2'} + ], + macroregion: [ + { id: 80, name: 'macroregion name', abbr: 'macroregion abbr'}, + { id: 81, name: 'macroregion name 2'} + ], + dependency: [ + { id: 90, name: 'dependency name', abbr: 'dependency abbr'}, + { id: 91, name: 'dependency name 2'} + ], + country: [ + { id: 100, name: 'country name', abbr: 'xyz'}, + { id: 101, name: 'country name 2'} + ] + }; + + callback(undefined, results); + }; + + const logger = require('pelias-mock-logger')(); + + const should_execute = () => { return true; }; + const controller = proxyquire('../../../controller/coarse_reverse', { + 'pelias-logger': logger + })(service, should_execute); + + const req = { + clean: { + layers: ['neighbourhood'], + point: { + lat: 12.121212, + lon: 21.212121 + } + } + }; + + const res = { }; + + // verify that next was called + let next_was_called = false; + const next = () => { + next_was_called = true; + }; + + controller(req, res, next); + + const expected = { + meta: {}, + data: [] + }; + + t.deepEquals(res, expected); + + t.notOk(logger.hasErrorMessages()); + t.ok(next_was_called); + t.end(); + + }); + +}; + +module.exports.all = (tape, common) => { + + function test(name, testFunction) { + return tape(`GET /coarse_reverse ${name}`, testFunction); + } + + for( const testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/controller/place.js b/test/unit/controller/place.js index 22622e05..25d99463 100644 --- a/test/unit/controller/place.js +++ b/test/unit/controller/place.js @@ -1,6 +1,6 @@ 'use strict'; -const setup = require('../../../controller/search'); +const setup = require('../../../controller/place'); const proxyquire = require('proxyquire').noCallThru(); module.exports.tests = {}; diff --git a/test/unit/controller/predicates/has_data.js b/test/unit/controller/predicates/has_data.js new file mode 100644 index 00000000..48d35e67 --- /dev/null +++ b/test/unit/controller/predicates/has_data.js @@ -0,0 +1,60 @@ +'use strict'; + +const _ = require('lodash'); +const has_data = require('../../../../controller/predicates/has_data'); + +module.exports.tests = {}; + +module.exports.tests.interface = (test, common) => { + test('valid interface', (t) => { + t.equal(typeof has_data, 'function', 'has_data is a function'); + t.end(); + }); +}; + +module.exports.tests.true_conditions = (test, common) => { + test('response with non-empty data should return true', (t) => { + const req = {}; + const res = { + data: [1] + }; + + t.ok(has_data(req, res)); + t.end(); + + }); + +}; + +module.exports.tests.false_conditions = (test, common) => { + test('response with undefined data should return true', (t) => { + const req = {}; + const res = {}; + + t.notOk(has_data(req, res)); + t.end(); + + }); + + test('response with empty data array should return true', (t) => { + const req = {}; + const res = { + data: [] + }; + + t.notOk(has_data(req, res)); + t.end(); + + }); + +}; + +module.exports.all = (tape, common) => { + function test(name, testFunction) { + return tape(`GET /has_data ${name}`, testFunction); + } + + for( const testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/controller/predicates/has_errors.js b/test/unit/controller/predicates/has_errors.js new file mode 100644 index 00000000..88c255ca --- /dev/null +++ b/test/unit/controller/predicates/has_errors.js @@ -0,0 +1,60 @@ +'use strict'; + +const _ = require('lodash'); +const has_errors = require('../../../../controller/predicates/has_errors'); + +module.exports.tests = {}; + +module.exports.tests.interface = (test, common) => { + test('valid interface', (t) => { + t.equal(typeof has_errors, 'function', 'has_errors is a function'); + t.end(); + }); +}; + +module.exports.tests.true_conditions = (test, common) => { + test('request with non-empty errors should return true', (t) => { + const req = { + errors: ['error'] + }; + const res = {}; + + t.ok(has_errors(req, res)); + t.end(); + + }); + +}; + +module.exports.tests.false_conditions = (test, common) => { + test('response with undefined errors should return false', (t) => { + const req = {}; + const res = {}; + + t.notOk(has_errors(req, res)); + t.end(); + + }); + + test('response with empty errors array should return false', (t) => { + const req = { + errors: [] + }; + const res = {}; + + t.notOk(has_errors(req, res)); + t.end(); + + }); + +}; + +module.exports.all = (tape, common) => { + function test(name, testFunction) { + return tape(`GET /has_errors ${name}`, testFunction); + } + + for( const testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/controller/predicates/is_coarse_reverse.js b/test/unit/controller/predicates/is_coarse_reverse.js new file mode 100644 index 00000000..78954cfe --- /dev/null +++ b/test/unit/controller/predicates/is_coarse_reverse.js @@ -0,0 +1,128 @@ +'use strict'; + +const _ = require('lodash'); +const is_coarse_reverse = require('../../../../controller/predicates/is_coarse_reverse'); + +const coarse_layers = [ + 'continent', + 'country', + 'dependency', + 'macroregion', + 'region', + 'locality', + 'localadmin', + 'macrocounty', + 'county', + 'macrohood', + 'borough', + 'neighbourhood', + 'microhood', + 'disputed' +]; + +module.exports.tests = {}; + +module.exports.tests.interface = (test, common) => { + test('valid interface', (t) => { + t.equal(typeof is_coarse_reverse, 'function', 'is_coarse_reverse is a function'); + t.end(); + }); +}; + +module.exports.tests.false_conditions = (test, common) => { + test('request without layers should return false', (t) => { + const req = { + clean: {} + }; + + t.notOk(is_coarse_reverse(req)); + t.end(); + + }); + + test('request with empty layers should return false', (t) => { + const req = { + clean: { + layers: [] + } + }; + + t.notOk(is_coarse_reverse(req)); + t.end(); + + }); + + test('request with layers just "address" or "venue" return false', (t) => { + ['address', 'venue'].forEach((non_coarse_layer) => { + const req = { + clean: { + layers: [non_coarse_layer] + } + }; + + t.notOk(is_coarse_reverse(req)); + + }); + + t.end(); + + }); + + test('request with layers containing "address" or "venue" and a coarse layer should return false', (t) => { + ['address', 'venue'].forEach((non_coarse_layer) => { + const req = { + clean: { + layers: [_.sample(coarse_layers), non_coarse_layer] + } + }; + + t.notOk(is_coarse_reverse(req)); + + }); + + t.end(); + + }); + + test('request with layers containing "address" and "venue" should return false', (t) => { + const req = { + clean: { + layers: ['address', 'venue'] + } + }; + + t.notOk(is_coarse_reverse(req)); + t.end(); + + }); + +}; + +module.exports.tests.true_conditions = (test, common) => { + test('request with non-empty layers and not containing "address" or "venue" should return true', (t) => { + coarse_layers.forEach((coarse_layer) => { + const req = { + clean: { + layers: [coarse_layer] + } + }; + + t.ok(is_coarse_reverse(req)); + + }); + + t.end(); + + }); + +}; + +module.exports.all = (tape, common) => { + function test(name, testFunction) { + return tape(`GET /is_coarse_reverse ${name}`, testFunction); + } + + for( const testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/controller/search.js b/test/unit/controller/search.js index 78e0b2d2..3f80f7d1 100644 --- a/test/unit/controller/search.js +++ b/test/unit/controller/search.js @@ -55,7 +55,7 @@ module.exports.tests.success = function(test, common) { }; } } - })(config, esclient, query); + })(config, esclient, query, () => { return true; }); const req = { clean: { }, errors: [], warnings: [] }; const res = {}; @@ -119,7 +119,7 @@ module.exports.tests.success = function(test, common) { }; } } - })(config, esclient, query); + })(config, esclient, query, () => { return true; }); const req = { clean: { }, errors: [], warnings: [] }; const res = {}; @@ -183,7 +183,7 @@ module.exports.tests.success = function(test, common) { }; } } - })(config, esclient, query); + })(config, esclient, query, () => { return true; }); const req = { clean: { }, errors: [], warnings: [] }; const res = {}; @@ -263,7 +263,7 @@ module.exports.tests.success = function(test, common) { }; } } - })(config, esclient, query); + })(config, esclient, query, () => { return true; }); const req = { clean: { }, errors: [], warnings: [] }; const res = {}; @@ -341,7 +341,7 @@ module.exports.tests.timeout = function(test, common) { }; } } - })(config, esclient, query); + })(config, esclient, query, () => { return true; }); const req = { clean: { }, errors: [], warnings: [] }; const res = {}; @@ -392,7 +392,7 @@ module.exports.tests.timeout = function(test, common) { callback(timeoutError); } - })(config, esclient, query); + })(config, esclient, query, () => { return true; }); const req = { clean: { }, errors: [], warnings: [] }; const res = {}; @@ -432,7 +432,7 @@ module.exports.tests.timeout = function(test, common) { callback(nonTimeoutError); } - })(config, esclient, query); + })(config, esclient, query, () => { return true; }); const req = { clean: { }, errors: [], warnings: [] }; const res = {}; @@ -473,7 +473,7 @@ module.exports.tests.timeout = function(test, common) { callback(stringTypeError); } - })(config, esclient, query); + })(config, esclient, query, () => { return true; }); const req = { clean: { }, errors: [], warnings: [] }; const res = {}; @@ -494,49 +494,21 @@ module.exports.tests.timeout = function(test, common) { }; -module.exports.tests.existing_errors = function(test, common) { - test('req with errors should not call esclient or query', function(t) { +module.exports.tests.should_execute = (test, common) => { + test('should_execute returning false and empty req.errors should call next', (t) => { const esclient = () => { throw new Error('esclient should not have been called'); }; const query = () => { throw new Error('query should not have been called'); }; - const controller = setup( {}, esclient, query ); - - // the existence of `errors` means that a sanitizer detected an error, - // so don't call the esclient - const req = { - errors: ['error'] - }; - const res = { }; - - t.doesNotThrow(() => { - controller(req, res, () => {}); - }); - t.end(); - - }); - -}; - -module.exports.tests.existing_results = function(test, common) { - test('res with existing data should not call esclient or query', function(t) { - const esclient = () => { - throw new Error('esclient should not have been called'); - }; - const query = () => { - throw new Error('query should not have been called'); - }; - const controller = setup( {}, esclient, query ); + const controller = setup( {}, esclient, query, () => { return false; } ); const req = { }; - // the existence of `data` means that there are already results so - // don't call esclient or query - const res = { data: [{}] }; + const res = { }; - const next = function() { - t.deepEqual(res, {data: [{}]}); + const next = () => { + t.deepEqual(res, { }); t.end(); }; controller(req, res, next); @@ -559,7 +531,7 @@ module.exports.tests.undefined_query = function(test, common) { search_service_was_called = true; throw new Error('search service should not have been called'); } - })(undefined, undefined, query); + })(undefined, undefined, query, () => { return true; }); const next = () => { t.notOk(search_service_was_called, 'should have returned before search service was called'); diff --git a/test/unit/run.js b/test/unit/run.js index 71b14e27..a20d2d3a 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -11,9 +11,13 @@ var common = { var tests = [ require('./app'), require('./schema'), + require('./controller/coarse_reverse'), require('./controller/index'), require('./controller/place'), require('./controller/search'), + require('./controller/predicates/has_data'), + require('./controller/predicates/has_errors'), + require('./controller/predicates/is_coarse_reverse'), require('./helper/diffPlaces'), require('./helper/geojsonify'), require('./helper/logging'), @@ -73,7 +77,8 @@ var tests = [ require('./sanitizer/wrap'), require('./service/mget'), require('./service/search'), - require('./service/interpolation') + require('./service/interpolation'), + require('./service/pointinpolygon') ]; tests.map(function(t) { diff --git a/test/unit/service/pointinpolygon.js b/test/unit/service/pointinpolygon.js new file mode 100644 index 00000000..a85fcd96 --- /dev/null +++ b/test/unit/service/pointinpolygon.js @@ -0,0 +1,140 @@ +const proxyquire = require('proxyquire').noCallThru(); + +const setup = require('../../../service/pointinpolygon'); + +module.exports.tests = {}; + +module.exports.tests.interface = (test, common) => { + test('valid interface', (t) => { + const logger = require('pelias-mock-logger')(); + + var service = proxyquire('../../../service/pointinpolygon', { + 'pelias-logger': logger + }); + + t.equal(typeof service, 'function', 'service is a function'); + t.end(); + }); +}; + +module.exports.tests.success = (test, common) => { + test('lat and lon should be passed to server', (t) => { + const pipServer = require('express')(); + pipServer.get('/:lon/:lat', (req, res, next) => { + t.equals(req.params.lat, '12.121212'); + t.equals(req.params.lon, '21.212121'); + + res.send('{ "field": "value" }'); + }); + + const server = pipServer.listen(); + + const service = setup(`http://localhost:${server.address().port}`); + + service({ lat: 12.121212, lon: 21.212121}, (err, results) => { + t.notOk(err); + t.deepEquals(results, { field: 'value' }); + t.end(); + + server.close(); + + }); + + }); + +}; + +module.exports.tests.failure = (test, common) => { + test('server returning success but non-JSON body should log error and return no results', (t) => { + const pipServer = require('express')(); + pipServer.get('/:lon/:lat', (req, res, next) => { + t.equals(req.params.lat, '12.121212'); + t.equals(req.params.lon, '21.212121'); + + res.send('this is not JSON'); + }); + + const server = pipServer.listen(); + const port = server.address().port; + + const logger = require('pelias-mock-logger')(); + + const service = proxyquire('../../../service/pointinpolygon', { + 'pelias-logger': logger + })(`http://localhost:${port}`); + + service({ lat: 12.121212, lon: 21.212121}, (err, results) => { + t.equals(err, `http://localhost:${port}/21.212121/12.121212 returned status 200 but with non-JSON response: this is not JSON`); + t.notOk(results); + t.ok(logger.isErrorMessage(`http://localhost:${port}/21.212121/12.121212: could not parse response body: this is not JSON`)); + t.end(); + + server.close(); + + }); + + }); + + test('server returning error should log it and return no results', (t) => { + const server = require('express')().listen(); + const port = server.address().port; + + // immediately close the server so to ensure an error response + server.close(); + + const logger = require('pelias-mock-logger')(); + + const service = proxyquire('../../../service/pointinpolygon', { + 'pelias-logger': logger + })(`http://localhost:${port}`); + + service({ lat: 12.121212, lon: 21.212121}, (err, results) => { + t.equals(err.code, 'ECONNREFUSED'); + t.notOk(results); + t.ok(logger.isErrorMessage(/ECONNREFUSED/), 'there should be a connection refused error message'); + t.end(); + + server.close(); + + }); + + }); + + test('non-OK status should log error and return no results', (t) => { + const pipServer = require('express')(); + pipServer.get('/:lat/:lon', (req, res, next) => { + res.status(400).send('a bad request was made'); + }); + + const server = pipServer.listen(); + const port = server.address().port; + + const logger = require('pelias-mock-logger')(); + + const service = proxyquire('../../../service/pointinpolygon', { + 'pelias-logger': logger + })(`http://localhost:${port}`); + + service({ lat: 12.121212, lon: 21.212121}, (err, results) => { + t.equals(err, `http://localhost:${port}/21.212121/12.121212 returned status 400: a bad request was made`); + t.notOk(results); + t.ok(logger.isErrorMessage(`http://localhost:${port}/21.212121/12.121212 returned status 400: a bad request was made`)); + t.end(); + + server.close(); + + }); + + }); + +}; + +module.exports.all = (tape, common) => { + function test(name, testFunction) { + return tape(`SERVICE /pointinpolygon ${name}`, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; From 58701f01690eedba2e07613f297cbeb30ac3cf39 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 6 Mar 2017 17:05:49 -0500 Subject: [PATCH 05/36] return do-nothing PiP lookup function if config lacks `pipService` - added URI-formatted `api.pipService` config support in schema - added predicate that determines whether pipService is enabled/disabled - reworked should-execute conditions for non-coarse reverse --- .../predicates/is_pip_service_enabled.js | 9 +++ routes/v1.js | 25 ++++-- schema.js | 3 +- service/pointinpolygon.js | 57 +++++++------ .../predicates/is_pip_service_enabled.js | 42 ++++++++++ test/unit/run.js | 1 + test/unit/schema.js | 80 +++++++++++++++++++ test/unit/service/pointinpolygon.js | 31 ++++++- 8 files changed, 218 insertions(+), 30 deletions(-) create mode 100644 controller/predicates/is_pip_service_enabled.js create mode 100644 test/unit/controller/predicates/is_pip_service_enabled.js diff --git a/controller/predicates/is_pip_service_enabled.js b/controller/predicates/is_pip_service_enabled.js new file mode 100644 index 00000000..0324da1d --- /dev/null +++ b/controller/predicates/is_pip_service_enabled.js @@ -0,0 +1,9 @@ +const _ = require('lodash'); + +module.exports = (uri) => { + // this predicate relies upon the fact that the schema has already validated + // that api.pipService is a URI-formatted string + return (request, response) => { + return uri !== undefined; + }; +}; diff --git a/routes/v1.js b/routes/v1.js index ecb19a3c..46f44893 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -60,9 +60,6 @@ var postProc = { assignLabels: require('../middleware/assignLabels') }; -// make configurable to return do-nothing service if PIP URL not set -const pipService = require('../service/pointinpolygon')('http://localhost:3102'); - // predicates that drive whether controller/search runs const hasData = require('../controller/predicates/has_data'); const hasErrors = require('../controller/predicates/has_errors'); @@ -80,6 +77,9 @@ const hasDataOrErrors = any(hasData, hasErrors); function addRoutes(app, peliasConfig) { const esclient = elasticsearch.Client(peliasConfig.esclient); + const isPipServiceEnabled = require('../controller/predicates/is_pip_service_enabled')(peliasConfig.api.pipService); + const pipService = require('../service/pointinpolygon')(peliasConfig.api.pipService); + var base = '/v1/'; /** ------------------------- routers ------------------------- **/ @@ -151,8 +151,23 @@ function addRoutes(app, peliasConfig) { reverse: createRouter([ sanitizers.reverse.middleware, middleware.calcSize(), - controllers.coarse_reverse(pipService, all(isCoarseReverse, not(hasErrors))), - controllers.search(peliasConfig.api, esclient, queries.reverse, not(any(hasDataOrErrors, isCoarseReverse))), + controllers.coarse_reverse(pipService, + all( + not(hasErrors), isPipServiceEnabled, isCoarseReverse + ) + ), + controllers.search(peliasConfig.api, esclient, queries.reverse, + // execute under the following conditions: + // - there are no errors or data + // - request is not coarse OR pip service is disabled + all( + not(hasDataOrErrors), + any( + not(isCoarseReverse), + not(isPipServiceEnabled) + ) + ) + ), postProc.distances('point.'), // reverse confidence scoring depends on distance from origin // so it must be calculated first diff --git a/schema.js b/schema.js index 61e6c258..78a9141f 100644 --- a/schema.js +++ b/schema.js @@ -25,7 +25,8 @@ module.exports = Joi.object().keys({ requestRetries: Joi.number().integer().min(0), localization: Joi.object().keys({ flipNumberAndStreetCountries: Joi.array().items(Joi.string().regex(/^[A-Z]{3}$/)) - }).unknown(false) + }).unknown(false), + pipService: Joi.string().uri({ scheme: /https?/ }) }).requiredKeys('version', 'indexName', 'host').unknown(true), esclient: Joi.object().keys({ diff --git a/service/pointinpolygon.js b/service/pointinpolygon.js index aaece195..21b6fdcf 100644 --- a/service/pointinpolygon.js +++ b/service/pointinpolygon.js @@ -1,33 +1,44 @@ const logger = require( 'pelias-logger' ).get( 'pointinpolygon' ); const request = require('request'); +const _ = require('lodash'); module.exports = (url) => { - function service( centroid, callback ){ - const requestUrl = `${url}/${centroid.lon}/${centroid.lat}`; - - request.get(requestUrl, (err, response, body) => { - if (err) { - logger.error(JSON.stringify(err)); - callback(err); - } - else if (response.statusCode === 200) { - try { - const parsed = JSON.parse(body); - callback(err, parsed); + if (!_.isEmpty(url)) { + logger.info(`using point-in-polygon service at ${url}`); + + return function pointinpolygon( centroid, callback ) { + const requestUrl = `${url}/${centroid.lon}/${centroid.lat}`; + + request.get(requestUrl, (err, response, body) => { + if (err) { + logger.error(JSON.stringify(err)); + callback(err); } - catch (err) { - logger.error(`${requestUrl}: could not parse response body: ${body}`); - callback(`${requestUrl} returned status 200 but with non-JSON response: ${body}`); + else if (response.statusCode === 200) { + try { + const parsed = JSON.parse(body); + callback(err, parsed); + } + catch (err) { + logger.error(`${requestUrl}: could not parse response body: ${body}`); + callback(`${requestUrl} returned status 200 but with non-JSON response: ${body}`); + } } - } - else { - logger.error(`${requestUrl} returned status ${response.statusCode}: ${body}`); - callback(`${requestUrl} returned status ${response.statusCode}: ${body}`); - } - }); + else { + logger.error(`${requestUrl} returned status ${response.statusCode}: ${body}`); + callback(`${requestUrl} returned status ${response.statusCode}: ${body}`); + } + }); - } + }; - return service; + } else { + logger.warn('point-in-polygon service disabled'); + + return (centroid, callback) => { + callback(`point-in-polygon service disabled, unable to resolve ${JSON.stringify(centroid)}`); + }; + + } }; diff --git a/test/unit/controller/predicates/is_pip_service_enabled.js b/test/unit/controller/predicates/is_pip_service_enabled.js new file mode 100644 index 00000000..1d3cd63a --- /dev/null +++ b/test/unit/controller/predicates/is_pip_service_enabled.js @@ -0,0 +1,42 @@ +'use strict'; + +const _ = require('lodash'); +const is_pip_service_enabled = require('../../../../controller/predicates/is_pip_service_enabled'); + +module.exports.tests = {}; + +module.exports.tests.interface = (test, common) => { + test('valid interface', (t) => { + t.equal(typeof is_pip_service_enabled, 'function', 'is_pip_service_enabled is a function'); + t.equal(typeof is_pip_service_enabled(), 'function', 'is_pip_service_enabled() is a function'); + t.end(); + }); +}; + +module.exports.tests.true_conditions = (test, common) => { + test('string uri should return true', (t) => { + t.ok(is_pip_service_enabled('pip uri')()); + t.end(); + + }); + +}; + +module.exports.tests.false_conditions = (test, common) => { + test('undefined uri should return false', (t) => { + t.notOk(is_pip_service_enabled()()); + t.end(); + + }); + +}; + +module.exports.all = (tape, common) => { + function test(name, testFunction) { + return tape(`GET /is_pip_service_enabled ${name}`, testFunction); + } + + for( const testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/run.js b/test/unit/run.js index a20d2d3a..b746d979 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -18,6 +18,7 @@ var tests = [ require('./controller/predicates/has_data'), require('./controller/predicates/has_errors'), require('./controller/predicates/is_coarse_reverse'), + require('./controller/predicates/is_pip_service_enabled'), require('./helper/diffPlaces'), require('./helper/geojsonify'), require('./helper/logging'), diff --git a/test/unit/schema.js b/test/unit/schema.js index e1f9cfd2..6ed11add 100644 --- a/test/unit/schema.js +++ b/test/unit/schema.js @@ -367,6 +367,86 @@ module.exports.tests.api_validation = (test, common) => { }); + test('non-string api.pipService should throw error', (t) => { + [null, 17, {}, [], true].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + pipService: value + }, + esclient: {} + }; + + t.throws(validate.bind(null, config), /"pipService" must be a string/); + + }); + + t.end(); + + }); + + test('non-URI-formatted api.pipService should throw error', (t) => { + ['this is not a URI'].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + pipService: value + }, + esclient: {} + }; + + t.throws(validate.bind(null, config), /"pipService" must be a valid uri/); + + }); + + t.end(); + + }); + + test('non-http/https api.pipService should throw error', (t) => { + ['ftp', 'git', 'unknown'].forEach((scheme) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + pipService: `${scheme}://localhost` + }, + esclient: {} + }; + + t.throws(validate.bind(null, config), /"pipService" must be a valid uri/); + + }); + + t.end(); + + }); + + test('http/https api.pipService should not throw error', (t) => { + ['http', 'https'].forEach((scheme) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + pipService: `${scheme}://localhost` + }, + esclient: {} + }; + + t.doesNotThrow(validate.bind(null, config), `${scheme} should be allowed`); + + }); + + t.end(); + + }); + }; module.exports.tests.esclient_validation = (test, common) => { diff --git a/test/unit/service/pointinpolygon.js b/test/unit/service/pointinpolygon.js index a85fcd96..ac08b3f7 100644 --- a/test/unit/service/pointinpolygon.js +++ b/test/unit/service/pointinpolygon.js @@ -17,6 +17,26 @@ module.exports.tests.interface = (test, common) => { }); }; +module.exports.tests.do_nothing_service = (test, common) => { + test('undefined PiP uri should return service that logs fact that PiP service is not available', (t) => { + const logger = require('pelias-mock-logger')(); + + const service = proxyquire('../../../service/pointinpolygon', { + 'pelias-logger': logger + })(); + + service({ lat: 12.121212, lon: 21.212121 }, (err) => { + t.deepEquals(logger.getWarnMessages(), [ + 'point-in-polygon service disabled' + ]); + t.equals(err, 'point-in-polygon service disabled, unable to resolve {"lat":12.121212,"lon":21.212121}'); + t.end(); + }); + + }); + +}; + module.exports.tests.success = (test, common) => { test('lat and lon should be passed to server', (t) => { const pipServer = require('express')(); @@ -28,12 +48,21 @@ module.exports.tests.success = (test, common) => { }); const server = pipServer.listen(); + const port = server.address().port; - const service = setup(`http://localhost:${server.address().port}`); + const logger = require('pelias-mock-logger')(); + + const service = proxyquire('../../../service/pointinpolygon', { + 'pelias-logger': logger + })(`http://localhost:${port}`); service({ lat: 12.121212, lon: 21.212121}, (err, results) => { t.notOk(err); t.deepEquals(results, { field: 'value' }); + + t.ok(logger.isInfoMessage(`using point-in-polygon service at http://localhost:${port}`)); + t.notOk(logger.hasErrorMessages()); + t.end(); server.close(); From c3fd896325f6934c3c7ccb5919e0aa711ca5c5d3 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 7 Mar 2017 11:51:23 -0500 Subject: [PATCH 06/36] added helper vars for clarity --- routes/v1.js | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/routes/v1.js b/routes/v1.js index 46f44893..545506d6 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -80,6 +80,21 @@ function addRoutes(app, peliasConfig) { const isPipServiceEnabled = require('../controller/predicates/is_pip_service_enabled')(peliasConfig.api.pipService); const pipService = require('../service/pointinpolygon')(peliasConfig.api.pipService); + const coarse_reverse_should_execute = all( + not(hasErrors), isPipServiceEnabled, isCoarseReverse + ); + + // execute under the following conditions: + // - there are no errors or data + // - request is not coarse OR pip service is disabled + const original_reverse_should_execute = all( + not(hasDataOrErrors), + any( + not(isCoarseReverse), + not(isPipServiceEnabled) + ) + ); + var base = '/v1/'; /** ------------------------- routers ------------------------- **/ @@ -151,23 +166,8 @@ function addRoutes(app, peliasConfig) { reverse: createRouter([ sanitizers.reverse.middleware, middleware.calcSize(), - controllers.coarse_reverse(pipService, - all( - not(hasErrors), isPipServiceEnabled, isCoarseReverse - ) - ), - controllers.search(peliasConfig.api, esclient, queries.reverse, - // execute under the following conditions: - // - there are no errors or data - // - request is not coarse OR pip service is disabled - all( - not(hasDataOrErrors), - any( - not(isCoarseReverse), - not(isPipServiceEnabled) - ) - ) - ), + controllers.coarse_reverse(pipService, coarse_reverse_should_execute), + controllers.search(peliasConfig.api, esclient, queries.reverse, original_reverse_should_execute), postProc.distances('point.'), // reverse confidence scoring depends on distance from origin // so it must be calculated first From 0d1d67d6a27aa7df9783c179a22bfb8d3ed17e17 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Wed, 15 Mar 2017 10:58:12 -0400 Subject: [PATCH 07/36] added street layer --- controller/predicates/is_coarse_reverse.js | 6 ++++-- test/unit/controller/predicates/is_coarse_reverse.js | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/controller/predicates/is_coarse_reverse.js b/controller/predicates/is_coarse_reverse.js index b2dd1a8d..c8a292dd 100644 --- a/controller/predicates/is_coarse_reverse.js +++ b/controller/predicates/is_coarse_reverse.js @@ -1,7 +1,9 @@ const _ = require('lodash'); +const non_coarse_layers = ['address', 'street', 'venue']; + module.exports = (req, res) => { - // returns true if layers is undefined, empty, or contains 'address' or 'venue' + // returns true if layers is undefined, empty, or contains 'address', 'street', or 'venue' return !_.isEmpty(req.clean.layers) && - _.intersection(req.clean.layers, ['address', 'venue']).length === 0; + _.intersection(req.clean.layers, non_coarse_layers).length === 0; }; diff --git a/test/unit/controller/predicates/is_coarse_reverse.js b/test/unit/controller/predicates/is_coarse_reverse.js index 78954cfe..2a38e401 100644 --- a/test/unit/controller/predicates/is_coarse_reverse.js +++ b/test/unit/controller/predicates/is_coarse_reverse.js @@ -53,7 +53,7 @@ module.exports.tests.false_conditions = (test, common) => { }); test('request with layers just "address" or "venue" return false', (t) => { - ['address', 'venue'].forEach((non_coarse_layer) => { + ['address', 'street', 'venue'].forEach((non_coarse_layer) => { const req = { clean: { layers: [non_coarse_layer] @@ -69,7 +69,7 @@ module.exports.tests.false_conditions = (test, common) => { }); test('request with layers containing "address" or "venue" and a coarse layer should return false', (t) => { - ['address', 'venue'].forEach((non_coarse_layer) => { + ['address', 'street', 'venue'].forEach((non_coarse_layer) => { const req = { clean: { layers: [_.sample(coarse_layers), non_coarse_layer] From f57ac6fa0550f452618e84a2560d38249ee54421 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Wed, 15 Mar 2017 10:58:49 -0400 Subject: [PATCH 08/36] removed lodash --- controller/predicates/is_pip_service_enabled.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/controller/predicates/is_pip_service_enabled.js b/controller/predicates/is_pip_service_enabled.js index 0324da1d..aa787d87 100644 --- a/controller/predicates/is_pip_service_enabled.js +++ b/controller/predicates/is_pip_service_enabled.js @@ -1,5 +1,3 @@ -const _ = require('lodash'); - module.exports = (uri) => { // this predicate relies upon the fact that the schema has already validated // that api.pipService is a URI-formatted string From f06704985ecea5317df4e3ef0dfd60c19bbfff12 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Wed, 15 Mar 2017 11:08:57 -0400 Subject: [PATCH 09/36] renamed predicates to be more accurate --- .../{has_errors.js => has_request_errors.js} | 0 .../{has_data.js => has_response_data.js} | 0 routes/v1.js | 20 +++++++++---------- .../{has_errors.js => has_request_errors.js} | 12 +++++------ .../{has_data.js => has_response_data.js} | 12 +++++------ test/unit/run.js | 4 ++-- 6 files changed, 24 insertions(+), 24 deletions(-) rename controller/predicates/{has_errors.js => has_request_errors.js} (100%) rename controller/predicates/{has_data.js => has_response_data.js} (100%) rename test/unit/controller/predicates/{has_errors.js => has_request_errors.js} (71%) rename test/unit/controller/predicates/{has_data.js => has_response_data.js} (71%) diff --git a/controller/predicates/has_errors.js b/controller/predicates/has_request_errors.js similarity index 100% rename from controller/predicates/has_errors.js rename to controller/predicates/has_request_errors.js diff --git a/controller/predicates/has_data.js b/controller/predicates/has_response_data.js similarity index 100% rename from controller/predicates/has_data.js rename to controller/predicates/has_response_data.js diff --git a/routes/v1.js b/routes/v1.js index 545506d6..60e48172 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -61,12 +61,12 @@ var postProc = { }; // predicates that drive whether controller/search runs -const hasData = require('../controller/predicates/has_data'); -const hasErrors = require('../controller/predicates/has_errors'); +const hasResponseData = require('../controller/predicates/has_response_data'); +const hasRequestErrors = require('../controller/predicates/has_request_errors'); const isCoarseReverse = require('../controller/predicates/is_coarse_reverse'); // shorthand for standard early-exit conditions -const hasDataOrErrors = any(hasData, hasErrors); +const hasResponseDataOrRequestErrors = any(hasResponseData, hasRequestErrors); /** * Append routes to app @@ -81,14 +81,14 @@ function addRoutes(app, peliasConfig) { const pipService = require('../service/pointinpolygon')(peliasConfig.api.pipService); const coarse_reverse_should_execute = all( - not(hasErrors), isPipServiceEnabled, isCoarseReverse + not(hasRequestErrors), isPipServiceEnabled, isCoarseReverse ); // execute under the following conditions: // - there are no errors or data // - request is not coarse OR pip service is disabled const original_reverse_should_execute = all( - not(hasDataOrErrors), + not(hasResponseDataOrRequestErrors), any( not(isCoarseReverse), not(isPipServiceEnabled) @@ -111,9 +111,9 @@ function addRoutes(app, peliasConfig) { middleware.calcSize(), // 3rd parameter is which query module to use, use fallback/geodisambiguation // first, then use original search strategy if first query didn't return anything - controllers.search(peliasConfig.api, esclient, queries.libpostal, not(hasDataOrErrors)), + controllers.search(peliasConfig.api, esclient, queries.libpostal, not(hasResponseDataOrRequestErrors)), sanitizers.search_fallback.middleware, - controllers.search(peliasConfig.api, esclient, queries.fallback_to_old_prod, not(hasDataOrErrors)), + controllers.search(peliasConfig.api, esclient, queries.fallback_to_old_prod, not(hasResponseDataOrRequestErrors)), postProc.trimByGranularity(), postProc.distances('focus.point.'), postProc.confidenceScores(peliasConfig.api), @@ -132,7 +132,7 @@ function addRoutes(app, peliasConfig) { structured: createRouter([ sanitizers.structured_geocoding.middleware, middleware.calcSize(), - controllers.search(peliasConfig.api, esclient, queries.structured_geocoding, not(hasDataOrErrors)), + controllers.search(peliasConfig.api, esclient, queries.structured_geocoding, not(hasResponseDataOrRequestErrors)), postProc.trimByGranularityStructured(), postProc.distances('focus.point.'), postProc.confidenceScores(peliasConfig.api), @@ -150,7 +150,7 @@ function addRoutes(app, peliasConfig) { ]), autocomplete: createRouter([ sanitizers.autocomplete.middleware, - controllers.search(peliasConfig.api, esclient, queries.autocomplete, not(hasDataOrErrors)), + controllers.search(peliasConfig.api, esclient, queries.autocomplete, not(hasResponseDataOrRequestErrors)), postProc.distances('focus.point.'), postProc.confidenceScores(peliasConfig.api), postProc.dedupe(), @@ -185,7 +185,7 @@ function addRoutes(app, peliasConfig) { nearby: createRouter([ sanitizers.nearby.middleware, middleware.calcSize(), - controllers.search(peliasConfig.api, esclient, queries.reverse, not(hasDataOrErrors)), + controllers.search(peliasConfig.api, esclient, queries.reverse, not(hasResponseDataOrRequestErrors)), postProc.distances('point.'), // reverse confidence scoring depends on distance from origin // so it must be calculated first diff --git a/test/unit/controller/predicates/has_errors.js b/test/unit/controller/predicates/has_request_errors.js similarity index 71% rename from test/unit/controller/predicates/has_errors.js rename to test/unit/controller/predicates/has_request_errors.js index 88c255ca..bd1868aa 100644 --- a/test/unit/controller/predicates/has_errors.js +++ b/test/unit/controller/predicates/has_request_errors.js @@ -1,13 +1,13 @@ 'use strict'; const _ = require('lodash'); -const has_errors = require('../../../../controller/predicates/has_errors'); +const has_request_errors = require('../../../../controller/predicates/has_request_errors'); module.exports.tests = {}; module.exports.tests.interface = (test, common) => { test('valid interface', (t) => { - t.equal(typeof has_errors, 'function', 'has_errors is a function'); + t.equal(typeof has_request_errors, 'function', 'has_request_errors is a function'); t.end(); }); }; @@ -19,7 +19,7 @@ module.exports.tests.true_conditions = (test, common) => { }; const res = {}; - t.ok(has_errors(req, res)); + t.ok(has_request_errors(req, res)); t.end(); }); @@ -31,7 +31,7 @@ module.exports.tests.false_conditions = (test, common) => { const req = {}; const res = {}; - t.notOk(has_errors(req, res)); + t.notOk(has_request_errors(req, res)); t.end(); }); @@ -42,7 +42,7 @@ module.exports.tests.false_conditions = (test, common) => { }; const res = {}; - t.notOk(has_errors(req, res)); + t.notOk(has_request_errors(req, res)); t.end(); }); @@ -51,7 +51,7 @@ module.exports.tests.false_conditions = (test, common) => { module.exports.all = (tape, common) => { function test(name, testFunction) { - return tape(`GET /has_errors ${name}`, testFunction); + return tape(`GET /has_request_errors ${name}`, testFunction); } for( const testCase in module.exports.tests ){ diff --git a/test/unit/controller/predicates/has_data.js b/test/unit/controller/predicates/has_response_data.js similarity index 71% rename from test/unit/controller/predicates/has_data.js rename to test/unit/controller/predicates/has_response_data.js index 48d35e67..f073fc4d 100644 --- a/test/unit/controller/predicates/has_data.js +++ b/test/unit/controller/predicates/has_response_data.js @@ -1,13 +1,13 @@ 'use strict'; const _ = require('lodash'); -const has_data = require('../../../../controller/predicates/has_data'); +const has_response_data = require('../../../../controller/predicates/has_response_data'); module.exports.tests = {}; module.exports.tests.interface = (test, common) => { test('valid interface', (t) => { - t.equal(typeof has_data, 'function', 'has_data is a function'); + t.equal(typeof has_response_data, 'function', 'has_response_data is a function'); t.end(); }); }; @@ -19,7 +19,7 @@ module.exports.tests.true_conditions = (test, common) => { data: [1] }; - t.ok(has_data(req, res)); + t.ok(has_response_data(req, res)); t.end(); }); @@ -31,7 +31,7 @@ module.exports.tests.false_conditions = (test, common) => { const req = {}; const res = {}; - t.notOk(has_data(req, res)); + t.notOk(has_response_data(req, res)); t.end(); }); @@ -42,7 +42,7 @@ module.exports.tests.false_conditions = (test, common) => { data: [] }; - t.notOk(has_data(req, res)); + t.notOk(has_response_data(req, res)); t.end(); }); @@ -51,7 +51,7 @@ module.exports.tests.false_conditions = (test, common) => { module.exports.all = (tape, common) => { function test(name, testFunction) { - return tape(`GET /has_data ${name}`, testFunction); + return tape(`GET /has_response_data ${name}`, testFunction); } for( const testCase in module.exports.tests ){ diff --git a/test/unit/run.js b/test/unit/run.js index b746d979..a16fb18e 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -15,8 +15,8 @@ var tests = [ require('./controller/index'), require('./controller/place'), require('./controller/search'), - require('./controller/predicates/has_data'), - require('./controller/predicates/has_errors'), + require('./controller/predicates/has_response_data'), + require('./controller/predicates/has_request_errors'), require('./controller/predicates/is_coarse_reverse'), require('./controller/predicates/is_pip_service_enabled'), require('./helper/diffPlaces'), From 97d736eda86b9248acaf20be48fb1c2aa90733ef Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Thu, 16 Mar 2017 20:33:55 +0000 Subject: [PATCH 10/36] fix(package): update pelias-query to version 8.14.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 69089d4c..881722a8 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ "pelias-labels": "1.5.1", "pelias-logger": "0.1.0", "pelias-model": "4.5.1", - "pelias-query": "8.13.0", + "pelias-query": "8.14.0", "pelias-text-analyzer": "1.7.2", "retry": "^0.10.1", "stats-lite": "^2.0.4", From fe38d2336d1a63acecc35da4058d38c27c2ac6ba Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Thu, 16 Mar 2017 16:56:54 -0400 Subject: [PATCH 11/36] feat: allow postalcode-only structured queries --- sanitizer/_synthesize_analysis.js | 5 +- .../structured_geocoding/fallback.json | 69 +++++++++++++++++++ .../structured_geocoding/postalcode_only.js | 68 ++++++++++++++++++ test/unit/query/structured_geocoding.js | 18 +++++ test/unit/sanitizer/_synthesize_analysis.js | 2 +- 5 files changed, 157 insertions(+), 5 deletions(-) create mode 100644 test/unit/fixture/structured_geocoding/postalcode_only.js diff --git a/sanitizer/_synthesize_analysis.js b/sanitizer/_synthesize_analysis.js index f26dfc6a..6c702f9e 100644 --- a/sanitizer/_synthesize_analysis.js +++ b/sanitizer/_synthesize_analysis.js @@ -50,10 +50,7 @@ function sanitize( raw, clean ){ }, {}); - if (isPostalCodeOnly(clean.parsed_text)) { - messages.errors.push('postalcode-only inputs are not supported'); - } - else if (_.isEmpty(Object.keys(clean.parsed_text))) { + if (_.isEmpty(Object.keys(clean.parsed_text))) { messages.errors.push( `at least one of the following fields is required: ${Object.keys(fields).join(', ')}`); } diff --git a/test/unit/fixture/structured_geocoding/fallback.json b/test/unit/fixture/structured_geocoding/fallback.json index cd12a864..58d0ce8c 100644 --- a/test/unit/fixture/structured_geocoding/fallback.json +++ b/test/unit/fixture/structured_geocoding/fallback.json @@ -287,6 +287,75 @@ "boost": 5 } }, + { + "bool": { + "_name": "fallback.postalcode", + "must": [ + { + "multi_match": { + "query": "postalcode value", + "type": "phrase", + "fields": [ + "parent.postalcode" + ] + } + }, + { + "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": "postalcode" + } + } + } + }, { "bool": { "_name": "fallback.neighbourhood", diff --git a/test/unit/fixture/structured_geocoding/postalcode_only.js b/test/unit/fixture/structured_geocoding/postalcode_only.js new file mode 100644 index 00000000..7dafbaa0 --- /dev/null +++ b/test/unit/fixture/structured_geocoding/postalcode_only.js @@ -0,0 +1,68 @@ +module.exports = { + 'query': { + 'function_score': { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'should': [ + { + 'bool': { + '_name': 'fallback.postalcode', + 'must': [ + { + 'multi_match': { + 'query': 'postalcode value', + 'type': 'phrase', + 'fields': [ + 'parent.postalcode' + ] + } + } + ], + 'filter': { + 'term': { + 'layer': 'postalcode' + } + } + } + } + ] + } + }, + 'filter': { + 'bool': { + 'must': [] + } + } + } + }, + 'max_boost': 20, + 'functions': [ + { + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'popularity', + 'missing': 1 + }, + 'weight': 1 + }, + { + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'population', + 'missing': 1 + }, + 'weight': 2 + } + ], + 'score_mode': 'avg', + 'boost_mode': 'multiply' + } + }, + 'size': 20, + 'track_scores': true, + 'sort': [ + '_score' + ] +}; diff --git a/test/unit/query/structured_geocoding.js b/test/unit/query/structured_geocoding.js index fc5f211a..d93a03aa 100644 --- a/test/unit/query/structured_geocoding.js +++ b/test/unit/query/structured_geocoding.js @@ -188,6 +188,24 @@ module.exports.tests.query = function(test, common) { }); + test('parsed_text with all fields should use FallbackQuery', function(t) { + var clean = { + parsed_text: { + postalcode: 'postalcode value' + } + }; + + var query = generate(clean); + + var compiled = JSON.parse(JSON.stringify(query)); + var expected = require('../fixture/structured_geocoding/postalcode_only'); + + t.deepEqual(compiled.type, 'fallback', 'query type set'); + t.deepEqual(compiled.body, expected, 'structured postalcode only'); + t.end(); + + }); + test('valid boundary.country search', function(t) { var clean = { parsed_text: { diff --git a/test/unit/sanitizer/_synthesize_analysis.js b/test/unit/sanitizer/_synthesize_analysis.js index 97d49b2a..82352c8d 100644 --- a/test/unit/sanitizer/_synthesize_analysis.js +++ b/test/unit/sanitizer/_synthesize_analysis.js @@ -131,7 +131,7 @@ module.exports.tests.text_parser = function(test, common) { const messages = sanitizer(raw, clean); t.deepEquals(clean, expected_clean); - t.deepEquals(messages.errors, ['postalcode-only inputs are not supported'], 'no errors'); + t.deepEquals(messages.errors, [], 'no errors'); t.deepEquals(messages.warnings, [], 'no warnings'); t.end(); From 3a7e8c522980b30409ae33d37ffff5966d8f0049 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 20 Mar 2017 15:45:38 +0100 Subject: [PATCH 12/36] fix ciao tests for postalcode layer added --- test/ciao/autocomplete/layers_alias_coarse.coffee | 3 ++- test/ciao/autocomplete/layers_invalid.coffee | 2 +- test/ciao/autocomplete/layers_mix_invalid_valid.coffee | 2 +- test/ciao/reverse/layers_alias_coarse.coffee | 3 ++- test/ciao/reverse/layers_invalid.coffee | 2 +- test/ciao/reverse/layers_mix_invalid_valid.coffee | 2 +- test/ciao/search/address_parsing.coffee | 2 +- test/ciao/search/layers_alias_coarse.coffee | 3 ++- test/ciao/search/layers_invalid.coffee | 2 +- test/ciao/search/layers_mix_invalid_valid.coffee | 2 +- 10 files changed, 13 insertions(+), 10 deletions(-) diff --git a/test/ciao/autocomplete/layers_alias_coarse.coffee b/test/ciao/autocomplete/layers_alias_coarse.coffee index 3db308be..5ceaaa46 100644 --- a/test/ciao/autocomplete/layers_alias_coarse.coffee +++ b/test/ciao/autocomplete/layers_alias_coarse.coffee @@ -44,5 +44,6 @@ json.geocoding.query.layers.should.eql [ "continent", "borough", "neighbourhood", "microhood", - "disputed" + "disputed", + "postalcode" ] diff --git a/test/ciao/autocomplete/layers_invalid.coffee b/test/ciao/autocomplete/layers_invalid.coffee index 136903a0..5396387a 100644 --- a/test/ciao/autocomplete/layers_invalid.coffee +++ b/test/ciao/autocomplete/layers_invalid.coffee @@ -24,7 +24,7 @@ json.features.should.be.instanceof Array #? expected errors should.exist json.geocoding.errors -json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,street,country,macroregion,region,county,localadmin,locality,borough,neighbourhood,continent,dependency,macrocounty,macrohood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,street,country,macroregion,region,county,localadmin,locality,borough,neighbourhood,continent,dependency,macrocounty,macrohood,microhood,disputed,postalcode' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/autocomplete/layers_mix_invalid_valid.coffee b/test/ciao/autocomplete/layers_mix_invalid_valid.coffee index b452409c..6a9cc488 100644 --- a/test/ciao/autocomplete/layers_mix_invalid_valid.coffee +++ b/test/ciao/autocomplete/layers_mix_invalid_valid.coffee @@ -24,7 +24,7 @@ json.features.should.be.instanceof Array #? expected errors should.exist json.geocoding.errors -json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,street,country,macroregion,region,county,localadmin,locality,borough,neighbourhood,continent,dependency,macrocounty,macrohood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,street,country,macroregion,region,county,localadmin,locality,borough,neighbourhood,continent,dependency,macrocounty,macrohood,microhood,disputed,postalcode' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/reverse/layers_alias_coarse.coffee b/test/ciao/reverse/layers_alias_coarse.coffee index 40ce2e37..f70eeb11 100644 --- a/test/ciao/reverse/layers_alias_coarse.coffee +++ b/test/ciao/reverse/layers_alias_coarse.coffee @@ -43,5 +43,6 @@ json.geocoding.query.layers.should.eql [ "continent", "borough", "neighbourhood", "microhood", - "disputed" + "disputed", + "postalcode" ] diff --git a/test/ciao/reverse/layers_invalid.coffee b/test/ciao/reverse/layers_invalid.coffee index 9d2daa82..7298285a 100644 --- a/test/ciao/reverse/layers_invalid.coffee +++ b/test/ciao/reverse/layers_invalid.coffee @@ -24,7 +24,7 @@ json.features.should.be.instanceof Array #? expected errors should.exist json.geocoding.errors -json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,street,country,macroregion,region,county,localadmin,locality,borough,neighbourhood,continent,dependency,macrocounty,macrohood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,street,country,macroregion,region,county,localadmin,locality,borough,neighbourhood,continent,dependency,macrocounty,macrohood,microhood,disputed,postalcode' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/reverse/layers_mix_invalid_valid.coffee b/test/ciao/reverse/layers_mix_invalid_valid.coffee index 21cfe14b..c3165cf7 100644 --- a/test/ciao/reverse/layers_mix_invalid_valid.coffee +++ b/test/ciao/reverse/layers_mix_invalid_valid.coffee @@ -24,7 +24,7 @@ json.features.should.be.instanceof Array #? expected errors should.exist json.geocoding.errors -json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,street,country,macroregion,region,county,localadmin,locality,borough,neighbourhood,continent,dependency,macrocounty,macrohood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,street,country,macroregion,region,county,localadmin,locality,borough,neighbourhood,continent,dependency,macrocounty,macrohood,microhood,disputed,postalcode' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/search/address_parsing.coffee b/test/ciao/search/address_parsing.coffee index 2293ffa6..74b4d75e 100644 --- a/test/ciao/search/address_parsing.coffee +++ b/test/ciao/search/address_parsing.coffee @@ -35,7 +35,7 @@ json.geocoding.query['size'].should.eql 10 #? address parsing json.geocoding.query.parsed_text['number'].should.eql '30' json.geocoding.query.parsed_text['street'].should.eql 'w 26th st' -json.geocoding.query.parsed_text['state'].should.eql 'NY' +json.geocoding.query.parsed_text['state'].should.eql 'ny' json.features[0].properties.confidence.should.eql 1 json.features[0].properties.match_type.should.eql "exact" diff --git a/test/ciao/search/layers_alias_coarse.coffee b/test/ciao/search/layers_alias_coarse.coffee index 48723853..af8d3d8e 100644 --- a/test/ciao/search/layers_alias_coarse.coffee +++ b/test/ciao/search/layers_alias_coarse.coffee @@ -44,5 +44,6 @@ json.geocoding.query.layers.should.eql [ "continent", "borough", "neighbourhood", "microhood", - "disputed" + "disputed", + "postalcode" ] diff --git a/test/ciao/search/layers_invalid.coffee b/test/ciao/search/layers_invalid.coffee index 10d70a76..52b370dd 100644 --- a/test/ciao/search/layers_invalid.coffee +++ b/test/ciao/search/layers_invalid.coffee @@ -24,7 +24,7 @@ json.features.should.be.instanceof Array #? expected errors should.exist json.geocoding.errors -json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,street,country,macroregion,region,county,localadmin,locality,borough,neighbourhood,continent,dependency,macrocounty,macrohood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,street,country,macroregion,region,county,localadmin,locality,borough,neighbourhood,continent,dependency,macrocounty,macrohood,microhood,disputed,postalcode' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/search/layers_mix_invalid_valid.coffee b/test/ciao/search/layers_mix_invalid_valid.coffee index 373838b0..da39b9e8 100644 --- a/test/ciao/search/layers_mix_invalid_valid.coffee +++ b/test/ciao/search/layers_mix_invalid_valid.coffee @@ -24,7 +24,7 @@ json.features.should.be.instanceof Array #? expected errors should.exist json.geocoding.errors -json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,street,country,macroregion,region,county,localadmin,locality,borough,neighbourhood,continent,dependency,macrocounty,macrohood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,street,country,macroregion,region,county,localadmin,locality,borough,neighbourhood,continent,dependency,macrocounty,macrohood,microhood,disputed,postalcode' ] #? expected warnings should.not.exist json.geocoding.warnings From 0bc66459538d8983b97394d8cb946bc3d0934bd9 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 20 Mar 2017 15:46:06 +0100 Subject: [PATCH 13/36] run ciao tests syncronously --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 80b46808..5f6c106a 100644 --- a/package.json +++ b/package.json @@ -67,7 +67,7 @@ "through2": "^2.0.3" }, "devDependencies": { - "ciao": "^0.6.0", + "ciao": "git://github.com/missinglink/ciao.git#sync", "difflet": "^1.0.1", "istanbul": "^0.4.2", "jshint": "^2.5.6", From 8c68bd64c0c65a91480f1fe2cc3b36cca2e9d777 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 20 Mar 2017 17:13:20 +0100 Subject: [PATCH 14/36] update dev dep: ciao --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 5f6c106a..3979a2a0 100644 --- a/package.json +++ b/package.json @@ -67,7 +67,7 @@ "through2": "^2.0.3" }, "devDependencies": { - "ciao": "git://github.com/missinglink/ciao.git#sync", + "ciao": "^1.0.0", "difflet": "^1.0.1", "istanbul": "^0.4.2", "jshint": "^2.5.6", From ae2c0e76710c4858c7e76247db74056ec0dd841a Mon Sep 17 00:00:00 2001 From: missinglink Date: Fri, 17 Mar 2017 14:49:07 +0100 Subject: [PATCH 15/36] negotiate HTTP locales for incoming browser requests --- middleware/requestLanguage.js | 74 +++++++++++++++++++++++++++++++++++ package.json | 2 + routes/v1.js | 9 ++++- 3 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 middleware/requestLanguage.js diff --git a/middleware/requestLanguage.js b/middleware/requestLanguage.js new file mode 100644 index 00000000..ebe21b9a --- /dev/null +++ b/middleware/requestLanguage.js @@ -0,0 +1,74 @@ + +/** + this middleware is responsible for negotiating HTTP locales for incoming + browser requests by reading 'Accept-Language' request headers. + + the preferred language will then be available on the $req object: + eg. for 'Accept-Language: fr': + ``` + console.log( req.language ); + + { + name: 'French', + type: 'living', + scope: 'individual', + iso6393: 'fra', + iso6392B: 'fre', + iso6392T: 'fra', + iso6391: 'fr', + defaulted: false + } + ``` + + for configuration options see: + https://github.com/florrain/locale +**/ + +const locale = require('locale'); + +/** + BCP47 language tags can contain three parts: + 1. A language subtag (en, zh). + 2. A script subtag (Hant, Latn). + 3. A region subtag (US, CN). + + at time of writing we will only be concerned with 1. (the language subtag) with + the intention of being compatible with the language standard of whosonfirst data. + + whosonfirst data is in ISO 639-3 format so we will need to configure the library + to support all ISO 639-1 (2 char) codes and convert them to 639-1 (3-char) codes. + + see: https://github.com/whosonfirst/whosonfirst-names +**/ +const iso6393 = require('iso-639-3'); + +// create a dictionary which maps the ISO 639-1 language subtags to a map +// of it's represenation in several different standards. +const language = {}; +iso6393.filter( i => !!i.iso6391 ).forEach( i => language[ i.iso6391 ] = i ); + +// a pre-processed locale list of language subtags we support (all of them). +const allLocales = new locale.Locales( Object.keys( language ) ); + +// return the middleware +module.exports = function middleware( req, res, next ){ + + // parse request & choose best locale + var locales = new locale.Locales( req.headers['accept-language'] || '' ); + var best = locales.best( allLocales ); + + // set $req.language property + req.language = language[ best.language ] || language.en; + req.language.defaulted = best.defaulted; + + // set $req.clean property in order to print language info in response header + req.clean = req.clean || {}; + req.clean.lang = { + name: req.language.name, + iso6391: req.language.iso6391, + iso6393: req.language.iso6393, + defaulted: req.language.defaulted + }; + + next(); +}; diff --git a/package.json b/package.json index e92c0018..9b0b32ea 100644 --- a/package.json +++ b/package.json @@ -47,8 +47,10 @@ "geojson": "^0.4.0", "geojson-extent": "^0.3.1", "geolib": "^2.0.18", + "iso-639-3": "^1.0.0", "iso3166-1": "^0.3.0", "joi": "^10.1.0", + "locale": "^0.1.0", "lodash": "^4.5.0", "markdown": "0.5.0", "morgan": "1.8.1", diff --git a/routes/v1.js b/routes/v1.js index 60e48172..8ee4173f 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -18,7 +18,8 @@ var sanitizers = { /** ----------------------- middleware ------------------------ **/ var middleware = { - calcSize: require('../middleware/sizeCalculator') + calcSize: require('../middleware/sizeCalculator'), + requestLanguage: require('../middleware/requestLanguage') }; /** ----------------------- controllers ----------------------- **/ @@ -108,6 +109,7 @@ function addRoutes(app, peliasConfig) { ]), search: createRouter([ sanitizers.search.middleware, + middleware.requestLanguage, middleware.calcSize(), // 3rd parameter is which query module to use, use fallback/geodisambiguation // first, then use original search strategy if first query didn't return anything @@ -131,6 +133,7 @@ function addRoutes(app, peliasConfig) { ]), structured: createRouter([ sanitizers.structured_geocoding.middleware, + middleware.requestLanguage, middleware.calcSize(), controllers.search(peliasConfig.api, esclient, queries.structured_geocoding, not(hasResponseDataOrRequestErrors)), postProc.trimByGranularityStructured(), @@ -150,6 +153,7 @@ function addRoutes(app, peliasConfig) { ]), autocomplete: createRouter([ sanitizers.autocomplete.middleware, + middleware.requestLanguage, controllers.search(peliasConfig.api, esclient, queries.autocomplete, not(hasResponseDataOrRequestErrors)), postProc.distances('focus.point.'), postProc.confidenceScores(peliasConfig.api), @@ -165,6 +169,7 @@ function addRoutes(app, peliasConfig) { ]), reverse: createRouter([ sanitizers.reverse.middleware, + middleware.requestLanguage, middleware.calcSize(), controllers.coarse_reverse(pipService, coarse_reverse_should_execute), controllers.search(peliasConfig.api, esclient, queries.reverse, original_reverse_should_execute), @@ -184,6 +189,7 @@ function addRoutes(app, peliasConfig) { ]), nearby: createRouter([ sanitizers.nearby.middleware, + middleware.requestLanguage, middleware.calcSize(), controllers.search(peliasConfig.api, esclient, queries.reverse, not(hasResponseDataOrRequestErrors)), postProc.distances('point.'), @@ -202,6 +208,7 @@ function addRoutes(app, peliasConfig) { ]), place: createRouter([ sanitizers.place.middleware, + middleware.requestLanguage, controllers.place(peliasConfig.api, esclient), postProc.accuracy(), postProc.localNamingConventions(), From 7749be549e4ae598cacaf955ebaf48d1dbe007b2 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 20 Mar 2017 17:42:56 +0100 Subject: [PATCH 16/36] add ciao tests for language --- .../ciao/autocomplete/language_default.coffee | 37 ++++++++++++++++++ .../language_header_invalid.coffee | 38 +++++++++++++++++++ .../autocomplete/language_header_valid.coffee | 38 +++++++++++++++++++ test/ciao/place/language_default.coffee | 37 ++++++++++++++++++ .../ciao/place/language_header_invalid.coffee | 38 +++++++++++++++++++ test/ciao/place/language_header_valid.coffee | 38 +++++++++++++++++++ test/ciao/reverse/language_default.coffee | 37 ++++++++++++++++++ .../reverse/language_header_invalid.coffee | 38 +++++++++++++++++++ .../ciao/reverse/language_header_valid.coffee | 38 +++++++++++++++++++ test/ciao/search/language_default.coffee | 37 ++++++++++++++++++ .../search/language_header_invalid.coffee | 38 +++++++++++++++++++ test/ciao/search/language_header_valid.coffee | 38 +++++++++++++++++++ 12 files changed, 452 insertions(+) create mode 100644 test/ciao/autocomplete/language_default.coffee create mode 100644 test/ciao/autocomplete/language_header_invalid.coffee create mode 100644 test/ciao/autocomplete/language_header_valid.coffee create mode 100644 test/ciao/place/language_default.coffee create mode 100644 test/ciao/place/language_header_invalid.coffee create mode 100644 test/ciao/place/language_header_valid.coffee create mode 100644 test/ciao/reverse/language_default.coffee create mode 100644 test/ciao/reverse/language_header_invalid.coffee create mode 100644 test/ciao/reverse/language_header_valid.coffee create mode 100644 test/ciao/search/language_default.coffee create mode 100644 test/ciao/search/language_header_invalid.coffee create mode 100644 test/ciao/search/language_header_valid.coffee diff --git a/test/ciao/autocomplete/language_default.coffee b/test/ciao/autocomplete/language_default.coffee new file mode 100644 index 00000000..3e39e8db --- /dev/null +++ b/test/ciao/autocomplete/language_default.coffee @@ -0,0 +1,37 @@ + +#> language +path: '/v1/autocomplete?text=example' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + name: 'English', + iso6391: 'en', + iso6393: 'eng', + defaulted: true +} diff --git a/test/ciao/autocomplete/language_header_invalid.coffee b/test/ciao/autocomplete/language_header_invalid.coffee new file mode 100644 index 00000000..0e9e8e41 --- /dev/null +++ b/test/ciao/autocomplete/language_header_invalid.coffee @@ -0,0 +1,38 @@ + +#> language +path: '/v1/autocomplete?text=example' +headers: 'Accept-Language': 'example' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + name: 'English', + iso6391: 'en', + iso6393: 'eng', + defaulted: true +} diff --git a/test/ciao/autocomplete/language_header_valid.coffee b/test/ciao/autocomplete/language_header_valid.coffee new file mode 100644 index 00000000..aea8c28c --- /dev/null +++ b/test/ciao/autocomplete/language_header_valid.coffee @@ -0,0 +1,38 @@ + +#> language +path: '/v1/autocomplete?text=example' +headers: 'Accept-Language': 'fr-CH, fr;q=0.9, en;q=0.8, de;q=0.7, *;q=0.5' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + defaulted: false, + iso6391: 'fr', + iso6393: 'fra', + name: 'French' +} diff --git a/test/ciao/place/language_default.coffee b/test/ciao/place/language_default.coffee new file mode 100644 index 00000000..444b6254 --- /dev/null +++ b/test/ciao/place/language_default.coffee @@ -0,0 +1,37 @@ + +#> language +path: '/v1/place?ids=geonames:venue:1' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + name: 'English', + iso6391: 'en', + iso6393: 'eng', + defaulted: true +} diff --git a/test/ciao/place/language_header_invalid.coffee b/test/ciao/place/language_header_invalid.coffee new file mode 100644 index 00000000..94cd167a --- /dev/null +++ b/test/ciao/place/language_header_invalid.coffee @@ -0,0 +1,38 @@ + +#> language +path: '/v1/place?ids=geonames:venue:1' +headers: 'Accept-Language': 'example' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + name: 'English', + iso6391: 'en', + iso6393: 'eng', + defaulted: true +} diff --git a/test/ciao/place/language_header_valid.coffee b/test/ciao/place/language_header_valid.coffee new file mode 100644 index 00000000..639d3b72 --- /dev/null +++ b/test/ciao/place/language_header_valid.coffee @@ -0,0 +1,38 @@ + +#> language +path: '/v1/place?ids=geonames:venue:1' +headers: 'Accept-Language': 'fr-CH, fr;q=0.9, en;q=0.8, de;q=0.7, *;q=0.5' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + defaulted: false, + iso6391: 'fr', + iso6393: 'fra', + name: 'French' +} diff --git a/test/ciao/reverse/language_default.coffee b/test/ciao/reverse/language_default.coffee new file mode 100644 index 00000000..659aa5c2 --- /dev/null +++ b/test/ciao/reverse/language_default.coffee @@ -0,0 +1,37 @@ + +#> language +path: '/v1/reverse?point.lat=1&point.lon=2' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + name: 'English', + iso6391: 'en', + iso6393: 'eng', + defaulted: true +} diff --git a/test/ciao/reverse/language_header_invalid.coffee b/test/ciao/reverse/language_header_invalid.coffee new file mode 100644 index 00000000..635de4e1 --- /dev/null +++ b/test/ciao/reverse/language_header_invalid.coffee @@ -0,0 +1,38 @@ + +#> language +path: '/v1/reverse?point.lat=1&point.lon=2' +headers: 'Accept-Language': 'example' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + name: 'English', + iso6391: 'en', + iso6393: 'eng', + defaulted: true +} diff --git a/test/ciao/reverse/language_header_valid.coffee b/test/ciao/reverse/language_header_valid.coffee new file mode 100644 index 00000000..bca45c1a --- /dev/null +++ b/test/ciao/reverse/language_header_valid.coffee @@ -0,0 +1,38 @@ + +#> language +path: '/v1/reverse?point.lat=1&point.lon=2' +headers: 'Accept-Language': 'fr-CH, fr;q=0.9, en;q=0.8, de;q=0.7, *;q=0.5' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + defaulted: false, + iso6391: 'fr', + iso6393: 'fra', + name: 'French' +} diff --git a/test/ciao/search/language_default.coffee b/test/ciao/search/language_default.coffee new file mode 100644 index 00000000..eb85265c --- /dev/null +++ b/test/ciao/search/language_default.coffee @@ -0,0 +1,37 @@ + +#> language +path: '/v1/search?text=example' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + name: 'English', + iso6391: 'en', + iso6393: 'eng', + defaulted: true +} diff --git a/test/ciao/search/language_header_invalid.coffee b/test/ciao/search/language_header_invalid.coffee new file mode 100644 index 00000000..af89aaf0 --- /dev/null +++ b/test/ciao/search/language_header_invalid.coffee @@ -0,0 +1,38 @@ + +#> language +path: '/v1/search?text=example' +headers: 'Accept-Language': 'example' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + name: 'English', + iso6391: 'en', + iso6393: 'eng', + defaulted: true +} diff --git a/test/ciao/search/language_header_valid.coffee b/test/ciao/search/language_header_valid.coffee new file mode 100644 index 00000000..4f3b958d --- /dev/null +++ b/test/ciao/search/language_header_valid.coffee @@ -0,0 +1,38 @@ + +#> language +path: '/v1/search?text=example' +headers: 'Accept-Language': 'fr-CH, fr;q=0.9, en;q=0.8, de;q=0.7, *;q=0.5' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + defaulted: false, + iso6391: 'fr', + iso6393: 'fra', + name: 'French' +} From 1847fb369217e4c6a00017d9762dea89696b5689 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 20 Mar 2017 17:55:04 +0100 Subject: [PATCH 17/36] add unit tests for language --- test/unit/middleware/requestLanguage.js | 97 +++++++++++++++++++++++++ test/unit/run.js | 1 + 2 files changed, 98 insertions(+) create mode 100644 test/unit/middleware/requestLanguage.js diff --git a/test/unit/middleware/requestLanguage.js b/test/unit/middleware/requestLanguage.js new file mode 100644 index 00000000..aa68b440 --- /dev/null +++ b/test/unit/middleware/requestLanguage.js @@ -0,0 +1,97 @@ + +var middleware = require('../../../middleware/requestLanguage'); +module.exports.tests = {}; + +var DEFAULTS = { + defaulted: true, + iso6391: 'en', + iso6392B: 'eng', + iso6392T: 'eng', + iso6393: 'eng', + name: 'English', + scope: 'individual', + type: 'living' +}; + +module.exports.tests.defaults = function(test, common) { + test('default language', function(t) { + + var req = { headers: {} }; + + middleware(req, {}, function () { + t.deepEqual( req.language, DEFAULTS, '$req.language set' ); + t.end(); + }); + }); +}; + +module.exports.tests.invalid = function(test, common) { + test('headers: invalid language', function(t) { + + var req = { headers: { + 'accept-language': 'invalid language' + }}; + + middleware(req, {}, function () { + t.deepEqual( req.language, DEFAULTS, '$req.language set' ); + t.end(); + }); + }); +}; + +module.exports.tests.valid = function(test, common) { + test('headers: valid language - french', function(t) { + + var req = { headers: { + 'accept-language': 'fr-CH, fr;q=0.9, en;q=0.8, de;q=0.7, *;q=0.5' + }}; + + var expected = { + defaulted: false, + iso6391: 'fr', + iso6392B: 'fre', + iso6392T: 'fra', + iso6393: 'fra', + name: 'French', + scope: 'individual', + type: 'living' + }; + + middleware(req, {}, function () { + t.deepEqual( req.language, expected, '$req.language set' ); + t.end(); + }); + }); + + test('headers: valid language - english', function(t) { + + var req = { headers: { + 'accept-language': 'en' + }}; + + var expected = { + defaulted: false, + iso6391: 'en', + iso6392B: 'eng', + iso6392T: 'eng', + iso6393: 'eng', + name: 'English', + scope: 'individual', + type: 'living' + }; + + middleware(req, {}, function () { + t.deepEqual( req.language, expected, '$req.language set' ); + t.end(); + }); + }); +}; + +module.exports.all = function (tape, common) { + function test(name, testFunction) { + return tape('[middleware] requestLanguage: ' + name, testFunction); + } + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/run.js b/test/unit/run.js index a16fb18e..ad358e30 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -39,6 +39,7 @@ var tests = [ require('./middleware/normalizeParentIds'), require('./middleware/trimByGranularity'), require('./middleware/trimByGranularityStructured'), + require('./middleware/requestLanguage'), require('./query/autocomplete'), require('./query/autocomplete_defaults'), require('./query/search_defaults'), From e761f9942239167592ae31d658ad41a5eda87db1 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 20 Mar 2017 17:59:17 +0100 Subject: [PATCH 18/36] add unit tests for language --- test/unit/middleware/requestLanguage.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/unit/middleware/requestLanguage.js b/test/unit/middleware/requestLanguage.js index aa68b440..3528ad6c 100644 --- a/test/unit/middleware/requestLanguage.js +++ b/test/unit/middleware/requestLanguage.js @@ -20,6 +20,14 @@ module.exports.tests.defaults = function(test, common) { middleware(req, {}, function () { t.deepEqual( req.language, DEFAULTS, '$req.language set' ); + + t.deepEqual( req.clean.lang, { + defaulted: req.language.defaulted, + iso6391: req.language.iso6391, + iso6393: req.language.iso6393, + name: req.language.name + }, '$req.clean.lang set' ); + t.end(); }); }); @@ -34,6 +42,14 @@ module.exports.tests.invalid = function(test, common) { middleware(req, {}, function () { t.deepEqual( req.language, DEFAULTS, '$req.language set' ); + + t.deepEqual( req.clean.lang, { + defaulted: req.language.defaulted, + iso6391: req.language.iso6391, + iso6393: req.language.iso6393, + name: req.language.name + }, '$req.clean.lang set' ); + t.end(); }); }); @@ -59,6 +75,14 @@ module.exports.tests.valid = function(test, common) { middleware(req, {}, function () { t.deepEqual( req.language, expected, '$req.language set' ); + + t.deepEqual( req.clean.lang, { + defaulted: req.language.defaulted, + iso6391: req.language.iso6391, + iso6393: req.language.iso6393, + name: req.language.name + }, '$req.clean.lang set' ); + t.end(); }); }); From 39b1367778267db7a9a807d6c0bb1030a0982b2b Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 20 Mar 2017 18:22:34 +0100 Subject: [PATCH 19/36] language: accept param as well as request headers --- middleware/requestLanguage.js | 9 +- .../language_querystring_invalid.coffee | 37 ++++++ .../language_querystring_valid.coffee | 37 ++++++ .../place/language_querystring_invalid.coffee | 37 ++++++ .../place/language_querystring_valid.coffee | 37 ++++++ .../language_querystring_invalid.coffee | 37 ++++++ .../reverse/language_querystring_valid.coffee | 37 ++++++ .../language_querystring_invalid.coffee | 37 ++++++ .../search/language_querystring_valid.coffee | 37 ++++++ test/unit/middleware/requestLanguage.js | 107 +++++++++++++++++- 10 files changed, 408 insertions(+), 4 deletions(-) create mode 100644 test/ciao/autocomplete/language_querystring_invalid.coffee create mode 100644 test/ciao/autocomplete/language_querystring_valid.coffee create mode 100644 test/ciao/place/language_querystring_invalid.coffee create mode 100644 test/ciao/place/language_querystring_valid.coffee create mode 100644 test/ciao/reverse/language_querystring_invalid.coffee create mode 100644 test/ciao/reverse/language_querystring_valid.coffee create mode 100644 test/ciao/search/language_querystring_invalid.coffee create mode 100644 test/ciao/search/language_querystring_valid.coffee diff --git a/middleware/requestLanguage.js b/middleware/requestLanguage.js index ebe21b9a..c3083d46 100644 --- a/middleware/requestLanguage.js +++ b/middleware/requestLanguage.js @@ -1,10 +1,10 @@ /** this middleware is responsible for negotiating HTTP locales for incoming - browser requests by reading 'Accept-Language' request headers. + browser requests by reading the querystring param 'lang' or 'Accept-Language' request headers. the preferred language will then be available on the $req object: - eg. for 'Accept-Language: fr': + eg. for '?lang=fr' or 'Accept-Language: fr': ``` console.log( req.language ); @@ -53,8 +53,11 @@ const allLocales = new locale.Locales( Object.keys( language ) ); // return the middleware module.exports = function middleware( req, res, next ){ + // input language, either from query param or header + var input = ( req.query && req.query.lang ) || ( req.headers && req.headers['accept-language'] ); + // parse request & choose best locale - var locales = new locale.Locales( req.headers['accept-language'] || '' ); + var locales = new locale.Locales( input || '' ); var best = locales.best( allLocales ); // set $req.language property diff --git a/test/ciao/autocomplete/language_querystring_invalid.coffee b/test/ciao/autocomplete/language_querystring_invalid.coffee new file mode 100644 index 00000000..5fd866fc --- /dev/null +++ b/test/ciao/autocomplete/language_querystring_invalid.coffee @@ -0,0 +1,37 @@ + +#> language +path: '/v1/autocomplete?lang=example&text=example' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + name: 'English', + iso6391: 'en', + iso6393: 'eng', + defaulted: true +} diff --git a/test/ciao/autocomplete/language_querystring_valid.coffee b/test/ciao/autocomplete/language_querystring_valid.coffee new file mode 100644 index 00000000..4b267ff2 --- /dev/null +++ b/test/ciao/autocomplete/language_querystring_valid.coffee @@ -0,0 +1,37 @@ + +#> language +path: '/v1/autocomplete?lang=fr&text=example' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + defaulted: false, + iso6391: 'fr', + iso6393: 'fra', + name: 'French' +} diff --git a/test/ciao/place/language_querystring_invalid.coffee b/test/ciao/place/language_querystring_invalid.coffee new file mode 100644 index 00000000..89a07045 --- /dev/null +++ b/test/ciao/place/language_querystring_invalid.coffee @@ -0,0 +1,37 @@ + +#> language +path: '/v1/place?lang=example&ids=geonames:venue:1' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + name: 'English', + iso6391: 'en', + iso6393: 'eng', + defaulted: true +} diff --git a/test/ciao/place/language_querystring_valid.coffee b/test/ciao/place/language_querystring_valid.coffee new file mode 100644 index 00000000..3192e754 --- /dev/null +++ b/test/ciao/place/language_querystring_valid.coffee @@ -0,0 +1,37 @@ + +#> language +path: '/v1/place?lang=fr&ids=geonames:venue:1' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + defaulted: false, + iso6391: 'fr', + iso6393: 'fra', + name: 'French' +} diff --git a/test/ciao/reverse/language_querystring_invalid.coffee b/test/ciao/reverse/language_querystring_invalid.coffee new file mode 100644 index 00000000..6c8130b2 --- /dev/null +++ b/test/ciao/reverse/language_querystring_invalid.coffee @@ -0,0 +1,37 @@ + +#> language +path: '/v1/reverse?lang=example&point.lat=1&point.lon=2' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + name: 'English', + iso6391: 'en', + iso6393: 'eng', + defaulted: true +} diff --git a/test/ciao/reverse/language_querystring_valid.coffee b/test/ciao/reverse/language_querystring_valid.coffee new file mode 100644 index 00000000..45f22e1b --- /dev/null +++ b/test/ciao/reverse/language_querystring_valid.coffee @@ -0,0 +1,37 @@ + +#> language +path: '/v1/reverse?lang=fr&point.lat=1&point.lon=2' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + defaulted: false, + iso6391: 'fr', + iso6393: 'fra', + name: 'French' +} diff --git a/test/ciao/search/language_querystring_invalid.coffee b/test/ciao/search/language_querystring_invalid.coffee new file mode 100644 index 00000000..c1798bf5 --- /dev/null +++ b/test/ciao/search/language_querystring_invalid.coffee @@ -0,0 +1,37 @@ + +#> language +path: '/v1/search?lang=example&text=example' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + name: 'English', + iso6391: 'en', + iso6393: 'eng', + defaulted: true +} diff --git a/test/ciao/search/language_querystring_valid.coffee b/test/ciao/search/language_querystring_valid.coffee new file mode 100644 index 00000000..8b389efd --- /dev/null +++ b/test/ciao/search/language_querystring_valid.coffee @@ -0,0 +1,37 @@ + +#> language +path: '/v1/search?lang=fr&text=example' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? language +json.geocoding.query['lang'].should.eql { + defaulted: false, + iso6391: 'fr', + iso6393: 'fra', + name: 'French' +} diff --git a/test/unit/middleware/requestLanguage.js b/test/unit/middleware/requestLanguage.js index 3528ad6c..a8d9e9a7 100644 --- a/test/unit/middleware/requestLanguage.js +++ b/test/unit/middleware/requestLanguage.js @@ -16,7 +16,7 @@ var DEFAULTS = { module.exports.tests.defaults = function(test, common) { test('default language', function(t) { - var req = { headers: {} }; + var req = {}; middleware(req, {}, function () { t.deepEqual( req.language, DEFAULTS, '$req.language set' ); @@ -40,6 +40,25 @@ module.exports.tests.invalid = function(test, common) { 'accept-language': 'invalid language' }}; + middleware(req, {}, function () { + t.deepEqual( req.language, DEFAULTS, '$req.language set' ); + + t.deepEqual( req.clean.lang, { + defaulted: req.language.defaulted, + iso6391: req.language.iso6391, + iso6393: req.language.iso6393, + name: req.language.name + }, '$req.clean.lang set' ); + + t.end(); + }); + }); + test('query: invalid language', function(t) { + + var req = { query: { + lang: 'invalid language' + }}; + middleware(req, {}, function () { t.deepEqual( req.language, DEFAULTS, '$req.language set' ); @@ -86,6 +105,36 @@ module.exports.tests.valid = function(test, common) { t.end(); }); }); + test('query: valid language - french', function(t) { + + var req = { query: { + lang: 'fr-CH, fr;q=0.9, en;q=0.8, de;q=0.7, *;q=0.5' + }}; + + var expected = { + defaulted: false, + iso6391: 'fr', + iso6392B: 'fre', + iso6392T: 'fra', + iso6393: 'fra', + name: 'French', + scope: 'individual', + type: 'living' + }; + + middleware(req, {}, function () { + t.deepEqual( req.language, expected, '$req.language set' ); + + t.deepEqual( req.clean.lang, { + defaulted: req.language.defaulted, + iso6391: req.language.iso6391, + iso6393: req.language.iso6393, + name: req.language.name + }, '$req.clean.lang set' ); + + t.end(); + }); + }); test('headers: valid language - english', function(t) { @@ -109,6 +158,62 @@ module.exports.tests.valid = function(test, common) { t.end(); }); }); + test('query: valid language - english', function(t) { + + var req = { query: { + lang: 'en' + }}; + + var expected = { + defaulted: false, + iso6391: 'en', + iso6392B: 'eng', + iso6392T: 'eng', + iso6393: 'eng', + name: 'English', + scope: 'individual', + type: 'living' + }; + + middleware(req, {}, function () { + t.deepEqual( req.language, expected, '$req.language set' ); + t.end(); + }); + }); +}; + +module.exports.tests.precedence = function(test, common) { + test('precedence: query has precedence over headers', function(t) { + + var req = { + headers: { 'accept-language': 'fr' }, + query: { 'lang': 'es' } + }; + + var expected = { + defaulted: false, + iso6391: 'es', + iso6392B: 'spa', + iso6392T: 'spa', + iso6393: 'spa', + name: 'Spanish', + scope: 'individual', + type: 'living' + }; + + middleware(req, {}, function () { + t.deepEqual( req.language, expected, '$req.language set' ); + + t.deepEqual( req.clean.lang, { + defaulted: req.language.defaulted, + iso6391: req.language.iso6391, + iso6393: req.language.iso6393, + name: req.language.name + }, '$req.clean.lang set' ); + + t.end(); + }); + }); }; module.exports.all = function (tape, common) { From 43131248b7d0d010605a9587afe6a3f816618870 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Wed, 22 Mar 2017 14:51:57 +0100 Subject: [PATCH 20/36] language: fallback scenario from invalid querystring to valid header --- middleware/requestLanguage.js | 45 +++++++++--- test/unit/middleware/requestLanguage.js | 96 +++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 8 deletions(-) diff --git a/middleware/requestLanguage.js b/middleware/requestLanguage.js index c3083d46..92d599c1 100644 --- a/middleware/requestLanguage.js +++ b/middleware/requestLanguage.js @@ -1,4 +1,6 @@ +const _ = require('lodash'); + /** this middleware is responsible for negotiating HTTP locales for incoming browser requests by reading the querystring param 'lang' or 'Accept-Language' request headers. @@ -53,19 +55,46 @@ const allLocales = new locale.Locales( Object.keys( language ) ); // return the middleware module.exports = function middleware( req, res, next ){ - // input language, either from query param or header - var input = ( req.query && req.query.lang ) || ( req.headers && req.headers['accept-language'] ); + // init an object to store clean (sanitized) input parameters if not initialized + req.clean = req.clean || {}; + + // init warnings array if not initialized + req.warnings = req.warnings || []; + + // set defaults + var lang = language.en; + var isDefault = true; + var locales, best; + + // input language via query param + if( isDefault && req.query && req.query.lang ){ + locales = new locale.Locales( req.query.lang ); + best = locales.best( allLocales ); + if( best.defaulted ){ + req.warnings.push( 'invalid language provided via querystring' ); + } else { + lang = language[ best.language ]; + isDefault = false; + } + } - // parse request & choose best locale - var locales = new locale.Locales( input || '' ); - var best = locales.best( allLocales ); + // input language via request headers + if( isDefault && req.headers && req.headers['accept-language'] ){ + locales = new locale.Locales( req.headers['accept-language'] ); + best = locales.best( allLocales ); + if( best.defaulted ){ + req.warnings.push( 'invalid language provided via header' ); + } else { + lang = language[ best.language ]; + isDefault = false; + } + } // set $req.language property - req.language = language[ best.language ] || language.en; - req.language.defaulted = best.defaulted; + req.language = _.clone( lang ); + req.language.defaulted = isDefault; // set $req.clean property in order to print language info in response header - req.clean = req.clean || {}; req.clean.lang = { name: req.language.name, iso6391: req.language.iso6391, diff --git a/test/unit/middleware/requestLanguage.js b/test/unit/middleware/requestLanguage.js index a8d9e9a7..6252e589 100644 --- a/test/unit/middleware/requestLanguage.js +++ b/test/unit/middleware/requestLanguage.js @@ -28,6 +28,33 @@ module.exports.tests.defaults = function(test, common) { name: req.language.name }, '$req.clean.lang set' ); + t.deepEqual( req.warnings, []); + + t.end(); + }); + }); + test('both querystring & header invalid', function(t) { + + var req = { + headers: { 'accept-language': 'foobar' }, + query: { 'lang': 'foobar' } + }; + + middleware(req, {}, function () { + t.deepEqual( req.language, DEFAULTS, '$req.language set' ); + + t.deepEqual( req.clean.lang, { + defaulted: req.language.defaulted, + iso6391: req.language.iso6391, + iso6393: req.language.iso6393, + name: req.language.name + }, '$req.clean.lang set' ); + + t.deepEqual( req.warnings, [ + 'invalid language provided via querystring', + 'invalid language provided via header' + ]); + t.end(); }); }); @@ -50,6 +77,10 @@ module.exports.tests.invalid = function(test, common) { name: req.language.name }, '$req.clean.lang set' ); + t.deepEqual( req.warnings, [ + 'invalid language provided via header' + ]); + t.end(); }); }); @@ -69,6 +100,10 @@ module.exports.tests.invalid = function(test, common) { name: req.language.name }, '$req.clean.lang set' ); + t.deepEqual( req.warnings, [ + 'invalid language provided via querystring' + ]); + t.end(); }); }); @@ -102,6 +137,8 @@ module.exports.tests.valid = function(test, common) { name: req.language.name }, '$req.clean.lang set' ); + t.deepEqual( req.warnings, []); + t.end(); }); }); @@ -132,6 +169,8 @@ module.exports.tests.valid = function(test, common) { name: req.language.name }, '$req.clean.lang set' ); + t.deepEqual( req.warnings, []); + t.end(); }); }); @@ -155,6 +194,16 @@ module.exports.tests.valid = function(test, common) { middleware(req, {}, function () { t.deepEqual( req.language, expected, '$req.language set' ); + + t.deepEqual( req.clean.lang, { + defaulted: req.language.defaulted, + iso6391: req.language.iso6391, + iso6393: req.language.iso6393, + name: req.language.name + }, '$req.clean.lang set' ); + + t.deepEqual( req.warnings, []); + t.end(); }); }); @@ -177,6 +226,16 @@ module.exports.tests.valid = function(test, common) { middleware(req, {}, function () { t.deepEqual( req.language, expected, '$req.language set' ); + + t.deepEqual( req.clean.lang, { + defaulted: req.language.defaulted, + iso6391: req.language.iso6391, + iso6393: req.language.iso6393, + name: req.language.name + }, '$req.clean.lang set' ); + + t.deepEqual( req.warnings, []); + t.end(); }); }); @@ -211,6 +270,43 @@ module.exports.tests.precedence = function(test, common) { name: req.language.name }, '$req.clean.lang set' ); + t.deepEqual( req.warnings, []); + + t.end(); + }); + }); + test('precedence: invalid querystring but valid header', function(t) { + + var req = { + headers: { 'accept-language': 'fr' }, + query: { 'lang': 'foobar' } + }; + + var expected = { + defaulted: false, + iso6391: 'fr', + iso6392B: 'fre', + iso6392T: 'fra', + iso6393: 'fra', + name: 'French', + scope: 'individual', + type: 'living' + }; + + middleware(req, {}, function () { + t.deepEqual( req.language, expected, '$req.language set' ); + + t.deepEqual( req.clean.lang, { + defaulted: req.language.defaulted, + iso6391: req.language.iso6391, + iso6393: req.language.iso6393, + name: req.language.name + }, '$req.clean.lang set' ); + + t.deepEqual( req.warnings, [ + 'invalid language provided via querystring' + ]); + t.end(); }); }); From af4b107612f43c24d91269508f3c52943083a969 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Wed, 22 Mar 2017 14:58:59 +0100 Subject: [PATCH 21/36] language: update ciao tests to assert warnings produced for invalid inputs --- test/ciao/autocomplete/language_header_invalid.coffee | 2 +- test/ciao/autocomplete/language_querystring_invalid.coffee | 2 +- test/ciao/place/language_header_invalid.coffee | 2 +- test/ciao/place/language_querystring_invalid.coffee | 2 +- test/ciao/reverse/language_header_invalid.coffee | 2 +- test/ciao/reverse/language_querystring_invalid.coffee | 2 +- test/ciao/search/language_header_invalid.coffee | 2 +- test/ciao/search/language_querystring_invalid.coffee | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/ciao/autocomplete/language_header_invalid.coffee b/test/ciao/autocomplete/language_header_invalid.coffee index 0e9e8e41..56941e3c 100644 --- a/test/ciao/autocomplete/language_header_invalid.coffee +++ b/test/ciao/autocomplete/language_header_invalid.coffee @@ -27,7 +27,7 @@ json.features.should.be.instanceof Array should.not.exist json.geocoding.errors #? expected warnings -should.not.exist json.geocoding.warnings +json.geocoding.warnings.should.eql [ 'invalid language provided via header' ] #? language json.geocoding.query['lang'].should.eql { diff --git a/test/ciao/autocomplete/language_querystring_invalid.coffee b/test/ciao/autocomplete/language_querystring_invalid.coffee index 5fd866fc..65ead1fc 100644 --- a/test/ciao/autocomplete/language_querystring_invalid.coffee +++ b/test/ciao/autocomplete/language_querystring_invalid.coffee @@ -26,7 +26,7 @@ json.features.should.be.instanceof Array should.not.exist json.geocoding.errors #? expected warnings -should.not.exist json.geocoding.warnings +json.geocoding.warnings.should.eql [ 'invalid language provided via querystring' ] #? language json.geocoding.query['lang'].should.eql { diff --git a/test/ciao/place/language_header_invalid.coffee b/test/ciao/place/language_header_invalid.coffee index 94cd167a..142464d6 100644 --- a/test/ciao/place/language_header_invalid.coffee +++ b/test/ciao/place/language_header_invalid.coffee @@ -27,7 +27,7 @@ json.features.should.be.instanceof Array should.not.exist json.geocoding.errors #? expected warnings -should.not.exist json.geocoding.warnings +json.geocoding.warnings.should.eql [ 'invalid language provided via header' ] #? language json.geocoding.query['lang'].should.eql { diff --git a/test/ciao/place/language_querystring_invalid.coffee b/test/ciao/place/language_querystring_invalid.coffee index 89a07045..3783aa4c 100644 --- a/test/ciao/place/language_querystring_invalid.coffee +++ b/test/ciao/place/language_querystring_invalid.coffee @@ -26,7 +26,7 @@ json.features.should.be.instanceof Array should.not.exist json.geocoding.errors #? expected warnings -should.not.exist json.geocoding.warnings +json.geocoding.warnings.should.eql [ 'invalid language provided via querystring' ] #? language json.geocoding.query['lang'].should.eql { diff --git a/test/ciao/reverse/language_header_invalid.coffee b/test/ciao/reverse/language_header_invalid.coffee index 635de4e1..35cd83d1 100644 --- a/test/ciao/reverse/language_header_invalid.coffee +++ b/test/ciao/reverse/language_header_invalid.coffee @@ -27,7 +27,7 @@ json.features.should.be.instanceof Array should.not.exist json.geocoding.errors #? expected warnings -should.not.exist json.geocoding.warnings +json.geocoding.warnings.should.eql [ 'invalid language provided via header' ] #? language json.geocoding.query['lang'].should.eql { diff --git a/test/ciao/reverse/language_querystring_invalid.coffee b/test/ciao/reverse/language_querystring_invalid.coffee index 6c8130b2..af1bff84 100644 --- a/test/ciao/reverse/language_querystring_invalid.coffee +++ b/test/ciao/reverse/language_querystring_invalid.coffee @@ -26,7 +26,7 @@ json.features.should.be.instanceof Array should.not.exist json.geocoding.errors #? expected warnings -should.not.exist json.geocoding.warnings +json.geocoding.warnings.should.eql [ 'invalid language provided via querystring' ] #? language json.geocoding.query['lang'].should.eql { diff --git a/test/ciao/search/language_header_invalid.coffee b/test/ciao/search/language_header_invalid.coffee index af89aaf0..0035956c 100644 --- a/test/ciao/search/language_header_invalid.coffee +++ b/test/ciao/search/language_header_invalid.coffee @@ -27,7 +27,7 @@ json.features.should.be.instanceof Array should.not.exist json.geocoding.errors #? expected warnings -should.not.exist json.geocoding.warnings +json.geocoding.warnings.should.eql [ 'invalid language provided via header' ] #? language json.geocoding.query['lang'].should.eql { diff --git a/test/ciao/search/language_querystring_invalid.coffee b/test/ciao/search/language_querystring_invalid.coffee index c1798bf5..cdfe0195 100644 --- a/test/ciao/search/language_querystring_invalid.coffee +++ b/test/ciao/search/language_querystring_invalid.coffee @@ -26,7 +26,7 @@ json.features.should.be.instanceof Array should.not.exist json.geocoding.errors #? expected warnings -should.not.exist json.geocoding.warnings +json.geocoding.warnings.should.eql [ 'invalid language provided via querystring' ] #? language json.geocoding.query['lang'].should.eql { From 6fcc0f6826d402ba5adad92f4e6ef12a8942c59d Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Wed, 22 Mar 2017 16:32:38 +0100 Subject: [PATCH 22/36] language: logging --- middleware/requestLanguage.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/middleware/requestLanguage.js b/middleware/requestLanguage.js index 92d599c1..c7532fc9 100644 --- a/middleware/requestLanguage.js +++ b/middleware/requestLanguage.js @@ -1,5 +1,6 @@ const _ = require('lodash'); +const logger = require( 'pelias-logger' ).get( 'api' ); /** this middleware is responsible for negotiating HTTP locales for incoming @@ -63,36 +64,35 @@ module.exports = function middleware( req, res, next ){ // set defaults var lang = language.en; - var isDefault = true; - var locales, best; + var locales, best, via = 'default'; // input language via query param - if( isDefault && req.query && req.query.lang ){ + if( via === 'default' && req.query && req.query.lang ){ locales = new locale.Locales( req.query.lang ); best = locales.best( allLocales ); if( best.defaulted ){ req.warnings.push( 'invalid language provided via querystring' ); } else { lang = language[ best.language ]; - isDefault = false; + via = 'querystring'; } } // input language via request headers - if( isDefault && req.headers && req.headers['accept-language'] ){ + if( via === 'default' && req.headers && req.headers['accept-language'] ){ locales = new locale.Locales( req.headers['accept-language'] ); best = locales.best( allLocales ); if( best.defaulted ){ req.warnings.push( 'invalid language provided via header' ); } else { lang = language[ best.language ]; - isDefault = false; + via = 'header'; } } // set $req.language property req.language = _.clone( lang ); - req.language.defaulted = isDefault; + req.language.defaulted = ( via === 'default' ); // set $req.clean property in order to print language info in response header req.clean.lang = { @@ -102,5 +102,8 @@ module.exports = function middleware( req, res, next ){ defaulted: req.language.defaulted }; + // logging + logger.info( '[lang] \'%s\' via \'%s\'', lang.iso6391, via ); + next(); }; From 15db2c7ead4caa96741aa9a822848c02334e1af4 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 22 Mar 2017 15:09:55 -0400 Subject: [PATCH 23/36] Move request to dependencies It's now required for the pip-service, not just tests. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 9b0b32ea..98ad08e0 100644 --- a/package.json +++ b/package.json @@ -64,6 +64,7 @@ "pelias-text-analyzer": "1.7.2", "predicates": "^1.0.1", "retry": "^0.10.1", + "request": "^2.79.0", "stats-lite": "^2.0.4", "superagent": "^3.2.1", "through2": "^2.0.3" @@ -76,7 +77,6 @@ "nsp": "^2.2.0", "precommit-hook": "^3.0.0", "proxyquire": "^1.7.10", - "request": "^2.79.0", "semantic-release": "^6.3.2", "source-map": "^0.5.6", "tap-dot": "1.0.5", From 87c939d47bfc60a642e40e927617b5ac887060da Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Tue, 28 Mar 2017 20:26:28 +0000 Subject: [PATCH 24/36] fix(package): update pelias-labels to version 1.6.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 98ad08e0..fdb0eeac 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ "morgan": "1.8.1", "pelias-categories": "1.1.0", "pelias-config": "2.8.0", - "pelias-labels": "1.5.1", + "pelias-labels": "1.6.0", "pelias-logger": "0.1.0", "pelias-mock-logger": "^1.0.1", "pelias-model": "4.5.1", From ebd0cf60954a6ab5d7a75c01a29e05e84a4d7a73 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Tue, 28 Mar 2017 20:29:34 +0000 Subject: [PATCH 25/36] fix(package): update pelias-config to version 2.9.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 98ad08e0..5e2e5ffd 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,7 @@ "markdown": "0.5.0", "morgan": "1.8.1", "pelias-categories": "1.1.0", - "pelias-config": "2.8.0", + "pelias-config": "2.9.0", "pelias-labels": "1.5.1", "pelias-logger": "0.1.0", "pelias-mock-logger": "^1.0.1", From ce9903ac9b2a12701c6d0fc4421db22138ad186b Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Wed, 29 Mar 2017 20:19:23 +0000 Subject: [PATCH 26/36] fix(package): update pelias-text-analyzer to version 1.7.3 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 98ad08e0..3350b48f 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,7 @@ "pelias-mock-logger": "^1.0.1", "pelias-model": "4.5.1", "pelias-query": "8.14.0", - "pelias-text-analyzer": "1.7.2", + "pelias-text-analyzer": "1.7.3", "predicates": "^1.0.1", "retry": "^0.10.1", "request": "^2.79.0", From 8d774ab6d3734432b4aff4855645cb24f9fabee3 Mon Sep 17 00:00:00 2001 From: missinglink Date: Mon, 3 Apr 2017 14:56:02 +0200 Subject: [PATCH 27/36] language service --- middleware/changeLanguage.js | 172 ++++++++++++++++++++ routes/v1.js | 9 +- service/language.js | 93 +++++++++++ test/unit/middleware/changeLanguage.js | 216 +++++++++++++++++++++++++ test/unit/run.js | 4 +- test/unit/service/language.js | 107 ++++++++++++ 6 files changed, 599 insertions(+), 2 deletions(-) create mode 100644 middleware/changeLanguage.js create mode 100644 service/language.js create mode 100644 test/unit/middleware/changeLanguage.js create mode 100644 test/unit/service/language.js diff --git a/middleware/changeLanguage.js b/middleware/changeLanguage.js new file mode 100644 index 00000000..d6449ae8 --- /dev/null +++ b/middleware/changeLanguage.js @@ -0,0 +1,172 @@ + +var logger = require( 'pelias-logger' ).get( 'api' ); +var service = require('../service/language'); + +/** +example response from language web service: +{ + "101748479": { + "wofid": 101748479, + "placetype": "locality", + "iso": "DE", + "area": 0.031614, + "lineage": { + "continent_id": 102191581, + "country_id": 85633111, + "county_id": 102063261, + "locality_id": 101748479, + "macrocounty_id": 404227567, + "region_id": 85682571 + }, + "rowid": 90425, + "names": { + "default": "München", + "eng": "Munich" + } + }, +} +**/ + +function setup() { + + var transport = service.findById(); + var middleware = function(req, res, next) { + + // no-op, request did not require a language change + if( !isLanguageChangeRequired( req, res ) ){ + return next(); + } + + // collect a list of parent ids to fetch translations for + var ids = extractIds( res ); + + // perform language lookup for all relevant ids + var timer = (new Date()).getTime(); + transport.query( ids, function( err, translations ){ + + // update documents using a translation map + if( err ){ + logger.error( '[language] [error]', err ); + } else { + updateDocs( req, res, translations ); + } + + logger.info( '[language] [took]', (new Date()).getTime() - timer, 'ms' ); + next(); + }); + }; + + middleware.transport = transport; + return middleware; +} + +// collect a list of parent ids to fetch translations for +function extractIds( res ){ + + // store ids in an object in order to avoid duplicates + var ids = {}; + + // convenience function for adding a new id to the object + function addId(id) { + ids[id] = true; + } + + // extract all parent ids from documents + res.data.forEach( function( doc ){ + + // skip invalid records + if( !doc || !doc.parent ){ return; } + + // iterate over doc.parent.* attributes + for( var attr in doc.parent ){ + + // match only attributes ending with '_id' + var match = attr.match(/_id$/); + if( !match ){ continue; } + + // skip invalid/empty arrays + if( !Array.isArray( doc.parent[attr] ) || !doc.parent[attr].length ){ + continue; + } + + // add each id as a key in the ids object + doc.parent[attr].forEach( addId ); + } + }); + + // return a deduplicated array of ids + return Object.keys( ids ); +} + +// update documents using a translation map +function updateDocs( req, res, translations ){ + + // sanity check arguments + if( !req || !res || !res.data || !translations ){ return; } + + // this is the target language we will be translating to + var requestLanguage = req.language.iso6393; + + // iterate over response documents + res.data.forEach( function( doc, p ){ + + // skip invalid records + if( !doc || !doc.parent ){ return; } + + // iterate over doc.parent.* attributes + for( var attr in doc.parent ){ + + // match only attributes ending with '_id' + var match = attr.match(/^(.*)_id$/); + if( !match ){ continue; } + + // adminKey is the property name without the '_id' + // eg. for 'country_id', adminKey would be 'country'. + var adminKey = match[1]; + var adminValues = doc.parent[adminKey]; + + // skip invalid/empty arrays + if( !Array.isArray( adminValues ) || !adminValues.length ){ continue; } + + // iterate over adminValues (it's an array and can have more than one value) + for( var i in adminValues ){ + + // find the corresponding key from the '_id' Array + var id = doc.parent[attr][i]; + if( !id ){ continue; } + + // id not found in translation service response + if( !translations.hasOwnProperty( id ) ){ + logger.error( '[language] [error]', 'failed to find translations for', id ); + continue; + } + + // skip invalid records + if( !translations[id].hasOwnProperty( 'names' ) ){ continue; } + + // requested language is not available + if( !translations[id].names.hasOwnProperty( requestLanguage ) ){ + logger.info( '[language] [info]', 'missing translation', requestLanguage, id ); + continue; + } + + // translate 'parent.*' property + adminValues[i] = translations[id].names[ requestLanguage ]; + + // if the record is an admin record we also translate + // the 'name.default' property. + if( adminKey === doc.layer ){ + doc.name.default = translations[id].names[ requestLanguage ]; + } + } + } + }); +} + +// boolean function to check if changing the language is required +function isLanguageChangeRequired( req, res ){ + return req && res && res.data && res.data.length && + req.hasOwnProperty('language'); +} + +module.exports = setup; diff --git a/routes/v1.js b/routes/v1.js index 8ee4173f..c1cc4554 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -58,7 +58,8 @@ var postProc = { sendJSON: require('../middleware/sendJSON'), parseBoundingBox: require('../middleware/parseBBox'), normalizeParentIds: require('../middleware/normalizeParentIds'), - assignLabels: require('../middleware/assignLabels') + assignLabels: require('../middleware/assignLabels'), + changeLanguage: require('../middleware/changeLanguage') }; // predicates that drive whether controller/search runs @@ -127,6 +128,7 @@ function addRoutes(app, peliasConfig) { postProc.renamePlacenames(), postProc.parseBoundingBox(), postProc.normalizeParentIds(), + postProc.changeLanguage(), postProc.assignLabels(), postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON @@ -147,6 +149,7 @@ function addRoutes(app, peliasConfig) { postProc.renamePlacenames(), postProc.parseBoundingBox(), postProc.normalizeParentIds(), + postProc.changeLanguage(), postProc.assignLabels(), postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON @@ -163,6 +166,7 @@ function addRoutes(app, peliasConfig) { postProc.renamePlacenames(), postProc.parseBoundingBox(), postProc.normalizeParentIds(), + postProc.changeLanguage(), postProc.assignLabels(), postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON @@ -183,6 +187,7 @@ function addRoutes(app, peliasConfig) { postProc.renamePlacenames(), postProc.parseBoundingBox(), postProc.normalizeParentIds(), + postProc.changeLanguage(), postProc.assignLabels(), postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON @@ -202,6 +207,7 @@ function addRoutes(app, peliasConfig) { postProc.renamePlacenames(), postProc.parseBoundingBox(), postProc.normalizeParentIds(), + postProc.changeLanguage(), postProc.assignLabels(), postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON @@ -215,6 +221,7 @@ function addRoutes(app, peliasConfig) { postProc.renamePlacenames(), postProc.parseBoundingBox(), postProc.normalizeParentIds(), + postProc.changeLanguage(), postProc.assignLabels(), postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON diff --git a/service/language.js b/service/language.js new file mode 100644 index 00000000..50cc29c0 --- /dev/null +++ b/service/language.js @@ -0,0 +1,93 @@ + +var logger = require( 'pelias-logger' ).get( 'api' ), + request = require( 'superagent' ), + peliasConfig = require( 'pelias-config' ); + +/** + + language subsitution service client + + this file provides a 'transport' which can be used to access the language + service via a network connnection. + + the exported method for this module checks pelias-config for a configuration block such as: + + "language": { + "client": { + "adapter": "http", + "host": "http://localhost:6100" + } + } + + for more info on running the service see: https://github.com/pelias/placeholder + +**/ + +/** + NullTransport + + disables the service completely +**/ +function NullTransport(){} +NullTransport.prototype.query = function( coord, number, street, cb ){ + cb(); // no-op +}; + +/** + HttpTransport + + allows the api to be used via a remote web service +**/ +function HttpTransport( host, settings ){ + this.query = function( ids, cb ){ + request + .get( host + '/parser/findbyid' ) + .set( 'Accept', 'application/json' ) + .query({ ids: Array.isArray( ids ) ? ids.join(',') : '' }) + .timeout( settings && settings.timeout || 1000 ) + .end( function( err, res ){ + if( err || !res ){ return cb( err ); } + if( 200 !== res.status ){ return cb( 'non 200 status' ); } + return cb( null, res.body ); + }); + }; +} +HttpTransport.prototype.query = function( coord, number, street, cb ){ + throw new Error( 'language: transport not connected' ); +}; + +/** + Setup + + allows instantiation of transport depending on configuration and preference +**/ +module.exports.findById = function setup(){ + + // user config + var config = peliasConfig.generate(); + + // ensure config variables set correctly + if( !config.hasOwnProperty('language') || !config.language.hasOwnProperty('client') ){ + logger.warn( 'language: configuration not found' ); + } + + // valid configuration found + else { + + // get adapter settings from config + var settings = config.language.client; + + // http adapter + if( 'http' === settings.adapter && settings.hasOwnProperty('host') ){ + logger.info( 'language: using http transport:', settings.host ); + if( settings.hasOwnProperty('timeout') ){ + return new HttpTransport( settings.host, { timeout: parseInt( settings.timeout, 10 ) } ); + } + return new HttpTransport( settings.host ); + } + } + + // default adapter + logger.info( 'language: using null transport' ); + return new NullTransport(); +}; diff --git a/test/unit/middleware/changeLanguage.js b/test/unit/middleware/changeLanguage.js new file mode 100644 index 00000000..66b6d127 --- /dev/null +++ b/test/unit/middleware/changeLanguage.js @@ -0,0 +1,216 @@ + +var fs = require('fs'), + tmp = require('tmp'), + setup = require('../../../middleware/changeLanguage'); + +// load middleware using the default pelias config +var load = function(){ + // adapter is driven by config + var tmpfile = tmp.tmpNameSync({ postfix: '.json' }); + fs.writeFileSync( tmpfile, '{}', { encoding: 'utf8' } ); + process.env.PELIAS_CONFIG = tmpfile; + var middleware = setup(); + delete process.env.PELIAS_CONFIG; + return middleware; +}; + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('valid interface', function(t) { + var middleware = load(); + t.equal(typeof middleware, 'function', 'middleware is a function'); + t.equal(middleware.length, 3, 'middleware is a function'); + t.end(); + }); +}; + +module.exports.tests.isLanguageChangeRequired = function(test, common) { + test('invalid query - null req/res', function(t) { + var middleware = load(); + middleware(null, null, t.end); + }); + + test('invalid query - no results', function(t) { + var req = { language: { iso6393: 'spa' } }; + var res = {}; + + var middleware = load(); + middleware(req, res, function(){ + t.deepEqual( req, { language: { iso6393: 'spa' } } ); + t.deepEqual( res, {} ); + t.end(); + }); + }); + + test('invalid query - empty results', function(t) { + var req = { language: { iso6393: 'spa' } }; + var res = { data: [] }; + + var middleware = load(); + middleware(req, res, function(){ + t.deepEqual( req, { language: { iso6393: 'spa' } } ); + t.deepEqual( res, { data: [] } ); + t.end(); + }); + }); + + test('invalid query - no target language', function(t) { + var req = {}; + var res = { data: [] }; + + var middleware = load(); + middleware(req, res, function(){ + t.deepEqual( req, {} ); + t.deepEqual( res, { data: [] } ); + t.end(); + }); + }); +}; + +// check the service is called and response mapped correctly +module.exports.tests.miss = function(test, common) { + test('miss', function(t) { + + var req = { language: { iso6393: 'spa' } }; + var res = { data: [ + { + layer: 'locality', + name: { default: 'London' }, + parent: { + locality_id: [ 101750367 ], + locality: [ 'London' ] + } + }, + { + layer: 'example', + name: { default: 'London' }, + parent: { + locality_id: [ 101735809 ], + locaity: [ 'London' ] + } + } + ]}; + + var middleware = load(); + + // mock out the transport + middleware.transport.query = function mock( ids, cb ){ + t.deepEqual( ids, [ '101735809', '101750367' ] ); + t.equal( typeof cb, 'function' ); + cb( 'error' ); + }; + + middleware(req, res, function(){ + t.deepEqual( res, { data: [ + { + layer: 'locality', + name: { default: 'London' }, + parent: { + locality_id: [ 101750367 ], + locality: [ 'London' ] + } + }, + { + layer: 'example', + name: { default: 'London' }, + parent: { + locality_id: [ 101735809 ], + locaity: [ 'London' ] + } + } + ]}); + t.end(); + }); + }); +}; + +// check the service is called and response mapped correctly +module.exports.tests.hit = function(test, common) { + test('hit', function(t) { + + var req = { language: { iso6393: 'spa' } }; + var res = { data: [ + { + layer: 'locality', + name: { default: 'London' }, + parent: { + locality_id: [ 101750367 ], + locality: [ 'London' ] + } + }, + { + layer: 'example', + name: { default: 'London' }, + parent: { + locality_id: [ 101735809 ], + locaity: [ 'London' ] + } + } + ]}; + + var middleware = load(); + + // mock out the transport + middleware.transport.query = function mock( ids, cb ){ + t.deepEqual( ids, [ '101735809', '101750367' ] ); + t.equal( typeof cb, 'function' ); + cb( null, { + '101750367': { + 'names': { + 'default':'London', + 'chi':'倫敦', + 'spa':'Londres', + 'eng':'London', + 'hin':'लंदन', + 'ara':'لندن', + 'por':'Londres', + 'ben':'লন্ডন', + 'rus':'Лондон', + 'jpn':'ロンドン', + 'kor':'런던' + } + }, + '101735809': { + 'names':{ + 'default':'London', + 'eng':'London' + } + } + }); + }; + + middleware(req, res, function(){ + t.deepEqual( res, { data: [ + { + layer: 'locality', + name: { default: 'Londres' }, + parent: { + locality_id: [ 101750367 ], + locality: [ 'Londres' ] + } + }, + { + layer: 'example', + name: { default: 'London' }, + parent: { + locality_id: [ 101735809 ], + locaity: [ 'London' ] + } + } + ]}); + t.end(); + }); + }); +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('[middleware] changeLanguage: ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/run.js b/test/unit/run.js index ad358e30..3c7d01e4 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -30,6 +30,7 @@ var tests = [ require('./middleware/confidenceScore'), require('./middleware/confidenceScoreFallback'), require('./middleware/confidenceScoreReverse'), + require('./middleware/changeLanguage'), require('./middleware/distance'), require('./middleware/interpolate'), require('./middleware/localNamingConventions'), @@ -80,7 +81,8 @@ var tests = [ require('./service/mget'), require('./service/search'), require('./service/interpolation'), - require('./service/pointinpolygon') + require('./service/pointinpolygon'), + require('./service/language') ]; tests.map(function(t) { diff --git a/test/unit/service/language.js b/test/unit/service/language.js new file mode 100644 index 00000000..151247fd --- /dev/null +++ b/test/unit/service/language.js @@ -0,0 +1,107 @@ + +var fs = require('fs'), + tmp = require('tmp'), + setup = require('../../../service/language').findById; + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('valid interface', function(t) { + t.equal(typeof setup, 'function', 'setup is a function'); + t.end(); + }); +}; + +// adapter factory +module.exports.tests.factory = function(test, common) { + + test('http adapter', function(t) { + var config = { language: { client: { + adapter: 'http', + host: 'http://example.com' + }}}; + + // adapter is driven by config + var tmpfile = tmp.tmpNameSync({ postfix: '.json' }); + fs.writeFileSync( tmpfile, JSON.stringify( config ), { encoding: 'utf8' } ); + process.env.PELIAS_CONFIG = tmpfile; + var adapter = setup(); + delete process.env.PELIAS_CONFIG; + + t.equal(adapter.constructor.name, 'HttpTransport', 'HttpTransport'); + t.equal(typeof adapter, 'object', 'adapter is an object'); + t.equal(typeof adapter.query, 'function', 'query is a function'); + t.equal(adapter.query.length, 2, 'query function signature'); + t.end(); + }); + + test('null adapter', function(t) { + var config = { language: { client: { + adapter: 'null' + }}}; + + // adapter is driven by config + var tmpfile = tmp.tmpNameSync({ postfix: '.json' }); + fs.writeFileSync( tmpfile, JSON.stringify( config ), { encoding: 'utf8' } ); + process.env.PELIAS_CONFIG = tmpfile; + var adapter = setup(); + delete process.env.PELIAS_CONFIG; + + t.equal(adapter.constructor.name, 'NullTransport', 'NullTransport'); + t.equal(typeof adapter, 'object', 'adapter is an object'); + t.equal(typeof adapter.query, 'function', 'query is a function'); + t.equal(adapter.query.length, 4, 'query function signature'); + t.end(); + }); + + test('default adapter', function(t) { + var config = {}; + + // adapter is driven by config + var tmpfile = tmp.tmpNameSync({ postfix: '.json' }); + fs.writeFileSync( tmpfile, JSON.stringify( config ), { encoding: 'utf8' } ); + process.env.PELIAS_CONFIG = tmpfile; + var adapter = setup(); + delete process.env.PELIAS_CONFIG; + + t.equal(adapter.constructor.name, 'NullTransport', 'NullTransport'); + t.equal(typeof adapter, 'object', 'adapter is an object'); + t.equal(typeof adapter.query, 'function', 'query is a function'); + t.equal(adapter.query.length, 4, 'query function signature'); + t.end(); + }); + +}; + +// null transport +module.exports.tests.NullTransport = function(test, common) { + + test('null transport', function(t) { + + // adapter is driven by config + var tmpfile = tmp.tmpNameSync({ postfix: '.json' }); + fs.writeFileSync( tmpfile, '{}', { encoding: 'utf8' } ); + process.env.PELIAS_CONFIG = tmpfile; + var adapter = setup(); + delete process.env.PELIAS_CONFIG; + + // test null transport performs a no-op + adapter.query( null, null, null, function( err, res ){ + t.equal(err, undefined, 'no-op'); + t.equal(res, undefined, 'no-op'); + t.end(); + }); + }); + +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('SERVICE language', testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; From e46c17a84aa9f2d9005d24bfb4f74f2b23ebf9db Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Tue, 4 Apr 2017 13:41:37 -0400 Subject: [PATCH 28/36] Sort interpolated results to show address first --- middleware/interpolate.js | 7 +++ test/unit/middleware/interpolate.js | 76 +++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/middleware/interpolate.js b/middleware/interpolate.js index a6d69649..7ab541cd 100644 --- a/middleware/interpolate.js +++ b/middleware/interpolate.js @@ -43,6 +43,13 @@ function setup() { res.data = results; } + // sort the results to ensure that addresses show up higher than street centroids + res.data = res.data.sort((a, b) => { + if (a.layer === 'address' && b.layer !== 'address') { return -1; } + if (a.layer !== 'address' && b.layer === 'address') { return 1; } + return 0; + }); + // log the execution time, continue logger.info( '[interpolation] [took]', (new Date()).getTime() - timer, 'ms' ); next(); diff --git a/test/unit/middleware/interpolate.js b/test/unit/middleware/interpolate.js index 26bf1e52..3548f3b3 100644 --- a/test/unit/middleware/interpolate.js +++ b/test/unit/middleware/interpolate.js @@ -190,6 +190,82 @@ module.exports.tests.hit = function(test, common) { }); }; +// check the service is called and response mapped correctly +module.exports.tests.hit = function(test, common) { + test('hit', function(t) { + + var req = { clean: { + parsed_text: { + number: '1', + street: 'sesame st' + }} + }; + var res = { data: [ + { + layer: 'street', + center_point: { lat: 1, lon: 1 }, + address_parts: { street: 'sesame rd' }, + name: { default: 'street name' }, + source_id: '123456' + }, + { + layer: 'street', + center_point: { lat: 2, lon: 2 }, + address_parts: { street: 'sesame rd' }, + name: { default: 'street name' }, + source_id: '654321' + } + ]}; + + var middleware = load(); + + // mock out the transport + middleware.transport.query = function mock(coord, number, street, cb) { + if (coord.lat === 2) { + t.deepEqual(coord, res.data[1].center_point); + t.deepEqual(number, req.clean.parsed_text.number); + t.deepEqual(street, res.data[1].address_parts.street); + t.equal(typeof cb, 'function'); + return cb(null, { + properties: { + number: '100A', + source: 'OSM', + source_id: 'way:111111', + lat: 22.2, + lon: -33.3, + } + }); + } + else { + return cb('miss'); + } + }; + + middleware(req, res, function(){ + t.deepEqual( res, { data: [ + { + layer: 'address', + match_type: 'interpolated', + center_point: { lat: 22.2, lon: -33.3 }, + address_parts: { street: 'sesame rd', number: '100A' }, + name: { default: '100A street name' }, + source: 'openstreetmap', + source_id: 'way:111111' + }, + { + layer: 'street', + center_point: { lat: 1, lon: 1 }, + address_parts: { street: 'sesame rd' }, + name: { default: 'street name' }, + source_id: '123456' + } + ]}); + t.end(); + }); + }); +}; + + module.exports.all = function (tape, common) { function test(name, testFunction) { From afe0e90f45d34d0709c88378364bc3ddfe766102 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Tue, 4 Apr 2017 16:12:58 -0400 Subject: [PATCH 29/36] move interpolation before deduping --- routes/v1.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routes/v1.js b/routes/v1.js index 8ee4173f..e9857559 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -120,8 +120,8 @@ function addRoutes(app, peliasConfig) { postProc.distances('focus.point.'), postProc.confidenceScores(peliasConfig.api), postProc.confidenceScoresFallback(), - postProc.dedupe(), postProc.interpolate(), + postProc.dedupe(), postProc.accuracy(), postProc.localNamingConventions(), postProc.renamePlacenames(), @@ -140,8 +140,8 @@ function addRoutes(app, peliasConfig) { postProc.distances('focus.point.'), postProc.confidenceScores(peliasConfig.api), postProc.confidenceScoresFallback(), - postProc.dedupe(), postProc.interpolate(), + postProc.dedupe(), postProc.accuracy(), postProc.localNamingConventions(), postProc.renamePlacenames(), From 9e589b3a47dee4d17ad4869a387c32e5824e5b64 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Thu, 6 Apr 2017 15:15:23 +0000 Subject: [PATCH 30/36] fix(package): update pelias-categories to version 1.2.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 98ad08e0..2ae59720 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,7 @@ "lodash": "^4.5.0", "markdown": "0.5.0", "morgan": "1.8.1", - "pelias-categories": "1.1.0", + "pelias-categories": "1.2.0", "pelias-config": "2.8.0", "pelias-labels": "1.5.1", "pelias-logger": "0.1.0", From 8a237c1a49b1bb4a87b55ee5473c8ce1dbebc0b3 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Thu, 6 Apr 2017 15:45:38 +0000 Subject: [PATCH 31/36] fix(package): update pelias-query to version 8.15.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 5134ef59..e8ef87e0 100644 --- a/package.json +++ b/package.json @@ -60,7 +60,7 @@ "pelias-logger": "0.1.0", "pelias-mock-logger": "^1.0.1", "pelias-model": "4.5.1", - "pelias-query": "8.14.0", + "pelias-query": "8.15.0", "pelias-text-analyzer": "1.7.3", "predicates": "^1.0.1", "retry": "^0.10.1", From 4b5a481e374da92433c363cc47d5281bb5f38df1 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Thu, 6 Apr 2017 16:48:39 +0000 Subject: [PATCH 32/36] fix(package): update pelias-model to version 4.6.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 838ea7e6..fc19dcdd 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,7 @@ "pelias-labels": "1.6.0", "pelias-logger": "0.1.0", "pelias-mock-logger": "^1.0.1", - "pelias-model": "4.5.1", + "pelias-model": "4.6.0", "pelias-query": "8.14.0", "pelias-text-analyzer": "1.7.3", "predicates": "^1.0.1", From 79e539d0081b617f3fa71def716bb2d442bf7418 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Thu, 6 Apr 2017 18:26:05 +0000 Subject: [PATCH 33/36] fix(package): update pelias-logger to version 0.2.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 3f39ca3a..e90a9acd 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ "pelias-categories": "1.2.0", "pelias-config": "2.8.0", "pelias-labels": "1.6.0", - "pelias-logger": "0.1.0", + "pelias-logger": "0.2.0", "pelias-mock-logger": "^1.0.1", "pelias-model": "4.6.0", "pelias-query": "8.15.0", From ed854d8154e000921936676e18772c4e060e5f27 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Mon, 27 Mar 2017 14:24:49 -0400 Subject: [PATCH 34/36] add dependency check to pre-commit and travis --- .travis.yml | 10 +++------- package.json | 9 ++++++--- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index e7b4bc62..28a218c8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,20 +1,16 @@ sudo: false language: node_js -cache: - directories: - - node_modules notifications: email: false node_js: - 4 - 6 matrix: - fast_finish: true - allow_failures: + fast_finish: true env: global: - CXX=g++-4.8 -script: "npm run travis" +script: npm run travis addons: apt: sources: @@ -22,7 +18,7 @@ addons: packages: - g++-4.8 before_install: - - npm i -g npm@^2.0.0 + - npm i -g npm@^3.0.0 before_script: - npm prune after_success: diff --git a/package.json b/package.json index ed6eedb2..ba623565 100644 --- a/package.json +++ b/package.json @@ -14,11 +14,12 @@ "lint": "jshint .", "start": "node index.js", "test": "npm run unit", - "travis": "npm test", + "travis": "npm run check-dependencies && npm test", "unit": "./bin/units", "validate": "npm ls", "semantic-release": "semantic-release pre && npm publish && semantic-release post", - "config": "node -e \"console.log(JSON.stringify(require( 'pelias-config' ).generate(require('./schema')), null, 2))\"" + "config": "node -e \"console.log(JSON.stringify(require( 'pelias-config' ).generate(require('./schema')), null, 2))\"", + "check-dependencies": "node_modules/.bin/npm-check --production" }, "repository": { "type": "git", @@ -74,6 +75,7 @@ "difflet": "^1.0.1", "istanbul": "^0.4.2", "jshint": "^2.5.6", + "npm-check": "^5.4.0", "nsp": "^2.2.0", "precommit-hook": "^3.0.0", "proxyquire": "^1.7.10", @@ -87,6 +89,7 @@ "pre-commit": [ "lint", "validate", - "test" + "test", + "check-dependencies" ] } From 73762b726b4ba826e9538a6c0cabcbb1d8495205 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Thu, 6 Apr 2017 15:38:51 -0400 Subject: [PATCH 35/36] ignore pelias-interpolation for now --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ba623565..e9d354cc 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "validate": "npm ls", "semantic-release": "semantic-release pre && npm publish && semantic-release post", "config": "node -e \"console.log(JSON.stringify(require( 'pelias-config' ).generate(require('./schema')), null, 2))\"", - "check-dependencies": "node_modules/.bin/npm-check --production" + "check-dependencies": "node_modules/.bin/npm-check --production --ignore pelias-interpolation" }, "repository": { "type": "git", From d6d328fdf6da5a40b2b50903947162505bfcf6d2 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Thu, 6 Apr 2017 18:01:00 -0400 Subject: [PATCH 36/36] fix: 500 errors returned when no language service is found --- service/language.js | 2 +- test/unit/service/language.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/service/language.js b/service/language.js index 50cc29c0..9e3702d0 100644 --- a/service/language.js +++ b/service/language.js @@ -29,7 +29,7 @@ var logger = require( 'pelias-logger' ).get( 'api' ), disables the service completely **/ function NullTransport(){} -NullTransport.prototype.query = function( coord, number, street, cb ){ +NullTransport.prototype.query = function( ids, cb ){ cb(); // no-op }; diff --git a/test/unit/service/language.js b/test/unit/service/language.js index 151247fd..f8091924 100644 --- a/test/unit/service/language.js +++ b/test/unit/service/language.js @@ -50,7 +50,7 @@ module.exports.tests.factory = function(test, common) { t.equal(adapter.constructor.name, 'NullTransport', 'NullTransport'); t.equal(typeof adapter, 'object', 'adapter is an object'); t.equal(typeof adapter.query, 'function', 'query is a function'); - t.equal(adapter.query.length, 4, 'query function signature'); + t.equal(adapter.query.length, 2, 'query function signature'); t.end(); }); @@ -67,7 +67,7 @@ module.exports.tests.factory = function(test, common) { t.equal(adapter.constructor.name, 'NullTransport', 'NullTransport'); t.equal(typeof adapter, 'object', 'adapter is an object'); t.equal(typeof adapter.query, 'function', 'query is a function'); - t.equal(adapter.query.length, 4, 'query function signature'); + t.equal(adapter.query.length, 2, 'query function signature'); t.end(); }); @@ -86,7 +86,7 @@ module.exports.tests.NullTransport = function(test, common) { delete process.env.PELIAS_CONFIG; // test null transport performs a no-op - adapter.query( null, null, null, function( err, res ){ + adapter.query( null, function( err, res ){ t.equal(err, undefined, 'no-op'); t.equal(res, undefined, 'no-op'); t.end();