simplifying things, DRY - one endpoint 'GET' handles single/multiple requests. plus test coverage

app.js

@@ -12,7 +12,6 @@ app.use( require('./middleware/jsonp') );
 
 var sanitisers = {};
 sanitisers.get      = require('./sanitiser/get');
-sanitisers.mget      = require('./sanitiser/mget');
 sanitisers.suggest  = require('./sanitiser/suggest');
 sanitisers.search   = sanitisers.suggest;
 sanitisers.reverse  = require('./sanitiser/reverse');
@@ -22,7 +21,6 @@ sanitisers.reverse  = require('./sanitiser/reverse');
 var controllers     = {};
 controllers.index   = require('./controller/index');
 controllers.get     = require('./controller/get');
-controllers.mget     = require('./controller/mget');
 controllers.suggest = require('./controller/suggest');
 controllers.search  = require('./controller/search');
 
@@ -33,7 +31,6 @@ app.get( '/', controllers.index() );
 
 // get doc API
 app.get( '/get', sanitisers.get.middleware, controllers.get() );
-app.get( '/mget', sanitisers.mget.middleware, controllers.mget() );
 
 // suggest API
 app.get( '/suggest', sanitisers.suggest.middleware, controllers.suggest() );

controller/get.js

@@ -5,25 +5,36 @@ function setup( backend ){
 
   // allow overriding of dependencies
   backend = backend || require('../src/backend');
-
+  backend = new backend(); 
+  
   function controller( req, res, next ){
     
+    var docs = req.clean.ids.map( function(id) {
+      return {
+        _index: 'pelias',
+        _type: id.type,
+        _id: id.id
+      };
+    });
+
     // backend command
     var cmd = {
-      index: 'pelias',
+      body: {
-      type: req.clean.type,
+        docs: docs
-      id: req.clean.id
+      }
     };
     
-    // query backend
+    // query new backend
-    backend().client.get( cmd, function( err, data ){
+    backend.client.mget( cmd, function( err, data ){
-
+      
       var docs = [];
       // handle backend errors
       if( err ){ return next( err ); }
 
-      if( data && data.found && data._source ){
+      if( data && data.docs && Array.isArray(data.docs) && data.docs.length ){
-        docs.push(data._source);
+        docs = data.docs.map( function( doc ){
+          return doc._source;
+        });
       }
 
       // convert docs to geojson

controller/mget.js

@@ -1,55 +0,0 @@
-
-var geojsonify = require('../helper/geojsonify').search;
-
-function setup( backend ){
-
-  // allow overriding of dependencies
-  backend = backend || require('../src/backend');
-
-  function controller( req, res, next ){
-    
-    // backend command
-    var cmd = req.clean.ids.map( function(id) {
-      return {
-        index: 'pelias',
-        type: id.type,
-        id: id.id
-      };
-    });
-    cmd = {
-      index: 'pelias',
-      type: 'geoname',
-      ids: cmd.map(function(c){ return c.id })
-    }
-    console.log('cmd:')
-    console.log(cmd)
-    // query backend
-    backend().client.mget( cmd, function( err, data ){
-      console.log('error:')
-      console.log(err)
-      var docs = [];
-      // handle backend errors
-      if( err ){ return next( err ); }
-
-      if( data && data.docs && Array.isArray(data.docs) && data.docs.length ){
-        docs = data.docs.map( function( doc ){
-          return doc._source;
-        });
-      }
-
-      // convert docs to geojson
-      var geojson = geojsonify( docs );
-
-      // response envelope
-      geojson.date = new Date().getTime();
-
-      // respond
-      return res.status(200).json( geojson );
-    });
-
-  }
-
-  return controller;
-}
-
-module.exports = setup;

sanitiser/_id.js

@@ -1,5 +1,5 @@
 // validate inputs, convert types and apply defaults
-// id generally looks like 'geoname/4163334' (type/id)
+// id generally looks like 'geoname:4163334' (type:id)
 // so, both type and id are required fields.
 
 function sanitize( req ){
@@ -7,6 +7,7 @@ function sanitize( req ){
   req.clean   = req.clean || {};
   var params  = req.query;
   var indeces = require('../query/indeces');
+  var delim   = ':';
 
   // ensure params is a valid object
   if( Object.prototype.toString.call( params ) !== '[object Object]' ){
@@ -20,38 +21,45 @@ function sanitize( req ){
     }
   };
 
-  // id text
+  if(('string' === typeof params.id && !params.id.length) || params.id === undefined){
-  if('string' !== typeof params.id || !params.id.length){
     return errormessage('id');
   }
 
-  // id format
+  if( params && params.id && params.id.length ){
-  if(params.id.indexOf('/') === -1) {
+    req.clean.ids = [];
-    return errormessage('id', 'invalid: must be of the format type/id for ex: \'geoname/4163334\'');
+    params.ids = Array.isArray(params.id) ? params.id : [params.id];
-  }
+    
-  req.clean.id = params.id;
+    for (var i=0; i<params.ids.length; i++) {
+      var thisparam = params.ids[i];
+    
+      // basic format/ presence of ':'
+      if(thisparam.indexOf(delim) === -1) {
+        return errormessage(null, 'invalid: must be of the format type:id for ex: \'geoname:4163334\'');
+      }
 
-  var param = params.id.split('/');
+      var param = thisparam.split(delim);
-  var param_type  = param[0];
+      var type= param[0];
-  var param_id    = param[1];
+      var id  = param[1];
 
-  // id text
+      // id text
-  if('string' !== typeof param_id || !param_id.length){
+      if('string' !== typeof id || !id.length){
-    return errormessage('id');
+        return errormessage(thisparam);
-  }
+      }
-
+      // type text
-  // type text
+      if('string' !== typeof type || !type.length){
-  if('string' !== typeof param_type || !param_type.length){
+        return errormessage(thisparam);
-    return errormessage('type');
+      }
-  }
+      // type text must be one of the indeces
-
+      if(indeces.indexOf(type) == -1){
-  // type text must be one of the indeces
+        return errormessage('type', type + ' is invalid. It must be one of these values - [' + indeces.join(", ") + ']');
-  if(indeces.indexOf(param_type) == -1){
+      }
-    return errormessage('type', 'type must be one of these values - [' + indeces.join(", ") + ']');
+      req.clean.ids.push({
+        id: id,
+        type: type
+      });
+    }
   }
-  req.clean.id   = param_id;
+  
-  req.clean.type = param_type;
-
   return { 'error': false };
 
 }

sanitiser/_ids.js

@@ -1,56 +0,0 @@
-// validate inputs, convert types and apply defaults
-// id generally looks like 'geoname/4163334' (type/id)
-// so, both type and id are required fields.
-
-function sanitize( req ){
-  
-  req.clean   = req.clean || {};
-  var params  = req.query;
-  var indeces = require('../query/indeces');
-
-  // ensure params is a valid object
-  if( Object.prototype.toString.call( params ) !== '[object Object]' ){
-    params = {};
-  }
-  console.log(params)
-
-  var errormessage = function(fieldname, message) {
-    return {
-      'error': true,
-      'message': message || ('invalid param \''+ fieldname + '\': text length, must be >0')
-    }
-  };
-
-  if( params && params.ids && params.ids.length ){
-    req.clean.ids = [];
-    console.log(params.ids)
-    params.ids.split(',').forEach( function(param) {
-      param   = param.split('/');
-      var type= param[0];
-      var id  = param[1];
-      console.log(param)
-      // id text
-      if('string' !== typeof id || !id.length){
-        return errormessage('id');
-      }
-      // type text
-      if('string' !== typeof type || !type.length){
-        return errormessage('type');
-      }
-      // type text must be one of the indeces
-      if(indeces.indexOf(type) == -1){
-        return errormessage('type', 'type must be one of these values - [' + indeces.join(", ") + ']');
-      }
-      req.clean.ids.push({
-        id: id,
-        type: type
-      });
-    });
-  }
-  console.log(req.clean)
-  return { 'error': false };
-
-}
-
-// export function
-module.exports = sanitize;

sanitiser/mget.js

@@ -1,23 +0,0 @@
-
-var logger = require('../src/logger'),
-    _sanitize = require('../sanitiser/_sanitize'),
-    sanitizers = {
-      id: require('../sanitiser/_ids')
-    };
-
-var sanitize = function(req, cb) { _sanitize(req, sanitizers, cb); }
-
-// export sanitize for testing
-module.exports.sanitize = sanitize;
-
-// middleware
-module.exports.middleware = function( req, res, next ){
-  sanitize( req, function( err, clean ){
-    if( err ){
-      res.status(400); // 400 Bad Request
-      return next(err);
-    }
-    req.clean = clean;
-    next();
-  });
-};

test/unit/sanitiser/get.js

@@ -3,13 +3,19 @@ var get  = require('../../../sanitiser/get'),
     _sanitize = get.sanitize,
     middleware = get.middleware,
     indeces = require('../../../query/indeces'),
-    defaultIdError = 'invalid param \'id\': text length, must be >0',
+    delimiter = ':',
-    defaultTypeError = 'invalid param \'type\': text length, must be >0',
+    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\'',
+    defaultFormatError = 'invalid: must be of the format type:id for ex: \'geoname:4163334\'',
-    defaultError = defaultIdError,
+    defaultError = 'invalid param \'id\': text length, must be >0',
-    defaultMissingTypeError = 'type must be one of these values - [' + indeces.join(", ") + ']',
+    defaultMissingTypeError = function(input) { 
-    defaultClean = { id: '123', type: 'geoname' },
+      var type = input.split(delimiter)[0];
-    sanitize = function(query, cb) { _sanitize({'query':query}, cb); }
+      return type + ' is invalid. It must be one of these values - [' + indeces.join(", ") + ']'},
+    defaultClean = { ids: [ { id: '123', type: 'geoname' } ] },
+    sanitize = function(query, cb) { _sanitize({'query':query}, cb); },
+    inputs = {
+      valid: [ 'geoname:1', 'osmnode:2', 'admin0:53', 'osmway:44', 'geoname:5' ],
+      invalid: [ ':', '', '::', 'geoname:', ':234', 'gibberish:23' ]
+    };
 
 module.exports.tests = {};
 
@@ -26,37 +32,19 @@ module.exports.tests.interface = function(test, common) {
   });
 };
 
-module.exports.tests.sanitize_id_and_type = function(test, common) {
+module.exports.tests.sanitize_id = function(test, common) {
-  var inputs = {
-    valid: [
-      'geoname/1',
-      'osmnode/2',
-      'admin0/53',
-      'osmway/44',
-      'geoname/5'
-    ],
-    invalid: [
-      '/',
-      '',
-      '//',
-      'geoname/',
-      '/234',
-      'gibberish/23'
-    ]
-  };
-
   test('invalid input', function(t) {
     inputs.invalid.forEach( function( input ){
       sanitize({ id: input }, function( err, clean ){
         switch (err) {
-          case defaultIdError:
+          case defaultError:
-            t.equal(err, defaultIdError, input + ' is invalid (missing id)'); break;
+            t.equal(err, defaultError, input + ' is invalid input'); break;
-          case defaultTypeError:
+          case defaultLengthError(input):
-            t.equal(err, defaultTypeError, input + ' is invalid (missing type)'); break;
+            t.equal(err, defaultLengthError(input), input + ' is invalid (missing id/type)'); break;
           case defaultFormatError:
             t.equal(err, defaultFormatError, input + ' is invalid (invalid format)'); break;
-          case defaultMissingTypeError:
+          case defaultMissingTypeError(input):
-            t.equal(err, defaultMissingTypeError, input + ' is an unknown type'); break;
+            t.equal(err, defaultMissingTypeError(input), input + ' is an unknown type'); break;
           default: break;
         }
         t.equal(clean, undefined, 'clean not set');
@@ -67,8 +55,8 @@ module.exports.tests.sanitize_id_and_type = function(test, common) {
 
   test('valid input', function(t) {
     inputs.valid.forEach( function( input ){
-      var input_parts = input.split('/');
+      var input_parts = input.split(delimiter);
-      var expected = { id: input_parts[1], type: input_parts[0] };
+      var expected = { ids: [ { id: input_parts[1], type: input_parts[0] } ] };
       sanitize({ id: input }, function( err, clean ){
         t.equal(err, undefined, 'no error (' + input + ')' );
         t.deepEqual(clean, expected, 'clean set correctly (' + input + ')');
@@ -78,6 +66,42 @@ module.exports.tests.sanitize_id_and_type = function(test, common) {
   });
 };
 
+
+module.exports.tests.sanitize_ids = function(test, common) {
+  test('invalid input', function(t) {
+    sanitize({ id: inputs.invalid }, function( err, clean ){
+      var input = inputs.invalid[0]; // since it breaks on the first invalid element
+      switch (err) {
+        case defaultError:
+          t.equal(err, defaultError, input + ' is invalid input'); break;
+        case defaultLengthError(input):
+          t.equal(err, defaultLengthError(input), input + ' is invalid (missing id/type)'); break;
+        case defaultFormatError:
+          t.equal(err, defaultFormatError, input + ' is invalid (invalid format)'); break;
+        case defaultMissingTypeError(input):
+          t.equal(err, defaultMissingTypeError(input), input + ' is an unknown type'); break;
+        default: break;
+      }
+      t.equal(clean, undefined, 'clean not set');
+    });
+    t.end();
+  });
+
+  test('valid input', function(t) {
+    var expected={};
+    expected.ids = [];
+    inputs.valid.forEach( function( input ){
+      var input_parts = input.split(delimiter);
+      expected.ids.push({ id: input_parts[1], type: input_parts[0] });
+    });
+    sanitize({ id: inputs.valid }, function( err, clean ){
+      t.equal(err, undefined, 'no error' );
+      t.deepEqual(clean, expected, 'clean set correctly');
+    });
+    t.end();
+  });
+};
+
 module.exports.tests.invalid_params = function(test, common) {
   test('invalid params', function(t) {
     sanitize( undefined, function( err, clean ){
@@ -102,7 +126,7 @@ module.exports.tests.middleware_failure = function(test, common) {
 
 module.exports.tests.middleware_success = function(test, common) {
   test('middleware success', function(t) {
-    var req = { query: { id: 'geoname/123' }};
+    var req = { query: { id: 'geoname' + delimiter + '123' }};
     var next = function( message ){
       t.equal(message, undefined, 'no error message set');
       t.deepEqual(req.clean, defaultClean);