From d721101f700799d3203296e661b337f7e646d97a Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Wed, 23 Nov 2016 00:11:59 -0500 Subject: [PATCH 01/20] tie query to specific hash --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index d9a588d1..80fd13e3 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ "pelias-labels": "1.5.1", "pelias-logger": "0.1.0", "pelias-model": "4.3.0", - "pelias-query": "8.9.0", + "pelias-query": "pelias/query#5338af9", "pelias-text-analyzer": "1.6.0", "stats-lite": "2.0.3", "through2": "2.0.1" From da72a968f9e619e0604ab5be33271fb1d8c3ea5f Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Wed, 23 Nov 2016 00:12:57 -0500 Subject: [PATCH 02/20] switch importance of address/street --- middleware/trimByGranularityComponent.js | 4 ++-- .../middleware/trimByGranularityComponent.js | 24 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/middleware/trimByGranularityComponent.js b/middleware/trimByGranularityComponent.js index afef01f0..1fe94ce8 100644 --- a/middleware/trimByGranularityComponent.js +++ b/middleware/trimByGranularityComponent.js @@ -22,8 +22,8 @@ const _ = require('lodash'); // supplied we want to retain borough=Manhattan and city=Manhattan results const layers = [ 'venue', - 'street', 'address', + 'street', 'neighbourhood', ['borough', 'locality'], 'localadmin', @@ -44,8 +44,8 @@ const layers = [ // city=Manhattan const explicit_borough_layers = [ 'venue', - 'street', 'address', + 'street', 'neighbourhood', 'borough', 'locality', diff --git a/test/unit/middleware/trimByGranularityComponent.js b/test/unit/middleware/trimByGranularityComponent.js index 69c80e92..d90a3803 100644 --- a/test/unit/middleware/trimByGranularityComponent.js +++ b/test/unit/middleware/trimByGranularityComponent.js @@ -19,8 +19,8 @@ module.exports.tests.trimByGranularity = function(test, common) { data: [ { name: 'venue 1', _matched_queries: ['fallback.venue'] }, { name: 'venue 2', _matched_queries: ['fallback.venue'] }, - { name: 'street 1', _matched_queries: ['fallback.street'] }, { name: 'address 1', _matched_queries: ['fallback.address'] }, + { name: 'street 1', _matched_queries: ['fallback.street'] }, { name: 'neighbourhood 1', _matched_queries: ['fallback.neighbourhood'] }, { name: 'borough 1', _matched_queries: ['fallback.borough'] }, { name: 'locality 1', _matched_queries: ['fallback.locality'] }, @@ -55,9 +55,9 @@ module.exports.tests.trimByGranularity = function(test, common) { var res = { data: [ - { name: 'street 1', _matched_queries: ['fallback.street'] }, - { name: 'street 2', _matched_queries: ['fallback.street'] }, { name: 'address 1', _matched_queries: ['fallback.address'] }, + { name: 'address 2', _matched_queries: ['fallback.address'] }, + { name: 'street 1', _matched_queries: ['fallback.street'] }, { name: 'neighbourhood 1', _matched_queries: ['fallback.neighbourhood'] }, { name: 'borough 1', _matched_queries: ['fallback.borough'] }, { name: 'locality 1', _matched_queries: ['fallback.locality'] }, @@ -73,13 +73,13 @@ module.exports.tests.trimByGranularity = function(test, common) { }; var expected_data = [ - { name: 'street 1', _matched_queries: ['fallback.street'] }, - { name: 'street 2', _matched_queries: ['fallback.street'] } + { name: 'address 1', _matched_queries: ['fallback.address'] }, + { name: 'address 2', _matched_queries: ['fallback.address'] } ]; function testIt() { trimByGranularity(req, res, function() { - t.deepEquals(res.data, expected_data, 'only street records should be here'); + t.deepEquals(res.data, expected_data, 'only address records should be here'); t.end(); }); } @@ -87,13 +87,13 @@ module.exports.tests.trimByGranularity = function(test, common) { testIt(); }); - test('all records with fallback.* matched_queries name should retain only address when they are most granular', function(t) { + test('all records with fallback.* matched_queries name should retain only street when they are most granular', function(t) { var req = { clean: {} }; var res = { data: [ - { name: 'address 1', _matched_queries: ['fallback.address'] }, - { name: 'address 2', _matched_queries: ['fallback.address'] }, + { name: 'street 1', _matched_queries: ['fallback.street'] }, + { name: 'street 2', _matched_queries: ['fallback.street'] }, { name: 'neighbourhood 1', _matched_queries: ['fallback.neighbourhood'] }, { name: 'borough 1', _matched_queries: ['fallback.borough'] }, { name: 'locality 1', _matched_queries: ['fallback.locality'] }, @@ -109,13 +109,13 @@ module.exports.tests.trimByGranularity = function(test, common) { }; var expected_data = [ - { name: 'address 1', _matched_queries: ['fallback.address'] }, - { name: 'address 2', _matched_queries: ['fallback.address'] }, + { name: 'street 1', _matched_queries: ['fallback.street'] }, + { name: 'street 2', _matched_queries: ['fallback.street'] }, ]; function testIt() { trimByGranularity(req, res, function() { - t.deepEquals(res.data, expected_data, 'only address records should be here'); + t.deepEquals(res.data, expected_data, 'only street records should be here'); t.end(); }); } From 7aad07368debee6e637acf17a617c18c790b5135 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Wed, 23 Nov 2016 00:13:28 -0500 Subject: [PATCH 03/20] modified tests to accommodate changes to ComponentFallbackQuery --- .../fixture/component_geocoding/fallback.json | 193 +++++++++++++++++- test/unit/query/component_geocoding.js | 9 - 2 files changed, 192 insertions(+), 10 deletions(-) diff --git a/test/unit/fixture/component_geocoding/fallback.json b/test/unit/fixture/component_geocoding/fallback.json index 11891881..965b188f 100644 --- a/test/unit/fixture/component_geocoding/fallback.json +++ b/test/unit/fixture/component_geocoding/fallback.json @@ -6,6 +6,197 @@ "query": { "bool": { "should": [ + { + "bool": { + "_name": "fallback.address", + "must": [ + { + "match_phrase": { + "address_parts.number": "number value" + } + }, + { + "match_phrase": { + "address_parts.street": "street value" + } + }, + { + "multi_match": { + "query": "neighbourhood value", + "type": "phrase", + "fields": [ + "parent.neighbourhood", + "parent.neighbourhood_a" + ] + } + }, + { + "multi_match": { + "query": "borough value", + "type": "phrase", + "fields": [ + "parent.borough", + "parent.borough_a" + ] + } + }, + { + "multi_match": { + "query": "city value", + "type": "phrase", + "fields": [ + "parent.locality", + "parent.locality_a", + "parent.localadmin", + "parent.localadmin_a" + ] + } + }, + { + "multi_match": { + "query": "county value", + "type": "phrase", + "fields": [ + "parent.county", + "parent.county_a", + "parent.macrocounty", + "parent.macrocounty_a" + ] + } + }, + { + "multi_match": { + "query": "state value", + "type": "phrase", + "fields": [ + "parent.region", + "parent.region_a", + "parent.macroregion", + "parent.macroregion_a" + ] + } + }, + { + "multi_match": { + "query": "country value", + "type": "phrase", + "fields": [ + "parent.country", + "parent.country_a", + "parent.dependency", + "parent.dependency_a" + ] + } + } + ], + "should": [ + { + "match_phrase": { + "address_parts.zip": "postalcode value" + } + } + ], + "filter": { + "term": { + "layer": "address" + } + }, + "boost": 50 + } + }, + { + "bool": { + "_name": "fallback.street", + "must": [ + { + "match_phrase": { + "address_parts.street": "street value" + } + }, + { + "multi_match": { + "query": "neighbourhood value", + "type": "phrase", + "fields": [ + "parent.neighbourhood", + "parent.neighbourhood_a" + ] + } + }, + { + "multi_match": { + "query": "borough value", + "type": "phrase", + "fields": [ + "parent.borough", + "parent.borough_a" + ] + } + }, + { + "multi_match": { + "query": "city value", + "type": "phrase", + "fields": [ + "parent.locality", + "parent.locality_a", + "parent.localadmin", + "parent.localadmin_a" + ] + } + }, + { + "multi_match": { + "query": "county value", + "type": "phrase", + "fields": [ + "parent.county", + "parent.county_a", + "parent.macrocounty", + "parent.macrocounty_a" + ] + } + }, + { + "multi_match": { + "query": "state value", + "type": "phrase", + "fields": [ + "parent.region", + "parent.region_a", + "parent.macroregion", + "parent.macroregion_a" + ] + } + }, + { + "multi_match": { + "query": "country value", + "type": "phrase", + "fields": [ + "parent.country", + "parent.country_a", + "parent.dependency", + "parent.dependency_a" + ] + } + } + ], + "should": [ + { + "match_phrase": { + "address_parts.zip": "postalcode value" + } + } + ], + "filter": { + "term": { + "layer": "street" + } + }, + "boost": 100 + } + }, { "bool": { "_name": "fallback.neighbourhood", @@ -514,4 +705,4 @@ ], "size": 20, "track_scores": true -} \ No newline at end of file +} diff --git a/test/unit/query/component_geocoding.js b/test/unit/query/component_geocoding.js index 68bedf6a..42e4736d 100644 --- a/test/unit/query/component_geocoding.js +++ b/test/unit/query/component_geocoding.js @@ -14,7 +14,6 @@ module.exports.tests.query = function(test, common) { test('valid search + focus + bbox', function(t) { var clean = { parsed_text: { - street: 'street value' }, text: 'test', querySize: 10, @@ -39,7 +38,6 @@ module.exports.tests.query = function(test, common) { test('valid search + bbox', function(t) { var clean = { parsed_text: { - street: 'street value' }, text: 'test', querySize: 10, @@ -63,7 +61,6 @@ module.exports.tests.query = function(test, common) { test('valid lingustic-only search', function(t) { var clean = { parsed_text: { - street: 'street value' }, text: 'test', querySize: 10, layers: ['test'] @@ -82,7 +79,6 @@ module.exports.tests.query = function(test, common) { test('search search + focus', function(t) { var clean = { parsed_text: { - street: 'street value' }, text: 'test', querySize: 10, 'focus.point.lat': 29.49136, 'focus.point.lon': -82.50622, @@ -102,7 +98,6 @@ module.exports.tests.query = function(test, common) { test('search search + viewport', function(t) { var clean = { parsed_text: { - street: 'street value' }, text: 'test', querySize: 10, 'focus.viewport.min_lat': 28.49136, @@ -127,7 +122,6 @@ module.exports.tests.query = function(test, common) { test('search with viewport diagonal < 1km should set scale to 1km', function(t) { var clean = { parsed_text: { - street: 'street value' }, text: 'test', querySize: 10, 'focus.viewport.min_lat': 28.49135, @@ -150,7 +144,6 @@ module.exports.tests.query = function(test, common) { test('search search + focus on null island', function(t) { var clean = { parsed_text: { - street: 'street value' }, text: 'test', querySize: 10, 'focus.point.lat': 0, 'focus.point.lon': 0, @@ -198,7 +191,6 @@ module.exports.tests.query = function(test, common) { test('valid boundary.country search', function(t) { var clean = { parsed_text: { - street: 'street value' }, text: 'test', querySize: 10, layers: ['test'], @@ -218,7 +210,6 @@ module.exports.tests.query = function(test, common) { test('valid sources filter', function(t) { var clean = { parsed_text: { - street: 'street value' }, 'text': 'test', 'sources': ['test_source'] From 75366f98c420c8b4fd3079bf35ff3aa24837e139 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Wed, 23 Nov 2016 00:14:25 -0500 Subject: [PATCH 04/20] added text-analyzer call to parse supplied `address` field --- sanitizer/_synthesize_analysis.js | 28 +++++ test/unit/sanitizer/_synthesize_analysis.js | 124 +++++++++++++++++++- 2 files changed, 149 insertions(+), 3 deletions(-) diff --git a/sanitizer/_synthesize_analysis.js b/sanitizer/_synthesize_analysis.js index 04ffeb34..79c53187 100644 --- a/sanitizer/_synthesize_analysis.js +++ b/sanitizer/_synthesize_analysis.js @@ -1,4 +1,5 @@ const _ = require('lodash'); +const text_analyzer = require('pelias-text-analyzer'); const fields = { 'address': 'address', @@ -20,6 +21,15 @@ function isPostalCodeOnly(parsed_text) { parsed_text.hasOwnProperty('postalcode'); } +function getHouseNumberField(analyzed_address) { + for (var field of ['number', 'postalcode']) { + if (analyzed_address.hasOwnProperty(field)) { + return field; + } + } + +} + function sanitize( raw, clean ){ // error & warning messages @@ -43,6 +53,24 @@ function sanitize( raw, clean ){ `at least one of the following fields is required: ${Object.keys(fields).join(', ')}`); } + if (clean.parsed_text.hasOwnProperty('address')) { + var analyzed_address = text_analyzer.parse(clean.parsed_text.address); + + const house_number_field = getHouseNumberField(analyzed_address); + + if (house_number_field) { + clean.parsed_text.number = analyzed_address[house_number_field]; + + clean.parsed_text.street = _.trim(_.replace(clean.parsed_text.address, clean.parsed_text.number, '')); + delete clean.parsed_text.address; + + } else { + clean.parsed_text.street = clean.parsed_text.address; + delete clean.parsed_text.address; + } + + } + return messages; } diff --git a/test/unit/sanitizer/_synthesize_analysis.js b/test/unit/sanitizer/_synthesize_analysis.js index 20f05fc0..dabacef8 100644 --- a/test/unit/sanitizer/_synthesize_analysis.js +++ b/test/unit/sanitizer/_synthesize_analysis.js @@ -1,13 +1,18 @@ -const sanitizer = require('../../../sanitizer/_synthesize_analysis'); const _ = require('lodash'); +const proxyquire = require('proxyquire').noCallThru(); module.exports.tests = {}; module.exports.tests.text_parser = function(test, common) { test('all variables should be parsed', function(t) { + var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { + 'pelias-text-analyzer': { parse: function(query) { + t.fail('parse should not have been called'); + } + }}); + const raw = { query: ' \t query \t value \t ', - address: ' \t address \t value \t ', neighbourhood: ' \t neighbourhood \t value \t ', borough: ' \t borough \t value \t ', locality: ' \t locality \t value \t ', @@ -21,7 +26,6 @@ module.exports.tests.text_parser = function(test, common) { const expected_clean = { parsed_text: { - address: 'address value', neighbourhood: 'neighbourhood value', borough: 'borough value', city: 'locality value', @@ -42,6 +46,12 @@ module.exports.tests.text_parser = function(test, common) { }); test('non-string and blank string values should be treated as not supplied', function(t) { + var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { + 'pelias-text-analyzer': { parse: function(query) { + t.fail('parse should not have been called'); + } + }}); + // helper to return a random value that's considered invalid function getInvalidValue() { return _.sample([{}, [], false, '', ' \t ', 17, undefined]); @@ -75,6 +85,12 @@ module.exports.tests.text_parser = function(test, common) { }); test('no supplied fields should return error', function(t) { + var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { + 'pelias-text-analyzer': { parse: function(query) { + t.fail('parse should not have been called'); + } + }}); + const raw = {}; const clean = {}; @@ -92,6 +108,12 @@ module.exports.tests.text_parser = function(test, common) { }); test('postalcode-only parsed_text should return error', function(t) { + var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { + 'pelias-text-analyzer': { parse: function(query) { + t.fail('parse should not have been called'); + } + }}); + const raw = { postalcode: 'postalcode value' }; @@ -113,6 +135,102 @@ module.exports.tests.text_parser = function(test, common) { }); + test('text_analyzer identifying house number should extract it and street', function(t) { + var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { + 'pelias-text-analyzer': { parse: function(query) { + t.equals(query, 'Number Value Street Value Number Value'); + + return { + number: 'Number Value' + }; + } + }}); + + const raw = { + address: 'Number Value Street Value Number Value' + }; + + const clean = {}; + + const expected_clean = { + parsed_text: { + number: 'Number Value', + street: 'Street Value Number Value' + } + }; + + const messages = sanitizer(raw, clean); + + t.deepEquals(clean, expected_clean); + t.deepEquals(messages.errors, [], 'no errors'); + t.deepEquals(messages.warnings, [], 'no warnings'); + t.end(); + + }); + + test('text_analyzer identifying postalcode but not house number should assign to number and remove from address', function(t) { + var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { + 'pelias-text-analyzer': { parse: function(query) { + t.equals(query, 'Number Value Street Value Number Value'); + + return { + postalcode: 'Number Value' + }; + } + }}); + + const raw = { + address: 'Number Value Street Value Number Value' + }; + + const clean = {}; + + const expected_clean = { + parsed_text: { + number: 'Number Value', + street: 'Street Value Number Value' + } + }; + + const messages = sanitizer(raw, clean); + + t.deepEquals(clean, expected_clean); + t.deepEquals(messages.errors, [], 'no errors'); + t.deepEquals(messages.warnings, [], 'no warnings'); + t.end(); + + }); + + test('text_analyzer not revealing possible number should move address to street', function(t) { + var sanitizer = proxyquire('../../../sanitizer/_synthesize_analysis', { + 'pelias-text-analyzer': { parse: function(query) { + t.equals(query, 'Street Value'); + + return {}; + } + }}); + + const raw = { + address: 'Street Value' + }; + + const clean = {}; + + const expected_clean = { + parsed_text: { + street: 'Street Value' + } + }; + + const messages = sanitizer(raw, clean); + + t.deepEquals(clean, expected_clean); + t.deepEquals(messages.errors, [], 'no errors'); + t.deepEquals(messages.warnings, [], 'no warnings'); + t.end(); + + }); + }; module.exports.all = function (tape, common) { From 563c0f94d017da863b3ddb2d1e1ff616935ee961 Mon Sep 17 00:00:00 2001 From: greenkeeperio-bot Date: Mon, 28 Nov 2016 17:57:21 -0500 Subject: [PATCH 05/20] chore(package): update through2 to version 2.0.3 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index d9a588d1..074161c2 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,7 @@ "pelias-query": "8.9.0", "pelias-text-analyzer": "1.6.0", "stats-lite": "2.0.3", - "through2": "2.0.1" + "through2": "2.0.3" }, "devDependencies": { "ciao": "^0.6.0", From 26b305e0d90bcad7d3f260a8e49509636e3504eb Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 29 Nov 2016 10:54:59 -0500 Subject: [PATCH 06/20] bumped query version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 80fd13e3..0e18e8c6 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ "pelias-labels": "1.5.1", "pelias-logger": "0.1.0", "pelias-model": "4.3.0", - "pelias-query": "pelias/query#5338af9", + "pelias-query": "8.10.0", "pelias-text-analyzer": "1.6.0", "stats-lite": "2.0.3", "through2": "2.0.1" From be2a33603816ab071f694a3f1e3dcdd7efb9fabf Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 29 Nov 2016 11:36:43 -0500 Subject: [PATCH 07/20] updated naming from component to structured --- ...nent.js => trimByGranularityStructured.js} | 0 ...t_geocoding.js => structured_geocoding.js} | 22 ++++++++--------- routes/v1.js | 16 ++++++------- ...t_geocoding.js => structured_geocoding.js} | 0 .../boundary_country.json | 0 .../fallback.json | 0 .../linguistic_bbox.json | 0 .../linguistic_focus.json | 0 .../linguistic_focus_bbox.json | 0 .../linguistic_focus_null_island.json | 0 .../linguistic_only.json | 0 .../linguistic_viewport.json | 0 .../linguistic_viewport_min_diagonal.json | 0 .../with_source_filtering.json | 0 ...nent.js => trimByGranularityStructured.js} | 2 +- ...t_geocoding.js => structured_geocoding.js} | 24 +++++++++---------- test/unit/run.js | 6 ++--- ...t_geocoding.js => structured_geocoding.js} | 4 ++-- 18 files changed, 37 insertions(+), 37 deletions(-) rename middleware/{trimByGranularityComponent.js => trimByGranularityStructured.js} (100%) rename query/{component_geocoding.js => structured_geocoding.js} (78%) rename sanitizer/{component_geocoding.js => structured_geocoding.js} (100%) rename test/unit/fixture/{component_geocoding => structured_geocoding}/boundary_country.json (100%) rename test/unit/fixture/{component_geocoding => structured_geocoding}/fallback.json (100%) rename test/unit/fixture/{component_geocoding => structured_geocoding}/linguistic_bbox.json (100%) rename test/unit/fixture/{component_geocoding => structured_geocoding}/linguistic_focus.json (100%) rename test/unit/fixture/{component_geocoding => structured_geocoding}/linguistic_focus_bbox.json (100%) rename test/unit/fixture/{component_geocoding => structured_geocoding}/linguistic_focus_null_island.json (100%) rename test/unit/fixture/{component_geocoding => structured_geocoding}/linguistic_only.json (100%) rename test/unit/fixture/{component_geocoding => structured_geocoding}/linguistic_viewport.json (100%) rename test/unit/fixture/{component_geocoding => structured_geocoding}/linguistic_viewport_min_diagonal.json (100%) rename test/unit/fixture/{component_geocoding => structured_geocoding}/with_source_filtering.json (100%) rename test/unit/middleware/{trimByGranularityComponent.js => trimByGranularityStructured.js} (99%) rename test/unit/query/{component_geocoding.js => structured_geocoding.js} (86%) rename test/unit/sanitizer/{component_geocoding.js => structured_geocoding.js} (96%) diff --git a/middleware/trimByGranularityComponent.js b/middleware/trimByGranularityStructured.js similarity index 100% rename from middleware/trimByGranularityComponent.js rename to middleware/trimByGranularityStructured.js diff --git a/query/component_geocoding.js b/query/structured_geocoding.js similarity index 78% rename from query/component_geocoding.js rename to query/structured_geocoding.js index c212c848..2d8a7ea2 100644 --- a/query/component_geocoding.js +++ b/query/structured_geocoding.js @@ -6,21 +6,21 @@ var peliasQuery = require('pelias-query'), //------------------------------ // general-purpose search query //------------------------------ -var componentQuery = new peliasQuery.layout.ComponentFallbackQuery(); +var structuredQuery = new peliasQuery.layout.ComponentFallbackQuery(); // scoring boost -componentQuery.score( peliasQuery.view.focus_only_function( peliasQuery.view.phrase ) ); -componentQuery.score( peliasQuery.view.popularity_only_function ); -componentQuery.score( peliasQuery.view.population_only_function ); +structuredQuery.score( peliasQuery.view.focus_only_function( peliasQuery.view.phrase ) ); +structuredQuery.score( peliasQuery.view.popularity_only_function ); +structuredQuery.score( peliasQuery.view.population_only_function ); // -------------------------------- // non-scoring hard filters -componentQuery.filter( peliasQuery.view.boundary_country ); -componentQuery.filter( peliasQuery.view.boundary_circle ); -componentQuery.filter( peliasQuery.view.boundary_rect ); -componentQuery.filter( peliasQuery.view.sources ); -componentQuery.filter( peliasQuery.view.layers ); -componentQuery.filter( peliasQuery.view.categories ); +structuredQuery.filter( peliasQuery.view.boundary_country ); +structuredQuery.filter( peliasQuery.view.boundary_circle ); +structuredQuery.filter( peliasQuery.view.boundary_rect ); +structuredQuery.filter( peliasQuery.view.sources ); +structuredQuery.filter( peliasQuery.view.layers ); +structuredQuery.filter( peliasQuery.view.categories ); // -------------------------------- /** @@ -105,7 +105,7 @@ function generateQuery( clean ){ function getQuery(vs) { return { type: 'fallback', - body: componentQuery.render(vs) + body: structuredQuery.render(vs) }; } diff --git a/routes/v1.js b/routes/v1.js index a1c71fb1..20fb1194 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -8,7 +8,7 @@ var sanitizers = { place: require('../sanitizer/place'), search: require('../sanitizer/search'), search_fallback: require('../sanitizer/search_fallback'), - component_geocoding: require('../sanitizer/component_geocoding'), + structured_geocoding: require('../sanitizer/structured_geocoding'), reverse: require('../sanitizer/reverse'), nearby: require('../sanitizer/nearby') }; @@ -30,14 +30,14 @@ var controllers = { var queries = { libpostal: require('../query/search'), fallback_to_old_prod: require('../query/search_original'), - component_geocoding: require('../query/component_geocoding') + structured_geocoding: require('../query/structured_geocoding') }; /** ----------------------- controllers ----------------------- **/ var postProc = { trimByGranularity: require('../middleware/trimByGranularity'), - trimByGranularityComponent: require('../middleware/trimByGranularityComponent'), + trimByGranularityStructured: require('../middleware/trimByGranularityStructured'), distances: require('../middleware/distance'), confidenceScores: require('../middleware/confidenceScore'), confidenceScoresFallback: require('../middleware/confidenceScoreFallback'), @@ -95,11 +95,11 @@ function addRoutes(app, peliasConfig) { postProc.geocodeJSON(peliasConfig, base), postProc.sendJSON ]), - component: createRouter([ - sanitizers.component_geocoding.middleware, + structured: createRouter([ + sanitizers.structured_geocoding.middleware, middleware.calcSize(), - controllers.search(peliasConfig, undefined, queries.component_geocoding), - postProc.trimByGranularityComponent(), + controllers.search(peliasConfig, undefined, queries.structured_geocoding), + postProc.trimByGranularityStructured(), postProc.distances('focus.point.'), postProc.confidenceScores(peliasConfig), postProc.confidenceScoresFallback(), @@ -193,7 +193,7 @@ function addRoutes(app, peliasConfig) { app.get ( base + 'autocomplete', routers.autocomplete ); app.get ( base + 'search', routers.search ); app.post( base + 'search', routers.search ); - app.get ( base + 'beta/component', routers.component ); + app.get ( base + 'search/structured', routers.structured ); app.get ( base + 'reverse', routers.reverse ); app.get ( base + 'nearby', routers.nearby ); diff --git a/sanitizer/component_geocoding.js b/sanitizer/structured_geocoding.js similarity index 100% rename from sanitizer/component_geocoding.js rename to sanitizer/structured_geocoding.js diff --git a/test/unit/fixture/component_geocoding/boundary_country.json b/test/unit/fixture/structured_geocoding/boundary_country.json similarity index 100% rename from test/unit/fixture/component_geocoding/boundary_country.json rename to test/unit/fixture/structured_geocoding/boundary_country.json diff --git a/test/unit/fixture/component_geocoding/fallback.json b/test/unit/fixture/structured_geocoding/fallback.json similarity index 100% rename from test/unit/fixture/component_geocoding/fallback.json rename to test/unit/fixture/structured_geocoding/fallback.json diff --git a/test/unit/fixture/component_geocoding/linguistic_bbox.json b/test/unit/fixture/structured_geocoding/linguistic_bbox.json similarity index 100% rename from test/unit/fixture/component_geocoding/linguistic_bbox.json rename to test/unit/fixture/structured_geocoding/linguistic_bbox.json diff --git a/test/unit/fixture/component_geocoding/linguistic_focus.json b/test/unit/fixture/structured_geocoding/linguistic_focus.json similarity index 100% rename from test/unit/fixture/component_geocoding/linguistic_focus.json rename to test/unit/fixture/structured_geocoding/linguistic_focus.json diff --git a/test/unit/fixture/component_geocoding/linguistic_focus_bbox.json b/test/unit/fixture/structured_geocoding/linguistic_focus_bbox.json similarity index 100% rename from test/unit/fixture/component_geocoding/linguistic_focus_bbox.json rename to test/unit/fixture/structured_geocoding/linguistic_focus_bbox.json diff --git a/test/unit/fixture/component_geocoding/linguistic_focus_null_island.json b/test/unit/fixture/structured_geocoding/linguistic_focus_null_island.json similarity index 100% rename from test/unit/fixture/component_geocoding/linguistic_focus_null_island.json rename to test/unit/fixture/structured_geocoding/linguistic_focus_null_island.json diff --git a/test/unit/fixture/component_geocoding/linguistic_only.json b/test/unit/fixture/structured_geocoding/linguistic_only.json similarity index 100% rename from test/unit/fixture/component_geocoding/linguistic_only.json rename to test/unit/fixture/structured_geocoding/linguistic_only.json diff --git a/test/unit/fixture/component_geocoding/linguistic_viewport.json b/test/unit/fixture/structured_geocoding/linguistic_viewport.json similarity index 100% rename from test/unit/fixture/component_geocoding/linguistic_viewport.json rename to test/unit/fixture/structured_geocoding/linguistic_viewport.json diff --git a/test/unit/fixture/component_geocoding/linguistic_viewport_min_diagonal.json b/test/unit/fixture/structured_geocoding/linguistic_viewport_min_diagonal.json similarity index 100% rename from test/unit/fixture/component_geocoding/linguistic_viewport_min_diagonal.json rename to test/unit/fixture/structured_geocoding/linguistic_viewport_min_diagonal.json diff --git a/test/unit/fixture/component_geocoding/with_source_filtering.json b/test/unit/fixture/structured_geocoding/with_source_filtering.json similarity index 100% rename from test/unit/fixture/component_geocoding/with_source_filtering.json rename to test/unit/fixture/structured_geocoding/with_source_filtering.json diff --git a/test/unit/middleware/trimByGranularityComponent.js b/test/unit/middleware/trimByGranularityStructured.js similarity index 99% rename from test/unit/middleware/trimByGranularityComponent.js rename to test/unit/middleware/trimByGranularityStructured.js index d90a3803..8e5fe498 100644 --- a/test/unit/middleware/trimByGranularityComponent.js +++ b/test/unit/middleware/trimByGranularityStructured.js @@ -1,4 +1,4 @@ -var trimByGranularity = require('../../../middleware/trimByGranularityComponent')(); +var trimByGranularity = require('../../../middleware/trimByGranularityStructured')(); module.exports.tests = {}; diff --git a/test/unit/query/component_geocoding.js b/test/unit/query/structured_geocoding.js similarity index 86% rename from test/unit/query/component_geocoding.js rename to test/unit/query/structured_geocoding.js index 42e4736d..fc5f211a 100644 --- a/test/unit/query/component_geocoding.js +++ b/test/unit/query/structured_geocoding.js @@ -1,4 +1,4 @@ -var generate = require('../../../query/component_geocoding'); +var generate = require('../../../query/structured_geocoding'); var fs = require('fs'); module.exports.tests = {}; @@ -28,7 +28,7 @@ module.exports.tests.query = function(test, common) { var query = generate(clean); var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = require('../fixture/component_geocoding/linguistic_focus_bbox'); + var expected = require('../fixture/structured_geocoding/linguistic_focus_bbox'); t.deepEqual(compiled.type, 'fallback', 'query type set'); t.deepEqual(compiled.body, expected, 'search_linguistic_focus_bbox'); @@ -51,7 +51,7 @@ module.exports.tests.query = function(test, common) { var query = generate(clean); var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = require('../fixture/component_geocoding/linguistic_bbox'); + var expected = require('../fixture/structured_geocoding/linguistic_bbox'); t.deepEqual(compiled.type, 'fallback', 'query type set'); t.deepEqual(compiled.body, expected, 'search_linguistic_bbox'); @@ -69,7 +69,7 @@ module.exports.tests.query = function(test, common) { var query = generate(clean); var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = require('../fixture/component_geocoding/linguistic_only'); + var expected = require('../fixture/structured_geocoding/linguistic_only'); t.deepEqual(compiled.type, 'fallback', 'query type set'); t.deepEqual(compiled.body, expected, 'search_linguistic_only'); @@ -88,7 +88,7 @@ module.exports.tests.query = function(test, common) { var query = generate(clean); var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = require('../fixture/component_geocoding/linguistic_focus'); + var expected = require('../fixture/structured_geocoding/linguistic_focus'); t.deepEqual(compiled.type, 'fallback', 'query type set'); t.deepEqual(compiled.body, expected, 'search_linguistic_focus'); @@ -110,7 +110,7 @@ module.exports.tests.query = function(test, common) { var query = generate(clean); var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = require('../fixture/component_geocoding/linguistic_viewport'); + var expected = require('../fixture/structured_geocoding/linguistic_viewport'); t.deepEqual(compiled.type, 'fallback', 'query type set'); t.deepEqual(compiled.body, expected, 'search_linguistic_viewport'); @@ -134,7 +134,7 @@ module.exports.tests.query = function(test, common) { var query = generate(clean); var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = require('../fixture/component_geocoding/linguistic_viewport_min_diagonal'); + var expected = require('../fixture/structured_geocoding/linguistic_viewport_min_diagonal'); t.deepEqual(compiled.type, 'fallback', 'query type set'); t.deepEqual(compiled.body, expected, 'valid search query'); @@ -153,7 +153,7 @@ module.exports.tests.query = function(test, common) { var query = generate(clean); var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = require('../fixture/component_geocoding/linguistic_focus_null_island'); + var expected = require('../fixture/structured_geocoding/linguistic_focus_null_island'); t.deepEqual(compiled.type, 'fallback', 'query type set'); t.deepEqual(compiled.body, expected, 'search_linguistic_focus_null_island'); @@ -180,7 +180,7 @@ module.exports.tests.query = function(test, common) { var query = generate(clean); var compiled = JSON.parse(JSON.stringify(query)); - var expected = require('../fixture/component_geocoding/fallback'); + var expected = require('../fixture/structured_geocoding/fallback'); t.deepEqual(compiled.type, 'fallback', 'query type set'); t.deepEqual(compiled.body, expected, 'fallbackQuery'); @@ -200,7 +200,7 @@ module.exports.tests.query = function(test, common) { var query = generate(clean); var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = require('../fixture/component_geocoding/boundary_country'); + var expected = require('../fixture/structured_geocoding/boundary_country'); t.deepEqual(compiled.type, 'fallback', 'query type set'); t.deepEqual(compiled.body, expected, 'search: valid boundary.country query'); @@ -218,7 +218,7 @@ module.exports.tests.query = function(test, common) { var query = generate(clean); var compiled = JSON.parse( JSON.stringify( query ) ); - var expected = require('../fixture/component_geocoding/with_source_filtering'); + var expected = require('../fixture/structured_geocoding/with_source_filtering'); t.deepEqual(compiled.type, 'fallback', 'query type set'); t.deepEqual(compiled.body, expected, 'search: valid search query with source filtering'); @@ -229,7 +229,7 @@ module.exports.tests.query = function(test, common) { module.exports.all = function (tape, common) { function test(name, testFunction) { - return tape('component_geocoding query ' + name, testFunction); + return tape('structured_geocoding query ' + name, testFunction); } for( var testCase in module.exports.tests ){ diff --git a/test/unit/run.js b/test/unit/run.js index 6ef7d417..a3109585 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -30,15 +30,15 @@ var tests = [ require('./middleware/sendJSON'), require('./middleware/normalizeParentIds'), require('./middleware/trimByGranularity'), - require('./middleware/trimByGranularityComponent'), + require('./middleware/trimByGranularityStructured'), require('./query/autocomplete'), require('./query/autocomplete_defaults'), - require('./query/component_geocoding'), require('./query/search_defaults'), require('./query/reverse_defaults'), require('./query/reverse'), require('./query/search'), require('./query/search_original'), + require('./query/structured_geocoding'), require('./query/text_parser'), require('./sanitizer/_boundary_country'), require('./sanitizer/_flag_bool'), @@ -61,7 +61,7 @@ var tests = [ require('./sanitizer/nearby'), require('./src/backend'), require('./sanitizer/autocomplete'), - require('./sanitizer/component_geocoding'), + require('./sanitizer/structured_geocoding'), require('./sanitizer/place'), require('./sanitizer/reverse'), require('./sanitizer/sanitizeAll'), diff --git a/test/unit/sanitizer/component_geocoding.js b/test/unit/sanitizer/structured_geocoding.js similarity index 96% rename from test/unit/sanitizer/component_geocoding.js rename to test/unit/sanitizer/structured_geocoding.js index 6477b738..3d6c606e 100644 --- a/test/unit/sanitizer/component_geocoding.js +++ b/test/unit/sanitizer/structured_geocoding.js @@ -8,7 +8,7 @@ module.exports.tests.sanitize = function(test, common) { // rather than re-verify the functionality of all the sanitizers, this test just verifies that they // were all called correctly - var search = proxyquire('../../../sanitizer/component_geocoding', { + var search = proxyquire('../../../sanitizer/structured_geocoding', { '../sanitizer/_deprecate_quattroshapes': function() { called_sanitizers.push('_deprecate_quattroshapes'); return { errors: [], warnings: [] }; @@ -109,7 +109,7 @@ module.exports.tests.sanitize = function(test, common) { module.exports.all = function (tape, common) { function test(name, testFunction) { - return tape('SANTIZE /component ' + name, testFunction); + return tape('SANTIZE /structured ' + name, testFunction); } for( var testCase in module.exports.tests ){ From dd448355dc0808673fa012127f8157c759f50cd2 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Tue, 29 Nov 2016 11:47:16 -0500 Subject: [PATCH 08/20] added comments, made things `const` --- sanitizer/_synthesize_analysis.js | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/sanitizer/_synthesize_analysis.js b/sanitizer/_synthesize_analysis.js index 79c53187..ab18b581 100644 --- a/sanitizer/_synthesize_analysis.js +++ b/sanitizer/_synthesize_analysis.js @@ -21,6 +21,10 @@ function isPostalCodeOnly(parsed_text) { parsed_text.hasOwnProperty('postalcode'); } +// figure out which field contains the probable house number, prefer number +// libpostal parses some inputs, like `3370 cobbe ave`, as a postcode+street +// so because we're treating the entire field as a street address, it's safe +// to assume that an identified postcode is actually a house number. function getHouseNumberField(analyzed_address) { for (var field of ['number', 'postalcode']) { if (analyzed_address.hasOwnProperty(field)) { @@ -54,21 +58,31 @@ function sanitize( raw, clean ){ } if (clean.parsed_text.hasOwnProperty('address')) { - var analyzed_address = text_analyzer.parse(clean.parsed_text.address); + const analyzed_address = text_analyzer.parse(clean.parsed_text.address); const house_number_field = getHouseNumberField(analyzed_address); + // if we're fairly certain that libpostal identified a house number + // (from either the house_number or postcode field), place it into the + // number field and remove the first instance of that value from address + // and assign to street + // eg - '1090 N Charlotte St' becomes number=1090 and street=N Charlotte St if (house_number_field) { clean.parsed_text.number = analyzed_address[house_number_field]; + // remove the first instance of the number and trim whitespace clean.parsed_text.street = _.trim(_.replace(clean.parsed_text.address, clean.parsed_text.number, '')); - delete clean.parsed_text.address; } else { + // otherwise no house number was identifiable, so treat the entire input + // as a street clean.parsed_text.street = clean.parsed_text.address; - delete clean.parsed_text.address; + } + // the address field no longer means anything since it's been parsed, so remove it + delete clean.parsed_text.address; + } return messages; From 31f5cc2537c2bf5391909f45ebe81b4c5f1577c2 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Fri, 2 Dec 2016 14:13:21 -0500 Subject: [PATCH 09/20] bumped query version, updated query name changes + boosts --- package.json | 2 +- query/structured_geocoding.js | 2 +- test/unit/fixture/structured_geocoding/fallback.json | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 0e18e8c6..ff2f8d71 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ "pelias-labels": "1.5.1", "pelias-logger": "0.1.0", "pelias-model": "4.3.0", - "pelias-query": "8.10.0", + "pelias-query": "pelias/query#c183594", "pelias-text-analyzer": "1.6.0", "stats-lite": "2.0.3", "through2": "2.0.1" diff --git a/query/structured_geocoding.js b/query/structured_geocoding.js index 2d8a7ea2..a69f42bb 100644 --- a/query/structured_geocoding.js +++ b/query/structured_geocoding.js @@ -6,7 +6,7 @@ var peliasQuery = require('pelias-query'), //------------------------------ // general-purpose search query //------------------------------ -var structuredQuery = new peliasQuery.layout.ComponentFallbackQuery(); +var structuredQuery = new peliasQuery.layout.StructuredFallbackQuery(); // scoring boost structuredQuery.score( peliasQuery.view.focus_only_function( peliasQuery.view.phrase ) ); diff --git a/test/unit/fixture/structured_geocoding/fallback.json b/test/unit/fixture/structured_geocoding/fallback.json index 965b188f..a957b72c 100644 --- a/test/unit/fixture/structured_geocoding/fallback.json +++ b/test/unit/fixture/structured_geocoding/fallback.json @@ -101,7 +101,7 @@ "layer": "address" } }, - "boost": 50 + "boost": 10 } }, { @@ -194,7 +194,7 @@ "layer": "street" } }, - "boost": 100 + "boost": 5 } }, { From 0dd31fb57bb4907cb48eb31cdffbeedca176a54e Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Fri, 2 Dec 2016 16:14:45 -0500 Subject: [PATCH 10/20] updated query version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ff2f8d71..65549bf1 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ "pelias-labels": "1.5.1", "pelias-logger": "0.1.0", "pelias-model": "4.3.0", - "pelias-query": "pelias/query#c183594", + "pelias-query": "8.11.0", "pelias-text-analyzer": "1.6.0", "stats-lite": "2.0.3", "through2": "2.0.1" From 52e7be556f845645ac2a57b8fdc9a7e8fc99b6ad Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Fri, 2 Dec 2016 16:17:43 -0500 Subject: [PATCH 11/20] fixed spacing issues --- routes/v1.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/routes/v1.js b/routes/v1.js index 20fb1194..89b86613 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -183,19 +183,19 @@ function addRoutes(app, peliasConfig) { // static data endpoints - app.get ( base, routers.index ); - app.get ( base + 'attribution', routers.attribution ); - app.get ( '/attribution', routers.attribution ); - app.get ( '/status', routers.status ); + app.get ( base, routers.index ); + app.get ( base + 'attribution', routers.attribution ); + app.get ( '/attribution', routers.attribution ); + app.get ( '/status', routers.status ); // backend dependent endpoints - app.get ( base + 'place', routers.place ); - app.get ( base + 'autocomplete', routers.autocomplete ); - app.get ( base + 'search', routers.search ); - app.post( base + 'search', routers.search ); + app.get ( base + 'place', routers.place ); + app.get ( base + 'autocomplete', routers.autocomplete ); + app.get ( base + 'search', routers.search ); + app.post( base + 'search', routers.search ); app.get ( base + 'search/structured', routers.structured ); - app.get ( base + 'reverse', routers.reverse ); - app.get ( base + 'nearby', routers.nearby ); + app.get ( base + 'reverse', routers.reverse ); + app.get ( base + 'nearby', routers.nearby ); } From 918574bb199153b99a31cca0cdf3ad43ba38a6a4 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Mon, 5 Dec 2016 11:18:20 -0500 Subject: [PATCH 12/20] Use caret range for through2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 074161c2..ee44201b 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,7 @@ "pelias-query": "8.9.0", "pelias-text-analyzer": "1.6.0", "stats-lite": "2.0.3", - "through2": "2.0.3" + "through2": "^2.0.3" }, "devDependencies": { "ciao": "^0.6.0", From 09f97a371d374d68810b3c0107fb3db67018fd9f Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Fri, 2 Dec 2016 14:32:29 +0100 Subject: [PATCH 13/20] add logging for error handling edge cases --- middleware/sendJSON.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/middleware/sendJSON.js b/middleware/sendJSON.js index be2e56b0..906145ed 100644 --- a/middleware/sendJSON.js +++ b/middleware/sendJSON.js @@ -1,5 +1,6 @@ var check = require('check-types'), es = require('elasticsearch'), + logger = require( 'pelias-logger' ).get( 'api' ), exceptions = require('elasticsearch-exceptions/lib/exceptions/SupportedExceptions'); // create a list of regular expressions to match against. @@ -42,7 +43,10 @@ function sendJSONResponse(req, res, next) { if( err instanceof es.errors.RequestTimeout ){ statusCode = Math.max( statusCode, 408 ); } else if( err instanceof es.errors.NoConnections ){ statusCode = Math.max( statusCode, 502 ); } else if( err instanceof es.errors.ConnectionFault ){ statusCode = Math.max( statusCode, 502 ); } - else { statusCode = Math.max( statusCode, 500 ); } + else { + logger.warn( 'unknown geocoding error object:', err.constructor.name, err ); + statusCode = Math.max( statusCode, 500 ); + } /* some elasticsearch errors are only returned as strings (not instances of Error). @@ -55,7 +59,10 @@ function sendJSONResponse(req, res, next) { statusCode = Math.max( statusCode, 500 ); break; // break on first match } + logger.warn( 'unknown geocoding error string:', err ); } + } else { + logger.warn( 'unknown geocoding error type:', typeof err, err ); } }); } From 1451e9038ea3a778f207aa81e87a2218e4c64e9e Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Fri, 2 Dec 2016 17:38:40 +0100 Subject: [PATCH 14/20] change all log labels to 'api' --- controller/place.js | 2 +- controller/search.js | 2 +- middleware/500.js | 2 +- middleware/confidenceScoreFallback.js | 2 +- sanitizer/search_fallback.js | 2 +- service/mget.js | 2 +- service/search.js | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/controller/place.js b/controller/place.js index 924292b6..ebf9df18 100644 --- a/controller/place.js +++ b/controller/place.js @@ -1,5 +1,5 @@ var service = { mget: require('../service/mget') }; -var logger = require('pelias-logger').get('api:controller:place'); +var logger = require('pelias-logger').get('api'); function setup( config, backend ){ diff --git a/controller/search.js b/controller/search.js index 39183fdf..3ac135d4 100644 --- a/controller/search.js +++ b/controller/search.js @@ -1,7 +1,7 @@ var _ = require('lodash'); var service = { search: require('../service/search') }; -var logger = require('pelias-logger').get('api:controller:search'); +var logger = require('pelias-logger').get('api'); var logging = require( '../helper/logging' ); function setup( config, backend, query ){ diff --git a/middleware/500.js b/middleware/500.js index 92acea60..17908729 100644 --- a/middleware/500.js +++ b/middleware/500.js @@ -1,4 +1,4 @@ -var logger = require( 'pelias-logger' ).get( 'middleware-500' ); +var logger = require( 'pelias-logger' ).get( 'api' ); // handle application errors function middleware(err, req, res, next) { diff --git a/middleware/confidenceScoreFallback.js b/middleware/confidenceScoreFallback.js index d208d02f..65606497 100644 --- a/middleware/confidenceScoreFallback.js +++ b/middleware/confidenceScoreFallback.js @@ -10,7 +10,7 @@ */ var check = require('check-types'); -var logger = require('pelias-logger').get('api-confidence'); +var logger = require('pelias-logger').get('api'); function setup() { return computeScores; diff --git a/sanitizer/search_fallback.js b/sanitizer/search_fallback.js index 07360fa6..d483d4e3 100644 --- a/sanitizer/search_fallback.js +++ b/sanitizer/search_fallback.js @@ -4,7 +4,7 @@ var sanitizeAll = require('../sanitizer/sanitizeAll'), }; var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; -var logger = require('pelias-logger').get('api:controller:search_fallback'); +var logger = require('pelias-logger').get('api'); var logging = require( '../helper/logging' ); // middleware diff --git a/service/mget.js b/service/mget.js index 529d0fc1..d021efd1 100644 --- a/service/mget.js +++ b/service/mget.js @@ -11,7 +11,7 @@ **/ -var peliasLogger = require( 'pelias-logger' ).get( 'service/mget' ); +var peliasLogger = require( 'pelias-logger' ).get( 'api' ); function service( backend, query, cb ){ diff --git a/service/search.js b/service/search.js index 780da2ae..2d9c9adf 100644 --- a/service/search.js +++ b/service/search.js @@ -5,7 +5,7 @@ **/ -var peliasLogger = require( 'pelias-logger' ).get( 'service/search' ); +var peliasLogger = require( 'pelias-logger' ).get( 'api' ); function service( backend, cmd, cb ){ From 00fae29fadb05b51fd31395d37e5df2ecb556d60 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Fri, 2 Dec 2016 17:43:54 +0100 Subject: [PATCH 15/20] update 500 middleware; add log statement, clean up code --- middleware/500.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/middleware/500.js b/middleware/500.js index 17908729..3fbc894e 100644 --- a/middleware/500.js +++ b/middleware/500.js @@ -1,13 +1,19 @@ -var logger = require( 'pelias-logger' ).get( 'api' ); +var check = require('check-types'), + logger = require( 'pelias-logger' ).get( 'api' ); // handle application errors function middleware(err, req, res, next) { + logger.error( 'Error: `%s`. Stack trace: `%s`.', err, err.stack ); - res.header('Cache-Control','public'); - var error = (err && err.message) ? err.message : err; - if( res.statusCode < 400 ){ res.status(500); } - res.json({ error: typeof error === 'string' ? error : 'internal server error' }); + if( res.statusCode < 400 ){ + logger.info( 'status code changed from', res.statusCode, 'to 500' ); + res.status(500); + } + + var error = ( err && err.message ) ? err.message : err; + res.header('Cache-Control','public'); + res.json({ error: check.nonEmptyString( error ) ? error : 'internal server error' }); } module.exports = middleware; From 72ec68f4c28bc644809f65e78158fe17ae38aecf Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Fri, 2 Dec 2016 17:48:07 +0100 Subject: [PATCH 16/20] additional backend error logging --- service/mget.js | 13 +++++++++++-- service/search.js | 15 ++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/service/mget.js b/service/mget.js index d021efd1..c709f28e 100644 --- a/service/mget.js +++ b/service/mget.js @@ -11,7 +11,7 @@ **/ -var peliasLogger = require( 'pelias-logger' ).get( 'api' ); +var logger = require( 'pelias-logger' ).get( 'api' ); function service( backend, query, cb ){ @@ -24,8 +24,17 @@ function service( backend, query, cb ){ // query new backend backend().client.mget( cmd, function( err, data ){ + + // log total ms elasticsearch reported the query took to execute + if( data && data.took ){ + logger.verbose( 'time elasticsearch reported:', data.took / 1000 ); + } + // handle backend errors - if( err ){ return cb( err ); } + if( err ){ + logger.error( 'backend error', err ); + return cb( err ); + } // map returned documents var docs = []; diff --git a/service/search.js b/service/search.js index 2d9c9adf..8e12b69e 100644 --- a/service/search.js +++ b/service/search.js @@ -5,18 +5,23 @@ **/ -var peliasLogger = require( 'pelias-logger' ).get( 'api' ); +var logger = require( 'pelias-logger' ).get( 'api' ); function service( backend, cmd, cb ){ // query new backend backend().client.search( cmd, function( err, data ){ - // handle backend errors - if( err ){ return cb( err ); } - // log total ms elasticsearch reported the query took to execute - peliasLogger.verbose( 'time elasticsearch reported:', data.took / 1000 ); + if( data && data.took ){ + logger.verbose( 'time elasticsearch reported:', data.took / 1000 ); + } + + // handle backend errors + if( err ){ + logger.error( 'backend error', err ); + return cb( err ); + } // map returned documents var docs = []; From 866ed42029ca742b7b5a094eb5b0535a0700fb62 Mon Sep 17 00:00:00 2001 From: greenkeeperio-bot Date: Mon, 5 Dec 2016 11:48:02 -0500 Subject: [PATCH 17/20] chore(package): update pelias-config to version 2.4.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index b1af4ba2..6c99fc04 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ "markdown": "0.5.0", "morgan": "1.7.0", "pelias-categories": "1.1.0", - "pelias-config": "2.3.1", + "pelias-config": "2.4.0", "pelias-labels": "1.5.1", "pelias-logger": "0.1.0", "pelias-model": "4.3.0", From f087a5badc4204079781feceec4fbfba6d19802a Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Fri, 2 Dec 2016 18:29:16 +0100 Subject: [PATCH 18/20] improvements for handling non-scalar request params. fixes #744 --- sanitizer/_single_scalar_parameters.js | 2 ++ sanitizer/reverse.js | 2 +- sanitizer/search.js | 2 +- sanitizer/structured_geocoding.js | 2 +- .../non_scalar_parameter_array.coffee | 30 +++++++++++++++++++ .../non_scalar_parameter_object.coffee | 30 +++++++++++++++++++ .../reverse/non_scalar_parameter_array.coffee | 30 +++++++++++++++++++ ...fee => non_scalar_parameter_object.coffee} | 2 +- test/ciao/search/address_parsing.coffee | 2 +- .../search/non_scalar_parameter_array.coffee | 30 +++++++++++++++++++ .../search/non_scalar_parameter_object.coffee | 30 +++++++++++++++++++ .../sanitizer/_single_scalar_parameters.js | 9 ++++++ test/unit/sanitizer/nearby.js | 2 +- test/unit/sanitizer/reverse.js | 2 +- test/unit/sanitizer/search.js | 2 +- test/unit/sanitizer/structured_geocoding.js | 2 +- 16 files changed, 170 insertions(+), 9 deletions(-) create mode 100644 test/ciao/autocomplete/non_scalar_parameter_array.coffee create mode 100644 test/ciao/autocomplete/non_scalar_parameter_object.coffee create mode 100644 test/ciao/reverse/non_scalar_parameter_array.coffee rename test/ciao/reverse/{non_scalar_parameter.coffee => non_scalar_parameter_object.coffee} (96%) create mode 100644 test/ciao/search/non_scalar_parameter_array.coffee create mode 100644 test/ciao/search/non_scalar_parameter_object.coffee diff --git a/sanitizer/_single_scalar_parameters.js b/sanitizer/_single_scalar_parameters.js index 6bf2fd55..2b06cc65 100644 --- a/sanitizer/_single_scalar_parameters.js +++ b/sanitizer/_single_scalar_parameters.js @@ -10,8 +10,10 @@ function sanitize( raw, clean ){ Object.keys(raw).forEach(function(key) { if (_.isArray(raw[key])) { messages.errors.push('\'' + key + '\' parameter can only have one value'); + delete raw[key]; } else if (_.isObject(raw[key])) { messages.errors.push('\'' + key + '\' parameter must be a scalar'); + delete raw[key]; } }); diff --git a/sanitizer/reverse.js b/sanitizer/reverse.js index 90795a19..285ab56c 100644 --- a/sanitizer/reverse.js +++ b/sanitizer/reverse.js @@ -2,8 +2,8 @@ var type_mapping = require('../helper/type_mapping'); var sanitizeAll = require('../sanitizer/sanitizeAll'), sanitizers = { - quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'), singleScalarParameters: require('../sanitizer/_single_scalar_parameters'), + quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'), layers: require('../sanitizer/_targets')('layers', type_mapping.layer_mapping), sources: require('../sanitizer/_targets')('sources', type_mapping.source_mapping), // depends on the layers and sources sanitizers, must be run after them diff --git a/sanitizer/search.js b/sanitizer/search.js index 450f6b62..d99a926e 100644 --- a/sanitizer/search.js +++ b/sanitizer/search.js @@ -2,8 +2,8 @@ var type_mapping = require('../helper/type_mapping'); var sanitizeAll = require('../sanitizer/sanitizeAll'), sanitizers = { - quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'), singleScalarParameters: require('../sanitizer/_single_scalar_parameters'), + quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'), text: require('../sanitizer/_text'), iso2_to_iso3: require('../sanitizer/_iso2_to_iso3'), size: require('../sanitizer/_size')(/* use defaults*/), diff --git a/sanitizer/structured_geocoding.js b/sanitizer/structured_geocoding.js index a5a95de9..ebd55a56 100644 --- a/sanitizer/structured_geocoding.js +++ b/sanitizer/structured_geocoding.js @@ -2,8 +2,8 @@ var type_mapping = require('../helper/type_mapping'); var sanitizeAll = require('../sanitizer/sanitizeAll'), sanitizers = { - quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'), singleScalarParameters: require('../sanitizer/_single_scalar_parameters'), + quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'), synthesize_analysis: require('../sanitizer/_synthesize_analysis'), iso2_to_iso3: require('../sanitizer/_iso2_to_iso3'), size: require('../sanitizer/_size')(/* use defaults*/), diff --git a/test/ciao/autocomplete/non_scalar_parameter_array.coffee b/test/ciao/autocomplete/non_scalar_parameter_array.coffee new file mode 100644 index 00000000..081cbb60 --- /dev/null +++ b/test/ciao/autocomplete/non_scalar_parameter_array.coffee @@ -0,0 +1,30 @@ + +#> define two sources +path: '/v1/autocomplete?text=A&sources=A&sources=B' + +#? 200 ok +response.statusCode.should.be.equal 400 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? expected errors +should.exist json.geocoding.errors +json.geocoding.errors.should.eql [ '\'sources\' parameter can only have one value' ] diff --git a/test/ciao/autocomplete/non_scalar_parameter_object.coffee b/test/ciao/autocomplete/non_scalar_parameter_object.coffee new file mode 100644 index 00000000..5c7e74af --- /dev/null +++ b/test/ciao/autocomplete/non_scalar_parameter_object.coffee @@ -0,0 +1,30 @@ + +#> define parameter as object +path: '/v1/autocomplete?text=A¶meter[idx]=value' + +#? 200 ok +response.statusCode.should.be.equal 400 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? expected errors +should.exist json.geocoding.errors +json.geocoding.errors.should.eql [ '\'parameter\' parameter must be a scalar' ] diff --git a/test/ciao/reverse/non_scalar_parameter_array.coffee b/test/ciao/reverse/non_scalar_parameter_array.coffee new file mode 100644 index 00000000..01792042 --- /dev/null +++ b/test/ciao/reverse/non_scalar_parameter_array.coffee @@ -0,0 +1,30 @@ + +#> define two sources +path: '/v1/reverse?point.lat=1&point.lon=1&sources=A&sources=B' + +#? 200 ok +response.statusCode.should.be.equal 400 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? expected errors +should.exist json.geocoding.errors +json.geocoding.errors.should.eql [ '\'sources\' parameter can only have one value' ] diff --git a/test/ciao/reverse/non_scalar_parameter.coffee b/test/ciao/reverse/non_scalar_parameter_object.coffee similarity index 96% rename from test/ciao/reverse/non_scalar_parameter.coffee rename to test/ciao/reverse/non_scalar_parameter_object.coffee index 757761e4..97f5fe6d 100644 --- a/test/ciao/reverse/non_scalar_parameter.coffee +++ b/test/ciao/reverse/non_scalar_parameter_object.coffee @@ -1,5 +1,5 @@ -#> set size +#> define parameter as object path: '/v1/reverse?point.lat=1&point.lon=1¶meter[idx]=value' #? 200 ok diff --git a/test/ciao/search/address_parsing.coffee b/test/ciao/search/address_parsing.coffee index 74b4d75e..2293ffa6 100644 --- a/test/ciao/search/address_parsing.coffee +++ b/test/ciao/search/address_parsing.coffee @@ -35,7 +35,7 @@ json.geocoding.query['size'].should.eql 10 #? address parsing json.geocoding.query.parsed_text['number'].should.eql '30' json.geocoding.query.parsed_text['street'].should.eql 'w 26th st' -json.geocoding.query.parsed_text['state'].should.eql 'ny' +json.geocoding.query.parsed_text['state'].should.eql 'NY' json.features[0].properties.confidence.should.eql 1 json.features[0].properties.match_type.should.eql "exact" diff --git a/test/ciao/search/non_scalar_parameter_array.coffee b/test/ciao/search/non_scalar_parameter_array.coffee new file mode 100644 index 00000000..9d5161fc --- /dev/null +++ b/test/ciao/search/non_scalar_parameter_array.coffee @@ -0,0 +1,30 @@ + +#> define two sources +path: '/v1/search?text=A&sources=A&sources=B' + +#? 200 ok +response.statusCode.should.be.equal 400 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? expected errors +should.exist json.geocoding.errors +json.geocoding.errors.should.eql [ '\'sources\' parameter can only have one value' ] diff --git a/test/ciao/search/non_scalar_parameter_object.coffee b/test/ciao/search/non_scalar_parameter_object.coffee new file mode 100644 index 00000000..80294db4 --- /dev/null +++ b/test/ciao/search/non_scalar_parameter_object.coffee @@ -0,0 +1,30 @@ + +#> define parameter as object +path: '/v1/search?text=A¶meter[idx]=value' + +#? 200 ok +response.statusCode.should.be.equal 400 +response.should.have.header 'charset', 'utf8' +response.should.have.header 'content-type', 'application/json; charset=utf-8' + +#? valid geocoding block +should.exist json.geocoding +should.exist json.geocoding.version +should.exist json.geocoding.attribution +should.exist json.geocoding.query +should.exist json.geocoding.engine +should.exist json.geocoding.engine.name +should.exist json.geocoding.engine.author +should.exist json.geocoding.engine.version +should.exist json.geocoding.timestamp + +#? valid geojson +json.type.should.be.equal 'FeatureCollection' +json.features.should.be.instanceof Array + +#? expected warnings +should.not.exist json.geocoding.warnings + +#? expected errors +should.exist json.geocoding.errors +json.geocoding.errors.should.eql [ '\'parameter\' parameter must be a scalar' ] diff --git a/test/unit/sanitizer/_single_scalar_parameters.js b/test/unit/sanitizer/_single_scalar_parameters.js index 4337d6d1..c3322703 100644 --- a/test/unit/sanitizer/_single_scalar_parameters.js +++ b/test/unit/sanitizer/_single_scalar_parameters.js @@ -18,6 +18,11 @@ module.exports.tests.single_scalar_parameters = function(test, common) { ], warnings: [] }); + + // erroneous params should be deleted to avoid middleware errors + t.false(raw.arrayParameter1); + t.false(raw.arrayParameter2); + t.end(); }); @@ -36,6 +41,10 @@ module.exports.tests.single_scalar_parameters = function(test, common) { ], warnings: [] }); + + // erroneous params should be deleted to avoid middleware errors + t.false(raw.objectParameter1); + t.false(raw.objectParameter2); t.end(); }); diff --git a/test/unit/sanitizer/nearby.js b/test/unit/sanitizer/nearby.js index 666944d0..4b5507e0 100644 --- a/test/unit/sanitizer/nearby.js +++ b/test/unit/sanitizer/nearby.js @@ -30,7 +30,7 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.sanitizers = function(test, common) { test('check sanitizer list', function (t) { - var expected = ['quattroshapes_deprecation', 'singleScalarParameters', 'layers', + var expected = ['singleScalarParameters', 'quattroshapes_deprecation', 'layers', 'sources', 'sources_and_layers', 'size', 'private', 'geo_reverse', 'boundary_country', 'categories']; t.deepEqual(Object.keys(nearby.sanitizer_list), expected); t.end(); diff --git a/test/unit/sanitizer/reverse.js b/test/unit/sanitizer/reverse.js index 96cec51a..b3bbdb95 100644 --- a/test/unit/sanitizer/reverse.js +++ b/test/unit/sanitizer/reverse.js @@ -36,7 +36,7 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.sanitizers = function(test, common) { test('check sanitizer list', function (t) { - var expected = ['quattroshapes_deprecation', 'singleScalarParameters', 'layers', + var expected = ['singleScalarParameters', 'quattroshapes_deprecation', 'layers', 'sources', 'sources_and_layers', 'size', 'private', 'geo_reverse', 'boundary_country']; t.deepEqual(Object.keys(reverse.sanitizer_list), expected); t.end(); diff --git a/test/unit/sanitizer/search.js b/test/unit/sanitizer/search.js index fa60e58a..e2c01f8d 100644 --- a/test/unit/sanitizer/search.js +++ b/test/unit/sanitizer/search.js @@ -82,8 +82,8 @@ module.exports.tests.sanitize = function(test, common) { }); var expected_sanitizers = [ - '_deprecate_quattroshapes', '_single_scalar_parameters', + '_deprecate_quattroshapes', '_text', '_iso2_to_iso3', '_size', diff --git a/test/unit/sanitizer/structured_geocoding.js b/test/unit/sanitizer/structured_geocoding.js index 3d6c606e..ef9711b3 100644 --- a/test/unit/sanitizer/structured_geocoding.js +++ b/test/unit/sanitizer/structured_geocoding.js @@ -82,8 +82,8 @@ module.exports.tests.sanitize = function(test, common) { }); var expected_sanitizers = [ - '_deprecate_quattroshapes', '_single_scalar_parameters', + '_deprecate_quattroshapes', '_synthesize_analysis', '_iso2_to_iso3', '_size', From f7287c448ea6f49384610195cddbc67e27f6947d Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Mon, 5 Dec 2016 14:31:52 -0500 Subject: [PATCH 19/20] fix: minimize debug logging --- query/text_parser_addressit.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/query/text_parser_addressit.js b/query/text_parser_addressit.js index 00e60724..9d97a683 100644 --- a/query/text_parser_addressit.js +++ b/query/text_parser_addressit.js @@ -31,9 +31,7 @@ function addParsedVariablesToQueryVariables( parsed_text, vs ){ // ? else { - logger.warn( 'chaos monkey asks: what happens now?' ); - logger.warn( parsed_text ); - try{ throw new Error(); } catch(e){ logger.warn( e.stack ); } // print a stack trace + logger.warn( 'chaos monkey asks: what happens now?', parsed_text ); } // ==== add parsed matches [address components] ==== From bccae79e7022ef6a23356ade51fae37919597e15 Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Mon, 5 Dec 2016 14:41:08 -0500 Subject: [PATCH 20/20] get rid of parsed_text logging because privacy --- query/text_parser_addressit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query/text_parser_addressit.js b/query/text_parser_addressit.js index 9d97a683..84979c15 100644 --- a/query/text_parser_addressit.js +++ b/query/text_parser_addressit.js @@ -31,7 +31,7 @@ function addParsedVariablesToQueryVariables( parsed_text, vs ){ // ? else { - logger.warn( 'chaos monkey asks: what happens now?', parsed_text ); + logger.warn( 'chaos monkey asks: what happens now?' ); } // ==== add parsed matches [address components] ====