diff --git a/helper/geojsonify.js b/helper/geojsonify.js index dfa177ca..9371d0c7 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -1,68 +1,18 @@ -var GeoJSON = require('geojson'), - extent = require('geojson-extent'), - labelGenerator = require('./labelGenerator'), - logger = require('pelias-logger').get('api'), - type_mapping = require('./type_mapping'), - Document = require('pelias-model').Document, - _ = require('lodash'); +var GeoJSON = require('geojson'); +var extent = require('geojson-extent'); +var labelGenerator = require('./labelGenerator'); +var logger = require('pelias-logger').get('api'); +var type_mapping = require('./type_mapping'); +var _ = require('lodash'); +var addDetails = require('./geojsonify_place_details'); +var addMetaData = require('./geojsonify_meta_data'); -// Properties to be copied -// If a property is identified as a single string, assume it should be presented as a string in response -// If something other than string is desired, use the following structure: { name: 'category', type: 'array' } -var DETAILS_PROPS = [ - 'housenumber', - 'street', - 'postalcode', - { name: 'confidence', type: 'default' }, - 'distance', - 'country', - 'country_gid', - 'country_a', - 'macroregion', - 'macroregion_gid', - 'macroregion_a', - 'region', - 'region_gid', - 'region_a', - 'macrocounty', - 'macrocounty_gid', - 'macrocounty_a', - 'county', - 'county_gid', - 'county_a', - 'localadmin', - 'localadmin_gid', - 'localadmin_a', - 'locality', - 'locality_gid', - 'locality_a', - 'borough', - 'borough_gid', - 'borough_a', - 'neighbourhood', - 'neighbourhood_gid', - { name: 'bounding_box', type: 'default' }, - { name: 'category', type: 'array' } -]; - -function lookupSource(src) { - return src.source; -} - -function lookupSourceId(src) { - return src.source_id; -} - -function lookupLayer(src) { - return src.layer; -} - -function geojsonifyPlaces( docs ){ +function geojsonifyPlaces( params, docs ){ // flatten & expand data for geojson conversion var geodata = docs - .map(geojsonifyPlace) + .map(geojsonifyPlace.bind(null, params)) .filter( function( doc ){ return !!doc; }); @@ -85,7 +35,7 @@ function geojsonifyPlaces( docs ){ return geojson; } -function geojsonifyPlace(place) { +function geojsonifyPlace(params, place) { // something went very wrong if( !place || !place.hasOwnProperty( 'center_point' ) ) { @@ -95,7 +45,8 @@ function geojsonifyPlace(place) { var output = {}; addMetaData(place, output); - addDetails(place, output); + addName(place, output); + addDetails(params, place, output); addLabel(place, output); @@ -108,17 +59,15 @@ function geojsonifyPlace(place) { } /** - * Add details properties + * Validate and add name property * * @param {object} src * @param {object} dst */ -function addDetails(src, dst) { +function addName(src, dst) { // map name if( !src.name || !src.name.default ) { return warning(src); } dst.name = src.name.default; - - copyProperties(src, DETAILS_PROPS, dst); } /** @@ -208,104 +157,6 @@ function computeBBox(geojson, geojsonExtentPoints) { } } -/** - * Copy specified properties from source to dest. - * Ignore missing properties. - * - * @param {object} source - * @param {[]} props - * @param {object} dst - */ -function copyProperties( source, props, dst ) { - props.forEach( function ( prop ) { - - var property = { - name: prop.name || prop, - type: prop.type || 'string' - }; - - var value = null; - if ( source.hasOwnProperty( property.name ) ) { - - switch (property.type) { - case 'string': - value = getStringValue(source[property.name]); - break; - case 'array': - value = getArrayValue(source[property.name]); - break; - // default behavior is to copy property exactly as is - default: - value = source[property.name]; - } - - if (_.isNumber(value) || (value && !_.isEmpty(value))) { - dst[property.name] = value; - } - } - }); -} - -function getStringValue(property) { - // isEmpty check works for all types of values: strings, arrays, objects - if (_.isEmpty(property)) { - return ''; - } - - if (_.isString(property)) { - return property; - } - - // array value, take first item in array (at this time only used for admin values) - if (_.isArray(property)) { - return property[0]; - } - - return _.toString(property); -} - - -function getArrayValue(property) { - // isEmpty check works for all types of values: strings, arrays, objects - if (_.isEmpty(property)) { - return ''; - } - - if (_.isArray(property)) { - return property; - } - - return [property]; -} - -/** - * Create a gid from a document - * @TODO modify all importers to create separate source and layer fields to remove mapping - * - * @param {object} src - */ -function makeGid(src) { - var doc = new Document(lookupSource(src), lookupLayer(src), src._id); - return doc.getGid(); -} - -/** - * Determine and set place id, type, and source - * - * @param {object} src - * @param {object} dst - */ -function addMetaData(src, dst) { - dst.id = src._id; - dst.gid = makeGid(src); - dst.layer = lookupLayer(src); - dst.source = lookupSource(src); - dst.source_id = lookupSourceId(src); - if (src.hasOwnProperty('bounding_box')) { - dst.bounding_box = src.bounding_box; - } -} - /** * emit a warning if the doc format is invalid * @@ -317,4 +168,4 @@ function warning( doc ) { } -module.exports.search = geojsonifyPlaces; +module.exports = geojsonifyPlaces; diff --git a/helper/geojsonify_meta_data.js b/helper/geojsonify_meta_data.js new file mode 100644 index 00000000..7fdf0394 --- /dev/null +++ b/helper/geojsonify_meta_data.js @@ -0,0 +1,42 @@ +var Document = require('pelias-model').Document; + +/** + * Determine and set place id, type, and source + * + * @param {object} src + * @param {object} dst + */ +function addMetaData(src, dst) { + dst.id = src._id; + dst.gid = makeGid(src); + dst.layer = lookupLayer(src); + dst.source = lookupSource(src); + dst.source_id = lookupSourceId(src); + if (src.hasOwnProperty('bounding_box')) { + dst.bounding_box = src.bounding_box; + } +} + +/** + * Create a gid from a document + * + * @param {object} src + */ +function makeGid(src) { + var doc = new Document(lookupSource(src), lookupLayer(src), src._id); + return doc.getGid(); +} + +function lookupSource(src) { + return src.source; +} + +function lookupSourceId(src) { + return src.source_id; +} + +function lookupLayer(src) { + return src.layer; +} + +module.exports = addMetaData; \ No newline at end of file diff --git a/helper/geojsonify_place_details.js b/helper/geojsonify_place_details.js new file mode 100644 index 00000000..abaefe27 --- /dev/null +++ b/helper/geojsonify_place_details.js @@ -0,0 +1,133 @@ +var _ = require('lodash'); + +// Properties to be copied +// If a property is identified as a single string, assume it should be presented as a string in response +// If something other than string is desired, use the following structure: { name: 'category', type: 'array' } +var DETAILS_PROPS = [ + { name: 'housenumber', type: 'string' }, + { name: 'street', type: 'string' }, + { name: 'postalcode', type: 'string' }, + { name: 'confidence', type: 'default' }, + { name: 'distance', type: 'default' }, + { name: 'country', type: 'string' }, + { name: 'country_gid', type: 'string' }, + { name: 'country_a', type: 'string' }, + { name: 'macroregion', type: 'string' }, + { name: 'macroregion_gid', type: 'string' }, + { name: 'macroregion_a', type: 'string' }, + { name: 'region', type: 'string' }, + { name: 'region_gid', type: 'string' }, + { name: 'region_a', type: 'string' }, + { name: 'macrocounty', type: 'string' }, + { name: 'macrocounty_gid', type: 'string' }, + { name: 'macrocounty_a', type: 'string' }, + { name: 'county', type: 'string' }, + { name: 'county_gid', type: 'string' }, + { name: 'county_a', type: 'string' }, + { name: 'localadmin', type: 'string' }, + { name: 'localadmin_gid', type: 'string' }, + { name: 'localadmin_a', type: 'string' }, + { name: 'locality', type: 'string' }, + { name: 'locality_gid', type: 'string' }, + { name: 'locality_a', type: 'string' }, + { name: 'borough', type: 'string' }, + { name: 'borough_gid', type: 'string' }, + { name: 'borough_a', type: 'string' }, + { name: 'neighbourhood', type: 'string' }, + { name: 'neighbourhood_gid', type: 'string' }, + { name: 'bounding_box', type: 'default' }, + { name: 'category', type: 'array', condition: checkCategoryParam } +]; + +function checkCategoryParam(params) { + return _.isObject(params) && params.hasOwnProperty('categories'); +} + +/** + * Add details properties + * + * @param {object} params clean query params + * @param {object} src + * @param {object} dst + */ +function addDetails(params, src, dst) { + copyProperties(params, src, DETAILS_PROPS, dst); +} + +/** + * Copy specified properties from source to dest. + * Ignore missing properties. + * + * @param {object} params clean query params + * @param {object} source + * @param {[]} props + * @param {object} dst + */ +function copyProperties( params, source, props, dst ) { + props.forEach( function ( prop ) { + + // if condition isn't met, just return without setting the property + if (_.isFunction(prop.condition) && !prop.condition(params)) { + return; + } + + var property = { + name: prop.name || prop, + type: prop.type || 'default' + }; + + var value = null; + if ( source.hasOwnProperty( property.name ) ) { + + switch (property.type) { + case 'string': + value = getStringValue(source[property.name]); + break; + case 'array': + value = getArrayValue(source[property.name]); + break; + // default behavior is to copy property exactly as is + default: + value = source[property.name]; + } + + if (_.isNumber(value) || (value && !_.isEmpty(value))) { + dst[property.name] = value; + } + } + }); +} + +function getStringValue(property) { + // isEmpty check works for all types of values: strings, arrays, objects + if (_.isEmpty(property)) { + return ''; + } + + if (_.isString(property)) { + return property; + } + + // array value, take first item in array (at this time only used for admin values) + if (_.isArray(property)) { + return property[0]; + } + + return _.toString(property); +} + + +function getArrayValue(property) { + // isEmpty check works for all types of values: strings, arrays, objects + if (_.isEmpty(property)) { + return ''; + } + + if (_.isArray(property)) { + return property; + } + + return [property]; +} + +module.exports = addDetails; diff --git a/middleware/geocodeJSON.js b/middleware/geocodeJSON.js index 414d216b..f4ee8c20 100644 --- a/middleware/geocodeJSON.js +++ b/middleware/geocodeJSON.js @@ -1,6 +1,6 @@ var url = require('url'); var extend = require('extend'); -var geojsonify = require('../helper/geojsonify').search; +var geojsonify = require('../helper/geojsonify'); /** * Returns a middleware function that converts elasticsearch @@ -72,7 +72,7 @@ function convertToGeocodeJSON(req, res, next, opts) { res.body.geocoding.timestamp = new Date().getTime(); // convert docs to geojson and merge with geocoding block - extend(res.body, geojsonify(res.data || [])); + extend(res.body, geojsonify(req.clean, res.data || [])); next(); } diff --git a/sanitiser/nearby.js b/sanitiser/nearby.js new file mode 100644 index 00000000..a32fe002 --- /dev/null +++ b/sanitiser/nearby.js @@ -0,0 +1,29 @@ + +var type_mapping = require('../helper/type_mapping'); +var sanitizeAll = require('../sanitiser/sanitizeAll'), + sanitizers = { + quattroshapes_deprecation: require('../sanitiser/_deprecate_quattroshapes'), + singleScalarParameters: require('../sanitiser/_single_scalar_parameters'), + layers: require('../sanitiser/_targets')('layers', type_mapping.layer_mapping), + sources: require('../sanitiser/_targets')('sources', type_mapping.source_mapping), + // depends on the layers and sources sanitisers, must be run after them + sources_and_layers: require('../sanitiser/_sources_and_layers'), + size: require('../sanitiser/_size')(/* use defaults*/), + private: require('../sanitiser/_flag_bool')('private', false), + geo_reverse: require('../sanitiser/_geo_reverse'), + boundary_country: require('../sanitiser/_boundary_country'), + categories: require('../sanitiser/_categories') + }; + +var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; + +// export sanitize for testing +module.exports.sanitize = sanitize; +module.exports.sanitiser_list = sanitizers; + +// middleware +module.exports.middleware = function( req, res, next ){ + sanitize( req, function( err, clean ){ + next(); + }); +}; diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index 8ec97396..00107af2 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -4,9 +4,9 @@ var geojsonify = require('../../../helper/geojsonify'); module.exports.tests = {}; module.exports.tests.interface = function(test, common) { - test('valid interface .search()', function(t) { - t.equal(typeof geojsonify.search, 'function', 'search is a function'); - t.equal(geojsonify.search.length, 1, 'accepts x arguments'); + test('valid interface', function(t) { + t.equal(typeof geojsonify, 'function', 'search is a function'); + t.equal(geojsonify.length, 2, 'accepts x arguments'); t.end(); }); }; @@ -30,14 +30,14 @@ module.exports.tests.earth = function(test, common) { test('earth', function(t) { t.doesNotThrow(function(){ - geojsonify.search( earth ); + geojsonify( {}, earth ); }); t.end(); }); }; -module.exports.tests.search = function(test, common) { +module.exports.tests.geojsonify = function(test, common) { var input = [ { @@ -222,8 +222,8 @@ module.exports.tests.search = function(test, common) { ] }; - test('geojsonify.search(doc)', function(t) { - var json = geojsonify.search( input ); + test('geojsonify(doc)', function(t) { + var json = geojsonify( {categories: 'foo'}, input ); t.deepEqual(json, expected, 'all docs mapped'); t.end(); @@ -370,7 +370,67 @@ module.exports.tests.search = function(test, common) { ] }; - var json = geojsonify.search( input ); + var json = geojsonify( {categories: 'foo'}, input ); + + t.deepEqual(json, expected, 'all wanted properties exposed'); + t.end(); + }); +}; + +module.exports.tests.categories = function (test, common) { + test('only set category if categories filter was used', function (t) { + var input = [ + { + '_id': '85816607', + 'bounding_box': { + 'min_lat': 40.6514712164, + 'max_lat': 40.6737320588, + 'min_lon': -73.8967895508, + 'max_lon': -73.8665771484 + }, + 'source': 'whosonfirst', + 'layer': 'neighbourhood', + 'center_point': { + 'lon': -73.881319, + 'lat': 40.663303 + }, + 'name': { + 'default': 'East New York' + }, + 'source_id': '85816607', + 'category': ['government'] + } + ]; + + var expected = { + 'type': 'FeatureCollection', + 'bbox': [-73.8967895508, 40.6514712164, -73.8665771484, 40.6737320588], + 'features': [ + { + 'type': 'Feature', + 'properties': { + 'id': '85816607', + 'gid': 'whosonfirst:neighbourhood:85816607', + 'layer': 'neighbourhood', + 'source': 'whosonfirst', + 'source_id': '85816607', + 'name': 'East New York', + 'category': ['government'], + 'label': 'East New York' + }, + 'bbox': [-73.8967895508,40.6514712164,-73.8665771484,40.6737320588], + 'geometry': { + 'type': 'Point', + 'coordinates': [ + -73.881319, + 40.663303 + ] + } + } + ] + }; + + var json = geojsonify( {categories: 'foo'}, input ); t.deepEqual(json, expected, 'all wanted properties exposed'); t.end(); diff --git a/test/unit/sanitiser/nearby.js b/test/unit/sanitiser/nearby.js new file mode 100644 index 00000000..b2d40b01 --- /dev/null +++ b/test/unit/sanitiser/nearby.js @@ -0,0 +1,204 @@ + +// @todo: refactor this test, it's pretty messy, brittle and hard to follow + +var reverse = require('../../../sanitiser/reverse'), + sanitize = reverse.sanitize, + middleware = reverse.middleware, + defaults = require('../../../query/reverse_defaults'), + defaultError = 'missing param \'lat\'', + defaultClean = { 'point.lat': 0, + 'point.lon': 0, + 'boundary.circle.lat': 0, + 'boundary.circle.lon': 0, + 'boundary.circle.radius': parseFloat(defaults['boundary:circle:radius']), + size: 10, + private: false + }; + +// these are the default values you would expect when no input params are specified. +// @todo: why is this different from $defaultClean? +var emptyClean = { private: false, size: 10 }; + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('sanitize interface', function(t) { + t.equal(typeof sanitize, 'function', 'sanitize is a function'); + t.equal(sanitize.length, 2, 'sanitize interface'); + t.end(); + }); + test('middleware interface', function(t) { + t.equal(typeof middleware, 'function', 'middleware is a function'); + t.equal(middleware.length, 3, 'sanitizee has a valid middleware'); + t.end(); + }); +}; + +module.exports.tests.sanitisers = function(test, common) { + test('check sanitiser list', function (t) { + var expected = ['quattroshapes_deprecation', 'singleScalarParameters', 'layers', + 'sources', 'sources_and_layers', 'size', 'private', 'geo_reverse', 'boundary_country', 'categories']; + t.deepEqual(Object.keys(reverse.sanitiser_list), expected); + t.end(); + }); +}; + +module.exports.tests.sanitize_lat = function(test, common) { + var lats = { + invalid: [], + valid: [ 0, 45, 90, -0, '0', '45', '90', -181, -120, -91, 91, 120, 181 ], + missing: ['', undefined, null] + }; + test('invalid lat', function(t) { + lats.invalid.forEach( function( lat ){ + var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.errors[0], 'invalid param \'point.lat\': must be >-90 and <90', lat + ' is an invalid latitude'); + t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); + }); + }); + t.end(); + }); + test('valid lat', function(t) { + lats.valid.forEach( function( lat ){ + var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; + sanitize(req, function(){ + var expected_lat = parseFloat( lat ); + t.deepEqual(req.errors, [], 'no errors'); + t.equal(req.clean['point.lat'], expected_lat, 'clean set correctly (' + lat + ')'); + }); + }); + t.end(); + }); + test('missing lat', function(t) { + lats.missing.forEach( function( lat ){ + var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.errors[0], 'missing param \'point.lat\'', 'latitude is a required field'); + t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); + }); + }); + t.end(); + }); +}; + +module.exports.tests.sanitize_lon = function(test, common) { + var lons = { + valid: [ -360, -181, 181, -180, -1, -0, 0, 45, 90, '-180', '0', '180' ], + missing: ['', undefined, null] + }; + test('valid lon', function(t) { + lons.valid.forEach( function( lon ){ + var req = { query: { 'point.lat': 0, 'point.lon': lon } }; + sanitize(req, function(){ + var expected_lon = parseFloat( lon ); + t.deepEqual(req.errors, [], 'no errors'); + t.equal(req.clean['point.lon'], expected_lon, 'clean set correctly (' + lon + ')'); + }); + }); + t.end(); + }); + test('missing lon', function(t) { + lons.missing.forEach( function( lon ){ + var req = { query: { 'point.lat': 0, 'point.lon': lon } }; + + // @todo: why is lat set? + var expected = { 'point.lat': 0, private: false, size: 10 }; + sanitize(req, function(){ + t.equal(req.errors[0], 'missing param \'point.lon\'', 'longitude is a required field'); + t.deepEqual(req.clean, expected, 'clean only has default values set'); + }); + }); + t.end(); + }); +}; + +module.exports.tests.sanitize_size = function(test, common) { + test('invalid size value', function(t) { + var req = { query: { size: 'a', 'point.lat': 0, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.clean.size, 10, 'default size set'); + t.end(); + }); + }); + test('below min size value', function(t) { + var req = { query: { size: -100, 'point.lat': 0, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.clean.size, 1, 'min size set'); + t.end(); + }); + }); + test('above max size value', function(t) { + var req = { query: { size: 9999, 'point.lat': 0, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.clean.size, 40, 'max size set'); + t.end(); + }); + }); +}; + +module.exports.tests.sanitize_private = function(test, common) { + var invalid_values = [null, -1, 123, NaN, 'abc']; + invalid_values.forEach(function(value) { + test('invalid private param ' + value, function(t) { + var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; + sanitize(req, function(){ + t.equal(req.clean.private, false, 'default private set (to false)'); + t.end(); + }); + }); + }); + + var valid_values = ['true', true, 1, '1']; + valid_values.forEach(function(value) { + test('valid private param ' + value, function(t) { + var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; + sanitize(req, function(){ + t.equal(req.clean.private, true, 'private set to true'); + t.end(); + }); + }); + }); + + var valid_false_values = ['false', false, 0]; + valid_false_values.forEach(function(value) { + test('test setting false explicitly ' + value, function(t) { + var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; + sanitize(req, function(){ + t.equal(req.clean.private, false, 'private set to false'); + t.end(); + }); + }); + }); + + test('test default behavior', function(t) { + var req = { query: { 'point.lat': 0, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.clean.private, false, 'private set to false'); + t.end(); + }); + }); +}; + +module.exports.tests.middleware_success = function(test, common) { + test('middleware success', function(t) { + var req = { query: { 'point.lat': 0, 'point.lon': 0 }}; + var next = function(){ + t.deepEqual(req.errors, [], 'no error message set'); + t.deepEqual(req.clean, defaultClean); + t.end(); + }; + middleware( req, undefined, next ); + }); +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('SANTIZE /reverse ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +};