From ebf162f29ef0524a4c164f8e6f90e40d7a8dad78 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Fri, 19 Dec 2014 16:08:18 -0500 Subject: [PATCH 1/6] search with optional bbox - initial commit +tests --- package.json | 2 +- query/search.js | 4 +++ sanitiser/_latlonzoom.js | 34 +++++++++++++++++++++++-- test/unit/query/search.js | 52 +++++++++++++++++---------------------- 4 files changed, 59 insertions(+), 33 deletions(-) diff --git a/package.json b/package.json index 46c326df..1c95d7bb 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.8", + "geopipes-elasticsearch-backend": "git://github.com/geopipes/elasticsearch-backend#geo_bbox", "pelias-esclient": "0.0.25", "toobusy": "^0.2.4" }, diff --git a/query/search.js b/query/search.js index ac07faa9..2dd99e81 100644 --- a/query/search.js +++ b/query/search.js @@ -11,6 +11,10 @@ function generate( params ){ var query = queries.distance( centroid, { size: params.size } ); + if (params.bbox) { + query = queries.bbox ( centroid, { size: params.size, bbox: params.bbox } ); + } + // add search condition to distance query query.query.filtered.query = { query_string : { diff --git a/sanitiser/_latlonzoom.js b/sanitiser/_latlonzoom.js index d1a17033..0854d891 100644 --- a/sanitiser/_latlonzoom.js +++ b/sanitiser/_latlonzoom.js @@ -9,9 +9,17 @@ function sanitize( req ){ params = {}; } + var is_invalid_lat = function(lat) { + return isNaN( lat ) || lat < -90 || lat > 90; + }; + + var is_invalid_lon = function(lon) { + return isNaN( lon ) || lon < -180 || lon > 180; + }; + // lat var lat = parseFloat( params.lat, 10 ); - if( isNaN( lat ) || lat < -90 || lat > 90 ){ + if( is_invalid_lat(lat) ){ return { 'error': true, 'message': 'invalid param \'lat\': must be >-90 and <90' @@ -21,7 +29,7 @@ function sanitize( req ){ // lon var lon = parseFloat( params.lon, 10 ); - if( isNaN( lon ) || lon < -180 || lon > 180 ){ + if( is_invalid_lon(lon) ){ return { 'error': true, 'message': 'invalid param \'lon\': must be >-180 and <180' @@ -37,6 +45,28 @@ function sanitize( req ){ clean.zoom = 10; // default } + // bbox + if (params.bbox) { + var bbox = []; + var bboxArr = params.bbox.split(','); + if( Array.isArray(bboxArr) && bboxArr.length === 4 ){ + bboxArr.forEach(function(latlon, index) { + latlon = parseFloat(latlon, 10); + if ( !(index % 2 === 0 ? is_invalid_lat(latlon) : is_invalid_lon(latlon)) ) { + bbox.push(latlon); + } + }); + if (bbox.length === 4) { + clean.bbox = bbox; + } else { + return { + 'error': true, + 'message': 'invalid bbox' + }; + } + } + } + req.clean = clean; return { 'error': false }; diff --git a/test/unit/query/search.js b/test/unit/query/search.js index 8dcc18ab..d4fdefc6 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -15,16 +15,17 @@ module.exports.tests.query = function(test, common) { var query = generate({ input: 'test', size: 10, lat: 29.49136, lon: -82.50622, - bbox: { - bottom_left: { - lat: 11.51053655297385, - lon: -103.16362455862279 - }, - top_right: { - lat: 47.472183447026154, - lon: -61.84881544137721 - } - }, + bbox: [47.47, -61.84, 11.51, -103.16], + // bbox: { //TODO write a test where no bbox is provided + // bottom_left: { + // lat: 11.51053655297385, + // lon: -103.16362455862279 + // }, + // top_right: { + // lat: 47.472183447026154, + // lon: -61.84881544137721 + // } + // }, layers: ['test'] }); @@ -44,34 +45,25 @@ module.exports.tests.query = function(test, common) { 'bool': { 'must': [ { - 'geo_distance': { - 'distance': '50km', - 'distance_type': 'plane', - 'optimize_bbox': 'indexed', - '_cache': true, + 'geo_bounding_box': { 'center_point': { - 'lat': '29.49', - 'lon': '-82.51' + 'top_right': { + 'lat': '47.47', + 'lon': '-61.84' + }, + 'bottom_left': { + 'lat': '11.51', + 'lon': '-103.16' + } } } } ] } - } - } - }, - 'sort': [ - { - '_geo_distance': { - 'center_point': { - 'lat': 29.49136, - 'lon': -82.50622 - }, - 'order': 'asc', - 'unit': 'km' } } - ], + }, + 'sort': [], 'size': 10 }; From 245abd78091176c950d2a8535425026f9e3d3c16 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Tue, 23 Dec 2014 11:36:06 -0500 Subject: [PATCH 2/6] elasticsearch-backend now only accepts vertices and not latlon as properties --- sanitiser/_latlonzoom.js | 7 ++++++- test/unit/query/search.js | 24 +++++++++++++----------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/sanitiser/_latlonzoom.js b/sanitiser/_latlonzoom.js index 0854d891..fd720cfe 100644 --- a/sanitiser/_latlonzoom.js +++ b/sanitiser/_latlonzoom.js @@ -57,7 +57,12 @@ function sanitize( req ){ } }); if (bbox.length === 4) { - clean.bbox = bbox; + clean.bbox = { + top : bbox[0], + right : bbox[1], + bottom: bbox[2], + left : bbox[3] + }; } else { return { 'error': true, diff --git a/test/unit/query/search.js b/test/unit/query/search.js index d4fdefc6..610223c6 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -15,7 +15,12 @@ module.exports.tests.query = function(test, common) { var query = generate({ input: 'test', size: 10, lat: 29.49136, lon: -82.50622, - bbox: [47.47, -61.84, 11.51, -103.16], + bbox: { + top: 47.47, + right: -61.84, + bottom: 11.51, + left: -103.16 + }, // bbox: { //TODO write a test where no bbox is provided // bottom_left: { // lat: 11.51053655297385, @@ -47,15 +52,12 @@ module.exports.tests.query = function(test, common) { { 'geo_bounding_box': { 'center_point': { - 'top_right': { - 'lat': '47.47', - 'lon': '-61.84' - }, - 'bottom_left': { - 'lat': '11.51', - 'lon': '-103.16' - } - } + 'top': '47.47', + 'right': '-61.84', + 'bottom':'11.51', + 'left': '-103.16' + }, + '_cache': true } } ] @@ -66,7 +68,7 @@ module.exports.tests.query = function(test, common) { 'sort': [], 'size': 10 }; - + console.log(JSON.stringify(query, 2, null)); t.deepEqual(query, expected, 'valid search query'); t.end(); }); From b6d1f1d4976dade17ce8acd4d1c9a2a249192241 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Tue, 23 Dec 2014 11:41:31 -0500 Subject: [PATCH 3/6] test valid search query without bbox (falls back to 50km distance query) --- test/unit/query/search.js | 70 +++++++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 11 deletions(-) diff --git a/test/unit/query/search.js b/test/unit/query/search.js index 610223c6..e077b9c9 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -21,16 +21,6 @@ module.exports.tests.query = function(test, common) { bottom: 11.51, left: -103.16 }, - // bbox: { //TODO write a test where no bbox is provided - // bottom_left: { - // lat: 11.51053655297385, - // lon: -103.16362455862279 - // }, - // top_right: { - // lat: 47.472183447026154, - // lon: -61.84881544137721 - // } - // }, layers: ['test'] }); @@ -68,7 +58,65 @@ module.exports.tests.query = function(test, common) { 'sort': [], 'size': 10 }; - console.log(JSON.stringify(query, 2, null)); + + t.deepEqual(query, expected, 'valid search query'); + t.end(); + }); + + test('valid query without bbox', function(t) { + var query = generate({ + input: 'test', size: 10, + lat: 29.49136, lon: -82.50622, + layers: ['test'] + }); + + var expected = { + 'query': { + 'filtered': { + 'query': { + 'query_string': { + 'query': 'test', + 'fields': [ + 'name.default' + ], + 'default_operator': 'OR' + } + }, + 'filter': { + 'bool': { + 'must': [ + { + 'geo_distance': { + 'distance': '50km', + 'distance_type': 'plane', + 'optimize_bbox': 'indexed', + '_cache': true, + 'center_point': { + 'lat': '29.49', + 'lon': '-82.51' + } + } + } + ] + } + } + } + }, + 'sort': [ + { + '_geo_distance': { + 'center_point': { + 'lat': 29.49136, + 'lon': -82.50622 + }, + 'order': 'asc', + 'unit': 'km' + } + } + ], + 'size': 10 + }; + t.deepEqual(query, expected, 'valid search query'); t.end(); }); From 84670180f729656eabaf5cd36b506caa57bbc29c Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Tue, 23 Dec 2014 11:48:00 -0500 Subject: [PATCH 4/6] renaming _latlonzoom to _geo --- sanitiser/{_latlonzoom.js => _geo.js} | 0 sanitiser/reverse.js | 2 +- sanitiser/suggest.js | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename sanitiser/{_latlonzoom.js => _geo.js} (100%) diff --git a/sanitiser/_latlonzoom.js b/sanitiser/_geo.js similarity index 100% rename from sanitiser/_latlonzoom.js rename to sanitiser/_geo.js diff --git a/sanitiser/reverse.js b/sanitiser/reverse.js index a99eff94..82b96ef1 100644 --- a/sanitiser/reverse.js +++ b/sanitiser/reverse.js @@ -2,7 +2,7 @@ var logger = require('../src/logger'), _sanitize = require('../sanitiser/_sanitize'), sanitiser = { - latlonzoom: require('../sanitiser/_latlonzoom'), + latlonzoom: require('../sanitiser/_geo'), size: function( req ) { var size = require('../sanitiser/_size'); return size(req, 1); diff --git a/sanitiser/suggest.js b/sanitiser/suggest.js index 7681233f..06288019 100644 --- a/sanitiser/suggest.js +++ b/sanitiser/suggest.js @@ -5,7 +5,7 @@ var logger = require('../src/logger'), input: require('../sanitiser/_input'), size: require('../sanitiser/_size'), layers: require('../sanitiser/_layers'), - latlonzoom: require('../sanitiser/_latlonzoom') + latlonzoom: require('../sanitiser/_geo') }; var sanitize = function(req, cb) { _sanitize(req, sanitizers, cb); }; From 8a7b41edc6385d4493d7a7b95ecf93b48dabcf4f Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Tue, 23 Dec 2014 12:17:43 -0500 Subject: [PATCH 5/6] test bbox sanitization --- test/unit/sanitiser/suggest.js | 60 ++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/test/unit/sanitiser/suggest.js b/test/unit/sanitiser/suggest.js index 28ec175e..c85d11c9 100644 --- a/test/unit/sanitiser/suggest.js +++ b/test/unit/sanitiser/suggest.js @@ -109,6 +109,66 @@ module.exports.tests.sanitize_lon = function(test, common) { }); }; +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', lat: 0, lon: 0, 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', lat: 0, lon: 0, bbox: bbox }, function( err, clean ){ + var expected = JSON.parse(JSON.stringify( defaultClean )); + t.equal(err, undefined, 'no error'); + t.deepEqual(clean, expected, 'clean set correctly'); + }); + }); + t.end(); + }); + test('valid bbox', function(t) { + bboxes.valid.forEach( function( bbox ){ + sanitize({ input: 'test', lat: 0, lon: 0, 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 : bboxArray[0], + right : bboxArray[1], + bottom: bboxArray[2], + left : 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 ){ From 3973039f192feab5d61b017d305187df882aacc7 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Tue, 23 Dec 2014 16:07:20 -0500 Subject: [PATCH 6/6] min/max lat/lon to form bbox from the input params --- sanitiser/_geo.js | 21 +++++++++++---------- test/unit/sanitiser/suggest.js | 10 +++++----- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/sanitiser/_geo.js b/sanitiser/_geo.js index fd720cfe..5e8baa64 100644 --- a/sanitiser/_geo.js +++ b/sanitiser/_geo.js @@ -45,23 +45,24 @@ function sanitize( req ){ clean.zoom = 10; // default } - // bbox + // bbox + // bbox = bottom_left lat, bottom_left lon, top_right lat, top_right lon + // bbox = left,bottom,right,top + // bbox = min Longitude , min Latitude , max Longitude , max Latitude if (params.bbox) { var bbox = []; var bboxArr = params.bbox.split(','); - if( Array.isArray(bboxArr) && bboxArr.length === 4 ){ - bboxArr.forEach(function(latlon, index) { + if( Array.isArray(bboxArr) && bboxArr.length === 4 ) { + bbox = bboxArr.filter(function(latlon, index) { latlon = parseFloat(latlon, 10); - if ( !(index % 2 === 0 ? is_invalid_lat(latlon) : is_invalid_lon(latlon)) ) { - bbox.push(latlon); - } + return !(index % 2 === 0 ? is_invalid_lat(latlon) : is_invalid_lon(latlon)); }); if (bbox.length === 4) { clean.bbox = { - top : bbox[0], - right : bbox[1], - bottom: bbox[2], - left : bbox[3] + top : Math.max(bbox[0], bbox[2]), + right : Math.max(bbox[1], bbox[3]), + bottom: Math.min(bbox[0], bbox[2]), + left : Math.min(bbox[1], bbox[3]) }; } else { return { diff --git a/test/unit/sanitiser/suggest.js b/test/unit/sanitiser/suggest.js index c85d11c9..9732b2d2 100644 --- a/test/unit/sanitiser/suggest.js +++ b/test/unit/sanitiser/suggest.js @@ -143,7 +143,7 @@ module.exports.tests.sanitize_bbox = function(test, common) { sanitize({ input: 'test', lat: 0, lon: 0, bbox: bbox }, function( err, clean ){ var expected = JSON.parse(JSON.stringify( defaultClean )); t.equal(err, undefined, 'no error'); - t.deepEqual(clean, expected, 'clean set correctly'); + t.deepEqual(clean, expected, 'falling back on 50km distance from centroid'); }); }); t.end(); @@ -156,10 +156,10 @@ module.exports.tests.sanitize_bbox = function(test, common) { return parseInt(i); }); expected.bbox = { - top : bboxArray[0], - right : bboxArray[1], - bottom: bboxArray[2], - left : bboxArray[3] + 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 + ')');