From 1f6b6b1ed0fe8b1fc6b405488440714087154032 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 2 May 2017 15:58:53 -0400 Subject: [PATCH] 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); - } -};