From 5f663ea8b3b20102377481307a1cf1fd176e421b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt?= Date: Fri, 10 Jul 2020 14:31:19 +0200 Subject: [PATCH 1/2] feat(nearby): Popularity sort --- query/reverse.js | 5 +++++ query/reverse_defaults.js | 2 ++ sanitizer/_sort.js | 32 ++++++++++++++++++++++++++++++++ sanitizer/nearby.js | 1 + 4 files changed, 40 insertions(+) create mode 100644 sanitizer/_sort.js diff --git a/query/reverse.js b/query/reverse.js index 6f145680b..aaf2f708c 100644 --- a/query/reverse.js +++ b/query/reverse.js @@ -12,6 +12,7 @@ var query = new peliasQuery.layout.FilteredBooleanQuery(); // scoring boost query.sort( peliasQuery.view.sort_distance ); +query.sort( peliasQuery.view.sort_popularity ); // non-scoring hard filters query.filter( peliasQuery.view.boundary_circle ); @@ -96,6 +97,10 @@ function generateQuery( clean ){ vs.var('input:categories', clean.categories); } + if (_.isString(clean.sort)) { + vs.var('sort:field', clean.sort); + } + return { type: 'reverse', body: query.render(vs) diff --git a/query/reverse_defaults.js b/query/reverse_defaults.js index 2bca3a59a..ddeef69cd 100644 --- a/query/reverse_defaults.js +++ b/query/reverse_defaults.js @@ -9,6 +9,8 @@ module.exports = _.merge({}, peliasQuery.defaults, { 'centroid:field': 'center_point', + 'sort:field': 'distance', + 'sort:distance:order': 'asc', 'sort:distance:distance_type': 'plane', diff --git a/sanitizer/_sort.js b/sanitizer/_sort.js new file mode 100644 index 000000000..6b737b85d --- /dev/null +++ b/sanitizer/_sort.js @@ -0,0 +1,32 @@ +const _ = require('lodash'); +const DEFAULT_SORT = 'distance'; + +const allowed_values = ['distance', 'popularity']; + +function _setup(){ + + return { + sanitize: function _sanitize( raw, clean ){ + + // error & warning messages + var messages = { errors: [], warnings: [] }; + + clean.sort = raw.sort; + + if( clean.sort && !allowed_values.includes(clean.sort) ){ + messages.warnings.push('invalid \'sort\', using \'distance\''); + clean.sort = DEFAULT_SORT; + } + + return messages; + }, + + expected: function _expected() { + // add size as a valid parameter + return [{ name: 'sort' }]; + } + }; +} + +// export function +module.exports = _setup; diff --git a/sanitizer/nearby.js b/sanitizer/nearby.js index e2103248d..e2e050100 100644 --- a/sanitizer/nearby.js +++ b/sanitizer/nearby.js @@ -12,6 +12,7 @@ module.exports.middleware = (_api_pelias_config) => { sources_and_layers: require('../sanitizer/_sources_and_layers')(), geonames_deprecation: require('../sanitizer/_geonames_deprecation')(), size: require('../sanitizer/_size')(/* use defaults*/), + sort: require('../sanitizer/_sort')(), private: require('../sanitizer/_flag_bool')('private', false), geo_reverse: require('../sanitizer/_geo_reverse')(), boundary_country: require('../sanitizer/_boundary_country')(), From 12886fe55bb5af8c2a25e0ebe05ec316990189e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt?= Date: Fri, 17 Jul 2020 15:33:54 +0200 Subject: [PATCH 2/2] Add tests --- sanitizer/_sort.js | 4 ++- sanitizer/nearby.js | 6 ++-- test/unit/query/reverse.js | 24 ++++++++++++++- test/unit/run.js | 1 + test/unit/sanitizer/_sort.js | 56 +++++++++++++++++++++++++++++++++++ test/unit/sanitizer/nearby.js | 13 ++++++-- 6 files changed, 97 insertions(+), 7 deletions(-) create mode 100644 test/unit/sanitizer/_sort.js diff --git a/sanitizer/_sort.js b/sanitizer/_sort.js index 6b737b85d..d9829c8a7 100644 --- a/sanitizer/_sort.js +++ b/sanitizer/_sort.js @@ -13,7 +13,9 @@ function _setup(){ clean.sort = raw.sort; - if( clean.sort && !allowed_values.includes(clean.sort) ){ + if (!clean.sort) { + clean.sort = DEFAULT_SORT; + } else if( !allowed_values.includes(clean.sort) ){ messages.warnings.push('invalid \'sort\', using \'distance\''); clean.sort = DEFAULT_SORT; } diff --git a/sanitizer/nearby.js b/sanitizer/nearby.js index e2e050100..91565ec55 100644 --- a/sanitizer/nearby.js +++ b/sanitizer/nearby.js @@ -12,16 +12,16 @@ module.exports.middleware = (_api_pelias_config) => { sources_and_layers: require('../sanitizer/_sources_and_layers')(), geonames_deprecation: require('../sanitizer/_geonames_deprecation')(), size: require('../sanitizer/_size')(/* use defaults*/), - sort: require('../sanitizer/_sort')(), private: require('../sanitizer/_flag_bool')('private', false), geo_reverse: require('../sanitizer/_geo_reverse')(), boundary_country: require('../sanitizer/_boundary_country')(), categories: require('../sanitizer/_categories')(), - request_language: require('../sanitizer/_request_language')() + request_language: require('../sanitizer/_request_language')(), + sort: require('../sanitizer/_sort')() }; return function( req, res, next ){ sanitizeAll.runAllChecks(req, sanitizers); next(); }; -}; \ No newline at end of file +}; diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index 9813bf2f6..c0d16acd4 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -12,6 +12,7 @@ const views = { layers: 'layers view', categories: 'categories view', sort_distance: 'sort_distance view', + sort_popularity: 'sort_popularity view', boundary_gid: 'boundary_gid view' }; @@ -70,7 +71,8 @@ module.exports.tests.query = (test, common) => { ]); t.deepEquals(query.body.sort_functions, [ - 'sort_distance view' + 'sort_distance view', + 'sort_popularity view' ]); t.end(); @@ -98,6 +100,26 @@ module.exports.tests.query = (test, common) => { }); + test('clean.sort should set sort:field parameter', t => { + const clean = { + sort: 'popularity' + }; + + const query = proxyquire('../../../query/reverse', { + 'pelias-query': { + layout: { + FilteredBooleanQuery: MockQuery + }, + view: views, + Vars: require('pelias-query').Vars + }, + './reverse_defaults': {} + })(clean); + + t.deepEquals(query.body.vs.var('sort:field').toString(), 'popularity'); + t.end(); + + }); }; module.exports.tests.sources = (test, common) => { diff --git a/test/unit/run.js b/test/unit/run.js index ac2bc7681..1d286d891 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -103,6 +103,7 @@ var tests = [ require('./sanitizer/_tokenizer'), require('./sanitizer/_categories'), require('./sanitizer/_boundary_gid'), + require('./sanitizer/_sort'), require('./sanitizer/nearby'), require('./sanitizer/autocomplete'), require('./sanitizer/structured_geocoding'), diff --git a/test/unit/sanitizer/_sort.js b/test/unit/sanitizer/_sort.js new file mode 100644 index 000000000..50db6fb97 --- /dev/null +++ b/test/unit/sanitizer/_sort.js @@ -0,0 +1,56 @@ +var sanitizer = require('../../../sanitizer/_sort'); + +module.exports.tests = {}; + +module.exports.tests.sanitize_sort = function(test, common) { + test('sort=foo', function(t) { + var raw = { sort: 'foo' }; + var clean = {}; + var res = sanitizer().sanitize(raw, clean); + t.equal(res.errors.length, 0, 'should return no errors'); + t.equal(res.warnings.length, 1, 'should return warning'); + t.equal(res.warnings[0], 'invalid \'sort\', using \'distance\''); + t.equal(clean.sort, 'distance', 'default to distance'); + t.end(); + }); + + test('sort not set', function(t) { + var raw = {}; + var clean = {}; + var res = sanitizer().sanitize(raw, clean); + t.equal(res.errors.length, 0, 'should return no errors'); + t.equal(res.warnings.length, 0, 'should return no warning'); + t.equal(clean.sort, 'distance', 'default to distance'); + t.end(); + }); + + test('return an array of expected parameters in object form for validation', function(t) { + const expected = [{ name: 'sort' }]; + const validParameters = sanitizer().expected(); + t.deepEquals(validParameters, expected); + t.end(); + }); + + var valid_sorts = ['popularity', 'distance']; + valid_sorts.forEach(function (sort) { + test('sort=' + sort, function (t) { + var raw = {sort: sort}; + var clean = {}; + var res = sanitizer().sanitize(raw, clean); + t.equal(res.errors.length, 0, 'should return no errors'); + t.equal(res.warnings.length, 0, 'should return warning'); + t.equal(clean.sort, sort, 'set to correct value'); + t.end(); + }); + }); +}; + +module.exports.all = function (tape, common) { + function test(name, testFunction) { + return tape('SANITIZE _sort ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/sanitizer/nearby.js b/test/unit/sanitizer/nearby.js index 730e5c388..ad23226a0 100644 --- a/test/unit/sanitizer/nearby.js +++ b/test/unit/sanitizer/nearby.js @@ -109,7 +109,15 @@ module.exports.tests.sanitize = function(test, common) { return { errors: [], warnings: [] }; } }; - } + }, + '../sanitizer/_sort': function () { + return { + sanitize: () => { + called_sanitizers.push('_sort'); + return { errors: [], warnings: [] }; + } + }; + }, }); const expected_sanitizers = [ @@ -124,7 +132,8 @@ module.exports.tests.sanitize = function(test, common) { '_geo_reverse', '_boundary_country', '_categories', - '_request_language' + '_request_language', + '_sort', ]; const req = {};