From fb0cf514e97f96fc78f8ca595928fb0b75edf075 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 3 Sep 2015 14:34:01 -0400 Subject: [PATCH 01/21] Whitespace --- controller/search.js | 1 - helper/layers.js | 2 +- helper/query_parser.js | 18 +++++++++--------- sanitiser/_id.js | 10 ++++------ sanitiser/_input.js | 11 ++++------- sanitiser/_layers.js | 3 +-- test/unit/helper/query_parser.js | 32 ++++++++++++++++---------------- 7 files changed, 35 insertions(+), 42 deletions(-) diff --git a/controller/search.js b/controller/search.js index f38091cd..2923f57d 100644 --- a/controller/search.js +++ b/controller/search.js @@ -1,4 +1,3 @@ - var service = { search: require('../service/search') }; function setup( backend, query ){ diff --git a/helper/layers.js b/helper/layers.js index aa625a6e..a5a57d7f 100644 --- a/helper/layers.js +++ b/helper/layers.js @@ -1,6 +1,6 @@ module.exports = function(alias_layers) { // make a copy of the array so, you are not modifying original ref - var layers = alias_layers.slice(0); + var layers = alias_layers.slice(0); // expand aliases var expand_aliases = function(alias, layers, layer_indeces) { diff --git a/helper/query_parser.js b/helper/query_parser.js index 88729c86..7c5ba4b0 100644 --- a/helper/query_parser.js +++ b/helper/query_parser.js @@ -5,7 +5,7 @@ var get_layers = require('../helper/layers'); var delim = ','; module.exports = function(query) { - + var tokenized = query.split(/[ ,]+/); var hasNumber = /\d/.test(query); @@ -17,11 +17,11 @@ module.exports = function(query) { if ( delimIndex !== -1 ) { address.name = query.substring(0, delimIndex); address.admin_parts = query.substring(delimIndex + 1).trim(); - } + } return address; }; - + var getTargetLayersWhenAddressParsingIsNotNecessary = function(query) { var address = {}; // set target_layer if input length <= 3 characters @@ -32,7 +32,7 @@ module.exports = function(query) { // no need to hit address layers if there's only one (or two) token(s) address.target_layer = get_layers(['admin', 'poi']); } - + return address.target_layer ? address : null; }; @@ -49,10 +49,10 @@ module.exports = function(query) { var addressWithAdminParts = getAdminPartsBySplittingOnDelim(query); var addressWithTargetLayers= getTargetLayersWhenAddressParsingIsNotNecessary(query); - var addressWithAddressParts= !addressWithTargetLayers ? getAddressParts(query) : {}; + var addressWithAddressParts= !addressWithTargetLayers ? getAddressParts(query) : {}; - var parsedAddress = extend(addressWithAdminParts, - addressWithTargetLayers, + var parsedAddress = extend(addressWithAdminParts, + addressWithTargetLayers, addressWithAddressParts); var address_parts = [ 'name', @@ -69,7 +69,7 @@ module.exports = function(query) { var parsed_input = {}; - address_parts.forEach(function(part){ + address_parts.forEach(function(part){ if (parsedAddress[part]) { parsed_input[part] = parsedAddress[part]; } @@ -90,4 +90,4 @@ module.exports = function(query) { // regions: parsedAddress.regions, // admin_parts: parsedAddress.admin_parts, // target_layer: parsedAddress.target_layer -// } \ No newline at end of file +// } diff --git a/sanitiser/_id.js b/sanitiser/_id.js index ac2432e6..a8424363 100644 --- a/sanitiser/_id.js +++ b/sanitiser/_id.js @@ -5,7 +5,6 @@ var isObject = require('is-object'); // so, both type and id are required fields. function sanitize( req ){ - req.clean = req.clean || {}; var params = req.query; var indeces = require('../query/indeces'); @@ -30,7 +29,7 @@ function sanitize( req ){ if( params && params.id && params.id.length ){ req.clean.ids = []; params.ids = Array.isArray(params.id) ? params.id : [params.id]; - + // de-dupe params.ids = params.ids.filter(function(item, pos) { return params.ids.indexOf(item) === pos; @@ -38,7 +37,7 @@ function sanitize( req ){ for (var i=0; i0' }; } - + req.clean.input = params.input; req.clean.parsed_input = query_parse(params.input); - return { 'error': false }; - } // export function -module.exports = sanitize; \ No newline at end of file +module.exports = sanitize; diff --git a/sanitiser/_layers.js b/sanitiser/_layers.js index fa44336e..2ad5c42f 100644 --- a/sanitiser/_layers.js +++ b/sanitiser/_layers.js @@ -5,7 +5,6 @@ var isObject = require('is-object'), // validate inputs, convert types and apply defaults function sanitize( req ){ - var clean = req.clean || {}; var params= req.query; @@ -42,7 +41,7 @@ function sanitize( req ){ // pass validated params to next middleware clean.layers = get_layers(layers); req.clean = clean; - + return { 'error': false }; } diff --git a/test/unit/helper/query_parser.js b/test/unit/helper/query_parser.js index bf10b154..c82a31a2 100644 --- a/test/unit/helper/query_parser.js +++ b/test/unit/helper/query_parser.js @@ -67,13 +67,13 @@ module.exports.tests.parse_one_or_more_tokens = function(test, common) { var target_layer = get_layers(['admin', 'poi']); t.equal(typeof address, 'object', 'valid object'); - + if (parse_address) { t.deepEqual(address.regions.join(''), query, 'since query contained a number, it went through address parsing'); } else { - t.deepEqual(address.target_layer, target_layer, 'admin_parts set correctly to ' + target_layer.join(', ')); + t.deepEqual(address.target_layer, target_layer, 'admin_parts set correctly to ' + target_layer.join(', ')); } - + t.end(); }); }; @@ -88,32 +88,32 @@ module.exports.tests.parse_one_or_more_tokens = function(test, common) { }; module.exports.tests.parse_address = function(test, common) { - var addresses_nonum = [{ non_street: 'main particle', city: 'new york'}, - { non_street: 'biggg city block' }, + var addresses_nonum = [{ non_street: 'main particle', city: 'new york'}, + { non_street: 'biggg city block' }, { non_street: 'the empire state building' } ]; - var address_with_num = [{ number: 123, street: 'main st', city: 'new york', state: 'ny'}, - { number: 456, street: 'pine ave', city: 'san francisco', state: 'CA'}, + var address_with_num = [{ number: 123, street: 'main st', city: 'new york', state: 'ny'}, + { number: 456, street: 'pine ave', city: 'san francisco', state: 'CA'}, { number: 1980, street: 'house st', city: 'hoboken', state: 'NY'} ]; - var address_with_zip = [{ number: 1, street: 'main st', city: 'new york', state: 'ny', zip: 10010}, - { number: 4, street: 'ape ave', city: 'san diego', state: 'CA', zip: 98970}, + var address_with_zip = [{ number: 1, street: 'main st', city: 'new york', state: 'ny', zip: 10010}, + { number: 4, street: 'ape ave', city: 'san diego', state: 'CA', zip: 98970}, { number: 19, street: 'house dr', city: 'houston', state: 'TX', zip: 79089} ]; var testParse = function(query, hasNumber, hasZip) { - var testcase = 'parse query with ' + (hasNumber ? 'a house number ': 'no house number '); + var testcase = 'parse query with ' + (hasNumber ? 'a house number ': 'no house number '); testcase += 'and ' + (hasZip ? 'a zip ' : 'no zip '); test(testcase, function(t) { var query_string = ''; - for (var k in query) { + for (var k in query) { query_string += ' ' + query[k]; } // remove leading whitespace query_string = query_string.substring(1); - + var address = parser(query_string); var non_address_layer = get_layers(['admin', 'poi']); @@ -126,14 +126,14 @@ module.exports.tests.parse_address = function(test, common) { } if ((hasNumber || hasZip) && query.street) { - t.equal(typeof address.number, 'number', 'valid house number format (' + address.number + ')'); + t.equal(typeof address.number, 'number', 'valid house number format (' + address.number + ')'); t.equal(address.number, query.number, 'correct house number (' + query.number + ')'); - t.equal(typeof address.street, 'string', 'valid street name format (' + address.street + ')'); + t.equal(typeof address.street, 'string', 'valid street name format (' + address.street + ')'); t.equal(address.street, query.street, 'correct street name (' + query.street + ')'); } - + if (hasZip) { - t.equal(typeof address.postalcode, 'number', 'valid zip (' + address.postalcode + ')'); + t.equal(typeof address.postalcode, 'number', 'valid zip (' + address.postalcode + ')'); t.equal(address.postalcode, query.zip, 'correct postal code (' + query.zip + ')'); } From 4037c49c4b15439a59143ceaf4c196eb97ab0ace Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 3 Sep 2015 14:58:58 -0400 Subject: [PATCH 02/21] Store sanitised types from layers parameter This moves the list of types created by sanitising the layer API parameter from clean.layers to clean.types.from_layers. In subsequent commits, types created from address parsing, and the yet-to-be-implemented source parameter will also live in the clean.types object. This will allow moving logic to set cmd.type out of controllers, and into separate logic that can be a littler smarter. Also, it will no longer require the clean.default_layers_set flag to be passed all around like a nasty global variable. --- controller/search.js | 4 ++-- sanitiser/_layers.js | 4 +++- test/unit/sanitiser/reverse.js | 24 +++++++++++++----------- test/unit/sanitiser/search.js | 24 +++++++++++++----------- test/unit/sanitiser/suggest.js | 2 +- 5 files changed, 32 insertions(+), 26 deletions(-) diff --git a/controller/search.js b/controller/search.js index 2923f57d..386af22f 100644 --- a/controller/search.js +++ b/controller/search.js @@ -15,8 +15,8 @@ function setup( backend, query ){ body: query( req.clean ) }; - if (req.clean.layers) { - cmd.type = req.clean.layers; + if (req.clean.types && req.clean.types.from_layers) { + cmd.type = req.clean.types.from_layers; } // set type if input suggests targeting a layer(s) diff --git a/sanitiser/_layers.js b/sanitiser/_layers.js index 2ad5c42f..25e8ba26 100644 --- a/sanitiser/_layers.js +++ b/sanitiser/_layers.js @@ -8,6 +8,8 @@ function sanitize( req ){ var clean = req.clean || {}; var params= req.query; + clean.types = clean.types || {}; + // ensure the input params are a valid object if( !isObject( params ) ){ params = {}; @@ -39,7 +41,7 @@ function sanitize( req ){ } // pass validated params to next middleware - clean.layers = get_layers(layers); + clean.types.from_layers = get_layers(layers); req.clean = clean; return { 'error': false }; diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js index d2ff8293..d8d666c1 100644 --- a/test/unit/sanitiser/reverse.js +++ b/test/unit/sanitiser/reverse.js @@ -5,8 +5,10 @@ var suggest = require('../../../sanitiser/reverse'), delim = ',', defaultError = 'missing param \'lat\'', defaultClean = { lat:0, - layers: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', - 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], + types: { + from_layers: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', + 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], + }, lon: 0, size: 10, details: true, @@ -158,7 +160,7 @@ module.exports.tests.sanitize_details = function(test, common) { module.exports.tests.sanitize_layers = function(test, common) { test('unspecified', function(t) { sanitize({ layers: undefined, input: 'test', lat: 0, lon: 0 }, function( err, clean ){ - t.deepEqual(clean.layers, defaultClean.layers, 'default layers set'); + t.deepEqual(clean.types.from_layers, defaultClean.types.from_layers, 'default layers set'); t.end(); }); }); @@ -173,21 +175,21 @@ module.exports.tests.sanitize_layers = function(test, common) { test('poi (alias) layer', function(t) { var poi_layers = ['geoname','osmnode','osmway']; sanitize({ layers: 'poi', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ - t.deepEqual(clean.layers, poi_layers, 'poi layers set'); + t.deepEqual(clean.types.from_layers, poi_layers, 'poi layers set'); t.end(); }); }); test('admin (alias) layer', function(t) { var admin_layers = ['admin0','admin1','admin2','neighborhood','locality','local_admin']; sanitize({ layers: 'admin', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ - t.deepEqual(clean.layers, admin_layers, 'admin layers set'); + t.deepEqual(clean.types.from_layers, admin_layers, 'admin layers set'); t.end(); }); }); test('address (alias) layer', function(t) { var address_layers = ['osmaddress','openaddresses']; sanitize({ layers: 'address', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ - t.deepEqual(clean.layers, address_layers, 'address layers set'); + t.deepEqual(clean.types.from_layers, address_layers, 'address layers set'); t.end(); }); }); @@ -195,7 +197,7 @@ module.exports.tests.sanitize_layers = function(test, common) { var poi_layers = ['geoname','osmnode','osmway']; var reg_layers = ['admin0', 'admin1']; sanitize({ layers: 'poi,admin0,admin1', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ - t.deepEqual(clean.layers, reg_layers.concat(poi_layers), 'poi + regular layers'); + t.deepEqual(clean.types.from_layers, reg_layers.concat(poi_layers), 'poi + regular layers'); t.end(); }); }); @@ -203,7 +205,7 @@ module.exports.tests.sanitize_layers = function(test, common) { var admin_layers = ['admin0','admin1','admin2','neighborhood','locality','local_admin']; var reg_layers = ['geoname', 'osmway']; sanitize({ layers: 'admin,geoname,osmway', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ - t.deepEqual(clean.layers, reg_layers.concat(admin_layers), 'admin + regular layers set'); + t.deepEqual(clean.types.from_layers, reg_layers.concat(admin_layers), 'admin + regular layers set'); t.end(); }); }); @@ -211,21 +213,21 @@ module.exports.tests.sanitize_layers = function(test, common) { var address_layers = ['osmaddress','openaddresses']; var reg_layers = ['geoname', 'osmway']; sanitize({ layers: 'address,geoname,osmway', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ - t.deepEqual(clean.layers, reg_layers.concat(address_layers), 'address + regular layers set'); + t.deepEqual(clean.types.from_layers, reg_layers.concat(address_layers), 'address + regular layers set'); t.end(); }); }); test('alias layer plus regular layers (no duplicates)', function(t) { var poi_layers = ['geoname','osmnode','osmway']; sanitize({ layers: 'poi,geoname,osmnode', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ - t.deepEqual(clean.layers, poi_layers, 'poi layers found (no duplicates)'); + t.deepEqual(clean.types.from_layers, poi_layers, 'poi 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']; sanitize({ layers: 'poi,admin', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ - t.deepEqual(clean.layers, alias_layers, 'all layers found (no duplicates)'); + t.deepEqual(clean.types.from_layers, alias_layers, 'all layers found (no duplicates)'); t.end(); }); }); diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index 702bbcb5..1f37fa13 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -8,8 +8,10 @@ var search = require('../../../sanitiser/search'), delim = ',', defaultError = 'invalid param \'input\': text length, must be >0', defaultClean = { input: 'test', - layers: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', - 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], + types: { + from_layers: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', + 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], + }, size: 10, details: true, parsed_input: defaultParsed, @@ -320,7 +322,7 @@ module.exports.tests.sanitize_details = function(test, common) { module.exports.tests.sanitize_layers = function(test, common) { test('unspecified', function(t) { sanitize({ layers: undefined, input: 'test' }, function( err, clean ){ - t.deepEqual(clean.layers, defaultClean.layers, 'default layers set'); + t.deepEqual(clean.types.from_layers, defaultClean.types.from_layers, 'default layers set'); t.end(); }); }); @@ -335,21 +337,21 @@ module.exports.tests.sanitize_layers = function(test, common) { test('poi (alias) layer', function(t) { var poi_layers = ['geoname','osmnode','osmway']; sanitize({ layers: 'poi', input: 'test' }, function( err, clean ){ - t.deepEqual(clean.layers, poi_layers, 'poi layers set'); + t.deepEqual(clean.types.from_layers, poi_layers, 'poi layers set'); t.end(); }); }); test('admin (alias) layer', function(t) { var admin_layers = ['admin0','admin1','admin2','neighborhood','locality','local_admin']; sanitize({ layers: 'admin', input: 'test' }, function( err, clean ){ - t.deepEqual(clean.layers, admin_layers, 'admin layers set'); + t.deepEqual(clean.types.from_layers, admin_layers, 'admin layers set'); t.end(); }); }); test('address (alias) layer', function(t) { var address_layers = ['osmaddress','openaddresses']; sanitize({ layers: 'address', input: 'test' }, function( err, clean ){ - t.deepEqual(clean.layers, address_layers, 'address layers set'); + t.deepEqual(clean.types.from_layers, address_layers, 'address layers set'); t.end(); }); }); @@ -357,7 +359,7 @@ module.exports.tests.sanitize_layers = function(test, common) { var poi_layers = ['geoname','osmnode','osmway']; var reg_layers = ['admin0', 'admin1']; sanitize({ layers: 'poi,admin0,admin1', input: 'test' }, function( err, clean ){ - t.deepEqual(clean.layers, reg_layers.concat(poi_layers), 'poi + regular layers'); + t.deepEqual(clean.types.from_layers, reg_layers.concat(poi_layers), 'poi + regular layers'); t.end(); }); }); @@ -365,7 +367,7 @@ module.exports.tests.sanitize_layers = function(test, common) { var admin_layers = ['admin0','admin1','admin2','neighborhood','locality','local_admin']; var reg_layers = ['geoname', 'osmway']; sanitize({ layers: 'admin,geoname,osmway', input: 'test' }, function( err, clean ){ - t.deepEqual(clean.layers, reg_layers.concat(admin_layers), 'admin + regular layers set'); + t.deepEqual(clean.types.from_layers, reg_layers.concat(admin_layers), 'admin + regular layers set'); t.end(); }); }); @@ -373,21 +375,21 @@ module.exports.tests.sanitize_layers = function(test, common) { var address_layers = ['osmaddress','openaddresses']; var reg_layers = ['geoname', 'osmway']; sanitize({ layers: 'address,geoname,osmway', input: 'test' }, function( err, clean ){ - t.deepEqual(clean.layers, reg_layers.concat(address_layers), 'address + regular layers set'); + t.deepEqual(clean.types.from_layers, reg_layers.concat(address_layers), 'address + regular layers set'); t.end(); }); }); test('alias layer plus regular layers (no duplicates)', function(t) { var poi_layers = ['geoname','osmnode','osmway']; sanitize({ layers: 'poi,geoname,osmnode', input: 'test' }, function( err, clean ){ - t.deepEqual(clean.layers, poi_layers, 'poi layers found (no duplicates)'); + t.deepEqual(clean.types.from_layers, poi_layers, 'poi 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']; sanitize({ layers: 'poi,admin', input: 'test' }, function( err, clean ){ - t.deepEqual(clean.layers, alias_layers, 'all layers found (no duplicates)'); + t.deepEqual(clean.types.from_layers, alias_layers, 'all layers found (no duplicates)'); t.end(); }); }); diff --git a/test/unit/sanitiser/suggest.js b/test/unit/sanitiser/suggest.js index b1ad1be6..003b44ae 100644 --- a/test/unit/sanitiser/suggest.js +++ b/test/unit/sanitiser/suggest.js @@ -272,7 +272,7 @@ module.exports.tests.sanitize_details = function(test, common) { module.exports.tests.sanitize_layers = function(test, common) { test('unspecified', function(t) { sanitize({ layers: undefined, input: 'test', lat: 0, lon: 0 }, function( err, clean ){ - t.deepEqual(clean.layers, defaultClean.layers, 'default layers set'); + t.deepEqual(clean.types.from_layers, defaultClean.types.from_layers, 'default layers set'); t.end(); }); }); From ad8db9b8e9aabebda9185fd73f0edc65e061e165 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 3 Sep 2015 16:15:31 -0400 Subject: [PATCH 03/21] Remove layer configuration based on address parsing This code doesn't seem like it will be triggered very often (due to it comapring space delimited words with comma delimited words from the text field), and also has the potential to cause quite a bit of weird behavior. --- helper/query_parser.js | 11 +++-------- test/unit/helper/query_parser.js | 8 ++------ 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/helper/query_parser.js b/helper/query_parser.js index 7c5ba4b0..ad9d57ac 100644 --- a/helper/query_parser.js +++ b/helper/query_parser.js @@ -37,14 +37,9 @@ module.exports = function(query) { }; var getAddressParts = function(query) { - // address parsing - var address = parser( query ); - // set target_layer if input suggests no address - if (address.text === address.regions.join(' ') && !hasNumber) { - address.target_layer = get_layers(['admin', 'poi']); - } - - return address; + // perform full address parsing + // except on queries so short they obviously can't contain an address + return parser( query ); }; var addressWithAdminParts = getAdminPartsBySplittingOnDelim(query); diff --git a/test/unit/helper/query_parser.js b/test/unit/helper/query_parser.js index c82a31a2..6f0f67cd 100644 --- a/test/unit/helper/query_parser.js +++ b/test/unit/helper/query_parser.js @@ -136,11 +136,7 @@ module.exports.tests.parse_address = function(test, common) { t.equal(typeof address.postalcode, 'number', 'valid zip (' + address.postalcode + ')'); t.equal(address.postalcode, query.zip, 'correct postal code (' + query.zip + ')'); } - - if (address.text === address.regions.join(' ')) { - t.deepEqual(address.target_layer, query.target_layer, 'admin_parts set correctly to ' + query.target_layer.join(', ')); - } - + t.end(); }); }; @@ -165,4 +161,4 @@ module.exports.all = function (tape, common) { for( var testCase in module.exports.tests ){ module.exports.tests[testCase](test, common); } -}; \ No newline at end of file +}; From 470458317811353db9ce3415617654f0ca653cbe Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 3 Sep 2015 16:22:33 -0400 Subject: [PATCH 04/21] Fix inconsistently named imported module --- sanitiser/_input.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sanitiser/_input.js b/sanitiser/_input.js index d73048a0..c4b03f2c 100644 --- a/sanitiser/_input.js +++ b/sanitiser/_input.js @@ -1,5 +1,5 @@ -var isObject = require('is-object'); -var query_parse= require('../helper/query_parser'); +var isObject = require('is-object'); +var query_parser = require('../helper/query_parser'); // validate inputs, convert types and apply defaults function sanitize( req ){ @@ -21,7 +21,7 @@ function sanitize( req ){ req.clean.input = params.input; - req.clean.parsed_input = query_parse(params.input); + req.clean.parsed_input = query_parser(params.input); return { 'error': false }; } From e42cb4a746becc8fdc643ecc7fc2fd288dc5d0a5 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 3 Sep 2015 16:24:20 -0400 Subject: [PATCH 05/21] Remove seemingly unhelpful comment --- helper/query_parser.js | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/helper/query_parser.js b/helper/query_parser.js index ad9d57ac..99242a5e 100644 --- a/helper/query_parser.js +++ b/helper/query_parser.js @@ -72,17 +72,3 @@ module.exports = function(query) { return parsed_input; }; - - -// parsed_input = { -// name : parsedAddress.name, -// number : parsedAddress.number, -// street : parsedAddress.street, -// city : parsedAddress.city, -// state : parsedAddress.state, -// country: parsedAddress.country, -// postalcode : parsedAddress.postalcode, -// regions: parsedAddress.regions, -// admin_parts: parsedAddress.admin_parts, -// target_layer: parsedAddress.target_layer -// } From d35544cdf4861bfb79b04c89ee5ff6c8312317c5 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 3 Sep 2015 17:21:16 -0400 Subject: [PATCH 06/21] Remove loops and many global checks from sanitiser tests Modifying these sanitiser tests became extremely hard because almost all of them were looping over lots of individual test cases, which places assumptions about the common behavior of potentialy very different test cases, as well as making assertions about huge swaths of output when only a small amount of that output was really under test. Hopefully these changes will make our tests easier to modify, and not really lose any ability to catch bugs. --- test/unit/sanitiser/_input.js | 4 +- test/unit/sanitiser/search.js | 91 +++++++++++++---------------------- 2 files changed, 36 insertions(+), 59 deletions(-) diff --git a/test/unit/sanitiser/_input.js b/test/unit/sanitiser/_input.js index 98eab084..378b8140 100644 --- a/test/unit/sanitiser/_input.js +++ b/test/unit/sanitiser/_input.js @@ -7,7 +7,7 @@ var input = require('../../../sanitiser/_input'), 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], nonAddressLayers = [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin' ], - defaultParsed= { target_layer: nonAddressLayers }, + defaultParsed= { }, defaultClean = { input: 'test', layers: allLayers, size: 10, @@ -26,4 +26,4 @@ module.exports = { defaultParsed: defaultParsed, defaultClean : defaultClean, getTargetLayers: getTargetLayers -}; \ No newline at end of file +}; diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index 1f37fa13..f6c050cf 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -34,13 +34,10 @@ module.exports.tests.interface = function(test, common) { }); }; -module.exports.tests.sanitize_input = function(test, common) { - var inputs = { - invalid: [ '', 100, null, undefined, new Date() ], - valid: [ 'a', 'aa', 'aaaaaaaa' ] - }; +module.exports.tests.sanitize_invalid_input = function(test, common) { test('invalid input', function(t) { - inputs.invalid.forEach( function( input ){ + var invalid = [ '', 100, null, undefined, new Date() ]; + invalid.forEach( function( input ){ sanitize({ input: input }, function( err, clean ){ t.equal(err, 'invalid param \'input\': text length, must be >0', input + ' is an invalid input'); t.equal(clean, undefined, 'clean not set'); @@ -48,16 +45,26 @@ module.exports.tests.sanitize_input = function(test, common) { }); t.end(); }); - test('valid input', function(t) { - inputs.valid.forEach( function( input ){ - sanitize({ input: input }, function( err, clean ){ - var expected = JSON.parse(JSON.stringify( defaultClean )); - expected.input = input; - expected.parsed_input.target_layer = _input.getTargetLayers(input); +}; - t.equal(err, undefined, 'no error'); - t.deepEqual(clean, expected, 'clean set correctly (' + input + ')'); - }); +module.exports.tests.sanitise_valid_input = function(test, common) { + test('valid short input', function(t) { + sanitize({ input: 'a' }, function( err, clean ){ + t.equal(err, undefined, 'no error'); + }); + t.end(); + }); + + test('valid not-quite-as-short input', function(t) { + sanitize({ input: 'aa' }, function( err, clean ){ + t.equal(err, undefined, 'no error'); + }); + t.end(); + }); + + test('valid longer input', function(t) { + sanitize({ input: 'aaaaaaaa' }, function( err, clean ){ + t.equal(err, undefined, 'no error'); }); t.end(); }); @@ -75,8 +82,6 @@ module.exports.tests.sanitize_input_with_delim = function(test, common) { expected.parsed_input = parser(input); t.equal(err, undefined, 'no error'); t.equal(clean.parsed_input.name, expected.parsed_input.name, 'clean name set correctly'); - t.equal(clean.parsed_input.admin_parts, expected.parsed_input.admin_parts, 'clean admin_parts set correctly'); - t.deepEqual(clean, expected, 'clean set correctly (' + input + ')'); }); }); @@ -103,10 +108,8 @@ module.exports.tests.sanitize_lat = function(test, common) { sanitize({ input: 'test', lat: lat, lon: 0 }, function( err, clean ){ var expected = JSON.parse(JSON.stringify( defaultClean )); expected.lat = parseFloat( lat ); - expected.lon = 0; t.equal(err, undefined, 'no error'); - expected.parsed_input = parser('test'); - t.deepEqual(clean, expected, 'clean set correctly (' + lat + ')'); + t.deepEqual(clean.lat, expected.lat, 'clean lat set correctly (' + lat + ')'); }); }); t.end(); @@ -122,10 +125,8 @@ module.exports.tests.sanitize_lon = function(test, common) { sanitize({ input: 'test', lat: 0, lon: lon }, function( err, clean ){ var expected = JSON.parse(JSON.stringify( defaultClean )); expected.lon = parseFloat( lon ); - expected.lat = 0; t.equal(err, undefined, 'no error'); - expected.parsed_input = parser('test'); - t.deepEqual(clean, expected, 'clean set correctly (' + lon + ')'); + t.deepEqual(clean.lon, expected.lon, 'clean set correctly (' + lon + ')'); }); }); t.end(); @@ -133,34 +134,27 @@ module.exports.tests.sanitize_lon = function(test, common) { }; module.exports.tests.sanitize_optional_geo = function(test, common) { - test('no lat/lon', function(t) { + test('no lat/lon', function(t) { sanitize({ input: 'test' }, function( err, clean ){ - var expected = defaultClean; t.equal(err, undefined, 'no error'); t.equal(clean.lat, undefined, 'clean set without lat'); t.equal(clean.lon, undefined, 'clean set without lon'); - expected.parsed_input = parser('test'); - t.deepEqual(clean, expected, 'clean set without lat/lon'); }); t.end(); }); - test('no lat', function(t) { + test('no lat', function(t) { sanitize({ input: 'test', lon: 0 }, function( err, clean ){ - var expected = JSON.parse(JSON.stringify( defaultClean )); - expected.lon = 0; + var expected_lon = 0; t.equal(err, undefined, 'no error'); - expected.parsed_input = parser('test'); - t.deepEqual(clean, expected, 'clean set correctly (without any lat)'); + t.deepEqual(clean.lon, expected_lon, 'clean set correctly (without any lat)'); }); t.end(); }); - test('no lon', function(t) { + test('no lon', function(t) { sanitize({ input: 'test', lat: 0 }, function( err, clean ){ - var expected = JSON.parse(JSON.stringify( defaultClean )); - expected.lat = 0; + var expected_lat = 0; t.equal(err, undefined, 'no error'); - expected.parsed_input = parser('test'); - t.deepEqual(clean, expected, 'clean set correctly (without any lon)'); + t.deepEqual(clean.lat, expected_lat, 'clean set correctly (without any lon)'); }); t.end(); }); @@ -168,8 +162,6 @@ module.exports.tests.sanitize_optional_geo = function(test, common) { module.exports.tests.sanitize_bbox = function(test, common) { var bboxes = { - invalid_coordinates: [ - ], invalid: [ '91;-181,-91,181', // invalid - semicolon between coordinates 'these,are,not,numbers', @@ -196,22 +188,11 @@ module.exports.tests.sanitize_bbox = function(test, common) { ] }; - test('invalid bbox coordinates', function(t) { - bboxes.invalid_coordinates.forEach( function( bbox ){ - sanitize({ input: 'test', bbox: bbox }, function( err, clean ){ - t.ok(err.match(/Invalid (lat|lon)/), bbox + ' is invalid'); - t.equal(clean, undefined, 'clean not set'); - }); - }); - t.end(); - }); test('invalid bbox', function(t) { bboxes.invalid.forEach( function( bbox ){ sanitize({ input: 'test', bbox: bbox }, function( err, clean ){ - var expected = JSON.parse(JSON.stringify( defaultClean )); t.equal(err, undefined, 'no error'); - expected.parsed_input = parser('test'); - t.deepEqual(clean, expected, 'falling back on 50km distance from centroid'); + t.equal(clean.bbox, undefined, 'falling back on 50km distance from centroid'); }); }); t.end(); @@ -219,19 +200,17 @@ module.exports.tests.sanitize_bbox = function(test, common) { test('valid bbox', function(t) { bboxes.valid.forEach( function( bbox ){ sanitize({ input: 'test', bbox: bbox }, function( err, clean ){ - var expected = JSON.parse(JSON.stringify( defaultClean )); var bboxArray = bbox.split(',').map(function(i) { return parseInt(i); }); - expected.bbox = { + var expected_bbox = { right: Math.max(bboxArray[0], bboxArray[2]), top: Math.max(bboxArray[1], bboxArray[3]), left: Math.min(bboxArray[0], bboxArray[2]), bottom: Math.min(bboxArray[1], bboxArray[3]) }; t.equal(err, undefined, 'no error'); - expected.parsed_input = parser('test'); - t.deepEqual(clean, expected, 'clean set correctly (' + bbox + ')'); + t.deepEqual(clean.bbox, expected_bbox, 'clean set correctly (' + bbox + ')'); }); }); t.end(); @@ -422,8 +401,6 @@ module.exports.tests.middleware_success = function(test, common) { var req = { query: { input: 'test' }}; var next = function( message ){ t.equal(message, undefined, 'no error message set'); - req.clean.parsed_input = parser('test'); - t.deepEqual(req.clean, defaultClean); t.end(); }; middleware( req, undefined, next ); From b9152dbe299bf231b5d8847c976090a2a4aff60e Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 3 Sep 2015 17:23:57 -0400 Subject: [PATCH 07/21] Separate concerns of address parser The address parser currently does two things: 1.) make some intelligent guesses as to possible admin regions to explicitly search against to improve the quality of results returned 2.) make some intelligent guesses as to when no part of the query needs to search against anything other than admin regions. This somewhat improves the quality of results returned but mostly improves the speed of the Elasticsearch query since it's searching significantly fewer recoords. These two concerns are now split into two separate methods within the query_parser helper module. They are mostly independent today, but don't have to be in the future. --- helper/query_parser.js | 44 ++++++++++++++++---------------- sanitiser/_input.js | 5 +++- test/unit/helper/query_parser.js | 17 +++++++----- test/unit/query/search.js | 6 ++--- test/unit/sanitiser/_input.js | 2 +- test/unit/sanitiser/search.js | 7 +++-- 6 files changed, 45 insertions(+), 36 deletions(-) diff --git a/helper/query_parser.js b/helper/query_parser.js index 99242a5e..dcdbbec6 100644 --- a/helper/query_parser.js +++ b/helper/query_parser.js @@ -1,10 +1,25 @@ var parser = require('addressit'); var extend = require('extend'); -var get_layers = require('../helper/layers'); +var get_layers_helper = require('../helper/layers'); var delim = ','; -module.exports = function(query) { +module.exports = {}; + +module.exports.get_layers = function get_layers(query) { + var tokenized = query.split(/[ ,]+/); + var hasNumber = /\d/.test(query); + + if (query.length <= 3 ) { + // no address parsing required + return get_layers_helper(['admin']); + } else if (tokenized.length === 1 || (tokenized.length < 3 && !hasNumber)) { + // no need to hit address layers if there's only one (or two) token(s) + return get_layers_helper(['admin', 'poi']); + } +}; + +module.exports.get_parsed_address = function get_parsed_address(query) { var tokenized = query.split(/[ ,]+/); var hasNumber = /\d/.test(query); @@ -22,32 +37,18 @@ module.exports = function(query) { return address; }; - var getTargetLayersWhenAddressParsingIsNotNecessary = function(query) { - var address = {}; - // set target_layer if input length <= 3 characters - if (query.length <= 3 ) { - // no address parsing required - address.target_layer = get_layers(['admin']); - } else if (tokenized.length === 1 || (tokenized.length < 3 && !hasNumber)) { - // no need to hit address layers if there's only one (or two) token(s) - address.target_layer = get_layers(['admin', 'poi']); - } - - return address.target_layer ? address : null; - }; - var getAddressParts = function(query) { // perform full address parsing // except on queries so short they obviously can't contain an address - return parser( query ); + if (query.length > 3) { + return parser( query ); + } }; var addressWithAdminParts = getAdminPartsBySplittingOnDelim(query); - var addressWithTargetLayers= getTargetLayersWhenAddressParsingIsNotNecessary(query); - var addressWithAddressParts= !addressWithTargetLayers ? getAddressParts(query) : {}; + var addressWithAddressParts= getAddressParts(query); var parsedAddress = extend(addressWithAdminParts, - addressWithTargetLayers, addressWithAddressParts); var address_parts = [ 'name', @@ -58,8 +59,7 @@ module.exports = function(query) { 'country', 'postalcode', 'regions', - 'admin_parts', - 'target_layer' + 'admin_parts' ]; var parsed_input = {}; diff --git a/sanitiser/_input.js b/sanitiser/_input.js index c4b03f2c..2ec61970 100644 --- a/sanitiser/_input.js +++ b/sanitiser/_input.js @@ -21,7 +21,10 @@ function sanitize( req ){ req.clean.input = params.input; - req.clean.parsed_input = query_parser(params.input); + req.clean.parsed_input = query_parser.get_parsed_address(params.input); + + req.clean.types = req.clean.layers || {}; + req.clean.types.from_address_parsing = query_parser.get_layers(params.input); return { 'error': false }; } diff --git a/test/unit/helper/query_parser.js b/test/unit/helper/query_parser.js index 6f0f67cd..5cb84b83 100644 --- a/test/unit/helper/query_parser.js +++ b/test/unit/helper/query_parser.js @@ -6,7 +6,8 @@ module.exports.tests = {}; module.exports.tests.interface = function(test, common) { test('interface', function(t) { - t.equal(typeof parser, 'function', 'valid function'); + t.equal(typeof parser.get_parsed_address, 'function', 'valid function'); + t.equal(typeof parser.get_layers, 'function', 'valid function'); t.end(); }); }; @@ -17,7 +18,7 @@ module.exports.tests.split_on_comma = function(test, common) { var testParse = function(query) { test('naive parsing ' + query, function(t) { - var address = parser(query); + var address = parser.get_parsed_address(query); var delimIndex = query.indexOf(delim); var name = query.substring(0, delimIndex); var admin_parts = query.substring(delimIndex + 1).trim(); @@ -41,11 +42,12 @@ module.exports.tests.parse_three_chars_or_less = function(test, common) { var testParse = function(query) { test('query length < 3 (' + query + ')', function(t) { - var address = parser(query); + var address = parser.get_parsed_address(query); var target_layer = get_layers(['admin']); + var layers = parser.get_layers(query); t.equal(typeof address, 'object', 'valid object'); - t.deepEqual(address.target_layer, target_layer, 'admin_parts set correctly to ' + target_layer.join(', ')); + t.deepEqual(layers, target_layer, 'admin_parts set correctly to ' + target_layer.join(', ')); t.end(); }); }; @@ -63,15 +65,16 @@ module.exports.tests.parse_one_or_more_tokens = function(test, common) { var testParse = function(query, parse_address) { test('query with one or more tokens (' + query + ')', function(t) { - var address = parser(query); + var address = parser.get_parsed_address(query); var target_layer = get_layers(['admin', 'poi']); + var layers = parser.get_layers(query); t.equal(typeof address, 'object', 'valid object'); if (parse_address) { t.deepEqual(address.regions.join(''), query, 'since query contained a number, it went through address parsing'); } else { - t.deepEqual(address.target_layer, target_layer, 'admin_parts set correctly to ' + target_layer.join(', ')); + t.deepEqual(layers, target_layer, 'admin_parts set correctly to ' + target_layer.join(', ')); } t.end(); @@ -114,7 +117,7 @@ module.exports.tests.parse_address = function(test, common) { // remove leading whitespace query_string = query_string.substring(1); - var address = parser(query_string); + var address = parser.get_parsed_address(query_string); var non_address_layer = get_layers(['admin', 'poi']); t.equal(typeof address, 'object', 'valid object for the address ('+query_string+')'); diff --git a/test/unit/query/search.js b/test/unit/query/search.js index f2e6d9dc..c4797f16 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -274,7 +274,7 @@ module.exports.tests.query = function(test, common) { 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], size: 10, details: true, - parsed_input: parser(address), + parsed_input: parser.get_parsed_address(address), default_layers_set: true }); @@ -476,7 +476,7 @@ module.exports.tests.query = function(test, common) { 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], size: 10, details: true, - parsed_input: parser(partial_address), + parsed_input: parser.get_parsed_address(partial_address), default_layers_set: true }); @@ -644,7 +644,7 @@ module.exports.tests.query = function(test, common) { 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], size: 10, details: true, - parsed_input: parser(partial_address), + parsed_input: parser.get_parsed_address(partial_address), default_layers_set: true }); diff --git a/test/unit/sanitiser/_input.js b/test/unit/sanitiser/_input.js index 378b8140..b6073358 100644 --- a/test/unit/sanitiser/_input.js +++ b/test/unit/sanitiser/_input.js @@ -17,7 +17,7 @@ var input = require('../../../sanitiser/_input'), lon:0 }, getTargetLayers = function(query) { - var address = parser(query); + var address = parser.get_parsed_address(query); return address.target_layer; }; diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index f6c050cf..e2ccd4ce 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -11,6 +11,8 @@ var search = require('../../../sanitiser/search'), types: { from_layers: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], + from_address_parsing: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', + 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], }, size: 10, details: true, @@ -79,7 +81,7 @@ module.exports.tests.sanitize_input_with_delim = function(test, common) { var expected = JSON.parse(JSON.stringify( defaultClean )); expected.input = input; - expected.parsed_input = parser(input); + expected.parsed_input = parser.get_parsed_address(input); t.equal(err, undefined, 'no error'); t.equal(clean.parsed_input.name, expected.parsed_input.name, 'clean name set correctly'); @@ -330,7 +332,8 @@ module.exports.tests.sanitize_layers = function(test, common) { test('address (alias) layer', function(t) { var address_layers = ['osmaddress','openaddresses']; sanitize({ layers: 'address', input: 'test' }, function( err, clean ){ - t.deepEqual(clean.types.from_layers, address_layers, 'address layers set'); + t.deepEqual(clean.types.from_layers, address_layers, 'types from layers set'); + t.deepEqual(clean.types.from_address_parser, _input.allLayers, 'address parser uses default layers'); t.end(); }); }); From 1573f3a3fac92aa96aab08176afc6d39240494f1 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 3 Sep 2015 18:06:40 -0400 Subject: [PATCH 08/21] Move cmd.type setting logic to types helper --- controller/search.js | 13 ++++---- helper/types.js | 15 +++++++++ test/unit/helper/types.js | 67 +++++++++++++++++++++++++++++++++++++++ test/unit/run.js | 1 + 4 files changed, 89 insertions(+), 7 deletions(-) create mode 100644 helper/types.js create mode 100644 test/unit/helper/types.js diff --git a/controller/search.js b/controller/search.js index 386af22f..c353c5be 100644 --- a/controller/search.js +++ b/controller/search.js @@ -1,4 +1,5 @@ var service = { search: require('../service/search') }; +var types = require ( '../helper/types' ); function setup( backend, query ){ @@ -15,13 +16,11 @@ function setup( backend, query ){ body: query( req.clean ) }; - if (req.clean.types && req.clean.types.from_layers) { - cmd.type = req.clean.types.from_layers; - } - - // set type if input suggests targeting a layer(s) - if (req.clean.default_layers_set && req.clean.parsed_input) { - cmd.type = req.clean.parsed_input.target_layer || cmd.type; + // don't directly set cmd.type from types helper to avoid sometimes + // setting cmd.type to undefined (having the key not set is cleaner) + var type = types(req.clean.types); + if (type !== undefined) { + cmd.type = type; } // query backend diff --git a/helper/types.js b/helper/types.js new file mode 100644 index 00000000..30ab74d5 --- /dev/null +++ b/helper/types.js @@ -0,0 +1,15 @@ +var valid_types = require( '../query/indeces' ); + +module.exports = function calculate_types(clean_types) { + if (!clean_types) { + return undefined; + } + + if (clean_types.from_layers) { + return clean_types.from_layers; + } + + if (clean_types.from_address_parser) { + return clean_types.from_address_parser; + } +}; diff --git a/test/unit/helper/types.js b/test/unit/helper/types.js new file mode 100644 index 00000000..1e7d1813 --- /dev/null +++ b/test/unit/helper/types.js @@ -0,0 +1,67 @@ +var types = require('../../../helper/types'); + +var valid_types = require( '../../../query/indeces' ); +module.exports.tests = {}; + +module.exports.tests.no_cleaned_types = function(test, common) { + test('no cleaned types', function(t) { + var actual = types(undefined); + t.equal(actual, undefined, 'all valid types returned for empty input'); + t.end(); + }); + + test('no cleaned types', function(t) { + var cleaned_types = {}; + var actual = types(cleaned_types); + t.equal(actual, undefined, 'all valid types returned for empty input'); + t.end(); + }); +}; + +module.exports.tests.address_parser = function(test, common) { + test('address parser specifies only admin layers', function(t) { + var cleaned_types = { + from_address_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: ['geonames'] + }; + var actual = types(cleaned_types); + var expected = ['geonames']; + 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: ['geonames'], + from_address_parser: ['admin0'] // simplified return value from address parse + }; + var actual = types(cleaned_types); + var expected = ['geonames']; + t.deepEqual(actual, expected, 'layers parameter overrides address parser completely'); + 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 619da765..90a14278 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -19,6 +19,7 @@ var tests = [ require('./helper/geojsonify'), require('./helper/outputSchema'), require('./helper/adminFields'), + require('./helper/types'), ]; tests.map(function(t) { From 3ff175243478bc8115251cf49e0aa439a822d03e Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 3 Sep 2015 18:24:25 -0400 Subject: [PATCH 09/21] Remove default_layers_set flag! After refactoring, this flag is no longer needed, as all areas of the code that care about layers do so by setting a key within clean.types, and then the types helper intelligently combines those together later. --- sanitiser/_layers.js | 4 ++-- test/unit/query/search.js | 3 --- test/unit/sanitiser/reverse.js | 4 +--- test/unit/sanitiser/search.js | 5 ----- test/unit/sanitiser/suggest.js | 1 - 5 files changed, 3 insertions(+), 14 deletions(-) diff --git a/sanitiser/_layers.js b/sanitiser/_layers.js index 25e8ba26..be13526a 100644 --- a/sanitiser/_layers.js +++ b/sanitiser/_layers.js @@ -16,9 +16,9 @@ function sanitize( req ){ } // default case (no layers specified in GET params) + // don't even set the from_layers key in this case if('string' !== typeof params.layers || !params.layers.length){ - params.layers = 'poi,admin,address'; // default layers - clean.default_layers_set = true; + return { 'error': false }; } // decide which layers can be queried diff --git a/test/unit/query/search.js b/test/unit/query/search.js index c4797f16..ff678401 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -275,7 +275,6 @@ module.exports.tests.query = function(test, common) { size: 10, details: true, parsed_input: parser.get_parsed_address(address), - default_layers_set: true }); var expected = { @@ -477,7 +476,6 @@ module.exports.tests.query = function(test, common) { size: 10, details: true, parsed_input: parser.get_parsed_address(partial_address), - default_layers_set: true }); var expected = { @@ -645,7 +643,6 @@ module.exports.tests.query = function(test, common) { size: 10, details: true, parsed_input: parser.get_parsed_address(partial_address), - default_layers_set: true }); var expected = { diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js index d8d666c1..73e3b381 100644 --- a/test/unit/sanitiser/reverse.js +++ b/test/unit/sanitiser/reverse.js @@ -6,16 +6,14 @@ var suggest = require('../../../sanitiser/reverse'), defaultError = 'missing param \'lat\'', defaultClean = { lat:0, types: { - from_layers: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', - 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], }, lon: 0, size: 10, details: true, - default_layers_set: true, categories: [] }, sanitize = function(query, cb) { _sanitize({'query':query}, cb); }; +var all_layers = ( '../../query/indeces' ); module.exports.tests = {}; diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index e2ccd4ce..9bf695e4 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -9,15 +9,10 @@ var search = require('../../../sanitiser/search'), defaultError = 'invalid param \'input\': text length, must be >0', defaultClean = { input: 'test', types: { - from_layers: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', - 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], - from_address_parsing: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', - 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], }, size: 10, details: true, parsed_input: defaultParsed, - default_layers_set: true }, sanitize = function(query, cb) { _sanitize({'query':query}, cb); }; diff --git a/test/unit/sanitiser/suggest.js b/test/unit/sanitiser/suggest.js index 003b44ae..71f2ed63 100644 --- a/test/unit/sanitiser/suggest.js +++ b/test/unit/sanitiser/suggest.js @@ -15,7 +15,6 @@ var suggest = require('../../../sanitiser/suggest'), lat:0, lon:0, parsed_input: defaultParsed, - default_layers_set: true }, sanitize = function(query, cb) { _sanitize({'query':query}, cb); }; From 24349a3839d3712bedecd57dbbbd3a03c34b6eb3 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Fri, 4 Sep 2015 11:56:48 -0400 Subject: [PATCH 10/21] Rename query/indidces to query/types It didn't really contain a list of indices. --- helper/types.js | 2 +- query/{indeces.js => types.js} | 4 ++-- sanitiser/_id.js | 8 ++++---- sanitiser/_layers.js | 8 ++++---- test/unit/helper/types.js | 2 +- test/unit/query/{indeces.js => types.js} | 10 +++++----- test/unit/run.js | 2 +- test/unit/sanitiser/place.js | 4 ++-- test/unit/sanitiser/reverse.js | 1 - 9 files changed, 20 insertions(+), 21 deletions(-) rename query/{indeces.js => types.js} (89%) rename test/unit/query/{indeces.js => types.js} (63%) diff --git a/helper/types.js b/helper/types.js index 30ab74d5..20443ad9 100644 --- a/helper/types.js +++ b/helper/types.js @@ -1,4 +1,4 @@ -var valid_types = require( '../query/indeces' ); +var valid_types = require( '../query/types' ); module.exports = function calculate_types(clean_types) { if (!clean_types) { diff --git a/query/indeces.js b/query/types.js similarity index 89% rename from query/indeces.js rename to query/types.js index 1fff5961..5f8cfab7 100644 --- a/query/indeces.js +++ b/query/types.js @@ -1,5 +1,5 @@ -// querable indeces +// querable types module.exports = [ 'geoname', @@ -13,4 +13,4 @@ module.exports = [ 'local_admin', 'osmaddress', 'openaddresses' -]; \ No newline at end of file +]; diff --git a/sanitiser/_id.js b/sanitiser/_id.js index a8424363..78ff6b1f 100644 --- a/sanitiser/_id.js +++ b/sanitiser/_id.js @@ -7,7 +7,7 @@ var isObject = require('is-object'); function sanitize( req ){ req.clean = req.clean || {}; var params = req.query; - var indeces = require('../query/indeces'); + var types = require('../query/types'); var delim = ':'; // ensure params is a valid object @@ -55,9 +55,9 @@ function sanitize( req ){ if('string' !== typeof type || !type.length){ return errormessage(thisparam); } - // type text must be one of the indeces - if(indeces.indexOf(type) === -1){ - return errormessage('type', type + ' is invalid. It must be one of these values - [' + indeces.join(', ') + ']'); + // type text must be one of the types + if(types.indexOf(type) === -1){ + return errormessage('type', type + ' is invalid. It must be one of these values - [' + types.join(', ') + ']'); } req.clean.ids.push({ id: id, diff --git a/sanitiser/_layers.js b/sanitiser/_layers.js index be13526a..8feb50e9 100644 --- a/sanitiser/_layers.js +++ b/sanitiser/_layers.js @@ -1,6 +1,6 @@ var isObject = require('is-object'), - indeces = require('../query/indeces'), + types = require('../query/types'), get_layers = require('../helper/layers'); // validate inputs, convert types and apply defaults @@ -23,7 +23,7 @@ function sanitize( req ){ // decide which layers can be queried var alias_layers = ['poi', 'admin', 'address']; - var alias_indeces = indeces.concat(alias_layers); + var alias_types = types.concat(alias_layers); // parse GET params var layers = params.layers.split(',').map( function( layer ){ @@ -32,10 +32,10 @@ function sanitize( req ){ // validate layer names for( var x=0; x0'; }, defaultFormatError = 'invalid: must be of the format type:id for ex: \'geoname:4163334\'', defaultError = 'invalid param \'id\': text length, must be >0', defaultMissingTypeError = function(input) { var type = input.split(delimiter)[0]; - return type + ' is invalid. It must be one of these values - [' + indeces.join(', ') + ']'; }, + return type + ' is invalid. It must be one of these values - [' + types.join(', ') + ']'; }, defaultClean = { ids: [ { id: '123', type: 'geoname' } ], details: true }, sanitize = function(query, cb) { _sanitize({'query':query}, cb); }, inputs = { diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js index 73e3b381..bf9462b8 100644 --- a/test/unit/sanitiser/reverse.js +++ b/test/unit/sanitiser/reverse.js @@ -13,7 +13,6 @@ var suggest = require('../../../sanitiser/reverse'), categories: [] }, sanitize = function(query, cb) { _sanitize({'query':query}, cb); }; -var all_layers = ( '../../query/indeces' ); module.exports.tests = {}; From ef42ad4d2ee8b17ddf1a25af515c651988b98669 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Fri, 4 Sep 2015 12:09:30 -0400 Subject: [PATCH 11/21] Add sources -> types mapping file --- query/sources.js | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 query/sources.js diff --git a/query/sources.js b/query/sources.js new file mode 100644 index 00000000..77be7fb9 --- /dev/null +++ b/query/sources.js @@ -0,0 +1,10 @@ +/* + * Mapping from data sources to type values + */ + +module.exports = { + 'geonames': ['geoname'], + 'openaddresses' : ['openaddresses'], + 'quattroshapes': ['admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin'], + 'openstreetmap' : ['osmaddress', 'osmnode', 'osmway'] +}; From 3479325fa84690b238e2fb40a725ee0268e68282 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Fri, 4 Sep 2015 14:45:37 -0400 Subject: [PATCH 12/21] Add source parameter sanitiser --- sanitiser/_source.js | 44 +++++++++++++ test/unit/run.js | 1 + test/unit/sanitiser/_source.js | 114 +++++++++++++++++++++++++++++++++ 3 files changed, 159 insertions(+) create mode 100644 sanitiser/_source.js create mode 100644 test/unit/sanitiser/_source.js diff --git a/sanitiser/_source.js b/sanitiser/_source.js new file mode 100644 index 00000000..f8f89a68 --- /dev/null +++ b/sanitiser/_source.js @@ -0,0 +1,44 @@ +var isObject = require('is-object'); +var sources_map = require( '../query/sources' ); +var all_sources = Object.keys(sources_map); + +function sanitize( req ) { + req.clean = req.clean || {}; + var params = req.query; + + req.clean.types = req.clean.types || {}; + + // ensure the input params are a valid object + if( !isObject( params ) ){ + params = {}; + } + + // default case (no layers specified in GET params) + // don't even set the from_layers key in this case + if('string' !== typeof params.source || !params.source.length){ + return { error: false }; + } + + var sources = params.source.split(','); + + var invalid_sources = sources.filter(function(source) { + return all_sources.indexOf(source) === -1; + }); + + if (invalid_sources.length > 0) { + return { + error: true, + msg: '`' + invalid_sources[0] + '` is an invalid source parameter. Valid options: ' + all_sources.join(', ') + }; + } + + var types = sources.reduce(function(acc, source) { + return acc.concat(sources_map[source]); + }, []); + + req.clean.types.from_source = types; + + return { error: false }; +} + +module.exports = sanitize; diff --git a/test/unit/run.js b/test/unit/run.js index 26a22f87..d5d55676 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -8,6 +8,7 @@ var tests = [ require('./controller/search'), require('./service/mget'), require('./service/search'), + require('./sanitiser/_source'), require('./sanitiser/search'), require('./sanitiser/reverse'), require('./sanitiser/place'), diff --git a/test/unit/sanitiser/_source.js b/test/unit/sanitiser/_source.js new file mode 100644 index 00000000..235e18c6 --- /dev/null +++ b/test/unit/sanitiser/_source.js @@ -0,0 +1,114 @@ +var sanitize = require( '../../../sanitiser/_source' ); + +var success_response = { error: false }; + +module.exports.tests = {}; + +module.exports.tests.no_sources = function(test, common) { + test('source is not set', function(t) { + var req = { + query: { } + }; + + var response = sanitize(req); + + t.deepEqual(req.clean.types, {}, 'clean.types should be empty object'); + t.deepEqual(response, success_response, 'no error returned'); + t.end(); + }); + + test('source is empty string', function(t) { + var req = { + query: { + source: '' + } + }; + + var response = sanitize(req); + + t.deepEqual(req.clean.types, {}, 'clean.types should be empty object'); + t.deepEqual(response, success_response, 'no error returned'); + t.end(); + }); +}; + +module.exports.tests.valid_sources = function(test, common) { + test('geonames source', function(t) { + var req = { + query: { + source: 'geonames' + } + }; + + var response = sanitize(req); + + t.deepEqual(req.clean.types, { from_source: ['geoname'] }, 'clean.types should contain from_source entry with geonames'); + t.deepEqual(response, success_response, 'no error returned'); + t.end(); + }); + + test('openstreetmap source', function(t) { + var req = { + query: { + source: 'openstreetmap' + } + }; + var expected_types = { + from_source: ['osmaddress', 'osmnode', 'osmway'] + }; + + var response = sanitize(req); + + t.deepEqual(req.clean.types, expected_types, 'clean.types should contain from_source entry with multiple entries for openstreetmap'); + t.deepEqual(response, success_response, 'no error returned'); + t.end(); + }); + + test('multiple sources', function(t) { + var req = { + query: { + source: 'openstreetmap,openaddresses' + } + }; + var expected_types = { + from_source: ['osmaddress', 'osmnode', 'osmway', 'openaddresses'] + }; + + var response = sanitize(req); + + t.deepEqual(req.clean.types, expected_types, + 'clean.types should contain from_source entry with multiple entries for openstreetmap and openadresses'); + t.deepEqual(response, success_response, 'no error returned'); + t.end(); + }); +}; + +module.exports.tests.invalid_sources = function(test, common) { + test('geonames source', function(t) { + var req = { + query: { + source: 'notasource' + } + }; + var expected_response = { + error: true, + msg: '`notasource` is an invalid source parameter. Valid options: geonames, openaddresses, quattroshapes, openstreetmap' + }; + + var response = sanitize(req); + + t.deepEqual(response, expected_response, 'error with message returned'); + t.deepEqual(req.clean.types, { }, 'clean.types should remain empty'); + t.end(); + }); +}; + +module.exports.all = function (tape, common) { + function test(name, testFunction) { + return tape('SANTIZE _source ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; From eed010835ba9b719e8014690c22fadae0a1f9fb9 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Fri, 4 Sep 2015 14:48:58 -0400 Subject: [PATCH 13/21] Fix reference to geoname This is just in a unit test, so it technically passes, but geonames is not a valid layer option (geoname, singular, is used instead), so it's best to correct. --- test/unit/helper/types.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit/helper/types.js b/test/unit/helper/types.js index b05db174..d56f12d2 100644 --- a/test/unit/helper/types.js +++ b/test/unit/helper/types.js @@ -33,10 +33,10 @@ module.exports.tests.address_parser = function(test, common) { module.exports.tests.layers_parameter = function(test, common) { test('layers parameter specifies only some layers', function(t) { var cleaned_types = { - from_layers: ['geonames'] + from_layers: ['geoname'] }; var actual = types(cleaned_types); - var expected = ['geonames']; + var expected = ['geoname']; t.deepEqual(actual, expected, 'only types specified by layers parameter returned'); t.end(); }); @@ -45,11 +45,11 @@ module.exports.tests.layers_parameter = function(test, common) { 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: ['geonames'], + from_layers: ['geoname'], from_address_parser: ['admin0'] // simplified return value from address parse }; var actual = types(cleaned_types); - var expected = ['geonames']; + var expected = ['geoname']; t.deepEqual(actual, expected, 'layers parameter overrides address parser completely'); t.end(); }); From a479de7527d1633a56880c53d3ddb41c16648c6d Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Fri, 4 Sep 2015 15:22:13 -0400 Subject: [PATCH 14/21] Calculate intersection of types requested by source and layers params --- helper/types.js | 24 ++++++++++++++++++++++-- test/unit/helper/types.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/helper/types.js b/helper/types.js index 20443ad9..a87a65b8 100644 --- a/helper/types.js +++ b/helper/types.js @@ -1,12 +1,32 @@ var valid_types = require( '../query/types' ); +/** + * Calculate the set-style intersection of two arrays + */ +var intersection = function intersection(set1, set2) { + return set2.filter(function(value) { + return set1.indexOf(value) !== -1; + }); +}; + module.exports = function calculate_types(clean_types) { if (!clean_types) { return undefined; } - if (clean_types.from_layers) { - return clean_types.from_layers; + + if (clean_types.from_layers || clean_types.from_source) { + var types = valid_types; + + if (clean_types.from_layers) { + types = intersection(types, clean_types.from_layers); + } + + if (clean_types.from_source) { + types = intersection(types, clean_types.from_source); + } + + return types; } if (clean_types.from_address_parser) { diff --git a/test/unit/helper/types.js b/test/unit/helper/types.js index d56f12d2..2398c6bb 100644 --- a/test/unit/helper/types.js +++ b/test/unit/helper/types.js @@ -55,6 +55,35 @@ module.exports.tests.layers_parameter_and_address_parser = function(test, common }); }; +module.exports.tests.source_parameter = function(test, common) { + test('source parameter specified', function(t) { + var cleaned_types = { + from_source: ['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_source: ['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) { From 6f0ad678eb91b9c089d71f67bf6a8cf4a1abf39e Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Fri, 4 Sep 2015 15:54:25 -0400 Subject: [PATCH 15/21] Add types middleware This middleware looks at the list of types that will be sent to Elasticsearch, if it's an empty array, it sends an error response before Elasticsearch is even quieried, because Elasticsearch interprets an empty type array as "search anything" rather than the intended "don't search anything". --- controller/search.js | 9 +++------ middleware/_types.js | 27 +++++++++++++++++++++++++++ routes/v1.js | 6 ++++++ sanitiser/search.js | 1 + 4 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 middleware/_types.js diff --git a/controller/search.js b/controller/search.js index c353c5be..d70c1b5c 100644 --- a/controller/search.js +++ b/controller/search.js @@ -1,5 +1,4 @@ var service = { search: require('../service/search') }; -var types = require ( '../helper/types' ); function setup( backend, query ){ @@ -16,11 +15,9 @@ function setup( backend, query ){ body: query( req.clean ) }; - // don't directly set cmd.type from types helper to avoid sometimes - // setting cmd.type to undefined (having the key not set is cleaner) - var type = types(req.clean.types); - if (type !== undefined) { - cmd.type = type; + if (req.clean.type !== undefined) { + cmd.type = req.clean.type; + delete req.clean.type; // remove type from clean to avoid clutter } // query backend diff --git a/middleware/_types.js b/middleware/_types.js new file mode 100644 index 00000000..8b12719f --- /dev/null +++ b/middleware/_types.js @@ -0,0 +1,27 @@ +var types_helper = require( '../helper/types' ); + +/** + * Validate the types specified to be searched. + * + * 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. + */ +function middleware(req, res, next) { + var types = types_helper(req.clean.types); + + if (types !== undefined && types.length !== undefined) { + if (types.length === 0) { + var err = 'You have specified both the `source` and `layers` ' + + 'parameters in a combination that will return no results.'; + res.status(400); // 400 Bad Request + return next(err); + } else { + req.clean.type = types; + } + } + + next(); +} + +module.exports = middleware; diff --git a/routes/v1.js b/routes/v1.js index cb3b53e2..d5a1c183 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -8,6 +8,11 @@ var sanitisers = { reverse: require('../sanitiser/reverse') }; +/** ----------------------- middleware ------------------------ **/ +var middleware = { + types: require('../middleware/_types') +}; + /** ----------------------- controllers ----------------------- **/ var controllers = { @@ -40,6 +45,7 @@ function addRoutes(app, peliasConfig) { ]), search: createRouter([ sanitisers.search.middleware, + middleware.types, controllers.search(), postProc.renamePlacenames(), postProc.geocodeJSON(peliasConfig), diff --git a/sanitiser/search.js b/sanitiser/search.js index d226dbd9..17409587 100644 --- a/sanitiser/search.js +++ b/sanitiser/search.js @@ -4,6 +4,7 @@ var _sanitize = require('../sanitiser/_sanitize'), input: require('../sanitiser/_input'), size: require('../sanitiser/_size'), layers: require('../sanitiser/_layers'), + source: require('../sanitiser/_source'), details: require('../sanitiser/_details'), latlonzoom: require('../sanitiser/_geo') }; From f5e182c63b41cb62d56b6a38d4cb06175abb91fa Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Tue, 8 Sep 2015 19:11:45 +0200 Subject: [PATCH 16/21] fix tests, clean by moving fixtures to fixtures dir --- test/unit/fixture/search_full_address.js | 206 +++++ test/unit/fixture/search_linguistic_bbox.js | 50 ++ test/unit/fixture/search_linguistic_focus.js | 68 ++ .../fixture/search_linguistic_focus_bbox.js | 79 ++ test/unit/fixture/search_linguistic_only.js | 39 + test/unit/fixture/search_partial_address.js | 98 +++ test/unit/fixture/search_regions_address.js | 190 +++++ test/unit/fixture/sort_default.js | 59 ++ test/unit/query/search.js | 724 +----------------- 9 files changed, 810 insertions(+), 703 deletions(-) create mode 100644 test/unit/fixture/search_full_address.js create mode 100644 test/unit/fixture/search_linguistic_bbox.js create mode 100644 test/unit/fixture/search_linguistic_focus.js create mode 100644 test/unit/fixture/search_linguistic_focus_bbox.js create mode 100644 test/unit/fixture/search_linguistic_only.js create mode 100644 test/unit/fixture/search_partial_address.js create mode 100644 test/unit/fixture/search_regions_address.js create mode 100644 test/unit/fixture/sort_default.js diff --git a/test/unit/fixture/search_full_address.js b/test/unit/fixture/search_full_address.js new file mode 100644 index 00000000..144dd0ca --- /dev/null +++ b/test/unit/fixture/search_full_address.js @@ -0,0 +1,206 @@ + +var peliasQuery = require('pelias-query'), + vs = new peliasQuery.Vars( peliasQuery.defaults ); + +module.exports = { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'must': [{ + 'match': { + 'name.default': { + 'query': '123 main st', + 'analyzer': 'peliasOneEdgeGram', + 'boost': 1 + } + } + }], + 'should': [{ + 'match': { + 'phrase.default': { + 'query': '123 main st', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2, + 'boost': 1 + } + } + },{ + 'match': { + 'address.number': { + 'query': 123, + 'boost': vs.var('address:housenumber:boost').get(), + 'analyzer': vs.var('address:housenumber:analyzer').get() + } + } + }, { + 'match': { + 'address.street': { + 'query': 'main st', + 'boost': vs.var('address:street:boost').get(), + 'analyzer': vs.var('address:street:analyzer').get() + } + } + }, { + 'match': { + 'address.zip': { + 'query': 10010, + 'boost': vs.var('address:postcode:boost').get(), + 'analyzer': vs.var('address:postcode:analyzer').get() + } + } + }, { + 'match': { + 'alpha3': { + 'query': 'USA', + 'boost': vs.var('admin:alpha3:boost').get(), + 'analyzer': vs.var('admin:alpha3:analyzer').get() + } + } + }, { + 'match': { + 'admin0': { + 'query': 'new york', + 'boost': vs.var('admin:admin0:boost').get(), + 'analyzer': vs.var('admin:admin0:analyzer').get() + } + } + }, { + 'match': { + 'admin1': { + 'query': 'new york', + 'boost': vs.var('admin:admin1:boost').get(), + 'analyzer': vs.var('admin:admin1:analyzer').get() + } + } + }, { + 'match': { + 'admin1_abbr': { + 'query': 'NY', + 'boost': vs.var('admin:admin1_abbr:boost').get(), + 'analyzer': vs.var('admin:admin1_abbr:analyzer').get() + } + } + }, { + 'match': { + 'admin2': { + 'query': 'new york', + 'boost': vs.var('admin:admin2:boost').get(), + 'analyzer': vs.var('admin:admin2:analyzer').get() + } + } + }, { + 'match': { + 'local_admin': { + 'query': 'new york', + 'boost': vs.var('admin:local_admin:boost').get(), + 'analyzer': vs.var('admin:local_admin:analyzer').get() + } + } + }, { + 'match': { + 'locality': { + 'query': 'new york', + 'boost': vs.var('admin:locality:boost').get(), + 'analyzer': vs.var('admin:locality:analyzer').get() + } + } + }, { + 'match': { + 'neighborhood': { + 'query': 'new york', + 'boost': vs.var('admin:neighborhood:boost').get(), + 'analyzer': vs.var('admin:neighborhood:analyzer').get() + } + } + }] + } + }, + 'filter': { + 'bool': { + 'must': [] + } + } + } + }, + 'size': 10, + 'sort': [ + '_score', + { + '_script': { + 'file': 'admin_boost', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'file': 'popularity', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'file': 'population', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'params': { + 'weights': { + 'admin0': 4, + 'admin1': 3, + 'admin2': 2, + 'local_admin': 1, + 'locality': 1, + 'neighborhood': 1 + } + }, + 'file': 'weights', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'params': { + 'category_weights': { + 'transport:air': 2, + 'transport:air:aerodrome': 2, + 'transport:air:airport': 2 + } + }, + 'file': 'category', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'params': { + 'weights': { + 'geoname': 0, + 'address': 4, + 'osmnode': 6, + 'osmway': 6, + 'poi-address': 8, + 'neighborhood': 10, + 'local_admin': 12, + 'locality': 12, + 'admin2': 12, + 'admin1': 14, + 'admin0': 2 + } + }, + 'file': 'weights', + 'type': 'number', + 'order': 'desc' + } + } + ], + 'track_scores': true +}; \ No newline at end of file diff --git a/test/unit/fixture/search_linguistic_bbox.js b/test/unit/fixture/search_linguistic_bbox.js new file mode 100644 index 00000000..69dfe4bb --- /dev/null +++ b/test/unit/fixture/search_linguistic_bbox.js @@ -0,0 +1,50 @@ + +module.exports = { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'must': [{ + 'match': { + 'name.default': { + 'query': 'test', + 'boost': 1, + 'analyzer': 'peliasOneEdgeGram' + } + } + }], + 'should': [{ + 'match': { + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'boost': 1, + 'slop': 2 + } + } + }] + } + }, + 'filter': { + 'bool': { + 'must': [{ + 'geo_bounding_box': { + 'center_point': { + 'top': 47.47, + 'right': -61.84, + 'bottom': 11.51, + 'left': -103.16 + }, + '_cache': true, + 'type': 'indexed' + } + }] + } + } + } + }, + 'sort': [ '_sort' ], + 'size': 10, + 'track_scores': true +}; \ No newline at end of file diff --git a/test/unit/fixture/search_linguistic_focus.js b/test/unit/fixture/search_linguistic_focus.js new file mode 100644 index 00000000..cceb87fd --- /dev/null +++ b/test/unit/fixture/search_linguistic_focus.js @@ -0,0 +1,68 @@ + +module.exports = { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'must': [{ + 'match': { + 'name.default': { + 'query': 'test', + 'boost': 1, + 'analyzer': 'peliasOneEdgeGram' + } + } + }], + 'should': [{ + 'match': { + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'boost': 1, + 'slop': 2 + } + } + }, { + 'function_score': { + 'query': { + 'match': { + 'phrase.default': { + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'boost': 1, + 'slop': 2, + 'query': 'test' + } + } + }, + 'functions': [{ + 'linear': { + 'center_point': { + 'origin': { + 'lat': 29.49136, + 'lon': -82.50622 + }, + 'offset': '1km', + 'scale': '50km', + 'decay': 0.5 + } + } + }], + 'score_mode': 'avg', + 'boost_mode': 'replace' + } + }] + } + }, + 'filter': { + 'bool': { + 'must': [] + } + } + } + }, + 'sort': [ '_sort' ], + 'size': 10, + 'track_scores': true +}; \ No newline at end of file diff --git a/test/unit/fixture/search_linguistic_focus_bbox.js b/test/unit/fixture/search_linguistic_focus_bbox.js new file mode 100644 index 00000000..d7aa6544 --- /dev/null +++ b/test/unit/fixture/search_linguistic_focus_bbox.js @@ -0,0 +1,79 @@ + +module.exports = { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'must': [{ + 'match': { + 'name.default': { + 'query': 'test', + 'boost': 1, + 'analyzer': 'peliasOneEdgeGram' + } + } + }], + 'should': [{ + 'match': { + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'boost': 1, + 'slop': 2 + } + } + }, { + 'function_score': { + 'query': { + 'match': { + 'phrase.default': { + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'boost': 1, + 'slop': 2, + 'query': 'test' + } + } + }, + 'functions': [{ + 'linear': { + 'center_point': { + 'origin': { + 'lat': 29.49136, + 'lon': -82.50622 + }, + 'offset': '1km', + 'scale': '50km', + 'decay': 0.5 + } + } + }], + 'score_mode': 'avg', + 'boost_mode': 'replace' + } + }] + } + }, + 'filter': { + 'bool': { + 'must': [{ + 'geo_bounding_box': { + 'center_point': { + 'top': 47.47, + 'right': -61.84, + 'bottom': 11.51, + 'left': -103.16 + }, + '_cache': true, + 'type': 'indexed' + } + }] + } + } + } + }, + 'sort': [ '_sort' ], + 'size': 10, + 'track_scores': true +}; \ No newline at end of file diff --git a/test/unit/fixture/search_linguistic_only.js b/test/unit/fixture/search_linguistic_only.js new file mode 100644 index 00000000..a680d3bc --- /dev/null +++ b/test/unit/fixture/search_linguistic_only.js @@ -0,0 +1,39 @@ + +module.exports = { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'must': [{ + 'match': { + 'name.default': { + 'query': 'test', + 'boost': 1, + 'analyzer': 'peliasOneEdgeGram' + } + } + }], + 'should': [{ + 'match': { + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'boost': 1, + 'slop': 2 + } + } + }] + } + }, + 'filter': { + 'bool': { + 'must': [] + } + } + } + }, + 'sort': [ '_score' ], + 'size': 10, + 'track_scores': true +}; \ No newline at end of file diff --git a/test/unit/fixture/search_partial_address.js b/test/unit/fixture/search_partial_address.js new file mode 100644 index 00000000..eed82e7a --- /dev/null +++ b/test/unit/fixture/search_partial_address.js @@ -0,0 +1,98 @@ + +var peliasQuery = require('pelias-query'), + vs = new peliasQuery.Vars( peliasQuery.defaults ); + +module.exports = { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'must': [{ + 'match': { + 'name.default': { + 'query': 'soho grand', + 'analyzer': 'peliasOneEdgeGram', + 'boost': 1 + } + } + }], + 'should': [{ + 'match': { + 'phrase.default': { + 'query': 'soho grand', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2, + 'boost': 1 + } + } + },{ + 'match': { + 'admin0': { + 'query': 'new york', + 'boost': vs.var('admin:admin0:boost').get(), + 'analyzer': vs.var('admin:admin0:analyzer').get() + } + } + }, { + 'match': { + 'admin1': { + 'query': 'new york', + 'boost': vs.var('admin:admin1:boost').get(), + 'analyzer': vs.var('admin:admin1:analyzer').get() + } + } + }, { + 'match': { + 'admin1_abbr': { + 'query': 'new york', + 'boost': vs.var('admin:admin1_abbr:boost').get(), + 'analyzer': vs.var('admin:admin1_abbr:analyzer').get() + } + } + }, { + 'match': { + 'admin2': { + 'query': 'new york', + 'boost': vs.var('admin:admin2:boost').get(), + 'analyzer': vs.var('admin:admin2:analyzer').get() + } + } + }, { + 'match': { + 'local_admin': { + 'query': 'new york', + 'boost': vs.var('admin:local_admin:boost').get(), + 'analyzer': vs.var('admin:local_admin:analyzer').get() + } + } + }, { + 'match': { + 'locality': { + 'query': 'new york', + 'boost': vs.var('admin:locality:boost').get(), + 'analyzer': vs.var('admin:locality:analyzer').get() + } + } + }, { + 'match': { + 'neighborhood': { + 'query': 'new york', + 'boost': vs.var('admin:neighborhood:boost').get(), + 'analyzer': vs.var('admin:neighborhood:analyzer').get() + } + } + }] + } + }, + 'filter': { + 'bool': { + 'must': [] + } + } + } + }, + 'size': 10, + 'sort': [ '_score' ], + 'track_scores': true +}; \ No newline at end of file diff --git a/test/unit/fixture/search_regions_address.js b/test/unit/fixture/search_regions_address.js new file mode 100644 index 00000000..b0ec4208 --- /dev/null +++ b/test/unit/fixture/search_regions_address.js @@ -0,0 +1,190 @@ + +var peliasQuery = require('pelias-query'), + vs = new peliasQuery.Vars( peliasQuery.defaults ); + +module.exports = { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'must': [{ + 'match': { + 'name.default': { + 'query': '1 water st', + 'analyzer': 'peliasOneEdgeGram', + 'boost': 1 + } + } + }], + 'should': [{ + 'match': { + 'phrase.default': { + 'query': '1 water st', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2, + 'boost': 1 + } + } + },{ + 'match': { + 'address.number': { + 'query': 1, + 'boost': vs.var('address:housenumber:boost').get(), + 'analyzer': vs.var('address:housenumber:analyzer').get() + } + } + }, { + 'match': { + 'address.street': { + 'query': 'water st', + 'boost': vs.var('address:street:boost').get(), + 'analyzer': vs.var('address:street:analyzer').get() + } + } + }, { + 'match': { + 'admin0': { + 'query': 'manhattan', + 'boost': vs.var('admin:admin0:boost').get(), + 'analyzer': vs.var('admin:admin0:analyzer').get() + } + } + }, { + 'match': { + 'admin1': { + 'query': 'manhattan', + 'boost': vs.var('admin:admin1:boost').get(), + 'analyzer': vs.var('admin:admin1:analyzer').get() + } + } + }, { + 'match': { + 'admin1_abbr': { + 'query': 'NY', + 'boost': vs.var('admin:admin1_abbr:boost').get(), + 'analyzer': vs.var('admin:admin1_abbr:analyzer').get() + } + } + }, { + 'match': { + 'admin2': { + 'query': 'manhattan', + 'boost': vs.var('admin:admin2:boost').get(), + 'analyzer': vs.var('admin:admin2:analyzer').get() + } + } + }, { + 'match': { + 'local_admin': { + 'query': 'manhattan', + 'boost': vs.var('admin:local_admin:boost').get(), + 'analyzer': vs.var('admin:local_admin:analyzer').get() + } + } + }, { + 'match': { + 'locality': { + 'query': 'manhattan', + 'boost': vs.var('admin:locality:boost').get(), + 'analyzer': vs.var('admin:locality:analyzer').get() + } + } + }, { + 'match': { + 'neighborhood': { + 'query': 'manhattan', + 'boost': vs.var('admin:neighborhood:boost').get(), + 'analyzer': vs.var('admin:neighborhood:analyzer').get() + } + } + }] + } + }, + 'filter': { + 'bool': { + 'must': [] + } + } + } + }, + 'size': 10, + 'sort': [ + '_score', + { + '_script': { + 'file': 'admin_boost', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'file': 'popularity', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'file': 'population', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'params': { + 'weights': { + 'admin0': 4, + 'admin1': 3, + 'admin2': 2, + 'local_admin': 1, + 'locality': 1, + 'neighborhood': 1 + } + }, + 'file': 'weights', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'params': { + 'category_weights': { + 'transport:air': 2, + 'transport:air:aerodrome': 2, + 'transport:air:airport': 2 + } + }, + 'file': 'category', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'params': { + 'weights': { + 'geoname': 0, + 'address': 4, + 'osmnode': 6, + 'osmway': 6, + 'poi-address': 8, + 'neighborhood': 10, + 'local_admin': 12, + 'locality': 12, + 'admin2': 12, + 'admin1': 14, + 'admin0': 2 + } + }, + 'file': 'weights', + 'type': 'number', + 'order': 'desc' + } + } + ], + 'track_scores': true +}; \ No newline at end of file diff --git a/test/unit/fixture/sort_default.js b/test/unit/fixture/sort_default.js new file mode 100644 index 00000000..255e3b29 --- /dev/null +++ b/test/unit/fixture/sort_default.js @@ -0,0 +1,59 @@ + +var category_weights = require('../../../helper/category_weights'); +var admin_weights = require('../../../helper/admin_weights'); +var weights = require('pelias-suggester-pipeline').weights; + +module.exports = [ + '_score', + { + '_script': { + 'file': 'admin_boost', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'file': 'popularity', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'file': 'population', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'params': { + 'weights': admin_weights + }, + 'file': 'weights', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'params': { + 'category_weights': category_weights.default + }, + 'file': 'category', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'params': { + 'weights': weights + }, + 'file': 'weights', + 'type': 'number', + 'order': 'desc' + } + } +]; \ No newline at end of file diff --git a/test/unit/query/search.js b/test/unit/query/search.js index ff678401..355c2043 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -20,113 +20,10 @@ module.exports.tests.interface = function(test, common) { }); }; -var sort = [ - '_score', - { - '_script': { - 'file': admin_boost, - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'file': popularity, - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'file': population, - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'weights': admin_weights - }, - 'file': 'weights', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'category_weights': category_weights.default - }, - 'file': category, - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'weights': weights - }, - 'file': 'weights', - 'type': 'number', - 'order': 'desc' - } - } -]; - -var expected = { - 'query': { - 'filtered': { - 'query': { - 'bool': { - 'must': [{ - 'match': { - 'name.default': { - 'query': 'test', - 'analyzer': 'peliasOneEdgeGram' - } - } - }], - 'should': [{ - 'match': { - 'phrase.default': { - 'query': 'test', - 'analyzer': 'peliasPhrase', - 'type': 'phrase', - 'slop': 2 - } - } - }] - } - }, - 'filter': { - 'bool': { - 'must': [ - { - 'geo_bounding_box': { - 'center_point': { - 'top': '47.47', - 'right': '-61.84', - 'bottom':'11.51', - 'left': '-103.16' - }, - '_cache': true, - 'type': 'indexed' - } - } - ] - } - } - } - }, - 'sort': sort, - 'size': 10, - 'track_scores': true -}; +var sort = require('../fixture/sort_default'); module.exports.tests.query = function(test, common) { - test('valid query', function(t) { + test('valid search + focus + bbox', function(t) { var query = generate({ input: 'test', size: 10, lat: 29.49136, lon: -82.50622, @@ -139,11 +36,14 @@ module.exports.tests.query = function(test, common) { layers: ['test'] }); + var expected = require('../fixture/search_linguistic_focus_bbox'); + expected.sort = sort; + t.deepEqual(query, expected, 'valid search query'); t.end(); }); - test('valid query without lat/lon', function(t) { + test('valid search + bbox', function(t) { var query = generate({ input: 'test', size: 10, bbox: { @@ -154,114 +54,36 @@ module.exports.tests.query = function(test, common) { }, layers: ['test'] }); + + var expected = require('../fixture/search_linguistic_bbox'); + expected.sort = sort; t.deepEqual(query, expected, 'valid search query'); t.end(); }); - test('valid query with no lat/lon and no bbox', function(t) { + test('valid lingustic-only search', function(t) { var query = generate({ input: 'test', size: 10, layers: ['test'] }); - var expected = { - 'query': { - 'filtered': { - 'query': { - 'bool': { - 'must': [{ - 'match': { - 'name.default': { - 'query': 'test', - 'analyzer': 'peliasOneEdgeGram' - } - } - }], - 'should': [{ - 'match': { - 'phrase.default': { - 'query': 'test', - 'analyzer': 'peliasPhrase', - 'type': 'phrase', - 'slop': 2 - } - } - }] - } - }, - 'filter': { - 'bool': { - 'must': [] - } - } - } - }, - 'size': 10, - 'sort': sort, - 'track_scores': true - }; - + var expected = require('../fixture/search_linguistic_only'); + expected.sort = sort; + t.deepEqual(query, expected, 'valid search query'); t.end(); }); - test('valid query without bbox', function(t) { + test('search search + focus', function(t) { var query = generate({ input: 'test', size: 10, lat: 29.49136, lon: -82.50622, layers: ['test'] }); - var expected = { - 'query': { - 'filtered': { - 'query': { - 'bool': { - 'must': [{ - 'match': { - 'name.default': { - 'query': 'test', - 'analyzer': 'peliasOneEdgeGram' - } - } - }], - 'should': [{ - 'match': { - 'phrase.default': { - 'query': 'test', - 'analyzer': 'peliasPhrase', - 'type': 'phrase', - 'slop': 2 - } - } - }] - } - }, - 'filter': { - 'bool': { - 'must': [ - { - 'geo_distance': { - 'distance': '50km', - 'distance_type': 'plane', - 'optimize_bbox': 'indexed', - '_cache': true, - 'center_point': { - 'lat': '29.49', - 'lon': '-82.51' - } - } - } - ] - } - } - } - }, - 'sort': ['_score'].concat(sort.slice(1)), - 'size': 10, - 'track_scores': true - }; + var expected = require('../fixture/search_linguistic_focus'); + expected.sort = sort; t.deepEqual(query, expected, 'valid search query'); t.end(); @@ -277,193 +99,8 @@ module.exports.tests.query = function(test, common) { parsed_input: parser.get_parsed_address(address), }); - var expected = { - 'query': { - 'filtered': { - 'query': { - 'bool': { - 'must': [ - { - 'match': { - 'name.default': { - 'query': '123 main st', - 'analyzer': 'peliasOneEdgeGram' - } - } - } - ], - 'should': [ - { - 'match': { - 'address.number': { - 'query': 123, - 'boost': address_weights.number - } - } - }, - { - 'match': { - 'address.street': { - 'query': 'main st', - 'boost': address_weights.street - } - } - }, - { - 'match': { - 'address.zip': { - 'query': 10010, - 'boost': address_weights.zip - } - } - }, - { - 'match': { - 'admin1_abbr': { - 'query': 'NY', - 'boost': address_weights.admin1_abbr - } - } - }, - { - 'match': { - 'alpha3': { - 'query': 'USA', - 'boost': address_weights.alpha3 - } - } - }, - { - match: { - admin0: 'new york' - } - }, - { - match: { - admin1: 'new york' - } - }, - { - match: { - admin2: 'new york' - } - }, - { - match: { - local_admin: 'new york' - } - }, - { - match: { - locality: 'new york' - } - }, - { - match: { - neighborhood: 'new york' - } - }, - { - 'match': { - 'phrase.default': { - 'query': '123 main st', - 'analyzer': 'peliasPhrase', - 'type': 'phrase', - 'slop': 2 - } - } - } - ] - } - }, - 'filter': { - 'bool': { - 'must': [] - } - } - } - }, - 'size': 10, - 'sort': [ - '_score', - { - '_script': { - 'file': 'admin_boost', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'file': 'popularity', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'file': 'population', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'weights': { - 'admin0': 4, - 'admin1': 3, - 'admin2': 2, - 'local_admin': 1, - 'locality': 1, - 'neighborhood': 1 - } - }, - 'file': 'weights', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'category_weights': { - 'transport:air': 2, - 'transport:air:aerodrome': 2, - 'transport:air:airport': 2 - } - }, - 'file': 'category', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'weights': { - 'geoname': 0, - 'address': 4, - 'osmnode': 6, - 'osmway': 6, - 'poi-address': 8, - 'neighborhood': 10, - 'local_admin': 12, - 'locality': 12, - 'admin2': 12, - 'admin1': 14, - 'admin0': 2 - } - }, - 'file': 'weights', - 'type': 'number', - 'order': 'desc' - } - } - ], - 'track_scores': true - }; - + var expected = require('../fixture/search_full_address'); + t.deepEqual(query, expected, 'valid search query'); t.end(); }); @@ -478,158 +115,8 @@ module.exports.tests.query = function(test, common) { parsed_input: parser.get_parsed_address(partial_address), }); - var expected = { - 'query': { - 'filtered': { - 'query': { - 'bool': { - 'must': [ - { - 'match': { - 'name.default': { - 'query': 'soho grand', - 'analyzer': 'peliasOneEdgeGram' - } - } - } - ], - 'should': [ - { - 'match': { - 'admin0': 'new york' - } - }, - { - 'match': { - 'admin1': 'new york' - } - }, - { - 'match': { - 'admin1_abbr': 'new york' - } - }, - { - 'match': { - 'admin2': 'new york' - } - }, - { - 'match': { - 'local_admin': 'new york' - } - }, - { - 'match': { - 'locality': 'new york' - } - }, - { - 'match': { - 'neighborhood': 'new york' - } - }, - { - 'match': { - 'phrase.default': { - 'query': 'soho grand', - 'analyzer': 'peliasPhrase', - 'type': 'phrase', - 'slop': 2 - } - } - } - ] - } - }, - 'filter': { - 'bool': { - 'must': [] - } - } - } - }, - 'size': 10, - 'sort': [ - '_score', - { - '_script': { - 'file': 'admin_boost', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'file': 'popularity', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'file': 'population', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'weights': { - 'admin0': 4, - 'admin1': 3, - 'admin2': 2, - 'local_admin': 1, - 'locality': 1, - 'neighborhood': 1 - } - }, - 'file': 'weights', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'category_weights': { - 'transport:air': 2, - 'transport:air:aerodrome': 2, - 'transport:air:airport': 2, - 'admin': 2 - } - }, - 'file': 'category', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'weights': { - 'geoname': 0, - 'address': 4, - 'osmnode': 6, - 'osmway': 6, - 'poi-address': 8, - 'neighborhood': 10, - 'local_admin': 12, - 'locality': 12, - 'admin2': 12, - 'admin1': 14, - 'admin0': 2 - } - }, - 'file': 'weights', - 'type': 'number', - 'order': 'desc' - } - } - ], - 'track_scores': true - }; + var expected = require('../fixture/search_partial_address'); + expected.sort = sort; t.deepEqual(query, expected, 'valid search query'); t.end(); @@ -645,176 +132,7 @@ module.exports.tests.query = function(test, common) { parsed_input: parser.get_parsed_address(partial_address), }); - var expected = { - 'query': { - 'filtered': { - 'query': { - 'bool': { - 'must': [ - { - 'match': { - 'name.default': { - 'query': '1 water st', - 'analyzer': 'peliasOneEdgeGram' - } - } - } - ], - 'should': [ - { - 'match': { - 'address.number': { - 'query': 1, - 'boost': address_weights.number - } - } - }, - { - 'match': { - 'address.street': { - 'query': 'water st', - 'boost': address_weights.street - } - } - }, - { - 'match': { - 'admin1_abbr': { - 'query': 'NY', - 'boost': address_weights.admin1_abbr - } - } - }, - { - 'match': { - 'admin0': 'manhattan' - } - }, - { - 'match': { - 'admin1': 'manhattan' - } - }, - { - 'match': { - 'admin2': 'manhattan' - } - }, - { - 'match': { - 'local_admin': 'manhattan' - } - }, - { - 'match': { - 'locality': 'manhattan' - } - }, - { - 'match': { - 'neighborhood': 'manhattan' - } - }, - { - 'match': { - 'phrase.default': { - 'query': '1 water st', - 'analyzer': 'peliasPhrase', - 'type': 'phrase', - 'slop': 2 - } - } - } - ] - } - }, - 'filter': { - 'bool': { - 'must': [] - } - } - } - }, - 'size': 10, - 'sort': [ - '_score', - { - '_script': { - 'file': 'admin_boost', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'file': 'popularity', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'file': 'population', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'weights': { - 'admin0': 4, - 'admin1': 3, - 'admin2': 2, - 'local_admin': 1, - 'locality': 1, - 'neighborhood': 1 - } - }, - 'file': 'weights', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'category_weights': { - 'transport:air': 2, - 'transport:air:aerodrome': 2, - 'transport:air:airport': 2 - } - }, - 'file': 'category', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'weights': { - 'geoname': 0, - 'address': 4, - 'osmnode': 6, - 'osmway': 6, - 'poi-address': 8, - 'neighborhood': 10, - 'local_admin': 12, - 'locality': 12, - 'admin2': 12, - 'admin1': 14, - 'admin0': 2 - } - }, - 'file': 'weights', - 'type': 'number', - 'order': 'desc' - } - } - ], - 'track_scores': true - }; + var expected = require('../fixture/search_regions_address'); t.deepEqual(query, expected, 'valid search query'); t.end(); From 6f817938bce0e675961738082d36f611236968b7 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Tue, 8 Sep 2015 19:17:02 +0200 Subject: [PATCH 17/21] refactor search --- query/search.js | 287 ++++++++++++++++++++++-------------------------- 1 file changed, 130 insertions(+), 157 deletions(-) diff --git a/query/search.js b/query/search.js index bb56197f..7a0a3b96 100644 --- a/query/search.js +++ b/query/search.js @@ -1,188 +1,161 @@ -var queries = require('geopipes-elasticsearch-backend').queries, +var peliasQuery = require('pelias-query'), sort = require('../query/sort'), - adminFields = require('../helper/adminFields')(), - addressWeights = require('../helper/address_weights'); + adminFields = require('../helper/adminFields')(); +//------------------------------ +// general-purpose search query +//------------------------------ -function generate( params ){ - var centroid = null; +var query = new peliasQuery.layout.FilteredBooleanQuery(); - if ( params.lat && params.lon ){ - centroid = { - lat: params.lat, - lon: params.lon - }; - } - - var query = queries.distance( centroid, { size: params.size } ); - var input = params.input; +// mandatory matches +query.score( peliasQuery.view.boundary_country, 'must' ); +query.score( peliasQuery.view.ngrams, 'must' ); - if (params.bbox) { - query = queries.bbox ( centroid, { size: params.size, bbox: params.bbox } ); +// scoring boost +query.score( peliasQuery.view.phrase ); +query.score( peliasQuery.view.focus ); + +// address components +query.score( peliasQuery.view.address('housenumber') ); +query.score( peliasQuery.view.address('street') ); +query.score( peliasQuery.view.address('postcode') ); + +// admin components +query.score( peliasQuery.view.admin('alpha3') ); +query.score( peliasQuery.view.admin('admin0') ); +query.score( peliasQuery.view.admin('admin1') ); +query.score( peliasQuery.view.admin('admin1_abbr') ); +query.score( peliasQuery.view.admin('admin2') ); +query.score( peliasQuery.view.admin('local_admin') ); +query.score( peliasQuery.view.admin('locality') ); +query.score( peliasQuery.view.admin('neighborhood') ); + +// non-scoring hard filters +query.filter( peliasQuery.view.boundary_circle, 'must' ); +query.filter( peliasQuery.view.boundary_rect, 'must' ); + +// -------------------------------- + +function generate( clean ){ + + var vs = new peliasQuery.Vars( peliasQuery.defaults ); + + // set input text + vs.var( 'input:name', clean.input ); + + // set size + if( clean.size ){ + vs.var( 'size', clean.size ); } - query.query.filtered.query = { - 'bool': { - 'must': [], - 'should': [] - } - }; - - if (params.parsed_input) { - // update input - if (params.parsed_input.number && params.parsed_input.street) { - input = params.parsed_input.number + ' ' + params.parsed_input.street; - } else if (params.parsed_input.admin_parts) { - input = params.parsed_input.name; - } + // focus point + if( clean.lat && clean.lon ){ + vs.set({ + 'focus:point:lat': clean.lat, + 'focus:point:lon': clean.lon + }); + } - addParsedMatch(query, input, params.parsed_input); + // bbox + if( clean.bbox ){ + vs.set({ + 'boundary:rect:top': clean.bbox.top, + 'boundary:rect:right': clean.bbox.right, + 'boundary:rect:bottom': clean.bbox.bottom, + 'boundary:rect:left': clean.bbox.left + }); } - // add search condition to distance query - query.query.filtered.query.bool.must.push({ - 'match': { - 'name.default': { - 'query': input, - 'analyzer': 'peliasOneEdgeGram' - } + // address parsing + if( clean.parsed_input ){ + + // is it a street address? + var isStreetAddress = clean.parsed_input.hasOwnProperty('number') && clean.parsed_input.hasOwnProperty('street'); + if( isStreetAddress ){ + vs.var( 'input:name', clean.parsed_input.number + ' ' + clean.parsed_input.street ); } - }); - - // add phrase matching query - // note: this is required for shingle/phrase matching - query.query.filtered.query.bool.should.push({ - 'match': { - 'phrase.default': { - 'query': input, - 'analyzer': 'peliasPhrase', - 'type': 'phrase', - 'slop': 2 - } + + // I don't understand this + else if( clean.parsed_input.admin_parts ) { + vs.var( 'input:name', clean.parsed_input.name ); } - }); - query.sort = query.sort.concat( sort( params ) ); + // or this.. + else { + console.warn( 'chaos monkey asks: what happens now?' ); + console.log( clean ); + try{ throw new Error(); } catch(e){ console.error( e.stack ); } // print a stack trace + } - return query; -} + // ==== add parsed matches [address components] ==== -/** - * Traverse the parsed input object, containing all the address parts detected in query string. - * Add matches to query for each identifiable component. - * - * @param {Object} query - * @param {string} defaultInput - * @param {Object} parsedInput - */ -function addParsedMatch(query, defaultInput, parsedInput) { - query.query.filtered.query.bool.should = query.query.filtered.query.bool.should || []; - - // copy expected admin fields so we can remove them as we parse the address - var unmatchedAdminFields = adminFields.slice(); - - // address - // number, street, postalcode - addMatch(query, unmatchedAdminFields, 'address.number', parsedInput.number, addressWeights.number); - addMatch(query, unmatchedAdminFields, 'address.street', parsedInput.street, addressWeights.street); - addMatch(query, unmatchedAdminFields, 'address.zip', parsedInput.postalcode, addressWeights.zip); - - // city - // admin2, locality, local_admin, neighborhood - addMatch(query, unmatchedAdminFields, 'admin2', parsedInput.city, addressWeights.admin2); - - // state - // admin1, admin1_abbr - addMatch(query, unmatchedAdminFields, 'admin1_abbr', parsedInput.state, addressWeights.admin1_abbr); - - // country - // admin0, alpha3 - addMatch(query, unmatchedAdminFields, 'alpha3', parsedInput.country, addressWeights.alpha3); - - addUnmatchedAdminFieldsToQuery(query, unmatchedAdminFields, parsedInput, defaultInput); -} + // house number + if( clean.parsed_input.hasOwnProperty('number') ){ + vs.var( 'input:housenumber', clean.parsed_input.number ); + } -/** - * Check for additional admin fields in the parsed input, and if any was found - * combine into single string and match against all unmatched admin fields. - * - * @param {Object} query - * @param {Array} unmatchedAdminFields - * @param {Object} parsedInput - * @param {string} defaultInput - */ -function addUnmatchedAdminFieldsToQuery(query, unmatchedAdminFields, parsedInput, defaultInput) { - if (unmatchedAdminFields.length === 0 ) { - return; - } + // street name + if( clean.parsed_input.hasOwnProperty('street') ){ + vs.var( 'input:street', clean.parsed_input.street ); + } - var leftovers = []; + // postal code + if( clean.parsed_input.hasOwnProperty('postalcode') ){ + vs.var( 'input:postcode', clean.parsed_input.postalcode ); + } - if (parsedInput.admin_parts) { - leftovers.push(parsedInput.admin_parts); - } - else if (parsedInput.regions) { - leftovers.push(parsedInput.regions); - } + // ==== add parsed matches [admin components] ==== - if (leftovers.length === 0) { - return; - } + // city + if( clean.parsed_input.hasOwnProperty('city') ){ + vs.var( 'input:admin2', clean.parsed_input.city ); + } - leftovers = leftovers.join(' '); + // state + if( clean.parsed_input.hasOwnProperty('state') ){ + vs.var( 'input:admin1_abbr', clean.parsed_input.state ); + } - // if there are additional regions/admin_parts found - if (leftovers !== defaultInput) { - unmatchedAdminFields.forEach(function (key) { - // combine all the leftover parts into one string - addMatch(query, [], key, leftovers); - }); - } -} + // country + if( clean.parsed_input.hasOwnProperty('country') ){ + vs.var( 'input:alpha3', clean.parsed_input.country ); + } -/** - * Add key:value match to query. Apply boost if specified. - * - * @param {Object} query - * @param {Array} unmatched - * @param {string} key - * @param {string|number|undefined} value - * @param {number|undefined} [boost] optional - */ -function addMatch(query, unmatched, key, value, boost) { // jshint ignore:line - if (typeof value === 'undefined') { - return; - } + // ==== deal with the 'leftover' components ==== + // @todo: clean up this code - var match = {}; + // a concept called 'leftovers' which is just 'admin_parts' plus 'regions'. + var leftovers = []; + if( clean.parsed_input.hasOwnProperty('admin_parts') ){ + leftovers.push( clean.parsed_input.admin_parts ); + } + else if( clean.parsed_input.hasOwnProperty('regions') ){ + leftovers.push( clean.parsed_input.regions ); + } - if (boost) { - match[key] = { - query: value, - boost: boost - }; - } - else { - match[key] = value; + // if we have 'leftovers' then assign them to any fields which + // currently don't have a value assigned. + if( leftovers.length ){ + var leftoversString = leftovers.join(' '); + var unmatchedAdminFields = adminFields.slice(); + + // cycle through fields and set fields which + // are still currently unset + unmatchedAdminFields.forEach( function( key ){ + if( !vs.isset( 'input:' + key ) ){ + vs.var( 'input:' + key, leftoversString ); + } + }); + } } - query.query.filtered.query.bool.should.push({ 'match': match }); - - removeFromUnmatched(unmatched, key); -} + var result = query.render( vs ); + result.sort = result.sort.concat( sort( clean ) ); -/** - * If key is found in unmatched list, remove it from the array - * - * @param {Array} unmatched - * @param {string} key - */ -function removeFromUnmatched(unmatched, key) { - var index = unmatched.indexOf(key); - if (index !== -1) { - unmatched.splice(index, 1); - } + // @todo: remove this hack + return JSON.parse( JSON.stringify( result ) ); } module.exports = generate; From 34cd25a023d6133fcdff731725b8b201c14eaf13 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Tue, 8 Sep 2015 19:46:03 +0200 Subject: [PATCH 18/21] refactor leftovers for clarity --- query/search.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/query/search.js b/query/search.js index 7a0a3b96..4ba311c9 100644 --- a/query/search.js +++ b/query/search.js @@ -6,7 +6,6 @@ var peliasQuery = require('pelias-query'), //------------------------------ // general-purpose search query //------------------------------ - var query = new peliasQuery.layout.FilteredBooleanQuery(); // mandatory matches @@ -127,18 +126,17 @@ function generate( clean ){ // @todo: clean up this code // a concept called 'leftovers' which is just 'admin_parts' plus 'regions'. - var leftovers = []; + var leftoversString = ''; if( clean.parsed_input.hasOwnProperty('admin_parts') ){ - leftovers.push( clean.parsed_input.admin_parts ); + leftoversString = clean.parsed_input.admin_parts; } else if( clean.parsed_input.hasOwnProperty('regions') ){ - leftovers.push( clean.parsed_input.regions ); + leftoversString = clean.parsed_input.regions.join(' '); } // if we have 'leftovers' then assign them to any fields which // currently don't have a value assigned. - if( leftovers.length ){ - var leftoversString = leftovers.join(' '); + if( leftoversString.length ){ var unmatchedAdminFields = adminFields.slice(); // cycle through fields and set fields which From dd715392545d81245785a8a5be3df9db71cc8f58 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Tue, 8 Sep 2015 19:48:30 +0200 Subject: [PATCH 19/21] add npm dependency --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 78304ebb..5c90c84e 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "pelias-config": "^1.0.1", "pelias-esclient": "0.0.25", "pelias-logger": "^0.0.8", + "pelias-query": "^1.1.0", "pelias-schema": "1.0.0", "pelias-suggester-pipeline": "2.0.2", "through2": "0.6.5" From 1f13bafab0ac8d935096a22dabc5484ca99855dc Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Wed, 9 Sep 2015 13:06:19 +0200 Subject: [PATCH 20/21] refactor reverse & tests --- query/reverse.js | 59 +++++--- query/search.js | 12 +- test/unit/fixture/reverse_standard.js | 43 ++++++ test/unit/query/reverse.js | 189 ++------------------------ 4 files changed, 104 insertions(+), 199 deletions(-) create mode 100644 test/unit/fixture/reverse_standard.js diff --git a/query/reverse.js b/query/reverse.js index fd835e8f..4e8b13c5 100644 --- a/query/reverse.js +++ b/query/reverse.js @@ -1,33 +1,52 @@ -var queries = require('geopipes-elasticsearch-backend').queries, +var peliasQuery = require('pelias-query'), sort = require('./sort'); -function generate( params ){ +//------------------------------ +// reverse geocode query +//------------------------------ +var query = new peliasQuery.layout.FilteredBooleanQuery(); - var centroid = { - lat: params.lat, - lon: params.lon - }; +// scoring boost +query.sort( peliasQuery.view.sort_distance ); - var query = queries.distance( centroid, { - size: params.size || 1, - sort: true, - distance: '500km' +// non-scoring hard filters +query.filter( peliasQuery.view.boundary_circle ); + +// -------------------------------- + +function generateQuery( clean ){ + + var vs = new peliasQuery.Vars( peliasQuery.defaults ); + + // set defaults + vs.set({ + 'size': 1, + 'boundary:circle:radius': '500km' }); - query.sort = query.sort.concat( sort( params ) ); + // set size + if( clean.size ){ + vs.var( 'size', clean.size ); + } - if ( params.categories && params.categories.length > 0 ) { - addCategoriesFilter( query, params.categories ); + // focus point centroid + if( clean.lat && clean.lon ){ + vs.set({ + // focus point to score by distance + 'focus:point:lat': clean.lat, + 'focus:point:lon': clean.lon, + + // bounding circle + 'boundary:circle:lat': clean.lat, + 'boundary:circle:lon': clean.lon, + }); } - return query; -} + var result = query.render( vs ); -function addCategoriesFilter( query, categories ) { - query.query.filtered.filter.bool.must.push({ - terms: { category: categories } - }); + // @todo: remove this hack + return JSON.parse( JSON.stringify( result ) ); } -module.exports = generate; +module.exports = generateQuery; diff --git a/query/search.js b/query/search.js index 4ba311c9..de523c20 100644 --- a/query/search.js +++ b/query/search.js @@ -32,12 +32,12 @@ query.score( peliasQuery.view.admin('locality') ); query.score( peliasQuery.view.admin('neighborhood') ); // non-scoring hard filters -query.filter( peliasQuery.view.boundary_circle, 'must' ); -query.filter( peliasQuery.view.boundary_rect, 'must' ); +query.filter( peliasQuery.view.boundary_circle ); +query.filter( peliasQuery.view.boundary_rect ); // -------------------------------- -function generate( clean ){ +function generateQuery( clean ){ var vs = new peliasQuery.Vars( peliasQuery.defaults ); @@ -125,7 +125,7 @@ function generate( clean ){ // ==== deal with the 'leftover' components ==== // @todo: clean up this code - // a concept called 'leftovers' which is just 'admin_parts' plus 'regions'. + // a concept called 'leftovers' which is just 'admin_parts' /or 'regions'. var leftoversString = ''; if( clean.parsed_input.hasOwnProperty('admin_parts') ){ leftoversString = clean.parsed_input.admin_parts; @@ -150,10 +150,12 @@ function generate( clean ){ } var result = query.render( vs ); + + // @todo: remove unnessesary sort conditions result.sort = result.sort.concat( sort( clean ) ); // @todo: remove this hack return JSON.parse( JSON.stringify( result ) ); } -module.exports = generate; +module.exports = generateQuery; diff --git a/test/unit/fixture/reverse_standard.js b/test/unit/fixture/reverse_standard.js new file mode 100644 index 00000000..b7cc058f --- /dev/null +++ b/test/unit/fixture/reverse_standard.js @@ -0,0 +1,43 @@ + +module.exports = { + 'query': { + 'filtered': { + 'query': { + 'bool': {} + }, + 'filter': { + 'bool': { + 'must': [ + { + 'geo_distance': { + 'distance': '500km', + 'distance_type': 'plane', + 'optimize_bbox': 'indexed', + '_cache': true, + 'center_point': { + 'lat': 29.49136, + 'lon': -82.50622 + } + } + } + ] + } + } + } + }, + 'sort': [ + '_score', + { + '_geo_distance': { + 'center_point': { + 'lat': 29.49136, + 'lon': -82.50622 + }, + 'order': 'asc', + 'distance_type': 'plane' + } + } + ], + 'size': 1, + 'track_scores': true +}; \ No newline at end of file diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index e200b832..a8dde064 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -1,15 +1,16 @@ var generate = require('../../../query/reverse'); -var admin_boost = 'admin_boost'; -var population = 'population'; -var popularity = 'popularity'; -var category = 'category'; -var category_weights = require('../../../helper/category_weights'); -var admin_weights = require('../../../helper/admin_weights'); -var weights = require('pelias-suggester-pipeline').weights; module.exports.tests = {}; +function debug( a,b ){ + console.log( '----------------------' ); + console.log( JSON.stringify( a, null, 2 ) ); + console.log( '----------------------' ); + console.log( JSON.stringify( b, null, 2 ) ); + console.log( '----------------------' ); +} + module.exports.tests.interface = function(test, common) { test('valid interface', function(t) { t.equal(typeof generate, 'function', 'valid function'); @@ -17,185 +18,25 @@ module.exports.tests.interface = function(test, common) { }); }; -var sort = [ - '_score', - { - '_script': { - 'file': admin_boost, - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'file': popularity, - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'file': population, - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'weights': admin_weights - }, - 'file': 'weights', - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'category_weights': category_weights.default - }, - 'file': category, - 'type': 'number', - 'order': 'desc' - } - }, - { - '_script': { - 'params': { - 'weights': weights - }, - 'file': 'weights', - 'type': 'number', - 'order': 'desc' - } - } -]; - module.exports.tests.query = function(test, common) { test('valid query', function(t) { var query = generate({ lat: 29.49136, lon: -82.50622 }); - var expected = { - 'query': { - 'filtered': { - 'query': { - 'match_all': {} - }, - 'filter': { - 'bool': { - 'must': [ - { - 'geo_distance': { - 'distance': '500km', - 'distance_type': 'plane', - 'optimize_bbox': 'indexed', - '_cache': true, - 'center_point': { - 'lat': '29.49', - 'lon': '-82.51' - } - } - } - ] - } - } - } - }, - 'sort': [ - '_score', - { - '_geo_distance': { - 'center_point': { - 'lat': 29.49136, - 'lon': -82.50622 - }, - 'order': 'asc', - 'unit': 'km' - } - } - ].concat(sort.slice(1)), - 'size': 1, - 'track_scores': true - }; - + var expected = require('../fixture/reverse_standard'); t.deepEqual(query, expected, 'valid reverse query'); - + t.end(); + }); + test('size fuzz test', function(t) { // test different sizes var sizes = [1,2,10,undefined,null]; - sizes.forEach( function(size) { - query = generate({ + sizes.forEach( function( size ){ + var query = generate({ lat: 29.49136, lon: -82.50622, size: size }); - expected.size = size ? size : 1; - t.deepEqual(query, expected, 'valid reverse query for size: '+ size); - }); - t.end(); - }); - - test('valid query with categories', function(t) { - var params = { lat: 29.49136, lon: -82.50622, categories: ['food', 'education', 'entertainment'] }; - var query = generate(params); - - var expected = { - 'query': { - 'filtered': { - 'query': { - 'match_all': {} - }, - 'filter': { - 'bool': { - 'must': [ - { - 'geo_distance': { - 'distance': '500km', - 'distance_type': 'plane', - 'optimize_bbox': 'indexed', - '_cache': true, - 'center_point': { - 'lat': '29.49', - 'lon': '-82.51' - } - } - }, - { - 'terms': { - 'category': params.categories - } - } - ] - } - } - } - }, - 'sort': [ - '_score', - { - '_geo_distance': { - 'center_point': { - 'lat': 29.49136, - 'lon': -82.50622 - }, - 'order': 'asc', - 'unit': 'km' - } - } - ].concat(sort.slice(1)), - 'size': 1, - 'track_scores': true - }; - t.deepEqual(query, expected, 'valid reverse query with categories'); - - // test different sizes - var sizes = [1,2,10,undefined,null]; - sizes.forEach( function(size) { - params.size = size; - query = generate(params); - expected.size = size ? size : 1; - t.deepEqual(query, expected, 'valid reverse query for size: '+ size); + t.equal( query.size, size ? size : 1, 'valid reverse query for size: '+ size); }); t.end(); }); From 22322eaa323928d2a9533b34e3e4d544b39de236 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Wed, 9 Sep 2015 13:26:09 +0200 Subject: [PATCH 21/21] change param:input to param:text --- helper/query_parser.js | 6 +- query/search.js | 44 ++++---- query/sort.js | 6 +- sanitiser/{_input.js => _text.js} | 16 +-- sanitiser/search.js | 2 +- test/unit/query/search.js | 20 ++-- test/unit/sanitiser/{_input.js => _text.js} | 8 +- test/unit/sanitiser/search.js | 112 ++++++++++---------- 8 files changed, 107 insertions(+), 107 deletions(-) rename sanitiser/{_input.js => _text.js} (55%) rename test/unit/sanitiser/{_input.js => _text.js} (80%) diff --git a/helper/query_parser.js b/helper/query_parser.js index dcdbbec6..d1c6672d 100644 --- a/helper/query_parser.js +++ b/helper/query_parser.js @@ -62,13 +62,13 @@ module.exports.get_parsed_address = function get_parsed_address(query) { 'admin_parts' ]; - var parsed_input = {}; + var parsed_text = {}; address_parts.forEach(function(part){ if (parsedAddress[part]) { - parsed_input[part] = parsedAddress[part]; + parsed_text[part] = parsedAddress[part]; } }); - return parsed_input; + return parsed_text; }; diff --git a/query/search.js b/query/search.js index de523c20..244bee4b 100644 --- a/query/search.js +++ b/query/search.js @@ -42,7 +42,7 @@ function generateQuery( clean ){ var vs = new peliasQuery.Vars( peliasQuery.defaults ); // set input text - vs.var( 'input:name', clean.input ); + vs.var( 'input:name', clean.text ); // set size if( clean.size ){ @@ -68,17 +68,17 @@ function generateQuery( clean ){ } // address parsing - if( clean.parsed_input ){ + if( clean.parsed_text ){ // is it a street address? - var isStreetAddress = clean.parsed_input.hasOwnProperty('number') && clean.parsed_input.hasOwnProperty('street'); + var isStreetAddress = clean.parsed_text.hasOwnProperty('number') && clean.parsed_text.hasOwnProperty('street'); if( isStreetAddress ){ - vs.var( 'input:name', clean.parsed_input.number + ' ' + clean.parsed_input.street ); + vs.var( 'input:name', clean.parsed_text.number + ' ' + clean.parsed_text.street ); } // I don't understand this - else if( clean.parsed_input.admin_parts ) { - vs.var( 'input:name', clean.parsed_input.name ); + else if( clean.parsed_text.admin_parts ) { + vs.var( 'input:name', clean.parsed_text.name ); } // or this.. @@ -91,35 +91,35 @@ function generateQuery( clean ){ // ==== add parsed matches [address components] ==== // house number - if( clean.parsed_input.hasOwnProperty('number') ){ - vs.var( 'input:housenumber', clean.parsed_input.number ); + if( clean.parsed_text.hasOwnProperty('number') ){ + vs.var( 'input:housenumber', clean.parsed_text.number ); } // street name - if( clean.parsed_input.hasOwnProperty('street') ){ - vs.var( 'input:street', clean.parsed_input.street ); + if( clean.parsed_text.hasOwnProperty('street') ){ + vs.var( 'input:street', clean.parsed_text.street ); } // postal code - if( clean.parsed_input.hasOwnProperty('postalcode') ){ - vs.var( 'input:postcode', clean.parsed_input.postalcode ); + if( clean.parsed_text.hasOwnProperty('postalcode') ){ + vs.var( 'input:postcode', clean.parsed_text.postalcode ); } // ==== add parsed matches [admin components] ==== // city - if( clean.parsed_input.hasOwnProperty('city') ){ - vs.var( 'input:admin2', clean.parsed_input.city ); + if( clean.parsed_text.hasOwnProperty('city') ){ + vs.var( 'input:admin2', clean.parsed_text.city ); } // state - if( clean.parsed_input.hasOwnProperty('state') ){ - vs.var( 'input:admin1_abbr', clean.parsed_input.state ); + if( clean.parsed_text.hasOwnProperty('state') ){ + vs.var( 'input:admin1_abbr', clean.parsed_text.state ); } // country - if( clean.parsed_input.hasOwnProperty('country') ){ - vs.var( 'input:alpha3', clean.parsed_input.country ); + if( clean.parsed_text.hasOwnProperty('country') ){ + vs.var( 'input:alpha3', clean.parsed_text.country ); } // ==== deal with the 'leftover' components ==== @@ -127,11 +127,11 @@ function generateQuery( clean ){ // a concept called 'leftovers' which is just 'admin_parts' /or 'regions'. var leftoversString = ''; - if( clean.parsed_input.hasOwnProperty('admin_parts') ){ - leftoversString = clean.parsed_input.admin_parts; + if( clean.parsed_text.hasOwnProperty('admin_parts') ){ + leftoversString = clean.parsed_text.admin_parts; } - else if( clean.parsed_input.hasOwnProperty('regions') ){ - leftoversString = clean.parsed_input.regions.join(' '); + else if( clean.parsed_text.hasOwnProperty('regions') ){ + leftoversString = clean.parsed_text.regions.join(' '); } // if we have 'leftovers' then assign them to any fields which diff --git a/query/sort.js b/query/sort.js index aaca76e5..5e0394a2 100644 --- a/query/sort.js +++ b/query/sort.js @@ -66,9 +66,9 @@ module.exports = function( params ){ }; function getCategoryWeights(params) { - if (params && params.hasOwnProperty('parsed_input') && - (params.parsed_input.hasOwnProperty('number') || - params.parsed_input.hasOwnProperty('street'))) { + if (params && params.hasOwnProperty('parsed_text') && + (params.parsed_text.hasOwnProperty('number') || + params.parsed_text.hasOwnProperty('street'))) { return category_weights.address; } return category_weights.default; diff --git a/sanitiser/_input.js b/sanitiser/_text.js similarity index 55% rename from sanitiser/_input.js rename to sanitiser/_text.js index 2ec61970..41f38d6c 100644 --- a/sanitiser/_input.js +++ b/sanitiser/_text.js @@ -1,30 +1,30 @@ var isObject = require('is-object'); var query_parser = require('../helper/query_parser'); -// validate inputs, convert types and apply defaults +// validate texts, convert types and apply defaults function sanitize( req ){ req.clean = req.clean || {}; var params= req.query; - // ensure the input params are a valid object + // ensure the text params are a valid object if( !isObject( params ) ){ params = {}; } - // input text - if('string' !== typeof params.input || !params.input.length){ + // text text + if('string' !== typeof params.text || !params.text.length){ return { 'error': true, - 'message': 'invalid param \'input\': text length, must be >0' + 'message': 'invalid param \'text\': text length, must be >0' }; } - req.clean.input = params.input; + req.clean.text = params.text; - req.clean.parsed_input = query_parser.get_parsed_address(params.input); + req.clean.parsed_text = query_parser.get_parsed_address(params.text); req.clean.types = req.clean.layers || {}; - req.clean.types.from_address_parsing = query_parser.get_layers(params.input); + req.clean.types.from_address_parsing = query_parser.get_layers(params.text); return { 'error': false }; } diff --git a/sanitiser/search.js b/sanitiser/search.js index 17409587..ff0441d9 100644 --- a/sanitiser/search.js +++ b/sanitiser/search.js @@ -1,7 +1,7 @@ var _sanitize = require('../sanitiser/_sanitize'), sanitizers = { - input: require('../sanitiser/_input'), + text: require('../sanitiser/_text'), size: require('../sanitiser/_size'), layers: require('../sanitiser/_layers'), source: require('../sanitiser/_source'), diff --git a/test/unit/query/search.js b/test/unit/query/search.js index 355c2043..667fdc26 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -25,7 +25,7 @@ var sort = require('../fixture/sort_default'); module.exports.tests.query = function(test, common) { test('valid search + focus + bbox', function(t) { var query = generate({ - input: 'test', size: 10, + text: 'test', size: 10, lat: 29.49136, lon: -82.50622, bbox: { top: 47.47, @@ -45,7 +45,7 @@ module.exports.tests.query = function(test, common) { test('valid search + bbox', function(t) { var query = generate({ - input: 'test', size: 10, + text: 'test', size: 10, bbox: { top: 47.47, right: -61.84, @@ -64,7 +64,7 @@ module.exports.tests.query = function(test, common) { test('valid lingustic-only search', function(t) { var query = generate({ - input: 'test', size: 10, + text: 'test', size: 10, layers: ['test'] }); @@ -77,7 +77,7 @@ module.exports.tests.query = function(test, common) { test('search search + focus', function(t) { var query = generate({ - input: 'test', size: 10, + text: 'test', size: 10, lat: 29.49136, lon: -82.50622, layers: ['test'] }); @@ -91,12 +91,12 @@ module.exports.tests.query = function(test, common) { test('valid query with a full valid address', function(t) { var address = '123 main st new york ny 10010 US'; - var query = generate({ input: address, + var query = generate({ text: address, layers: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], size: 10, details: true, - parsed_input: parser.get_parsed_address(address), + parsed_text: parser.get_parsed_address(address), }); var expected = require('../fixture/search_full_address'); @@ -107,12 +107,12 @@ module.exports.tests.query = function(test, common) { test('valid query with partial address', function(t) { var partial_address = 'soho grand, new york'; - var query = generate({ input: partial_address, + var query = generate({ text: partial_address, layers: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], size: 10, details: true, - parsed_input: parser.get_parsed_address(partial_address), + parsed_text: parser.get_parsed_address(partial_address), }); var expected = require('../fixture/search_partial_address'); @@ -124,12 +124,12 @@ module.exports.tests.query = function(test, common) { test('valid query with regions in address', function(t) { var partial_address = '1 water st manhattan ny'; - var query = generate({ input: partial_address, + var query = generate({ text: partial_address, layers: [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], size: 10, details: true, - parsed_input: parser.get_parsed_address(partial_address), + parsed_text: parser.get_parsed_address(partial_address), }); var expected = require('../fixture/search_regions_address'); diff --git a/test/unit/sanitiser/_input.js b/test/unit/sanitiser/_text.js similarity index 80% rename from test/unit/sanitiser/_input.js rename to test/unit/sanitiser/_text.js index b6073358..18d37189 100644 --- a/test/unit/sanitiser/_input.js +++ b/test/unit/sanitiser/_text.js @@ -1,18 +1,18 @@ -var input = require('../../../sanitiser/_input'), +var text = require('../../../sanitiser/_text'), parser = require('../../../helper/query_parser'), delim = ',', - defaultError = 'invalid param \'input\': text length, must be >0', + defaultError = 'invalid param \'text\': text length, must be >0', allLayers = [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin', 'osmaddress', 'openaddresses' ], nonAddressLayers = [ 'geoname', 'osmnode', 'osmway', 'admin0', 'admin1', 'admin2', 'neighborhood', 'locality', 'local_admin' ], defaultParsed= { }, - defaultClean = { input: 'test', + defaultClean = { text: 'test', layers: allLayers, size: 10, details: true, - parsed_input: defaultParsed, + parsed_text: defaultParsed, lat:0, lon:0 }, diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index 9bf695e4..d0c20f18 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -1,18 +1,18 @@ var search = require('../../../sanitiser/search'), - _input = require('../sanitiser/_input'), + _text = require('../sanitiser/_text'), parser = require('../../../helper/query_parser'), - defaultParsed = _input.defaultParsed, + defaultParsed = _text.defaultParsed, _sanitize = search.sanitize, middleware = search.middleware, delim = ',', - defaultError = 'invalid param \'input\': text length, must be >0', - defaultClean = { input: 'test', + defaultError = 'invalid param \'text\': text length, must be >0', + defaultClean = { text: 'test', types: { }, size: 10, details: true, - parsed_input: defaultParsed, + parsed_text: defaultParsed, }, sanitize = function(query, cb) { _sanitize({'query':query}, cb); }; @@ -31,12 +31,12 @@ module.exports.tests.interface = function(test, common) { }); }; -module.exports.tests.sanitize_invalid_input = function(test, common) { - test('invalid input', function(t) { +module.exports.tests.sanitize_invalid_text = function(test, common) { + test('invalid text', function(t) { var invalid = [ '', 100, null, undefined, new Date() ]; - invalid.forEach( function( input ){ - sanitize({ input: input }, function( err, clean ){ - t.equal(err, 'invalid param \'input\': text length, must be >0', input + ' is an invalid input'); + invalid.forEach( function( text ){ + sanitize({ text: text }, function( err, clean ){ + t.equal(err, 'invalid param \'text\': text length, must be >0', text + ' is an invalid text'); t.equal(clean, undefined, 'clean not set'); }); }); @@ -44,41 +44,41 @@ module.exports.tests.sanitize_invalid_input = function(test, common) { }); }; -module.exports.tests.sanitise_valid_input = function(test, common) { - test('valid short input', function(t) { - sanitize({ input: 'a' }, function( err, clean ){ +module.exports.tests.sanitise_valid_text = function(test, common) { + test('valid short text', function(t) { + sanitize({ text: 'a' }, function( err, clean ){ t.equal(err, undefined, 'no error'); }); t.end(); }); - test('valid not-quite-as-short input', function(t) { - sanitize({ input: 'aa' }, function( err, clean ){ + test('valid not-quite-as-short text', function(t) { + sanitize({ text: 'aa' }, function( err, clean ){ t.equal(err, undefined, 'no error'); }); t.end(); }); - test('valid longer input', function(t) { - sanitize({ input: 'aaaaaaaa' }, function( err, clean ){ + test('valid longer text', function(t) { + sanitize({ text: 'aaaaaaaa' }, function( err, clean ){ t.equal(err, undefined, 'no error'); }); t.end(); }); }; -module.exports.tests.sanitize_input_with_delim = function(test, common) { - var inputs = [ 'a,bcd', '123 main st, admin1', ',,,', ' ' ]; +module.exports.tests.sanitize_text_with_delim = function(test, common) { + var texts = [ 'a,bcd', '123 main st, admin1', ',,,', ' ' ]; - test('valid inputs with a comma', function(t) { - inputs.forEach( function( input ){ - sanitize({ input: input }, function( err, clean ){ + test('valid texts with a comma', function(t) { + texts.forEach( function( text ){ + sanitize({ text: text }, function( err, clean ){ var expected = JSON.parse(JSON.stringify( defaultClean )); - expected.input = input; + expected.text = text; - expected.parsed_input = parser.get_parsed_address(input); + expected.parsed_text = parser.get_parsed_address(text); t.equal(err, undefined, 'no error'); - t.equal(clean.parsed_input.name, expected.parsed_input.name, 'clean name set correctly'); + t.equal(clean.parsed_text.name, expected.parsed_text.name, 'clean name set correctly'); }); }); @@ -93,7 +93,7 @@ module.exports.tests.sanitize_lat = function(test, common) { }; test('invalid lat', function(t) { lats.invalid.forEach( function( lat ){ - sanitize({ input: 'test', lat: lat, lon: 0 }, function( err, clean ){ + sanitize({ text: 'test', lat: lat, lon: 0 }, function( err, clean ){ t.equal(err, 'invalid param \'lat\': must be >-90 and <90', lat + ' is an invalid latitude'); t.equal(clean, undefined, 'clean not set'); }); @@ -102,7 +102,7 @@ module.exports.tests.sanitize_lat = function(test, common) { }); test('valid lat', function(t) { lats.valid.forEach( function( lat ){ - sanitize({ input: 'test', lat: lat, lon: 0 }, function( err, clean ){ + sanitize({ text: 'test', lat: lat, lon: 0 }, function( err, clean ){ var expected = JSON.parse(JSON.stringify( defaultClean )); expected.lat = parseFloat( lat ); t.equal(err, undefined, 'no error'); @@ -119,7 +119,7 @@ module.exports.tests.sanitize_lon = function(test, common) { }; test('valid lon', function(t) { lons.valid.forEach( function( lon ){ - sanitize({ input: 'test', lat: 0, lon: lon }, function( err, clean ){ + sanitize({ text: 'test', lat: 0, lon: lon }, function( err, clean ){ var expected = JSON.parse(JSON.stringify( defaultClean )); expected.lon = parseFloat( lon ); t.equal(err, undefined, 'no error'); @@ -132,7 +132,7 @@ module.exports.tests.sanitize_lon = function(test, common) { module.exports.tests.sanitize_optional_geo = function(test, common) { test('no lat/lon', function(t) { - sanitize({ input: 'test' }, function( err, clean ){ + sanitize({ text: 'test' }, function( err, clean ){ t.equal(err, undefined, 'no error'); t.equal(clean.lat, undefined, 'clean set without lat'); t.equal(clean.lon, undefined, 'clean set without lon'); @@ -140,7 +140,7 @@ module.exports.tests.sanitize_optional_geo = function(test, common) { t.end(); }); test('no lat', function(t) { - sanitize({ input: 'test', lon: 0 }, function( err, clean ){ + sanitize({ text: 'test', lon: 0 }, function( err, clean ){ var expected_lon = 0; t.equal(err, undefined, 'no error'); t.deepEqual(clean.lon, expected_lon, 'clean set correctly (without any lat)'); @@ -148,7 +148,7 @@ module.exports.tests.sanitize_optional_geo = function(test, common) { t.end(); }); test('no lon', function(t) { - sanitize({ input: 'test', lat: 0 }, function( err, clean ){ + sanitize({ text: 'test', lat: 0 }, function( err, clean ){ var expected_lat = 0; t.equal(err, undefined, 'no error'); t.deepEqual(clean.lat, expected_lat, 'clean set correctly (without any lon)'); @@ -187,7 +187,7 @@ module.exports.tests.sanitize_bbox = function(test, common) { }; test('invalid bbox', function(t) { bboxes.invalid.forEach( function( bbox ){ - sanitize({ input: 'test', bbox: bbox }, function( err, clean ){ + sanitize({ text: 'test', bbox: bbox }, function( err, clean ){ t.equal(err, undefined, 'no error'); t.equal(clean.bbox, undefined, 'falling back on 50km distance from centroid'); }); @@ -196,7 +196,7 @@ module.exports.tests.sanitize_bbox = function(test, common) { }); test('valid bbox', function(t) { bboxes.valid.forEach( function( bbox ){ - sanitize({ input: 'test', bbox: bbox }, function( err, clean ){ + sanitize({ text: 'test', bbox: bbox }, function( err, clean ){ var bboxArray = bbox.split(',').map(function(i) { return parseInt(i); }); @@ -216,19 +216,19 @@ module.exports.tests.sanitize_bbox = function(test, common) { module.exports.tests.sanitize_zoom = function(test, common) { test('invalid zoom value', function(t) { - sanitize({ zoom: 'a', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + sanitize({ zoom: 'a', text: 'test', lat: 0, lon: 0 }, function( err, clean ){ t.equal(clean.zoom, undefined, 'zoom not set'); t.end(); }); }); test('below min zoom value', function(t) { - sanitize({ zoom: -100, input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + sanitize({ zoom: -100, text: 'test', lat: 0, lon: 0 }, function( err, clean ){ t.equal(clean.zoom, 1, 'min zoom set'); t.end(); }); }); test('above max zoom value', function(t) { - sanitize({ zoom: 9999, input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + sanitize({ zoom: 9999, text: 'test', lat: 0, lon: 0 }, function( err, clean ){ t.equal(clean.zoom, 18, 'max zoom set'); t.end(); }); @@ -237,19 +237,19 @@ module.exports.tests.sanitize_zoom = function(test, common) { module.exports.tests.sanitize_size = function(test, common) { test('invalid size value', function(t) { - sanitize({ size: 'a', input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + sanitize({ size: 'a', text: 'test', lat: 0, lon: 0 }, function( err, clean ){ t.equal(clean.size, 10, 'default size set'); t.end(); }); }); test('below min size value', function(t) { - sanitize({ size: -100, input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + sanitize({ size: -100, text: 'test', lat: 0, lon: 0 }, function( err, clean ){ t.equal(clean.size, 1, 'min size set'); t.end(); }); }); test('above max size value', function(t) { - sanitize({ size: 9999, input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + sanitize({ size: 9999, text: 'test', lat: 0, lon: 0 }, function( err, clean ){ t.equal(clean.size, 40, 'max size set'); t.end(); }); @@ -260,7 +260,7 @@ module.exports.tests.sanitize_details = function(test, common) { var invalid_values = [null, -1, 123, NaN, 'abc']; invalid_values.forEach(function(details) { test('invalid details param ' + details, function(t) { - sanitize({ input: 'test', lat: 0, lon: 0, details: details }, function( err, clean ){ + sanitize({ text: 'test', lat: 0, lon: 0, details: details }, function( err, clean ){ t.equal(clean.details, false, 'default details set (to false)'); t.end(); }); @@ -270,7 +270,7 @@ module.exports.tests.sanitize_details = function(test, common) { var valid_values = ['true', true, 1, '1', 'yes', 'y']; valid_values.forEach(function(details) { test('valid details param ' + details, function(t) { - sanitize({ input: 'test', lat: 0, lon: 0, details: details }, function( err, clean ){ + sanitize({ text: 'test', lat: 0, lon: 0, details: details }, function( err, clean ){ t.equal(clean.details, true, 'details set to true'); t.end(); }); @@ -280,7 +280,7 @@ module.exports.tests.sanitize_details = function(test, common) { var valid_false_values = ['false', false, 0, '0', 'no', 'n']; valid_false_values.forEach(function(details) { test('test setting false explicitly ' + details, function(t) { - sanitize({ input: 'test', lat: 0, lon: 0, details: details }, function( err, clean ){ + sanitize({ text: 'test', lat: 0, lon: 0, details: details }, function( err, clean ){ t.equal(clean.details, false, 'details set to false'); t.end(); }); @@ -288,7 +288,7 @@ module.exports.tests.sanitize_details = function(test, common) { }); test('test default behavior', function(t) { - sanitize({ input: 'test', lat: 0, lon: 0 }, function( err, clean ){ + sanitize({ text: 'test', lat: 0, lon: 0 }, function( err, clean ){ t.equal(clean.details, true, 'details set to true'); t.end(); }); @@ -297,13 +297,13 @@ module.exports.tests.sanitize_details = function(test, common) { module.exports.tests.sanitize_layers = function(test, common) { test('unspecified', function(t) { - sanitize({ layers: undefined, input: 'test' }, function( err, clean ){ + sanitize({ layers: undefined, text: 'test' }, function( err, clean ){ t.deepEqual(clean.types.from_layers, defaultClean.types.from_layers, 'default layers set'); t.end(); }); }); test('invalid layer', function(t) { - sanitize({ layers: 'test_layer', input: 'test' }, function( err, clean ){ + sanitize({ layers: 'test_layer', text: 'test' }, function( err, clean ){ var msg = 'invalid param \'layers\': must be one or more of '; t.true(err.match(msg), 'invalid layer requested'); t.true(err.length > msg.length, 'invalid error message'); @@ -312,30 +312,30 @@ module.exports.tests.sanitize_layers = function(test, common) { }); test('poi (alias) layer', function(t) { var poi_layers = ['geoname','osmnode','osmway']; - sanitize({ layers: 'poi', input: 'test' }, function( err, clean ){ + sanitize({ layers: 'poi', text: 'test' }, function( err, clean ){ t.deepEqual(clean.types.from_layers, poi_layers, 'poi layers set'); t.end(); }); }); test('admin (alias) layer', function(t) { var admin_layers = ['admin0','admin1','admin2','neighborhood','locality','local_admin']; - sanitize({ layers: 'admin', input: 'test' }, function( err, clean ){ + sanitize({ layers: 'admin', text: 'test' }, function( err, clean ){ t.deepEqual(clean.types.from_layers, admin_layers, 'admin layers set'); t.end(); }); }); test('address (alias) layer', function(t) { var address_layers = ['osmaddress','openaddresses']; - sanitize({ layers: 'address', input: 'test' }, function( err, clean ){ + sanitize({ layers: 'address', text: 'test' }, function( err, clean ){ t.deepEqual(clean.types.from_layers, address_layers, 'types from layers set'); - t.deepEqual(clean.types.from_address_parser, _input.allLayers, 'address parser uses default layers'); + t.deepEqual(clean.types.from_address_parser, _text.allLayers, 'address parser uses default layers'); t.end(); }); }); test('poi alias layer plus regular layers', function(t) { var poi_layers = ['geoname','osmnode','osmway']; var reg_layers = ['admin0', 'admin1']; - sanitize({ layers: 'poi,admin0,admin1', input: 'test' }, function( err, clean ){ + sanitize({ layers: 'poi,admin0,admin1', text: 'test' }, function( err, clean ){ t.deepEqual(clean.types.from_layers, reg_layers.concat(poi_layers), 'poi + regular layers'); t.end(); }); @@ -343,7 +343,7 @@ module.exports.tests.sanitize_layers = function(test, common) { test('admin alias layer plus regular layers', function(t) { var admin_layers = ['admin0','admin1','admin2','neighborhood','locality','local_admin']; var reg_layers = ['geoname', 'osmway']; - sanitize({ layers: 'admin,geoname,osmway', input: 'test' }, function( err, clean ){ + sanitize({ layers: 'admin,geoname,osmway', text: 'test' }, function( err, clean ){ t.deepEqual(clean.types.from_layers, reg_layers.concat(admin_layers), 'admin + regular layers set'); t.end(); }); @@ -351,21 +351,21 @@ module.exports.tests.sanitize_layers = function(test, common) { test('address alias layer plus regular layers', function(t) { var address_layers = ['osmaddress','openaddresses']; var reg_layers = ['geoname', 'osmway']; - sanitize({ layers: 'address,geoname,osmway', input: 'test' }, function( err, clean ){ + sanitize({ layers: 'address,geoname,osmway', text: 'test' }, function( err, clean ){ t.deepEqual(clean.types.from_layers, reg_layers.concat(address_layers), 'address + regular layers set'); t.end(); }); }); test('alias layer plus regular layers (no duplicates)', function(t) { var poi_layers = ['geoname','osmnode','osmway']; - sanitize({ layers: 'poi,geoname,osmnode', input: 'test' }, function( err, clean ){ + sanitize({ layers: 'poi,geoname,osmnode', text: 'test' }, function( err, clean ){ t.deepEqual(clean.types.from_layers, poi_layers, 'poi 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']; - sanitize({ layers: 'poi,admin', input: 'test' }, function( err, clean ){ + sanitize({ layers: 'poi,admin', text: 'test' }, function( err, clean ){ t.deepEqual(clean.types.from_layers, alias_layers, 'all layers found (no duplicates)'); t.end(); }); @@ -373,7 +373,7 @@ module.exports.tests.sanitize_layers = function(test, common) { }; module.exports.tests.invalid_params = function(test, common) { - test('invalid input params', function(t) { + test('invalid text params', function(t) { sanitize( undefined, function( err, clean ){ t.equal(err, defaultError, 'handle invalid params gracefully'); t.end(); @@ -396,7 +396,7 @@ module.exports.tests.middleware_failure = function(test, common) { module.exports.tests.middleware_success = function(test, common) { test('middleware success', function(t) { - var req = { query: { input: 'test' }}; + var req = { query: { text: 'test' }}; var next = function( message ){ t.equal(message, undefined, 'no error message set'); t.end();