Merge branch 'parameter_groups'

sanitiser/_geo_common.js

@@ -1,7 +1,8 @@
 /**
  * helper sanitiser methods for geo parameters
  */
-var util = require('util'),
+var groups = require('./_groups'),
+    util = require('util'),
     check = require('check-types'),
     _ = require('lodash');
 
@@ -14,37 +15,29 @@ var util = require('util'),
  * @param {bool} bbox_is_required
  */
 function sanitize_rect( key_prefix, clean, raw, bbox_is_required ) {
-
+  // calculate full property names from the key_prefix
-  // the names we use to define the corners of the rect
+  var properties = [ 'min_lat', 'max_lat', 'min_lon', 'max_lon' ].map(function(prop) {
-  var mandatoryProps = [ 'min_lat', 'max_lat', 'min_lon', 'max_lon' ];
+    return key_prefix + '.' + prop;
-
-  // count up how many fields the user actually specified
-  var totalFieldsSpecified = 0;
-  mandatoryProps.forEach( function( prop ){
-    if( raw.hasOwnProperty( key_prefix + '.' + prop ) ){
-      totalFieldsSpecified++;
-    }
   });
 
-  // all fields specified
+  // sanitize the rect property group, this throws an exception if
-  if( 4 === totalFieldsSpecified ) {
+  // the group is not complete
-    // reuse the coord sanitizer and set required:true so we get a fatal error if
+  var bbox_present;
-    // any one of the corners is not specified.
+  if (bbox_is_required) {
-    sanitize_coord( key_prefix + '.min_lat', clean, raw[ key_prefix + '.min_lat' ], true );
+    bbox_present = groups.required(raw, properties);
-    sanitize_coord( key_prefix + '.max_lat', clean, raw[ key_prefix + '.max_lat' ], true );
+  } else {
-    sanitize_coord( key_prefix + '.min_lon', clean, raw[ key_prefix + '.min_lon' ], true );
+    bbox_present = groups.optional(raw, properties);
-    sanitize_coord( key_prefix + '.max_lon', clean, raw[ key_prefix + '.max_lon' ], true );
-  }
-  // fields only partially specified
-  else if( totalFieldsSpecified > 0 ){
-    var format1 = 'missing rect param \'%s\' requires all of: \'%s\' to be present';
-    throw new Error( util.format( format1, key_prefix, mandatoryProps.join('\',\'') ) );
-  }
-  // fields required, eg. ( totalFieldsSpecified === 0 && bbox_is_required === true )
-  else if( bbox_is_required ){
-    var format2 = 'missing rect param \'%s\' requires all of: \'%s\' to be present';
-    throw new Error( util.format( format2, key_prefix, mandatoryProps.join('\',\'') ) );
   }
+
+  // don't bother checking individual elements if bbox is not required
+  // and not present
+  if (!bbox_present) { return; }
+
+  // check each property individually. now that it is known a bbox is present,
+  // all properties must exist, so pass the true flag for coord_is_required
+  properties.forEach(function(prop) {
+    sanitize_coord(prop, clean, raw[prop], true);
+  });
 }
 
 /**
@@ -56,43 +49,13 @@ 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 ) {
-
+  // sanitize both a point and a radius if radius is present
-  // the names we use to define the centroid
+  // otherwise just sanittize the point
-  var mandatoryProps = [ 'lat', 'lon' ];
-
-  // count up how many fields the user actually specified
-  var totalFieldsSpecified = 0;
-  mandatoryProps.forEach( function( prop ){
-    if( raw.hasOwnProperty( key_prefix + '.' + prop ) ){
-      totalFieldsSpecified++;
-    }
-  });
-
-  // all fields specified
-  if( 2 === totalFieldsSpecified ) {
-    // reuse the coord sanitizer and set required:true so we get a fatal error if
-    // any one of the coords is not specified.
-    sanitize_coord( key_prefix + '.lat', clean, raw[ key_prefix + '.lat' ], true );
-    sanitize_coord( key_prefix + '.lon', clean, raw[ key_prefix + '.lon' ], true );
-
   if( check.assigned( raw[ key_prefix + '.radius' ] ) ){
     sanitize_coord( key_prefix + '.radius', clean, raw[ key_prefix + '.radius' ], true );
-    }
+    sanitize_point( key_prefix, clean, raw, true);
-  }
+  } else {
-  // fields only partially specified
+    sanitize_point( key_prefix, clean, raw, circle_is_required);
-  else if( totalFieldsSpecified > 0 ){
-    var format1 = 'missing circle param \'%s\' requires all of: \'%s\' to be present';
-    throw new Error( util.format( format1, key_prefix, mandatoryProps.join('\',\'') ) );
-  }
-  // 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('\',\'') ) );
-  }
-  // fields required, eg. ( totalFieldsSpecified === 0 && bbox_is_required === true )
-  else if( circle_is_required ){
-    var format3 = 'missing circle param \'%s\' requires all of: \'%s\' to be present';
-    throw new Error( util.format( format3, key_prefix, mandatoryProps.join('\',\'') ) );
   }
 }
 
@@ -105,35 +68,29 @@ function sanitize_circle( key_prefix, clean, raw, circle_is_required ) {
  * @param {bool} point_is_required
  */
 function sanitize_point( key_prefix, clean, raw, point_is_required ) {
-
+  // calculate full property names from the key_prefix
-  // the names we use to define the point
+  var properties = [ 'lat', 'lon'].map(function(prop) {
-  var mandatoryProps = [ 'lat', 'lon' ];
+    return key_prefix + '.' + prop;
-
-  // count up how many fields the user actually specified
-  var totalFieldsSpecified = 0;
-  mandatoryProps.forEach( function( prop ){
-    if( raw.hasOwnProperty( key_prefix + '.' + prop ) ){
-      totalFieldsSpecified++;
-    }
   });
 
-  // all fields specified
+  // sanitize the rect property group, this throws an exception if
-  if( 2 === totalFieldsSpecified ) {
+  // the group is not complete
-    // reuse the coord sanitizer and set required:true so we get a fatal error if
+  var point_present;
-    // any one of the coords is not specified.
+  if (point_is_required) {
-    sanitize_coord( key_prefix + '.lat', clean, raw[ key_prefix + '.lat' ], true );
+    point_present = groups.required(raw, properties);
-    sanitize_coord( key_prefix + '.lon', clean, raw[ key_prefix + '.lon' ], true );
+  } else {
-  }
+    point_present = groups.optional(raw, properties);
-  // fields only partially specified
-  else if( totalFieldsSpecified > 0 ){
-    var format1 = 'missing point param \'%s\' requires all of: \'%s\' to be present';
-    throw new Error( util.format( format1, key_prefix, mandatoryProps.join('\',\'') ) );
-  }
-  // fields required, eg. ( totalFieldsSpecified === 0 && bbox_is_required === true )
-  else if( point_is_required ){
-    var format2 = 'missing point param \'%s\' requires all of: \'%s\' to be present';
-    throw new Error( util.format( format2, key_prefix, mandatoryProps.join('\',\'') ) );
   }
+
+  // don't bother checking individual elements if point is not required
+  // and not present
+  if (!point_present) { return; }
+
+  // check each property individually. now that it is known a bbox is present,
+  // all properties must exist, so pass the true flag for coord_is_required
+  properties.forEach(function(prop) {
+    sanitize_coord(prop, clean, raw[prop], true);
+  });
 }
 
 /**

sanitiser/_groups.js

@@ -0,0 +1,57 @@
+var _ = require('lodash');
+
+/*
+ * Specify an object and array of keys to check.
+ * An error will be thrown if at least one, but not all of them are specified
+ *
+ * @param {object} - the object
+ * @param {array} - keys to check
+ *
+ * returns true if all are present, false if none are present, throws an exception otherwise
+ */
+function optional_group(object, keys) {
+  var contained_in_object = _.contains.bind(null, Object.keys(object));
+
+  if (keys.every(contained_in_object)) {
+    return true;
+  } else if (!keys.some(contained_in_object)) {
+    return false;
+  } else {
+    throw new Error(error_message(keys));
+  }
+}
+
+/*
+ * Specify an object and array of keys to check.
+ * An error will be thrown if any of the keys are missing from the object
+ */
+function required_group(object, keys) {
+  var contained_in_object = _.contains.bind(null, Object.keys(object));
+
+  if (keys.every(contained_in_object)) {
+    return true;
+  } else {
+    throw new Error(error_message(keys));
+  }
+}
+
+/*
+ * Create a friendly error message from a list of keys
+ */
+function error_message(keys) {
+  var start = 'parameters ';
+
+  var listStart = keys.slice(0, -1).join(', ');
+  var listEnd = ' and ' + keys[keys.length - 1];
+
+  var adjective = (keys.length > 2) ? 'all' : 'both';
+
+  var end = ' must ' + adjective + ' be specified';
+
+  return start + listStart + listEnd + end;
+}
+
+module.exports = {
+  optional: optional_group,
+  required: required_group
+};

test/unit/run.js

@@ -6,34 +6,35 @@ var tests = [
   require('./controller/index'),
   require('./controller/place'),
   require('./controller/search'),
-  require('./service/mget'),
-  require('./service/search'),
-  require('./sanitiser/_ids'),
-  require('./sanitiser/_flag_bool'),
-  require('./sanitiser/autocomplete'),
-  require('./sanitiser/_sources'),
-  require('./sanitiser/_boundary_country'),
-  require('./sanitiser/search'),
-  require('./sanitiser/_layers'),
-  require('./sanitiser/reverse'),
-  require('./sanitiser/place'),
-  require('./query/search'),
-  require('./query/autocomplete'),
-  require('./query/reverse'),
-  require('./query/defaults'),
-  require('./helper/query_parser'),
   require('./helper/geojsonify'),
-  require('./helper/labelSchema'),
   require('./helper/labelGenerator'),
-  require('./helper/types'),
+  require('./helper/labelSchema'),
+  require('./helper/query_parser'),
   require('./helper/type_mapping'),
-  require('./sanitiser/_geo_common'),
+  require('./helper/types'),
-  require('./middleware/distance'),
-  require('./middleware/confidenceScoreReverse'),
   require('./middleware/confidenceScore'),
-  require('./sanitiser/_size'),
+  require('./middleware/confidenceScoreReverse'),
-  require('./sanitiser/_single_scalar_parameters'),
+  require('./middleware/distance'),
+  require('./query/autocomplete'),
+  require('./query/defaults'),
+  require('./query/reverse'),
+  require('./query/search'),
+  require('./sanitiser/_boundary_country'),
+  require('./sanitiser/_flag_bool'),
+  require('./sanitiser/_geo_common'),
   require('./sanitiser/_geo_reverse'),
+  require('./sanitiser/_groups'),
+  require('./sanitiser/_ids'),
+  require('./sanitiser/_layers'),
+  require('./sanitiser/_single_scalar_parameters'),
+  require('./sanitiser/_size'),
+  require('./sanitiser/_sources'),
+  require('./sanitiser/autocomplete'),
+  require('./sanitiser/place'),
+  require('./sanitiser/reverse'),
+  require('./sanitiser/search'),
+  require('./service/mget'),
+  require('./service/search'),
 ];
 
 tests.map(function(t) {

test/unit/sanitiser/_groups.js

@@ -0,0 +1,75 @@
+var groups = require('../../../sanitiser/_groups');
+
+module.exports.tests = {};
+
+module.exports.tests.optional_group = function(test, common) {
+  test('optional group none present ', function(t) {
+    var object = {};
+    t.doesNotThrow(function() {
+      var present = groups.optional(object, [ 'a', 'b' ]);
+      t.equal(present, false, 'group reported not present');
+    });
+    t.end();
+  });
+
+  test('optional group all present ', function(t) {
+    var object = { 'a': 5, 'b': 9 };
+    t.doesNotThrow(function() {
+      var present = groups.optional(object, [ 'a', 'b' ]);
+      t.equal(present, true, 'group reported present');
+    });
+    t.end();
+  });
+
+  test('optional group some present ', function(t) {
+    var object = { 'b': 9 };
+    t.throws(function() {
+      groups.optional(object, [ 'a', 'b' ]);
+    }, new RegExp('parameters a and b must both be specified'));
+    t.end();
+  });
+
+  test('optional group some present (larger group) ', function(t) {
+    var object = { 'b': 9, 'd': 5 };
+    t.throws(function() {
+      groups.optional(object, [ 'a', 'b', 'c', 'd', 'e' ]);
+    }, new RegExp('parameters a, b, c, d and e must all be specified'));
+    t.end();
+  });
+};
+
+module.exports.tests.required_group = function(test, common) {
+  test('required group none present ', function(t) {
+    var object = {};
+    t.throws(function() {
+      groups.required(object, [ 'a', 'b' ]);
+    }, new RegExp('parameters a and b must both be specified'));
+    t.end();
+  });
+
+  test('required group all present ', function(t) {
+    var object = { 'a': 5, 'b': 9 };
+    t.doesNotThrow(function() {
+      groups.required(object, [ 'a', 'b' ]);
+    });
+    t.end();
+  });
+
+  test('required group some present ', function(t) {
+    var object = { 'b': 9 };
+    t.throws(function() {
+      groups.required(object, [ 'a', 'b' ]);
+    }, new RegExp('parameters a and b must both be specified'));
+    t.end();
+  });
+};
+
+module.exports.all = function (tape, common) {
+  function test(name, testFunction) {
+    return tape('SANTIZE _groups ' + name, testFunction);
+  }
+
+  for( var testCase in module.exports.tests ){
+    module.exports.tests[testCase](test, common);
+  }
+};

test/unit/sanitiser/search.js

@@ -167,7 +167,7 @@ module.exports.tests.sanitize_optional_geo = function(test, common) {
     var req = { query: { text: 'test', 'focus.point.lon': 0 } };
     sanitize(req, function(){
       var expected_lon = 0;
-      t.equal(req.errors[0], 'missing point param \'focus.point\' requires all of: \'lat\',\'lon\' to be present');
+      t.equal(req.errors[0], 'parameters focus.point.lat and focus.point.lon must both be specified');
       t.equal(req.clean['focus.point.lat'], undefined);
       t.equal(req.clean['focus.point.lon'], undefined);
     });
@@ -177,7 +177,7 @@ module.exports.tests.sanitize_optional_geo = function(test, common) {
     var req = { query: { text: 'test', 'focus.point.lat': 0 } };
     sanitize(req, function(){
       var expected_lat = 0;
-      t.equal(req.errors[0], 'missing point param \'focus.point\' requires all of: \'lat\',\'lon\' to be present');
+      t.equal(req.errors[0], 'parameters focus.point.lat and focus.point.lon must both be specified');
       t.equal(req.clean['focus.point.lat'], undefined);
       t.equal(req.clean['focus.point.lon'], undefined);
     });