From 30590a41d3ca4cb294eedbe55fc3ea5108a123af Mon Sep 17 00:00:00 2001 From: Diana Shkolnikov Date: Fri, 13 Jan 2017 18:27:56 -0500 Subject: [PATCH 01/10] fix: match_type and confidence score were not correct for certain queries --- middleware/confidenceScoreFallback.js | 129 ++++++++++++++++-- .../middleware/confidenceScoreFallback.js | 80 ++++++++++- 2 files changed, 193 insertions(+), 16 deletions(-) diff --git a/middleware/confidenceScoreFallback.js b/middleware/confidenceScoreFallback.js index 65606497..472dd841 100644 --- a/middleware/confidenceScoreFallback.js +++ b/middleware/confidenceScoreFallback.js @@ -1,3 +1,5 @@ +'use strict'; + /** * * Basic confidence score should be computed and returned for each item in the results. @@ -11,6 +13,7 @@ var check = require('check-types'); var logger = require('pelias-logger').get('api'); +const _ = require('lodash'); function setup() { return computeScores; @@ -67,9 +70,10 @@ function checkFallbackLevel(req, hit) { // if we know a fallback occurred, deduct points based on layer granularity switch (hit.layer) { case 'venue': - case 'address': logger.warn('Fallback scenarios should not result in address or venue records!', req.clean.parsed_text); return 0.8; + case 'address': + return 0.8; case 'street': return 0.8; case 'localadmin': @@ -96,26 +100,121 @@ function checkFallbackLevel(req, hit) { return 1.0; } +/** + * In parsed_text we might find any of the following properties: + * query + * number + * street + * neighbourhood + * borough + * city + * county + * state + * postalcode + * country + * + * They do not map 1:1 to our layers so the following somewhat complicated + * mapping structure is needed to set clear rules for comparing what was requested + * by the query and what has been received as a result to determine if a fallback occurred. + */ +const fallbackRules = [ + { + name: 'venue', + notSet: [], + set: ['query'], + expectedLayers: ['venue'] + }, + { + name: 'address', + notSet: ['query'], + set: ['number', 'street'], + expectedLayers: ['address'] + }, + { + name: 'street', + notSet: ['query', 'number'], + set: ['street'], + expectedLayers: ['street'] + }, + { + name: 'neighbourhood', + notSet: ['query', 'number', 'street'], + set: ['neighbourhood'], + expectedLayers: ['neighbourhood'] + }, + { + name: 'borough', + notSet: ['query', 'number', 'street', 'neighbourhood'], + set: ['borough'], + expectedLayers: ['borough'] + }, + { + name: 'city', + notSet: ['query', 'number', 'street', 'neighbourhood', 'borough'], + set: ['city'], + expectedLayers: ['borough', 'locality', 'localadmin'] + }, + { + name: 'county', + notSet: ['query', 'number', 'street', 'neighbourhood', 'borough', 'city'], + set: ['county'], + expectedLayers: ['county'] + }, + { + name: 'state', + notSet: ['query', 'number', 'street', 'neighbourhood', 'borough', 'city', 'county'], + set: ['state'], + expectedLayers: ['region'] + }, + { + name: 'country', + notSet: ['query', 'number', 'street', 'neighbourhood', 'borough', 'city', 'county', 'state'], + set: ['country'], + expectedLayers: ['country'] + } +]; + function checkFallbackOccurred(req, hit) { - return (requestedAddress(req) && hit.layer !== 'address') || - (requestedStreet(req) && hit.layer !== 'street') || - (requestedCity(req) && hit.layer !== 'locality' && hit.layer !== 'localadmin'); -} -function requestedAddress(req) { - // house number and street name were specified - return req.clean.parsed_text.hasOwnProperty('number') && - req.clean.parsed_text.hasOwnProperty('street'); + // short-circuit after finding the first fallback scenario + const res = _.find(fallbackRules, (rule) => { + + return ( + // verify that more granular properties are not set + notSet(req.clean.parsed_text, rule.notSet) && + // verify that expected property is set + areSet(req.clean.parsed_text, rule.set) && + // verify that expected layer(s) was not returned + rule.expectedLayers.indexOf(hit.layer) === -1 + ); + }); + + return !!res; } -function requestedStreet(req) { - // only street name was specified - return !req.clean.parsed_text.hasOwnProperty('number') && - req.clean.parsed_text.hasOwnProperty('street'); +function notSet(parsed_text, notSet) { + if (notSet.length === 0) { + return true; + } + + return ( + _.every(notSet, (prop) => { + return !_.get(parsed_text, prop, false); + }) + ); } -function requestedCity(req) { - return req.clean.parsed_text.hasOwnProperty('city'); +function areSet(parsed_text, areSet) { + if (areSet.length === 0) { + logger.warn('Expected properties in fallbackRules should never be empty'); + return true; + } + + return ( + _.every(areSet, (prop) => { + return _.get(parsed_text, prop, false); + }) + ); } module.exports = setup; diff --git a/test/unit/middleware/confidenceScoreFallback.js b/test/unit/middleware/confidenceScoreFallback.js index 138bee8c..3e783de7 100644 --- a/test/unit/middleware/confidenceScoreFallback.js +++ b/test/unit/middleware/confidenceScoreFallback.js @@ -95,6 +95,7 @@ module.exports.tests.confidenceScore = function(test, common) { confidenceScore(req, res, function() {}); t.equal(res.data[0].confidence, 0.1, 'score was set'); + t.equal(res.data[0].match_type, 'unknown', 'exact match indicated'); t.end(); }); @@ -105,6 +106,7 @@ module.exports.tests.confidenceScore = function(test, common) { parsed_text: { number: 123, street: 'Main St', + city: 'City', state: 'NM' } } @@ -131,6 +133,7 @@ module.exports.tests.confidenceScore = function(test, common) { confidenceScore(req, res, function() {}); t.equal(res.data[0].confidence, 1.0, 'max score was set'); + t.equal(res.data[0].match_type, 'exact', 'exact match indicated'); t.end(); }); @@ -166,10 +169,78 @@ module.exports.tests.confidenceScore = function(test, common) { confidenceScore(req, res, function() {}); t.equal(res.data[0].confidence, 1.0, 'max score was set'); + t.equal(res.data[0].match_type, 'exact', 'exact match indicated'); t.end(); }); - test('fallback to locality should have score deduction', function(t) { + test('no fallback state query should have max score', function(t) { + var req = { + clean: { + text: 'Region Name, Country', + parsed_text: { + state: 'Region Name', + country: 'Country' + } + } + }; + var res = { + data: [{ + _score: 10, + found: true, + value: 1, + layer: 'region', + center_point: { lat: 100.1, lon: -50.5 }, + name: { default: 'Region Name' }, + parent: { + country: ['country1'] + } + }], + meta: { + scores: [10], + query_type: 'fallback' + } + }; + + confidenceScore(req, res, function() {}); + t.equal(res.data[0].confidence, 1.0, 'max score was set'); + t.equal(res.data[0].match_type, 'exact', 'exact match indicated'); + t.end(); + }); + + test('no fallback country query should have max score', function(t) { + var req = { + clean: { + text: 'Country Name', + parsed_text: { + country: 'Country Name' + } + } + }; + var res = { + data: [{ + _score: 10, + found: true, + value: 1, + layer: 'country', + center_point: { lat: 100.1, lon: -50.5 }, + name: { default: 'test name1' }, + parent: { + country: ['country1'] + } + }], + meta: { + scores: [10], + query_type: 'fallback' + } + }; + + confidenceScore(req, res, function() {}); + t.equal(res.data[0].confidence, 1.0, 'max score was set'); + t.equal(res.data[0].match_type, 'exact', 'exact match indicated'); + t.end(); + }); + + test('fallback to locality when searching for address should have score deduction', function(t) { var req = { clean: { text: '123 Main St, City, NM', @@ -200,6 +271,7 @@ module.exports.tests.confidenceScore = function(test, common) { confidenceScore(req, res, function() {}); t.equal(res.data[0].confidence, 0.6, 'score was set'); + t.equal(res.data[0].match_type, 'fallback', 'fallback match indicated'); t.end(); }); @@ -234,6 +306,7 @@ module.exports.tests.confidenceScore = function(test, common) { confidenceScore(req, res, function() {}); t.equal(res.data[0].confidence, 0.6, 'score was set'); + t.equal(res.data[0].match_type, 'fallback', 'fallback match indicated'); t.end(); }); @@ -269,6 +342,7 @@ module.exports.tests.confidenceScore = function(test, common) { confidenceScore(req, res, function() {}); t.equal(res.data[0].confidence, 0.1, 'score was set'); + t.equal(res.data[0].match_type, 'fallback', 'fallback match indicated'); t.end(); }); @@ -292,6 +366,7 @@ module.exports.tests.confidenceScore = function(test, common) { confidenceScore(req, res, function() {}); t.equal(res.data[0].confidence, 1.0, 'score was set'); + t.equal(res.data[0].match_type, 'exact', 'exact match indicated'); t.end(); }); @@ -315,6 +390,7 @@ module.exports.tests.confidenceScore = function(test, common) { confidenceScore(req, res, function() {}); t.equal(res.data[0].confidence, 1.0, 'score was set'); + t.equal(res.data[0].match_type, 'exact', 'exact match indicated'); t.end(); }); @@ -338,6 +414,7 @@ module.exports.tests.confidenceScore = function(test, common) { confidenceScore(req, res, function() {}); t.equal(res.data[0].confidence, 0.3, 'score was set'); + t.equal(res.data[0].match_type, 'fallback', 'fallback match indicated'); t.end(); }); @@ -362,6 +439,7 @@ module.exports.tests.confidenceScore = function(test, common) { confidenceScore(req, res, function() {}); t.equal(res.data[0].confidence, 0.1, 'score was set'); + t.equal(res.data[0].match_type, 'fallback', 'fallback match indicated'); t.end(); }); From 576d1a1809e7388739bbb18a4f90b15d9a02ce47 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Thu, 15 Dec 2016 15:24:16 +0100 Subject: [PATCH 02/10] interpolation: integration v1 --- middleware/interpolate.js | 136 ++++++++++++++++++++++++++++++++++++++ package.json | 1 + routes/v1.js | 3 + service/interpolation.js | 58 ++++++++++++++++ 4 files changed, 198 insertions(+) create mode 100644 middleware/interpolate.js create mode 100644 service/interpolation.js diff --git a/middleware/interpolate.js b/middleware/interpolate.js new file mode 100644 index 00000000..d9974a1b --- /dev/null +++ b/middleware/interpolate.js @@ -0,0 +1,136 @@ + +var async = require('async'); +var service = require('../service/interpolation')(); +var logger = require( 'pelias-logger' ).get( 'api' ); + +/** +example response from interpolation web service: +{ + type: 'Feature', + properties: { + type: 'interpolated', + source: 'mixed', + number: '17', + lat: -41.2887032, + lon: 174.767089 + }, + geometry: { + type: 'Point', + coordinates: [ 174.767089, -41.2887032 ] + } +} +**/ + +function setup() { + return function middleware(req, res, next) { + + // no-op, user did not request an address + if( !isAddressQuery( req ) ){ + return next(); + } + + // bind parsed_text variables to function call + var bound = interpolate.bind( null, req.clean.parsed_text ); + + // perform interpolations asynchronously for all relevant hits + var timer = (new Date()).getTime(); + async.map( res.data, bound, function( err, results ){ + + // update res.data with the mapped values + if( !err ){ + res.data = results; + } + + // log the execution time, continue + logger.info( '[interpolation] [took]', (new Date()).getTime() - timer, 'ms' ); + next(); + }); + }; +} + +function interpolate( parsed_text, hit, cb ){ + + // no-op, this hit is not from the 'street' layer + // note: no network request is performed. + if( !hit || hit.layer !== 'street' ){ + return cb( null, hit ); + } + + // query variables + var coord = hit.center_point; + var number = parsed_text.number; + var street = hit.address_parts.street || parsed_text.street; + + // query interpolation service + service.query( coord, number, street, function( err, data ){ + + // an error occurred + // note: leave this hit unmodified + if( err ){ + logger.error( '[interpolation] [error]', err ); + return cb( null, hit ); + } + + // invalid / not useful response + // note: leave this hit unmodified + if( !data || !data.hasOwnProperty('properties') ){ + logger.info( '[interpolation] [miss]', parsed_text ); + return cb( null, hit ); + } + + // the interpolation service returned a valid result + // note: we now merge thos values with the existing 'street' record. + logger.info( '[interpolation] [hit]', parsed_text, data ); + + // safety first! + try { + + // -- metatdata -- + hit.layer = 'address'; + hit.match_type = 'interpolated'; + + // -- name -- + hit.name.default = data.properties.number + ' ' + hit.name.default; + + // -- source -- + var source = 'mixed'; + if( data.properties.source === 'osm' ){ source = 'openstreetmap'; } + else if( data.properties.source === 'oa' ){ source = 'openaddresses'; } + hit.source = source; + + // -- source_id -- + hit.source_id = 'derived:'+ hit.source_id; + + // -- address_parts -- + hit.address_parts.number = data.properties.number; + + // -- geo -- + hit.center_point = { + lat: data.properties.lat, + lon: data.properties.lon + }; + + // -- bbox -- + delete hit.bounding_box; + + // return the modified hit + return cb( null, hit ); + + // a syntax error occurred in the code above (this shouldn't happen!) + // note: the hit object may be partially modified, could possibly be invalid + } catch( e ){ + logger.error( '[interpolation] [error]', e, e.stack ); + return cb( null, hit ); + } + }); +} + +// boolean function to check if an address was requested by the user +function isAddressQuery( req ){ + return req && req.hasOwnProperty('clean') && + req.clean.hasOwnProperty('parsed_text') && + req.clean.parsed_text.hasOwnProperty('number') && + req.clean.parsed_text.hasOwnProperty('street'); +} + +module.exports = setup; diff --git a/package.json b/package.json index 398ce8c8..95b9aa53 100644 --- a/package.json +++ b/package.json @@ -60,6 +60,7 @@ "pelias-query": "8.12.0", "pelias-text-analyzer": "1.7.0", "stats-lite": "2.0.3", + "superagent": "^3.2.1", "through2": "^2.0.3" }, "devDependencies": { diff --git a/routes/v1.js b/routes/v1.js index 89b86613..a18dc797 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -44,6 +44,7 @@ var postProc = { confidenceScoresReverse: require('../middleware/confidenceScoreReverse'), accuracy: require('../middleware/accuracy'), dedupe: require('../middleware/dedupe'), + interpolate: require('../middleware/interpolate'), localNamingConventions: require('../middleware/localNamingConventions'), renamePlacenames: require('../middleware/renamePlacenames'), geocodeJSON: require('../middleware/geocodeJSON'), @@ -86,6 +87,7 @@ function addRoutes(app, peliasConfig) { postProc.confidenceScores(peliasConfig), postProc.confidenceScoresFallback(), postProc.dedupe(), + postProc.interpolate(), postProc.accuracy(), postProc.localNamingConventions(), postProc.renamePlacenames(), @@ -104,6 +106,7 @@ function addRoutes(app, peliasConfig) { postProc.confidenceScores(peliasConfig), postProc.confidenceScoresFallback(), postProc.dedupe(), + postProc.interpolate(), postProc.accuracy(), postProc.localNamingConventions(), postProc.renamePlacenames(), diff --git a/service/interpolation.js b/service/interpolation.js new file mode 100644 index 00000000..4e6fe405 --- /dev/null +++ b/service/interpolation.js @@ -0,0 +1,58 @@ + +var logger = require( 'pelias-logger' ).get( 'api' ); + +/** + + street address interpolation service + + see: https://github.com/pelias/interpolation + +**/ + +/** + RequireTransport + + allows the api to be used by simply requiring the module +**/ +function RequireTransport( addressDbPath, streetDbPath ){ + try { + var lib = require('pelias-interpolation'); + this.query = lib.api.search( addressDbPath, streetDbPath ); + } catch( e ){ + logger.error( 'RequireTransport: failed to connect to interpolation service' ); + } +} +RequireTransport.prototype.query = function( coord, number, street, cb ){ + throw new Error( 'transport not connected' ); +}; + +/** + HttpTransport + + allows the api to be used via a remote web service +**/ +function HttpTransport( host ){ + var request = require('superagent'); + this.query = function( coord, number, street, cb ){ + request + .get( host + '/search/geojson' ) + .set( 'Accept', 'application/json' ) + .query({ lat: coord.lat, lon: coord.lon, number: number, street: street }) + .end( function( err, res ){ + if( err || !res ){ return cb( err ); } + return cb( null, res.body ); + }); + }; +} +HttpTransport.prototype.query = function( coord, number, street, cb ){ + throw new Error( 'transport not connected' ); +}; + +/** + Setup + + allows instantiation of transport depending on configuration and preference +**/ +module.exports = function setup(){ + return new HttpTransport( 'http://interpolation.wiz.co.nz' ); +}; From 3881861886871a02c73071c679b144f3a5f0177f Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Thu, 12 Jan 2017 13:19:39 +0100 Subject: [PATCH 03/10] typo --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index a90bda50..f696320e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,7 +13,7 @@ ENV HOME=/opt/pelias WORKDIR ${WORK} ADD . ${WORK} -# Build and set permissions for arbitary non-root user +# Build and set permissions for arbitrary non-root user RUN npm install && \ npm test && \ chmod -R a+rwX . From 33341b7c998508c2c5a48399918eabb148060db5 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Thu, 12 Jan 2017 15:36:49 +0100 Subject: [PATCH 04/10] service: interpolation: tests --- package.json | 5 +- service/interpolation.js | 74 +++++++++++++++-- test/unit/run.js | 3 +- test/unit/service/interpolation.js | 129 +++++++++++++++++++++++++++++ 4 files changed, 199 insertions(+), 12 deletions(-) create mode 100644 test/unit/service/interpolation.js diff --git a/package.json b/package.json index 95b9aa53..141ad06e 100644 --- a/package.json +++ b/package.json @@ -71,11 +71,12 @@ "nsp": "^2.2.0", "precommit-hook": "^3.0.0", "proxyquire": "^1.7.10", + "semantic-release": "^6.3.2", "source-map": "^0.5.6", "tap-dot": "1.0.5", "tape": "^4.5.1", - "uglify-js": "^2.6.2", - "semantic-release": "^6.3.2" + "tmp": "0.0.31", + "uglify-js": "^2.6.2" }, "pre-commit": [ "lint", diff --git a/service/interpolation.js b/service/interpolation.js index 4e6fe405..4006e150 100644 --- a/service/interpolation.js +++ b/service/interpolation.js @@ -1,14 +1,38 @@ -var logger = require( 'pelias-logger' ).get( 'api' ); +var logger = require( 'pelias-logger' ).get( 'api' ), + request = require( 'superagent' ), + peliasConfig = require( 'pelias-config' ); /** - street address interpolation service + street address interpolation service client - see: https://github.com/pelias/interpolation + this file provides several different 'transports' which can be used to access the interpolation + service, either directly from disk or via a network connnection. + + the exported method for this module checks pelias-config for a configuration block such as: + + "interpolation": { + "client": { + "adapter": "http", + "host": "http://interpolation.wiz.co.nz" + } + } + + for more info on running the service see: https://github.com/pelias/interpolation **/ +/** + NullTransport + + disables the service completely +**/ +function NullTransport(){} +NullTransport.prototype.query = function( coord, number, street, cb ){ + cb(); // no-op +}; + /** RequireTransport @@ -16,14 +40,14 @@ var logger = require( 'pelias-logger' ).get( 'api' ); **/ function RequireTransport( addressDbPath, streetDbPath ){ try { - var lib = require('pelias-interpolation'); + var lib = require('pelias-interpolation'); // lazy load dependency this.query = lib.api.search( addressDbPath, streetDbPath ); } catch( e ){ logger.error( 'RequireTransport: failed to connect to interpolation service' ); } } RequireTransport.prototype.query = function( coord, number, street, cb ){ - throw new Error( 'transport not connected' ); + throw new Error( 'interpolation: transport not connected' ); }; /** @@ -32,7 +56,6 @@ RequireTransport.prototype.query = function( coord, number, street, cb ){ allows the api to be used via a remote web service **/ function HttpTransport( host ){ - var request = require('superagent'); this.query = function( coord, number, street, cb ){ request .get( host + '/search/geojson' ) @@ -40,12 +63,13 @@ function HttpTransport( host ){ .query({ lat: coord.lat, lon: coord.lon, number: number, street: street }) .end( function( err, res ){ if( err || !res ){ return cb( err ); } + if( 200 !== res.status ){ return cb( 'non 200 status' ); } return cb( null, res.body ); }); }; } HttpTransport.prototype.query = function( coord, number, street, cb ){ - throw new Error( 'transport not connected' ); + throw new Error( 'interpolation: transport not connected' ); }; /** @@ -53,6 +77,38 @@ HttpTransport.prototype.query = function( coord, number, street, cb ){ allows instantiation of transport depending on configuration and preference **/ -module.exports = function setup(){ - return new HttpTransport( 'http://interpolation.wiz.co.nz' ); +module.exports.search = function setup(){ + + // user config + var config = peliasConfig.generate(); + + // ensure config variables set correctly + if( !config.hasOwnProperty('interpolation') || !config.interpolation.hasOwnProperty('client') ){ + logger.warn( 'interpolation: configuration not found' ); + } + + // valid configuration found + else { + + // get adapter settings from config + var settings = config.interpolation.client; + + // http adapter + if( 'http' === settings.adapter && settings.hasOwnProperty('host') ){ + logger.info( 'interpolation: using http transport:', settings.host ); + return new HttpTransport( settings.host ); + } + + // require adapter + else if( 'require' === settings.adapter ){ + if( settings.hasOwnProperty('streetdb') && settings.hasOwnProperty('addressdb') ){ + logger.info( 'interpolation: using require transport' ); + return new RequireTransport( settings.addressdb, settings.streetdb ); + } + } + } + + // default adapter + logger.info( 'interpolation: using null transport' ); + return new NullTransport(); }; diff --git a/test/unit/run.js b/test/unit/run.js index 017492b5..7d5a3446 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -72,7 +72,8 @@ var tests = [ require('./sanitizer/search_fallback'), require('./sanitizer/wrap'), require('./service/mget'), - require('./service/search') + require('./service/search'), + require('./service/interpolation') ]; tests.map(function(t) { diff --git a/test/unit/service/interpolation.js b/test/unit/service/interpolation.js new file mode 100644 index 00000000..a11ed80f --- /dev/null +++ b/test/unit/service/interpolation.js @@ -0,0 +1,129 @@ + +var fs = require('fs'), + tmp = require('tmp'), + setup = require('../../../service/interpolation').search; + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('valid interface', function(t) { + t.equal(typeof setup, 'function', 'setup is a function'); + t.end(); + }); +}; + +// adapter factory +module.exports.tests.factory = function(test, common) { + + test('http adapter', function(t) { + var config = { interpolation: { client: { + adapter: 'http', + host: 'http://example.com' + }}}; + + // adapter is driven by config + var tmpfile = tmp.tmpNameSync({ postfix: '.json' }); + fs.writeFileSync( tmpfile, JSON.stringify( config ), { encoding: 'utf8' } ); + process.env.PELIAS_CONFIG = tmpfile; + var adapter = setup(); + delete process.env.PELIAS_CONFIG; + + t.equal(adapter.constructor.name, 'HttpTransport', 'HttpTransport'); + t.equal(typeof adapter, 'object', 'adapter is an object'); + t.equal(typeof adapter.query, 'function', 'query is a function'); + t.equal(adapter.query.length, 4, 'query function signature'); + t.end(); + }); + + test('require adapter', function(t) { + var config = { interpolation: { client: { + adapter: 'require', + addressdb: '/tmp/address.db', + streetdb: '/tmp/street.db' + }}}; + + // adapter is driven by config + var tmpfile = tmp.tmpNameSync({ postfix: '.json' }); + fs.writeFileSync( tmpfile, JSON.stringify( config ), { encoding: 'utf8' } ); + process.env.PELIAS_CONFIG = tmpfile; + var adapter = setup(); + delete process.env.PELIAS_CONFIG; + + t.equal(adapter.constructor.name, 'RequireTransport', 'RequireTransport'); + t.equal(typeof adapter, 'object', 'adapter is an object'); + t.equal(typeof adapter.query, 'function', 'query is a function'); + t.equal(adapter.query.length, 4, 'query function signature'); + t.end(); + }); + + test('null adapter', function(t) { + var config = { interpolation: { client: { + adapter: 'null' + }}}; + + // adapter is driven by config + var tmpfile = tmp.tmpNameSync({ postfix: '.json' }); + fs.writeFileSync( tmpfile, JSON.stringify( config ), { encoding: 'utf8' } ); + process.env.PELIAS_CONFIG = tmpfile; + var adapter = setup(); + delete process.env.PELIAS_CONFIG; + + t.equal(adapter.constructor.name, 'NullTransport', 'NullTransport'); + t.equal(typeof adapter, 'object', 'adapter is an object'); + t.equal(typeof adapter.query, 'function', 'query is a function'); + t.equal(adapter.query.length, 4, 'query function signature'); + t.end(); + }); + + test('default adapter', function(t) { + var config = {}; + + // adapter is driven by config + var tmpfile = tmp.tmpNameSync({ postfix: '.json' }); + fs.writeFileSync( tmpfile, JSON.stringify( config ), { encoding: 'utf8' } ); + process.env.PELIAS_CONFIG = tmpfile; + var adapter = setup(); + delete process.env.PELIAS_CONFIG; + + t.equal(adapter.constructor.name, 'NullTransport', 'NullTransport'); + t.equal(typeof adapter, 'object', 'adapter is an object'); + t.equal(typeof adapter.query, 'function', 'query is a function'); + t.equal(adapter.query.length, 4, 'query function signature'); + t.end(); + }); + +}; + +// null transport +module.exports.tests.NullTransport = function(test, common) { + + test('http adapter', function(t) { + var config = {}; + + // adapter is driven by config + var tmpfile = tmp.tmpNameSync({ postfix: '.json' }); + fs.writeFileSync( tmpfile, JSON.stringify( config ), { encoding: 'utf8' } ); + process.env.PELIAS_CONFIG = tmpfile; + var adapter = setup(); + delete process.env.PELIAS_CONFIG; + + // test null transport performs a no-op + adapter.query( null, null, null, function( err, res ){ + t.equal(err, undefined, 'no-op'); + t.equal(res, undefined, 'no-op'); + t.end(); + }); + }); + +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('SERVICE interpolation', testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; From 69be6e062ac0d0c84d1910e87b5bee627ec669b9 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Thu, 12 Jan 2017 17:00:43 +0100 Subject: [PATCH 05/10] service: interpolation: tests --- middleware/interpolate.js | 22 ++- test/unit/middleware/interpolate.js | 200 ++++++++++++++++++++++++++++ test/unit/run.js | 1 + 3 files changed, 216 insertions(+), 7 deletions(-) create mode 100644 test/unit/middleware/interpolate.js diff --git a/middleware/interpolate.js b/middleware/interpolate.js index d9974a1b..eaff913d 100644 --- a/middleware/interpolate.js +++ b/middleware/interpolate.js @@ -1,7 +1,7 @@ var async = require('async'); -var service = require('../service/interpolation')(); var logger = require( 'pelias-logger' ).get( 'api' ); +var service = require('../service/interpolation'); /** example response from interpolation web service: @@ -22,7 +22,9 @@ example response from interpolation web service: **/ function setup() { - return function middleware(req, res, next) { + + var transport = service.search(); + var middleware = function(req, res, next) { // no-op, user did not request an address if( !isAddressQuery( req ) ){ @@ -30,7 +32,7 @@ function setup() { } // bind parsed_text variables to function call - var bound = interpolate.bind( null, req.clean.parsed_text ); + var bound = interpolate.bind( transport, req.clean.parsed_text ); // perform interpolations asynchronously for all relevant hits var timer = (new Date()).getTime(); @@ -46,6 +48,9 @@ function setup() { next(); }); }; + + middleware.transport = transport; + return middleware; } function interpolate( parsed_text, hit, cb ){ @@ -62,7 +67,7 @@ function interpolate( parsed_text, hit, cb ){ var street = hit.address_parts.street || parsed_text.street; // query interpolation service - service.query( coord, number, street, function( err, data ){ + this.query( coord, number, street, function( err, data ){ // an error occurred // note: leave this hit unmodified @@ -94,12 +99,15 @@ function interpolate( parsed_text, hit, cb ){ // -- source -- var source = 'mixed'; - if( data.properties.source === 'osm' ){ source = 'openstreetmap'; } - else if( data.properties.source === 'oa' ){ source = 'openaddresses'; } + if( data.properties.source === 'OSM' ){ source = 'openstreetmap'; } + else if( data.properties.source === 'OA' ){ source = 'openaddresses'; } hit.source = source; // -- source_id -- - hit.source_id = 'derived:'+ hit.source_id; + // note: interpolated values have no source_id + if( hit.hasOwnProperty( 'source_id' ) ){ + hit.source_id = hit.source_id; + } // -- address_parts -- hit.address_parts.number = data.properties.number; diff --git a/test/unit/middleware/interpolate.js b/test/unit/middleware/interpolate.js new file mode 100644 index 00000000..6560bef4 --- /dev/null +++ b/test/unit/middleware/interpolate.js @@ -0,0 +1,200 @@ + +var fs = require('fs'), + tmp = require('tmp'), + setup = require('../../../middleware/interpolate'); + +// load middleware using the default pelias config +var load = function(){ + // adapter is driven by config + var tmpfile = tmp.tmpNameSync({ postfix: '.json' }); + fs.writeFileSync( tmpfile, '{}', { encoding: 'utf8' } ); + process.env.PELIAS_CONFIG = tmpfile; + var middleware = setup(); + delete process.env.PELIAS_CONFIG; + return middleware; +}; + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('valid interface', function(t) { + var middleware = load(); + t.equal(typeof middleware, 'function', 'middleware is a function'); + t.equal(middleware.length, 3, 'middleware is a function'); + t.end(); + }); +}; + +module.exports.tests.isAddressQuery = function(test, common) { + test('invalid address query - no parsed text', function(t) { + var req = { clean: {} }; + + var middleware = load(); + middleware(req, null, t.end); + }); + + test('invalid address query - no number', function(t) { + var req = { clean: { + parsed_text: { + street: 'sesame st' + }} + }; + + var middleware = load(); + middleware(req, null, t.end); + }); + + test('invalid address query - no street', function(t) { + var req = { clean: { + parsed_text: { + number: '1', + }} + }; + + var middleware = load(); + middleware(req, null, t.end); + }); +}; + +// test results are correctly mapped to the transport +module.exports.tests.map = function(test, common) { + test('documents mapped to transport: no hits', function(t) { + var req = { clean: { + parsed_text: { + number: '1', + street: 'sesame st' + }} + }; + var res = { data: [] }; + + var middleware = load(); + middleware(req, res, function(){ + t.deepEqual( res, { data: [] } ); + t.end(); + }); + }); + test('documents mapped to transport: no street layer hits', function(t) { + var req = { clean: { + parsed_text: { + number: '1', + street: 'sesame st' + }} + }; + var res = { data: [{ layer: 'foo' }] }; + + var middleware = load(); + middleware(req, res, function(){ + t.deepEqual( res, { data: [{ layer: 'foo' }] } ); + t.end(); + }); + }); +}; + +// check the service is called and response mapped correctly +module.exports.tests.miss = function(test, common) { + test('miss', function(t) { + + var req = { clean: { + parsed_text: { + number: '1', + street: 'sesame st' + }} + }; + var res = { data: [ + { + layer: 'street', + center_point: { lat: 1, lon: 1 }, + address_parts: { street: 'sesame rd' }, + name: { default: 'example' } + } + ]}; + + var middleware = load(); + + // mock out the transport + middleware.transport.query = function mock( coord, number, street, cb ){ + t.deepEqual( coord, res.data[0].center_point ); + t.deepEqual( number, req.clean.parsed_text.number ); + t.deepEqual( street, res.data[0].address_parts.street ); + t.equal( typeof cb, 'function' ); + cb( 'error' ); + }; + + middleware(req, res, function(){ + t.deepEqual( res, { data: [ + { + layer: 'street', + center_point: { lat: 1, lon: 1 }, + address_parts: { street: 'sesame rd' }, + name: { default: 'example' } + } + ]}); + t.end(); + }); + }); +}; + +// check the service is called and response mapped correctly +module.exports.tests.hit = function(test, common) { + test('hit', function(t) { + + var req = { clean: { + parsed_text: { + number: '1', + street: 'sesame st' + }} + }; + var res = { data: [ + { + layer: 'street', + center_point: { lat: 1, lon: 1 }, + address_parts: { street: 'sesame rd' }, + name: { default: 'street name' } + } + ]}; + + var middleware = load(); + + // mock out the transport + middleware.transport.query = function mock( coord, number, street, cb ){ + t.deepEqual( coord, res.data[0].center_point ); + t.deepEqual( number, req.clean.parsed_text.number ); + t.deepEqual( street, res.data[0].address_parts.street ); + t.equal( typeof cb, 'function' ); + cb( null, { + properties: { + number: '100A', + source: 'OSM', + source_id: 'way:111111', + lat: 22.2, + lon: -33.3, + } + }); + }; + + middleware(req, res, function(){ + t.deepEqual( res, { data: [ + { + layer: 'address', + match_type: 'interpolated', + center_point: { lat: 22.2, lon: -33.3 }, + address_parts: { street: 'sesame rd', number: '100A' }, + name: { default: '100A street name' }, + source: 'openstreetmap' + } + ]}); + t.end(); + }); + }); +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('[middleware] interpolate: ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/run.js b/test/unit/run.js index 7d5a3446..c92a45f0 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -25,6 +25,7 @@ var tests = [ require('./middleware/confidenceScoreFallback'), require('./middleware/confidenceScoreReverse'), require('./middleware/distance'), + require('./middleware/interpolate'), require('./middleware/localNamingConventions'), require('./middleware/dedupe'), require('./middleware/parseBBox'), From 5c278b22e96ea18a8c2fb59e39634a8326e8b2c9 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Thu, 12 Jan 2017 17:09:46 +0100 Subject: [PATCH 06/10] service: interpolation: handle backend timeouts --- service/interpolation.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/service/interpolation.js b/service/interpolation.js index 4006e150..d1d01550 100644 --- a/service/interpolation.js +++ b/service/interpolation.js @@ -55,12 +55,13 @@ RequireTransport.prototype.query = function( coord, number, street, cb ){ allows the api to be used via a remote web service **/ -function HttpTransport( host ){ +function HttpTransport( host, settings ){ this.query = function( coord, number, street, cb ){ request .get( host + '/search/geojson' ) .set( 'Accept', 'application/json' ) .query({ lat: coord.lat, lon: coord.lon, number: number, street: street }) + .timeout( settings && settings.timeout || 1000 ) .end( function( err, res ){ if( err || !res ){ return cb( err ); } if( 200 !== res.status ){ return cb( 'non 200 status' ); } @@ -96,6 +97,9 @@ module.exports.search = function setup(){ // http adapter if( 'http' === settings.adapter && settings.hasOwnProperty('host') ){ logger.info( 'interpolation: using http transport:', settings.host ); + if( settings.hasOwnProperty('timeout') ){ + return new HttpTransport( settings.host, { timeout: parseInt( settings.timeout, 10 ) } ); + } return new HttpTransport( settings.host ); } From f948ab26fc018d34c63ec0422922a12a7886e94e Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Thu, 12 Jan 2017 17:21:28 +0100 Subject: [PATCH 07/10] service: interpolation: remove non-production host name --- service/interpolation.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/interpolation.js b/service/interpolation.js index d1d01550..e3b36ded 100644 --- a/service/interpolation.js +++ b/service/interpolation.js @@ -15,7 +15,7 @@ var logger = require( 'pelias-logger' ).get( 'api' ), "interpolation": { "client": { "adapter": "http", - "host": "http://interpolation.wiz.co.nz" + "host": "http://localhost:4444" } } From d135596c80652d531eed64a6a7e96ae7d3f346c6 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Thu, 12 Jan 2017 17:23:34 +0100 Subject: [PATCH 08/10] service: interpolation: typo --- test/unit/service/interpolation.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/unit/service/interpolation.js b/test/unit/service/interpolation.js index a11ed80f..3393b4ce 100644 --- a/test/unit/service/interpolation.js +++ b/test/unit/service/interpolation.js @@ -97,12 +97,11 @@ module.exports.tests.factory = function(test, common) { // null transport module.exports.tests.NullTransport = function(test, common) { - test('http adapter', function(t) { - var config = {}; + test('null transport', function(t) { // adapter is driven by config var tmpfile = tmp.tmpNameSync({ postfix: '.json' }); - fs.writeFileSync( tmpfile, JSON.stringify( config ), { encoding: 'utf8' } ); + fs.writeFileSync( tmpfile, '{}', { encoding: 'utf8' } ); process.env.PELIAS_CONFIG = tmpfile; var adapter = setup(); delete process.env.PELIAS_CONFIG; From 1216ca783d3a2077993a3739c50a3b5ea0274cac Mon Sep 17 00:00:00 2001 From: missinglink Date: Fri, 13 Jan 2017 15:18:53 +0100 Subject: [PATCH 09/10] deps: version bump pelias-config --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 141ad06e..2340c415 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "markdown": "0.5.0", "morgan": "1.7.0", "pelias-categories": "1.1.0", - "pelias-config": "2.4.0", + "pelias-config": "2.6.0", "pelias-labels": "1.5.1", "pelias-logger": "0.1.0", "pelias-model": "4.4.0", From 3a4d26077a0d1918a4b353bbcd1c31b935371247 Mon Sep 17 00:00:00 2001 From: missinglink Date: Tue, 17 Jan 2017 16:20:06 +0100 Subject: [PATCH 10/10] interpolation: fix: source_id mapping --- middleware/interpolate.js | 5 +++-- test/unit/middleware/interpolate.js | 6 ++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/middleware/interpolate.js b/middleware/interpolate.js index eaff913d..a6d69649 100644 --- a/middleware/interpolate.js +++ b/middleware/interpolate.js @@ -105,8 +105,9 @@ function interpolate( parsed_text, hit, cb ){ // -- source_id -- // note: interpolated values have no source_id - if( hit.hasOwnProperty( 'source_id' ) ){ - hit.source_id = hit.source_id; + delete hit.source_id; // remove original street source_id + if( data.properties.hasOwnProperty( 'source_id' ) ){ + hit.source_id = data.properties.source_id; } // -- address_parts -- diff --git a/test/unit/middleware/interpolate.js b/test/unit/middleware/interpolate.js index 6560bef4..26bf1e52 100644 --- a/test/unit/middleware/interpolate.js +++ b/test/unit/middleware/interpolate.js @@ -149,7 +149,8 @@ module.exports.tests.hit = function(test, common) { layer: 'street', center_point: { lat: 1, lon: 1 }, address_parts: { street: 'sesame rd' }, - name: { default: 'street name' } + name: { default: 'street name' }, + source_id: '123456' } ]}; @@ -180,7 +181,8 @@ module.exports.tests.hit = function(test, common) { center_point: { lat: 22.2, lon: -33.3 }, address_parts: { street: 'sesame rd', number: '100A' }, name: { default: '100A street name' }, - source: 'openstreetmap' + source: 'openstreetmap', + source_id: 'way:111111' } ]}); t.end();