From aa7942cbb90b1b95199318c475ad92ea35cadc52 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Fri, 22 Jul 2016 15:36:12 -0400 Subject: [PATCH 1/4] Set up layer filtering for autocomplete and reverse This was missed by me when working on https://github.com/pelias/api/pull/580, but caught by the acceptance tests! Unfortunately it was caught after going to production. --- query/autocomplete.js | 6 ++ query/reverse.js | 4 ++ .../autocomplete_with_layer_filtering.js | 66 +++++++++++++++++++ .../fixture/reverse_with_layer_filtering.js | 41 ++++++++++++ test/unit/query/autocomplete.js | 16 +++++ test/unit/query/reverse.js | 17 +++++ 6 files changed, 150 insertions(+) create mode 100644 test/unit/fixture/autocomplete_with_layer_filtering.js create mode 100644 test/unit/fixture/reverse_with_layer_filtering.js diff --git a/query/autocomplete.js b/query/autocomplete.js index 6d5863a8..4233969a 100644 --- a/query/autocomplete.js +++ b/query/autocomplete.js @@ -47,6 +47,7 @@ query.score( peliasQuery.view.population( views.pop_subquery ) ); // non-scoring hard filters query.filter( peliasQuery.view.sources ); +query.filter( peliasQuery.view.layers ); // -------------------------------- @@ -63,6 +64,11 @@ function generateQuery( clean ){ vs.var( 'sources', clean.sources ); } + // layers + if( check.array(clean.layers) && clean.layers.length ){ + vs.var( 'layers', clean.layers); + } + // pass the input tokens to the views so they can choose which tokens // are relevant for their specific function. if( check.array( clean.tokens ) ){ diff --git a/query/reverse.js b/query/reverse.js index 13e9263c..f937e0c0 100644 --- a/query/reverse.js +++ b/query/reverse.js @@ -16,6 +16,7 @@ query.sort( peliasQuery.view.sort_distance ); // non-scoring hard filters query.filter( peliasQuery.view.boundary_circle ); query.filter( peliasQuery.view.sources ); +query.filter( peliasQuery.view.layers ); // -------------------------------- @@ -31,6 +32,9 @@ function generateQuery( clean ){ // sources vs.var( 'sources', clean.sources); + // layers + vs.var( 'layers', clean.layers); + // focus point to score by distance if( check.number(clean['point.lat']) && check.number(clean['point.lon']) ){ diff --git a/test/unit/fixture/autocomplete_with_layer_filtering.js b/test/unit/fixture/autocomplete_with_layer_filtering.js new file mode 100644 index 00000000..6f9d5e0f --- /dev/null +++ b/test/unit/fixture/autocomplete_with_layer_filtering.js @@ -0,0 +1,66 @@ + +module.exports = { + 'query': { + 'bool': { + 'must': [{ + 'constant_score': { + 'query': { + 'match': { + 'name.default': { + 'analyzer': 'peliasQueryPartialToken', + 'boost': 100, + 'query': 'test', + 'type': 'phrase', + 'operator': 'and', + 'slop': 3 + } + } + } + } + }], + 'should':[{ + 'function_score': { + 'query': { + 'match_all': {} + }, + 'max_boost': 20, + 'score_mode': 'first', + 'boost_mode': 'replace', + 'functions': [{ + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'popularity', + 'missing': 1 + }, + 'weight': 1 + }] + } + },{ + 'function_score': { + 'query': { + 'match_all': {} + }, + 'max_boost': 20, + 'score_mode': 'first', + 'boost_mode': 'replace', + 'functions': [{ + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'population', + 'missing': 1 + }, + 'weight': 3 + }] + } + }], + 'filter': [{ + 'terms': { + 'layer': ['country'] + } + }] + } + }, + 'sort': [ '_score' ], + 'size': 20, + 'track_scores': true +}; diff --git a/test/unit/fixture/reverse_with_layer_filtering.js b/test/unit/fixture/reverse_with_layer_filtering.js new file mode 100644 index 00000000..ee7ab7b0 --- /dev/null +++ b/test/unit/fixture/reverse_with_layer_filtering.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': ['country'] + } + } + ] + } + }, + '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/query/autocomplete.js b/test/unit/query/autocomplete.js index bb368fc9..676549fb 100644 --- a/test/unit/query/autocomplete.js +++ b/test/unit/query/autocomplete.js @@ -129,6 +129,22 @@ module.exports.tests.query = function(test, common) { t.end(); }); + test('valid layers filter', function(t) { + var query = generate({ + 'text': 'test', + 'layers': ['country'], + tokens: ['test'], + tokens_complete: [], + tokens_incomplete: ['test'] + }); + + var compiled = JSON.parse( JSON.stringify( query ) ); + var expected = require('../fixture/autocomplete_with_layer_filtering'); + + t.deepEqual(compiled, expected, 'valid autocomplete query with layer filtering'); + t.end(); + }); + test('single character street address', function(t) { var query = generate({ text: 'k road, laird', diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index db0daa12..03985fbd 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -125,6 +125,23 @@ module.exports.tests.query = function(test, common) { t.deepEqual(compiled, expected, 'valid reverse query with source filtering'); t.end(); }); + + test('valid layers filter', function(t) { + var 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'] + }); + + var compiled = JSON.parse( JSON.stringify( query ) ); + var expected = require('../fixture/reverse_with_layer_filtering'); + + t.deepEqual(compiled, expected, 'valid reverse query with source filtering'); + t.end(); + }); }; module.exports.all = function (tape, common) { From f18e278e51df9e79e2911d79c2675eea52dcd14f Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 25 Jul 2016 15:20:42 -0400 Subject: [PATCH 2/4] Handle case where record has no region_ a field --- middleware/confidenceScore.js | 2 +- test/unit/middleware/confidenceScore.js | 30 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/middleware/confidenceScore.js b/middleware/confidenceScore.js index 39ea45b2..71a39bad 100644 --- a/middleware/confidenceScore.js +++ b/middleware/confidenceScore.js @@ -93,7 +93,7 @@ function checkForDealBreakers(req, hit) { return false; } - if (check.assigned(req.clean.parsed_text.state) && req.clean.parsed_text.state !== hit.parent.region_a[0]) { + if (check.assigned(req.clean.parsed_text.state) && hit.parent.region_a && req.clean.parsed_text.state !== hit.parent.region_a[0]) { logger.debug('[confidence][deal-breaker]: state !== region_a'); return true; } diff --git a/test/unit/middleware/confidenceScore.js b/test/unit/middleware/confidenceScore.js index f629874b..e9b80177 100644 --- a/test/unit/middleware/confidenceScore.js +++ b/test/unit/middleware/confidenceScore.js @@ -94,6 +94,36 @@ module.exports.tests.confidenceScore = function(test, common) { t.end(); }); + test('undefined region fields should be handled gracefully', function(t) { + var req = { + clean: { + text: 'test name1, TX', + parsed_text: { + state: 'TX' + } + } + }; + var res = { + data: [{ + _score: 10, + found: true, + value: 1, + center_point: { lat: 100.1, lon: -50.5 }, + name: { default: 'test name1' }, + parent: { + country: ['country1'], + region: undefined, + region_a: undefined, + county: ['city1'] + } + }], + meta: {scores: [10]} + }; + + confidenceScore(req, res, function() {}); + t.equal(res.data[0].confidence, 0.54, 'score was set'); + t.end(); + }); }; module.exports.all = function (tape, common) { From 8cae743c80a5e638d53abd0146d44eb2efc40358 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 25 Jul 2016 15:24:14 -0400 Subject: [PATCH 3/4] Handle empty country_a values as well --- middleware/confidenceScore.js | 4 ++-- test/unit/middleware/confidenceScore.js | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/middleware/confidenceScore.js b/middleware/confidenceScore.js index 71a39bad..2e9eb1c6 100644 --- a/middleware/confidenceScore.js +++ b/middleware/confidenceScore.js @@ -219,8 +219,8 @@ function checkAddress(text, hit) { res += propMatch(text.number, (hit.address_parts ? hit.address_parts.number : null), false); res += propMatch(text.street, (hit.address_parts ? hit.address_parts.street : null), false); res += propMatch(text.postalcode, (hit.address_parts ? hit.address_parts.zip: null), true); - res += propMatch(text.state, hit.parent.region_a[0], true); - res += propMatch(text.country, hit.parent.country_a[0], true); + res += propMatch(text.state, (hit.parent.region_a ? hit.parent.region_a[0] : null), true); + res += propMatch(text.country, (hit.parent.country_a ? hit.parent.country_a[0] :null), true); res /= checkCount; } diff --git a/test/unit/middleware/confidenceScore.js b/test/unit/middleware/confidenceScore.js index e9b80177..7d6ba87d 100644 --- a/test/unit/middleware/confidenceScore.js +++ b/test/unit/middleware/confidenceScore.js @@ -97,9 +97,11 @@ module.exports.tests.confidenceScore = function(test, common) { test('undefined region fields should be handled gracefully', function(t) { var req = { clean: { - text: 'test name1, TX', + text: '123 Main St, City, NM', parsed_text: { - state: 'TX' + number: 123, + street: 'Main St', + state: 'NM' } } }; @@ -121,7 +123,7 @@ module.exports.tests.confidenceScore = function(test, common) { }; confidenceScore(req, res, function() {}); - t.equal(res.data[0].confidence, 0.54, 'score was set'); + t.equal(res.data[0].confidence, 0.28, 'score was set'); t.end(); }); }; From c210b50c119de11cedef47ab841f40c365db92ae Mon Sep 17 00:00:00 2001 From: greenkeeperio-bot Date: Wed, 27 Jul 2016 11:18:01 -0700 Subject: [PATCH 4/4] chore(package): update pelias-text-analyzer to version 1.2.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c3a3d01a..19314d04 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "pelias-logger": "0.0.8", "pelias-model": "4.1.0", "pelias-query": "8.1.3", - "pelias-text-analyzer": "1.1.0", + "pelias-text-analyzer": "1.2.0", "stats-lite": "2.0.3", "through2": "2.0.1" },