From dd172d782ee3b178485b36baca3ff2de7fd6449a Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Tue, 2 May 2017 15:31:16 -0400
Subject: [PATCH 01/17] added PointInPolygon service configuration

---
 service/configurations/PointInPolygon.js      | 18 ++++
 .../service/configurations/PointInPolygon.js  | 82 +++++++++++++++++++
 2 files changed, 100 insertions(+)
 create mode 100644 service/configurations/PointInPolygon.js
 create mode 100644 test/unit/service/configurations/PointInPolygon.js

diff --git a/service/configurations/PointInPolygon.js b/service/configurations/PointInPolygon.js
new file mode 100644
index 00000000..2bf7e721
--- /dev/null
+++ b/service/configurations/PointInPolygon.js
@@ -0,0 +1,18 @@
+'use strict';
+
+const _ = require('lodash');
+
+const ServiceConfiguration = require('./ServiceConfiguration');
+
+class PointInPolygon extends ServiceConfiguration {
+  constructor(o) {
+    super('pip', o);
+  }
+
+  getUrl(req) {
+    return `${this.baseUrl}/${req.clean.point.lon}/${req.clean.point.lat}`;
+  }
+
+}
+
+module.exports = PointInPolygon;
diff --git a/test/unit/service/configurations/PointInPolygon.js b/test/unit/service/configurations/PointInPolygon.js
new file mode 100644
index 00000000..5fb04d2d
--- /dev/null
+++ b/test/unit/service/configurations/PointInPolygon.js
@@ -0,0 +1,82 @@
+module.exports.tests = {};
+
+const PointInPolygon = require('../../../../service/configurations/PointInPolygon');
+
+module.exports.tests.all = (test, common) => {
+  test('getName should return \'pip\'', (t) => {
+    const configBlob = {
+      url: 'base url',
+      timeout: 17,
+      retries: 19
+    };
+
+    const pointInPolygon = new PointInPolygon(configBlob);
+
+    t.equals(pointInPolygon.getName(), 'pip');
+    t.equals(pointInPolygon.getBaseUrl(), 'base url');
+    t.equals(pointInPolygon.getTimeout(), 17);
+    t.equals(pointInPolygon.getRetries(), 19);
+    t.end();
+
+  });
+
+  test('getUrl should return value formatted with point.lon/lat passed to constructor', (t) => {
+    const configBlob = {
+      url: 'base url'
+    };
+
+    const pointInPolygon = new PointInPolygon(configBlob);
+
+    const req = {
+      clean: {
+        point: {
+          lat: 12.121212,
+          lon: 21.212121
+        }
+      }
+    };
+
+    t.equals(pointInPolygon.getUrl(req), 'base url/21.212121/12.121212');
+    t.end();
+
+  });
+
+  test('getHeaders should return an empty object', (t) => {
+    const configBlob = {
+      url: 'base url',
+      timeout: 17,
+      retries: 19
+    };
+
+    const pointInPolygon = new PointInPolygon(configBlob);
+
+    t.deepEquals(pointInPolygon.getHeaders(), {});
+    t.end();
+
+  });
+
+  test('getParameters should return an empty object', (t) => {
+    const configBlob = {
+      url: 'base url',
+      timeout: 17,
+      retries: 19
+    };
+
+    const pointInPolygon = new PointInPolygon(configBlob);
+
+    t.deepEquals(pointInPolygon.getParameters(), {});
+    t.end();
+
+  });
+
+};
+
+module.exports.all = (tape, common) => {
+  function test(name, testFunction) {
+    return tape(`SERVICE CONFIGURATION /PointInPolygon ${name}`, testFunction);
+  }
+
+  for( var testCase in module.exports.tests ){
+    module.exports.tests[testCase](test, common);
+  }
+};

From e97950c142c8e18b74daa627da7cec4d471f85d8 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Tue, 2 May 2017 15:40:58 -0400
Subject: [PATCH 02/17] adapted coarse reverse controller to generic service

---
 controller/coarse_reverse.js           |   8 +-
 test/unit/controller/coarse_reverse.js | 107 +++++++------------------
 2 files changed, 30 insertions(+), 85 deletions(-)

diff --git a/controller/coarse_reverse.js b/controller/coarse_reverse.js
index e8acbf32..07a370d7 100644
--- a/controller/coarse_reverse.js
+++ b/controller/coarse_reverse.js
@@ -1,7 +1,6 @@
 const logger = require('pelias-logger').get('coarse_reverse');
 const _ = require('lodash');
 const Document = require('pelias-model').Document;
-const isDNT = require('../helper/logging').isDNT;
 
 const granularities = [
   'neighbourhood',
@@ -84,12 +83,7 @@ function setup(service, should_execute) {
       return next();
     }
 
-    const centroid = {
-      lat: req.clean['point.lat'],
-      lon: req.clean['point.lon']
-    };
-
-    service(centroid, isDNT(req), (err, results) => {
+    service(req, (err, results) => {
       // if there's an error, log it and bail
       if (err) {
         logger.info(`[controller:coarse_reverse][error]`);
diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js
index 2389cc43..c37c76e0 100644
--- a/test/unit/controller/coarse_reverse.js
+++ b/test/unit/controller/coarse_reverse.js
@@ -46,8 +46,8 @@ module.exports.tests.early_exit_conditions = (test, common) => {
 
 module.exports.tests.error_conditions = (test, common) => {
   test('service error should log and call next', (t) => {
-    const service = (point, do_not_track, callback) => {
-      t.equals(do_not_track, 'do_not_track value');
+    const service = (req, callback) => {
+      t.deepEquals(req, { clean: { layers: ['locality'] } } );
       callback('this is an error');
     };
 
@@ -55,21 +55,12 @@ module.exports.tests.error_conditions = (test, common) => {
 
     const should_execute = () => { return true; };
     const controller = proxyquire('../../../controller/coarse_reverse', {
-      'pelias-logger': logger,
-      '../helper/logging': {
-        isDNT: () => {
-          return 'do_not_track value';
-        }
-      }
+      'pelias-logger': logger
     })(service, should_execute);
 
     const req = {
       clean: {
-        layers: ['locality'],
-        point: {
-          lat: 12.121212,
-          lon: 21.212121
-        }
+        layers: ['locality']
       }
     };
 
@@ -92,8 +83,9 @@ module.exports.tests.error_conditions = (test, common) => {
 
 module.exports.tests.success_conditions = (test, common) => {
   test('service returning results should use first entry for each layer', (t) => {
-    const service = (point, do_not_track, callback) => {
-      t.equals(do_not_track, 'do_not_track value');
+    const service = (req, callback) => {
+      t.deepEquals(req, { clean: { layers: ['neighbourhood'] } } );
+
       const results = {
         neighbourhood: [
           {
@@ -153,21 +145,12 @@ module.exports.tests.success_conditions = (test, common) => {
 
     const should_execute = () => { return true; };
     const controller = proxyquire('../../../controller/coarse_reverse', {
-      'pelias-logger': logger,
-      '../helper/logging': {
-        isDNT: () => {
-          return 'do_not_track value';
-        }
-      }
+      'pelias-logger': logger
     })(service, should_execute);
 
     const req = {
       clean: {
-        layers: ['neighbourhood'],
-        point: {
-          lat: 12.121212,
-          lon: 21.212121
-        }
+        layers: ['neighbourhood']
       }
     };
 
@@ -247,8 +230,9 @@ module.exports.tests.success_conditions = (test, common) => {
   });
 
   test('layers missing from results should be ignored', (t) => {
-    const service = (point, do_not_track, callback) => {
-      t.equals(do_not_track, 'do_not_track value');
+    const service = (req, callback) => {
+      t.deepEquals(req, { clean: { layers: ['neighbourhood'] } } );
+
       const results = {
         neighbourhood: [
           {
@@ -271,21 +255,12 @@ module.exports.tests.success_conditions = (test, common) => {
 
     const should_execute = () => { return true; };
     const controller = proxyquire('../../../controller/coarse_reverse', {
-      'pelias-logger': logger,
-      '../helper/logging': {
-        isDNT: () => {
-          return 'do_not_track value';
-        }
-      }
+      'pelias-logger': logger
     })(service, should_execute);
 
     const req = {
       clean: {
-        layers: ['neighbourhood'],
-        point: {
-          lat: 12.121212,
-          lon: 21.212121
-        }
+        layers: ['neighbourhood']
       }
     };
 
@@ -337,8 +312,9 @@ module.exports.tests.success_conditions = (test, common) => {
   });
 
   test('most granular layer missing centroid should not set', (t) => {
-    const service = (point, do_not_track, callback) => {
-      t.equals(do_not_track, 'do_not_track value');
+    const service = (req, callback) => {
+      t.deepEquals(req, { clean: { layers: ['neighbourhood'] } } );
+
       const results = {
         neighbourhood: [
           {
@@ -357,21 +333,12 @@ module.exports.tests.success_conditions = (test, common) => {
 
     const should_execute = () => { return true; };
     const controller = proxyquire('../../../controller/coarse_reverse', {
-      'pelias-logger': logger,
-      '../helper/logging': {
-        isDNT: () => {
-          return 'do_not_track value';
-        }
-      }
+      'pelias-logger': logger
     })(service, should_execute);
 
     const req = {
       clean: {
-        layers: ['neighbourhood'],
-        point: {
-          lat: 12.121212,
-          lon: 21.212121
-        }
+        layers: ['neighbourhood']
       }
     };
 
@@ -419,8 +386,9 @@ module.exports.tests.success_conditions = (test, common) => {
   });
 
   test('most granular layer missing bounding_box should not set', (t) => {
-    const service = (point, do_not_track, callback) => {
-      t.equals(do_not_track, 'do_not_track value');
+    const service = (req, callback) => {
+      t.deepEquals(req, { clean: { layers: ['neighbourhood'] } } );
+
       const results = {
         neighbourhood: [
           {
@@ -442,21 +410,12 @@ module.exports.tests.success_conditions = (test, common) => {
 
     const should_execute = () => { return true; };
     const controller = proxyquire('../../../controller/coarse_reverse', {
-      'pelias-logger': logger,
-      '../helper/logging': {
-        isDNT: () => {
-          return 'do_not_track value';
-        }
-      }
+      'pelias-logger': logger
     })(service, should_execute);
 
     const req = {
       clean: {
-        layers: ['neighbourhood'],
-        point: {
-          lat: 12.121212,
-          lon: 21.212121
-        }
+        layers: ['neighbourhood']
       }
     };
 
@@ -510,8 +469,9 @@ module.exports.tests.success_conditions = (test, common) => {
 
 module.exports.tests.failure_conditions = (test, common) => {
   test('service returning 0 results at the requested layer should return nothing', (t) => {
-    const service = (point, do_not_track, callback) => {
-      t.equals(do_not_track, 'do_not_track value');
+    const service = (req, callback) => {
+      t.deepEquals(req, { clean: { layers: ['neighbourhood'] } } );
+
       // response without neighbourhood results
       const results = {
         borough: [
@@ -559,21 +519,12 @@ module.exports.tests.failure_conditions = (test, common) => {
 
     const should_execute = () => { return true; };
     const controller = proxyquire('../../../controller/coarse_reverse', {
-      'pelias-logger': logger,
-      '../helper/logging': {
-        isDNT: () => {
-          return 'do_not_track value';
-        }
-      }
+      'pelias-logger': logger
     })(service, should_execute);
 
     const req = {
       clean: {
-        layers: ['neighbourhood'],
-        point: {
-          lat: 12.121212,
-          lon: 21.212121
-        }
+        layers: ['neighbourhood']
       }
     };
 

From 1f6b6b1ed0fe8b1fc6b405488440714087154032 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Tue, 2 May 2017 15:58:53 -0400
Subject: [PATCH 03/17] finished conversion to use generic http/json service

---
 package.json                                  |   1 -
 routes/v1.js                                  |   6 +-
 service/configurations/PointInPolygon.js      |   2 +-
 service/pointinpolygon.js                     |  86 -----
 test/unit/run.js                              |   2 +-
 .../service/configurations/PointInPolygon.js  |  10 +-
 test/unit/service/pointinpolygon.js           | 323 ------------------
 7 files changed, 9 insertions(+), 421 deletions(-)
 delete mode 100644 service/pointinpolygon.js
 delete mode 100644 test/unit/service/pointinpolygon.js

diff --git a/package.json b/package.json
index 1ca188d3..2f9d3c26 100644
--- a/package.json
+++ b/package.json
@@ -66,7 +66,6 @@
     "pelias-sorting": "1.0.1",
     "pelias-text-analyzer": "1.8.2",
     "predicates": "^1.0.1",
-    "request": "^2.81.0",
     "retry": "^0.10.1",
     "stats-lite": "^2.0.4",
     "superagent": "^3.2.1",
diff --git a/routes/v1.js b/routes/v1.js
index 821db197..6d68d0ee 100644
--- a/routes/v1.js
+++ b/routes/v1.js
@@ -88,9 +88,9 @@ const PlaceHolder = require('../service/configurations/PlaceHolder');
 function addRoutes(app, peliasConfig) {
   const esclient = elasticsearch.Client(peliasConfig.esclient);
 
-  const isPipServiceEnabled = require('../controller/predicates/is_service_enabled')(peliasConfig.api.pipService);
-
-  const pipService = require('../service/pointinpolygon')(peliasConfig.api.pipService);
+  const pipConfiguration = new PointInPolygon(peliasConfig.api.services.placeholder);
+  const pipService = serviceWrapper(pipConfiguration);
+  const isPipServiceEnabled = _.constant(pipConfiguration.isEnabled());
 
   const placeholderConfiguration = new PlaceHolder(_.get(peliasConfig.api.services, 'placeholder', {}));
   const placeholderService = serviceWrapper(placeholderConfiguration);
diff --git a/service/configurations/PointInPolygon.js b/service/configurations/PointInPolygon.js
index 2bf7e721..95edf51b 100644
--- a/service/configurations/PointInPolygon.js
+++ b/service/configurations/PointInPolygon.js
@@ -10,7 +10,7 @@ class PointInPolygon extends ServiceConfiguration {
   }
 
   getUrl(req) {
-    return `${this.baseUrl}/${req.clean.point.lon}/${req.clean.point.lat}`;
+    return `${this.baseUrl}/${req.clean['point.lon']}/${req.clean['point.lat']}`;
   }
 
 }
diff --git a/service/pointinpolygon.js b/service/pointinpolygon.js
deleted file mode 100644
index 1e58dfa3..00000000
--- a/service/pointinpolygon.js
+++ /dev/null
@@ -1,86 +0,0 @@
-const logger = require( 'pelias-logger' ).get( 'pointinpolygon' );
-const request = require('request');
-const _ = require('lodash');
-
-function sanitizeUrl(requestUrl) {
-  return requestUrl.replace(/^(.+)\/.+\/.+$/, (match, base) => {
-    return `${base}/[removed]/[removed]`;
-  });
-}
-
-function parseErrorMessage(requestUrl, body, do_not_track) {
-  if (do_not_track) {
-    return `${sanitizeUrl(requestUrl)} returned status 200 but with non-JSON response: ${body}`;
-  }
-
-  return `${requestUrl} returned status 200 but with non-JSON response: ${body}`;
-
-}
-
-function serviceErrorMessage(requestUrl, statusCode, body, do_not_track) {
-  if (do_not_track) {
-    return `${sanitizeUrl(requestUrl)} returned status ${statusCode}: ${body}`;
-
-  } else {
-    return `${requestUrl} returned status ${statusCode}: ${body}`;
-
-  }
-
-}
-
-module.exports = (url) => {
-  if (!_.isEmpty(url)) {
-    logger.info(`using point-in-polygon service at ${url}`);
-
-    return function pointinpolygon( centroid, do_not_track, callback ) {
-      const requestUrl = `${url}/${centroid.lon}/${centroid.lat}`;
-
-      const options = {
-        method: 'GET',
-        url: requestUrl
-      };
-
-      if (do_not_track) {
-        options.headers = {
-          dnt: '1'
-        };
-      }
-
-      request(options, (err, response, body) => {
-        if (err) {
-          logger.error(JSON.stringify(err));
-          callback(err);
-        }
-        else if (response.statusCode === 200) {
-          try {
-            const parsed = JSON.parse(body);
-            callback(err, parsed);
-          }
-          catch (err) {
-            const parseMsg = parseErrorMessage(requestUrl, body, do_not_track);
-
-            logger.error(parseMsg);
-            callback(parseMsg);
-          }
-        }
-        else {
-          const errorMsg = serviceErrorMessage(requestUrl, response.statusCode, body, do_not_track);
-
-          logger.error(errorMsg);
-          callback(errorMsg);
-        }
-      });
-
-    };
-
-  } else {
-    logger.warn('point-in-polygon service disabled');
-
-    return (centroid, do_not_track, callback) => {
-      callback(`point-in-polygon service disabled, unable to resolve ` +
-        (do_not_track ? 'centroid' : JSON.stringify(centroid)));
-    };
-
-  }
-
-};
diff --git a/test/unit/run.js b/test/unit/run.js
index 3d994ac5..c7647eee 100644
--- a/test/unit/run.js
+++ b/test/unit/run.js
@@ -84,10 +84,10 @@ var tests = [
   require('./sanitizer/search_fallback'),
   require('./sanitizer/wrap'),
   require('./service/configurations/PlaceHolder'),
+  require('./service/configurations/PointInPolygon'),
   require('./service/mget'),
   require('./service/search'),
   require('./service/interpolation'),
-  require('./service/pointinpolygon'),
   require('./service/language')
 ];
 
diff --git a/test/unit/service/configurations/PointInPolygon.js b/test/unit/service/configurations/PointInPolygon.js
index 5fb04d2d..0151ff4b 100644
--- a/test/unit/service/configurations/PointInPolygon.js
+++ b/test/unit/service/configurations/PointInPolygon.js
@@ -28,14 +28,12 @@ module.exports.tests.all = (test, common) => {
     const pointInPolygon = new PointInPolygon(configBlob);
 
     const req = {
-      clean: {
-        point: {
-          lat: 12.121212,
-          lon: 21.212121
-        }
-      }
+      clean: { }
     };
 
+    req.clean['point.lon'] = 21.212121;
+    req.clean['point.lat'] = 12.121212;
+
     t.equals(pointInPolygon.getUrl(req), 'base url/21.212121/12.121212');
     t.end();
 
diff --git a/test/unit/service/pointinpolygon.js b/test/unit/service/pointinpolygon.js
deleted file mode 100644
index 7280aa7f..00000000
--- a/test/unit/service/pointinpolygon.js
+++ /dev/null
@@ -1,323 +0,0 @@
-const proxyquire = require('proxyquire').noCallThru();
-
-const setup = require('../../../service/pointinpolygon');
-
-module.exports.tests = {};
-
-module.exports.tests.interface = (test, common) => {
-  test('valid interface', (t) => {
-    const logger = require('pelias-mock-logger')();
-
-    var service = proxyquire('../../../service/pointinpolygon', {
-      'pelias-logger': logger
-    });
-
-    t.equal(typeof service, 'function', 'service is a function');
-    t.end();
-  });
-};
-
-module.exports.tests.do_nothing_service = (test, common) => {
-  test('undefined PiP uri should return service that logs fact that PiP service is not available', (t) => {
-    const logger = require('pelias-mock-logger')();
-
-    const service = proxyquire('../../../service/pointinpolygon', {
-      'pelias-logger': logger
-    })();
-
-    service({ lat: 12.121212, lon: 21.212121 }, false, (err) => {
-      t.deepEquals(logger.getWarnMessages(), [
-        'point-in-polygon service disabled'
-      ]);
-      t.equals(err, 'point-in-polygon service disabled, unable to resolve {"lat":12.121212,"lon":21.212121}');
-      t.end();
-    });
-
-  });
-
-  test('service unavailable message should not output centroid when do_not_track is true', (t) => {
-    const logger = require('pelias-mock-logger')();
-
-    const service = proxyquire('../../../service/pointinpolygon', {
-      'pelias-logger': logger
-    })();
-
-    service({ lat: 12.121212, lon: 21.212121 }, true, (err) => {
-      t.deepEquals(logger.getWarnMessages(), [
-        'point-in-polygon service disabled'
-      ]);
-      t.equals(err, 'point-in-polygon service disabled, unable to resolve centroid');
-      t.end();
-    });
-
-  });
-
-};
-
-module.exports.tests.success = (test, common) => {
-  test('lat and lon should be passed to server', (t) => {
-    const pipServer = require('express')();
-    pipServer.get('/:lon/:lat', (req, res, next) => {
-      t.equals(req.params.lat, '12.121212');
-      t.equals(req.params.lon, '21.212121');
-
-      res.send('{ "field": "value" }');
-    });
-
-    const server = pipServer.listen();
-    const port = server.address().port;
-
-    const logger = require('pelias-mock-logger')();
-
-    const service = proxyquire('../../../service/pointinpolygon', {
-      'pelias-logger': logger
-    })(`http://localhost:${port}`);
-
-    service({ lat: 12.121212, lon: 21.212121}, false, (err, results) => {
-      t.notOk(err);
-      t.deepEquals(results, { field: 'value' });
-
-      t.ok(logger.isInfoMessage(`using point-in-polygon service at http://localhost:${port}`));
-      t.notOk(logger.hasErrorMessages());
-
-      t.end();
-
-      server.close();
-
-    });
-
-  });
-
-  test('do_not_track=true should pass DNT header', (t) => {
-    const pipServer = require('express')();
-    pipServer.get('/:lon/:lat', (req, res, next) => {
-      t.ok(req.headers.hasOwnProperty('dnt'));
-      t.equals(req.params.lat, '12.121212');
-      t.equals(req.params.lon, '21.212121');
-
-      res.send('{ "field": "value" }');
-    });
-
-    const server = pipServer.listen();
-    const port = server.address().port;
-
-    const logger = require('pelias-mock-logger')();
-
-    const service = proxyquire('../../../service/pointinpolygon', {
-      'pelias-logger': logger
-    })(`http://localhost:${port}`);
-
-    service({ lat: 12.121212, lon: 21.212121}, true, (err, results) => {
-      t.notOk(err);
-      t.deepEquals(results, { field: 'value' });
-
-      t.ok(logger.isInfoMessage(`using point-in-polygon service at http://localhost:${port}`));
-      t.notOk(logger.hasErrorMessages());
-
-      t.end();
-
-      server.close();
-
-    });
-
-  });
-
-  test('do_not_track=false should not pass DNT header', (t) => {
-    const pipServer = require('express')();
-    pipServer.get('/:lon/:lat', (req, res, next) => {
-      t.notOk(req.headers.hasOwnProperty('dnt'));
-      t.equals(req.params.lat, '12.121212');
-      t.equals(req.params.lon, '21.212121');
-
-      res.send('{ "field": "value" }');
-    });
-
-    const server = pipServer.listen();
-    const port = server.address().port;
-
-    const logger = require('pelias-mock-logger')();
-
-    const service = proxyquire('../../../service/pointinpolygon', {
-      'pelias-logger': logger
-    })(`http://localhost:${port}`);
-
-    service({ lat: 12.121212, lon: 21.212121}, false, (err, results) => {
-      t.notOk(err);
-      t.deepEquals(results, { field: 'value' });
-
-      t.ok(logger.isInfoMessage(`using point-in-polygon service at http://localhost:${port}`));
-      t.notOk(logger.hasErrorMessages());
-
-      t.end();
-
-      server.close();
-
-    });
-
-  });
-
-};
-
-module.exports.tests.failure = (test, common) => {
-  test('server returning success but non-JSON body should log error and return no results', (t) => {
-    const pipServer = require('express')();
-    pipServer.get('/:lon/:lat', (req, res, next) => {
-      t.equals(req.params.lat, '12.121212');
-      t.equals(req.params.lon, '21.212121');
-
-      res.send('this is not JSON');
-    });
-
-    const server = pipServer.listen();
-    const port = server.address().port;
-
-    const logger = require('pelias-mock-logger')();
-
-    const service = proxyquire('../../../service/pointinpolygon', {
-      'pelias-logger': logger
-    })(`http://localhost:${port}`);
-
-    const expectedErrorMsg = `http://localhost:${port}/21.212121/12.121212 returned ` +
-      `status 200 but with non-JSON response: this is not JSON`;
-
-    service({ lat: 12.121212, lon: 21.212121}, false, (err, results) => {
-      t.equals(err, expectedErrorMsg);
-      t.notOk(results);
-      t.ok(logger.isErrorMessage(expectedErrorMsg));
-      t.end();
-
-      server.close();
-
-    });
-
-  });
-
-  test('server returning 200 & non-JSON body should log sanitized error and return no results when do_not_track', (t) => {
-    const pipServer = require('express')();
-    pipServer.get('/:lon/:lat', (req, res, next) => {
-      t.equals(req.params.lat, '12.121212');
-      t.equals(req.params.lon, '21.212121');
-
-      res.send('this is not JSON');
-    });
-
-    const server = pipServer.listen();
-    const port = server.address().port;
-
-    const logger = require('pelias-mock-logger')();
-
-    const service = proxyquire('../../../service/pointinpolygon', {
-      'pelias-logger': logger
-    })(`http://localhost:${port}`);
-
-    const expectedErrorMsg = `http://localhost:${port}/[removed]/[removed] returned ` +
-      `status 200 but with non-JSON response: this is not JSON`;
-
-    service({ lat: 12.121212, lon: 21.212121}, true, (err, results) => {
-      t.equals(err, expectedErrorMsg);
-      t.notOk(results);
-      t.ok(logger.isErrorMessage(expectedErrorMsg));
-      t.end();
-
-      server.close();
-
-    });
-
-  });
-
-  test('server returning error should log it and return no results', (t) => {
-    const server = require('express')().listen();
-    const port = server.address().port;
-
-    // immediately close the server so to ensure an error response
-    server.close();
-
-    const logger = require('pelias-mock-logger')();
-
-    const service = proxyquire('../../../service/pointinpolygon', {
-      'pelias-logger': logger
-    })(`http://localhost:${port}`);
-
-    service({ lat: 12.121212, lon: 21.212121}, false, (err, results) => {
-      t.equals(err.code, 'ECONNREFUSED');
-      t.notOk(results);
-      t.ok(logger.isErrorMessage(/ECONNREFUSED/), 'there should be a connection refused error message');
-      t.end();
-
-      server.close();
-
-    });
-
-  });
-
-  test('non-OK status should log error and return no results', (t) => {
-    const pipServer = require('express')();
-    pipServer.get('/:lat/:lon', (req, res, next) => {
-      res.status(400).send('a bad request was made');
-    });
-
-    const server = pipServer.listen();
-    const port = server.address().port;
-
-    const logger = require('pelias-mock-logger')();
-
-    const service = proxyquire('../../../service/pointinpolygon', {
-      'pelias-logger': logger
-    })(`http://localhost:${port}`);
-
-    const expectedErrorMsg = `http://localhost:${port}/21.212121/12.121212 returned ` +
-      `status 400: a bad request was made`;
-
-    service({ lat: 12.121212, lon: 21.212121}, false, (err, results) => {
-      t.equals(err, expectedErrorMsg);
-      t.notOk(results);
-      t.ok(logger.isErrorMessage(expectedErrorMsg));
-      t.end();
-
-      server.close();
-
-    });
-
-  });
-
-  test('non-OK status should log sanitized error and return no results when do_not_track=true', (t) => {
-    const pipServer = require('express')();
-    pipServer.get('/:lat/:lon', (req, res, next) => {
-      res.status(400).send('a bad request was made');
-    });
-
-    const server = pipServer.listen();
-    const port = server.address().port;
-
-    const logger = require('pelias-mock-logger')();
-
-    const service = proxyquire('../../../service/pointinpolygon', {
-      'pelias-logger': logger
-    })(`http://localhost:${port}`);
-
-    const expectedErrorMsg = `http://localhost:${port}/[removed]/[removed] returned ` +
-      `status 400: a bad request was made`;
-
-    service({ lat: 12.121212, lon: 21.212121}, true, (err, results) => {
-      t.equals(err, expectedErrorMsg);
-      t.notOk(results);
-      t.ok(logger.isErrorMessage(expectedErrorMsg));
-      t.end();
-
-      server.close();
-
-    });
-
-  });
-
-};
-
-module.exports.all = (tape, common) => {
-  function test(name, testFunction) {
-    return tape(`SERVICE /pointinpolygon ${name}`, testFunction);
-  }
-
-  for( var testCase in module.exports.tests ){
-    module.exports.tests[testCase](test, common);
-  }
-};

From ce64a1da5c27257ddfbb1d5f01c1c39fdd3f87ca Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Fri, 5 May 2017 12:58:05 -0400
Subject: [PATCH 04/17] swap out for pelias-microservice-wrapper

---
 package.json                             | 1 +
 service/configurations/PointInPolygon.js | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/package.json b/package.json
index 2f9d3c26..2a84af29 100644
--- a/package.json
+++ b/package.json
@@ -62,6 +62,7 @@
     "pelias-logger": "0.2.0",
     "pelias-microservice-wrapper": "1.1.0",
     "pelias-model": "4.8.1",
+    "pelias-mock-logger": "^1.0.1",
     "pelias-query": "8.15.0",
     "pelias-sorting": "1.0.1",
     "pelias-text-analyzer": "1.8.2",
diff --git a/service/configurations/PointInPolygon.js b/service/configurations/PointInPolygon.js
index 95edf51b..c10bdb64 100644
--- a/service/configurations/PointInPolygon.js
+++ b/service/configurations/PointInPolygon.js
@@ -2,7 +2,7 @@
 
 const _ = require('lodash');
 
-const ServiceConfiguration = require('./ServiceConfiguration');
+const ServiceConfiguration = require('pelias-microservice-wrapper').ServiceConfiguration;
 
 class PointInPolygon extends ServiceConfiguration {
   constructor(o) {

From 0437ecdd157d8f88d34bcb8c2f43b39b26c15c41 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Fri, 5 May 2017 16:00:56 -0400
Subject: [PATCH 05/17] bound pelias-microservice-config to 1.0.0

---
 package.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package.json b/package.json
index 2a84af29..9ed9e223 100644
--- a/package.json
+++ b/package.json
@@ -62,7 +62,7 @@
     "pelias-logger": "0.2.0",
     "pelias-microservice-wrapper": "1.1.0",
     "pelias-model": "4.8.1",
-    "pelias-mock-logger": "^1.0.1",
+    "pelias-mock-logger": "1.0.1",
     "pelias-query": "8.15.0",
     "pelias-sorting": "1.0.1",
     "pelias-text-analyzer": "1.8.2",

From 143971e43e5ee57321eab7c50489da6b99de53a1 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Fri, 5 May 2017 17:46:44 -0400
Subject: [PATCH 06/17] use url.resolve to eliminate duplicate slashes

---
 service/configurations/PointInPolygon.js      |  5 ++-
 .../service/configurations/PointInPolygon.js  | 33 +++++++++++++++----
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/service/configurations/PointInPolygon.js b/service/configurations/PointInPolygon.js
index c10bdb64..66a766f9 100644
--- a/service/configurations/PointInPolygon.js
+++ b/service/configurations/PointInPolygon.js
@@ -1,5 +1,7 @@
 'use strict';
 
+const url = require('url');
+
 const _ = require('lodash');
 
 const ServiceConfiguration = require('pelias-microservice-wrapper').ServiceConfiguration;
@@ -10,7 +12,8 @@ class PointInPolygon extends ServiceConfiguration {
   }
 
   getUrl(req) {
-    return `${this.baseUrl}/${req.clean['point.lon']}/${req.clean['point.lat']}`;
+    // use resolve to eliminate possibility of duplicate /'s in URL
+    return url.resolve(this.baseUrl, `${req.clean['point.lon']}/${req.clean['point.lat']}`);
   }
 
 }
diff --git a/test/unit/service/configurations/PointInPolygon.js b/test/unit/service/configurations/PointInPolygon.js
index 0151ff4b..c754e7f9 100644
--- a/test/unit/service/configurations/PointInPolygon.js
+++ b/test/unit/service/configurations/PointInPolygon.js
@@ -5,7 +5,7 @@ const PointInPolygon = require('../../../../service/configurations/PointInPolygo
 module.exports.tests.all = (test, common) => {
   test('getName should return \'pip\'', (t) => {
     const configBlob = {
-      url: 'base url',
+      url: 'http://localhost:1234',
       timeout: 17,
       retries: 19
     };
@@ -13,7 +13,7 @@ module.exports.tests.all = (test, common) => {
     const pointInPolygon = new PointInPolygon(configBlob);
 
     t.equals(pointInPolygon.getName(), 'pip');
-    t.equals(pointInPolygon.getBaseUrl(), 'base url');
+    t.equals(pointInPolygon.getBaseUrl(), 'http://localhost:1234/');
     t.equals(pointInPolygon.getTimeout(), 17);
     t.equals(pointInPolygon.getRetries(), 19);
     t.end();
@@ -22,7 +22,7 @@ module.exports.tests.all = (test, common) => {
 
   test('getUrl should return value formatted with point.lon/lat passed to constructor', (t) => {
     const configBlob = {
-      url: 'base url'
+      url: 'http://localhost:1234'
     };
 
     const pointInPolygon = new PointInPolygon(configBlob);
@@ -34,14 +34,14 @@ module.exports.tests.all = (test, common) => {
     req.clean['point.lon'] = 21.212121;
     req.clean['point.lat'] = 12.121212;
 
-    t.equals(pointInPolygon.getUrl(req), 'base url/21.212121/12.121212');
+    t.equals(pointInPolygon.getUrl(req), 'http://localhost:1234/21.212121/12.121212');
     t.end();
 
   });
 
   test('getHeaders should return an empty object', (t) => {
     const configBlob = {
-      url: 'base url',
+      url: 'http://localhost:1234',
       timeout: 17,
       retries: 19
     };
@@ -55,7 +55,7 @@ module.exports.tests.all = (test, common) => {
 
   test('getParameters should return an empty object', (t) => {
     const configBlob = {
-      url: 'base url',
+      url: 'http://localhost:1234',
       timeout: 17,
       retries: 19
     };
@@ -67,6 +67,27 @@ module.exports.tests.all = (test, common) => {
 
   });
 
+  test('baseUrl ending in / should not have double /\'s return by getUrl', (t) => {
+    const configBlob = {
+      url: 'http://localhost:1234/',
+      timeout: 17,
+      retries: 19
+    };
+
+    const pointInPolygon = new PointInPolygon(configBlob);
+
+    const req = {
+      clean: { }
+    };
+
+    req.clean['point.lon'] = 21.212121;
+    req.clean['point.lat'] = 12.121212;
+
+    t.equals(pointInPolygon.getUrl(req), 'http://localhost:1234/21.212121/12.121212');
+    t.end();
+
+  });
+
 };
 
 module.exports.all = (tape, common) => {

From f473503929b80a86ba4be37aa7eb0b264c0d4b87 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Mon, 8 May 2017 16:43:15 -0400
Subject: [PATCH 07/17] corrected config service

---
 routes/v1.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/routes/v1.js b/routes/v1.js
index 6d68d0ee..4791b67a 100644
--- a/routes/v1.js
+++ b/routes/v1.js
@@ -88,7 +88,7 @@ const PlaceHolder = require('../service/configurations/PlaceHolder');
 function addRoutes(app, peliasConfig) {
   const esclient = elasticsearch.Client(peliasConfig.esclient);
 
-  const pipConfiguration = new PointInPolygon(peliasConfig.api.services.placeholder);
+  const pipConfiguration = new PointInPolygon(peliasConfig.api.services.pip);
   const pipService = serviceWrapper(pipConfiguration);
   const isPipServiceEnabled = _.constant(pipConfiguration.isEnabled());
 

From eb54919ea0203c4344b24b61eccda3704199e14b Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Mon, 5 Jun 2017 10:18:56 -0400
Subject: [PATCH 08/17] removed unneeded pelias-mock-logger dependency

---
 package.json | 1 -
 1 file changed, 1 deletion(-)

diff --git a/package.json b/package.json
index 9ed9e223..2f9d3c26 100644
--- a/package.json
+++ b/package.json
@@ -62,7 +62,6 @@
     "pelias-logger": "0.2.0",
     "pelias-microservice-wrapper": "1.1.0",
     "pelias-model": "4.8.1",
-    "pelias-mock-logger": "1.0.1",
     "pelias-query": "8.15.0",
     "pelias-sorting": "1.0.1",
     "pelias-text-analyzer": "1.8.2",

From c129dec3bf78b2c802ebcdc90467d0f61275eab2 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Mon, 5 Jun 2017 16:11:15 -0400
Subject: [PATCH 09/17] switch to _.defaultTo since it's more descriptive

---
 routes/v1.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/routes/v1.js b/routes/v1.js
index 4791b67a..872c846f 100644
--- a/routes/v1.js
+++ b/routes/v1.js
@@ -88,11 +88,11 @@ const PlaceHolder = require('../service/configurations/PlaceHolder');
 function addRoutes(app, peliasConfig) {
   const esclient = elasticsearch.Client(peliasConfig.esclient);
 
-  const pipConfiguration = new PointInPolygon(peliasConfig.api.services.pip);
+  const pipConfiguration = new PointInPolygon(_.defaultTo(peliasConfig.api.services.pip, {}));
   const pipService = serviceWrapper(pipConfiguration);
   const isPipServiceEnabled = _.constant(pipConfiguration.isEnabled());
 
-  const placeholderConfiguration = new PlaceHolder(_.get(peliasConfig.api.services, 'placeholder', {}));
+  const placeholderConfiguration = new PlaceHolder(_.defaultTo(peliasConfig.api.services.placeholder, {}));
   const placeholderService = serviceWrapper(placeholderConfiguration);
   const isPlaceholderServiceEnabled = _.constant(placeholderConfiguration.isEnabled());
 

From 914e508e41570aafec4c5662015a730da313303c Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Mon, 5 Jun 2017 16:14:33 -0400
Subject: [PATCH 10/17] added pip config + tests to schema

---
 schema.js           |  3 ++
 test/unit/schema.js | 98 +++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 93 insertions(+), 8 deletions(-)

diff --git a/schema.js b/schema.js
index 003c5fb7..cf385b83 100644
--- a/schema.js
+++ b/schema.js
@@ -29,6 +29,9 @@ module.exports = Joi.object().keys({
     pipService: Joi.string().uri({ scheme: /https?/ }),
     placeholderService: Joi.any().forbidden(), // got moved to services
     services: Joi.object().keys({
+      pip: Joi.object().keys({
+        url: Joi.string().uri({ scheme: /https?/ })
+      }).unknown(false).requiredKeys('url'),
       placeholder: Joi.object().keys({
         url: Joi.string().uri({ scheme: /https?/ })
       }).unknown(false).requiredKeys('url')
diff --git a/test/unit/schema.js b/test/unit/schema.js
index cd53fdce..d127bc77 100644
--- a/test/unit/schema.js
+++ b/test/unit/schema.js
@@ -523,6 +523,85 @@ module.exports.tests.api_services_validation = (test, common) => {
 
   });
 
+};
+
+module.exports.tests.placeholder_service_validation = (test, common) => {
+  test('when api.services.placeholder is defined, url is required', (t) => {
+    var config = {
+      api: {
+        version: 'version value',
+        indexName: 'index name value',
+        host: 'host value',
+        services: {
+          placeholder: {
+          }
+        }
+      },
+      esclient: {}
+    };
+
+    const result = Joi.validate(config, schema);
+
+    t.equals(result.error.details.length, 1);
+    t.equals(result.error.details[0].message, '"url" is required');
+    t.end();
+
+  });
+
+  test('non-string api.services.placeholder.url should throw error', (t) => {
+    [null, 17, {}, [], true].forEach((value) => {
+      var config = {
+        api: {
+          version: 'version value',
+          indexName: 'index name value',
+          host: 'host value',
+          services: {
+            placeholder: {
+              url: value
+            }
+          }
+        },
+        esclient: {}
+      };
+
+      const result = Joi.validate(config, schema);
+
+      t.equals(result.error.details.length, 1);
+      t.equals(result.error.details[0].message, '"url" must be a string');
+
+    });
+
+    t.end();
+
+  });
+
+  test('non-http/https api.services.placeholder.url should throw error', (t) => {
+    ['ftp', 'git', 'unknown'].forEach((scheme) => {
+      var config = {
+        api: {
+          version: 'version value',
+          indexName: 'index name value',
+          host: 'host value',
+          services: {
+            placeholder: {
+              url: `${scheme}://localhost`
+            }
+          }
+        },
+        esclient: {}
+      };
+
+      const result = Joi.validate(config, schema);
+
+      t.equals(result.error.details.length, 1);
+      t.equals(result.error.details[0].message, '"url" must be a valid uri with a scheme matching the https\? pattern');
+
+    });
+
+    t.end();
+
+  });
+
   test('non-url children of api.services.placeholder should be disallowed', (t) => {
     var config = {
       api: {
@@ -547,14 +626,17 @@ module.exports.tests.api_services_validation = (test, common) => {
 
   });
 
-  test('when api.services.placeholder is defined, url is required', (t) => {
+};
+
+module.exports.tests.pip_service_validation = (test, common) => {
+  test('when api.services.pip is defined, url is required', (t) => {
     var config = {
       api: {
         version: 'version value',
         indexName: 'index name value',
         host: 'host value',
         services: {
-          placeholder: {
+          pip: {
           }
         }
       },
@@ -569,7 +651,7 @@ module.exports.tests.api_services_validation = (test, common) => {
 
   });
 
-  test('non-string api.services.placeholder.url should throw error', (t) => {
+  test('non-string api.services.pip.url should throw error', (t) => {
     [null, 17, {}, [], true].forEach((value) => {
       var config = {
         api: {
@@ -577,7 +659,7 @@ module.exports.tests.api_services_validation = (test, common) => {
           indexName: 'index name value',
           host: 'host value',
           services: {
-            placeholder: {
+            pip: {
               url: value
             }
           }
@@ -596,7 +678,7 @@ module.exports.tests.api_services_validation = (test, common) => {
 
   });
 
-  test('non-http/https api.services.placeholder.url should throw error', (t) => {
+  test('non-http/https api.services.pip.url should throw error', (t) => {
     ['ftp', 'git', 'unknown'].forEach((scheme) => {
       var config = {
         api: {
@@ -604,7 +686,7 @@ module.exports.tests.api_services_validation = (test, common) => {
           indexName: 'index name value',
           host: 'host value',
           services: {
-            placeholder: {
+            pip: {
               url: `${scheme}://localhost`
             }
           }
@@ -623,14 +705,14 @@ module.exports.tests.api_services_validation = (test, common) => {
 
   });
 
-  test('non-url children of api.services.placeholder should be disallowed', (t) => {
+  test('non-url children of api.services.pip should be disallowed', (t) => {
     var config = {
       api: {
         version: 'version value',
         indexName: 'index name value',
         host: 'host value',
         services: {
-          placeholder: {
+          pip: {
             url: 'http://localhost',
             unknown_property: 'value'
           }

From bc57876f707b9868a1cde11526c7e16b5a401c2a Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Mon, 5 Jun 2017 16:51:46 -0400
Subject: [PATCH 11/17] added PointInPolygon service definitino

---
 routes/v1.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/routes/v1.js b/routes/v1.js
index 872c846f..da4936be 100644
--- a/routes/v1.js
+++ b/routes/v1.js
@@ -78,6 +78,7 @@ const hasAdminOnlyResults = not(hasResultsAtLayers(['venue', 'address', 'street'
 
 const serviceWrapper = require('pelias-microservice-wrapper').service;
 const PlaceHolder = require('../service/configurations/PlaceHolder');
+const PointInPolygon = require('../service/configurations/PointInPolygon');
 
 /**
  * Append routes to app

From aa09419b9cd4aeafbf663d61cf3a7ed694eff766 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Mon, 5 Jun 2017 17:18:17 -0400
Subject: [PATCH 12/17] removed support for api.pipService (relocated to
 api.services)

---
 schema.js           |  2 +-
 test/unit/schema.js | 62 +++++++--------------------------------------
 2 files changed, 10 insertions(+), 54 deletions(-)

diff --git a/schema.js b/schema.js
index cf385b83..45e404f1 100644
--- a/schema.js
+++ b/schema.js
@@ -26,7 +26,7 @@ module.exports = Joi.object().keys({
     localization: Joi.object().keys({
       flipNumberAndStreetCountries: Joi.array().items(Joi.string().regex(/^[A-Z]{3}$/))
     }).unknown(false),
-    pipService: Joi.string().uri({ scheme: /https?/ }),
+    pipService: Joi.any().forbidden(), // got moved to services
     placeholderService: Joi.any().forbidden(), // got moved to services
     services: Joi.object().keys({
       pip: Joi.object().keys({
diff --git a/test/unit/schema.js b/test/unit/schema.js
index d127bc77..6f593094 100644
--- a/test/unit/schema.js
+++ b/test/unit/schema.js
@@ -407,37 +407,15 @@ module.exports.tests.api_validation = (test, common) => {
 
   });
 
-  test('non-string api.pipService should throw error', (t) => {
-    [null, 17, {}, [], true].forEach((value) => {
-      var config = {
-        api: {
-          version: 'version value',
-          indexName: 'index name value',
-          host: 'host value',
-          pipService: value
-        },
-        esclient: {}
-      };
-
-      const result = Joi.validate(config, schema);
-
-      t.equals(result.error.details.length, 1);
-      t.equals(result.error.details[0].message, '"pipService" must be a string');
-
-    });
-
-    t.end();
-
-  });
-
-  test('non-http/https api.pipService should throw error', (t) => {
-    ['ftp', 'git', 'unknown'].forEach((scheme) => {
+  // api.placeholderService has been moved to api.services.placeholder.url
+  test('any api.placeholderService value should be disallowed', (t) => {
+    [null, 17, {}, [], true, 'http://localhost'].forEach((value) => {
       var config = {
         api: {
           version: 'version value',
           indexName: 'index name value',
           host: 'host value',
-          pipService: `${scheme}://localhost`
+          placeholderService: value
         },
         esclient: {}
       };
@@ -445,29 +423,7 @@ module.exports.tests.api_validation = (test, common) => {
       const result = Joi.validate(config, schema);
 
       t.equals(result.error.details.length, 1);
-      t.equals(result.error.details[0].message, '"pipService" must be a valid uri with a scheme matching the https? pattern');
-
-    });
-
-    t.end();
-
-  });
-
-  test('http/https api.pipService should not throw error', (t) => {
-    ['http', 'https'].forEach((scheme) => {
-      var config = {
-        api: {
-          version: 'version value',
-          indexName: 'index name value',
-          host: 'host value',
-          pipService: `${scheme}://localhost`
-        },
-        esclient: {}
-      };
-
-      const result = Joi.validate(config, schema);
-
-      t.notOk(result.error);
+      t.equals(result.error.details[0].message, '"placeholderService" is not allowed');
 
     });
 
@@ -475,15 +431,15 @@ module.exports.tests.api_validation = (test, common) => {
 
   });
 
-  // api.placeholderService has been moved to api.services.placeholder.url
-  test('any api.placeholderService value should be disallowed', (t) => {
+  // api.pipService has been moved to api.services.pip.url
+  test('any api.pipService value should be disallowed', (t) => {
     [null, 17, {}, [], true, 'http://localhost'].forEach((value) => {
       var config = {
         api: {
           version: 'version value',
           indexName: 'index name value',
           host: 'host value',
-          placeholderService: value
+          pipService: value
         },
         esclient: {}
       };
@@ -491,7 +447,7 @@ module.exports.tests.api_validation = (test, common) => {
       const result = Joi.validate(config, schema);
 
       t.equals(result.error.details.length, 1);
-      t.equals(result.error.details[0].message, '"placeholderService" is not allowed');
+      t.equals(result.error.details[0].message, '"pipService" is not allowed');
 
     });
 

From 88882d01999e6edc0e8041d041bf4629517a73a5 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Mon, 5 Jun 2017 17:18:55 -0400
Subject: [PATCH 13/17] added services.pip to successful test

---
 test/unit/schema.js | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/test/unit/schema.js b/test/unit/schema.js
index 6f593094..d33e7984 100644
--- a/test/unit/schema.js
+++ b/test/unit/schema.js
@@ -20,6 +20,9 @@ module.exports.tests.completely_valid = (test, common) => {
         },
         requestRetries: 19,
         services: {
+          pip: {
+            url: 'http://locahost'
+          },
           placeholder: {
             url: 'http://locahost'
           }

From 2bc12d0d7259db04c61a196d60184e5b01f6808a Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Mon, 5 Jun 2017 23:58:25 -0400
Subject: [PATCH 14/17] added support for pip retries and timeout in schema

---
 schema.js           |   4 +-
 test/unit/schema.js | 175 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 178 insertions(+), 1 deletion(-)

diff --git a/schema.js b/schema.js
index 45e404f1..f9d033c6 100644
--- a/schema.js
+++ b/schema.js
@@ -30,7 +30,9 @@ module.exports = Joi.object().keys({
     placeholderService: Joi.any().forbidden(), // got moved to services
     services: Joi.object().keys({
       pip: Joi.object().keys({
-        url: Joi.string().uri({ scheme: /https?/ })
+        url: Joi.string().uri({ scheme: /https?/ }),
+        timeout: Joi.number().integer().optional().default(250).min(0),
+        retries: Joi.number().integer().optional().default(3).min(0),
       }).unknown(false).requiredKeys('url'),
       placeholder: Joi.object().keys({
         url: Joi.string().uri({ scheme: /https?/ })
diff --git a/test/unit/schema.js b/test/unit/schema.js
index d33e7984..8b9485be 100644
--- a/test/unit/schema.js
+++ b/test/unit/schema.js
@@ -588,6 +588,29 @@ module.exports.tests.placeholder_service_validation = (test, common) => {
 };
 
 module.exports.tests.pip_service_validation = (test, common) => {
+  test('timeout and retries not specified should default to 250 and 3', (t) => {
+    const config = {
+      api: {
+        version: 'version value',
+        indexName: 'index name value',
+        host: 'host value',
+        services: {
+          pip: {
+            url: 'http://localhost'
+          }
+        }
+      },
+      esclient: {}
+    };
+
+    const result = Joi.validate(config, schema);
+
+    t.equals(result.value.api.services.pip.timeout, 250);
+    t.equals(result.value.api.services.pip.retries, 3);
+    t.end();
+
+  });
+
   test('when api.services.pip is defined, url is required', (t) => {
     var config = {
       api: {
@@ -688,6 +711,158 @@ module.exports.tests.pip_service_validation = (test, common) => {
 
   });
 
+  test('non-number timeout should throw error', (t) => {
+    [null, 'string', {}, [], false].forEach((value) => {
+      const config = {
+        api: {
+          version: 'version value',
+          indexName: 'index name value',
+          host: 'host value',
+          services: {
+            pip: {
+              url: 'http://localhost',
+              timeout: value
+            }
+          }
+        },
+        esclient: {}
+      };
+
+      const result = Joi.validate(config, schema);
+
+      t.equals(result.error.details.length, 1);
+      t.equals(result.error.details[0].message, '"timeout" must be a number');
+
+    });
+
+    t.end();
+
+  });
+
+  test('non-integer timeout should throw error', (t) => {
+    const config = {
+      api: {
+        version: 'version value',
+        indexName: 'index name value',
+        host: 'host value',
+        services: {
+          pip: {
+            url: 'http://localhost',
+            timeout: 17.3
+          }
+        }
+      },
+      esclient: {}
+    };
+
+    const result = Joi.validate(config, schema);
+
+    t.equals(result.error.details.length, 1);
+    t.equals(result.error.details[0].message, '"timeout" must be an integer');
+    t.end();
+
+  });
+
+  test('negative timeout should throw error', (t) => {
+    const config = {
+      api: {
+        version: 'version value',
+        indexName: 'index name value',
+        host: 'host value',
+        services: {
+          pip: {
+            url: 'http://localhost',
+            timeout: -1
+          }
+        }
+      },
+      esclient: {}
+    };
+
+    const result = Joi.validate(config, schema);
+
+    t.equals(result.error.details.length, 1);
+    t.equals(result.error.details[0].message, '"timeout" must be larger than or equal to 0');
+    t.end();
+
+  });
+
+  test('non-number retries should throw error', (t) => {
+    [null, 'string', {}, [], false].forEach((value) => {
+      const config = {
+        api: {
+          version: 'version value',
+          indexName: 'index name value',
+          host: 'host value',
+          services: {
+            pip: {
+              url: 'http://localhost',
+              retries: value
+            }
+          }
+        },
+        esclient: {}
+      };
+
+      const result = Joi.validate(config, schema);
+
+      t.equals(result.error.details.length, 1);
+      t.equals(result.error.details[0].message, '"retries" must be a number');
+
+    });
+
+    t.end();
+
+  });
+
+  test('non-integer retries should throw error', (t) => {
+    const config = {
+      api: {
+        version: 'version value',
+        indexName: 'index name value',
+        host: 'host value',
+        services: {
+          pip: {
+            url: 'http://localhost',
+            retries: 17.3
+          }
+        }
+      },
+      esclient: {}
+    };
+
+    const result = Joi.validate(config, schema);
+
+    t.equals(result.error.details.length, 1);
+    t.equals(result.error.details[0].message, '"retries" must be an integer');
+    t.end();
+
+  });
+
+  test('negative retries should throw error', (t) => {
+    const config = {
+      api: {
+        version: 'version value',
+        indexName: 'index name value',
+        host: 'host value',
+        services: {
+          pip: {
+            url: 'http://localhost',
+            retries: -1
+          }
+        }
+      },
+      esclient: {}
+    };
+
+    const result = Joi.validate(config, schema);
+
+    t.equals(result.error.details.length, 1);
+    t.equals(result.error.details[0].message, '"retries" must be larger than or equal to 0');
+    t.end();
+
+  });
+
 };
 
 module.exports.tests.esclient_validation = (test, common) => {

From b33ad6c7aec42edc40f31d43b50de61894f07065 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Tue, 6 Jun 2017 10:38:51 -0400
Subject: [PATCH 15/17] updated README for pip/placeholder configuration

---
 README.md | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index ff7d003f..b55e3a1e 100644
--- a/README.md
+++ b/README.md
@@ -45,7 +45,7 @@ The API recognizes the following properties under the top-level `api` key in you
 |`legacyUrl`|*no*||the url to redirect to in case the user does not specify a version such as `v1`
 |`relativeScores`|*no*|true|if set to true, confidence scores will be normalized, realistically at this point setting this to false is not tested or desirable
 |`accessLog`|*no*||name of the format to use for access logs; may be any one of the [predefined values](https://github.com/expressjs/morgan#predefined-formats) in the `morgan` package. Defaults to `"common"`; if set to `false`, or an otherwise falsy value, disables access-logging entirely.|
-|`pipService`|*yes*||full url to the pip service to be used for coarse reverse queries. if missing, which is not recommended, the service will default to using nearby lookups instead of point-in-polygon.|
+|`services`|*no*||service definitions for [point-in-polygon](https://github.com/pelias/pip-service) and [placholder](https://github.com/pelias/placeholder) services.  If missing (which is not recommended), the new point-in-polygon and placeholder services will not be called.|
 
 Example configuration file would look something like this:
 
@@ -68,7 +68,14 @@ Example configuration file would look something like this:
     "legacyUrl": "pelias.mapzen.com",
     "relativeScores": true,
     "textAnalyzer": "libpostal",
-    "pipService": "http://mypipservice.com/3000"
+    "services": {
+      "pip": {
+        "url": "http://mypipservice.com:3000"
+      },
+      "placeholder": {
+        "url": "http://myplaceholderservice.com:5000"
+      }
+    }
   },
   "interpolation": {
     "client": {

From 28b459d9baf88d743dbc11cc071435fb33fc0f89 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Tue, 6 Jun 2017 11:12:03 -0400
Subject: [PATCH 16/17] allow api.pipService even though it will be ignored

---
 schema.js           | 2 +-
 test/unit/schema.js | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/schema.js b/schema.js
index f9d033c6..6c9de60c 100644
--- a/schema.js
+++ b/schema.js
@@ -26,7 +26,7 @@ module.exports = Joi.object().keys({
     localization: Joi.object().keys({
       flipNumberAndStreetCountries: Joi.array().items(Joi.string().regex(/^[A-Z]{3}$/))
     }).unknown(false),
-    pipService: Joi.any().forbidden(), // got moved to services
+    pipService: Joi.any(), // got moved to services, ignored for now
     placeholderService: Joi.any().forbidden(), // got moved to services
     services: Joi.object().keys({
       pip: Joi.object().keys({
diff --git a/test/unit/schema.js b/test/unit/schema.js
index 8b9485be..ccd9bec0 100644
--- a/test/unit/schema.js
+++ b/test/unit/schema.js
@@ -435,7 +435,7 @@ module.exports.tests.api_validation = (test, common) => {
   });
 
   // api.pipService has been moved to api.services.pip.url
-  test('any api.pipService value should be disallowed', (t) => {
+  test('any api.pipService value should be allowed', (t) => {
     [null, 17, {}, [], true, 'http://localhost'].forEach((value) => {
       var config = {
         api: {
@@ -449,8 +449,7 @@ module.exports.tests.api_validation = (test, common) => {
 
       const result = Joi.validate(config, schema);
 
-      t.equals(result.error.details.length, 1);
-      t.equals(result.error.details[0].message, '"pipService" is not allowed');
+      t.notOk(result.error);
 
     });
 

From ff39600de68c78f73abedbeb9024c8dcfa203dd8 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Tue, 6 Jun 2017 11:15:17 -0400
Subject: [PATCH 17/17] removed new

---
 README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index b55e3a1e..e066a575 100644
--- a/README.md
+++ b/README.md
@@ -45,7 +45,7 @@ The API recognizes the following properties under the top-level `api` key in you
 |`legacyUrl`|*no*||the url to redirect to in case the user does not specify a version such as `v1`
 |`relativeScores`|*no*|true|if set to true, confidence scores will be normalized, realistically at this point setting this to false is not tested or desirable
 |`accessLog`|*no*||name of the format to use for access logs; may be any one of the [predefined values](https://github.com/expressjs/morgan#predefined-formats) in the `morgan` package. Defaults to `"common"`; if set to `false`, or an otherwise falsy value, disables access-logging entirely.|
-|`services`|*no*||service definitions for [point-in-polygon](https://github.com/pelias/pip-service) and [placholder](https://github.com/pelias/placeholder) services.  If missing (which is not recommended), the new point-in-polygon and placeholder services will not be called.|
+|`services`|*no*||service definitions for [point-in-polygon](https://github.com/pelias/pip-service) and [placholder](https://github.com/pelias/placeholder) services.  If missing (which is not recommended), the point-in-polygon and placeholder services will not be called.|
 
 Example configuration file would look something like this: