return do-nothing PiP lookup function if config lacks `pipService`

- added URI-formatted `api.pipService` config support in schema
- added predicate that determines whether pipService is enabled/disabled
- reworked should-execute conditions for non-coarse reverse

controller/predicates/is_pip_service_enabled.js

@@ -0,0 +1,9 @@
+const _ = require('lodash');
+
+module.exports = (uri) => {
+  // this predicate relies upon the fact that the schema has already validated
+  //  that api.pipService is a URI-formatted string
+  return (request, response) => {
+    return uri !== undefined;
+  };
+};

routes/v1.js

@@ -60,9 +60,6 @@ var postProc = {
   assignLabels: require('../middleware/assignLabels')
 };
 
-// make configurable to return do-nothing service if PIP URL not set
-const pipService = require('../service/pointinpolygon')('http://localhost:3102');
-
 // predicates that drive whether controller/search runs
 const hasData = require('../controller/predicates/has_data');
 const hasErrors = require('../controller/predicates/has_errors');
@@ -80,6 +77,9 @@ const hasDataOrErrors = any(hasData, hasErrors);
 function addRoutes(app, peliasConfig) {
   const esclient = elasticsearch.Client(peliasConfig.esclient);
 
+  const isPipServiceEnabled = require('../controller/predicates/is_pip_service_enabled')(peliasConfig.api.pipService);
+  const pipService = require('../service/pointinpolygon')(peliasConfig.api.pipService);
+
   var base = '/v1/';
 
   /** ------------------------- routers ------------------------- **/
@@ -151,8 +151,23 @@ function addRoutes(app, peliasConfig) {
     reverse: createRouter([
       sanitizers.reverse.middleware,
       middleware.calcSize(),
-      controllers.coarse_reverse(pipService, all(isCoarseReverse, not(hasErrors))),
+      controllers.coarse_reverse(pipService,
-      controllers.search(peliasConfig.api, esclient, queries.reverse, not(any(hasDataOrErrors, isCoarseReverse))),
+        all(
+          not(hasErrors), isPipServiceEnabled, isCoarseReverse
+        )
+      ),
+      controllers.search(peliasConfig.api, esclient, queries.reverse,
+        // execute under the following conditions:
+        // - there are no errors or data
+        // - request is not coarse OR pip service is disabled
+        all(
+          not(hasDataOrErrors),
+          any(
+            not(isCoarseReverse),
+            not(isPipServiceEnabled)
+          )
+        )
+      ),
       postProc.distances('point.'),
       // reverse confidence scoring depends on distance from origin
       //  so it must be calculated first

schema.js

@@ -25,7 +25,8 @@ module.exports = Joi.object().keys({
     requestRetries: Joi.number().integer().min(0),
     localization: Joi.object().keys({
       flipNumberAndStreetCountries: Joi.array().items(Joi.string().regex(/^[A-Z]{3}$/))
-    }).unknown(false)
+    }).unknown(false),
+    pipService: Joi.string().uri({ scheme: /https?/ })
 
   }).requiredKeys('version', 'indexName', 'host').unknown(true),
   esclient: Joi.object().keys({

service/pointinpolygon.js

@@ -1,8 +1,12 @@
 const logger = require( 'pelias-logger' ).get( 'pointinpolygon' );
 const request = require('request');
+const _ = require('lodash');
 
 module.exports = (url) => {
-  function service( centroid, callback ){
+  if (!_.isEmpty(url)) {
+    logger.info(`using point-in-polygon service at ${url}`);
+
+    return function pointinpolygon( centroid, callback ) {
       const requestUrl = `${url}/${centroid.lon}/${centroid.lat}`;
 
       request.get(requestUrl, (err, response, body) => {
@@ -26,8 +30,15 @@ module.exports = (url) => {
         }
       });
 
-  }
+    };
+
+  } else {
+    logger.warn('point-in-polygon service disabled');
 
-  return service;
+    return (centroid, callback) => {
+      callback(`point-in-polygon service disabled, unable to resolve ${JSON.stringify(centroid)}`);
+    };
+
+  }
 
 };

test/unit/controller/predicates/is_pip_service_enabled.js

@@ -0,0 +1,42 @@
+'use strict';
+
+const _ = require('lodash');
+const is_pip_service_enabled = require('../../../../controller/predicates/is_pip_service_enabled');
+
+module.exports.tests = {};
+
+module.exports.tests.interface = (test, common) => {
+  test('valid interface', (t) => {
+    t.equal(typeof is_pip_service_enabled, 'function', 'is_pip_service_enabled  is a function');
+    t.equal(typeof is_pip_service_enabled(), 'function', 'is_pip_service_enabled() is a function');
+    t.end();
+  });
+};
+
+module.exports.tests.true_conditions = (test, common) => {
+  test('string uri should return true', (t) => {
+    t.ok(is_pip_service_enabled('pip uri')());
+    t.end();
+
+  });
+
+};
+
+module.exports.tests.false_conditions = (test, common) => {
+  test('undefined uri should return false', (t) => {
+    t.notOk(is_pip_service_enabled()());
+    t.end();
+
+  });
+
+};
+
+module.exports.all = (tape, common) => {
+  function test(name, testFunction) {
+    return tape(`GET /is_pip_service_enabled ${name}`, testFunction);
+  }
+
+  for( const testCase in module.exports.tests ){
+    module.exports.tests[testCase](test, common);
+  }
+};

test/unit/run.js

@@ -18,6 +18,7 @@ var tests = [
   require('./controller/predicates/has_data'),
   require('./controller/predicates/has_errors'),
   require('./controller/predicates/is_coarse_reverse'),
+  require('./controller/predicates/is_pip_service_enabled'),
   require('./helper/diffPlaces'),
   require('./helper/geojsonify'),
   require('./helper/logging'),

test/unit/schema.js

@@ -367,6 +367,86 @@ 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: {}
+      };
+
+      t.throws(validate.bind(null, config), /"pipService" must be a string/);
+
+    });
+
+    t.end();
+
+  });
+
+  test('non-URI-formatted api.pipService should throw error', (t) => {
+    ['this is not a URI'].forEach((value) => {
+      var config = {
+        api: {
+          version: 'version value',
+          indexName: 'index name value',
+          host: 'host value',
+          pipService: value
+        },
+        esclient: {}
+      };
+
+      t.throws(validate.bind(null, config), /"pipService" must be a valid uri/);
+
+    });
+
+    t.end();
+
+  });
+
+  test('non-http/https api.pipService should throw error', (t) => {
+    ['ftp', 'git', 'unknown'].forEach((scheme) => {
+      var config = {
+        api: {
+          version: 'version value',
+          indexName: 'index name value',
+          host: 'host value',
+          pipService: `${scheme}://localhost`
+        },
+        esclient: {}
+      };
+
+      t.throws(validate.bind(null, config), /"pipService" must be a valid uri/);
+
+    });
+
+    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: {}
+      };
+
+      t.doesNotThrow(validate.bind(null, config), `${scheme} should be allowed`);
+
+    });
+
+    t.end();
+
+  });
+
 };
 
 module.exports.tests.esclient_validation = (test, common) => {

test/unit/service/pointinpolygon.js

@@ -17,6 +17,26 @@ module.exports.tests.interface = (test, common) => {
   });
 };
 
+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 }, (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();
+    });
+
+  });
+
+};
+
 module.exports.tests.success = (test, common) => {
   test('lat and lon should be passed to server', (t) => {
     const pipServer = require('express')();
@@ -28,12 +48,21 @@ module.exports.tests.success = (test, common) => {
     });
 
     const server = pipServer.listen();
+    const port = server.address().port;
+
+    const logger = require('pelias-mock-logger')();
 
-    const service = setup(`http://localhost:${server.address().port}`);
+    const service = proxyquire('../../../service/pointinpolygon', {
+      'pelias-logger': logger
+    })(`http://localhost:${port}`);
 
     service({ lat: 12.121212, lon: 21.212121}, (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();