diff --git a/controller/predicates/is_pip_service_enabled.js b/controller/predicates/is_pip_service_enabled.js new file mode 100644 index 00000000..0324da1d --- /dev/null +++ b/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; + }; +}; diff --git a/routes/v1.js b/routes/v1.js index ecb19a3c..46f44893 100644 --- a/routes/v1.js +++ b/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.search(peliasConfig.api, esclient, queries.reverse, not(any(hasDataOrErrors, isCoarseReverse))), + controllers.coarse_reverse(pipService, + 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 diff --git a/schema.js b/schema.js index 61e6c258..78a9141f 100644 --- a/schema.js +++ b/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({ diff --git a/service/pointinpolygon.js b/service/pointinpolygon.js index aaece195..21b6fdcf 100644 --- a/service/pointinpolygon.js +++ b/service/pointinpolygon.js @@ -1,33 +1,44 @@ const logger = require( 'pelias-logger' ).get( 'pointinpolygon' ); const request = require('request'); +const _ = require('lodash'); module.exports = (url) => { - function service( centroid, callback ){ - const requestUrl = `${url}/${centroid.lon}/${centroid.lat}`; - - request.get(requestUrl, (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); + 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) => { + if (err) { + logger.error(JSON.stringify(err)); + callback(err); } - catch (err) { - logger.error(`${requestUrl}: could not parse response body: ${body}`); - callback(`${requestUrl} returned status 200 but with non-JSON response: ${body}`); + else if (response.statusCode === 200) { + try { + const parsed = JSON.parse(body); + callback(err, parsed); + } + catch (err) { + logger.error(`${requestUrl}: could not parse response body: ${body}`); + callback(`${requestUrl} returned status 200 but with non-JSON response: ${body}`); + } } - } - else { - logger.error(`${requestUrl} returned status ${response.statusCode}: ${body}`); - callback(`${requestUrl} returned status ${response.statusCode}: ${body}`); - } - }); + else { + logger.error(`${requestUrl} returned status ${response.statusCode}: ${body}`); + callback(`${requestUrl} returned status ${response.statusCode}: ${body}`); + } + }); - } + }; - return service; + } else { + logger.warn('point-in-polygon service disabled'); + + return (centroid, callback) => { + callback(`point-in-polygon service disabled, unable to resolve ${JSON.stringify(centroid)}`); + }; + + } }; diff --git a/test/unit/controller/predicates/is_pip_service_enabled.js b/test/unit/controller/predicates/is_pip_service_enabled.js new file mode 100644 index 00000000..1d3cd63a --- /dev/null +++ b/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); + } +}; diff --git a/test/unit/run.js b/test/unit/run.js index a20d2d3a..b746d979 100644 --- a/test/unit/run.js +++ b/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'), diff --git a/test/unit/schema.js b/test/unit/schema.js index e1f9cfd2..6ed11add 100644 --- a/test/unit/schema.js +++ b/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) => { diff --git a/test/unit/service/pointinpolygon.js b/test/unit/service/pointinpolygon.js index a85fcd96..ac08b3f7 100644 --- a/test/unit/service/pointinpolygon.js +++ b/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 service = setup(`http://localhost:${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}, (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();