From 947797f41e84247e7ee4a1e7c1bc093937ca1a1c Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Tue, 22 Sep 2015 13:28:48 -0400
Subject: [PATCH 1/6] added warning if any of boundary.circle.lat/lon/radius
 are supplied + tests

---
 sanitiser/_geo_common.js            |  3 +-
 sanitiser/_geo_reverse.js           | 11 +++++
 test/unit/run.js                    |  1 +
 test/unit/sanitiser/_geo_reverse.js | 67 +++++++++++++++++++++++++++++
 4 files changed, 80 insertions(+), 2 deletions(-)
 create mode 100644 test/unit/sanitiser/_geo_reverse.js

diff --git a/sanitiser/_geo_common.js b/sanitiser/_geo_common.js
index b88e9453..4e0f8c9b 100644
--- a/sanitiser/_geo_common.js
+++ b/sanitiser/_geo_common.js
@@ -55,7 +55,6 @@ function sanitize_rect( key_prefix, clean, raw, bbox_is_required ) {
  * @param {bool} circle_is_required
  */
 function sanitize_circle( key_prefix, clean, raw, circle_is_required ) {
-
   // the names we use to define the centroid
   var mandatoryProps = [ 'lat', 'lon' ];
 
@@ -86,7 +85,7 @@ function sanitize_circle( key_prefix, clean, raw, circle_is_required ) {
   // radius was specified without lat or lon
   else if( raw.hasOwnProperty( key_prefix + '.radius' ) ){
     var format2 = 'missing circle param \'%s\' requires all of: \'%s\' to be present';
-    throw new Error( util.format( format2, key_prefix, mandatoryProps.join('\',\'') ) ); 
+    throw new Error( util.format( format2, key_prefix, mandatoryProps.join('\',\'') ) );
   }
   // fields required, eg. ( totalFieldsSpecified === 0 && bbox_is_required === true )
   else if( circle_is_required ){
diff --git a/sanitiser/_geo_reverse.js b/sanitiser/_geo_reverse.js
index 9de74fb0..6dc134cc 100644
--- a/sanitiser/_geo_reverse.js
+++ b/sanitiser/_geo_reverse.js
@@ -10,11 +10,22 @@ module.exports = function sanitize( raw, clean ){
   // error & warning messages
   var messages = { errors: [], warnings: [] };
 
+  // helper function to determine if raw has a boundary.circle property
+  var hasBoundaryCircleField = function(field) {
+    return raw.hasOwnProperty('boundary.circle.' + field);
+  };
+
+  if (['lat', 'lon', 'radius'].some(hasBoundaryCircleField)) {
+    messages.warnings.push('boundary.circle is currently unsupported and being ignored');
+  }
+
   try {
     geo_common.sanitize_point( 'point', clean, raw, LAT_LON_IS_REQUIRED );
 
     // this hack is to allow point.lat/point.lon to be used interchanagbly
     // with boundary.circle.lat/boundary.circle.lon
+    //
+    // if clean doesn't have boundary.circle.lat but clean has point.lat, set boundary.circle.lat to point.lat
     if( !clean.hasOwnProperty('boundary.circle.lat') && clean.hasOwnProperty('point.lat') ){
       raw['boundary.circle.lat'] = clean['point.lat'];
     }
diff --git a/test/unit/run.js b/test/unit/run.js
index efa0c4de..5e9aca75 100644
--- a/test/unit/run.js
+++ b/test/unit/run.js
@@ -30,6 +30,7 @@ var tests = [
   require('./middleware/distance'),
   require('./middleware/confidenceScoreReverse'),
   require('./sanitiser/_size'),
+  require('./sanitiser/_geo_reverse'),
 ];
 
 tests.map(function(t) {
diff --git a/test/unit/sanitiser/_geo_reverse.js b/test/unit/sanitiser/_geo_reverse.js
new file mode 100644
index 00000000..74f349d0
--- /dev/null
+++ b/test/unit/sanitiser/_geo_reverse.js
@@ -0,0 +1,67 @@
+var sanitize = require('../../../sanitiser/_geo_reverse');
+
+module.exports.tests = {};
+
+module.exports.tests.sanitize_boundary_country = function(test, common) {
+  test('raw with boundary.circle.lat should add warning about ignored boundary.circle', function(t) {
+    var raw = {
+      'point.lat': '12.121212',
+      'point.lon': '21.212121',
+      'boundary.circle.lat': '13.131313'
+    };
+    var clean = {};
+    var errorsAndWarnings = sanitize(raw, clean);
+
+    t.equals(clean['boundary.circle.lat'], 12.121212, 'should be set to point.lat');
+    t.deepEquals(errorsAndWarnings, {
+      errors: [],
+      warnings: ['boundary.circle is currently unsupported and being ignored']
+    }, 'no warnings/errors');
+    t.end();
+  });
+
+  test('raw with boundary.circle.lon should add warning about ignored boundary.circle', function(t) {
+    var raw = {
+      'point.lat': '12.121212',
+      'point.lon': '21.212121',
+      'boundary.circle.lon': '31.313131'
+    };
+    var clean = {};
+    var errorsAndWarnings = sanitize(raw, clean);
+
+    t.equals(clean['boundary.circle.lon'], 21.212121, 'should be set to point.lon');
+    t.deepEquals(errorsAndWarnings, {
+      errors: [],
+      warnings: ['boundary.circle is currently unsupported and being ignored']
+    }, 'no warnings/errors');
+    t.end();
+  });
+
+  test('raw with boundary.circle.radius should add warning about ignored boundary.circle', function(t) {
+    var raw = {
+      'point.lat': '12.121212',
+      'point.lon': '21.212121',
+      'boundary.circle.radius': '17'
+    };
+    var clean = {};
+    var errorsAndWarnings = sanitize(raw, clean);
+
+    // t.equals(clean['boundary.circle.radius'], 12.121212, 'should be set to point.lat')
+    t.deepEquals(errorsAndWarnings, {
+      errors: [],
+      warnings: ['boundary.circle is currently unsupported and being ignored']
+    }, 'no warnings/errors');
+    t.end();
+  });
+
+};
+
+module.exports.all = function (tape, common) {
+  function test(name, testFunction) {
+    return tape('SANTIZE _geo_reverse ' + name, testFunction);
+  }
+
+  for( var testCase in module.exports.tests ){
+    module.exports.tests[testCase](test, common);
+  }
+};

From 9fdddd3834bbd1d14ced16d237ff7363d2880e84 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Tue, 22 Sep 2015 13:56:54 -0400
Subject: [PATCH 2/6] unrolled not-not conditional to positive conditional

---
 sanitiser/_geo_common.js | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sanitiser/_geo_common.js b/sanitiser/_geo_common.js
index 4e0f8c9b..00112d89 100644
--- a/sanitiser/_geo_common.js
+++ b/sanitiser/_geo_common.js
@@ -2,7 +2,8 @@
  * helper sanitiser methods for geo parameters
  */
 var util = require('util'),
-    check = require('check-types');
+    check = require('check-types'),
+    _ = require('lodash');
 
 /**
  * Parse and validate rect parameter
@@ -55,6 +56,8 @@ function sanitize_rect( key_prefix, clean, raw, bbox_is_required ) {
  * @param {bool} circle_is_required
  */
 function sanitize_circle( key_prefix, clean, raw, circle_is_required ) {
+  // "boundary.circle", clean, raw, true
+
   // the names we use to define the centroid
   var mandatoryProps = [ 'lat', 'lon' ];
 
@@ -144,7 +147,7 @@ function sanitize_point( key_prefix, clean, raw, point_is_required ) {
  */
 function sanitize_coord( key, clean, param, latlon_is_required ) {
   var value = parseFloat( param );
-  if ( !isNaN( value ) ) {
+  if ( _.isFinite( value ) ) {
     clean[key] = value;
   }
   else if (latlon_is_required) {

From 0595a290524d9fbfd720bcc6f7d0703a4fbe9d46 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Tue, 22 Sep 2015 14:11:54 -0400
Subject: [PATCH 3/6] renamed parameters for readability

---
 sanitiser/_geo_common.js | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/sanitiser/_geo_common.js b/sanitiser/_geo_common.js
index 00112d89..c74db609 100644
--- a/sanitiser/_geo_common.js
+++ b/sanitiser/_geo_common.js
@@ -56,8 +56,6 @@ function sanitize_rect( key_prefix, clean, raw, bbox_is_required ) {
  * @param {bool} circle_is_required
  */
 function sanitize_circle( key_prefix, clean, raw, circle_is_required ) {
-  // "boundary.circle", clean, raw, true
-
   // the names we use to define the centroid
   var mandatoryProps = [ 'lat', 'lon' ];
 
@@ -142,13 +140,13 @@ function sanitize_point( key_prefix, clean, raw, point_is_required ) {
  *
  * @param {string} key
  * @param {object} clean
- * @param {string} param
+ * @param {string} rawValue
  * @param {bool} latlon_is_required
  */
-function sanitize_coord( key, clean, param, latlon_is_required ) {
-  var value = parseFloat( param );
-  if ( _.isFinite( value ) ) {
-    clean[key] = value;
+function sanitize_coord( key, clean, rawValue, latlon_is_required ) {
+  var parsedValue = parseFloat( rawValue );
+  if ( _.isFinite( parsedValue ) ) {
+    clean[key] = parsedValue;
   }
   else if (latlon_is_required) {
     throw new Error( util.format( 'missing param \'%s\'', key ) );

From 0f6d08c0ecff8bc963905c7afa103df44a2dcb33 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Tue, 22 Sep 2015 14:51:13 -0400
Subject: [PATCH 4/6] set boundary.circle.radius to default if caller did not
 supply it

---
 sanitiser/_geo_common.js            |  2 ++
 sanitiser/_geo_reverse.js           | 21 +++++++------
 test/unit/sanitiser/_geo_reverse.js | 47 +++++++++++++++++++++++++++++
 test/unit/sanitiser/reverse.js      |  2 ++
 4 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/sanitiser/_geo_common.js b/sanitiser/_geo_common.js
index c74db609..153128bc 100644
--- a/sanitiser/_geo_common.js
+++ b/sanitiser/_geo_common.js
@@ -56,6 +56,8 @@ function sanitize_rect( key_prefix, clean, raw, bbox_is_required ) {
  * @param {bool} circle_is_required
  */
 function sanitize_circle( key_prefix, clean, raw, circle_is_required ) {
+  // "boundary.circle", clean, raw, false
+
   // the names we use to define the centroid
   var mandatoryProps = [ 'lat', 'lon' ];
 
diff --git a/sanitiser/_geo_reverse.js b/sanitiser/_geo_reverse.js
index 6dc134cc..bac20b29 100644
--- a/sanitiser/_geo_reverse.js
+++ b/sanitiser/_geo_reverse.js
@@ -1,5 +1,7 @@
 
 var geo_common = require ('./_geo_common');
+var check = require('check-types');
+var defaults = require('../query/defaults');
 var LAT_LON_IS_REQUIRED = true,
     CIRCLE_IS_REQUIRED = false,
     CIRCLE_MUST_BE_COMPLETE = false;
@@ -20,20 +22,21 @@ module.exports = function sanitize( raw, clean ){
   }
 
   try {
+    // first verify that point.lat/point.lon are valid
     geo_common.sanitize_point( 'point', clean, raw, LAT_LON_IS_REQUIRED );
 
-    // this hack is to allow point.lat/point.lon to be used interchanagbly
-    // with boundary.circle.lat/boundary.circle.lon
-    //
-    // if clean doesn't have boundary.circle.lat but clean has point.lat, set boundary.circle.lat to point.lat
-    if( !clean.hasOwnProperty('boundary.circle.lat') && clean.hasOwnProperty('point.lat') ){
-      raw['boundary.circle.lat'] = clean['point.lat'];
-    }
-    if( !clean.hasOwnProperty('boundary.circle.lon') && clean.hasOwnProperty('point.lon') ){
-      raw['boundary.circle.lon'] = clean['point.lon'];
+    // overwrite boundary.circle.lat/lon with point.lat/lon
+    raw['boundary.circle.lat'] = clean['point.lat'];
+    raw['boundary.circle.lon'] = clean['point.lon'];
+
+    // if no radius was passed, set the default
+    if (!check.assigned(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 );
+
   }
   catch (err) {
     messages.errors.push( err.message );
diff --git a/test/unit/sanitiser/_geo_reverse.js b/test/unit/sanitiser/_geo_reverse.js
index 74f349d0..c7fb778a 100644
--- a/test/unit/sanitiser/_geo_reverse.js
+++ b/test/unit/sanitiser/_geo_reverse.js
@@ -1,4 +1,5 @@
 var sanitize = require('../../../sanitiser/_geo_reverse');
+var defaults = require('../../../query/defaults');
 
 module.exports.tests = {};
 
@@ -54,6 +55,52 @@ module.exports.tests.sanitize_boundary_country = function(test, common) {
     t.end();
   });
 
+  test('boundary.circle.lat/lon should be overridden with point.lat/lon', function(t) {
+    var raw = {
+      'point.lat': '12.121212',
+      'point.lon': '21.212121',
+      'boundary.circle.lat': '13.131313',
+      'boundary.circle.lon': '31.313131'
+    };
+    var clean = {};
+    var errorsAndWarnings = sanitize(raw, clean);
+
+    t.equals(raw['boundary.circle.lat'], 12.121212, 'should be set to point.lat');
+    t.equals(raw['boundary.circle.lon'], 21.212121, 'should be set to point.lon');
+    t.equals(clean['boundary.circle.lat'], 12.121212, 'should be set to point.lat');
+    t.equals(clean['boundary.circle.lon'], 21.212121, 'should be set to point.lon');
+    t.end();
+  });
+
+  test('no boundary.circle.radius supplied should be set to default', function(t) {
+    var raw = {
+      'point.lat': '12.121212',
+      'point.lon': '21.212121'
+    };
+    var clean = {};
+    var errorsAndWarnings = sanitize(raw, clean);
+
+    t.equals(raw['boundary.circle.radius'], defaults['boundary:circle:radius'], 'should be from defaults');
+    t.equals(clean['boundary.circle.radius'], parseFloat(defaults['boundary:circle:radius']), 'should be same as raw');
+    t.end();
+
+  });
+
+  test('explicit boundary.circle.radius should be used instead of default', function(t) {
+    var raw = {
+      'point.lat': '12.121212',
+      'point.lon': '21.212121',
+      'boundary.circle.radius': '3248732857km' // this will never be the default
+    };
+    var clean = {};
+    var errorsAndWarnings = sanitize(raw, clean);
+
+    t.equals(raw['boundary.circle.radius'], '3248732857km', 'should be parsed float');
+    t.equals(clean['boundary.circle.radius'], 3248732857.0, 'should be copied from raw');
+    t.end();
+
+  });
+
 };
 
 module.exports.all = function (tape, common) {
diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js
index 759aef28..85943c73 100644
--- a/test/unit/sanitiser/reverse.js
+++ b/test/unit/sanitiser/reverse.js
@@ -4,11 +4,13 @@
 var reverse  = require('../../../sanitiser/reverse'),
     sanitize = reverse.sanitize,
     middleware = reverse.middleware,
+    defaults = require('../../../query/defaults'),
     defaultError = 'missing param \'lat\'',
     defaultClean =  { 'point.lat': 0,
                       'point.lon': 0,
                       'boundary.circle.lat': 0,
                       'boundary.circle.lon': 0,
+                      'boundary.circle.radius': parseFloat(defaults['boundary:circle:radius']),
                       types: {
                       },
                       size: 10,

From 5da4e4b36bdc8dbbb8870b2de5327da38c5ec4fa Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Tue, 22 Sep 2015 14:52:54 -0400
Subject: [PATCH 5/6] removed unused variable

---
 sanitiser/_geo_reverse.js | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sanitiser/_geo_reverse.js b/sanitiser/_geo_reverse.js
index bac20b29..a930c66b 100644
--- a/sanitiser/_geo_reverse.js
+++ b/sanitiser/_geo_reverse.js
@@ -3,8 +3,7 @@ var geo_common = require ('./_geo_common');
 var check = require('check-types');
 var defaults = require('../query/defaults');
 var LAT_LON_IS_REQUIRED = true,
-    CIRCLE_IS_REQUIRED = false,
-    CIRCLE_MUST_BE_COMPLETE = false;
+    CIRCLE_IS_REQUIRED = false;
 
 // validate inputs, convert types and apply defaults
 module.exports = function sanitize( raw, clean ){

From 56d8600218546b5ab13d399d1a7b49cfbfe63aaf Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Tue, 22 Sep 2015 14:54:10 -0400
Subject: [PATCH 6/6] modified warning message for brevity

---
 sanitiser/_geo_reverse.js           | 2 +-
 test/unit/sanitiser/_geo_reverse.js | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/sanitiser/_geo_reverse.js b/sanitiser/_geo_reverse.js
index a930c66b..fb77add3 100644
--- a/sanitiser/_geo_reverse.js
+++ b/sanitiser/_geo_reverse.js
@@ -17,7 +17,7 @@ module.exports = function sanitize( raw, clean ){
   };
 
   if (['lat', 'lon', 'radius'].some(hasBoundaryCircleField)) {
-    messages.warnings.push('boundary.circle is currently unsupported and being ignored');
+    messages.warnings.push('boundary.circle is currently unsupported');
   }
 
   try {
diff --git a/test/unit/sanitiser/_geo_reverse.js b/test/unit/sanitiser/_geo_reverse.js
index c7fb778a..e03e7ddd 100644
--- a/test/unit/sanitiser/_geo_reverse.js
+++ b/test/unit/sanitiser/_geo_reverse.js
@@ -16,7 +16,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) {
     t.equals(clean['boundary.circle.lat'], 12.121212, 'should be set to point.lat');
     t.deepEquals(errorsAndWarnings, {
       errors: [],
-      warnings: ['boundary.circle is currently unsupported and being ignored']
+      warnings: ['boundary.circle is currently unsupported']
     }, 'no warnings/errors');
     t.end();
   });
@@ -33,7 +33,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) {
     t.equals(clean['boundary.circle.lon'], 21.212121, 'should be set to point.lon');
     t.deepEquals(errorsAndWarnings, {
       errors: [],
-      warnings: ['boundary.circle is currently unsupported and being ignored']
+      warnings: ['boundary.circle is currently unsupported']
     }, 'no warnings/errors');
     t.end();
   });
@@ -50,7 +50,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) {
     // t.equals(clean['boundary.circle.radius'], 12.121212, 'should be set to point.lat')
     t.deepEquals(errorsAndWarnings, {
       errors: [],
-      warnings: ['boundary.circle is currently unsupported and being ignored']
+      warnings: ['boundary.circle is currently unsupported']
     }, 'no warnings/errors');
     t.end();
   });