diff --git a/query/reverse.js b/query/reverse.js index 72a3c3a6..fbe1251d 100644 --- a/query/reverse.js +++ b/query/reverse.js @@ -3,6 +3,7 @@ const peliasQuery = require('pelias-query'); const defaults = require('./reverse_defaults'); const check = require('check-types'); +const _ = require('lodash'); const logger = require('pelias-logger').get('api'); //------------------------------ @@ -44,7 +45,8 @@ function generateQuery( clean ){ // layers if( check.array(clean.layers) && clean.layers.length ) { - vs.var( 'layers', clean.layers); + // only include non-coarse layers + vs.var( 'layers', _.intersection(clean.layers, ['address', 'street', 'venue'])); logStr += '[param:layers] '; } diff --git a/query/reverse_defaults.js b/query/reverse_defaults.js index 3b468a05..d8b72292 100644 --- a/query/reverse_defaults.js +++ b/query/reverse_defaults.js @@ -6,6 +6,7 @@ module.exports = _.merge({}, peliasQuery.defaults, { 'size': 1, 'track_scores': true, + 'layers': ['venue', 'address', 'street'], 'centroid:field': 'center_point', diff --git a/routes/v1.js b/routes/v1.js index 80c3d9b5..9fb8f8d2 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -97,14 +97,9 @@ function addRoutes(app, peliasConfig) { const placeholderService = serviceWrapper(placeholderConfiguration); const isPlaceholderServiceEnabled = _.constant(placeholderConfiguration.isEnabled()); - // use coarse reverse when requested layers are all coarse - const coarse_reverse_should_execute = all( - isPipServiceEnabled, isCoarseReverse, not(hasRequestErrors) - ); - // fallback to coarse reverse when regular reverse didn't return anything - const coarse_reverse_fallback_should_execute = all( - isPipServiceEnabled, not(isCoarseReverse), not(hasRequestErrors), not(hasResponseData) + const coarse_reverse_should_execute = all( + isPipServiceEnabled, not(hasRequestErrors), not(hasResponseData) ); const placeholderShouldExecute = all( @@ -114,7 +109,7 @@ function addRoutes(app, peliasConfig) { // execute under the following conditions: // - there are no errors or data // - request is not coarse OR pip service is disabled - const original_reverse_should_execute = all( + const non_coarse_reverse_should_execute = all( not(hasResponseDataOrRequestErrors), any( not(isCoarseReverse), @@ -202,9 +197,8 @@ function addRoutes(app, peliasConfig) { sanitizers.reverse.middleware, middleware.requestLanguage, middleware.calcSize(), + controllers.search(peliasConfig.api, esclient, queries.reverse, non_coarse_reverse_should_execute), controllers.coarse_reverse(pipService, coarse_reverse_should_execute), - controllers.search(peliasConfig.api, esclient, queries.reverse, original_reverse_should_execute), - controllers.coarse_reverse(pipService, coarse_reverse_fallback_should_execute), postProc.distances('point.'), // reverse confidence scoring depends on distance from origin // so it must be calculated first diff --git a/test/unit/controller/coarse_reverse.js b/test/unit/controller/coarse_reverse.js index bb170a37..5fbe854c 100644 --- a/test/unit/controller/coarse_reverse.js +++ b/test/unit/controller/coarse_reverse.js @@ -449,6 +449,7 @@ module.exports.tests.success_conditions = (test, common) => { }; t.deepEquals(res, expected); + t.notOk(logger.hasErrorMessages()); t.end(); diff --git a/test/unit/fixture/reverse_null_island.js b/test/unit/fixture/reverse_null_island.js index df6972ca..ff9ef745 100644 --- a/test/unit/fixture/reverse_null_island.js +++ b/test/unit/fixture/reverse_null_island.js @@ -13,6 +13,11 @@ module.exports = { 'lon': 0 } } + }, + { + 'terms': { + 'layer': ['venue', 'address', 'street'] + } }] } }, diff --git a/test/unit/fixture/reverse_standard.js b/test/unit/fixture/reverse_standard.js index b70fe958..21bc08d3 100644 --- a/test/unit/fixture/reverse_standard.js +++ b/test/unit/fixture/reverse_standard.js @@ -13,6 +13,11 @@ module.exports = { 'lon': -82.50622 } } + }, + { + 'terms': { + 'layer': ['venue', 'address', 'street'] + } }] } }, diff --git a/test/unit/fixture/reverse_with_boundary_country.js b/test/unit/fixture/reverse_with_boundary_country.js index c21cb44b..e50ed14d 100644 --- a/test/unit/fixture/reverse_with_boundary_country.js +++ b/test/unit/fixture/reverse_with_boundary_country.js @@ -23,6 +23,11 @@ module.exports = { 'lon': -82.50622 } } + }, + { + 'terms': { + 'layer': ['venue', 'address', 'street'] + } }] } }, diff --git a/test/unit/fixture/reverse_with_layer_filtering.js b/test/unit/fixture/reverse_with_layer_filtering.js index ee7ab7b0..dd9571ad 100644 --- a/test/unit/fixture/reverse_with_layer_filtering.js +++ b/test/unit/fixture/reverse_with_layer_filtering.js @@ -17,7 +17,7 @@ module.exports = { }, { 'terms': { - 'layer': ['country'] + 'layer': ['venue', 'address', 'street'] } } ] diff --git a/test/unit/fixture/reverse_with_layer_filtering_non_coarse_subset.js b/test/unit/fixture/reverse_with_layer_filtering_non_coarse_subset.js new file mode 100644 index 00000000..046a34e5 --- /dev/null +++ b/test/unit/fixture/reverse_with_layer_filtering_non_coarse_subset.js @@ -0,0 +1,41 @@ +var vs = require('../../../query/reverse_defaults'); + +module.exports = { + 'query': { + 'bool': { + 'filter': [ + { + 'geo_distance': { + 'distance': '500km', + 'distance_type': 'plane', + 'optimize_bbox': 'indexed', + 'center_point': { + 'lat': 29.49136, + 'lon': -82.50622 + } + } + }, + { + 'terms': { + 'layer': ['venue', 'street'] + } + } + ] + } + }, + 'sort': [ + '_score', + { + '_geo_distance': { + 'center_point': { + 'lat': 29.49136, + 'lon': -82.50622 + }, + 'order': 'asc', + 'distance_type': 'plane' + } + } + ], + 'size': vs.size, + 'track_scores': true +}; diff --git a/test/unit/fixture/reverse_with_source_filtering.js b/test/unit/fixture/reverse_with_source_filtering.js index 21e04662..3eb32bb4 100644 --- a/test/unit/fixture/reverse_with_source_filtering.js +++ b/test/unit/fixture/reverse_with_source_filtering.js @@ -19,6 +19,11 @@ module.exports = { 'terms': { 'source': ['test'] } + }, + { + 'terms': { + 'layer': ['venue', 'address', 'street'] + } } ] } diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index 24ada46a..a638fc53 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -132,23 +132,46 @@ module.exports.tests.query = function(test, common) { t.end(); }); - test('valid layers filter', function(t) { - var query = generate({ + test('valid layers filter', (t) => { + const query = generate({ 'point.lat': 29.49136, 'point.lon': -82.50622, 'boundary.circle.lat': 29.49136, 'boundary.circle.lon': -82.50622, 'boundary.circle.radius': 500, - 'layers': ['country'] + // only venue, address, and street layers should be retained + 'layers': ['neighbourhood', 'venue', 'locality', 'address', 'region', 'street', 'country'] }); - var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = require('../fixture/reverse_with_layer_filtering'); + const compiled = JSON.parse( JSON.stringify( query ) ); + const expected = require('../fixture/reverse_with_layer_filtering'); + + t.deepEqual(compiled.type, 'reverse', 'query type set'); + t.deepEqual(compiled.body, expected, 'valid reverse query with source filtering'); + t.end(); + + }); + + test('valid layers filter - subset of non-coarse layers', (t) => { + const query = generate({ + 'point.lat': 29.49136, + 'point.lon': -82.50622, + 'boundary.circle.lat': 29.49136, + 'boundary.circle.lon': -82.50622, + 'boundary.circle.radius': 500, + // only venue, address, and street layers should be retained + 'layers': ['neighbourhood', 'venue', 'street', 'locality'] + }); + + const compiled = JSON.parse( JSON.stringify( query ) ); + const expected = require('../fixture/reverse_with_layer_filtering_non_coarse_subset'); t.deepEqual(compiled.type, 'reverse', 'query type set'); t.deepEqual(compiled.body, expected, 'valid reverse query with source filtering'); t.end(); + }); + }; module.exports.all = function (tape, common) {