From d7e4ebdafb01a4b052981c375c6e521c201a1b57 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Fri, 20 Oct 2017 18:14:45 +0000 Subject: [PATCH 01/24] chore(package): update dependencies --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 2d50759c..703658b4 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ "geolib": "^2.0.18", "iso-639-3": "^1.0.0", "iso3166-1": "^0.3.0", - "joi": "^12.0.0", + "joi": "^13.0.1", "locale": "^0.1.0", "lodash": "^4.17.4", "markdown": "0.5.0", @@ -62,7 +62,7 @@ "pelias-model": "5.2.0", "pelias-query": "9.1.0", "pelias-sorting": "1.0.1", - "pelias-text-analyzer": "1.10.0", + "pelias-text-analyzer": "1.10.1", "predicates": "^2.0.0", "retry": "^0.10.1", "stats-lite": "^2.0.4", From 393a51703d68c3461497fb57e664bf5e66b36819 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Fri, 20 Oct 2017 15:20:39 -0400 Subject: [PATCH 02/24] revert joi version, keep pelias-text-analyzer --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 703658b4..c9d0b018 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ "geolib": "^2.0.18", "iso-639-3": "^1.0.0", "iso3166-1": "^0.3.0", - "joi": "^13.0.1", + "joi": "^12.0.0", "locale": "^0.1.0", "lodash": "^4.17.4", "markdown": "0.5.0", From ca993823947afd95376a212e4521d08abd4ab1f9 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Mon, 23 Oct 2017 13:23:57 +0000 Subject: [PATCH 03/24] fix(package): update pelias-text-analyzer to version 1.10.2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c9d0b018..147c87e4 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,7 @@ "pelias-model": "5.2.0", "pelias-query": "9.1.0", "pelias-sorting": "1.0.1", - "pelias-text-analyzer": "1.10.1", + "pelias-text-analyzer": "1.10.2", "predicates": "^2.0.0", "retry": "^0.10.1", "stats-lite": "^2.0.4", From f9281190ac1da0d1c67ba8d54b144453262e1d11 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 18 Oct 2017 11:35:32 -0400 Subject: [PATCH 04/24] Corrrectly filter /place queries by layer `/place` queries have been executing in a way where only the ID, but not the layer, has been considered when returning records from Elasticsearch. It turns out this bug was introduced almost a year and a half ago in https://github.com/pelias/api/pull/407. A little, relatively unimportant bit of code was looking for a property called `layers`: ``` const cmd = req.clean.ids.map( function(id) { return { _index: apiConfig.indexName, _type: id.layers, _id: id.id }; }); ``` The correct property was `layer`, so no filtering on layer was done in the resulting [mget](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-multi-get.html) query. There was never an acceptance test for these sorts of queries, but there is now one in https://github.com/pelias/acceptance-tests/pull/446. The unit tests were enforcing the incorrect behavior. Fixes https://github.com/pelias/pelias/issues/643 --- controller/place.js | 2 +- test/unit/controller/place.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/controller/place.js b/controller/place.js index 2b5490aa..51ce58d9 100644 --- a/controller/place.js +++ b/controller/place.js @@ -38,7 +38,7 @@ function setup( apiConfig, esclient ){ const cmd = req.clean.ids.map( function(id) { return { _index: apiConfig.indexName, - _type: id.layers, + _type: id.layer, _id: id.id }; }); diff --git a/test/unit/controller/place.js b/test/unit/controller/place.js index 25d99463..b76d69c9 100644 --- a/test/unit/controller/place.js +++ b/test/unit/controller/place.js @@ -51,11 +51,11 @@ module.exports.tests.success = (test, common) => { ids: [ { id: 'id1', - layers: 'layer1' + layer: 'layer1' }, { id: 'id2', - layers: 'layer2' + layer: 'layer2' } ] }, From 110a258130c102ca8689da467282f171fb245785 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 23 Oct 2017 11:14:29 -0400 Subject: [PATCH 05/24] added initial tests for using vs --- test/unit/query/MockQuery.js | 7 + test/unit/query/reverse.js | 335 +++++++++++++++++++---------------- test/unit/run.js | 188 ++++++++++---------- 3 files changed, 287 insertions(+), 243 deletions(-) diff --git a/test/unit/query/MockQuery.js b/test/unit/query/MockQuery.js index f97bfb27..e7202cc9 100644 --- a/test/unit/query/MockQuery.js +++ b/test/unit/query/MockQuery.js @@ -3,6 +3,7 @@ module.exports = class MockQuery { constructor() { this._score_functions = []; + this._sort_functions = []; this._filter_functions = []; } @@ -10,6 +11,7 @@ module.exports = class MockQuery { return { vs: vs, score_functions: this._score_functions, + sort_functions: this._sort_functions, filter_functions: this._filter_functions }; } @@ -19,6 +21,11 @@ module.exports = class MockQuery { return this; } + sort(view) { + this._sort_functions.push(view); + return this; + } + filter(view) { this._filter_functions.push(view); return this; diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index eab97188..0e47eaed 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -1,190 +1,227 @@ -var generate = require('../../../query/reverse'); +const generate = require('../../../query/reverse'); +const _ = require('lodash'); +const proxyquire = require('proxyquire').noCallThru(); +const MockQuery = require('./MockQuery'); + +// helper for canned views +const views = { + boundary_country: 'boundary_country view', + boundary_circle: 'boundary_circle view', + sources: 'sources view', + layers: 'layers view', + categories: 'categories view', + sort_distance: 'sort_distance view' +}; module.exports.tests = {}; -module.exports.tests.interface = function(test, common) { - test('valid interface', function(t) { +module.exports.tests.interface = (test, common) => { + test('valid interface', t => { t.equal(typeof generate, 'function', 'valid function'); t.end(); }); }; -module.exports.tests.query = function(test, common) { - test('valid query', function(t) { - var query = generate({ - 'point.lat': 29.49136, - 'point.lon': -82.50622, - 'boundary.circle.lat': 29.49136, - 'boundary.circle.lon': -82.50622, - 'boundary.circle.radius': 3 - }); - - var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = require('../fixture/reverse_standard'); - - t.deepEqual(compiled.type, 'reverse', 'query type set'); - t.deepEqual(compiled.body, expected, 'reverse_standard'); +module.exports.tests.query = (test, common) => { + test('base no frills', t => { + const clean = {}; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': { + default_parameter_1: 'first default parameter', + default_parameter_2: 'second default parameter' + } + })(clean); + + t.equals(query.type, 'reverse', 'query type set'); + t.deepEquals(query.body.vs.var('default_parameter_1').toString(), 'first default parameter'); + t.deepEquals(query.body.vs.var('default_parameter_2').toString(), 'second default parameter'); + t.notOk(query.body.vs.isset('size')); + t.notOk(query.body.vs.isset('sources')); + t.notOk(query.body.vs.isset('layers')); + t.notOk(query.body.vs.isset('focus:point:lat')); + t.notOk(query.body.vs.isset('focus:point:lon')); + t.notOk(query.body.vs.isset('boundary:circle:lat')); + t.notOk(query.body.vs.isset('boundary:circle:lon')); + t.notOk(query.body.vs.isset('boundary:circle:radius')); + t.notOk(query.body.vs.isset('boundary:country')); + t.notOk(query.body.vs.isset('input:categories')); + + t.deepEquals(query.body.score_functions, [ + 'boundary_country view' + ]); + + t.deepEquals(query.body.filter_functions, [ + 'boundary_circle view', + 'sources view', + 'layers view', + 'categories view' + ]); + + t.deepEquals(query.body.sort_functions, [ + 'sort_distance view' + ]); + t.end(); - }); - test('valid query - null island', function(t) { - var query = generate({ - 'point.lat': 0, - 'point.lon': 0, - 'boundary.circle.lat': 0, - 'boundary.circle.lon': 0, - 'boundary.circle.radius': 3 - }); + }); - var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = require('../fixture/reverse_null_island'); + test('clean.querySize should set size parameter', t => { + const clean = { + querySize: 17 + }; - t.deepEqual(compiled.type, 'reverse', 'query type set'); - t.deepEqual(compiled.body, expected, 'reverse_null_island'); + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.deepEquals(query.body.vs.var('size').toString(), 17); t.end(); + }); - test('valid query with radius', function(t) { - var query = generate({ - 'point.lat': 29.49136, - 'point.lon': -82.50622, - 'boundary.circle.lat': 29.49136, - 'boundary.circle.lon': -82.50622, - 'boundary.circle.radius': 123 - }); +}; - var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = '123km'; +module.exports.tests.sources = (test, common) => { + test('non-array clean.sources should not set sources in vs', t => { + const clean = { + sources: 'this is not an array' + }; - t.deepEqual(compiled.type, 'reverse', 'query type set'); - t.deepEqual(compiled.body.query.bool.filter[0].geo_distance.distance, expected, 'distance set to boundary circle radius'); + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.notOk(query.body.vs.isset('sources')); t.end(); - }); - // for coarse reverse cases where boundary circle radius isn't used - test('undefined radius set to default radius', function(t) { - var query = generate({ - 'point.lat': 12.12121, - 'point.lon': 21.21212, - 'boundary.circle.lat': 12.12121, - 'boundary.circle.lon': 21.21212 - }); + }); - var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = '1km'; + test('empty array clean.sources should not set sources in vs', t => { + const clean = { + sources: [] + }; - t.deepEqual(compiled.type, 'reverse', 'query type set'); - t.deepEqual(compiled.body.query.bool.filter[0].geo_distance.distance, expected, 'distance set to default boundary circle radius'); + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.notOk(query.body.vs.isset('sources')); t.end(); + }); - test('boundary.circle lat/lon/radius - overrides point.lat/lon when set', function(t) { - var clean = { - 'point.lat': 29.49136, - 'point.lon': -82.50622, - 'boundary.circle.lat': 111, - 'boundary.circle.lon': 333, - 'boundary.circle.radius': 3 + test('non-empty array clean.sources should set sources in vs', t => { + const clean = { + sources: ['source 1', 'source 2'] }; - var query = generate(clean); - var compiled = JSON.parse( JSON.stringify( query ) ); - - // this should not equal `point.lat` and `point.lon` as it was explitely specified - var expected = { lat: clean['boundary.circle.lat'], lon: clean['boundary.circle.lon'] }; - var centroid = compiled.body.query.bool.filter[0].geo_distance.center_point; - t.deepEqual(compiled.type, 'reverse', 'query type set'); - t.deepEqual(centroid, expected, 'reverse: boundary.circle/lon overrides point.lat/lon'); + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.deepEquals(query.body.vs.var('sources').toString(), ['source 1', 'source 2']); t.end(); - }); - test('size fuzz test', function(t) { - // test different sizes - var sizes = [1,4,20,undefined,null]; - var expected = [1,4,20,1,1]; - sizes.forEach( function( size, index ){ - var query = generate({ - 'point.lat': 29.49136, 'point.lon': -82.50622, querySize: size - }); - - var compiled = JSON.parse( JSON.stringify( query ) ); - t.equal( compiled.body.size, expected[index], 'valid reverse query for size: '+ size); - }); - t.end(); }); - test('valid boundary.country reverse search', function(t) { - var query = generate({ - 'point.lat': 29.49136, - 'point.lon': -82.50622, - 'boundary.circle.lat': 29.49136, - 'boundary.circle.lon': -82.50622, - 'boundary.circle.radius': 3, - 'boundary.country': 'ABC' - }); - - var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = require('../fixture/reverse_with_boundary_country'); - - t.deepEqual(compiled.type, 'reverse', 'query type set'); - t.deepEqual(compiled.body, expected, 'valid reverse query with boundary.country'); - t.end(); - }); +}; + +module.exports.tests.layers = (test, common) => { + test('non-array clean.layers should not set sources in vs', t => { + const clean = { + layers: 'this is not an array' + }; - test('valid sources filter', function(t) { - var query = generate({ - 'point.lat': 29.49136, - 'point.lon': -82.50622, - 'boundary.circle.lat': 29.49136, - 'boundary.circle.lon': -82.50622, - 'boundary.circle.radius': 3, - 'sources': ['test'] - }); - - var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = require('../fixture/reverse_with_source_filtering'); - - t.deepEqual(compiled.type, 'reverse', 'query type set'); - t.deepEqual(compiled.body, expected, 'valid reverse query with source filtering'); + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.notOk(query.body.vs.isset('layers')); t.end(); + }); - test('valid layers filter', (t) => { - const query = generate({ - 'point.lat': 29.49136, - 'point.lon': -82.50622, - 'boundary.circle.lat': 29.49136, - 'boundary.circle.lon': -82.50622, - 'boundary.circle.radius': 3, - // only venue, address, and street layers should be retained - 'layers': ['neighbourhood', 'venue', 'locality', 'address', 'region', 'street', 'country'] - }); - - const compiled = JSON.parse( JSON.stringify( query ) ); - const expected = require('../fixture/reverse_with_layer_filtering'); - - t.deepEqual(compiled.type, 'reverse', 'query type set'); - t.deepEqual(compiled.body, expected, 'valid reverse query with source filtering'); + test('empty array clean.layers should not set sources in vs', t => { + const clean = { + layers: [] + }; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.notOk(query.body.vs.isset('layers')); t.end(); }); - test('valid layers filter - subset of non-coarse layers', (t) => { - const query = generate({ - 'point.lat': 29.49136, - 'point.lon': -82.50622, - 'boundary.circle.lat': 29.49136, - 'boundary.circle.lon': -82.50622, - 'boundary.circle.radius': 3, - // only venue, address, and street layers should be retained - 'layers': ['neighbourhood', 'venue', 'street', 'locality'] - }); - - const compiled = JSON.parse( JSON.stringify( query ) ); - const expected = require('../fixture/reverse_with_layer_filtering_non_coarse_subset'); - - t.deepEqual(compiled.type, 'reverse', 'query type set'); - t.deepEqual(compiled.body, expected, 'valid reverse query with source filtering'); + test('non-empty array clean.layers should only set non-coarse layers in vs', t => { + const clean = { + sources: ['source 1', 'source 2'] + }; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.deepEquals(query.body.vs.var('sources').toString(), ['source 1', 'source 2']); t.end(); }); diff --git a/test/unit/run.js b/test/unit/run.js index 5631b9f3..cfe60255 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -9,101 +9,101 @@ var common = { }; var tests = [ - require('./app'), - require('./schema'), - require('./controller/coarse_reverse'), - require('./controller/index'), - require('./controller/libpostal'), - require('./controller/place'), - require('./controller/placeholder'), - require('./controller/search'), - require('./controller/search_with_ids'), - require('./controller/predicates/has_parsed_text_properties'), - require('./controller/predicates/has_request_parameter'), - require('./controller/predicates/has_response_data'), - require('./controller/predicates/has_results_at_layers'), - require('./controller/predicates/has_request_parameter'), - require('./controller/predicates/has_request_errors'), - require('./controller/predicates/is_addressit_parse'), - require('./controller/predicates/is_admin_only_analysis'), - require('./controller/predicates/is_coarse_reverse'), - require('./controller/predicates/is_only_non_admin_layers'), - require('./controller/predicates/is_request_sources_only_whosonfirst'), - require('./helper/debug'), - require('./helper/diffPlaces'), - require('./helper/geojsonify_place_details'), - require('./helper/geojsonify'), - require('./helper/logging'), - require('./helper/type_mapping'), - require('./helper/sizeCalculator'), - require('./helper/stackTraceLine'), - require('./middleware/access_log'), - require('./middleware/accuracy'), - require('./middleware/assignLabels'), - require('./middleware/confidenceScore'), - require('./middleware/confidenceScoreFallback'), - require('./middleware/confidenceScoreReverse'), - require('./middleware/changeLanguage'), - require('./middleware/distance'), - require('./middleware/interpolate'), - require('./middleware/localNamingConventions'), - require('./middleware/dedupe'), - require('./middleware/parseBBox'), - require('./middleware/sendJSON'), - require('./middleware/normalizeParentIds'), - require('./middleware/sortResponseData'), - require('./middleware/trimByGranularity'), - require('./middleware/trimByGranularityStructured'), - require('./middleware/requestLanguage'), - require('./query/address_search_using_ids'), - require('./query/autocomplete'), - require('./query/autocomplete_defaults'), - require('./query/search_defaults'), - require('./query/reverse_defaults'), + // require('./app'), + // require('./schema'), + // require('./controller/coarse_reverse'), + // require('./controller/index'), + // require('./controller/libpostal'), + // require('./controller/place'), + // require('./controller/placeholder'), + // require('./controller/search'), + // require('./controller/search_with_ids'), + // require('./controller/predicates/has_parsed_text_properties'), + // require('./controller/predicates/has_request_parameter'), + // require('./controller/predicates/has_response_data'), + // require('./controller/predicates/has_results_at_layers'), + // require('./controller/predicates/has_request_parameter'), + // require('./controller/predicates/has_request_errors'), + // require('./controller/predicates/is_addressit_parse'), + // require('./controller/predicates/is_admin_only_analysis'), + // require('./controller/predicates/is_coarse_reverse'), + // require('./controller/predicates/is_only_non_admin_layers'), + // require('./controller/predicates/is_request_sources_only_whosonfirst'), + // require('./helper/debug'), + // require('./helper/diffPlaces'), + // require('./helper/geojsonify_place_details'), + // require('./helper/geojsonify'), + // require('./helper/logging'), + // require('./helper/type_mapping'), + // require('./helper/sizeCalculator'), + // require('./helper/stackTraceLine'), + // require('./middleware/access_log'), + // require('./middleware/accuracy'), + // require('./middleware/assignLabels'), + // require('./middleware/confidenceScore'), + // require('./middleware/confidenceScoreFallback'), + // require('./middleware/confidenceScoreReverse'), + // require('./middleware/changeLanguage'), + // require('./middleware/distance'), + // require('./middleware/interpolate'), + // require('./middleware/localNamingConventions'), + // require('./middleware/dedupe'), + // require('./middleware/parseBBox'), + // require('./middleware/sendJSON'), + // require('./middleware/normalizeParentIds'), + // require('./middleware/sortResponseData'), + // require('./middleware/trimByGranularity'), + // require('./middleware/trimByGranularityStructured'), + // require('./middleware/requestLanguage'), + // require('./query/address_search_using_ids'), + // require('./query/autocomplete'), + // require('./query/autocomplete_defaults'), + // require('./query/search_defaults'), + // require('./query/reverse_defaults'), require('./query/reverse'), - require('./query/search'), - require('./query/search_original'), - require('./query/structured_geocoding'), - require('./query/text_parser'), - require('./sanitizer/_boundary_country'), - require('./sanitizer/_debug'), - require('./sanitizer/_flag_bool'), - require('./sanitizer/_geonames_deprecation'), - require('./sanitizer/_geonames_warnings'), - require('./sanitizer/_geo_common'), - require('./sanitizer/_geo_reverse'), - require('./sanitizer/_groups'), - require('./sanitizer/_ids'), - require('./sanitizer/_iso2_to_iso3'), - require('./sanitizer/_layers'), - require('./sanitizer/_location_bias'), - require('./sanitizer/_city_name_standardizer'), - require('./sanitizer/_request_language'), - require('./sanitizer/_single_scalar_parameters'), - require('./sanitizer/_size'), - require('./sanitizer/_sources'), - require('./sanitizer/_sources_and_layers'), - require('./sanitizer/_synthesize_analysis'), - require('./sanitizer/_text'), - require('./sanitizer/_text_addressit'), - require('./sanitizer/_tokenizer'), - require('./sanitizer/_deprecate_quattroshapes'), - require('./sanitizer/_categories'), - require('./sanitizer/nearby'), - require('./sanitizer/autocomplete'), - require('./sanitizer/structured_geocoding'), - require('./sanitizer/place'), - require('./sanitizer/reverse'), - require('./sanitizer/sanitizeAll'), - require('./sanitizer/search'), - require('./sanitizer/defer_to_addressit'), - require('./sanitizer/wrap'), - require('./service/configurations/Interpolation'), - require('./service/configurations/Language'), - require('./service/configurations/PlaceHolder'), - require('./service/configurations/PointInPolygon'), - require('./service/mget'), - require('./service/search') + // require('./query/search'), + // require('./query/search_original'), + // require('./query/structured_geocoding'), + // require('./query/text_parser'), + // require('./sanitizer/_boundary_country'), + // require('./sanitizer/_debug'), + // require('./sanitizer/_flag_bool'), + // require('./sanitizer/_geonames_deprecation'), + // require('./sanitizer/_geonames_warnings'), + // require('./sanitizer/_geo_common'), + // require('./sanitizer/_geo_reverse'), + // require('./sanitizer/_groups'), + // require('./sanitizer/_ids'), + // require('./sanitizer/_iso2_to_iso3'), + // require('./sanitizer/_layers'), + // require('./sanitizer/_location_bias'), + // require('./sanitizer/_city_name_standardizer'), + // require('./sanitizer/_request_language'), + // require('./sanitizer/_single_scalar_parameters'), + // require('./sanitizer/_size'), + // require('./sanitizer/_sources'), + // require('./sanitizer/_sources_and_layers'), + // require('./sanitizer/_synthesize_analysis'), + // require('./sanitizer/_text'), + // require('./sanitizer/_text_addressit'), + // require('./sanitizer/_tokenizer'), + // require('./sanitizer/_deprecate_quattroshapes'), + // require('./sanitizer/_categories'), + // require('./sanitizer/nearby'), + // require('./sanitizer/autocomplete'), + // require('./sanitizer/structured_geocoding'), + // require('./sanitizer/place'), + // require('./sanitizer/reverse'), + // require('./sanitizer/sanitizeAll'), + // require('./sanitizer/search'), + // require('./sanitizer/defer_to_addressit'), + // require('./sanitizer/wrap'), + // require('./service/configurations/Interpolation'), + // require('./service/configurations/Language'), + // require('./service/configurations/PlaceHolder'), + // require('./service/configurations/PointInPolygon'), + // require('./service/mget'), + // require('./service/search') ]; tests.map(function(t) { From 4d3ea56a397455ec819066ba1dd73c98c3a07503 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 23 Oct 2017 13:00:03 -0400 Subject: [PATCH 06/24] Change default listen parameter to undefined This will ensure our default listen address is whatever the Node.js default is. It's currently set to listen on all addresses, and probably won't change. --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 4fffd846..baefb894 100644 --- a/index.js +++ b/index.js @@ -1,7 +1,7 @@ const app = require('./app'), port = ( process.env.PORT || 3100 ), - host = ( process.env.HOST || 'localhost' ); + host = ( process.env.HOST || undefined ); /** run server on the default setup (single core) **/ console.log( `pelias is now running on ${host}:${port}` ); From 65507807acfe25da6b2db379cd72b01e3b304e62 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 23 Oct 2017 13:03:10 -0400 Subject: [PATCH 07/24] Ask Node's `net.server` for the actual address its listening on This allows us to emit a helpful and correct message when Pelias starts. --- index.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index baefb894..f541c0cc 100644 --- a/index.js +++ b/index.js @@ -3,6 +3,8 @@ const app = require('./app'), port = ( process.env.PORT || 3100 ), host = ( process.env.HOST || undefined ); -/** run server on the default setup (single core) **/ -console.log( `pelias is now running on ${host}:${port}` ); -app.listen( port, host ); +const server = app.listen( port, host, () => { + // ask server for the actual address and port its listening on + const listenAddress = server.address(); + console.log( `pelias is now running on ${listenAddress.address}:${listenAddress.port}` ); +}); From d577d42d9feb019ae1f0982f147e78039abcfd73 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 23 Oct 2017 14:58:54 -0400 Subject: [PATCH 08/24] Support geonames non-coarse reverse in deprecation sanitizer --- sanitizer/_geonames_deprecation.js | 15 +++++++- test/unit/sanitizer/_geonames_deprecation.js | 40 +++++++++++++++++--- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/sanitizer/_geonames_deprecation.js b/sanitizer/_geonames_deprecation.js index b3cc8185..f6608e96 100644 --- a/sanitizer/_geonames_deprecation.js +++ b/sanitizer/_geonames_deprecation.js @@ -1,14 +1,25 @@ const _ = require('lodash'); /** -with the release of coarse reverse as a separate thing and ES reverse only -handling venues, addresses, and streets, geonames make no sense in the reverse context + * Now that we have the pip-service, we have stopped supporting returning Geonames for coarse reverse. + * + * However, until the `/nearby` endpoint is finalized, we still want to support Geonames for + * _non-coarse_ reverse. **/ function _sanitize( raw, clean, opts ) { // error & warning messages const messages = { errors: [], warnings: [] }; + // return taking no action unless this is a coarse-only reverse request + const non_coarse_layers = ['address', 'street', 'venue']; + const is_coarse_reverse = !_.isEmpty(clean.layers) && + _.isEmpty(_.intersection(clean.layers, non_coarse_layers)); + + if (!is_coarse_reverse) { + return messages; + } + if (_.isEqual(clean.sources, ['geonames']) || _.isEqual(clean.sources, ['gn'])) { messages.errors.push('/reverse does not support geonames'); diff --git a/test/unit/sanitizer/_geonames_deprecation.js b/test/unit/sanitizer/_geonames_deprecation.js index 41bd86c6..707c6774 100644 --- a/test/unit/sanitizer/_geonames_deprecation.js +++ b/test/unit/sanitizer/_geonames_deprecation.js @@ -27,13 +27,26 @@ module.exports.tests.no_warnings_or_errors_conditions = (test, common) => { }); + test('geonames/gn in sources with layers=venue should add neither warnings nor errors', (t) => { + const clean = { + sources: ['geonames'], + layers: ['venue'] + }; + + const messages = sanitizer.sanitize(undefined, clean); + + t.deepEquals(clean.sources, ['geonames']); + t.deepEquals(messages, { errors: [], warnings: [] }); + t.end(); + }); }; module.exports.tests.error_conditions = (test, common) => { - test('only geonames in sources should not modify clean.sources and add error message', (t) => { + test('only geonames in sources with layers=coarse should not modify clean.sources and add error message', (t) => { ['gn', 'geonames'].forEach((sources) => { const clean = { - sources: [sources] + sources: [sources], + layers: ['coarse'] }; const messages = sanitizer.sanitize(undefined, clean); @@ -45,16 +58,33 @@ module.exports.tests.error_conditions = (test, common) => { }); t.end(); - }); + test('only geonames in sources with layers=locality should not modify clean.sources and add error message', (t) => { + ['gn', 'geonames'].forEach((sources) => { + const clean = { + sources: [sources], + layers: ['locality'] + }; + + const messages = sanitizer.sanitize(undefined, clean); + + t.deepEquals(clean.sources, [sources]); + t.deepEquals(messages.errors, ['/reverse does not support geonames']); + t.deepEquals(messages.warnings, []); + + }); + + t.end(); + }); }; module.exports.tests.warning_conditions = (test, common) => { - test('only geonames in sources should not modify clean.sources and add error message', (t) => { + test('only geonames in sources and layers=coarse should not modify clean.sources and add error message', (t) => { ['gn', 'geonames'].forEach((sources) => { const clean = { - sources: ['another source', sources, 'yet another source'] + sources: ['another source', sources, 'yet another source'], + layers: ['coarse'] }; const messages = sanitizer.sanitize(undefined, clean); From 004e3c02b125d49487b369e4ffe4168079b82738 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 23 Oct 2017 15:13:50 -0400 Subject: [PATCH 09/24] completed switch to vs tests over fixtures --- test/unit/fixture/reverse_null_island.js | 39 --- test/unit/fixture/reverse_standard.js | 39 --- .../fixture/reverse_with_boundary_country.js | 49 --- .../fixture/reverse_with_layer_filtering.js | 41 --- ..._with_layer_filtering_non_coarse_subset.js | 41 --- .../fixture/reverse_with_source_filtering.js | 46 --- test/unit/query/reverse.js | 280 +++++++++++++++++- test/unit/run.js | 188 ++++++------ 8 files changed, 372 insertions(+), 351 deletions(-) delete mode 100644 test/unit/fixture/reverse_null_island.js delete mode 100644 test/unit/fixture/reverse_standard.js delete mode 100644 test/unit/fixture/reverse_with_boundary_country.js delete mode 100644 test/unit/fixture/reverse_with_layer_filtering.js delete mode 100644 test/unit/fixture/reverse_with_layer_filtering_non_coarse_subset.js delete mode 100644 test/unit/fixture/reverse_with_source_filtering.js diff --git a/test/unit/fixture/reverse_null_island.js b/test/unit/fixture/reverse_null_island.js deleted file mode 100644 index 874518d4..00000000 --- a/test/unit/fixture/reverse_null_island.js +++ /dev/null @@ -1,39 +0,0 @@ -var vs = require('../../../query/reverse_defaults'); - -module.exports = { - 'query': { - 'bool': { - 'filter': [{ - 'geo_distance': { - 'distance': '3km', - 'distance_type': 'plane', - 'optimize_bbox': 'indexed', - 'center_point': { - 'lat': 0, - 'lon': 0 - } - } - }, - { - 'terms': { - 'layer': ['venue', 'address', 'street'] - } - }] - } - }, - 'sort': [ - '_score', - { - '_geo_distance': { - 'center_point': { - 'lat': 0, - 'lon': 0 - }, - 'order': 'asc', - 'distance_type': 'plane' - } - } - ], - 'size': vs.size, - 'track_scores': true -}; diff --git a/test/unit/fixture/reverse_standard.js b/test/unit/fixture/reverse_standard.js deleted file mode 100644 index 4a477ce1..00000000 --- a/test/unit/fixture/reverse_standard.js +++ /dev/null @@ -1,39 +0,0 @@ -var vs = require('../../../query/reverse_defaults'); - -module.exports = { - 'query': { - 'bool': { - 'filter': [{ - 'geo_distance': { - 'distance': '3km', - 'distance_type': 'plane', - 'optimize_bbox': 'indexed', - 'center_point': { - 'lat': 29.49136, - 'lon': -82.50622 - } - } - }, - { - 'terms': { - 'layer': ['venue', 'address', 'street'] - } - }] - } - }, - 'sort': [ - '_score', - { - '_geo_distance': { - 'center_point': { - 'lat': 29.49136, - 'lon': -82.50622 - }, - 'order': 'asc', - 'distance_type': 'plane' - } - } - ], - 'size': vs.size, - 'track_scores': true -}; diff --git a/test/unit/fixture/reverse_with_boundary_country.js b/test/unit/fixture/reverse_with_boundary_country.js deleted file mode 100644 index 6677247a..00000000 --- a/test/unit/fixture/reverse_with_boundary_country.js +++ /dev/null @@ -1,49 +0,0 @@ -var vs = require('../../../query/reverse_defaults'); - -module.exports = { - 'query': { - 'bool': { - 'must': [ - { - 'match': { - 'parent.country_a': { - 'analyzer': 'standard', - 'query': 'ABC' - } - } - } - ], - 'filter': [{ - 'geo_distance': { - 'distance': '3km', - 'distance_type': 'plane', - 'optimize_bbox': 'indexed', - 'center_point': { - 'lat': 29.49136, - 'lon': -82.50622 - } - } - }, - { - 'terms': { - 'layer': ['venue', 'address', 'street'] - } - }] - } - }, - 'sort': [ - '_score', - { - '_geo_distance': { - 'center_point': { - 'lat': 29.49136, - 'lon': -82.50622 - }, - 'order': 'asc', - 'distance_type': 'plane' - } - } - ], - 'size': vs.size, - 'track_scores': true -}; diff --git a/test/unit/fixture/reverse_with_layer_filtering.js b/test/unit/fixture/reverse_with_layer_filtering.js deleted file mode 100644 index 5d6c53b5..00000000 --- a/test/unit/fixture/reverse_with_layer_filtering.js +++ /dev/null @@ -1,41 +0,0 @@ -var vs = require('../../../query/reverse_defaults'); - -module.exports = { - 'query': { - 'bool': { - 'filter': [ - { - 'geo_distance': { - 'distance': '3km', - 'distance_type': 'plane', - 'optimize_bbox': 'indexed', - 'center_point': { - 'lat': 29.49136, - 'lon': -82.50622 - } - } - }, - { - 'terms': { - 'layer': ['venue', 'address', 'street'] - } - } - ] - } - }, - 'sort': [ - '_score', - { - '_geo_distance': { - 'center_point': { - 'lat': 29.49136, - 'lon': -82.50622 - }, - 'order': 'asc', - 'distance_type': 'plane' - } - } - ], - 'size': vs.size, - 'track_scores': true -}; diff --git a/test/unit/fixture/reverse_with_layer_filtering_non_coarse_subset.js b/test/unit/fixture/reverse_with_layer_filtering_non_coarse_subset.js deleted file mode 100644 index aba18b08..00000000 --- a/test/unit/fixture/reverse_with_layer_filtering_non_coarse_subset.js +++ /dev/null @@ -1,41 +0,0 @@ -var vs = require('../../../query/reverse_defaults'); - -module.exports = { - 'query': { - 'bool': { - 'filter': [ - { - 'geo_distance': { - 'distance': '3km', - 'distance_type': 'plane', - 'optimize_bbox': 'indexed', - 'center_point': { - 'lat': 29.49136, - 'lon': -82.50622 - } - } - }, - { - 'terms': { - 'layer': ['venue', 'street'] - } - } - ] - } - }, - 'sort': [ - '_score', - { - '_geo_distance': { - 'center_point': { - 'lat': 29.49136, - 'lon': -82.50622 - }, - 'order': 'asc', - 'distance_type': 'plane' - } - } - ], - 'size': vs.size, - 'track_scores': true -}; diff --git a/test/unit/fixture/reverse_with_source_filtering.js b/test/unit/fixture/reverse_with_source_filtering.js deleted file mode 100644 index b2bcb0d9..00000000 --- a/test/unit/fixture/reverse_with_source_filtering.js +++ /dev/null @@ -1,46 +0,0 @@ -var vs = require('../../../query/reverse_defaults'); - -module.exports = { - 'query': { - 'bool': { - 'filter': [ - { - 'geo_distance': { - 'distance': '3km', - 'distance_type': 'plane', - 'optimize_bbox': 'indexed', - 'center_point': { - 'lat': 29.49136, - 'lon': -82.50622 - } - } - }, - { - 'terms': { - 'source': ['test'] - } - }, - { - 'terms': { - 'layer': ['venue', 'address', 'street'] - } - } - ] - } - }, - 'sort': [ - '_score', - { - '_geo_distance': { - 'center_point': { - 'lat': 29.49136, - 'lon': -82.50622 - }, - 'order': 'asc', - 'distance_type': 'plane' - } - } - ], - 'size': vs.size, - 'track_scores': true -}; diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index 0e47eaed..78c98e0c 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -2,6 +2,7 @@ const generate = require('../../../query/reverse'); const _ = require('lodash'); const proxyquire = require('proxyquire').noCallThru(); const MockQuery = require('./MockQuery'); +const all_layers = require('../../../helper/type_mapping').layers; // helper for canned views const views = { @@ -207,7 +208,7 @@ module.exports.tests.layers = (test, common) => { test('non-empty array clean.layers should only set non-coarse layers in vs', t => { const clean = { - sources: ['source 1', 'source 2'] + layers: all_layers }; const query = proxyquire('../../../query/reverse', { @@ -221,7 +222,282 @@ module.exports.tests.layers = (test, common) => { './reverse_defaults': {} })(clean); - t.deepEquals(query.body.vs.var('sources').toString(), ['source 1', 'source 2']); + t.deepEquals(query.body.vs.var('layers').toString(), ['address', 'venue', 'street']); + t.end(); + + }); + +}; + +module.exports.tests.focus_point = (test, common) => { + test('numeric point.lat and non-numeric point.lon should not add focus:point:* fields', t => { + const clean = { + 'point.lat': 12.121212, + 'point.lon': 'this is non-numeric' + }; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.notOk(query.body.vs.isset('focus:point:lat')); + t.notOk(query.body.vs.isset('focus:point:lon')); + t.end(); + + }); + + test('non-numeric point.lat and numeric point.lon should not add focus:point:* fields', t => { + const clean = { + 'point.lat': 'this is non-numeric', + 'point.lon': 21.212121 + }; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.notOk(query.body.vs.isset('focus:point:lat')); + t.notOk(query.body.vs.isset('focus:point:lon')); + t.end(); + + }); + + test('numeric point.lat and point.lon should add focus:point:* fields', t => { + const clean = { + 'point.lat': 12.121212, + 'point.lon': 21.212121 + }; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.deepEquals(query.body.vs.var('focus:point:lat').toString(), 12.121212); + t.deepEquals(query.body.vs.var('focus:point:lon').toString(), 21.212121); + t.end(); + + }); + +}; + +module.exports.tests.boundary_circle = (test, common) => { + test('numeric lat and non-numeric lon should not add boundary:circle:* fields', t => { + const clean = { + 'boundary.circle.lat': 12.121212, + 'boundary.circle.lon': 'this is non-numeric' + }; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.notOk(query.body.vs.isset('boundary:circle:lat')); + t.notOk(query.body.vs.isset('boundary:circle:lon')); + t.notOk(query.body.vs.isset('boundary:circle:radius')); + t.end(); + + }); + + test('non-numeric lat and numeric lon should not add boundary:circle:* fields', t => { + const clean = { + 'boundary.circle.lat': 'this is non-numeric', + 'boundary.circle.lon': 21.212121 + }; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.notOk(query.body.vs.isset('boundary:circle:lat')); + t.notOk(query.body.vs.isset('boundary:circle:lon')); + t.notOk(query.body.vs.isset('boundary:circle:radius')); + t.end(); + + }); + + test('radius not supplied should default to value from reverse_defaults', t => { + const clean = { + 'boundary.circle.lat': 12.121212, + 'boundary.circle.lon': 21.212121 + }; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': { + 'boundary:circle:radius': 17 + } + })(clean); + + t.deepEquals(query.body.vs.var('boundary:circle:lat').toString(), 12.121212); + t.deepEquals(query.body.vs.var('boundary:circle:lon').toString(), 21.212121); + t.deepEquals(query.body.vs.var('boundary:circle:radius').toString(), 17); + t.end(); + + }); + + test('numeric radius supplied should be used instead of value from reverse_defaults', t => { + const clean = { + 'boundary.circle.lat': 12.121212, + 'boundary.circle.lon': 21.212121, + 'boundary.circle.radius': 17 + }; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': { + 'boundary:circle:radius': 18 + } + })(clean); + + t.deepEquals(query.body.vs.var('boundary:circle:lat').toString(), 12.121212); + t.deepEquals(query.body.vs.var('boundary:circle:lon').toString(), 21.212121); + t.deepEquals(query.body.vs.var('boundary:circle:radius').toString(), '17km'); + t.end(); + + }); + + test('non-numeric radius supplied should not set any boundary:circle:radius', t => { + const clean = { + 'boundary.circle.lat': 12.121212, + 'boundary.circle.lon': 21.212121, + 'boundary.circle.radius': 'this is non-numeric' + }; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': { + 'boundary:circle:radius': 18 + } + })(clean); + + t.deepEquals(query.body.vs.var('boundary:circle:lat').toString(), 12.121212); + t.deepEquals(query.body.vs.var('boundary:circle:lon').toString(), 21.212121); + t.deepEquals(query.body.vs.var('boundary:circle:radius').toString(), 18); + t.end(); + + }); + +}; + +module.exports.tests.boundary_country = (test, common) => { + test('non-string boundary.country should not set boundary:country', t => { + [17, undefined, {}, [], true, null].forEach(value => { + const clean = { + 'boundary.country': value + }; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.notOk(query.body.vs.isset('boundary:country')); + }); + + t.end(); + + }); + + test('string boundary.country should set boundary:country', t => { + const clean = { + 'boundary.country': 'boundary country value' + }; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.deepEquals(query.body.vs.var('boundary:country').toString(), 'boundary country value'); + t.end(); + + }); + +}; + +module.exports.tests.categories = (test, common) => { + test('categories supplied should set input:categories', t => { + const clean = { + categories: 'categories value' + }; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.deepEquals(query.body.vs.var('input:categories').toString(), 'categories value'); t.end(); }); diff --git a/test/unit/run.js b/test/unit/run.js index cfe60255..5631b9f3 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -9,101 +9,101 @@ var common = { }; var tests = [ - // require('./app'), - // require('./schema'), - // require('./controller/coarse_reverse'), - // require('./controller/index'), - // require('./controller/libpostal'), - // require('./controller/place'), - // require('./controller/placeholder'), - // require('./controller/search'), - // require('./controller/search_with_ids'), - // require('./controller/predicates/has_parsed_text_properties'), - // require('./controller/predicates/has_request_parameter'), - // require('./controller/predicates/has_response_data'), - // require('./controller/predicates/has_results_at_layers'), - // require('./controller/predicates/has_request_parameter'), - // require('./controller/predicates/has_request_errors'), - // require('./controller/predicates/is_addressit_parse'), - // require('./controller/predicates/is_admin_only_analysis'), - // require('./controller/predicates/is_coarse_reverse'), - // require('./controller/predicates/is_only_non_admin_layers'), - // require('./controller/predicates/is_request_sources_only_whosonfirst'), - // require('./helper/debug'), - // require('./helper/diffPlaces'), - // require('./helper/geojsonify_place_details'), - // require('./helper/geojsonify'), - // require('./helper/logging'), - // require('./helper/type_mapping'), - // require('./helper/sizeCalculator'), - // require('./helper/stackTraceLine'), - // require('./middleware/access_log'), - // require('./middleware/accuracy'), - // require('./middleware/assignLabels'), - // require('./middleware/confidenceScore'), - // require('./middleware/confidenceScoreFallback'), - // require('./middleware/confidenceScoreReverse'), - // require('./middleware/changeLanguage'), - // require('./middleware/distance'), - // require('./middleware/interpolate'), - // require('./middleware/localNamingConventions'), - // require('./middleware/dedupe'), - // require('./middleware/parseBBox'), - // require('./middleware/sendJSON'), - // require('./middleware/normalizeParentIds'), - // require('./middleware/sortResponseData'), - // require('./middleware/trimByGranularity'), - // require('./middleware/trimByGranularityStructured'), - // require('./middleware/requestLanguage'), - // require('./query/address_search_using_ids'), - // require('./query/autocomplete'), - // require('./query/autocomplete_defaults'), - // require('./query/search_defaults'), - // require('./query/reverse_defaults'), + require('./app'), + require('./schema'), + require('./controller/coarse_reverse'), + require('./controller/index'), + require('./controller/libpostal'), + require('./controller/place'), + require('./controller/placeholder'), + require('./controller/search'), + require('./controller/search_with_ids'), + require('./controller/predicates/has_parsed_text_properties'), + require('./controller/predicates/has_request_parameter'), + require('./controller/predicates/has_response_data'), + require('./controller/predicates/has_results_at_layers'), + require('./controller/predicates/has_request_parameter'), + require('./controller/predicates/has_request_errors'), + require('./controller/predicates/is_addressit_parse'), + require('./controller/predicates/is_admin_only_analysis'), + require('./controller/predicates/is_coarse_reverse'), + require('./controller/predicates/is_only_non_admin_layers'), + require('./controller/predicates/is_request_sources_only_whosonfirst'), + require('./helper/debug'), + require('./helper/diffPlaces'), + require('./helper/geojsonify_place_details'), + require('./helper/geojsonify'), + require('./helper/logging'), + require('./helper/type_mapping'), + require('./helper/sizeCalculator'), + require('./helper/stackTraceLine'), + require('./middleware/access_log'), + require('./middleware/accuracy'), + require('./middleware/assignLabels'), + require('./middleware/confidenceScore'), + require('./middleware/confidenceScoreFallback'), + require('./middleware/confidenceScoreReverse'), + require('./middleware/changeLanguage'), + require('./middleware/distance'), + require('./middleware/interpolate'), + require('./middleware/localNamingConventions'), + require('./middleware/dedupe'), + require('./middleware/parseBBox'), + require('./middleware/sendJSON'), + require('./middleware/normalizeParentIds'), + require('./middleware/sortResponseData'), + require('./middleware/trimByGranularity'), + require('./middleware/trimByGranularityStructured'), + require('./middleware/requestLanguage'), + require('./query/address_search_using_ids'), + require('./query/autocomplete'), + require('./query/autocomplete_defaults'), + require('./query/search_defaults'), + require('./query/reverse_defaults'), require('./query/reverse'), - // require('./query/search'), - // require('./query/search_original'), - // require('./query/structured_geocoding'), - // require('./query/text_parser'), - // require('./sanitizer/_boundary_country'), - // require('./sanitizer/_debug'), - // require('./sanitizer/_flag_bool'), - // require('./sanitizer/_geonames_deprecation'), - // require('./sanitizer/_geonames_warnings'), - // require('./sanitizer/_geo_common'), - // require('./sanitizer/_geo_reverse'), - // require('./sanitizer/_groups'), - // require('./sanitizer/_ids'), - // require('./sanitizer/_iso2_to_iso3'), - // require('./sanitizer/_layers'), - // require('./sanitizer/_location_bias'), - // require('./sanitizer/_city_name_standardizer'), - // require('./sanitizer/_request_language'), - // require('./sanitizer/_single_scalar_parameters'), - // require('./sanitizer/_size'), - // require('./sanitizer/_sources'), - // require('./sanitizer/_sources_and_layers'), - // require('./sanitizer/_synthesize_analysis'), - // require('./sanitizer/_text'), - // require('./sanitizer/_text_addressit'), - // require('./sanitizer/_tokenizer'), - // require('./sanitizer/_deprecate_quattroshapes'), - // require('./sanitizer/_categories'), - // require('./sanitizer/nearby'), - // require('./sanitizer/autocomplete'), - // require('./sanitizer/structured_geocoding'), - // require('./sanitizer/place'), - // require('./sanitizer/reverse'), - // require('./sanitizer/sanitizeAll'), - // require('./sanitizer/search'), - // require('./sanitizer/defer_to_addressit'), - // require('./sanitizer/wrap'), - // require('./service/configurations/Interpolation'), - // require('./service/configurations/Language'), - // require('./service/configurations/PlaceHolder'), - // require('./service/configurations/PointInPolygon'), - // require('./service/mget'), - // require('./service/search') + require('./query/search'), + require('./query/search_original'), + require('./query/structured_geocoding'), + require('./query/text_parser'), + require('./sanitizer/_boundary_country'), + require('./sanitizer/_debug'), + require('./sanitizer/_flag_bool'), + require('./sanitizer/_geonames_deprecation'), + require('./sanitizer/_geonames_warnings'), + require('./sanitizer/_geo_common'), + require('./sanitizer/_geo_reverse'), + require('./sanitizer/_groups'), + require('./sanitizer/_ids'), + require('./sanitizer/_iso2_to_iso3'), + require('./sanitizer/_layers'), + require('./sanitizer/_location_bias'), + require('./sanitizer/_city_name_standardizer'), + require('./sanitizer/_request_language'), + require('./sanitizer/_single_scalar_parameters'), + require('./sanitizer/_size'), + require('./sanitizer/_sources'), + require('./sanitizer/_sources_and_layers'), + require('./sanitizer/_synthesize_analysis'), + require('./sanitizer/_text'), + require('./sanitizer/_text_addressit'), + require('./sanitizer/_tokenizer'), + require('./sanitizer/_deprecate_quattroshapes'), + require('./sanitizer/_categories'), + require('./sanitizer/nearby'), + require('./sanitizer/autocomplete'), + require('./sanitizer/structured_geocoding'), + require('./sanitizer/place'), + require('./sanitizer/reverse'), + require('./sanitizer/sanitizeAll'), + require('./sanitizer/search'), + require('./sanitizer/defer_to_addressit'), + require('./sanitizer/wrap'), + require('./service/configurations/Interpolation'), + require('./service/configurations/Language'), + require('./service/configurations/PlaceHolder'), + require('./service/configurations/PointInPolygon'), + require('./service/mget'), + require('./service/search') ]; tests.map(function(t) { From bc6401842c9046af3ea5984523f6b8d111becfb2 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 23 Oct 2017 16:16:30 -0400 Subject: [PATCH 10/24] Add friendly, coarse specific error message to reverse This error message is updated to mention the coarse-reverse only nature of Geonames support removal. It also links to the [tracking ticket](https://github.com/pelias/acceptance-tests/pull/447) for Geonames removal which should help provide more context to users. --- sanitizer/_geonames_deprecation.js | 8 ++++---- test/unit/sanitizer/_geonames_deprecation.js | 8 +++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/sanitizer/_geonames_deprecation.js b/sanitizer/_geonames_deprecation.js index f6608e96..2cdef1d1 100644 --- a/sanitizer/_geonames_deprecation.js +++ b/sanitizer/_geonames_deprecation.js @@ -7,6 +7,8 @@ const _ = require('lodash'); * _non-coarse_ reverse. **/ +const coarse_reverse_message ='coarse /reverse does not support geonames. See https://github.com/pelias/pelias/issues/675 for more info'; + function _sanitize( raw, clean, opts ) { // error & warning messages const messages = { errors: [], warnings: [] }; @@ -15,18 +17,16 @@ function _sanitize( raw, clean, opts ) { const non_coarse_layers = ['address', 'street', 'venue']; const is_coarse_reverse = !_.isEmpty(clean.layers) && _.isEmpty(_.intersection(clean.layers, non_coarse_layers)); - if (!is_coarse_reverse) { return messages; } if (_.isEqual(clean.sources, ['geonames']) || _.isEqual(clean.sources, ['gn'])) { - messages.errors.push('/reverse does not support geonames'); + messages.errors.push(coarse_reverse_message); } else if (_.includes(clean.sources, 'geonames') || _.includes(clean.sources, 'gn')) { clean.sources = _.without(clean.sources, 'geonames', 'gn'); - messages.warnings.push('/reverse does not support geonames'); - + messages.warnings.push(coarse_reverse_message); } return messages; diff --git a/test/unit/sanitizer/_geonames_deprecation.js b/test/unit/sanitizer/_geonames_deprecation.js index 707c6774..13461e15 100644 --- a/test/unit/sanitizer/_geonames_deprecation.js +++ b/test/unit/sanitizer/_geonames_deprecation.js @@ -2,6 +2,8 @@ const sanitizer = require('../../../sanitizer/_geonames_deprecation')(); module.exports.tests = {}; +const coarse_reverse_message ='coarse /reverse does not support geonames. See https://github.com/pelias/pelias/issues/675 for more info'; + module.exports.tests.no_warnings_or_errors_conditions = (test, common) => { test('undefined sources should add neither warnings nor errors', (t) => { const clean = {}; @@ -52,7 +54,7 @@ module.exports.tests.error_conditions = (test, common) => { const messages = sanitizer.sanitize(undefined, clean); t.deepEquals(clean.sources, [sources]); - t.deepEquals(messages.errors, ['/reverse does not support geonames']); + t.deepEquals(messages.errors, [ coarse_reverse_message ]); t.deepEquals(messages.warnings, []); }); @@ -70,7 +72,7 @@ module.exports.tests.error_conditions = (test, common) => { const messages = sanitizer.sanitize(undefined, clean); t.deepEquals(clean.sources, [sources]); - t.deepEquals(messages.errors, ['/reverse does not support geonames']); + t.deepEquals(messages.errors, [ coarse_reverse_message ]); t.deepEquals(messages.warnings, []); }); @@ -91,7 +93,7 @@ module.exports.tests.warning_conditions = (test, common) => { t.deepEquals(clean.sources, ['another source', 'yet another source']); t.deepEquals(messages.errors, []); - t.deepEquals(messages.warnings, ['/reverse does not support geonames']); + t.deepEquals(messages.warnings, [ coarse_reverse_message ]); }); From c730513084d49b69d59f94b3d1ce99a1ae9d26d0 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Tue, 24 Oct 2017 19:17:58 +0000 Subject: [PATCH 11/24] fix(package): update pelias-microservice-wrapper to version 1.3.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 147c87e4..39698c60 100644 --- a/package.json +++ b/package.json @@ -58,7 +58,7 @@ "pelias-config": "2.13.0", "pelias-labels": "1.7.0", "pelias-logger": "0.3.0", - "pelias-microservice-wrapper": "1.2.1", + "pelias-microservice-wrapper": "1.3.0", "pelias-model": "5.2.0", "pelias-query": "9.1.0", "pelias-sorting": "1.0.1", From f16565df9e823b899b05790919769cfe9c4041da Mon Sep 17 00:00:00 2001 From: Peter Backx Date: Sat, 28 Oct 2017 21:51:27 +0200 Subject: [PATCH 12/24] Verify min and max longitude and latitude Otherwise this can potentially lead to a fairly weird Elasticsearch error mentioned in https://github.com/pelias/pelias/issues/656 --- sanitizer/_geo_common.js | 12 ++++++++++++ test/unit/sanitizer/_geo_common.js | 19 +++++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/sanitizer/_geo_common.js b/sanitizer/_geo_common.js index d7620b08..a3b15865 100644 --- a/sanitizer/_geo_common.js +++ b/sanitizer/_geo_common.js @@ -39,6 +39,18 @@ function sanitize_rect( key_prefix, clean, raw, bbox_is_required ) { properties.forEach(function(prop) { sanitize_coord(prop, clean, raw, true); }); + + var min_lat = parseFloat( raw[key_prefix + '.' + 'min_lat'] ); + var max_lat = parseFloat( raw[key_prefix + '.' + 'max_lat'] ); + if (min_lat > max_lat) { + throw new Error( util.format( 'min_lat is larger than max_lat in \'%s\'', key_prefix ) ); + } + + var min_lon = parseFloat( raw[key_prefix + '.' + 'min_lon'] ); + var max_lon = parseFloat( raw[key_prefix + '.' + 'max_lon'] ); + if (min_lon > max_lon) { + throw new Error( util.format( 'min_lon is larger than max_lon in \'%s\'', key_prefix ) ); + } } /** diff --git a/test/unit/sanitizer/_geo_common.js b/test/unit/sanitizer/_geo_common.js index bdb58078..09c008ba 100644 --- a/test/unit/sanitizer/_geo_common.js +++ b/test/unit/sanitizer/_geo_common.js @@ -216,8 +216,8 @@ module.exports.tests.rect = function(test, common) { test('valid rect quad', function (t) { var clean = {}; var params = { - 'boundary.rect.min_lat': -40.659, - 'boundary.rect.max_lat': -41.614, + 'boundary.rect.min_lat': -41.614, + 'boundary.rect.max_lat': -40.659, 'boundary.rect.min_lon': 174.612, 'boundary.rect.max_lon': 176.333 }; @@ -283,6 +283,21 @@ module.exports.tests.rect = function(test, common) { t.end(); }); + test('invalid rect - max/min switched', function (t) { + var clean = {}; + var params = { + 'boundary.rect.max_lat': 52.2387, + 'boundary.rect.max_lon': 14.1367, + 'boundary.rect.min_lat': 52.7945, + 'boundary.rect.min_lon': 12.6398 + }; + var mandatory = false; + + t.throws( function() { + sanitize.sanitize_rect( 'boundary.rect', clean, params, mandatory ); + }); + t.end(); + }); }; module.exports.tests.circle = function(test, common) { From bd940ec691c61021ecad7134f6902045a620cb39 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Fri, 3 Nov 2017 20:53:42 +0000 Subject: [PATCH 13/24] fix(package): update pelias-sorting to version 1.1.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 39698c60..6b35dd7f 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,7 @@ "pelias-microservice-wrapper": "1.3.0", "pelias-model": "5.2.0", "pelias-query": "9.1.0", - "pelias-sorting": "1.0.1", + "pelias-sorting": "1.1.0", "pelias-text-analyzer": "1.10.2", "predicates": "^2.0.0", "retry": "^0.10.1", From cb59e305b0a78cd7438df1f5cb9c66ac6aa8c28b Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Mon, 6 Nov 2017 12:27:13 -0500 Subject: [PATCH 14/24] log number of streets to be interpolated --- middleware/interpolate.js | 2 ++ test/unit/middleware/interpolate.js | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/middleware/interpolate.js b/middleware/interpolate.js index a7d3e1d8..10c3f7e8 100644 --- a/middleware/interpolate.js +++ b/middleware/interpolate.js @@ -57,6 +57,8 @@ function setup(service, should_execute) { // perform interpolations asynchronously for all relevant hits const start = (new Date()).getTime(); + logger.info(`[interpolation] [street_results] count=${street_results.length}`); + // call the interpolation service asynchronously on every street result async.map(street_results, error_intercepting_service(service, req), (err, interpolation_results) => { diff --git a/test/unit/middleware/interpolate.js b/test/unit/middleware/interpolate.js index f020628d..6dbf1ae6 100644 --- a/test/unit/middleware/interpolate.js +++ b/test/unit/middleware/interpolate.js @@ -59,6 +59,7 @@ module.exports.tests.success_conditions = (test, common) => { controller(req, undefined, () => { t.notOk(logger.hasErrorMessages(), 'there shouldn\'t be any error messages'); t.ok(logger.isInfoMessage(/\[interpolation\] \[took\] \d+ ms/)); + t.ok(logger.isInfoMessage(/\[interpolation\] \[street_results\] count=0/)); t.end(); }); @@ -87,6 +88,7 @@ module.exports.tests.success_conditions = (test, common) => { controller(req, res, () => { t.notOk(logger.hasErrorMessages(), 'there shouldn\'t be any error messages'); t.ok(logger.isInfoMessage(/\[interpolation\] \[took\] \d+ ms/)); + t.ok(logger.isInfoMessage(/\[interpolation\] \[street_results\] count=0/)); t.deepEquals(res, {}); @@ -201,6 +203,7 @@ module.exports.tests.success_conditions = (test, common) => { controller(req, res, () => { t.notOk(logger.hasErrorMessages(), 'there shouldn\'t be any error messages'); t.ok(logger.isInfoMessage(/\[interpolation\] \[took\] \d+ ms/), 'timing should be info-logged'); + t.ok(logger.isInfoMessage(/\[interpolation\] \[street_results\] count=3/), 'street count should be info-logged'); // test debug messages very vaguely to avoid brittle tests t.ok(logger.isDebugMessage(/^\[interpolation\] \[hit\] this is req.clean.parsed_text \{.+?\}$/), @@ -375,6 +378,7 @@ module.exports.tests.success_conditions = (test, common) => { '[middleware:interpolation] id 3 produced an error object' ]); t.ok(logger.isInfoMessage(/\[interpolation\] \[took\] \d+ ms/), 'timing should be info-logged'); + t.ok(logger.isInfoMessage(/\[interpolation\] \[street_results\] count=4/), 'street count should be info-logged'); // test debug messages very vaguely to avoid brittle tests t.ok(logger.isDebugMessage(/^\[interpolation\] \[hit\] this is req.clean.parsed_text \{.+?\}$/), @@ -491,6 +495,7 @@ module.exports.tests.success_conditions = (test, common) => { controller(req, res, () => { t.notOk(logger.hasErrorMessages(), 'there shouldn\'t be any error messages'); t.ok(logger.isInfoMessage(/\[interpolation\] \[took\] \d+ ms/)); + t.ok(logger.isInfoMessage(/\[interpolation\] \[street_results\] count=1/), 'street count should be info-logged'); t.deepEquals(res, { data: [ @@ -579,6 +584,7 @@ module.exports.tests.success_conditions = (test, common) => { controller(req, res, () => { t.notOk(logger.hasErrorMessages(), 'there shouldn\'t be any error messages'); t.ok(logger.isInfoMessage(/\[interpolation\] \[took\] \d+ ms/)); + t.ok(logger.isInfoMessage(/\[interpolation\] \[street_results\] count=2/), 'street count should be info-logged'); // test debug messages very vaguely to avoid brittle tests t.ok(logger.isDebugMessage('[interpolation] [miss] this is req.clean.parsed_text')); @@ -679,6 +685,7 @@ module.exports.tests.success_conditions = (test, common) => { controller(req, res, () => { t.notOk(logger.hasErrorMessages(), 'there shouldn\'t be any error messages'); t.ok(logger.isInfoMessage(/\[interpolation\] \[took\] \d+ ms/)); + t.ok(logger.isInfoMessage(/\[interpolation\] \[street_results\] count=2/), 'street count should be info-logged'); // test debug messages very vaguely to avoid brittle tests t.ok(logger.isDebugMessage('[interpolation] [miss] this is req.clean.parsed_text')); From bdd6573118b09bf927e5ed56858864d34984c107 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Wed, 1 Nov 2017 15:02:13 +0000 Subject: [PATCH 15/24] fix(package): update pelias-query to version 9.1.1 Closes #844 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6b35dd7f..ca0c1908 100644 --- a/package.json +++ b/package.json @@ -60,7 +60,7 @@ "pelias-logger": "0.3.0", "pelias-microservice-wrapper": "1.3.0", "pelias-model": "5.2.0", - "pelias-query": "9.1.0", + "pelias-query": "9.1.1", "pelias-sorting": "1.1.0", "pelias-text-analyzer": "1.10.2", "predicates": "^2.0.0", From f023facc97ee5b570346e5dd24f8391b61dfeb44 Mon Sep 17 00:00:00 2001 From: "greenkeeper[bot]" Date: Mon, 6 Nov 2017 19:31:59 +0000 Subject: [PATCH 16/24] chore(package): update nsp to version 3.0.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ca0c1908..3dbdedf7 100644 --- a/package.json +++ b/package.json @@ -74,7 +74,7 @@ "istanbul": "^0.4.2", "jshint": "^2.5.6", "npm-check": "git://github.com/orangejulius/npm-check.git#disable-update-check", - "nsp": "^2.2.0", + "nsp": "^3.0.0", "pelias-mock-logger": "1.2.0", "precommit-hook": "^3.0.0", "proxyquire": "^1.7.10", From 28f5aa5d6baa6fe67bda9730e3b0878f6f78a61e Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 9 Nov 2017 14:39:17 -0500 Subject: [PATCH 17/24] Update npm audit script The location of the nsp binary changed, probably a while ago. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 3dbdedf7..80243ad1 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "license": "MIT", "main": "index.js", "scripts": { - "audit": "npm shrinkwrap; node node_modules/nsp/bin/nspCLI.js audit-shrinkwrap; rm npm-shrinkwrap.json;", + "audit": "npm shrinkwrap; node node_modules/nsp/bin/nsp check; rm npm-shrinkwrap.json;", "ciao": "node node_modules/ciao/bin/ciao -c test/ciao.json test/ciao", "coverage": "node_modules/.bin/istanbul cover test/unit/run.js", "docs": "./bin/generate-docs", From e9eed8d4ea85fd4404d20c81be339f41b0a020d0 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 14 Nov 2017 11:27:29 -0500 Subject: [PATCH 18/24] disable package-lock --- .npmrc | 1 + 1 file changed, 1 insertion(+) create mode 100644 .npmrc diff --git a/.npmrc b/.npmrc new file mode 100644 index 00000000..43c97e71 --- /dev/null +++ b/.npmrc @@ -0,0 +1 @@ +package-lock=false From f56deffb8f9e8cf86fe155bcb5285489030d234d Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 14 Nov 2017 11:28:13 -0500 Subject: [PATCH 19/24] allow libpostal configuration in schema w/other services --- schema.js | 5 +++++ test/unit/schema.js | 15 +++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/schema.js b/schema.js index 3203bb7d..440da818 100644 --- a/schema.js +++ b/schema.js @@ -41,6 +41,11 @@ module.exports = Joi.object().keys({ url: Joi.string().uri({ scheme: /https?/ }), timeout: Joi.number().integer().optional().default(250).min(0), retries: Joi.number().integer().optional().default(3).min(0), + }).unknown(false).requiredKeys('url'), + libpostal: Joi.object().keys({ + url: Joi.string().uri({ scheme: /https?/ }), + timeout: Joi.number().integer().optional().default(250).min(0), + retries: Joi.number().integer().optional().default(3).min(0), }).unknown(false).requiredKeys('url') }).unknown(false).default({}), // default api.services to an empty object defaultParameters: Joi.object().keys({ diff --git a/test/unit/schema.js b/test/unit/schema.js index 7c62f109..1278ee59 100644 --- a/test/unit/schema.js +++ b/test/unit/schema.js @@ -28,6 +28,9 @@ module.exports.tests.completely_valid = (test, common) => { }, interpolation: { url: 'http://localhost' + }, + libpostal: { + url: 'http://localhost' } }, defaultParameters: { @@ -419,7 +422,7 @@ module.exports.tests.api_validation = (test, common) => { }); // api.pipService has been moved to api.services.pip.url - test('any api.pipService value should be allowed', (t) => { + test('any api.pipService value should fail', (t) => { [null, 17, {}, [], true, 'http://localhost'].forEach((value) => { var config = { api: { @@ -543,7 +546,7 @@ module.exports.tests.api_services_validation = (test, common) => { module.exports.tests.service_validation = (test, common) => { // these tests apply for all the individual service definitions - const services = ['pip', 'placeholder', 'interpolation']; + const services = ['pip', 'placeholder', 'interpolation', 'libpostal']; test('timeout and retries not specified should default to 250 and 3', (t) => { services.forEach(service => { @@ -572,7 +575,7 @@ module.exports.tests.service_validation = (test, common) => { }); - test('when api.services.service is defined, url is required', (t) => { + test('when api.services. is defined, url is required', (t) => { services.forEach(service => { const config = { api: { @@ -596,7 +599,7 @@ module.exports.tests.service_validation = (test, common) => { }); - test('non-string api.services.pip.url should throw error', (t) => { + test('non-string api.services..url should throw error', (t) => { services.forEach(service => { [null, 17, {}, [], true].forEach(value => { const config = { @@ -626,7 +629,7 @@ module.exports.tests.service_validation = (test, common) => { }); - test('non-http/https api.services.pip.url should throw error', (t) => { + test('non-http/https api.services..url should throw error', (t) => { services.forEach(service => { ['ftp', 'git', 'unknown'].forEach((scheme) => { const config = { @@ -656,7 +659,7 @@ module.exports.tests.service_validation = (test, common) => { }); - test('non-url children of api.services.pip should be disallowed', (t) => { + test('non-url/timeout/retries children of api.services. should be disallowed', (t) => { services.forEach(service => { const config = { api: { From b5e48afb352b651f2c80783aceef40931302c89b Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 14 Nov 2017 11:29:54 -0500 Subject: [PATCH 20/24] remove text-analyzer address operation --- sanitizer/_synthesize_analysis.js | 44 +----- test/unit/sanitizer/_synthesize_analysis.js | 153 +------------------- 2 files changed, 4 insertions(+), 193 deletions(-) diff --git a/sanitizer/_synthesize_analysis.js b/sanitizer/_synthesize_analysis.js index 3822d9d4..da67c752 100644 --- a/sanitizer/_synthesize_analysis.js +++ b/sanitizer/_synthesize_analysis.js @@ -1,5 +1,4 @@ const _ = require('lodash'); -const text_analyzer = require('pelias-text-analyzer'); const fields = { 'venue': 'query', @@ -17,20 +16,6 @@ function normalizeWhitespaceToSingleSpace(val) { return _.replace(_.trim(val), /\s+/g, ' '); } -function isPostalCodeOnly(parsed_text) { - return Object.keys(parsed_text).length === 1 && - parsed_text.hasOwnProperty('postalcode'); -} - -// figure out which field contains the probable house number, prefer number -// libpostal parses some inputs, like `3370 cobbe ave`, as a postcode+street -// so because we're treating the entire field as a street address, it's safe -// to assume that an identified postcode is actually a house number. -function getHouseNumberField(analyzed_address) { - // return the first field available in the libpostal response, undefined if none - return _.find(['number', 'postalcode'], _.partial(_.has, analyzed_address)); -} - function _sanitize( raw, clean ){ // error & warning messages @@ -51,35 +36,8 @@ function _sanitize( raw, clean ){ `at least one of the following fields is required: ${Object.keys(fields).join(', ')}`); } - if (clean.parsed_text.hasOwnProperty('address')) { - const analyzed_address = text_analyzer.parse(clean.parsed_text.address); - - const house_number_field = getHouseNumberField(analyzed_address); - - // if we're fairly certain that libpostal identified a house number - // (from either the house_number or postcode field), place it into the - // number field and remove the first instance of that value from address - // and assign to street - // eg - '1090 N Charlotte St' becomes number=1090 and street=N Charlotte St - if (house_number_field) { - clean.parsed_text.number = analyzed_address[house_number_field]; - - // remove the first instance of the number and trim whitespace - clean.parsed_text.street = _.trim(_.replace(clean.parsed_text.address, clean.parsed_text.number, '')); - - } else { - // otherwise no house number was identifiable, so treat the entire input - // as a street - clean.parsed_text.street = clean.parsed_text.address; - - } - - // the address field no longer means anything since it's been parsed, so remove it - delete clean.parsed_text.address; - - } - return messages; + } function _expected() { diff --git a/test/unit/sanitizer/_synthesize_analysis.js b/test/unit/sanitizer/_synthesize_analysis.js index 64243dc9..19ff57bd 100644 --- a/test/unit/sanitizer/_synthesize_analysis.js +++ b/test/unit/sanitizer/_synthesize_analysis.js @@ -1,18 +1,14 @@ const _ = require('lodash'); const proxyquire = require('proxyquire').noCallThru(); +const sanitizer = require('../../../sanitizer/_synthesize_analysis'); module.exports.tests = {}; module.exports.tests.text_parser = function(test, common) { test('all variables should be parsed', function(t) { - var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { - 'pelias-text-analyzer': { parse: function(query) { - t.fail('parse should not have been called'); - } - }}); - const raw = { venue: ' \t venue \t value \t ', + address: ' \t address \t value \t ', neighbourhood: ' \t neighbourhood \t value \t ', borough: ' \t borough \t value \t ', locality: ' \t locality \t value \t ', @@ -27,6 +23,7 @@ module.exports.tests.text_parser = function(test, common) { const expected_clean = { parsed_text: { query: 'venue value', + address: 'address value', neighbourhood: 'neighbourhood value', borough: 'borough value', city: 'locality value', @@ -47,12 +44,6 @@ module.exports.tests.text_parser = function(test, common) { }); test('non-string and blank string values should be treated as not supplied', function(t) { - var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { - 'pelias-text-analyzer': { parse: function(query) { - t.fail('parse should not have been called'); - } - }}); - // helper to return a random value that's considered invalid function getInvalidValue() { return _.sample([{}, [], false, '', ' \t ', 17, undefined]); @@ -87,12 +78,6 @@ module.exports.tests.text_parser = function(test, common) { }); test('no supplied fields should return error', function(t) { - var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { - 'pelias-text-analyzer': { parse: function(query) { - t.fail('parse should not have been called'); - } - }}); - const raw = {}; const clean = {}; @@ -110,12 +95,6 @@ module.exports.tests.text_parser = function(test, common) { }); test('postalcode-only parsed_text should return error', function(t) { - var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { - 'pelias-text-analyzer': { parse: function(query) { - t.fail('parse should not have been called'); - } - }}); - const raw = { postalcode: 'postalcode value' }; @@ -137,132 +116,6 @@ module.exports.tests.text_parser = function(test, common) { }); - test('text_analyzer identifying house number should extract it and street', function(t) { - var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { - 'pelias-text-analyzer': { parse: function(query) { - t.equals(query, 'Number Value Street Value Number Value'); - - return { - number: 'Number Value' - }; - } - }}); - - const raw = { - address: 'Number Value Street Value Number Value' - }; - - const clean = {}; - - const expected_clean = { - parsed_text: { - number: 'Number Value', - street: 'Street Value Number Value' - } - }; - - const messages = sanitizer().sanitize(raw, clean); - - t.deepEquals(clean, expected_clean); - t.deepEquals(messages.errors, [], 'no errors'); - t.deepEquals(messages.warnings, [], 'no warnings'); - t.end(); - - }); - - test('text_analyzer identifying postalcode but not house number should assign to number and remove from address', function(t) { - var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { - 'pelias-text-analyzer': { parse: function(query) { - t.equals(query, 'Number Value Street Value Number Value'); - - return { - postalcode: 'Number Value' - }; - } - }}); - - const raw = { - address: 'Number Value Street Value Number Value' - }; - - const clean = {}; - - const expected_clean = { - parsed_text: { - number: 'Number Value', - street: 'Street Value Number Value' - } - }; - - const messages = sanitizer().sanitize(raw, clean); - - t.deepEquals(clean, expected_clean); - t.deepEquals(messages.errors, [], 'no errors'); - t.deepEquals(messages.warnings, [], 'no warnings'); - t.end(); - - }); - - test('text_analyzer not revealing possible number should move address to street', function(t) { - var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { - 'pelias-text-analyzer': { parse: function(query) { - t.equals(query, 'Street Value'); - - return {}; - } - }}); - - const raw = { - address: 'Street Value' - }; - - const clean = {}; - - const expected_clean = { - parsed_text: { - street: 'Street Value' - } - }; - - const messages = sanitizer().sanitize(raw, clean); - - t.deepEquals(clean, expected_clean); - t.deepEquals(messages.errors, [], 'no errors'); - t.deepEquals(messages.warnings, [], 'no warnings'); - t.end(); - - }); - - test('text_analyzer returning undefined on address resolution should treat as if no house number field was found', t => { - var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { - 'pelias-text-analyzer': { parse: function(query) { - t.equals(query, 'Street Value'); - - return undefined; - } - }}); - - const raw = { - address: 'Street Value' - }; - - const clean = {}; - - const expected_clean = { - parsed_text: { - street: 'Street Value' - } - }; - - const messages = sanitizer().sanitize(raw, clean); - - t.deepEquals(clean, expected_clean); - t.deepEquals(messages.errors, [], 'no errors'); - t.deepEquals(messages.warnings, [], 'no warnings'); - t.end(); - - }); - test('return an array of expected parameters in object form for validation', function (t) { const sanitizer = require('../../../sanitizer/_synthesize_analysis'); const expected = [ From e0c25d0f573e34203a8d503c3e31f8396a56f55a Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 14 Nov 2017 11:44:20 -0500 Subject: [PATCH 21/24] convert libpostal calls to a microservice --- controller/libpostal.js | 105 ++++- controller/structured_libpostal.js | 75 +++ package.json | 1 - routes/v1.js | 23 +- service/configurations/Libpostal.js | 33 ++ test/unit/controller/libpostal.js | 343 +++++++------- test/unit/controller/structured_libpostal.js | 440 ++++++++++++++++++ test/unit/run.js | 2 + test/unit/service/configurations/Libpostal.js | 99 ++++ 9 files changed, 945 insertions(+), 176 deletions(-) create mode 100644 controller/structured_libpostal.js create mode 100644 service/configurations/Libpostal.js create mode 100644 test/unit/controller/structured_libpostal.js create mode 100644 test/unit/service/configurations/Libpostal.js diff --git a/controller/libpostal.js b/controller/libpostal.js index 23c82f85..7ecbe3b6 100644 --- a/controller/libpostal.js +++ b/controller/libpostal.js @@ -1,31 +1,108 @@ -const text_analyzer = require('pelias-text-analyzer'); const _ = require('lodash'); const iso3166 = require('iso3166-1'); const Debug = require('../helper/debug'); const debugLog = new Debug('controller:libpostal'); +const logger = require('pelias-logger').get('api'); -function setup(should_execute) { +// mapping object from libpostal fields to pelias fields +var field_mapping = { + island: 'island', + category: 'category', + house: 'query', + house_number: 'number', + road: 'street', + suburb: 'neighbourhood', + city_district: 'borough', + city: 'city', + state_district: 'county', + state: 'state', + postcode: 'postalcode', + country: 'country' +}; + +// This controller calls the hosted libpostal service and converts the response +// to a generic format for later use. The hosted service returns an array like: +// +// ``` +// [ +// { +// label: 'house_number', +// value: '30' +// }, +// { +// label: 'road', +// value: 'west 26th street' +// }, +// { +// label: 'city', +// value: 'new york' +// }, +// { +// label: 'state', +// value: 'ny' +// } +//] +// ``` +// +// where `label` can be any of (currently): +// - house (generally interpreted as unknown, treated by pelias like a query term) +// - category (like "restaurants") +// - house_number +// - road +// - unit (apt or suite #) +// - suburb (like a neighbourhood) +// - city +// - city_district (like an NYC borough) +// - state_district (like a county) +// - state +// - postcode +// - country +// +// The Pelias query module is not concerned with unit. +// +function setup(libpostalService, should_execute) { function controller( req, res, next ){ // bail early if req/res don't pass conditions for execution if (!should_execute(req, res)) { return next(); } + const initialTime = debugLog.beginTimer(req); - // parse text with query parser - const parsed_text = text_analyzer.parse(req.clean.text); - if (parsed_text !== undefined) { - // if a known ISO2 country was parsed, convert it to ISO3 - if (_.has(parsed_text, 'country') && iso3166.is2(_.toUpper(parsed_text.country))) { - parsed_text.country = iso3166.to3(_.toUpper(parsed_text.country)); + libpostalService(req, (err, response) => { + if (err) { + // push err.message or err onto req.errors + req.errors.push( _.get(err, 'message', err) ); + + } else if (_.some(_.countBy(response, o => o.label), count => count > 1)) { + logger.warn(`discarding libpostal parse of '${req.clean.text}' due to duplicate field assignments`); + return next(); + + } else if (_.isEmpty(response)) { + return next(); + + } else { + req.clean.parser = 'libpostal'; + req.clean.parsed_text = response.reduce(function(o, f) { + if (field_mapping.hasOwnProperty(f.label)) { + o[field_mapping[f.label]] = f.value; + } + + return o; + }, {}); + + if (_.has(req.clean.parsed_text, 'country') && iso3166.is2(_.toUpper(req.clean.parsed_text.country))) { + req.clean.parsed_text.country = iso3166.to3(_.toUpper(req.clean.parsed_text.country)); + } + + debugLog.push(req, {parsed_text: req.clean.parsed_text}); + } - req.clean.parser = 'libpostal'; - req.clean.parsed_text = parsed_text; - debugLog.push(req, {parsed_text: req.clean.parsed_text}); - } - debugLog.stopTimer(req, initialTime); - return next(); + debugLog.stopTimer(req, initialTime); + return next(); + + }); } diff --git a/controller/structured_libpostal.js b/controller/structured_libpostal.js new file mode 100644 index 00000000..626bf945 --- /dev/null +++ b/controller/structured_libpostal.js @@ -0,0 +1,75 @@ +const _ = require('lodash'); +const Debug = require('../helper/debug'); +const debugLog = new Debug('controller:libpostal'); +const logger = require('pelias-logger').get('api'); + +// if there's a house_number in the libpostal response, return it +// otherwise return the postcode field (which may be undefined) +function findHouseNumberField(response) { + const house_number_field = response.find(f => f.label === 'house_number'); + + if (house_number_field) { + return house_number_field; + } + + return response.find(f => f.label === 'postcode'); + +} + +function setup(libpostalService, should_execute) { + function controller( req, res, next ){ + // bail early if req/res don't pass conditions for execution + if (!should_execute(req, res)) { + return next(); + } + + const initialTime = debugLog.beginTimer(req); + + libpostalService(req, (err, response) => { + if (err) { + // push err.message or err onto req.errors + req.errors.push( _.get(err, 'message', err) ); + + } else { + // figure out which field contains the probable house number, prefer house_number + // libpostal parses some inputs, like `3370 cobbe ave`, as a postcode+street + // so because we're treating the entire field as a street address, it's safe + // to assume that an identified postcode is actually a house number. + const house_number_field = findHouseNumberField(response); + + // if we're fairly certain that libpostal identified a house number + // (from either the house_number or postcode field), place it into the + // number field and remove the first instance of that value from address + // and assign to street + // eg - '1090 N Charlotte St' becomes number=1090 and street=N Charlotte St + if (house_number_field) { + req.clean.parsed_text.number = house_number_field.value; + + // remove the first instance of the number and trim whitespace + req.clean.parsed_text.street = _.trim(_.replace(req.clean.parsed_text.address, req.clean.parsed_text.number, '')); + + } else { + // otherwise no house number was identifiable, so treat the entire input + // as a street + req.clean.parsed_text.street = req.clean.parsed_text.address; + + } + + // the address field no longer means anything since it's been parsed, so remove it + delete req.clean.parsed_text.address; + + debugLog.push(req, {parsed_text: response}); + + } + + debugLog.stopTimer(req, initialTime); + return next(); + + }); + + } + + return controller; +} + +module.exports = setup; diff --git a/package.json b/package.json index 80243ad1..d32e37cc 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,6 @@ "pelias-model": "5.2.0", "pelias-query": "9.1.1", "pelias-sorting": "1.1.0", - "pelias-text-analyzer": "1.10.2", "predicates": "^2.0.0", "retry": "^0.10.1", "stats-lite": "^2.0.4", diff --git a/routes/v1.js b/routes/v1.js index d300d20e..fb3539a8 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -29,6 +29,7 @@ var controllers = { coarse_reverse: require('../controller/coarse_reverse'), mdToHTML: require('../controller/markdownToHtml'), libpostal: require('../controller/libpostal'), + structured_libpostal: require('../controller/structured_libpostal'), place: require('../controller/place'), placeholder: require('../controller/placeholder'), search: require('../controller/search'), @@ -96,6 +97,7 @@ const PlaceHolder = require('../service/configurations/PlaceHolder'); const PointInPolygon = require('../service/configurations/PointInPolygon'); const Language = require('../service/configurations/Language'); const Interpolation = require('../service/configurations/Interpolation'); +const Libpostal = require('../service/configurations/Libpostal'); /** * Append routes to app @@ -122,6 +124,18 @@ function addRoutes(app, peliasConfig) { const interpolationService = serviceWrapper(interpolationConfiguration); const isInterpolationEnabled = _.constant(interpolationConfiguration.isEnabled()); + // standard libpostal should use req.clean.text for the `address` parameter + const libpostalConfiguration = new Libpostal( + _.defaultTo(peliasConfig.api.services.libpostal, {}), + _.property('clean.text')); + const libpostalService = serviceWrapper(libpostalConfiguration); + + // structured libpostal should use req.clean.parsed_text.address for the `address` parameter + const structuredLibpostalConfiguration = new Libpostal( + _.defaultTo(peliasConfig.api.services.libpostal, {}), + _.property('clean.parsed_text.address')); + const structuredLibpostalService = serviceWrapper(structuredLibpostalConfiguration); + // fallback to coarse reverse when regular reverse didn't return anything const coarseReverseShouldExecute = all( isPipServiceEnabled, not(hasRequestErrors), not(hasResponseData) @@ -132,6 +146,12 @@ function addRoutes(app, peliasConfig) { not(isRequestSourcesOnlyWhosOnFirst) ); + // for libpostal to execute for structured requests, req.clean.parsed_text.address must exist + const structuredLibpostalShouldExecute = all( + not(hasRequestErrors), + hasParsedTextProperties.all('address') + ); + // execute placeholder if libpostal only parsed as admin-only and needs to // be geodisambiguated const placeholderGeodisambiguationShouldExecute = all( @@ -256,7 +276,7 @@ function addRoutes(app, peliasConfig) { sanitizers.search.middleware(peliasConfig.api), middleware.requestLanguage, middleware.calcSize(), - controllers.libpostal(libpostalShouldExecute), + controllers.libpostal(libpostalService, libpostalShouldExecute), controllers.placeholder(placeholderService, geometricFiltersApply, placeholderGeodisambiguationShouldExecute), controllers.placeholder(placeholderService, geometricFiltersDontApply, placeholderIdsLookupShouldExecute), controllers.search_with_ids(peliasConfig.api, esclient, queries.address_using_ids, searchWithIdsShouldExecute), @@ -286,6 +306,7 @@ function addRoutes(app, peliasConfig) { sanitizers.structured_geocoding.middleware(peliasConfig.api), middleware.requestLanguage, middleware.calcSize(), + controllers.structured_libpostal(structuredLibpostalService, structuredLibpostalShouldExecute), controllers.search(peliasConfig.api, esclient, queries.structured_geocoding, not(hasResponseDataOrRequestErrors)), postProc.trimByGranularityStructured(), postProc.distances('focus.point.'), diff --git a/service/configurations/Libpostal.js b/service/configurations/Libpostal.js new file mode 100644 index 00000000..9cf1de2b --- /dev/null +++ b/service/configurations/Libpostal.js @@ -0,0 +1,33 @@ +'use strict'; + +const url = require('url'); + +const ServiceConfiguration = require('pelias-microservice-wrapper').ServiceConfiguration; + +class Libpostal extends ServiceConfiguration { + constructor(o, propertyExtractor) { + super('libpostal', o); + + // save off the propertyExtractor function + // this is used to extract a single property from req. eg: + // * _.property('clean.text') + // * _.property('clean.parsed_text.address') + // will return those properties from req + this.propertyExtractor = propertyExtractor; + + } + + getParameters(req) { + return { + address: this.propertyExtractor(req) + }; + + } + + getUrl(req) { + return url.resolve(this.baseUrl, 'parse'); + } + +} + +module.exports = Libpostal; diff --git a/test/unit/controller/libpostal.js b/test/unit/controller/libpostal.js index f117007d..cf62fff5 100644 --- a/test/unit/controller/libpostal.js +++ b/test/unit/controller/libpostal.js @@ -1,281 +1,304 @@ 'use strict'; const proxyquire = require('proxyquire').noCallThru(); +const libpostal = require('../../../controller/libpostal'); const _ = require('lodash'); +const mock_logger = require('pelias-mock-logger'); module.exports.tests = {}; module.exports.tests.interface = (test, common) => { - test('valid interface', t => { - const controller = proxyquire('../../../controller/libpostal', { - 'pelias-text-analyzer': { - parse: () => undefined - } - }); - - t.equal(typeof controller, 'function', 'libpostal is a function'); - t.equal(typeof controller(), 'function', 'libpostal returns a controller'); + test('valid interface', (t) => { + t.equal(typeof libpostal, 'function', 'libpostal is a function'); + t.equal(typeof libpostal(), 'function', 'libpostal returns a controller'); t.end(); - }); - }; -module.exports.tests.should_execute = (test, common) => { - test('should_execute returning false should not call text-analyzer', t => { - const should_execute = (req, res) => { +module.exports.tests.early_exit_conditions = (test, common) => { + test('should_execute returning false should not call service', t => { + const service = () => { + t.fail('service should not have been called'); + }; + + const should_execute = (req) => { // req and res should be passed to should_execute t.deepEquals(req, { clean: { text: 'original query' } }); - t.deepEquals(res, { b: 2 }); + return false; }; - const controller = proxyquire('../../../controller/libpostal', { - 'pelias-text-analyzer': { - parse: () => { - t.fail('parse should not have been called'); - } - } - })(should_execute); + const controller = libpostal(service, should_execute); const req = { clean: { text: 'original query' } }; - const res = { b: 2 }; - controller(req, res, () => { + controller(req, undefined, () => { t.deepEquals(req, { clean: { text: 'original query' } }, 'req should not have been modified'); - t.deepEquals(res, { b: 2 }); + t.end(); }); }); - test('should_execute returning false should not call text-analyzer', t => { - t.plan(5); +}; - const should_execute = (req, res) => { - // req and res should be passed to should_execute - t.deepEquals(req, { - clean: { - text: 'original query' - } - }); - t.deepEquals(res, { b: 2 }); - return true; +module.exports.tests.error_conditions = (test, common) => { + test('service returning error should append and not modify req.clean', t => { + const service = (req, callback) => { + callback('libpostal service error', []); }; - const controller = proxyquire('../../../controller/libpostal', { - 'pelias-text-analyzer': { - parse: (query) => { - t.equals(query, 'original query'); - return undefined; - } - } - })(should_execute); + const controller = libpostal(service, () => true); const req = { clean: { text: 'original query' - } + }, + errors: [] }; - const res = { b: 2 }; - controller(req, res, () => { + controller(req, undefined, () => { t.deepEquals(req, { clean: { text: 'original query' - } + }, + errors: ['libpostal service error'] }, 'req should not have been modified'); - t.deepEquals(res, { b: 2 }); + t.end(); + }); }); }; -module.exports.tests.parse_is_called = (test, common) => { - test('parse returning undefined should not overwrite clean.parsed_text', t => { - const controller = proxyquire('../../../controller/libpostal', { - 'pelias-text-analyzer': { - parse: () => undefined - } - })(() => true); - - const req = { - clean: { - parsed_text: 'original parsed_text' - } - }; - const res = 'this is the response'; +module.exports.tests.failure_conditions = (test, common) => { + test('service returning 2 or more of a label should return undefined and log message', t => { + const logger = mock_logger(); - controller(req, res, () => { - t.deepEquals(req, { - clean: { - parsed_text: 'original parsed_text' + const service = (req, callback) => { + const response = [ + { + label: 'road', + value: 'road value 1' + }, + { + label: 'city', + value: 'city value' + }, + { + label: 'road', + value: 'road value 2' } - }); - t.deepEquals(res, 'this is the response'); - t.end(); - }); + ]; - }); + callback(null, response); + }; - test('parse returning something should overwrite clean.parsed_text', t => { const controller = proxyquire('../../../controller/libpostal', { - 'pelias-text-analyzer': { - parse: () => 'replacement parsed_text' - } - })(() => true); + 'pelias-logger': logger + })(service, () => true); const req = { clean: { - parsed_text: 'original parsed_text' - } + text: 'query value' + }, + errors: [] }; - const res = 'this is the response'; - controller(req, res, () => { + controller(req, undefined, () => { + t.ok(logger.isWarnMessage('discarding libpostal parse of \'query value\' due to duplicate field assignments')); + t.deepEquals(req, { clean: { - parser: 'libpostal', - parsed_text: 'replacement parsed_text' - } - }); - t.deepEquals(res, 'this is the response'); + text: 'query value' + }, + errors: [] + }, 'req should not have been modified'); + t.end(); + }); }); -}; + test('service returning empty array should not set parsed_text or parser', t => { + const logger = mock_logger(); + + const service = (req, callback) => { + callback(null, []); + }; -module.exports.tests.iso2_conversion = (test, common) => { - test('no country in parse response should not leave country unset', t => { const controller = proxyquire('../../../controller/libpostal', { - 'pelias-text-analyzer': { - parse: () => ({ - locality: 'this is the locality' - }) - }, - 'iso3166-1': { - is2: () => t.fail('should not have been called'), - to3: () => t.fail('should not have been called') - } - })(() => true); + 'pelias-logger': logger + })(service, () => true); const req = { clean: { - parsed_text: 'original parsed_text' - } + text: 'query value' + }, + errors: [] }; - const res = 'this is the response'; - controller(req, res, () => { + controller(req, undefined, () => { t.deepEquals(req, { clean: { - parser: 'libpostal', - parsed_text: { - locality: 'this is the locality' - } - } - }); - t.deepEquals(res, 'this is the response'); + text: 'query value' + }, + errors: [] + }, 'req should not have been modified'); + t.end(); + }); }); - test('unknown country should not be converted', t => { - t.plan(3); +}; - const controller = proxyquire('../../../controller/libpostal', { - 'pelias-text-analyzer': { - parse: () => ({ - country: 'unknown country code' - }) - }, - 'iso3166-1': { - is2: country => { - t.equals(country, 'UNKNOWN COUNTRY CODE'); - return false; +module.exports.tests.success_conditions = (test, common) => { + test('service returning valid response should convert and append', t => { + const service = (req, callback) => { + const response = [ + { + label: 'island', + value: 'island value' }, - to3: () => t.fail('should not have been called') - } - })(() => true); + { + label: 'category', + value: 'category value' + }, + { + label: 'house', + value: 'house value' + }, + { + label: 'house_number', + value: 'house_number value' + }, + { + label: 'road', + value: 'road value' + }, + { + label: 'suburb', + value: 'suburb value' + }, + { + label: 'city_district', + value: 'city_district value' + }, + { + label: 'city', + value: 'city value' + }, + { + label: 'state_district', + value: 'state_district value' + }, + { + label: 'state', + value: 'state value' + }, + { + label: 'postcode', + value: 'postcode value' + }, + { + label: 'country', + value: 'country value' + } + ]; + + callback(null, response); + }; + + const controller = libpostal(service, () => true); const req = { clean: { - parsed_text: 'original parsed_text' - } + text: 'original query' + }, + errors: [] }; - const res = 'this is the response'; - controller(req, res, () => { + controller(req, undefined, () => { t.deepEquals(req, { clean: { + text: 'original query', parser: 'libpostal', parsed_text: { - country: 'unknown country code' + island: 'island value', + category: 'category value', + query: 'house value', + number: 'house_number value', + street: 'road value', + neighbourhood: 'suburb value', + borough: 'city_district value', + city: 'city value', + county: 'state_district value', + state: 'state value', + postalcode: 'postcode value', + country: 'country value' } - } - }); - t.deepEquals(res, 'this is the response'); + }, + errors: [] + }, 'req should not have been modified'); + t.end(); + }); }); - test('ISO2 country should be converted to ISO3', t => { - t.plan(4); - - const controller = proxyquire('../../../controller/libpostal', { - 'pelias-text-analyzer': { - parse: () => ({ - country: 'ISO2 COUNTRY CODE' - }) - }, - 'iso3166-1': { - is2: country => { - t.equals(country, 'ISO2 COUNTRY CODE'); - return true; - }, - to3: country => { - t.equals(country, 'ISO2 COUNTRY CODE'); - return 'ISO3 COUNTRY CODE'; + test('ISO-2 country should be converted to ISO-3', t => { + const service = (req, callback) => { + const response = [ + { + label: 'country', + value: 'ca' } - } - })(() => true); + ]; + + callback(null, response); + }; + + const controller = libpostal(service, () => true); const req = { clean: { - parsed_text: 'original parsed_text' - } + text: 'original query' + }, + errors: [] }; - const res = 'this is the response'; - controller(req, res, () => { + controller(req, undefined, () => { t.deepEquals(req, { clean: { + text: 'original query', parser: 'libpostal', parsed_text: { - country: 'ISO3 COUNTRY CODE' + country: 'CAN' } - } - }); - t.deepEquals(res, 'this is the response'); + }, + errors: [] + }, 'req should not have been modified'); + t.end(); + }); }); diff --git a/test/unit/controller/structured_libpostal.js b/test/unit/controller/structured_libpostal.js new file mode 100644 index 00000000..eb4aea12 --- /dev/null +++ b/test/unit/controller/structured_libpostal.js @@ -0,0 +1,440 @@ +'use strict'; + +const proxyquire = require('proxyquire').noCallThru(); +const libpostal = require('../../../controller/structured_libpostal'); +const _ = require('lodash'); +const mock_logger = require('pelias-mock-logger'); + +module.exports.tests = {}; + +module.exports.tests.interface = (test, common) => { + test('valid interface', (t) => { + t.equal(typeof libpostal, 'function', 'libpostal is a function'); + t.equal(typeof libpostal(), 'function', 'libpostal 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 = () => { + t.fail('service should not have been called'); + }; + + const should_execute = (req) => { + // req and res should be passed to should_execute + t.deepEquals(req, { + clean: { + text: 'original query' + } + }); + + return false; + }; + + const controller = libpostal(service, should_execute); + + const req = { + clean: { + text: 'original query' + } + }; + + controller(req, undefined, () => { + t.deepEquals(req, { + clean: { + text: 'original query' + } + }, 'req should not have been modified'); + + t.end(); + }); + + }); + +}; + +module.exports.tests.error_conditions = (test, common) => { + test('service returning error should append and not modify req.clean', t => { + const service = (req, callback) => { + callback('libpostal service error', []); + }; + + const controller = libpostal(service, () => true); + + const req = { + clean: { + text: 'original query' + }, + errors: [] + }; + + controller(req, undefined, () => { + t.deepEquals(req, { + clean: { + text: 'original query' + }, + errors: ['libpostal service error'] + }, 'req should not have been modified'); + + t.end(); + + }); + + }); + +}; + +// module.exports.tests.failure_conditions = (test, common) => { +// test('service returning 2 or more of a label should return undefined and log message', t => { +// const logger = mock_logger(); +// +// const service = (req, callback) => { +// const response = [ +// { +// label: 'road', +// value: 'road value 1' +// }, +// { +// label: 'city', +// value: 'city value' +// }, +// { +// label: 'road', +// value: 'road value 2' +// } +// ]; +// +// callback(null, response); +// }; +// +// const controller = proxyquire('../../../controller/libpostal', { +// 'pelias-logger': logger +// })(service, () => true); +// +// const req = { +// clean: { +// text: 'query value' +// }, +// errors: [] +// }; +// +// controller(req, undefined, () => { +// t.ok(logger.isWarnMessage('discarding libpostal parse of \'query value\' due to duplicate field assignments')); +// +// t.deepEquals(req, { +// clean: { +// text: 'query value' +// }, +// errors: [] +// }, 'req should not have been modified'); +// +// t.end(); +// +// }); +// +// }); +// +// test('service returning empty array should not set parsed_text or parser', t => { +// const logger = mock_logger(); +// +// const service = (req, callback) => { +// callback(null, []); +// }; +// +// const controller = proxyquire('../../../controller/libpostal', { +// 'pelias-logger': logger +// })(service, () => true); +// +// const req = { +// clean: { +// text: 'query value' +// }, +// errors: [] +// }; +// +// controller(req, undefined, () => { +// t.deepEquals(req, { +// clean: { +// text: 'query value' +// }, +// errors: [] +// }, 'req should not have been modified'); +// +// t.end(); +// +// }); +// +// }); +// +// }; +// +module.exports.tests.success_conditions = (test, common) => { + test('service returning house_number should set req.clean.parsed_text.', t => { + const service = (req, callback) => { + const response = [ + { + label: 'house_number', + value: 'house_number value' + }, + { + label: 'postcode', + value: 'postcode value' + } + ]; + + callback(null, response); + }; + + const controller = libpostal(service, () => true); + + const req = { + clean: { + parsed_text: { + address: 'other value house_number value street value' + } + }, + errors: [] + }; + + controller(req, undefined, () => { + t.deepEquals(req, { + clean: { + parsed_text: { + number: 'house_number value', + street: 'other value street value' + } + }, + errors: [] + }, 'req should not have been modified'); + + t.end(); + + }); + + }); + + test('service returning postcode should set req.clean.parsed_text.', t => { + const service = (req, callback) => { + const response = [ + { + label: 'postcode', + value: 'postcode value' + } + ]; + + callback(null, response); + }; + + const controller = libpostal(service, () => true); + + const req = { + clean: { + parsed_text: { + address: 'other value postcode value street value' + } + }, + errors: [] + }; + + controller(req, undefined, () => { + t.deepEquals(req, { + clean: { + parsed_text: { + number: 'postcode value', + street: 'other value street value' + } + }, + errors: [] + }, 'req should not have been modified'); + + t.end(); + + }); + + }); + + test('service returning neither house_number nor postcode should not set req.clean.parsed_text.number', t => { + const service = (req, callback) => { + const response = [ + { + label: 'city', + value: 'city value' + } + ]; + + callback(null, response); + }; + + const controller = libpostal(service, () => true); + + const req = { + clean: { + parsed_text: { + address: 'street value' + } + }, + errors: [] + }; + + controller(req, undefined, () => { + t.deepEquals(req, { + clean: { + parsed_text: { + street: 'street value' + } + }, + errors: [] + }, 'req should not have been modified'); + + t.end(); + + }); + + }); + + // test('service returning valid response should convert and append', t => { + // const service = (req, callback) => { + // const response = [ + // { + // label: 'island', + // value: 'island value' + // }, + // { + // label: 'category', + // value: 'category value' + // }, + // { + // label: 'house', + // value: 'house value' + // }, + // { + // label: 'house_number', + // value: 'house_number value' + // }, + // { + // label: 'road', + // value: 'road value' + // }, + // { + // label: 'suburb', + // value: 'suburb value' + // }, + // { + // label: 'city_district', + // value: 'city_district value' + // }, + // { + // label: 'city', + // value: 'city value' + // }, + // { + // label: 'state_district', + // value: 'state_district value' + // }, + // { + // label: 'state', + // value: 'state value' + // }, + // { + // label: 'postcode', + // value: 'postcode value' + // }, + // { + // label: 'country', + // value: 'country value' + // } + // ]; + // + // callback(null, response); + // }; + // + // const controller = libpostal(service, () => true); + // + // const req = { + // clean: { + // text: 'original query' + // }, + // errors: [] + // }; + // + // controller(req, undefined, () => { + // t.deepEquals(req, { + // clean: { + // text: 'original query', + // parser: 'libpostal', + // parsed_text: { + // island: 'island value', + // category: 'category value', + // query: 'house value', + // number: 'house_number value', + // street: 'road value', + // neighbourhood: 'suburb value', + // borough: 'city_district value', + // city: 'city value', + // county: 'state_district value', + // state: 'state value', + // postalcode: 'postcode value', + // country: 'country value' + // } + // }, + // errors: [] + // }, 'req should not have been modified'); + // + // t.end(); + // + // }); + // + // }); + // + // test('ISO-2 country should be converted to ISO-3', t => { + // const service = (req, callback) => { + // const response = [ + // { + // label: 'country', + // value: 'ca' + // } + // ]; + // + // callback(null, response); + // }; + // + // const controller = libpostal(service, () => true); + // + // const req = { + // clean: { + // text: 'original query' + // }, + // errors: [] + // }; + // + // controller(req, undefined, () => { + // t.deepEquals(req, { + // clean: { + // text: 'original query', + // parser: 'libpostal', + // parsed_text: { + // country: 'CAN' + // } + // }, + // errors: [] + // }, 'req should not have been modified'); + // + // t.end(); + // + // }); + // + // }); + // +}; + +module.exports.all = (tape, common) => { + + function test(name, testFunction) { + return tape(`GET /libpostal ${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 5631b9f3..ffebf9ea 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -14,6 +14,7 @@ var tests = [ require('./controller/coarse_reverse'), require('./controller/index'), require('./controller/libpostal'), + require('./controller/structured_libpostal'), require('./controller/place'), require('./controller/placeholder'), require('./controller/search'), @@ -100,6 +101,7 @@ var tests = [ require('./sanitizer/wrap'), require('./service/configurations/Interpolation'), require('./service/configurations/Language'), + require('./service/configurations/Libpostal'), require('./service/configurations/PlaceHolder'), require('./service/configurations/PointInPolygon'), require('./service/mget'), diff --git a/test/unit/service/configurations/Libpostal.js b/test/unit/service/configurations/Libpostal.js new file mode 100644 index 00000000..2dcbc78c --- /dev/null +++ b/test/unit/service/configurations/Libpostal.js @@ -0,0 +1,99 @@ +const Libpostal = require('../../../../service/configurations/Libpostal'); + +module.exports.tests = {}; + +module.exports.tests.all = (test, common) => { + test('getName should return \'libpostal\'', (t) => { + const configBlob = { + url: 'http://localhost:1234', + timeout: 17, + retries: 19 + }; + + const libpostal = new Libpostal(configBlob); + + t.equals(libpostal.getName(), 'libpostal'); + t.equals(libpostal.getBaseUrl(), 'http://localhost:1234/'); + t.equals(libpostal.getTimeout(), 17); + t.equals(libpostal.getRetries(), 19); + t.end(); + + }); + + test('getUrl should return value passed to constructor', (t) => { + const configBlob = { + url: 'http://localhost:1234', + timeout: 17, + retries: 19 + }; + + const libpostal = new Libpostal(configBlob); + + t.equals(libpostal.getUrl(), 'http://localhost:1234/parse'); + t.end(); + + }); + + test('getParameters should return object with text and lang from req', (t) => { + const configBlob = { + url: 'http://localhost:1234', + timeout: 17, + retries: 19 + }; + + const propertyExtractor = (req) => { + t.deepEquals(req, { a: 1, b: 2}); + return 'property value'; + }; + + const libpostal = new Libpostal(configBlob, propertyExtractor); + + const req = { + a: 1, + b: 2 + }; + + t.deepEquals(libpostal.getParameters(req), { address: 'property value' }); + t.end(); + + }); + + test('getHeaders should return empty object', (t) => { + const configBlob = { + url: 'base url', + timeout: 17, + retries: 19 + }; + + const libpostal = new Libpostal(configBlob); + + t.deepEquals(libpostal.getHeaders(), {}); + t.end(); + + }); + + test('baseUrl ending in / should not have double /\'s return by getUrl', (t) => { + const configBlob = { + url: 'http://localhost:1234/', + timeout: 17, + retries: 19 + }; + + const libpostal = new Libpostal(configBlob); + + t.deepEquals(libpostal.getUrl(), 'http://localhost:1234/parse'); + t.end(); + + }); + +}; + +module.exports.all = (tape, common) => { + function test(name, testFunction) { + return tape(`SERVICE CONFIGURATION /Libpostal ${name}`, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; From d4d52443dcbdde0e48daff5d68612a6e0dd3b00e Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 14 Nov 2017 11:02:33 -0500 Subject: [PATCH 22/24] Update Dockerfile for libpostal service The Dockerfile no longer needs to be build on the libpostal_baseimage (which will probably go away soon) --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 041f00dc..e28228c3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # base image -FROM pelias/libpostal_baseimage +FROM pelias/baseimage # maintainer information LABEL maintainer="pelias@mapzen.com" From 0a95dd25838fc8cf12d7aecbf01603a477cf25d2 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 14 Nov 2017 12:00:29 -0500 Subject: [PATCH 23/24] removed references to `textAnalyzer` config setting --- README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/README.md b/README.md index 4253d15e..fd42a7f6 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,6 @@ The API recognizes the following properties under the top-level `api` key in you |parameter|required|default|description| |---|---|---|---| |`host`|*yes*||specifies the url under which the http service is to run| -|`textAnalyzer`|*no*|*addressit*|can be either `libpostal` or `addressit` however will soon be **deprecated** and only `libpostal` will be supported going forward| |`indexName`|*no*|*pelias*|name of the Elasticsearch index to be used when building queries| |`relativeScores`|*no*|true|if set to true, confidence scores will be normalized, realistically at this point setting this to false is not tested or desirable |`accessLog`|*no*||name of the format to use for access logs; may be any one of the [predefined values](https://github.com/expressjs/morgan#predefined-formats) in the `morgan` package. Defaults to `"common"`; if set to `false`, or an otherwise falsy value, disables access-logging entirely.| @@ -66,7 +65,6 @@ Example configuration file would look something like this: "host": "localhost:3100/v1/", "indexName": "foobar", "relativeScores": true, - "textAnalyzer": "libpostal", "services": { "pip": { "url": "http://mypipservice.com:3000" From 51709b58ff24277e64193b5e9cc5cd2ff610a58f Mon Sep 17 00:00:00 2001 From: sweco-semhul Date: Thu, 16 Nov 2017 12:45:37 +0100 Subject: [PATCH 24/24] Updating ciao tests to reflect current status in master to make them run --- .../autocomplete/layers_alias_coarse.coffee | 5 +++- test/ciao/autocomplete/layers_invalid.coffee | 2 +- .../layers_mix_invalid_valid.coffee | 2 +- ...boundary_circle_valid_radius_coarse.coffee | 23 ++++++++++++++++--- test/ciao/reverse/layers_alias_coarse.coffee | 5 +++- test/ciao/reverse/layers_invalid.coffee | 2 +- .../reverse/layers_mix_invalid_valid.coffee | 2 +- test/ciao/reverse/sources_multiple.coffee | 4 ++-- test/ciao/search/layers_alias_coarse.coffee | 5 +++- test/ciao/search/layers_invalid.coffee | 2 +- .../search/layers_mix_invalid_valid.coffee | 2 +- 11 files changed, 40 insertions(+), 14 deletions(-) diff --git a/test/ciao/autocomplete/layers_alias_coarse.coffee b/test/ciao/autocomplete/layers_alias_coarse.coffee index 5ceaaa46..fe934f55 100644 --- a/test/ciao/autocomplete/layers_alias_coarse.coffee +++ b/test/ciao/autocomplete/layers_alias_coarse.coffee @@ -32,6 +32,7 @@ should.not.exist json.geocoding.warnings json.geocoding.query['text'].should.eql 'a' json.geocoding.query['size'].should.eql 10 json.geocoding.query.layers.should.eql [ "continent", + "empire", "country", "dependency", "macroregion", @@ -45,5 +46,7 @@ json.geocoding.query.layers.should.eql [ "continent", "neighbourhood", "microhood", "disputed", - "postalcode" + "postalcode", + "ocean", + "marinearea" ] diff --git a/test/ciao/autocomplete/layers_invalid.coffee b/test/ciao/autocomplete/layers_invalid.coffee index 5396387a..74b04379 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,postalcode' ] +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,empire,dependency,macrocounty,macrohood,microhood,disputed,postalcode,ocean,marinearea' ] #? 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 6a9cc488..7357fda5 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,postalcode' ] +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,empire,dependency,macrocounty,macrohood,microhood,disputed,postalcode,ocean,marinearea' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/reverse/boundary_circle_valid_radius_coarse.coffee b/test/ciao/reverse/boundary_circle_valid_radius_coarse.coffee index b3f76c69..7d5d9bfc 100644 --- a/test/ciao/reverse/boundary_circle_valid_radius_coarse.coffee +++ b/test/ciao/reverse/boundary_circle_valid_radius_coarse.coffee @@ -26,14 +26,31 @@ json.features.should.be.instanceof Array should.not.exist json.geocoding.errors #? expected warnings -should.exist json.geocoding.warnings -json.geocoding.warnings.should.eql [ 'boundary.circle.radius is not applicable for coarse reverse' ] +should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['size'].should.eql 10 -json.geocoding.query['layers'].should.eql 'coarse' json.geocoding.query['point.lat'].should.eql 40.744243 json.geocoding.query['point.lon'].should.eql -73.990342 json.geocoding.query['boundary.circle.lat'].should.eql 40.744243 json.geocoding.query['boundary.circle.lon'].should.eql -73.990342 json.geocoding.query['boundary.circle.radius'].should.eql 999.9 +json.geocoding.query['layers'].should.eql [ "continent", + "empire", + "country", + "dependency", + "macroregion", + "region", + "locality", + "localadmin", + "macrocounty", + "county", + "macrohood", + "borough", + "neighbourhood", + "microhood", + "disputed", + "postalcode", + "ocean", + "marinearea" +] \ No newline at end of file diff --git a/test/ciao/reverse/layers_alias_coarse.coffee b/test/ciao/reverse/layers_alias_coarse.coffee index f70eeb11..276ca93f 100644 --- a/test/ciao/reverse/layers_alias_coarse.coffee +++ b/test/ciao/reverse/layers_alias_coarse.coffee @@ -31,6 +31,7 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['size'].should.eql 10 json.geocoding.query.layers.should.eql [ "continent", + "empire", "country", "dependency", "macroregion", @@ -44,5 +45,7 @@ json.geocoding.query.layers.should.eql [ "continent", "neighbourhood", "microhood", "disputed", - "postalcode" + "postalcode", + "ocean", + "marinearea" ] diff --git a/test/ciao/reverse/layers_invalid.coffee b/test/ciao/reverse/layers_invalid.coffee index 7298285a..cbd46a89 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,postalcode' ] +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,empire,dependency,macrocounty,macrohood,microhood,disputed,postalcode,ocean,marinearea' ] #? 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 c3165cf7..90d0ea41 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,postalcode' ] +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,empire,dependency,macrocounty,macrohood,microhood,disputed,postalcode,ocean,marinearea' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/reverse/sources_multiple.coffee b/test/ciao/reverse/sources_multiple.coffee index 4fd24366..986adc0e 100644 --- a/test/ciao/reverse/sources_multiple.coffee +++ b/test/ciao/reverse/sources_multiple.coffee @@ -1,6 +1,6 @@ #> sources filter -path: '/v1/reverse?point.lat=1&point.lon=2&sources=openstreetmap,geonames' +path: '/v1/reverse?point.lat=1&point.lon=2&sources=openstreetmap,openaddresses' #? 200 ok response.statusCode.should.be.equal 200 @@ -30,4 +30,4 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['size'].should.eql 10 -json.geocoding.query.sources.should.eql ["openstreetmap", "geonames"] +json.geocoding.query.sources.should.eql ["openstreetmap", "openaddresses"] diff --git a/test/ciao/search/layers_alias_coarse.coffee b/test/ciao/search/layers_alias_coarse.coffee index af8d3d8e..c9ecf7d3 100644 --- a/test/ciao/search/layers_alias_coarse.coffee +++ b/test/ciao/search/layers_alias_coarse.coffee @@ -32,6 +32,7 @@ should.not.exist json.geocoding.warnings json.geocoding.query['text'].should.eql 'a' json.geocoding.query['size'].should.eql 10 json.geocoding.query.layers.should.eql [ "continent", + "empire", "country", "dependency", "macroregion", @@ -45,5 +46,7 @@ json.geocoding.query.layers.should.eql [ "continent", "neighbourhood", "microhood", "disputed", - "postalcode" + "postalcode", + "ocean", + "marinearea" ] diff --git a/test/ciao/search/layers_invalid.coffee b/test/ciao/search/layers_invalid.coffee index 52b370dd..2d8b6275 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,postalcode' ] +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,empire,dependency,macrocounty,macrohood,microhood,disputed,postalcode,ocean,marinearea' ] #? 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 da39b9e8..91393881 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,postalcode' ] +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,empire,dependency,macrocounty,macrohood,microhood,disputed,postalcode,ocean,marinearea' ] #? expected warnings should.not.exist json.geocoding.warnings