rewrote tests to proxyquire service/search to reduce complexity

added a few more tests for coverage, removed unused things from unit/mock/backend

controller/search.js

@@ -1,10 +1,10 @@
 'use strict';
 
-var _ = require('lodash');
+const _ = require('lodash');
 
-var searchService = require('../service/search');
+const searchService = require('../service/search');
-var logger = require('pelias-logger').get('api');
+const logger = require('pelias-logger').get('api');
-var logging = require( '../helper/logging' );
+const logging = require( '../helper/logging' );
 const retry = require('retry');
 
 function setup( apiConfig, esclient, query ){
@@ -68,7 +68,7 @@ function setup( apiConfig, esclient, query ){
         // (handles bookkeeping of maxRetries)
         // only consider for status 408 (request timeout)
         if (isRequestTimeout(err) && operation.retry(err)) {
-          logger.info('request timed out, retrying');
+          logger.info(`request timed out on attempt ${currentAttempt}, retrying`);
           return;
         }
 
@@ -94,7 +94,7 @@ function setup( apiConfig, esclient, query ){
           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) + ']');
+            _.get(res, 'data', []).length + ']');
         }
         logger.debug('[ES response]', docs);
         next();

test/unit/controller/search.js

@@ -15,150 +15,286 @@ module.exports.tests.interface = function(test, common) {
   });
 };
 
-// reminder: this is only the api subsection of the full config
+module.exports.tests.success = function(test, common) {
-var fakeDefaultConfig = {
+  test('successful request to search service should set data and meta', (t) => {
-  indexName: 'pelias'
+    const config = {
-};
+      indexName: 'indexName value'
-
+    };
-// functionally test controller (backend success)
+    const esclient = 'this is the esclient';
-// module.exports.tests.functional_success = function(test, common) {
+    const query = () => {
-//
+      return {
-//   // expected geojson features for 'client/suggest/ok/1' fixture
+        body: 'this is the query body',
-//   var expected = [{
+        type: 'this is the query type'
-//     type: 'Feature',
+      };
-//     geometry: {
+    };
-//       type: 'Point',
+
-//       coordinates: [-50.5, 100.1]
+    // request timeout messages willl be written here
-//     },
+    const infoMesssages = [];
-//     properties: {
+
-//       id: 'myid1',
+    // a controller that validates the esclient and cmd that was passed to the search service
-//       layer: 'mytype1',
+    const controller = proxyquire('../../../controller/search', {
-//       text: 'test name1, city1, state1'
+      '../service/search': (esclient, cmd, callback) => {
-//     }
+        t.equal(esclient, 'this is the esclient');
-//   }, {
+        t.deepEqual(cmd, {
-//     type: 'Feature',
+          index: 'indexName value',
-//     geometry: {
+          searchType: 'dfs_query_then_fetch',
-//       type: 'Point',
+          body: 'this is the query body'
-//       coordinates: [-51.5, 100.2]
+        });
-//     },
+
-//     properties: {
+        const docs = [{}, {}];
-//       id: 'myid2',
+        const meta = { key: 'value' };
-//       layer: 'mytype2',
+
-//       text: 'test name2, city2, state2'
+        callback(undefined, docs, meta);
-//     }
+      },
-//   }];
+      'pelias-logger': {
-//
+        get: (service) => {
-//   var expectedMeta = {
+          t.equal(service, 'api');
-//     scores: [10, 20],
+          return {
-//     query_type: 'mock'
+            info: (msg) => {
-//   };
+              infoMesssages.push(msg);
-//
+            },
-//   var expectedData = [
+            debug: () => {}
-//     {
+          };
-//       _id: 'myid1',
+        }
-//       _score: 10,
+      }
-//       _type: 'mytype1',
+    })(config, esclient, query);
-//       _matched_queries: ['query 1', 'query 2'],
+
-//       parent: {
+    const req = { clean: { }, errors: [], warnings: [] };
-//         country: ['country1'],
+    const res = {};
-//         region: ['state1'],
+
-//         county: ['city1']
+    var next = function() {
-//       },
+      t.deepEqual(req, {
-//       center_point: { lat: 100.1, lon: -50.5 },
+        clean: {},
-//       name: { default: 'test name1' },
+        errors: [],
-//       value: 1
+        warnings: []
-//     },
+      });
-//     {
+      t.deepEquals(res.data, [{}, {}]);
-//       _id: 'myid2',
+      t.deepEquals(res.meta, { key: 'value', query_type: 'this is the query type' });
-//       _score: 20,
+
-//       _type: 'mytype2',
+      t.ok(infoMesssages.find((msg) => {
-//       _matched_queries: ['query 3'],
+        return msg === '[controller:search] [queryType:this is the query type] [es_result_count:2]';
-//       parent: {
+      }));
-//         country: ['country2'],
+      t.end();
-//         region: ['state2'],
+    };
-//         county: ['city2']
+
-//       },
+    controller(req, res, next);
-//       center_point: { lat: 100.2, lon: -51.5 },
+
-//       name: { default: 'test name2' },
+  });
-//       value: 2
+
-//     }
+  test('undefined meta should set empty object into res', (t) => {
-//   ];
+    const config = {
-//
+      indexName: 'indexName value'
-//   test('functional success', function (t) {
+    };
-//     var backend = mockBackend('client/search/ok/1', function (cmd) {
+    const esclient = 'this is the esclient';
-//       t.deepEqual(cmd, {
+    const query = () => {
-//         body: {a: 'b'},
+      return {
-//         index: 'pelias',
+        body: 'this is the query body',
-//         searchType: 'dfs_query_then_fetch'
+        type: 'this is the query type'
-//       }, 'correct backend command');
+      };
-//     });
+    };
-//     var controller = setup(fakeDefaultConfig, backend, mockQuery());
+
-//     var res = {
+    // request timeout messages willl be written here
-//       status: function (code) {
+    const infoMesssages = [];
-//         t.equal(code, 200, 'status set');
+
-//         return res;
+    // a controller that validates the esclient and cmd that was passed to the search service
-//       },
+    const controller = proxyquire('../../../controller/search', {
-//       json: function (json) {
+      '../service/search': (esclient, cmd, callback) => {
-//         t.equal(typeof json, 'object', 'returns json');
+        t.equal(esclient, 'this is the esclient');
-//         t.equal(typeof json.date, 'number', 'date set');
+        t.deepEqual(cmd, {
-//         t.equal(json.type, 'FeatureCollection', 'valid geojson');
+          index: 'indexName value',
-//         t.true(Array.isArray(json.features), 'features is array');
+          searchType: 'dfs_query_then_fetch',
-//         t.deepEqual(json.features, expected, 'values correctly mapped');
+          body: 'this is the query body'
-//       }
+        });
-//     };
+
-//     var req = { clean: { a: 'b' }, errors: [], warnings: [] };
+        const docs = [{}, {}];
-//     var next = function next() {
+
-//       t.equal(req.errors.length, 0, 'next was called without error');
+        callback(undefined, docs, undefined);
-//       t.deepEqual(res.meta, expectedMeta, 'meta data was set');
+      },
-//       t.deepEqual(res.data, expectedData, 'data was set');
+      'pelias-logger': {
-//       t.end();
+        get: (service) => {
-//     };
+          t.equal(service, 'api');
-//     controller(req, res, next);
+          return {
-//   });
+            info: (msg) => {
-//
+              infoMesssages.push(msg);
-//   test('functional success with alternate index name', function(t) {
+            },
-//     var fakeCustomizedConfig = {
+            debug: () => {}
-//       indexName: 'alternateindexname'
+          };
-//     };
+        }
-//
+      }
-//     var backend = mockBackend('client/search/ok/1', function (cmd) {
+    })(config, esclient, query);
-//       t.deepEqual(cmd, {
+
-//         body: {a: 'b'},
+    const req = { clean: { }, errors: [], warnings: [] };
-//         index: 'alternateindexname',
+    const res = {};
-//         searchType: 'dfs_query_then_fetch'
+
-//       }, 'correct backend command');
+    var next = function() {
-//     });
+      t.deepEqual(req, {
-//     var controller = setup(fakeCustomizedConfig, backend, mockQuery());
+        clean: {},
-//     var res = {
+        errors: [],
-//       status: function (code) {
+        warnings: []
-//         t.equal(code, 200, 'status set');
+      });
-//         return res;
+      t.deepEquals(res.data, [{}, {}]);
-//       }
+      t.deepEquals(res.meta, { query_type: 'this is the query type' });
-//     };
+
-//     var req = { clean: { a: 'b' }, errors: [], warnings: [] };
+      t.ok(infoMesssages.find((msg) => {
-//     var next = function next() {
+        return msg === '[controller:search] [queryType:this is the query type] [es_result_count:2]';
-//       t.equal(req.errors.length, 0, 'next was called without error');
+      }));
-//       t.end();
+      t.end();
-//     };
+    };
-//     controller(req, res, next);
+
-//   });
+    controller(req, res, next);
-// };
+
-//
+  });
-// // functionally test controller (backend failure)
+
-// module.exports.tests.functional_failure = function(test, common) {
+  test('undefined docs should log 0 results', (t) => {
-//   test('functional failure', function(t) {
+    const config = {
-//     var backend = mockBackend( 'client/search/fail/1', function( cmd ){
+      indexName: 'indexName value'
-//       t.deepEqual(cmd, { body: { a: 'b' }, index: 'pelias', searchType: 'dfs_query_then_fetch' }, 'correct backend command');
+    };
-//     });
+    const esclient = 'this is the esclient';
-//     var controller = setup( fakeDefaultConfig, backend, mockQuery() );
+    const query = () => {
-//     var req = { clean: { a: 'b' }, errors: [], warnings: [] };
+      return {
-//     var next = function(){
+        body: 'this is the query body',
-//       t.equal(req.errors[0],'an elasticsearch error occurred');
+        type: 'this is the query type'
-//       t.end();
+      };
-//     };
+    };
-//     controller(req, undefined, next );
+
-//   });
+    // 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);
+      },
+      '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 = {};
+
+    var next = function() {
+      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();
+    };
+
+    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 = {};
+
+    var next = function() {
+      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);
+
+  });
+
+};
 
 module.exports.tests.timeout = function(test, common) {
   test('default # of request timeout retries should be 3', (t) => {
@@ -214,11 +350,11 @@ module.exports.tests.timeout = function(test, common) {
 
     var next = function() {
       t.equal(searchServiceCallCount, 3+1);
-      t.deepEqual(
+
-        infoMesssages.filter((msg)=> { return msg === 'request timed out, retrying'; } ).length,
+      t.ok(infoMesssages.indexOf('request timed out on attempt 1, retrying') !== -1);
-        3,
+      t.ok(infoMesssages.indexOf('request timed out on attempt 2, retrying') !== -1);
-        'there should be 3 request timed out info messages'
+      t.ok(infoMesssages.indexOf('request timed out on attempt 3, retrying') !== -1);
-      );
+
       t.deepEqual(req, {
         clean: {},
         errors: [timeoutError.message],
@@ -365,7 +501,7 @@ module.exports.tests.existing_errors = function(test, common) {
     var esclient = function() {
       throw new Error('esclient should not have been called');
     };
-    var controller = setup( fakeDefaultConfig, esclient, mockQuery() );
+    var controller = setup( {}, esclient, mockQuery() );
 
     // the existence of `errors` means that a sanitizer detected an error,
     //  so don't call the esclient
@@ -385,10 +521,10 @@ module.exports.tests.existing_errors = function(test, common) {
 
 module.exports.tests.existing_results = function(test, common) {
   test('res with existing data should not call backend', function(t) {
-    var backend = function() {
+    var esclient = function() {
       throw new Error('backend should not have been called');
     };
-    var controller = setup( fakeDefaultConfig, backend, mockQuery() );
+    var controller = setup( {}, esclient, mockQuery() );
 
     var req = { };
     // the existence of `data` means that there are already results so

test/unit/mock/backend.js

@@ -1,8 +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( 'an elasticsearch error occurred' );
 };