diff --git a/README.md b/README.md index 2f6e6627..bda2627a 100644 --- a/README.md +++ b/README.md @@ -7,8 +7,9 @@ This is the API server for the Pelias project. It's the service that runs to process user HTTP requests and return results as GeoJSON by querying Elasticsearch. +[![NPM](https://nodei.co/npm/pelias-api.png?downloads=true&stars=true)](https://nodei.co/npm/pelias-api) + [![Gitter](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/pelias/api?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) -[![Build Status](https://travis-ci.org/pelias/api.png?branch=master)](https://travis-ci.org/pelias/api) ## Documentation @@ -31,6 +32,7 @@ The API ships with several convenience commands (runnable via `npm`): * `npm run ciao`: run functional tests (this requires that the server be running) * `npm run docs`: generate API documentation * `npm run coverage`: generate code coverage reports + * `npm run config`: dump the configuration to the command line, which is useful for debugging configuration issues ## pelias-config The API recognizes the following properties under the top-level `api` key in your `pelias.json` config file: @@ -81,3 +83,16 @@ $ curl localhost:9200/pelias/_count?pretty ... } ``` + +### Continuous Integration + +Travis tests every release against Node.js versions `4` and `6`. + +[![Build Status](https://travis-ci.org/pelias/api.png?branch=master)](https://travis-ci.org/pelias/api) + + +### Versioning + +We rely on semantic-release and Greenkeeper to maintain our module and dependency versions. + +[![Greenkeeper badge](https://badges.greenkeeper.io/pelias/api.svg)](https://greenkeeper.io/) 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/helper/geojsonify_place_details.js b/helper/geojsonify_place_details.js index 44e827d5..d9a000ee 100644 --- a/helper/geojsonify_place_details.js +++ b/helper/geojsonify_place_details.js @@ -7,6 +7,7 @@ var DETAILS_PROPS = [ { name: 'housenumber', type: 'string' }, { name: 'street', type: 'string' }, { name: 'postalcode', type: 'string' }, + { name: 'postalcode_gid', type: 'string' }, { name: 'confidence', type: 'default' }, { name: 'match_type', type: 'string' }, { name: 'distance', type: 'default' }, diff --git a/helper/placeTypes.js b/helper/placeTypes.js index c9550ce7..defc9274 100644 --- a/helper/placeTypes.js +++ b/helper/placeTypes.js @@ -8,5 +8,6 @@ module.exports = [ 'localadmin', 'locality', 'borough', - 'neighbourhood' + 'neighbourhood', + 'postalcode' ]; diff --git a/helper/type_mapping.js b/helper/type_mapping.js index c43a96bb..7b9de5a0 100644 --- a/helper/type_mapping.js +++ b/helper/type_mapping.js @@ -51,7 +51,7 @@ var LAYERS_BY_SOURCE = { 'locality','borough', 'neighbourhood', 'venue' ], whosonfirst: [ 'continent', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'borough', - 'neighbourhood', 'microhood', 'disputed', 'venue'] + 'neighbourhood', 'microhood', 'disputed', 'venue', 'postalcode'] }; /* @@ -62,7 +62,7 @@ var LAYERS_BY_SOURCE = { var LAYER_ALIASES = { 'coarse': [ 'continent', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'borough', - 'neighbourhood', 'microhood', 'disputed'] + 'neighbourhood', 'microhood', 'disputed', 'postalcode' ] }; // create a list of all layers by combining each entry from LAYERS_BY_SOURCE diff --git a/middleware/normalizeParentIds.js b/middleware/normalizeParentIds.js index e0e499e2..ae22a3c6 100644 --- a/middleware/normalizeParentIds.js +++ b/middleware/normalizeParentIds.js @@ -49,6 +49,10 @@ function normalizeParentIds(place) { * @return {string} */ function makeNewId(placeType, id) { + if (!id) { + return; + } + var doc = new Document('whosonfirst', placeType, id); return doc.getGid(); } diff --git a/middleware/renamePlacenames.js b/middleware/renamePlacenames.js index d89a43ad..9f63cb6e 100644 --- a/middleware/renamePlacenames.js +++ b/middleware/renamePlacenames.js @@ -1,12 +1,14 @@ -var _ = require('lodash'); +'use strict'; -var PARENT_PROPS = require('../helper/placeTypes'); +const _ = require('lodash'); -var ADDRESS_PROPS = { - 'number': 'housenumber', - 'zip': 'postalcode', - 'street': 'street' -}; +const PARENT_PROPS = require('../helper/placeTypes'); + +const ADDRESS_PROPS = [ + { name: 'number', newName: 'housenumber' }, + { name: 'zip', newName: 'postalcode', transform: (value) => { return [value]; } }, + { name: 'street', newName: 'street' } +]; function setup() { @@ -28,22 +30,39 @@ function renamePlacenames(req, res, next) { * Rename the fields in one record */ function renameOneRecord(place) { - if (place.address_parts) { - Object.keys(ADDRESS_PROPS).forEach(function (prop) { - place[ADDRESS_PROPS[prop]] = place.address_parts[prop]; - }); - } // merge the parent block into the top level object to flatten the structure + // only copy the properties if they have values if (place.parent) { - PARENT_PROPS.forEach(function (prop) { + PARENT_PROPS.forEach( (prop) => { place[prop] = place.parent[prop]; place[prop + '_a'] = place.parent[prop + '_a']; place[prop + '_gid'] = place.parent[prop + '_id']; }); } + // copy the address parts after parent hierarchy in order to prefer + // the postalcode specified by the original source data + if (place.address_parts) { + ADDRESS_PROPS.forEach( (prop) => { + renameAddressProperty(place, prop); + }); + } + return place; } +function renameAddressProperty(place, prop) { + if (!place.address_parts.hasOwnProperty(prop.name)) { + return; + } + + if (prop.hasOwnProperty('transform')) { + place[prop.newName] = prop.transform(place.address_parts[prop.name]); + } + else { + place[prop.newName] = place.address_parts[prop.name]; + } +} + module.exports = setup; diff --git a/middleware/trimByGranularity.js b/middleware/trimByGranularity.js index da4ef4f9..06bf9141 100644 --- a/middleware/trimByGranularity.js +++ b/middleware/trimByGranularity.js @@ -19,6 +19,7 @@ var layers = [ 'street', 'neighbourhood', 'borough', + 'postalcode', 'locality', 'localadmin', 'county', diff --git a/package.json b/package.json index 5a4c23ea..86e6a7f4 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,8 @@ "travis": "npm test", "unit": "./bin/units", "validate": "npm ls", - "semantic-release": "semantic-release pre && npm publish && semantic-release post" + "semantic-release": "semantic-release pre && npm publish && semantic-release post", + "config": "node -e \"console.log(JSON.stringify(require( 'pelias-config' ).generate(require('./schema')), null, 2))\"" }, "repository": { "type": "git", @@ -50,14 +51,14 @@ "joi": "^10.1.0", "lodash": "^4.5.0", "markdown": "0.5.0", - "morgan": "1.7.0", + "morgan": "1.8.1", "pelias-categories": "1.1.0", - "pelias-config": "2.7.1", + "pelias-config": "2.8.0", "pelias-labels": "1.5.1", "pelias-logger": "0.1.0", - "pelias-model": "4.4.0", - "pelias-query": "8.12.0", - "pelias-text-analyzer": "1.7.0", + "pelias-model": "4.5.1", + "pelias-query": "8.13.0", + "pelias-text-analyzer": "1.7.2", "retry": "^0.10.1", "stats-lite": "2.0.3", "superagent": "^3.2.1", diff --git a/query/search.js b/query/search.js index 48b47f31..91c22299 100644 --- a/query/search.js +++ b/query/search.js @@ -144,7 +144,10 @@ function generateQuery( clean ){ } function getQuery(vs) { - if (hasStreet(vs) || isCityStateOnlyWithOptionalCountry(vs) || isCityCountryOnly(vs)) { + if (hasStreet(vs) || + isCityStateOnlyWithOptionalCountry(vs) || + isCityCountryOnly(vs) || + isPostalCodeOnly(vs)) { return { type: 'fallback', body: fallbackQuery.render(vs) @@ -188,4 +191,18 @@ function isCityCountryOnly(vs) { } +function isPostalCodeOnly(vs) { + var isSet = (layer) => { + return vs.isset(`input:${layer}`); + }; + + var allowedFields = ['postcode']; + var disallowedFields = ['query', 'category', 'housenumber', 'street', + 'neighbourhood', 'borough', 'county', 'region', 'country']; + + return allowedFields.every(isSet) && + !disallowedFields.some(isSet); + +} + module.exports = generateQuery; 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/fixture/search_fallback.js b/test/unit/fixture/search_fallback.js index 857e47e9..3835cadb 100644 --- a/test/unit/fixture/search_fallback.js +++ b/test/unit/fixture/search_fallback.js @@ -193,6 +193,75 @@ module.exports = { } } }, + { + 'bool': { + '_name': 'fallback.postalcode', + 'must': [ + { + 'multi_match': { + 'query': 'postalcode value', + 'type': 'phrase', + 'fields': [ + 'parent.postalcode' + ] + } + }, + { + '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' + ] + } + } + ], + 'filter': { + 'term': { + 'layer': 'postalcode' + } + } + } + }, { 'bool': { '_name': 'fallback.street', diff --git a/test/unit/fixture/search_fallback_postalcode_only.js b/test/unit/fixture/search_fallback_postalcode_only.js new file mode 100644 index 00000000..4519e166 --- /dev/null +++ b/test/unit/fixture/search_fallback_postalcode_only.js @@ -0,0 +1,68 @@ +module.exports = { + 'query': { + 'function_score': { + 'query': { + 'filtered': { + 'query': { + 'bool': { + 'should': [ + { + 'bool': { + '_name': 'fallback.postalcode', + 'must': [ + { + 'multi_match': { + 'query': '90210', + 'type': 'phrase', + 'fields': [ + 'parent.postalcode' + ] + } + } + ], + 'filter': { + 'term': { + 'layer': 'postalcode' + } + } + } + } + ] + } + }, + 'filter': { + 'bool': { + 'must': [] + } + } + } + }, + 'max_boost': 20, + 'functions': [ + { + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'popularity', + 'missing': 1 + }, + 'weight': 1 + }, + { + 'field_value_factor': { + 'modifier': 'log1p', + 'field': 'population', + 'missing': 1 + }, + 'weight': 2 + } + ], + 'score_mode': 'avg', + 'boost_mode': 'multiply' + } + }, + 'size': 20, + 'track_scores': true, + 'sort': [ + '_score' + ] +}; diff --git a/test/unit/helper/type_mapping.js b/test/unit/helper/type_mapping.js index a9ec4721..d67218c5 100644 --- a/test/unit/helper/type_mapping.js +++ b/test/unit/helper/type_mapping.js @@ -14,7 +14,7 @@ module.exports.tests.interfaces = function(test, common) { t.deepEquals(type_mapping.layer_mapping.coarse, [ 'continent', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', - 'borough', 'neighbourhood', 'microhood', 'disputed' ]); + 'borough', 'neighbourhood', 'microhood', 'disputed', 'postalcode' ]); t.end(); }); 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/query/search.js b/test/unit/query/search.js index 1f312d0d..aa582082 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -603,6 +603,25 @@ module.exports.tests.city_country = function(test, common) { }); + test('valid postalcode only search', function(t) { + var clean = { + parsed_text: { + postalcode: '90210' + }, + text: '90210' + }; + + var query = generate(clean); + + var compiled = JSON.parse( JSON.stringify( query ) ); + var expected = require('../fixture/search_fallback_postalcode_only'); + + t.deepEqual(compiled.type, 'fallback', 'query type set'); + t.deepEqual(compiled.body, expected, 'search_fallback_postalcode_only'); + t.end(); + }); + + }; module.exports.all = function (tape, common) { diff --git a/test/unit/sanitizer/_layers.js b/test/unit/sanitizer/_layers.js index 5792e947..5a0c0693 100644 --- a/test/unit/sanitizer/_layers.js +++ b/test/unit/sanitizer/_layers.js @@ -43,7 +43,7 @@ module.exports.tests.sanitize_layers = function(test, common) { var admin_layers = [ 'continent', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', - 'macrohood', 'borough', 'neighbourhood', 'microhood', 'disputed' ]; + 'macrohood', 'borough', 'neighbourhood', 'microhood', 'disputed', 'postalcode' ]; t.deepEqual(clean.layers, admin_layers, 'coarse layers set'); t.end(); @@ -78,7 +78,7 @@ module.exports.tests.sanitize_layers = function(test, common) { var expected_layers = [ 'continent', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', - 'macrohood', 'borough', 'neighbourhood', 'microhood', 'disputed' ]; + 'macrohood', 'borough', 'neighbourhood', 'microhood', 'disputed', 'postalcode' ]; t.deepEqual(clean.layers, expected_layers, 'coarse + regular layers set'); t.end(); @@ -115,7 +115,7 @@ module.exports.tests.sanitize_layers = function(test, common) { var coarse_layers = [ 'continent', 'country', 'dependency', 'macroregion', 'region', 'locality', 'localadmin', 'macrocounty', 'county', 'macrohood', 'borough', 'neighbourhood', 'microhood', - 'disputed' ]; + 'disputed', 'postalcode' ]; var venue_layers = [ 'venue' ]; var expected_layers = venue_layers.concat(coarse_layers); 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);