diff --git a/controller/place.js b/controller/place.js index 8622f234..101430c5 100644 --- a/controller/place.js +++ b/controller/place.js @@ -16,7 +16,16 @@ function setup( backend ){ var query = req.clean.ids.map( function(id) { return { _index: 'pelias', - _type: id.type, + /* + * some gids aren't resolvable to a single type (ex: osmnode and osmway + * both have source osm and layer venue), so expect an array of + * possible values. It's important to use `type` here instead of + * `_type`, as the former actually queries against the type, and thus + * can accept multiple match values. `_type`, on the other hand, + * simply changes the actual URL of the query sent to Elasticsearch to + * contain a type, which obviously can only take a single type. + */ + type: id.types, _id: id.id }; }); diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 9ce77ad4..ca499613 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -2,7 +2,9 @@ var GeoJSON = require('geojson'), extent = require('geojson-extent'), outputGenerator = require('./outputGenerator'), - logger = require('pelias-logger').get('api'); + logger = require('pelias-logger').get('api'), + type_mapping = require('./type_mapping'), + _ = require('lodash'); // Properties to be copied var DETAILS_PROPS = [ @@ -22,54 +24,30 @@ var DETAILS_PROPS = [ ]; -var SOURCES = { - 'geoname': 'gn', - 'osmnode': 'osm', - 'osmway': 'osm', - 'admin0': 'qs', - 'admin1': 'qs', - 'admin2': 'qs', - 'neighborhood': 'qs', - 'locality': 'qs', - 'local_admin': 'qs', - 'osmaddress': 'osm', - 'openaddresses': 'oa' -}; - function lookupSource(src) { - return SOURCES.hasOwnProperty(src._type) ? SOURCES[src._type] : src._type; + var sources = type_mapping.type_to_source; + return sources.hasOwnProperty(src._type) ? sources[src._type] : src._type; } +/* + * Use the type to layer mapping, except for Geonames, where having a full + * Elasticsearch document source allows a more specific layer name to be chosen + */ function lookupLayer(src) { - switch(src._type) { - case 'osmnode': - case 'osmway': - return 'venue'; - case 'admin0': - return 'country'; - case 'admin1': - return 'region'; - case 'admin2': - return 'county'; - case 'neighborhood': - return 'neighbourhood'; - case 'locality': - return 'locality'; - case 'local_admin': - return 'localadmin'; - case 'osmaddress': - case 'openaddresses': - return 'address'; - case 'geoname': - if (src.category && src.category.indexOf('admin') !== -1) { - if (src.category.indexOf('admin:city') !== -1) { return 'locality'; } - if (src.category.indexOf('admin:admin1') !== -1) { return 'region'; } - if (src.category.indexOf('admin:admin2') !== -1) { return 'county'; } - return 'neighbourhood'; // this could also be 'local_admin' - } - - if (src.name) { return 'venue'; } - if (src.address) { return 'address'; } + if (src._type === 'geoname') { + if (_.contains(src.category, 'admin')) { + if (_.contains(src.category, 'admin:city')) { return 'locality'; } + if (_.contains(src.category, 'admin:admin1')) { return 'region'; } + if (_.contains(src.category, 'admin:admin2')) { return 'county'; } + return 'neighbourhood'; // this could also be 'local_admin' + } + + if (src.name) { return 'venue'; } + if (src.address) { return 'address'; } + } + + if (_.contains(type_mapping.types, src._type)) { + return type_mapping.type_to_layer[src._type]; } logger.warn('[geojsonify]: could not map _type ', src._type); @@ -177,6 +155,16 @@ function copyProperties( source, props, dst ) { }); } +/** + * 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) { + return lookupSource(src) + ':' + lookupLayer(src) + ':' + src._id; +} + /** * Determine and set place id, type, and source * @@ -185,6 +173,7 @@ function copyProperties( source, props, dst ) { */ function addMetaData(src, dst) { dst.id = src._id; + dst.gid = makeGid(src); dst.layer = lookupLayer(src); dst.source = lookupSource(src); } diff --git a/helper/query_parser.js b/helper/query_parser.js index b60ebcd8..4af009aa 100644 --- a/helper/query_parser.js +++ b/helper/query_parser.js @@ -1,7 +1,7 @@ var parser = require('addressit'); var extend = require('extend'); -var layers_map = require('../query/layers'); +var type_mapping = require('../helper/type_mapping'); var delim = ','; module.exports = {}; @@ -9,7 +9,7 @@ module.exports = {}; module.exports.get_layers = function get_layers(query) { if (query.length <= 3 ) { // no address parsing required - return layers_map.coarse; + return type_mapping.layer_with_aliases_to_type.coarse; } }; diff --git a/helper/type_mapping.js b/helper/type_mapping.js new file mode 100644 index 00000000..27fce81c --- /dev/null +++ b/helper/type_mapping.js @@ -0,0 +1,87 @@ +var extend = require('extend'), + _ = require('lodash'); + +var TYPE_TO_SOURCE = { + 'geoname': 'gn', + 'osmnode': 'osm', + 'osmway': 'osm', + 'admin0': 'qs', + 'admin1': 'qs', + 'admin2': 'qs', + 'neighborhood': 'qs', + 'locality': 'qs', + 'local_admin': 'qs', + 'osmaddress': 'osm', + 'openaddresses': 'oa' +}; + +/* + * This doesn't include alias layers such as coarse + */ +var TYPE_TO_LAYER = { + 'geoname': 'venue', + 'osmnode': 'venue', + 'osmway': 'venue', + 'admin0': 'country', + 'admin1': 'region', + 'admin2': 'county', + 'neighborhood': 'neighbourhood', + 'locality': 'locality', + 'local_admin': 'localadmin', + 'osmaddress': 'address', + 'openaddresses': 'address' +}; + +var SOURCE_TO_TYPE = { + 'gn' : ['geoname'], + 'geonames' : ['geoname'], + 'oa' : ['openaddresses'], + 'openaddresses' : ['openaddresses'], + 'qs' : ['admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin'], + 'quattroshapes' : ['admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin'], + 'osm' : ['osmaddress', 'osmnode', 'osmway'], + 'openstreetmap' : ['osmaddress', 'osmnode', 'osmway'] +}; + +/** + * This does not included alias layers, those are built separately + */ +var LAYER_TO_TYPE = { + 'venue': ['geoname','osmnode','osmway'], + 'address': ['osmaddress','openaddresses', 'geoname'], + 'country': ['admin0', 'geoname'], + 'region': ['admin1', 'geoname'], + 'county': ['admin2', 'geoname'], + 'locality': ['locality', 'geoname'], + 'localadmin': ['local_admin'], + 'neighbourhood': ['neighborhood', 'geoname'] +}; + +var LAYER_ALIASES = { + 'coarse': ['admin0','admin1','admin2','neighborhood','locality','local_admin'] +}; + +var LAYER_WITH_ALIASES_TO_TYPE = extend({}, LAYER_ALIASES, LAYER_TO_TYPE); + +/* + * derive the list of types, sources, and layers from above mappings + */ +var TYPES = Object.keys(TYPE_TO_SOURCE); +var SOURCES = Object.keys(SOURCE_TO_TYPE); +var LAYERS = Object.keys(LAYER_TO_TYPE); + +var sourceAndLayerToType = function sourceAndLayerToType(source, layer) { + return _.intersection(SOURCE_TO_TYPE[source], LAYER_WITH_ALIASES_TO_TYPE[layer]); +}; + +module.exports = { + types: TYPES, + sources: SOURCES, + layers: LAYERS, + type_to_source: TYPE_TO_SOURCE, + type_to_layer: TYPE_TO_LAYER, + source_to_type: SOURCE_TO_TYPE, + layer_to_type: LAYER_TO_TYPE, + layer_with_aliases_to_type: LAYER_WITH_ALIASES_TO_TYPE, + source_and_layer_to_type: sourceAndLayerToType +}; diff --git a/helper/types.js b/helper/types.js index 249c27a0..610d1aba 100644 --- a/helper/types.js +++ b/helper/types.js @@ -1,13 +1,5 @@ -var valid_types = require( '../query/types' ); - -/** - * Calculate the set-style intersection of two arrays - */ -var intersection = function intersection(set1, set2) { - return set2.filter(function(value) { - return set1.indexOf(value) !== -1; - }); -}; +var type_mapping = require( '../helper/type_mapping' ); +var _ = require('lodash'); /** * Combine all types and determine the unique subset @@ -24,14 +16,14 @@ module.exports = function calculate_types(clean_types) { * perform a set intersection of their specified types */ if (clean_types.from_layers || clean_types.from_sources) { - var types = valid_types; + var types = type_mapping.types; if (clean_types.from_layers) { - types = intersection(types, clean_types.from_layers); + types = _.intersection(types, clean_types.from_layers); } if (clean_types.from_sources) { - types = intersection(types, clean_types.from_sources); + types = _.intersection(types, clean_types.from_sources); } return types; @@ -46,4 +38,4 @@ module.exports = function calculate_types(clean_types) { } throw new Error('no types specified'); -}; \ No newline at end of file +}; diff --git a/query/layers.js b/query/layers.js deleted file mode 100644 index 7f16d1fb..00000000 --- a/query/layers.js +++ /dev/null @@ -1,15 +0,0 @@ -/* - * Mapping from data layers to type values - */ - -module.exports = { - 'venue': ['geoname','osmnode','osmway'], - 'address': ['osmaddress','openaddresses'], - 'country': ['admin0'], - 'region': ['admin1'], - 'county': ['admin2'], - 'locality': ['locality'], - 'localadmin': ['local_admin'], - 'neighbourhood': ['neighborhood'], - 'coarse': ['admin0','admin1','admin2','neighborhood','locality','local_admin'], -}; diff --git a/query/sources.js b/query/sources.js deleted file mode 100644 index 3715be86..00000000 --- a/query/sources.js +++ /dev/null @@ -1,14 +0,0 @@ -/* - * Mapping from data sources to type values - */ - -module.exports = { - 'gn' : ['geoname'], - 'geonames' : ['geoname'], - 'oa' : ['openaddresses'], - 'openaddresses' : ['openaddresses'], - 'qs' : ['admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin'], - 'quattroshapes' : ['admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin'], - 'osm' : ['osmaddress', 'osmnode', 'osmway'], - 'openstreetmap' : ['osmaddress', 'osmnode', 'osmway'] -}; diff --git a/query/types.js b/query/types.js deleted file mode 100644 index 5f8cfab7..00000000 --- a/query/types.js +++ /dev/null @@ -1,16 +0,0 @@ - -// querable types - -module.exports = [ - 'geoname', - 'osmnode', - 'osmway', - 'admin0', - 'admin1', - 'admin2', - 'neighborhood', - 'locality', - 'local_admin', - 'osmaddress', - 'openaddresses' -]; diff --git a/sanitiser/_ids.js b/sanitiser/_ids.js index 0194da02..0b4d91c7 100644 --- a/sanitiser/_ids.js +++ b/sanitiser/_ids.js @@ -1,19 +1,58 @@ var _ = require('lodash'), check = require('check-types'), - types = require('../query/types'); + type_mapping = require('../helper/type_mapping'); var ID_DELIM = ':'; -// validate inputs, convert types and apply defaults -// id generally looks like 'geoname:4163334' (type:id) -// so, both type and id are required fields. +// validate inputs, convert types and apply defaults id generally looks like +// 'geonames:venue:4163334' (source:layer:id) so, all three are required var lengthError = 'invalid param \'ids\': length must be >0'; var formatError = function(input) { - return 'id `' + input + 'is invalid: must be of the format type:id for ex: \'geoname:4163334\''; + return 'id `' + input + ' is invalid: must be of the format source:layer:id for ex: \'geonames:venue:4163334\''; }; +var targetError = function(target, target_list) { + return target + ' is invalid. It must be one of these values - [' + target_list.join(', ') + ']'; +}; + +function sanitizeId(rawId, messages) { + var parts = rawId.split(ID_DELIM); + + if ( parts.length < 3 ) { + messages.errors.push( formatError(rawId) ); + return; + } + + var source = parts[0]; + var layer = parts[1]; + var id = parts.slice(2).join(ID_DELIM); + + // check if any parts of the gid are empty + if (_.contains([source, layer, id], '')) { + messages.errors.push( formatError(rawId) ); + return; + } + + if (!_.contains(type_mapping.sources, source)) { + messages.errors.push( targetError(source, type_mapping.sources) ); + return; + } + + if (!_.contains(type_mapping.layers, layer)) { + messages.errors.push( targetError(layer, type_mapping.layers) ); + return; + } + + var types = type_mapping.source_and_layer_to_type(source, layer); + + return { + id: id, + types: types + }; +} + function sanitize( raw, clean ){ // error & warning messages var messages = { errors: [], warnings: [] }; @@ -42,25 +81,8 @@ function sanitize( raw, clean ){ } // cycle through raw ids and set those which are valid - var validIds = rawIds.map( function( rawId ){ - var param_index = rawId.indexOf(ID_DELIM); - var type = rawId.substring(0, param_index ); - var id = rawId.substring(param_index + 1); - - // check id format - if(!check.contains(rawId, ID_DELIM) || !check.unemptyString( id ) || !check.unemptyString( type )) { - messages.errors.push( formatError(rawId) ); - } - // type text must be one of the types - else if( !_.contains( types, type ) ){ - messages.errors.push( type + ' is invalid. It must be one of these values - [' + types.join(', ') + ']' ); - } - else { - return { - id: id, - type: type - }; - } + var validIds = rawIds.map(function(rawId) { + return sanitizeId(rawId, messages); }); if (validIds.every(check.object)) { diff --git a/sanitiser/reverse.js b/sanitiser/reverse.js index dd9eeb1d..154fd3d9 100644 --- a/sanitiser/reverse.js +++ b/sanitiser/reverse.js @@ -1,9 +1,10 @@ +var type_mapping = require('../helper/type_mapping'); var sanitizeAll = require('../sanitiser/sanitizeAll'), sanitizers = { singleScalarParameters: require('../sanitiser/_single_scalar_parameters'), - layers: require('../sanitiser/_targets')('layers', require('../query/layers')), - sources: require('../sanitiser/_targets')('sources', require('../query/sources')), + layers: require('../sanitiser/_targets')('layers', type_mapping.layer_with_aliases_to_type), + sources: require('../sanitiser/_targets')('sources', type_mapping.source_to_type), size: require('../sanitiser/_size'), private: require('../sanitiser/_flag_bool')('private', false), geo_reverse: require('../sanitiser/_geo_reverse'), diff --git a/sanitiser/search.js b/sanitiser/search.js index e1b79528..cc596b26 100644 --- a/sanitiser/search.js +++ b/sanitiser/search.js @@ -1,11 +1,12 @@ +var type_mapping = require('../helper/type_mapping'); var sanitizeAll = require('../sanitiser/sanitizeAll'), sanitizers = { singleScalarParameters: require('../sanitiser/_single_scalar_parameters'), text: require('../sanitiser/_text'), size: require('../sanitiser/_size'), - layers: require('../sanitiser/_targets')('layers', require( '../query/layers' )), - sources: require('../sanitiser/_targets')('sources', require( '../query/sources' )), + layers: require('../sanitiser/_targets')('layers', type_mapping.layer_with_aliases_to_type), + sources: require('../sanitiser/_targets')('sources', type_mapping.source_to_type), private: require('../sanitiser/_flag_bool')('private', false), geo_search: require('../sanitiser/_geo_search'), boundary_country: require('../sanitiser/_boundary_country'), diff --git a/test/ciao/place/basic_place.coffee b/test/ciao/place/basic_place.coffee index 39cfda18..b4a18711 100644 --- a/test/ciao/place/basic_place.coffee +++ b/test/ciao/place/basic_place.coffee @@ -1,6 +1,6 @@ #> basic place -path: '/v1/place?ids=geoname:1' +path: '/v1/place?ids=geonames:venue:1' #? 200 ok response.statusCode.should.be.equal 200 @@ -29,5 +29,5 @@ should.not.exist json.geocoding.errors should.not.exist json.geocoding.warnings #? inputs -json.geocoding.query['ids'].should.eql [{ id: '1', type: 'geoname' }] +json.geocoding.query['ids'].should.eql [{ id: '1', types: [ 'geoname' ] }] should.not.exist json.geocoding.query['size'] diff --git a/test/ciao/reverse/layers_invalid.coffee b/test/ciao/reverse/layers_invalid.coffee index 0da5790e..c4d858fc 100644 --- a/test/ciao/reverse/layers_invalid.coffee +++ b/test/ciao/reverse/layers_invalid.coffee @@ -24,7 +24,7 @@ json.features.should.be.instanceof Array #? expected errors should.exist json.geocoding.errors -json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: venue,address,country,region,county,locality,localadmin,neighbourhood,coarse' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,venue,address,country,region,county,locality,localadmin,neighbourhood' ] #? expected warnings should.not.exist json.geocoding.warnings @@ -32,4 +32,4 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['size'].should.eql 10 should.not.exist json.geocoding.query['types'] -should.not.exist json.geocoding.query['type'] \ No newline at end of file +should.not.exist json.geocoding.query['type'] diff --git a/test/ciao/reverse/layers_mix_invalid_valid.coffee b/test/ciao/reverse/layers_mix_invalid_valid.coffee index daa7bc2d..71a95fa4 100644 --- a/test/ciao/reverse/layers_mix_invalid_valid.coffee +++ b/test/ciao/reverse/layers_mix_invalid_valid.coffee @@ -24,7 +24,7 @@ json.features.should.be.instanceof Array #? expected errors should.exist json.geocoding.errors -json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: venue,address,country,region,county,locality,localadmin,neighbourhood,coarse' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,venue,address,country,region,county,locality,localadmin,neighbourhood' ] #? expected warnings should.not.exist json.geocoding.warnings @@ -32,4 +32,4 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['size'].should.eql 10 should.not.exist json.geocoding.query['types'] -should.not.exist json.geocoding.query['type'] \ No newline at end of file +should.not.exist json.geocoding.query['type'] diff --git a/test/ciao/reverse/layers_multiple.coffee b/test/ciao/reverse/layers_multiple.coffee index efa27e83..2fd0e1dc 100644 --- a/test/ciao/reverse/layers_multiple.coffee +++ b/test/ciao/reverse/layers_multiple.coffee @@ -30,5 +30,5 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['size'].should.eql 10 -json.geocoding.query.types['from_layers'].should.eql ["admin0","admin1"] -json.geocoding.query['type'].should.eql ["admin0","admin1"] \ No newline at end of file +json.geocoding.query.types['from_layers'].should.eql ["admin0","geoname","admin1"] +json.geocoding.query['type'].should.eql ["admin0","geoname","admin1"] diff --git a/test/ciao/reverse/layers_single.coffee b/test/ciao/reverse/layers_single.coffee index 641edad1..e6642fb9 100644 --- a/test/ciao/reverse/layers_single.coffee +++ b/test/ciao/reverse/layers_single.coffee @@ -30,5 +30,5 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['size'].should.eql 10 -json.geocoding.query.types['from_layers'].should.eql ["admin0"] -json.geocoding.query['type'].should.eql ["admin0"] \ No newline at end of file +json.geocoding.query.types['from_layers'].should.eql ["admin0","geoname"] +json.geocoding.query['type'].should.eql ["admin0","geoname"] diff --git a/test/ciao/reverse/sources_layers_invalid_combo.coffee b/test/ciao/reverse/sources_layers_invalid_combo.coffee index 06d18c37..dc74ba4a 100644 --- a/test/ciao/reverse/sources_layers_invalid_combo.coffee +++ b/test/ciao/reverse/sources_layers_invalid_combo.coffee @@ -31,6 +31,6 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['size'].should.eql 10 -json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"] +json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses","geoname"] json.geocoding.query.types['from_sources'].should.eql ["admin0","admin1","admin2","neighborhood","locality","local_admin"] -should.not.exist json.geocoding.query['type'] \ No newline at end of file +should.not.exist json.geocoding.query['type'] diff --git a/test/ciao/reverse/sources_layers_valid_combo.coffee b/test/ciao/reverse/sources_layers_valid_combo.coffee index 286d4916..cc593768 100644 --- a/test/ciao/reverse/sources_layers_valid_combo.coffee +++ b/test/ciao/reverse/sources_layers_valid_combo.coffee @@ -30,5 +30,5 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['size'].should.eql 10 -json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"] -json.geocoding.query['type'].should.eql ["openaddresses"] \ No newline at end of file +json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses","geoname"] +json.geocoding.query['type'].should.eql ["openaddresses"] diff --git a/test/ciao/search/layers_alias_address.coffee b/test/ciao/search/layers_alias_address.coffee index c7c58472..14074bf9 100644 --- a/test/ciao/search/layers_alias_address.coffee +++ b/test/ciao/search/layers_alias_address.coffee @@ -31,5 +31,5 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['text'].should.eql 'a' json.geocoding.query['size'].should.eql 10 -json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"] -json.geocoding.query['type'].should.eql ["osmaddress","openaddresses"] \ No newline at end of file +json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses","geoname"] +json.geocoding.query['type'].should.eql ["osmaddress","openaddresses","geoname"] diff --git a/test/ciao/search/layers_invalid.coffee b/test/ciao/search/layers_invalid.coffee index babe7ca6..3a7c9d38 100644 --- a/test/ciao/search/layers_invalid.coffee +++ b/test/ciao/search/layers_invalid.coffee @@ -24,7 +24,7 @@ json.features.should.be.instanceof Array #? expected errors should.exist json.geocoding.errors -json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: venue,address,country,region,county,locality,localadmin,neighbourhood,coarse' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,venue,address,country,region,county,locality,localadmin,neighbourhood' ] #? expected warnings should.not.exist json.geocoding.warnings @@ -33,4 +33,4 @@ should.not.exist json.geocoding.warnings json.geocoding.query['text'].should.eql 'a' json.geocoding.query['size'].should.eql 10 should.not.exist json.geocoding.query['types'] -should.not.exist json.geocoding.query['type'] \ No newline at end of file +should.not.exist json.geocoding.query['type'] diff --git a/test/ciao/search/layers_mix_invalid_valid.coffee b/test/ciao/search/layers_mix_invalid_valid.coffee index a496bdb7..c1a53645 100644 --- a/test/ciao/search/layers_mix_invalid_valid.coffee +++ b/test/ciao/search/layers_mix_invalid_valid.coffee @@ -24,7 +24,7 @@ json.features.should.be.instanceof Array #? expected errors should.exist json.geocoding.errors -json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: venue,address,country,region,county,locality,localadmin,neighbourhood,coarse' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,venue,address,country,region,county,locality,localadmin,neighbourhood' ] #? expected warnings should.not.exist json.geocoding.warnings @@ -33,4 +33,4 @@ should.not.exist json.geocoding.warnings json.geocoding.query['text'].should.eql 'a' json.geocoding.query['size'].should.eql 10 should.not.exist json.geocoding.query['types'] -should.not.exist json.geocoding.query['type'] \ No newline at end of file +should.not.exist json.geocoding.query['type'] diff --git a/test/ciao/search/layers_multiple.coffee b/test/ciao/search/layers_multiple.coffee index db83fed7..20e67eba 100644 --- a/test/ciao/search/layers_multiple.coffee +++ b/test/ciao/search/layers_multiple.coffee @@ -31,5 +31,5 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['text'].should.eql 'a' json.geocoding.query['size'].should.eql 10 -json.geocoding.query.types['from_layers'].should.eql ["admin0","admin1"] -json.geocoding.query['type'].should.eql ["admin0","admin1"] \ No newline at end of file +json.geocoding.query.types['from_layers'].should.eql ["admin0","geoname","admin1"] +json.geocoding.query['type'].should.eql ["admin0","geoname","admin1"] diff --git a/test/ciao/search/layers_single.coffee b/test/ciao/search/layers_single.coffee index b0d130eb..9dc79a00 100644 --- a/test/ciao/search/layers_single.coffee +++ b/test/ciao/search/layers_single.coffee @@ -31,5 +31,5 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['text'].should.eql 'a' json.geocoding.query['size'].should.eql 10 -json.geocoding.query.types['from_layers'].should.eql ["admin0"] -json.geocoding.query['type'].should.eql ["admin0"] \ No newline at end of file +json.geocoding.query.types['from_layers'].should.eql ["admin0","geoname"] +json.geocoding.query['type'].should.eql ["admin0","geoname"] diff --git a/test/ciao/search/sources_layers_invalid_combo.coffee b/test/ciao/search/sources_layers_invalid_combo.coffee index ce388de7..6d9702ed 100644 --- a/test/ciao/search/sources_layers_invalid_combo.coffee +++ b/test/ciao/search/sources_layers_invalid_combo.coffee @@ -32,6 +32,6 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['text'].should.eql 'a' json.geocoding.query['size'].should.eql 10 -json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"] +json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses","geoname"] json.geocoding.query.types['from_sources'].should.eql ["admin0","admin1","admin2","neighborhood","locality","local_admin"] -should.not.exist json.geocoding.query['type'] \ No newline at end of file +should.not.exist json.geocoding.query['type'] diff --git a/test/ciao/search/sources_layers_valid_combo.coffee b/test/ciao/search/sources_layers_valid_combo.coffee index 6e11c9c7..d20cc6ca 100644 --- a/test/ciao/search/sources_layers_valid_combo.coffee +++ b/test/ciao/search/sources_layers_valid_combo.coffee @@ -31,5 +31,5 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['text'].should.eql 'a' json.geocoding.query['size'].should.eql 10 -json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses"] -json.geocoding.query['type'].should.eql ["openaddresses"] \ No newline at end of file +json.geocoding.query.types['from_layers'].should.eql ["osmaddress","openaddresses","geoname"] +json.geocoding.query['type'].should.eql ["openaddresses"] diff --git a/test/unit/controller/place.js b/test/unit/controller/place.js index 1fde7942..20455e3a 100644 --- a/test/unit/controller/place.js +++ b/test/unit/controller/place.js @@ -41,7 +41,7 @@ module.exports.tests.functional_success = function(test, common) { test('functional success', function(t) { var backend = mockBackend( 'client/mget/ok/1', function( cmd ){ - t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: 'a' } ] } }, 'correct backend command'); + t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', type: [ 'a' ] } ] } }, 'correct backend command'); }); var controller = setup( backend ); var res = { @@ -57,7 +57,7 @@ module.exports.tests.functional_success = function(test, common) { t.deepEqual(json.features, expected, 'values correctly mapped'); } }; - var req = { clean: { ids: [ {'id' : 123, 'type': 'a' } ] }, errors: [], warnings: [] }; + var req = { clean: { ids: [ {'id' : 123, types: [ 'a' ] } ] }, errors: [], warnings: [] }; var next = function next() { t.equal(req.errors.length, 0, 'next was called without error'); t.end(); @@ -70,10 +70,10 @@ module.exports.tests.functional_success = function(test, common) { module.exports.tests.functional_failure = function(test, common) { test('functional failure', function(t) { var backend = mockBackend( 'client/mget/fail/1', function( cmd ){ - t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: 'b' } ] } }, 'correct backend command'); + t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', type: [ 'b' ] } ] } }, 'correct backend command'); }); var controller = setup( backend ); - var req = { clean: { ids: [ {'id' : 123, 'type': 'b' } ] }, errors: [], warnings: [] }; + var req = { clean: { ids: [ {'id' : 123, types: [ 'b' ] } ] }, errors: [], warnings: [] }; var next = function( message ){ t.equal(req.errors[0],'a backend error occurred','error passed to errorHandler'); t.end(); @@ -85,7 +85,7 @@ module.exports.tests.functional_failure = function(test, common) { module.exports.all = function (tape, common) { function test(name, testFunction) { - return tape('GET /doc ' + name, testFunction); + return tape('GET /place ' + name, testFunction); } for( var testCase in module.exports.tests ){ diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index 9ed4ed0a..a66421de 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -141,6 +141,7 @@ module.exports.tests.search = function(test, common) { }, 'properties': { 'id': 'id1', + 'gid': 'type1:type1:id1', 'layer': 'type1', 'source': 'type1', 'label': '\'Round Midnight Jazz and Blues Bar, test3, Angel', @@ -169,6 +170,7 @@ module.exports.tests.search = function(test, common) { }, 'properties': { 'id': 'id2', + 'gid': 'type2:type2:id2', 'layer': 'type2', 'source': 'type2', 'label': 'Blues Cafe, test3, Smithfield', @@ -194,6 +196,7 @@ module.exports.tests.search = function(test, common) { }, 'properties': { 'id': '34633854', + 'gid': 'osm:venue:34633854', 'layer': 'venue', 'source': 'osm', 'label': 'Empire State Building, Manhattan, NY', diff --git a/test/unit/helper/query_parser.js b/test/unit/helper/query_parser.js index ad669ece..02c96531 100644 --- a/test/unit/helper/query_parser.js +++ b/test/unit/helper/query_parser.js @@ -1,6 +1,7 @@ - var parser = require('../../../helper/query_parser'); -var layers_map = require('../../../query/layers'); + +var type_mapping = require('../../../helper/type_mapping'); +var layers_map = type_mapping.layer_with_aliases_to_type; module.exports.tests = {}; diff --git a/test/unit/helper/type_mapping.js b/test/unit/helper/type_mapping.js new file mode 100644 index 00000000..144fadfb --- /dev/null +++ b/test/unit/helper/type_mapping.js @@ -0,0 +1,52 @@ +var check = require('check-types'); +var type_mapping = require('../../../helper/type_mapping'); + +module.exports.tests = {}; + +module.exports.tests.interfaces = function(test, common) { + test('types list', function(t) { + t.ok(check.array(type_mapping.types), 'is array'); + t.ok(check.hasLength(type_mapping.types, 11), 'has correct number of elements'); + t.end(); + }); + + test('type to source mapping', function(t) { + t.ok(check.object(type_mapping.type_to_source), 'is object'); + t.ok(check.hasLength(Object.keys(type_mapping.type_to_source), 11), 'has correct number of elements'); + t.end(); + }); + + test('type to layer mapping', function(t) { + t.ok(check.object(type_mapping.type_to_layer), 'is object'); + t.ok(check.hasLength(Object.keys(type_mapping.type_to_layer), 11), 'has correct number of elements'); + t.end(); + }); + + test('source to type mapping', function(t) { + t.ok(check.object(type_mapping.source_to_type), 'is object'); + t.ok(check.hasLength(Object.keys(type_mapping.source_to_type), 8), 'has correct number of elements'); + t.end(); + }); + + test('layer to type mapping', function(t) { + t.ok(check.object(type_mapping.layer_to_type), 'is object'); + t.equal(Object.keys(type_mapping.layer_to_type).length, 8, 'has correct number of elements'); + t.end(); + }); + + test('layer to type mapping (with aliases)', function(t) { + t.ok(check.object(type_mapping.layer_with_aliases_to_type), 'is object'); + t.ok(check.hasLength(Object.keys(type_mapping.layer_with_aliases_to_type), 9), 'has correct number of elements'); + t.end(); + }); +}; + +module.exports.all = function (tape, common) { + function test(name, testFunction) { + return tape('type_mapping: ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/mock/backend.js b/test/unit/mock/backend.js index 201ab7b5..7d0eb8d7 100644 --- a/test/unit/mock/backend.js +++ b/test/unit/mock/backend.js @@ -94,4 +94,4 @@ function searchEnvelope( options ){ return { hits: { total: options.length, hits: options } }; } -module.exports = setup; \ No newline at end of file +module.exports = setup; diff --git a/test/unit/query/types.js b/test/unit/query/types.js deleted file mode 100644 index eee830a6..00000000 --- a/test/unit/query/types.js +++ /dev/null @@ -1,23 +0,0 @@ - -var types = require('../../../query/types'); - -module.exports.tests = {}; - -module.exports.tests.interface = function(test, common) { - test('valid interface', function(t) { - t.true(Array.isArray(types), 'valid array'); - t.equal(types.length, 11, 'valid array'); - t.end(); - }); -}; - -module.exports.all = function (tape, common) { - - function test(name, testFunction) { - return tape('types ' + name, testFunction); - } - - for( var testCase in module.exports.tests ){ - module.exports.tests[testCase](test, common); - } -}; diff --git a/test/unit/run.js b/test/unit/run.js index 50c301de..a95764e5 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -17,7 +17,6 @@ var tests = [ require('./sanitiser/_layers'), require('./sanitiser/reverse'), require('./sanitiser/place'), - require('./query/types'), require('./query/search'), require('./query/autocomplete'), require('./query/reverse'), @@ -26,6 +25,7 @@ var tests = [ require('./helper/geojsonify'), require('./helper/outputSchema'), require('./helper/types'), + require('./helper/type_mapping'), require('./sanitiser/_geo_common'), require('./middleware/distance'), require('./middleware/confidenceScoreReverse'), diff --git a/test/unit/sanitiser/_ids.js b/test/unit/sanitiser/_ids.js index 02e25f18..9438f3ee 100644 --- a/test/unit/sanitiser/_ids.js +++ b/test/unit/sanitiser/_ids.js @@ -1,20 +1,16 @@ var sanitize = require('../../../sanitiser/_ids'); var delimiter = ':'; -var types = require('../../../query/types'); +var type_mapping = require('../../../helper/type_mapping'); var inputs = { valid: [ 'geoname:1', 'osmnode:2', 'admin0:53', 'osmway:44', 'geoname:5' ], invalid: [ ':', '', '::', 'geoname:', ':234', 'gibberish:23' ] }; var formatError = function(input) { - return 'id `' + input + 'is invalid: must be of the format type:id for ex: \'geoname:4163334\''; + return 'id `' + input + ' is invalid: must be of the format source:layer:id for ex: \'geonames:venue:4163334\''; }; var lengthError = 'invalid param \'ids\': length must be >0'; -var defaultMissingTypeError = function(input) { - var type = input.split(delimiter)[0]; - return type + ' is invalid. It must be one of these values - [' + types.join(', ') + ']'; - }; module.exports.tests = {}; @@ -74,50 +70,58 @@ module.exports.tests.invalid_ids = function(test, common) { t.end(); }); - test('invalid id: type name invalid', function(t) { - var raw = { ids: 'gibberish:23' }; + test('invalid id: source name invalid', function(t) { + var raw = { ids: 'invalidsource:venue:23' }; var clean = {}; + var expected_error = 'invalidsource is invalid. It must be one of these values - [' + type_mapping.sources.join(', ') + ']'; var messages = sanitize(raw, clean); - t.equal(messages.errors[0], defaultMissingTypeError('gibberish:23'), 'format error returned'); + t.equal(messages.errors[0], expected_error, 'format error returned'); + t.equal(clean.ids, undefined, 'ids unset in clean object'); + t.end(); + }); + + test('invalid id: old style 2 part id', function(t) { + var raw = { ids: 'geonames:23' }; + var clean = {}; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors[0], formatError('geonames:23'), 'format error returned'); t.equal(clean.ids, undefined, 'ids unset in clean object'); t.end(); }); }; module.exports.tests.valid_ids = function(test, common) { - test('ids: valid input', function(t) { - inputs.valid.forEach( function( input ){ - var input_parts = input.split(delimiter); - var expected_clean = { ids: [ { id: input_parts[1], type: input_parts[0] } ]}; - var raw = { ids: input }; - var clean = {}; - - var messages = sanitize( raw, clean ); - - t.deepEqual( messages.errors, [], 'no error (' + input + ')' ); - t.deepEqual( clean, expected_clean, 'clean set correctly (' + input + ')'); - }); + test('ids: valid input (openaddresses)', function(t) { + var raw = { ids: 'openaddresses:address:20' }; + var clean = {}; + var expected_ids = [{ + id: '20', + types: [ 'openaddresses' ] + }]; + + var messages = sanitize( raw, clean ); + + t.deepEqual( messages.errors, [], ' no errors'); + t.deepEqual( clean.ids, expected_ids, 'single type value returned'); t.end(); }); - test('ids: valid input with multiple values' , function(t) { - var raw = { ids: inputs.valid.join(',') }; + test('ids: valid input (osm)', function(t) { + var raw = { ids: 'osm:venue:500' }; var clean = {}; - var expected_clean={ - ids: [], - }; - // construct the expected id and type for each valid input - inputs.valid.forEach( function( input ){ - var input_parts = input.split(delimiter); - expected_clean.ids.push({ id: input_parts[1], type: input_parts[0] }); - }); + var expected_ids = [{ + id: '500', + types: [ 'osmnode', 'osmway' ] + }]; var messages = sanitize( raw, clean ); - t.deepEqual( messages.errors, [], 'no errors' ); - t.deepEqual( clean, expected_clean, 'clean set correctly' ); + t.deepEqual( messages.errors, [], ' no errors'); + t.deepEqual( clean.ids, expected_ids, 'osm could be two types, but that\'s ok'); t.end(); }); }; @@ -137,10 +141,10 @@ module.exports.tests.array_of_ids = function(test, common) { }; module.exports.tests.multiple_ids = function(test, common) { - test('duplicate ids', function(t) { - var expected_clean = { ids: [ { id: '1', type: 'geoname' }, { id: '2', type: 'osmnode' } ] }; - var raw = { ids: 'geoname:1,osmnode:2' }; + test('multiple ids', function(t) { + var raw = { ids: 'geonames:venue:1,osm:venue:2' }; var clean = {}; + var expected_clean = { ids: [ { id: '1', types: [ 'geoname' ] }, { id: '2', types: [ 'osmnode', 'osmway' ] } ] }; var messages = sanitize( raw, clean); @@ -153,8 +157,8 @@ module.exports.tests.multiple_ids = function(test, common) { module.exports.tests.de_dupe = function(test, common) { test('duplicate ids', function(t) { - var expected_clean = { ids: [ { id: '1', type: 'geoname' }, { id: '2', type: 'osmnode' } ]}; - var raw = { ids: 'geoname:1,osmnode:2,geoname:1' }; + var expected_clean = { ids: [ { id: '1', types: [ 'geoname' ] }, { id: '2', types: [ 'osmnode', 'osmway' ] } ] }; + var raw = { ids: 'geonames:venue:1,osm:venue:2,geonames:venue:1' }; var clean = {}; var messages = sanitize( raw, clean ); diff --git a/test/unit/sanitiser/_layers.js b/test/unit/sanitiser/_layers.js index 12d2cfd3..3264f434 100644 --- a/test/unit/sanitiser/_layers.js +++ b/test/unit/sanitiser/_layers.js @@ -1,5 +1,6 @@ +var type_mapping = require('../../../helper/type_mapping'); -var sanitize = require('../../../sanitiser/_targets')('layers', require('../../../query/layers')); +var sanitize = require('../../../sanitiser/_targets')('layers', type_mapping.layer_with_aliases_to_type); module.exports.tests = {}; @@ -42,7 +43,7 @@ module.exports.tests.sanitize_layers = function(test, common) { t.end(); }); test('address (alias) layer', function(t) { - var address_layers = ['osmaddress','openaddresses']; + var address_layers = ['osmaddress','openaddresses','geoname']; var raw = { layers: 'address' }; var clean = {}; @@ -75,7 +76,7 @@ module.exports.tests.sanitize_layers = function(test, common) { t.end(); }); test('address alias layer plus regular layers', function(t) { - var address_layers = ['osmaddress','openaddresses']; + var address_layers = ['osmaddress','openaddresses','geoname']; var reg_layers = ['admin0', 'locality']; var raw = { layers: 'address,country,locality' }; diff --git a/test/unit/sanitiser/_sources.js b/test/unit/sanitiser/_sources.js index 05507a36..d2ef405f 100644 --- a/test/unit/sanitiser/_sources.js +++ b/test/unit/sanitiser/_sources.js @@ -1,4 +1,5 @@ -var sanitize = require( '../../../sanitiser/_targets' )('sources', require('../../../query/sources')); +var type_mapping = require('../../../helper/type_mapping'); +var sanitize = require( '../../../sanitiser/_targets' )('sources', type_mapping.source_to_type); var success_messages = { error: false }; diff --git a/test/unit/sanitiser/place.js b/test/unit/sanitiser/place.js index c3cd81cf..f7cda1e9 100644 --- a/test/unit/sanitiser/place.js +++ b/test/unit/sanitiser/place.js @@ -1,7 +1,7 @@ var place = require('../../../sanitiser/place'), sanitize = place.sanitize, middleware = place.middleware, - defaultClean = { ids: [ { id: '123', type: 'geoname' } ], private: false }; + defaultClean = { ids: [ { id: '123', types: [ 'geoname' ] } ], private: false }; // these are the default values you would expect when no input params are specified. module.exports.tests = {}; @@ -31,7 +31,7 @@ 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: { ids:'geoname:123', 'private': value } }; + var req = { query: { ids:'geonames:venue:123', 'private': value } }; sanitize(req, function(){ t.deepEqual( req.errors, [], 'no errors' ); t.deepEqual( req.warnings, [], 'no warnings' ); @@ -44,7 +44,7 @@ module.exports.tests.sanitize_private = function(test, common) { var valid_values = ['true', true, 1]; valid_values.forEach(function(value) { test('valid private param ' + value, function(t) { - var req = { query: { ids:'geoname:123', 'private': value } }; + var req = { query: { ids:'geonames:venue:123', 'private': value } }; sanitize(req, function(){ t.deepEqual( req.errors, [], 'no errors' ); t.deepEqual( req.warnings, [], 'no warnings' ); @@ -57,7 +57,7 @@ module.exports.tests.sanitize_private = function(test, common) { var valid_false_values = ['false', false, 0]; valid_false_values.forEach(function(value) { test('test setting false explicitly ' + value, function(t) { - var req = { query: { ids:'geoname:123', 'private': value } }; + var req = { query: { ids:'geonames:venue:123', 'private': value } }; sanitize(req, function(){ t.deepEqual( req.errors, [], 'no errors' ); t.deepEqual( req.warnings, [], 'no warnings' ); @@ -68,7 +68,7 @@ module.exports.tests.sanitize_private = function(test, common) { }); test('test default behavior', function(t) { - var req = { query: { ids:'geoname:123' } }; + var req = { query: { ids:'geonames:venue:123' } }; sanitize(req, function(){ t.deepEqual( req.errors, [], 'no errors' ); t.deepEqual( req.warnings, [], 'no warnings' ); @@ -91,7 +91,7 @@ module.exports.tests.invalid_params = function(test, common) { module.exports.tests.middleware_success = function(test, common) { test('middleware success', function(t) { - var req = { query: { ids: 'geoname:123' }}; + var req = { query: { ids: 'geonames:venue:123' }}; var next = function(){ t.deepEqual( req.errors, [], 'no errors' ); t.deepEqual( req.warnings, [], 'no warnings' );