From 68c067ee8cf76d702501f3347236a0f8cf61e974 Mon Sep 17 00:00:00 2001 From: Tyler Pedelose Date: Tue, 12 Jun 2018 15:39:55 -0400 Subject: [PATCH 1/6] Fix for Pelias/Pelias#670 --- routes/v1.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/routes/v1.js b/routes/v1.js index 6b40f1d6..f356d5b1 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -166,11 +166,11 @@ function addRoutes(app, peliasConfig) { hasRequestCategories ) ), - any( - // only geodisambiguate if libpostal returned only admin areas or libpostal was skipped - isAdminOnlyAnalysis, - isRequestSourcesOnlyWhosOnFirst - ) + isRequestSourcesOnlyWhosOnFirst + // all( + // only geodisambiguate if libpostal was skipped libpostal returned only admin areas or + // isAdminOnlyAnalysis, + // ) ); // execute placeholder if libpostal identified address parts but ids need to From a0893cfb701e75a7b72e60b102efa14968e2c089 Mon Sep 17 00:00:00 2001 From: Tyler Pedelose Date: Wed, 20 Jun 2018 12:04:15 -0400 Subject: [PATCH 2/6] Removed extra comments --- routes/v1.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/routes/v1.js b/routes/v1.js index f356d5b1..eb217086 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -167,10 +167,6 @@ function addRoutes(app, peliasConfig) { ) ), isRequestSourcesOnlyWhosOnFirst - // all( - // only geodisambiguate if libpostal was skipped libpostal returned only admin areas or - // isAdminOnlyAnalysis, - // ) ); // execute placeholder if libpostal identified address parts but ids need to From 52a66e1bc0e098707906659445bb44dbb0787e0e Mon Sep 17 00:00:00 2001 From: Tyler Pedelose Date: Fri, 22 Jun 2018 16:49:54 -0400 Subject: [PATCH 3/6] Added new predicates to aid placeholderGeodisambiguationShouldExecute --- ...is_request_sources_includes_whosonfirst.js | 17 ++++ .../is_request_sources_undefined.js | 17 ++++ routes/v1.js | 14 ++- ...is_request_sources_includes_whosonfirst.js | 92 +++++++++++++++++++ .../is_request_sources_undefined.js | 78 ++++++++++++++++ test/unit/run.js | 2 + 6 files changed, 219 insertions(+), 1 deletion(-) create mode 100644 controller/predicates/is_request_sources_includes_whosonfirst.js create mode 100644 controller/predicates/is_request_sources_undefined.js create mode 100644 test/unit/controller/predicates/is_request_sources_includes_whosonfirst.js create mode 100644 test/unit/controller/predicates/is_request_sources_undefined.js diff --git a/controller/predicates/is_request_sources_includes_whosonfirst.js b/controller/predicates/is_request_sources_includes_whosonfirst.js new file mode 100644 index 00000000..0f43921b --- /dev/null +++ b/controller/predicates/is_request_sources_includes_whosonfirst.js @@ -0,0 +1,17 @@ +const _ = require('lodash'); +const Debug = require('../../helper/debug'); +const debugLog = new Debug('controller:predicates:is_request_sources_includes_whosonfirst'); +const stackTraceLine = require('../../helper/stackTraceLine'); + +// returns true IFF 'whosonfirst' is included in the requested sources +module.exports = (req, res) => { + const is_request_sources_includes_whosonfirst = _.includes( + _.get(req, 'clean.sources', []), + 'whosonfirst' + ); + debugLog.push(req, () => ({ + reply: is_request_sources_includes_whosonfirst, + stack_trace: stackTraceLine() + })); + return is_request_sources_includes_whosonfirst; +}; diff --git a/controller/predicates/is_request_sources_undefined.js b/controller/predicates/is_request_sources_undefined.js new file mode 100644 index 00000000..504a5f14 --- /dev/null +++ b/controller/predicates/is_request_sources_undefined.js @@ -0,0 +1,17 @@ +const _ = require('lodash'); +const Debug = require('../../helper/debug'); +const debugLog = new Debug('controller:predicates:is_request_sources_undefined'); +const stackTraceLine = require('../../helper/stackTraceLine'); + +// returns true IFF there are no requested sources +module.exports = (req, res) => { + const is_request_sources_undefined = _.isEqual( + _.get(req, 'clean.sources', []), + [] + ); + debugLog.push(req, () => ({ + reply: is_request_sources_undefined, + stack_trace: stackTraceLine() + })); + return is_request_sources_undefined; +}; diff --git a/routes/v1.js b/routes/v1.js index eb217086..d0f5a99a 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -80,6 +80,9 @@ const hasRequestCategories = require('../controller/predicates/has_request_param const isOnlyNonAdminLayers = require('../controller/predicates/is_only_non_admin_layers'); // this can probably be more generalized const isRequestSourcesOnlyWhosOnFirst = require('../controller/predicates/is_request_sources_only_whosonfirst'); +// const isRequestSourcesIncludesWhosOnFirst = require('../controller/predicates/is_request_sources_includes_whosonfirst'); +// const isRequestSourcesUndefined = require('../controller/predicates/is_request_sources_undefined'); + const hasRequestParameter = require('../controller/predicates/has_request_parameter'); const hasParsedTextProperties = require('../controller/predicates/has_parsed_text_properties'); @@ -166,7 +169,16 @@ function addRoutes(app, peliasConfig) { hasRequestCategories ) ), - isRequestSourcesOnlyWhosOnFirst + any( + isRequestSourcesOnlyWhosOnFirst, + all( + isAdminOnlyAnalysis, + any( + isRequestSourcesUndefined, + isRequestSourcesIncludesWhosOnFirst + ) + ) + ) ); // execute placeholder if libpostal identified address parts but ids need to diff --git a/test/unit/controller/predicates/is_request_sources_includes_whosonfirst.js b/test/unit/controller/predicates/is_request_sources_includes_whosonfirst.js new file mode 100644 index 00000000..d3e51004 --- /dev/null +++ b/test/unit/controller/predicates/is_request_sources_includes_whosonfirst.js @@ -0,0 +1,92 @@ +const _ = require('lodash'); +const is_request_sources_includes_whosonfirst = require('../../../../controller/predicates/is_request_sources_includes_whosonfirst'); + +module.exports.tests = {}; + +module.exports.tests.interface = (test, common) => { + test('valid interface', (t) => { + t.ok(_.isFunction(is_request_sources_includes_whosonfirst), 'is_request_sources_includes_whosonfirst is a function'); + t.end(); + }); +}; + +module.exports.tests.true_conditions = (test, common) => { + test('sources includes \'whosonfirst\' should return true', (t) => { + const req = { + clean: { + sources: [ + 'whosonfirst', + 'not whosonfirst' + ] + } + }; + + t.ok(is_request_sources_includes_whosonfirst(req)); + t.end(); + + }); + + test('empty req.clean.sources should return false', (t) => { + const req = { + clean: { + sources: [] + } + }; + + t.notOk(is_request_sources_includes_whosonfirst(req)); + t.end(); + + }); + +}; + +module.exports.tests.false_conditions = (test, common) => { + test('undefined req should return false', (t) => { + t.notOk(is_request_sources_includes_whosonfirst(undefined)); + t.end(); + + }); + + test('undefined req.clean should return false', (t) => { + const req = {}; + + t.notOk(is_request_sources_includes_whosonfirst(req)); + t.end(); + + }); + + test('undefined req.clean.sources should return false', (t) => { + const req = { + clean: {} + }; + + t.notOk(is_request_sources_includes_whosonfirst(req)); + t.end(); + + }); + + test('sources not \'whosonfirst\' should return false', (t) => { + const req = { + clean: { + sources: [ + 'not whosonfirst' + ] + } + }; + + t.notOk(is_request_sources_includes_whosonfirst(req)); + t.end(); + + }) + +}; + +module.exports.all = (tape, common) => { + function test(name, testFunction) { + return tape(`GET /is_request_sources_includes_whosonfirst ${name}`, testFunction); + } + + for( const testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/controller/predicates/is_request_sources_undefined.js b/test/unit/controller/predicates/is_request_sources_undefined.js new file mode 100644 index 00000000..d7486bb8 --- /dev/null +++ b/test/unit/controller/predicates/is_request_sources_undefined.js @@ -0,0 +1,78 @@ +const _ = require('lodash'); +const is_request_sources_undefined = require('../../../../controller/predicates/is_request_sources_undefined'); + +module.exports.tests = {}; + +module.exports.tests.interface = (test, common) => { + test('valid interface', (t) => { + t.ok(_.isFunction(is_request_sources_undefined), 'is_request_sources_undefined is a function'); + t.end(); + }); +}; + +module.exports.tests.true_conditions = (test, common) => { + test('undefined req should return true', (t) => { + + t.ok(is_request_sources_undefined(undefined)); + t.end(); + + }); + + test('undefined req.clean should return true', (t) => { + const req = {}; + + t.ok(is_request_sources_undefined(req)); + t.end(); + + }); + + test('undefined req.clean.sources should return true', (t) => { + const req = { + clean: {} + }; + + t.ok(is_request_sources_undefined(req)); + t.end(); + + }); + + test('empty req.clean.sources should return true', (t) => { + const req = { + clean: { + sources: [] + } + }; + + t.ok(is_request_sources_undefined(req)); + t.end(); + + }); + +}; + +module.exports.tests.false_conditions = (test, common) => { + test('sources not empty should return false', (t) => { + const req = { + clean: { + sources: [ + 'not empty' + ] + } + }; + + t.notOk(is_request_sources_undefined(req)); + t.end(); + + }); + +}; + +module.exports.all = (tape, common) => { + function test(name, testFunction) { + return tape(`GET /is_request_sources_undefined ${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 404cd86b..ae93bd4f 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -29,7 +29,9 @@ var tests = [ require('./controller/predicates/is_admin_only_analysis'), require('./controller/predicates/is_coarse_reverse'), require('./controller/predicates/is_only_non_admin_layers'), + require('./controller/predicates/is_request_sources_includes_whosonfirst'), require('./controller/predicates/is_request_sources_only_whosonfirst'), + require('./controller/predicates/is_request_sources_undefined'), require('./helper/debug'), require('./helper/diffPlaces'), require('./helper/fieldValue'), From df3e78ecdd701a4d69669e31b52a194b0cf19238 Mon Sep 17 00:00:00 2001 From: Tyler Pedelose Date: Mon, 25 Jun 2018 19:01:51 -0400 Subject: [PATCH 4/6] Uncomment predicate imports --- routes/v1.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routes/v1.js b/routes/v1.js index d0f5a99a..3af07fc9 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -80,8 +80,8 @@ const hasRequestCategories = require('../controller/predicates/has_request_param const isOnlyNonAdminLayers = require('../controller/predicates/is_only_non_admin_layers'); // this can probably be more generalized const isRequestSourcesOnlyWhosOnFirst = require('../controller/predicates/is_request_sources_only_whosonfirst'); -// const isRequestSourcesIncludesWhosOnFirst = require('../controller/predicates/is_request_sources_includes_whosonfirst'); -// const isRequestSourcesUndefined = require('../controller/predicates/is_request_sources_undefined'); +const isRequestSourcesIncludesWhosOnFirst = require('../controller/predicates/is_request_sources_includes_whosonfirst'); +const isRequestSourcesUndefined = require('../controller/predicates/is_request_sources_undefined'); const hasRequestParameter = require('../controller/predicates/has_request_parameter'); const hasParsedTextProperties = require('../controller/predicates/has_parsed_text_properties'); From 85df33cdaa3de42db1faae3b6809b29f2603e326 Mon Sep 17 00:00:00 2001 From: Tyler Pedelose Date: Tue, 26 Jun 2018 12:22:20 -0400 Subject: [PATCH 5/6] Made is_request_sources_undefined more direct --- controller/predicates/is_request_sources_undefined.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/controller/predicates/is_request_sources_undefined.js b/controller/predicates/is_request_sources_undefined.js index 504a5f14..77e8afea 100644 --- a/controller/predicates/is_request_sources_undefined.js +++ b/controller/predicates/is_request_sources_undefined.js @@ -5,9 +5,8 @@ const stackTraceLine = require('../../helper/stackTraceLine'); // returns true IFF there are no requested sources module.exports = (req, res) => { - const is_request_sources_undefined = _.isEqual( - _.get(req, 'clean.sources', []), - [] + const is_request_sources_undefined = _.isEmpty( + _.get(req, 'clean.sources') ); debugLog.push(req, () => ({ reply: is_request_sources_undefined, From be82c05c92c7fd869fc611e61f0bbd5d5c415d1c Mon Sep 17 00:00:00 2001 From: Tyler Pedelose Date: Tue, 26 Jun 2018 12:32:30 -0400 Subject: [PATCH 6/6] Moved includes check from lodash to native array in is_request_sources_includes_whosonfirst --- .../predicates/is_request_sources_includes_whosonfirst.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/controller/predicates/is_request_sources_includes_whosonfirst.js b/controller/predicates/is_request_sources_includes_whosonfirst.js index 0f43921b..fe3fcd8b 100644 --- a/controller/predicates/is_request_sources_includes_whosonfirst.js +++ b/controller/predicates/is_request_sources_includes_whosonfirst.js @@ -5,8 +5,7 @@ const stackTraceLine = require('../../helper/stackTraceLine'); // returns true IFF 'whosonfirst' is included in the requested sources module.exports = (req, res) => { - const is_request_sources_includes_whosonfirst = _.includes( - _.get(req, 'clean.sources', []), + const is_request_sources_includes_whosonfirst = _.get(req, 'clean.sources', []).includes( 'whosonfirst' ); debugLog.push(req, () => ({