From 254b7ff66bf69673644aebcfbd76656980e9b3bd Mon Sep 17 00:00:00 2001 From: Alec Vulfson Date: Tue, 2 Aug 2016 16:41:24 -0400 Subject: [PATCH 01/22] Updated warning for boundary.circle --- sanitiser/_geo_reverse.js | 4 ++-- test/unit/sanitiser/_geo_reverse.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sanitiser/_geo_reverse.js b/sanitiser/_geo_reverse.js index 06785c44..3b031089 100644 --- a/sanitiser/_geo_reverse.js +++ b/sanitiser/_geo_reverse.js @@ -15,8 +15,8 @@ module.exports = function sanitize( raw, clean ){ return raw.hasOwnProperty('boundary.circle.' + field); }; - if (['lat', 'lon', 'radius'].some(hasBoundaryCircleField)) { - messages.warnings.push('boundary.circle is currently unsupported'); + if (['lat', 'lon'].some(hasBoundaryCircleField)) { + messages.warnings.push('boundary.circle.lat/boundary.circle.lon are currently unsupported'); } try { diff --git a/test/unit/sanitiser/_geo_reverse.js b/test/unit/sanitiser/_geo_reverse.js index 480d06b2..2f6a30c6 100644 --- a/test/unit/sanitiser/_geo_reverse.js +++ b/test/unit/sanitiser/_geo_reverse.js @@ -16,7 +16,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) { t.equals(clean['boundary.circle.lat'], 12.121212, 'should be set to point.lat'); t.deepEquals(errorsAndWarnings, { errors: [], - warnings: ['boundary.circle is currently unsupported'] + warnings: ['boundary.circle.lat/boundary.circle.lon are currently unsupported'] }, 'no warnings/errors'); t.end(); }); @@ -33,7 +33,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) { t.equals(clean['boundary.circle.lon'], 21.212121, 'should be set to point.lon'); t.deepEquals(errorsAndWarnings, { errors: [], - warnings: ['boundary.circle is currently unsupported'] + warnings: ['boundary.circle.lat/boundary.circle.lon are currently unsupported'] }, 'no warnings/errors'); t.end(); }); @@ -50,7 +50,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) { // t.equals(clean['boundary.circle.radius'], 12.121212, 'should be set to point.lat') t.deepEquals(errorsAndWarnings, { errors: [], - warnings: ['boundary.circle is currently unsupported'] + warnings: [] }, 'no warnings/errors'); t.end(); }); From c5d2817ab432af3c88e01a3674713a39bffe1088 Mon Sep 17 00:00:00 2001 From: avulfson17 Date: Thu, 4 Aug 2016 13:08:49 -0400 Subject: [PATCH 02/22] update test name --- test/unit/sanitiser/_geo_reverse.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/sanitiser/_geo_reverse.js b/test/unit/sanitiser/_geo_reverse.js index 2f6a30c6..107630a8 100644 --- a/test/unit/sanitiser/_geo_reverse.js +++ b/test/unit/sanitiser/_geo_reverse.js @@ -38,7 +38,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) { t.end(); }); - test('raw with boundary.circle.radius should add warning about ignored boundary.circle', function(t) { + test('raw with boundary.circle.radius shouldn\'t add warning about ignored boundary.circle', function(t) { var raw = { 'point.lat': '12.121212', 'point.lon': '21.212121', From 1c6436d12377a8b6708b201df091d35d711f9f3c Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Tue, 24 May 2016 16:32:54 -0400 Subject: [PATCH 03/22] Update categories sanitizer and implement unit tests for it --- sanitiser/_categories.js | 42 +++++--- test/unit/run.js | 1 + test/unit/sanitiser/_categories.js | 151 +++++++++++++++++++++++++++++ 3 files changed, 181 insertions(+), 13 deletions(-) create mode 100644 test/unit/sanitiser/_categories.js diff --git a/sanitiser/_categories.js b/sanitiser/_categories.js index 29737922..63d6324e 100644 --- a/sanitiser/_categories.js +++ b/sanitiser/_categories.js @@ -1,34 +1,50 @@ var check = require('check-types'); +var ERRORS = { + empty: 'Categories parameter cannot be left blank. See documentation of service for valid options.', + invalid: 'Invalid categories parameter value(s). See documentation of service for valid options.' +}; + // validate inputs, convert types and apply defaults -function sanitize( raw, clean ){ +function sanitize( raw, clean, validCategories ) { // error & warning messages - var messages = { errors: [], warnings: [] }; + var messages = {errors: [], warnings: []}; - // default case (no categories specified in GET params) - clean.categories = []; + // it's not a required parameter, so if it's not provided just move on + if (!raw.hasOwnProperty('categories')) { + return messages; + } - // if categories string has been set - if( check.nonEmptyString( raw.categories ) ){ + if (!check.nonEmptyString(raw.categories)) { + messages.errors.push(ERRORS.empty); + return messages; + } - // map input categories to valid format + // if categories string has been set + // map input categories to valid format + try { clean.categories = raw.categories.split(',') .map(function (cat) { return cat.toLowerCase().trim(); // lowercase inputs }) - .filter( function( cat ) { - return ( cat.length > 0 ); + .filter(function (cat) { + if (check.nonEmptyString(cat) && validCategories && validCategories.indexOf(cat) !== -1) { + return true; + } + throw new Error('Empty string value'); }); + } catch (err) { + // remove everything from the list if there was any error + delete clean.categories; + } - if( !clean.categories.length ){ - messages.warnings.push( 'invalid \'categories\': no valid category strings found'); - } + if (check.undefined(clean.categories) || check.emptyArray(clean.categories)) { + messages.errors.push(ERRORS.invalid); } return messages; - } // export function diff --git a/test/unit/run.js b/test/unit/run.js index 2a89b161..b6e40c71 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -51,6 +51,7 @@ var tests = [ require('./sanitiser/_text'), require('./sanitiser/_tokenizer'), require('./sanitiser/_deprecate_quattroshapes'), + require('./sanitiser/_categories'), require('./src/backend'), require('./sanitiser/autocomplete'), require('./sanitiser/place'), diff --git a/test/unit/sanitiser/_categories.js b/test/unit/sanitiser/_categories.js new file mode 100644 index 00000000..b896915d --- /dev/null +++ b/test/unit/sanitiser/_categories.js @@ -0,0 +1,151 @@ +var sanitize = require( '../../../sanitiser/_categories'); + +module.exports.tests = {}; + +module.exports.tests.no_categories = function(test, common) { + test('categories not set', function(t) { + var req = { + query: { }, + clean: { } + }; + + var messages = sanitize(req.query, req.clean); + + t.equal(req.clean.categories, undefined, 'no categories should be defined'); + t.deepEqual(messages.errors, [], 'no error returned'); + t.deepEqual(messages.warnings, [], 'no warnings returned'); + t.end(); + }); + + test('categories is empty string', function(t) { + var req = { + query: { + categories: '' + }, + clean: { } + }; + + var expected_error = 'Categories parameter cannot be left blank. See documentation of service for valid options.'; + + var messages = sanitize(req.query, req.clean); + + t.equal(req.clean.categories, undefined, 'no categories should be defined'); + t.deepEqual(messages.errors.length, 1, 'error returned'); + t.deepEqual(messages.errors[0], expected_error, 'error returned'); + t.deepEqual(messages.warnings, [], 'no warnings returned'); + t.end(); + }); + + test('categories is an array of empty strings', function(t) { + var req = { + query: { + categories: ',,' + }, + clean: { } + }; + + var expected_error = 'Invalid categories parameter value(s). See documentation of service for valid options.'; + + var messages = sanitize(req.query, req.clean); + + t.equal(req.clean.categories, undefined, 'no categories should be defined'); + t.deepEqual(messages.errors.length, 1, 'error returned'); + t.deepEqual(messages.errors[0], expected_error, 'error returned'); + t.deepEqual(messages.warnings, [], 'no warnings returned'); + t.end(); + }); +}; + +module.exports.tests.valid_categories = function(test, common) { + var validCategories = ['food','health','financial','education','government']; + + test('single category', function(t) { + var req = { + query: { + categories: 'food' + }, + clean: { } + }; + + var messages = sanitize(req.query, req.clean, validCategories); + + t.deepEqual(req.clean.categories, ['food'], 'categories should contain food'); + t.deepEqual(messages.errors, [], 'no error returned'); + t.deepEqual(messages.warnings, [], 'no warnings returned'); + + t.end(); + }); + + test('multiple categories', function(t) { + var req = { + query: { + categories: 'food,health' + }, + clean: { } + }; + + var messages = sanitize(req.query, req.clean, validCategories); + + t.deepEqual(req.clean.categories, ['food', 'health'], + 'clean.categories should be an array with proper values'); + t.deepEqual(messages.errors, [], 'no error returned'); + t.deepEqual(messages.warnings, [], 'no warnings returned'); + t.end(); + }); +}; + +module.exports.tests.invalid_categories = function(test, common) { + var validCategories = ['food','health','financial','education','government']; + + test('garbage category', function(t) { + var req = { + query: { + categories: 'barf' + }, + clean: { } + }; + var expected_messages = { + errors: [ + 'Invalid categories parameter value(s). See documentation of service for valid options.' + ], + warnings: [] + }; + + var messages = sanitize(req.query, req.clean, validCategories); + + t.deepEqual(messages, expected_messages, 'error with message returned'); + t.equal(req.clean.categories, undefined, 'clean.categories should remain empty'); + t.end(); + }); + + test('all garbage categories', function(t) { + var req = { + query: { + categories: 'barf,bleh' + }, + clean: { } + }; + var expected_messages = { + errors: [ + 'Invalid categories parameter value(s). See documentation of service for valid options.' + ], + warnings: [] + }; + + var messages = sanitize(req.query, req.clean); + + t.deepEqual(messages, expected_messages, 'error with message returned'); + t.equal(req.clean.categories, undefined, 'clean.categories should remain empty'); + t.end(); + }); +}; + +module.exports.all = function (tape, common) { + function test(name, testFunction) { + return tape('SANTIZE _categories ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; From 24c7370ce742ba677507a24f8dd1e0977b399c55 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Tue, 24 May 2016 16:34:19 -0400 Subject: [PATCH 04/22] Add categories sanitizer to autocomplete --- sanitiser/autocomplete.js | 1 + test/unit/sanitiser/autocomplete.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/sanitiser/autocomplete.js b/sanitiser/autocomplete.js index 8ab6fd9c..66bc0802 100644 --- a/sanitiser/autocomplete.js +++ b/sanitiser/autocomplete.js @@ -12,6 +12,7 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), sources_and_layers: require('../sanitiser/_sources_and_layers'), private: require('../sanitiser/_flag_bool')('private', false), geo_autocomplete: require('../sanitiser/_geo_autocomplete'), + categories: require('../sanitiser/_categories') }; var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; diff --git a/test/unit/sanitiser/autocomplete.js b/test/unit/sanitiser/autocomplete.js index 186cb4b6..6581c689 100644 --- a/test/unit/sanitiser/autocomplete.js +++ b/test/unit/sanitiser/autocomplete.js @@ -6,7 +6,7 @@ module.exports.tests.sanitisers = function(test, common) { test('check sanitiser list', function (t) { var expected = [ 'singleScalarParameters', 'text', 'tokenizer', 'size', 'layers', 'sources', - 'sources_and_layers', 'private', 'geo_autocomplete' + 'sources_and_layers', 'private', 'geo_autocomplete', 'categories' ]; t.deepEqual(Object.keys(autocomplete.sanitiser_list), expected); t.end(); From 818711f509e8cc2081234d381f1cc364ebcb1c09 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Tue, 24 May 2016 16:34:43 -0400 Subject: [PATCH 05/22] Add categories sanitizer to search --- sanitiser/search.js | 1 + test/unit/sanitiser/search.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/sanitiser/search.js b/sanitiser/search.js index b2976af3..7fcc6ab6 100644 --- a/sanitiser/search.js +++ b/sanitiser/search.js @@ -13,6 +13,7 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), private: require('../sanitiser/_flag_bool')('private', false), geo_search: require('../sanitiser/_geo_search'), boundary_country: require('../sanitiser/_boundary_country'), + categories: require('../sanitiser/_categories') }; var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index 70c4400f..35dbcda5 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -25,7 +25,7 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.sanitisers = function(test, common) { test('check sanitiser list', function (t) { var expected = ['quattroshapes_deprecation', 'singleScalarParameters', 'text', 'size', - 'layers', 'sources', 'sources_and_layers', 'private', 'geo_search', 'boundary_country' ]; + 'layers', 'sources', 'sources_and_layers', 'private', 'geo_search', 'boundary_country', 'categories' ]; t.deepEqual(Object.keys(search.sanitiser_list), expected); t.end(); }); From 68c6b15e0f9414eb4bc70142722adf1155bf85ec Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Tue, 24 May 2016 16:35:07 -0400 Subject: [PATCH 06/22] Add categories sanitizer to reverse --- sanitiser/reverse.js | 1 + test/unit/sanitiser/reverse.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/sanitiser/reverse.js b/sanitiser/reverse.js index a85e5bf8..a32fe002 100644 --- a/sanitiser/reverse.js +++ b/sanitiser/reverse.js @@ -12,6 +12,7 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), private: require('../sanitiser/_flag_bool')('private', false), geo_reverse: require('../sanitiser/_geo_reverse'), boundary_country: require('../sanitiser/_boundary_country'), + categories: require('../sanitiser/_categories') }; var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js index 089e1aaf..aabb84e9 100644 --- a/test/unit/sanitiser/reverse.js +++ b/test/unit/sanitiser/reverse.js @@ -37,7 +37,7 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.sanitisers = function(test, common) { test('check sanitiser list', function (t) { var expected = ['quattroshapes_deprecation', 'singleScalarParameters', 'layers', - 'sources', 'sources_and_layers', 'size', 'private', 'geo_reverse', 'boundary_country']; + 'sources', 'sources_and_layers', 'size', 'private', 'geo_reverse', 'boundary_country', 'categories']; t.deepEqual(Object.keys(reverse.sanitiser_list), expected); t.end(); }); From 269733b70c4c8c765dc2cd21d44cacd9a6a4d290 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Thu, 2 Jun 2016 13:01:39 -0400 Subject: [PATCH 07/22] Add type property to allow proper handling of arrays --- helper/geojsonify.js | 73 ++++++++++++++++++++++++++-------- test/unit/helper/geojsonify.js | 15 +++++-- 2 files changed, 69 insertions(+), 19 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index b4b57536..dfa177ca 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -8,11 +8,13 @@ var GeoJSON = require('geojson'), _ = require('lodash'); // Properties to be copied +// If a property is identified as a single string, assume it should be presented as a string in response +// If something other than string is desired, use the following structure: { name: 'category', type: 'array' } var DETAILS_PROPS = [ 'housenumber', 'street', 'postalcode', - 'confidence', + { name: 'confidence', type: 'default' }, 'distance', 'country', 'country_gid', @@ -40,10 +42,10 @@ var DETAILS_PROPS = [ 'borough_a', 'neighbourhood', 'neighbourhood_gid', - 'bounding_box' + { name: 'bounding_box', type: 'default' }, + { name: 'category', type: 'array' } ]; - function lookupSource(src) { return src.source; } @@ -217,26 +219,65 @@ function computeBBox(geojson, geojsonExtentPoints) { function copyProperties( source, props, dst ) { props.forEach( function ( prop ) { - if ( source.hasOwnProperty( prop ) ) { - - // array value, take first item in array (at this time only used for admin values) - if (source[prop] instanceof Array) { - if (source[prop].length === 0) { - return; - } - if (source[prop][0]) { - dst[prop] = source[prop][0]; - } + var property = { + name: prop.name || prop, + type: prop.type || 'string' + }; + + var value = null; + if ( source.hasOwnProperty( property.name ) ) { + + switch (property.type) { + case 'string': + value = getStringValue(source[property.name]); + break; + case 'array': + value = getArrayValue(source[property.name]); + break; + // default behavior is to copy property exactly as is + default: + value = source[property.name]; } - // simple value - else { - dst[prop] = source[prop]; + if (_.isNumber(value) || (value && !_.isEmpty(value))) { + dst[property.name] = value; } } }); } +function getStringValue(property) { + // isEmpty check works for all types of values: strings, arrays, objects + if (_.isEmpty(property)) { + return ''; + } + + if (_.isString(property)) { + return property; + } + + // array value, take first item in array (at this time only used for admin values) + if (_.isArray(property)) { + return property[0]; + } + + return _.toString(property); +} + + +function getArrayValue(property) { + // isEmpty check works for all types of values: strings, arrays, objects + if (_.isEmpty(property)) { + return ''; + } + + if (_.isArray(property)) { + return property; + } + + return [property]; +} + /** * Create a gid from a document * @TODO modify all importers to create separate source and layer fields to remove mapping diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index 6f3581c5..8ec97396 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -153,7 +153,11 @@ module.exports.tests.search = function(test, common) { 'neighbourhood': 'test3', 'housenumber': '13', 'street': 'Liverpool Road', - 'postalcode': 'N1 0RW' + 'postalcode': 'N1 0RW', + 'category': [ + 'food', + 'nightlife' + ] } }, { @@ -208,7 +212,11 @@ module.exports.tests.search = function(test, common) { 'county': 'New York', 'borough': 'Manhattan', 'locality': 'New York', - 'neighbourhood': 'Koreatown' + 'neighbourhood': 'Koreatown', + 'category': [ + 'tourism', + 'transport' + ] } } ] @@ -245,7 +253,7 @@ module.exports.tests.search = function(test, common) { 'default': 'East New York' }, 'source_id': '85816607', - 'category': [], + 'category': ['government'], '_id': '85816607', '_type': 'neighbourhood', '_score': 21.434, @@ -328,6 +336,7 @@ module.exports.tests.search = function(test, common) { 'source': 'whosonfirst', 'source_id': '85816607', 'name': 'East New York', + 'category': ['government'], 'confidence': 0.888, 'country': 'United States', 'country_gid': '85633793', From 07c637f5a6e6b3a3ee62dfc3b176ffee8e92d9f6 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Fri, 3 Jun 2016 15:28:58 -0400 Subject: [PATCH 08/22] Include a new pelias-categories module for category validation --- package.json | 1 + sanitiser/_categories.js | 7 +++++-- test/unit/sanitiser/_categories.js | 27 +++++++++++++++++++++++---- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 4fa1488e..53559104 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ "pelias-query": "8.3.0", "pelias-text-analyzer": "1.3.0", "stats-lite": "2.0.3", + "pelias-categories": "1.0.0", "through2": "2.0.1" }, "devDependencies": { diff --git a/sanitiser/_categories.js b/sanitiser/_categories.js index 63d6324e..06e65800 100644 --- a/sanitiser/_categories.js +++ b/sanitiser/_categories.js @@ -1,5 +1,6 @@ var check = require('check-types'); +var categoryTaxonomy = require('pelias-categories'); var ERRORS = { empty: 'Categories parameter cannot be left blank. See documentation of service for valid options.', @@ -7,7 +8,9 @@ var ERRORS = { }; // validate inputs, convert types and apply defaults -function sanitize( raw, clean, validCategories ) { +function sanitize( raw, clean, categories ) { + + categories = categories || categoryTaxonomy; // error & warning messages var messages = {errors: [], warnings: []}; @@ -30,7 +33,7 @@ function sanitize( raw, clean, validCategories ) { return cat.toLowerCase().trim(); // lowercase inputs }) .filter(function (cat) { - if (check.nonEmptyString(cat) && validCategories && validCategories.indexOf(cat) !== -1) { + if (check.nonEmptyString(cat) && categories.isValidCategory(cat)) { return true; } throw new Error('Empty string value'); diff --git a/test/unit/sanitiser/_categories.js b/test/unit/sanitiser/_categories.js index b896915d..743423a9 100644 --- a/test/unit/sanitiser/_categories.js +++ b/test/unit/sanitiser/_categories.js @@ -57,9 +57,16 @@ module.exports.tests.no_categories = function(test, common) { }; module.exports.tests.valid_categories = function(test, common) { - var validCategories = ['food','health','financial','education','government']; + var isValidCategoryCalled = 0; + var validCategories = { + isValidCategory: function (cat) { + isValidCategoryCalled++; + return ['food','health','financial','education','government'].indexOf(cat) !== -1; } + }; test('single category', function(t) { + isValidCategoryCalled = 0; + var req = { query: { categories: 'food' @@ -73,29 +80,41 @@ module.exports.tests.valid_categories = function(test, common) { t.deepEqual(messages.errors, [], 'no error returned'); t.deepEqual(messages.warnings, [], 'no warnings returned'); + t.equal(isValidCategoryCalled, 1); + t.end(); }); test('multiple categories', function(t) { + isValidCategoryCalled = 0; var req = { query: { categories: 'food,health' }, clean: { } }; + var expectedCategories = ['food', 'health']; var messages = sanitize(req.query, req.clean, validCategories); - t.deepEqual(req.clean.categories, ['food', 'health'], + t.deepEqual(req.clean.categories, expectedCategories, 'clean.categories should be an array with proper values'); t.deepEqual(messages.errors, [], 'no error returned'); t.deepEqual(messages.warnings, [], 'no warnings returned'); + + t.equal(isValidCategoryCalled, expectedCategories.length); + t.end(); }); }; module.exports.tests.invalid_categories = function(test, common) { - var validCategories = ['food','health','financial','education','government']; + var isValidCategoryCalled = 0; + var validCategories = { + isValidCategory: function (cat) { + isValidCategoryCalled++; + return ['food','health','financial','education','government'].indexOf(cat) !== -1; } + }; test('garbage category', function(t) { var req = { @@ -132,7 +151,7 @@ module.exports.tests.invalid_categories = function(test, common) { warnings: [] }; - var messages = sanitize(req.query, req.clean); + var messages = sanitize(req.query, req.clean, validCategories); t.deepEqual(messages, expected_messages, 'error with message returned'); t.equal(req.clean.categories, undefined, 'clean.categories should remain empty'); From 1df1a0765c315d12881ec37f9040d1cca98ef911 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Fri, 3 Jun 2016 15:46:21 -0400 Subject: [PATCH 09/22] Add categories filter to search query --- query/search.js | 7 ++ .../fixture/search_with_category_filtering.js | 93 +++++++++++++++++++ test/unit/query/search.js | 12 +++ 3 files changed, 112 insertions(+) create mode 100644 test/unit/fixture/search_with_category_filtering.js diff --git a/query/search.js b/query/search.js index 92a9db00..5ab96248 100644 --- a/query/search.js +++ b/query/search.js @@ -44,6 +44,8 @@ query.filter( peliasQuery.view.boundary_circle ); query.filter( peliasQuery.view.boundary_rect ); query.filter( peliasQuery.view.sources ); query.filter( peliasQuery.view.layers ); +query.filter( peliasQuery.view.categories ); + // -------------------------------- /** @@ -63,6 +65,11 @@ function generateQuery( clean ){ // layers vs.var( 'layers', clean.layers); + // categories + if (clean.categories) { + vs.var('input:categories', clean.categories); + } + // size if( clean.querySize ) { vs.var( 'size', clean.querySize ); diff --git a/test/unit/fixture/search_with_category_filtering.js b/test/unit/fixture/search_with_category_filtering.js new file mode 100644 index 00000000..57014821 --- /dev/null +++ b/test/unit/fixture/search_with_category_filtering.js @@ -0,0 +1,93 @@ + +module.exports = { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'must': [{ + 'match': { + 'name.default': { + 'query': 'test', + 'boost': 1, + 'analyzer': 'peliasIndexOneEdgeGram' + } + } + }], + 'should': [{ + 'match': { + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'boost': 1, + 'slop': 2 + } + } + }, { + 'function_score': { + 'query': { + 'match': { + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2, + 'boost': 1 + } + } + }, + 'max_boost': 20, + 'score_mode': 'first', + 'boost_mode': 'replace', + 'functions': [{ + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'popularity', + 'missing': 1 + }, + 'weight': 1 + }] + } + }, { + 'function_score': { + 'query': { + 'match': { + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2, + 'boost': 1 + } + } + }, + 'max_boost': 20, + 'score_mode': 'first', + 'boost_mode': 'replace', + 'functions': [{ + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'population', + 'missing': 1 + }, + 'weight': 2 + }] + } + }] + } + }, + 'filter': { + 'bool': { + 'must': [{ + 'terms': { + 'category': ['retail','food'] + } + }] + } + } + } + }, + 'sort': [ '_score' ], + 'size': 20, + 'track_scores': true +}; diff --git a/test/unit/query/search.js b/test/unit/query/search.js index 4ff414c0..61d00984 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -159,6 +159,18 @@ module.exports.tests.query = function(test, common) { t.end(); }); + test('categories filter', function(t) { + var query = generate({ + 'text': 'test', + 'categories': ['retail','food'] + }); + + var compiled = JSON.parse( JSON.stringify( query ) ); + var expected = require('../fixture/search_with_category_filtering'); + + t.deepEqual(compiled, expected, 'valid search query with category filtering'); + t.end(); + }); }; module.exports.all = function (tape, common) { From 28564e0f76902ab504356712b31c0a08a1cb8b3c Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Fri, 3 Jun 2016 18:12:59 -0400 Subject: [PATCH 10/22] Use github links for in-progress denpendency updates --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 53559104..fad97cd6 100644 --- a/package.json +++ b/package.json @@ -50,13 +50,13 @@ "lodash": "^4.5.0", "markdown": "0.5.0", "morgan": "1.7.0", + "pelias-categories": "1.0.0", "pelias-config": "2.1.0", "pelias-logger": "0.0.8", "pelias-model": "4.1.0", "pelias-query": "8.3.0", "pelias-text-analyzer": "1.3.0", "stats-lite": "2.0.3", - "pelias-categories": "1.0.0", "through2": "2.0.1" }, "devDependencies": { From cc9c4148bb3bd9d4aa811e6aab8ac35797f81b83 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Mon, 13 Jun 2016 14:04:29 -0400 Subject: [PATCH 11/22] Refactor geojsonify code and add ability to pass in clean query params --- helper/geojsonify.js | 181 +++---------------------- helper/geojsonify_meta_data.js | 42 ++++++ helper/geojsonify_place_details.js | 133 +++++++++++++++++++ middleware/geocodeJSON.js | 4 +- sanitiser/nearby.js | 29 ++++ test/unit/helper/geojsonify.js | 76 +++++++++-- test/unit/sanitiser/nearby.js | 204 +++++++++++++++++++++++++++++ 7 files changed, 494 insertions(+), 175 deletions(-) create mode 100644 helper/geojsonify_meta_data.js create mode 100644 helper/geojsonify_place_details.js create mode 100644 sanitiser/nearby.js create mode 100644 test/unit/sanitiser/nearby.js diff --git a/helper/geojsonify.js b/helper/geojsonify.js index dfa177ca..9371d0c7 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -1,68 +1,18 @@ -var GeoJSON = require('geojson'), - extent = require('geojson-extent'), - labelGenerator = require('./labelGenerator'), - logger = require('pelias-logger').get('api'), - type_mapping = require('./type_mapping'), - Document = require('pelias-model').Document, - _ = require('lodash'); +var GeoJSON = require('geojson'); +var extent = require('geojson-extent'); +var labelGenerator = require('./labelGenerator'); +var logger = require('pelias-logger').get('api'); +var type_mapping = require('./type_mapping'); +var _ = require('lodash'); +var addDetails = require('./geojsonify_place_details'); +var addMetaData = require('./geojsonify_meta_data'); -// Properties to be copied -// If a property is identified as a single string, assume it should be presented as a string in response -// If something other than string is desired, use the following structure: { name: 'category', type: 'array' } -var DETAILS_PROPS = [ - 'housenumber', - 'street', - 'postalcode', - { name: 'confidence', type: 'default' }, - 'distance', - 'country', - 'country_gid', - 'country_a', - 'macroregion', - 'macroregion_gid', - 'macroregion_a', - 'region', - 'region_gid', - 'region_a', - 'macrocounty', - 'macrocounty_gid', - 'macrocounty_a', - 'county', - 'county_gid', - 'county_a', - 'localadmin', - 'localadmin_gid', - 'localadmin_a', - 'locality', - 'locality_gid', - 'locality_a', - 'borough', - 'borough_gid', - 'borough_a', - 'neighbourhood', - 'neighbourhood_gid', - { name: 'bounding_box', type: 'default' }, - { name: 'category', type: 'array' } -]; - -function lookupSource(src) { - return src.source; -} - -function lookupSourceId(src) { - return src.source_id; -} - -function lookupLayer(src) { - return src.layer; -} - -function geojsonifyPlaces( docs ){ +function geojsonifyPlaces( params, docs ){ // flatten & expand data for geojson conversion var geodata = docs - .map(geojsonifyPlace) + .map(geojsonifyPlace.bind(null, params)) .filter( function( doc ){ return !!doc; }); @@ -85,7 +35,7 @@ function geojsonifyPlaces( docs ){ return geojson; } -function geojsonifyPlace(place) { +function geojsonifyPlace(params, place) { // something went very wrong if( !place || !place.hasOwnProperty( 'center_point' ) ) { @@ -95,7 +45,8 @@ function geojsonifyPlace(place) { var output = {}; addMetaData(place, output); - addDetails(place, output); + addName(place, output); + addDetails(params, place, output); addLabel(place, output); @@ -108,17 +59,15 @@ function geojsonifyPlace(place) { } /** - * Add details properties + * Validate and add name property * * @param {object} src * @param {object} dst */ -function addDetails(src, dst) { +function addName(src, dst) { // map name if( !src.name || !src.name.default ) { return warning(src); } dst.name = src.name.default; - - copyProperties(src, DETAILS_PROPS, dst); } /** @@ -208,104 +157,6 @@ function computeBBox(geojson, geojsonExtentPoints) { } } -/** - * Copy specified properties from source to dest. - * Ignore missing properties. - * - * @param {object} source - * @param {[]} props - * @param {object} dst - */ -function copyProperties( source, props, dst ) { - props.forEach( function ( prop ) { - - var property = { - name: prop.name || prop, - type: prop.type || 'string' - }; - - var value = null; - if ( source.hasOwnProperty( property.name ) ) { - - switch (property.type) { - case 'string': - value = getStringValue(source[property.name]); - break; - case 'array': - value = getArrayValue(source[property.name]); - break; - // default behavior is to copy property exactly as is - default: - value = source[property.name]; - } - - if (_.isNumber(value) || (value && !_.isEmpty(value))) { - dst[property.name] = value; - } - } - }); -} - -function getStringValue(property) { - // isEmpty check works for all types of values: strings, arrays, objects - if (_.isEmpty(property)) { - return ''; - } - - if (_.isString(property)) { - return property; - } - - // array value, take first item in array (at this time only used for admin values) - if (_.isArray(property)) { - return property[0]; - } - - return _.toString(property); -} - - -function getArrayValue(property) { - // isEmpty check works for all types of values: strings, arrays, objects - if (_.isEmpty(property)) { - return ''; - } - - if (_.isArray(property)) { - return property; - } - - return [property]; -} - -/** - * Create a gid from a document - * @TODO modify all importers to create separate source and layer fields to remove mapping - * - * @param {object} src - */ -function makeGid(src) { - var doc = new Document(lookupSource(src), lookupLayer(src), src._id); - return doc.getGid(); -} - -/** - * Determine and set place id, type, and source - * - * @param {object} src - * @param {object} dst - */ -function addMetaData(src, dst) { - dst.id = src._id; - dst.gid = makeGid(src); - dst.layer = lookupLayer(src); - dst.source = lookupSource(src); - dst.source_id = lookupSourceId(src); - if (src.hasOwnProperty('bounding_box')) { - dst.bounding_box = src.bounding_box; - } -} - /** * emit a warning if the doc format is invalid * @@ -317,4 +168,4 @@ function warning( doc ) { } -module.exports.search = geojsonifyPlaces; +module.exports = geojsonifyPlaces; diff --git a/helper/geojsonify_meta_data.js b/helper/geojsonify_meta_data.js new file mode 100644 index 00000000..7fdf0394 --- /dev/null +++ b/helper/geojsonify_meta_data.js @@ -0,0 +1,42 @@ +var Document = require('pelias-model').Document; + +/** + * Determine and set place id, type, and source + * + * @param {object} src + * @param {object} dst + */ +function addMetaData(src, dst) { + dst.id = src._id; + dst.gid = makeGid(src); + dst.layer = lookupLayer(src); + dst.source = lookupSource(src); + dst.source_id = lookupSourceId(src); + if (src.hasOwnProperty('bounding_box')) { + dst.bounding_box = src.bounding_box; + } +} + +/** + * Create a gid from a document + * + * @param {object} src + */ +function makeGid(src) { + var doc = new Document(lookupSource(src), lookupLayer(src), src._id); + return doc.getGid(); +} + +function lookupSource(src) { + return src.source; +} + +function lookupSourceId(src) { + return src.source_id; +} + +function lookupLayer(src) { + return src.layer; +} + +module.exports = addMetaData; \ No newline at end of file diff --git a/helper/geojsonify_place_details.js b/helper/geojsonify_place_details.js new file mode 100644 index 00000000..abaefe27 --- /dev/null +++ b/helper/geojsonify_place_details.js @@ -0,0 +1,133 @@ +var _ = require('lodash'); + +// Properties to be copied +// If a property is identified as a single string, assume it should be presented as a string in response +// If something other than string is desired, use the following structure: { name: 'category', type: 'array' } +var DETAILS_PROPS = [ + { name: 'housenumber', type: 'string' }, + { name: 'street', type: 'string' }, + { name: 'postalcode', type: 'string' }, + { name: 'confidence', type: 'default' }, + { name: 'distance', type: 'default' }, + { name: 'country', type: 'string' }, + { name: 'country_gid', type: 'string' }, + { name: 'country_a', type: 'string' }, + { name: 'macroregion', type: 'string' }, + { name: 'macroregion_gid', type: 'string' }, + { name: 'macroregion_a', type: 'string' }, + { name: 'region', type: 'string' }, + { name: 'region_gid', type: 'string' }, + { name: 'region_a', type: 'string' }, + { name: 'macrocounty', type: 'string' }, + { name: 'macrocounty_gid', type: 'string' }, + { name: 'macrocounty_a', type: 'string' }, + { name: 'county', type: 'string' }, + { name: 'county_gid', type: 'string' }, + { name: 'county_a', type: 'string' }, + { name: 'localadmin', type: 'string' }, + { name: 'localadmin_gid', type: 'string' }, + { name: 'localadmin_a', type: 'string' }, + { name: 'locality', type: 'string' }, + { name: 'locality_gid', type: 'string' }, + { name: 'locality_a', type: 'string' }, + { name: 'borough', type: 'string' }, + { name: 'borough_gid', type: 'string' }, + { name: 'borough_a', type: 'string' }, + { name: 'neighbourhood', type: 'string' }, + { name: 'neighbourhood_gid', type: 'string' }, + { name: 'bounding_box', type: 'default' }, + { name: 'category', type: 'array', condition: checkCategoryParam } +]; + +function checkCategoryParam(params) { + return _.isObject(params) && params.hasOwnProperty('categories'); +} + +/** + * Add details properties + * + * @param {object} params clean query params + * @param {object} src + * @param {object} dst + */ +function addDetails(params, src, dst) { + copyProperties(params, src, DETAILS_PROPS, dst); +} + +/** + * Copy specified properties from source to dest. + * Ignore missing properties. + * + * @param {object} params clean query params + * @param {object} source + * @param {[]} props + * @param {object} dst + */ +function copyProperties( params, source, props, dst ) { + props.forEach( function ( prop ) { + + // if condition isn't met, just return without setting the property + if (_.isFunction(prop.condition) && !prop.condition(params)) { + return; + } + + var property = { + name: prop.name || prop, + type: prop.type || 'default' + }; + + var value = null; + if ( source.hasOwnProperty( property.name ) ) { + + switch (property.type) { + case 'string': + value = getStringValue(source[property.name]); + break; + case 'array': + value = getArrayValue(source[property.name]); + break; + // default behavior is to copy property exactly as is + default: + value = source[property.name]; + } + + if (_.isNumber(value) || (value && !_.isEmpty(value))) { + dst[property.name] = value; + } + } + }); +} + +function getStringValue(property) { + // isEmpty check works for all types of values: strings, arrays, objects + if (_.isEmpty(property)) { + return ''; + } + + if (_.isString(property)) { + return property; + } + + // array value, take first item in array (at this time only used for admin values) + if (_.isArray(property)) { + return property[0]; + } + + return _.toString(property); +} + + +function getArrayValue(property) { + // isEmpty check works for all types of values: strings, arrays, objects + if (_.isEmpty(property)) { + return ''; + } + + if (_.isArray(property)) { + return property; + } + + return [property]; +} + +module.exports = addDetails; diff --git a/middleware/geocodeJSON.js b/middleware/geocodeJSON.js index 414d216b..f4ee8c20 100644 --- a/middleware/geocodeJSON.js +++ b/middleware/geocodeJSON.js @@ -1,6 +1,6 @@ var url = require('url'); var extend = require('extend'); -var geojsonify = require('../helper/geojsonify').search; +var geojsonify = require('../helper/geojsonify'); /** * Returns a middleware function that converts elasticsearch @@ -72,7 +72,7 @@ function convertToGeocodeJSON(req, res, next, opts) { res.body.geocoding.timestamp = new Date().getTime(); // convert docs to geojson and merge with geocoding block - extend(res.body, geojsonify(res.data || [])); + extend(res.body, geojsonify(req.clean, res.data || [])); next(); } diff --git a/sanitiser/nearby.js b/sanitiser/nearby.js new file mode 100644 index 00000000..a32fe002 --- /dev/null +++ b/sanitiser/nearby.js @@ -0,0 +1,29 @@ + +var type_mapping = require('../helper/type_mapping'); +var sanitizeAll = require('../sanitiser/sanitizeAll'), + sanitizers = { + quattroshapes_deprecation: require('../sanitiser/_deprecate_quattroshapes'), + singleScalarParameters: require('../sanitiser/_single_scalar_parameters'), + layers: require('../sanitiser/_targets')('layers', type_mapping.layer_mapping), + sources: require('../sanitiser/_targets')('sources', type_mapping.source_mapping), + // depends on the layers and sources sanitisers, must be run after them + sources_and_layers: require('../sanitiser/_sources_and_layers'), + size: require('../sanitiser/_size')(/* use defaults*/), + private: require('../sanitiser/_flag_bool')('private', false), + geo_reverse: require('../sanitiser/_geo_reverse'), + boundary_country: require('../sanitiser/_boundary_country'), + categories: require('../sanitiser/_categories') + }; + +var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; + +// export sanitize for testing +module.exports.sanitize = sanitize; +module.exports.sanitiser_list = sanitizers; + +// middleware +module.exports.middleware = function( req, res, next ){ + sanitize( req, function( err, clean ){ + next(); + }); +}; diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index 8ec97396..00107af2 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -4,9 +4,9 @@ var geojsonify = require('../../../helper/geojsonify'); module.exports.tests = {}; module.exports.tests.interface = function(test, common) { - test('valid interface .search()', function(t) { - t.equal(typeof geojsonify.search, 'function', 'search is a function'); - t.equal(geojsonify.search.length, 1, 'accepts x arguments'); + test('valid interface', function(t) { + t.equal(typeof geojsonify, 'function', 'search is a function'); + t.equal(geojsonify.length, 2, 'accepts x arguments'); t.end(); }); }; @@ -30,14 +30,14 @@ module.exports.tests.earth = function(test, common) { test('earth', function(t) { t.doesNotThrow(function(){ - geojsonify.search( earth ); + geojsonify( {}, earth ); }); t.end(); }); }; -module.exports.tests.search = function(test, common) { +module.exports.tests.geojsonify = function(test, common) { var input = [ { @@ -222,8 +222,8 @@ module.exports.tests.search = function(test, common) { ] }; - test('geojsonify.search(doc)', function(t) { - var json = geojsonify.search( input ); + test('geojsonify(doc)', function(t) { + var json = geojsonify( {categories: 'foo'}, input ); t.deepEqual(json, expected, 'all docs mapped'); t.end(); @@ -370,7 +370,67 @@ module.exports.tests.search = function(test, common) { ] }; - var json = geojsonify.search( input ); + var json = geojsonify( {categories: 'foo'}, input ); + + t.deepEqual(json, expected, 'all wanted properties exposed'); + t.end(); + }); +}; + +module.exports.tests.categories = function (test, common) { + test('only set category if categories filter was used', function (t) { + var input = [ + { + '_id': '85816607', + 'bounding_box': { + 'min_lat': 40.6514712164, + 'max_lat': 40.6737320588, + 'min_lon': -73.8967895508, + 'max_lon': -73.8665771484 + }, + 'source': 'whosonfirst', + 'layer': 'neighbourhood', + 'center_point': { + 'lon': -73.881319, + 'lat': 40.663303 + }, + 'name': { + 'default': 'East New York' + }, + 'source_id': '85816607', + 'category': ['government'] + } + ]; + + var expected = { + 'type': 'FeatureCollection', + 'bbox': [-73.8967895508, 40.6514712164, -73.8665771484, 40.6737320588], + 'features': [ + { + 'type': 'Feature', + 'properties': { + 'id': '85816607', + 'gid': 'whosonfirst:neighbourhood:85816607', + 'layer': 'neighbourhood', + 'source': 'whosonfirst', + 'source_id': '85816607', + 'name': 'East New York', + 'category': ['government'], + 'label': 'East New York' + }, + 'bbox': [-73.8967895508,40.6514712164,-73.8665771484,40.6737320588], + 'geometry': { + 'type': 'Point', + 'coordinates': [ + -73.881319, + 40.663303 + ] + } + } + ] + }; + + var json = geojsonify( {categories: 'foo'}, input ); t.deepEqual(json, expected, 'all wanted properties exposed'); t.end(); diff --git a/test/unit/sanitiser/nearby.js b/test/unit/sanitiser/nearby.js new file mode 100644 index 00000000..b2d40b01 --- /dev/null +++ b/test/unit/sanitiser/nearby.js @@ -0,0 +1,204 @@ + +// @todo: refactor this test, it's pretty messy, brittle and hard to follow + +var reverse = require('../../../sanitiser/reverse'), + sanitize = reverse.sanitize, + middleware = reverse.middleware, + defaults = require('../../../query/reverse_defaults'), + defaultError = 'missing param \'lat\'', + defaultClean = { 'point.lat': 0, + 'point.lon': 0, + 'boundary.circle.lat': 0, + 'boundary.circle.lon': 0, + 'boundary.circle.radius': parseFloat(defaults['boundary:circle:radius']), + size: 10, + private: false + }; + +// these are the default values you would expect when no input params are specified. +// @todo: why is this different from $defaultClean? +var emptyClean = { private: false, size: 10 }; + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('sanitize interface', function(t) { + t.equal(typeof sanitize, 'function', 'sanitize is a function'); + t.equal(sanitize.length, 2, 'sanitize interface'); + t.end(); + }); + test('middleware interface', function(t) { + t.equal(typeof middleware, 'function', 'middleware is a function'); + t.equal(middleware.length, 3, 'sanitizee has a valid middleware'); + t.end(); + }); +}; + +module.exports.tests.sanitisers = function(test, common) { + test('check sanitiser list', function (t) { + var expected = ['quattroshapes_deprecation', 'singleScalarParameters', 'layers', + 'sources', 'sources_and_layers', 'size', 'private', 'geo_reverse', 'boundary_country', 'categories']; + t.deepEqual(Object.keys(reverse.sanitiser_list), expected); + t.end(); + }); +}; + +module.exports.tests.sanitize_lat = function(test, common) { + var lats = { + invalid: [], + valid: [ 0, 45, 90, -0, '0', '45', '90', -181, -120, -91, 91, 120, 181 ], + missing: ['', undefined, null] + }; + test('invalid lat', function(t) { + lats.invalid.forEach( function( lat ){ + var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.errors[0], 'invalid param \'point.lat\': must be >-90 and <90', lat + ' is an invalid latitude'); + t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); + }); + }); + t.end(); + }); + test('valid lat', function(t) { + lats.valid.forEach( function( lat ){ + var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; + sanitize(req, function(){ + var expected_lat = parseFloat( lat ); + t.deepEqual(req.errors, [], 'no errors'); + t.equal(req.clean['point.lat'], expected_lat, 'clean set correctly (' + lat + ')'); + }); + }); + t.end(); + }); + test('missing lat', function(t) { + lats.missing.forEach( function( lat ){ + var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.errors[0], 'missing param \'point.lat\'', 'latitude is a required field'); + t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); + }); + }); + t.end(); + }); +}; + +module.exports.tests.sanitize_lon = function(test, common) { + var lons = { + valid: [ -360, -181, 181, -180, -1, -0, 0, 45, 90, '-180', '0', '180' ], + missing: ['', undefined, null] + }; + test('valid lon', function(t) { + lons.valid.forEach( function( lon ){ + var req = { query: { 'point.lat': 0, 'point.lon': lon } }; + sanitize(req, function(){ + var expected_lon = parseFloat( lon ); + t.deepEqual(req.errors, [], 'no errors'); + t.equal(req.clean['point.lon'], expected_lon, 'clean set correctly (' + lon + ')'); + }); + }); + t.end(); + }); + test('missing lon', function(t) { + lons.missing.forEach( function( lon ){ + var req = { query: { 'point.lat': 0, 'point.lon': lon } }; + + // @todo: why is lat set? + var expected = { 'point.lat': 0, private: false, size: 10 }; + sanitize(req, function(){ + t.equal(req.errors[0], 'missing param \'point.lon\'', 'longitude is a required field'); + t.deepEqual(req.clean, expected, 'clean only has default values set'); + }); + }); + t.end(); + }); +}; + +module.exports.tests.sanitize_size = function(test, common) { + test('invalid size value', function(t) { + var req = { query: { size: 'a', 'point.lat': 0, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.clean.size, 10, 'default size set'); + t.end(); + }); + }); + test('below min size value', function(t) { + var req = { query: { size: -100, 'point.lat': 0, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.clean.size, 1, 'min size set'); + t.end(); + }); + }); + test('above max size value', function(t) { + var req = { query: { size: 9999, 'point.lat': 0, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.clean.size, 40, 'max size set'); + t.end(); + }); + }); +}; + +module.exports.tests.sanitize_private = function(test, common) { + var invalid_values = [null, -1, 123, NaN, 'abc']; + invalid_values.forEach(function(value) { + test('invalid private param ' + value, function(t) { + var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; + sanitize(req, function(){ + t.equal(req.clean.private, false, 'default private set (to false)'); + t.end(); + }); + }); + }); + + var valid_values = ['true', true, 1, '1']; + valid_values.forEach(function(value) { + test('valid private param ' + value, function(t) { + var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; + sanitize(req, function(){ + t.equal(req.clean.private, true, 'private set to true'); + t.end(); + }); + }); + }); + + var valid_false_values = ['false', false, 0]; + valid_false_values.forEach(function(value) { + test('test setting false explicitly ' + value, function(t) { + var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; + sanitize(req, function(){ + t.equal(req.clean.private, false, 'private set to false'); + t.end(); + }); + }); + }); + + test('test default behavior', function(t) { + var req = { query: { 'point.lat': 0, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.clean.private, false, 'private set to false'); + t.end(); + }); + }); +}; + +module.exports.tests.middleware_success = function(test, common) { + test('middleware success', function(t) { + var req = { query: { 'point.lat': 0, 'point.lon': 0 }}; + var next = function(){ + t.deepEqual(req.errors, [], 'no error message set'); + t.deepEqual(req.clean, defaultClean); + t.end(); + }; + middleware( req, undefined, next ); + }); +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('SANTIZE /reverse ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; From 127137f42792084f3163e2939f8580443ac96190 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Mon, 13 Jun 2016 14:05:32 -0400 Subject: [PATCH 12/22] Add category filter to reverse query --- query/reverse.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/query/reverse.js b/query/reverse.js index f937e0c0..930cba49 100644 --- a/query/reverse.js +++ b/query/reverse.js @@ -17,6 +17,7 @@ query.sort( peliasQuery.view.sort_distance ); query.filter( peliasQuery.view.boundary_circle ); query.filter( peliasQuery.view.sources ); query.filter( peliasQuery.view.layers ); +query.filter( peliasQuery.view.categories ); // -------------------------------- @@ -65,6 +66,11 @@ function generateQuery( clean ){ }); } + // categories + if (clean.categories) { + vs.var('input:categories', clean.categories); + } + return query.render( vs ); } From c38b67aeb4394d2b0bf80ab83077ba5ef3a52450 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Mon, 13 Jun 2016 14:07:41 -0400 Subject: [PATCH 13/22] Add nearby sanitizer and revert reverse to not have categories --- routes/v1.js | 20 ++++- sanitiser/nearby.js | 22 ++--- sanitiser/reverse.js | 3 +- test/unit/run.js | 1 + test/unit/sanitiser/nearby.js | 157 ++------------------------------- test/unit/sanitiser/reverse.js | 2 +- 6 files changed, 36 insertions(+), 169 deletions(-) diff --git a/routes/v1.js b/routes/v1.js index 9ef95856..7345310c 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -7,7 +7,8 @@ var sanitisers = { autocomplete: require('../sanitiser/autocomplete'), place: require('../sanitiser/place'), search: require('../sanitiser/search'), - reverse: require('../sanitiser/reverse') + reverse: require('../sanitiser/reverse'), + nearby: require('../sanitiser/nearby') }; /** ----------------------- middleware ------------------------ **/ @@ -101,6 +102,22 @@ function addRoutes(app, peliasConfig) { postProc.geocodeJSON(peliasConfig, base), postProc.sendJSON ]), + nearby: createRouter([ + sanitisers.nearby.middleware, + middleware.calcSize(), + controllers.search(undefined, reverseQuery), + postProc.distances('point.'), + // reverse confidence scoring depends on distance from origin + // so it must be calculated first + postProc.confidenceScoresReverse(), + postProc.dedupe(), + postProc.localNamingConventions(), + postProc.renamePlacenames(), + postProc.parseBoundingBox(), + postProc.normalizeParentIds(), + postProc.geocodeJSON(peliasConfig, base), + postProc.sendJSON + ]), place: createRouter([ sanitisers.place.middleware, controllers.place(peliasConfig), @@ -129,6 +146,7 @@ function addRoutes(app, peliasConfig) { app.get ( base + 'search', routers.search ); app.post( base + 'search', routers.search ); app.get ( base + 'reverse', routers.reverse ); + app.get ( base + 'nearby', routers.nearby ); } diff --git a/sanitiser/nearby.js b/sanitiser/nearby.js index a32fe002..109c1ef0 100644 --- a/sanitiser/nearby.js +++ b/sanitiser/nearby.js @@ -1,19 +1,11 @@ +var _ = require('lodash'); +var sanitizeAll = require('../sanitiser/sanitizeAll'); +var reverseSanitizers = require('./reverse').sanitiser_list; -var type_mapping = require('../helper/type_mapping'); -var sanitizeAll = require('../sanitiser/sanitizeAll'), - sanitizers = { - quattroshapes_deprecation: require('../sanitiser/_deprecate_quattroshapes'), - singleScalarParameters: require('../sanitiser/_single_scalar_parameters'), - layers: require('../sanitiser/_targets')('layers', type_mapping.layer_mapping), - sources: require('../sanitiser/_targets')('sources', type_mapping.source_mapping), - // depends on the layers and sources sanitisers, must be run after them - sources_and_layers: require('../sanitiser/_sources_and_layers'), - size: require('../sanitiser/_size')(/* use defaults*/), - private: require('../sanitiser/_flag_bool')('private', false), - geo_reverse: require('../sanitiser/_geo_reverse'), - boundary_country: require('../sanitiser/_boundary_country'), - categories: require('../sanitiser/_categories') - }; +// add categories to the sanitizer list +var sanitizers = _.merge({}, reverseSanitizers, { + categories: require('../sanitiser/_categories') +}); var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; diff --git a/sanitiser/reverse.js b/sanitiser/reverse.js index a32fe002..b994455a 100644 --- a/sanitiser/reverse.js +++ b/sanitiser/reverse.js @@ -11,8 +11,7 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), size: require('../sanitiser/_size')(/* use defaults*/), private: require('../sanitiser/_flag_bool')('private', false), geo_reverse: require('../sanitiser/_geo_reverse'), - boundary_country: require('../sanitiser/_boundary_country'), - categories: require('../sanitiser/_categories') + boundary_country: require('../sanitiser/_boundary_country') }; var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; diff --git a/test/unit/run.js b/test/unit/run.js index b6e40c71..d6c65b56 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -52,6 +52,7 @@ var tests = [ require('./sanitiser/_tokenizer'), require('./sanitiser/_deprecate_quattroshapes'), require('./sanitiser/_categories'), + require('./sanitiser/nearby'), require('./src/backend'), require('./sanitiser/autocomplete'), require('./sanitiser/place'), diff --git a/test/unit/sanitiser/nearby.js b/test/unit/sanitiser/nearby.js index b2d40b01..1441b903 100644 --- a/test/unit/sanitiser/nearby.js +++ b/test/unit/sanitiser/nearby.js @@ -1,12 +1,10 @@ -// @todo: refactor this test, it's pretty messy, brittle and hard to follow +var nearby = require('../../../sanitiser/nearby'); +var defaults = require('../../../query/reverse_defaults'); +var sanitize = nearby.sanitize; +var middleware = nearby.middleware; -var reverse = require('../../../sanitiser/reverse'), - sanitize = reverse.sanitize, - middleware = reverse.middleware, - defaults = require('../../../query/reverse_defaults'), - defaultError = 'missing param \'lat\'', - defaultClean = { 'point.lat': 0, +var defaultClean = { 'point.lat': 0, 'point.lon': 0, 'boundary.circle.lat': 0, 'boundary.circle.lon': 0, @@ -15,10 +13,6 @@ var reverse = require('../../../sanitiser/reverse'), private: false }; -// these are the default values you would expect when no input params are specified. -// @todo: why is this different from $defaultClean? -var emptyClean = { private: false, size: 10 }; - module.exports.tests = {}; module.exports.tests.interface = function(test, common) { @@ -38,148 +32,11 @@ module.exports.tests.sanitisers = function(test, common) { test('check sanitiser list', function (t) { var expected = ['quattroshapes_deprecation', 'singleScalarParameters', 'layers', 'sources', 'sources_and_layers', 'size', 'private', 'geo_reverse', 'boundary_country', 'categories']; - t.deepEqual(Object.keys(reverse.sanitiser_list), expected); - t.end(); - }); -}; - -module.exports.tests.sanitize_lat = function(test, common) { - var lats = { - invalid: [], - valid: [ 0, 45, 90, -0, '0', '45', '90', -181, -120, -91, 91, 120, 181 ], - missing: ['', undefined, null] - }; - test('invalid lat', function(t) { - lats.invalid.forEach( function( lat ){ - var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; - sanitize(req, function(){ - t.equal(req.errors[0], 'invalid param \'point.lat\': must be >-90 and <90', lat + ' is an invalid latitude'); - t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); - }); - }); - t.end(); - }); - test('valid lat', function(t) { - lats.valid.forEach( function( lat ){ - var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; - sanitize(req, function(){ - var expected_lat = parseFloat( lat ); - t.deepEqual(req.errors, [], 'no errors'); - t.equal(req.clean['point.lat'], expected_lat, 'clean set correctly (' + lat + ')'); - }); - }); - t.end(); - }); - test('missing lat', function(t) { - lats.missing.forEach( function( lat ){ - var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; - sanitize(req, function(){ - t.equal(req.errors[0], 'missing param \'point.lat\'', 'latitude is a required field'); - t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); - }); - }); + t.deepEqual(Object.keys(nearby.sanitiser_list), expected); t.end(); }); }; -module.exports.tests.sanitize_lon = function(test, common) { - var lons = { - valid: [ -360, -181, 181, -180, -1, -0, 0, 45, 90, '-180', '0', '180' ], - missing: ['', undefined, null] - }; - test('valid lon', function(t) { - lons.valid.forEach( function( lon ){ - var req = { query: { 'point.lat': 0, 'point.lon': lon } }; - sanitize(req, function(){ - var expected_lon = parseFloat( lon ); - t.deepEqual(req.errors, [], 'no errors'); - t.equal(req.clean['point.lon'], expected_lon, 'clean set correctly (' + lon + ')'); - }); - }); - t.end(); - }); - test('missing lon', function(t) { - lons.missing.forEach( function( lon ){ - var req = { query: { 'point.lat': 0, 'point.lon': lon } }; - - // @todo: why is lat set? - var expected = { 'point.lat': 0, private: false, size: 10 }; - sanitize(req, function(){ - t.equal(req.errors[0], 'missing param \'point.lon\'', 'longitude is a required field'); - t.deepEqual(req.clean, expected, 'clean only has default values set'); - }); - }); - t.end(); - }); -}; - -module.exports.tests.sanitize_size = function(test, common) { - test('invalid size value', function(t) { - var req = { query: { size: 'a', 'point.lat': 0, 'point.lon': 0 } }; - sanitize(req, function(){ - t.equal(req.clean.size, 10, 'default size set'); - t.end(); - }); - }); - test('below min size value', function(t) { - var req = { query: { size: -100, 'point.lat': 0, 'point.lon': 0 } }; - sanitize(req, function(){ - t.equal(req.clean.size, 1, 'min size set'); - t.end(); - }); - }); - test('above max size value', function(t) { - var req = { query: { size: 9999, 'point.lat': 0, 'point.lon': 0 } }; - sanitize(req, function(){ - t.equal(req.clean.size, 40, 'max size set'); - t.end(); - }); - }); -}; - -module.exports.tests.sanitize_private = function(test, common) { - var invalid_values = [null, -1, 123, NaN, 'abc']; - invalid_values.forEach(function(value) { - test('invalid private param ' + value, function(t) { - var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; - sanitize(req, function(){ - t.equal(req.clean.private, false, 'default private set (to false)'); - t.end(); - }); - }); - }); - - var valid_values = ['true', true, 1, '1']; - valid_values.forEach(function(value) { - test('valid private param ' + value, function(t) { - var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; - sanitize(req, function(){ - t.equal(req.clean.private, true, 'private set to true'); - t.end(); - }); - }); - }); - - var valid_false_values = ['false', false, 0]; - valid_false_values.forEach(function(value) { - test('test setting false explicitly ' + value, function(t) { - var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; - sanitize(req, function(){ - t.equal(req.clean.private, false, 'private set to false'); - t.end(); - }); - }); - }); - - test('test default behavior', function(t) { - var req = { query: { 'point.lat': 0, 'point.lon': 0 } }; - sanitize(req, function(){ - t.equal(req.clean.private, false, 'private set to false'); - t.end(); - }); - }); -}; - module.exports.tests.middleware_success = function(test, common) { test('middleware success', function(t) { var req = { query: { 'point.lat': 0, 'point.lon': 0 }}; @@ -195,7 +52,7 @@ module.exports.tests.middleware_success = function(test, common) { module.exports.all = function (tape, common) { function test(name, testFunction) { - return tape('SANTIZE /reverse ' + name, testFunction); + return tape('SANTIZE /nearby ' + name, testFunction); } for( var testCase in module.exports.tests ){ diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js index aabb84e9..089e1aaf 100644 --- a/test/unit/sanitiser/reverse.js +++ b/test/unit/sanitiser/reverse.js @@ -37,7 +37,7 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.sanitisers = function(test, common) { test('check sanitiser list', function (t) { var expected = ['quattroshapes_deprecation', 'singleScalarParameters', 'layers', - 'sources', 'sources_and_layers', 'size', 'private', 'geo_reverse', 'boundary_country', 'categories']; + 'sources', 'sources_and_layers', 'size', 'private', 'geo_reverse', 'boundary_country']; t.deepEqual(Object.keys(reverse.sanitiser_list), expected); t.end(); }); From b0aab1afd3393eba5d7faacc66ee75def0d25524 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Fri, 29 Jul 2016 11:37:39 -0400 Subject: [PATCH 14/22] Fixed broken test due to rebase with changes from master. --- .../fixture/search_with_category_filtering.js | 139 +++++++++--------- 1 file changed, 66 insertions(+), 73 deletions(-) diff --git a/test/unit/fixture/search_with_category_filtering.js b/test/unit/fixture/search_with_category_filtering.js index 57014821..ca1f26bb 100644 --- a/test/unit/fixture/search_with_category_filtering.js +++ b/test/unit/fixture/search_with_category_filtering.js @@ -1,93 +1,86 @@ - module.exports = { 'query': { - 'filtered': { - 'query': { - 'bool': { - 'must': [{ + 'bool': { + 'must': [{ + 'match': { + 'name.default': { + 'query': 'test', + 'boost': 1, + 'analyzer': 'peliasQueryFullToken' + } + } + }], + 'should': [{ + 'match': { + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'boost': 1, + 'slop': 2 + } + } + }, { + 'function_score': { + 'query': { 'match': { - 'name.default': { + 'phrase.default': { 'query': 'test', - 'boost': 1, - 'analyzer': 'peliasIndexOneEdgeGram' + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'slop': 2, + 'boost': 1 } } - }], - 'should': [{ + }, + 'max_boost': 20, + 'score_mode': 'first', + 'boost_mode': 'replace', + 'functions': [{ + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'popularity', + 'missing': 1 + }, + 'weight': 1 + }] + } + }, { + 'function_score': { + 'query': { 'match': { 'phrase.default': { 'query': 'test', 'analyzer': 'peliasPhrase', 'type': 'phrase', - 'boost': 1, - 'slop': 2 + 'slop': 2, + 'boost': 1 } } - }, { - 'function_score': { - 'query': { - 'match': { - 'phrase.default': { - 'query': 'test', - 'analyzer': 'peliasPhrase', - 'type': 'phrase', - 'slop': 2, - 'boost': 1 - } - } - }, - 'max_boost': 20, - 'score_mode': 'first', - 'boost_mode': 'replace', - 'functions': [{ - 'field_value_factor': { - 'modifier': 'log1p', - 'field': 'popularity', - 'missing': 1 - }, - 'weight': 1 - }] - } - }, { - 'function_score': { - 'query': { - 'match': { - 'phrase.default': { - 'query': 'test', - 'analyzer': 'peliasPhrase', - 'type': 'phrase', - 'slop': 2, - 'boost': 1 - } - } - }, - 'max_boost': 20, - 'score_mode': 'first', - 'boost_mode': 'replace', - 'functions': [{ - 'field_value_factor': { - 'modifier': 'log1p', - 'field': 'population', - 'missing': 1 - }, - 'weight': 2 - }] - } + }, + 'max_boost': 20, + 'score_mode': 'first', + 'boost_mode': 'replace', + 'functions': [{ + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'population', + 'missing': 1 + }, + 'weight': 2 }] } - }, - 'filter': { - 'bool': { - 'must': [{ - 'terms': { - 'category': ['retail','food'] - } - }] + }], + 'filter': [{ + 'terms': { + 'category': ['retail', 'food'] } - } + }] } }, - 'sort': [ '_score' ], 'size': 20, - 'track_scores': true + 'track_scores': true, + 'sort': [ + '_score' + ] }; From afc585bb8747ad4cc6c6a24ef4f3e6e8cde9dbcf Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Mon, 1 Aug 2016 21:28:04 -0400 Subject: [PATCH 15/22] pass config to controller in nearby router setup --- routes/v1.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routes/v1.js b/routes/v1.js index 7345310c..674c1f5c 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -105,7 +105,7 @@ function addRoutes(app, peliasConfig) { nearby: createRouter([ sanitisers.nearby.middleware, middleware.calcSize(), - controllers.search(undefined, reverseQuery), + controllers.search(peliasConfig, undefined, reverseQuery), postProc.distances('point.'), // reverse confidence scoring depends on distance from origin // so it must be calculated first From eddc51213330c2d690bd5e1320f5a817478e3193 Mon Sep 17 00:00:00 2001 From: greenkeeperio-bot Date: Mon, 8 Aug 2016 13:35:13 -0700 Subject: [PATCH 16/22] chore(package): update pelias-model to version 4.2.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index fad97cd6..7f13d323 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "pelias-categories": "1.0.0", "pelias-config": "2.1.0", "pelias-logger": "0.0.8", - "pelias-model": "4.1.0", + "pelias-model": "4.2.0", "pelias-query": "8.3.0", "pelias-text-analyzer": "1.3.0", "stats-lite": "2.0.3", From dc6070bc7802baab5d76b71a4a071fc299a79bd9 Mon Sep 17 00:00:00 2001 From: greenkeeperio-bot Date: Thu, 11 Aug 2016 14:14:44 -0700 Subject: [PATCH 17/22] chore(package): update pelias-query to version 8.4.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 7f13d323..efad21b3 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,7 @@ "pelias-config": "2.1.0", "pelias-logger": "0.0.8", "pelias-model": "4.2.0", - "pelias-query": "8.3.0", + "pelias-query": "8.4.0", "pelias-text-analyzer": "1.3.0", "stats-lite": "2.0.3", "through2": "2.0.1" From 31148dacdb1aae33f7a09313e1655357f2aa6205 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 11 Aug 2016 17:36:18 -0400 Subject: [PATCH 18/22] Remove Node.js v5 from TravisCI --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 3f462ba9..3b4e0475 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,7 +8,6 @@ notifications: node_js: - 0.12 - 4 - - 5 - 6 matrix: fast_finish: true From 8615eff88462e897cb0c14dc86849b668a1284f1 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Thu, 11 Aug 2016 17:36:24 -0400 Subject: [PATCH 19/22] Require Node.js v6 tests to pass --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 3b4e0475..7c327439 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,7 +12,6 @@ node_js: matrix: fast_finish: true allow_failures: - - node_js: 6 env: global: - CXX=g++-4.8 From 8c12452fff099de601f73081f6ff8d16b6260a5d Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Fri, 12 Aug 2016 11:57:24 -0400 Subject: [PATCH 20/22] added hardcoded values in tests where text-analyzer was being used before --- test/unit/query/search.js | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/test/unit/query/search.js b/test/unit/query/search.js index 61d00984..5abb35f7 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -1,5 +1,4 @@ var generate = require('../../../query/search'); -var text_analyzer = require('pelias-text-analyzer'); module.exports.tests = {}; @@ -88,11 +87,17 @@ 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({ text: address, + var query = generate({ text: '123 main st new york ny 10010 US', layers: [ 'address', 'venue', 'country', 'region', 'county', 'neighbourhood', 'locality', 'localadmin' ], querySize: 10, - parsed_text: text_analyzer.parse(address), + parsed_text: { + number: '123', + street: 'main st', + state: 'NY', + country: 'USA', + postalcode: '10010', + regions: [ 'new york' ] + } }); var compiled = JSON.parse( JSON.stringify( query ) ); @@ -103,11 +108,14 @@ 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({ text: partial_address, + var query = generate({ text: 'soho grand, new york', layers: [ 'address', 'venue', 'country', 'region', 'county', 'neighbourhood', 'locality', 'localadmin' ], querySize: 10, - parsed_text: text_analyzer.parse(partial_address), + parsed_text: { name: 'soho grand', + state: 'NY', + regions: [ 'soho grand' ], + admin_parts: 'new york' + } }); var compiled = JSON.parse( JSON.stringify( query ) ); @@ -118,11 +126,14 @@ 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({ text: partial_address, + var query = generate({ text: '1 water st manhattan ny', layers: [ 'address', 'venue', 'country', 'region', 'county', 'neighbourhood', 'locality', 'localadmin' ], querySize: 10, - parsed_text: text_analyzer.parse(partial_address), + parsed_text: { number: '1', + street: 'water st', + state: 'NY', + regions: [ 'manhattan' ] + }, }); var compiled = JSON.parse( JSON.stringify( query ) ); From c5e5bbcf56344f5a642fe6dd529b8b3dd54fa1bb Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Fri, 12 Aug 2016 16:29:26 -0400 Subject: [PATCH 21/22] Add boundary.country filter to /v1/autocomplete --- query/autocomplete.js | 8 +++ sanitiser/autocomplete.js | 1 + .../fixture/autocomplete_boundary_country.js | 68 +++++++++++++++++++ test/unit/query/autocomplete.js | 16 +++++ test/unit/sanitiser/autocomplete.js | 2 +- 5 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 test/unit/fixture/autocomplete_boundary_country.js diff --git a/query/autocomplete.js b/query/autocomplete.js index 4233969a..33f394f5 100644 --- a/query/autocomplete.js +++ b/query/autocomplete.js @@ -22,6 +22,7 @@ var query = new peliasQuery.layout.FilteredBooleanQuery(); // mandatory matches query.score( views.phrase_first_tokens_only, 'must' ); query.score( views.ngrams_last_token_only, 'must' ); +query.score( peliasQuery.view.boundary_country, 'must' ); // address components query.score( peliasQuery.view.address('housenumber') ); @@ -69,6 +70,13 @@ function generateQuery( clean ){ vs.var( 'layers', clean.layers); } + // boundary country + if( check.string(clean['boundary.country']) ){ + vs.set({ + 'boundary:country': clean['boundary.country'] + }); + } + // pass the input tokens to the views so they can choose which tokens // are relevant for their specific function. if( check.array( clean.tokens ) ){ diff --git a/sanitiser/autocomplete.js b/sanitiser/autocomplete.js index 66bc0802..a7ee68f6 100644 --- a/sanitiser/autocomplete.js +++ b/sanitiser/autocomplete.js @@ -12,6 +12,7 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), sources_and_layers: require('../sanitiser/_sources_and_layers'), private: require('../sanitiser/_flag_bool')('private', false), geo_autocomplete: require('../sanitiser/_geo_autocomplete'), + boundary_country: require('../sanitiser/_boundary_country'), categories: require('../sanitiser/_categories') }; diff --git a/test/unit/fixture/autocomplete_boundary_country.js b/test/unit/fixture/autocomplete_boundary_country.js new file mode 100644 index 00000000..f0ac8fb7 --- /dev/null +++ b/test/unit/fixture/autocomplete_boundary_country.js @@ -0,0 +1,68 @@ + +module.exports = { + 'query': { + 'bool': { + 'must': [{ + 'constant_score': { + 'query': { + 'match': { + 'name.default': { + 'analyzer': 'peliasQueryPartialToken', + 'boost': 100, + 'query': 'test', + 'type': 'phrase', + 'operator': 'and', + 'slop': 3 + } + } + } + } + }, { + 'match': { + 'parent.country_a': { + 'analyzer': 'standard', + 'query': 'ABC' + } + } + }], + 'should':[{ + 'function_score': { + 'query': { + 'match_all': {} + }, + 'max_boost': 20, + 'score_mode': 'first', + 'boost_mode': 'replace', + 'functions': [{ + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'popularity', + 'missing': 1 + }, + 'weight': 1 + }] + } + },{ + 'function_score': { + 'query': { + 'match_all': {} + }, + 'max_boost': 20, + 'score_mode': 'first', + 'boost_mode': 'replace', + 'functions': [{ + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'population', + 'missing': 1 + }, + 'weight': 3 + }] + } + }] + } + }, + 'sort': [ '_score' ], + 'size': 20, + 'track_scores': true +}; diff --git a/test/unit/query/autocomplete.js b/test/unit/query/autocomplete.js index 676549fb..437142f3 100644 --- a/test/unit/query/autocomplete.js +++ b/test/unit/query/autocomplete.js @@ -164,6 +164,22 @@ module.exports.tests.query = function(test, common) { t.deepEqual(compiled, expected, 'autocomplete_single_character_street'); t.end(); }); + + test('valid boundary.country search', function(t) { + var query = generate({ + text: 'test', + tokens: ['test'], + tokens_complete: [], + tokens_incomplete: ['test'], + 'boundary.country': 'ABC' + }); + + var compiled = JSON.parse( JSON.stringify( query ) ); + var expected = require('../fixture/autocomplete_boundary_country'); + + t.deepEqual(compiled, expected, 'autocomplete: valid boundary.country query'); + t.end(); + }); }; module.exports.all = function (tape, common) { diff --git a/test/unit/sanitiser/autocomplete.js b/test/unit/sanitiser/autocomplete.js index 6581c689..289bf774 100644 --- a/test/unit/sanitiser/autocomplete.js +++ b/test/unit/sanitiser/autocomplete.js @@ -6,7 +6,7 @@ module.exports.tests.sanitisers = function(test, common) { test('check sanitiser list', function (t) { var expected = [ 'singleScalarParameters', 'text', 'tokenizer', 'size', 'layers', 'sources', - 'sources_and_layers', 'private', 'geo_autocomplete', 'categories' + 'sources_and_layers', 'private', 'geo_autocomplete', 'boundary_country', 'categories' ]; t.deepEqual(Object.keys(autocomplete.sanitiser_list), expected); t.end(); From f863fb84684c91cfcc243c727ea1f5b64b697562 Mon Sep 17 00:00:00 2001 From: greenkeeperio-bot Date: Wed, 17 Aug 2016 13:18:51 -0700 Subject: [PATCH 22/22] chore(package): update pelias-query to version 8.5.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index efad21b3..31c8d36f 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,7 @@ "pelias-config": "2.1.0", "pelias-logger": "0.0.8", "pelias-model": "4.2.0", - "pelias-query": "8.4.0", + "pelias-query": "8.5.0", "pelias-text-analyzer": "1.3.0", "stats-lite": "2.0.3", "through2": "2.0.1"