From 66bf2bb46bf365e7ec6c06a22c7d14e3e6c8fcad Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 14 Sep 2015 13:38:44 -0400 Subject: [PATCH 1/7] added sanitizer for boundary country parameter sets to undefined if input country not found as ISO2/3 --- package.json | 1 + sanitiser/_boundary_country.js | 40 +++++++++++ test/unit/run.js | 1 + test/unit/sanitiser/_boundary_country.js | 88 ++++++++++++++++++++++++ 4 files changed, 130 insertions(+) create mode 100644 sanitiser/_boundary_country.js create mode 100644 test/unit/sanitiser/_boundary_country.js diff --git a/package.json b/package.json index 8018919c..3acd057d 100644 --- a/package.json +++ b/package.json @@ -45,6 +45,7 @@ "geolib": "^2.0.18", "geopipes-elasticsearch-backend": "^0.2.0", "lodash": "^3.10.1", + "iso3166-1": "^0.2.3", "markdown": "0.5.0", "microtime": "1.4.0", "morgan": "1.5.2", diff --git a/sanitiser/_boundary_country.js b/sanitiser/_boundary_country.js new file mode 100644 index 00000000..05760f73 --- /dev/null +++ b/sanitiser/_boundary_country.js @@ -0,0 +1,40 @@ +var isObject = require('is-object'); +var iso3166 = require('iso3166-1'); + +function sanitize(raw, clean) { + // error & warning messages + var messages = { errors: [], warnings: [] }; + + // init clean.boundary (if not already init) + clean.boundary = clean.boundary || {}; + + if (raw.boundary && raw.boundary.country) { + var country = raw.boundary.country + + if (typeof country !== 'string') { + messages.warnings.push('boundary.country is not a string'); + clean.boundary.country = undefined; + } + else if (!containsIsoCode(country.toUpperCase())) { + messages.warnings.push(country + ' is not a valid ISO2/ISO3 country code'); + clean.boundary.country = undefined; + } + else { + clean.boundary.country = iso3166.to2(country.toUpperCase()); + } + + } else { + clean.boundary.country = undefined; + } + + return messages; + +} + +function containsIsoCode(isoCode) { + return iso3166.list().filter(function(row) { + return row.alpha2 === isoCode || row.alpha3 === isoCode; + }).length > 0; +} + +module.exports = sanitize; diff --git a/test/unit/run.js b/test/unit/run.js index 761d2fef..6d86509b 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -9,6 +9,7 @@ var tests = [ require('./service/mget'), require('./service/search'), require('./sanitiser/_sources'), + require('./sanitiser/_boundary_country'), require('./sanitiser/search'), require('./sanitiser/_layers'), require('./sanitiser/reverse'), diff --git a/test/unit/sanitiser/_boundary_country.js b/test/unit/sanitiser/_boundary_country.js new file mode 100644 index 00000000..68816f3d --- /dev/null +++ b/test/unit/sanitiser/_boundary_country.js @@ -0,0 +1,88 @@ +var sanitize = require('../../../sanitiser/_boundary_country'); + +module.exports.tests = {}; + +module.exports.tests.sanitize_boundary_country = function(test, common) { + test('raw w/o boundary should set boundary.country undefined', function(t) { + var raw = { }; + var clean = {}; + var errorsAndWarnings = sanitize(raw, clean); + t.equals(clean.boundary.country, undefined, 'should be undefined'); + t.deepEquals(errorsAndWarnings, { errors: [], warnings: [] }, 'no warnings or errors'); + t.end(); + }); + + test('boundary w/o country in raw should leave boundary.country undefined', function(t) { + var raw = { boundary: {} }; + var clean = {}; + var errorsAndWarnings = sanitize(raw, clean); + t.equals(clean.boundary.country, undefined, 'should be undefined'); + t.deepEquals(errorsAndWarnings, { errors: [], warnings: [] }, 'no warnings or errors'); + t.end(); + }); + + test('boundary.country explicitly undefined in raw should leave boundary.country undefined', function(t) { + var raw = { boundary: {country: undefined } }; + var clean = {}; + var errorsAndWarnings = sanitize(raw, clean); + t.equals(clean.boundary.country, undefined, 'should be undefined'); + t.deepEquals(errorsAndWarnings, { errors: [], warnings: [] }, 'no warnings or errors'); + t.end(); + }); + + test('non-string boundary.country should set boundary.country to undefined and return warning', function(t) { + var raw = { boundary: {country: ['this isn\'t a string primitive'] } }; + var clean = {}; + var errorsAndWarnings = sanitize(raw, clean); + t.equals(clean.boundary.country, undefined, 'should be undefined'); + t.deepEquals(errorsAndWarnings, { errors: [], warnings: ['boundary.country is not a string'] }, 'non-string country warning'); + t.end(); + }); + + test('iso2 boundary.country in raw should set boundary.country to ISO2 uppercased', function(t) { + var raw = { boundary: {country: 'aq'} }; + var clean = {}; + var errorsAndWarnings = sanitize(raw, clean); + t.equals(clean.boundary.country, 'AQ', 'should be uppercased ISO2'); + t.deepEquals(errorsAndWarnings, { errors: [], warnings: [] }, 'no warnings or errors'); + t.end(); + }); + + test('iso3 boundary.country in raw should set boundary.country to matching ISO2 uppercased', function(t) { + var raw = { boundary: {country: 'aTa'} }; + var clean = {}; + var errorsAndWarnings = sanitize(raw, clean); + t.equals(clean.boundary.country, 'AQ', 'should be uppercased ISO2'); + t.deepEquals(errorsAndWarnings, { errors: [], warnings: [] }, 'no warnings or errors'); + t.end(); + }); + + test('unknown 2-character boundary.country should set boundary.country to undefined', function(t) { + var raw = { boundary: {country: 'zq'} }; + var clean = {}; + var errorsAndWarnings = sanitize(raw, clean); + t.equals(clean.boundary.country, undefined, 'should be undefined'); + t.deepEquals(errorsAndWarnings, { errors: [], warnings: ['zq is not a valid ISO2/ISO3 country code'] }, 'country not found warning`'); + t.end(); + }); + + test('unknown 3-character boundary.country should set boundary.country to undefined', function(t) { + var raw = { boundary: {country: 'zqx'} }; + var clean = {}; + var errorsAndWarnings = sanitize(raw, clean); + t.equals(clean.boundary.country, undefined, 'should be undefined'); + t.deepEquals(errorsAndWarnings, { errors: [], warnings: ['zqx is not a valid ISO2/ISO3 country code'] }, 'country not found warning`'); + t.end(); + }); + +}; + +module.exports.all = function (tape, common) { + function test(name, testFunction) { + return tape('SANTIZE _boundary_country ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; From d07a50168228ae03ee49ec03f6ec03794bab8dd3 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 14 Sep 2015 14:20:16 -0400 Subject: [PATCH 2/7] requirements updates set boundary country to iso3 instead of iso2 set to undefined if boundary.country input is not a string --- sanitiser/_boundary_country.js | 6 +++--- sanitiser/search.js | 4 ++-- test/unit/sanitiser/_boundary_country.js | 8 ++++---- test/unit/sanitiser/search.js | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/sanitiser/_boundary_country.js b/sanitiser/_boundary_country.js index 05760f73..2a586252 100644 --- a/sanitiser/_boundary_country.js +++ b/sanitiser/_boundary_country.js @@ -20,7 +20,7 @@ function sanitize(raw, clean) { clean.boundary.country = undefined; } else { - clean.boundary.country = iso3166.to2(country.toUpperCase()); + clean.boundary.country = iso3166.to3(country.toUpperCase()); } } else { @@ -32,9 +32,9 @@ function sanitize(raw, clean) { } function containsIsoCode(isoCode) { - return iso3166.list().filter(function(row) { + return iso3166.list().some(function(row) { return row.alpha2 === isoCode || row.alpha3 === isoCode; - }).length > 0; + }); } module.exports = sanitize; diff --git a/sanitiser/search.js b/sanitiser/search.js index dd042761..2c867b54 100644 --- a/sanitiser/search.js +++ b/sanitiser/search.js @@ -7,7 +7,8 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), sources: require('../sanitiser/_targets')('sources', require( '../query/sources' )), details: require('../sanitiser/_details'), geo_search: require('../sanitiser/_geo_search'), - categories: require('../sanitiser/_categories') + categories: require('../sanitiser/_categories'), + boundary_country: require('../sanitiser/_boundary_country'), }; var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; @@ -26,4 +27,3 @@ module.exports.middleware = function( req, res, next ){ next(); }); }; - diff --git a/test/unit/sanitiser/_boundary_country.js b/test/unit/sanitiser/_boundary_country.js index 68816f3d..de70cb8b 100644 --- a/test/unit/sanitiser/_boundary_country.js +++ b/test/unit/sanitiser/_boundary_country.js @@ -39,20 +39,20 @@ module.exports.tests.sanitize_boundary_country = function(test, common) { t.end(); }); - test('iso2 boundary.country in raw should set boundary.country to ISO2 uppercased', function(t) { + test('iso2 boundary.country in raw should set boundary.country to ISO3 uppercased', function(t) { var raw = { boundary: {country: 'aq'} }; var clean = {}; var errorsAndWarnings = sanitize(raw, clean); - t.equals(clean.boundary.country, 'AQ', 'should be uppercased ISO2'); + t.equals(clean.boundary.country, 'ATA', 'should be uppercased ISO3'); t.deepEquals(errorsAndWarnings, { errors: [], warnings: [] }, 'no warnings or errors'); t.end(); }); - test('iso3 boundary.country in raw should set boundary.country to matching ISO2 uppercased', function(t) { + test('iso3 boundary.country in raw should set boundary.country to matching ISO3 uppercased', function(t) { var raw = { boundary: {country: 'aTa'} }; var clean = {}; var errorsAndWarnings = sanitize(raw, clean); - t.equals(clean.boundary.country, 'AQ', 'should be uppercased ISO2'); + t.equals(clean.boundary.country, 'ATA', 'should be uppercased ISO3'); t.deepEquals(errorsAndWarnings, { errors: [], warnings: [] }, 'no warnings or errors'); t.end(); }); diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index e6c39284..0ed32e4c 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -32,7 +32,7 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.sanitisers = function(test, common) { test('check sanitiser list', function (t) { - var expected = ['text', 'size', 'layers', 'sources', 'details', 'geo_search', 'categories' ]; + var expected = ['text', 'size', 'layers', 'sources', 'details', 'geo_search', 'categories', 'boundary_country' ]; t.deepEqual(Object.keys(search.sanitiser_list), expected); t.end(); }); @@ -77,7 +77,7 @@ module.exports.tests.sanitise_valid_text = function(test, common) { module.exports.tests.sanitize_text_with_delim = function(test, common) { var texts = [ 'a,bcd', '123 main st, admin1', ',,,', ' ' ]; - test('valid texts with a comma', function(t) { + 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 )); From 78c335d072149e0ec594723e1a74f2796abc8b00 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 14 Sep 2015 16:11:22 -0400 Subject: [PATCH 3/7] modified sanitizer to read from flattened parameter structure --- sanitiser/_boundary_country.js | 4 ++-- test/unit/sanitiser/_boundary_country.js | 21 ++++++--------------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/sanitiser/_boundary_country.js b/sanitiser/_boundary_country.js index 2a586252..f2824cec 100644 --- a/sanitiser/_boundary_country.js +++ b/sanitiser/_boundary_country.js @@ -8,8 +8,8 @@ function sanitize(raw, clean) { // init clean.boundary (if not already init) clean.boundary = clean.boundary || {}; - if (raw.boundary && raw.boundary.country) { - var country = raw.boundary.country + if (raw['boundary.country']) { + var country = raw['boundary.country']; if (typeof country !== 'string') { messages.warnings.push('boundary.country is not a string'); diff --git a/test/unit/sanitiser/_boundary_country.js b/test/unit/sanitiser/_boundary_country.js index de70cb8b..66b07d7c 100644 --- a/test/unit/sanitiser/_boundary_country.js +++ b/test/unit/sanitiser/_boundary_country.js @@ -12,17 +12,8 @@ module.exports.tests.sanitize_boundary_country = function(test, common) { t.end(); }); - test('boundary w/o country in raw should leave boundary.country undefined', function(t) { - var raw = { boundary: {} }; - var clean = {}; - var errorsAndWarnings = sanitize(raw, clean); - t.equals(clean.boundary.country, undefined, 'should be undefined'); - t.deepEquals(errorsAndWarnings, { errors: [], warnings: [] }, 'no warnings or errors'); - t.end(); - }); - test('boundary.country explicitly undefined in raw should leave boundary.country undefined', function(t) { - var raw = { boundary: {country: undefined } }; + var raw = { 'boundary.country': undefined }; var clean = {}; var errorsAndWarnings = sanitize(raw, clean); t.equals(clean.boundary.country, undefined, 'should be undefined'); @@ -31,7 +22,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) { }); test('non-string boundary.country should set boundary.country to undefined and return warning', function(t) { - var raw = { boundary: {country: ['this isn\'t a string primitive'] } }; + var raw = { 'boundary.country': ['this isn\'t a string primitive'] }; var clean = {}; var errorsAndWarnings = sanitize(raw, clean); t.equals(clean.boundary.country, undefined, 'should be undefined'); @@ -40,7 +31,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) { }); test('iso2 boundary.country in raw should set boundary.country to ISO3 uppercased', function(t) { - var raw = { boundary: {country: 'aq'} }; + var raw = { 'boundary.country': 'aq' }; var clean = {}; var errorsAndWarnings = sanitize(raw, clean); t.equals(clean.boundary.country, 'ATA', 'should be uppercased ISO3'); @@ -49,7 +40,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) { }); test('iso3 boundary.country in raw should set boundary.country to matching ISO3 uppercased', function(t) { - var raw = { boundary: {country: 'aTa'} }; + var raw = { 'boundary.country': 'aTa' }; var clean = {}; var errorsAndWarnings = sanitize(raw, clean); t.equals(clean.boundary.country, 'ATA', 'should be uppercased ISO3'); @@ -58,7 +49,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) { }); test('unknown 2-character boundary.country should set boundary.country to undefined', function(t) { - var raw = { boundary: {country: 'zq'} }; + var raw = { 'boundary.country': 'zq' }; var clean = {}; var errorsAndWarnings = sanitize(raw, clean); t.equals(clean.boundary.country, undefined, 'should be undefined'); @@ -67,7 +58,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) { }); test('unknown 3-character boundary.country should set boundary.country to undefined', function(t) { - var raw = { boundary: {country: 'zqx'} }; + var raw = { 'boundary.country': 'zqx' }; var clean = {}; var errorsAndWarnings = sanitize(raw, clean); t.equals(clean.boundary.country, undefined, 'should be undefined'); From a4c43ee482860bf55252f470663d36f44a553a67 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 14 Sep 2015 16:43:45 -0400 Subject: [PATCH 4/7] added fixture for boundary.country in query building --- test/unit/fixture/search_boundary_country.js | 43 ++++++++++++++++++++ test/unit/query/search.js | 16 ++++++++ 2 files changed, 59 insertions(+) create mode 100644 test/unit/fixture/search_boundary_country.js diff --git a/test/unit/fixture/search_boundary_country.js b/test/unit/fixture/search_boundary_country.js new file mode 100644 index 00000000..94122797 --- /dev/null +++ b/test/unit/fixture/search_boundary_country.js @@ -0,0 +1,43 @@ + +module.exports = { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'must': [ + { + 'match': { + 'alpha3': { + 'analyzer': 'standard', + 'query': 'ABC' + } + } + }, + { + 'match': { + 'name.default': { + 'query': 'test', + 'boost': 1, + 'analyzer': 'peliasOneEdgeGram' + } + } + }], + 'should': [{ + 'match': { + 'phrase.default': { + 'query': 'test', + 'analyzer': 'peliasPhrase', + 'type': 'phrase', + 'boost': 1, + 'slop': 2 + } + } + }] + } + } + } + }, + 'sort': [ '_score' ], + 'size': 10, + 'track_scores': true +}; diff --git a/test/unit/query/search.js b/test/unit/query/search.js index 399fd20a..2b6194cf 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -136,6 +136,22 @@ module.exports.tests.query = function(test, common) { t.deepEqual(compiled, expected, 'valid search query'); t.end(); }); + + test('valid boundary.country search', function(t) { + var query = generate({ + text: 'test', size: 10, + layers: ['test'], + boundary: { country: 'ABC' } + }); + + var compiled = JSON.parse( JSON.stringify( query ) ); + var expected = require('../fixture/search_boundary_country'); + expected.sort = sort; + + t.deepEqual(compiled, expected, 'valid boundary.country query'); + t.end(); + }); + }; module.exports.all = function (tape, common) { From 13635a2c16f3b2e90c363be40759e07ad099491b Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 14 Sep 2015 17:20:10 -0400 Subject: [PATCH 5/7] added boundary.country to search and reverse queries (with test fixtures) --- query/reverse.js | 10 ++++ query/search.js | 1 - sanitiser/reverse.js | 4 +- test/unit/fixture/reverse_standard.js | 6 ++- .../fixture/reverse_with_boundary_country.js | 54 +++++++++++++++++++ test/unit/fixture/search_boundary_country.js | 29 +++++----- test/unit/query/reverse.js | 14 +++++ test/unit/sanitiser/reverse.js | 9 ++-- 8 files changed, 104 insertions(+), 23 deletions(-) create mode 100644 test/unit/fixture/reverse_with_boundary_country.js diff --git a/query/reverse.js b/query/reverse.js index 1a2332bb..f91561aa 100644 --- a/query/reverse.js +++ b/query/reverse.js @@ -7,6 +7,9 @@ var peliasQuery = require('pelias-query'), //------------------------------ var query = new peliasQuery.layout.FilteredBooleanQuery(); +// mandatory matches +query.score( peliasQuery.view.boundary_country, 'must' ); + // scoring boost query.sort( peliasQuery.view.sort_distance ); @@ -49,6 +52,13 @@ function generateQuery( clean ){ }); } + // boundary country + if( clean.boundary && clean.boundary.country ){ + vs.set({ + 'boundary:country': clean.boundary.country + }); + } + return query.render( vs ); } diff --git a/query/search.js b/query/search.js index 977e7c1f..bc88516f 100644 --- a/query/search.js +++ b/query/search.js @@ -98,7 +98,6 @@ function generateQuery( clean ){ } // boundary country - // @todo: change these to the correct request variable names if( clean.boundary && clean.boundary.country ){ vs.set({ 'boundary:country': clean.boundary.country diff --git a/sanitiser/reverse.js b/sanitiser/reverse.js index ec86e289..da6bd558 100644 --- a/sanitiser/reverse.js +++ b/sanitiser/reverse.js @@ -6,7 +6,8 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), size: require('../sanitiser/_size'), details: require('../sanitiser/_details'), geo_reverse: require('../sanitiser/_geo_reverse'), - categories: require('../sanitiser/_categories') + categories: require('../sanitiser/_categories'), + boundary_country: require('../sanitiser/_boundary_country'), }; var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; @@ -25,4 +26,3 @@ module.exports.middleware = function( req, res, next ){ next(); }); }; - diff --git a/test/unit/fixture/reverse_standard.js b/test/unit/fixture/reverse_standard.js index b7cc058f..08a005e6 100644 --- a/test/unit/fixture/reverse_standard.js +++ b/test/unit/fixture/reverse_standard.js @@ -3,7 +3,9 @@ module.exports = { 'query': { 'filtered': { 'query': { - 'bool': {} + 'bool': { + 'must': [] + } }, 'filter': { 'bool': { @@ -40,4 +42,4 @@ module.exports = { ], 'size': 1, 'track_scores': true -}; \ No newline at end of file +}; diff --git a/test/unit/fixture/reverse_with_boundary_country.js b/test/unit/fixture/reverse_with_boundary_country.js new file mode 100644 index 00000000..f29569b2 --- /dev/null +++ b/test/unit/fixture/reverse_with_boundary_country.js @@ -0,0 +1,54 @@ + +module.exports = { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'must': [ + { + 'match': { + 'alpha3': { + 'analyzer': 'standard', + 'query': 'ABC' + } + } + } + ] + } + }, + '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 +}; diff --git a/test/unit/fixture/search_boundary_country.js b/test/unit/fixture/search_boundary_country.js index 94122797..f4a873bd 100644 --- a/test/unit/fixture/search_boundary_country.js +++ b/test/unit/fixture/search_boundary_country.js @@ -5,23 +5,24 @@ module.exports = { 'query': { 'bool': { 'must': [ - { - 'match': { - 'alpha3': { - 'analyzer': 'standard', - 'query': 'ABC' + { + 'match': { + 'alpha3': { + 'analyzer': 'standard', + 'query': 'ABC' + } } - } - }, - { - 'match': { - 'name.default': { - 'query': 'test', - 'boost': 1, - 'analyzer': 'peliasOneEdgeGram' + }, + { + 'match': { + 'name.default': { + 'query': 'test', + 'boost': 1, + 'analyzer': 'peliasOneEdgeGram' + } } } - }], + ], 'should': [{ 'match': { 'phrase.default': { diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index 521a4ffe..0d37fd1c 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -60,6 +60,20 @@ module.exports.tests.query = function(test, common) { }); t.end(); }); + + test('valid boundary.country reverse search', function(t) { + var query = generate({ + lat: 29.49136, lon: -82.50622, + boundary: { country: 'ABC' } + }); + + var compiled = JSON.parse( JSON.stringify( query ) ); + var expected = require('../fixture/reverse_with_boundary_country'); + + t.deepEqual(compiled, expected, 'valid reverse query with boundary.country'); + t.end(); + }); + }; module.exports.all = function (tape, common) { diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js index 5c2a3a9f..1529c1e1 100644 --- a/test/unit/sanitiser/reverse.js +++ b/test/unit/sanitiser/reverse.js @@ -9,7 +9,8 @@ var reverse = require('../../../sanitiser/reverse'), lon: 0, size: 10, details: true, - categories: [] + categories: [], + boundary: { country: undefined } }, sanitize = function(query, cb) { _sanitize({'query':query}, cb); }; @@ -30,7 +31,7 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.sanitisers = function(test, common) { test('check sanitiser list', function (t) { - var expected = ['layers', 'sources', 'size', 'details', 'geo_reverse', 'categories']; + var expected = ['layers', 'sources', 'size', 'details', 'geo_reverse', 'categories', 'boundary_country']; t.deepEqual(Object.keys(reverse.sanitiser_list), expected); t.end(); }); @@ -57,7 +58,7 @@ module.exports.tests.sanitize_lat = function(test, common) { var expected = JSON.parse(JSON.stringify( defaultClean )); expected.lat = parseFloat( lat ); t.equal(err, undefined, 'no error'); - t.deepEqual(clean, expected, 'clean set correctly (' + lat + ')'); + t.equal(clean.lat, parseFloat(lat), 'clean set correctly (' + lat + ')'); }); }); t.end(); @@ -84,7 +85,7 @@ module.exports.tests.sanitize_lon = function(test, common) { var expected = JSON.parse(JSON.stringify( defaultClean )); expected.lon = parseFloat( lon ); t.equal(err, undefined, 'no error'); - t.deepEqual(clean, expected, 'clean set correctly (' + lon + ')'); + t.equal(clean.lon, parseFloat(lon), 'clean set correctly (' + lon + ')'); }); }); t.end(); From ba22912507b186b15340176de40371f739b621ed Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 14 Sep 2015 21:01:46 -0400 Subject: [PATCH 6/7] implemented Diana's suggestions to use check-types and delete key rather than set to undefined --- sanitiser/_boundary_country.js | 14 ++++++++------ test/unit/sanitiser/reverse.js | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/sanitiser/_boundary_country.js b/sanitiser/_boundary_country.js index f2824cec..192a9167 100644 --- a/sanitiser/_boundary_country.js +++ b/sanitiser/_boundary_country.js @@ -1,4 +1,4 @@ -var isObject = require('is-object'); +var check = require('check-types'); var iso3166 = require('iso3166-1'); function sanitize(raw, clean) { @@ -8,23 +8,25 @@ function sanitize(raw, clean) { // init clean.boundary (if not already init) clean.boundary = clean.boundary || {}; - if (raw['boundary.country']) { + if (check.assigned(raw['boundary.country'])) { var country = raw['boundary.country']; - if (typeof country !== 'string') { + if (!check.string(country)) { messages.warnings.push('boundary.country is not a string'); - clean.boundary.country = undefined; + delete clean.boundary.country; } else if (!containsIsoCode(country.toUpperCase())) { messages.warnings.push(country + ' is not a valid ISO2/ISO3 country code'); - clean.boundary.country = undefined; + delete clean.boundary.country; } else { + // the only way for boundary.country to be assigned is if input is + // a string and a known ISO2 or ISO3 clean.boundary.country = iso3166.to3(country.toUpperCase()); } } else { - clean.boundary.country = undefined; + delete clean.boundary.country; } return messages; diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js index 1529c1e1..f181b69b 100644 --- a/test/unit/sanitiser/reverse.js +++ b/test/unit/sanitiser/reverse.js @@ -10,7 +10,7 @@ var reverse = require('../../../sanitiser/reverse'), size: 10, details: true, categories: [], - boundary: { country: undefined } + boundary: { } }, sanitize = function(query, cb) { _sanitize({'query':query}, cb); }; From 6c85e5892e029ee472fcb8697a7b950194dcc14d Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 15 Sep 2015 14:36:18 -0400 Subject: [PATCH 7/7] converted warnings to errors --- sanitiser/_boundary_country.js | 4 ++-- test/unit/sanitiser/_boundary_country.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sanitiser/_boundary_country.js b/sanitiser/_boundary_country.js index 192a9167..6376e434 100644 --- a/sanitiser/_boundary_country.js +++ b/sanitiser/_boundary_country.js @@ -12,11 +12,11 @@ function sanitize(raw, clean) { var country = raw['boundary.country']; if (!check.string(country)) { - messages.warnings.push('boundary.country is not a string'); + messages.errors.push('boundary.country is not a string'); delete clean.boundary.country; } else if (!containsIsoCode(country.toUpperCase())) { - messages.warnings.push(country + ' is not a valid ISO2/ISO3 country code'); + messages.errors.push(country + ' is not a valid ISO2/ISO3 country code'); delete clean.boundary.country; } else { diff --git a/test/unit/sanitiser/_boundary_country.js b/test/unit/sanitiser/_boundary_country.js index 66b07d7c..2a2e9e48 100644 --- a/test/unit/sanitiser/_boundary_country.js +++ b/test/unit/sanitiser/_boundary_country.js @@ -26,7 +26,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) { var clean = {}; var errorsAndWarnings = sanitize(raw, clean); t.equals(clean.boundary.country, undefined, 'should be undefined'); - t.deepEquals(errorsAndWarnings, { errors: [], warnings: ['boundary.country is not a string'] }, 'non-string country warning'); + t.deepEquals(errorsAndWarnings, { errors: ['boundary.country is not a string'], warnings: [] }, 'non-string country warning'); t.end(); }); @@ -53,7 +53,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) { var clean = {}; var errorsAndWarnings = sanitize(raw, clean); t.equals(clean.boundary.country, undefined, 'should be undefined'); - t.deepEquals(errorsAndWarnings, { errors: [], warnings: ['zq is not a valid ISO2/ISO3 country code'] }, 'country not found warning`'); + t.deepEquals(errorsAndWarnings, { errors: ['zq is not a valid ISO2/ISO3 country code'], warnings: [] }, 'country not found warning`'); t.end(); }); @@ -62,7 +62,7 @@ module.exports.tests.sanitize_boundary_country = function(test, common) { var clean = {}; var errorsAndWarnings = sanitize(raw, clean); t.equals(clean.boundary.country, undefined, 'should be undefined'); - t.deepEquals(errorsAndWarnings, { errors: [], warnings: ['zqx is not a valid ISO2/ISO3 country code'] }, 'country not found warning`'); + t.deepEquals(errorsAndWarnings, { errors: ['zqx is not a valid ISO2/ISO3 country code'], warnings: [] }, 'country not found warning`'); t.end(); });