From 6ddbe86ac5634553340bc5708249e6f3f1a3fc41 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 3 Nov 2014 17:30:21 +0000 Subject: [PATCH 1/5] perform mget during suggest. requires suggester output to be a gid --- controller/suggest.js | 48 ++++++++++++++++++++++++++------ test/ciao/suggest/success.coffee | 1 + test/unit/controller/suggest.js | 31 ++++++++++++++------- test/unit/mock/backend.js | 31 +++++++++++++++++---- 4 files changed, 88 insertions(+), 23 deletions(-) diff --git a/controller/suggest.js b/controller/suggest.js index 956363b8..b0a30f70 100644 --- a/controller/suggest.js +++ b/controller/suggest.js @@ -1,6 +1,9 @@ -var service = { suggest: require('../service/suggest') }; -var geojsonify = require('../helper/geojsonify').suggest; +var service = { + suggest: require('../service/suggest'), + mget: require('../service/mget') +}; +var geojsonify = require('../helper/geojsonify').search; function setup( backend, query ){ @@ -16,12 +19,9 @@ function setup( backend, query ){ body: query( req.clean ) }; - // query backend - service.suggest( backend, cmd, function( err, docs ){ - - // error handler - if( err ){ return next( err ); } - + // responder + function reply( docs ){ + // convert docs to geojson var geojson = geojsonify( docs ); @@ -30,6 +30,38 @@ function setup( backend, query ){ // respond return res.status(200).json( geojson ); + } + + // query backend + service.suggest( backend, cmd, function( err, suggested ){ + + // error handler + if( err ){ return next( err ); } + + // no documents suggested, return empty array to avoid ActionRequestValidationException + if( !Array.isArray( suggested ) || !suggested.length ){ + return reply([]); + } + + // map suggester output to mget query + var query = suggested.map( function( doc ) { + var idParts = doc.text.split(':'); + return { + _index: 'pelias', + _type: idParts[0], + _id: idParts[1] + }; + }); + + service.mget( backend, query, function( err, docs ){ + + // error handler + if( err ){ return next( err ); } + + // reply + return reply( docs ); + + }); }); } diff --git a/test/ciao/suggest/success.coffee b/test/ciao/suggest/success.coffee index e54e6424..08e504d2 100644 --- a/test/ciao/suggest/success.coffee +++ b/test/ciao/suggest/success.coffee @@ -12,5 +12,6 @@ should.not.exist json.error json.date.should.be.within now-5000, now+5000 #? valid geojson +console.log( json ); json.type.should.equal 'FeatureCollection' json.features.should.be.instanceof Array \ No newline at end of file diff --git a/test/unit/controller/suggest.js b/test/unit/controller/suggest.js index f67f5da7..1553d348 100644 --- a/test/unit/controller/suggest.js +++ b/test/unit/controller/suggest.js @@ -16,34 +16,42 @@ module.exports.tests.interface = function(test, common) { // functionally test controller (backend success) module.exports.tests.functional_success = function(test, common) { - // expected geojson features for 'client/suggest/ok/1' fixture + // expected geojson features for 'client/mget/ok/1' fixture var expected = [{ type: 'Feature', geometry: { type: 'Point', - coordinates: [ 101, -10.1 ] + coordinates: [ -50.5, 100.1 ] }, properties: { - id: 'mockid', - type: 'mocktype', - value: 1 + name: 'test name1', + admin0: 'country1', + admin1: 'state1', + admin2: 'city1' } }, { type: 'Feature', geometry: { type: 'Point', - coordinates: [ 101, -10.1 ] + coordinates: [ -51.5, 100.2 ] }, properties: { - id: 'mockid', - type: 'mocktype', - value: 2 + name: 'test name2', + admin0: 'country2', + admin1: 'state2', + admin2: 'city2' } }]; test('functional success', function(t) { + var i = 0; var backend = mockBackend( 'client/suggest/ok/1', function( cmd ){ - t.deepEqual(cmd, { body: { a: 'b' }, index: 'pelias' }, 'correct backend command'); + // the backend executes 2 commands, so we check them both + if( ++i === 1 ){ + t.deepEqual(cmd, { body: { a: 'b' }, index: 'pelias' }, 'correct suggest command'); + } else { + t.deepEqual(cmd, { body: { docs: [ { _id: 'mockid', _index: 'pelias', _type: 'mocktype' }, { _id: 'mockid', _index: 'pelias', _type: 'mocktype' } ] } }, 'correct mget command'); + } }); var controller = setup( backend, mockQuery() ); var res = { @@ -52,6 +60,9 @@ module.exports.tests.functional_success = function(test, common) { return res; }, json: function( json ){ + + console.log( 'json', json ); + t.equal(typeof json, 'object', 'returns json'); t.equal(typeof json.date, 'number', 'date set'); t.equal(json.type, 'FeatureCollection', 'valid geojson'); diff --git a/test/unit/mock/backend.js b/test/unit/mock/backend.js index 63378423..f2759168 100644 --- a/test/unit/mock/backend.js +++ b/test/unit/mock/backend.js @@ -1,12 +1,8 @@ -var mockPayload = { - id: 'mocktype/mockid', - geo: '101,-10.1' -}; var responses = {}; responses['client/suggest/ok/1'] = function( cmd, cb ){ - return cb( undefined, suggestEnvelope([ { value: 1, payload: mockPayload }, { value: 2, payload: mockPayload } ]) ); + return cb( undefined, suggestEnvelope([ { score: 1, text: 'mocktype:mockid' }, { score: 2, text: 'mocktype:mockid' } ]) ); }; responses['client/suggest/fail/1'] = function( cmd, cb ){ return cb( 'a backend error occurred' ); @@ -28,6 +24,23 @@ responses['client/search/ok/1'] = function( cmd, cb ){ } }])); }; +responses['client/mget/ok/1'] = function( cmd, cb ){ + return cb( undefined, mgetEnvelope([{ + _source: { + value: 1, + center_point: { lat: 100.1, lon: -50.5 }, + name: { default: 'test name1' }, + admin0: 'country1', admin1: 'state1', admin2: 'city1' + } + }, { + _source: { + value: 2, + center_point: { lat: 100.2, lon: -51.5 }, + name: { default: 'test name2' }, + admin0: 'country2', admin1: 'state2', admin2: 'city2' + } + }])); +}; responses['client/search/fail/1'] = function( cmd, cb ){ return cb( 'a backend error occurred' ); }; @@ -43,6 +56,10 @@ function setup( key, cmdCb ){ search: function( cmd, cb ){ if( 'function' === typeof cmdCb ){ cmdCb( cmd ); } return responses[key].apply( this, arguments ); + }, + mget: function( cmd, cb ){ + if( 'function' === typeof cmdCb ){ cmdCb( cmd ); } + return responses['client/mget/ok/1'].apply( this, arguments ); } } }; @@ -58,4 +75,8 @@ function searchEnvelope( options ){ return { hits: { total: options.length, hits: options } }; } +function mgetEnvelope( options ){ + return { docs: options }; +} + module.exports = setup; \ No newline at end of file From 88aa48ec016ed36b37a8f9cc40613ba5ef98c6b3 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 3 Nov 2014 17:33:57 +0000 Subject: [PATCH 2/5] formatting --- test/ciao/suggest/success.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/test/ciao/suggest/success.coffee b/test/ciao/suggest/success.coffee index 08e504d2..e54e6424 100644 --- a/test/ciao/suggest/success.coffee +++ b/test/ciao/suggest/success.coffee @@ -12,6 +12,5 @@ should.not.exist json.error json.date.should.be.within now-5000, now+5000 #? valid geojson -console.log( json ); json.type.should.equal 'FeatureCollection' json.features.should.be.instanceof Array \ No newline at end of file From fcd60463cca0cf61137d0e2e130f6a8cf72b923e Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 3 Nov 2014 17:41:37 +0000 Subject: [PATCH 3/5] remove geojsonify.suggest --- helper/geojsonify.js | 42 ---------------------- test/unit/helper/geojsonify.js | 66 ++-------------------------------- 2 files changed, 3 insertions(+), 105 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index d4263900..196bd18f 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -1,47 +1,6 @@ var GeoJSON = require('geojson'); -function suggest( docs ){ - - // emit a warning if the doc format is invalid - // @note: if you see this error, fix it ASAP! - function warning(){ - console.error( 'error: invalid doc', __filename ); - return false; // remove offending doc from results - } - - // flatten & expand data for geojson conversion - var geodata = docs.map( function( doc ){ - - // something went very wrong - if( !doc || !doc.payload ) return warning(); - - // split payload id string in to geojson properties - if( 'string' !== typeof doc.payload.id ) return warning(); - var idParts = doc.payload.id.split('/'); - doc.type = idParts[0]; - doc.id = idParts[1]; - - // split payload geo string in to geojson properties - if( 'string' !== typeof doc.payload.geo ) return warning(); - var geoParts = doc.payload.geo.split(','); - doc.lat = parseFloat( geoParts[1] ); - doc.lng = parseFloat( geoParts[0] ); - - // remove payload from doc - delete doc.payload; - return doc; - - // filter-out invalid entries - }).filter( function( doc ){ - return doc; - }); - - // convert to geojson - return GeoJSON.parse( geodata, { Point: ['lat', 'lng'] } ); - -} - function search( docs ){ // emit a warning if the doc format is invalid @@ -95,5 +54,4 @@ function search( docs ){ } -module.exports.suggest = suggest; module.exports.search = search; \ No newline at end of file diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index f6bdff2b..d855e390 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -4,73 +4,13 @@ var geojsonify = require('../../../helper/geojsonify'); module.exports.tests = {}; module.exports.tests.interface = function(test, common) { - test('valid interface .suggest()', function(t) { - t.equal(typeof geojsonify.suggest, 'function', 'suggest is a function'); - t.equal(geojsonify.suggest.length, 1, 'accepts x arguments'); + test('valid interface .search()', function(t) { + t.equal(typeof geojsonify.search, 'function', 'search is a function'); + t.equal(geojsonify.search.length, 1, 'accepts x arguments'); t.end(); }); }; -module.exports.tests.suggest = function(test, common) { - - var input = [{ - bingo1: 'bango1', - payload: { - id: 'foo1/bar1', - geo: '100,-10.5' - } - },{ - bingo2: 'bango2', - payload: { - id: 'foo2/bar2', - geo: '10,-1.5' - } - }]; - - var expected = { - "type": "FeatureCollection", - "features": [ - { - "type": "Feature", - "geometry": { - "type": "Point", - "coordinates": [ - 100, - -10.5 - ] - }, - "properties": { - "bingo1": "bango1", - "type": "foo1", - "id": "bar1" - } - }, - { - "type": "Feature", - "geometry": { - "type": "Point", - "coordinates": [ - 10, - -1.5 - ] - }, - "properties": { - "bingo2": "bango2", - "type": "foo2", - "id": "bar2" - } - } - ] - }; - - test('geojsonify.suggest()', function(t) { - var json = geojsonify.suggest( input ); - t.deepEqual(json, expected, 'all docs mapped'); - t.end(); - }); -}; - - module.exports.tests.search = function(test, common) { var input = [ From 27e2c4e5b8bf0eebf2aebbcad9d4eb5405464a16 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 3 Nov 2014 17:54:22 +0000 Subject: [PATCH 4/5] migrate outputGenerator from pelias/suggester-pipeline --- helper/geojsonify.js | 9 +- helper/outputGenerator.js | 37 +++++ helper/outputSchema.json | 14 ++ test/unit/controller/search.js | 6 +- test/unit/controller/suggest.js | 6 +- test/unit/helper/geojsonify.js | 2 +- test/unit/helper/outputSchema.js | 55 +++++++ test/unit/mock/alpha3.json | 251 +++++++++++++++++++++++++++++++ test/unit/run.js | 3 +- 9 files changed, 372 insertions(+), 11 deletions(-) create mode 100644 helper/outputGenerator.js create mode 100644 helper/outputSchema.json create mode 100644 test/unit/helper/outputSchema.js create mode 100644 test/unit/mock/alpha3.json diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 196bd18f..7bdae990 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -1,5 +1,6 @@ -var GeoJSON = require('geojson'); +var GeoJSON = require('geojson'), + outputGenerator = require('./outputGenerator'); function search( docs ){ @@ -37,10 +38,8 @@ function search( docs ){ if( doc.locality ){ output.locality = doc.locality; } if( doc.neighborhood ){ output.neighborhood = doc.neighborhood; } - // map suggest output - if( doc.suggest && doc.suggest.output ){ - output.text = doc.suggest.output; - } + // generate region-specific text string + output.text = outputGenerator( doc ); return output; diff --git a/helper/outputGenerator.js b/helper/outputGenerator.js new file mode 100644 index 00000000..42fa1279 --- /dev/null +++ b/helper/outputGenerator.js @@ -0,0 +1,37 @@ + +var schemas = require('./outputSchema.json'); + +module.exports = function( record ){ + + var adminParts = []; + + var schema = schemas.default; + + if (record.alpha3 && record.alpha3.length && schemas[record.alpha3]) { + schema = schemas[record.alpha3]; + } + + var buildOutput = function(parts, schemaArr, record) { + for (var i=0; i Date: Mon, 3 Nov 2014 18:21:27 +0000 Subject: [PATCH 5/5] expose id/type/layer --- helper/geojsonify.js | 9 +++++++-- service/mget.js | 14 +++++++++++++- service/search.js | 6 ++++++ test/unit/controller/search.js | 6 ++++++ test/unit/controller/suggest.js | 6 ++++++ test/unit/helper/geojsonify.js | 10 ++++++++++ test/unit/mock/backend.js | 10 ++++++++++ 7 files changed, 58 insertions(+), 3 deletions(-) diff --git a/helper/geojsonify.js b/helper/geojsonify.js index 7bdae990..630f12be 100644 --- a/helper/geojsonify.js +++ b/helper/geojsonify.js @@ -14,11 +14,16 @@ function search( docs ){ // flatten & expand data for geojson conversion var geodata = docs.map( function( doc ){ - var output = {}; - // something went very wrong if( !doc ) return warning(); + var output = {}; + + // provide metadata to consumer + output.id = doc._id; + output.type = doc._type; + output.layer = doc._type; + // map center_point if( !doc.center_point ) return warning(); output.lat = parseFloat( doc.center_point.lat ); diff --git a/service/mget.js b/service/mget.js index a5713f03..cfc79ca3 100644 --- a/service/mget.js +++ b/service/mget.js @@ -29,7 +29,19 @@ function service( backend, query, cb ){ // map returned documents var docs = []; if( data && Array.isArray(data.docs) ){ - docs = data.docs.map( function( doc ){ + + docs = data.docs.filter( function( doc ){ + + // remove docs not actually found + return doc.found; + + }).map( function( doc ){ + + // map metadata in to _source so we + // can serve it up to the consumer + doc._source._id = doc._id; + doc._source._type = doc._type; + return doc._source; }); } diff --git a/service/search.js b/service/search.js index 6f525123..efe10cea 100644 --- a/service/search.js +++ b/service/search.js @@ -17,6 +17,12 @@ function service( backend, cmd, cb ){ var docs = []; if( data && data.hits && data.hits.total && Array.isArray(data.hits.hits)){ docs = data.hits.hits.map( function( hit ){ + + // map metadata in to _source so we + // can serve it up to the consumer + hit._source._id = hit._id; + hit._source._type = hit._type; + return hit._source; }); } diff --git a/test/unit/controller/search.js b/test/unit/controller/search.js index 7e714118..19482eb9 100644 --- a/test/unit/controller/search.js +++ b/test/unit/controller/search.js @@ -24,6 +24,9 @@ module.exports.tests.functional_success = function(test, common) { coordinates: [ -50.5, 100.1 ] }, properties: { + id: 'myid1', + type: 'mytype1', + layer: 'mytype1', name: 'test name1', admin0: 'country1', admin1: 'state1', @@ -37,6 +40,9 @@ module.exports.tests.functional_success = function(test, common) { coordinates: [ -51.5, 100.2 ] }, properties: { + id: 'myid2', + type: 'mytype2', + layer: 'mytype2', name: 'test name2', admin0: 'country2', admin1: 'state2', diff --git a/test/unit/controller/suggest.js b/test/unit/controller/suggest.js index 80eb9eea..283e78cc 100644 --- a/test/unit/controller/suggest.js +++ b/test/unit/controller/suggest.js @@ -24,6 +24,9 @@ module.exports.tests.functional_success = function(test, common) { coordinates: [ -50.5, 100.1 ] }, properties: { + id: 'myid1', + type: 'mytype1', + layer: 'mytype1', name: 'test name1', admin0: 'country1', admin1: 'state1', @@ -37,6 +40,9 @@ module.exports.tests.functional_success = function(test, common) { coordinates: [ -51.5, 100.2 ] }, properties: { + id: 'myid2', + type: 'mytype2', + layer: 'mytype2', name: 'test name2', admin0: 'country2', admin1: 'state2', diff --git a/test/unit/helper/geojsonify.js b/test/unit/helper/geojsonify.js index 61a71eeb..27bb8b31 100644 --- a/test/unit/helper/geojsonify.js +++ b/test/unit/helper/geojsonify.js @@ -15,6 +15,8 @@ module.exports.tests.search = function(test, common) { var input = [ { + "_id": "id1", + "_type": "type1", "center_point": { "lat": 51.5337144, "lon": -0.1069716 @@ -48,6 +50,8 @@ module.exports.tests.search = function(test, common) { } }, { + "_id": "id2", + "_type": "type2", "type": "way", "name": { "default": "Blues Cafe" @@ -90,6 +94,9 @@ module.exports.tests.search = function(test, common) { ] }, "properties": { + "id": "id1", + "type": "type1", + "layer": "type1", "text": "'Round Midnight Jazz and Blues Bar, Angel, United Kingdom", "name": "'Round Midnight Jazz and Blues Bar", "alpha3": "GBR", @@ -112,6 +119,9 @@ module.exports.tests.search = function(test, common) { ] }, "properties": { + "id": "id2", + "type": "type2", + "layer": "type2", "text": "Blues Cafe, Smithfield, United Kingdom", "name": "Blues Cafe", "alpha3": "GBR", diff --git a/test/unit/mock/backend.js b/test/unit/mock/backend.js index f2759168..91a67b00 100644 --- a/test/unit/mock/backend.js +++ b/test/unit/mock/backend.js @@ -9,6 +9,8 @@ responses['client/suggest/fail/1'] = function( cmd, cb ){ }; responses['client/search/ok/1'] = function( cmd, cb ){ return cb( undefined, searchEnvelope([{ + _id: 'myid1', + _type: 'mytype1', _source: { value: 1, center_point: { lat: 100.1, lon: -50.5 }, @@ -16,6 +18,8 @@ responses['client/search/ok/1'] = function( cmd, cb ){ admin0: 'country1', admin1: 'state1', admin2: 'city1' } }, { + _id: 'myid2', + _type: 'mytype2', _source: { value: 2, center_point: { lat: 100.2, lon: -51.5 }, @@ -26,6 +30,9 @@ responses['client/search/ok/1'] = function( cmd, cb ){ }; responses['client/mget/ok/1'] = function( cmd, cb ){ return cb( undefined, mgetEnvelope([{ + _id: 'myid1', + _type: 'mytype1', + found: true, _source: { value: 1, center_point: { lat: 100.1, lon: -50.5 }, @@ -33,6 +40,9 @@ responses['client/mget/ok/1'] = function( cmd, cb ){ admin0: 'country1', admin1: 'state1', admin2: 'city1' } }, { + _id: 'myid2', + _type: 'mytype2', + found: true, _source: { value: 2, center_point: { lat: 100.2, lon: -51.5 },