From cdd00af6bc766ac3c0c94494b119242ff07db605 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 21 Jan 2016 15:29:31 -0500 Subject: [PATCH 01/13] Build responses with new source and layer fields There is no longer any ambiguity in the source field and the layer (which is stored as _type), so a lot of logic to compute them can simply read from the elasticsearch document. Hooray! --- helper/geojsonify.js | 21 +-------------------- test/unit/helper/geojsonify.js | 25 ++++++++++++++----------- 2 files changed, 15 insertions(+), 31 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index d77792ee..b415b18f 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -25,8 +25,7 @@ var DETAILS_PROPS = [ function lookupSource(src) { - var sources = type_mapping.type_to_source; - return sources.hasOwnProperty(src._type) ? sources[src._type] : src._type; + return src.source; } /* @@ -34,24 +33,6 @@ function lookupSource(src) { * Elasticsearch document source allows a more specific layer name to be chosen */ function lookupLayer(src) { - if (src._type === 'geoname') { - if (_.includes(src.category, 'admin')) { - if (_.includes(src.category, 'admin:city')) { return 'locality'; } - if (_.includes(src.category, 'admin:admin1')) { return 'region'; } - if (_.includes(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 (_.includes(type_mapping.types, src._type)) { - return type_mapping.type_to_layer[src._type]; - } - - logger.warn('[geojsonify]: could not map _type ', src._type); - return src._type; } diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index 3c969302..a5311050 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -40,7 +40,8 @@ module.exports.tests.search = function(test, common) { var input = [ { '_id': 'id1', - '_type': 'type1', + '_type': 'layer1', + 'source': 'source1', 'center_point': { 'lat': 51.5337144, 'lon': -0.1069716 @@ -72,7 +73,8 @@ module.exports.tests.search = function(test, common) { }, { '_id': 'id2', - '_type': 'type2', + '_type': 'layer2', + 'source': 'source2', 'name': { 'default': 'Blues Cafe' }, @@ -97,7 +99,8 @@ module.exports.tests.search = function(test, common) { }, { '_id': '34633854', - '_type': 'osmway', + '_type': 'way', + 'source': 'osm', 'name': { 'default': 'Empire State Building' }, @@ -141,9 +144,9 @@ module.exports.tests.search = function(test, common) { }, 'properties': { 'id': 'id1', - 'gid': 'type1:type1:id1', - 'layer': 'type1', - 'source': 'type1', + 'gid': 'source1:layer1:id1', + 'layer': 'layer1', + 'source': 'source1', 'label': '\'Round Midnight Jazz and Blues Bar, test3, Angel', 'name': '\'Round Midnight Jazz and Blues Bar', 'country_a': 'GBR', @@ -170,9 +173,9 @@ module.exports.tests.search = function(test, common) { }, 'properties': { 'id': 'id2', - 'gid': 'type2:type2:id2', - 'layer': 'type2', - 'source': 'type2', + 'gid': 'source2:layer2:id2', + 'layer': 'layer2', + 'source': 'source2', 'label': 'Blues Cafe, test3, Smithfield', 'name': 'Blues Cafe', 'country_a': 'GBR', @@ -196,8 +199,8 @@ module.exports.tests.search = function(test, common) { }, 'properties': { 'id': '34633854', - 'gid': 'osm:venue:34633854', - 'layer': 'venue', + 'gid': 'osm:way:34633854', + 'layer': 'way', 'source': 'osm', 'label': 'Empire State Building, Manhattan, NY, USA', 'name': 'Empire State Building', From 9ec2cd41d943b0f7a845f166c16a4ae8c80d8a0d Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 21 Jan 2016 16:07:21 -0500 Subject: [PATCH 02/13] Remove unused file: sanitiser/_source.js It was replaced by the dual purpose sanitiser/_targets.js --- sanitiser/_source.js | 46 -------------------------------------------- 1 file changed, 46 deletions(-) delete mode 100644 sanitiser/_source.js diff --git a/sanitiser/_source.js b/sanitiser/_source.js deleted file mode 100644 index 6a4661dd..00000000 --- a/sanitiser/_source.js +++ /dev/null @@ -1,46 +0,0 @@ - -var _ = require('lodash'), - check = require('check-types'), - sources_map = require( '../query/sources' ); - -var ALL_SOURCES = Object.keys(sources_map), - ALL_SOURCES_JOINED = ALL_SOURCES.join(','); - -function sanitize( raw, clean ) { - - // error & warning messages - var messages = { errors: [], warnings: [] }; - - // init clean.types (if not already init) - clean.types = clean.types || {}; - - // default case (no layers specified in GET params) - // don't even set the from_layers key in this case - if( check.nonEmptyString( raw.source ) ){ - - var sources = raw.source.split(','); - - var invalid_sources = sources.filter(function(source) { - return !_.includes( ALL_SOURCES, source ); - }); - - if( invalid_sources.length > 0 ){ - invalid_sources.forEach( function( invalid ){ - messages.errors.push('\'' + invalid + '\' is an invalid source parameter. Valid options: ' + ALL_SOURCES_JOINED); - }); - } - - else { - var types = sources.reduce(function(acc, source) { - return acc.concat(sources_map[source]); - }, []); - - clean.types.from_source = types; - } - - } - - return messages; -} - -module.exports = sanitize; From fa07ba9ddcd26b6a1666022b90473b5acf76d39a Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 21 Jan 2016 16:43:10 -0500 Subject: [PATCH 03/13] Add much needed whitespace --- test/unit/sanitiser/_layers.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/unit/sanitiser/_layers.js b/test/unit/sanitiser/_layers.js index 519d95d9..16ca19ae 100644 --- a/test/unit/sanitiser/_layers.js +++ b/test/unit/sanitiser/_layers.js @@ -10,6 +10,7 @@ module.exports.tests.sanitize_layers = function(test, common) { t.equal(messages.errors.length, 0, 'no errors'); t.end(); }); + test('invalid layer', function(t) { var raw = { layers: 'test_layer' }; var clean = {}; @@ -22,6 +23,7 @@ module.exports.tests.sanitize_layers = function(test, common) { t.true(messages.errors[0].length > msg.length, 'invalid error message'); t.end(); }); + test('venue (alias) layer', function(t) { var venue_layers = ['geoname','osmnode','osmway']; var raw = { layers: 'venue' }; @@ -32,6 +34,7 @@ module.exports.tests.sanitize_layers = function(test, common) { t.deepEqual(clean.types.from_layers, venue_layers, 'venue layers set'); t.end(); }); + test('coarse (alias) layer', function(t) { var admin_layers = ['admin0','admin1','admin2','neighborhood','locality','local_admin']; var raw = { layers: 'coarse' }; @@ -42,6 +45,7 @@ module.exports.tests.sanitize_layers = function(test, common) { t.deepEqual(clean.types.from_layers, admin_layers, 'coarse layers set'); t.end(); }); + test('address (alias) layer', function(t) { var address_layers = ['osmaddress','openaddresses']; var raw = { layers: 'address' }; @@ -52,6 +56,7 @@ module.exports.tests.sanitize_layers = function(test, common) { t.deepEqual(clean.types.from_layers, address_layers, 'address layers set'); t.end(); }); + test('venue alias layer plus regular layers', function(t) { var venue_layers = ['geoname','osmnode','osmway']; var reg_layers = ['admin0', 'admin1']; @@ -63,6 +68,7 @@ module.exports.tests.sanitize_layers = function(test, common) { t.deepEqual(clean.types.from_layers, venue_layers.concat(reg_layers), 'venue + regular layers'); t.end(); }); + test('coarse alias layer plus regular layers', function(t) { var admin_layers = ['admin0','admin1','admin2','neighborhood','locality','local_admin']; var reg_layers = ['geoname', 'osmnode', 'osmway']; @@ -75,6 +81,7 @@ module.exports.tests.sanitize_layers = function(test, common) { t.deepEqual(clean.types.from_layers, admin_layers.concat(reg_layers), 'coarse + regular layers set'); t.end(); }); + test('address alias layer plus regular layers', function(t) { var address_layers = ['osmaddress','openaddresses']; var reg_layers = ['admin0', 'locality']; @@ -87,6 +94,7 @@ module.exports.tests.sanitize_layers = function(test, common) { t.deepEqual(clean.types.from_layers, address_layers.concat(reg_layers), 'address + regular layers set'); t.end(); }); + test('alias layer plus regular layers (no duplicates)', function(t) { var venue_layers = ['geoname','osmnode','osmway','admin0']; var raw = { layers: 'venue,country' }; @@ -97,6 +105,7 @@ module.exports.tests.sanitize_layers = function(test, common) { t.deepEqual(clean.types.from_layers, venue_layers, 'venue layers found (no duplicates)'); t.end(); }); + test('multiple alias layers (no duplicates)', function(t) { var alias_layers = ['geoname','osmnode','osmway','admin0','admin1','admin2','neighborhood','locality','local_admin']; var raw = { layers: 'venue,coarse' }; @@ -110,7 +119,6 @@ module.exports.tests.sanitize_layers = function(test, common) { }; module.exports.all = function (tape, common) { - function test(name, testFunction) { return tape('SANTIZE _layers ' + name, testFunction); } From 0ee1fdb4d946d7e572c754934cea2a51b2c8bce5 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 16 Feb 2016 17:11:51 -0500 Subject: [PATCH 04/13] Remove obsolete Geonames ID tests These are fixed now with our better source/layer mapping --- test/unit/sanitiser/_ids.js | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/test/unit/sanitiser/_ids.js b/test/unit/sanitiser/_ids.js index 40af368f..98237180 100644 --- a/test/unit/sanitiser/_ids.js +++ b/test/unit/sanitiser/_ids.js @@ -122,34 +122,6 @@ module.exports.tests.valid_ids = function(test, common) { }); }; -module.exports.tests.geonames = function(test, common) { - test('geonames venue maps correctly as normal', function(t) { - var raw = { ids: 'geonames:venue:15' }; - var clean = {}; - - var messages = sanitize( raw, clean); - - var expected_clean = { ids: [ { id: '15', types: [ 'geoname' ] } ] }; - t.deepEqual( messages.errors, [], 'no errors' ); - t.deepEqual( messages.warnings, [], 'no warnings' ); - t.deepEqual(clean, expected_clean, 'clean set correctly'); - t.end(); - }); - - test('arbitrary geonames layer maps to geoname type', function(t) { - var raw = { ids: 'geonames:address:16' }; // geonames technically has no address records! - var clean = {}; - - var messages = sanitize( raw, clean); - - var expected_clean = { ids: [ { id: '16', types: [ 'geoname' ] } ] }; - t.deepEqual( messages.errors, [], 'no errors' ); - t.deepEqual( messages.warnings, [], 'no warnings' ); - t.deepEqual(clean, expected_clean, 'clean set correctly'); - t.end(); - }); -}; - module.exports.tests.multiple_ids = function(test, common) { test('multiple ids', function(t) { var raw = { ids: 'geonames:venue:1,osm:venue:2' }; From 8fdb522f11f840cd050387e7abb0152895003781 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 24 Feb 2016 18:46:41 -0500 Subject: [PATCH 05/13] Upgrade to pelias-query 5.1.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1d681bf8..180576f6 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "pelias-config": "^1.0.1", "pelias-esclient": "1.0.0", "pelias-logger": "^0.0.8", - "pelias-query": "^5.0.0", + "pelias-query": "^5.1.0", "pelias-schema": "3.0.0", "pelias-suggester-pipeline": "2.0.4", "stats-lite": "1.0.3", From ef78b2e0fda3ae64f522e25b8c84ea80c2326994 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Fri, 22 Jan 2016 17:35:15 -0500 Subject: [PATCH 06/13] Filter source and layer by new fields This is functionally the same code as before, except that it uses the new source and layer mapping. --- controller/place.js | 30 +------ controller/search.js | 7 +- helper/text_parser.js | 2 +- helper/type_mapping.js | 126 +++++++++++++++--------------- helper/types.js | 43 ---------- middleware/_types.js | 40 ++-------- query/search.js | 4 +- sanitiser/_ids.js | 11 +-- sanitiser/_targets.js | 21 ++--- sanitiser/_text.js | 4 - sanitiser/reverse.js | 4 +- sanitiser/search.js | 4 +- service/mget.js | 1 - test/unit/controller/place.js | 8 +- test/unit/helper/text_parser.js | 2 +- test/unit/helper/type_mapping.js | 130 ++++--------------------------- test/unit/helper/types.js | 92 ---------------------- test/unit/run.js | 1 - test/unit/sanitiser/_ids.js | 47 +++++++---- test/unit/sanitiser/_layers.js | 54 +++++++------ test/unit/sanitiser/_sources.js | 31 ++++---- test/unit/sanitiser/_text.js | 1 - test/unit/sanitiser/place.js | 2 +- test/unit/sanitiser/reverse.js | 9 +-- test/unit/sanitiser/search.js | 5 +- 25 files changed, 197 insertions(+), 482 deletions(-) delete mode 100644 helper/types.js delete mode 100644 test/unit/helper/types.js diff --git a/controller/place.js b/controller/place.js index 1aa22aeb..29acfe8b 100644 --- a/controller/place.js +++ b/controller/place.js @@ -14,35 +14,10 @@ function setup( backend ){ return next(); } - /* req.clean.ids contains an array of objects with id and types properties. - * types is an array of one or more types, since it can't always be known which single - * type a gid might belong to (osmnode and osmway both have source osm and layer venue). - * - * However, the mget Elasticsearch query only accepts a single type at a - * time. - * - * So, first create a new array that, has an entry - * with each type and id combination. This requires creating a new array with more entries - * than req.clean.ids in the case where entries have multiple types. - */ - - var recordsToReturn = req.clean.ids.reduce(function (acc, ids_element) { - ids_element.types.forEach(function(type) { - acc.push({ - id: ids_element.id, - type: type - }); - }); - return acc; - }, []); - - /* - * Next, map the list of records to an Elasticsearch mget query - */ - var query = recordsToReturn.map( function(id) { + var query = req.clean.ids.map( function(id) { return { _index: 'pelias', - _type: id.type, + _type: id.layers, _id: id.id }; }); @@ -50,6 +25,7 @@ function setup( backend ){ logger.debug( '[ES req]', JSON.stringify(query) ); service.mget( backend, query, function( err, docs ) { + console.log('err:' + err); // error handler if( err ){ diff --git a/controller/search.js b/controller/search.js index 9fafdd20..4adaf31d 100644 --- a/controller/search.js +++ b/controller/search.js @@ -8,10 +8,10 @@ function setup( backend, query ){ query = query || require('../query/search'); function controller( req, res, next ){ - // do not run controller when a request // validation error has occurred. if( req.errors && req.errors.length ){ + delete req.clean.type; //to declutter output return next(); } @@ -25,9 +25,12 @@ function setup( backend, query ){ body: query( req.clean ) }; - // ? + // set the Elasticsearch types to filter by, + // and remove the property from clean so the API + // response output is cleaner if( req.clean.hasOwnProperty('type') ){ cmd.type = req.clean.type; + delete req.clean.type; } logger.debug( '[ES req]', JSON.stringify(cmd) ); diff --git a/helper/text_parser.js b/helper/text_parser.js index a561aef9..7b9ffcfe 100644 --- a/helper/text_parser.js +++ b/helper/text_parser.js @@ -15,7 +15,7 @@ module.exports = {}; module.exports.get_layers = function get_layers(query) { if (query.length <= 3 ) { // no address parsing required - return type_mapping.layer_with_aliases_to_type.coarse; + return type_mapping.layer_mapping.coarse; } }; diff --git a/helper/type_mapping.js b/helper/type_mapping.js index 06a8ec2d..ced5f1f3 100644 --- a/helper/type_mapping.js +++ b/helper/type_mapping.js @@ -1,87 +1,85 @@ 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' -}; +function addStandardTargetsToAliases(standard, aliases) { + var combined = _.extend({}, aliases); + standard.forEach(function(target) { + if (combined[target] === undefined) { + combined[target] = [target]; + } + }); + + return combined; +} /* - * This doesn't include alias layers such as coarse + * Sources */ -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'] +// a list of all sources +var SOURCES = ['openstreetmap', 'openaddresses', 'geonames', 'quattroshapes', 'whosonfirst']; + +/* + * A list of alternate names for sources, mostly used to save typing + */ +var SOURCE_ALIASES = { + 'osm': ['openstreetmap'], + 'oa': ['openaddresses'], + 'gn': ['geonames'], + 'qs': ['quattroshapes'], + 'wof': ['whosonfirst'] }; -/** - * This does not included alias layers, those are built separately +/* + * Create an object that contains all sources or aliases. The key is the source or alias, + * the value is either that source, or the canonical name for that alias if it's an alias. */ -var LAYER_TO_TYPE = { - 'venue': ['geoname','osmnode','osmway'], - 'address': ['osmaddress','openaddresses'], - 'country': ['admin0'], - 'region': ['admin1'], - 'county': ['admin2'], - 'locality': ['locality'], - 'localadmin': ['local_admin'], - 'neighbourhood': ['neighborhood'] +var SOURCE_MAPPING = addStandardTargetsToAliases(SOURCES, SOURCE_ALIASES); + +/* + * Layers + */ + +/* + * A list of all layers in each source. This is used for convenience elswhere + * and to determine when a combination of source and layer parameters is + * not going to match any records and will return no results. + */ +var LAYERS_BY_SOURCE = { + openstreetmap: [ 'address', 'venue' ], + openaddresses: [ 'address' ], + geonames: [ 'country', 'region', 'county', 'locality', 'venue' ], + quattroshapes: ['admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin'], + whosonfirst: [ 'continent', 'macrocountry', 'country', 'dependency', 'region', + 'locality', 'localadmin', 'county', 'macrohood', 'neighbourhood', 'microhood', 'disputed'] }; +/* + * A list of layer aliases that can be used to support specific use cases + * (like coarse geocoding) * or work around the fact that different sources + * may have layers that mean the same thing but have a different name + */ var LAYER_ALIASES = { - 'coarse': ['admin0','admin1','admin2','neighborhood','locality','local_admin'] + 'coarse': LAYERS_BY_SOURCE.whosonfirst.concat(LAYERS_BY_SOURCE.quattroshapes), + 'country': ['country', 'admin0'], // Include both QS and WOF layers for various types of places + 'region': ['region', 'admin1'] // Alias layers that include themselves look weird, but do work }; -var LAYER_WITH_ALIASES_TO_TYPE = extend({}, LAYER_ALIASES, LAYER_TO_TYPE); +// create a list of all layers by combining each entry from LAYERS_BY_SOURCE +var LAYERS = _.uniq(Object.keys(LAYERS_BY_SOURCE).reduce(function(acc, key) { + return acc.concat(LAYERS_BY_SOURCE[key]); +}, [])); /* - * derive the list of types, sources, and layers from above mappings + * Create the an object that has a key for each possible layer or alias, + * and returns either that layer, or all the layers in the alias */ -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]); -}; +var LAYER_MAPPING = addStandardTargetsToAliases(LAYERS, LAYER_ALIASES); 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 + source_mapping: SOURCE_MAPPING, + layer_mapping: LAYER_MAPPING, + layers_by_source: LAYERS_BY_SOURCE }; diff --git a/helper/types.js b/helper/types.js deleted file mode 100644 index d9dec59d..00000000 --- a/helper/types.js +++ /dev/null @@ -1,43 +0,0 @@ -var type_mapping = require( '../helper/type_mapping' ); -var _ = require('lodash'); - -/** - * Different parts of the code express "preferences" for which Elasticsearch types are going to be searched - * This method decides how to combine all the preferences. - * - * @param {Array} clean_types - * @returns {Array} - */ -module.exports = function calculate_types(clean_types) { - //Check that at least one preference of types is defined - if (!clean_types || !(clean_types.from_layers || clean_types.from_sources || clean_types.from_text_parser)) { - throw new Error('clean_types should not be null or undefined'); - } - - /* the layers and source parameters are cumulative: - * perform a set intersection of their specified types - */ - if (clean_types.from_layers || clean_types.from_sources) { - var types = type_mapping.types; - - if (clean_types.from_layers) { - types = _.intersection(types, clean_types.from_layers); - } - - if (clean_types.from_sources) { - types = _.intersection(types, clean_types.from_sources); - } - - return types; - } - - /* - * Type restrictions requested by the address parser should only be used - * if both the source and layers parameters are empty, so do this last - */ - if (clean_types.from_text_parser) { - return clean_types.from_text_parser; - } - - throw new Error('no types specified'); -}; diff --git a/middleware/_types.js b/middleware/_types.js index 3e604f7b..ccabdab9 100644 --- a/middleware/_types.js +++ b/middleware/_types.js @@ -1,43 +1,15 @@ -var types_helper = require( '../helper/types' ); - /** - * Validate the types specified to be searched. + * Take the layers specified by the layers parameter and use them to set the + * list of Elasticsearch types to filter. * - * Elasticsearch interprets an empty array of types as "search anything" rather - * than "search nothing", so in the case of an empty array, return an error - * message instead of searching at all. + * This has to be done outside the layers sanitizer since it doesn't know that + * the layers property is eventualy used to choose the _type. */ function middleware(req, res, next) { req.clean = req.clean || {}; - if (req.clean.hasOwnProperty('types')) { - - try { - var types = types_helper(req.clean.types); - - if ((types instanceof Array) && types.length === 0) { - var err = 'You have specified both the `sources` and `layers` ' + - 'parameters in a combination that will return no results.'; - req.errors.push( err ); - } - - else { - req.clean.type = types; - } - - } - - // @todo: refactor this flow, it is confusing as `types_helper()` can throw - // with an error "clean_types should not be null or undefined" which is - // not returned to the user yet the return value CAN trigger a user error. - // I would have liked to throw for BOTH cases and then handle the users errors - // inside the 'catch' but this is not possible. - // also: why are we deleting things from $clean? - catch (err) { - // this means there were no types specified - delete req.clean.types; - } - + if (req.clean.hasOwnProperty('layers')) { + req.clean.type = req.clean.layers; } next(); diff --git a/query/search.js b/query/search.js index 764cf999..804cb88b 100644 --- a/query/search.js +++ b/query/search.js @@ -37,7 +37,7 @@ query.score( peliasQuery.view.admin('neighborhood') ); // non-scoring hard filters query.filter( peliasQuery.view.boundary_circle ); query.filter( peliasQuery.view.boundary_rect ); - +query.filter( peliasQuery.view.sources ); // -------------------------------- /** @@ -51,6 +51,8 @@ function generateQuery( clean ){ // input text vs.var( 'input:name', clean.text ); + vs.var( 'sources', clean.sources); + // size if( clean.querySize ) { vs.var( 'size', clean.querySize ); diff --git a/sanitiser/_ids.js b/sanitiser/_ids.js index a4d135a0..4a1844be 100644 --- a/sanitiser/_ids.js +++ b/sanitiser/_ids.js @@ -45,17 +45,10 @@ function sanitizeId(rawId, messages) { return; } - //TODO: remove this once we have a better set of layers for Geonames - var types; - if (source === 'gn' || source === 'geonames') { - types = ['geoname']; - } else { - types = type_mapping.source_and_layer_to_type(source, layer); - } - return { + source: source, + layer: layer, id: id, - types: types }; } diff --git a/sanitiser/_targets.js b/sanitiser/_targets.js index 550d3a0c..a8a50400 100644 --- a/sanitiser/_targets.js +++ b/sanitiser/_targets.js @@ -1,25 +1,24 @@ - var _ = require('lodash'), check = require('check-types'); +function getValidKeys(mapping) { + return _.uniq(Object.keys(mapping)).join(','); +} + function setup( paramName, targetMap ) { return function( raw, clean ){ return sanitize( raw, clean, { paramName: paramName, targetMap: targetMap, - targetMapKeysString: Object.keys(targetMap).join(',') + targetMapKeysString: getValidKeys(targetMap) }); }; } function sanitize( raw, clean, opts ) { - // error & warning messages var messages = { errors: [], warnings: [] }; - // init clean.types - clean.types = clean.types || {}; - // the string of targets (comma delimeted) var targetsString = raw[opts.paramName]; @@ -51,19 +50,13 @@ function sanitize( raw, clean, opts ) { // only set types value when no error occured if( !messages.errors.length ){ - - // store the values under a new key as 'clean.types.from_*' - var typesKey = 'from_' + opts.paramName; - - // ? - clean.types[typesKey] = targets.reduce(function(acc, target) { + clean[opts.paramName] = targets.reduce(function(acc, target) { return acc.concat(opts.targetMap[target]); }, []); // dedupe in case aliases expanded to common things or user typed in duplicates - clean.types[typesKey] = _.uniq(clean.types[typesKey]); + clean[opts.paramName] = _.uniq(clean[opts.paramName]); } - } } diff --git a/sanitiser/_text.js b/sanitiser/_text.js index 53898ff0..e6897a5e 100644 --- a/sanitiser/_text.js +++ b/sanitiser/_text.js @@ -23,10 +23,6 @@ function sanitize( raw, clean ){ if (check.assigned(parsed_text)) { clean.parsed_text = parsed_text; } - - // try to set layers from query parser results - clean.types = clean.layers || {}; - clean.types.from_text_parser = text_parser.get_layers(clean.text); } return messages; diff --git a/sanitiser/reverse.js b/sanitiser/reverse.js index 154fd3d9..045ff3a1 100644 --- a/sanitiser/reverse.js +++ b/sanitiser/reverse.js @@ -3,8 +3,8 @@ 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_with_aliases_to_type), - sources: require('../sanitiser/_targets')('sources', type_mapping.source_to_type), + layers: require('../sanitiser/_targets')('layers', type_mapping.layer_mapping), + sources: require('../sanitiser/_targets')('sources', type_mapping.source_mapping), 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 cc596b26..1be32b02 100644 --- a/sanitiser/search.js +++ b/sanitiser/search.js @@ -5,8 +5,8 @@ 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_with_aliases_to_type), - sources: require('../sanitiser/_targets')('sources', type_mapping.source_to_type), + layers: require('../sanitiser/_targets')('layers', type_mapping.layer_mapping), + sources: require('../sanitiser/_targets')('sources', type_mapping.source_mapping), private: require('../sanitiser/_flag_bool')('private', false), geo_search: require('../sanitiser/_geo_search'), boundary_country: require('../sanitiser/_boundary_country'), diff --git a/service/mget.js b/service/mget.js index a878e8cb..529d0fc1 100644 --- a/service/mget.js +++ b/service/mget.js @@ -24,7 +24,6 @@ function service( backend, query, cb ){ // query new backend backend().client.mget( cmd, function( err, data ){ - // handle backend errors if( err ){ return cb( err ); } diff --git a/test/unit/controller/place.js b/test/unit/controller/place.js index e8a8c333..977be59e 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, types: [ 'a' ] } ] }, errors: [], warnings: [] }; + var req = { clean: { ids: [ {'id' : 123, layers: [ '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, types: [ 'b' ] } ] }, errors: [], warnings: [] }; + var req = { clean: { ids: [ {'id' : 123, layers: [ '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/helper/text_parser.js b/test/unit/helper/text_parser.js index 3accf7b9..90b8fa52 100644 --- a/test/unit/helper/text_parser.js +++ b/test/unit/helper/text_parser.js @@ -1,7 +1,7 @@ var parser = require('../../../helper/text_parser'); var type_mapping = require('../../../helper/type_mapping'); -var layers_map = type_mapping.layer_with_aliases_to_type; +var layers_map = type_mapping.layer_mapping; module.exports.tests = {}; diff --git a/test/unit/helper/type_mapping.js b/test/unit/helper/type_mapping.js index b44b00a9..39d9c564 100644 --- a/test/unit/helper/type_mapping.js +++ b/test/unit/helper/type_mapping.js @@ -4,132 +4,32 @@ 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'); + test('basic layer mapping', function(t) { + t.deepEquals(type_mapping.layer_mapping.venue, ['venue']); + t.deepEquals(type_mapping.layer_mapping.address, ['address']); 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'); + test('alias layer mapping', function(t) { + t.deepEquals(type_mapping.layer_mapping.coarse, + [ 'continent', 'macrocountry', 'country', 'dependency', + 'region', 'locality', 'localadmin', 'county', 'macrohood', + 'neighbourhood', 'microhood', 'disputed', 'admin0', + 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin']); 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'); + test('basic source mapping', function(t) { + t.deepEquals(type_mapping.source_mapping.osm, ['openstreetmap']); + t.deepEquals(type_mapping.source_mapping.openaddresses, ['openaddresses']); 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'); + test('alias source mapping', function(t) { + t.deepEquals(type_mapping.source_mapping.openstreetmap, ['openstreetmap']); + t.deepEquals(type_mapping.source_mapping.wof, ['whosonfirst']); 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(); - }); - - test('\'osm\' and \'openstreetmap\' sources should only successfully map to \'venue\' and \'address\' layers', function(t) { - t.deepEquals(type_mapping.source_and_layer_to_type('osm', 'venue'), ['osmnode', 'osmway']); - t.deepEquals(type_mapping.source_and_layer_to_type('osm', 'address'), ['osmaddress']); - t.deepEquals(type_mapping.source_and_layer_to_type('osm', 'country'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('osm', 'region'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('osm', 'county'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('osm', 'locality'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('osm', 'localadmin'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('osm', 'neighbourhood'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('osm', 'coarse'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openstreetmap', 'venue'), ['osmnode', 'osmway']); - t.deepEquals(type_mapping.source_and_layer_to_type('openstreetmap', 'address'), ['osmaddress']); - t.deepEquals(type_mapping.source_and_layer_to_type('openstreetmap', 'country'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openstreetmap', 'region'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openstreetmap', 'county'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openstreetmap', 'locality'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openstreetmap', 'localadmin'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openstreetmap', 'neighbourhood'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openstreetmap', 'coarse'), []); - t.end(); - }); - - test('\'gn\' and \'geonames\' sources should only successfully map to \'venue\' layers', function(t) { - t.deepEquals(type_mapping.source_and_layer_to_type('gn', 'venue'), ['geoname']); - t.deepEquals(type_mapping.source_and_layer_to_type('gn', 'address'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('gn', 'country'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('gn', 'region'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('gn', 'county'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('gn', 'locality'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('gn', 'localadmin'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('gn', 'neighbourhood'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('gn', 'coarse'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('geonames', 'venue'), ['geoname']); - t.deepEquals(type_mapping.source_and_layer_to_type('geonames', 'address'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('geonames', 'country'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('geonames', 'region'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('geonames', 'county'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('geonames', 'locality'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('geonames', 'localadmin'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('geonames', 'neighbourhood'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('geonames', 'coarse'), []); - t.end(); - }); - - test('\'oa\' and \'openaddresses\' sources should only successfully map to \'address\' layer', function(t) { - t.deepEquals(type_mapping.source_and_layer_to_type('oa', 'venue'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('oa', 'address'), ['openaddresses']); - t.deepEquals(type_mapping.source_and_layer_to_type('oa', 'country'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('oa', 'region'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('oa', 'county'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('oa', 'locality'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('oa', 'localadmin'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('oa', 'neighbourhood'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('oa', 'coarse'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openaddresses', 'venue'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openaddresses', 'address'), ['openaddresses']); - t.deepEquals(type_mapping.source_and_layer_to_type('openaddresses', 'country'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openaddresses', 'region'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openaddresses', 'county'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openaddresses', 'locality'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openaddresses', 'localadmin'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openaddresses', 'neighbourhood'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('openaddresses', 'coarse'), []); - t.end(); - }); - - test('\'qs\' and \'quattroshapes\' sources should only successfully map to \'address\' layer', function(t) { - t.deepEquals(type_mapping.source_and_layer_to_type('qs', 'venue'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('qs', 'address'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('qs', 'country'), ['admin0']); - t.deepEquals(type_mapping.source_and_layer_to_type('qs', 'region'), ['admin1']); - t.deepEquals(type_mapping.source_and_layer_to_type('qs', 'county'), ['admin2']); - t.deepEquals(type_mapping.source_and_layer_to_type('qs', 'locality'), ['locality']); - t.deepEquals(type_mapping.source_and_layer_to_type('qs', 'localadmin'), ['local_admin']); - t.deepEquals(type_mapping.source_and_layer_to_type('qs', 'neighbourhood'), ['neighborhood']); - t.deepEquals(type_mapping.source_and_layer_to_type('qs', 'coarse'), - ['admin0','admin1','admin2','neighborhood','locality','local_admin']); - t.deepEquals(type_mapping.source_and_layer_to_type('quattroshapes', 'venue'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('quattroshapes', 'address'), []); - t.deepEquals(type_mapping.source_and_layer_to_type('quattroshapes', 'country'), ['admin0']); - t.deepEquals(type_mapping.source_and_layer_to_type('quattroshapes', 'region'), ['admin1']); - t.deepEquals(type_mapping.source_and_layer_to_type('quattroshapes', 'county'), ['admin2']); - t.deepEquals(type_mapping.source_and_layer_to_type('quattroshapes', 'locality'), ['locality']); - t.deepEquals(type_mapping.source_and_layer_to_type('quattroshapes', 'localadmin'), ['local_admin']); - t.deepEquals(type_mapping.source_and_layer_to_type('quattroshapes', 'neighbourhood'), ['neighborhood']); - t.deepEquals(type_mapping.source_and_layer_to_type('quattroshapes', 'coarse'), - ['admin0','admin1','admin2','neighborhood','locality','local_admin']); - t.end(); - }); - }; module.exports.all = function (tape, common) { diff --git a/test/unit/helper/types.js b/test/unit/helper/types.js deleted file mode 100644 index 4c68b46c..00000000 --- a/test/unit/helper/types.js +++ /dev/null @@ -1,92 +0,0 @@ -var types = require('../../../helper/types'); - -module.exports.tests = {}; - -module.exports.tests.no_cleaned_types = function(test, common) { - test('no cleaned types', function(t) { - function testIt() { - types({}); - } - - t.throws(testIt, /clean_types should not be null or undefined/, 'no input should result in exception'); - - t.end(); - }); -}; - -module.exports.tests.address_parser = function(test, common) { - test('address parser specifies only admin layers', function(t) { - var cleaned_types = { - from_text_parser: ['admin0'] // simplified return value from address parser - }; - var actual = types(cleaned_types); - var expected = ['admin0']; // simplified expected value for all admin layers - t.deepEqual(actual, expected, 'only layers specified by address parser returned'); - t.end(); - }); -}; - -module.exports.tests.layers_parameter = function(test, common) { - test('layers parameter specifies only some layers', function(t) { - var cleaned_types = { - from_layers: ['geoname'] - }; - var actual = types(cleaned_types); - var expected = ['geoname']; - t.deepEqual(actual, expected, 'only types specified by layers parameter returned'); - t.end(); - }); -}; - -module.exports.tests.layers_parameter_and_address_parser = function(test, common) { - test('layers parameter and address parser present', function(t) { - var cleaned_types = { - from_layers: ['geoname'], - from_text_parser: ['admin0'] // simplified return value from address parse - }; - var actual = types(cleaned_types); - var expected = ['geoname']; - t.deepEqual(actual, expected, 'layers parameter overrides address parser completely'); - t.end(); - }); -}; - -module.exports.tests.source_parameter = function(test, common) { - test('source parameter specified', function(t) { - var cleaned_types = { - from_sources: ['openaddresses'] - }; - - var actual = types(cleaned_types); - - var expected = ['openaddresses']; - t.deepEqual(actual, expected, 'type parameter set to types specified by source'); - t.end(); - }); -}; - -module.exports.tests.source_and_layers_parameters = function(test, common) { - test('source and layers parameter both specified', function(t) { - var cleaned_types = { - from_sources: ['openaddresses'], - from_layers: ['osmaddress', 'openaddresses'] - }; - - var actual = types(cleaned_types); - - var expected = ['openaddresses']; - t.deepEqual(actual, expected, 'type set to intersection of source and layer types'); - 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 c600e0ad..89b7c31b 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -15,7 +15,6 @@ var tests = [ require('./helper/labelSchema'), require('./helper/text_parser'), require('./helper/type_mapping'), - require('./helper/types'), require('./helper/sizeCalculator'), require('./middleware/confidenceScore'), require('./middleware/confidenceScoreReverse'), diff --git a/test/unit/sanitiser/_ids.js b/test/unit/sanitiser/_ids.js index 98237180..64e749b0 100644 --- a/test/unit/sanitiser/_ids.js +++ b/test/unit/sanitiser/_ids.js @@ -94,60 +94,79 @@ module.exports.tests.valid_ids = function(test, common) { 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 ); + var expected_ids = [{ + source: 'openaddresses', + layer: 'address', + id: '20', + }]; t.deepEqual( messages.errors, [], ' no errors'); t.deepEqual( clean.ids, expected_ids, 'single type value returned'); t.end(); }); test('ids: valid input (osm)', function(t) { - var raw = { ids: 'osm:venue:500' }; + var raw = { ids: 'openstreetmap:venue:node:500' }; var clean = {}; var expected_ids = [{ - id: '500', - types: [ 'osmnode', 'osmway' ] + source: 'openstreetmap', + layer: 'venue', + id: 'node:500', }]; var messages = sanitize( raw, clean ); t.deepEqual( messages.errors, [], ' no errors'); - t.deepEqual( clean.ids, expected_ids, 'osm could be two types, but that\'s ok'); + t.deepEqual( clean.ids, expected_ids, 'osm has node: or way: in id field'); t.end(); }); }; module.exports.tests.multiple_ids = function(test, common) { test('multiple ids', function(t) { - var raw = { ids: 'geonames:venue:1,osm:venue:2' }; + var raw = { ids: 'geonames:venue:1,openstreetmap:address:way:2' }; var clean = {}; - var expected_clean = { ids: [ { id: '1', types: [ 'geoname' ] }, { id: '2', types: [ 'osmnode', 'osmway' ] } ] }; var messages = sanitize( raw, clean); + var expected_ids = [ { + source: 'geonames', + layer: 'venue', + id: '1' + }, { + source: 'openstreetmap', + layer: 'address', + id: 'way:2' + } ]; + t.deepEqual( messages.errors, [], 'no errors' ); t.deepEqual( messages.warnings, [], 'no warnings' ); - t.deepEqual(clean, expected_clean, 'clean set correctly'); + t.deepEqual(clean.ids, expected_ids, 'clean set correctly'); t.end(); }); }; 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', 'osmway' ] } ] }; - var raw = { ids: 'geonames:venue:1,osm:venue:2,geonames:venue:1' }; + var raw = { ids: 'geonames:venue:1,openstreetmap:venue:node:2,geonames:venue:1' }; var clean = {}; var messages = sanitize( raw, clean ); + var expected_ids = [ { + source: 'geonames', + layer: 'venue', + id: '1' + }, { + source: 'openstreetmap', + layer: 'venue', + id: 'node:2' + } ]; t.deepEqual( messages.errors, [], 'no errors' ); t.deepEqual( messages.warnings, [], 'no warnings' ); - t.deepEqual(clean, expected_clean, 'clean set correctly'); + t.deepEqual(clean.ids, expected_ids, 'clean set correctly'); t.end(); }); }; diff --git a/test/unit/sanitiser/_layers.js b/test/unit/sanitiser/_layers.js index 16ca19ae..bd499fc7 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_with_aliases_to_type); +var sanitize = require('../../../sanitiser/_targets')('layers', type_mapping.layer_mapping); module.exports.tests = {}; @@ -25,95 +25,101 @@ module.exports.tests.sanitize_layers = function(test, common) { }); test('venue (alias) layer', function(t) { - var venue_layers = ['geoname','osmnode','osmway']; var raw = { layers: 'venue' }; var clean = {}; sanitize(raw, clean); - t.deepEqual(clean.types.from_layers, venue_layers, 'venue layers set'); + var venue_layers = ['venue']; + t.deepEqual(clean.layers, venue_layers, 'venue layers set'); t.end(); }); test('coarse (alias) layer', function(t) { - var admin_layers = ['admin0','admin1','admin2','neighborhood','locality','local_admin']; var raw = { layers: 'coarse' }; var clean = {}; sanitize(raw, clean); - t.deepEqual(clean.types.from_layers, admin_layers, 'coarse layers set'); + var admin_layers = [ 'continent', 'macrocountry', 'country', 'dependency', + 'region', 'locality', 'localadmin', 'county', 'macrohood', 'neighbourhood', + 'microhood', 'disputed', 'admin0', 'admin1', 'admin2', 'neighborhood', 'local_admin' ]; + + t.deepEqual(clean.layers, admin_layers, 'coarse layers set'); t.end(); }); - test('address (alias) layer', function(t) { - var address_layers = ['osmaddress','openaddresses']; + test('address layer', function(t) { var raw = { layers: 'address' }; var clean = {}; sanitize(raw, clean); - t.deepEqual(clean.types.from_layers, address_layers, 'address layers set'); + t.deepEqual(clean.layers, ['address'], 'address layer set'); t.end(); }); test('venue alias layer plus regular layers', function(t) { - var venue_layers = ['geoname','osmnode','osmway']; - var reg_layers = ['admin0', 'admin1']; var raw = { layers: 'venue,country,region' }; var clean = {}; sanitize(raw, clean); - t.deepEqual(clean.types.from_layers, venue_layers.concat(reg_layers), 'venue + regular layers'); + var expected_layers = ['venue', 'country', 'admin0', 'region', 'admin1']; + t.deepEqual(clean.layers, expected_layers, 'venue + regular layers'); t.end(); }); test('coarse alias layer plus regular layers', function(t) { - var admin_layers = ['admin0','admin1','admin2','neighborhood','locality','local_admin']; - var reg_layers = ['geoname', 'osmnode', 'osmway']; - - var raw = { layers: 'coarse,venue,country' }; + var raw = { layers: 'coarse,country' }; var clean = {}; sanitize(raw, clean); - t.deepEqual(clean.types.from_layers, admin_layers.concat(reg_layers), 'coarse + regular layers set'); + var expected_layers = [ 'continent', 'macrocountry', 'country', 'dependency', + 'region', 'locality', 'localadmin', 'county', 'macrohood', 'neighbourhood', + 'microhood', 'disputed', 'admin0', 'admin1', 'admin2', 'neighborhood', 'local_admin' ]; + + t.deepEqual(clean.layers, expected_layers, 'coarse + regular layers set'); t.end(); }); test('address alias layer plus regular layers', function(t) { - var address_layers = ['osmaddress','openaddresses']; - var reg_layers = ['admin0', 'locality']; - var raw = { layers: 'address,country,locality' }; var clean = {}; sanitize(raw, clean); - t.deepEqual(clean.types.from_layers, address_layers.concat(reg_layers), 'address + regular layers set'); + var expected_layers = ['address', 'country', 'admin0', 'locality' ]; + t.deepEqual(clean.layers, expected_layers, 'address + regular layers set'); t.end(); }); test('alias layer plus regular layers (no duplicates)', function(t) { - var venue_layers = ['geoname','osmnode','osmway','admin0']; var raw = { layers: 'venue,country' }; var clean = {}; sanitize(raw, clean); - t.deepEqual(clean.types.from_layers, venue_layers, 'venue layers found (no duplicates)'); + var expected_layers = ['venue', 'country', 'admin0']; + t.deepEqual(clean.layers, expected_layers, 'venue layers found (no duplicates)'); t.end(); }); test('multiple alias layers (no duplicates)', function(t) { - var alias_layers = ['geoname','osmnode','osmway','admin0','admin1','admin2','neighborhood','locality','local_admin']; var raw = { layers: 'venue,coarse' }; var clean = {}; sanitize(raw, clean); - t.deepEqual(clean.types.from_layers, alias_layers, 'all layers found (no duplicates)'); + var coarse_layers = [ 'continent', 'macrocountry', + 'country', 'dependency', 'region', 'locality', 'localadmin', + 'county', 'macrohood', 'neighbourhood', 'microhood', + 'disputed', 'admin0', 'admin1', 'admin2', 'neighborhood', 'local_admin']; + + var venue_layers = [ 'venue' ]; + var expected_layers = venue_layers.concat(coarse_layers); + t.deepEqual(clean.layers, expected_layers, 'all layers found (no duplicates)'); t.end(); }); }; diff --git a/test/unit/sanitiser/_sources.js b/test/unit/sanitiser/_sources.js index d2ef405f..61b2be90 100644 --- a/test/unit/sanitiser/_sources.js +++ b/test/unit/sanitiser/_sources.js @@ -1,5 +1,5 @@ var type_mapping = require('../../../helper/type_mapping'); -var sanitize = require( '../../../sanitiser/_targets' )('sources', type_mapping.source_to_type); +var sanitize = require( '../../../sanitiser/_targets' )('sources', type_mapping.source_mapping); var success_messages = { error: false }; @@ -14,7 +14,7 @@ module.exports.tests.no_sources = function(test, common) { var messages = sanitize(req.query, req.clean); - t.deepEqual(req.clean.types, {}, 'clean.types should be empty object'); + t.equal(req.clean.sources, undefined, 'no sources should be defined'); t.deepEqual(messages.errors, [], 'no error returned'); t.deepEqual(messages.warnings, [], 'no warnings returned'); t.end(); @@ -29,11 +29,11 @@ module.exports.tests.no_sources = function(test, common) { }; var expected_error = 'sources parameter cannot be an empty string. ' + - 'Valid options: gn,geonames,oa,openaddresses,qs,quattroshapes,osm,openstreetmap'; + 'Valid options: osm,oa,gn,qs,wof,openstreetmap,openaddresses,geonames,quattroshapes,whosonfirst'; var messages = sanitize(req.query, req.clean); - t.deepEqual(req.clean.types, {}, 'clean.types should be empty object'); + t.equal(req.clean.sources, undefined, 'no sources should be defined'); t.deepEqual(messages.errors.length, 1, 'error returned'); t.deepEqual(messages.errors[0], expected_error, 'error returned'); t.deepEqual(messages.warnings, [], 'no warnings returned'); @@ -52,27 +52,24 @@ module.exports.tests.valid_sources = function(test, common) { var messages = sanitize(req.query, req.clean); - t.deepEqual(req.clean.types, { from_sources: ['geoname'] }, 'clean.types should contain from_source entry with geonames'); + t.deepEqual(req.clean.sources, ['geonames'], 'sources should contain geonames'); t.deepEqual(messages.errors, [], 'no error returned'); t.deepEqual(messages.warnings, [], 'no warnings returned'); t.end(); }); - test('openstreetmap source', function(t) { + test('openstreetmap (abbreviated) source', function(t) { var req = { query: { - sources: 'openstreetmap' + sources: 'osm' }, clean: { } }; - var expected_types = { - from_sources: ['osmaddress', 'osmnode', 'osmway'] - }; var messages = sanitize(req.query, req.clean); - t.deepEqual(req.clean.types, expected_types, 'clean.types should contain from_source entry with multiple entries for openstreetmap'); + t.deepEqual(req.clean.sources, ['openstreetmap'], 'abbreviation is expanded to full version'); t.deepEqual(messages.errors, [], 'no error returned'); t.deepEqual(messages.warnings, [], 'no warnings returned'); t.end(); @@ -85,14 +82,11 @@ module.exports.tests.valid_sources = function(test, common) { }, clean: { } }; - var expected_types = { - from_sources: ['osmaddress', 'osmnode', 'osmway', 'openaddresses'] - }; var messages = sanitize(req.query, req.clean); - t.deepEqual(req.clean.types, expected_types, - 'clean.types should contain from_source entry with multiple entries for openstreetmap and openadresses'); + t.deepEqual(req.clean.sources, ['openstreetmap', 'openaddresses'], + 'clean.sources should contain openstreetmap and openadresses'); t.deepEqual(messages.errors, [], 'no error returned'); t.deepEqual(messages.warnings, [], 'no warnings returned'); t.end(); @@ -109,7 +103,8 @@ module.exports.tests.invalid_sources = function(test, common) { }; var expected_messages = { errors: [ - '\'notasource\' is an invalid sources parameter. Valid options: gn,geonames,oa,openaddresses,qs,quattroshapes,osm,openstreetmap' + '\'notasource\' is an invalid sources parameter. ' + + 'Valid options: osm,oa,gn,qs,wof,openstreetmap,openaddresses,geonames,quattroshapes,whosonfirst' ], warnings: [] }; @@ -117,7 +112,7 @@ module.exports.tests.invalid_sources = function(test, common) { var messages = sanitize(req.query, req.clean); t.deepEqual(messages, expected_messages, 'error with message returned'); - t.deepEqual(req.clean.types, { }, 'clean.types should remain empty'); + t.equal(req.clean.sources, undefined, 'clean.sources should remain empty'); t.end(); }); }; diff --git a/test/unit/sanitiser/_text.js b/test/unit/sanitiser/_text.js index 8bc0710f..3868c86c 100644 --- a/test/unit/sanitiser/_text.js +++ b/test/unit/sanitiser/_text.js @@ -15,7 +15,6 @@ module.exports.tests.text_parser = function(test, common) { t.deepEquals(messages.errors, [], 'no errors'); t.deepEquals(messages.warnings, [], 'no warnings'); - t.equal(clean.types.from_text_parser, type_mapping.layer_with_aliases_to_type.coarse, 'coarse layers preferred'); t.end(); }); diff --git a/test/unit/sanitiser/place.js b/test/unit/sanitiser/place.js index f7cda1e9..dd9d1645 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', types: [ 'geoname' ] } ], private: false }; + defaultClean = { ids: [ { source: 'geonames', layer: 'venue', id: '123' } ], private: false }; // these are the default values you would expect when no input params are specified. module.exports.tests = {}; diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js index 4e727b5b..a55568e8 100644 --- a/test/unit/sanitiser/reverse.js +++ b/test/unit/sanitiser/reverse.js @@ -11,15 +11,13 @@ var reverse = require('../../../sanitiser/reverse'), 'boundary.circle.lat': 0, 'boundary.circle.lon': 0, 'boundary.circle.radius': parseFloat(defaults['boundary:circle:radius']), - types: { - }, size: 10, private: false }; // these are the default values you would expect when no input params are specified. // @todo: why is this different from $defaultClean? -var emptyClean = { private: false, size: 10, types: {} }; +var emptyClean = { private: false, size: 10 }; module.exports.tests = {}; @@ -38,7 +36,8 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.sanitisers = function(test, common) { test('check sanitiser list', function (t) { - var expected = ['singleScalarParameters', 'layers', 'sources', 'size', 'private', 'geo_reverse', 'boundary_country']; + var expected = ['singleScalarParameters', 'layers', 'sources', + 'sources_and_layers', 'size', 'private', 'geo_reverse', 'boundary_country']; t.deepEqual(Object.keys(reverse.sanitiser_list), expected); t.end(); }); @@ -104,7 +103,7 @@ module.exports.tests.sanitize_lon = function(test, common) { var req = { query: { 'point.lat': 0, 'point.lon': lon } }; // @todo: why is lat set? - var expected = { 'point.lat': 0, private: false, size: 10, types: {} }; + var expected = { 'point.lat': 0, private: false, size: 10 }; sanitize(req, function(){ t.equal(req.errors[0], 'missing param \'point.lon\'', 'longitude is a required field'); t.deepEqual(req.clean, expected, 'clean only has default values set'); diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index b00160dd..d42d87f4 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -5,7 +5,7 @@ var extend = require('extend'), middleware = search.middleware, defaultError = 'invalid param \'text\': text length, must be >0'; // these are the default values you would expect when no input params are specified. -var emptyClean = { private: false, size: 10, types: {} }; +var emptyClean = { private: false, size: 10 }; module.exports.tests = {}; @@ -24,7 +24,8 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.sanitisers = function(test, common) { test('check sanitiser list', function (t) { - var expected = ['singleScalarParameters', 'text', 'size', 'layers', 'sources', 'private', 'geo_search', 'boundary_country' ]; + var expected = ['singleScalarParameters', 'text', 'size', 'layers', 'sources', + 'sources_and_layers', 'private', 'geo_search', 'boundary_country' ]; t.deepEqual(Object.keys(search.sanitiser_list), expected); t.end(); }); From ce292baeafbdacf11c12c271995183dc60da610a Mon Sep 17 00:00:00 2001 From: missinglink Date: Wed, 24 Feb 2016 12:37:54 +0100 Subject: [PATCH 07/13] fixes for focus.point to work with wof integration --- query/view/focus_selected_layers.js | 6 ++---- .../fixture/autocomplete_linguistic_focus.js | 18 ++++-------------- ...utocomplete_linguistic_focus_null_island.js | 18 ++++-------------- 3 files changed, 10 insertions(+), 32 deletions(-) diff --git a/query/view/focus_selected_layers.js b/query/view/focus_selected_layers.js index 038d7ffa..ba19919f 100644 --- a/query/view/focus_selected_layers.js +++ b/query/view/focus_selected_layers.js @@ -22,10 +22,8 @@ module.exports = function( subview ){ if( view && view.hasOwnProperty('function_score') ){ view.function_score.filter = { 'or': [ - { 'type': { 'value': 'osmnode' } }, - { 'type': { 'value': 'osmway' } }, - { 'type': { 'value': 'osmaddress' } }, - { 'type': { 'value': 'openaddresses' } } + { 'term': { 'layer': 'venue' } }, + { 'term': { 'layer': 'address' } } ] }; } diff --git a/test/unit/fixture/autocomplete_linguistic_focus.js b/test/unit/fixture/autocomplete_linguistic_focus.js index 7f3c7174..83156c0b 100644 --- a/test/unit/fixture/autocomplete_linguistic_focus.js +++ b/test/unit/fixture/autocomplete_linguistic_focus.js @@ -47,23 +47,13 @@ module.exports = { 'filter': { 'or': [ { - 'type': { - 'value': 'osmnode' + 'term': { + 'layer': 'venue' } }, { - 'type': { - 'value': 'osmway' - } - }, - { - 'type': { - 'value': 'osmaddress' - } - }, - { - 'type': { - 'value': 'openaddresses' + 'term': { + 'layer': 'address' } } ] diff --git a/test/unit/fixture/autocomplete_linguistic_focus_null_island.js b/test/unit/fixture/autocomplete_linguistic_focus_null_island.js index 46554715..f28b6234 100644 --- a/test/unit/fixture/autocomplete_linguistic_focus_null_island.js +++ b/test/unit/fixture/autocomplete_linguistic_focus_null_island.js @@ -47,23 +47,13 @@ module.exports = { 'filter': { 'or': [ { - 'type': { - 'value': 'osmnode' + 'term': { + 'layer': 'venue' } }, { - 'type': { - 'value': 'osmway' - } - }, - { - 'type': { - 'value': 'osmaddress' - } - }, - { - 'type': { - 'value': 'openaddresses' + 'term': { + 'layer': 'address' } } ] From 616dd606a731ae5e57bd7cf0cacc999b9d820d33 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 2 Mar 2016 15:44:00 -0500 Subject: [PATCH 08/13] Add sources and layers sanitiser This sanitiser can do a better job of determining when an invalid combination of sources and layers was specified, and produce helpful error messages. --- sanitiser/_sources_and_layers.js | 37 +++++++ sanitiser/reverse.js | 2 + sanitiser/search.js | 2 + test/unit/run.js | 1 + test/unit/sanitiser/_sources_and_layers.js | 115 +++++++++++++++++++++ 5 files changed, 157 insertions(+) create mode 100644 sanitiser/_sources_and_layers.js create mode 100644 test/unit/sanitiser/_sources_and_layers.js diff --git a/sanitiser/_sources_and_layers.js b/sanitiser/_sources_and_layers.js new file mode 100644 index 00000000..448a7af1 --- /dev/null +++ b/sanitiser/_sources_and_layers.js @@ -0,0 +1,37 @@ +var _ = require( 'lodash' ); +var type_mapping = require( '../helper/type_mapping' ); + +/* + * This sanitiser depends on clean.layers and clean.sources + * so it has to be run after those sanitisers have been run + */ +function sanitize( raw, clean ){ + var messages = { errors: [], warnings: [] }; + + var possible_errors = []; + var at_least_one_valid_combination = false; + + if (clean.layers && clean.sources) { + clean.sources.forEach(function(source) { + var layers_for_source = type_mapping.layers_by_source[source]; + clean.layers.forEach(function(layer) { + if (_.includes(layers_for_source, layer)) { + at_least_one_valid_combination = true; + } else { + var message = 'You have specified both the `sources` and `layers` ' + + 'parameters in a combination that will return no results: the ' + + source + ' source has nothing in the ' + layer + ' layer'; + possible_errors.push(message); + } + }); + }); + + if (!at_least_one_valid_combination) { + messages.errors = possible_errors; + } + } + + return messages; +} + +module.exports = sanitize; diff --git a/sanitiser/reverse.js b/sanitiser/reverse.js index 045ff3a1..d707fcea 100644 --- a/sanitiser/reverse.js +++ b/sanitiser/reverse.js @@ -5,6 +5,8 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), singleScalarParameters: require('../sanitiser/_single_scalar_parameters'), layers: require('../sanitiser/_targets')('layers', type_mapping.layer_mapping), sources: require('../sanitiser/_targets')('sources', type_mapping.source_mapping), + // depends on the layers and sources sanitisers, must be run after them + sources_and_layers: require('../sanitiser/_sources_and_layers'), size: require('../sanitiser/_size'), private: require('../sanitiser/_flag_bool')('private', false), geo_reverse: require('../sanitiser/_geo_reverse'), diff --git a/sanitiser/search.js b/sanitiser/search.js index 1be32b02..980b5a04 100644 --- a/sanitiser/search.js +++ b/sanitiser/search.js @@ -7,6 +7,8 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), size: require('../sanitiser/_size'), layers: require('../sanitiser/_targets')('layers', type_mapping.layer_mapping), sources: require('../sanitiser/_targets')('sources', type_mapping.source_mapping), + // depends on the layers and sources sanitisers, must be run after them + sources_and_layers: require('../sanitiser/_sources_and_layers'), private: require('../sanitiser/_flag_bool')('private', false), geo_search: require('../sanitiser/_geo_search'), boundary_country: require('../sanitiser/_boundary_country'), diff --git a/test/unit/run.js b/test/unit/run.js index 89b7c31b..9d210853 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -37,6 +37,7 @@ var tests = [ require('./sanitiser/_single_scalar_parameters'), require('./sanitiser/_size'), require('./sanitiser/_sources'), + require('./sanitiser/_sources_and_layers'), require('./sanitiser/_text'), require('./sanitiser/autocomplete'), require('./sanitiser/place'), diff --git a/test/unit/sanitiser/_sources_and_layers.js b/test/unit/sanitiser/_sources_and_layers.js new file mode 100644 index 00000000..96001f4e --- /dev/null +++ b/test/unit/sanitiser/_sources_and_layers.js @@ -0,0 +1,115 @@ +var sanitize = require('../../../sanitiser/_sources_and_layers'); + +var type_mapping = require('../../../helper/type_mapping'); + +module.exports.tests = {}; + +module.exports.tests.inactive = function(test, common) { + test('no source or layer specified', function(t) { + var raw = {}; + var clean = {}; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 0, 'should return no errors'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); + + test('only layers specified', function(t) { + var raw = {}; + var clean = { layers: ['venue'] }; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 0, 'should return no errors'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); + + test('only sources specified', function(t) { + var raw = {}; + var clean = { sources: ['openstreetmap'] }; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 0, 'should return no errors'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); +}; + +module.exports.tests.no_errors = function(test, common) { + test('valid combination', function(t) { + var raw = {}; + var clean = { sources: ['openstreetmap'], layers: ['venue'] }; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 0, 'should return no errors'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); + + test('valid combination because of multiple sources', function(t) { + var raw = {}; + var clean = { sources: ['openstreetmap', 'openaddresses'], layers: ['venue'] }; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 0, 'should return no errors'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); + + test('valid combination because of multiple layers', function(t) { + var raw = {}; + var clean = { sources: ['openaddresses'], layers: ['address', 'country'] }; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 0, 'should return no errors'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); +}; + +module.exports.tests.invalid_combination = function(test, common) { + test('address layer with wof', function(t) { + var raw = {}; + var clean = { sources: ['whosonfirst'], layers: ['address'] }; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 1, 'should return an error'); + t.equal(messages.errors[0], 'You have specified both the `sources` and `layers` ' + + 'parameters in a combination that will return no results: the whosonfirst source has nothing in the address layer'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); + + test('admin layers with osm', function(t) { + var raw = {}; + var clean = { sources: ['openstreetmap'], layers: ['country', 'locality'] }; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 2, 'should return an error'); + t.equal(messages.errors[0], 'You have specified both the `sources` and `layers` ' + + 'parameters in a combination that will return no results: the openstreetmap source has nothing in the country layer'); + t.equal(messages.errors[1], 'You have specified both the `sources` and `layers` ' + + 'parameters in a combination that will return no results: the openstreetmap source has nothing in the locality layer'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); +}; + +module.exports.all = function (tape, common) { + function test(name, testFunction) { + return tape('SANTIZE _sources_and_layers ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; From 67449dcae074501d66fd24f00babb25b07a6b7ba Mon Sep 17 00:00:00 2001 From: missinglink Date: Wed, 24 Feb 2016 12:37:54 +0100 Subject: [PATCH 09/13] Tweak weights for focus --- query/autocomplete_defaults.js | 6 +++--- test/unit/fixture/autocomplete_linguistic_final_token.js | 2 +- test/unit/fixture/autocomplete_linguistic_focus.js | 6 +++--- .../fixture/autocomplete_linguistic_focus_null_island.js | 6 +++--- .../unit/fixture/autocomplete_linguistic_multiple_tokens.js | 2 +- test/unit/fixture/autocomplete_linguistic_only.js | 2 +- test/unit/fixture/autocomplete_linguistic_with_admin.js | 2 +- 7 files changed, 13 insertions(+), 13 deletions(-) diff --git a/query/autocomplete_defaults.js b/query/autocomplete_defaults.js index 634f5165..df1b206f 100644 --- a/query/autocomplete_defaults.js +++ b/query/autocomplete_defaults.js @@ -30,10 +30,10 @@ module.exports = _.merge({}, peliasQuery.defaults, { 'phrase:slop': 2, 'focus:function': 'linear', - 'focus:offset': '10km', + 'focus:offset': '0km', 'focus:scale': '250km', 'focus:decay': 0.5, - 'focus:weight': 3, + 'focus:weight': 10, 'function_score:score_mode': 'avg', 'function_score:boost_mode': 'multiply', @@ -90,6 +90,6 @@ module.exports = _.merge({}, peliasQuery.defaults, { 'population:field': 'population', 'population:modifier': 'log1p', 'population:max_boost': 20, - 'population:weight': 2 + 'population:weight': 3 }); diff --git a/test/unit/fixture/autocomplete_linguistic_final_token.js b/test/unit/fixture/autocomplete_linguistic_final_token.js index e63c84b6..1641ac44 100644 --- a/test/unit/fixture/autocomplete_linguistic_final_token.js +++ b/test/unit/fixture/autocomplete_linguistic_final_token.js @@ -70,7 +70,7 @@ module.exports = { 'modifier': 'log1p', 'field': 'population' }, - 'weight': 2 + 'weight': 3 }] } }] diff --git a/test/unit/fixture/autocomplete_linguistic_focus.js b/test/unit/fixture/autocomplete_linguistic_focus.js index 83156c0b..513136b9 100644 --- a/test/unit/fixture/autocomplete_linguistic_focus.js +++ b/test/unit/fixture/autocomplete_linguistic_focus.js @@ -35,12 +35,12 @@ module.exports = { 'lat': 29.49136, 'lon': -82.50622 }, - 'offset': '10km', + 'offset': '0km', 'scale': '250km', 'decay': 0.5 } }, - 'weight': 3 + 'weight': 10 }], 'score_mode': 'avg', 'boost_mode': 'multiply', @@ -114,7 +114,7 @@ module.exports = { 'modifier': 'log1p', 'field': 'population' }, - 'weight': 2 + 'weight': 3 }] } }] diff --git a/test/unit/fixture/autocomplete_linguistic_focus_null_island.js b/test/unit/fixture/autocomplete_linguistic_focus_null_island.js index f28b6234..3985d061 100644 --- a/test/unit/fixture/autocomplete_linguistic_focus_null_island.js +++ b/test/unit/fixture/autocomplete_linguistic_focus_null_island.js @@ -35,12 +35,12 @@ module.exports = { 'lat': 0, 'lon': 0 }, - 'offset': '10km', + 'offset': '0km', 'scale': '250km', 'decay': 0.5 } }, - 'weight': 3 + 'weight': 10 }], 'score_mode': 'avg', 'boost_mode': 'multiply', @@ -114,7 +114,7 @@ module.exports = { 'modifier': 'log1p', 'field': 'population' }, - 'weight': 2 + 'weight': 3 }] } }] diff --git a/test/unit/fixture/autocomplete_linguistic_multiple_tokens.js b/test/unit/fixture/autocomplete_linguistic_multiple_tokens.js index dab5e31a..16a25605 100644 --- a/test/unit/fixture/autocomplete_linguistic_multiple_tokens.js +++ b/test/unit/fixture/autocomplete_linguistic_multiple_tokens.js @@ -81,7 +81,7 @@ module.exports = { 'modifier': 'log1p', 'field': 'population' }, - 'weight': 2 + 'weight': 3 }] } }] diff --git a/test/unit/fixture/autocomplete_linguistic_only.js b/test/unit/fixture/autocomplete_linguistic_only.js index 2ec025d4..05ff6c43 100644 --- a/test/unit/fixture/autocomplete_linguistic_only.js +++ b/test/unit/fixture/autocomplete_linguistic_only.js @@ -70,7 +70,7 @@ module.exports = { 'modifier': 'log1p', 'field': 'population' }, - 'weight': 2 + 'weight': 3 }] } }] diff --git a/test/unit/fixture/autocomplete_linguistic_with_admin.js b/test/unit/fixture/autocomplete_linguistic_with_admin.js index 215a5fda..2174fcbf 100644 --- a/test/unit/fixture/autocomplete_linguistic_with_admin.js +++ b/test/unit/fixture/autocomplete_linguistic_with_admin.js @@ -133,7 +133,7 @@ module.exports = { 'modifier': 'log1p', 'field': 'population' }, - 'weight': 2 + 'weight': 3 } ], 'score_mode': 'first', From ab9dbdf7f070642960296e46c86be7cc26cc9cf0 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 24 Feb 2016 18:50:00 -0500 Subject: [PATCH 10/13] Update ciao tests --- test/ciao/place/basic_place.coffee | 2 +- test/ciao/reverse/layers_alias_coarse.coffee | 20 +++++++++++++++++-- test/ciao/reverse/layers_invalid.coffee | 2 +- .../reverse/layers_mix_invalid_valid.coffee | 2 +- test/ciao/reverse/layers_multiple.coffee | 3 +-- test/ciao/reverse/layers_single.coffee | 3 +-- test/ciao/reverse/sources_invalid.coffee | 2 +- .../sources_layers_invalid_combo.coffee | 7 +++---- .../reverse/sources_layers_valid_combo.coffee | 4 ++-- test/ciao/reverse/sources_multiple.coffee | 3 +-- test/ciao/reverse/sources_single.coffee | 3 +-- test/ciao/search/layers_alias_address.coffee | 3 +-- test/ciao/search/layers_alias_coarse.coffee | 20 +++++++++++++++++-- test/ciao/search/layers_invalid.coffee | 2 +- .../search/layers_mix_invalid_valid.coffee | 3 ++- test/ciao/search/layers_multiple.coffee | 3 +-- test/ciao/search/layers_single.coffee | 3 +-- test/ciao/search/sources_invalid.coffee | 2 +- .../sources_layers_invalid_combo.coffee | 6 +++--- .../search/sources_layers_valid_combo.coffee | 4 ++-- test/ciao/search/sources_multiple.coffee | 3 +-- test/ciao/search/sources_single.coffee | 3 +-- 22 files changed, 63 insertions(+), 40 deletions(-) diff --git a/test/ciao/place/basic_place.coffee b/test/ciao/place/basic_place.coffee index b4a18711..e46bbee8 100644 --- a/test/ciao/place/basic_place.coffee +++ b/test/ciao/place/basic_place.coffee @@ -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', types: [ 'geoname' ] }] +json.geocoding.query['ids'].should.eql [{ id: '1', layer: 'venue', source: 'geonames' }] should.not.exist json.geocoding.query['size'] diff --git a/test/ciao/reverse/layers_alias_coarse.coffee b/test/ciao/reverse/layers_alias_coarse.coffee index 13671a05..6b911083 100644 --- a/test/ciao/reverse/layers_alias_coarse.coffee +++ b/test/ciao/reverse/layers_alias_coarse.coffee @@ -30,5 +30,21 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['size'].should.eql 10 -json.geocoding.query.types['from_layers'].should.eql ["admin0","admin1","admin2","neighborhood","locality","local_admin"] -json.geocoding.query['type'].should.eql ["admin0","admin1","admin2","neighborhood","locality","local_admin"] \ No newline at end of file +json.geocoding.query.layers.should.eql [ "continent", + "macrocountry", + "country", + "dependency", + "region", + "locality", + "localadmin", + "county", + "macrohood", + "neighbourhood", + "microhood", + "disputed", + "admin0", + "admin1", + "admin2", + "neighborhood", + "local_admin" +] diff --git a/test/ciao/reverse/layers_invalid.coffee b/test/ciao/reverse/layers_invalid.coffee index 354efb02..a7105815 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: coarse,venue,address,country,region,county,locality,localadmin,neighbourhood' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,country,region,address,venue,county,locality,admin0,admin1,admin2,neighborhood,local_admin,continent,macrocountry,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/reverse/layers_mix_invalid_valid.coffee b/test/ciao/reverse/layers_mix_invalid_valid.coffee index a636a26b..f8e07adf 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: coarse,venue,address,country,region,county,locality,localadmin,neighbourhood' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,country,region,address,venue,county,locality,admin0,admin1,admin2,neighborhood,local_admin,continent,macrocountry,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/reverse/layers_multiple.coffee b/test/ciao/reverse/layers_multiple.coffee index f5f2d7dd..d85c07a2 100644 --- a/test/ciao/reverse/layers_multiple.coffee +++ b/test/ciao/reverse/layers_multiple.coffee @@ -30,5 +30,4 @@ 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"] +json.geocoding.query.layers.should.eql ["country","admin0","region","admin1"] diff --git a/test/ciao/reverse/layers_single.coffee b/test/ciao/reverse/layers_single.coffee index 4b65c331..ab1f8e08 100644 --- a/test/ciao/reverse/layers_single.coffee +++ b/test/ciao/reverse/layers_single.coffee @@ -30,5 +30,4 @@ 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"] +json.geocoding.query.layers.should.eql ["country","admin0"] diff --git a/test/ciao/reverse/sources_invalid.coffee b/test/ciao/reverse/sources_invalid.coffee index bdc6c826..f0b54803 100644 --- a/test/ciao/reverse/sources_invalid.coffee +++ b/test/ciao/reverse/sources_invalid.coffee @@ -24,7 +24,7 @@ json.features.should.be.instanceof Array #? expected errors should.exist json.geocoding.errors -json.geocoding.errors.should.eql [ '\'notasource\' is an invalid sources parameter. Valid options: gn,geonames,oa,openaddresses,qs,quattroshapes,osm,openstreetmap' ] +json.geocoding.errors.should.eql [ '\'notasource\' is an invalid sources parameter. Valid options: osm,oa,gn,qs,wof,openstreetmap,openaddresses,geonames,quattroshapes,whosonfirst' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/reverse/sources_layers_invalid_combo.coffee b/test/ciao/reverse/sources_layers_invalid_combo.coffee index 17de70ca..984830a7 100644 --- a/test/ciao/reverse/sources_layers_invalid_combo.coffee +++ b/test/ciao/reverse/sources_layers_invalid_combo.coffee @@ -24,13 +24,12 @@ json.features.should.be.instanceof Array #? expected errors should.exist json.geocoding.errors -json.geocoding.errors.should.eql [ 'You have specified both the `sources` and `layers` parameters in a combination that will return no results.' ] +json.geocoding.errors.should.eql [ 'You have specified both the `sources` and `layers` parameters in a combination that will return no results: the quattroshapes source has nothing in the address layer' ] #? expected warnings 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_sources'].should.eql ["admin0","admin1","admin2","neighborhood","locality","local_admin"] -should.not.exist json.geocoding.query['type'] +json.geocoding.query.layers.should.eql ["address"] +json.geocoding.query.sources.should.eql ["quattroshapes"] diff --git a/test/ciao/reverse/sources_layers_valid_combo.coffee b/test/ciao/reverse/sources_layers_valid_combo.coffee index a24ea221..fc2fee76 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"] +json.geocoding.query.layers.should.eql ["address"] +json.geocoding.query.sources.should.eql ["openaddresses"] diff --git a/test/ciao/reverse/sources_multiple.coffee b/test/ciao/reverse/sources_multiple.coffee index 8728109c..4fd24366 100644 --- a/test/ciao/reverse/sources_multiple.coffee +++ b/test/ciao/reverse/sources_multiple.coffee @@ -30,5 +30,4 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['size'].should.eql 10 -json.geocoding.query.types['from_sources'].should.eql ["osmaddress","osmnode","osmway","geoname"] -json.geocoding.query['type'].should.eql ["geoname","osmnode","osmway","osmaddress"] +json.geocoding.query.sources.should.eql ["openstreetmap", "geonames"] diff --git a/test/ciao/reverse/sources_single.coffee b/test/ciao/reverse/sources_single.coffee index 2c638503..6a062c9a 100644 --- a/test/ciao/reverse/sources_single.coffee +++ b/test/ciao/reverse/sources_single.coffee @@ -30,5 +30,4 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['size'].should.eql 10 -json.geocoding.query.types['from_sources'].should.eql ["osmaddress","osmnode","osmway"] -json.geocoding.query['type'].should.eql ["osmnode","osmway","osmaddress"] +json.geocoding.query.sources.should.eql ["openstreetmap"] diff --git a/test/ciao/search/layers_alias_address.coffee b/test/ciao/search/layers_alias_address.coffee index 9bf32a04..e3e39d15 100644 --- a/test/ciao/search/layers_alias_address.coffee +++ b/test/ciao/search/layers_alias_address.coffee @@ -31,5 +31,4 @@ 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"] +json.geocoding.query.layers.should.eql ["address"] diff --git a/test/ciao/search/layers_alias_coarse.coffee b/test/ciao/search/layers_alias_coarse.coffee index 262be88e..c2527df0 100644 --- a/test/ciao/search/layers_alias_coarse.coffee +++ b/test/ciao/search/layers_alias_coarse.coffee @@ -31,5 +31,21 @@ 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","admin2","neighborhood","locality","local_admin"] -json.geocoding.query['type'].should.eql ["admin0","admin1","admin2","neighborhood","locality","local_admin"] \ No newline at end of file +json.geocoding.query.layers.should.eql [ "continent", + "macrocountry", + "country", + "dependency", + "region", + "locality", + "localadmin", + "county", + "macrohood", + "neighbourhood", + "microhood", + "disputed", + "admin0", + "admin1", + "admin2", + "neighborhood", + "local_admin" +] diff --git a/test/ciao/search/layers_invalid.coffee b/test/ciao/search/layers_invalid.coffee index 1e8e65fc..40de3f85 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: coarse,venue,address,country,region,county,locality,localadmin,neighbourhood' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,country,region,address,venue,county,locality,admin0,admin1,admin2,neighborhood,local_admin,continent,macrocountry,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/search/layers_mix_invalid_valid.coffee b/test/ciao/search/layers_mix_invalid_valid.coffee index 11a40ddc..6f1ccec6 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: coarse,venue,address,country,region,county,locality,localadmin,neighbourhood' ] +json.geocoding.errors.should.eql [ '\'notlayer\' is an invalid layers parameter. Valid options: coarse,country,region,address,venue,county,locality,admin0,admin1,admin2,neighborhood,local_admin,continent,macrocountry,dependency,localadmin,macrohood,neighbourhood,microhood,disputed' ] #? expected warnings should.not.exist json.geocoding.warnings @@ -32,3 +32,4 @@ should.not.exist json.geocoding.warnings #? inputs json.geocoding.query['text'].should.eql 'a' json.geocoding.query['size'].should.eql 10 +should.not.exist json.geocoding.query['layers'] diff --git a/test/ciao/search/layers_multiple.coffee b/test/ciao/search/layers_multiple.coffee index 5f714d4d..c735a206 100644 --- a/test/ciao/search/layers_multiple.coffee +++ b/test/ciao/search/layers_multiple.coffee @@ -31,5 +31,4 @@ 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"] +json.geocoding.query.layers.should.eql ["country","admin0","region","admin1"] diff --git a/test/ciao/search/layers_single.coffee b/test/ciao/search/layers_single.coffee index 227f7bc9..ad104fab 100644 --- a/test/ciao/search/layers_single.coffee +++ b/test/ciao/search/layers_single.coffee @@ -31,5 +31,4 @@ 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"] +json.geocoding.query.layers.should.eql ["country","admin0"] diff --git a/test/ciao/search/sources_invalid.coffee b/test/ciao/search/sources_invalid.coffee index 1d8df87c..8b6b9d27 100644 --- a/test/ciao/search/sources_invalid.coffee +++ b/test/ciao/search/sources_invalid.coffee @@ -24,7 +24,7 @@ json.features.should.be.instanceof Array #? expected errors should.exist json.geocoding.errors -json.geocoding.errors.should.eql [ '\'notasource\' is an invalid sources parameter. Valid options: gn,geonames,oa,openaddresses,qs,quattroshapes,osm,openstreetmap' ] +json.geocoding.errors.should.eql [ '\'notasource\' is an invalid sources parameter. Valid options: osm,oa,gn,qs,wof,openstreetmap,openaddresses,geonames,quattroshapes,whosonfirst' ] #? expected warnings should.not.exist json.geocoding.warnings diff --git a/test/ciao/search/sources_layers_invalid_combo.coffee b/test/ciao/search/sources_layers_invalid_combo.coffee index c0f84b13..d1e0fc07 100644 --- a/test/ciao/search/sources_layers_invalid_combo.coffee +++ b/test/ciao/search/sources_layers_invalid_combo.coffee @@ -24,7 +24,7 @@ json.features.should.be.instanceof Array #? expected errors should.exist json.geocoding.errors -json.geocoding.errors.should.eql [ 'You have specified both the `sources` and `layers` parameters in a combination that will return no results.' ] +json.geocoding.errors.should.eql [ 'You have specified both the `sources` and `layers` parameters in a combination that will return no results: the quattroshapes source has nothing in the address layer' ] #? expected warnings should.not.exist json.geocoding.warnings @@ -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_sources'].should.eql ["admin0","admin1","admin2","neighborhood","locality","local_admin"] +json.geocoding.query.layers.should.eql ["address"] +json.geocoding.query.sources.should.eql ["quattroshapes"] 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 eeb2a663..b17a81b0 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"] +json.geocoding.query.layers.should.eql ["address"] +json.geocoding.query.sources.should.eql ["openaddresses"] diff --git a/test/ciao/search/sources_multiple.coffee b/test/ciao/search/sources_multiple.coffee index fa5a650c..08352113 100644 --- a/test/ciao/search/sources_multiple.coffee +++ b/test/ciao/search/sources_multiple.coffee @@ -31,5 +31,4 @@ 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_sources'].should.eql ["osmaddress","osmnode","osmway","geoname"] -json.geocoding.query['type'].should.eql ["geoname","osmnode","osmway","osmaddress"] +json.geocoding.query.sources.should.eql ["openstreetmap", "geonames"] diff --git a/test/ciao/search/sources_single.coffee b/test/ciao/search/sources_single.coffee index 3b3e0bf4..e639a8b5 100644 --- a/test/ciao/search/sources_single.coffee +++ b/test/ciao/search/sources_single.coffee @@ -31,5 +31,4 @@ 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_sources'].should.eql ["osmaddress","osmnode","osmway"] -json.geocoding.query['type'].should.eql ["osmnode","osmway","osmaddress"] +json.geocoding.query.sources.should.eql ["openstreetmap"] From 74d05867cfee895d16a217e32b9dd371941178e7 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 2 Mar 2016 14:49:50 -0500 Subject: [PATCH 11/13] Update mock Elasticsearch response data --- test/unit/helper/geojsonify.js | 35 ++++++++++------------------------ 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index a5311050..3ab4d975 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -42,6 +42,7 @@ module.exports.tests.search = function(test, common) { '_id': 'id1', '_type': 'layer1', 'source': 'source1', + 'layer': 'layer1', 'center_point': { 'lat': 51.5337144, 'lon': -0.1069716 @@ -60,12 +61,6 @@ module.exports.tests.search = function(test, common) { 'localadmin': 'test1', 'locality': 'test2', 'neighbourhood': 'test3', - 'suggest': { - 'input': [ - '\'round midnight jazz and blues bar' - ], - 'output': 'osmnode:2208150035' - }, 'category': [ 'food', 'nightlife' @@ -75,6 +70,7 @@ module.exports.tests.search = function(test, common) { '_id': 'id2', '_type': 'layer2', 'source': 'source2', + 'layer': 'layer2', 'name': { 'default': 'Blues Cafe' }, @@ -90,17 +86,12 @@ module.exports.tests.search = function(test, common) { 'localadmin': 'test1', 'locality': 'test2', 'neighbourhood': 'test3', - 'suggest': { - 'input': [ - 'blues cafe' - ], - 'output': 'osmway:147495160' - } }, { - '_id': '34633854', - '_type': 'way', - 'source': 'osm', + '_id': 'node:34633854', + '_type': 'venue', + 'source': 'openstreetmap', + 'layer': 'venue', 'name': { 'default': 'Empire State Building' }, @@ -116,12 +107,6 @@ module.exports.tests.search = function(test, common) { 'localadmin': 'Manhattan', 'locality': 'New York', 'neighbourhood': 'Koreatown', - 'suggest': { - 'input': [ - 'empire state building' - ], - 'output': 'osmway:34633854' - }, 'category': [ 'tourism', 'transport' @@ -198,10 +183,10 @@ module.exports.tests.search = function(test, common) { ] }, 'properties': { - 'id': '34633854', - 'gid': 'osm:way:34633854', - 'layer': 'way', - 'source': 'osm', + 'id': 'node:34633854', + 'gid': 'openstreetmap:venue:node:34633854', + 'layer': 'venue', + 'source': 'openstreetmap', 'label': 'Empire State Building, Manhattan, NY, USA', 'name': 'Empire State Building', 'country_a': 'USA', From 692b6da17dcea409cf9cd2edc69bf5bb7736fa28 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 2 Mar 2016 14:50:49 -0500 Subject: [PATCH 12/13] Use layer instead of _type field to determine layer --- helper/geojsonify.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index b415b18f..8ed808f5 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -28,12 +28,8 @@ function lookupSource(src) { return src.source; } -/* - * 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) { - return src._type; + return src.layer; } function geojsonifyPlaces( docs ){ From 79883594b8611391fba187997d9e2069c38860a6 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 2 Mar 2016 14:55:43 -0500 Subject: [PATCH 13/13] Use clean.layers to determine types This allows removing the types middleware and simplifies some code. --- controller/search.js | 10 +++------- middleware/_types.js | 18 ------------------ routes/v1.js | 4 ---- 3 files changed, 3 insertions(+), 29 deletions(-) delete mode 100644 middleware/_types.js diff --git a/controller/search.js b/controller/search.js index 4adaf31d..a564e7b4 100644 --- a/controller/search.js +++ b/controller/search.js @@ -11,7 +11,6 @@ function setup( backend, query ){ // do not run controller when a request // validation error has occurred. if( req.errors && req.errors.length ){ - delete req.clean.type; //to declutter output return next(); } @@ -25,12 +24,9 @@ function setup( backend, query ){ body: query( req.clean ) }; - // set the Elasticsearch types to filter by, - // and remove the property from clean so the API - // response output is cleaner - if( req.clean.hasOwnProperty('type') ){ - cmd.type = req.clean.type; - delete req.clean.type; + // use layers field for filtering by type + if( req.clean.hasOwnProperty('layers') ){ + cmd.type = req.clean.layers; } logger.debug( '[ES req]', JSON.stringify(cmd) ); diff --git a/middleware/_types.js b/middleware/_types.js deleted file mode 100644 index ccabdab9..00000000 --- a/middleware/_types.js +++ /dev/null @@ -1,18 +0,0 @@ -/** - * Take the layers specified by the layers parameter and use them to set the - * list of Elasticsearch types to filter. - * - * This has to be done outside the layers sanitizer since it doesn't know that - * the layers property is eventualy used to choose the _type. - */ -function middleware(req, res, next) { - req.clean = req.clean || {}; - - if (req.clean.hasOwnProperty('layers')) { - req.clean.type = req.clean.layers; - } - - next(); -} - -module.exports = middleware; diff --git a/routes/v1.js b/routes/v1.js index 49c9ea4c..00c99aa5 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -12,7 +12,6 @@ var sanitisers = { /** ----------------------- middleware ------------------------ **/ var middleware = { - types: require('../middleware/_types'), calcSize: require('../middleware/sizeCalculator') }; @@ -59,7 +58,6 @@ function addRoutes(app, peliasConfig) { ]), search: createRouter([ sanitisers.search.middleware, - middleware.types, middleware.calcSize(), controllers.search(), postProc.distances('focus.point.'), @@ -72,7 +70,6 @@ function addRoutes(app, peliasConfig) { ]), autocomplete: createRouter([ sanitisers.autocomplete.middleware, - middleware.types, controllers.search(null, require('../query/autocomplete')), postProc.distances('focus.point.'), postProc.confidenceScores(peliasConfig), @@ -84,7 +81,6 @@ function addRoutes(app, peliasConfig) { ]), reverse: createRouter([ sanitisers.reverse.middleware, - middleware.types, middleware.calcSize(), controllers.search(undefined, reverseQuery), postProc.distances('point.'),