diff --git a/app.js b/app.js index 2dabb3c9..2034e96b 100644 --- a/app.js +++ b/app.js @@ -1,10 +1,7 @@ var app = require('express')(); -var peliasConfig = require( 'pelias-config' ).generate(); - -// validate the configuration before attempting to load the app -require('./src/configValidation').validate(peliasConfig); +var peliasConfig = require( 'pelias-config' ).generate(require('./schema')); if( peliasConfig.api.accessLog ){ app.use( require( './middleware/access_log' ).createAccessLogger( peliasConfig.api.accessLog ) ); @@ -23,7 +20,7 @@ var legacy = require('./routes/legacy'); legacy.addRoutes(app, peliasConfig.api); var v1 = require('./routes/v1'); -v1.addRoutes(app, peliasConfig.api); +v1.addRoutes(app, peliasConfig); /** ----------------------- error middleware ----------------------- **/ diff --git a/controller/place.js b/controller/place.js index ebf9df18..9e41a47c 100644 --- a/controller/place.js +++ b/controller/place.js @@ -1,44 +1,82 @@ -var service = { mget: require('../service/mget') }; -var logger = require('pelias-logger').get('api'); +'use strict'; -function setup( config, backend ){ +const _ = require('lodash'); +const retry = require('retry'); - // allow overriding of dependencies - backend = backend || require('../src/backend'); +const mgetService = require('../service/mget'); +const logger = require('pelias-logger').get('api'); - function controller( req, res, next ){ +function requestHasErrors(request) { + return _.get(request, 'errors', []).length > 0; +} - // do not run controller when a request - // validation error has occurred. - if( req.errors && req.errors.length ){ +function isRequestTimeout(err) { + return _.get(err, 'status') === 408; +} + +function setup( apiConfig, esclient ){ + function controller( req, res, next ){ + // do not run controller when a request validation error has occurred. + if (requestHasErrors(req)){ return next(); } - var query = req.clean.ids.map( function(id) { + // options for retry + // maxRetries is from the API config with default of 3 + // factor of 1 means that each retry attempt will esclient requestTimeout + const operationOptions = { + retries: _.get(apiConfig, 'requestRetries', 3), + factor: 1, + minTimeout: _.get(esclient, 'transport.requestTimeout') + }; + + // setup a new operation + const operation = retry.operation(operationOptions); + + const cmd = req.clean.ids.map( function(id) { return { - _index: config.indexName, + _index: apiConfig.indexName, _type: id.layers, _id: id.id }; }); - logger.debug( '[ES req]', query ); + logger.debug( '[ES req]', cmd ); - service.mget( backend, query, function( err, docs ) { - console.log('err:' + err); + operation.attempt((currentAttempt) => { + mgetService( esclient, cmd, function( err, docs ) { + // returns true if the operation should be attempted again + // (handles bookkeeping of maxRetries) + // only consider for status 408 (request timeout) + if (isRequestTimeout(err) && operation.retry(err)) { + logger.info(`request timed out on attempt ${currentAttempt}, retrying`); + return; + } - // error handler - if( err ){ - req.errors.push( err ); - } - // set response data - else { - res.data = docs; - } - logger.debug('[ES response]', docs); + // error handler + if( err ){ + if (_.isObject(err) && err.message) { + req.errors.push( err.message ); + } else { + req.errors.push( err ); + } + } + // set response data + else { + // log that a retry was successful + // most requests succeed on first attempt so this declutters log files + if (currentAttempt > 1) { + logger.info(`succeeded on retry ${currentAttempt-1}`); + } - next(); + res.data = docs; + } + logger.debug('[ES response]', docs); + + next(); + }); }); + } return controller; diff --git a/controller/search.js b/controller/search.js index f4aa3ab5..168e47c4 100644 --- a/controller/search.js +++ b/controller/search.js @@ -1,75 +1,118 @@ -var _ = require('lodash'); +'use strict'; -var service = { search: require('../service/search') }; -var logger = require('pelias-logger').get('api'); -var logging = require( '../helper/logging' ); +const _ = require('lodash'); -function setup( config, backend, query ){ +const searchService = require('../service/search'); +const logger = require('pelias-logger').get('api'); +const logging = require( '../helper/logging' ); +const retry = require('retry'); - // allow overriding of dependencies - backend = backend || require('../src/backend'); - query = query || require('../query/search'); +function requestHasErrors(request) { + return _.get(request, 'errors', []).length > 0; +} + +function responseHasData(response) { + return _.get(response, 'data', []).length > 0; +} +function isRequestTimeout(err) { + return _.get(err, 'status') === 408; +} + +function setup( apiConfig, esclient, query ){ function controller( req, res, next ){ // do not run controller when a request // validation error has occurred. - if( req.errors && req.errors.length ){ + if (requestHasErrors(req)) { return next(); } // do not run controller if there are already results // this was added during libpostal integration. if the libpostal parse/query // doesn't return anything then fallback to old search-engine-y behavior - if (res && res.hasOwnProperty('data') && res.data.length > 0) { + if (responseHasData(res)) { return next(); } - var cleanOutput = _.cloneDeep(req.clean); + let cleanOutput = _.cloneDeep(req.clean); if (logging.isDNT(req)) { cleanOutput = logging.removeFields(cleanOutput); } // log clean parameters for stats logger.info('[req]', 'endpoint=' + req.path, cleanOutput); - var renderedQuery = query(req.clean); + const renderedQuery = query(req.clean); // if there's no query to call ES with, skip the service if (_.isUndefined(renderedQuery)) { return next(); } - // backend command - var cmd = { - index: config.indexName, + // options for retry + // maxRetries is from the API config with default of 3 + // factor of 1 means that each retry attempt will esclient requestTimeout + const operationOptions = { + retries: _.get(apiConfig, 'requestRetries', 3), + factor: 1, + minTimeout: _.get(esclient, 'transport.requestTimeout') + }; + + // setup a new operation + const operation = retry.operation(operationOptions); + + // elasticsearch command + const cmd = { + index: apiConfig.indexName, searchType: 'dfs_query_then_fetch', body: renderedQuery.body }; logger.debug( '[ES req]', cmd ); - // query backend - service.search( backend, cmd, function( err, docs, meta ){ + operation.attempt((currentAttempt) => { + // query elasticsearch + searchService( esclient, cmd, function( err, docs, meta ){ + // returns true if the operation should be attempted again + // (handles bookkeeping of maxRetries) + // only consider for status 408 (request timeout) + if (isRequestTimeout(err) && operation.retry(err)) { + logger.info(`request timed out on attempt ${currentAttempt}, retrying`); + return; + } - // error handler - if( err ){ - if (_.isObject(err) && err.message) { - req.errors.push( err.message ); - } else { - req.errors.push( err ); + // error handler + if( err ){ + if (_.isObject(err) && err.message) { + req.errors.push( err.message ); + } else { + req.errors.push( err ); + } } - } - // set response data - else { - res.data = docs; - res.meta = meta || {}; - // store the query_type for subsequent middleware - res.meta.query_type = renderedQuery.type; - - logger.info(`[controller:search] [queryType:${renderedQuery.type}] [es_result_count:` + - (res.data && res.data.length ? res.data.length : 0)); - } - logger.debug('[ES response]', docs); - next(); + // set response data + else { + // log that a retry was successful + // most requests succeed on first attempt so this declutters log files + if (currentAttempt > 1) { + logger.info(`succeeded on retry ${currentAttempt-1}`); + } + + res.data = docs; + res.meta = meta || {}; + // store the query_type for subsequent middleware + res.meta.query_type = renderedQuery.type; + + const messageParts = [ + '[controller:search]', + `[queryType:${renderedQuery.type}]`, + `[es_result_count:${_.get(res, 'data', []).length}]` + ]; + + logger.info(messageParts.join(' ')); + } + logger.debug('[ES response]', docs); + next(); + }); + }); } diff --git a/package.json b/package.json index 2340c415..5a4c23ea 100644 --- a/package.json +++ b/package.json @@ -41,24 +41,24 @@ "elasticsearch": "^12.0.1", "elasticsearch-exceptions": "0.0.4", "express": "^4.8.8", - "express-http-proxy": "^0.10.0", + "express-http-proxy": "^0.11.0", "extend": "3.0.0", "geojson": "^0.4.0", "geojson-extent": "^0.3.1", "geolib": "^2.0.18", - "geopipes-elasticsearch-backend": "^0.2.0", "iso3166-1": "^0.2.3", "joi": "^10.1.0", "lodash": "^4.5.0", "markdown": "0.5.0", "morgan": "1.7.0", "pelias-categories": "1.1.0", - "pelias-config": "2.6.0", + "pelias-config": "2.7.1", "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", + "retry": "^0.10.1", "stats-lite": "2.0.3", "superagent": "^3.2.1", "through2": "^2.0.3" diff --git a/routes/v1.js b/routes/v1.js index a18dc797..64f820db 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -1,6 +1,5 @@ -var express = require('express'); var Router = require('express').Router; -var reverseQuery = require('../query/reverse'); +var elasticsearch = require('elasticsearch'); /** ----------------------- sanitizers ----------------------- **/ var sanitizers = { @@ -30,7 +29,9 @@ var controllers = { var queries = { libpostal: require('../query/search'), fallback_to_old_prod: require('../query/search_original'), - structured_geocoding: require('../query/structured_geocoding') + structured_geocoding: require('../query/structured_geocoding'), + reverse: require('../query/reverse'), + autocomplete: require('../query/autocomplete') }; /** ----------------------- controllers ----------------------- **/ @@ -61,6 +62,7 @@ var postProc = { * @param {object} peliasConfig */ function addRoutes(app, peliasConfig) { + const esclient = elasticsearch.Client(peliasConfig.esclient); var base = '/v1/'; @@ -68,23 +70,22 @@ function addRoutes(app, peliasConfig) { var routers = { index: createRouter([ - controllers.mdToHTML(peliasConfig, './public/apiDoc.md') + controllers.mdToHTML(peliasConfig.api, './public/apiDoc.md') ]), attribution: createRouter([ - controllers.mdToHTML(peliasConfig, './public/attribution.md') + controllers.mdToHTML(peliasConfig.api, './public/attribution.md') ]), search: createRouter([ sanitizers.search.middleware, middleware.calcSize(), - // 2nd parameter is `backend` which gets initialized internally // 3rd parameter is which query module to use, use fallback/geodisambiguation // first, then use original search strategy if first query didn't return anything - controllers.search(peliasConfig, undefined, queries.libpostal), + controllers.search(peliasConfig.api, esclient, queries.libpostal), sanitizers.search_fallback.middleware, - controllers.search(peliasConfig, undefined, queries.fallback_to_old_prod), + controllers.search(peliasConfig.api, esclient, queries.fallback_to_old_prod), postProc.trimByGranularity(), postProc.distances('focus.point.'), - postProc.confidenceScores(peliasConfig), + postProc.confidenceScores(peliasConfig.api), postProc.confidenceScoresFallback(), postProc.dedupe(), postProc.interpolate(), @@ -94,16 +95,16 @@ function addRoutes(app, peliasConfig) { postProc.parseBoundingBox(), postProc.normalizeParentIds(), postProc.assignLabels(), - postProc.geocodeJSON(peliasConfig, base), + postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON ]), structured: createRouter([ sanitizers.structured_geocoding.middleware, middleware.calcSize(), - controllers.search(peliasConfig, undefined, queries.structured_geocoding), + controllers.search(peliasConfig.api, esclient, queries.structured_geocoding), postProc.trimByGranularityStructured(), postProc.distances('focus.point.'), - postProc.confidenceScores(peliasConfig), + postProc.confidenceScores(peliasConfig.api), postProc.confidenceScoresFallback(), postProc.dedupe(), postProc.interpolate(), @@ -113,14 +114,14 @@ function addRoutes(app, peliasConfig) { postProc.parseBoundingBox(), postProc.normalizeParentIds(), postProc.assignLabels(), - postProc.geocodeJSON(peliasConfig, base), + postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON ]), autocomplete: createRouter([ sanitizers.autocomplete.middleware, - controllers.search(peliasConfig, null, require('../query/autocomplete')), + controllers.search(peliasConfig.api, esclient, queries.autocomplete), postProc.distances('focus.point.'), - postProc.confidenceScores(peliasConfig), + postProc.confidenceScores(peliasConfig.api), postProc.dedupe(), postProc.accuracy(), postProc.localNamingConventions(), @@ -128,13 +129,13 @@ function addRoutes(app, peliasConfig) { postProc.parseBoundingBox(), postProc.normalizeParentIds(), postProc.assignLabels(), - postProc.geocodeJSON(peliasConfig, base), + postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON ]), reverse: createRouter([ sanitizers.reverse.middleware, middleware.calcSize(), - controllers.search(peliasConfig, undefined, reverseQuery), + controllers.search(peliasConfig.api, esclient, queries.reverse), postProc.distances('point.'), // reverse confidence scoring depends on distance from origin // so it must be calculated first @@ -146,13 +147,13 @@ function addRoutes(app, peliasConfig) { postProc.parseBoundingBox(), postProc.normalizeParentIds(), postProc.assignLabels(), - postProc.geocodeJSON(peliasConfig, base), + postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON ]), nearby: createRouter([ sanitizers.nearby.middleware, middleware.calcSize(), - controllers.search(peliasConfig, undefined, reverseQuery), + controllers.search(peliasConfig.api, esclient, queries.reverse), postProc.distances('point.'), // reverse confidence scoring depends on distance from origin // so it must be calculated first @@ -164,19 +165,19 @@ function addRoutes(app, peliasConfig) { postProc.parseBoundingBox(), postProc.normalizeParentIds(), postProc.assignLabels(), - postProc.geocodeJSON(peliasConfig, base), + postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON ]), place: createRouter([ sanitizers.place.middleware, - controllers.place(peliasConfig), + controllers.place(peliasConfig.api, esclient), postProc.accuracy(), postProc.localNamingConventions(), postProc.renamePlacenames(), postProc.parseBoundingBox(), postProc.normalizeParentIds(), postProc.assignLabels(), - postProc.geocodeJSON(peliasConfig, base), + postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON ]), status: createRouter([ diff --git a/sanitizer/search_fallback.js b/sanitizer/search_fallback.js index d483d4e3..9bdb0c1e 100644 --- a/sanitizer/search_fallback.js +++ b/sanitizer/search_fallback.js @@ -6,20 +6,21 @@ var sanitizeAll = require('../sanitizer/sanitizeAll'), var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; var logger = require('pelias-logger').get('api'); var logging = require( '../helper/logging' ); +var _ = require('lodash'); // middleware module.exports.middleware = function( req, res, next ){ // if res.data already has results then don't call the _text_autocomplete sanitizer // this has been put into place for when the libpostal integration way of querying // ES doesn't return anything and we want to fallback to the old logic - if (res && res.hasOwnProperty('data') && res.data.length > 0) { + if (_.get(res, 'data', []).length > 0) { return next(); } // log the query that caused a fallback since libpostal+new-queries didn't return anything if (req.path === '/v1/search') { - var queryText = logging.isDNT(req) ? '[text removed]' : req.clean.text; - logger.info(queryText); + const queryText = logging.isDNT(req) ? '[text removed]' : req.clean.text; + logger.info(`fallback queryText: ${queryText}`); } sanitize( req, function( err, clean ){ diff --git a/src/configValidation.js b/schema.js similarity index 80% rename from src/configValidation.js rename to schema.js index 621f883b..61e6c258 100644 --- a/src/configValidation.js +++ b/schema.js @@ -14,7 +14,7 @@ const Joi = require('joi'); // * api.relativeScores (boolean) // * api.legacyUrl (string) // * api.localization (flipNumberAndStreetCountries is array of 3 character strings) -const schema = Joi.object().keys({ +module.exports = Joi.object().keys({ api: Joi.object().keys({ version: Joi.string(), indexName: Joi.string(), @@ -22,6 +22,7 @@ const schema = Joi.object().keys({ legacyUrl: Joi.string(), accessLog: Joi.string(), relativeScores: Joi.boolean(), + requestRetries: Joi.number().integer().min(0), localization: Joi.object().keys({ flipNumberAndStreetCountries: Joi.array().items(Joi.string().regex(/^[A-Z]{3}$/)) }).unknown(false) @@ -31,14 +32,3 @@ const schema = Joi.object().keys({ requestTimeout: Joi.number().integer().min(0) }).unknown(true) }).requiredKeys('api', 'esclient').unknown(true); - -module.exports = { - validate: function validate(config) { - Joi.validate(config, schema, (err) => { - if (err) { - throw new Error(err.details[0].message); - } - }); - } - -}; diff --git a/service/mget.js b/service/mget.js index c709f28e..b68a5bed 100644 --- a/service/mget.js +++ b/service/mget.js @@ -13,26 +13,26 @@ var logger = require( 'pelias-logger' ).get( 'api' ); -function service( backend, query, cb ){ +function service( esclient, query, cb ){ - // backend command + // elasticsearch command var cmd = { body: { docs: query } }; - // query new backend - backend().client.mget( cmd, function( err, data ){ + // query elasticsearch + esclient.mget( cmd, function( err, data ){ // log total ms elasticsearch reported the query took to execute if( data && data.took ){ logger.verbose( 'time elasticsearch reported:', data.took / 1000 ); } - // handle backend errors + // handle elasticsearch errors if( err ){ - logger.error( 'backend error', err ); + logger.error( `elasticsearch error ${err}`); return cb( err ); } diff --git a/service/search.js b/service/search.js index 8e12b69e..9453aaa2 100644 --- a/service/search.js +++ b/service/search.js @@ -7,19 +7,19 @@ var logger = require( 'pelias-logger' ).get( 'api' ); -function service( backend, cmd, cb ){ +function service( esclient, cmd, cb ){ - // query new backend - backend().client.search( cmd, function( err, data ){ + // query elasticsearch + esclient.search( cmd, function( err, data ){ // log total ms elasticsearch reported the query took to execute if( data && data.took ){ logger.verbose( 'time elasticsearch reported:', data.took / 1000 ); } - // handle backend errors + // handle elasticsearch errors if( err ){ - logger.error( 'backend error', err ); + logger.error( `elasticsearch error ${err}` ); return cb( err ); } diff --git a/src/backend.js b/src/backend.js deleted file mode 100644 index c983dbba..00000000 --- a/src/backend.js +++ /dev/null @@ -1,14 +0,0 @@ -var config = require( 'pelias-config' ).generate().esclient; -var Backend = require('geopipes-elasticsearch-backend'), - client = require('elasticsearch').Client(config), - backends = {}; - -function getBackend( index, type ){ - var key = ( index + ':' + type ); - if( !backends[key] ){ - backends[key] = new Backend( client, index, type ); - } - return backends[key]; -} - -module.exports = getBackend; diff --git a/test/unit/app.js b/test/unit/app.js index 58ac8cdb..479e2af0 100644 --- a/test/unit/app.js +++ b/test/unit/app.js @@ -4,12 +4,16 @@ const proxyquire = require('proxyquire').noCallThru(); module.exports.tests = {}; -module.exports.tests.invalid_configuration = function(test, common) { - test('configuration validation throwing error should rethrow', function(t) { - t.throws(function() { +module.exports.tests.invalid_configuration = (test, common) => { + test('configuration validation throwing error should rethrow', (t) => { + t.throws(() => { proxyquire('../../app', { - './src/configValidation': { - validate: () => { + './schema': 'this is the schema', + 'pelias-config': { + generate: (schema) => { + // the schema passed to generate should be the require'd schema + t.equals(schema, 'this is the schema'); + throw Error('config is not valid'); } } @@ -23,7 +27,7 @@ module.exports.tests.invalid_configuration = function(test, common) { }; -module.exports.all = function (tape, common) { +module.exports.all = (tape, common) => { function test(name, testFunction) { return tape('app: ' + name, testFunction); diff --git a/test/unit/controller/place.js b/test/unit/controller/place.js index 0376eec6..22622e05 100644 --- a/test/unit/controller/place.js +++ b/test/unit/controller/place.js @@ -1,129 +1,372 @@ -var setup = require('../../../controller/place'), - mockBackend = require('../mock/backend'); +'use strict'; + +const setup = require('../../../controller/search'); +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) => { t.equal(typeof setup, 'function', 'setup is a function'); t.equal(typeof setup(), 'function', 'setup returns a controller'); t.end(); }); }; -// reminder: this is only the api subsection of the full config -var fakeDefaultConfig = { - indexName: 'pelias' +module.exports.tests.success = (test, common) => { + test('successful request to search service should set data and meta', (t) => { + const config = { + indexName: 'indexName value' + }; + const esclient = 'this is the esclient'; + + // request timeout messages willl be written here + const infoMesssages = []; + + // a controller that validates the esclient and cmd that was passed to the search service + const controller = proxyquire('../../../controller/place', { + '../service/mget': (esclient, query, callback) => { + t.equal(esclient, 'this is the esclient'); + t.deepEqual(query, [ + { + _index: 'indexName value', + _type: 'layer1', + _id: 'id1' + }, + { + _index: 'indexName value', + _type: 'layer2', + _id: 'id2' + } + ]); + + const docs = [{}, {}]; + + callback(undefined, docs); + } + })(config, esclient); + + const req = { + clean: { + ids: [ + { + id: 'id1', + layers: 'layer1' + }, + { + id: 'id2', + layers: 'layer2' + } + ] + }, + errors: [], + warnings: [] + }; + const res = {}; + + const next = () => { + t.deepEqual(req.errors, []); + t.deepEqual(req.warnings, []); + t.deepEquals(res.data, [{}, {}]); + t.end(); + }; + + controller(req, res, next); + + }); + }; -// functionally test controller (backend success) -module.exports.tests.functional_success = function(test, common) { - - // expected geojson features for 'client/place/ok/1' fixture - var expected = [{ - type: 'Feature', - geometry: { - type: 'Point', - coordinates: [ -50.5, 100.1 ] - }, - properties: { - id: 'myid1', - layer: 'mytype1', - text: 'test name1, city1, state1' - } - }, { - type: 'Feature', - geometry: { - type: 'Point', - coordinates: [ -51.5, 100.2 ] - }, - properties: { - id: 'myid2', - layer: 'mytype2', - text: 'test name2, city2, state2' - } - }]; - - test('functional success', 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('non-empty req.errors should ', (t) => { + const esclient = () => { + throw new Error('esclient should not have been called'); + }; + const controller = setup( {}, esclient ); + + // the existence of `errors` means that a sanitizer detected an error, + // so don't call the esclient + const req = { + errors: ['error'] + }; + const res = { }; + + t.doesNotThrow(() => { + controller(req, res, () => {}); }); - var controller = setup( fakeDefaultConfig, backend ); - var res = { - status: function( code ){ - t.equal(code, 200, 'status set'); - return res; + t.end(); + + }); + + test('mgetService returning error should add to req.errors and ignore docs', (t) => { + const config = { + indexName: 'indexName value' + }; + const esclient = 'this is the esclient'; + + // request timeout messages willl be written here + const infoMesssages = []; + + const nonTimeoutError = { + status: 500, + displayName: 'InternalServerError', + message: 'an internal server error occurred' + }; + + // a controller that validates the esclient and cmd that was passed to the search service + const controller = proxyquire('../../../controller/place', { + '../service/mget': (esclient, query, callback) => { + const docs = [{}, {}]; + + callback(nonTimeoutError, docs); + } + })(config, esclient); + + const req = { + clean: { + ids: [ + { + id: 'id1', + layers: 'layer1' + } + ] }, - json: function( json ){ - t.equal(typeof json, 'object', 'returns json'); - t.equal(typeof json.date, 'number', 'date set'); - t.equal(json.type, 'FeatureCollection', 'valid geojson'); - t.true(Array.isArray(json.features), 'features is array'); - t.deepEqual(json.features, expected, 'values correctly mapped'); + errors: [], + warnings: [] + }; + const res = {}; + + const next = () => { + t.deepEqual(req.errors, [nonTimeoutError.message]); + t.deepEqual(req.warnings, []); + t.deepEquals(res.data, undefined); + t.end(); + }; + + controller(req, res, next); + + }); + +}; + +module.exports.tests.timeout = function(test, common) { + test('default # of request timeout retries should be 3', (t) => { + const config = { + indexName: 'indexName value' + }; + const esclient = 'this is the esclient'; + + let searchServiceCallCount = 0; + + const timeoutError = { + status: 408, + displayName: 'RequestTimeout', + message: 'Request Timeout after 17ms' + }; + + // request timeout messages willl be written here + const infoMesssages = []; + + // a controller that validates the esclient and cmd that was passed to the search service + const controller = proxyquire('../../../controller/place', { + '../service/mget': (esclient, cmd, callback) => { + // not that the searchService got called + searchServiceCallCount++; + + callback(timeoutError); + }, + 'pelias-logger': { + get: (service) => { + t.equal(service, 'api'); + return { + info: (msg) => { + infoMesssages.push(msg); + }, + debug: () => {} + }; + } } + })(config, esclient); + + const req = { + clean: { + ids: [ + { + id: 'id1', + layers: 'layer1' + } + ] + }, + errors: [], + warnings: [] }; - var req = { clean: { ids: [ {'id' : 123, layers: [ 'a' ] } ] }, errors: [], warnings: [] }; - var next = function next() { - t.equal(req.errors.length, 0, 'next was called without error'); + const res = {}; + + const next = () => { + t.equal(searchServiceCallCount, 3+1); + + t.ok(infoMesssages.indexOf('request timed out on attempt 1, retrying') !== -1); + t.ok(infoMesssages.indexOf('request timed out on attempt 2, retrying') !== -1); + t.ok(infoMesssages.indexOf('request timed out on attempt 3, retrying') !== -1); + + t.deepEqual(req.errors, [timeoutError.message]); + t.deepEqual(res, {}); t.end(); }; - controller(req, res, next ); + + controller(req, res, next); + }); - test('functional success with custom index name', function(t) { - var fakeCustomizedConfig = { - indexName: 'alternateindexname' + test('explicit apiConfig.requestRetries should retry that many times', (t) => { + const config = { + indexName: 'indexName value', + requestRetries: 17 }; + const esclient = 'this is the esclient'; - var backend = mockBackend( 'client/mget/ok/1', function( cmd ){ - t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'alternateindexname', _type: [ 'a' ] } ] } }, 'correct backend command'); - }); - var controller = setup( fakeCustomizedConfig, backend ); - var res = { - status: function( code ){ - t.equal(code, 200, 'status set'); - return res; + let searchServiceCallCount = 0; + + const timeoutError = { + status: 408, + displayName: 'RequestTimeout', + message: 'Request Timeout after 17ms' + }; + + // a controller that validates the esclient and cmd that was passed to the search service + const controller = proxyquire('../../../controller/place', { + '../service/mget': (esclient, cmd, callback) => { + // not that the searchService got called + searchServiceCallCount++; + + callback(timeoutError); + } + })(config, esclient); + + const req = { + clean: { + ids: [ + { + id: 'id1', + layers: 'layer1' + } + ] }, - json: function( json ){ - t.equal(typeof json, 'object', 'returns json'); - t.equal(typeof json.date, 'number', 'date set'); - t.equal(json.type, 'FeatureCollection', 'valid geojson'); - t.true(Array.isArray(json.features), 'features is array'); - t.deepEqual(json.features, expected, 'values correctly mapped'); + errors: [], + warnings: [] + }; + const res = {}; + + const next = () => { + t.equal(searchServiceCallCount, 17+1); + t.end(); + }; + + controller(req, res, next); + + }); + + test('only status code 408 should be considered a retryable request', (t) => { + const config = { + indexName: 'indexName value' + }; + const esclient = 'this is the esclient'; + + let searchServiceCallCount = 0; + + const nonTimeoutError = { + status: 500, + displayName: 'InternalServerError', + message: 'an internal server error occurred' + }; + + // a controller that validates the esclient and cmd that was passed to the search service + const controller = proxyquire('../../../controller/place', { + '../service/mget': (esclient, cmd, callback) => { + // not that the searchService got called + searchServiceCallCount++; + + callback(nonTimeoutError); } + })(config, esclient); + + const req = { + clean: { + ids: [ + { + id: 'id1', + layers: 'layer1' + } + ] + }, + errors: [], + warnings: [] }; - var req = { clean: { ids: [ {'id' : 123, layers: [ 'a' ] } ] }, errors: [], warnings: [] }; - var next = function next() { - t.equal(req.errors.length, 0, 'next was called without error'); + const res = {}; + + const next = () => { + t.equal(searchServiceCallCount, 1); + t.deepEqual(req.errors, [nonTimeoutError.message]); t.end(); }; - controller(req, res, next ); + + controller(req, res, next); + }); -}; -// functionally test controller (backend failure) -module.exports.tests.functional_failure = function(test, common) { - test('functional failure', function(t) { - var backend = mockBackend( 'client/mget/fail/1', function( cmd ){ - t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: ['b'] } ] } }, 'correct backend command'); - }); - var controller = setup( fakeDefaultConfig, backend ); - var req = { clean: { ids: [ {'id' : 123, layers: [ 'b' ] } ] }, errors: [], warnings: [] }; - var next = function( message ){ - t.equal(req.errors[0],'a backend error occurred','error passed to errorHandler'); + test('string error should not retry and be logged as-is', (t) => { + const config = { + indexName: 'indexName value' + }; + const esclient = 'this is the esclient'; + + let searchServiceCallCount = 0; + + const stringTypeError = 'this is an error string'; + + // a controller that validates the esclient and cmd that was passed to the search service + const controller = proxyquire('../../../controller/place', { + '../service/mget': (esclient, cmd, callback) => { + // not that the searchService got called + searchServiceCallCount++; + + callback(stringTypeError); + } + })(config, esclient); + + const req = { + clean: { + ids: [ + { + id: 'id1', + layers: 'layer1' + } + ] + }, + errors: [], + warnings: [] + }; + const res = {}; + + const next = () => { + t.equal(searchServiceCallCount, 1); + t.deepEqual(req.errors, [stringTypeError]); t.end(); }; - controller(req, undefined, next ); + + controller(req, res, next); + }); + }; -module.exports.all = function (tape, common) { +module.exports.all = (tape, common) => { function test(name, testFunction) { return tape('GET /place ' + name, testFunction); } - for( var testCase in module.exports.tests ){ + for( const testCase in module.exports.tests ){ module.exports.tests[testCase](test, common); } }; diff --git a/test/unit/controller/search.js b/test/unit/controller/search.js index 86f07b97..78e0b2d2 100644 --- a/test/unit/controller/search.js +++ b/test/unit/controller/search.js @@ -1,7 +1,7 @@ -var setup = require('../../../controller/search'), - mockBackend = require('../mock/backend'), - mockQuery = require('../mock/query'); -var proxyquire = require('proxyquire').noCallThru(); +'use strict'; + +const setup = require('../../../controller/search'); +const proxyquire = require('proxyquire').noCallThru(); module.exports.tests = {}; @@ -13,179 +13,529 @@ module.exports.tests.interface = function(test, common) { }); }; -// reminder: this is only the api subsection of the full config -var fakeDefaultConfig = { - indexName: 'pelias' -}; +module.exports.tests.success = function(test, common) { + test('successful request to search service should set data and meta', (t) => { + const config = { + indexName: 'indexName value' + }; + const esclient = 'this is the esclient'; + const query = () => { + return { + body: 'this is the query body', + type: 'this is the query type' + }; + }; + + // request timeout messages willl be written here + const infoMesssages = []; -// functionally test controller (backend success) -module.exports.tests.functional_success = function(test, common) { - - // expected geojson features for 'client/suggest/ok/1' fixture - var expected = [{ - type: 'Feature', - geometry: { - type: 'Point', - coordinates: [-50.5, 100.1] - }, - properties: { - id: 'myid1', - layer: 'mytype1', - text: 'test name1, city1, state1' - } - }, { - type: 'Feature', - geometry: { - type: 'Point', - coordinates: [-51.5, 100.2] - }, - properties: { - id: 'myid2', - layer: 'mytype2', - text: 'test name2, city2, state2' - } - }]; - - var expectedMeta = { - scores: [10, 20], - query_type: 'mock' - }; - - var expectedData = [ - { - _id: 'myid1', - _score: 10, - _type: 'mytype1', - _matched_queries: ['query 1', 'query 2'], - parent: { - country: ['country1'], - region: ['state1'], - county: ['city1'] + // a controller that validates the esclient and cmd that was passed to the search service + const controller = proxyquire('../../../controller/search', { + '../service/search': (esclient, cmd, callback) => { + t.equal(esclient, 'this is the esclient'); + t.deepEqual(cmd, { + index: 'indexName value', + searchType: 'dfs_query_then_fetch', + body: 'this is the query body' + }); + + const docs = [{}, {}]; + const meta = { key: 'value' }; + + callback(undefined, docs, meta); }, - center_point: { lat: 100.1, lon: -50.5 }, - name: { default: 'test name1' }, - value: 1 - }, - { - _id: 'myid2', - _score: 20, - _type: 'mytype2', - _matched_queries: ['query 3'], - parent: { - country: ['country2'], - region: ['state2'], - county: ['city2'] + 'pelias-logger': { + get: (service) => { + t.equal(service, 'api'); + return { + info: (msg) => { + infoMesssages.push(msg); + }, + debug: () => {} + }; + } + } + })(config, esclient, query); + + const req = { clean: { }, errors: [], warnings: [] }; + const res = {}; + + const next = () => { + t.deepEqual(req, { + clean: {}, + errors: [], + warnings: [] + }); + t.deepEquals(res.data, [{}, {}]); + t.deepEquals(res.meta, { key: 'value', query_type: 'this is the query type' }); + + t.ok(infoMesssages.find((msg) => { + return msg === '[controller:search] [queryType:this is the query type] [es_result_count:2]'; + })); + t.end(); + }; + + controller(req, res, next); + + }); + + test('undefined meta should set empty object into res', (t) => { + const config = { + indexName: 'indexName value' + }; + const esclient = 'this is the esclient'; + const query = () => { + return { + body: 'this is the query body', + type: 'this is the query type' + }; + }; + + // request timeout messages willl be written here + const infoMesssages = []; + + // a controller that validates the esclient and cmd that was passed to the search service + const controller = proxyquire('../../../controller/search', { + '../service/search': (esclient, cmd, callback) => { + t.equal(esclient, 'this is the esclient'); + t.deepEqual(cmd, { + index: 'indexName value', + searchType: 'dfs_query_then_fetch', + body: 'this is the query body' + }); + + const docs = [{}, {}]; + + callback(undefined, docs, undefined); }, - center_point: { lat: 100.2, lon: -51.5 }, - name: { default: 'test name2' }, - value: 2 - } - ]; - - test('functional success', function (t) { - var backend = mockBackend('client/search/ok/1', function (cmd) { - t.deepEqual(cmd, { - body: {a: 'b'}, - index: 'pelias', - searchType: 'dfs_query_then_fetch' - }, 'correct backend command'); - }); - var controller = setup(fakeDefaultConfig, backend, mockQuery()); - var res = { - status: function (code) { - t.equal(code, 200, 'status set'); - return res; + 'pelias-logger': { + get: (service) => { + t.equal(service, 'api'); + return { + info: (msg) => { + infoMesssages.push(msg); + }, + debug: () => {} + }; + } + } + })(config, esclient, query); + + const req = { clean: { }, errors: [], warnings: [] }; + const res = {}; + + const next = () => { + t.deepEqual(req, { + clean: {}, + errors: [], + warnings: [] + }); + t.deepEquals(res.data, [{}, {}]); + t.deepEquals(res.meta, { query_type: 'this is the query type' }); + + t.ok(infoMesssages.find((msg) => { + return msg === '[controller:search] [queryType:this is the query type] [es_result_count:2]'; + })); + t.end(); + }; + + controller(req, res, next); + + }); + + test('undefined docs should log 0 results', (t) => { + const config = { + indexName: 'indexName value' + }; + const esclient = 'this is the esclient'; + const query = () => { + return { + body: 'this is the query body', + type: 'this is the query type' + }; + }; + + // request timeout messages willl be written here + const infoMesssages = []; + + // a controller that validates the esclient and cmd that was passed to the search service + const controller = proxyquire('../../../controller/search', { + '../service/search': (esclient, cmd, callback) => { + t.equal(esclient, 'this is the esclient'); + t.deepEqual(cmd, { + index: 'indexName value', + searchType: 'dfs_query_then_fetch', + body: 'this is the query body' + }); + + const meta = { key: 'value' }; + + callback(undefined, undefined, meta); }, - json: function (json) { - t.equal(typeof json, 'object', 'returns json'); - t.equal(typeof json.date, 'number', 'date set'); - t.equal(json.type, 'FeatureCollection', 'valid geojson'); - t.true(Array.isArray(json.features), 'features is array'); - t.deepEqual(json.features, expected, 'values correctly mapped'); + 'pelias-logger': { + get: (service) => { + t.equal(service, 'api'); + return { + info: (msg) => { + infoMesssages.push(msg); + }, + debug: () => {} + }; + } } + })(config, esclient, query); + + const req = { clean: { }, errors: [], warnings: [] }; + const res = {}; + + const next = () => { + t.deepEqual(req, { + clean: {}, + errors: [], + warnings: [] + }); + t.equals(res.data, undefined); + t.deepEquals(res.meta, { key: 'value', query_type: 'this is the query type' }); + + t.ok(infoMesssages.find((msg) => { + return msg === '[controller:search] [queryType:this is the query type] [es_result_count:0]'; + })); + t.end(); }; - var req = { clean: { a: 'b' }, errors: [], warnings: [] }; - var next = function next() { - t.equal(req.errors.length, 0, 'next was called without error'); - t.deepEqual(res.meta, expectedMeta, 'meta data was set'); - t.deepEqual(res.data, expectedData, 'data was set'); + + controller(req, res, next); + + }); + + test('successful request on retry to search service should log info message', (t) => { + const config = { + indexName: 'indexName value' + }; + const esclient = 'this is the esclient'; + const query = () => { + return { + body: 'this is the query body', + type: 'this is the query type' + }; + }; + + let searchServiceCallCount = 0; + + const timeoutError = { + status: 408, + displayName: 'RequestTimeout', + message: 'Request Timeout after 17ms' + }; + + // request timeout messages willl be written here + const infoMesssages = []; + + // a controller that validates the esclient and cmd that was passed to the search service + const controller = proxyquire('../../../controller/search', { + '../service/search': (esclient, cmd, callback) => { + t.equal(esclient, 'this is the esclient'); + t.deepEqual(cmd, { + index: 'indexName value', + searchType: 'dfs_query_then_fetch', + body: 'this is the query body' + }); + + if (searchServiceCallCount < 2) { + // note that the searchService got called + searchServiceCallCount++; + callback(timeoutError); + } else { + const docs = [{}, {}]; + const meta = { key: 'value' }; + + callback(undefined, docs, meta); + } + + }, + 'pelias-logger': { + get: (service) => { + t.equal(service, 'api'); + return { + info: (msg) => { + infoMesssages.push(msg); + }, + debug: () => {} + }; + } + } + })(config, esclient, query); + + const req = { clean: { }, errors: [], warnings: [] }; + const res = {}; + + const next = () => { + t.deepEqual(req, { + clean: {}, + errors: [], + warnings: [] + }); + t.deepEquals(res.data, [{}, {}]); + t.deepEquals(res.meta, { key: 'value', query_type: 'this is the query type' }); + + t.ok(infoMesssages.find((msg) => { + return msg === '[controller:search] [queryType:this is the query type] [es_result_count:2]'; + })); + + t.ok(infoMesssages.find((msg) => { + return msg === 'succeeded on retry 2'; + })); + t.end(); }; + controller(req, res, next); + }); - test('functional success with alternate index name', function(t) { - var fakeCustomizedConfig = { - indexName: 'alternateindexname' +}; + +module.exports.tests.timeout = function(test, common) { + test('default # of request timeout retries should be 3', (t) => { + const config = { + indexName: 'indexName value' + }; + const esclient = 'this is the esclient'; + const query = () => { + return { body: 'this is the query body' }; }; - var backend = mockBackend('client/search/ok/1', function (cmd) { - t.deepEqual(cmd, { - body: {a: 'b'}, - index: 'alternateindexname', - searchType: 'dfs_query_then_fetch' - }, 'correct backend command'); - }); - var controller = setup(fakeCustomizedConfig, backend, mockQuery()); - var res = { - status: function (code) { - t.equal(code, 200, 'status set'); - return res; + let searchServiceCallCount = 0; + + const timeoutError = { + status: 408, + displayName: 'RequestTimeout', + message: 'Request Timeout after 17ms' + }; + + // request timeout messages willl be written here + const infoMesssages = []; + + // a controller that validates the esclient and cmd that was passed to the search service + const controller = proxyquire('../../../controller/search', { + '../service/search': (esclient, cmd, callback) => { + t.equal(esclient, 'this is the esclient'); + t.deepEqual(cmd, { + index: 'indexName value', + searchType: 'dfs_query_then_fetch', + body: 'this is the query body' + }); + + // not that the searchService got called + searchServiceCallCount++; + + callback(timeoutError); + }, + 'pelias-logger': { + get: (service) => { + t.equal(service, 'api'); + return { + info: (msg) => { + infoMesssages.push(msg); + }, + debug: () => {} + }; + } } + })(config, esclient, query); + + const req = { clean: { }, errors: [], warnings: [] }; + const res = {}; + + const next = () => { + t.equal(searchServiceCallCount, 3+1); + + t.ok(infoMesssages.indexOf('request timed out on attempt 1, retrying') !== -1); + t.ok(infoMesssages.indexOf('request timed out on attempt 2, retrying') !== -1); + t.ok(infoMesssages.indexOf('request timed out on attempt 3, retrying') !== -1); + + t.deepEqual(req, { + clean: {}, + errors: [timeoutError.message], + warnings: [] + }); + t.deepEqual(res, {}); + t.end(); + }; + + controller(req, res, next); + + }); + + test('explicit apiConfig.requestRetries should retry that many times', (t) => { + const config = { + indexName: 'indexName value', + requestRetries: 17 + }; + const esclient = 'this is the esclient'; + const query = () => { + return { }; }; - var req = { clean: { a: 'b' }, errors: [], warnings: [] }; - var next = function next() { - t.equal(req.errors.length, 0, 'next was called without error'); + + let searchServiceCallCount = 0; + + const timeoutError = { + status: 408, + displayName: 'RequestTimeout', + message: 'Request Timeout after 17ms' + }; + + // a controller that validates the esclient and cmd that was passed to the search service + const controller = proxyquire('../../../controller/search', { + '../service/search': (esclient, cmd, callback) => { + // not that the searchService got called + searchServiceCallCount++; + + callback(timeoutError); + } + })(config, esclient, query); + + const req = { clean: { }, errors: [], warnings: [] }; + const res = {}; + + const next = () => { + t.equal(searchServiceCallCount, 17+1); t.end(); }; + controller(req, res, next); + }); -}; -// functionally test controller (backend failure) -module.exports.tests.functional_failure = function(test, common) { - test('functional failure', function(t) { - var backend = mockBackend( 'client/search/fail/1', function( cmd ){ - t.deepEqual(cmd, { body: { a: 'b' }, index: 'pelias', searchType: 'dfs_query_then_fetch' }, 'correct backend command'); - }); - var controller = setup( fakeDefaultConfig, backend, mockQuery() ); - var req = { clean: { a: 'b' }, errors: [], warnings: [] }; - var next = function(){ - t.equal(req.errors[0],'a backend error occurred'); + test('only status code 408 should be considered a retryable request', (t) => { + const config = { + indexName: 'indexName value', + requestRetries: 17 + }; + const esclient = 'this is the esclient'; + const query = () => { + return { }; + }; + + let searchServiceCallCount = 0; + + const nonTimeoutError = { + status: 500, + displayName: 'InternalServerError', + message: 'an internal server error occurred' + }; + + // a controller that validates the esclient and cmd that was passed to the search service + const controller = proxyquire('../../../controller/search', { + '../service/search': (esclient, cmd, callback) => { + // not that the searchService got called + searchServiceCallCount++; + + callback(nonTimeoutError); + } + })(config, esclient, query); + + const req = { clean: { }, errors: [], warnings: [] }; + const res = {}; + + const next = () => { + t.equal(searchServiceCallCount, 1); + t.deepEqual(req, { + clean: {}, + errors: [nonTimeoutError.message], + warnings: [] + }); t.end(); }; - controller(req, undefined, next ); + + controller(req, res, next); + }); -}; -module.exports.tests.timeout = function(test, common) { - test('timeout', function(t) { - var backend = mockBackend( 'client/search/timeout/1', function( cmd ){ - t.deepEqual(cmd, { body: { a: 'b' }, index: 'pelias', searchType: 'dfs_query_then_fetch' }, 'correct backend command'); - }); - var controller = setup( fakeDefaultConfig, backend, mockQuery() ); - var req = { clean: { a: 'b' }, errors: [], warnings: [] }; - var next = function(){ - t.equal(req.errors[0],'Request Timeout after 5000ms'); + test('string error should not retry and be logged as-is', (t) => { + const config = { + indexName: 'indexName value', + requestRetries: 17 + }; + const esclient = 'this is the esclient'; + const query = () => { + return { }; + }; + + let searchServiceCallCount = 0; + + const stringTypeError = 'this is an error string'; + + // a controller that validates the esclient and cmd that was passed to the search service + const controller = proxyquire('../../../controller/search', { + '../service/search': (esclient, cmd, callback) => { + // not that the searchService got called + searchServiceCallCount++; + + callback(stringTypeError); + } + })(config, esclient, query); + + const req = { clean: { }, errors: [], warnings: [] }; + const res = {}; + + const next = () => { + t.equal(searchServiceCallCount, 1); + t.deepEqual(req, { + clean: {}, + errors: [stringTypeError], + warnings: [] + }); t.end(); }; - controller(req, undefined, next ); + + controller(req, res, next); + + }); + +}; + +module.exports.tests.existing_errors = function(test, common) { + test('req with errors should not call esclient or query', function(t) { + const esclient = () => { + throw new Error('esclient should not have been called'); + }; + const query = () => { + throw new Error('query should not have been called'); + }; + const controller = setup( {}, esclient, query ); + + // the existence of `errors` means that a sanitizer detected an error, + // so don't call the esclient + const req = { + errors: ['error'] + }; + const res = { }; + + t.doesNotThrow(() => { + controller(req, res, () => {}); + }); + t.end(); + }); + }; module.exports.tests.existing_results = function(test, common) { - test('res with existing data should not call backend', function(t) { - var backend = function() { - throw new Error('backend should not have been called'); + test('res with existing data should not call esclient or query', function(t) { + const esclient = () => { + throw new Error('esclient should not have been called'); }; - var controller = setup( fakeDefaultConfig, backend, mockQuery() ); + const query = () => { + throw new Error('query should not have been called'); + }; + const controller = setup( {}, esclient, query ); - var req = { }; + const req = { }; // the existence of `data` means that there are already results so - // don't call the backend/query - var res = { data: [{}] }; + // don't call esclient or query + const res = { data: [{}] }; - var next = function() { + const next = function() { t.deepEqual(res, {data: [{}]}); t.end(); }; @@ -198,18 +548,20 @@ module.exports.tests.existing_results = function(test, common) { module.exports.tests.undefined_query = function(test, common) { test('query returning undefined should not call service', function(t) { // a function that returns undefined - var query = function () { return; }; + const query = () => { + return undefined; + }; - var search_service_was_called = false; + let search_service_was_called = false; - var controller = proxyquire('../../../controller/search', { + const controller = proxyquire('../../../controller/search', { '../service/search': function() { search_service_was_called = true; throw new Error('search service should not have been called'); } })(undefined, undefined, query); - var next = function() { + const next = () => { t.notOk(search_service_was_called, 'should have returned before search service was called'); t.end(); }; @@ -225,7 +577,7 @@ module.exports.all = function (tape, common) { return tape('GET /search ' + name, testFunction); } - for( var testCase in module.exports.tests ){ + for( const testCase in module.exports.tests ){ module.exports.tests[testCase](test, common); } }; diff --git a/test/unit/middleware/parseBBox.js b/test/unit/middleware/parseBBox.js index 4e2cd802..78186805 100644 --- a/test/unit/middleware/parseBBox.js +++ b/test/unit/middleware/parseBBox.js @@ -1,5 +1,7 @@ var parseBBox = require('../../../middleware/parseBBox')(); +const proxyquire = require('proxyquire').noCallThru(); + module.exports.tests = {}; module.exports.tests.computeDistance = function(test, common) { @@ -46,6 +48,20 @@ module.exports.tests.computeDistance = function(test, common) { ] }; + const parseBBox = proxyquire('../../../middleware/parseBBox', { + 'pelias-logger': { + get: () => { + return { + error: (msg1, msg2) => { + t.equals(msg1, 'Invalid bounding_box json string:'); + t.deepEquals(msg2, { bounding_box: 'garbage json' }); + } + }; + + } + } + })(); + parseBBox({}, res, function () { t.deepEquals(res, expected, 'correct bounding_box'); t.end(); diff --git a/test/unit/mock/alpha3.json b/test/unit/mock/alpha3.json deleted file mode 100644 index 7148e878..00000000 --- a/test/unit/mock/alpha3.json +++ /dev/null @@ -1,251 +0,0 @@ -{ - "ABW": "Aruba", - "AFG": "Afghanistan", - "AGO": "Angola", - "AIA": "Anguilla", - "ALA": "Åland Islands", - "ALB": "Albania", - "AND": "Andorra", - "ARE": "United Arab Emirates", - "ARG": "Argentina", - "ARM": "Armenia", - "ASM": "American Samoa", - "ATA": "Antarctica", - "ATF": "French Southern Territories", - "ATG": "Antigua and Barbuda", - "AUS": "Australia", - "AUT": "Austria", - "AZE": "Azerbaijan", - "BDI": "Burundi", - "BEL": "Belgium", - "BEN": "Benin", - "BES": "Bonaire, Sint Eustatius and Saba", - "BFA": "Burkina Faso", - "BGD": "Bangladesh", - "BGR": "Bulgaria", - "BHR": "Bahrain", - "BHS": "Bahamas", - "BIH": "Bosnia and Herzegovina", - "BLM": "Saint Barthélemy", - "BLR": "Belarus", - "BLZ": "Belize", - "BMU": "Bermuda", - "BOL": "Bolivia, Plurinational State of", - "BRA": "Brazil", - "BRB": "Barbados", - "BRN": "Brunei Darussalam", - "BTN": "Bhutan", - "BVT": "Bouvet Island", - "BWA": "Botswana", - "CAF": "Central African Republic", - "CAN": "Canada", - "CCK": "Cocos (Keeling) Islands", - "CHE": "Switzerland", - "CHL": "Chile", - "CHN": "China", - "CIV": "Côte d'Ivoire", - "CMR": "Cameroon", - "COD": "Congo, the Democratic Republic of the", - "COG": "Congo", - "COK": "Cook Islands", - "COL": "Colombia", - "COM": "Comoros", - "CPV": "Cabo Verde", - "CRI": "Costa Rica", - "CUB": "Cuba", - "CUW": "Curaçao", - "CXR": "Christmas Island", - "CYM": "Cayman Islands", - "CYP": "Cyprus", - "CZE": "Czech Republic", - "DEU": "Germany", - "DJI": "Djibouti", - "DMA": "Dominica", - "DNK": "Denmark", - "DOM": "Dominican Republic", - "DZA": "Algeria", - "ECU": "Ecuador", - "EGY": "Egypt", - "ERI": "Eritrea", - "ESH": "Western Sahara", - "ESP": "Spain", - "EST": "Estonia", - "ETH": "Ethiopia", - "FIN": "Finland", - "FJI": "Fiji", - "FLK": "Falkland Islands (Malvinas)", - "FRA": "France", - "FRO": "Faroe Islands", - "FSM": "Micronesia, Federated States of", - "GAB": "Gabon", - "GBR": "United Kingdom", - "GEO": "Georgia", - "GGY": "Guernsey", - "GHA": "Ghana", - "GIB": "Gibraltar", - "GIN": "Guinea", - "GLP": "Guadeloupe", - "GMB": "Gambia", - "GNB": "Guinea-Bissau", - "GNQ": "Equatorial Guinea", - "GRC": "Greece", - "GRD": "Grenada", - "GRL": "Greenland", - "GTM": "Guatemala", - "GUF": "French Guiana", - "GUM": "Guam", - "GUY": "Guyana", - "HKG": "Hong Kong", - "HMD": "Heard Island and McDonald Islands", - "HND": "Honduras", - "HRV": "Croatia", - "HTI": "Haiti", - "HUN": "Hungary", - "IDN": "Indonesia", - "IMN": "Isle of Man", - "IND": "India", - "IOT": "British Indian Ocean Territory", - "IRL": "Ireland", - "IRN": "Iran, Islamic Republic of", - "IRQ": "Iraq", - "ISL": "Iceland", - "ISR": "Israel", - "ITA": "Italy", - "JAM": "Jamaica", - "JEY": "Jersey", - "JOR": "Jordan", - "JPN": "Japan", - "KAZ": "Kazakhstan", - "KEN": "Kenya", - "KGZ": "Kyrgyzstan", - "KHM": "Cambodia", - "KIR": "Kiribati", - "KNA": "Saint Kitts and Nevis", - "KOR": "Korea, Republic of", - "KWT": "Kuwait", - "LAO": "Lao People's Democratic Republic", - "LBN": "Lebanon", - "LBR": "Liberia", - "LBY": "Libya", - "LCA": "Saint Lucia", - "LIE": "Liechtenstein", - "LKA": "Sri Lanka", - "LSO": "Lesotho", - "LTU": "Lithuania", - "LUX": "Luxembourg", - "LVA": "Latvia", - "MAC": "Macao", - "MAF": "Saint Martin (French part)", - "MAR": "Morocco", - "MCO": "Monaco", - "MDA": "Moldova, Republic of", - "MDG": "Madagascar", - "MDV": "Maldives", - "MEX": "Mexico", - "MHL": "Marshall Islands", - "MKD": "Macedonia, the former Yugoslav Republic of", - "MLI": "Mali", - "MLT": "Malta", - "MMR": "Myanmar", - "MNE": "Montenegro", - "MNG": "Mongolia", - "MNP": "Northern Mariana Islands", - "MOZ": "Mozambique", - "MRT": "Mauritania", - "MSR": "Montserrat", - "MTQ": "Martinique", - "MUS": "Mauritius", - "MWI": "Malawi", - "MYS": "Malaysia", - "MYT": "Mayotte", - "NAM": "Namibia", - "NCL": "New Caledonia", - "NER": "Niger", - "NFK": "Norfolk Island", - "NGA": "Nigeria", - "NIC": "Nicaragua", - "NIU": "Niue", - "NLD": "Netherlands", - "NOR": "Norway", - "NPL": "Nepal", - "NRU": "Nauru", - "NZL": "New Zealand", - "OMN": "Oman", - "PAK": "Pakistan", - "PAN": "Panama", - "PCN": "Pitcairn", - "PER": "Peru", - "PHL": "Philippines", - "PLW": "Palau", - "PNG": "Papua New Guinea", - "POL": "Poland", - "PRI": "Puerto Rico", - "PRK": "Korea, Democratic People's Republic of", - "PRT": "Portugal", - "PRY": "Paraguay", - "PSE": "Palestine, State of", - "PYF": "French Polynesia", - "QAT": "Qatar", - "REU": "Réunion", - "ROU": "Romania", - "RUS": "Russian Federation", - "RWA": "Rwanda", - "SAU": "Saudi Arabia", - "SDN": "Sudan", - "SEN": "Senegal", - "SGP": "Singapore", - "SGS": "South Georgia and the South Sandwich Islands", - "SHN": "Saint Helena, Ascension and Tristan da Cunha", - "SJM": "Svalbard and Jan Mayen", - "SLB": "Solomon Islands", - "SLE": "Sierra Leone", - "SLV": "El Salvador", - "SMR": "San Marino", - "SOM": "Somalia", - "SPM": "Saint Pierre and Miquelon", - "SRB": "Serbia", - "SSD": "South Sudan", - "STP": "Sao Tome and Principe", - "SUR": "Suriname", - "SVK": "Slovakia", - "SVN": "Slovenia", - "SWE": "Sweden", - "SWZ": "Swaziland", - "SXM": "Sint Maarten (Dutch part)", - "SYC": "Seychelles", - "SYR": "Syrian Arab Republic", - "TCA": "Turks and Caicos Islands", - "TCD": "Chad", - "TGO": "Togo", - "THA": "Thailand", - "TJK": "Tajikistan", - "TKL": "Tokelau", - "TKM": "Turkmenistan", - "TLS": "Timor-Leste", - "TON": "Tonga", - "TTO": "Trinidad and Tobago", - "TUN": "Tunisia", - "TUR": "Turkey", - "TUV": "Tuvalu", - "TWN": "Taiwan, Province of China", - "TZA": "Tanzania, United Republic of", - "UGA": "Uganda", - "UKR": "Ukraine", - "UMI": "United States Minor Outlying Islands", - "URY": "Uruguay", - "USA": "United States", - "UZB": "Uzbekistan", - "VAT": "Holy See (Vatican City State)", - "VCT": "Saint Vincent and the Grenadines", - "VEN": "Venezuela, Bolivarian Republic of", - "VGB": "Virgin Islands, British", - "VIR": "Virgin Islands, U.S.", - "VNM": "Viet Nam", - "VUT": "Vanuatu", - "WLF": "Wallis and Futuna", - "WSM": "Samoa", - "YEM": "Yemen", - "ZAF": "South Africa", - "ZMB": "Zambia", - "ZWE": "Zimbabwe" - } \ No newline at end of file diff --git a/test/unit/mock/backend.js b/test/unit/mock/backend.js index 739ed2cb..4751acc0 100644 --- a/test/unit/mock/backend.js +++ b/test/unit/mock/backend.js @@ -1,11 +1,5 @@ var responses = {}; -responses['client/suggest/ok/1'] = function( cmd, cb ){ - return cb( undefined, suggestEnvelope([ { score: 1, text: 'mocktype:mockid1' } ], [ { score: 2, text: 'mocktype:mockid2' } ]) ); -}; -responses['client/suggest/fail/1'] = function( cmd, cb ){ - return cb( 'a backend error occurred' ); -}; responses['client/search/ok/1'] = function( cmd, cb ){ return cb( undefined, searchEnvelope([{ _id: 'myid1', @@ -32,15 +26,7 @@ responses['client/search/ok/1'] = function( cmd, cb ){ }])); }; responses['client/search/fail/1'] = function( cmd, cb ){ - return cb( 'a backend error occurred' ); -}; - -responses['client/search/timeout/1'] = function( cmd, cb) { - // timeout errors are objects - return cb({ - status: 408, - message: 'Request Timeout after 5000ms' - }); + return cb( 'an elasticsearch error occurred' ); }; responses['client/mget/ok/1'] = function( cmd, cb ){ @@ -73,23 +59,21 @@ responses['client/mget/fail/1'] = responses['client/search/fail/1']; function setup( key, cmdCb ){ function backend( a, b ){ return { - client: { - 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 ); - } + 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; + return backend(); } function mgetEnvelope( options ){ diff --git a/test/unit/mock/query.js b/test/unit/mock/query.js deleted file mode 100644 index a3209a4d..00000000 --- a/test/unit/mock/query.js +++ /dev/null @@ -1,13 +0,0 @@ - -function setup(){ - return query; -} - -function query( clean ){ - return { - type: 'mock', - body: clean - }; -} - -module.exports = setup; \ No newline at end of file diff --git a/test/unit/run.js b/test/unit/run.js index c92a45f0..71b14e27 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -10,6 +10,7 @@ var common = { var tests = [ require('./app'), + require('./schema'), require('./controller/index'), require('./controller/place'), require('./controller/search'), @@ -62,8 +63,6 @@ var tests = [ require('./sanitizer/_deprecate_quattroshapes'), require('./sanitizer/_categories'), require('./sanitizer/nearby'), - require('./src/backend'), - require('./src/configValidation'), require('./sanitizer/autocomplete'), require('./sanitizer/structured_geocoding'), require('./sanitizer/place'), diff --git a/test/unit/sanitizer/search_fallback.js b/test/unit/sanitizer/search_fallback.js index 82bad1b0..69e35fcd 100644 --- a/test/unit/sanitizer/search_fallback.js +++ b/test/unit/sanitizer/search_fallback.js @@ -107,6 +107,70 @@ module.exports.tests.sanitize = function(test, common) { }); + test('req.clean.text should be logged when isDNT=false', (t) => { + const infoLog = []; + + const search = proxyquire('../../../sanitizer/search_fallback', { + 'pelias-logger': { + get: () => { + return { + info: (msg) => { + infoLog.push(msg); + } + }; + } + }, + '../helper/logging': { + isDNT: () => { return false; } + } + }); + + const req = { + path: '/v1/search', + clean: { + text: 'this is the query text' + } + }; + + search.middleware(req, undefined, () => { + t.deepEquals(infoLog, [`fallback queryText: ${req.clean.text}`]); + t.end(); + }); + + }); + + test('req.clean.text should not be logged when isDNT=true', (t) => { + const infoLog = []; + + const search = proxyquire('../../../sanitizer/search_fallback', { + 'pelias-logger': { + get: () => { + return { + info: (msg) => { + infoLog.push(msg); + } + }; + } + }, + '../helper/logging': { + isDNT: () => { return true; } + } + }); + + const req = { + path: '/v1/search', + clean: { + text: 'this is the query text' + } + }; + + search.middleware(req, undefined, () => { + t.deepEquals(infoLog, ['fallback queryText: [text removed]']); + t.end(); + }); + + }); + }; module.exports.all = function (tape, common) { diff --git a/test/unit/src/configValidation.js b/test/unit/schema.js similarity index 56% rename from test/unit/src/configValidation.js rename to test/unit/schema.js index 91b34312..e1f9cfd2 100644 --- a/test/unit/src/configValidation.js +++ b/test/unit/schema.js @@ -1,11 +1,20 @@ 'use strict'; -const configValidation = require('../../../src/configValidation'); +const Joi = require('joi'); +const schema = require('../../schema'); + +function validate(config) { + Joi.validate(config, schema, (err, value) => { + if (err) { + throw new Error(err.details[0].message); + } + }); +} module.exports.tests = {}; -module.exports.tests.completely_valid = function(test, common) { - test('all valid configuration elements should not throw error', function(t) { +module.exports.tests.completely_valid = (test, common) => { + test('all valid configuration elements should not throw error', (t) => { var config = { api: { version: 'version value', @@ -16,22 +25,20 @@ module.exports.tests.completely_valid = function(test, common) { relativeScores: true, localization: { flipNumberAndStreetCountries: ['ABC', 'DEF'] - } + }, + requestRetries: 19 }, esclient: { requestTimeout: 17 } }; - t.doesNotThrow(function() { - configValidation.validate(config); - }); - + t.doesNotThrow(validate.bind(config)); t.end(); }); - test('basic valid configurtaion should not throw error', function(t) { + test('basic valid configuration should not throw error', (t) => { var config = { api: { version: 'version value', @@ -43,30 +50,25 @@ module.exports.tests.completely_valid = function(test, common) { } }; - t.doesNotThrow(function() { - configValidation.validate(config); - }); - + t.doesNotThrow(validate.bind(config)); t.end(); }); }; -module.exports.tests.api_validation = function(test, common) { - test('config without api should throw error', function(t) { +module.exports.tests.api_validation = (test, common) => { + test('config without api should throw error', (t) => { var config = { esclient: {} }; - t.throws(function() { - configValidation.validate(config); - }, /"api" is required/, 'api should exist'); + t.throws(validate.bind(null, config), /"api" is required/, 'api should exist'); t.end(); }); - test('config without unknown field in api should not throw error', function(t) { + test('config without unknown field in api should not throw error', (t) => { var config = { api: { version: 'version value', @@ -77,14 +79,12 @@ module.exports.tests.api_validation = function(test, common) { esclient: {} }; - t.doesNotThrow(function() { - configValidation.validate(config); - }, 'unknown properties should be allowed'); + t.doesNotThrow(validate.bind(null, config), 'unknown properties should be allowed'); t.end(); }); - test('non-string api.version should throw error', function(t) { + test('non-string api.version should throw error', (t) => { [null, 17, {}, [], true].forEach((value) => { var config = { api: { @@ -95,16 +95,15 @@ module.exports.tests.api_validation = function(test, common) { esclient: {} }; - t.throws(function() { - configValidation.validate(config); - }, /"version" must be a string/); + t.throws(validate.bind(null, config), /"version" must be a string/); + }); t.end(); }); - test('non-string api.indexName should throw error', function(t) { + test('non-string api.indexName should throw error', (t) => { [null, 17, {}, [], true].forEach((value) => { var config = { api: { @@ -115,16 +114,15 @@ module.exports.tests.api_validation = function(test, common) { esclient: {} }; - t.throws(function() { - configValidation.validate(config); - }, /"indexName" must be a string/); + t.throws(validate.bind(null, config), /"indexName" must be a string/); + }); t.end(); }); - test('non-string api.host should throw error', function(t) { + test('non-string api.host should throw error', (t) => { [null, 17, {}, [], true].forEach((value) => { var config = { api: { @@ -135,16 +133,15 @@ module.exports.tests.api_validation = function(test, common) { esclient: {} }; - t.throws(function() { - configValidation.validate(config); - }, /"host" must be a string/); + t.throws(validate.bind(null, config), /"host" must be a string/); + }); t.end(); }); - test('non-string api.legacyUrl should throw error', function(t) { + test('non-string api.legacyUrl should throw error', (t) => { [null, 17, {}, [], true].forEach((value) => { var config = { api: { @@ -156,16 +153,15 @@ module.exports.tests.api_validation = function(test, common) { esclient: {} }; - t.throws(function() { - configValidation.validate(config); - }, /"legacyUrl" must be a string/); + t.throws(validate.bind(null, config), /"legacyUrl" must be a string/); + }); t.end(); }); - test('non-string api.accessLog should throw error', function(t) { + test('non-string api.accessLog should throw error', (t) => { [null, 17, {}, [], true].forEach((value) => { var config = { api: { @@ -177,16 +173,15 @@ module.exports.tests.api_validation = function(test, common) { esclient: {} }; - t.throws(function() { - configValidation.validate(config); - }, /"accessLog" must be a string/); + t.throws(validate.bind(null, config), /"accessLog" must be a string/); + }); t.end(); }); - test('non-boolean api.relativeScores should throw error', function(t) { + test('non-boolean api.relativeScores should throw error', (t) => { [null, 17, {}, [], 'string'].forEach((value) => { var config = { api: { @@ -198,16 +193,15 @@ module.exports.tests.api_validation = function(test, common) { esclient: {} }; - t.throws(function() { - configValidation.validate(config); - }, /"relativeScores" must be a boolean/); + t.throws(validate.bind(null, config), /"relativeScores" must be a boolean/); + }); t.end(); }); - test('non-object api.localization should throw error', function(t) { + test('non-object api.localization should throw error', (t) => { [null, 17, false, [], 'string'].forEach((value) => { var config = { api: { @@ -219,16 +213,15 @@ module.exports.tests.api_validation = function(test, common) { esclient: {} }; - t.throws(function() { - configValidation.validate(config); - }, /"localization" must be an object/); + t.throws(validate.bind(null, config), /"localization" must be an object/); + }); t.end(); }); - test('unknown properties in api.localization should throw error', function(t) { + test('unknown properties in api.localization should throw error', (t) => { var config = { api: { version: 'version value', @@ -241,15 +234,13 @@ module.exports.tests.api_validation = function(test, common) { esclient: {} }; - t.throws(function() { - configValidation.validate(config); - }, /"unknown_property" is not allowed/); + t.throws(validate.bind(null, config), /"unknown_property" is not allowed/); t.end(); }); - test('non-array api.localization.flipNumberAndStreetCountries should throw error', function(t) { + test('non-array api.localization.flipNumberAndStreetCountries should throw error', (t) => { [null, 17, {}, false, 'string'].forEach((value) => { var config = { api: { @@ -263,16 +254,17 @@ module.exports.tests.api_validation = function(test, common) { esclient: {} }; - t.throws(function() { - configValidation.validate(config); - }, /"flipNumberAndStreetCountries" must be an array/); + t.throws( + validate.bind(null, config), + /"flipNumberAndStreetCountries" must be an array/); + }); t.end(); }); - test('non-string api.localization.flipNumberAndStreetCountries elements should throw error', function(t) { + test('non-string api.localization.flipNumberAndStreetCountries elements should throw error', (t) => { [null, 17, {}, false, []].forEach((value) => { var config = { api: { @@ -286,16 +278,15 @@ module.exports.tests.api_validation = function(test, common) { esclient: {} }; - t.throws(function() { - configValidation.validate(config); - }, /"0" must be a string/); + t.throws(validate.bind(null, config), /"0" must be a string/); + }); t.end(); }); - test('non-3-char api.localization.flipNumberAndStreetCountries elements should throw error', function(t) { + test('non-3-char api.localization.flipNumberAndStreetCountries elements should throw error', (t) => { ['AB', 'ABCD'].forEach((value) => { var config = { api: { @@ -309,19 +300,77 @@ module.exports.tests.api_validation = function(test, common) { esclient: {} }; - t.throws(function() { - configValidation.validate(config); - }, /fails to match the required pattern/); + t.throws(validate.bind(null, config), /fails to match the required pattern/); + }); + + t.end(); + + }); + + test('config with non-number api.requestRetries should throw error', (t) => { + [null, 'string', {}, [], false].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + requestRetries: value + }, + esclient: {} + }; + + t.throws( + validate.bind(null, config), + /"requestRetries" must be a number/, 'api.requestRetries should be a number'); + }); t.end(); }); + test('config with non-integer api.requestRetries should throw error', (t) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + requestRetries: 17.3 + }, + esclient: {} + }; + + t.throws( + validate.bind(null, config), + /"requestRetries" must be an integer/, 'api.requestRetries should be an integer'); + + t.end(); + + }); + + test('config with negative api.requestRetries should throw error', (t) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + requestRetries: -1 + }, + esclient: {} + }; + + t.throws( + validate.bind(null, config), + /"requestRetries" must be larger than or equal to 0/, 'api.requestRetries must be positive'); + + t.end(); + + }); + }; -module.exports.tests.esclient_validation = function(test, common) { - test('config without esclient should throw error', function(t) { +module.exports.tests.esclient_validation = (test, common) => { + test('config without esclient should throw error', (t) => { var config = { api: { version: 'version value', @@ -330,14 +379,14 @@ module.exports.tests.esclient_validation = function(test, common) { } }; - t.throws(function() { - configValidation.validate(config); - }, /"esclient" is required/, 'esclient should exist'); + t.throws( + validate.bind(null, config), + /"esclient" is required/, 'esclient should exist'); t.end(); }); - test('config with non-object esclient should throw error', function(t) { + test('config with non-object esclient should throw error', (t) => { [null, 17, [], 'string', true].forEach((value) => { var config = { api: { @@ -348,9 +397,9 @@ module.exports.tests.esclient_validation = function(test, common) { esclient: value }; - t.throws(function() { - configValidation.validate(config); - }, /"esclient" must be an object/, 'esclient should be an object'); + t.throws( + validate.bind(null, config), + /"esclient" must be an object/, 'esclient should be an object'); }); @@ -358,7 +407,7 @@ module.exports.tests.esclient_validation = function(test, common) { }); - test('config with non-integer esclient.requestTimeout should throw error', function(t) { + test('config with non-number esclient.requestTimeout should throw error', (t) => { [null, 'string', {}, [], false].forEach((value) => { var config = { api: { @@ -371,16 +420,17 @@ module.exports.tests.esclient_validation = function(test, common) { } }; - t.throws(function() { - configValidation.validate(config); - }, /"requestTimeout" must be a number/, 'esclient.requestTimeout should be a number'); + t.throws( + validate.bind(null, config), + /"requestTimeout" must be a number/, 'esclient.requestTimeout should be a number'); + }); t.end(); }); - test('config with non-integer esclient.requestTimeout should throw error', function(t) { + test('config with non-integer esclient.requestTimeout should throw error', (t) => { var config = { api: { version: 'version value', @@ -392,15 +442,15 @@ module.exports.tests.esclient_validation = function(test, common) { } }; - t.throws(function() { - configValidation.validate(config); - }, /"requestTimeout" must be an integer/, 'esclient.requestTimeout should be an integer'); + t.throws( + validate.bind(null, config), + /"requestTimeout" must be an integer/, 'esclient.requestTimeout should be an integer'); t.end(); }); - test('config with negative esclient.requestTimeout should throw error', function(t) { + test('config with negative esclient.requestTimeout should throw error', (t) => { var config = { api: { version: 'version value', @@ -412,9 +462,9 @@ module.exports.tests.esclient_validation = function(test, common) { } }; - t.throws(function() { - configValidation.validate(config); - }, /"requestTimeout" must be larger than or equal to 0/, 'esclient.requestTimeout must be positive'); + t.throws( + validate.bind(null, config), + /"requestTimeout" must be larger than or equal to 0/, 'esclient.requestTimeout must be positive'); t.end(); @@ -422,7 +472,7 @@ module.exports.tests.esclient_validation = function(test, common) { }; -module.exports.all = function (tape, common) { +module.exports.all = (tape, common) => { function test(name, testFunction) { return tape('configValidation: ' + name, testFunction); diff --git a/test/unit/service/mget.js b/test/unit/service/mget.js index e2723e6e..6d347111 100644 --- a/test/unit/service/mget.js +++ b/test/unit/service/mget.js @@ -1,17 +1,28 @@ -var setup = require('../../../service/mget'), +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) { - t.equal(typeof setup, 'function', 'setup is a function'); + var service = proxyquire('../../../service/mget', { + 'pelias-logger': { + get: (section) => { + t.equal(section, 'api'); + } + } + + }); + + t.equal(typeof service, 'function', 'service is a function'); t.end(); }); }; -// functionally test service +// functionally test service module.exports.tests.functional_success = function(test, common) { var expected = [ @@ -21,7 +32,7 @@ module.exports.tests.functional_success = function(test, common) { 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, @@ -35,7 +46,7 @@ module.exports.tests.functional_success = function(test, common) { var backend = mockBackend( 'client/mget/ok/1', function( cmd ){ t.deepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: 'a' } ] } }, 'correct backend command'); }); - setup( backend, [ { _id: 123, _index: 'pelias', _type: 'a' } ], function(err, data) { + 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'); @@ -62,8 +73,22 @@ module.exports.tests.functional_failure = function(test, common) { t.notDeepEqual(cmd, { body: { docs: [ { _id: 123, _index: 'pelias', _type: 'a' } ] } }, 'incorrect backend command'); }); invalid_queries.forEach(function(query) { - setup( backend, [ query ], function(err, data) { - t.equal(err, 'a backend error occurred','error passed to errorHandler'); + // 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'); + } + }; + } + } + + }); + + service( backend, [ query ], function(err, data) { + t.equal(err, 'an elasticsearch error occurred','error passed to errorHandler'); t.equal(data, undefined, 'data is undefined'); }); }); @@ -81,4 +106,4 @@ module.exports.all = function (tape, common) { for( var testCase in module.exports.tests ){ module.exports.tests[testCase](test, common); } -}; \ No newline at end of file +}; diff --git a/test/unit/service/search.js b/test/unit/service/search.js index cf72b75e..146b9744 100644 --- a/test/unit/service/search.js +++ b/test/unit/service/search.js @@ -1,14 +1,25 @@ -var setup = require('../../../service/search'), +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) { - t.equal(typeof setup, 'function', 'setup is a function'); + var service = proxyquire('../../../service/mget', { + 'pelias-logger': { + get: (section) => { + t.equal(section, 'api'); + } + } + + }); + + t.equal(typeof service, 'function', 'service is a function'); t.end(); }); }; @@ -45,7 +56,7 @@ module.exports.tests.functional_success = function(test, common) { var backend = mockBackend( 'client/search/ok/1', function( cmd ){ t.deepEqual(cmd, example_valid_es_query, 'no change to the command'); }); - setup( backend, example_valid_es_query, function(err, data, meta) { + 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'); @@ -71,8 +82,22 @@ module.exports.tests.functional_failure = function(test, common) { t.notDeepEqual(cmd, example_valid_es_query, 'incorrect backend command'); }); invalid_queries.forEach(function(query) { - setup( backend, [ query ], function(err, data) { - t.equal(err, 'a backend error occurred','error passed to errorHandler'); + // 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'); + } + }; + } + } + + }); + + service( backend, [ query ], function(err, data) { + t.equal(err, 'an elasticsearch error occurred','error passed to errorHandler'); t.equal(data, undefined, 'data is undefined'); }); }); diff --git a/test/unit/src/backend.js b/test/unit/src/backend.js deleted file mode 100644 index 1b750396..00000000 --- a/test/unit/src/backend.js +++ /dev/null @@ -1,40 +0,0 @@ -var proxyquire = require('proxyquire'); - -var stubConfig = { - generate: function generate() { - return { - esclient: { - hosts: [ - 'http://notLocalhost:9200', - 'http://anotherHost:9200' - ] - } - }; - } -}; - - -module.exports.tests = {}; - -module.exports.tests.config_properly_passed = function(test, common) { - test('Elasticsearch config is properly passed to elasticsearch module', function(t) { - var stubElasticsearchClient = { - Client: function(config) { - t.deepEquals(config.hosts, [ 'http://notLocalhost:9200', 'http://anotherHost:9200' ], 'hosts set correctly' ); - t.end(); - } - }; - - proxyquire('../../../src/backend', { 'pelias-config': stubConfig, 'elasticsearch': stubElasticsearchClient } ); - }); -}; - -module.exports.all = function (tape, common) { - function test(name, testFunction) { - return tape('SANTIZE src/backend ' + name, testFunction); - } - - for( var testCase in module.exports.tests ){ - module.exports.tests[testCase](test, common); - } -};