From 616dd606a731ae5e57bd7cf0cacc999b9d820d33 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Wed, 2 Mar 2016 15:44:00 -0500 Subject: [PATCH] Add sources and layers sanitiser This sanitiser can do a better job of determining when an invalid combination of sources and layers was specified, and produce helpful error messages. --- sanitiser/_sources_and_layers.js | 37 +++++++ sanitiser/reverse.js | 2 + sanitiser/search.js | 2 + test/unit/run.js | 1 + test/unit/sanitiser/_sources_and_layers.js | 115 +++++++++++++++++++++ 5 files changed, 157 insertions(+) create mode 100644 sanitiser/_sources_and_layers.js create mode 100644 test/unit/sanitiser/_sources_and_layers.js diff --git a/sanitiser/_sources_and_layers.js b/sanitiser/_sources_and_layers.js new file mode 100644 index 00000000..448a7af1 --- /dev/null +++ b/sanitiser/_sources_and_layers.js @@ -0,0 +1,37 @@ +var _ = require( 'lodash' ); +var type_mapping = require( '../helper/type_mapping' ); + +/* + * This sanitiser depends on clean.layers and clean.sources + * so it has to be run after those sanitisers have been run + */ +function sanitize( raw, clean ){ + var messages = { errors: [], warnings: [] }; + + var possible_errors = []; + var at_least_one_valid_combination = false; + + if (clean.layers && clean.sources) { + clean.sources.forEach(function(source) { + var layers_for_source = type_mapping.layers_by_source[source]; + clean.layers.forEach(function(layer) { + if (_.includes(layers_for_source, layer)) { + at_least_one_valid_combination = true; + } else { + var message = 'You have specified both the `sources` and `layers` ' + + 'parameters in a combination that will return no results: the ' + + source + ' source has nothing in the ' + layer + ' layer'; + possible_errors.push(message); + } + }); + }); + + if (!at_least_one_valid_combination) { + messages.errors = possible_errors; + } + } + + return messages; +} + +module.exports = sanitize; diff --git a/sanitiser/reverse.js b/sanitiser/reverse.js index 045ff3a1..d707fcea 100644 --- a/sanitiser/reverse.js +++ b/sanitiser/reverse.js @@ -5,6 +5,8 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), singleScalarParameters: require('../sanitiser/_single_scalar_parameters'), layers: require('../sanitiser/_targets')('layers', type_mapping.layer_mapping), sources: require('../sanitiser/_targets')('sources', type_mapping.source_mapping), + // depends on the layers and sources sanitisers, must be run after them + sources_and_layers: require('../sanitiser/_sources_and_layers'), size: require('../sanitiser/_size'), private: require('../sanitiser/_flag_bool')('private', false), geo_reverse: require('../sanitiser/_geo_reverse'), diff --git a/sanitiser/search.js b/sanitiser/search.js index 1be32b02..980b5a04 100644 --- a/sanitiser/search.js +++ b/sanitiser/search.js @@ -7,6 +7,8 @@ var sanitizeAll = require('../sanitiser/sanitizeAll'), size: require('../sanitiser/_size'), layers: require('../sanitiser/_targets')('layers', type_mapping.layer_mapping), sources: require('../sanitiser/_targets')('sources', type_mapping.source_mapping), + // depends on the layers and sources sanitisers, must be run after them + sources_and_layers: require('../sanitiser/_sources_and_layers'), private: require('../sanitiser/_flag_bool')('private', false), geo_search: require('../sanitiser/_geo_search'), boundary_country: require('../sanitiser/_boundary_country'), diff --git a/test/unit/run.js b/test/unit/run.js index 89b7c31b..9d210853 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -37,6 +37,7 @@ var tests = [ require('./sanitiser/_single_scalar_parameters'), require('./sanitiser/_size'), require('./sanitiser/_sources'), + require('./sanitiser/_sources_and_layers'), require('./sanitiser/_text'), require('./sanitiser/autocomplete'), require('./sanitiser/place'), diff --git a/test/unit/sanitiser/_sources_and_layers.js b/test/unit/sanitiser/_sources_and_layers.js new file mode 100644 index 00000000..96001f4e --- /dev/null +++ b/test/unit/sanitiser/_sources_and_layers.js @@ -0,0 +1,115 @@ +var sanitize = require('../../../sanitiser/_sources_and_layers'); + +var type_mapping = require('../../../helper/type_mapping'); + +module.exports.tests = {}; + +module.exports.tests.inactive = function(test, common) { + test('no source or layer specified', function(t) { + var raw = {}; + var clean = {}; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 0, 'should return no errors'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); + + test('only layers specified', function(t) { + var raw = {}; + var clean = { layers: ['venue'] }; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 0, 'should return no errors'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); + + test('only sources specified', function(t) { + var raw = {}; + var clean = { sources: ['openstreetmap'] }; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 0, 'should return no errors'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); +}; + +module.exports.tests.no_errors = function(test, common) { + test('valid combination', function(t) { + var raw = {}; + var clean = { sources: ['openstreetmap'], layers: ['venue'] }; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 0, 'should return no errors'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); + + test('valid combination because of multiple sources', function(t) { + var raw = {}; + var clean = { sources: ['openstreetmap', 'openaddresses'], layers: ['venue'] }; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 0, 'should return no errors'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); + + test('valid combination because of multiple layers', function(t) { + var raw = {}; + var clean = { sources: ['openaddresses'], layers: ['address', 'country'] }; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 0, 'should return no errors'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); +}; + +module.exports.tests.invalid_combination = function(test, common) { + test('address layer with wof', function(t) { + var raw = {}; + var clean = { sources: ['whosonfirst'], layers: ['address'] }; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 1, 'should return an error'); + t.equal(messages.errors[0], 'You have specified both the `sources` and `layers` ' + + 'parameters in a combination that will return no results: the whosonfirst source has nothing in the address layer'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); + + test('admin layers with osm', function(t) { + var raw = {}; + var clean = { sources: ['openstreetmap'], layers: ['country', 'locality'] }; + + var messages = sanitize(raw, clean); + + t.equal(messages.errors.length, 2, 'should return an error'); + t.equal(messages.errors[0], 'You have specified both the `sources` and `layers` ' + + 'parameters in a combination that will return no results: the openstreetmap source has nothing in the country layer'); + t.equal(messages.errors[1], 'You have specified both the `sources` and `layers` ' + + 'parameters in a combination that will return no results: the openstreetmap source has nothing in the locality layer'); + t.equal(messages.warnings.length, 0, 'should return no warnings'); + t.end(); + }); +}; + +module.exports.all = function (tape, common) { + function test(name, testFunction) { + return tape('SANTIZE _sources_and_layers ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +};