From 63558eb15dbf1f6fb98d5dc5ab42ef6545fe65cd Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Thu, 24 Mar 2016 12:53:48 +0100 Subject: [PATCH 01/20] add ciao dummy data script and README on how to use it --- README.md | 38 +++++++++++++++++++++++++++ test/ciao_test_data.js | 58 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 test/ciao_test_data.js diff --git a/README.md b/README.md index 652f289c..53074f58 100644 --- a/README.md +++ b/README.md @@ -34,3 +34,41 @@ The API recognizes the following properties under the top-level `api` key in you Please fork and pull request against upstream master on a feature branch. Pretty please; provide unit tests and script fixtures in the `test` directory. + +## Unit tests + +You can run the unit test suite using the command: + +```bash +$ npm test +``` + +## HTTP tests + +We have another set of tests which are used to test the HTTP API layer, these tests send expected HTTP requests and then +assert that the responses coming back have the correct geoJSON format and HTTP status codes. + +You can run the HTTP test suite using the command: + +```bash +$ npm run ciao +``` + +Note: some of the tests in this suite fail when no data is present in the index, there is a small set of test documents +provided in `./test/ciao_test_data` which can be inserted in order to avoid these errors. + +To inject dummy data in to your local index: + +```bash +$ node test/ciao_test_data.js +``` + +You can confirm the dummy data has been inserted with the command: + +```bash +$ curl localhost:9200/pelias/_count?pretty +{ + "count" : 9, + ... +} +``` diff --git a/test/ciao_test_data.js b/test/ciao_test_data.js new file mode 100644 index 00000000..70b329f6 --- /dev/null +++ b/test/ciao_test_data.js @@ -0,0 +1,58 @@ + +/** + Test data required by the ciao test suite. + + Some tests will fail when run against an empty index, you can use this script + to insert some dummy data in to your index before running the tests. + + note: as this is dummy data, care should be taken in order to make sure these + documents don't end up in your production index; for that reason the HTTP port + has been hard-coded as port:9200. +**/ + +// we use the default config to avoid making calls to +// a cluster running on a non-standard port. +var client = require('elasticsearch').Client(), + async = require('async'), + actions = []; + +// add one record per 'type' in order to cause the _default_ mapping +// to be copied when the new type is created. +var types = ['venue','address','county','region','county','country','admin0','admin1','admin2'], + sources = ['test'], + layers = ['geonames']; + +// iterate over all types/sources/layers and index a test document +types.forEach( function( type, i1 ){ + sources.forEach( function( source, i2 ){ + layers.forEach( function( layer, i3 ){ + actions.push( function( done ){ + client.index({ + index: 'pelias', + type: type, + id: [i1,i2,i3].join(':'), + body: { + source: source, + layer: layer, + name: { default: 'test' }, + phrase: { default: 'test' }, + parent: { + country_a: ['USA'] + } + } + }); + done(); + }); + }); + }); +}); + +// call refresh so the index merges the changes +actions.push( function( done ){ + client.indices.refresh( { index: 'pelias' }, done); +}); + +// perform all actions in series +async.series( actions, function( err, resp ){ + console.log('test data inported'); +}); From 8431763a122736a571fbacc468cc92721e88e051 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Thu, 24 Mar 2016 13:15:26 +0100 Subject: [PATCH 02/20] fix handling of ?size= param for autocomplete --- sanitiser/_size.js | 60 +++++++++++-------- sanitiser/autocomplete.js | 2 +- sanitiser/reverse.js | 2 +- sanitiser/search.js | 2 +- .../ciao/autocomplete/size_not_default.coffee | 34 +++++++++++ 5 files changed, 71 insertions(+), 29 deletions(-) create mode 100644 test/ciao/autocomplete/size_not_default.coffee diff --git a/sanitiser/_size.js b/sanitiser/_size.js index 3fb8115b..b2b5e62e 100644 --- a/sanitiser/_size.js +++ b/sanitiser/_size.js @@ -5,32 +5,40 @@ var MIN_SIZE = 1, DEFAULT_SIZE = 10; // validate inputs, convert types and apply defaults -function sanitize( raw, clean ){ - - // error & warning messages - var messages = { errors: [], warnings: [] }; - - // coercions - clean.size = parseInt( raw.size, 10 ); - - // invalid numeric input - if( isNaN( clean.size ) ){ - clean.size = DEFAULT_SIZE; - } - // ensure size falls within defined range - else if( clean.size > MAX_SIZE ){ - // set the max size - messages.warnings.push('out-of-range integer \'size\', using MAX_SIZE'); - clean.size = MAX_SIZE; - } - else if( clean.size < MIN_SIZE ){ - // set the min size - messages.warnings.push('out-of-range integer \'size\', using MIN_SIZE'); - clean.size = MIN_SIZE; - } - - return messages; +function setup( size_min, size_max, size_def ){ + + // allow caller to inject custom min/max/default values + if( !check.number( size_min ) ){ size_min = MIN_SIZE; } + if( !check.number( size_max ) ){ size_max = MAX_SIZE; } + if( !check.number( size_def ) ){ size_def = DEFAULT_SIZE; } + + return function sanitize( raw, clean ){ + + // error & warning messages + var messages = { errors: [], warnings: [] }; + + // coercions + clean.size = parseInt( raw.size, 10 ); + + // invalid numeric input + if( isNaN( clean.size ) ){ + clean.size = size_def; + } + // ensure size falls within defined range + else if( clean.size > size_max ){ + // set the max size + messages.warnings.push('out-of-range integer \'size\', using MAX_SIZE'); + clean.size = size_max; + } + else if( clean.size < size_min ){ + // set the min size + messages.warnings.push('out-of-range integer \'size\', using MIN_SIZE'); + clean.size = size_min; + } + + return messages; + }; } // export function -module.exports = sanitize; +module.exports = setup; diff --git a/sanitiser/autocomplete.js b/sanitiser/autocomplete.js index c6dc08da..7bb621c1 100644 --- a/sanitiser/autocomplete.js +++ b/sanitiser/autocomplete.js @@ -2,7 +2,7 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), sanitizers = { singleScalarParameters: require('../sanitiser/_single_scalar_parameters'), text: require('../sanitiser/_text'), - size: require('../sanitiser/_size'), + size: require('../sanitiser/_size')(10, 10, 10), private: require('../sanitiser/_flag_bool')('private', false), geo_autocomplete: require('../sanitiser/_geo_autocomplete'), }; diff --git a/sanitiser/reverse.js b/sanitiser/reverse.js index d707fcea..fabfcf65 100644 --- a/sanitiser/reverse.js +++ b/sanitiser/reverse.js @@ -7,7 +7,7 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), sources: require('../sanitiser/_targets')('sources', type_mapping.source_mapping), // depends on the layers and sources sanitisers, must be run after them sources_and_layers: require('../sanitiser/_sources_and_layers'), - size: require('../sanitiser/_size'), + size: require('../sanitiser/_size')(/* use defaults*/), private: require('../sanitiser/_flag_bool')('private', false), geo_reverse: require('../sanitiser/_geo_reverse'), boundary_country: require('../sanitiser/_boundary_country'), diff --git a/sanitiser/search.js b/sanitiser/search.js index 980b5a04..c9c68330 100644 --- a/sanitiser/search.js +++ b/sanitiser/search.js @@ -4,7 +4,7 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), sanitizers = { singleScalarParameters: require('../sanitiser/_single_scalar_parameters'), text: require('../sanitiser/_text'), - size: require('../sanitiser/_size'), + size: require('../sanitiser/_size')(/* use defaults*/), layers: require('../sanitiser/_targets')('layers', type_mapping.layer_mapping), sources: require('../sanitiser/_targets')('sources', type_mapping.source_mapping), // depends on the layers and sources sanitisers, must be run after them diff --git a/test/ciao/autocomplete/size_not_default.coffee b/test/ciao/autocomplete/size_not_default.coffee new file mode 100644 index 00000000..8baf233d --- /dev/null +++ b/test/ciao/autocomplete/size_not_default.coffee @@ -0,0 +1,34 @@ + +#> set size (autocomplete does not allow size to be changed) +path: '/v1/autocomplete?text=a&size=3' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.exist json.geocoding.warnings +json.geocoding.warnings.should.eql [ 'out-of-range integer \'size\', using MIN_SIZE' ] + +#? inputs +json.geocoding.query['text'].should.eql 'a' +json.geocoding.query['size'].should.eql 10 # should remain the default size From 00b123adfbec4f362c66772ea5d0e3c66b3e0d42 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Thu, 24 Mar 2016 13:22:03 +0100 Subject: [PATCH 03/20] enable ?source=s and ?layers params for autocomplete --- sanitiser/autocomplete.js | 6 +++ .../autocomplete/layers_alias_address.coffee | 34 ++++++++++++++ .../autocomplete/layers_alias_coarse.coffee | 46 +++++++++++++++++++ test/ciao/autocomplete/layers_invalid.coffee | 34 ++++++++++++++ .../layers_mix_invalid_valid.coffee | 35 ++++++++++++++ test/ciao/autocomplete/layers_multiple.coffee | 34 ++++++++++++++ test/ciao/autocomplete/layers_single.coffee | 34 ++++++++++++++ test/ciao/autocomplete/sources_invalid.coffee | 34 ++++++++++++++ .../sources_layers_invalid_combo.coffee | 37 +++++++++++++++ .../sources_layers_valid_combo.coffee | 35 ++++++++++++++ .../ciao/autocomplete/sources_multiple.coffee | 34 ++++++++++++++ test/ciao/autocomplete/sources_single.coffee | 34 ++++++++++++++ 12 files changed, 397 insertions(+) create mode 100644 test/ciao/autocomplete/layers_alias_address.coffee create mode 100644 test/ciao/autocomplete/layers_alias_coarse.coffee create mode 100644 test/ciao/autocomplete/layers_invalid.coffee create mode 100644 test/ciao/autocomplete/layers_mix_invalid_valid.coffee create mode 100644 test/ciao/autocomplete/layers_multiple.coffee create mode 100644 test/ciao/autocomplete/layers_single.coffee create mode 100644 test/ciao/autocomplete/sources_invalid.coffee create mode 100644 test/ciao/autocomplete/sources_layers_invalid_combo.coffee create mode 100644 test/ciao/autocomplete/sources_layers_valid_combo.coffee create mode 100644 test/ciao/autocomplete/sources_multiple.coffee create mode 100644 test/ciao/autocomplete/sources_single.coffee diff --git a/sanitiser/autocomplete.js b/sanitiser/autocomplete.js index 7bb621c1..f9698956 100644 --- a/sanitiser/autocomplete.js +++ b/sanitiser/autocomplete.js @@ -1,8 +1,14 @@ +var type_mapping = require('../helper/type_mapping'); + var sanitizeAll = require('../sanitiser/sanitizeAll'), sanitizers = { singleScalarParameters: require('../sanitiser/_single_scalar_parameters'), text: require('../sanitiser/_text'), size: require('../sanitiser/_size')(10, 10, 10), + layers: require('../sanitiser/_targets')('layers', type_mapping.layer_mapping), + sources: require('../sanitiser/_targets')('sources', type_mapping.source_mapping), + // depends on the layers and sources sanitisers, must be run after them + sources_and_layers: require('../sanitiser/_sources_and_layers'), private: require('../sanitiser/_flag_bool')('private', false), geo_autocomplete: require('../sanitiser/_geo_autocomplete'), }; diff --git a/test/ciao/autocomplete/layers_alias_address.coffee b/test/ciao/autocomplete/layers_alias_address.coffee new file mode 100644 index 00000000..d393971a --- /dev/null +++ b/test/ciao/autocomplete/layers_alias_address.coffee @@ -0,0 +1,34 @@ + +#> layer alias +path: '/v1/autocomplete?text=a&layers=address' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? inputs +json.geocoding.query['text'].should.eql 'a' +json.geocoding.query['size'].should.eql 10 +json.geocoding.query.layers.should.eql ["address"] diff --git a/test/ciao/autocomplete/layers_alias_coarse.coffee b/test/ciao/autocomplete/layers_alias_coarse.coffee new file mode 100644 index 00000000..4a359bdd --- /dev/null +++ b/test/ciao/autocomplete/layers_alias_coarse.coffee @@ -0,0 +1,46 @@ + +#> layer alias +path: '/v1/autocomplete?text=a&layers=coarse' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? inputs +json.geocoding.query['text'].should.eql 'a' +json.geocoding.query['size'].should.eql 10 +json.geocoding.query.layers.should.eql [ "continent", + "macrocountry", + "country", + "dependency", + "region", + "locality", + "localadmin", + "county", + "macrohood", + "neighbourhood", + "microhood", + "disputed" +] diff --git a/test/ciao/autocomplete/layers_invalid.coffee b/test/ciao/autocomplete/layers_invalid.coffee new file mode 100644 index 00000000..b4ce0565 --- /dev/null +++ b/test/ciao/autocomplete/layers_invalid.coffee @@ -0,0 +1,34 @@ + +#> layer alias +path: '/v1/autocomplete?text=a&layers=notlayer' + +#? 200 ok +response.statusCode.should.be.equal 400 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.exist json.geocoding.errors +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,country,region,county,locality,continent,macrocountry,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? inputs +json.geocoding.query['text'].should.eql 'a' +json.geocoding.query['size'].should.eql 10 diff --git a/test/ciao/autocomplete/layers_mix_invalid_valid.coffee b/test/ciao/autocomplete/layers_mix_invalid_valid.coffee new file mode 100644 index 00000000..8bac1ccf --- /dev/null +++ b/test/ciao/autocomplete/layers_mix_invalid_valid.coffee @@ -0,0 +1,35 @@ + +#> layer alias +path: '/v1/autocomplete?text=a&layers=country,notlayer' + +#? 200 ok +response.statusCode.should.be.equal 400 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.exist json.geocoding.errors +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,country,region,county,locality,continent,macrocountry,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? inputs +json.geocoding.query['text'].should.eql 'a' +json.geocoding.query['size'].should.eql 10 +should.not.exist json.geocoding.query['layers'] diff --git a/test/ciao/autocomplete/layers_multiple.coffee b/test/ciao/autocomplete/layers_multiple.coffee new file mode 100644 index 00000000..fed6f6f7 --- /dev/null +++ b/test/ciao/autocomplete/layers_multiple.coffee @@ -0,0 +1,34 @@ + +#> layer alias +path: '/v1/autocomplete?text=a&layers=country,region' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? inputs +json.geocoding.query['text'].should.eql 'a' +json.geocoding.query['size'].should.eql 10 +json.geocoding.query.layers.should.eql ["country","region"] diff --git a/test/ciao/autocomplete/layers_single.coffee b/test/ciao/autocomplete/layers_single.coffee new file mode 100644 index 00000000..fdae272a --- /dev/null +++ b/test/ciao/autocomplete/layers_single.coffee @@ -0,0 +1,34 @@ + +#> layer alias +path: '/v1/autocomplete?text=a&layers=country' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? inputs +json.geocoding.query['text'].should.eql 'a' +json.geocoding.query['size'].should.eql 10 +json.geocoding.query.layers.should.eql ["country"] diff --git a/test/ciao/autocomplete/sources_invalid.coffee b/test/ciao/autocomplete/sources_invalid.coffee new file mode 100644 index 00000000..38a12162 --- /dev/null +++ b/test/ciao/autocomplete/sources_invalid.coffee @@ -0,0 +1,34 @@ + +#> sources filter +path: '/v1/autocomplete?text=a&sources=openstreetmap,notasource' + +#? 200 ok +response.statusCode.should.be.equal 400 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.exist json.geocoding.errors +json.geocoding.errors.should.eql [ '\'notasource\' is an invalid sources parameter. Valid options: osm,oa,gn,wof,openstreetmap,openaddresses,geonames,whosonfirst' ] + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? inputs +json.geocoding.query['text'].should.eql 'a' +json.geocoding.query['size'].should.eql 10 diff --git a/test/ciao/autocomplete/sources_layers_invalid_combo.coffee b/test/ciao/autocomplete/sources_layers_invalid_combo.coffee new file mode 100644 index 00000000..3d27a200 --- /dev/null +++ b/test/ciao/autocomplete/sources_layers_invalid_combo.coffee @@ -0,0 +1,37 @@ + +#> sources and layers specified (invalid combo) +path: '/v1/autocomplete?text=a&sources=whosonfirst&layers=address' + +#? 200 ok +response.statusCode.should.be.equal 400 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.exist json.geocoding.errors +json.geocoding.errors.should.eql [ 'You have specified both the `sources` and `layers` parameters in a combination that will return no results: the whosonfirst source has nothing in the address layer' ] + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? inputs +json.geocoding.query['text'].should.eql 'a' +json.geocoding.query['size'].should.eql 10 +json.geocoding.query.layers.should.eql ["address"] +json.geocoding.query.sources.should.eql ["whosonfirst"] +should.not.exist json.geocoding.query['type'] diff --git a/test/ciao/autocomplete/sources_layers_valid_combo.coffee b/test/ciao/autocomplete/sources_layers_valid_combo.coffee new file mode 100644 index 00000000..2779009f --- /dev/null +++ b/test/ciao/autocomplete/sources_layers_valid_combo.coffee @@ -0,0 +1,35 @@ + +#> sources and layers specified +path: '/v1/autocomplete?text=a&sources=openaddresses&layers=address' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? inputs +json.geocoding.query['text'].should.eql 'a' +json.geocoding.query['size'].should.eql 10 +json.geocoding.query.layers.should.eql ["address"] +json.geocoding.query.sources.should.eql ["openaddresses"] diff --git a/test/ciao/autocomplete/sources_multiple.coffee b/test/ciao/autocomplete/sources_multiple.coffee new file mode 100644 index 00000000..5ddad82f --- /dev/null +++ b/test/ciao/autocomplete/sources_multiple.coffee @@ -0,0 +1,34 @@ + +#> sources filter +path: "/v1/autocomplete?text=a&sources=openstreetmap,geonames" + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header "charset", 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? inputs +json.geocoding.query['text'].should.eql 'a' +json.geocoding.query['size'].should.eql 10 +json.geocoding.query.sources.should.eql ["openstreetmap", "geonames"] diff --git a/test/ciao/autocomplete/sources_single.coffee b/test/ciao/autocomplete/sources_single.coffee new file mode 100644 index 00000000..d1df9c9d --- /dev/null +++ b/test/ciao/autocomplete/sources_single.coffee @@ -0,0 +1,34 @@ + +#> sources filter +path: '/v1/autocomplete?text=a&sources=openstreetmap' + +#? 200 ok +response.statusCode.should.be.equal 200 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected errors +should.not.exist json.geocoding.errors + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? inputs +json.geocoding.query['text'].should.eql 'a' +json.geocoding.query['size'].should.eql 10 +json.geocoding.query.sources.should.eql ["openstreetmap"] From 0abda099a6722cde040c60251ed7310f5e3dc193 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Thu, 24 Mar 2016 14:49:20 +0100 Subject: [PATCH 04/20] bugfixes and more tests --- query/autocomplete.js | 8 ++ .../autocomplete_with_source_filtering.js | 85 +++++++++++++++++ .../fixture/reverse_with_source_filtering.js | 51 ++++++++++ .../fixture/search_with_source_filtering.js | 93 +++++++++++++++++++ test/unit/query/autocomplete.js | 13 +++ test/unit/query/search.js | 13 +++ test/unit/sanitiser/_size.js | 8 +- test/unit/sanitiser/autocomplete.js | 2 +- 8 files changed, 268 insertions(+), 5 deletions(-) create mode 100644 test/unit/fixture/autocomplete_with_source_filtering.js create mode 100644 test/unit/fixture/reverse_with_source_filtering.js create mode 100644 test/unit/fixture/search_with_source_filtering.js diff --git a/query/autocomplete.js b/query/autocomplete.js index 4bc98c7c..ffc57396 100644 --- a/query/autocomplete.js +++ b/query/autocomplete.js @@ -41,6 +41,9 @@ query.score( views.focus_selected_layers( views.ngrams_strict ) ); query.score( peliasQuery.view.popularity( views.ngrams_strict ) ); query.score( peliasQuery.view.population( views.ngrams_strict ) ); +// non-scoring hard filters +query.filter( peliasQuery.view.sources ); + // -------------------------------- /** @@ -51,6 +54,11 @@ function generateQuery( clean ){ var vs = new peliasQuery.Vars( defaults ); + // sources + if( check.array(clean.sources) && clean.sources.length ){ + vs.var( 'sources', clean.sources ); + } + // mark the name as incomplete (user has not yet typed a comma) vs.var( 'input:name:isComplete', false ); diff --git a/test/unit/fixture/autocomplete_with_source_filtering.js b/test/unit/fixture/autocomplete_with_source_filtering.js new file mode 100644 index 00000000..22c12a5d --- /dev/null +++ b/test/unit/fixture/autocomplete_with_source_filtering.js @@ -0,0 +1,85 @@ + +module.exports = { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'must': [{ + 'match': { + 'name.default': { + 'analyzer': 'peliasPhrase', + 'boost': 100, + 'query': 'test', + 'type': 'phrase', + 'operator': 'and' + } + } + }], + 'should':[{ + 'function_score': { + 'query': { + 'match': { + 'name.default': { + 'analyzer': 'peliasPhrase', + 'boost': 100, + 'query': 'test', + 'type': 'phrase', + 'operator': 'and' + } + } + }, + 'max_boost': 20, + 'score_mode': 'first', + 'boost_mode': 'replace', + 'functions': [{ + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'popularity', + 'missing': 1 + }, + 'weight': 1 + }] + } + },{ + 'function_score': { + 'query': { + 'match': { + 'name.default': { + 'analyzer': 'peliasPhrase', + 'boost': 100, + 'query': 'test', + 'type': 'phrase', + 'operator': 'and' + } + } + }, + 'max_boost': 20, + 'score_mode': 'first', + 'boost_mode': 'replace', + 'functions': [{ + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'population', + 'missing': 1 + }, + 'weight': 3 + }] + } + }] + } + }, + 'filter': { + 'bool': { + 'must': [{ + 'terms': { + 'source': ['test_source'] + } + }] + } + } + } + }, + 'sort': [ '_score' ], + 'size': 20, + 'track_scores': true +}; diff --git a/test/unit/fixture/reverse_with_source_filtering.js b/test/unit/fixture/reverse_with_source_filtering.js new file mode 100644 index 00000000..28fce18b --- /dev/null +++ b/test/unit/fixture/reverse_with_source_filtering.js @@ -0,0 +1,51 @@ +var vs = require('../../../query/reverse_defaults'); + +module.exports = { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'must': [] + } + }, + 'filter': { + 'bool': { + 'must': [ + { + 'geo_distance': { + 'distance': '500km', + 'distance_type': 'plane', + 'optimize_bbox': 'indexed', + '_cache': true, + 'center_point': { + 'lat': 29.49136, + 'lon': -82.50622 + } + } + }, + { + 'terms': { + 'source': ['test'] + } + } + ] + } + } + } + }, + '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/search_with_source_filtering.js b/test/unit/fixture/search_with_source_filtering.js new file mode 100644 index 00000000..593eac5b --- /dev/null +++ b/test/unit/fixture/search_with_source_filtering.js @@ -0,0 +1,93 @@ + +module.exports = { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'must': [{ + 'match': { + 'name.default': { + 'query': 'test', + 'boost': 1, + 'analyzer': 'peliasOneEdgeGram' + } + } + }], + 'should': [{ + 'match': { + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'boost': 1, + 'slop': 2 + } + } + },{ + 'function_score': { + 'query': { + 'match': { + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2, + 'boost': 1 + } + } + }, + 'max_boost': 20, + 'score_mode': 'first', + 'boost_mode': 'replace', + 'functions': [{ + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'popularity', + 'missing': 1 + }, + 'weight': 1 + }] + } + },{ + 'function_score': { + 'query': { + 'match': { + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2, + 'boost': 1 + } + } + }, + 'max_boost': 20, + 'score_mode': 'first', + 'boost_mode': 'replace', + 'functions': [{ + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'population', + 'missing': 1 + }, + 'weight': 2 + }] + } + }] + } + }, + 'filter': { + 'bool': { + 'must': [{ + 'terms': { + 'source': ['test_source'] + } + }] + } + } + } + }, + 'sort': [ '_score' ], + 'size': 20, + 'track_scores': true +}; diff --git a/test/unit/query/autocomplete.js b/test/unit/query/autocomplete.js index dc973ddc..0e09457b 100644 --- a/test/unit/query/autocomplete.js +++ b/test/unit/query/autocomplete.js @@ -95,6 +95,19 @@ module.exports.tests.query = function(test, common) { t.deepEqual(compiled, expected, 'valid autocomplete query'); t.end(); }); + + test('valid sources filter', function(t) { + var query = generate({ + 'text': 'test', + 'sources': ['test_source'] + }); + + var compiled = JSON.parse( JSON.stringify( query ) ); + var expected = require('../fixture/autocomplete_with_source_filtering'); + + t.deepEqual(compiled, expected, 'valid autocomplete query with source filtering'); + t.end(); + }); }; module.exports.all = function (tape, common) { diff --git a/test/unit/query/search.js b/test/unit/query/search.js index f20122bb..e503311b 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -182,6 +182,19 @@ module.exports.tests.query = function(test, common) { t.end(); }); + test('valid sources filter', function(t) { + var query = generate({ + 'text': 'test', + 'sources': ['test_source'] + }); + + var compiled = JSON.parse( JSON.stringify( query ) ); + var expected = require('../fixture/search_with_source_filtering'); + + t.deepEqual(compiled, expected, 'valid search query with source filtering'); + t.end(); + }); + }; module.exports.all = function (tape, common) { diff --git a/test/unit/sanitiser/_size.js b/test/unit/sanitiser/_size.js index e4e61ac9..67c8bd84 100644 --- a/test/unit/sanitiser/_size.js +++ b/test/unit/sanitiser/_size.js @@ -6,7 +6,7 @@ module.exports.tests.sanitize_size = function(test, common) { test('size=0', function(t) { var raw = { size: 0 }; var clean = {}; - var res = sanitize(raw, clean); + var res = sanitize(/*defaults*/)(raw, clean); t.equal(res.errors.length, 0, 'should return no errors'); t.equal(res.warnings.length, 1, 'should return warning'); t.equal(res.warnings[0], 'out-of-range integer \'size\', using MIN_SIZE', 'check warning text'); @@ -17,7 +17,7 @@ module.exports.tests.sanitize_size = function(test, common) { test('size=10000', function(t) { var raw = { size: 10000 }; var clean = {}; - var res = sanitize(raw, clean); + var res = sanitize(/*defaults*/)(raw, clean); t.equal(res.errors.length, 0, 'should return no errors'); t.equal(res.warnings.length, 1, 'should return warning'); t.equal(res.warnings[0], 'out-of-range integer \'size\', using MAX_SIZE', 'check warning text'); @@ -28,7 +28,7 @@ module.exports.tests.sanitize_size = function(test, common) { test('size not set', function(t) { var raw = {}; var clean = {}; - var res = sanitize(raw, clean); + var res = sanitize(/*defaults*/)(raw, clean); t.equal(res.errors.length, 0, 'should return no errors'); t.equal(res.warnings.length, 0, 'should return no warning'); t.equal(clean.size, 10, 'default to 10'); @@ -41,7 +41,7 @@ module.exports.tests.sanitize_size = function(test, common) { test('size=' + size, function (t) { var raw = {size: size}; var clean = {}; - var res = sanitize(raw, clean); + var res = sanitize(/*defaults*/)(raw, clean); t.equal(res.errors.length, 0, 'should return no errors'); t.equal(res.warnings.length, 0, 'should return warning'); t.equal(clean.size, 5, 'set to correct integer'); diff --git a/test/unit/sanitiser/autocomplete.js b/test/unit/sanitiser/autocomplete.js index 4044d7f1..26bf9afb 100644 --- a/test/unit/sanitiser/autocomplete.js +++ b/test/unit/sanitiser/autocomplete.js @@ -4,7 +4,7 @@ module.exports.tests = {}; module.exports.tests.sanitisers = function(test, common) { test('check sanitiser list', function (t) { - var expected = ['singleScalarParameters', 'text', 'size', 'private', 'geo_autocomplete' ]; + var expected = ['singleScalarParameters', 'text', 'size', 'layers', 'sources', 'sources_and_layers', 'private', 'geo_autocomplete' ]; t.deepEqual(Object.keys(autocomplete.sanitiser_list), expected); t.end(); }); From 928597e02932a5fcb15df3c899974148fdcbae0a Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Thu, 7 Apr 2016 13:57:29 -0400 Subject: [PATCH 05/20] Fix admin parts parsing without spaces after delimeters --- helper/text_parser.js | 31 ++++++++---- test/unit/helper/text_parser.js | 13 +++++ test/unit/run.js | 88 ++++++++++++++++----------------- 3 files changed, 79 insertions(+), 53 deletions(-) diff --git a/helper/text_parser.js b/helper/text_parser.js index 7b9ffcfe..dcce1f63 100644 --- a/helper/text_parser.js +++ b/helper/text_parser.js @@ -2,11 +2,10 @@ var parser = require('addressit'); var extend = require('extend'); var type_mapping = require('../helper/type_mapping'); -var delim = ','; var check = require('check-types'); var logger = require('pelias-logger').get('api'); -module.exports = {}; +var DELIM = ','; /* * For performance, and to prefer POI and admin records, express a preference @@ -21,14 +20,26 @@ module.exports.get_layers = function get_layers(query) { module.exports.get_parsed_address = function get_parsed_address(query) { - var getAdminPartsBySplittingOnDelim = function(query) { + var getAdminPartsBySplittingOnDelim = function(queryParts) { // naive approach - for admin matching during query time // split 'flatiron, new york, ny' into 'flatiron' and 'new york, ny' - var delimIndex = query.indexOf(delim); + + //var delimIndex = query.indexOf(DELIM); + //var address = {}; + //if ( delimIndex !== -1 ) { + // address.name = query.substring(0, delimIndex); + // address.admin_parts = query.substring(delimIndex + 1).trim(); + //} + + var address = {}; - if ( delimIndex !== -1 ) { - address.name = query.substring(0, delimIndex); - address.admin_parts = query.substring(delimIndex + 1).trim(); + + if (queryParts.length > 1) { + address.name = queryParts[0].trim(); + + address.adminParts = queryParts.slice(1) + .map(function (part) { return part.trim(); }) + .join(DELIM + ' '); } return address; @@ -42,8 +53,10 @@ module.exports.get_parsed_address = function get_parsed_address(query) { } }; - var addressWithAdminParts = getAdminPartsBySplittingOnDelim(query); - var addressWithAddressParts= getAddressParts(query); + var queryParts = query.split(DELIM); + + var addressWithAdminParts = getAdminPartsBySplittingOnDelim(queryParts); + var addressWithAddressParts= getAddressParts(queryParts.join(DELIM + ' ')); var parsedAddress = extend(addressWithAdminParts, addressWithAddressParts); diff --git a/test/unit/helper/text_parser.js b/test/unit/helper/text_parser.js index 90b8fa52..93bd4a97 100644 --- a/test/unit/helper/text_parser.js +++ b/test/unit/helper/text_parser.js @@ -115,6 +115,19 @@ module.exports.tests.parse_address = function(test, common) { t.equal(address.postalcode, '06410', 'parsed zip'); t.end(); }); + test('valid address without spaces after commas', function(t) { + var query_string = '339 W Main St,Lancaster,PA'; + var address = parser.get_parsed_address(query_string); + + console.log(address); + + t.equal(typeof address, 'object', 'valid object for the address'); + t.equal(address.number, '339', 'parsed house number'); + t.equal(address.street, 'W Main St', 'parsed street'); + t.deepEqual(address.regions, ['Lancaster'], 'parsed city'); + t.deepEqual(address.state, 'PA', 'parsed state'); + t.end(); + }); }; diff --git a/test/unit/run.js b/test/unit/run.js index 2e8b382e..477c226f 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -9,51 +9,51 @@ var common = { }; var tests = [ - require('./controller/index'), - require('./controller/place'), - require('./controller/search'), - require('./helper/geojsonify'), - require('./helper/labelGenerator_default'), - require('./helper/labelGenerator_GBR'), - require('./helper/labelGenerator_SGP'), - require('./helper/labelGenerator_SWE'), - require('./helper/labelGenerator_USA'), - require('./helper/labelSchema'), + //require('./controller/index'), + //require('./controller/place'), + //require('./controller/search'), + //require('./helper/geojsonify'), + //require('./helper/labelGenerator_default'), + //require('./helper/labelGenerator_GBR'), + //require('./helper/labelGenerator_SGP'), + //require('./helper/labelGenerator_SWE'), + //require('./helper/labelGenerator_USA'), + //require('./helper/labelSchema'), require('./helper/text_parser'), - require('./helper/type_mapping'), - require('./helper/sizeCalculator'), - require('./middleware/confidenceScore'), - require('./middleware/confidenceScoreReverse'), - require('./middleware/distance'), - require('./middleware/localNamingConventions'), - require('./middleware/dedupe'), - require('./middleware/parseBBox'), - require('./query/autocomplete'), - require('./query/autocomplete_defaults'), - require('./query/search_defaults'), - require('./query/reverse_defaults'), - require('./query/reverse'), - require('./query/search'), - require('./sanitiser/_boundary_country'), - require('./sanitiser/_flag_bool'), - require('./sanitiser/_geo_common'), - require('./sanitiser/_geo_reverse'), - require('./sanitiser/_groups'), - require('./sanitiser/_ids'), - require('./sanitiser/_layers'), - require('./sanitiser/_single_scalar_parameters'), - require('./sanitiser/_size'), - require('./sanitiser/_sources'), - require('./sanitiser/_sources_and_layers'), - require('./sanitiser/_text'), - require('./sanitiser/_deprecate_quattroshapes'), - require('./src/backend'), - require('./sanitiser/autocomplete'), - require('./sanitiser/place'), - require('./sanitiser/reverse'), - require('./sanitiser/search'), - require('./service/mget'), - require('./service/search'), + //require('./helper/type_mapping'), + //require('./helper/sizeCalculator'), + //require('./middleware/confidenceScore'), + //require('./middleware/confidenceScoreReverse'), + //require('./middleware/distance'), + //require('./middleware/localNamingConventions'), + //require('./middleware/dedupe'), + //require('./middleware/parseBBox'), + //require('./query/autocomplete'), + //require('./query/autocomplete_defaults'), + //require('./query/search_defaults'), + //require('./query/reverse_defaults'), + //require('./query/reverse'), + //require('./query/search'), + //require('./sanitiser/_boundary_country'), + //require('./sanitiser/_flag_bool'), + //require('./sanitiser/_geo_common'), + //require('./sanitiser/_geo_reverse'), + //require('./sanitiser/_groups'), + //require('./sanitiser/_ids'), + //require('./sanitiser/_layers'), + //require('./sanitiser/_single_scalar_parameters'), + //require('./sanitiser/_size'), + //require('./sanitiser/_sources'), + //require('./sanitiser/_sources_and_layers'), + //require('./sanitiser/_text'), + //require('./sanitiser/_deprecate_quattroshapes'), + //require('./src/backend'), + //require('./sanitiser/autocomplete'), + //require('./sanitiser/place'), + //require('./sanitiser/reverse'), + //require('./sanitiser/search'), + //require('./service/mget'), + //require('./service/search'), ]; tests.map(function(t) { From 0870e303660aa616908711273a6fb5f605601f5d Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 12 Apr 2016 08:38:06 -0400 Subject: [PATCH 06/20] Refactored the label generation code to be simpler Labels are now, outside US/CA/UK, just name+city+country. Deduping now only occurs on the first 2 parts of the labels which fixes the "Luxembourg, Luxembourg" issue. --- helper/geojsonify.js | 2 + helper/labelGenerator.js | 45 +- helper/labelSchema.js | 70 +- test/unit/helper/geojsonify.js | 28 +- test/unit/helper/labelGenerator_CAN.js | 205 ++++++ test/unit/helper/labelGenerator_GBR.js | 167 ++++- test/unit/helper/labelGenerator_SGP.js | 51 -- test/unit/helper/labelGenerator_SWE.js | 53 -- test/unit/helper/labelGenerator_USA.js | 267 ++++--- test/unit/helper/labelGenerator_default.js | 345 +++------ test/unit/helper/labelGenerator_examples.js | 114 +++ test/unit/helper/labelSchema.js | 754 +------------------- test/unit/run.js | 4 +- 13 files changed, 765 insertions(+), 1340 deletions(-) create mode 100644 test/unit/helper/labelGenerator_CAN.js delete mode 100644 test/unit/helper/labelGenerator_SGP.js delete mode 100644 test/unit/helper/labelGenerator_SWE.js create mode 100644 test/unit/helper/labelGenerator_examples.js diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 1f9a60a8..67fed28e 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -34,6 +34,8 @@ var DETAILS_PROPS = [ 'locality', 'locality_id', 'locality_a', + 'borough', + 'borough_id', 'neighbourhood', 'neighbourhood_id', 'bounding_box' diff --git a/helper/labelGenerator.js b/helper/labelGenerator.js index 0f0eb9dd..eb35c86c 100644 --- a/helper/labelGenerator.js +++ b/helper/labelGenerator.js @@ -4,37 +4,28 @@ var _ = require('lodash'), module.exports = function( record ){ var schema = getSchema(record.country_a); + // in virtually all cases, this will be the `name` field var labelParts = getInitialLabel(record); - for (var key in schema) { - var valueFunction = schema[key]; - - labelParts = valueFunction(record, labelParts); + // iterate the schema + for (var field in schema) { + var valueFunction = schema[field]; + labelParts.push(valueFunction(record)); } - // NOTE: while it may seem odd to call `uniq` on the list of label parts, - // the effect is quite subtle. Take, for instance, a result for "Lancaster, PA" - // the pseudo-object is: - // { - // 'name': 'Lancaster', - // 'locality': 'Lancaster', - // 'region_a': 'PA', - // 'country_a': 'USA' - // } - // - // the code up to this point generates the label: - // `Lancaster, Lancaster, PA, USA` - // - // then the `unique` call reduces this to: - // `Lancaster, PA, USA` - // - // this code doesn't have the same effect in the case of a venue or address - // where the `name` field would contain the address or name of a point-of-interest - // - // Also see https://github.com/pelias/api/issues/429 for other ways that this is bad - // - // de-dupe, join, trim - return _.uniq( labelParts ).join(', ').trim(); + // retain only things that are defined + labelParts = labelParts.filter(function(v) { return !_.isUndefined(v); }); + + // first, dedupe the name and 1st label array elements + // this is used to ensure that the `name` and first admin hierarchy elements aren't repeated + // eg - `["Lancaster", "Lancaster", "PA", "United States"]` -> `["Lancaster", "PA", "United States"]` + var dedupedNameAndFirstLabelElement = _.uniq([labelParts.shift(), labelParts.shift()]); + + // second, unshift the deduped parts back onto the labelParts + labelParts.unshift.apply(labelParts, dedupedNameAndFirstLabelElement); + + // third, join with a comma and return + return labelParts.join(', '); }; diff --git a/helper/labelSchema.js b/helper/labelSchema.js index 5d3eba5d..e1f9921a 100644 --- a/helper/labelSchema.js +++ b/helper/labelSchema.js @@ -1,66 +1,62 @@ -var _ = require('lodash'), - check = require('check-types'); +var _ = require('lodash'); module.exports = { - 'USA': { - 'local': getFirstProperty(['localadmin', 'locality', 'neighbourhood', 'county']), - 'regional': getUsState, - 'country': getFirstProperty(['country_a']) + 'default': { + 'local': getFirstProperty(['locality']), + 'country': getFirstProperty(['country']) }, 'GBR': { - 'local': getFirstProperty(['neighbourhood', 'county', 'localadmin', 'locality', 'macroregion', 'region']), - 'regional': getFirstProperty(['county','country','region']) - }, - 'SGP': { - 'local': getFirstProperty(['neighbourhood', 'region', 'county', 'localadmin', 'locality']), - 'regional': getFirstProperty(['county','country','region']) + 'local': getFirstProperty(['locality']), + 'regional': getFirstProperty(['macroregion']), + 'country': getFirstProperty(['country']) }, - 'SWE': { - 'local': getFirstProperty(['neighbourhood', 'region', 'county', 'localadmin', 'locality']), - 'regional': getFirstProperty(['country']) + 'USA': { + 'borough': getFirstProperty(['borough']), + 'local': getFirstProperty(['locality']), + 'regional': getUsOrCaState, + 'country': getFirstProperty(['country']) }, - 'default': { - 'local': getFirstProperty(['localadmin', 'locality', 'neighbourhood', 'county', 'macroregion', 'region']), - 'regional': getFirstProperty(['country']) + 'CAN': { + 'local': getFirstProperty(['locality']), + 'regional': getUsOrCaState, + 'country': getFirstProperty(['country']) } }; // find the first field of record that has a non-empty value that's not already in labelParts function getFirstProperty(fields) { - return function(record, labelParts) { + return function(record) { for (var i = 0; i < fields.length; i++) { var fieldValue = record[fields[i]]; - if (check.nonEmptyString(fieldValue) && !_.includes(labelParts, fieldValue)) { - labelParts.push( fieldValue ); - return labelParts; + if (!_.isEmpty(fieldValue)) { + return fieldValue; } } - return labelParts; - }; } -// this function is exclusively used for figuring out which field to use for US States -// 1. if a US state is the most granular bit of info entered, the label should contain -// the full state name, eg: Pennsylvania, USA -// 2. otherwise, the state abbreviation should be used, eg: Lancaster, PA, USA +// this function is exclusively used for figuring out which field to use for US/CA States +// 1. if a US/CA state is the most granular bit of info entered, the label should contain +// the full state name, eg: Pennsylvania, USA and Ontario, CA +// 2. otherwise, the state abbreviation should be used, eg: Lancaster, PA, USA and Bruce, ON, CA // 3. if for some reason the abbreviation isn't available, use the full state name -function getUsState(record, labelParts) { +function getUsOrCaState(record) { if ('region' === record.layer && record.region) { - // add full state name when state is the most granular piece of info - labelParts.push(record.region); + // return full state name when state is the most granular piece of info + return record.region; + } else if (record.region_a) { - // otherwise just add the region code when available - labelParts.push(record.region_a); + // otherwise just return the region code when available + return record.region_a; + } else if (record.region) { - // add the full name when there's no region code available () - labelParts.push(record.region); - } + // return the full name when there's no region code available + return record.region; - return labelParts; + } } diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index 7b371b27..51704170 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -57,6 +57,7 @@ module.exports.tests.search = function(test, common) { 'country': 'United Kingdom', 'region': 'Islington', 'region_a': 'ISL', + 'macroregion': 'England', 'county': 'Angel', 'localadmin': 'test1', 'locality': 'test2', @@ -82,6 +83,7 @@ module.exports.tests.search = function(test, common) { 'country': 'United Kingdom', 'region': 'City And County Of The City Of London', 'region_a': 'COL', + 'macroregion': 'England', 'county': 'Smithfield', 'localadmin': 'test1', 'locality': 'test2', @@ -104,7 +106,7 @@ module.exports.tests.search = function(test, common) { 'region': 'New York', 'region_a': 'NY', 'county': 'New York', - 'localadmin': 'Manhattan', + 'borough': 'Manhattan', 'locality': 'New York', 'neighbourhood': 'Koreatown', 'category': [ @@ -132,10 +134,11 @@ module.exports.tests.search = function(test, common) { 'gid': 'source1:layer1:id1', 'layer': 'layer1', 'source': 'source1', - 'label': '\'Round Midnight Jazz and Blues Bar, test3, Angel', + 'label': '\'Round Midnight Jazz and Blues Bar, test2, England, United Kingdom', 'name': '\'Round Midnight Jazz and Blues Bar', 'country_a': 'GBR', 'country': 'United Kingdom', + 'macroregion': 'England', 'region': 'Islington', 'region_a': 'ISL', 'county': 'Angel', @@ -161,10 +164,11 @@ module.exports.tests.search = function(test, common) { 'gid': 'source2:layer2:id2', 'layer': 'layer2', 'source': 'source2', - 'label': 'Blues Cafe, test3, Smithfield', + 'label': 'Blues Cafe, test2, England, United Kingdom', 'name': 'Blues Cafe', 'country_a': 'GBR', 'country': 'United Kingdom', + 'macroregion': 'England', 'region': 'City And County Of The City Of London', 'region_a': 'COL', 'county': 'Smithfield', @@ -187,14 +191,14 @@ module.exports.tests.search = function(test, common) { 'gid': 'openstreetmap:venue:node:34633854', 'layer': 'venue', 'source': 'openstreetmap', - 'label': 'Empire State Building, Manhattan, NY, USA', + 'label': 'Empire State Building, Manhattan, New York, NY, United States', 'name': 'Empire State Building', 'country_a': 'USA', 'country': 'United States', 'region': 'New York', 'region_a': 'NY', 'county': 'New York', - 'localadmin': 'Manhattan', + 'borough': 'Manhattan', 'locality': 'New York', 'neighbourhood': 'Koreatown' } @@ -204,6 +208,7 @@ module.exports.tests.search = function(test, common) { test('geojsonify.search(doc)', function(t) { var json = geojsonify.search( input ); + t.deepEqual(json, expected, 'all docs mapped'); t.end(); }); @@ -282,13 +287,13 @@ module.exports.tests.search = function(test, common) { 'county_a': [ null ], - 'localadmin': [ + 'borough': [ 'Brooklyn' ], - 'localadmin_id': [ + 'borough_id': [ '404521211' ], - 'localadmin_a': [ + 'borough_a': [ null ], 'locality_id': [ @@ -329,8 +334,8 @@ module.exports.tests.search = function(test, common) { 'macrocounty_a': 'MacroCounty Abbreviation', 'county': 'Kings County', 'county_id': '102082361', - 'localadmin': 'Brooklyn', - 'localadmin_id': '404521211', + 'borough': 'Brooklyn', + 'borough_id': '404521211', 'locality': 'New York', 'locality_id': '85977539', 'bounding_box': { @@ -339,7 +344,7 @@ module.exports.tests.search = function(test, common) { 'min_lon': -73.8967895508, 'max_lon': -73.8665771484 }, - 'label': 'East New York, Brooklyn, NY, USA' + 'label': 'East New York, Brooklyn, New York, NY, United States' }, 'geometry': { 'type': 'Point', @@ -353,6 +358,7 @@ module.exports.tests.search = function(test, common) { }; var json = geojsonify.search( input ); + t.deepEqual(json, expected, 'all wanted properties exposed'); t.end(); }); diff --git a/test/unit/helper/labelGenerator_CAN.js b/test/unit/helper/labelGenerator_CAN.js new file mode 100644 index 00000000..f78a5a53 --- /dev/null +++ b/test/unit/helper/labelGenerator_CAN.js @@ -0,0 +1,205 @@ +var generator = require('../../../helper/labelGenerator'); + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('interface', function(t) { + t.equal(typeof generator, 'function', 'valid function'); + t.end(); + }); +}; + +module.exports.tests.canada = function(test, common) { + test('venue', function(t) { + var doc = { + 'name': 'venue name', + 'layer': 'venue', + 'housenumber': '1', + 'street': 'Main St', + 'neighbourhood': 'neighbourhood name', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'CAN', + 'country': 'Canada' + }; + t.equal(generator(doc),'venue name, locality name, region abbr, Canada'); + t.end(); + }); + + test('street', function(t) { + var doc = { + 'name': '1 Main St', + 'layer': 'address', + 'housenumber': '1', + 'street': 'Main St', + 'neighbourhood': 'neighbourhood name', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'CAN', + 'country': 'Canada' + }; + t.equal(generator(doc),'1 Main St, locality name, region abbr, Canada'); + t.end(); + }); + + test('neighbourhood', function(t) { + var doc = { + 'name': 'neighbourhood name', + 'layer': 'neighbourhood', + 'neighbourhood': 'neighbourhood name', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'CAN', + 'country': 'Canada' + }; + t.equal(generator(doc),'neighbourhood name, locality name, region abbr, Canada'); + t.end(); + }); + + test('locality', function(t) { + var doc = { + 'name': 'locality name', + 'layer': 'locality', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'CAN', + 'country': 'Canada' + }; + t.equal(generator(doc),'locality name, region abbr, Canada'); + t.end(); + }); + + test('localadmin', function(t) { + var doc = { + 'name': 'localadmin name', + 'layer': 'localadmin', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'CAN', + 'country': 'Canada' + }; + t.equal(generator(doc),'localadmin name, region abbr, Canada'); + t.end(); + }); + + test('county', function(t) { + var doc = { + 'name': 'county name', + 'layer': 'county', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'CAN', + 'country': 'Canada' + }; + t.equal(generator(doc),'county name, region abbr, Canada'); + t.end(); + }); + + test('macrocounty', function(t) { + var doc = { + 'name': 'macrocounty name', + 'layer': 'macrocounty', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'CAN', + 'country': 'Canada' + }; + t.equal(generator(doc),'macrocounty name, region abbr, Canada'); + t.end(); + }); + + test('region', function(t) { + var doc = { + 'name': 'region name', + 'layer': 'region', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'CAN', + 'country': 'Canada' + }; + t.equal(generator(doc),'region name, Canada'); + t.end(); + }); + + test('macroregion', function(t) { + var doc = { + 'name': 'macroregion name', + 'layer': 'macroregion', + 'macroregion': 'macroregion name', + 'country_a': 'CAN', + 'country': 'Canada' + }; + t.equal(generator(doc),'macroregion name, Canada'); + t.end(); + }); + + test('country', function(t) { + var doc = { + 'name': 'Canada', + 'layer': 'country', + 'country_a': 'CAN', + 'country': 'Canada' + }; + t.equal(generator(doc),'Canada'); + t.end(); + }); + + test('region should be used when region_a is not available', function(t) { + var doc = { + 'name': 'locality name', + 'layer': 'region', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'CAN', + 'country': 'Canada' + }; + t.equal(generator(doc),'locality name, region name, Canada', 'region should be used'); + t.end(); + }); + +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('label generator (CAN): ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/helper/labelGenerator_GBR.js b/test/unit/helper/labelGenerator_GBR.js index a0f74312..4b619464 100644 --- a/test/unit/helper/labelGenerator_GBR.js +++ b/test/unit/helper/labelGenerator_GBR.js @@ -10,69 +10,164 @@ module.exports.tests.interface = function(test, common) { }); }; -// GBR street address -module.exports.tests.one_main_street_uk = function(test, common) { - test('one main street uk', function(t) { +module.exports.tests.united_kingdom = function(test, common) { + test('venue', function(t) { var doc = { - 'name': '1 Main St', + 'name': 'venue name', + 'layer': 'venue', 'housenumber': '1', 'street': 'Main St', - 'postalcode': 'BT77 0BG', + 'neighbourhood': 'neighbourhood name', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', 'country_a': 'GBR', - 'country': 'United Kingdom', - 'region': 'Dungannon' + 'country': 'United Kingdom' }; - t.equal(generator(doc),'1 Main St, Dungannon, United Kingdom'); + t.equal(generator(doc),'venue name, locality name, macroregion name, United Kingdom'); t.end(); }); -}; -// GBR venue -module.exports.tests.hackney_city_farm = function(test, common) { - test('hackney city farm', function(t) { + test('street', function(t) { var doc = { - 'name': 'Hackney City Farm', + 'name': 'street address', + 'layer': 'address', + 'housenumber': '1', + 'street': 'Main St', + 'neighbourhood': 'neighbourhood name', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', 'country_a': 'GBR', - 'country': 'United Kingdom', - 'region': 'Hackney', - 'county': 'Greater London', - 'locality': 'London', - 'neighbourhood': 'Haggerston' + 'country': 'United Kingdom' }; - t.equal(generator(doc),'Hackney City Farm, Haggerston, Greater London'); + t.equal(generator(doc),'street address, locality name, macroregion name, United Kingdom'); t.end(); }); -}; -// GBR country -module.exports.tests.wales = function(test, common) { - test('wales', function(t) { + test('neighbourhood', function(t) { var doc = { - 'name': 'Wales', + 'name': 'neighbourhood name', + 'layer': 'neighbourhood', + 'neighbourhood': 'neighbourhood name', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', 'country_a': 'GBR', - 'country': 'United Kingdom', - 'region': 'Wales' + 'country': 'United Kingdom' }; - t.equal(generator(doc),'Wales, United Kingdom'); + t.equal(generator(doc),'neighbourhood name, locality name, macroregion name, United Kingdom'); + t.end(); + }); + + test('locality', function(t) { + var doc = { + 'name': 'locality name', + 'layer': 'locality', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'GBR', + 'country': 'United Kingdom' + }; + t.equal(generator(doc),'locality name, macroregion name, United Kingdom'); + t.end(); + }); + + test('localadmin', function(t) { + var doc = { + 'name': 'localadmin name', + 'layer': 'localadmin', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'GBR', + 'country': 'United Kingdom' + }; + t.equal(generator(doc),'localadmin name, macroregion name, United Kingdom'); + t.end(); + }); + + test('county', function(t) { + var doc = { + 'name': 'county name', + 'layer': 'county', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'GBR', + 'country': 'United Kingdom' + }; + t.equal(generator(doc),'county name, macroregion name, United Kingdom'); + t.end(); + }); + + test('macrocounty', function(t) { + var doc = { + 'name': 'macrocounty name', + 'layer': 'macrocounty', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'GBR', + 'country': 'United Kingdom' + }; + t.equal(generator(doc),'macrocounty name, macroregion name, United Kingdom'); t.end(); }); -}; -// GBR macroregion -module.exports.tests.macroregion_trumps_region = function(test, common) { - test('macroregion should trump region when none of neighbourhood, county, localadmin, locality are available', function(t) { + test('region', function(t) { var doc = { - 'name': 'Name', + 'name': 'region name', + 'layer': 'region', + 'region': 'region name', + 'macroregion': 'macroregion name', 'country_a': 'GBR', - 'country': 'Country Name', - 'macroregion': 'Macroregion Name', - 'region': 'Region Name' + 'country': 'United Kingdom' }; + t.equal(generator(doc),'region name, macroregion name, United Kingdom'); + t.end(); + }); - t.equal(generator(doc), 'Name, Macroregion Name, Country Name'); + test('macroregion', function(t) { + var doc = { + 'name': 'macroregion name', + 'layer': 'macroregion', + 'macroregion': 'macroregion name', + 'country_a': 'GBR', + 'country': 'United Kingdom' + }; + t.equal(generator(doc),'macroregion name, United Kingdom'); t.end(); + }); + test('country', function(t) { + var doc = { + 'name': 'United Kingdom', + 'layer': 'country', + 'postalcode': 'postalcode', + 'country_a': 'GBR', + 'country': 'United Kingdom' + }; + t.equal(generator(doc),'United Kingdom'); + t.end(); }); + }; module.exports.all = function (tape, common) { diff --git a/test/unit/helper/labelGenerator_SGP.js b/test/unit/helper/labelGenerator_SGP.js deleted file mode 100644 index 622f8915..00000000 --- a/test/unit/helper/labelGenerator_SGP.js +++ /dev/null @@ -1,51 +0,0 @@ - -var generator = require('../../../helper/labelGenerator'); - -module.exports.tests = {}; - -module.exports.tests.interface = function(test, common) { - test('interface', function(t) { - t.equal(typeof generator, 'function', 'valid function'); - t.end(); - }); -}; - -// SGP region -module.exports.tests.north_west_singapore = function(test, common) { - test('north west singapore', function(t) { - var doc = { - 'name': 'North West', - 'country_a': 'SGP', - 'country': 'Singapore', - 'region': 'North West' - }; - t.equal(generator(doc),'North West, Singapore'); - t.end(); - }); -}; - -// SGP venue -module.exports.tests.singapore_mcdonalds = function(test, common) { - test('singapore_mcdonalds', function(t) { - var doc = { - 'name': 'McDonald\'s', - 'country_a': 'SGP', - 'country': 'Singapore', - 'region': 'Central Singapore', - 'locality': 'Singapore' - }; - t.equal(generator(doc),'McDonald\'s, Central Singapore, Singapore'); - t.end(); - }); -}; - -module.exports.all = function (tape, common) { - - function test(name, testFunction) { - return tape('label generator: ' + name, testFunction); - } - - for( var testCase in module.exports.tests ){ - module.exports.tests[testCase](test, common); - } -}; diff --git a/test/unit/helper/labelGenerator_SWE.js b/test/unit/helper/labelGenerator_SWE.js deleted file mode 100644 index e3f8534d..00000000 --- a/test/unit/helper/labelGenerator_SWE.js +++ /dev/null @@ -1,53 +0,0 @@ - -var generator = require('../../../helper/labelGenerator'); - -module.exports.tests = {}; - -module.exports.tests.interface = function(test, common) { - test('interface', function(t) { - t.equal(typeof generator, 'function', 'valid function'); - t.end(); - }); -}; - -// SWE city -module.exports.tests.skane1 = function(test, common) { - test('skåne 1', function(t) { - var doc = { - 'name': 'Malmö', - 'country_a': 'SWE', - 'country': 'Sweden', - 'region': 'Skåne', - 'county': 'Malmö' - }; - t.equal(generator(doc),'Malmö, Skåne, Sweden'); - t.end(); - }); -}; - -// SWE city -module.exports.tests.skane2 = function(test, common) { - test('skåne 2', function(t) { - var doc = { - 'name': 'Malmö', - 'country_a': 'SWE', - 'country': 'Sweden', - 'region': 'Skåne', - 'county': 'Malmö', - 'locality': 'Malmö' - }; - t.equal(generator(doc),'Malmö, Skåne, Sweden'); - t.end(); - }); -}; - -module.exports.all = function (tape, common) { - - function test(name, testFunction) { - return tape('label generator: ' + name, testFunction); - } - - for( var testCase in module.exports.tests ){ - module.exports.tests[testCase](test, common); - } -}; diff --git a/test/unit/helper/labelGenerator_USA.js b/test/unit/helper/labelGenerator_USA.js index 7791aa24..57e77f32 100644 --- a/test/unit/helper/labelGenerator_USA.js +++ b/test/unit/helper/labelGenerator_USA.js @@ -9,218 +9,213 @@ module.exports.tests.interface = function(test, common) { }); }; -module.exports.tests.localadmin = function(test, common) { - test('localadmin should trump locality, neighbourhood, and county', function(t) { +module.exports.tests.united_states = function(test, common) { + test('venue', function(t) { var doc = { - 'name': 'Default Name', + 'name': 'venue name', + 'layer': 'venue', + 'housenumber': '1', + 'street': 'Main St', + 'neighbourhood': 'neighbourhood name', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', 'country_a': 'USA', - 'country': 'United States', - 'region': 'Region Name', - 'region_a': 'Region Abbr', - 'county': 'County Name', - 'localadmin': 'LocalAdmin Name', - 'locality': 'Locality Name', - 'neighbourhood': 'Neighbourhood Name' + 'country': 'United States' }; - t.equal(generator(doc),'Default Name, LocalAdmin Name, Region Abbr, USA'); + t.equal(generator(doc),'venue name, locality name, region abbr, United States'); t.end(); }); -}; -module.exports.tests.locality = function(test, common) { - test('locality should trump neighbourhood and county when localadmin not available', function(t) { + test('street', function(t) { var doc = { - 'name': 'Default Name', + 'name': '1 Main St', + 'layer': 'address', + 'housenumber': '1', + 'street': 'Main St', + 'neighbourhood': 'neighbourhood name', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', 'country_a': 'USA', - 'country': 'United States', - 'region': 'Region Name', - 'region_a': 'Region Abbr', - 'county': 'County Name', - 'locality': 'Locality Name', - 'neighbourhood': 'Neighbourhood Name' + 'country': 'United States' }; - t.equal(generator(doc),'Default Name, Locality Name, Region Abbr, USA'); + t.equal(generator(doc),'1 Main St, locality name, region abbr, United States'); t.end(); }); -}; -module.exports.tests.neighbourhood = function(test, common) { - test('neighbourhood should trump county when neither localadmin nor locality', function(t) { + test('neighbourhood', function(t) { var doc = { - 'name': 'Default Name', + 'name': 'neighbourhood name', + 'layer': 'neighbourhood', + 'neighbourhood': 'neighbourhood name', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', 'country_a': 'USA', - 'country': 'United States', - 'region': 'Region Name', - 'region_a': 'Region Abbr', - 'county': 'County Name', - 'neighbourhood': 'Neighbourhood Name' + 'country': 'United States' }; - t.equal(generator(doc),'Default Name, Neighbourhood Name, Region Abbr, USA'); + t.equal(generator(doc),'neighbourhood name, locality name, region abbr, United States'); t.end(); }); -}; -module.exports.tests.county = function(test, common) { - test('county should be used when localadmin, locality, and neighbourhood are not available', function(t) { + test('venue in borough', function(t) { var doc = { - 'name': 'Default Name', + 'name': 'venue name', + 'layer': 'borough', + 'neighbourhood': 'neighbourhood name', + 'borough': 'borough name', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', 'country_a': 'USA', - 'country': 'United States', - 'region': 'Region Name', - 'region_a': 'Region Abbr', - 'county': 'County Name' + 'country': 'United States' }; - t.equal(generator(doc),'Default Name, County Name, Region Abbr, USA'); + t.equal(generator(doc),'venue name, borough name, locality name, region abbr, United States'); t.end(); }); -}; -module.exports.tests.region = function(test, common) { - test('region should be used when region_a is not available', function(t) { + test('locality', function(t) { var doc = { - 'name': 'Default Name', + 'name': 'locality name', + 'layer': 'locality', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', 'country_a': 'USA', - 'country': 'United States', - 'region': 'Region Name' + 'country': 'United States' }; - t.equal(generator(doc),'Default Name, Region Name, USA'); + t.equal(generator(doc),'locality name, region abbr, United States'); t.end(); }); -}; -// USA geonames state -module.exports.tests.region_geonames = function(test, common) { - test('default name should not be prepended when source=geonames and layer=region', function(t) { + test('localadmin', function(t) { var doc = { - 'name': 'Region Name', + 'name': 'localadmin name', + 'layer': 'localadmin', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', 'country_a': 'USA', - 'country': 'United States', - 'region': 'Region Name', - 'region_a': 'Region Abbr', - 'source': 'geonames', - 'layer': 'region' + 'country': 'United States' }; - t.equal(generator(doc),'Region Name, USA'); + t.equal(generator(doc),'localadmin name, region abbr, United States'); t.end(); }); -}; -// USA whosonfirst state -module.exports.tests.region_whosonfirst = function(test, common) { - test('default name should not be prepended when source=whosonfirst and layer=region', function(t) { + test('county', function(t) { var doc = { - 'name': 'Region Name', + 'name': 'county name', + 'layer': 'county', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', 'country_a': 'USA', - 'country': 'United States', - 'region': 'Region Name', - 'region_a': 'Region Abbr', - 'source': 'whosonfirst', - 'layer': 'region' + 'country': 'United States' }; - t.equal(generator(doc),'Region Name, USA'); + t.equal(generator(doc),'county name, region abbr, United States'); t.end(); }); -}; -// USA non-geonames/whosonfirst state -module.exports.tests.region_other_source = function(test, common) { - test('default name should be prepended when layer=region and source is not whosonfirst or geonames', function(t) { + test('macrocounty', function(t) { var doc = { - 'name': 'Default Name', + 'name': 'macrocounty name', + 'layer': 'macrocounty', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', 'country_a': 'USA', - 'country': 'United States', - 'region': 'Region Name', - 'region_a': 'Region Abbr', - 'source': 'not geonames or whosonfirst', - 'layer': 'region' + 'country': 'United States' }; - t.equal(generator(doc),'Default Name, Region Name, USA',generator(doc)); + t.equal(generator(doc),'macrocounty name, region abbr, United States'); t.end(); }); -}; -// major USA city -module.exports.tests.san_francisco = function(test, common) { - test('san francisco', function(t) { + test('region', function(t) { var doc = { - 'name': 'San Francisco', + 'name': 'region name', + 'layer': 'region', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', 'country_a': 'USA', - 'country': 'United States', - 'region': 'California', - 'region_a': 'CA', - 'county': 'San Francisco County', - 'locality': 'San Francisco' + 'country': 'United States' }; - t.equal(generator(doc),'San Francisco, San Francisco County, CA, USA'); + t.equal(generator(doc),'region name, United States'); t.end(); }); -}; -// USA venue -module.exports.tests.nyc_office = function(test, common) { - test('30 West 26th Street', function(t) { + test('macroregion', function(t) { var doc = { - 'name': '30 West 26th Street', - 'housenumber': '30', - 'street': 'West 26th Street', - 'postalcode': '10010', + 'name': 'macroregion name', + 'layer': 'macroregion', + 'macroregion': 'macroregion name', 'country_a': 'USA', - 'country': 'United States', - 'region': 'New York', - 'region_a': 'NY', - 'county': 'New York County', - 'localadmin': 'Manhattan', - 'locality': 'New York', - 'neighbourhood': 'Flatiron District' + 'country': 'United States' }; - t.equal(generator(doc),'30 West 26th Street, Manhattan, NY, USA'); + t.equal(generator(doc),'macroregion name, United States'); t.end(); }); -}; -// USA NYC eatery -module.exports.tests.nyc_bakery = function(test, common) { - test('New York Bakery', function(t) { + test('country', function(t) { var doc = { - 'name': 'New York Bakery', - 'housenumber': '51 W', - 'street': '29th', + 'name': 'United States', + 'layer': 'country', 'country_a': 'USA', - 'country': 'United States', - 'region': 'New York', - 'region_a': 'NY', - 'county': 'New York County', - 'localadmin': 'Manhattan', - 'locality': 'New York', - 'neighbourhood': 'Koreatown' + 'country': 'United States' }; - t.equal(generator(doc),'New York Bakery, Manhattan, NY, USA'); + t.equal(generator(doc),'United States'); t.end(); }); -}; -// USA SFC building -module.exports.tests.ferry_building = function(test, common) { - test('Ferry Building', function(t) { + test('region should be used when region_a is not available', function(t) { var doc = { - 'name': 'Ferry Building', + 'name': 'locality name', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', 'country_a': 'USA', - 'country': 'United States', - 'region': 'California', - 'region_a': 'CA', - 'county': 'San Francisco County', - 'locality': 'San Francisco', - 'neighbourhood': 'Financial District' + 'country': 'United States' }; - t.equal(generator(doc),'Ferry Building, San Francisco, CA, USA'); + t.equal(generator(doc),'locality name, region name, United States', 'region should be used'); t.end(); }); + }; module.exports.all = function (tape, common) { function test(name, testFunction) { - return tape('label generator: ' + name, testFunction); + return tape('label generator (USA): ' + name, testFunction); } for( var testCase in module.exports.tests ){ diff --git a/test/unit/helper/labelGenerator_default.js b/test/unit/helper/labelGenerator_default.js index cbb76f05..7645610f 100644 --- a/test/unit/helper/labelGenerator_default.js +++ b/test/unit/helper/labelGenerator_default.js @@ -10,296 +10,163 @@ module.exports.tests.interface = function(test, common) { }); }; -// AUS state -module.exports.tests.new_south_wales = function(test, common) { - test('new south wales', function(t) { +module.exports.tests.default_country = function(test, common) { + test('venue', function(t) { var doc = { - 'name': 'New South Wales', - 'country_a': 'AUS', - 'country': 'Australia', - 'region': 'New South Wales' - }; - t.equal(generator(doc),'New South Wales, Australia'); - t.end(); - }); -}; - -// IND state -module.exports.tests.west_bengal = function(test, common) { - test('west bengal', function(t) { - var doc = { - 'name': 'West Bengal', - 'country_a': 'IND', - 'country': 'India', - 'region': 'West Bengal' - }; - t.equal(generator(doc),'West Bengal, India'); - t.end(); - }); -}; - -// IND city -module.exports.tests.bangalore = function(test, common) { - test('bangalore', function(t) { - var doc = { - 'name': 'Bangalore', - 'country_a': 'IND', - 'country': 'India', - 'region': 'Karnataka', - 'county': 'Bangalore', - 'locality': 'Bangalore' - }; - t.equal(generator(doc),'Bangalore, Karnataka, India'); - t.end(); - }); -}; - -// IND region of city -module.exports.tests.sarjapur = function(test, common) { - test('Sarjapur', function(t) { - var doc = { - 'name': 'Sarjapur', - 'country_a': 'IND', - 'country': 'India', - 'region': 'Karnataka' - }; - t.equal(generator(doc),'Sarjapur, Karnataka, India'); - t.end(); - }); -}; - -// IND region of city 2 -module.exports.tests.bengaluru_east = function(test, common) { - test('Bengaluru East', function(t) { - var doc = { - 'name': 'Bengaluru East', - 'country_a': 'IND', - 'country': 'India', - 'region': 'Karnataka', - 'county': 'Bangalore', - 'locality': 'Bangalore', - 'neighbourhood': 'Fraser Town' - }; - t.equal(generator(doc),'Bengaluru East, Bangalore, India'); - t.end(); - }); -}; - -// AUS area -// https://en.wikipedia.org/wiki/Shire_of_Wellington -module.exports.tests.wellington_victoria = function(test, common) { - test('Wellington, Victoria, Australia', function(t) { - var doc = { - 'name': 'Wellington', - 'country_a': 'AUS', - 'country': 'Australia', - 'region': 'Victoria', - 'county': 'Wellington' - }; - t.equal(generator(doc),'Wellington, Victoria, Australia'); - t.end(); - }); -}; - -// IRQ region -module.exports.tests.arbil = function(test, common) { - test('arbil', function(t) { - var doc = { - 'name': 'Arbil', - 'country_a': 'IRQ', - 'country': 'Iraq', - 'region': 'Arbil' - }; - t.equal(generator(doc),'Arbil, Iraq'); - t.end(); - }); -}; - -// ESP city -module.exports.tests.madrid = function(test, common) { - test('madrid', function(t) { - var doc = { - 'name': 'Madrid', - 'country_a': 'ESP', - 'country': 'Spain', - 'region': 'Madrid' - }; - t.equal(generator(doc),'Madrid, Spain'); - t.end(); - }); -}; - -// DEU street address -module.exports.tests.one_grolmanstrasse = function(test, common) { - test('one grolmanstrasse', function(t) { - var doc = { - 'name': '1 Grolmanstraße', + 'name': 'venue name', + 'layer': 'venue', 'housenumber': '1', - 'street': 'Grolmanstraße', - 'postalcode': '10623', - 'country_a': 'DEU', - 'country': 'Germany', - 'region': 'Berlin', - 'county': 'Berlin', - 'locality': 'Berlin', - 'neighbourhood': 'Halensee' + 'street': 'Main St', + 'neighbourhood': 'neighbourhood name', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'country code', + 'country': 'country name' }; - t.equal(generator(doc),'1 Grolmanstraße, Berlin, Germany'); + t.equal(generator(doc),'venue name, locality name, country name'); t.end(); }); -}; - -// NZD country -module.exports.tests.new_zealand = function(test, common) { - test('new zealand', function(t) { - var doc = { - 'name': 'New Zealand', - 'country_a': 'NZL', - 'country': 'New Zealand' - }; - t.equal(generator(doc),'New Zealand'); - t.end(); - }); -}; -// IRL country -module.exports.tests.republic_of_ireland = function(test, common) { - test('northern ireland', function(t) { + test('street', function(t) { var doc = { - 'name': 'Ireland', - 'country_a': 'IRL', - 'country': 'Ireland' + 'name': 'address', + 'layer': 'address', + 'housenumber': '1', + 'street': 'Main St', + 'neighbourhood': 'neighbourhood name', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'country code', + 'country': 'country name' }; - // !! this is not part of the UK !! - t.equal(generator(doc),'Ireland'); + t.equal(generator(doc),'address, locality name, country name'); t.end(); }); -}; -// THA province -module.exports.tests.krabi_province = function(test, common) { - test('Krabi Provence', function(t) { + test('neighbourhood', function(t) { var doc = { - 'name': 'Krabi', - 'country_a': 'THA', - 'country': 'Thailand', - 'region': 'Krabi' + 'name': 'neighbourhood name', + 'layer': 'neighbourhood', + 'neighbourhood': 'neighbourhood name', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'country code', + 'country': 'country name' }; - t.equal(generator(doc),'Krabi, Thailand'); + t.equal(generator(doc),'neighbourhood name, locality name, country name'); t.end(); }); -}; -// THA island -module.exports.tests.koh_lanta = function(test, common) { - test('Koh Lanta', function(t) { + test('locality', function(t) { var doc = { - 'name': 'Ko Lanta', - 'country_a': 'THA', - 'country': 'Thailand', - 'region': 'Krabi' + 'name': 'locality name', + 'layer': 'locality', + 'locality': 'locality name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'country code', + 'country': 'country name' }; - t.equal(generator(doc),'Ko Lanta, Krabi, Thailand'); + t.equal(generator(doc),'locality name, country name'); t.end(); }); -}; -// NZD cafe -module.exports.tests.black_dog_cafe = function(test, common) { - test('Black Dog Cafe', function(t) { + test('localadmin', function(t) { var doc = { - 'name': 'Black Dog Cafe', - 'country_a': 'NZL', - 'country': 'New Zealand', - 'region': 'Auckland Region', - 'county': 'Auckland' + 'name': 'localadmin name', + 'layer': 'localadmin', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'country code', + 'country': 'country name' }; - t.equal(generator(doc),'Black Dog Cafe, Auckland, New Zealand'); + t.equal(generator(doc),'localadmin name, country name'); t.end(); }); -}; -// NZD cafe 2 -module.exports.tests.beach_bablyon = function(test, common) { - test('Beach Bablyon', function(t) { + test('county', function(t) { var doc = { - 'name': 'Beach Bablyon', - 'country_a': 'NZL', - 'country': 'New Zealand', - 'region': 'Wellington Region', - 'county': 'Wellington City', - 'locality': 'Wellington', - 'neighbourhood': 'Oriental Bay' + 'name': 'county name', + 'layer': 'county', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'country code', + 'country': 'country name' }; - t.equal(generator(doc),'Beach Bablyon, Wellington, New Zealand'); + t.equal(generator(doc),'county name, country name'); t.end(); }); -}; -// NZD tourism -module.exports.tests.waiotapu = function(test, common) { - test('Waiotapu', function(t) { + test('macrocounty', function(t) { var doc = { - 'name': 'Waiotapu', - 'country_a': 'NZL', - 'country': 'New Zealand', - 'region': 'Bay of Plenty', - 'county': 'Rotorua District' + 'name': 'macrocounty name', + 'layer': 'macrocounty', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'country code', + 'country': 'country name' }; - t.equal(generator(doc),'Waiotapu, Rotorua District, New Zealand'); + t.equal(generator(doc),'macrocounty name, country name'); t.end(); }); -}; - -module.exports.tests.non_us_or_ca_region = function(test, common) { - test('geonames US', function(t) { + test('region', function(t) { var doc = { - 'name': 'Default Name', - 'country_a': 'XYZ', - 'country': 'Full Country Name', - 'region': 'Full Region Name', + 'name': 'region name', 'layer': 'region', - 'source': 'geonames' + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'country code', + 'country': 'country name' }; - - t.equal(generator(doc), 'Default Name, Full Region Name, Full Country Name'); + t.equal(generator(doc),'region name, country name'); t.end(); - }); - test('whosonfirst US', function(t) { + test('macroregion', function(t) { var doc = { - 'name': 'Default Name', - 'country_a': 'XYZ', - 'country': 'Full Country Name', - 'region': 'Full Region Name', - 'layer': 'region', - 'source': 'whosonfirst' + 'name': 'macroregion name', + 'layer': 'macroregion', + 'macroregion': 'macroregion name', + 'country_a': 'country code', + 'country': 'country name' }; - - t.equal(generator(doc), 'Default Name, Full Region Name, Full Country Name'); + t.equal(generator(doc),'macroregion name, country name'); t.end(); - }); -}; - -// macroregion -module.exports.tests.macroregion_trumps_region = function(test, common) { - test('macroregion should trump region when none of localadmin, locality, neighbourhood, county are available', function(t) { + test('country', function(t) { var doc = { - 'name': 'Name', - 'country_a': 'Country abbreviation', - 'country': 'Country Name', - 'macroregion': 'Macroregion Name', - 'region': 'Region Name' + 'name': 'country name', + 'layer': 'country', + 'country_a': 'country code', + 'country': 'country name' }; - - t.equal(generator(doc), 'Name, Macroregion Name, Country Name'); + t.equal(generator(doc),'country name'); t.end(); - }); + }; module.exports.all = function (tape, common) { diff --git a/test/unit/helper/labelGenerator_examples.js b/test/unit/helper/labelGenerator_examples.js new file mode 100644 index 00000000..180c9b90 --- /dev/null +++ b/test/unit/helper/labelGenerator_examples.js @@ -0,0 +1,114 @@ +var generator = require('../../../helper/labelGenerator'); + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('interface', function(t) { + t.equal(typeof generator, 'function', 'valid function'); + t.end(); + }); +}; + +module.exports.tests.canada = function(test, common) { + test('venue', function(t) { + var doc = { + 'name': 'Tim Horton\'s', + 'layer': 'venue', + 'housenumber': '1', + 'street': 'Main St', + 'neighbourhood': 'College Heights', + 'locality': 'Thunder Bay', + 'region_a': 'ON', + 'region': 'Ontario', + 'country_a': 'CAN', + 'country': 'Canada' + }; + t.equal(generator(doc),'Tim Horton\'s, Thunder Bay, ON, Canada'); + t.end(); + }); + + test('address', function(t) { + var doc = { + 'name': '1 Main St', + 'layer': 'venue', + 'housenumber': '1', + 'street': 'Main St', + 'locality': 'Truth or Consequences', + 'region_a': 'NM', + 'region': 'New Mexico', + 'country_a': 'USA', + 'country': 'United States' + }; + t.equal(generator(doc),'1 Main St, Truth or Consequences, NM, United States'); + t.end(); + }); + + test('eiffel tower', function(t) { + var doc = { + 'name': 'Tour Eiffel', + 'layer': 'venue', + 'neighbourhood': 'Quartier du Gros-Caillou', + 'locality': 'Paris', + 'region': 'Paris', + 'country_a': 'FRA', + 'country': 'France' + }; + t.equal(generator(doc),'Tour Eiffel, Paris, France'); + t.end(); + }); + + test('France street address', function(t) { + var doc = { + 'name': '74 rue de rivoli', + 'layer': 'address', + 'housenumber': '74', + 'street': 'Rue de Rivoli', + 'neighbourhood': 'Quartier Saint-Merri', + 'locality': 'Paris', + 'region': 'Paris', + 'country_a': 'FRA', + 'country': 'France' + }; + t.equal(generator(doc),'74 rue de rivoli, Paris, France'); + t.end(); + }); + + test('France neighbourhood', function(t) { + var doc = { + 'name': 'Grange aux Belles Terrage', + 'layer': 'neighbourhood', + 'neighbourhood': 'Grange aux Belles Terrage', + 'locality': 'Paris', + 'region': 'Paris', + 'country_a': 'FRA', + 'country': 'France' + }; + t.equal(generator(doc),'Grange aux Belles Terrage, Paris, France'); + t.end(); + }); + + test('Luxembourg (the city) in Luxembourg', function(t) { + var doc = { + 'name': 'Luxembourg', + 'layer': 'locality', + 'locality': 'Luxembourg', + 'country_a': 'LUX', + 'country': 'Luxembourg' + }; + // console.error(generator(doc)); + t.equal(generator(doc),'Luxembourg, Luxembourg'); + t.end(); + }); + +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('label generator (CAN): ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/helper/labelSchema.js b/test/unit/helper/labelSchema.js index edbf2b77..9b0a35f3 100644 --- a/test/unit/helper/labelSchema.js +++ b/test/unit/helper/labelSchema.js @@ -13,20 +13,18 @@ module.exports.tests.interface = function(test, common) { }; module.exports.tests.supported_countries = function(test, common) { - test('support countries', function(t) { + test('supported countries', function(t) { var supported_countries = Object.keys(schemas); t.notEquals(supported_countries.indexOf('USA'), -1); + t.notEquals(supported_countries.indexOf('CAN'), -1); t.notEquals(supported_countries.indexOf('GBR'), -1); - t.notEquals(supported_countries.indexOf('SGP'), -1); - t.notEquals(supported_countries.indexOf('SWE'), -1); t.notEquals(supported_countries.indexOf('default'), -1); - t.equals(supported_countries.length, 5); + t.equals(supported_countries.length, 4); - t.equals(Object.keys(schemas.USA).length, 3); - t.equals(Object.keys(schemas.GBR).length, 2); - t.equals(Object.keys(schemas.SGP).length, 2); - t.equals(Object.keys(schemas.SWE).length, 2); + t.equals(Object.keys(schemas.USA).length, 4); + t.equals(Object.keys(schemas.CAN).length, 3); + t.equals(Object.keys(schemas.GBR).length, 3); t.equals(Object.keys(schemas.default).length, 2); t.end(); @@ -34,746 +32,6 @@ module.exports.tests.supported_countries = function(test, common) { }); }; -module.exports.tests.usa = function(test, common) { - test('USA.local should use localadmin value over locality, neighbourhood, and county', function(t) { - var record = { - localadmin: 'localadmin value', - locality: 'locality value', - neighbourhood: 'neighbourhood value', - county: 'county value' - }; - - var labelParts = ['initial value']; - - var f = schemas.USA.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'localadmin value']); - t.end(); - - }); - - test('USA.local should use locality value over neighbourhood and county when no localadmin', function(t) { - var record = { - locality: 'locality value', - neighbourhood: 'neighbourhood value', - county: 'county value' - }; - - var labelParts = ['initial value']; - - var f = schemas.USA.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'locality value']); - t.end(); - - }); - - test('USA.local should use neighbourhood value over county when no localadmin or locality', function(t) { - var record = { - neighbourhood: 'neighbourhood value', - county: 'county value' - }; - - var labelParts = ['initial value']; - - var f = schemas.USA.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'neighbourhood value']); - t.end(); - - }); - - test('USA.local should use county value when no localadmin, locality, or neighbourhood', function(t) { - var record = { - county: 'county value' - }; - - var labelParts = ['initial value']; - - var f = schemas.USA.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'county value']); - t.end(); - - }); - - test('USA.local should not modify labelParts if none of localadmin, locality, neighbourhood, or county is available', function(t) { - var record = {}; - - var labelParts = ['initial value']; - - var f = schemas.USA.local; - - t.deepEqual(f(record, labelParts), ['initial value']); - t.end(); - - }); - - test('USA.regional should use region when layer=region and region is available', function(t) { - var record = { - layer: 'region', - region: 'region name', - region_a: 'region_a name' - }; - - var labelParts = ['initial value']; - - var f = schemas.USA.regional; - - t.deepEqual(f(record, labelParts), ['initial value', 'region name']); - t.end(); - - }); - - test('USA.regional should use region_a when layer=region and region is unavailable', function(t) { - var record = { - layer: 'region', - region_a: 'region_a name' - }; - - var labelParts = ['initial value']; - - var f = schemas.USA.regional; - - t.deepEqual(f(record, labelParts), ['initial value', 'region_a name']); - t.end(); - - }); - - test('USA.regional should use region_a when layer!=region and both region and region_a are available', function(t) { - var record = { - layer: 'not region', - region: 'region name', - region_a: 'region_a name' - }; - - var labelParts = ['initial value']; - - var f = schemas.USA.regional; - - t.deepEqual(f(record, labelParts), ['initial value', 'region_a name']); - t.end(); - - }); - - test('USA.regional should use region when layer!=region and region_a is unavailable', function(t) { - var record = { - layer: 'region', - region: 'region name', - region_a: 'region_a name' - }; - - var labelParts = ['initial value']; - - var f = schemas.USA.regional; - - t.deepEqual(f(record, labelParts), ['initial value', 'region name'], 'region should have been appended'); - t.end(); - - }); - - test('USA.regional should not append anything when neither region nor region_a are available', function(t) { - var record = { - layer: 'region', - }; - - var labelParts = ['initial value']; - - var f = schemas.USA.regional; - - t.deepEqual(f(record, labelParts), ['initial value'], 'no USA.region should have appended'); - t.end(); - - }); - - test('USA.country should append country_a when available', function(t) { - var record = { - country_a: 'country_a name', - country: 'country name' - }; - - var labelParts = ['initial value']; - - var f = schemas.USA.country; - - t.deepEqual(f(record, labelParts), ['initial value', 'country_a name'], 'country_a should have appended'); - t.end(); - - }); - - test('USA.country should not append anything when country_a is unavailable', function(t) { - var record = { - country: 'country name' - }; - - var labelParts = ['initial value']; - - var f = schemas.USA.country; - - t.deepEqual(f(record, labelParts), ['initial value'], 'no USA.country should have appended'); - t.end(); - - }); - -}; - -module.exports.tests.gbr = function(test, common) { - test('GBR.local should use neighbourhood value over county, localadmin, locality, region', function(t) { - var record = { - neighbourhood: 'neighbourhood value', - county: 'county value', - localadmin: 'localadmin value', - locality: 'locality value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.GBR.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'neighbourhood value']); - t.end(); - - }); - - test('GBR.local should use county value over county, localadmin, locality, region', function(t) { - var record = { - county: 'county value', - localadmin: 'localadmin value', - locality: 'locality value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.GBR.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'county value']); - t.end(); - - }); - - test('GBR.local should use localadmin value over locality, region', function(t) { - var record = { - localadmin: 'localadmin value', - locality: 'locality value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.GBR.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'localadmin value']); - t.end(); - - }); - - test('GBR.local should use locality value over region', function(t) { - var record = { - locality: 'locality value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.GBR.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'locality value']); - t.end(); - - }); - - test('GBR.local should use region value when nothing else is available', function(t) { - var record = { - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.GBR.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'region value']); - t.end(); - - }); - - test('GBR.local should not append anything when none of neighbourhood, county, localadmin, locality, region are available', function(t) { - var record = {}; - - var labelParts = ['initial value']; - - var f = schemas.GBR.local; - - t.deepEqual(f(record, labelParts), ['initial value']); - t.end(); - - }); - - test('GBR.regional should use county over country and region', function(t) { - var record = { - county: 'county value', - country: 'country value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.GBR.regional; - - t.deepEqual(f(record, labelParts), ['initial value', 'county value']); - t.end(); - - }); - - test('GBR.regional should use country over region', function(t) { - var record = { - country: 'country value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.GBR.regional; - - t.deepEqual(f(record, labelParts), ['initial value', 'country value']); - t.end(); - - }); - - test('GBR.regional should use region when county and country aren not available', function(t) { - var record = { - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.GBR.regional; - - t.deepEqual(f(record, labelParts), ['initial value', 'region value']); - t.end(); - - }); - - test('GBR.regional should not append anything when none of county, country, or region are available', function(t) { - var record = {}; - - var labelParts = ['initial value']; - - var f = schemas.GBR.regional; - - t.deepEqual(f(record, labelParts), ['initial value']); - t.end(); - - }); - -}; - -module.exports.tests.sgp = function(test, common) { - test('SGP.local should use neighbourhood value over region, county, localadmin, locality', function(t) { - var record = { - neighbourhood: 'neighbourhood value', - county: 'county value', - localadmin: 'localadmin value', - locality: 'locality value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.SGP.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'neighbourhood value']); - t.end(); - - }); - - test('SGP.local should use region value over county, localadmin, locality', function(t) { - var record = { - county: 'county value', - localadmin: 'localadmin value', - locality: 'locality value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.SGP.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'region value']); - t.end(); - - }); - - test('SGP.local should use county value over localadmin, locality', function(t) { - var record = { - localadmin: 'localadmin value', - locality: 'locality value', - county: 'county value' - }; - - var labelParts = ['initial value']; - - var f = schemas.SGP.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'county value']); - t.end(); - - }); - - test('SGP.local should use localadmin value over locality', function(t) { - var record = { - localadmin: 'localadmin value', - locality: 'locality value' - }; - - var labelParts = ['initial value']; - - var f = schemas.SGP.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'localadmin value']); - t.end(); - - }); - - test('SGP.local should use locality value when nothing else is available', function(t) { - var record = { - locality: 'locality value' - }; - - var labelParts = ['initial value']; - - var f = schemas.SGP.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'locality value']); - t.end(); - - }); - - test('SGP.local should not append anything when none of neighbourhood, region, county, localadmin, locality are available', function(t) { - var record = {}; - - var labelParts = ['initial value']; - - var f = schemas.SGP.local; - - t.deepEqual(f(record, labelParts), ['initial value']); - t.end(); - - }); - - test('SGP.regional should use county over country and region', function(t) { - var record = { - county: 'county value', - country: 'country value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.SGP.regional; - - t.deepEqual(f(record, labelParts), ['initial value', 'county value']); - t.end(); - - }); - - test('SGP.regional should use country over region', function(t) { - var record = { - country: 'country value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.SGP.regional; - - t.deepEqual(f(record, labelParts), ['initial value', 'country value']); - t.end(); - - }); - - test('SGP.regional should use region when county and country aren not available', function(t) { - var record = { - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.SGP.regional; - - t.deepEqual(f(record, labelParts), ['initial value', 'region value']); - t.end(); - - }); - - test('SGP.regional should not append anything when none of county, country, or region are available', function(t) { - var record = {}; - - var labelParts = ['initial value']; - - var f = schemas.SGP.regional; - - t.deepEqual(f(record, labelParts), ['initial value']); - t.end(); - - }); - -}; - -module.exports.tests.swe = function(test, common) { - test('SWE.local should use neighbourhood value over region, county, localadmin, locality', function(t) { - var record = { - neighbourhood: 'neighbourhood value', - county: 'county value', - localadmin: 'localadmin value', - locality: 'locality value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.SWE.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'neighbourhood value']); - t.end(); - - }); - - test('SWE.local should use region value over county, localadmin, locality', function(t) { - var record = { - county: 'county value', - localadmin: 'localadmin value', - locality: 'locality value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.SWE.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'region value']); - t.end(); - - }); - - test('SWE.local should use county value over localadmin, locality', function(t) { - var record = { - localadmin: 'localadmin value', - locality: 'locality value', - county: 'county value' - }; - - var labelParts = ['initial value']; - - var f = schemas.SWE.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'county value']); - t.end(); - - }); - - test('SWE.local should use localadmin value over locality', function(t) { - var record = { - localadmin: 'localadmin value', - locality: 'locality value' - }; - - var labelParts = ['initial value']; - - var f = schemas.SWE.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'localadmin value']); - t.end(); - - }); - - test('SWE.local should use locality value when nothing else is available', function(t) { - var record = { - locality: 'locality value' - }; - - var labelParts = ['initial value']; - - var f = schemas.SWE.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'locality value']); - t.end(); - - }); - - test('SWE.local should not append anything when none of neighbourhood, region, county, localadmin, locality are available', function(t) { - var record = {}; - - var labelParts = ['initial value']; - - var f = schemas.SWE.local; - - t.deepEqual(f(record, labelParts), ['initial value']); - t.end(); - - }); - - test('SGP.regional should use country when available', function(t) { - var record = { - country: 'country value', - country_a: 'country_a value', - }; - - var labelParts = ['initial value']; - - var f = schemas.SGP.regional; - - t.deepEqual(f(record, labelParts), ['initial value', 'country value']); - t.end(); - - }); - - test('SGP.regional should not append anything when country is not available', function(t) { - var record = { - country_a: 'country_a value' - }; - - var labelParts = ['initial value']; - - var f = schemas.SGP.regional; - - t.deepEqual(f(record, labelParts), ['initial value']); - t.end(); - - }); - -}; - -module.exports.tests.default = function(test, common) { - test('default.local should use localadmin value over locality, neighbourhood, county, region', function(t) { - var record = { - neighbourhood: 'neighbourhood value', - county: 'county value', - localadmin: 'localadmin value', - locality: 'locality value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.default.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'localadmin value']); - t.end(); - - }); - - test('default.local should use locality value over neighbourhood, county, region', function(t) { - var record = { - neighbourhood: 'neighbourhood value', - county: 'county value', - locality: 'locality value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.default.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'locality value']); - t.end(); - - }); - - test('default.local should use neighbourhood value over county, region', function(t) { - var record = { - neighbourhood: 'neighbourhood value', - county: 'county value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.default.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'neighbourhood value']); - t.end(); - - }); - - test('default.local should use county value over region', function(t) { - var record = { - county: 'county value', - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.default.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'county value']); - t.end(); - - }); - - test('default.local should use region value when nothing else is available', function(t) { - var record = { - region: 'region value' - }; - - var labelParts = ['initial value']; - - var f = schemas.default.local; - - t.deepEqual(f(record, labelParts), ['initial value', 'region value']); - t.end(); - - }); - - test('default.local should not append anything when none of neighbourhood, region, county, localadmin, ' + - 'locality are available', function(t) { - var record = {}; - - var labelParts = ['initial value']; - - var f = schemas.default.local; - - t.deepEqual(f(record, labelParts), ['initial value']); - t.end(); - - }); - - test('default.regional should use country over region, region_a, or country_a', function(t) { - var record = { - region: 'region value', - region_a: 'region_a value', - country: 'country value', - country_a: 'country_a value' - }; - - var labelParts = ['initial value']; - - var f = schemas.default.regional; - - t.deepEqual(f(record, labelParts), ['initial value', 'country value']); - t.end(); - - }); - - test('default.regional should not append any value if country is not available', function(t) { - var record = { - region: 'region value', - region_a: 'region_a value', - country_a: 'country_a value' - }; - - var labelParts = ['initial value']; - - var f = schemas.default.regional; - - t.deepEqual(f(record, labelParts), ['initial value']); - t.end(); - - }); - -}; - module.exports.all = function (tape, common) { function test(name, testFunction) { diff --git a/test/unit/run.js b/test/unit/run.js index 2e8b382e..5f2ad49b 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -13,10 +13,10 @@ var tests = [ require('./controller/place'), require('./controller/search'), require('./helper/geojsonify'), + require('./helper/labelGenerator_examples'), require('./helper/labelGenerator_default'), + require('./helper/labelGenerator_CAN'), require('./helper/labelGenerator_GBR'), - require('./helper/labelGenerator_SGP'), - require('./helper/labelGenerator_SWE'), require('./helper/labelGenerator_USA'), require('./helper/labelSchema'), require('./helper/text_parser'), From 96af0070441806f11a78ac1c171f8620a55d59f1 Mon Sep 17 00:00:00 2001 From: greenkeeperio-bot Date: Tue, 5 Apr 2016 17:30:31 -0700 Subject: [PATCH 07/20] chore(package): update elasticsearch to version 11.0.0 http://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f97df8eb..f54db70c 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,7 @@ "async": "^1.5.2", "check-types": "^6.0.0", "cluster2": "git://github.com/missinglink/cluster2.git#node_zero_twelve", - "elasticsearch": "^10.1.3", + "elasticsearch": "^11.0.0", "express": "^4.8.8", "express-http-proxy": "^0.6.0", "extend": "3.0.0", From e6e9c1c574598c4ddddc450c4c8eacd598d7c61d Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 13 Apr 2016 12:07:01 -0400 Subject: [PATCH 08/20] Do not apply local naming conventions to records with no admin info There's no way to decide what to do, and without handling this case the API will return an error. --- middleware/localNamingConventions.js | 3 +++ .../unit/middleware/localNamingConventions.js | 19 ++++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/middleware/localNamingConventions.js b/middleware/localNamingConventions.js index dd026bb3..e2851370 100644 --- a/middleware/localNamingConventions.js +++ b/middleware/localNamingConventions.js @@ -23,6 +23,9 @@ function applyLocalNamingConventions(req, res, next) { // loop through data items and flip relevant number/street res.data.filter(function(place){ + // do nothing for records with no admin info + if (!place.parent || !place.parent.country_a) { return false; } + // relevant for some countries var flip = place.parent.country_a.some(function(country) { return _.includes(flipNumberAndStreetCountries, country); diff --git a/test/unit/middleware/localNamingConventions.js b/test/unit/middleware/localNamingConventions.js index c544abe7..1d4102f0 100644 --- a/test/unit/middleware/localNamingConventions.js +++ b/test/unit/middleware/localNamingConventions.js @@ -77,8 +77,21 @@ module.exports.tests.flipNumberAndStreet = function(test, common) { } }; + var unknownCountryAddress = { + '_id': 'test4', + '_type': 'test', + 'name': { 'default': '123 Main Street' }, + 'center_point': { 'lon': 30.1, 'lat': -50 }, + 'address_parts': { + 'number': '123', + 'street': 'Main Street' + }, + 'parent': { + } + }; + var req = {}, - res = { data: [ ukAddress, deAddress, nlAddress ] }, + res = { data: [ ukAddress, deAddress, nlAddress, unknownCountryAddress ] }, middleware = localNamingConventions(); test('flipNumberAndStreet', function(t) { @@ -96,6 +109,10 @@ module.exports.tests.flipNumberAndStreet = function(test, common) { // this definition comes from pelias configuration t.equal( res.data[2].name.default, 'Keizersgracht 117', 'flipped name' ); + // addresses without a known country (either due to missing data or admin lookup + // being disabled), don't have the name flipped + t.equal( res.data[3].name.default, '123 Main Street', 'standard name'); + t.end(); }); }); From d98973c21f2fb7da55ec22bb4e45e996005c6c86 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 13 Apr 2016 12:14:12 -0400 Subject: [PATCH 09/20] Handle fatal errors in unit tests Due to the way our unit tests are run, a fatal error in the API code that causes the node process to quit with an error will not be picked up by `tap-dot`, and the unit tests will still return a passing status code. This is due to the way status codes from pipes are handled by default. Fortunately, there is an ["unofficial bash strict mode"](http://redsymbol.net/articles/unofficial-bash-strict-mode/) that helps fix this sort of thing, and by running our unit tests with a few tweaks we can ensure that any failure in the unit tests is captured. --- bin/units | 5 +++++ package.json | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100755 bin/units diff --git a/bin/units b/bin/units new file mode 100755 index 00000000..b011e2ee --- /dev/null +++ b/bin/units @@ -0,0 +1,5 @@ +#!/bin/bash + +set -euo pipefail + +node test/unit/run.js | ./node_modules/.bin/tap-dot diff --git a/package.json b/package.json index f54db70c..0c0cc1f7 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ "scripts": { "start": "node index.js", "test": "npm run unit", - "unit": "node test/unit/run.js | tap-dot", + "unit": "./bin/units", "ciao": "node node_modules/ciao/bin/ciao -c test/ciao.json test/ciao", "coverage": "node_modules/.bin/istanbul cover test/unit/run.js", "audit": "npm shrinkwrap; node node_modules/nsp/bin/nspCLI.js audit-shrinkwrap; rm npm-shrinkwrap.json;", From 8d0b0e87807abd3e44459897d58fc9ba5d2bf581 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Thu, 14 Apr 2016 09:24:26 -0400 Subject: [PATCH 10/20] switched to `_.compact` to remove `undefined` values --- helper/labelGenerator.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helper/labelGenerator.js b/helper/labelGenerator.js index eb35c86c..20c73a98 100644 --- a/helper/labelGenerator.js +++ b/helper/labelGenerator.js @@ -13,8 +13,8 @@ module.exports = function( record ){ labelParts.push(valueFunction(record)); } - // retain only things that are defined - labelParts = labelParts.filter(function(v) { return !_.isUndefined(v); }); + // retain only things that are truthy + labelParts = _.compact(labelParts); // first, dedupe the name and 1st label array elements // this is used to ensure that the `name` and first admin hierarchy elements aren't repeated From c5626d6c5eea5a13dacf3d9197c68994351fa59a Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Thu, 14 Apr 2016 09:28:23 -0400 Subject: [PATCH 11/20] added `localadmin` for USA/GBR/default when `locality` isn't available --- helper/labelSchema.js | 8 ++++---- test/unit/helper/labelGenerator_GBR.js | 19 +++++++++++++++++++ test/unit/helper/labelGenerator_USA.js | 20 ++++++++++++++++++++ test/unit/helper/labelGenerator_default.js | 19 +++++++++++++++++++ 4 files changed, 62 insertions(+), 4 deletions(-) diff --git a/helper/labelSchema.js b/helper/labelSchema.js index e1f9921a..1bce7133 100644 --- a/helper/labelSchema.js +++ b/helper/labelSchema.js @@ -2,22 +2,22 @@ var _ = require('lodash'); module.exports = { 'default': { - 'local': getFirstProperty(['locality']), + 'local': getFirstProperty(['locality', 'localadmin']), 'country': getFirstProperty(['country']) }, 'GBR': { - 'local': getFirstProperty(['locality']), + 'local': getFirstProperty(['locality', 'localadmin']), 'regional': getFirstProperty(['macroregion']), 'country': getFirstProperty(['country']) }, 'USA': { 'borough': getFirstProperty(['borough']), - 'local': getFirstProperty(['locality']), + 'local': getFirstProperty(['locality', 'localadmin']), 'regional': getUsOrCaState, 'country': getFirstProperty(['country']) }, 'CAN': { - 'local': getFirstProperty(['locality']), + 'local': getFirstProperty(['locality']), // no localadmins in CAN 'regional': getUsOrCaState, 'country': getFirstProperty(['country']) } diff --git a/test/unit/helper/labelGenerator_GBR.js b/test/unit/helper/labelGenerator_GBR.js index 4b619464..9a774fc7 100644 --- a/test/unit/helper/labelGenerator_GBR.js +++ b/test/unit/helper/labelGenerator_GBR.js @@ -31,6 +31,25 @@ module.exports.tests.united_kingdom = function(test, common) { t.end(); }); + test('localadmin value should be used when locality is not available', function(t) { + var doc = { + 'name': 'venue name', + 'layer': 'venue', + 'housenumber': '1', + 'street': 'Main St', + 'neighbourhood': 'neighbourhood name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'GBR', + 'country': 'United Kingdom' + }; + t.equal(generator(doc),'venue name, localadmin name, macroregion name, United Kingdom'); + t.end(); + }); + test('street', function(t) { var doc = { 'name': 'street address', diff --git a/test/unit/helper/labelGenerator_USA.js b/test/unit/helper/labelGenerator_USA.js index 57e77f32..ecf4f925 100644 --- a/test/unit/helper/labelGenerator_USA.js +++ b/test/unit/helper/labelGenerator_USA.js @@ -31,6 +31,26 @@ module.exports.tests.united_states = function(test, common) { t.end(); }); + test('localadmin value should be used when there is no locality', function(t) { + var doc = { + 'name': 'venue name', + 'layer': 'venue', + 'housenumber': '1', + 'street': 'Main St', + 'neighbourhood': 'neighbourhood name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region_a': 'region abbr', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'USA', + 'country': 'United States' + }; + t.equal(generator(doc),'venue name, localadmin name, region abbr, United States'); + t.end(); + }); + test('street', function(t) { var doc = { 'name': '1 Main St', diff --git a/test/unit/helper/labelGenerator_default.js b/test/unit/helper/labelGenerator_default.js index 7645610f..bdde2ac6 100644 --- a/test/unit/helper/labelGenerator_default.js +++ b/test/unit/helper/labelGenerator_default.js @@ -31,6 +31,25 @@ module.exports.tests.default_country = function(test, common) { t.end(); }); + test('localadmin value should be used when locality is not available', function(t) { + var doc = { + 'name': 'venue name', + 'layer': 'venue', + 'housenumber': '1', + 'street': 'Main St', + 'neighbourhood': 'neighbourhood name', + 'localadmin': 'localadmin name', + 'county': 'county name', + 'macrocounty': 'macrocounty name', + 'region': 'region name', + 'macroregion': 'macroregion name', + 'country_a': 'country code', + 'country': 'country name' + }; + t.equal(generator(doc),'venue name, localadmin name, country name'); + t.end(); + }); + test('street', function(t) { var doc = { 'name': 'address', From f78356022491ea91a36682850bd6c9e5b108e2f0 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Thu, 14 Apr 2016 13:22:25 -0400 Subject: [PATCH 12/20] split tests into logical units --- test/unit/helper/labelGenerator_examples.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unit/helper/labelGenerator_examples.js b/test/unit/helper/labelGenerator_examples.js index 180c9b90..eed9e506 100644 --- a/test/unit/helper/labelGenerator_examples.js +++ b/test/unit/helper/labelGenerator_examples.js @@ -42,7 +42,9 @@ module.exports.tests.canada = function(test, common) { t.equal(generator(doc),'1 Main St, Truth or Consequences, NM, United States'); t.end(); }); +}; +module.exports.tests.france = function(test, common) { test('eiffel tower', function(t) { var doc = { 'name': 'Tour Eiffel', From caa9745aaa1804221ae284683f9ada0b428af459 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Thu, 14 Apr 2016 13:25:33 -0400 Subject: [PATCH 13/20] standardized on naming conventions --- test/unit/helper/labelGenerator_CAN.js | 12 ++++++------ test/unit/helper/labelGenerator_GBR.js | 16 ++++++++-------- test/unit/helper/labelGenerator_USA.js | 16 ++++++++-------- test/unit/helper/labelGenerator_default.js | 16 ++++++++-------- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/test/unit/helper/labelGenerator_CAN.js b/test/unit/helper/labelGenerator_CAN.js index f78a5a53..814a9315 100644 --- a/test/unit/helper/labelGenerator_CAN.js +++ b/test/unit/helper/labelGenerator_CAN.js @@ -14,8 +14,8 @@ module.exports.tests.canada = function(test, common) { var doc = { 'name': 'venue name', 'layer': 'venue', - 'housenumber': '1', - 'street': 'Main St', + 'housenumber': 'house number', + 'street': 'street name', 'neighbourhood': 'neighbourhood name', 'locality': 'locality name', 'localadmin': 'localadmin name', @@ -33,10 +33,10 @@ module.exports.tests.canada = function(test, common) { test('street', function(t) { var doc = { - 'name': '1 Main St', + 'name': 'house number street name', 'layer': 'address', - 'housenumber': '1', - 'street': 'Main St', + 'housenumber': 'house number', + 'street': 'street name', 'neighbourhood': 'neighbourhood name', 'locality': 'locality name', 'localadmin': 'localadmin name', @@ -48,7 +48,7 @@ module.exports.tests.canada = function(test, common) { 'country_a': 'CAN', 'country': 'Canada' }; - t.equal(generator(doc),'1 Main St, locality name, region abbr, Canada'); + t.equal(generator(doc),'house number street name, locality name, region abbr, Canada'); t.end(); }); diff --git a/test/unit/helper/labelGenerator_GBR.js b/test/unit/helper/labelGenerator_GBR.js index 9a774fc7..44fe0d59 100644 --- a/test/unit/helper/labelGenerator_GBR.js +++ b/test/unit/helper/labelGenerator_GBR.js @@ -15,8 +15,8 @@ module.exports.tests.united_kingdom = function(test, common) { var doc = { 'name': 'venue name', 'layer': 'venue', - 'housenumber': '1', - 'street': 'Main St', + 'housenumber': 'house number', + 'street': 'street name', 'neighbourhood': 'neighbourhood name', 'locality': 'locality name', 'localadmin': 'localadmin name', @@ -35,8 +35,8 @@ module.exports.tests.united_kingdom = function(test, common) { var doc = { 'name': 'venue name', 'layer': 'venue', - 'housenumber': '1', - 'street': 'Main St', + 'housenumber': 'house number', + 'street': 'street name', 'neighbourhood': 'neighbourhood name', 'localadmin': 'localadmin name', 'county': 'county name', @@ -52,10 +52,10 @@ module.exports.tests.united_kingdom = function(test, common) { test('street', function(t) { var doc = { - 'name': 'street address', + 'name': 'house number street name', 'layer': 'address', - 'housenumber': '1', - 'street': 'Main St', + 'housenumber': 'house number', + 'street': 'street name', 'neighbourhood': 'neighbourhood name', 'locality': 'locality name', 'localadmin': 'localadmin name', @@ -66,7 +66,7 @@ module.exports.tests.united_kingdom = function(test, common) { 'country_a': 'GBR', 'country': 'United Kingdom' }; - t.equal(generator(doc),'street address, locality name, macroregion name, United Kingdom'); + t.equal(generator(doc),'house number street name, locality name, macroregion name, United Kingdom'); t.end(); }); diff --git a/test/unit/helper/labelGenerator_USA.js b/test/unit/helper/labelGenerator_USA.js index ecf4f925..5000cac1 100644 --- a/test/unit/helper/labelGenerator_USA.js +++ b/test/unit/helper/labelGenerator_USA.js @@ -14,8 +14,8 @@ module.exports.tests.united_states = function(test, common) { var doc = { 'name': 'venue name', 'layer': 'venue', - 'housenumber': '1', - 'street': 'Main St', + 'housenumber': 'house number', + 'street': 'street name', 'neighbourhood': 'neighbourhood name', 'locality': 'locality name', 'localadmin': 'localadmin name', @@ -35,8 +35,8 @@ module.exports.tests.united_states = function(test, common) { var doc = { 'name': 'venue name', 'layer': 'venue', - 'housenumber': '1', - 'street': 'Main St', + 'housenumber': 'house number', + 'street': 'street name', 'neighbourhood': 'neighbourhood name', 'localadmin': 'localadmin name', 'county': 'county name', @@ -53,10 +53,10 @@ module.exports.tests.united_states = function(test, common) { test('street', function(t) { var doc = { - 'name': '1 Main St', + 'name': 'house number street name', 'layer': 'address', - 'housenumber': '1', - 'street': 'Main St', + 'housenumber': 'house number', + 'street': 'street name', 'neighbourhood': 'neighbourhood name', 'locality': 'locality name', 'localadmin': 'localadmin name', @@ -68,7 +68,7 @@ module.exports.tests.united_states = function(test, common) { 'country_a': 'USA', 'country': 'United States' }; - t.equal(generator(doc),'1 Main St, locality name, region abbr, United States'); + t.equal(generator(doc),'house number street name, locality name, region abbr, United States'); t.end(); }); diff --git a/test/unit/helper/labelGenerator_default.js b/test/unit/helper/labelGenerator_default.js index bdde2ac6..25991ab8 100644 --- a/test/unit/helper/labelGenerator_default.js +++ b/test/unit/helper/labelGenerator_default.js @@ -15,8 +15,8 @@ module.exports.tests.default_country = function(test, common) { var doc = { 'name': 'venue name', 'layer': 'venue', - 'housenumber': '1', - 'street': 'Main St', + 'housenumber': 'house number', + 'street': 'street name', 'neighbourhood': 'neighbourhood name', 'locality': 'locality name', 'localadmin': 'localadmin name', @@ -35,8 +35,8 @@ module.exports.tests.default_country = function(test, common) { var doc = { 'name': 'venue name', 'layer': 'venue', - 'housenumber': '1', - 'street': 'Main St', + 'housenumber': 'house number', + 'street': 'street name', 'neighbourhood': 'neighbourhood name', 'localadmin': 'localadmin name', 'county': 'county name', @@ -52,10 +52,10 @@ module.exports.tests.default_country = function(test, common) { test('street', function(t) { var doc = { - 'name': 'address', + 'name': 'house number street name', 'layer': 'address', - 'housenumber': '1', - 'street': 'Main St', + 'housenumber': 'house number', + 'street': 'street name', 'neighbourhood': 'neighbourhood name', 'locality': 'locality name', 'localadmin': 'localadmin name', @@ -66,7 +66,7 @@ module.exports.tests.default_country = function(test, common) { 'country_a': 'country code', 'country': 'country name' }; - t.equal(generator(doc),'address, locality name, country name'); + t.equal(generator(doc),'house number street name, locality name, country name'); t.end(); }); From c415b96eadd2bc1b2bbdee2a7a42f036c282a032 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Thu, 14 Apr 2016 13:50:37 -0400 Subject: [PATCH 14/20] Named admin_parts adminParts which caused problems --- helper/text_parser.js | 13 ++++--------- test/unit/helper/text_parser.js | 2 -- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/helper/text_parser.js b/helper/text_parser.js index dcce1f63..0db8bede 100644 --- a/helper/text_parser.js +++ b/helper/text_parser.js @@ -24,20 +24,15 @@ module.exports.get_parsed_address = function get_parsed_address(query) { // naive approach - for admin matching during query time // split 'flatiron, new york, ny' into 'flatiron' and 'new york, ny' - //var delimIndex = query.indexOf(DELIM); - //var address = {}; - //if ( delimIndex !== -1 ) { - // address.name = query.substring(0, delimIndex); - // address.admin_parts = query.substring(delimIndex + 1).trim(); - //} - - var address = {}; if (queryParts.length > 1) { address.name = queryParts[0].trim(); - address.adminParts = queryParts.slice(1) + // 1. slice away all parts after the first one + // 2. trim spaces from each part just in case + // 3. join the parts back together with appropriate delimiter and spacing + address.admin_parts = queryParts.slice(1) .map(function (part) { return part.trim(); }) .join(DELIM + ' '); } diff --git a/test/unit/helper/text_parser.js b/test/unit/helper/text_parser.js index 93bd4a97..d85deca1 100644 --- a/test/unit/helper/text_parser.js +++ b/test/unit/helper/text_parser.js @@ -119,8 +119,6 @@ module.exports.tests.parse_address = function(test, common) { var query_string = '339 W Main St,Lancaster,PA'; var address = parser.get_parsed_address(query_string); - console.log(address); - t.equal(typeof address, 'object', 'valid object for the address'); t.equal(address.number, '339', 'parsed house number'); t.equal(address.street, 'W Main St', 'parsed street'); From 37d261c047080f774277bfd2638f437b7c23c6d3 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Thu, 14 Apr 2016 13:55:30 -0400 Subject: [PATCH 15/20] Removed a new test when merging --- test/unit/run.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/run.js b/test/unit/run.js index 2e8b382e..c1ab11b7 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -28,6 +28,7 @@ var tests = [ require('./middleware/localNamingConventions'), require('./middleware/dedupe'), require('./middleware/parseBBox'), + require('./middleware/normalizeParentIds'), require('./query/autocomplete'), require('./query/autocomplete_defaults'), require('./query/search_defaults'), From 05f4374b0a77c7042f0337b52651dee0fca8d46e Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Thu, 14 Apr 2016 14:00:44 -0400 Subject: [PATCH 16/20] Add more tests without spaces --- test/unit/helper/text_parser.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/unit/helper/text_parser.js b/test/unit/helper/text_parser.js index d85deca1..ca5b05f0 100644 --- a/test/unit/helper/text_parser.js +++ b/test/unit/helper/text_parser.js @@ -29,6 +29,15 @@ module.exports.tests.split_on_comma = function(test, common) { t.equal(address.admin_parts, query.admin_parts, 'admin_parts set correctly to ' + address.admin_parts); t.end(); }); + + test('naive parsing ' + query + 'without spaces', function(t) { + var address = parser.get_parsed_address(query.name + ',' + query.admin_parts); + + t.equal(typeof address, 'object', 'valid object'); + t.equal(address.name, query.name, 'name set correctly to ' + address.name); + t.equal(address.admin_parts, query.admin_parts, 'admin_parts set correctly to ' + address.admin_parts); + t.end(); + }); }); }; From 8a54dbeef01c0b29d78c265f323e2f802bc430ae Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Thu, 14 Apr 2016 14:15:50 -0400 Subject: [PATCH 17/20] switched from `macrocountry` to `macrocounty` --- helper/type_mapping.js | 4 ++-- test/ciao/autocomplete/layers_alias_coarse.coffee | 2 +- test/ciao/autocomplete/layers_invalid.coffee | 2 +- .../autocomplete/layers_mix_invalid_valid.coffee | 2 +- test/ciao/reverse/layers_alias_coarse.coffee | 2 +- test/ciao/reverse/layers_invalid.coffee | 2 +- test/ciao/reverse/layers_mix_invalid_valid.coffee | 2 +- test/ciao/search/layers_alias_coarse.coffee | 2 +- test/ciao/search/layers_invalid.coffee | 2 +- test/ciao/search/layers_mix_invalid_valid.coffee | 2 +- test/unit/helper/type_mapping.js | 4 ++-- test/unit/sanitiser/_layers.js | 12 ++++++------ 12 files changed, 19 insertions(+), 19 deletions(-) diff --git a/helper/type_mapping.js b/helper/type_mapping.js index 39abb835..3323fe6f 100644 --- a/helper/type_mapping.js +++ b/helper/type_mapping.js @@ -48,8 +48,8 @@ var LAYERS_BY_SOURCE = { openstreetmap: [ 'address', 'venue' ], openaddresses: [ 'address' ], geonames: [ 'country', 'region', 'county', 'locality', 'venue' ], - whosonfirst: [ 'continent', 'macrocountry', 'country', 'dependency', 'region', - 'locality', 'localadmin', 'county', 'macrohood', 'neighbourhood', 'microhood', 'disputed'] + whosonfirst: [ 'continent', 'country', 'dependency', 'region', + 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'neighbourhood', 'microhood', 'disputed'] }; /* diff --git a/test/ciao/autocomplete/layers_alias_coarse.coffee b/test/ciao/autocomplete/layers_alias_coarse.coffee index 4a359bdd..b41f39b1 100644 --- a/test/ciao/autocomplete/layers_alias_coarse.coffee +++ b/test/ciao/autocomplete/layers_alias_coarse.coffee @@ -32,12 +32,12 @@ 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", - "macrocountry", "country", "dependency", "region", "locality", "localadmin", + "macrocounty", "county", "macrohood", "neighbourhood", diff --git a/test/ciao/autocomplete/layers_invalid.coffee b/test/ciao/autocomplete/layers_invalid.coffee index b4ce0565..7be866fd 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,country,region,county,locality,continent,macrocountry,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,country,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? 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 8bac1ccf..1fd34389 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,country,region,county,locality,continent,macrocountry,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,country,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/reverse/layers_alias_coarse.coffee b/test/ciao/reverse/layers_alias_coarse.coffee index f16f5a17..a9107259 100644 --- a/test/ciao/reverse/layers_alias_coarse.coffee +++ b/test/ciao/reverse/layers_alias_coarse.coffee @@ -31,7 +31,7 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['size'].should.eql 10 json.geocoding.query.layers.should.eql [ "continent", - "macrocountry", + "macrocounty", "country", "dependency", "region", diff --git a/test/ciao/reverse/layers_invalid.coffee b/test/ciao/reverse/layers_invalid.coffee index 9d89e05a..831aa97a 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,country,region,county,locality,continent,macrocountry,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,country,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? 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 3e6a57fe..5e3bbbc5 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,country,region,county,locality,continent,macrocountry,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,country,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/search/layers_alias_coarse.coffee b/test/ciao/search/layers_alias_coarse.coffee index 9dda2a48..b319815a 100644 --- a/test/ciao/search/layers_alias_coarse.coffee +++ b/test/ciao/search/layers_alias_coarse.coffee @@ -32,12 +32,12 @@ 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", - "macrocountry", "country", "dependency", "region", "locality", "localadmin", + "macrocounty", "county", "macrohood", "neighbourhood", diff --git a/test/ciao/search/layers_invalid.coffee b/test/ciao/search/layers_invalid.coffee index 5d07f99c..80ade1ac 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,country,region,county,locality,continent,macrocountry,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,country,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? 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 0a13c70b..290d1d6f 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,country,region,county,locality,continent,macrocountry,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,country,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/unit/helper/type_mapping.js b/test/unit/helper/type_mapping.js index 2f075940..77ce946b 100644 --- a/test/unit/helper/type_mapping.js +++ b/test/unit/helper/type_mapping.js @@ -12,8 +12,8 @@ module.exports.tests.interfaces = function(test, common) { test('alias layer mapping', function(t) { t.deepEquals(type_mapping.layer_mapping.coarse, - [ 'continent', 'macrocountry', 'country', 'dependency', - 'region', 'locality', 'localadmin', 'county', 'macrohood', + [ 'continent', 'country', 'dependency', + 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'neighbourhood', 'microhood', 'disputed' ]); t.end(); }); diff --git a/test/unit/sanitiser/_layers.js b/test/unit/sanitiser/_layers.js index a1cdd7c5..8266094c 100644 --- a/test/unit/sanitiser/_layers.js +++ b/test/unit/sanitiser/_layers.js @@ -41,8 +41,8 @@ module.exports.tests.sanitize_layers = function(test, common) { sanitize(raw, clean); - var admin_layers = [ 'continent', 'macrocountry', 'country', 'dependency', - 'region', 'locality', 'localadmin', 'county', 'macrohood', 'neighbourhood', + var admin_layers = [ 'continent', 'country', 'dependency', + 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'neighbourhood', 'microhood', 'disputed' ]; t.deepEqual(clean.layers, admin_layers, 'coarse layers set'); @@ -76,8 +76,8 @@ module.exports.tests.sanitize_layers = function(test, common) { sanitize(raw, clean); - var expected_layers = [ 'continent', 'macrocountry', 'country', 'dependency', - 'region', 'locality', 'localadmin', 'county', 'macrohood', 'neighbourhood', + var expected_layers = [ 'continent', 'country', 'dependency', + 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'neighbourhood', 'microhood', 'disputed' ]; t.deepEqual(clean.layers, expected_layers, 'coarse + regular layers set'); @@ -112,9 +112,9 @@ module.exports.tests.sanitize_layers = function(test, common) { sanitize(raw, clean); - var coarse_layers = [ 'continent', 'macrocountry', + var coarse_layers = [ 'continent', 'country', 'dependency', 'region', 'locality', 'localadmin', - 'county', 'macrohood', 'neighbourhood', 'microhood', + 'macrocounty', 'county', 'macrohood', 'neighbourhood', 'microhood', 'disputed' ]; var venue_layers = [ 'venue' ]; From 3c1d7582298b6faf483169cd4af64d33fac13c97 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 14 Apr 2016 15:35:11 -0400 Subject: [PATCH 18/20] Fix timeout error handling Timeout errors come back from the Elasticsearch module as Error objects, so instead of pushing the entire object onto the errors array, we should just push the message. I'm not sure if there are other cases where errors are just strings, errors that aren't objects with a message property are handled as before. --- controller/search.js | 8 +++++++- test/unit/controller/search.js | 17 ++++++++++++++++- test/unit/mock/backend.js | 8 ++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/controller/search.js b/controller/search.js index a564e7b4..7949c8ad 100644 --- a/controller/search.js +++ b/controller/search.js @@ -1,3 +1,5 @@ +var _ = require('lodash'); + var service = { search: require('../service/search') }; var logger = require('pelias-logger').get('api:controller:search'); @@ -36,7 +38,11 @@ function setup( backend, query ){ // error handler if( err ){ - req.errors.push( err ); + if (_.isObject(err) && err.message) { + req.errors.push( err.message ); + } else { + req.errors.push( err ); + } } // set response data else { diff --git a/test/unit/controller/search.js b/test/unit/controller/search.js index cfb715fa..09127167 100644 --- a/test/unit/controller/search.js +++ b/test/unit/controller/search.js @@ -123,6 +123,21 @@ module.exports.tests.functional_failure = function(test, common) { }); }; +module.exports.tests.timeout = function(test, common) { + test('timeout', function(t) { + var backend = mockBackend( 'client/search/timeout/1', function( cmd ){ + t.deepEqual(cmd, { body: { a: 'b' }, index: 'pelias', searchType: 'dfs_query_then_fetch' }, 'correct backend command'); + }); + var controller = setup( backend, mockQuery() ); + var req = { clean: { a: 'b' }, errors: [], warnings: [] }; + var next = function(){ + t.equal(req.errors[0],'Request Timeout after 5000ms'); + t.end(); + }; + controller(req, undefined, next ); + }); +}; + module.exports.all = function (tape, common) { function test(name, testFunction) { @@ -132,4 +147,4 @@ module.exports.all = function (tape, common) { for( var testCase in module.exports.tests ){ module.exports.tests[testCase](test, common); } -}; \ No newline at end of file +}; diff --git a/test/unit/mock/backend.js b/test/unit/mock/backend.js index 84147783..7d347a30 100644 --- a/test/unit/mock/backend.js +++ b/test/unit/mock/backend.js @@ -33,6 +33,14 @@ responses['client/search/fail/1'] = function( cmd, cb ){ return cb( 'a backend error occurred' ); }; +responses['client/search/timeout/1'] = function( cmd, cb) { + // timeout errors are objects + return cb({ + status: 408, + message: 'Request Timeout after 5000ms' + }); +}; + responses['client/mget/ok/1'] = function( cmd, cb ){ return cb( undefined, mgetEnvelope([{ _id: 'myid1', From c99fa9f0efa73906056026b749536c719b41a0bf Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Thu, 14 Apr 2016 16:18:55 -0400 Subject: [PATCH 19/20] added support for `macroregion` --- helper/type_mapping.js | 2 +- test/ciao/autocomplete/layers_alias_coarse.coffee | 1 + test/ciao/autocomplete/layers_invalid.coffee | 2 +- test/ciao/autocomplete/layers_mix_invalid_valid.coffee | 2 +- test/ciao/reverse/layers_alias_coarse.coffee | 3 ++- test/ciao/reverse/layers_invalid.coffee | 2 +- test/ciao/reverse/layers_mix_invalid_valid.coffee | 2 +- test/ciao/search/layers_alias_coarse.coffee | 1 + test/ciao/search/layers_invalid.coffee | 2 +- test/ciao/search/layers_mix_invalid_valid.coffee | 2 +- test/unit/helper/type_mapping.js | 2 +- test/unit/sanitiser/_layers.js | 6 +++--- 12 files changed, 15 insertions(+), 12 deletions(-) diff --git a/helper/type_mapping.js b/helper/type_mapping.js index 3323fe6f..0b20c111 100644 --- a/helper/type_mapping.js +++ b/helper/type_mapping.js @@ -48,7 +48,7 @@ var LAYERS_BY_SOURCE = { openstreetmap: [ 'address', 'venue' ], openaddresses: [ 'address' ], geonames: [ 'country', 'region', 'county', 'locality', 'venue' ], - whosonfirst: [ 'continent', 'country', 'dependency', 'region', + whosonfirst: [ 'continent', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'neighbourhood', 'microhood', 'disputed'] }; diff --git a/test/ciao/autocomplete/layers_alias_coarse.coffee b/test/ciao/autocomplete/layers_alias_coarse.coffee index b41f39b1..2fa2265c 100644 --- a/test/ciao/autocomplete/layers_alias_coarse.coffee +++ b/test/ciao/autocomplete/layers_alias_coarse.coffee @@ -34,6 +34,7 @@ json.geocoding.query['size'].should.eql 10 json.geocoding.query.layers.should.eql [ "continent", "country", "dependency", + "macroregion", "region", "locality", "localadmin", diff --git a/test/ciao/autocomplete/layers_invalid.coffee b/test/ciao/autocomplete/layers_invalid.coffee index 7be866fd..620b5586 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,country,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,country,macroregion,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? 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 1fd34389..963b79ab 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,country,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,country,macroregion,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/reverse/layers_alias_coarse.coffee b/test/ciao/reverse/layers_alias_coarse.coffee index a9107259..09c91483 100644 --- a/test/ciao/reverse/layers_alias_coarse.coffee +++ b/test/ciao/reverse/layers_alias_coarse.coffee @@ -31,12 +31,13 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['size'].should.eql 10 json.geocoding.query.layers.should.eql [ "continent", - "macrocounty", "country", "dependency", + "macroregion", "region", "locality", "localadmin", + "macrocounty", "county", "macrohood", "neighbourhood", diff --git a/test/ciao/reverse/layers_invalid.coffee b/test/ciao/reverse/layers_invalid.coffee index 831aa97a..aaec4864 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,country,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,country,macroregion,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? 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 5e3bbbc5..307b225d 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,country,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,country,macroregion,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/search/layers_alias_coarse.coffee b/test/ciao/search/layers_alias_coarse.coffee index b319815a..bf7cdb52 100644 --- a/test/ciao/search/layers_alias_coarse.coffee +++ b/test/ciao/search/layers_alias_coarse.coffee @@ -34,6 +34,7 @@ json.geocoding.query['size'].should.eql 10 json.geocoding.query.layers.should.eql [ "continent", "country", "dependency", + "macroregion", "region", "locality", "localadmin", diff --git a/test/ciao/search/layers_invalid.coffee b/test/ciao/search/layers_invalid.coffee index 80ade1ac..4f2da456 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,country,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,country,macroregion,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? 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 290d1d6f..f004c69e 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,country,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,address,venue,country,macroregion,region,county,locality,continent,macrocounty,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/unit/helper/type_mapping.js b/test/unit/helper/type_mapping.js index 77ce946b..355fd4e6 100644 --- a/test/unit/helper/type_mapping.js +++ b/test/unit/helper/type_mapping.js @@ -12,7 +12,7 @@ module.exports.tests.interfaces = function(test, common) { test('alias layer mapping', function(t) { t.deepEquals(type_mapping.layer_mapping.coarse, - [ 'continent', 'country', 'dependency', + [ 'continent', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'neighbourhood', 'microhood', 'disputed' ]); t.end(); diff --git a/test/unit/sanitiser/_layers.js b/test/unit/sanitiser/_layers.js index 8266094c..b9dcbd0f 100644 --- a/test/unit/sanitiser/_layers.js +++ b/test/unit/sanitiser/_layers.js @@ -42,7 +42,7 @@ module.exports.tests.sanitize_layers = function(test, common) { sanitize(raw, clean); var admin_layers = [ 'continent', 'country', 'dependency', - 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'neighbourhood', + 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'neighbourhood', 'microhood', 'disputed' ]; t.deepEqual(clean.layers, admin_layers, 'coarse layers set'); @@ -77,7 +77,7 @@ module.exports.tests.sanitize_layers = function(test, common) { sanitize(raw, clean); var expected_layers = [ 'continent', 'country', 'dependency', - 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'neighbourhood', + 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'neighbourhood', 'microhood', 'disputed' ]; t.deepEqual(clean.layers, expected_layers, 'coarse + regular layers set'); @@ -113,7 +113,7 @@ module.exports.tests.sanitize_layers = function(test, common) { sanitize(raw, clean); var coarse_layers = [ 'continent', - 'country', 'dependency', 'region', 'locality', 'localadmin', + 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'neighbourhood', 'microhood', 'disputed' ]; From 0076200728c5ab8a46e45476deb00160fce9c21e Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Thu, 14 Apr 2016 17:12:44 -0400 Subject: [PATCH 20/20] added special case for USA in labels if record is for literal United States, use the full name, otherwise use USA --- helper/labelSchema.js | 13 ++++++++++- test/unit/helper/geojsonify.js | 4 ++-- test/unit/helper/labelGenerator_USA.js | 24 ++++++++++----------- test/unit/helper/labelGenerator_examples.js | 2 +- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/helper/labelSchema.js b/helper/labelSchema.js index 1bce7133..0547e2ad 100644 --- a/helper/labelSchema.js +++ b/helper/labelSchema.js @@ -14,7 +14,7 @@ module.exports = { 'borough': getFirstProperty(['borough']), 'local': getFirstProperty(['locality', 'localadmin']), 'regional': getUsOrCaState, - 'country': getFirstProperty(['country']) + 'country': getUSACountryValue }, 'CAN': { 'local': getFirstProperty(['locality']), // no localadmins in CAN @@ -60,3 +60,14 @@ function getUsOrCaState(record) { } } + +// this function returns the full name of a country if the result is in the +// country layer (for "United States" record). It returns the abbreviation +// otherwise (eg - Lancaster, PA, USA). +function getUSACountryValue(record) { + if ('country' === record.layer && record.country) { + return record.country; + } + + return record.country_a; +} diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index 7297b67d..f8bb9779 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -193,7 +193,7 @@ module.exports.tests.search = function(test, common) { 'gid': 'openstreetmap:venue:node:34633854', 'layer': 'venue', 'source': 'openstreetmap', - 'label': 'Empire State Building, Manhattan, New York, NY, United States', + 'label': 'Empire State Building, Manhattan, New York, NY, USA', 'name': 'Empire State Building', 'country_a': 'USA', 'country': 'United States', @@ -340,7 +340,7 @@ module.exports.tests.search = function(test, common) { 'localadmin_gid': '404521211', 'locality': 'New York', 'locality_gid': '85977539', - 'label': 'East New York, Brooklyn, New York, NY, United States' + 'label': 'East New York, Brooklyn, New York, NY, USA' }, 'bbox': [-73.8967895508,40.6514712164,-73.8665771484,40.6737320588], 'geometry': { diff --git a/test/unit/helper/labelGenerator_USA.js b/test/unit/helper/labelGenerator_USA.js index 5000cac1..bdcc5f2f 100644 --- a/test/unit/helper/labelGenerator_USA.js +++ b/test/unit/helper/labelGenerator_USA.js @@ -27,7 +27,7 @@ module.exports.tests.united_states = function(test, common) { 'country_a': 'USA', 'country': 'United States' }; - t.equal(generator(doc),'venue name, locality name, region abbr, United States'); + t.equal(generator(doc),'venue name, locality name, region abbr, USA'); t.end(); }); @@ -47,7 +47,7 @@ module.exports.tests.united_states = function(test, common) { 'country_a': 'USA', 'country': 'United States' }; - t.equal(generator(doc),'venue name, localadmin name, region abbr, United States'); + t.equal(generator(doc),'venue name, localadmin name, region abbr, USA'); t.end(); }); @@ -68,7 +68,7 @@ module.exports.tests.united_states = function(test, common) { 'country_a': 'USA', 'country': 'United States' }; - t.equal(generator(doc),'house number street name, locality name, region abbr, United States'); + t.equal(generator(doc),'house number street name, locality name, region abbr, USA'); t.end(); }); @@ -87,7 +87,7 @@ module.exports.tests.united_states = function(test, common) { 'country_a': 'USA', 'country': 'United States' }; - t.equal(generator(doc),'neighbourhood name, locality name, region abbr, United States'); + t.equal(generator(doc),'neighbourhood name, locality name, region abbr, USA'); t.end(); }); @@ -107,7 +107,7 @@ module.exports.tests.united_states = function(test, common) { 'country_a': 'USA', 'country': 'United States' }; - t.equal(generator(doc),'venue name, borough name, locality name, region abbr, United States'); + t.equal(generator(doc),'venue name, borough name, locality name, region abbr, USA'); t.end(); }); @@ -125,7 +125,7 @@ module.exports.tests.united_states = function(test, common) { 'country_a': 'USA', 'country': 'United States' }; - t.equal(generator(doc),'locality name, region abbr, United States'); + t.equal(generator(doc),'locality name, region abbr, USA'); t.end(); }); @@ -142,7 +142,7 @@ module.exports.tests.united_states = function(test, common) { 'country_a': 'USA', 'country': 'United States' }; - t.equal(generator(doc),'localadmin name, region abbr, United States'); + t.equal(generator(doc),'localadmin name, region abbr, USA'); t.end(); }); @@ -158,7 +158,7 @@ module.exports.tests.united_states = function(test, common) { 'country_a': 'USA', 'country': 'United States' }; - t.equal(generator(doc),'county name, region abbr, United States'); + t.equal(generator(doc),'county name, region abbr, USA'); t.end(); }); @@ -173,7 +173,7 @@ module.exports.tests.united_states = function(test, common) { 'country_a': 'USA', 'country': 'United States' }; - t.equal(generator(doc),'macrocounty name, region abbr, United States'); + t.equal(generator(doc),'macrocounty name, region abbr, USA'); t.end(); }); @@ -187,7 +187,7 @@ module.exports.tests.united_states = function(test, common) { 'country_a': 'USA', 'country': 'United States' }; - t.equal(generator(doc),'region name, United States'); + t.equal(generator(doc),'region name, USA'); t.end(); }); @@ -199,7 +199,7 @@ module.exports.tests.united_states = function(test, common) { 'country_a': 'USA', 'country': 'United States' }; - t.equal(generator(doc),'macroregion name, United States'); + t.equal(generator(doc),'macroregion name, USA'); t.end(); }); @@ -226,7 +226,7 @@ module.exports.tests.united_states = function(test, common) { 'country_a': 'USA', 'country': 'United States' }; - t.equal(generator(doc),'locality name, region name, United States', 'region should be used'); + t.equal(generator(doc),'locality name, region name, USA', 'region should be used'); t.end(); }); diff --git a/test/unit/helper/labelGenerator_examples.js b/test/unit/helper/labelGenerator_examples.js index eed9e506..416b7598 100644 --- a/test/unit/helper/labelGenerator_examples.js +++ b/test/unit/helper/labelGenerator_examples.js @@ -39,7 +39,7 @@ module.exports.tests.canada = function(test, common) { 'country_a': 'USA', 'country': 'United States' }; - t.equal(generator(doc),'1 Main St, Truth or Consequences, NM, United States'); + t.equal(generator(doc),'1 Main St, Truth or Consequences, NM, USA'); t.end(); }); };