diff --git a/sanitizer/_geonames_deprecation.js b/sanitizer/_geonames_deprecation.js index b3cc8185..2cdef1d1 100644 --- a/sanitizer/_geonames_deprecation.js +++ b/sanitizer/_geonames_deprecation.js @@ -1,21 +1,32 @@ const _ = require('lodash'); /** -with the release of coarse reverse as a separate thing and ES reverse only -handling venues, addresses, and streets, geonames make no sense in the reverse context + * Now that we have the pip-service, we have stopped supporting returning Geonames for coarse reverse. + * + * However, until the `/nearby` endpoint is finalized, we still want to support Geonames for + * _non-coarse_ reverse. **/ +const coarse_reverse_message ='coarse /reverse does not support geonames. See https://github.com/pelias/pelias/issues/675 for more info'; + function _sanitize( raw, clean, opts ) { // error & warning messages const messages = { errors: [], warnings: [] }; + // return taking no action unless this is a coarse-only reverse request + const non_coarse_layers = ['address', 'street', 'venue']; + const is_coarse_reverse = !_.isEmpty(clean.layers) && + _.isEmpty(_.intersection(clean.layers, non_coarse_layers)); + if (!is_coarse_reverse) { + return messages; + } + if (_.isEqual(clean.sources, ['geonames']) || _.isEqual(clean.sources, ['gn'])) { - messages.errors.push('/reverse does not support geonames'); + messages.errors.push(coarse_reverse_message); } else if (_.includes(clean.sources, 'geonames') || _.includes(clean.sources, 'gn')) { clean.sources = _.without(clean.sources, 'geonames', 'gn'); - messages.warnings.push('/reverse does not support geonames'); - + messages.warnings.push(coarse_reverse_message); } return messages; diff --git a/test/unit/sanitizer/_geonames_deprecation.js b/test/unit/sanitizer/_geonames_deprecation.js index 41bd86c6..13461e15 100644 --- a/test/unit/sanitizer/_geonames_deprecation.js +++ b/test/unit/sanitizer/_geonames_deprecation.js @@ -2,6 +2,8 @@ const sanitizer = require('../../../sanitizer/_geonames_deprecation')(); module.exports.tests = {}; +const coarse_reverse_message ='coarse /reverse does not support geonames. See https://github.com/pelias/pelias/issues/675 for more info'; + module.exports.tests.no_warnings_or_errors_conditions = (test, common) => { test('undefined sources should add neither warnings nor errors', (t) => { const clean = {}; @@ -27,41 +29,71 @@ module.exports.tests.no_warnings_or_errors_conditions = (test, common) => { }); + test('geonames/gn in sources with layers=venue should add neither warnings nor errors', (t) => { + const clean = { + sources: ['geonames'], + layers: ['venue'] + }; + + const messages = sanitizer.sanitize(undefined, clean); + + t.deepEquals(clean.sources, ['geonames']); + t.deepEquals(messages, { errors: [], warnings: [] }); + t.end(); + }); }; module.exports.tests.error_conditions = (test, common) => { - test('only geonames in sources should not modify clean.sources and add error message', (t) => { + test('only geonames in sources with layers=coarse should not modify clean.sources and add error message', (t) => { ['gn', 'geonames'].forEach((sources) => { const clean = { - sources: [sources] + sources: [sources], + layers: ['coarse'] }; const messages = sanitizer.sanitize(undefined, clean); t.deepEquals(clean.sources, [sources]); - t.deepEquals(messages.errors, ['/reverse does not support geonames']); + t.deepEquals(messages.errors, [ coarse_reverse_message ]); t.deepEquals(messages.warnings, []); }); t.end(); - }); + test('only geonames in sources with layers=locality should not modify clean.sources and add error message', (t) => { + ['gn', 'geonames'].forEach((sources) => { + const clean = { + sources: [sources], + layers: ['locality'] + }; + + const messages = sanitizer.sanitize(undefined, clean); + + t.deepEquals(clean.sources, [sources]); + t.deepEquals(messages.errors, [ coarse_reverse_message ]); + t.deepEquals(messages.warnings, []); + + }); + + t.end(); + }); }; module.exports.tests.warning_conditions = (test, common) => { - test('only geonames in sources should not modify clean.sources and add error message', (t) => { + test('only geonames in sources and layers=coarse should not modify clean.sources and add error message', (t) => { ['gn', 'geonames'].forEach((sources) => { const clean = { - sources: ['another source', sources, 'yet another source'] + sources: ['another source', sources, 'yet another source'], + layers: ['coarse'] }; const messages = sanitizer.sanitize(undefined, clean); t.deepEquals(clean.sources, ['another source', 'yet another source']); t.deepEquals(messages.errors, []); - t.deepEquals(messages.warnings, ['/reverse does not support geonames']); + t.deepEquals(messages.warnings, [ coarse_reverse_message ]); });