Merge pull request #2 from pelias/fix-urls

pass `req` to `synthesizeUrl` and conditionally append `/` to `baseUrl`

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;
 

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('&');
 

test/ServiceConfiguration.js

@@ -4,27 +4,39 @@ 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
     };
 
     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);

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');