From 7a8422025a4aad461871b90ee27e99f397244e69 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Thu, 2 Apr 2015 11:47:55 -0400 Subject: [PATCH 01/10] latlon is a required field for reverse (updated _geo sanitizer) and added ability to filter by layers on /reverse --- sanitiser/_geo.js | 21 ++++++++++++++++----- sanitiser/reverse.js | 6 +++++- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/sanitiser/_geo.js b/sanitiser/_geo.js index 0b874837..2b7106af 100644 --- a/sanitiser/_geo.js +++ b/sanitiser/_geo.js @@ -1,8 +1,9 @@ // validate inputs, convert types and apply defaults -function sanitize( req ){ +function sanitize( req, latlon_is_required ){ var clean = req.clean || {}; var params= req.query; + latlon_is_required = latlon_is_required || false; // ensure the input params are a valid object if( Object.prototype.toString.call( params ) !== '[object Object]' ){ @@ -18,8 +19,8 @@ function sanitize( req ){ }; // lat - if (!isNaN(params.lat)) { - var lat = parseFloat( params.lat, 10 ); + var lat = parseFloat( params.lat, 10 ); + if (!isNaN(lat)) { if( is_invalid_lat(lat) ){ return { 'error': true, @@ -27,11 +28,16 @@ function sanitize( req ){ }; } clean.lat = lat; + } else if (latlon_is_required) { + return { + 'error': true, + 'message': 'missing param \'lat\': must be >-90 and <90' + }; } // lon - if (!isNaN(params.lon)) { - var lon = parseFloat( params.lon, 10 ); + var lon = parseFloat( params.lon, 10 ); + if (!isNaN(lon)) { if( is_invalid_lon(lon) ){ return { 'error': true, @@ -39,6 +45,11 @@ function sanitize( req ){ }; } clean.lon = lon; + } else if (latlon_is_required) { + return { + 'error': true, + 'message': 'missing param \'lon\': must be >-180 and <180' + }; } // zoom level diff --git a/sanitiser/reverse.js b/sanitiser/reverse.js index 82b96ef1..117b67d2 100644 --- a/sanitiser/reverse.js +++ b/sanitiser/reverse.js @@ -2,7 +2,11 @@ var logger = require('../src/logger'), _sanitize = require('../sanitiser/_sanitize'), sanitiser = { - latlonzoom: require('../sanitiser/_geo'), + latlonzoom: function( req ) { + var geo = require('../sanitiser/_geo'); + return geo(req, true); + }, + layers: require('../sanitiser/_layers'), size: function( req ) { var size = require('../sanitiser/_size'); return size(req, 1); From ec2507f1554b733c3e7bb5f6a3daf11f2dfe09c5 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Thu, 2 Apr 2015 12:20:37 -0400 Subject: [PATCH 02/10] tests: reverse sanitizers (latlon is required, size defaults to 1, supports layers) --- test/unit/run.js | 1 + test/unit/sanitiser/reverse.js | 237 +++++++++++++++++++++++++++++++++ 2 files changed, 238 insertions(+) create mode 100644 test/unit/sanitiser/reverse.js diff --git a/test/unit/run.js b/test/unit/run.js index 4f5cffde..5c270d15 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -11,6 +11,7 @@ var tests = [ require('./service/search'), require('./service/suggest'), require('./sanitiser/suggest'), + require('./sanitiser/reverse'), require('./sanitiser/doc'), require('./sanitiser/coarse'), require('./query/indeces'), diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js new file mode 100644 index 00000000..a6627273 --- /dev/null +++ b/test/unit/sanitiser/reverse.js @@ -0,0 +1,237 @@ + +var suggest = require('../../../sanitiser/reverse'), + _sanitize = suggest.sanitize, + middleware = suggest.middleware, + delim = ',', + defaultError = 'missing param \'lat\': must be >-90 and <90', + defaultClean = { lat:0, + layers: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', + 'osmaddress', 'openaddresses' ], + lon: 0, + size: 1 + }, + sanitize = function(query, cb) { _sanitize({'query':query}, cb); }; + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('sanitize interface', function(t) { + t.equal(typeof sanitize, 'function', 'sanitize is a function'); + t.equal(sanitize.length, 2, 'sanitize interface'); + t.end(); + }); + test('middleware interface', function(t) { + t.equal(typeof middleware, 'function', 'middleware is a function'); + t.equal(middleware.length, 3, 'sanitizee has a valid middleware'); + t.end(); + }); +}; + +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' ], + missing: ['', undefined, ,null] + }; + test('invalid lat', function(t) { + lats.invalid.forEach( function( lat ){ + sanitize({ lat: lat, lon: 0 }, function( err, clean ){ + t.equal(err, 'invalid param \'lat\': must be >-90 and <90', lat + ' is an invalid latitude'); + t.equal(clean, undefined, 'clean not set'); + }); + }); + t.end(); + }); + test('valid lat', function(t) { + lats.valid.forEach( function( lat ){ + sanitize({ lat: lat, lon: 0 }, function( err, clean ){ + var expected = JSON.parse(JSON.stringify( defaultClean )); + expected.lat = parseFloat( lat ); + t.equal(err, undefined, 'no error'); + t.deepEqual(clean, expected, 'clean set correctly (' + lat + ')'); + }); + }); + t.end(); + }); + 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(clean, undefined, 'clean not set'); + }); + }); + t.end(); + }); +}; + +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' ], + 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 ){ + var expected = JSON.parse(JSON.stringify( defaultClean )); + expected.lon = parseFloat( lon ); + t.equal(err, undefined, 'no error'); + t.deepEqual(clean, expected, 'clean set correctly (' + lon + ')'); + }); + }); + t.end(); + }); + 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(clean, undefined, 'clean not set'); + }); + }); + t.end(); + }); +}; + + +module.exports.tests.sanitize_size = function(test, common) { + test('invalid size value', function(t) { + sanitize({ size: 'a', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.equal(clean.size, 1, 'default size set'); + t.end(); + }); + }); + test('below min size value', function(t) { + sanitize({ size: -100, input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.equal(clean.size, 1, 'min size set'); + t.end(); + }); + }); + test('above max size value', function(t) { + sanitize({ size: 9999, input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.equal(clean.size, 40, 'max size set'); + t.end(); + }); + }); +}; + +module.exports.tests.sanitize_layers = function(test, common) { + test('unspecified', function(t) { + sanitize({ layers: undefined, input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.deepEqual(clean.layers, defaultClean.layers, 'default layers set'); + t.end(); + }); + }); + test('invalid layer', function(t) { + sanitize({ layers: 'test_layer', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + var msg = 'invalid param \'layer\': must be one or more of '; + t.true(err.match(msg), 'invalid layer requested'); + t.true(err.length > msg.length, 'invalid error message'); + t.end(); + }); + }); + test('poi (alias) layer', function(t) { + var poi_layers = ['geoname','osmnode','osmway']; + sanitize({ layers: 'poi', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.deepEqual(clean.layers, poi_layers, 'poi layers set'); + t.end(); + }); + }); + test('admin (alias) layer', function(t) { + var admin_layers = ['admin0','admin1','admin2','neighborhood']; + sanitize({ layers: 'admin', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.deepEqual(clean.layers, admin_layers, 'admin layers set'); + t.end(); + }); + }); + test('address (alias) layer', function(t) { + var address_layers = ['osmaddress','openaddresses']; + sanitize({ layers: 'address', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.deepEqual(clean.layers, address_layers, 'address layers set'); + t.end(); + }); + }); + test('poi alias layer plus regular layers', function(t) { + var poi_layers = ['geoname','osmnode','osmway']; + var reg_layers = ['admin0', 'admin1']; + sanitize({ layers: 'poi,admin0,admin1', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.deepEqual(clean.layers, reg_layers.concat(poi_layers), 'poi + regular layers'); + t.end(); + }); + }); + test('admin alias layer plus regular layers', function(t) { + var admin_layers = ['admin0','admin1','admin2','neighborhood']; + var reg_layers = ['geoname', 'osmway']; + sanitize({ layers: 'admin,geoname,osmway', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.deepEqual(clean.layers, reg_layers.concat(admin_layers), 'admin + regular layers set'); + t.end(); + }); + }); + test('address alias layer plus regular layers', function(t) { + var address_layers = ['osmaddress','openaddresses']; + var reg_layers = ['geoname', 'osmway']; + sanitize({ layers: 'address,geoname,osmway', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.deepEqual(clean.layers, reg_layers.concat(address_layers), 'address + regular layers set'); + t.end(); + }); + }); + test('alias layer plus regular layers (no duplicates)', function(t) { + var poi_layers = ['geoname','osmnode','osmway']; + sanitize({ layers: 'poi,geoname,osmnode', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.deepEqual(clean.layers, poi_layers, 'poi layers found (no duplicates)'); + t.end(); + }); + }); + test('multiple alias layers (no duplicates)', function(t) { + var alias_layers = ['geoname','osmnode','osmway','admin0','admin1','admin2','neighborhood']; + sanitize({ layers: 'poi,admin', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.deepEqual(clean.layers, alias_layers, 'all layers found (no duplicates)'); + t.end(); + }); + }); +}; + +module.exports.tests.middleware_failure = function(test, common) { + test('middleware failure', function(t) { + var res = { status: function( code ){ + t.equal(code, 400, 'status set'); + }}; + var next = function( message ){ + t.equal(message, defaultError); + t.end(); + }; + middleware( {}, res, next ); + }); +}; + +module.exports.tests.middleware_success = function(test, common) { + test('middleware success', function(t) { + var req = { query: { input: 'test', lat: 0, lon: 0 }}; + var next = function( message ){ + t.equal(message, undefined, 'no error message set'); + t.deepEqual(req.clean, defaultClean); + t.end(); + }; + middleware( req, undefined, next ); + }); +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('SANTIZE /reverse ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; \ No newline at end of file From 898896bc24020f9bc7bfb3e287fb1f7f1e1ff585 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Thu, 2 Apr 2015 14:22:48 -0400 Subject: [PATCH 03/10] giving request layers higher precedence (compared to the layers specified in queryMixer) --- query/suggest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query/suggest.js b/query/suggest.js index 571b135e..249f7c38 100644 --- a/query/suggest.js +++ b/query/suggest.js @@ -34,7 +34,7 @@ function generate( params, query_mixer, fuzziness ){ 'size' : this.params.size, 'field' : 'suggest', 'context': { - 'dataset': layers || this.params.layers, + 'dataset': this.params.layers || layers, 'location': { 'value': null, 'precision': precision || this.get_precision() From e586b49d1d3e868344749f712cbe83ad7b5e5f31 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Thu, 2 Apr 2015 15:38:47 -0400 Subject: [PATCH 04/10] tests - that check if the layers are being set correctly for suggesters and using query mixers right --- test/unit/query/suggest.js | 53 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/test/unit/query/suggest.js b/test/unit/query/suggest.js index de442ffb..d1067af0 100644 --- a/test/unit/query/suggest.js +++ b/test/unit/query/suggest.js @@ -1,5 +1,6 @@ var generate = require('../../../query/suggest'); +var queryMixer = require('../../../helper/queryMixer'); module.exports.tests = {}; @@ -152,6 +153,58 @@ module.exports.tests.fuzziness = function(test, common) { }); }; +module.exports.tests.queryMixer = function(test, common) { + test('valid query mixer', function(t) { + for (var suggester in queryMixer) { + var queryMix = queryMixer[suggester]; + + var number_of_suggesters = queryMix.reduce(function(sum, query) { + return sum + (query.precision.length || 1); + }, 0); + + var query = generate({ + input: 'test', size: 10, + lat: 0, lon: 0, zoom:0 + }, queryMix); + + // adding one to number_of_suggesters to account for the key "text" in query. + t.deepEqual(Object.keys(query).length, number_of_suggesters + 1, + suggester + ' has correct number of suggesters' + ); + } + + t.end(); + }); +}; + +var isValidLayer = function(t, query, layers) { + for(var qKey in query) { + var q = query[qKey]; + if (q.completion) { + var query_layers = q.completion.context.dataset; + t.deepEqual(query_layers, layers, layers + ' layers set correctly'); + } + } +}; + +module.exports.tests.layers = function(test, common) { + test('valid layers with query-mixers', function(t) { + for (var suggester in queryMixer) { + var queryMix = queryMixer[suggester]; + var layers= ['geoname', 'osm']; + var query = generate({ + input: 'test', size: 10, + lat: 0, lon: 0, zoom:0, + layers: layers + }, queryMix); + + isValidLayer(t, query, layers); + } + + t.end(); + }); +}; + module.exports.all = function (tape, common) { function test(name, testFunction) { From 217a5c1049deabc713229eb35a2089c47909ec5a Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Thu, 2 Apr 2015 16:07:36 -0400 Subject: [PATCH 05/10] seperating suggest and search sanitizers because suggest requires geoBias (lat/lon) until ES 2.0.0 lands. --- app.js | 2 +- sanitiser/search.js | 26 ++++++++++++++++++++++++++ sanitiser/suggest.js | 5 ++++- 3 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 sanitiser/search.js diff --git a/app.js b/app.js index 047aa903..49d07b94 100644 --- a/app.js +++ b/app.js @@ -12,7 +12,7 @@ app.use( require('./middleware/jsonp') ); var sanitisers = {}; sanitisers.doc = require('./sanitiser/doc'); sanitisers.suggest = require('./sanitiser/suggest'); -sanitisers.search = sanitisers.suggest; +sanitisers.search = require('./sanitiser/search'); sanitisers.coarse = require('./sanitiser/coarse'); sanitisers.reverse = require('./sanitiser/reverse'); diff --git a/sanitiser/search.js b/sanitiser/search.js new file mode 100644 index 00000000..1748846a --- /dev/null +++ b/sanitiser/search.js @@ -0,0 +1,26 @@ + +var logger = require('../src/logger'), + _sanitize = require('../sanitiser/_sanitize'), + sanitizers = { + input: require('../sanitiser/_input'), + size: require('../sanitiser/_size'), + layers: require('../sanitiser/_layers'), + latlonzoom: require('../sanitiser/_geo') + }; + +var sanitize = function(req, cb) { _sanitize(req, sanitizers, cb); }; + +// export sanitize for testing +module.exports.sanitize = sanitize; + +// middleware +module.exports.middleware = function( req, res, next ){ + sanitize( req, function( err, clean ){ + if( err ){ + res.status(400); // 400 Bad Request + return next(err); + } + req.clean = clean; + next(); + }); +}; diff --git a/sanitiser/suggest.js b/sanitiser/suggest.js index 06288019..1897dcb8 100644 --- a/sanitiser/suggest.js +++ b/sanitiser/suggest.js @@ -5,7 +5,10 @@ var logger = require('../src/logger'), input: require('../sanitiser/_input'), size: require('../sanitiser/_size'), layers: require('../sanitiser/_layers'), - latlonzoom: require('../sanitiser/_geo') + latlonzoom: function( req ) { + var geo = require('../sanitiser/_geo'); + return geo(req, true); + } }; var sanitize = function(req, cb) { _sanitize(req, sanitizers, cb); }; From 5b34767e8e473bef9be6489d12ff78653805abc6 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Mon, 6 Apr 2015 17:06:41 -0400 Subject: [PATCH 06/10] tests for sanitizer/search (tests optional geo basically in addition to the usual stuff - suggest/search sanitizer tests should eventually be merged) --- test/unit/run.js | 1 + test/unit/sanitiser/search.js | 388 ++++++++++++++++++++++++++++++++++ 2 files changed, 389 insertions(+) create mode 100644 test/unit/sanitiser/search.js diff --git a/test/unit/run.js b/test/unit/run.js index 5c270d15..a8663fad 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -11,6 +11,7 @@ var tests = [ require('./service/search'), require('./service/suggest'), require('./sanitiser/suggest'), + require('./sanitiser/search'), require('./sanitiser/reverse'), require('./sanitiser/doc'), require('./sanitiser/coarse'), diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js new file mode 100644 index 00000000..e685a442 --- /dev/null +++ b/test/unit/sanitiser/search.js @@ -0,0 +1,388 @@ + +var search = require('../../../sanitiser/search'), + _sanitize = search.sanitize, + middleware = search.middleware, + delim = ',', + defaultError = 'invalid param \'input\': text length, must be >0', + defaultClean = { input: 'test', + layers: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', + 'osmaddress', 'openaddresses' ], + size: 10 + }, + sanitize = function(query, cb) { _sanitize({'query':query}, cb); }; + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('sanitize interface', function(t) { + t.equal(typeof sanitize, 'function', 'sanitize is a function'); + t.equal(sanitize.length, 2, 'sanitize interface'); + t.end(); + }); + test('middleware interface', function(t) { + t.equal(typeof middleware, 'function', 'middleware is a function'); + t.equal(middleware.length, 3, 'sanitizee has a valid middleware'); + t.end(); + }); +}; + +module.exports.tests.sanitize_input = function(test, common) { + var inputs = { + invalid: [ '', 100, null, undefined, new Date() ], + valid: [ 'a', 'aa', 'aaaaaaaa' ] + }; + test('invalid input', function(t) { + inputs.invalid.forEach( function( input ){ + sanitize({ input: input }, function( err, clean ){ + t.equal(err, 'invalid param \'input\': text length, must be >0', input + ' is an invalid input'); + t.equal(clean, undefined, 'clean not set'); + }); + }); + t.end(); + }); + test('valid input', function(t) { + inputs.valid.forEach( function( input ){ + sanitize({ input: input }, function( err, clean ){ + var expected = JSON.parse(JSON.stringify( defaultClean )); + expected.input = input; + t.equal(err, undefined, 'no error'); + t.deepEqual(clean, expected, 'clean set correctly (' + input + ')'); + }); + }); + t.end(); + }); +}; + +module.exports.tests.sanitize_input_with_delim = function(test, common) { + var inputs = [ 'a,bcd', '123 main st, admin1', ',,,', ' ' ]; + + test('valid inputs with a comma', function(t) { + inputs.forEach( function( input ){ + sanitize({ input: input }, function( err, clean ){ + var expected = JSON.parse(JSON.stringify( defaultClean )); + expected.input = input; + + var delim_index = input.indexOf(delim); + if (delim_index!==-1) { + expected.input = input.substring(0, input.indexOf(delim)); + expected.input_admin = input.substring(delim_index + 1).trim(); + } + + t.equal(err, undefined, 'no error'); + t.deepEqual(clean, expected, 'clean set correctly (' + input + ')'); + }); + }); + t.end(); + }); +}; + +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' ] + }; + test('invalid lat', function(t) { + lats.invalid.forEach( function( lat ){ + sanitize({ input: 'test', lat: lat, lon: 0 }, function( err, clean ){ + t.equal(err, 'invalid param \'lat\': must be >-90 and <90', lat + ' is an invalid latitude'); + t.equal(clean, undefined, 'clean not set'); + }); + }); + t.end(); + }); + test('valid lat', function(t) { + lats.valid.forEach( function( lat ){ + sanitize({ input: 'test', lat: lat, lon: 0 }, function( err, clean ){ + var expected = JSON.parse(JSON.stringify( defaultClean )); + expected.lat = parseFloat( lat ); + expected.lon = 0; + t.equal(err, undefined, 'no error'); + t.deepEqual(clean, expected, 'clean set correctly (' + lat + ')'); + }); + }); + t.end(); + }); +}; + +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' ] + }; + 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 ){ + var expected = JSON.parse(JSON.stringify( defaultClean )); + expected.lon = parseFloat( lon ); + expected.lat = 0; + t.equal(err, undefined, 'no error'); + t.deepEqual(clean, expected, 'clean set correctly (' + lon + ')'); + }); + }); + t.end(); + }); +}; + +module.exports.tests.sanitize_optional_geo = function(test, common) { + test('no lat/lon', function(t) { + sanitize({ input: 'test' }, function( err, clean ){ + var expected = defaultClean; + t.equal(err, undefined, 'no error'); + t.equal(clean.lat, undefined, 'clean set without lat'); + t.equal(clean.lon, undefined, 'clean set without lon'); + t.deepEqual(clean, expected, 'clean set without lat/lon'); + }); + t.end(); + }); + test('no lat', function(t) { + sanitize({ input: 'test', lon: 0 }, function( err, clean ){ + var expected = JSON.parse(JSON.stringify( defaultClean )); + expected.lon = 0; + t.equal(err, undefined, 'no error'); + t.deepEqual(clean, expected, 'clean set correctly (without any lat)'); + }); + t.end(); + }); + test('no lon', function(t) { + sanitize({ input: 'test', lat: 0 }, function( err, clean ){ + var expected = JSON.parse(JSON.stringify( defaultClean )); + expected.lat = 0; + t.equal(err, undefined, 'no error'); + t.deepEqual(clean, expected, 'clean set correctly (without any lon)'); + }); + t.end(); + }); +}; + +module.exports.tests.sanitize_bbox = function(test, common) { + var bboxes = { + invalid_coordinates: [ + '90,-181,-180,34', // invalid top_right lon, bottom_left lat + '91,-170,45,-181', // invalid top_right lat, bottom_left lon + '91,-181,-91,181', // invalid top_right lat/lon, bottom_left lat/lon + '91, -181,-91,181',// invalid - spaces between coordinates + ], + invalid: [ + '91;-181,-91,181', // invalid - semicolon between coordinates + '91, -181, -91', // invalid - missing a coordinate + '123,12', // invalid - missing coordinates + '' // invalid - empty param + ], + valid: [ + '90,-179,-80,34', // valid top_right lat/lon, bottom_left lat/lon + '0,0,0,0' // valid top_right lat/lon, bottom_left lat/lon + ] + + }; + 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.equal(clean, undefined, 'clean not set'); + }); + }); + t.end(); + }); + test('invalid bbox', function(t) { + bboxes.invalid.forEach( function( bbox ){ + sanitize({ input: 'test', bbox: bbox }, function( err, clean ){ + var expected = JSON.parse(JSON.stringify( defaultClean )); + t.equal(err, undefined, 'no error'); + t.deepEqual(clean, expected, 'falling back on 50km distance from centroid'); + }); + }); + t.end(); + }); + test('valid bbox', function(t) { + bboxes.valid.forEach( function( bbox ){ + sanitize({ input: 'test', bbox: bbox }, function( err, clean ){ + var expected = JSON.parse(JSON.stringify( defaultClean )); + var bboxArray = bbox.split(',').map(function(i) { + return parseInt(i); + }); + expected.bbox = { + top : Math.max(bboxArray[0], bboxArray[2]), + right : Math.max(bboxArray[1], bboxArray[3]), + bottom: Math.min(bboxArray[0], bboxArray[2]), + left : Math.min(bboxArray[1], bboxArray[3]) + }; + t.equal(err, undefined, 'no error'); + t.deepEqual(clean, expected, 'clean set correctly (' + bbox + ')'); + }); + }); + t.end(); + }); +}; + +module.exports.tests.sanitize_zoom = function(test, common) { + test('invalid zoom value', function(t) { + sanitize({ zoom: 'a', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.equal(clean.zoom, undefined, 'zoom not set'); + t.end(); + }); + }); + test('below min zoom value', function(t) { + sanitize({ zoom: -100, input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.equal(clean.zoom, 1, 'min zoom set'); + t.end(); + }); + }); + test('above max zoom value', function(t) { + sanitize({ zoom: 9999, input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.equal(clean.zoom, 18, 'max zoom set'); + t.end(); + }); + }); +}; + +module.exports.tests.sanitize_size = function(test, common) { + test('invalid size value', function(t) { + sanitize({ size: 'a', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.equal(clean.size, 10, 'default size set'); + t.end(); + }); + }); + test('below min size value', function(t) { + sanitize({ size: -100, input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.equal(clean.size, 1, 'min size set'); + t.end(); + }); + }); + test('above max size value', function(t) { + sanitize({ size: 9999, input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + t.equal(clean.size, 40, 'max size set'); + t.end(); + }); + }); +}; + +module.exports.tests.sanitize_layers = function(test, common) { + test('unspecified', function(t) { + sanitize({ layers: undefined, input: 'test' }, function( err, clean ){ + t.deepEqual(clean.layers, defaultClean.layers, 'default layers set'); + t.end(); + }); + }); + test('invalid layer', function(t) { + sanitize({ layers: 'test_layer', input: 'test' }, function( err, clean ){ + var msg = 'invalid param \'layer\': must be one or more of '; + t.true(err.match(msg), 'invalid layer requested'); + t.true(err.length > msg.length, 'invalid error message'); + t.end(); + }); + }); + test('poi (alias) layer', function(t) { + var poi_layers = ['geoname','osmnode','osmway']; + sanitize({ layers: 'poi', input: 'test' }, function( err, clean ){ + t.deepEqual(clean.layers, poi_layers, 'poi layers set'); + t.end(); + }); + }); + test('admin (alias) layer', function(t) { + var admin_layers = ['admin0','admin1','admin2','neighborhood']; + sanitize({ layers: 'admin', input: 'test' }, function( err, clean ){ + t.deepEqual(clean.layers, admin_layers, 'admin layers set'); + t.end(); + }); + }); + test('address (alias) layer', function(t) { + var address_layers = ['osmaddress','openaddresses']; + sanitize({ layers: 'address', input: 'test' }, function( err, clean ){ + t.deepEqual(clean.layers, address_layers, 'address layers set'); + t.end(); + }); + }); + test('poi alias layer plus regular layers', function(t) { + var poi_layers = ['geoname','osmnode','osmway']; + var reg_layers = ['admin0', 'admin1']; + sanitize({ layers: 'poi,admin0,admin1', input: 'test' }, function( err, clean ){ + t.deepEqual(clean.layers, reg_layers.concat(poi_layers), 'poi + regular layers'); + t.end(); + }); + }); + test('admin alias layer plus regular layers', function(t) { + var admin_layers = ['admin0','admin1','admin2','neighborhood']; + var reg_layers = ['geoname', 'osmway']; + sanitize({ layers: 'admin,geoname,osmway', input: 'test' }, function( err, clean ){ + t.deepEqual(clean.layers, reg_layers.concat(admin_layers), 'admin + regular layers set'); + t.end(); + }); + }); + test('address alias layer plus regular layers', function(t) { + var address_layers = ['osmaddress','openaddresses']; + var reg_layers = ['geoname', 'osmway']; + sanitize({ layers: 'address,geoname,osmway', input: 'test' }, function( err, clean ){ + t.deepEqual(clean.layers, reg_layers.concat(address_layers), 'address + regular layers set'); + t.end(); + }); + }); + test('alias layer plus regular layers (no duplicates)', function(t) { + var poi_layers = ['geoname','osmnode','osmway']; + sanitize({ layers: 'poi,geoname,osmnode', input: 'test' }, function( err, clean ){ + t.deepEqual(clean.layers, poi_layers, 'poi layers found (no duplicates)'); + t.end(); + }); + }); + test('multiple alias layers (no duplicates)', function(t) { + var alias_layers = ['geoname','osmnode','osmway','admin0','admin1','admin2','neighborhood']; + sanitize({ layers: 'poi,admin', input: 'test' }, function( err, clean ){ + t.deepEqual(clean.layers, alias_layers, 'all layers found (no duplicates)'); + t.end(); + }); + }); +}; + +module.exports.tests.invalid_params = function(test, common) { + test('invalid input params', function(t) { + sanitize( undefined, function( err, clean ){ + t.equal(err, defaultError, 'handle invalid params gracefully'); + t.end(); + }); + }); +}; + +module.exports.tests.middleware_failure = function(test, common) { + test('middleware failure', function(t) { + var res = { status: function( code ){ + t.equal(code, 400, 'status set'); + }}; + var next = function( message ){ + t.equal(message, defaultError); + t.end(); + }; + middleware( {}, res, next ); + }); +}; + +module.exports.tests.middleware_success = function(test, common) { + test('middleware success', function(t) { + var req = { query: { input: 'test' }}; + var next = function( message ){ + t.equal(message, undefined, 'no error message set'); + t.deepEqual(req.clean, defaultClean); + t.end(); + }; + middleware( req, undefined, next ); + }); +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('SANTIZE /search ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; \ No newline at end of file From b51db53213a1721911528a0d4bff84c5fdab517c Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Tue, 7 Apr 2015 11:43:57 -0400 Subject: [PATCH 07/10] using an optimized bbox query --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 7a541a0b..65ee7691 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,7 @@ "express": "^4.8.8", "geojson": "^0.2.0", "geojson-extent": "^0.3.1", - "geopipes-elasticsearch-backend": "0.0.11", + "geopipes-elasticsearch-backend": "git://github.com/geopipes/elasticsearch-backend#bbox-optimize", "pelias-suggester-pipeline": "2.0.2", "is-object": "^1.0.1", "pelias-esclient": "0.0.25" From f13f1cdf2cc4cc2fa6fcd95e22026b0ec6792d3b Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Tue, 7 Apr 2015 14:40:38 -0400 Subject: [PATCH 08/10] tests --- test/unit/query/search.js | 112 +++++++++++++------------------------- 1 file changed, 38 insertions(+), 74 deletions(-) diff --git a/test/unit/query/search.js b/test/unit/query/search.js index 1e5301e3..82a23239 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -49,6 +49,44 @@ var sort = [ } ]; +var expected = { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'must': [{ + 'match': { + 'name.default': 'test' + } + } + ] + } + }, + 'filter': { + 'bool': { + 'must': [ + { + 'geo_bounding_box': { + 'center_point': { + 'top': '47.47', + 'right': '-61.84', + 'bottom':'11.51', + 'left': '-103.16' + }, + '_cache': true, + 'type': 'indexed' + } + } + ] + } + } + } + }, + 'sort': sort, + 'size': 10, + 'track_scores': true +}; + module.exports.tests.query = function(test, common) { test('valid query', function(t) { var query = generate({ @@ -63,43 +101,6 @@ module.exports.tests.query = function(test, common) { layers: ['test'] }); - var expected = { - 'query': { - 'filtered': { - 'query': { - 'bool': { - 'must': [{ - 'match': { - 'name.default': 'test' - } - } - ] - } - }, - 'filter': { - 'bool': { - 'must': [ - { - 'geo_bounding_box': { - 'center_point': { - 'top': '47.47', - 'right': '-61.84', - 'bottom':'11.51', - 'left': '-103.16' - }, - '_cache': true - } - } - ] - } - } - } - }, - 'sort': sort, - 'size': 10, - 'track_scores': true - }; - t.deepEqual(query, expected, 'valid search query'); t.end(); }); @@ -115,43 +116,6 @@ module.exports.tests.query = function(test, common) { }, layers: ['test'] }); - - var expected = { - 'query': { - 'filtered': { - 'query': { - 'bool': { - 'must': [{ - 'match': { - 'name.default': 'test' - } - } - ] - } - }, - 'filter': { - 'bool': { - 'must': [ - { - 'geo_bounding_box': { - 'center_point': { - 'top': '47.47', - 'right': '-61.84', - 'bottom':'11.51', - 'left': '-103.16' - }, - '_cache': true - } - } - ] - } - } - } - }, - 'sort': sort, - 'size': 10, - 'track_scores': true - }; t.deepEqual(query, expected, 'valid search query'); t.end(); From 5094ecb25e9e642dce1356f6f7c4ba48e811a3e1 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Tue, 7 Apr 2015 15:16:46 -0400 Subject: [PATCH 09/10] adding 'locality', 'local_admin' to the list of layers (part of admin alias) plus tests --- helper/layers.js | 2 +- query/indeces.js | 2 ++ test/unit/query/indeces.js | 2 +- test/unit/sanitiser/coarse.js | 4 ++-- test/unit/sanitiser/suggest.js | 8 ++++---- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/helper/layers.js b/helper/layers.js index 8efce727..aa625a6e 100644 --- a/helper/layers.js +++ b/helper/layers.js @@ -13,7 +13,7 @@ module.exports = function(alias_layers) { }; layers = expand_aliases('poi', layers, ['geoname','osmnode','osmway']); - layers = expand_aliases('admin', layers, ['admin0','admin1','admin2','neighborhood']); + layers = expand_aliases('admin', layers, ['admin0','admin1','admin2','neighborhood','locality','local_admin']); layers = expand_aliases('address', layers, ['osmaddress','openaddresses']); // de-dupe diff --git a/query/indeces.js b/query/indeces.js index 04d744b6..1fff5961 100644 --- a/query/indeces.js +++ b/query/indeces.js @@ -9,6 +9,8 @@ module.exports = [ 'admin1', 'admin2', 'neighborhood', + 'locality', + 'local_admin', 'osmaddress', 'openaddresses' ]; \ No newline at end of file diff --git a/test/unit/query/indeces.js b/test/unit/query/indeces.js index 924aed69..fd552809 100644 --- a/test/unit/query/indeces.js +++ b/test/unit/query/indeces.js @@ -6,7 +6,7 @@ module.exports.tests = {}; module.exports.tests.interface = function(test, common) { test('valid interface', function(t) { t.true(Array.isArray(indeces), 'valid array'); - t.equal(indeces.length, 9, 'valid array'); + t.equal(indeces.length, 11, 'valid array'); t.end(); }); }; diff --git a/test/unit/sanitiser/coarse.js b/test/unit/sanitiser/coarse.js index f389b423..3409cbc4 100644 --- a/test/unit/sanitiser/coarse.js +++ b/test/unit/sanitiser/coarse.js @@ -2,7 +2,7 @@ var coarse = require('../../../sanitiser/coarse'), _sanitize = coarse.sanitize, middleware = coarse.middleware, - valid_layers = [ 'admin0', 'admin1', 'admin2', 'neighborhood' ], + valid_layers = [ 'admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin' ], sanitize = function(query, cb) { _sanitize({'query':query}, cb); }; module.exports.tests = {}; @@ -51,7 +51,7 @@ module.exports.tests.middleware_success = function(test, common) { var defaultClean = { input: 'test', size: 10, - layers: [ 'admin0', 'admin1', 'admin2', 'neighborhood' ], + layers: [ 'admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin' ], lat: 0, lon: 0 }; diff --git a/test/unit/sanitiser/suggest.js b/test/unit/sanitiser/suggest.js index a576f6de..88e75d56 100644 --- a/test/unit/sanitiser/suggest.js +++ b/test/unit/sanitiser/suggest.js @@ -7,7 +7,7 @@ var suggest = require('../../../sanitiser/suggest'), defaultClean = { input: 'test', lat:0, layers: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', - 'osmaddress', 'openaddresses' ], + 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], lon: 0, size: 10 }, @@ -258,7 +258,7 @@ module.exports.tests.sanitize_layers = function(test, common) { }); }); test('admin (alias) layer', function(t) { - var admin_layers = ['admin0','admin1','admin2','neighborhood']; + var admin_layers = ['admin0','admin1','admin2','neighborhood','locality','local_admin']; sanitize({ layers: 'admin', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ t.deepEqual(clean.layers, admin_layers, 'admin layers set'); t.end(); @@ -280,7 +280,7 @@ module.exports.tests.sanitize_layers = function(test, common) { }); }); test('admin alias layer plus regular layers', function(t) { - var admin_layers = ['admin0','admin1','admin2','neighborhood']; + var admin_layers = ['admin0','admin1','admin2','neighborhood','locality','local_admin']; var reg_layers = ['geoname', 'osmway']; sanitize({ layers: 'admin,geoname,osmway', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ t.deepEqual(clean.layers, reg_layers.concat(admin_layers), 'admin + regular layers set'); @@ -303,7 +303,7 @@ module.exports.tests.sanitize_layers = function(test, common) { }); }); test('multiple alias layers (no duplicates)', function(t) { - var alias_layers = ['geoname','osmnode','osmway','admin0','admin1','admin2','neighborhood']; + var alias_layers = ['geoname','osmnode','osmway','admin0','admin1','admin2','neighborhood','locality','local_admin']; sanitize({ layers: 'poi,admin', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ t.deepEqual(clean.layers, alias_layers, 'all layers found (no duplicates)'); t.end(); From 94b044d17904cba5e8015f26f8f52a8697da0963 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Wed, 8 Apr 2015 16:39:50 -0400 Subject: [PATCH 10/10] using elasticsearch-backend 0.0.12 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 65ee7691..57ddf4cb 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,7 @@ "express": "^4.8.8", "geojson": "^0.2.0", "geojson-extent": "^0.3.1", - "geopipes-elasticsearch-backend": "git://github.com/geopipes/elasticsearch-backend#bbox-optimize", + "geopipes-elasticsearch-backend": "0.0.12", "pelias-suggester-pipeline": "2.0.2", "is-object": "^1.0.1", "pelias-esclient": "0.0.25"