From 345c1be87d7cb3075df1b9ab101dbe6008487c9c Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Mon, 23 Oct 2017 09:34:24 -0400 Subject: [PATCH] allow venues lookup to proceed if venues are allowed --- controller/predicates/is_layer_requested.js | 19 +++ routes/v1.js | 5 +- .../predicates/is_layer_requested.js | 109 ++++++++++++++++++ test/unit/run.js | 1 + 4 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 controller/predicates/is_layer_requested.js create mode 100644 test/unit/controller/predicates/is_layer_requested.js diff --git a/controller/predicates/is_layer_requested.js b/controller/predicates/is_layer_requested.js new file mode 100644 index 00000000..1f635601 --- /dev/null +++ b/controller/predicates/is_layer_requested.js @@ -0,0 +1,19 @@ +const _ = require('lodash'); +const Debug = require('../../helper/debug'); +const debugLog = new Debug('controller:predicates:is_admin_only_analysis'); +const stackTraceLine = require('../../helper/stackTraceLine'); + +// this function returns true IFF request.clean.layers is either undefined or +// contains the supplied layer +module.exports = layer => { + return (request, response) => { + // default request.clean.layers to the requested layer for simpler "contains" logic + const is_layer_requested = _.get(request, ['clean', 'layers'], [layer]).indexOf(layer) >= 0; + + debugLog.push(request, () => ({ + reply: is_layer_requested, + stack_trace: stackTraceLine() + })); + return is_layer_requested; + }; +}; diff --git a/routes/v1.js b/routes/v1.js index 44bcd474..fcc027cd 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -86,10 +86,11 @@ const isRequestSourcesOnlyWhosOnFirst = require('../controller/predicates/is_req const hasRequestParameter = require('../controller/predicates/has_request_parameter'); const hasParsedTextProperties = require('../controller/predicates/has_parsed_text_properties'); const isSingleFieldAnalysis = require('../controller/predicates/is_single_field_analysis'); +const isVenueRequested = require('../controller/predicates/is_layer_requested')('venue'); // shorthand for standard early-exit conditions const hasResponseDataOrRequestErrors = any(hasResponseData, hasRequestErrors); -const hasAdminOnlyResults = not(hasResultsAtLayers.any(['venue', 'address', 'street'])); +const hasAdminOnlyResults = not(hasResultsAtLayers.any(['address', 'street'])); const hasNumberButNotStreet = all( hasParsedTextProperties.any('number'), @@ -266,9 +267,11 @@ function addRoutes(app, peliasConfig) { // - analysis is only admin (no address, query, or street) // - there's a single field in analysis // - request has a focus.point available + // - TODO: needs check for venues is in layers // https://github.com/pelias/pelias/issues/564 const venuesSearchShouldExecute = all( not(hasRequestErrors), + isVenueRequested, isAdminOnlyAnalysis, isSingleFieldAnalysis, hasRequestFocusPoint diff --git a/test/unit/controller/predicates/is_layer_requested.js b/test/unit/controller/predicates/is_layer_requested.js new file mode 100644 index 00000000..5710b041 --- /dev/null +++ b/test/unit/controller/predicates/is_layer_requested.js @@ -0,0 +1,109 @@ +'use strict'; + +const _ = require('lodash'); +const is_layer_requested = require('../../../../controller/predicates/is_layer_requested'); +const all_layers = require('../../../../helper/type_mapping').layers; + +module.exports.tests = {}; + +module.exports.tests.interface = (test, common) => { + test('valid interface', t => { + t.equal(typeof is_layer_requested, 'function', 'is_layer_requested is a function'); + t.end(); + }); +}; + +module.exports.tests.false_conditions = (test, common) => { + test('request with empty layers should return false', t => { + const is_venue_requested = is_layer_requested('venue'); + + const req = { + clean: { + layers: [] + } + }; + + t.notOk(is_venue_requested(req)); + t.end(); + + }); + + test('request with layers just "address" or "venue" return false', t => { + const is_venue_requested = is_layer_requested('venue'); + + _.without(all_layers, 'venue').forEach(non_venue_layer => { + const req = { + clean: { + layers: [non_venue_layer] + } + }; + + t.notOk(is_venue_requested(req)); + + }); + + t.end(); + + }); + +}; + +module.exports.tests.true_conditions = (test, common) => { + test('undefined layers should return true since all layers are implied', t => { + const req = { + clean: {} + }; + + t.ok(is_layer_requested(req)); + t.end(); + + }); + + test('layers with venue and any others should return true', t => { + const is_venue_requested = is_layer_requested('venue'); + + _.without(all_layers, 'venue').forEach(non_venue_layer => { + const req = { + clean: { + layers: [non_venue_layer, 'venue'] + } + }; + + t.ok(is_layer_requested(req)); + + }); + + t.end(); + + }); + + test('layers with locality and any others should return true', t => { + // this test shows that predicate isn't venue-specific since all other tests use venue + const is_venue_requested = is_layer_requested('locality'); + + _.without(all_layers, 'locality').forEach(non_locality_layer => { + const req = { + clean: { + layers: [non_locality_layer, 'venue'] + } + }; + + t.ok(is_layer_requested(req)); + + }); + + t.end(); + + }); + +}; + +module.exports.all = (tape, common) => { + function test(name, testFunction) { + return tape(`GET /is_layer_requested ${name}`, testFunction); + } + + for( const testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/run.js b/test/unit/run.js index f553a2ff..d7de2636 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -29,6 +29,7 @@ var tests = [ require('./controller/predicates/is_addressit_parse'), require('./controller/predicates/is_admin_only_analysis'), require('./controller/predicates/is_coarse_reverse'), + require('./controller/predicates/is_layer_requested'), require('./controller/predicates/is_only_non_admin_layers'), require('./controller/predicates/is_request_sources_only_whosonfirst'), require('./controller/predicates/is_single_field_analysis'),