From edaf175aa74a8c7b2d8b7fd037d8626eaceb53d9 Mon Sep 17 00:00:00 2001
From: Julian Simioni <julian@simioni.org>
Date: Thu, 17 Sep 2015 15:49:38 -0400
Subject: [PATCH] Provide better, more consistent, errors on invalid ids

All error messages for invalid id formats now use a common format that
explains the nature of the error a bit better.
---
 sanitiser/_ids.js           | 19 +++++++------------
 test/unit/sanitiser/_ids.js | 17 ++++++++++-------
 2 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/sanitiser/_ids.js b/sanitiser/_ids.js
index 08232f81..df3925d2 100644
--- a/sanitiser/_ids.js
+++ b/sanitiser/_ids.js
@@ -12,6 +12,10 @@ function errorMessage(fieldname, message) {
   return message || 'invalid param \''+ fieldname + '\': text length, must be >0';
 }
 
+var formatError = function(input) {
+  return 'id `' + input + 'is invalid: must be of the format type:id for ex: \'geoname:4163334\'';
+};
+
 function sanitize( raw, clean ){
   // error & warning messages
   var messages = { errors: [], warnings: [] };
@@ -45,23 +49,14 @@ function sanitize( raw, clean ){
     var type = rawId.substring(0, param_index );
     var id   = rawId.substring(param_index + 1);
 
-    // basic format/ presence of ':'
-    if(param_index === -1) {
-      messages.errors.push( 'invalid: must be of the format type:id for ex: \'geoname:4163334\'' );
-    }
-    // id text
-    else if( !check.unemptyString( id ) ){
-      messages.errors.push( errorMessage( rawId ) );
-    }
-    // type text
-    else if( !check.unemptyString( type ) ){
-      messages.errors.push( errorMessage( rawId ) );
+    // check id format
+    if(param_index === -1 || !check.unemptyString( id ) || !check.unemptyString( type )) {
+      messages.errors.push( formatError(rawId) );
     }
     // type text must be one of the types
     else if( !_.contains( types, type ) ){
       messages.errors.push( type + ' is invalid. It must be one of these values - [' + types.join(', ') + ']' );
     }
-    // add valid id to 'clean.ids' array
     else {
       return {
         id: id,
diff --git a/test/unit/sanitiser/_ids.js b/test/unit/sanitiser/_ids.js
index 52624f1b..c98556eb 100644
--- a/test/unit/sanitiser/_ids.js
+++ b/test/unit/sanitiser/_ids.js
@@ -6,9 +6,12 @@ var inputs = {
   valid: [ 'geoname:1', 'osmnode:2', 'admin0:53', 'osmway:44', 'geoname:5' ],
   invalid: [ ':', '', '::', 'geoname:', ':234', 'gibberish:23' ]
 };
-var defaultLengthError = function(input) { return 'invalid param \''+ input + '\': text length, must be >0'; },
-  defaultFormatError = 'invalid: must be of the format type:id for ex: \'geoname:4163334\'',
-  defaultError = 'invalid param \'ids\': text length, must be >0',
+
+var formatError = function(input) {
+  return 'id `' + input + 'is invalid: must be of the format type:id for ex: \'geoname:4163334\'';
+};
+
+var defaultError = 'invalid param \'ids\': text length, must be >0',
   defaultMissingTypeError = function(input) {
     var type = input.split(delimiter)[0];
     return type + ' is invalid. It must be one of these values - [' + types.join(', ') + ']';
@@ -34,7 +37,7 @@ module.exports.tests.invalid_ids = function(test, common) {
 
     var messages = sanitize(raw, clean);
 
-    t.equal(messages.errors[0], defaultLengthError(':'), 'format error returned');
+    t.equal(messages.errors[0], formatError(':'), 'format error returned');
     t.equal(clean.ids, undefined, 'ids unset in clean object');
     t.end();
   });
@@ -45,7 +48,7 @@ module.exports.tests.invalid_ids = function(test, common) {
 
     var messages = sanitize(raw, clean);
 
-    t.equal(messages.errors[0], defaultLengthError('::'), 'format error returned');
+    t.equal(messages.errors[0], formatError('::'), 'format error returned');
     t.equal(clean.ids, undefined, 'ids unset in clean object');
     t.end();
   });
@@ -56,7 +59,7 @@ module.exports.tests.invalid_ids = function(test, common) {
 
     var messages = sanitize(raw, clean);
 
-    t.equal(messages.errors[0], defaultLengthError('geoname:'), 'format error returned');
+    t.equal(messages.errors[0], formatError('geoname:'), 'format error returned');
     t.equal(clean.ids, undefined, 'ids unset in clean object');
     t.end();
   });
@@ -67,7 +70,7 @@ module.exports.tests.invalid_ids = function(test, common) {
 
     var messages = sanitize(raw, clean);
 
-    t.equal(messages.errors[0], defaultLengthError(':234'), 'format error returned');
+    t.equal(messages.errors[0], formatError(':234'), 'format error returned');
     t.equal(clean.ids, undefined, 'ids unset in clean object');
     t.end();
   });