From 66e1cab00704618058c46b16ff1f49597198bfef Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 15 Sep 2015 14:09:36 -0400 Subject: [PATCH 01/11] Whitespace --- query/reverse.js | 1 - query/search.js | 1 - sanitiser/_geo_search.js | 1 - sanitiser/_text.js | 1 - test/unit/query/reverse.js | 4 +++- test/unit/sanitiser/_geo_common.js | 2 +- test/unit/sanitiser/search.js | 1 - 7 files changed, 4 insertions(+), 7 deletions(-) diff --git a/query/reverse.js b/query/reverse.js index f91561aa..be024072 100644 --- a/query/reverse.js +++ b/query/reverse.js @@ -1,4 +1,3 @@ - var peliasQuery = require('pelias-query'), defaults = require('./defaults'); diff --git a/query/search.js b/query/search.js index bc88516f..0884298d 100644 --- a/query/search.js +++ b/query/search.js @@ -1,4 +1,3 @@ - var peliasQuery = require('pelias-query'), defaults = require('./defaults'), textParser = require('./text_parser'); diff --git a/sanitiser/_geo_search.js b/sanitiser/_geo_search.js index cc2fafcb..237555b4 100644 --- a/sanitiser/_geo_search.js +++ b/sanitiser/_geo_search.js @@ -1,4 +1,3 @@ - var geo_common = require ('./_geo_common'); var LAT_LON_IS_REQUIRED = false; diff --git a/sanitiser/_text.js b/sanitiser/_text.js index 2f1f96af..5df79225 100644 --- a/sanitiser/_text.js +++ b/sanitiser/_text.js @@ -1,4 +1,3 @@ - var check = require('check-types'), query_parser = require('../helper/query_parser'); diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index 0d37fd1c..bb838626 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -21,6 +21,7 @@ module.exports.tests.query = function(test, common) { t.deepEqual(compiled, expected, 'valid reverse query'); t.end(); }); + test('valid query with radius', function(t) { var query = generate({ lat: 29.49136, lon: -82.50622, boundary_circle_radius: 123 @@ -32,6 +33,7 @@ module.exports.tests.query = function(test, common) { t.deepEqual(compiled, expected, 'distance set to boundary circle radius'); t.end(); }); + test('valid query with boundary.circle lat/lon/radius', function(t) { var clean = { lat: 29.49136, @@ -47,6 +49,7 @@ module.exports.tests.query = function(test, common) { t.deepEqual(compiled, expected, 'point.lat/lon overrides boundary.circle.lat/lon'); t.end(); }); + test('size fuzz test', function(t) { // test different sizes var sizes = [1,2,10,undefined,null]; @@ -73,7 +76,6 @@ module.exports.tests.query = function(test, common) { t.deepEqual(compiled, expected, 'valid reverse query with boundary.country'); t.end(); }); - }; module.exports.all = function (tape, common) { diff --git a/test/unit/sanitiser/_geo_common.js b/test/unit/sanitiser/_geo_common.js index 9c3d5b3f..dc5b4823 100644 --- a/test/unit/sanitiser/_geo_common.js +++ b/test/unit/sanitiser/_geo_common.js @@ -21,7 +21,7 @@ module.exports.tests.sanitize = function(test, common) { }; var is_required = true; var all_required = true; - + sanitize.sanitize_boundary_circle(clean, params, is_required, all_required); t.equal(clean.boundary_circle_lat, params['boundary.circle.lat'], 'lat approved'); t.equal(clean.boundary_circle_lon, params['boundary.circle.lon'], 'lon approved'); diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index 5992f94e..e6aee6ac 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -1,4 +1,3 @@ - var search = require('../../../sanitiser/search'), _text = require('../sanitiser/_text'), parser = require('../../../helper/query_parser'), From a5ba251afe30a2db3b5e40f574e136546ade62c5 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 15 Sep 2015 14:06:35 -0400 Subject: [PATCH 02/11] Remove brittle test fixture from search sanitiser Some of the tests were checking the entire clean object, when they only cared about one tiny element. This made making changes really hard. --- test/unit/sanitiser/_text.js | 24 ------------------------ test/unit/sanitiser/search.js | 28 +++++++++------------------- 2 files changed, 9 insertions(+), 43 deletions(-) delete mode 100644 test/unit/sanitiser/_text.js diff --git a/test/unit/sanitiser/_text.js b/test/unit/sanitiser/_text.js deleted file mode 100644 index 370a4061..00000000 --- a/test/unit/sanitiser/_text.js +++ /dev/null @@ -1,24 +0,0 @@ - -var text = require('../../../sanitiser/_text'), - parser = require('../../../helper/query_parser'), - allLayers = [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', - 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], - defaultParsed= { }, - defaultClean = { text: 'test', - layers: allLayers, - size: 10, - parsed_text: defaultParsed, - lat:0, - lon:0 - }, - getTargetLayers = function(query) { - var address = parser.get_parsed_address(query); - return address.target_layer; - }; - - -module.exports = { - defaultParsed: defaultParsed, - defaultClean : defaultClean, - getTargetLayers: getTargetLayers -}; diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index e6aee6ac..cb7125a1 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -1,17 +1,8 @@ var search = require('../../../sanitiser/search'), - _text = require('../sanitiser/_text'), parser = require('../../../helper/query_parser'), - defaultParsed = _text.defaultParsed, sanitize = search.sanitize, middleware = search.middleware, - defaultError = 'invalid param \'text\': text length, must be >0', - defaultClean = { text: 'test', - types: { - }, - size: 10, - parsed_text: defaultParsed, - }; - + defaultError = 'invalid param \'text\': text length, must be >0'; // these are the default values you would expect when no input params are specified. // @todo: why is this different from $defaultClean? var emptyClean = { boundary: {}, private: false, size: 10, types: {} }; @@ -85,13 +76,13 @@ module.exports.tests.sanitize_text_with_delim = function(test, common) { test('valid texts with a comma', function(t) { texts.forEach( function( text ){ var req = { query: { text: text } }; - sanitize(req, function(){ - var expected = JSON.parse(JSON.stringify( defaultClean )); - expected.text = text; + sanitize( req, function( ){ + var expected_text = text; - expected.parsed_text = parser.get_parsed_address(text); + var expected_parsed_text = parser.get_parsed_address(text); t.equal(req.errors[0], undefined, 'no error'); - t.equal(req.clean.parsed_text.name, expected.parsed_text.name, 'clean name set correctly'); + t.equal(req.clean.parsed_text.name, expected_parsed_text.name, 'clean name set correctly'); + t.equal(req.clean.text, expected_text, 'text should match'); }); }); @@ -164,11 +155,10 @@ module.exports.tests.sanitize_lon = function(test, common) { test('valid lon', function(t) { lons.valid.forEach( function( lon ){ var req = { query: { text: 'test', 'focus.point.lat': 0, 'focus.point.lon': lon } }; - sanitize(req, function(){ - var expected = JSON.parse(JSON.stringify( defaultClean )); - expected.lon = parseFloat( lon ); + sanitize( req, function(){ + var expected_lon = parseFloat( lon ); t.equal(req.errors[0], undefined, 'no error'); - t.equal(req.clean.lon, expected.lon, 'clean set correctly (' + lon + ')'); + t.deepEqual(req.clean.lon, expected_lon, 'clean set correctly (' + lon + ')'); }); }); t.end(); From 1bfa2fde40e4c6e18491f33ffd9e78d2de0dafa8 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 16 Sep 2015 13:13:03 -0400 Subject: [PATCH 03/11] Remove empty test of invalid lats --- test/unit/sanitiser/search.js | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index cb7125a1..72465bdc 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -121,22 +121,9 @@ module.exports.tests.sanitize_private_explicit_false_value = function(test, comm }; module.exports.tests.sanitize_lat = function(test, common) { - var lats = { - 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 ){ - var req = { query: { text: 'test', 'focus.point.lat': lat, 'focus.point.lon': 0 } }; - sanitize(req, function(){ - t.equal(req.errors[0], 'invalid param \'lat\': must be >-90 and <90', lat + ' is an invalid latitude'); - t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); - }); - }); - t.end(); - }); + var valid_lats = [ 0, 45, 90, -0, '0', '45', '90', -181, -120, -91, 91, 120, 181 ]; test('valid lat', function(t) { - lats.valid.forEach( function( lat ){ + valid_lats.forEach( function( lat ){ var req = { query: { text: 'test', 'focus.point.lat': lat, 'focus.point.lon': 0 } }; sanitize(req, function(){ var expected_lat = parseFloat( lat ); From e5ed04858bc07de7677d9b80e4bbb13c3518cd0d Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 16 Sep 2015 15:16:58 -0400 Subject: [PATCH 04/11] Test individual clean attribute intead of using deepEquals --- test/unit/sanitiser/reverse.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js index 1465a949..3a580f1b 100644 --- a/test/unit/sanitiser/reverse.js +++ b/test/unit/sanitiser/reverse.js @@ -61,10 +61,9 @@ module.exports.tests.sanitize_lat = function(test, common) { lats.valid.forEach( function( lat ){ var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; sanitize(req, function(){ - var expected = JSON.parse(JSON.stringify( defaultClean )); - expected.lat = parseFloat( lat ); + var expected_lat = parseFloat( lat ); t.deepEqual(req.errors, [], 'no errors'); - t.equal(req.clean.lat, parseFloat(lat), 'clean set correctly (' + lat + ')'); + t.equal(req.clean.lat, expected_lat, 'clean set correctly (' + lat + ')'); }); }); t.end(); @@ -90,10 +89,9 @@ module.exports.tests.sanitize_lon = function(test, common) { lons.valid.forEach( function( lon ){ var req = { query: { 'point.lat': 0, 'point.lon': lon } }; sanitize(req, function(){ - var expected = JSON.parse(JSON.stringify( defaultClean )); - expected.lon = parseFloat( lon ); + var expected_lon = parseFloat( lon ); t.deepEqual(req.errors, [], 'no errors'); - t.equal(req.clean.lon, parseFloat(lon), 'clean set correctly (' + lon + ')'); + t.equal(req.clean.lon, expected_lon, 'clean set correctly (' + lon + ')'); }); }); t.end(); From 4260442f59c194cef3ff420f6eac44a46c666059 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 16 Sep 2015 12:55:32 -0400 Subject: [PATCH 05/11] Change boundary.circle params to flat string structure --- query/reverse.js | 4 ++-- query/search.js | 4 ++-- sanitiser/_geo_common.js | 6 +++--- test/unit/query/reverse.js | 6 +++--- test/unit/query/search.js | 2 +- test/unit/sanitiser/_geo_common.js | 9 +++++---- 6 files changed, 16 insertions(+), 15 deletions(-) diff --git a/query/reverse.js b/query/reverse.js index be024072..95b788a3 100644 --- a/query/reverse.js +++ b/query/reverse.js @@ -33,8 +33,8 @@ function generateQuery( clean ){ // set radius, default to 500km if not specified in request var radius = 500; - if (clean.hasOwnProperty('boundary_circle_radius')) { - radius = clean.boundary_circle_radius; + if (clean.hasOwnProperty('boundary.circle.radius')) { + radius = clean['boundary.circle.radius']; } // focus point centroid diff --git a/query/search.js b/query/search.js index 0884298d..fff3c172 100644 --- a/query/search.js +++ b/query/search.js @@ -97,9 +97,9 @@ function generateQuery( clean ){ } // boundary country - if( clean.boundary && clean.boundary.country ){ + if( clean['boundary.country'] ){ vs.set({ - 'boundary:country': clean.boundary.country + 'boundary:country': clean['boundary.country'] }); } diff --git a/sanitiser/_geo_common.js b/sanitiser/_geo_common.js index 71b728cb..2ba50240 100644 --- a/sanitiser/_geo_common.js +++ b/sanitiser/_geo_common.js @@ -64,9 +64,9 @@ function sanitize_coord( coord, clean, param, latlon_is_required ) { */ function sanitize_boundary_circle( clean, params, is_required, all_required ) { var props = { - lat: 'boundary_circle_lat', - lon: 'boundary_circle_lon', - rad: 'boundary_circle_radius' + lat: 'boundary.circle.lat', + lon: 'boundary.circle.lon', + rad: 'boundary.circle.radius' }; // get values for each property diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index bb838626..d89568ae 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -24,7 +24,7 @@ module.exports.tests.query = function(test, common) { test('valid query with radius', function(t) { var query = generate({ - lat: 29.49136, lon: -82.50622, boundary_circle_radius: 123 + lat: 29.49136, lon: -82.50622, 'boundary.circle.radius': 123 }); var compiled = JSON.parse( JSON.stringify( query )).query.filtered.filter.bool.must[0].geo_distance.distance; @@ -38,8 +38,8 @@ module.exports.tests.query = function(test, common) { var clean = { lat: 29.49136, lon: -82.50622, - boundary_circle_lat: 111, - boundary_circle_long: 333 + 'boundary.circle.lat': 111, + 'boundary.circle.long': 333 }; var query = generate(clean); diff --git a/test/unit/query/search.js b/test/unit/query/search.js index b7b6ea9f..d595d1a7 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -138,7 +138,7 @@ module.exports.tests.query = function(test, common) { var query = generate({ text: 'test', size: 10, layers: ['test'], - boundary: { country: 'ABC' } + 'boundary.country': 'ABC' }); var compiled = JSON.parse( JSON.stringify( query ) ); diff --git a/test/unit/sanitiser/_geo_common.js b/test/unit/sanitiser/_geo_common.js index dc5b4823..8771e72c 100644 --- a/test/unit/sanitiser/_geo_common.js +++ b/test/unit/sanitiser/_geo_common.js @@ -23,11 +23,12 @@ module.exports.tests.sanitize = function(test, common) { var all_required = true; sanitize.sanitize_boundary_circle(clean, params, is_required, all_required); - t.equal(clean.boundary_circle_lat, params['boundary.circle.lat'], 'lat approved'); - t.equal(clean.boundary_circle_lon, params['boundary.circle.lon'], 'lon approved'); - t.equal(clean.boundary_circle_radius, params['boundary.circle.radius'], 'radius approved'); + t.equal(clean['boundary.circle.lat'], params['boundary.circle.lat'], 'lat approved'); + t.equal(clean['boundary.circle.lon'], params['boundary.circle.lon'], 'lon approved'); + t.equal(clean['boundary.circle.radius'], params['boundary.circle.radius'], 'radius approved'); t.end(); }); + test('valid circle, radius only, all not required', function (t) { var clean = {}; var params = { @@ -37,7 +38,7 @@ module.exports.tests.sanitize = function(test, common) { var all_required = false; sanitize.sanitize_boundary_circle(clean, params, is_required, all_required); - t.equal(clean.boundary_circle_radius, params['boundary.circle.radius'], 'radius approved'); + t.equal(clean['boundary.circle.radius'], params['boundary.circle.radius'], 'radius approved'); t.end(); }); }; From 97044f655beef7d363a0a8b913d7b9b2a96c90d1 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 16 Sep 2015 13:16:15 -0400 Subject: [PATCH 06/11] Flatten structure of clean for /search focus.point.lat|lon --- sanitiser/_geo_search.js | 4 ++-- test/unit/sanitiser/search.js | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/sanitiser/_geo_search.js b/sanitiser/_geo_search.js index 237555b4..5280efd0 100644 --- a/sanitiser/_geo_search.js +++ b/sanitiser/_geo_search.js @@ -8,8 +8,8 @@ module.exports = function sanitize( raw, clean ){ var messages = { errors: [], warnings: [] }; try { - geo_common.sanitize_coord( 'lat', clean, raw['focus.point.lat'], LAT_LON_IS_REQUIRED ); - geo_common.sanitize_coord( 'lon', clean, raw['focus.point.lon'], LAT_LON_IS_REQUIRED ); + geo_common.sanitize_coord( 'focus.point.lat', clean, raw['focus.point.lat'], LAT_LON_IS_REQUIRED ); + geo_common.sanitize_coord( 'focus.point.lon', clean, raw['focus.point.lon'], LAT_LON_IS_REQUIRED ); geo_common.sanitize_bbox(raw, clean); } catch (err) { diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index 72465bdc..9d8579e5 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -128,7 +128,7 @@ module.exports.tests.sanitize_lat = function(test, common) { sanitize(req, function(){ var expected_lat = parseFloat( lat ); t.equal(req.errors[0], undefined, 'no error'); - t.equal(req.clean.lat, expected_lat, 'clean lat set correctly (' + lat + ')'); + t.equal(req.clean['focus.point.lat'], expected_lat, 'clean lat set correctly (' + lat + ')'); }); }); t.end(); @@ -145,7 +145,7 @@ module.exports.tests.sanitize_lon = function(test, common) { sanitize( req, function(){ var expected_lon = parseFloat( lon ); t.equal(req.errors[0], undefined, 'no error'); - t.deepEqual(req.clean.lon, expected_lon, 'clean set correctly (' + lon + ')'); + t.deepEqual(req.clean['focus.point.lon'], expected_lon, 'clean set correctly (' + lon + ')'); }); }); t.end(); @@ -157,8 +157,8 @@ module.exports.tests.sanitize_optional_geo = function(test, common) { var req = { query: { text: 'test' } }; sanitize(req, function(){ t.equal(req.errors[0], undefined, 'no error'); - t.equal(req.clean.lat, undefined, 'clean set without lat'); - t.equal(req.clean.lon, undefined, 'clean set without lon'); + t.equal(req.clean['focus.point.lat'], undefined, 'clean set without lat'); + t.equal(req.clean['focus.point.lon'], undefined, 'clean set without lon'); }); t.end(); }); @@ -167,7 +167,7 @@ module.exports.tests.sanitize_optional_geo = function(test, common) { sanitize(req, function(){ var expected_lon = 0; t.equal(req.errors[0], undefined, 'no error'); - t.deepEqual(req.clean.lon, expected_lon, 'clean set correctly (without any lat)'); + t.deepEqual(req.clean['focus.point.lon'], expected_lon, 'clean set correctly (without any lat)'); }); t.end(); }); @@ -176,7 +176,7 @@ module.exports.tests.sanitize_optional_geo = function(test, common) { sanitize(req, function(){ var expected_lat = 0; t.equal(req.errors[0], undefined, 'no error'); - t.deepEqual(req.clean.lat, expected_lat, 'clean set correctly (without any lon)'); + t.deepEqual(req.clean['focus.point.lat'], expected_lat, 'clean set correctly (without any lon)'); }); t.end(); }); From d6abe09447234b8607add24bfc06cb96032fd6f4 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 16 Sep 2015 13:29:23 -0400 Subject: [PATCH 07/11] Use flat clean structure in /reverse --- sanitiser/_geo_reverse.js | 10 +++++----- test/unit/sanitiser/reverse.js | 16 ++++++++-------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/sanitiser/_geo_reverse.js b/sanitiser/_geo_reverse.js index da2bb79c..d97ca1c3 100644 --- a/sanitiser/_geo_reverse.js +++ b/sanitiser/_geo_reverse.js @@ -11,14 +11,14 @@ module.exports = function sanitize( raw, clean ){ var messages = { errors: [], warnings: [] }; try { - geo_common.sanitize_coord( 'lat', clean, raw['point.lat'], LAT_LON_IS_REQUIRED ); - geo_common.sanitize_coord( 'lon', clean, raw['point.lon'], LAT_LON_IS_REQUIRED ); + geo_common.sanitize_coord( 'point.lat', clean, raw['point.lat'], LAT_LON_IS_REQUIRED ); + geo_common.sanitize_coord( 'point.lon', clean, raw['point.lon'], LAT_LON_IS_REQUIRED ); // remove both if only one is set // @todo: clean this up! - if( !clean.hasOwnProperty('lat') || !clean.hasOwnProperty('lon') ){ - delete clean.lat; - delete clean.lon; + if( !clean.hasOwnProperty('point.lat') || !clean.hasOwnProperty('point.lon') ){ + delete clean['point.lat']; + delete clean['point.lon']; } // boundary.circle.* is not mandatory, and only specifying radius is fine, diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js index 3a580f1b..02adaaef 100644 --- a/test/unit/sanitiser/reverse.js +++ b/test/unit/sanitiser/reverse.js @@ -5,10 +5,10 @@ var reverse = require('../../../sanitiser/reverse'), sanitize = reverse.sanitize, middleware = reverse.middleware, defaultError = 'missing param \'lat\'', - defaultClean = { lat:0, + defaultClean = { 'point.lat': 0, types: { }, - lon: 0, + 'point.lon': 0, size: 10, private: false, boundary: { } @@ -51,7 +51,7 @@ module.exports.tests.sanitize_lat = function(test, common) { lats.invalid.forEach( function( lat ){ var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; sanitize(req, function(){ - t.equal(req.errors[0], 'invalid param \'lat\': must be >-90 and <90', lat + ' is an invalid latitude'); + t.equal(req.errors[0], 'invalid param \'point.lat\': must be >-90 and <90', lat + ' is an invalid latitude'); t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); }); }); @@ -63,7 +63,7 @@ module.exports.tests.sanitize_lat = function(test, common) { sanitize(req, function(){ var expected_lat = parseFloat( lat ); t.deepEqual(req.errors, [], 'no errors'); - t.equal(req.clean.lat, expected_lat, 'clean set correctly (' + lat + ')'); + t.equal(req.clean['point.lat'], expected_lat, 'clean set correctly (' + lat + ')'); }); }); t.end(); @@ -72,7 +72,7 @@ module.exports.tests.sanitize_lat = function(test, common) { lats.missing.forEach( function( lat ){ var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; sanitize(req, function(){ - t.equal(req.errors[0], 'missing param \'lat\'', 'latitude is a required field'); + t.equal(req.errors[0], 'missing param \'point.lat\'', 'latitude is a required field'); t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); }); }); @@ -91,7 +91,7 @@ module.exports.tests.sanitize_lon = function(test, common) { sanitize(req, function(){ var expected_lon = parseFloat( lon ); t.deepEqual(req.errors, [], 'no errors'); - t.equal(req.clean.lon, expected_lon, 'clean set correctly (' + lon + ')'); + t.equal(req.clean['point.lon'], expected_lon, 'clean set correctly (' + lon + ')'); }); }); t.end(); @@ -101,9 +101,9 @@ module.exports.tests.sanitize_lon = function(test, common) { var req = { query: { 'point.lat': 0, 'point.lon': lon } }; // @todo: why is lat set? - var expected = { boundary: {}, lat: 0, private: false, size: 10, types: {} }; + var expected = { boundary: {}, 'point.lat': 0, private: false, size: 10, types: {} }; sanitize(req, function(){ - t.equal(req.errors[0], 'missing param \'lon\'', 'longitude is a required field'); + t.equal(req.errors[0], 'missing param \'point.lon\'', 'longitude is a required field'); t.deepEqual(req.clean, expected, 'clean only has default values set'); }); }); From 190304b0390c7d9b32051406ad9a12ae1deb5a74 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 16 Sep 2015 14:15:03 -0400 Subject: [PATCH 08/11] Use flat structure for focus.point.{lat|lon} in query/search.js --- query/search.js | 6 +++--- test/unit/query/search.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/query/search.js b/query/search.js index fff3c172..9dc7702b 100644 --- a/query/search.js +++ b/query/search.js @@ -58,10 +58,10 @@ function generateQuery( clean ){ } // focus point - if( clean.lat && clean.lon ){ + if( clean['focus.point.lat'] && clean['focus.point.lon'] ){ vs.set({ - 'focus:point:lat': clean.lat, - 'focus:point:lon': clean.lon + 'focus:point:lat': clean['focus.point.lat'], + 'focus:point:lon': clean['focus.point.lon'] }); } diff --git a/test/unit/query/search.js b/test/unit/query/search.js index d595d1a7..3baa14f8 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -16,7 +16,7 @@ module.exports.tests.query = function(test, common) { test('valid search + focus + bbox', function(t) { var query = generate({ text: 'test', size: 10, - lat: 29.49136, lon: -82.50622, + 'focus.point.lat': 29.49136, 'focus.point.lon': -82.50622, bbox: { top: 47.47, right: -61.84, @@ -71,7 +71,7 @@ module.exports.tests.query = function(test, common) { test('search search + focus', function(t) { var query = generate({ text: 'test', size: 10, - lat: 29.49136, lon: -82.50622, + 'focus.point.lat': 29.49136, 'focus.point.lon': -82.50622, layers: ['test'] }); From 109c28a58851a3458cf750206e8283ce5b31d6f3 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 16 Sep 2015 14:25:13 -0400 Subject: [PATCH 09/11] Use flat point.{lat|lon} in query/reverse.js --- query/reverse.js | 10 +++++----- test/unit/query/reverse.js | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/query/reverse.js b/query/reverse.js index 95b788a3..029cb4e9 100644 --- a/query/reverse.js +++ b/query/reverse.js @@ -38,15 +38,15 @@ function generateQuery( clean ){ } // focus point centroid - if( clean.lat && clean.lon ){ + if( clean['point.lat'] && clean['point.lon'] ){ vs.set({ // focus point to score by distance - 'focus:point:lat': clean.lat, - 'focus:point:lon': clean.lon, + 'focus:point:lat': clean['point.lat'], + 'focus:point:lon': clean['point.lon'], // bounding circle - 'boundary:circle:lat': clean.lat, - 'boundary:circle:lon': clean.lon, + 'boundary:circle:lat': clean['point.lat'], + 'boundary:circle:lon': clean['point.lon'], 'boundary:circle:radius': radius + 'km' }); } diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index d89568ae..86a57aa9 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -12,7 +12,7 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.query = function(test, common) { test('valid query', function(t) { var query = generate({ - lat: 29.49136, lon: -82.50622 + 'point.lat': 29.49136, 'point.lon': -82.50622 }); var compiled = JSON.parse( JSON.stringify( query ) ); @@ -24,7 +24,7 @@ module.exports.tests.query = function(test, common) { test('valid query with radius', function(t) { var query = generate({ - lat: 29.49136, lon: -82.50622, 'boundary.circle.radius': 123 + 'point.lat': 29.49136, 'point.lon': -82.50622, 'boundary.circle.radius': 123 }); var compiled = JSON.parse( JSON.stringify( query )).query.filtered.filter.bool.must[0].geo_distance.distance; @@ -36,15 +36,15 @@ module.exports.tests.query = function(test, common) { test('valid query with boundary.circle lat/lon/radius', function(t) { var clean = { - lat: 29.49136, - lon: -82.50622, + 'point.lat': 29.49136, + 'point.lon': -82.50622, 'boundary.circle.lat': 111, 'boundary.circle.long': 333 }; var query = generate(clean); var compiled = JSON.parse( JSON.stringify( query )).query.filtered.filter.bool.must[0].geo_distance.center_point; - var expected = { lat: clean.lat, lon: clean.lon }; + var expected = { lat: clean['point.lat'], lon: clean['point.lon'] }; t.deepEqual(compiled, expected, 'point.lat/lon overrides boundary.circle.lat/lon'); t.end(); @@ -55,7 +55,7 @@ module.exports.tests.query = function(test, common) { var sizes = [1,2,10,undefined,null]; sizes.forEach( function( size ){ var query = generate({ - lat: 29.49136, lon: -82.50622, size: size + 'point.lat': 29.49136, 'point.lon': -82.50622, size: size }); var compiled = JSON.parse( JSON.stringify( query ) ); @@ -66,7 +66,7 @@ module.exports.tests.query = function(test, common) { test('valid boundary.country reverse search', function(t) { var query = generate({ - lat: 29.49136, lon: -82.50622, + 'point.lat': 29.49136, 'point.lon': -82.50622, boundary: { country: 'ABC' } }); From dd9654657e4ba931efab751840ac433591249849 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 16 Sep 2015 15:01:26 -0400 Subject: [PATCH 10/11] Use flat clean structure in query/autocomplete.js --- query/autocomplete.js | 6 +++--- test/unit/query/autocomplete.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/query/autocomplete.js b/query/autocomplete.js index 405fe486..d0b82d41 100644 --- a/query/autocomplete.js +++ b/query/autocomplete.js @@ -31,10 +31,10 @@ function generateQuery( clean ){ vs.var( 'size', 10 ); // focus point - if( clean.lat && clean.lon ){ + if( clean['focus.point.lat'] && clean['focus.point.lon'] ){ vs.set({ - 'focus:point:lat': clean.lat, - 'focus:point:lon': clean.lon + 'focus:point:lat': clean['focus.point.lat'], + 'focus:point:lon': clean['focus.point.lon'] }); } diff --git a/test/unit/query/autocomplete.js b/test/unit/query/autocomplete.js index 1167c17b..3b137fb2 100644 --- a/test/unit/query/autocomplete.js +++ b/test/unit/query/autocomplete.js @@ -27,8 +27,8 @@ module.exports.tests.query = function(test, common) { test('autocomplete + focus', function(t) { var query = generate({ text: 'test', - lat: 29.49136, - lon: -82.50622 + 'focus.point.lat': 29.49136, + 'focus.point.lon': -82.50622 }); var compiled = JSON.parse( JSON.stringify( query ) ); From f3acf3b30891fb38ed926b4fbd3ca5a7e1e2e5c8 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 17 Sep 2015 13:35:38 -0400 Subject: [PATCH 11/11] Check for numeric value of lat/lon to avoid null island bug Using the check-types module, check that lat/lon values are numbers, instead of checking their truthyness, to ensure that queries for null island work correctly. --- query/autocomplete.js | 5 +- query/reverse.js | 5 +- query/search.js | 5 +- ...tocomplete_linguistic_focus_null_island.js | 63 +++++++++++++++++++ test/unit/fixture/reverse_null_island.js | 45 +++++++++++++ .../search_linguistic_focus_null_island.js | 63 +++++++++++++++++++ test/unit/query/autocomplete.js | 14 +++++ test/unit/query/reverse.js | 12 ++++ test/unit/query/search.js | 15 +++++ 9 files changed, 221 insertions(+), 6 deletions(-) create mode 100644 test/unit/fixture/autocomplete_linguistic_focus_null_island.js create mode 100644 test/unit/fixture/reverse_null_island.js create mode 100644 test/unit/fixture/search_linguistic_focus_null_island.js diff --git a/query/autocomplete.js b/query/autocomplete.js index d0b82d41..5c956785 100644 --- a/query/autocomplete.js +++ b/query/autocomplete.js @@ -1,6 +1,7 @@ var peliasQuery = require('pelias-query'), - defaults = require('./defaults'); + defaults = require('./defaults'), + check = require('check-types'); //------------------------------ // autocomplete query @@ -31,7 +32,7 @@ function generateQuery( clean ){ vs.var( 'size', 10 ); // focus point - if( clean['focus.point.lat'] && clean['focus.point.lon'] ){ + if( check.number(clean['focus.point.lat']) && check.number(clean['focus.point.lon']) ){ vs.set({ 'focus:point:lat': clean['focus.point.lat'], 'focus:point:lon': clean['focus.point.lon'] diff --git a/query/reverse.js b/query/reverse.js index 029cb4e9..2ee283e3 100644 --- a/query/reverse.js +++ b/query/reverse.js @@ -1,5 +1,6 @@ var peliasQuery = require('pelias-query'), - defaults = require('./defaults'); + defaults = require('./defaults'), + check = require('check-types'); //------------------------------ // reverse geocode query @@ -38,7 +39,7 @@ function generateQuery( clean ){ } // focus point centroid - if( clean['point.lat'] && clean['point.lon'] ){ + if( check.number(clean['point.lat']) && check.number(clean['point.lon']) ){ vs.set({ // focus point to score by distance 'focus:point:lat': clean['point.lat'], diff --git a/query/search.js b/query/search.js index 9dc7702b..9dcf0273 100644 --- a/query/search.js +++ b/query/search.js @@ -1,6 +1,7 @@ var peliasQuery = require('pelias-query'), defaults = require('./defaults'), - textParser = require('./text_parser'); + textParser = require('./text_parser'), + check = require('check-types'); //------------------------------ // general-purpose search query @@ -58,7 +59,7 @@ function generateQuery( clean ){ } // focus point - if( clean['focus.point.lat'] && clean['focus.point.lon'] ){ + if( check.number(clean['focus.point.lat']) && check.number(clean['focus.point.lon']) ){ vs.set({ 'focus:point:lat': clean['focus.point.lat'], 'focus:point:lon': clean['focus.point.lon'] diff --git a/test/unit/fixture/autocomplete_linguistic_focus_null_island.js b/test/unit/fixture/autocomplete_linguistic_focus_null_island.js new file mode 100644 index 00000000..ded463bf --- /dev/null +++ b/test/unit/fixture/autocomplete_linguistic_focus_null_island.js @@ -0,0 +1,63 @@ + +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': { + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'boost': 1, + 'slop': 2, + 'query': 'test' + } + } + }, + 'functions': [{ + 'linear': { + 'center_point': { + 'origin': { + 'lat': 0, + 'lon': 0 + }, + 'offset': '1km', + 'scale': '50km', + 'decay': 0.5 + } + } + }], + 'score_mode': 'avg', + 'boost_mode': 'replace' + } + }] + } + } + } + }, + 'sort': [ '_score' ], + 'size': 10, + 'track_scores': true +}; diff --git a/test/unit/fixture/reverse_null_island.js b/test/unit/fixture/reverse_null_island.js new file mode 100644 index 00000000..f29ab293 --- /dev/null +++ b/test/unit/fixture/reverse_null_island.js @@ -0,0 +1,45 @@ + +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': 0, + 'lon': 0 + } + } + } + ] + } + } + } + }, + 'sort': [ + '_score', + { + '_geo_distance': { + 'center_point': { + 'lat': 0, + 'lon': 0 + }, + 'order': 'asc', + 'distance_type': 'plane' + } + } + ], + 'size': 1, + 'track_scores': true +}; diff --git a/test/unit/fixture/search_linguistic_focus_null_island.js b/test/unit/fixture/search_linguistic_focus_null_island.js new file mode 100644 index 00000000..506dcea9 --- /dev/null +++ b/test/unit/fixture/search_linguistic_focus_null_island.js @@ -0,0 +1,63 @@ + +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': { + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'boost': 1, + 'slop': 2, + 'query': 'test' + } + } + }, + 'functions': [{ + 'linear': { + 'center_point': { + 'origin': { + 'lat': 0, + 'lon': 0 + }, + 'offset': '1km', + 'scale': '50km', + 'decay': 0.5 + } + } + }], + 'score_mode': 'avg', + 'boost_mode': 'replace' + } + }] + } + } + } + }, + 'sort': [ '_sort' ], + 'size': 10, + 'track_scores': true +}; diff --git a/test/unit/query/autocomplete.js b/test/unit/query/autocomplete.js index 3b137fb2..ecdf9553 100644 --- a/test/unit/query/autocomplete.js +++ b/test/unit/query/autocomplete.js @@ -37,6 +37,20 @@ module.exports.tests.query = function(test, common) { t.deepEqual(compiled, expected, 'valid autocomplete query'); t.end(); }); + + test('autocomplete + focus on null island', function(t) { + var query = generate({ + text: 'test', + 'focus.point.lat': 0, + 'focus.point.lon': 0 + }); + + var compiled = JSON.parse( JSON.stringify( query ) ); + var expected = require('../fixture/autocomplete_linguistic_focus_null_island'); + + t.deepEqual(compiled, expected, 'valid autocomplete query'); + t.end(); + }); }; module.exports.all = function (tape, common) { diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index 86a57aa9..cb5e7513 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -22,6 +22,18 @@ module.exports.tests.query = function(test, common) { t.end(); }); + test('valid query', function(t) { + var query = generate({ + 'point.lat': 0, 'point.lon': 0 + }); + + var compiled = JSON.parse( JSON.stringify( query ) ); + var expected = require('../fixture/reverse_null_island'); + + t.deepEqual(compiled, expected, 'valid reverse query'); + t.end(); + }); + test('valid query with radius', function(t) { var query = generate({ 'point.lat': 29.49136, 'point.lon': -82.50622, 'boundary.circle.radius': 123 diff --git a/test/unit/query/search.js b/test/unit/query/search.js index 3baa14f8..7da298dd 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -83,6 +83,21 @@ module.exports.tests.query = function(test, common) { t.end(); }); + test('search search + focus on null island', function(t) { + var query = generate({ + text: 'test', size: 10, + 'focus.point.lat': 0, 'focus.point.lon': 0, + layers: ['test'] + }); + + var compiled = JSON.parse( JSON.stringify( query ) ); + var expected = require('../fixture/search_linguistic_focus_null_island'); + expected.sort = sort; + + t.deepEqual(compiled, expected, 'valid search query'); + t.end(); + }); + test('valid query with a full valid address', function(t) { var address = '123 main st new york ny 10010 US'; var query = generate({ text: address,