diff --git a/controller/place.js b/controller/place.js index 9e41a47c..b2efdf04 100644 --- a/controller/place.js +++ b/controller/place.js @@ -53,6 +53,12 @@ function setup( apiConfig, esclient ){ return; } + // if execution has gotten this far then one of three things happened: + // - the request didn't time out + // - maxRetries has been hit so we're giving up + // - another error occurred + // in either case, handle the error or results + // error handler if( err ){ if (_.isObject(err) && err.message) { diff --git a/controller/search.js b/controller/search.js index 168e47c4..2813bf3c 100644 --- a/controller/search.js +++ b/controller/search.js @@ -80,6 +80,12 @@ function setup( apiConfig, esclient, query ){ return; } + // if execution has gotten this far then one of three things happened: + // - the request didn't time out + // - maxRetries has been hit so we're giving up + // - another error occurred + // in either case, handle the error or results + // error handler if( err ){ if (_.isObject(err) && err.message) { diff --git a/service/search.js b/service/search.js index 9453aaa2..dcdd7093 100644 --- a/service/search.js +++ b/service/search.js @@ -30,7 +30,6 @@ function service( esclient, cmd, cb ){ }; if( data && data.hits && data.hits.total && Array.isArray(data.hits.hits)){ - docs = data.hits.hits.map( function( hit ){ meta.scores.push(hit._score); diff --git a/test/unit/mock/backend.js b/test/unit/mock/backend.js deleted file mode 100644 index 4751acc0..00000000 --- a/test/unit/mock/backend.js +++ /dev/null @@ -1,91 +0,0 @@ - -var responses = {}; -responses['client/search/ok/1'] = function( cmd, cb ){ - return cb( undefined, searchEnvelope([{ - _id: 'myid1', - _type: 'mytype1', - _score: 10, - matched_queries: ['query 1', 'query 2'], - _source: { - value: 1, - center_point: { lat: 100.1, lon: -50.5 }, - name: { default: 'test name1' }, - parent: { country: ['country1'], region: ['state1'], county: ['city1'] } - } - }, { - _id: 'myid2', - _type: 'mytype2', - _score: 20, - matched_queries: ['query 3'], - _source: { - value: 2, - center_point: { lat: 100.2, lon: -51.5 }, - name: { default: 'test name2' }, - parent: { country: ['country2'], region: ['state2'], county: ['city2'] } - } - }])); -}; -responses['client/search/fail/1'] = function( cmd, cb ){ - return cb( 'an elasticsearch error occurred' ); -}; - -responses['client/mget/ok/1'] = function( cmd, cb ){ - return cb( undefined, mgetEnvelope([{ - _id: 'myid1', - _type: 'mytype1', - _score: 10, - found: true, - _source: { - value: 1, - center_point: { lat: 100.1, lon: -50.5 }, - name: { default: 'test name1' }, - parent: { country: ['country1'], region: ['state1'], county: ['city1'] } - } - }, { - _id: 'myid2', - _type: 'mytype2', - _score: 20, - found: true, - _source: { - value: 2, - center_point: { lat: 100.2, lon: -51.5 }, - name: { default: 'test name2' }, - parent: { country: ['country2'], region: ['state2'], county: ['city2'] } - } - }])); -}; -responses['client/mget/fail/1'] = responses['client/search/fail/1']; - -function setup( key, cmdCb ){ - function backend( a, b ){ - return { - mget: function( cmd, cb ){ - if( 'function' === typeof cmdCb ){ cmdCb( cmd ); } - return responses[key.indexOf('mget') === -1 ? 'client/mget/ok/1' : key].apply( this, arguments ); - }, - suggest: function( cmd, cb ){ - if( 'function' === typeof cmdCb ){ cmdCb( cmd ); } - return responses[key].apply( this, arguments ); - }, - search: function( cmd, cb ){ - if( 'function' === typeof cmdCb ){ cmdCb( cmd ); } - return responses[key].apply( this, arguments ); - } - }; - } - return backend(); -} - -function mgetEnvelope( options ){ - return { docs: options }; -} - -function suggestEnvelope( options1, options2 ){ - return { 0: [{ options: options1 }], 1: [{ options: options2 }]}; -} - -function searchEnvelope( options ){ - return { hits: { total: options.length, hits: options } }; -} - -module.exports = setup; diff --git a/test/unit/service/mget.js b/test/unit/service/mget.js index 6d347111..de3ac882 100644 --- a/test/unit/service/mget.js +++ b/test/unit/service/mget.js @@ -1,13 +1,9 @@ - -var service = require('../../../service/mget'), - mockBackend = require('../mock/backend'); - const proxyquire = require('proxyquire').noCallThru(); module.exports.tests = {}; -module.exports.tests.interface = function(test, common) { - test('valid interface', function(t) { +module.exports.tests.interface = (test, common) => { + test('valid interface', (t) => { var service = proxyquire('../../../service/mget', { 'pelias-logger': { get: (section) => { @@ -22,82 +18,243 @@ module.exports.tests.interface = function(test, common) { }); }; -// functionally test service -module.exports.tests.functional_success = function(test, common) { - - var expected = [ - { - _id: 'myid1', _type: 'mytype1', - value: 1, - center_point: { lat: 100.1, lon: -50.5 }, - name: { default: 'test name1' }, - parent: { country: ['country1'], region: ['state1'], county: ['city1'] } - }, - { - _id: 'myid2', _type: 'mytype2', - value: 2, - center_point: { lat: 100.2, lon: -51.5 }, - name: { default: 'test name2' }, - parent: { country: ['country2'], region: ['state2'], county: ['city2'] } - } - ]; - - test('valid query', function(t) { - var backend = mockBackend( 'client/mget/ok/1', function( cmd ){ - t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: 'a' } ] } }, 'correct backend command'); +module.exports.tests.error_conditions = (test, common) => { + test('esclient.mget returning error should log and pass it on', (t) => { + const errorMessages = []; + + const service = proxyquire('../../../service/mget', { + 'pelias-logger': { + get: () => { + return { + error: (msg) => { + errorMessages.push(msg); + } + }; + } + } }); - service( backend, [ { _id: 123, _index: 'pelias', _type: 'a' } ], function(err, data) { - t.true(Array.isArray(data), 'returns an array'); - data.forEach(function(d) { - t.true(typeof d === 'object', 'valid object'); - }); - t.deepEqual(data, expected, 'values correctly mapped'); + + const expectedCmd = { + body: { + docs: 'this is the query' + } + }; + + const esclient = { + mget: (cmd, callback) => { + t.deepEquals(cmd, expectedCmd); + + const err = 'this is an error'; + const data = { + docs: [ + { + found: true, + _id: 'doc id', + _type: 'doc type', + _source: {} + } + ] + }; + + callback('this is an error', data); + + } + }; + + const next = (err, docs) => { + t.equals(err, 'this is an error', 'err should have been passed on'); + t.equals(docs, undefined); + + t.ok(errorMessages.find((msg) => { + return msg === `elasticsearch error ${err}`; + })); t.end(); - }); - }); + }; + + service(esclient, 'this is the query', next); + }); }; -// functionally test service -module.exports.tests.functional_failure = function(test, common) { +module.exports.tests.success_conditions = (test, common) => { + test('esclient.mget returning data.docs should filter and map', (t) => { + const errorMessages = []; - test('invalid query', function(t) { - var invalid_queries = [ - { _id: 123, _index: 'pelias' }, - { _id: 123, _type: 'a' }, - { _index: 'pelias', _type: 'a' }, - { } - ]; - - var backend = mockBackend( 'client/mget/fail/1', function( cmd ){ - t.notDeepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: 'a' } ] } }, 'incorrect backend command'); + const service = proxyquire('../../../service/mget', { + 'pelias-logger': { + get: () => { + return { + error: (msg) => { + errorMessages.push(msg); + } + }; + } + } }); - invalid_queries.forEach(function(query) { - // mock out pelias-logger so we can assert what's being logged - var service = proxyquire('../../../service/mget', { - 'pelias-logger': { - get: () => { - return { - error: (msg) => { - t.equal(msg, 'elasticsearch error an elasticsearch error occurred'); + + const expectedCmd = { + body: { + docs: 'this is the query' + } + }; + + const esclient = { + mget: (cmd, callback) => { + t.deepEquals(cmd, expectedCmd); + + const data = { + docs: [ + { + found: true, + _id: 'doc id 1', + _type: 'doc type 1', + _source: { + random_key: 'value 1' } - }; - } + }, + { + found: false, + _id: 'doc id 2', + _type: 'doc type 2', + _source: {} + }, + { + found: true, + _id: 'doc id 3', + _type: 'doc type 3', + _source: { + random_key: 'value 3' + } + } + ] + }; + + callback(undefined, data); + + } + }; + + const expectedDocs = [ + { + _id: 'doc id 1', + _type: 'doc type 1', + random_key: 'value 1' + }, + { + _id: 'doc id 3', + _type: 'doc type 3', + random_key: 'value 3' + } + + ]; + + const next = (err, docs) => { + t.equals(err, null); + t.deepEquals(docs, expectedDocs); + + t.equals(errorMessages.length, 0, 'no errors should have been logged'); + t.end(); + }; + + service(esclient, 'this is the query', next); + + }); + + test('esclient.mget callback with falsy data should return empty array', (t) => { + const errorMessages = []; + + const service = proxyquire('../../../service/mget', { + 'pelias-logger': { + get: () => { + return { + error: (msg) => { + errorMessages.push(msg); + } + }; } + } + }); + + const expectedCmd = { + body: { + docs: 'this is the query' + } + }; + + const esclient = { + mget: (cmd, callback) => { + t.deepEquals(cmd, expectedCmd); + + callback(undefined, undefined); + + } + }; + + const expectedDocs = []; + + const next = (err, docs) => { + t.equals(err, null); + t.deepEquals(docs, expectedDocs); + + t.equals(errorMessages.length, 0, 'no errors should have been logged'); + t.end(); + }; + + service(esclient, 'this is the query', next); - }); + }); + + test('esclient.mget callback with non-array data.docs should return empty array', (t) => { + const errorMessages = []; - service( backend, [ query ], function(err, data) { - t.equal(err, 'an elasticsearch error occurred','error passed to errorHandler'); - t.equal(data, undefined, 'data is undefined'); - }); + const service = proxyquire('../../../service/mget', { + 'pelias-logger': { + get: () => { + return { + error: (msg) => { + errorMessages.push(msg); + } + }; + } + } }); - t.end(); + + const expectedCmd = { + body: { + docs: 'this is the query' + } + }; + + const esclient = { + mget: (cmd, callback) => { + t.deepEquals(cmd, expectedCmd); + + const data = { + docs: 'this isn\'t an array' + }; + + callback(undefined, data); + + } + }; + + const expectedDocs = []; + + const next = (err, docs) => { + t.equals(err, null); + t.deepEquals(docs, expectedDocs); + + t.equals(errorMessages.length, 0, 'no errors should have been logged'); + t.end(); + }; + + service(esclient, 'this is the query', next); + }); }; -module.exports.all = function (tape, common) { +module.exports.all = (tape, common) => { function test(name, testFunction) { return tape('SERVICE /mget ' + name, testFunction); diff --git a/test/unit/service/search.js b/test/unit/service/search.js index 146b9744..5a8cdae9 100644 --- a/test/unit/service/search.js +++ b/test/unit/service/search.js @@ -1,16 +1,10 @@ - -var service = require('../../../service/search'), - mockBackend = require('../mock/backend'); - const proxyquire = require('proxyquire').noCallThru(); -var example_valid_es_query = { body: { a: 'b' }, index: 'pelias' }; - module.exports.tests = {}; -module.exports.tests.interface = function(test, common) { - test('valid interface', function(t) { - var service = proxyquire('../../../service/mget', { +module.exports.tests.interface = (test, common) => { + test('valid interface', (t) => { + var service = proxyquire('../../../service/search', { 'pelias-logger': { get: (section) => { t.equal(section, 'api'); @@ -24,89 +18,344 @@ module.exports.tests.interface = function(test, common) { }); }; -// functionally test service -module.exports.tests.functional_success = function(test, common) { - - var expected = [ - { - _id: 'myid1', _type: 'mytype1', - _score: 10, - _matched_queries: ['query 1', 'query 2'], - value: 1, - center_point: { lat: 100.1, lon: -50.5 }, - name: { default: 'test name1' }, - parent: { country: ['country1'], region: ['state1'], county: ['city1'] } - }, - { - _id: 'myid2', _type: 'mytype2', - _score: 20, - _matched_queries: ['query 3'], - value: 2, - center_point: { lat: 100.2, lon: -51.5 }, - name: { default: 'test name2' }, - parent: { country: ['country2'], region: ['state2'], county: ['city2'] } - } - ]; - - var expectedMeta = { - scores: [10, 20] - }; - - test('valid ES query', function(t) { - var backend = mockBackend( 'client/search/ok/1', function( cmd ){ - t.deepEqual(cmd, example_valid_es_query, 'no change to the command'); +module.exports.tests.error_conditions = (test, common) => { + test('esclient.search returning error should log and pass it on', (t) => { + const errorMessages = []; + + const service = proxyquire('../../../service/search', { + 'pelias-logger': { + get: () => { + return { + error: (msg) => { + errorMessages.push(msg); + } + }; + } + } }); - service( backend, example_valid_es_query, function(err, data, meta) { - t.true(Array.isArray(data), 'returns an array'); - data.forEach(function(d) { - t.true(typeof d === 'object', 'valid object'); - }); - t.deepEqual(data, expected, 'values correctly mapped'); - t.deepEqual(meta, expectedMeta, 'meta data correctly mapped'); + + const esclient = { + search: (cmd, callback) => { + t.deepEquals(cmd, 'this is the query'); + + const err = 'this is an error'; + const data = { + docs: [ + { + found: true, + _id: 'doc id', + _type: 'doc type', + _source: {} + } + ] + }; + + callback('this is an error', data); + + } + }; + + const next = (err, docs) => { + t.equals(err, 'this is an error', 'err should have been passed on'); + t.equals(docs, undefined); + + t.ok(errorMessages.find((msg) => { + return msg === `elasticsearch error ${err}`; + })); t.end(); - }); - }); + }; + service(esclient, 'this is the query', next); + + }); }; -// functionally test service -module.exports.tests.functional_failure = function(test, common) { +module.exports.tests.success_conditions = (test, common) => { + test('esclient.search returning data.docs should filter and map', (t) => { + const errorMessages = []; + + const service = proxyquire('../../../service/search', { + 'pelias-logger': { + get: () => { + return { + error: (msg) => { + errorMessages.push(msg); + } + }; + } + } + }); + + const esclient = { + search: (cmd, callback) => { + t.deepEquals(cmd, 'this is the query'); + + const data = { + hits: { + total: 17, + hits: [ + { + _score: 'score 1', + _id: 'doc id 1', + _type: 'doc type 1', + matched_queries: 'matched_queries 1', + _source: { + random_key: 'value 1' + } + }, + { + _score: 'score 2', + _id: 'doc id 2', + _type: 'doc type 2', + matched_queries: 'matched_queries 2', + _source: { + random_key: 'value 2' + } + } + ] + } + }; + + callback(undefined, data); + + } + }; - test('invalid ES query', function(t) { - var invalid_queries = [ - { }, - { foo: 'bar' } + const expectedDocs = [ + { + _score: 'score 1', + _id: 'doc id 1', + _type: 'doc type 1', + random_key: 'value 1', + _matched_queries: 'matched_queries 1' + }, + { + _score: 'score 2', + _id: 'doc id 2', + _type: 'doc type 2', + random_key: 'value 2', + _matched_queries: 'matched_queries 2' + } ]; - var backend = mockBackend( 'client/search/fail/1', function( cmd ){ - t.notDeepEqual(cmd, example_valid_es_query, 'incorrect backend command'); + const expectedMeta = { + scores: ['score 1', 'score 2'] + }; + + const next = (err, docs, meta) => { + t.equals(err, null); + t.deepEquals(docs, expectedDocs); + t.deepEquals(meta, expectedMeta); + + t.equals(errorMessages.length, 0, 'no errors should have been logged'); + t.end(); + }; + + service(esclient, 'this is the query', next); + + }); + + test('esclient.search returning falsy data should return empty docs and meta', (t) => { + const errorMessages = []; + + const service = proxyquire('../../../service/search', { + 'pelias-logger': { + get: () => { + return { + error: (msg) => { + errorMessages.push(msg); + } + }; + } + } }); - invalid_queries.forEach(function(query) { - // mock out pelias-logger so we can assert what's being logged - var service = proxyquire('../../../service/search', { - 'pelias-logger': { - get: () => { - return { - error: (msg) => { - t.equal(msg, 'elasticsearch error an elasticsearch error occurred'); - } - }; + + const esclient = { + search: (cmd, callback) => { + t.deepEquals(cmd, 'this is the query'); + + callback(undefined, undefined); + + } + }; + + const expectedDocs = []; + const expectedMeta = { scores: [] }; + + const next = (err, docs, meta) => { + t.equals(err, null); + t.deepEquals(docs, expectedDocs); + t.deepEquals(meta, expectedMeta); + + t.equals(errorMessages.length, 0, 'no errors should have been logged'); + t.end(); + }; + + service(esclient, 'this is the query', next); + + }); + + test('esclient.search returning falsy data.hits should return empty docs and meta', (t) => { + const errorMessages = []; + + const service = proxyquire('../../../service/search', { + 'pelias-logger': { + get: () => { + return { + error: (msg) => { + errorMessages.push(msg); + } + }; + } + } + }); + + const esclient = { + search: (cmd, callback) => { + t.deepEquals(cmd, 'this is the query'); + + const data = { + hits: { + total: 17 } + }; + + callback(undefined, data); + + } + }; + + const expectedDocs = []; + const expectedMeta = { scores: [] }; + + const next = (err, docs, meta) => { + t.equals(err, null); + t.deepEquals(docs, expectedDocs); + t.deepEquals(meta, expectedMeta); + + t.equals(errorMessages.length, 0, 'no errors should have been logged'); + t.end(); + }; + + service(esclient, 'this is the query', next); + + }); + + test('esclient.search returning falsy data.hits.total should return empty docs and meta', (t) => { + const errorMessages = []; + + const service = proxyquire('../../../service/search', { + 'pelias-logger': { + get: () => { + return { + error: (msg) => { + errorMessages.push(msg); + } + }; } + } + }); + + const esclient = { + search: (cmd, callback) => { + t.deepEquals(cmd, 'this is the query'); + + const data = { + hits: { + hits: [ + { + _score: 'score 1', + _id: 'doc id 1', + _type: 'doc type 1', + matched_queries: 'matched_queries 1', + _source: { + random_key: 'value 1' + } + }, + { + _score: 'score 2', + _id: 'doc id 2', + _type: 'doc type 2', + matched_queries: 'matched_queries 2', + _source: { + random_key: 'value 2' + } + } + ] + } + }; - }); + callback(undefined, data); - service( backend, [ query ], function(err, data) { - t.equal(err, 'an elasticsearch error occurred','error passed to errorHandler'); - t.equal(data, undefined, 'data is undefined'); - }); + } + }; + + const expectedDocs = []; + const expectedMeta = { scores: [] }; + + const next = (err, docs, meta) => { + t.equals(err, null); + t.deepEquals(docs, expectedDocs); + t.deepEquals(meta, expectedMeta); + + t.equals(errorMessages.length, 0, 'no errors should have been logged'); + t.end(); + }; + + service(esclient, 'this is the query', next); + + }); + + test('esclient.search returning non-array data.hits.hits should return empty docs and meta', (t) => { + const errorMessages = []; + + const service = proxyquire('../../../service/search', { + 'pelias-logger': { + get: () => { + return { + error: (msg) => { + errorMessages.push(msg); + } + }; + } + } }); - t.end(); + + const esclient = { + search: (cmd, callback) => { + t.deepEquals(cmd, 'this is the query'); + + const data = { + hits: { + total: 17, + hits: 'this isn\'t an array' + } + }; + + callback(undefined, data); + + } + }; + + const expectedDocs = []; + const expectedMeta = { scores: [] }; + + const next = (err, docs, meta) => { + t.equals(err, null); + t.deepEquals(docs, expectedDocs); + t.deepEquals(meta, expectedMeta); + + t.equals(errorMessages.length, 0, 'no errors should have been logged'); + t.end(); + }; + + service(esclient, 'this is the query', next); + }); }; -module.exports.all = function (tape, common) { +module.exports.all = (tape, common) => { function test(name, testFunction) { return tape('SERVICE /search ' + name, testFunction);