From 57b133ba420aa256788e006c873ae9a5bcd308f4 Mon Sep 17 00:00:00 2001 From: Lily He Date: Thu, 13 Jul 2017 18:01:51 -0400 Subject: [PATCH 1/2] check and set default for undefined boundary circle radius in reverse --- query/reverse.js | 12 ++++++++++++ sanitizer/_geo_reverse.js | 5 ----- test/unit/query/reverse.js | 17 +++++++++++++++++ test/unit/sanitizer/_geo_reverse.js | 22 +--------------------- test/unit/sanitizer/nearby.js | 1 - test/unit/sanitizer/reverse.js | 1 - 6 files changed, 30 insertions(+), 28 deletions(-) diff --git a/query/reverse.js b/query/reverse.js index fbe1251d..dba05935 100644 --- a/query/reverse.js +++ b/query/reverse.js @@ -75,6 +75,18 @@ function generateQuery( clean ){ logStr += '[param:boundary_circle] '; } + // for coarse reverse when boundary circle radius is undefined + if( check.number(clean['boundary.circle.lat']) && + check.number(clean['boundary.circle.lon']) && + check.undefined(clean['boundary.circle.radius']) ){ + vs.set({ + 'boundary:circle:lat': clean['boundary.circle.lat'], + 'boundary:circle:lon': clean['boundary.circle.lon'], + 'boundary:circle:radius': defaults['boundary:circle:radius'] + }); + logStr += '[param:boundary_circle] '; + } + // boundary country if( check.string(clean['boundary.country']) ){ vs.set({ diff --git a/sanitizer/_geo_reverse.js b/sanitizer/_geo_reverse.js index 5a7195eb..1bb85043 100644 --- a/sanitizer/_geo_reverse.js +++ b/sanitizer/_geo_reverse.js @@ -29,11 +29,6 @@ module.exports = function sanitize( raw, clean ){ raw['boundary.circle.lat'] = clean['point.lat']; raw['boundary.circle.lon'] = clean['point.lon']; - // if no radius was passed, set the default - if ( _.isUndefined( raw['boundary.circle.radius'] ) ) { - raw['boundary.circle.radius'] = defaults['boundary:circle:radius']; - } - // santize the boundary.circle geo_common.sanitize_circle( 'boundary.circle', clean, raw, CIRCLE_IS_REQUIRED ); diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index d773224b..eab97188 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -61,6 +61,23 @@ module.exports.tests.query = function(test, common) { t.end(); }); + // for coarse reverse cases where boundary circle radius isn't used + test('undefined radius set to default radius', function(t) { + var query = generate({ + 'point.lat': 12.12121, + 'point.lon': 21.21212, + 'boundary.circle.lat': 12.12121, + 'boundary.circle.lon': 21.21212 + }); + + var compiled = JSON.parse( JSON.stringify( query ) ); + var expected = '1km'; + + t.deepEqual(compiled.type, 'reverse', 'query type set'); + t.deepEqual(compiled.body.query.bool.filter[0].geo_distance.distance, expected, 'distance set to default boundary circle radius'); + t.end(); + }); + test('boundary.circle lat/lon/radius - overrides point.lat/lon when set', function(t) { var clean = { 'point.lat': 29.49136, diff --git a/test/unit/sanitizer/_geo_reverse.js b/test/unit/sanitizer/_geo_reverse.js index 4df32711..223cf9fa 100644 --- a/test/unit/sanitizer/_geo_reverse.js +++ b/test/unit/sanitizer/_geo_reverse.js @@ -84,31 +84,11 @@ module.exports.tests.success_conditions = (test, common) => { }); - test('boundary.circle.radius not specified should use default', (t) => { - const raw = { - 'point.lat': '12.121212', - 'point.lon': '21.212121' - }; - const clean = {}; - const errorsAndWarnings = sanitize(raw, clean); - - t.equals(raw['boundary.circle.lat'], 12.121212); - t.equals(raw['boundary.circle.lon'], 21.212121); - t.equals(raw['boundary.circle.radius'], defaults['boundary:circle:radius'], 'should be from defaults'); - t.equals(clean['boundary.circle.lat'], 12.121212); - t.equals(clean['boundary.circle.lon'], 21.212121); - t.equals(clean['boundary.circle.radius'], parseFloat(defaults['boundary:circle:radius']), 'should be same as raw'); - - t.deepEquals(errorsAndWarnings, { errors: [], warnings: [] }); - t.end(); - - }); - }; module.exports.all = (tape, common) => { function test(name, testFunction) { - return tape(`SANTIZE _geo_reverse ${name}`, testFunction); + return tape(`SANITIZE _geo_reverse ${name}`, testFunction); } for( const testCase in module.exports.tests ){ diff --git a/test/unit/sanitizer/nearby.js b/test/unit/sanitizer/nearby.js index 1ba41f67..72be019a 100644 --- a/test/unit/sanitizer/nearby.js +++ b/test/unit/sanitizer/nearby.js @@ -8,7 +8,6 @@ var defaultClean = { 'point.lat': 0, 'point.lon': 0, 'boundary.circle.lat': 0, 'boundary.circle.lon': 0, - 'boundary.circle.radius': parseFloat(defaults['boundary:circle:radius']), size: 10, private: false }; diff --git a/test/unit/sanitizer/reverse.js b/test/unit/sanitizer/reverse.js index 430d3ffa..c7125743 100644 --- a/test/unit/sanitizer/reverse.js +++ b/test/unit/sanitizer/reverse.js @@ -10,7 +10,6 @@ var reverse = require('../../../sanitizer/reverse'), 'point.lon': 0, 'boundary.circle.lat': 0, 'boundary.circle.lon': 0, - 'boundary.circle.radius': parseFloat(defaults['boundary:circle:radius']), size: 10, private: false }; From 2a73bb34acb7ed22b214609cbc94a315a2a4aaba Mon Sep 17 00:00:00 2001 From: Lily He Date: Wed, 19 Jul 2017 10:38:07 -0400 Subject: [PATCH 2/2] combined checks for boundary circle radius into one block --- query/reverse.js | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/query/reverse.js b/query/reverse.js index dba05935..a9c1d25c 100644 --- a/query/reverse.js +++ b/query/reverse.js @@ -65,25 +65,24 @@ function generateQuery( clean ){ // where point.lan/point.lon are provided in the // absense of boundary.circle.lat/boundary.circle.lon if( check.number(clean['boundary.circle.lat']) && - check.number(clean['boundary.circle.lon']) && - check.number(clean['boundary.circle.radius']) ){ - vs.set({ - 'boundary:circle:lat': clean['boundary.circle.lat'], - 'boundary:circle:lon': clean['boundary.circle.lon'], - 'boundary:circle:radius': clean['boundary.circle.radius'] + 'km' - }); - logStr += '[param:boundary_circle] '; - } - - // for coarse reverse when boundary circle radius is undefined - if( check.number(clean['boundary.circle.lat']) && - check.number(clean['boundary.circle.lon']) && - check.undefined(clean['boundary.circle.radius']) ){ - vs.set({ - 'boundary:circle:lat': clean['boundary.circle.lat'], - 'boundary:circle:lon': clean['boundary.circle.lon'], - 'boundary:circle:radius': defaults['boundary:circle:radius'] - }); + check.number(clean['boundary.circle.lon']) ){ + + vs.set({ + 'boundary:circle:lat': clean['boundary.circle.lat'], + 'boundary:circle:lon': clean['boundary.circle.lon'] + }); + + if (check.undefined(clean['boundary.circle.radius'])){ + // for coarse reverse when boundary circle radius is undefined + vs.set({ + 'boundary:circle:radius': defaults['boundary:circle:radius'] + }); + } else if (check.number(clean['boundary.circle.radius'])){ + // plain reverse where boundary circle is a valid number + vs.set({ + 'boundary:circle:radius': clean['boundary.circle.radius'] + 'km' + }); + } logStr += '[param:boundary_circle] '; }