diff --git a/README.md b/README.md index ff7d003f..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.| -|`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 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": { 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/package.json b/package.json index 66fe2989..0a3eb9af 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,6 @@ "pelias-sorting": "1.0.1", "pelias-text-analyzer": "1.8.3", "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..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 @@ -88,11 +89,11 @@ 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 pipConfiguration = new PointInPolygon(_.defaultTo(peliasConfig.api.services.pip, {})); + const pipService = serviceWrapper(pipConfiguration); + const isPipServiceEnabled = _.constant(pipConfiguration.isEnabled()); - const pipService = require('../service/pointinpolygon')(peliasConfig.api.pipService); - - 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()); diff --git a/schema.js b/schema.js index 003c5fb7..6c9de60c 100644 --- a/schema.js +++ b/schema.js @@ -26,9 +26,14 @@ 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(), // got moved to services, ignored for now placeholderService: Joi.any().forbidden(), // got moved to services services: Joi.object().keys({ + pip: Joi.object().keys({ + 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?/ }) }).unknown(false).requiredKeys('url') diff --git a/service/configurations/PointInPolygon.js b/service/configurations/PointInPolygon.js new file mode 100644 index 00000000..66a766f9 --- /dev/null +++ b/service/configurations/PointInPolygon.js @@ -0,0 +1,21 @@ +'use strict'; + +const url = require('url'); + +const _ = require('lodash'); + +const ServiceConfiguration = require('pelias-microservice-wrapper').ServiceConfiguration; + +class PointInPolygon extends ServiceConfiguration { + constructor(o) { + super('pip', o); + } + + getUrl(req) { + // use resolve to eliminate possibility of duplicate /'s in URL + return url.resolve(this.baseUrl, `${req.clean['point.lon']}/${req.clean['point.lat']}`); + } + +} + +module.exports = PointInPolygon; 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/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'] } }; 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/schema.js b/test/unit/schema.js index cd53fdce..ccd9bec0 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' } @@ -407,14 +410,15 @@ module.exports.tests.api_validation = (test, common) => { }); - test('non-string api.pipService should throw error', (t) => { - [null, 17, {}, [], true].forEach((value) => { + // 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: value + placeholderService: value }, esclient: {} }; @@ -422,7 +426,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 string'); + t.equals(result.error.details[0].message, '"placeholderService" is not allowed'); }); @@ -430,22 +434,22 @@ module.exports.tests.api_validation = (test, common) => { }); - test('non-http/https api.pipService should throw error', (t) => { - ['ftp', 'git', 'unknown'].forEach((scheme) => { + // api.pipService has been moved to api.services.pip.url + test('any api.pipService value should be allowed', (t) => { + [null, 17, {}, [], true, 'http://localhost'].forEach((value) => { var config = { api: { version: 'version value', indexName: 'index name value', host: 'host value', - pipService: `${scheme}://localhost` + 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 valid uri with a scheme matching the https? pattern'); + t.notOk(result.error); }); @@ -453,21 +457,75 @@ module.exports.tests.api_validation = (test, common) => { }); - test('http/https api.pipService should not throw error', (t) => { - ['http', 'https'].forEach((scheme) => { +}; + +module.exports.tests.api_services_validation = (test, common) => { + test('unsupported children of api.services should be disallowed', (t) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + services: { + unknown_property: 'value' + } + }, + esclient: {} + }; + + const result = Joi.validate(config, schema); + + t.equals(result.error.details.length, 1); + t.equals(result.error.details[0].message, '"unknown_property" is not allowed'); + t.end(); + + }); + +}; + +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', - pipService: `${scheme}://localhost` + services: { + placeholder: { + url: value + } + } }, esclient: {} }; const result = Joi.validate(config, schema); - t.notOk(result.error); + t.equals(result.error.details.length, 1); + t.equals(result.error.details[0].message, '"url" must be a string'); }); @@ -475,15 +533,18 @@ 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) => { - [null, 17, {}, [], true, 'http://localhost'].forEach((value) => { + 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', - placeholderService: value + services: { + placeholder: { + url: `${scheme}://localhost` + } + } }, esclient: {} }; @@ -491,7 +552,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, '"url" must be a valid uri with a scheme matching the https\? pattern'); }); @@ -499,17 +560,17 @@ module.exports.tests.api_validation = (test, common) => { }); -}; - -module.exports.tests.api_services_validation = (test, common) => { - test('unsupported children of api.services should be disallowed', (t) => { + test('non-url children of api.services.placeholder should be disallowed', (t) => { var config = { api: { version: 'version value', indexName: 'index name value', host: 'host value', services: { - unknown_property: 'value' + placeholder: { + url: 'http://localhost', + unknown_property: 'value' + } } }, esclient: {} @@ -523,16 +584,18 @@ module.exports.tests.api_services_validation = (test, common) => { }); - test('non-url children of api.services.placeholder should be disallowed', (t) => { - var config = { +}; + +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: { - placeholder: { - url: 'http://localhost', - unknown_property: 'value' + pip: { + url: 'http://localhost' } } }, @@ -541,20 +604,20 @@ module.exports.tests.api_services_validation = (test, common) => { const result = Joi.validate(config, schema); - t.equals(result.error.details.length, 1); - t.equals(result.error.details[0].message, '"unknown_property" is not allowed'); + t.equals(result.value.api.services.pip.timeout, 250); + t.equals(result.value.api.services.pip.retries, 3); t.end(); }); - test('when api.services.placeholder is defined, url is required', (t) => { + 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 +632,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 +640,7 @@ module.exports.tests.api_services_validation = (test, common) => { indexName: 'index name value', host: 'host value', services: { - placeholder: { + pip: { url: value } } @@ -596,7 +659,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 +667,7 @@ module.exports.tests.api_services_validation = (test, common) => { indexName: 'index name value', host: 'host value', services: { - placeholder: { + pip: { url: `${scheme}://localhost` } } @@ -623,14 +686,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' } @@ -647,6 +710,158 @@ module.exports.tests.api_services_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) => { diff --git a/test/unit/service/configurations/PointInPolygon.js b/test/unit/service/configurations/PointInPolygon.js new file mode 100644 index 00000000..c754e7f9 --- /dev/null +++ b/test/unit/service/configurations/PointInPolygon.js @@ -0,0 +1,101 @@ +module.exports.tests = {}; + +const PointInPolygon = require('../../../../service/configurations/PointInPolygon'); + +module.exports.tests.all = (test, common) => { + test('getName should return \'pip\'', (t) => { + const configBlob = { + url: 'http://localhost:1234', + timeout: 17, + retries: 19 + }; + + const pointInPolygon = new PointInPolygon(configBlob); + + t.equals(pointInPolygon.getName(), 'pip'); + t.equals(pointInPolygon.getBaseUrl(), 'http://localhost:1234/'); + 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: 'http://localhost:1234' + }; + + 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(); + + }); + + test('getHeaders should return an empty object', (t) => { + const configBlob = { + url: 'http://localhost:1234', + 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: 'http://localhost:1234', + timeout: 17, + retries: 19 + }; + + const pointInPolygon = new PointInPolygon(configBlob); + + t.deepEquals(pointInPolygon.getParameters(), {}); + t.end(); + + }); + + 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) => { + function test(name, testFunction) { + return tape(`SERVICE CONFIGURATION /PointInPolygon ${name}`, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; 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); - } -};