From 9092b9c46286e0c1f57b495226bb37de82b629e7 Mon Sep 17 00:00:00 2001
From: Stephen Hess <stephen.k.hess@gmail.com>
Date: Fri, 5 May 2017 17:09:45 -0400
Subject: [PATCH] pass `req` to `synthesizeUrl` and conditionally append `/` to
 `baseUrl`

---
 ServiceConfiguration.js      |  6 +++++-
 service.js                   |  2 +-
 test/ServiceConfiguration.js | 20 ++++++++++++++++----
 test/service.js              | 12 ++++++------
 4 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/ServiceConfiguration.js b/ServiceConfiguration.js
index 7f8ad1b..c6ae0ee 100644
--- a/ServiceConfiguration.js
+++ b/ServiceConfiguration.js
@@ -9,7 +9,11 @@ class ServiceConfiguration {
     }
 
     this.name = name;
-    this.baseUrl = config.url;
+    if (config.url && !_.endsWith(config.url, '/')) {
+      this.baseUrl = config.url + '/';
+    } else {
+      this.baseUrl = config.url;
+    }
     this.timeout = config.timeout || 250;
     this.retries = config.retries || 3;
 
diff --git a/service.js b/service.js
index f004dd7..fcdd911 100644
--- a/service.js
+++ b/service.js
@@ -11,7 +11,7 @@ function isDoNotTrack(headers) {
 
 // superagent doesn't exposed the assembled GET request, so synthesize it
 function synthesizeUrl(serviceConfig, req) {
-  const parameters = _.map(serviceConfig.getParameters(), (value, key) => {
+  const parameters = _.map(serviceConfig.getParameters(req), (value, key) => {
     return `${key}=${value}`;
   }).join('&');
 
diff --git a/test/ServiceConfiguration.js b/test/ServiceConfiguration.js
index d4a9a94..78d2b3d 100644
--- a/test/ServiceConfiguration.js
+++ b/test/ServiceConfiguration.js
@@ -4,7 +4,7 @@ const ServiceConfiguration = require('../ServiceConfiguration');
 tape('ServiceConfiguration tests', (test) => {
   test.test('timeout and retries overrides should be returned by getters', (t) => {
     const configBlob = {
-      url: 'base url',
+      url: 'http://localhost:1234/',
       timeout: 17,
       retries: 19
     };
@@ -12,19 +12,31 @@ tape('ServiceConfiguration tests', (test) => {
     const serviceConfiguration = new ServiceConfiguration('service name', configBlob);
 
     t.equals(serviceConfiguration.getName(), 'service name');
-    t.equals(serviceConfiguration.getBaseUrl(), 'base url');
+    t.equals(serviceConfiguration.getBaseUrl(), 'http://localhost:1234/');
     t.deepEquals(serviceConfiguration.getParameters(), {});
     t.deepEquals(serviceConfiguration.getHeaders(), {});
-    t.equals(serviceConfiguration.getUrl(), 'base url');
+    t.equals(serviceConfiguration.getUrl(), 'http://localhost:1234/');
     t.equals(serviceConfiguration.getRetries(), 19);
     t.equals(serviceConfiguration.getTimeout(), 17);
     t.end();
 
   });
 
+  test.test('url not ending with / should append /', (t) => {
+    const configBlob = {
+      url: 'http://localhost:1234'
+    };
+
+    const serviceConfiguration = new ServiceConfiguration('service name', configBlob);
+
+    t.equals(serviceConfiguration.getBaseUrl(), 'http://localhost:1234/');
+    t.end();
+
+  });
+
   test.test('configBlob w/o timeout or retries should default to 250 and 3, respectively', (t) => {
     const configBlob = {
-      url: 'base url'
+      url: 'http://localhost:1234/'
     };
 
     const serviceConfiguration = new ServiceConfiguration('service name', configBlob);
diff --git a/test/service.js b/test/service.js
index 300aa5c..0dd3d9d 100644
--- a/test/service.js
+++ b/test/service.js
@@ -128,7 +128,7 @@ tape('failure conditions tests', (test) => {
     service(req, (err, results) => {
       t.equals(err.code, 'ECONNREFUSED');
       t.notOk(results);
-      t.ok(logger.isErrorMessage(new RegExp(`^http://localhost:${port} \\[do_not_track\\]: .*ECONNREFUSED`)),
+      t.ok(logger.isErrorMessage(new RegExp(`^http://localhost:${port}/ \\[do_not_track\\]: .*ECONNREFUSED`)),
         'there should be a connection refused error message');
       t.end();
 
@@ -233,9 +233,9 @@ tape('failure conditions tests', (test) => {
     };
 
     service(req, (err, results) => {
-      t.equals(err, `http://localhost:${port} [do_not_track] returned status 400: a bad request was made`);
+      t.equals(err, `http://localhost:${port}/ [do_not_track] returned status 400: a bad request was made`);
       t.notOk(results);
-      t.ok(logger.isErrorMessage(`http://localhost:${port} [do_not_track] ` +
+      t.ok(logger.isErrorMessage(`http://localhost:${port}/ [do_not_track] ` +
         `returned status 400: a bad request was made`));
       t.end();
 
@@ -340,10 +340,10 @@ tape('failure conditions tests', (test) => {
     };
 
     service(req, (err, results) => {
-      t.equals(err, `http://localhost:${port} [do_not_track] ` +
+      t.equals(err, `http://localhost:${port}/ [do_not_track] ` +
         `could not parse response: this is not parseable as JSON`);
       t.notOk(results, 'should return undefined');
-      t.ok(logger.isErrorMessage(`http://localhost:${port} [do_not_track] ` +
+      t.ok(logger.isErrorMessage(`http://localhost:${port}/ [do_not_track] ` +
         `could not parse response: this is not parseable as JSON`));
       t.end();
 
@@ -553,7 +553,7 @@ tape('success conditions tests', (test) => {
       'pelias-logger': logger
     })(new MockServiceConfig());
 
-    t.ok(logger.isInfoMessage(new RegExp(`using foo service at http://localhost:${port}`)));
+    t.ok(logger.isInfoMessage(new RegExp(`using foo service at http://localhost:${port}/`)));
 
     service({}, (err, results) => {
       t.notOk(err, 'should be no error');