From d8e86d5f17133eedb8fde96f3fe754a99cb0a373 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 27 Jul 2015 16:47:55 +0200 Subject: [PATCH 01/15] phrase slop query modifications --- query/search.js | 12 ++++++-- test/unit/query/search.js | 60 ++++++++++++++++++++++++++++++++------- 2 files changed, 60 insertions(+), 12 deletions(-) diff --git a/query/search.js b/query/search.js index 8e34b404..49945534 100644 --- a/query/search.js +++ b/query/search.js @@ -102,7 +102,10 @@ function generate( params ){ // add search condition to distance query query.query.filtered.query.bool.must.push({ 'match': { - 'name.default': input + 'name.default': { + 'query': input, + 'analyzer': 'peliasOneEdgeGram' + } } }); @@ -110,7 +113,12 @@ function generate( params ){ // note: this is required for shingle/phrase matching query.query.filtered.query.bool.should.push({ 'match': { - 'phrase.default': input + 'phrase.default': { + 'query': input, + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2 + } } }); diff --git a/test/unit/query/search.js b/test/unit/query/search.js index dcc9d745..d3dccce9 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -80,12 +80,20 @@ var expected = { 'bool': { 'must': [{ 'match': { - 'name.default': 'test' + 'name.default': { + 'query': 'test', + 'analyzer': 'peliasOneEdgeGram' + } } }], 'should': [{ 'match': { - 'phrase.default': 'test' + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2 + } } }] } @@ -162,12 +170,20 @@ module.exports.tests.query = function(test, common) { 'bool': { 'must': [{ 'match': { - 'name.default': 'test' + 'name.default': { + 'query': 'test', + 'analyzer': 'peliasOneEdgeGram' + } } }], 'should': [{ 'match': { - 'phrase.default': 'test' + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2 + } } }] } @@ -202,12 +218,20 @@ module.exports.tests.query = function(test, common) { 'bool': { 'must': [{ 'match': { - 'name.default': 'test' + 'name.default': { + 'query': 'test', + 'analyzer': 'peliasOneEdgeGram' + } } }], 'should': [{ 'match': { - 'phrase.default': 'test' + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2 + } } }] } @@ -260,7 +284,10 @@ module.exports.tests.query = function(test, common) { 'must': [ { 'match': { - 'name.default': '123 main st' + 'name.default': { + 'query': '123 main st', + 'analyzer': 'peliasOneEdgeGram' + } } } ], @@ -292,7 +319,12 @@ module.exports.tests.query = function(test, common) { }, { 'match': { - 'phrase.default': '123 main st' + 'phrase.default': { + 'query': '123 main st', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2 + } } } ] @@ -409,7 +441,10 @@ module.exports.tests.query = function(test, common) { 'must': [ { 'match': { - 'name.default': 'soho grand' + 'name.default': { + 'query': 'soho grand', + 'analyzer': 'peliasOneEdgeGram' + } } } ], @@ -441,7 +476,12 @@ module.exports.tests.query = function(test, common) { }, { 'match': { - 'phrase.default': 'soho grand' + 'phrase.default': { + 'query': 'soho grand', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2 + } } } ] From 776b0ce1e1c9c56ba49a6ec6a6992b8f05252aa4 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Fri, 28 Aug 2015 14:43:17 +0200 Subject: [PATCH 02/15] update unit test --- test/unit/query/search.js | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/test/unit/query/search.js b/test/unit/query/search.js index fb533ec8..f2e6d9dc 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -656,13 +656,16 @@ module.exports.tests.query = function(test, common) { 'must': [ { 'match': { - 'name.default': '1 water st' + 'name.default': { + 'query': '1 water st', + 'analyzer': 'peliasOneEdgeGram' + } } } ], 'should': [ { - match: { + 'match': { 'address.number': { 'query': 1, 'boost': address_weights.number @@ -670,7 +673,7 @@ module.exports.tests.query = function(test, common) { } }, { - match: { + 'match': { 'address.street': { 'query': 'water st', 'boost': address_weights.street @@ -701,23 +704,28 @@ module.exports.tests.query = function(test, common) { } }, { - match: { - local_admin: 'manhattan' + 'match': { + 'local_admin': 'manhattan' } }, { - match: { - locality: 'manhattan' + 'match': { + 'locality': 'manhattan' } }, { - match: { - neighborhood: 'manhattan' + 'match': { + 'neighborhood': 'manhattan' } }, { 'match': { - 'phrase.default': '1 water st' + 'phrase.default': { + 'query': '1 water st', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2 + } } } ] From 4e74e9b681219601309fc84b3ac2dc083d900fb8 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 18 Aug 2015 13:31:56 -0400 Subject: [PATCH 03/15] Whitespace --- app.js | 2 +- controller/doc.js | 8 +------- sanitiser/doc.js | 1 - test/ciao/doc/msuccess.coffee | 2 +- test/ciao/doc/success.coffee | 2 +- test/unit/controller/doc.js | 5 ++--- test/unit/run.js | 2 +- test/unit/sanitiser/doc.js | 10 +++++----- 8 files changed, 12 insertions(+), 20 deletions(-) diff --git a/app.js b/app.js index c0b72610..32b8f788 100644 --- a/app.js +++ b/app.js @@ -54,4 +54,4 @@ app.use( require('./middleware/404') ); app.use( require('./middleware/408') ); app.use( require('./middleware/500') ); -module.exports = app; \ No newline at end of file +module.exports = app; diff --git a/controller/doc.js b/controller/doc.js index 76e10c3e..42cb0069 100644 --- a/controller/doc.js +++ b/controller/doc.js @@ -1,14 +1,11 @@ - var service = { mget: require('../service/mget') }; var geojsonify = require('../helper/geojsonify').search; function setup( backend ){ - // allow overriding of dependencies backend = backend || require('../src/backend'); - + function controller( req, res, next ){ - var query = req.clean.ids.map( function(id) { return { _index: 'pelias', @@ -18,7 +15,6 @@ function setup( backend ){ }); service.mget( backend, query, function( err, docs ){ - // error handler if( err ){ return next( err ); } @@ -30,9 +26,7 @@ function setup( backend ){ // respond return res.status(200).json( geojson ); - }); - } return controller; diff --git a/sanitiser/doc.js b/sanitiser/doc.js index 07f07aa1..352777a6 100644 --- a/sanitiser/doc.js +++ b/sanitiser/doc.js @@ -1,4 +1,3 @@ - var _sanitize = require('../sanitiser/_sanitize'), sanitizers = { id: require('../sanitiser/_id'), diff --git a/test/ciao/doc/msuccess.coffee b/test/ciao/doc/msuccess.coffee index 56808be4..ae49b82a 100644 --- a/test/ciao/doc/msuccess.coffee +++ b/test/ciao/doc/msuccess.coffee @@ -13,4 +13,4 @@ json.date.should.be.within now-5000, now+5000 #? valid geojson json.type.should.equal 'FeatureCollection' -json.features.should.be.instanceof Array \ No newline at end of file +json.features.should.be.instanceof Array diff --git a/test/ciao/doc/success.coffee b/test/ciao/doc/success.coffee index 3818aca6..bd1b64bd 100644 --- a/test/ciao/doc/success.coffee +++ b/test/ciao/doc/success.coffee @@ -13,4 +13,4 @@ json.date.should.be.within now-5000, now+5000 #? valid geojson json.type.should.equal 'FeatureCollection' -json.features.should.be.instanceof Array \ No newline at end of file +json.features.should.be.instanceof Array diff --git a/test/unit/controller/doc.js b/test/unit/controller/doc.js index 0bab18f2..e9e3918b 100644 --- a/test/unit/controller/doc.js +++ b/test/unit/controller/doc.js @@ -1,4 +1,3 @@ - var setup = require('../../../controller/doc'), mockBackend = require('../mock/backend'); @@ -93,7 +92,7 @@ module.exports.tests.functional_success = function(test, common) { text: 'test name2, city2, state2' } }]; - + test('functional success (with details)', function(t) { var backend = mockBackend( 'client/mget/ok/1', function( cmd ){ t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: 'a' } ] } }, 'correct backend command'); @@ -141,4 +140,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/run.js b/test/unit/run.js index 75051927..222a1836 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -25,4 +25,4 @@ var tests = [ tests.map(function(t) { t.all(tape, common); -}); \ No newline at end of file +}); diff --git a/test/unit/sanitiser/doc.js b/test/unit/sanitiser/doc.js index c44f8808..2f88dc0a 100644 --- a/test/unit/sanitiser/doc.js +++ b/test/unit/sanitiser/doc.js @@ -7,7 +7,7 @@ var doc = require('../../../sanitiser/doc'), defaultLengthError = function(input) { return 'invalid param \''+ input + '\': text length, must be >0'; }, defaultFormatError = 'invalid: must be of the format type:id for ex: \'geoname:4163334\'', defaultError = 'invalid param \'id\': text length, must be >0', - defaultMissingTypeError = function(input) { + defaultMissingTypeError = function(input) { var type = input.split(delimiter)[0]; return type + ' is invalid. It must be one of these values - [' + indeces.join(', ') + ']'; }, defaultClean = { ids: [ { id: '123', type: 'geoname' } ], details: true }, @@ -111,7 +111,7 @@ module.exports.tests.sanitize_details = function(test, common) { t.equal(clean.details, false, 'default details set (to false)'); t.end(); }); - }); + }); }); var valid_values = ['true', true, 1]; @@ -121,7 +121,7 @@ module.exports.tests.sanitize_details = function(test, common) { t.equal(clean.details, true, 'details set to true'); t.end(); }); - }); + }); }); var valid_false_values = ['false', false, 0]; @@ -131,7 +131,7 @@ module.exports.tests.sanitize_details = function(test, common) { t.equal(clean.details, false, 'details set to false'); t.end(); }); - }); + }); }); test('test default behavior', function(t) { @@ -196,4 +196,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 +}; From 2fce64f75a6dddbc1548430513459228a0a9a8d4 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 18 Aug 2015 16:27:19 -0400 Subject: [PATCH 04/15] Move files --- controller/{doc.js => place.js} | 0 sanitiser/{doc.js => place.js} | 0 test/ciao/{doc => place}/msuccess.coffee | 0 test/ciao/{doc => place}/success.coffee | 0 test/unit/controller/{doc.js => place.js} | 0 test/unit/sanitiser/{doc.js => place.js} | 0 6 files changed, 0 insertions(+), 0 deletions(-) rename controller/{doc.js => place.js} (100%) rename sanitiser/{doc.js => place.js} (100%) rename test/ciao/{doc => place}/msuccess.coffee (100%) rename test/ciao/{doc => place}/success.coffee (100%) rename test/unit/controller/{doc.js => place.js} (100%) rename test/unit/sanitiser/{doc.js => place.js} (100%) diff --git a/controller/doc.js b/controller/place.js similarity index 100% rename from controller/doc.js rename to controller/place.js diff --git a/sanitiser/doc.js b/sanitiser/place.js similarity index 100% rename from sanitiser/doc.js rename to sanitiser/place.js diff --git a/test/ciao/doc/msuccess.coffee b/test/ciao/place/msuccess.coffee similarity index 100% rename from test/ciao/doc/msuccess.coffee rename to test/ciao/place/msuccess.coffee diff --git a/test/ciao/doc/success.coffee b/test/ciao/place/success.coffee similarity index 100% rename from test/ciao/doc/success.coffee rename to test/ciao/place/success.coffee diff --git a/test/unit/controller/doc.js b/test/unit/controller/place.js similarity index 100% rename from test/unit/controller/doc.js rename to test/unit/controller/place.js diff --git a/test/unit/sanitiser/doc.js b/test/unit/sanitiser/place.js similarity index 100% rename from test/unit/sanitiser/doc.js rename to test/unit/sanitiser/place.js From 167547a6c1cda52ec3a1b1ecb5a5d1839a143a18 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 18 Aug 2015 18:21:59 -0400 Subject: [PATCH 05/15] Rename doc endpoint to place --- app.js | 8 ++++---- test/ciao/place/msuccess.coffee | 4 ++-- test/ciao/place/success.coffee | 4 ++-- test/unit/controller/place.js | 4 ++-- test/unit/run.js | 4 ++-- test/unit/sanitiser/place.js | 8 ++++---- 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/app.js b/app.js index 32b8f788..ccdd5116 100644 --- a/app.js +++ b/app.js @@ -15,7 +15,7 @@ app.use( require('./middleware/jsonp') ); /** ----------------------- sanitisers ----------------------- **/ var sanitisers = {}; -sanitisers.doc = require('./sanitiser/doc'); +sanitisers.place = require('./sanitiser/place'); sanitisers.suggest = require('./sanitiser/suggest'); sanitisers.search = require('./sanitiser/search'); sanitisers.coarse = require('./sanitiser/coarse'); @@ -25,7 +25,7 @@ sanitisers.reverse = require('./sanitiser/reverse'); var controllers = {}; controllers.index = require('./controller/index'); -controllers.doc = require('./controller/doc'); +controllers.place = require('./controller/place'); controllers.search = require('./controller/search'); /** ----------------------- routes ----------------------- **/ @@ -33,8 +33,8 @@ controllers.search = require('./controller/search'); // api root app.get( '/', controllers.index() ); -// doc API -app.get( '/doc', sanitisers.doc.middleware, controllers.doc() ); +// place API +app.get( '/place', sanitisers.place.middleware, controllers.place() ); // suggest APIs app.get( '/suggest', sanitisers.search.middleware, controllers.search() ); diff --git a/test/ciao/place/msuccess.coffee b/test/ciao/place/msuccess.coffee index ae49b82a..5c69eab1 100644 --- a/test/ciao/place/msuccess.coffee +++ b/test/ciao/place/msuccess.coffee @@ -1,6 +1,6 @@ -#> valid doc query -path: '/doc?id=geoname:4221195&id=geoname:4193595' +#> valid place query +path: '/place?id=geoname:4221195&id=geoname:4193595' #? 200 ok response.statusCode.should.equal 200 diff --git a/test/ciao/place/success.coffee b/test/ciao/place/success.coffee index bd1b64bd..d4fdea2b 100644 --- a/test/ciao/place/success.coffee +++ b/test/ciao/place/success.coffee @@ -1,6 +1,6 @@ -#> valid doc query -path: '/doc?id=geoname:4221195' +#> valid place query +path: '/place?id=geoname:4221195' #? 200 ok response.statusCode.should.equal 200 diff --git a/test/unit/controller/place.js b/test/unit/controller/place.js index e9e3918b..9fe9115a 100644 --- a/test/unit/controller/place.js +++ b/test/unit/controller/place.js @@ -1,4 +1,4 @@ -var setup = require('../../../controller/doc'), +var setup = require('../../../controller/place'), mockBackend = require('../mock/backend'); module.exports.tests = {}; @@ -14,7 +14,7 @@ module.exports.tests.interface = function(test, common) { // functionally test controller (backend success) module.exports.tests.functional_success = function(test, common) { - // expected geojson features for 'client/doc/ok/1' fixture + // expected geojson features for 'client/place/ok/1' fixture var expected = [{ type: 'Feature', geometry: { diff --git a/test/unit/run.js b/test/unit/run.js index 222a1836..96e146cf 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -4,14 +4,14 @@ var common = {}; var tests = [ require('./controller/index'), - require('./controller/doc'), + require('./controller/place'), require('./controller/search'), require('./service/mget'), require('./service/search'), require('./sanitiser/suggest'), require('./sanitiser/search'), require('./sanitiser/reverse'), - require('./sanitiser/doc'), + require('./sanitiser/place'), require('./sanitiser/coarse'), require('./query/indeces'), require('./query/sort'), diff --git a/test/unit/sanitiser/place.js b/test/unit/sanitiser/place.js index 2f88dc0a..03456765 100644 --- a/test/unit/sanitiser/place.js +++ b/test/unit/sanitiser/place.js @@ -1,7 +1,7 @@ -var doc = require('../../../sanitiser/doc'), - _sanitize = doc.sanitize, - middleware = doc.middleware, +var place = require('../../../sanitiser/place'), + _sanitize = place.sanitize, + middleware = place.middleware, indeces = require('../../../query/indeces'), delimiter = ':', defaultLengthError = function(input) { return 'invalid param \''+ input + '\': text length, must be >0'; }, @@ -190,7 +190,7 @@ module.exports.tests.middleware_success = function(test, common) { module.exports.all = function (tape, common) { function test(name, testFunction) { - return tape('SANTIZE /doc ' + name, testFunction); + return tape('SANTIZE /place ' + name, testFunction); } for( var testCase in module.exports.tests ){ From a976b7b258a587478ad6c91bc27869e827209a1a Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 18 Aug 2015 18:25:07 -0400 Subject: [PATCH 06/15] Add places endpoint to API docs --- DOCS.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/DOCS.md b/DOCS.md index 57f742dc..a75b044a 100644 --- a/DOCS.md +++ b/DOCS.md @@ -89,14 +89,15 @@ The reverse geocoding endpoint; matches a point on the planet to the name of tha * `details` (default: `true`) -## /doc +## /place -The endpoint for retrieving one or more elasticsearch documents with specific ids. +The endpoint for retrieving one or more places with specific ids. These correspond +directly with Elasticsearch documents. #### Required Parameters * one of `id` or `ids` * `id`: - * unique id of the document to be retrieved + * unique id of the places to be retrieved * should be in the form of type:id, for example: `geoname:4163334` * `ids`: - * if multiple docs are to be fetched in bulk, an array of ids + * if multiple places are to be fetched in bulk, an array of ids From d000868800fdcd5eb2c0052d9e87f3f194f8d629 Mon Sep 17 00:00:00 2001 From: Severyn Kozak Date: Mon, 4 May 2015 14:46:52 -0400 Subject: [PATCH 07/15] Remove checks for longitude validity. sanitiser/_geo.js -Remove the code that verified `longitude` validity, to allow longitude values outside of the value's real range, [-180, 180]. elasticsearch appears to handle them gracefully, so this resolves #56. --- sanitiser/_geo.js | 51 +++++++++++++++++++---------------------------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/sanitiser/_geo.js b/sanitiser/_geo.js index cf1f0a5e..2090ce5e 100644 --- a/sanitiser/_geo.js +++ b/sanitiser/_geo.js @@ -1,3 +1,4 @@ +var util = require( 'util' ); var isObject = require('is-object'); @@ -46,26 +47,25 @@ function sanitize_bbox( clean, param ) { return; } - var bbox = []; var bboxArr = param.split( ',' ); if( Array.isArray( bboxArr ) && bboxArr.length === 4 ) { - bbox = bboxArr.filter( function( latlon, index ) { - latlon = parseFloat( latlon, 10 ); - return !(lat_lon_checks[(index % 2 === 0 ? 'lon' : 'lat')].is_invalid( latlon )); + var bbox = bboxArr.map( function ( latlon ){ + return parseFloat( latlon, 10 ); + }); + bboxArr.forEach( function( latlon, index ) { + if( index % 2 === 1 && check_lat.is_invalid( latlon ) ){ + throw new Error( 'Invalid lat: ' + latlon ); + } }); - if( bbox.length === 4 ) { - clean.bbox = { - right: Math.max( bbox[0], bbox[2] ), - top: Math.max( bbox[1], bbox[3] ), - left: Math.min( bbox[0], bbox[2] ), - bottom: Math.min( bbox[1], bbox[3] ) - }; - } else { - throw new Error('invalid bbox'); - } + clean.bbox = { + right: Math.max( bbox[0], bbox[2] ), + top: Math.max( bbox[1], bbox[3] ), + left: Math.min( bbox[0], bbox[2] ), + bottom: Math.min( bbox[1], bbox[3] ) + }; } } @@ -80,13 +80,13 @@ function sanitize_bbox( clean, param ) { function sanitize_coord( coord, clean, param, latlon_is_required ) { var value = parseFloat( param, 10 ); if ( !isNaN( value ) ) { - if( lat_lon_checks[coord].is_invalid( value ) ){ - throw new Error( 'invalid ' + lat_lon_checks[coord].error_msg ); + if( coord === 'lat' && check_lat.is_invalid( value ) ){ + throw new Error( 'invalid ' + check_lat.error_msg ); } clean[coord] = value; } else if (latlon_is_required) { - throw new Error('missing ' + lat_lon_checks[coord].error_msg); + throw new Error( util.format( 'missing param \'%s\'', coord ) ); } } @@ -97,18 +97,9 @@ function sanitize_zoom_level( clean, param ) { } } -var lat_lon_checks = { - lat: { - is_invalid: function is_invalid_lat(lat) { - return isNaN( lat ) || lat < -90 || lat > 90; - }, - error_msg: 'param \'lat\': must be >-90 and <90' +var check_lat = { + is_invalid: function is_invalid_lat( lat ){ + return isNaN( lat ) || lat < -90 || lat > 90; }, - lon: { - is_invalid: function is_invalid_lon(lon) { - return isNaN(lon) || lon < -180 || lon > 180; - }, - error_msg: 'param \'lon\': must be >-180 and <180' - } + error_msg: 'param \'lat\': must be >-90 and <90' }; - From 762ef85f5c3b5b387c505e034a2044eee9cbbf8b Mon Sep 17 00:00:00 2001 From: Severyn Kozak Date: Mon, 4 May 2015 14:49:35 -0400 Subject: [PATCH 08/15] Fix longitude-sanitization tests. test/unit/sanitiser/(search, suggest, reverse).js -Update/fix all of the tests that started failing as a result of the removal of longitude sanitization in 90a7683. --- test/unit/sanitiser/reverse.js | 23 ++++++----------------- test/unit/sanitiser/search.js | 17 +++-------------- test/unit/sanitiser/suggest.js | 17 +++-------------- 3 files changed, 12 insertions(+), 45 deletions(-) diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js index 90366a41..b85e4f40 100644 --- a/test/unit/sanitiser/reverse.js +++ b/test/unit/sanitiser/reverse.js @@ -3,7 +3,7 @@ var suggest = require('../../../sanitiser/reverse'), _sanitize = suggest.sanitize, middleware = suggest.middleware, delim = ',', - defaultError = 'missing param \'lat\': must be >-90 and <90', + defaultError = 'missing param \'lat\'', defaultClean = { lat:0, layers: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], @@ -59,7 +59,7 @@ module.exports.tests.sanitize_lat = function(test, common) { test('missing lat', function(t) { lats.missing.forEach( function( lat ){ sanitize({ lat: lat, lon: 0 }, function( err, clean ){ - t.equal(err, 'missing param \'lat\': must be >-90 and <90', 'latitude is a required field'); + t.equal(err, 'missing param \'lat\'', 'latitude is a required field'); t.equal(clean, undefined, 'clean not set'); }); }); @@ -69,20 +69,9 @@ module.exports.tests.sanitize_lat = function(test, common) { module.exports.tests.sanitize_lon = function(test, common) { var lons = { - invalid: [ -360, -181, 181, 360 ], - valid: [ -180, -1, -0, 0, 45, 90, '-180', '0', '180' ], + valid: [ -360, -181, 181, -180, -1, -0, 0, 45, 90, '-180', '0', '180' ], missing: ['', undefined, null] }; - test('invalid lon', function(t) { - lons.invalid.forEach( function( lon ){ - sanitize({ input: 'test', lat: 0, lon: lon }, function( err, clean ){ - t.equal(err, 'invalid param \'lon\': must be >-180 and <180', lon + ' is an invalid longitude'); - t.equal(clean, undefined, 'clean not set'); - - }); - }); - t.end(); - }); test('valid lon', function(t) { lons.valid.forEach( function( lon ){ sanitize({ input: 'test', lat: 0, lon: lon }, function( err, clean ){ @@ -97,7 +86,7 @@ module.exports.tests.sanitize_lon = function(test, common) { test('missing lon', function(t) { lons.missing.forEach( function( lon ){ sanitize({ lat: 0, lon: lon }, function( err, clean ){ - t.equal(err, 'missing param \'lon\': must be >-180 and <180', 'longitude is a required field'); + t.equal(err, 'missing param \'lon\'', 'longitude is a required field'); t.equal(clean, undefined, 'clean not set'); }); }); @@ -287,7 +276,7 @@ module.exports.tests.middleware_failure = function(test, common) { t.equal(code, 400, 'status set'); }}; var next = function( message ){ - t.equal(message, defaultError); + t.equals(message, defaultError); t.end(); }; middleware( {}, res, next ); @@ -315,4 +304,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/sanitiser/search.js b/test/unit/sanitiser/search.js index dfed13aa..08d02597 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -113,19 +113,8 @@ module.exports.tests.sanitize_lat = function(test, common) { module.exports.tests.sanitize_lon = function(test, common) { var lons = { - invalid: [ -360, -181, 181, 360 ], - valid: [ -180, -1, -0, 0, 45, 90, '-180', '0', '180' ] + valid: [ -381, -181, -180, -1, -0, 0, 45, 90, '-180', '0', '180', 181 ] }; - test('invalid lon', function(t) { - lons.invalid.forEach( function( lon ){ - sanitize({ input: 'test', lat: 0, lon: lon }, function( err, clean ){ - t.equal(err, 'invalid param \'lon\': must be >-180 and <180', lon + ' is an invalid longitude'); - t.equal(clean, undefined, 'clean not set'); - - }); - }); - t.end(); - }); test('valid lon', function(t) { lons.valid.forEach( function( lon ){ sanitize({ input: 'test', lat: 0, lon: lon }, function( err, clean ){ @@ -198,7 +187,7 @@ module.exports.tests.sanitize_bbox = function(test, common) { test('invalid bbox coordinates', function(t) { bboxes.invalid_coordinates.forEach( function( bbox ){ sanitize({ input: 'test', bbox: bbox }, function( err, clean ){ - t.equal(err, 'invalid bbox', bbox + ' is invalid'); + t.ok(err.match(/Invalid (lat|lon)/), bbox + ' is invalid'); t.equal(clean, undefined, 'clean not set'); }); }); @@ -438,4 +427,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/sanitiser/suggest.js b/test/unit/sanitiser/suggest.js index 75b1a88c..c429a896 100644 --- a/test/unit/sanitiser/suggest.js +++ b/test/unit/sanitiser/suggest.js @@ -110,19 +110,8 @@ module.exports.tests.sanitize_lat = function(test, common) { module.exports.tests.sanitize_lon = function(test, common) { var lons = { - invalid: [ -360, -181, 181, 360 ], - valid: [ -180, -1, -0, 0, 45, 90, '-180', '0', '180' ] + valid: [ -381, -181, -180, -1, -0, 0, 45, 90, '-180', '0', '180', 181 ] }; - test('invalid lon', function(t) { - lons.invalid.forEach( function( lon ){ - sanitize({ input: 'test', lat: 0, lon: lon }, function( err, clean ){ - t.equal(err, 'invalid param \'lon\': must be >-180 and <180', lon + ' is an invalid longitude'); - t.equal(clean, undefined, 'clean not set'); - - }); - }); - t.end(); - }); test('valid lon', function(t) { lons.valid.forEach( function( lon ){ sanitize({ input: 'test', lat: 0, lon: lon }, function( err, clean ){ @@ -160,7 +149,7 @@ module.exports.tests.sanitize_bbox = function(test, common) { test('invalid bbox coordinates', function(t) { bboxes.invalid_coordinates.forEach( function( bbox ){ sanitize({ input: 'test', lat: 0, lon: 0, bbox: bbox }, function( err, clean ){ - t.equal(err, 'invalid bbox', bbox + ' is invalid'); + t.ok(err.match(/Invalid (lat|lon)/), bbox + ' is invalid'); t.equal(clean, undefined, 'clean not set'); }); }); @@ -400,4 +389,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 +}; From 88680398355342c50d3aba9fd2f3447209ddd416 Mon Sep 17 00:00:00 2001 From: Severyn Kozak Date: Wed, 6 May 2015 10:37:47 -0400 Subject: [PATCH 09/15] Verify that bbox longitude is not NaN. sanitiser/_geo.js -The removal of the longitude sanitizer might result in a NaN longitude value passing through in the bbox. Add an `isNaN()` check. --- sanitiser/_geo.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/sanitiser/_geo.js b/sanitiser/_geo.js index 2090ce5e..3c90c48e 100644 --- a/sanitiser/_geo.js +++ b/sanitiser/_geo.js @@ -51,14 +51,17 @@ function sanitize_bbox( clean, param ) { if( Array.isArray( bboxArr ) && bboxArr.length === 4 ) { - var bbox = bboxArr.map( function ( latlon ){ - return parseFloat( latlon, 10 ); - }); - bboxArr.forEach( function( latlon, index ) { - if( index % 2 === 1 && check_lat.is_invalid( latlon ) ){ - throw new Error( 'Invalid lat: ' + latlon ); + var bbox = []; + for( var ind = 0; ind < bboxArr.length; ind++ ){ + var num = parseFloat( bboxArr[ ind ] ); + if( isNaN( num ) ){ + return; } - }); + if( ind % 2 === 1 && check_lat.is_invalid( num ) ){ + throw new Error( 'Invalid lat: ' + num ); + } + bbox.push( num ); + } clean.bbox = { right: Math.max( bbox[0], bbox[2] ), From 9b77b4434ffcaf62540893c1d38c9367199c6a3e Mon Sep 17 00:00:00 2001 From: Severyn Kozak Date: Wed, 6 May 2015 10:51:49 -0400 Subject: [PATCH 10/15] Add more test-cases for invalid bbox values. test/unit/sanitiser/search.js -Add some more sanity-check test-cases to verify that bbox validation is working as expected. --- test/unit/sanitiser/search.js | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index 08d02597..4f110a4d 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -167,20 +167,30 @@ module.exports.tests.sanitize_optional_geo = function(test, common) { module.exports.tests.sanitize_bbox = function(test, common) { var bboxes = { invalid_coordinates: [ - '-181,90,34,-180', // invalid top_right lon, bottom_left lat - '-170,91,-181,45', // invalid top_right lat, bottom_left lon - '-181,91,181,-91', // invalid top_right lon/lat, bottom_left lon/lat - '91, -181,-91,181',// invalid - spaces between coordinates + // invalid latitude coordinates + '-181,90,34,-180', + '-170,91,-181,45', + '-181,91,181,-91', + '91, -181,-91,11', + '91, -11,-91,181' ], invalid: [ '91;-181,-91,181', // invalid - semicolon between coordinates + 'these,are,not,numbers', + '0,0,0,notANumber', + ',,,', '91, -181, -91', // invalid - missing a coordinate '123,12', // invalid - missing coordinates '' // invalid - empty param ], valid: [ '-179,90,34,-80', // valid top_right lon/lat, bottom_left lon/lat - '0,0,0,0' // valid top_right lat/lon, bottom_left lat/lon + '0,0,0,0', + '10,20,30,40', + '-40,-20,10,30', + '-40,-20,10,30', + '-1200,20,1000,20', + '-1400,90,1400,-90' ] }; From 78bea9d8149a4773e02493c71b4edf3c835c1a45 Mon Sep 17 00:00:00 2001 From: Severyn Kozak Date: Wed, 6 May 2015 11:04:05 -0400 Subject: [PATCH 11/15] sanitiser/_geo.js: don't pass base to parseFloat(). --- sanitiser/_geo.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sanitiser/_geo.js b/sanitiser/_geo.js index 3c90c48e..c3ae409b 100644 --- a/sanitiser/_geo.js +++ b/sanitiser/_geo.js @@ -81,7 +81,7 @@ function sanitize_bbox( clean, param ) { * @param {bool} latlon_is_required */ function sanitize_coord( coord, clean, param, latlon_is_required ) { - var value = parseFloat( param, 10 ); + var value = parseFloat( param ); if ( !isNaN( value ) ) { if( coord === 'lat' && check_lat.is_invalid( value ) ){ throw new Error( 'invalid ' + check_lat.error_msg ); From b68bae25550885d6eace8589dba99261e9194548 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 23 Jul 2015 15:37:50 -0400 Subject: [PATCH 12/15] Remove latitude limits --- sanitiser/_geo.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/sanitiser/_geo.js b/sanitiser/_geo.js index c3ae409b..5bd2bb7c 100644 --- a/sanitiser/_geo.js +++ b/sanitiser/_geo.js @@ -57,9 +57,6 @@ function sanitize_bbox( clean, param ) { if( isNaN( num ) ){ return; } - if( ind % 2 === 1 && check_lat.is_invalid( num ) ){ - throw new Error( 'Invalid lat: ' + num ); - } bbox.push( num ); } From 396f27dcba05b2d1d2812d216b8eeda4302bfcf0 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 23 Jul 2015 15:38:44 -0400 Subject: [PATCH 13/15] Mark previously invalid lat/lon test values as valid These examples should all now pass with the lat/lon wrapping. --- test/unit/sanitiser/search.js | 14 +++++++------- test/unit/sanitiser/suggest.js | 10 +++++----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index 4f110a4d..a2bd4d64 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -167,12 +167,6 @@ module.exports.tests.sanitize_optional_geo = function(test, common) { module.exports.tests.sanitize_bbox = function(test, common) { var bboxes = { invalid_coordinates: [ - // invalid latitude coordinates - '-181,90,34,-180', - '-170,91,-181,45', - '-181,91,181,-91', - '91, -181,-91,11', - '91, -11,-91,181' ], invalid: [ '91;-181,-91,181', // invalid - semicolon between coordinates @@ -190,7 +184,13 @@ module.exports.tests.sanitize_bbox = function(test, common) { '-40,-20,10,30', '-40,-20,10,30', '-1200,20,1000,20', - '-1400,90,1400,-90' + '-1400,90,1400,-90', + // wrapped latitude coordinates + '-181,90,34,-180', + '-170,91,-181,45', + '-181,91,181,-91', + '91, -181,-91,11', + '91, -11,-91,181' ] }; diff --git a/test/unit/sanitiser/suggest.js b/test/unit/sanitiser/suggest.js index c429a896..a0ec198a 100644 --- a/test/unit/sanitiser/suggest.js +++ b/test/unit/sanitiser/suggest.js @@ -129,10 +129,6 @@ module.exports.tests.sanitize_lon = function(test, common) { module.exports.tests.sanitize_bbox = function(test, common) { var bboxes = { invalid_coordinates: [ - '-181,90,34,-180', // invalid top_right lon, bottom_left lat - '-170,91,-181,45', // invalid top_right lat, bottom_left lon - '-181,91,181,-91', // invalid top_right lon/lat, bottom_left lon/lat - '91, -181,-91,181',// invalid - spaces between coordinates ], invalid: [ '91;-181,-91,181', // invalid - semicolon between coordinates @@ -142,7 +138,11 @@ module.exports.tests.sanitize_bbox = function(test, common) { ], valid: [ '-179,90,34,-80', // valid top_right lon/lat, bottom_left lon/lat - '0,0,0,0' // valid top_right lat/lon, bottom_left lat/lon + '0,0,0,0', // valid top_right lat/lon, bottom_left lat/lon + '-181,90,34,-180', // wrapped top_right lon, bottom_left lat + '-170,91,-181,45', // wrapped top_right lat, bottom_left lon + '-181,91,181,-91', // wrapped top_right lon/lat, bottom_left lon/lat + '91, -181,-91,181',// valid - spaces between coordinates ] }; From 17ba5da6d592fd08ee357bcfe9f408c326cb6e82 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 23 Jul 2015 15:43:01 -0400 Subject: [PATCH 14/15] Replace array iteration boilerplate with .map Keeps the focus of the code on the actions to be performed, rather than the grunt work of iterating through the array. --- sanitiser/_geo.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/sanitiser/_geo.js b/sanitiser/_geo.js index 5bd2bb7c..28b92812 100644 --- a/sanitiser/_geo.js +++ b/sanitiser/_geo.js @@ -50,14 +50,10 @@ function sanitize_bbox( clean, param ) { var bboxArr = param.split( ',' ); if( Array.isArray( bboxArr ) && bboxArr.length === 4 ) { + var bbox = bboxArr.map(parseFloat); - var bbox = []; - for( var ind = 0; ind < bboxArr.length; ind++ ){ - var num = parseFloat( bboxArr[ ind ] ); - if( isNaN( num ) ){ - return; - } - bbox.push( num ); + if (bbox.some(isNaN)) { + return; } clean.bbox = { From b547111d524b9ddc97692959a92b2f9b8be016bc Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 23 Jul 2015 17:26:42 -0400 Subject: [PATCH 15/15] Remove latitude restrictions --- sanitiser/_geo.js | 10 ---------- test/unit/sanitiser/reverse.js | 4 ++-- test/unit/sanitiser/search.js | 4 ++-- test/unit/sanitiser/suggest.js | 4 ++-- 4 files changed, 6 insertions(+), 16 deletions(-) diff --git a/sanitiser/_geo.js b/sanitiser/_geo.js index 28b92812..b6b68f31 100644 --- a/sanitiser/_geo.js +++ b/sanitiser/_geo.js @@ -76,9 +76,6 @@ function sanitize_bbox( clean, param ) { function sanitize_coord( coord, clean, param, latlon_is_required ) { var value = parseFloat( param ); if ( !isNaN( value ) ) { - if( coord === 'lat' && check_lat.is_invalid( value ) ){ - throw new Error( 'invalid ' + check_lat.error_msg ); - } clean[coord] = value; } else if (latlon_is_required) { @@ -92,10 +89,3 @@ function sanitize_zoom_level( clean, param ) { clean.zoom = Math.min( Math.max( zoom, 1 ), 18 ); // max } } - -var check_lat = { - is_invalid: function is_invalid_lat( lat ){ - return isNaN( lat ) || lat < -90 || lat > 90; - }, - error_msg: 'param \'lat\': must be >-90 and <90' -}; diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js index b85e4f40..d2ff8293 100644 --- a/test/unit/sanitiser/reverse.js +++ b/test/unit/sanitiser/reverse.js @@ -32,8 +32,8 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.sanitize_lat = function(test, common) { var lats = { - invalid: [ -181, -120, -91, 91, 120, 181 ], - valid: [ 0, 45, 90, -0, '0', '45', '90' ], + invalid: [], + valid: [ 0, 45, 90, -0, '0', '45', '90', -181, -120, -91, 91, 120, 181 ], missing: ['', undefined, null] }; test('invalid lat', function(t) { diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index a2bd4d64..702bbcb5 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -84,8 +84,8 @@ module.exports.tests.sanitize_input_with_delim = function(test, common) { module.exports.tests.sanitize_lat = function(test, common) { var lats = { - invalid: [ -181, -120, -91, 91, 120, 181 ], - valid: [ 0, 45, 90, -0, '0', '45', '90' ] + invalid: [], + valid: [ 0, 45, 90, -0, '0', '45', '90', -181, -120, -91, 91, 120, 181 ] }; test('invalid lat', function(t) { lats.invalid.forEach( function( lat ){ diff --git a/test/unit/sanitiser/suggest.js b/test/unit/sanitiser/suggest.js index a0ec198a..b1ad1be6 100644 --- a/test/unit/sanitiser/suggest.js +++ b/test/unit/sanitiser/suggest.js @@ -82,8 +82,8 @@ module.exports.tests.sanitize_input_with_delim = function(test, common) { module.exports.tests.sanitize_lat = function(test, common) { var lats = { - invalid: [ -181, -120, -91, 91, 120, 181 ], - valid: [ 0, 45, 90, -0, '0', '45', '90' ] + invalid: [], + valid: [ 0, 45, 90, -0, '0', '45', '90', -181, -120, -91, 91, 120, 181 ] }; test('invalid lat', function(t) { lats.invalid.forEach( function( lat ){