From 1f6fefb3ebc04c94eaf512a7e6ca967f105d0787 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Mon, 1 May 2017 16:19:17 -0400
Subject: [PATCH] converted placeholder controller to use generic service

---
 controller/placeholder.js           |   2 +-
 routes/v1.js                        |   6 +-
 test/unit/controller/placeholder.js | 227 +++++-----------------------
 3 files changed, 40 insertions(+), 195 deletions(-)

diff --git a/controller/placeholder.js b/controller/placeholder.js
index 1fa0a3ac..c1b34a43 100644
--- a/controller/placeholder.js
+++ b/controller/placeholder.js
@@ -88,7 +88,7 @@ function setup(placeholderService, should_execute) {
       return next();
     }
 
-    placeholderService.search(req.clean.text, req.clean.lang.iso6393, logging.isDNT(req), (err, results) => {
+    placeholderService(req, (err, results) => {
       if (err) {
         // bubble up an error if one occurred
         if (_.isObject(err) && err.message) {
diff --git a/routes/v1.js b/routes/v1.js
index a0d1577f..c3ee4fa4 100644
--- a/routes/v1.js
+++ b/routes/v1.js
@@ -72,6 +72,8 @@ const isAdminOnlyAnalysis = require('../controller/predicates/is_admin_only_anal
 // shorthand for standard early-exit conditions
 const hasResponseDataOrRequestErrors = any(hasResponseData, hasRequestErrors);
 
+const PlaceHolder = require('../service/configurations/placeholder');
+
 /**
  * Append routes to app
  *
@@ -85,7 +87,9 @@ function addRoutes(app, peliasConfig) {
   const isPlaceholderServiceEnabled = require('../controller/predicates/is_service_enabled')(peliasConfig.api.placeholderService);
 
   const pipService = require('../service/pointinpolygon')(peliasConfig.api.pipService);
-  const placeholderService = require('../service/placeholder')(peliasConfig.api.placeholderService);
+
+  const placeholderService = require('../service/http_json')(
+    new PlaceHolder(peliasConfig.api.placeholderService));
 
   const coarse_reverse_should_execute = all(
     not(hasRequestErrors), isPipServiceEnabled, isCoarseReverse
diff --git a/test/unit/controller/placeholder.js b/test/unit/controller/placeholder.js
index 3cf7c0fb..6e4545da 100644
--- a/test/unit/controller/placeholder.js
+++ b/test/unit/controller/placeholder.js
@@ -13,14 +13,12 @@ module.exports.tests.interface = (test, common) => {
   });
 };
 
-module.exports.tests.should_execute_failure = function(test, common) {
+module.exports.tests.should_execute_failure = (test, common) => {
   test('should_execute returning false should return without calling service', (t) => {
     let placeholderService_was_called = false;
 
-    const placeholderService = {
-      search: () => {
-        placeholderService_was_called = true;
-      }
+    const placeholderService = () => {
+      placeholderService_was_called = true;
     };
 
     const should_execute = (req, res) => {
@@ -44,17 +42,14 @@ module.exports.tests.should_execute_failure = function(test, common) {
 
 };
 
-module.exports.tests.success = function(test, common) {
+module.exports.tests.success = (test, common) => {
   test('should_execute returning true should call service', (t) => {
     let placeholderService_was_called = false;
 
-    const placeholderService = {
-      search: (text, language, do_not_track, callback) => {
-        t.equals(text, 'query value');
-        t.equals(language, 'language value');
-        placeholderService_was_called = true;
-        callback(null, []);
-      }
+    const placeholderService = (req, callback) => {
+      t.deepEqual(req, { param1: 'param1 value' });
+      placeholderService_was_called = true;
+      callback(null, []);
     };
 
     const should_execute = (req, res) => {
@@ -63,14 +58,7 @@ module.exports.tests.success = function(test, common) {
 
     const controller = placeholder(placeholderService, should_execute);
 
-    const req = {
-      clean: {
-        text: 'query value',
-        lang: {
-          iso6393: 'language value'
-        }
-      }
-    };
+    const req = { param1: 'param1 value' };
     const res = { b: 2 };
 
     controller(req, res, () => {
@@ -206,13 +194,10 @@ module.exports.tests.success = function(test, common) {
       }
     ];
 
-    const placeholderService = {
-      search: (text, language, do_not_track, callback) => {
-        t.equals(text, 'query value');
-        t.equals(language, 'language value');
-        placeholderService_was_called = true;
-        callback(null, placeholder_response);
-      }
+    const placeholderService = (req, callback) => {
+      t.deepEqual(req, { param1: 'param1 value' });
+      placeholderService_was_called = true;
+      callback(null, placeholder_response);
     };
 
     const should_execute = (req, res) => {
@@ -221,14 +206,7 @@ module.exports.tests.success = function(test, common) {
 
     const controller = placeholder(placeholderService, should_execute);
 
-    const req = {
-      clean: {
-        text: 'query value',
-        lang: {
-          iso6393: 'language value'
-        }
-      }
-    };
+    const req = { param1: 'param1 value' };
     const res = { };
 
     const expected_res = {
@@ -381,12 +359,9 @@ module.exports.tests.success = function(test, common) {
       }
     ];
 
-    const placeholderService = {
-      search: (text, language, do_not_track, callback) => {
-        t.equals(text, 'query value');
-        t.equals(language, 'language value');
-        callback(null, placeholder_response);
-      }
+    const placeholderService = (req, callback) => {
+      t.deepEqual(req, { param1: 'param1 value' });
+      callback(null, placeholder_response);
     };
 
     const should_execute = (req, res) => {
@@ -395,14 +370,7 @@ module.exports.tests.success = function(test, common) {
 
     const controller = placeholder(placeholderService, should_execute);
 
-    const req = {
-      clean: {
-        text: 'query value',
-        lang: {
-          iso6393: 'language value'
-        }
-      }
-    };
+    const req = { param1: 'param1 value' };
     const res = { };
 
     const expected_res = {
@@ -451,12 +419,9 @@ module.exports.tests.success = function(test, common) {
       }
     ];
 
-    const placeholderService = {
-      search: (text, language, do_not_track, callback) => {
-        t.equals(text, 'query value');
-        t.equals(language, 'language value');
-        callback(null, placeholder_response);
-      }
+    const placeholderService = (req, callback) => {
+      t.deepEqual(req, { param1: 'param1 value' });
+      callback(null, placeholder_response);
     };
 
     const should_execute = (req, res) => {
@@ -465,14 +430,7 @@ module.exports.tests.success = function(test, common) {
 
     const controller = placeholder(placeholderService, should_execute);
 
-    const req = {
-      clean: {
-        text: 'query value',
-        lang: {
-          iso6393: 'language value'
-        }
-      }
-    };
+    const req = { param1: 'param1 value' };
     const res = { };
 
     const expected_res = {
@@ -519,12 +477,9 @@ module.exports.tests.success = function(test, common) {
       }
     ];
 
-    const placeholderService = {
-      search: (text, language, do_not_track, callback) => {
-        t.equals(text, 'query value');
-        t.equals(language, 'language value');
-        callback(null, placeholder_response);
-      }
+    const placeholderService = (req, callback) => {
+      t.deepEqual(req, { param1: 'param1 value' });
+      callback(null, placeholder_response);
     };
 
     const should_execute = (req, res) => {
@@ -533,14 +488,7 @@ module.exports.tests.success = function(test, common) {
 
     const controller = placeholder(placeholderService, should_execute);
 
-    const req = {
-      clean: {
-        text: 'query value',
-        lang: {
-          iso6393: 'language value'
-        }
-      }
-    };
+    const req = { param1: 'param1 value' };
     const res = { };
 
     const expected_res = {
@@ -573,95 +521,10 @@ module.exports.tests.success = function(test, common) {
 
 };
 
-module.exports.tests.do_not_track = function(test, common) {
-  test('do_not_track enabled should pass header with `true` value to service', (t) => {
-    let placeholderService_was_called = false;
-
-    const placeholderService = {
-      search: (text, language, do_not_track, callback) => {
-        t.ok(do_not_track, 'should be true');
-        placeholderService_was_called = true;
-        callback(null, []);
-      }
-    };
-
-    const should_execute = (req, res) => {
-      return true;
-    };
-
-    const controller = proxyquire('../../../controller/placeholder', {
-      '../helper/logging': {
-        isDNT: (req) => {
-          return true;
-        }
-      }
-    })(placeholderService, should_execute);
-
-    const req = {
-      clean: {
-        text: 'query value',
-        lang: {
-          iso6393: 'language value'
-        }
-      }
-    };
-    const res = { b: 2 };
-
-    controller(req, res, () => {
-      t.ok(placeholderService_was_called);
-      t.end();
-    });
-
-  });
-
-  test('do_not_track disabled should pass header with `false` value to service', (t) => {
-    let placeholderService_was_called = false;
-
-    const placeholderService = {
-      search: (text, language, do_not_track, callback) => {
-        t.notOk(do_not_track, 'should be false');
-        placeholderService_was_called = true;
-        callback(null, []);
-      }
-    };
-
-    const should_execute = (req, res) => {
-      return true;
-    };
-
-    const controller = proxyquire('../../../controller/placeholder', {
-      '../helper/logging': {
-        isDNT: (req) => {
-          return false;
-        }
-      }
-    })(placeholderService, should_execute);
-
-    const req = {
-      clean: {
-        text: 'query value',
-        lang: {
-          iso6393: 'language value'
-        }
-      }
-    };
-    const res = { b: 2 };
-
-    controller(req, res, () => {
-      t.ok(placeholderService_was_called);
-      t.end();
-    });
-
-  });
-
-};
-
 module.exports.tests.error_conditions = (test, common) => {
   test('service return error string should add to req.errors', (t) => {
-    const placeholderService = {
-      search: (text, language, do_not_track, callback) => {
-        callback('placeholder service error', []);
-      }
+    const placeholderService = (req, callback) => {
+      callback('placeholder service error', []);
     };
 
     const should_execute = (req, res) => {
@@ -671,12 +534,6 @@ module.exports.tests.error_conditions = (test, common) => {
     const controller = placeholder(placeholderService, should_execute);
 
     const req = {
-      clean: {
-        text: 'query value',
-        lang: {
-          iso6393: 'language value'
-        }
-      },
       errors: []
     };
     const res = {};
@@ -689,10 +546,8 @@ module.exports.tests.error_conditions = (test, common) => {
   });
 
   test('service return error object should add message to req.errors', (t) => {
-    const placeholderService = {
-      search: (text, language, do_not_track, callback) => {
-        callback(Error('placeholder service error'), []);
-      }
+    const placeholderService = (req, callback) => {
+      callback(Error('placeholder service error'), []);
     };
 
     const should_execute = (req, res) => {
@@ -702,12 +557,6 @@ module.exports.tests.error_conditions = (test, common) => {
     const controller = placeholder(placeholderService, should_execute);
 
     const req = {
-      clean: {
-        text: 'query value',
-        lang: {
-          iso6393: 'language value'
-        }
-      },
       errors: []
     };
     const res = {};
@@ -720,10 +569,8 @@ module.exports.tests.error_conditions = (test, common) => {
   });
 
   test('service return error object should add stringified error to req.errors', (t) => {
-    const placeholderService = {
-      search: (text, language, do_not_track, callback) => {
-        callback({ error_key: 'error_value' }, []);
-      }
+    const placeholderService = (req, callback) => {
+      callback({ error_key: 'error_value' }, []);
     };
 
     const should_execute = (req, res) => {
@@ -733,12 +580,6 @@ module.exports.tests.error_conditions = (test, common) => {
     const controller = placeholder(placeholderService, should_execute);
 
     const req = {
-      clean: {
-        text: 'query value',
-        lang: {
-          iso6393: 'language value'
-        }
-      },
       errors: []
     };
     const res = {};
@@ -752,10 +593,10 @@ module.exports.tests.error_conditions = (test, common) => {
 
 };
 
-module.exports.all = function (tape, common) {
+module.exports.all = (tape, common) => {
 
   function test(name, testFunction) {
-    return tape('GET /placeholder ' + name, testFunction);
+    return tape(`GET /placeholder ${name}`, testFunction);
   }
 
   for( const testCase in module.exports.tests ){