converted middleware/interpolate to use injected service

middleware/interpolate.js

@@ -1,7 +1,6 @@
-
+const async = require('async');
-var async = require('async');
+const logger = require( 'pelias-logger' ).get( 'api' );
-var logger = require( 'pelias-logger' ).get( 'api' );
+const _ = require('lodash');
-var service = require('../service/interpolation');
 
 /**
 example response from interpolation web service:
@@ -21,132 +20,96 @@ example response from interpolation web service:
 }
 **/
 
-function setup() {
+function setup(service, should_execute) {
-
+  return function controller(req, res, next) {
-  var transport = service.search();
+    if (!should_execute(req, res)) {
-  var middleware = function(req, res, next) {
-
-    // no-op, user did not request an address
-    if( !isAddressQuery( req ) ){
       return next();
     }
 
-    // bind parsed_text variables to function call
+    // bind the service to the req which doesn't change
-    var bound = interpolate.bind( transport, req.clean.parsed_text );
+    const req_bound_service = _.partial(service, req);
 
-    // perform interpolations asynchronously for all relevant hits
+    // only interpolate the street-layer results
-    var timer = (new Date()).getTime();
+    // save this off into a separate array so that when docs are annotated
-    async.map( res.data, bound, function( err, results ){
+    //  after the interpolate results are returned, no complicated bookkeeping is needed
+    const street_results = _.get(res, 'data', []).filter(result => result.layer === 'street');
 
-      // update res.data with the mapped values
+    // perform interpolations asynchronously for all relevant hits
-      if( !err ){
+    const start = (new Date()).getTime();
-        res.data = results;
+    async.map(street_results, req_bound_service, (err, interpolation_results) => {
+      if (err) {
+        logger.error(`[middleware:interpolation] ${_.defaultTo(err.message, err)}`);
+        return next();
       }
 
-      // sort the results to ensure that addresses show up higher than street centroids      
+      interpolation_results.forEach((interpolation_result, idx) => {
-      res.data = res.data.sort((a, b) => {
+        const source_result = street_results[idx];
-        if (a.layer === 'address' && b.layer !== 'address') { return -1; }
+
-        if (a.layer !== 'address' && b.layer === 'address') { return 1; }
+        // invalid / not useful response, debug log for posterity
-        return 0;
+        // note: leave this hit unmodified
-      });
+        if (!_.has(interpolation_result, 'properties')) {
-
+          logger.debug(`[interpolation] [miss] ${req.clean.parsed_text}`);
-      // log the execution time, continue
+          return;
-      logger.info( '[interpolation] [took]', (new Date()).getTime() - timer, 'ms' );
+        }
-      next();
+
-    });
+        // the interpolation service returned a valid result, debug log for posterity
-  };
+        // note: we now merge those values with the existing 'street' record
-
+        logger.debug(`[interpolation] [hit] ${req.clean.parsed_text} ${JSON.stringify(interpolation_result)}`);
-  middleware.transport = transport;
+
-  return middleware;
+        // -- metatdata --
-}
+        source_result.layer = 'address';
-
+        source_result.match_type = 'interpolated';
-function interpolate( parsed_text, hit, cb ){
+
-
+        // -- name --
-  // no-op, this hit is not from the 'street' layer
+        source_result.name.default = `${interpolation_result.properties.number} ${source_result.name.default}`;
-  // note: no network request is performed.
+
-  if( !hit || hit.layer !== 'street' ){
+        // -- source --
-    return cb( null, hit );
+        if (interpolation_result.properties.source === 'OSM') {
-  }
+          source_result.source = 'openstreetmap';
-
+        } else if (interpolation_result.properties.source === 'OA') {
-  // query variables
+          source_result.source = 'openaddresses';
-  var coord = hit.center_point;
+        } else {
-  var number = parsed_text.number;
+          source_result.source = 'mixed';
-  var street = hit.address_parts.street || parsed_text.street;
+        }
-
+
-  // query interpolation service
+        // -- source_id --
-  this.query( coord, number, street, function( err, data ){
+        // note: interpolated values have no source_id
+        delete source_result.source_id; // remove original street source_id
+        if( interpolation_result.properties.hasOwnProperty( 'source_id' ) ){
+          source_result.source_id = interpolation_result.properties.source_id;
+        }
+
+        // -- address_parts --
+        source_result.address_parts.number = interpolation_result.properties.number;
+
+        // -- geo --
+        source_result.center_point = {
+          lat: interpolation_result.properties.lat,
+          lon: interpolation_result.properties.lon
+        };
+
+        // -- bbox --
+        delete source_result.bounding_box;
 
-    // an error occurred
+      });
-    // note: leave this hit unmodified
-    if( err ){
-      logger.error( '[interpolation] [error]', err );
-      return cb( null, hit );
-    }
-
-    // invalid / not useful response
-    // note: leave this hit unmodified
-    if( !data || !data.hasOwnProperty('properties') ){
-      logger.info( '[interpolation] [miss]', parsed_text );
-      return cb( null, hit );
-    }
-
-    // the interpolation service returned a valid result
-    // note: we now merge thos values with the existing 'street' record.
-    logger.info( '[interpolation] [hit]', parsed_text, data );
-
-    // safety first!
-    try {
-
-      // -- metatdata --
-      hit.layer = 'address';
-      hit.match_type = 'interpolated';
-
-      // -- name --
-      hit.name.default = data.properties.number + ' ' + hit.name.default;
-
-      // -- source --
-      var source = 'mixed';
-      if( data.properties.source === 'OSM' ){ source = 'openstreetmap'; }
-      else if( data.properties.source === 'OA' ){ source = 'openaddresses'; }
-      hit.source = source;
 
-      // -- source_id --
+      // sort the results to ensure that addresses show up higher than street centroids
-      // note: interpolated values have no source_id
+      if (_.has(res, 'data')) {
-      delete hit.source_id; // remove original street source_id
+        res.data.sort((a, b) => {
-      if( data.properties.hasOwnProperty( 'source_id' ) ){
+          if (a.layer === 'address' && b.layer !== 'address') { return -1; }
-        hit.source_id = data.properties.source_id;
+          if (a.layer !== 'address' && b.layer === 'address') { return 1; }
+          return 0;
+        });
       }
 
-      // -- address_parts --
+      // log the execution time, continue
-      hit.address_parts.number = data.properties.number;
+      logger.info( `[interpolation] [took] ${(new Date()).getTime() - start} ms`);
-
+      next();
-      // -- geo --
-      hit.center_point = {
-        lat: data.properties.lat,
-        lon: data.properties.lon
-      };
-
-      // -- bbox --
-      delete hit.bounding_box;
-
-      // return the modified hit
-      return cb( null, hit );
 
-    // a syntax error occurred in the code above (this shouldn't happen!)
+    });
-    // note: the hit object may be partially modified, could possibly be invalid
-    } catch( e ){
-      logger.error( '[interpolation] [error]', e, e.stack );
-      return cb( null, hit );
-    }
-  });
-}
 
-// boolean function to check if an address was requested by the user
+  };
-function isAddressQuery( req ){
+  
-  return req && req.hasOwnProperty('clean') &&
-         req.clean.hasOwnProperty('parsed_text') &&
-         req.clean.parsed_text.hasOwnProperty('number') &&
-         req.clean.parsed_text.hasOwnProperty('street');
 }
 
 module.exports = setup;

package.json

@@ -66,7 +66,6 @@
     "predicates": "^1.0.1",
     "retry": "^0.10.1",
     "stats-lite": "^2.0.4",
-    "superagent": "^3.2.1",
     "through2": "^2.0.3"
   },
   "devDependencies": {

service/interpolation.js

@@ -1,118 +0,0 @@
-
-var logger = require( 'pelias-logger' ).get( 'api' ),
-    request = require( 'superagent' ),
-    peliasConfig = require( 'pelias-config' );
-
-/**
-
-  street address interpolation service client
-
-  this file provides several different 'transports' which can be used to access the interpolation
-  service, either directly from disk or via a network connnection.
-
-  the exported method for this module checks pelias-config for a configuration block such as:
-
-  "interpolation": {
-    "client": {
-      "adapter": "http",
-      "host": "http://localhost:4444"
-    }
-  }
-
-  for more info on running the service see: https://github.com/pelias/interpolation
-
-**/
-
-/**
-  NullTransport
-
-  disables the service completely
-**/
-function NullTransport(){}
-NullTransport.prototype.query = function( coord, number, street, cb ){
-  cb(); // no-op
-};
-
-/**
-  RequireTransport
-
-  allows the api to be used by simply requiring the module
-**/
-function RequireTransport( addressDbPath, streetDbPath ){
-  try {
-    var lib = require('pelias-interpolation'); // lazy load dependency
-    this.query = lib.api.search( addressDbPath, streetDbPath );
-  } catch( e ){
-    logger.error( 'RequireTransport: failed to connect to interpolation service' );
-  }
-}
-RequireTransport.prototype.query = function( coord, number, street, cb ){
-  throw new Error( 'interpolation: transport not connected' );
-};
-
-/**
-  HttpTransport
-
-  allows the api to be used via a remote web service
-**/
-function HttpTransport( host, settings ){
-  this.query = function( coord, number, street, cb ){
-    request
-      .get( host + '/search/geojson' )
-      .set( 'Accept', 'application/json' )
-      .query({ lat: coord.lat, lon: coord.lon, number: number, street: street })
-      .timeout( settings && settings.timeout || 1000 )
-      .end( function( err, res ){
-        if( err || !res ){ return cb( err ); }
-        if( 200 !== res.status ){ return cb( 'non 200 status' ); }
-        return cb( null, res.body );
-      });
-  };
-}
-HttpTransport.prototype.query = function( coord, number, street, cb ){
-  throw new Error( 'interpolation: transport not connected' );
-};
-
-/**
-  Setup
-
-  allows instantiation of transport depending on configuration and preference
-**/
-module.exports.search = function setup(){
-
-  // user config
-  var config = peliasConfig.generate();
-
-  // ensure config variables set correctly
-  if( !config.hasOwnProperty('interpolation') || !config.interpolation.hasOwnProperty('client') ){
-    logger.warn( 'interpolation: configuration not found' );
-  }
-
-  // valid configuration found
-  else {
-
-    // get adapter settings from config
-    var settings = config.interpolation.client;
-
-    // http adapter
-    if( 'http' === settings.adapter && settings.hasOwnProperty('host') ){
-      logger.info( 'interpolation: using http transport:', settings.host );
-      if( settings.hasOwnProperty('timeout') ){
-        return new HttpTransport( settings.host, { timeout: parseInt( settings.timeout, 10 ) } );
-      }
-      return new HttpTransport( settings.host );
-    }
-
-    // require adapter
-    else if( 'require' === settings.adapter ){
-      if( settings.hasOwnProperty('streetdb') && settings.hasOwnProperty('addressdb') ){
-        logger.info( 'interpolation: using require transport' );
-        return new RequireTransport( settings.addressdb, settings.streetdb );
-      }
-    }
-  }
-
-  // default adapter
-  logger.info( 'interpolation: using null transport' );
-  return new NullTransport();
-};

test/unit/middleware/interpolate.js

@@ -1,275 +1,681 @@
+'use strict';
 
-var fs = require('fs'),
+const setup = require('../../../middleware/interpolate');
-    tmp = require('tmp'),
+const proxyquire =  require('proxyquire').noCallThru();
-    setup = require('../../../middleware/interpolate');
+const _  = require('lodash');
-
-// load middleware using the default pelias config
-var load = function(){
-  // adapter is driven by config
-  var tmpfile = tmp.tmpNameSync({ postfix: '.json' });
-  fs.writeFileSync( tmpfile, '{}', { encoding: 'utf8' } );
-  process.env.PELIAS_CONFIG = tmpfile;
-  var middleware = setup();
-  delete process.env.PELIAS_CONFIG;
-  return middleware;
-};
 
 module.exports.tests = {};
 
-module.exports.tests.interface = function(test, common) {
+module.exports.tests.interface = (test, common) => {
-  test('valid interface', function(t) {
+  test('valid interface', t => {
-    var middleware = load();
+    t.equal(typeof setup, 'function', 'setup is a function');
-    t.equal(typeof middleware, 'function', 'middleware is a function');
+    t.equal(typeof setup(), 'function', 'setup returns a controller');
-    t.equal(middleware.length, 3, 'middleware is a function');
     t.end();
   });
 };
 
-module.exports.tests.isAddressQuery = function(test, common) {
+module.exports.tests.early_exit_conditions = (test, common) => {
-  test('invalid address query - no parsed text', function(t) {
+  test('should_execute returning false should not call service', t => {
-    var req = { clean: {} };
+    t.plan(3, 'should_execute will assert 2 things + 1 for next() was called');
+
+    const service = () => {
+      t.fail('service should not have been called');
+    };
+
+    const should_execute = (req, res) => {
+      t.deepEquals(req, { a: 1 });
+      t.deepEquals(res, { b: 2 });
+      return false;
+    };
+
+    const controller = setup(service, should_execute);
+
+    controller({ a: 1 }, { b: 2 }, () => {
+      t.pass('next was called');
+    });
 
-    var middleware = load();
-    middleware(req, null, t.end);
   });
 
-  test('invalid address query - no number', function(t) {
+};
-    var req = { clean: {
+
-      parsed_text: {
+module.exports.tests.error_conditions = (test, common) => {
-        street: 'sesame st'
+  test('service error string should log and not modify any results', t => {
-      }}
+    t.plan(2);
+
+    const service = (req, res, callback) => {
+      callback('this is an error', {
+        properties: {
+          number: 17,
+          source: 'OSM',
+          source_id: 'openstreetmap source id',
+          lat: 12.121212,
+          lon: 21.212121
+        }
+      });
+
+    };
+
+    const logger = require('pelias-mock-logger')();
+
+    const controller = proxyquire('../../../middleware/interpolate', {
+      'pelias-logger': logger
+    })(service, () => true);
+
+    const req = { a: 1 };
+    const res = {
+      data: [
+        {
+          id: 1,
+          layer: 'street',
+          name: {
+            default: 'street name 1'
+          },
+          address_parts: {},
+          // bounding_box should be removed
+          bounding_box: {}
+        }
+      ]
     };
 
-    var middleware = load();
+    controller(req, res, () => {
-    middleware(req, null, t.end);
+      t.ok(logger.isErrorMessage('[middleware:interpolation] this is an error'));
+
+      t.deepEquals(res, {
+        data: [
+          {
+            id: 1,
+            layer: 'street',
+            name: {
+              default: 'street name 1'
+            },
+            address_parts: {},
+            // bounding_box should be removed
+            bounding_box: {}
+          }
+        ]
+      }, 'res should not have been modified');
+
+    });
+
   });
 
-  test('invalid address query - no street', function(t) {
+  test('service error object should log message and not modify any results', t => {
-    var req = { clean: {
+    t.plan(2);
-      parsed_text: {
+
-        number: '1',
+    const service = (req, res, callback) => {
-      }}
+      callback({ message: 'this is an error' }, {
+        properties: {
+          number: 17,
+          source: 'OSM',
+          source_id: 'openstreetmap source id',
+          lat: 12.121212,
+          lon: 21.212121
+        }
+      });
+
+    };
+
+    const logger = require('pelias-mock-logger')();
+
+    const controller = proxyquire('../../../middleware/interpolate', {
+      'pelias-logger': logger
+    })(service, () => true);
+
+    const req = { a: 1 };
+    const res = {
+      data: [
+        {
+          id: 1,
+          layer: 'street',
+          name: {
+            default: 'street name 1'
+          },
+          address_parts: {},
+          // bounding_box should be removed
+          bounding_box: {}
+        }
+      ]
     };
 
-    var middleware = load();
+    controller(req, res, () => {
-    middleware(req, null, t.end);
+      t.ok(logger.isErrorMessage('[middleware:interpolation] this is an error'));
+
+      t.deepEquals(res, {
+        data: [
+          {
+            id: 1,
+            layer: 'street',
+            name: {
+              default: 'street name 1'
+            },
+            address_parts: {},
+            // bounding_box should be removed
+            bounding_box: {}
+          }
+        ]
+      }, 'res should not have been modified');
+
+    });
+
   });
+
 };
 
-// test results are correctly mapped to the transport
+module.exports.tests.success_conditions = (test, common) => {
-module.exports.tests.map = function(test, common) {
+  test('undefined res should not cause errors', t => {
-  test('documents mapped to transport: no hits', function(t) {
+    const service = (req, res, callback) => {
-    var req = { clean: {
+      t.fail('should not have been called');
-      parsed_text: {
-        number: '1',
-        street: 'sesame st'
-      }}
     };
-    var res = { data: [] };
 
-    var middleware = load();
+    const logger = require('pelias-mock-logger')();
-    middleware(req, res, function(){
+
-      t.deepEqual( res, { data: [] } );
+    const controller = proxyquire('../../../middleware/interpolate', {
+      'pelias-logger': logger
+    })(service, () => true);
+
+    const req = {
+      clean: {
+        parsed_text: 'this is req.clean.parsed_text'
+      }
+    };
+
+    controller(req, undefined, () => {
+      t.notOk(logger.hasErrorMessages(), 'there shouldn\'t be any error messages');
+      t.ok(logger.isInfoMessage(/\[interpolation\] \[took\] \d+ ms/));
       t.end();
+
     });
+
   });
-  test('documents mapped to transport: no street layer hits', function(t) {
+
-    var req = { clean: {
+  test('undefined res.data should not cause errors', t => {
-      parsed_text: {
+    const service = (req, res, callback) => {
-        number: '1',
+      t.fail('should not have been called');
-        street: 'sesame st'
-      }}
     };
-    var res = { data: [{ layer: 'foo' }] };
 
-    var middleware = load();
+    const logger = require('pelias-mock-logger')();
-    middleware(req, res, function(){
+
-      t.deepEqual( res, { data: [{ layer: 'foo' }] } );
+    const controller = proxyquire('../../../middleware/interpolate', {
+      'pelias-logger': logger
+    })(service, () => true);
+
+    const req = {
+      clean: {
+        parsed_text: 'this is req.clean.parsed_text'
+      }
+    };
+
+    const res = {};
+
+    controller(req, res, () => {
+      t.notOk(logger.hasErrorMessages(), 'there shouldn\'t be any error messages');
+      t.ok(logger.isInfoMessage(/\[interpolation\] \[took\] \d+ ms/));
+
+      t.deepEquals(res, {});
+
       t.end();
+
     });
+
   });
-};
 
-// check the service is called and response mapped correctly
+  test('interpolated results should be mapped in', t => {
-module.exports.tests.miss = function(test, common) {
+    const service = (req, res, callback) => {
-  test('miss', function(t) {
+      if (res.id === 1) {
+        callback(null, {
+          properties: {
+            number: 17,
+            source: 'OSM',
+            source_id: 'openstreetmap source id',
+            lat: 12.121212,
+            lon: 21.212121
+          }
+        });
+
+      } else if (res.id === 3) {
+        callback(null, {
+          properties: {
+            number: 18,
+            source: 'OA',
+            source_id: 'openaddresses source id',
+            lat: 13.131313,
+            lon: 31.313131
+          }
+        });
+
+      } else if (res.id === 4) {
+        callback(null, {
+          properties: {
+            number: 19,
+            source: 'non-OSM/OA',
+            source_id: 'mixed source id',
+            lat: 14.141414,
+            lon: 41.414141
+          }
+        });
+
+      } else {
+        t.fail(`unexpected id ${res.id}`);
 
-    var req = { clean: {
-      parsed_text: {
-        number: '1',
-        street: 'sesame st'
-      }}
-    };
-    var res = { data: [
-      {
-        layer: 'street',
-        center_point: { lat: 1, lon: 1 },
-        address_parts: { street: 'sesame rd' },
-        name: { default: 'example' }
       }
-    ]};
 
-    var middleware = load();
+    };
+
+    const logger = require('pelias-mock-logger')();
+
+    const controller = proxyquire('../../../middleware/interpolate', {
+      'pelias-logger': logger
+    })(service, () => true);
 
-    // mock out the transport
+    const req = {
-    middleware.transport.query = function mock( coord, number, street, cb ){
+      clean: {
-      t.deepEqual( coord, res.data[0].center_point );
+        parsed_text: 'this is req.clean.parsed_text'
-      t.deepEqual( number, req.clean.parsed_text.number );
+      }
-      t.deepEqual( street, res.data[0].address_parts.street );
-      t.equal( typeof cb, 'function' );
-      cb( 'error' );
     };
 
-    middleware(req, res, function(){
+    const res = {
-      t.deepEqual( res, { data: [
+      data: [
+        {
+          id: 1,
+          layer: 'street',
+          name: {
+            default: 'street name 1'
+          },
+          address_parts: {},
+          // will be replaced
+          source_id: 'original source_id',
+          // bounding_box should be removed
+          bounding_box: {}
+        },
+        {
+          id: 2,
+          layer: 'not street',
+          name: {
+            default: 'name 2'
+          },
+          address_parts: {}
+        },
         {
+          id: 3,
           layer: 'street',
-          center_point: { lat: 1, lon: 1 },
+          name: {
-          address_parts: { street: 'sesame rd' },
+            default: 'street name 3'
-          name: { default: 'example' }
+          },
+          address_parts: {}
+        },
+        {
+          id: 4,
+          layer: 'street',
+          name: {
+            default: 'street name 4'
+          },
+          address_parts: {}
         }
-      ]});
+      ]
+    };
+
+    controller(req, res, () => {
+      t.notOk(logger.hasErrorMessages(), 'there shouldn\'t be any error messages');
+      t.ok(logger.isInfoMessage(/\[interpolation\] \[took\] \d+ ms/), 'timing should be info-logged');
+
+      // test debug messages very vaguely to avoid brittle tests
+      t.ok(logger.isDebugMessage(/^\[interpolation\] \[hit\] this is req.clean.parsed_text \{.+?\}$/),
+        'hits should be debug-logged');
+
+      t.deepEquals(res, {
+        data: [
+          {
+            id: 1,
+            layer: 'address',
+            match_type: 'interpolated',
+            name: {
+              default: '17 street name 1'
+            },
+            source: 'openstreetmap',
+            source_id: 'openstreetmap source id',
+            address_parts: {
+              number: 17
+            },
+            center_point: {
+              lat: 12.121212,
+              lon: 21.212121
+            }
+          },
+          {
+            id: 3,
+            layer: 'address',
+            match_type: 'interpolated',
+            name: {
+              default: '18 street name 3'
+            },
+            source: 'openaddresses',
+            source_id: 'openaddresses source id',
+            address_parts: {
+              number: 18
+            },
+            center_point: {
+              lat: 13.131313,
+              lon: 31.313131
+            }
+          },
+          {
+            id: 4,
+            layer: 'address',
+            match_type: 'interpolated',
+            name: {
+              default: '19 street name 4'
+            },
+            source: 'mixed',
+            source_id: 'mixed source id',
+            address_parts: {
+              number: 19
+            },
+            center_point: {
+              lat: 14.141414,
+              lon: 41.414141
+            }
+          },
+          {
+            id: 2,
+            layer: 'not street',
+            name: {
+              default: 'name 2'
+            },
+            address_parts: {}
+          }
+        ]
+      }, 'hits should be mapped in and res.data sorted with addresses first and non-addresses last');
+
       t.end();
+
     });
+
   });
-};
 
-// check the service is called and response mapped correctly
+  test('interpolation result without source_id should remove all together', t => {
-module.exports.tests.hit = function(test, common) {
+    const service = (req, res, callback) => {
-  test('hit', function(t) {
+      if (res.id === 1) {
+        callback(null, {
+          properties: {
+            number: 17,
+            source: 'OA',
+            lat: 12.121212,
+            lon: 21.212121
+          }
+        });
+
+      } else {
+        t.fail(`should not have been called with id ${res.id}`);
 
-    var req = { clean: {
-      parsed_text: {
-        number: '1',
-        street: 'sesame st'
-      }}
-    };
-    var res = { data: [
-      {
-        layer: 'street',
-        center_point: { lat: 1, lon: 1 },
-        address_parts: { street: 'sesame rd' },
-        name: { default: 'street name' },
-        source_id: '123456'
       }
-    ]};
 
-    var middleware = load();
+    };
 
-    // mock out the transport
+    const logger = require('pelias-mock-logger')();
-    middleware.transport.query = function mock( coord, number, street, cb ){
+
-      t.deepEqual( coord, res.data[0].center_point );
+    const controller = proxyquire('../../../middleware/interpolate', {
-      t.deepEqual( number, req.clean.parsed_text.number );
+      'pelias-logger': logger
-      t.deepEqual( street, res.data[0].address_parts.street );
+    })(service, () => true);
-      t.equal( typeof cb, 'function' );
+
-      cb( null, {
+    const req = {
-        properties: {
+      clean: {
-          number: '100A',
+        parsed_text: 'this is req.clean.parsed_text'
-          source: 'OSM',
+      }
-          source_id: 'way:111111',
-          lat: 22.2,
-          lon: -33.3,
-        }
-      });
     };
 
-    middleware(req, res, function(){
+    const res = {
-      t.deepEqual( res, { data: [
+      data: [
+        // doc with 2 layer names that will be changed
         {
-          layer: 'address',
+          id: 1,
-          match_type: 'interpolated',
+          layer: 'street',
-          center_point: { lat: 22.2, lon: -33.3 },
+          name: {
-          address_parts: { street: 'sesame rd', number: '100A' },
+            default: 'street name 1'
-          name: { default: '100A street name' },
+          },
-          source: 'openstreetmap',
+          // will be removed
-          source_id: 'way:111111'
+          source_id: 'original source_id',
+          address_parts: {}
         }
-      ]});
+      ]
+    };
+
+    controller(req, res, () => {
+      t.notOk(logger.hasErrorMessages(), 'there shouldn\'t be any error messages');
+      t.ok(logger.isInfoMessage(/\[interpolation\] \[took\] \d+ ms/));
+
+      t.deepEquals(res, {
+        data: [
+          {
+            id: 1,
+            layer: 'address',
+            match_type: 'interpolated',
+            name: {
+              default: '17 street name 1'
+            },
+            source: 'openaddresses',
+            address_parts: {
+              number: 17
+            },
+            center_point: {
+              lat: 12.121212,
+              lon: 21.212121
+            }
+          }
+        ]
+      }, 'interpolation result did not have source_id so removed from source result');
+
       t.end();
+
     });
+
   });
-};
 
-// check the service is called and response mapped correctly
+  test('undefined results should be skipped and not be fatal', t => {
-module.exports.tests.hit = function(test, common) {
+    const service = (req, res, callback) => {
-  test('hit', function(t) {
+      if (res.id === 1) {
+        callback(null, undefined);
+
+      } else if (res.id === 2) {
+        callback(null, {
+          properties: {
+            number: 18,
+            source: 'OA',
+            source_id: 'openaddresses source id',
+            lat: 13.131313,
+            lon: 31.313131
+          }
+        });
+
+      } else {
+        t.fail(`should not have been called with id ${res.id}`);
+
+      }
 
-    var req = { clean: {
-      parsed_text: {
-        number: '1',
-        street: 'sesame st'
-      }}
     };
-    var res = { data: [
+
-      {
+    const logger = require('pelias-mock-logger')();
-        layer: 'street',
+
-        center_point: { lat: 1, lon: 1 },
+    const controller = proxyquire('../../../middleware/interpolate', {
-        address_parts: { street: 'sesame rd' },
+      'pelias-logger': logger
-        name: { default: 'street name' },
+    })(service, () => true);
-        source_id: '123456'
+
-      },
+    const req = {
-      {
+      clean: {
-        layer: 'street',
+        parsed_text: 'this is req.clean.parsed_text'
-        center_point: { lat: 2, lon: 2 },
-        address_parts: { street: 'sesame rd' },
-        name: { default: 'street name' },
-        source_id: '654321'
       }
-    ]};
+    };
-
+
-    var middleware = load();
+    const res = {
-
+      data: [
-    // mock out the transport
+        // doc with 2 layer names that will be changed
-    middleware.transport.query = function mock(coord, number, street, cb) {
+        {
-      if (coord.lat === 2) {
+          id: 1,
-        t.deepEqual(coord, res.data[1].center_point);
+          layer: 'street',
-        t.deepEqual(number, req.clean.parsed_text.number);
+          name: {
-        t.deepEqual(street, res.data[1].address_parts.street);
+            default: 'street name 1'
-        t.equal(typeof cb, 'function');
+          },
-        return cb(null, {
+          address_parts: {}
+        },
+        {
+          id: 2,
+          layer: 'street',
+          name: {
+            default: 'street name 2'
+          },
+          address_parts: {}
+        }
+      ]
+    };
+
+    controller(req, res, () => {
+      t.notOk(logger.hasErrorMessages(), 'there shouldn\'t be any error messages');
+      t.ok(logger.isInfoMessage(/\[interpolation\] \[took\] \d+ ms/));
+
+      // test debug messages very vaguely to avoid brittle tests
+      t.ok(logger.isDebugMessage('[interpolation] [miss] this is req.clean.parsed_text'));
+
+      t.deepEquals(res, {
+        data: [
+          {
+            id: 2,
+            layer: 'address',
+            match_type: 'interpolated',
+            name: {
+              default: '18 street name 2'
+            },
+            source: 'openaddresses',
+            source_id: 'openaddresses source id',
+            address_parts: {
+              number: 18
+            },
+            center_point: {
+              lat: 13.131313,
+              lon: 31.313131
+            }
+          },
+          {
+            id: 1,
+            layer: 'street',
+            name: {
+              default: 'street name 1'
+            },
+            address_parts: {}
+          }
+        ]
+      }, 'only hits should have been mapped in');
+
+      t.end();
+
+    });
+
+  });
+
+  test('results missing \'properties\' should be skipped and not be fatal', t => {
+    const service = (req, res, callback) => {
+      if (res.id === 1) {
+        callback(null, {});
+
+      } else if (res.id === 2) {
+        callback(null, {
           properties: {
-            number: '100A',
+            number: 18,
-            source: 'OSM',
+            source: 'OA',
-            source_id: 'way:111111',
+            source_id: 'openaddresses source id',
-            lat: 22.2,
+            lat: 13.131313,
-            lon: -33.3,
+            lon: 31.313131
           }
         });
+
+      } else {
+        t.fail(`should not have been called with id ${res.id}`);
+
       }
-      else {
+
-        return cb('miss');
+    };
+
+    const logger = require('pelias-mock-logger')();
+
+    const controller = proxyquire('../../../middleware/interpolate', {
+      'pelias-logger': logger
+    })(service, () => true);
+
+    const req = {
+      clean: {
+        parsed_text: 'this is req.clean.parsed_text'
       }
     };
 
-    middleware(req, res, function(){
+    const res = {
-      t.deepEqual( res, { data: [
+      data: [
+        // doc with 2 layer names that will be changed
         {
-          layer: 'address',
+          id: 1,
-          match_type: 'interpolated',
+          layer: 'street',
-          center_point: { lat: 22.2, lon: -33.3 },
+          name: {
-          address_parts: { street: 'sesame rd', number: '100A' },
+            default: 'street name 1'
-          name: { default: '100A street name' },
+          },
-          source: 'openstreetmap',
+          address_parts: {}
-          source_id: 'way:111111'
         },
         {
+          id: 2,
           layer: 'street',
-          center_point: { lat: 1, lon: 1 },
+          name: {
-          address_parts: { street: 'sesame rd' },
+            default: 'street name 2'
-          name: { default: 'street name' },
+          },
-          source_id: '123456'
+          address_parts: {}
         }
-      ]});
+      ]
+    };
+
+    controller(req, res, () => {
+      t.notOk(logger.hasErrorMessages(), 'there shouldn\'t be any error messages');
+      t.ok(logger.isInfoMessage(/\[interpolation\] \[took\] \d+ ms/));
+
+      // test debug messages very vaguely to avoid brittle tests
+      t.ok(logger.isDebugMessage('[interpolation] [miss] this is req.clean.parsed_text'));
+
+      t.deepEquals(res, {
+        data: [
+          {
+            id: 2,
+            layer: 'address',
+            match_type: 'interpolated',
+            name: {
+              default: '18 street name 2'
+            },
+            source: 'openaddresses',
+            source_id: 'openaddresses source id',
+            address_parts: {
+              number: 18
+            },
+            center_point: {
+              lat: 13.131313,
+              lon: 31.313131
+            }
+          },
+          {
+            id: 1,
+            layer: 'street',
+            name: {
+              default: 'street name 1'
+            },
+            address_parts: {}
+          }
+        ]
+      });
+
       t.end();
-    });
+
+    }, 'only hits should have been mapped in');
+
   });
-};
 
+};
 
 module.exports.all = function (tape, common) {
 
   function test(name, testFunction) {
-    return tape('[middleware] interpolate: ' + name, testFunction);
+    return tape(`[middleware] interpolate: ${name}`, testFunction);
   }
 
   for( var testCase in module.exports.tests ){

test/unit/run.js

@@ -101,8 +101,7 @@ var tests = [
   require('./service/configurations/PlaceHolder'),
   require('./service/configurations/PointInPolygon'),
   require('./service/mget'),
-  require('./service/search'),
+  require('./service/search')
-  require('./service/interpolation')
 ];
 
 tests.map(function(t) {

test/unit/service/interpolation.js

@@ -1,128 +0,0 @@
-
-var fs = require('fs'),
-    tmp = require('tmp'),
-    setup = require('../../../service/interpolation').search;
-
-module.exports.tests = {};
-
-module.exports.tests.interface = function(test, common) {
-  test('valid interface', function(t) {
-    t.equal(typeof setup, 'function', 'setup is a function');
-    t.end();
-  });
-};
-
-// adapter factory
-module.exports.tests.factory = function(test, common) {
-
-  test('http adapter', function(t) {
-    var config = { interpolation: { client: {
-      adapter: 'http',
-      host: 'http://example.com'
-    }}};
-
-    // adapter is driven by config
-    var tmpfile = tmp.tmpNameSync({ postfix: '.json' });
-    fs.writeFileSync( tmpfile, JSON.stringify( config ), { encoding: 'utf8' } );
-    process.env.PELIAS_CONFIG = tmpfile;
-    var adapter = setup();
-    delete process.env.PELIAS_CONFIG;
-
-    t.equal(adapter.constructor.name, 'HttpTransport', 'HttpTransport');
-    t.equal(typeof adapter, 'object', 'adapter is an object');
-    t.equal(typeof adapter.query, 'function', 'query is a function');
-    t.equal(adapter.query.length, 4, 'query function signature');
-    t.end();
-  });
-
-  test('require adapter', function(t) {
-    var config = { interpolation: { client: {
-      adapter: 'require',
-      addressdb: '/tmp/address.db',
-      streetdb: '/tmp/street.db'
-    }}};
-
-    // adapter is driven by config
-    var tmpfile = tmp.tmpNameSync({ postfix: '.json' });
-    fs.writeFileSync( tmpfile, JSON.stringify( config ), { encoding: 'utf8' } );
-    process.env.PELIAS_CONFIG = tmpfile;
-    var adapter = setup();
-    delete process.env.PELIAS_CONFIG;
-
-    t.equal(adapter.constructor.name, 'RequireTransport', 'RequireTransport');
-    t.equal(typeof adapter, 'object', 'adapter is an object');
-    t.equal(typeof adapter.query, 'function', 'query is a function');
-    t.equal(adapter.query.length, 4, 'query function signature');
-    t.end();
-  });
-
-  test('null adapter', function(t) {
-    var config = { interpolation: { client: {
-      adapter: 'null'
-    }}};
-
-    // adapter is driven by config
-    var tmpfile = tmp.tmpNameSync({ postfix: '.json' });
-    fs.writeFileSync( tmpfile, JSON.stringify( config ), { encoding: 'utf8' } );
-    process.env.PELIAS_CONFIG = tmpfile;
-    var adapter = setup();
-    delete process.env.PELIAS_CONFIG;
-
-    t.equal(adapter.constructor.name, 'NullTransport', 'NullTransport');
-    t.equal(typeof adapter, 'object', 'adapter is an object');
-    t.equal(typeof adapter.query, 'function', 'query is a function');
-    t.equal(adapter.query.length, 4, 'query function signature');
-    t.end();
-  });
-
-  test('default adapter', function(t) {
-    var config = {};
-
-    // adapter is driven by config
-    var tmpfile = tmp.tmpNameSync({ postfix: '.json' });
-    fs.writeFileSync( tmpfile, JSON.stringify( config ), { encoding: 'utf8' } );
-    process.env.PELIAS_CONFIG = tmpfile;
-    var adapter = setup();
-    delete process.env.PELIAS_CONFIG;
-
-    t.equal(adapter.constructor.name, 'NullTransport', 'NullTransport');
-    t.equal(typeof adapter, 'object', 'adapter is an object');
-    t.equal(typeof adapter.query, 'function', 'query is a function');
-    t.equal(adapter.query.length, 4, 'query function signature');
-    t.end();
-  });
-
-};
-
-// null transport
-module.exports.tests.NullTransport = function(test, common) {
-
-  test('null transport', function(t) {
-
-    // adapter is driven by config
-    var tmpfile = tmp.tmpNameSync({ postfix: '.json' });
-    fs.writeFileSync( tmpfile, '{}', { encoding: 'utf8' } );
-    process.env.PELIAS_CONFIG = tmpfile;
-    var adapter = setup();
-    delete process.env.PELIAS_CONFIG;
-
-    // test null transport performs a no-op
-    adapter.query( null, null, null, function( err, res ){
-      t.equal(err, undefined, 'no-op');
-      t.equal(res, undefined, 'no-op');
-      t.end();
-    });
-  });
-
-};
-
-module.exports.all = function (tape, common) {
-
-  function test(name, testFunction) {
-    return tape('SERVICE interpolation', testFunction);
-  }
-
-  for( var testCase in module.exports.tests ){
-    module.exports.tests[testCase](test, common);
-  }
-};