pelias
/
api
mirror of https://github.com/pelias/api.git
1
0
Fork 0
Code Issues Releases Activity
Browse Source

Merge pull request #856 from pelias/staging 

Merge staging into production
pull/1005/head
Julian Simioni 8 years ago committed by GitHub
parent
82e11a014f 4774ee2fc4
commit
510cde32d4
8 changed files with 383 additions and 78 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 3
      controller/coarse_reverse.js
  2. 15
      middleware/normalizeParentIds.js
  3. 2
      package.json
  4. 58
      service/pointinpolygon.js
  5. 60
      test/unit/controller/coarse_reverse.js
  6. 43
      test/unit/fixture/reverse_boundary_circle.js
  7. 106
      test/unit/middleware/normalizeParentIds.js
  8. 174
      test/unit/service/pointinpolygon.js

3
controller/coarse_reverse.js
Unescape View File

@ -1,6 +1,7 @@
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',
@ -88,7 +89,7 @@ function setup(service, should_execute) {
lon: req.clean['point.lon']
};
service(centroid, (err, results) => {
service(centroid, isDNT(req), (err, results) => {
// if there's an error, log it and bail
if (err) {
logger.error(err);

15
middleware/normalizeParentIds.js
Unescape View File

@ -32,7 +32,16 @@ function normalizeParentIds(place) {
if (place) {
placeTypes.forEach(function (placeType) {
if (place[placeType] && place[placeType].length > 0 && place[placeType][0]) {
place[placeType + '_gid'] = [ makeNewId(placeType, place[placeType + '_gid']) ];
var source = 'whosonfirst';
// looking forward to the day we can remove all geonames specific hacks, but until then...
// geonames sometimes has its own ids in the parent hierarchy, so it's dangerous to assume that
// it's always WOF ids and hardcode to that
if (place.source === 'geonames' && place.source_id === place[placeType + '_gid'][0]) {
source = place.source;
}
place[placeType + '_gid'] = [ makeNewId(source, placeType, place[placeType + '_gid']) ];
}
});
}
@ -48,12 +57,12 @@ function normalizeParentIds(place) {
* @param {number} id
* @return {string}
*/
function makeNewId(placeType, id) {
function makeNewId(source, placeType, id) {
if (!id) {
return;
}
var doc = new Document('whosonfirst', placeType, id);
var doc = new Document(source, placeType, id);
return doc.getGid();
}

2
package.json
Unescape View File

@ -75,7 +75,7 @@
"difflet": "^1.0.1",
"istanbul": "^0.4.2",
"jshint": "^2.5.6",
"npm-check": "^5.4.0",
"npm-check": "git://github.com/orangejulius/npm-check.git#disable-update-check",
"nsp": "^2.2.0",
"precommit-hook": "^3.0.0",
"proxyquire": "^1.7.10",

58
service/pointinpolygon.js
Unescape View File

@ -2,14 +2,51 @@ 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, callback ) {
return function pointinpolygon( centroid, do_not_track, callback ) {
const requestUrl = `${url}/${centroid.lon}/${centroid.lat}`;
request.get(requestUrl, (err, response, body) => {
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);
@ -20,13 +57,17 @@ module.exports = (url) => {
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}`);
const parseMsg = parseErrorMessage(requestUrl, body, do_not_track);
logger.error(parseMsg);
callback(parseMsg);
}
}
else {
logger.error(`${requestUrl} returned status ${response.statusCode}: ${body}`);
callback(`${requestUrl} returned status ${response.statusCode}: ${body}`);
const errorMsg = serviceErrorMessage(requestUrl, response.statusCode, body, do_not_track);
logger.error(errorMsg);
callback(errorMsg);
}
});
@ -35,8 +76,9 @@ module.exports = (url) => {
} else {
logger.warn('point-in-polygon service disabled');
return (centroid, callback) => {
callback(`point-in-polygon service disabled, unable to resolve ${JSON.stringify(centroid)}`);
return (centroid, do_not_track, callback) => {
callback(`point-in-polygon service disabled, unable to resolve ` +
(do_not_track ? 'centroid' : JSON.stringify(centroid)));
};
}

60
test/unit/controller/coarse_reverse.js
Unescape View File

@ -46,7 +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, callback) => {
const service = (point, do_not_track, callback) => {
t.equals(do_not_track, 'do_not_track value');
callback('this is an error');
};
@ -54,7 +55,12 @@ module.exports.tests.error_conditions = (test, common) => {
const should_execute = () => { return true; };
const controller = proxyquire('../../../controller/coarse_reverse', {
'pelias-logger': logger
'pelias-logger': logger,
'../helper/logging': {
isDNT: () => {
return 'do_not_track value';
}
}
})(service, should_execute);
const req = {
@ -86,7 +92,8 @@ 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, callback) => {
const service = (point, do_not_track, callback) => {
t.equals(do_not_track, 'do_not_track value');
const results = {
neighbourhood: [
{
@ -146,7 +153,12 @@ module.exports.tests.success_conditions = (test, common) => {
const should_execute = () => { return true; };
const controller = proxyquire('../../../controller/coarse_reverse', {
'pelias-logger': logger
'pelias-logger': logger,
'../helper/logging': {
isDNT: () => {
return 'do_not_track value';
}
}
})(service, should_execute);
const req = {
@ -235,7 +247,8 @@ module.exports.tests.success_conditions = (test, common) => {
});
test('layers missing from results should be ignored', (t) => {
const service = (point, callback) => {
const service = (point, do_not_track, callback) => {
t.equals(do_not_track, 'do_not_track value');
const results = {
neighbourhood: [
{
@ -258,7 +271,12 @@ module.exports.tests.success_conditions = (test, common) => {
const should_execute = () => { return true; };
const controller = proxyquire('../../../controller/coarse_reverse', {
'pelias-logger': logger
'pelias-logger': logger,
'../helper/logging': {
isDNT: () => {
return 'do_not_track value';
}
}
})(service, should_execute);
const req = {
@ -319,7 +337,8 @@ module.exports.tests.success_conditions = (test, common) => {
});
test('most granular layer missing centroid should not set', (t) => {
const service = (point, callback) => {
const service = (point, do_not_track, callback) => {
t.equals(do_not_track, 'do_not_track value');
const results = {
neighbourhood: [
{
@ -338,7 +357,12 @@ module.exports.tests.success_conditions = (test, common) => {
const should_execute = () => { return true; };
const controller = proxyquire('../../../controller/coarse_reverse', {
'pelias-logger': logger
'pelias-logger': logger,
'../helper/logging': {
isDNT: () => {
return 'do_not_track value';
}
}
})(service, should_execute);
const req = {
@ -395,7 +419,8 @@ module.exports.tests.success_conditions = (test, common) => {
});
test('most granular layer missing bounding_box should not set', (t) => {
const service = (point, callback) => {
const service = (point, do_not_track, callback) => {
t.equals(do_not_track, 'do_not_track value');
const results = {
neighbourhood: [
{
@ -417,7 +442,12 @@ module.exports.tests.success_conditions = (test, common) => {
const should_execute = () => { return true; };
const controller = proxyquire('../../../controller/coarse_reverse', {
'pelias-logger': logger
'pelias-logger': logger,
'../helper/logging': {
isDNT: () => {
return 'do_not_track value';
}
}
})(service, should_execute);
const req = {
@ -480,7 +510,8 @@ 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, callback) => {
const service = (point, do_not_track, callback) => {
t.equals(do_not_track, 'do_not_track value');
// response without neighbourhood results
const results = {
borough: [
@ -528,7 +559,12 @@ module.exports.tests.failure_conditions = (test, common) => {
const should_execute = () => { return true; };
const controller = proxyquire('../../../controller/coarse_reverse', {
'pelias-logger': logger
'pelias-logger': logger,
'../helper/logging': {
isDNT: () => {
return 'do_not_track value';
}
}
})(service, should_execute);
const req = {

43
test/unit/fixture/reverse_boundary_circle.js
Unescape View File

@ -1,43 +0,0 @@
var vs = require('../../../query/reverse_defaults');
module.exports = {
'query': {
'filtered': {
'query': {
'bool': {}
},
'filter': {
'bool': {
'must': [
{
'geo_distance': {
'distance': vs.distance,
'distance_type': 'plane',
'optimize_bbox': 'indexed',
'center_point': {
'lat': 29.49136,
'lon': -82.50622
}
}
}
]
}
}
}
},
'sort': [
'_score',
{
'_geo_distance': {
'center_point': {
'lat': 29.49136,
'lon': -82.50622
},
'order': 'asc',
'distance_type': 'plane'
}
}
],
'size': vs.size,
'track_scores': true
};

106
test/unit/middleware/normalizeParentIds.js
Unescape View File

@ -77,6 +77,112 @@ module.exports.tests.interface = function(test, common) {
});
});
test('Geonames ids do not override parent hierarchy with WOF equivalents', function(t) {
var input = {
data: [{
'parent': {
'country_id': [ '85633793' ],
'country': [ 'United States' ],
'country_a': [ 'USA' ],
'region_id': [ '85688579' ],
'region': [ 'Mississippi' ],
'region_a': [ 'MS' ]
},
'source': 'geonames',
'source_id': '4436296',
'layer': 'region',
'country': [ 'United States' ],
'country_a': [ 'USA' ],
'country_gid': [ '85633793' ],
'region': [ 'Mississippi' ],
'region_a': [ 'MS' ],
'region_gid': [ '85688579' ]
}]
};
var expected = {
data: [{
'parent': {
'country_id': [ '85633793' ],
'country': [ 'United States' ],
'country_a': [ 'USA' ],
'region_id': [ '85688579' ],
'region': [ 'Mississippi' ],
'region_a': [ 'MS' ]
},
'layer': 'region',
'source': 'geonames',
'source_id': '4436296',
'country': ['United States'],
'country_gid': ['whosonfirst:country:85633793'],
'country_a': ['USA'],
'region': ['Mississippi'],
'region_gid': ['whosonfirst:region:85688579'],
'region_a': ['MS']
}]
};
normalizer({}, input, function () {
t.deepEqual(input, expected);
t.end();
});
});
test('Geonames ids do not override parent hierarchy with WOF equivalents', function(t) {
var input = {
data: [{
'parent': {
'country_id': [ '85633793' ],
'country': [ 'United States' ],
'country_a': ['USA'],
'region_id': [ '4436296' ],
'region': [ 'Mississippi' ],
'region_a': [ 'MS' ]
},
'source': 'geonames',
'source_id': '4436296',
'layer': 'region',
'country': [ 'United States' ],
'country_a': [ 'USA' ],
'country_gid': ['85633793'],
'region': [ 'Mississippi' ],
'region_a': [ 'MS' ],
'region_gid': [ '4436296' ]
}]
};
var expected = {
data: [{
'parent': {
'country_id': [ '85633793' ],
'country': [ 'United States' ],
'country_a': [ 'USA' ],
'region_id': [ '4436296' ],
'region': [ 'Mississippi' ],
'region_a': [ 'MS' ]
},
'layer': 'region',
'source': 'geonames',
'source_id': '4436296',
'country': ['United States'],
'country_gid': ['whosonfirst:country:85633793'],
'country_a': ['USA'],
'region': ['Mississippi'],
'region_gid': ['geonames:region:4436296'],
'region_a': ['MS']
}]
};
normalizer({}, input, function () {
t.deepEqual(input, expected);
t.end();
});
});
};
module.exports.all = function (tape, common) {

174
test/unit/service/pointinpolygon.js
Unescape View File

@ -25,7 +25,7 @@ module.exports.tests.do_nothing_service = (test, common) => {
'pelias-logger': logger
})();
service({ lat: 12.121212, lon: 21.212121 }, (err) => {
service({ lat: 12.121212, lon: 21.212121 }, false, (err) => {
t.deepEquals(logger.getWarnMessages(), [
'point-in-polygon service disabled'
]);
@ -35,6 +35,23 @@ module.exports.tests.do_nothing_service = (test, common) => {
});
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) => {
@ -56,13 +73,81 @@ module.exports.tests.success = (test, common) => {
'pelias-logger': logger
})(`http://localhost:${port}`);
service({ lat: 12.121212, lon: 21.212121}, (err, results) => {
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();
@ -92,10 +177,46 @@ module.exports.tests.failure = (test, common) => {
'pelias-logger': logger
})(`http://localhost:${port}`);
service({ lat: 12.121212, lon: 21.212121}, (err, results) => {
t.equals(err, `http://localhost:${port}/21.212121/12.121212 returned status 200 but with non-JSON response: this is not JSON`);
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(`http://localhost:${port}/21.212121/12.121212: could not parse response body: this is not JSON`));
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();
@ -117,7 +238,7 @@ module.exports.tests.failure = (test, common) => {
'pelias-logger': logger
})(`http://localhost:${port}`);
service({ lat: 12.121212, lon: 21.212121}, (err, results) => {
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');
@ -144,10 +265,43 @@ module.exports.tests.failure = (test, common) => {
'pelias-logger': logger
})(`http://localhost:${port}`);
service({ lat: 12.121212, lon: 21.212121}, (err, results) => {
t.equals(err, `http://localhost:${port}/21.212121/12.121212 returned status 400: a bad request was made`);
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(`http://localhost:${port}/21.212121/12.121212 returned status 400: a bad request was made`));
t.ok(logger.isErrorMessage(expectedErrorMsg));
t.end();
server.close();

Write Preview
Loading…
Cancel
Save