From 8ebae9a2ae520f046a3b1165f05645b3ab9e38ce Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Thu, 11 May 2017 12:23:04 -0400 Subject: [PATCH] added warnings/errors when sources includes geonames --- sanitizer/_geonames_warnings.js | 33 ++++++++ sanitizer/search.js | 3 +- test/unit/run.js | 1 + test/unit/sanitizer/_geonames_warnings.js | 99 +++++++++++++++++++++++ test/unit/sanitizer/search.js | 62 +++++++------- 5 files changed, 169 insertions(+), 29 deletions(-) create mode 100644 sanitizer/_geonames_warnings.js create mode 100644 test/unit/sanitizer/_geonames_warnings.js diff --git a/sanitizer/_geonames_warnings.js b/sanitizer/_geonames_warnings.js new file mode 100644 index 00000000..e3fdfb1f --- /dev/null +++ b/sanitizer/_geonames_warnings.js @@ -0,0 +1,33 @@ +const _ = require('lodash'); + +function isAdminOnly(parsed_text) { + return ['number', 'street', 'query', 'category'].every((prop) => { + return _.isEmpty(parsed_text[prop]); + }); +} + +function sanitize( raw, clean ){ + // error & warning messages + const messages = { errors: [], warnings: [] }; + + // bail early if analysis wasn't admin-only + if (!isAdminOnly(clean.parsed_text)) { + return messages; + } + + if (_.isEqual(clean.sources, ['geonames'])) { + // if requested sources is only geonames, return an error + messages.errors.push('input contains only administrative area data, ' + + 'no results will be returned when sources=geonames'); + + } else if (_.indexOf(clean.sources, 'geonames') !== -1) { + // if there are other sources besides geonames, return an warning + messages.warnings.push('input contains only administrative area data, ' + + 'geonames results will not be returned'); + + } + + return messages; +} + +module.exports = sanitize; diff --git a/sanitizer/search.js b/sanitizer/search.js index 5694c9eb..62b33212 100644 --- a/sanitizer/search.js +++ b/sanitizer/search.js @@ -15,7 +15,8 @@ var sanitizeAll = require('../sanitizer/sanitizeAll'), private: require('../sanitizer/_flag_bool')('private', false), geo_search: require('../sanitizer/_geo_search'), boundary_country: require('../sanitizer/_boundary_country'), - categories: require('../sanitizer/_categories') + categories: require('../sanitizer/_categories'), + geonames_warnings: require('../sanitizer/_geonames_warnings') }; var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; diff --git a/test/unit/run.js b/test/unit/run.js index 5f5776d8..639672f6 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -54,6 +54,7 @@ var tests = [ require('./query/text_parser'), require('./sanitizer/_boundary_country'), require('./sanitizer/_flag_bool'), + require('./sanitizer/_geonames_warnings'), require('./sanitizer/_geo_common'), require('./sanitizer/_geo_reverse'), require('./sanitizer/_groups'), diff --git a/test/unit/sanitizer/_geonames_warnings.js b/test/unit/sanitizer/_geonames_warnings.js new file mode 100644 index 00000000..ced8ad67 --- /dev/null +++ b/test/unit/sanitizer/_geonames_warnings.js @@ -0,0 +1,99 @@ +const _ = require('lodash'); + +const geonames_warnings = require('../../../sanitizer/_geonames_warnings'); + +const nonAdminProperties = ['number', 'street', 'query', 'category']; +const adminProperties = ['neighbourhood', 'borough', 'city', 'county', 'state', 'postalcode', 'country']; + +module.exports.tests = {}; + +module.exports.tests.no_errors = (test, common) => { + test('any non-admin analysis field with only geonames sources should exit early', (t) => { + adminProperties.forEach((adminProperty) => { + nonAdminProperties.forEach((nonAdminProperty) => { + const clean = { + sources: ['geonames'], + parsed_text: {} + }; + clean.parsed_text[nonAdminProperty] = `${nonAdminProperty} value`; + clean.parsed_text[adminProperty] = `${adminProperty} value`; + + const messages = geonames_warnings(undefined, clean); + + t.deepEquals(messages, { errors: [], warnings: [] }); + + }); + }); + t.end(); + + }); + + test('any non-admin analysis field with non-geonames sources should exit early', (t) => { + adminProperties.forEach((adminProperty) => { + nonAdminProperties.forEach((nonAdminProperty) => { + const clean = { + sources: ['this is not geonames'], + parsed_text: {} + }; + clean.parsed_text[nonAdminProperty] = `${nonAdminProperty} value`; + clean.parsed_text[adminProperty] = `${adminProperty} value`; + + const messages = geonames_warnings(undefined, clean); + + t.deepEquals(messages, { errors: [], warnings: [] }); + + }); + }); + t.end(); + + }); + +}; + +module.exports.tests.error_conditions = (test, common) => { + test('any admin analysis field and only geonames sources should return error', (t) => { + adminProperties.forEach((property) => { + const clean = _.set({ sources: ['geonames'] }, + ['parsed_text', property], `${property} value`); + + const messages = geonames_warnings(undefined, clean); + + t.deepEquals(messages.errors, ['input contains only administrative area data, ' + + 'no results will be returned when sources=geonames']); + t.deepEquals(messages.warnings, []); + + }); + t.end(); + + }); + +}; + +module.exports.tests.warning_conditions = (test, common) => { + test('any admin analysis field and only geonames sources should return warning', (t) => { + adminProperties.forEach((property) => { + const clean = _.set({ sources: ['source 1', 'geonames', 'source 2'] }, + ['parsed_text', property], `${property} value`); + + const messages = geonames_warnings(undefined, clean); + + t.deepEquals(messages.errors, []); + t.deepEquals(messages.warnings, ['input contains only administrative area data, ' + + 'geonames results will not be returned']); + + }); + t.end(); + + }); + +}; + +module.exports.all = (tape, common) => { + function test(name, testFunction) { + return tape(`SANTIZE _geonames_warnings ${name}`, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/sanitizer/search.js b/test/unit/sanitizer/search.js index 0d44103d..e7a8e3fe 100644 --- a/test/unit/sanitizer/search.js +++ b/test/unit/sanitizer/search.js @@ -1,37 +1,38 @@ -var proxyquire = require('proxyquire').noCallThru(); +const proxyquire = require('proxyquire').noCallThru(); +const _ = require('lodash'); module.exports.tests = {}; -module.exports.tests.sanitize = function(test, common) { - test('verify that all sanitizers were called as expected', function(t) { - var called_sanitizers = []; +module.exports.tests.sanitize = (test, common) => { + test('verify that all sanitizers were called as expected', (t) => { + const called_sanitizers = []; // rather than re-verify the functionality of all the sanitizers, this test just verifies that they // were all called correctly - var search = proxyquire('../../../sanitizer/search', { - '../sanitizer/_deprecate_quattroshapes': function() { + const search = proxyquire('../../../sanitizer/search', { + '../sanitizer/_deprecate_quattroshapes': () => { called_sanitizers.push('_deprecate_quattroshapes'); return { errors: [], warnings: [] }; }, - '../sanitizer/_single_scalar_parameters': function() { + '../sanitizer/_single_scalar_parameters': () => { called_sanitizers.push('_single_scalar_parameters'); return { errors: [], warnings: [] }; }, - '../sanitizer/_text': function() { + '../sanitizer/_text': () => { called_sanitizers.push('_text'); return { errors: [], warnings: [] }; }, - '../sanitizer/_iso2_to_iso3': function() { + '../sanitizer/_iso2_to_iso3': () => { called_sanitizers.push('_iso2_to_iso3'); return { errors: [], warnings: [] }; }, - '../sanitizer/_city_name_standardizer': function() { + '../sanitizer/_city_name_standardizer': () => { called_sanitizers.push('_city_name_standardizer'); return { errors: [], warnings: [] }; }, '../sanitizer/_size': function() { - if (arguments.length === 0) { - return function() { + if (_.isEmpty(arguments)) { + return () => { called_sanitizers.push('_size'); return { errors: [], warnings: [] }; }; @@ -41,10 +42,10 @@ module.exports.tests.sanitize = function(test, common) { } }, - '../sanitizer/_targets': function(type) { + '../sanitizer/_targets': (type) => { if (['layers', 'sources'].indexOf(type) !== -1) { - return function() { - called_sanitizers.push('_targets/' + type); + return () => { + called_sanitizers.push(`_targets/${type}`); return { errors: [], warnings: [] }; }; @@ -54,13 +55,13 @@ module.exports.tests.sanitize = function(test, common) { } }, - '../sanitizer/_sources_and_layers': function() { + '../sanitizer/_sources_and_layers': () => { called_sanitizers.push('_sources_and_layers'); return { errors: [], warnings: [] }; }, '../sanitizer/_flag_bool': function() { if (arguments[0] === 'private' && arguments[1] === false) { - return function() { + return () => { called_sanitizers.push('_flag_bool'); return { errors: [], warnings: [] }; }; @@ -71,21 +72,25 @@ module.exports.tests.sanitize = function(test, common) { } }, - '../sanitizer/_geo_search': function() { + '../sanitizer/_geo_search': () => { called_sanitizers.push('_geo_search'); return { errors: [], warnings: [] }; }, - '../sanitizer/_boundary_country': function() { + '../sanitizer/_boundary_country': () => { called_sanitizers.push('_boundary_country'); return { errors: [], warnings: [] }; }, - '../sanitizer/_categories': function() { + '../sanitizer/_categories': () => { called_sanitizers.push('_categories'); return { errors: [], warnings: [] }; }, + '../sanitizer/_geonames_warnings': () => { + called_sanitizers.push('_geonames_warnings'); + return { errors: [], warnings: [] }; + } }); - var expected_sanitizers = [ + const expected_sanitizers = [ '_single_scalar_parameters', '_deprecate_quattroshapes', '_text', @@ -98,26 +103,27 @@ module.exports.tests.sanitize = function(test, common) { '_flag_bool', '_geo_search', '_boundary_country', - '_categories' + '_categories', + '_geonames_warnings' ]; - var req = {}; - var res = {}; + const req = {}; + const res = {}; - search.middleware(req, res, function(){ + search.middleware(req, res, () => { t.deepEquals(called_sanitizers, expected_sanitizers); t.end(); }); }); }; -module.exports.all = function (tape, common) { +module.exports.all = (tape, common) => { function test(name, testFunction) { - return tape('SANTIZE /search ' + name, testFunction); + return tape(`SANTIZE /search ${name}`, testFunction); } - for( var testCase in module.exports.tests ){ + for( const testCase in module.exports.tests ){ module.exports.tests[testCase](test, common); } };