From 2615f39d01899bf409da6ded7a042ef9d831d1f2 Mon Sep 17 00:00:00 2001
From: Mike Larsson <mikelarssonftw@gmail.com>
Date: Thu, 16 May 2013 15:46:44 -0400
Subject: [PATCH] Allow api origins to have paths. Add basic logging wrapper
 for winston to start using. Connect express.logger to logger module. Use
 example.org instead of origin.org juuuuuust to be safe.

---
 api.js                      | 14 ++++++++++----
 app.js                      |  7 ++++++-
 logger.js                   |  4 ++++
 openbadger.js               |  2 ++
 test/api-middleware.test.js | 20 +++++++++++++++-----
 5 files changed, 37 insertions(+), 10 deletions(-)
 create mode 100644 logger.js

diff --git a/api.js b/api.js
index 59b815c..3f1912e 100644
--- a/api.js
+++ b/api.js
@@ -1,7 +1,7 @@
 var request = require('request');
 var errors = require('./lib/errors');
 var _ = require('underscore');
-var logger = require('winston');
+var logger = require('./logger');
 var url = require('url');
 
 
@@ -76,6 +76,14 @@ function apiMethod (method) {
   }
 }
 
+function getFullUrl(origin, path) {
+  path = path || '';
+  path = path.replace(/^\/?/, '');
+  return url.format(_.extend(
+    origin,
+    { pathname: origin.path + path }));
+}
+
 // Load data from remote endpoint
 function remote (method, path, callback) {
 
@@ -84,7 +92,7 @@ function remote (method, path, callback) {
 
   // TODO - need to add ability to pass data through
   // TODO - might want to cache this at some point
-  var endpointUrl = url.format(_.extend(this.origin, { pathname: path }));
+  var endpointUrl = getFullUrl(this.origin, path);
   request[method](endpointUrl, function(err, response, body) {
 
     logger.log('info', 'API request: "%s %s" %s',
@@ -157,8 +165,6 @@ module.exports = function Api(origin, config) {
   config = config || {};
 
   origin = url.parse(origin);
-  if (origin.pathname !== '/')
-    throw new Error('Api origin url must not contain a path');
   this.origin = origin;
 
   _.each(['get', 'post', 'put', 'patch', 'head', 'del'], function(method) {
diff --git a/app.js b/app.js
index e0fc7fd..6ce5c57 100644
--- a/app.js
+++ b/app.js
@@ -4,6 +4,7 @@ const express = require('express');
 const nunjucks = require('nunjucks');
 const middleware = require('./middleware');
 const helpers = require('./helpers');
+const logger = require('./logger');
 
 const app = express();
 const env = new nunjucks.Environment(new nunjucks.FileSystemLoader(path.join(__dirname, 'views')), {autoescape: true});
@@ -11,7 +12,11 @@ env.express(app);
 
 app.use(express.cookieParser());
 app.use(middleware.session());
-app.use(express.logger());
+app.use(express.logger({stream:{
+  write: function(msg, encoding) {
+    logger.info(msg.trim());
+  }
+}}));
 app.use(express.compress());
 app.use(express.bodyParser());
 app.use(express.csrf());
diff --git a/logger.js b/logger.js
new file mode 100644
index 0000000..c0cd3d2
--- /dev/null
+++ b/logger.js
@@ -0,0 +1,4 @@
+var winston = require('winston');
+/* This space reserved for future config/transports */
+/* e.g. winston.add(winston.transports.File, { filename: 'somelog.log' }); */
+module.exports = winston;
\ No newline at end of file
diff --git a/openbadger.js b/openbadger.js
index e82b5e6..9477936 100644
--- a/openbadger.js
+++ b/openbadger.js
@@ -3,6 +3,8 @@ const errors = require('./lib/errors');
 const _ = require('underscore');
 
 const ENDPOINT = process.env['CSOL_OPENBADGER_URL'];
+if (!ENDPOINT)
+  throw new Error('Must specify CSOL_OPENBADGER_URL in the environment');
 
 // Make sure badges returned from remote API
 // contain all the information we need
diff --git a/test/api-middleware.test.js b/test/api-middleware.test.js
index 2eb17c9..ddc14bb 100644
--- a/test/api-middleware.test.js
+++ b/test/api-middleware.test.js
@@ -41,15 +41,12 @@ function fakeRequest(func, config, callback) {
   callback(req, res, next);
 }
 
-const ORIGIN = 'http://origin.org';
+const ORIGIN = 'http://example.org';
 
 test('Api()', function(t) {
 
-  t.test('requires pathless origin', function(t) {
+  t.test('requires origin', function(t) {
     t.throws(function(){ new Api(); }, 'throws without');
-    t.throws(function(){ new Api('http://foo.org/bar'); },
-      { name: 'Error', message: 'Api origin url must not contain a path' },
-      'throws with path');
     t.end();
   });
 
@@ -336,6 +333,19 @@ test('api.get', function(t) {
     t.end();
   });
 
+  t.test('leading slashes don\'t indicate absolute path', function(t) {
+    const WITH_PATH = 'http://example.org/base/';
+    var api = new Api(WITH_PATH);
+    var requestMock = sinon.mock(request);
+    var get = requestMock.expects('get').twice();
+
+    api.get('/foo/bar', function(){});
+    api.get('foo/bar', function(){});
+    t.ok(get.alwaysCalledWith(sinon.match(WITH_PATH + 'foo/bar')), 'with origin/endpoint');
+    requestMock.restore();
+    t.end();
+  });
+
   t.test('calls against different origins don\'t collide', function(t) {
     const ANOTHER = 'http://another.org';
     var api2 = new Api(ANOTHER);