From dc623d5af7450332ef1ffc8510180b38a81c62de Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 17 Sep 2015 17:47:37 -0400 Subject: [PATCH 01/16] Send 3 part gid (source, layer, id) in GeoJSON responses --- helper/geojsonify.js | 11 +++++++++++ test/unit/helper/geojsonify.js | 3 +++ 2 files changed, 14 insertions(+) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 9ce77ad4..63b3cd88 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -177,6 +177,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 +195,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/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', From 4df0f98b142a401caaf6c22784bb487bd8629b55 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Fri, 18 Sep 2015 16:24:00 -0400 Subject: [PATCH 02/16] Add type list, and raw mappings to/from source/layer and type --- helper/type_mapping.js | 78 ++++++++++++++++++++++++++++++++ test/unit/helper/type_mapping.js | 46 +++++++++++++++++++ test/unit/run.js | 1 + 3 files changed, 125 insertions(+) create mode 100644 helper/type_mapping.js create mode 100644 test/unit/helper/type_mapping.js diff --git a/helper/type_mapping.js b/helper/type_mapping.js new file mode 100644 index 00000000..0ac2a30d --- /dev/null +++ b/helper/type_mapping.js @@ -0,0 +1,78 @@ +var ALL_TYPES = [ + 'geoname', + 'osmnode', + 'osmway', + 'admin0', + 'admin1', + 'admin2', + 'neighborhood', + 'locality', + 'local_admin', + 'osmaddress', + 'openaddresses' +]; + +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 includeds alias layers + */ +var LAYER_TO_TYPE = { + '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'], +}; + +module.exports = { + types_list: ALL_TYPES, + type_to_source: TYPE_TO_SOURCE, + type_to_layer: TYPE_TO_LAYER, + source_to_type: SOURCE_TO_TYPE, + layer_to_type: LAYER_TO_TYPE +}; diff --git a/test/unit/helper/type_mapping.js b/test/unit/helper/type_mapping.js new file mode 100644 index 00000000..1d1c945e --- /dev/null +++ b/test/unit/helper/type_mapping.js @@ -0,0 +1,46 @@ +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_list), 'is array'); + t.ok(check.hasLength(type_mapping.types_list, 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.ok(check.hasLength(Object.keys(type_mapping.layer_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/run.js b/test/unit/run.js index 50c301de..6b24259f 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -26,6 +26,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'), From 68c9661c7092018becea0c83da569b995d80453c Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Fri, 18 Sep 2015 16:40:05 -0400 Subject: [PATCH 03/16] Remove all mappings other than helper/type_mapping --- helper/geojsonify.js | 20 ++++---------------- helper/query_parser.js | 4 ++-- helper/types.js | 6 +++--- query/layers.js | 15 --------------- query/sources.js | 14 -------------- query/types.js | 16 ---------------- sanitiser/_ids.js | 3 ++- sanitiser/reverse.js | 5 +++-- sanitiser/search.js | 5 +++-- test/unit/helper/query_parser.js | 5 +++-- test/unit/query/types.js | 23 ----------------------- test/unit/run.js | 1 - test/unit/sanitiser/_ids.js | 4 ++-- test/unit/sanitiser/_layers.js | 3 ++- test/unit/sanitiser/_sources.js | 3 ++- 15 files changed, 26 insertions(+), 101 deletions(-) delete mode 100644 query/layers.js delete mode 100644 query/sources.js delete mode 100644 query/types.js delete mode 100644 test/unit/query/types.js diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 63b3cd88..d1010962 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -2,7 +2,8 @@ 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'); // Properties to be copied var DETAILS_PROPS = [ @@ -22,22 +23,9 @@ 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; } function lookupLayer(src) { diff --git a/helper/query_parser.js b/helper/query_parser.js index b60ebcd8..cee589b0 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_to_type.coarse; } }; diff --git a/helper/types.js b/helper/types.js index 249c27a0..c057345f 100644 --- a/helper/types.js +++ b/helper/types.js @@ -1,4 +1,4 @@ -var valid_types = require( '../query/types' ); +var type_mapping = require( '../helper/type_mapping' ); /** * Calculate the set-style intersection of two arrays @@ -24,7 +24,7 @@ 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_list; if (clean_types.from_layers) { types = intersection(types, clean_types.from_layers); @@ -46,4 +46,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..da509b0d 100644 --- a/sanitiser/_ids.js +++ b/sanitiser/_ids.js @@ -1,6 +1,7 @@ var _ = require('lodash'), check = require('check-types'), - types = require('../query/types'); + type_mapping = require('../helper/type_mapping'), + types = type_mapping.types_list; var ID_DELIM = ':'; diff --git a/sanitiser/reverse.js b/sanitiser/reverse.js index dd9eeb1d..11937bbc 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_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..dd46f419 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_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/unit/helper/query_parser.js b/test/unit/helper/query_parser.js index ad669ece..c1e3fd52 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_to_type; module.exports.tests = {}; 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 6b24259f..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'), diff --git a/test/unit/sanitiser/_ids.js b/test/unit/sanitiser/_ids.js index 02e25f18..26445018 100644 --- a/test/unit/sanitiser/_ids.js +++ b/test/unit/sanitiser/_ids.js @@ -1,7 +1,7 @@ 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' ] @@ -13,7 +13,7 @@ var formatError = function(input) { 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(', ') + ']'; + return type + ' is invalid. It must be one of these values - [' + type_mapping.types_list.join(', ') + ']'; }; module.exports.tests = {}; diff --git a/test/unit/sanitiser/_layers.js b/test/unit/sanitiser/_layers.js index 12d2cfd3..50117c08 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_to_type); module.exports.tests = {}; 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 }; From 8b1037d7c88e5ec51b5157a85a8cc5b48996ed04 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 21 Sep 2015 14:59:39 -0400 Subject: [PATCH 04/16] Separate real layers and alias layers into separate objects --- helper/query_parser.js | 2 +- helper/type_mapping.js | 16 ++++++++++++---- sanitiser/reverse.js | 2 +- sanitiser/search.js | 2 +- test/unit/helper/query_parser.js | 2 +- test/unit/helper/type_mapping.js | 8 +++++++- test/unit/sanitiser/_layers.js | 2 +- 7 files changed, 24 insertions(+), 10 deletions(-) diff --git a/helper/query_parser.js b/helper/query_parser.js index cee589b0..4af009aa 100644 --- a/helper/query_parser.js +++ b/helper/query_parser.js @@ -9,7 +9,7 @@ module.exports = {}; module.exports.get_layers = function get_layers(query) { if (query.length <= 3 ) { // no address parsing required - return type_mapping.layer_to_type.coarse; + return type_mapping.layer_with_aliases_to_type.coarse; } }; diff --git a/helper/type_mapping.js b/helper/type_mapping.js index 0ac2a30d..d2b0f758 100644 --- a/helper/type_mapping.js +++ b/helper/type_mapping.js @@ -1,3 +1,5 @@ +var extend = require('extend'); + var ALL_TYPES = [ 'geoname', 'osmnode', @@ -55,7 +57,7 @@ var SOURCE_TO_TYPE = { }; /** - * This includeds alias layers + * This does not included alias layers, those are built separately */ var LAYER_TO_TYPE = { 'venue': ['geoname','osmnode','osmway'], @@ -65,14 +67,20 @@ var LAYER_TO_TYPE = { 'county': ['admin2'], 'locality': ['locality'], 'localadmin': ['local_admin'], - 'neighbourhood': ['neighborhood'], - 'coarse': ['admin0','admin1','admin2','neighborhood','locality','local_admin'], + 'neighbourhood': ['neighborhood'] +}; + +var LAYER_ALIASES = { + 'coarse': ['admin0','admin1','admin2','neighborhood','locality','local_admin'] }; +var LAYER_WITH_ALIASES_TO_TYPE = extend({}, LAYER_ALIASES, LAYER_TO_TYPE); + module.exports = { types_list: ALL_TYPES, 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_to_type: LAYER_TO_TYPE, + layer_with_aliases_to_type: LAYER_WITH_ALIASES_TO_TYPE }; diff --git a/sanitiser/reverse.js b/sanitiser/reverse.js index 11937bbc..154fd3d9 100644 --- a/sanitiser/reverse.js +++ b/sanitiser/reverse.js @@ -3,7 +3,7 @@ var type_mapping = require('../helper/type_mapping'); var sanitizeAll = require('../sanitiser/sanitizeAll'), sanitizers = { singleScalarParameters: require('../sanitiser/_single_scalar_parameters'), - layers: require('../sanitiser/_targets')('layers', type_mapping.layer_to_type), + 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), diff --git a/sanitiser/search.js b/sanitiser/search.js index dd46f419..cc596b26 100644 --- a/sanitiser/search.js +++ b/sanitiser/search.js @@ -5,7 +5,7 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), singleScalarParameters: require('../sanitiser/_single_scalar_parameters'), text: require('../sanitiser/_text'), size: require('../sanitiser/_size'), - layers: require('../sanitiser/_targets')('layers', type_mapping.layer_to_type), + 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'), diff --git a/test/unit/helper/query_parser.js b/test/unit/helper/query_parser.js index c1e3fd52..02c96531 100644 --- a/test/unit/helper/query_parser.js +++ b/test/unit/helper/query_parser.js @@ -1,7 +1,7 @@ var parser = require('../../../helper/query_parser'); var type_mapping = require('../../../helper/type_mapping'); -var layers_map = type_mapping.layer_to_type; +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 index 1d1c945e..fa50c191 100644 --- a/test/unit/helper/type_mapping.js +++ b/test/unit/helper/type_mapping.js @@ -30,7 +30,13 @@ module.exports.tests.interfaces = function(test, common) { test('layer to type mapping', function(t) { t.ok(check.object(type_mapping.layer_to_type), 'is object'); - t.ok(check.hasLength(Object.keys(type_mapping.layer_to_type), 9), 'has correct number of elements'); + 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(); }); }; diff --git a/test/unit/sanitiser/_layers.js b/test/unit/sanitiser/_layers.js index 50117c08..519d95d9 100644 --- a/test/unit/sanitiser/_layers.js +++ b/test/unit/sanitiser/_layers.js @@ -1,6 +1,6 @@ var type_mapping = require('../../../helper/type_mapping'); -var sanitize = require('../../../sanitiser/_targets')('layers', type_mapping.layer_to_type); +var sanitize = require('../../../sanitiser/_targets')('layers', type_mapping.layer_with_aliases_to_type); module.exports.tests = {}; From 882aad4916592749075924efc47277219aa97e29 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 21 Sep 2015 17:32:00 -0400 Subject: [PATCH 05/16] Extract code to sanitize single id to separate function --- sanitiser/_ids.js | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/sanitiser/_ids.js b/sanitiser/_ids.js index da509b0d..b45f0d1c 100644 --- a/sanitiser/_ids.js +++ b/sanitiser/_ids.js @@ -15,6 +15,27 @@ var formatError = function(input) { return 'id `' + input + 'is invalid: must be of the format type:id for ex: \'geoname:4163334\''; }; +function sanitizeId(rawId, messages) { + 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 + }; + } +} + function sanitize( raw, clean ){ // error & warning messages var messages = { errors: [], warnings: [] }; @@ -43,25 +64,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)) { From fa44effdac7621d3e9f78910cee5bb1ae57be01f Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 21 Sep 2015 17:57:32 -0400 Subject: [PATCH 06/16] Add geoname type to most layers The Geonames dataset includes lots of different kinds of places, so add them to the mapping. --- helper/type_mapping.js | 12 ++++++------ test/unit/sanitiser/_layers.js | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/helper/type_mapping.js b/helper/type_mapping.js index d2b0f758..2f058543 100644 --- a/helper/type_mapping.js +++ b/helper/type_mapping.js @@ -61,13 +61,13 @@ var SOURCE_TO_TYPE = { */ var LAYER_TO_TYPE = { 'venue': ['geoname','osmnode','osmway'], - 'address': ['osmaddress','openaddresses'], - 'country': ['admin0'], - 'region': ['admin1'], - 'county': ['admin2'], - 'locality': ['locality'], + 'address': ['osmaddress','openaddresses', 'geoname'], + 'country': ['admin0', 'geoname'], + 'region': ['admin1', 'geoname'], + 'county': ['admin2', 'geoname'], + 'locality': ['locality', 'geoname'], 'localadmin': ['local_admin'], - 'neighbourhood': ['neighborhood'] + 'neighbourhood': ['neighborhood', 'geoname'] }; var LAYER_ALIASES = { diff --git a/test/unit/sanitiser/_layers.js b/test/unit/sanitiser/_layers.js index 519d95d9..3264f434 100644 --- a/test/unit/sanitiser/_layers.js +++ b/test/unit/sanitiser/_layers.js @@ -43,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 = {}; @@ -76,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' }; From b2f3b54f662c442dadd7e0dded656599301b3d7e Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 21 Sep 2015 18:10:08 -0400 Subject: [PATCH 07/16] Leave only geonames specific mapping in gejsonify helper --- helper/geojsonify.js | 50 ++++++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index d1010962..9822ad02 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -3,7 +3,8 @@ var GeoJSON = require('geojson'), extent = require('geojson-extent'), outputGenerator = require('./outputGenerator'), logger = require('pelias-logger').get('api'), - type_mapping = require('./type_mapping'); + type_mapping = require('./type_mapping'), + _ = require('lodash'); // Properties to be copied var DETAILS_PROPS = [ @@ -28,36 +29,25 @@ function lookupSource(src) { 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 (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 (_.contains(type_mapping.types_list, src._type)) { + return type_mapping.type_to_layer[src._type]; } logger.warn('[geojsonify]: could not map _type ', src._type); From aabca1569cc7c00a4ef00aa646532e26d63d92f1 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 21 Sep 2015 18:16:46 -0400 Subject: [PATCH 08/16] Fix old reference to /doc endpoint --- test/unit/controller/place.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/controller/place.js b/test/unit/controller/place.js index 1fde7942..755ac1b9 100644 --- a/test/unit/controller/place.js +++ b/test/unit/controller/place.js @@ -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 ){ From 2505e92a6211fd5ceffcb75a9c4b06c52a562c39 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 21 Sep 2015 18:22:24 -0400 Subject: [PATCH 09/16] Expect an array of types from _ids sanitiser This doesn't have any effect by itself but allows for the 3-part gid sanitiser to possibly return multiple types (i.e. in the case of osm:venue:1000) --- controller/place.js | 2 +- sanitiser/_ids.js | 2 +- test/unit/controller/place.js | 8 ++++---- test/unit/mock/backend.js | 2 +- test/unit/sanitiser/_ids.js | 8 ++++---- test/unit/sanitiser/place.js | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/controller/place.js b/controller/place.js index 8622f234..42e7c626 100644 --- a/controller/place.js +++ b/controller/place.js @@ -16,7 +16,7 @@ function setup( backend ){ var query = req.clean.ids.map( function(id) { return { _index: 'pelias', - _type: id.type, + type: id.types, _id: id.id }; }); diff --git a/sanitiser/_ids.js b/sanitiser/_ids.js index b45f0d1c..46effa41 100644 --- a/sanitiser/_ids.js +++ b/sanitiser/_ids.js @@ -31,7 +31,7 @@ function sanitizeId(rawId, messages) { else { return { id: id, - type: type + types: [type] }; } } diff --git a/test/unit/controller/place.js b/test/unit/controller/place.js index 755ac1b9..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(); 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/sanitiser/_ids.js b/test/unit/sanitiser/_ids.js index 26445018..3be9183b 100644 --- a/test/unit/sanitiser/_ids.js +++ b/test/unit/sanitiser/_ids.js @@ -90,7 +90,7 @@ 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 expected_clean = { ids: [ { id: input_parts[1], types: [ input_parts[0] ]} ]}; var raw = { ids: input }; var clean = {}; @@ -111,7 +111,7 @@ module.exports.tests.valid_ids = function(test, common) { // 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] }); + expected_clean.ids.push({ id: input_parts[1], types: [ input_parts[0] ]}); }); var messages = sanitize( raw, clean ); @@ -138,7 +138,7 @@ 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 expected_clean = { ids: [ { id: '1', types: [ 'geoname' ] }, { id: '2', types: [ 'osmnode' ] } ] }; var raw = { ids: 'geoname:1,osmnode:2' }; var clean = {}; @@ -153,7 +153,7 @@ 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 expected_clean = { ids: [ { id: '1', types: [ 'geoname' ] }, { id: '2', types: [ 'osmnode' ] } ] }; var raw = { ids: 'geoname:1,osmnode:2,geoname:1' }; var clean = {}; diff --git a/test/unit/sanitiser/place.js b/test/unit/sanitiser/place.js index c3cd81cf..26042c5e 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 = {}; From da1314eeff13bd86c37c8f4679391d2c6e02f70e Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 21 Sep 2015 17:34:20 -0400 Subject: [PATCH 10/16] Add mapping function to get type from source and layer --- helper/type_mapping.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/helper/type_mapping.js b/helper/type_mapping.js index 2f058543..a00979b7 100644 --- a/helper/type_mapping.js +++ b/helper/type_mapping.js @@ -76,11 +76,25 @@ var LAYER_ALIASES = { var LAYER_WITH_ALIASES_TO_TYPE = extend({}, LAYER_ALIASES, LAYER_TO_TYPE); +/** + * 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 sourceAndLayerToType = function sourceAndLayerToType(source, layer) { + return intersection(SOURCE_TO_TYPE[source], LAYER_WITH_ALIASES_TO_TYPE[layer]); +}; + module.exports = { types_list: ALL_TYPES, 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 + layer_with_aliases_to_type: LAYER_WITH_ALIASES_TO_TYPE, + source_and_layer_to_type: sourceAndLayerToType }; From d11d1854292f5ef75742c6a8f6085b2d829dba73 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 21 Sep 2015 18:30:24 -0400 Subject: [PATCH 11/16] Fix missing space in error message --- sanitiser/_ids.js | 2 +- test/unit/sanitiser/_ids.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sanitiser/_ids.js b/sanitiser/_ids.js index 46effa41..d7648960 100644 --- a/sanitiser/_ids.js +++ b/sanitiser/_ids.js @@ -12,7 +12,7 @@ var ID_DELIM = ':'; 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 type:id for ex: \'geoname:4163334\''; }; function sanitizeId(rawId, messages) { diff --git a/test/unit/sanitiser/_ids.js b/test/unit/sanitiser/_ids.js index 3be9183b..d7c65d33 100644 --- a/test/unit/sanitiser/_ids.js +++ b/test/unit/sanitiser/_ids.js @@ -8,7 +8,7 @@ var inputs = { }; 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 type:id for ex: \'geoname:4163334\''; }; var lengthError = 'invalid param \'ids\': length must be >0'; var defaultMissingTypeError = function(input) { From d4fff265748ba1f6945f02c908ef45d1a11fb8d7 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 21 Sep 2015 18:56:52 -0400 Subject: [PATCH 12/16] Derive type, source, and layer list instead of hardcoding --- helper/geojsonify.js | 2 +- helper/type_mapping.js | 25 ++++++++++--------------- helper/types.js | 2 +- test/unit/helper/type_mapping.js | 6 +++--- 4 files changed, 15 insertions(+), 20 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 9822ad02..94b41b84 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -46,7 +46,7 @@ function lookupLayer(src) { if (src.address) { return 'address'; } } - if (_.contains(type_mapping.types_list, src._type)) { + if (_.contains(type_mapping.types, src._type)) { return type_mapping.type_to_layer[src._type]; } diff --git a/helper/type_mapping.js b/helper/type_mapping.js index a00979b7..ab7bbe56 100644 --- a/helper/type_mapping.js +++ b/helper/type_mapping.js @@ -1,19 +1,5 @@ var extend = require('extend'); -var ALL_TYPES = [ - 'geoname', - 'osmnode', - 'osmway', - 'admin0', - 'admin1', - 'admin2', - 'neighborhood', - 'locality', - 'local_admin', - 'osmaddress', - 'openaddresses' -]; - var TYPE_TO_SOURCE = { 'geoname': 'gn', 'osmnode': 'osm', @@ -76,6 +62,13 @@ var LAYER_ALIASES = { 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); + /** * Calculate the set-style intersection of two arrays */ @@ -90,7 +83,9 @@ var sourceAndLayerToType = function sourceAndLayerToType(source, layer) { }; module.exports = { - types_list: ALL_TYPES, + types: TYPES, + sources: SOURCES, + layers: LAYERS, type_to_source: TYPE_TO_SOURCE, type_to_layer: TYPE_TO_LAYER, source_to_type: SOURCE_TO_TYPE, diff --git a/helper/types.js b/helper/types.js index c057345f..aa3337e8 100644 --- a/helper/types.js +++ b/helper/types.js @@ -24,7 +24,7 @@ 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 = type_mapping.types_list; + var types = type_mapping.types; if (clean_types.from_layers) { types = intersection(types, clean_types.from_layers); diff --git a/test/unit/helper/type_mapping.js b/test/unit/helper/type_mapping.js index fa50c191..144fadfb 100644 --- a/test/unit/helper/type_mapping.js +++ b/test/unit/helper/type_mapping.js @@ -4,9 +4,9 @@ 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_list), 'is array'); - t.ok(check.hasLength(type_mapping.types_list, 11), 'has correct number of elements'); + 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(); }); From 2811569bcd10957c8c239e04d168190a2b22152e Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 21 Sep 2015 18:59:11 -0400 Subject: [PATCH 13/16] Switch to source:layer:id ids format in /place --- sanitiser/_ids.js | 55 ++++++++----- test/ciao/place/basic_place.coffee | 4 +- test/ciao/reverse/layers_invalid.coffee | 4 +- .../reverse/layers_mix_invalid_valid.coffee | 4 +- test/ciao/reverse/layers_multiple.coffee | 4 +- test/ciao/reverse/layers_single.coffee | 4 +- .../sources_layers_invalid_combo.coffee | 4 +- .../reverse/sources_layers_valid_combo.coffee | 4 +- test/ciao/search/layers_alias_address.coffee | 4 +- test/ciao/search/layers_invalid.coffee | 4 +- .../search/layers_mix_invalid_valid.coffee | 4 +- test/ciao/search/layers_multiple.coffee | 4 +- test/ciao/search/layers_single.coffee | 4 +- .../sources_layers_invalid_combo.coffee | 4 +- .../search/sources_layers_valid_combo.coffee | 4 +- test/unit/sanitiser/_ids.js | 78 ++++++++++--------- test/unit/sanitiser/place.js | 10 +-- 17 files changed, 110 insertions(+), 89 deletions(-) diff --git a/sanitiser/_ids.js b/sanitiser/_ids.js index d7648960..0b4d91c7 100644 --- a/sanitiser/_ids.js +++ b/sanitiser/_ids.js @@ -1,39 +1,56 @@ var _ = require('lodash'), check = require('check-types'), - type_mapping = require('../helper/type_mapping'), - types = type_mapping.types_list; + 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 param_index = rawId.indexOf(ID_DELIM); - var type = rawId.substring(0, param_index ); - var id = rawId.substring(param_index + 1); + var parts = rawId.split(ID_DELIM); + + if ( parts.length < 3 ) { + messages.errors.push( formatError(rawId) ); + return; + } - // check id format - if(!check.contains(rawId, ID_DELIM) || !check.unemptyString( id ) || !check.unemptyString( type )) { + 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; } - // 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(', ') + ']' ); + + if (!_.contains(type_mapping.sources, source)) { + messages.errors.push( targetError(source, type_mapping.sources) ); + return; } - else { - return { - id: id, - types: [type] - }; + + 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 ){ 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/sanitiser/_ids.js b/test/unit/sanitiser/_ids.js index d7c65d33..9438f3ee 100644 --- a/test/unit/sanitiser/_ids.js +++ b/test/unit/sanitiser/_ids.js @@ -8,13 +8,9 @@ var inputs = { }; 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 - [' + type_mapping.types_list.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], types: [ 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], types: [ 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', types: [ 'geoname' ] }, { id: '2', types: [ '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', types: [ 'geoname' ] }, { id: '2', types: [ '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/place.js b/test/unit/sanitiser/place.js index 26042c5e..f7cda1e9 100644 --- a/test/unit/sanitiser/place.js +++ b/test/unit/sanitiser/place.js @@ -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' ); From 7b9d61c5ecddc0438742e83186b43b75c55354a2 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 22 Sep 2015 13:51:46 -0400 Subject: [PATCH 14/16] Use lodash intersection method --- helper/type_mapping.js | 14 +++----------- helper/types.js | 14 +++----------- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/helper/type_mapping.js b/helper/type_mapping.js index ab7bbe56..27fce81c 100644 --- a/helper/type_mapping.js +++ b/helper/type_mapping.js @@ -1,4 +1,5 @@ -var extend = require('extend'); +var extend = require('extend'), + _ = require('lodash'); var TYPE_TO_SOURCE = { 'geoname': 'gn', @@ -69,17 +70,8 @@ var TYPES = Object.keys(TYPE_TO_SOURCE); var SOURCES = Object.keys(SOURCE_TO_TYPE); var LAYERS = Object.keys(LAYER_TO_TYPE); -/** - * 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 sourceAndLayerToType = function sourceAndLayerToType(source, layer) { - return intersection(SOURCE_TO_TYPE[source], LAYER_WITH_ALIASES_TO_TYPE[layer]); + return _.intersection(SOURCE_TO_TYPE[source], LAYER_WITH_ALIASES_TO_TYPE[layer]); }; module.exports = { diff --git a/helper/types.js b/helper/types.js index aa3337e8..610d1aba 100644 --- a/helper/types.js +++ b/helper/types.js @@ -1,13 +1,5 @@ var type_mapping = require( '../helper/type_mapping' ); - -/** - * 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 _ = require('lodash'); /** * Combine all types and determine the unique subset @@ -27,11 +19,11 @@ module.exports = function calculate_types(clean_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; From e157749de33d9d7fb3f37dc53c476f2ed7f02f4c Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 22 Sep 2015 13:57:53 -0400 Subject: [PATCH 15/16] Use _.contains instead of .indexOf --- helper/geojsonify.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 94b41b84..ca499613 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -35,10 +35,10 @@ function lookupSource(src) { */ function lookupLayer(src) { if (src._type === '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'; } + 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' } From a2cc31f142cb0b197d3c08db9b0274b296097439 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 22 Sep 2015 14:12:12 -0400 Subject: [PATCH 16/16] Leave a note for future us A small essay on Elasticsearch queries and fields and types. --- controller/place.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/controller/place.js b/controller/place.js index 42e7c626..101430c5 100644 --- a/controller/place.js +++ b/controller/place.js @@ -16,6 +16,15 @@ function setup( backend ){ var query = req.clean.ids.map( function(id) { return { _index: 'pelias', + /* + * 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 };