Merge branch 'dev' into servo-level-up

* dev:
  Remove `self` references in favor of #bind
  Stop using #fetch here for now, it just breaks tests
  Just use arguments directly instead of array
  Remove Utils#bind in preference of built-in fn
  Consolidate logstring generation into private fn
  Experiment with Utils.fetch in Driver class
  Add #fetch Utility function
  Fix JSHint errors
  Fix an edge case and improve tests for API

lib/adaptor.js

@@ -29,7 +29,6 @@ module.exports = Adaptor = function Adaptor(opts) {
     opts = {};
   }
 
-  this.self = this;
   this.name = opts.name;
   this.connection = opts.connection;
 };

lib/api.js

@@ -17,8 +17,6 @@ var express = require('express'),
 var Logger = require('./logger');
 
 var API = module.exports = function API(opts) {
-  var self = this;
-
   if (opts == null) {
     opts = {};
   }
@@ -31,17 +29,17 @@ var API = module.exports = function API(opts) {
 
   this.express.set('title', 'Cylon API Server');
 
-  this.express.use(self.setupAuth());
+  this.express.use(this.setupAuth());
   this.express.use(bodyParser());
   this.express.use(express["static"](__dirname + "/../node_modules/robeaux/"));
 
   // set CORS headers for API requests
   this.express.use(function(req, res, next) {
-    res.set("Access-Control-Allow-Origin", self.CORS || "*");
+    res.set("Access-Control-Allow-Origin", this.CORS || "*");
     res.set("Access-Control-Allow-Headers", "Content-Type");
     res.set('Content-Type', 'application/json');
     return next();
-  });
+  }.bind(this));
 
   // extracts command params from request
   this.express.use(function(req, res, next) {
@@ -58,13 +56,13 @@ var API = module.exports = function API(opts) {
 
     for (var p in container) {
       req.commandParams.push(container[p]);
-    };
+    }
 
     return next();
   });
 
   // load route definitions
-  this.express.use('/', require('./api/routes'))
+  this.express.use('/', require('./api/routes'));
 };
 
 API.prototype.defaults = {
@@ -90,7 +88,7 @@ API.prototype.createServer = function createServer() {
       cert: fs.readFileSync(this.ssl.cert)
     }, this.express);
   } else {
-    Logger.warn("API using insecure connection. We recommend using an SSL certificate with Cylon.")
+    Logger.warn("API using insecure connection. We recommend using an SSL certificate with Cylon.");
     this.server = this.express;
   }
 };
@@ -98,7 +96,7 @@ API.prototype.createServer = function createServer() {
 API.prototype.setupAuth = function setupAuth() {
   var authfn = function auth(req, res, next) { next(); };
 
-  if (typeof(this.auth) === 'object' && this.auth.type) {
+  if (!!this.auth && typeof(this.auth) === 'object' && this.auth.type) {
     var type = this.auth.type,
         module = "./api/auth/" + type,
         filename = path.normalize(__dirname + "/" + module + ".js"),
@@ -113,13 +111,11 @@ API.prototype.setupAuth = function setupAuth() {
 };
 
 API.prototype.listen = function() {
-  var self = this;
-
   this.server.listen(this.port, this.host, null, function() {
-    var title    = self.express.get('title');
+    var title    = this.express.get('title');
-    var protocol = self.ssl ? "https" : "http";
+    var protocol = this.ssl ? "https" : "http";
 
     Logger.info(title + " is now online.");
-    Logger.info("Listening at " + protocol + "://" + self.host + ":" + self.port);
+    Logger.info("Listening at " + protocol + "://" + this.host + ":" + this.port);
-  });
+  }.bind(this));
 };

lib/connection.js

@@ -37,17 +37,16 @@ module.exports = Connection = function Connection(opts) {
     opts.id = Math.floor(Math.random() * 10000);
   }
 
-  this.connect = Utils.bind(this.connect, this);
+  this.connect = this.connect.bind(this);
 
-  this.self = this;
   this.robot = opts.robot;
   this.name = opts.name;
   this.connection_id = opts.id;
   this.port = opts.port;
   this.adaptor = this.initAdaptor(opts);
 
-  Utils.proxyFunctionsToObject(this.adaptor.commands, this.adaptor, this.self);
+  Utils.proxyFunctionsToObject(this.adaptor.commands, this.adaptor, this);
-}
+};
 
 Utils.subclass(Connection, EventEmitter);
 
@@ -69,15 +68,7 @@ Connection.prototype.toJSON = function() {
 //
 // Returns the result of the supplied callback function
 Connection.prototype.connect = function(callback) {
-  var msg = "Connecting to " + this.name;
+  var msg = this._logstring("Connecting to");
-  var msg = "Connecting to '" + this.name + "'";
-
-  if (this.port != null) {
-    msg += " on port " + this.port;
-  }
-
-  msg += ".";
-
   Logger.info(msg);
   return this.adaptor.connect(callback);
 };
@@ -88,14 +79,7 @@ Connection.prototype.connect = function(callback) {
 //
 // Returns nothing
 Connection.prototype.disconnect = function(callback) {
-  var msg = "Disconnecting from '" + this.name + "'";
+  var msg = this._logstring("Disconnecting from");
-
-  if (this.port != null) {
-    msg += " on port " + this.port;
-  }
-
-  msg += ".";
-
   Logger.info(msg);
   return this.adaptor.disconnect(callback);
 };
@@ -108,7 +92,7 @@ Connection.prototype.disconnect = function(callback) {
 // Returns the set-up adaptor
 Connection.prototype.initAdaptor = function(opts) {
   Logger.debug("Loading adaptor '" + opts.adaptor + "'.");
-  return this.robot.initAdaptor(opts.adaptor, this.self, opts);
+  return this.robot.initAdaptor(opts.adaptor, this, opts);
 };
 
 // Public: Halt the adaptor's connection
@@ -126,3 +110,15 @@ Connection.prototype.halt = function(callback) {
   Logger.info(msg);
   return this.disconnect(callback);
 };
+
+Connection.prototype._logstring = function _logstring(action) {
+  var msg = action + " '" + this.name + "'";
+
+  if (this.port != null) {
+    msg += " on port " + this.port;
+  }
+
+  msg += ".";
+
+  return msg;
+};

lib/cylon.js

@@ -101,7 +101,7 @@ if (process.platform === "win32") {
   readline.createInterface(io).on("SIGINT", function() {
     process.emit("SIGINT");
   });
-};
+}
 
 process.on("SIGINT", function() {
   Cylon.halt(function() {

lib/device.js

@@ -28,8 +28,8 @@ var Device = module.exports = function Device(opts) {
     opts = {};
   }
 
-  this.halt = Utils.bind(this.halt, this);
+  this.halt = this.halt.bind(this);
-  this.start = Utils.bind(this.start, this);
+  this.start = this.start.bind(this);
 
   this.robot = opts.robot;
   this.name = opts.name;

lib/driver.js

@@ -12,11 +12,6 @@ var Basestar = require('./basestar'),
     Logger = require('./logger'),
     Utils = require('./utils');
 
-// The Driver class is a base class for Driver classes in external Cylon
-// modules to use. It offers basic functions for starting/halting that
-// descendant classes can use.
-var Driver;
-
 // Public: Creates a new Driver
 //
 // opts - hash of acceptable params
@@ -24,12 +19,9 @@ var Driver;
 //   device - Device the driver will use to proxy commands/events
 //
 // Returns a new Driver
-module.exports = Driver = function Driver(opts) {
+var Driver = module.exports = function Driver(opts) {
-  if (opts == null) {
+  opts = opts || {};
-    opts = {};
-  }
 
-  this.self = this;
   this.name = opts.name;
   this.device = opts.device;
   this.connection = this.device.connection;

lib/io/digital-pin.js

@@ -28,7 +28,7 @@ var DigitalPin = module.exports = function DigitalPin(opts) {
   this.status = 'low';
   this.ready = false;
   this.mode = opts.mode;
-}
+};
 
 Utils.subclass(DigitalPin, EventEmitter);
 

lib/logger/null_logger.js

@@ -1,6 +1,6 @@
 // The NullLogger is designed for cases where you want absolutely nothing to
 // print to anywhere. Every proxied method from the Logger returns a noop.
-var NullLogger = module.exports = function NullLogger() {}
+var NullLogger = module.exports = function NullLogger() {};
 
 NullLogger.prototype.toString = function() {
   return "NullLogger";

lib/robot.js

@@ -53,8 +53,6 @@ var Robot = module.exports = function Robot(opts) {
     opts = {};
   }
 
-  var self = this;
-
   var methods = [
     "toString",
     "registerDriver",
@@ -70,8 +68,8 @@ var Robot = module.exports = function Robot(opts) {
   ];
 
   methods.forEach(function(method) {
-    self[method] = Utils.bind(self[method], self);
+    this[method] = this[method].bind(this);
-  });
+  }.bind(this));
 
   this.name = opts.name || this.constructor.randomName();
   this.connections = {};
@@ -209,21 +207,19 @@ Robot.prototype._createDevice = function(device) {
 //
 // Returns the result of the work
 Robot.prototype.start = function() {
-  var self = this;
-
   var begin = function(callback) {
-    self.work.call(self, self);
+    this.work.call(this, this);
-    self.running = true;
+    this.running = true;
-    self.emit('working');
+    this.emit('working');
 
     Logger.info('Working.');
 
     callback(null, true);
-  };
+  }.bind(this);
 
   Async.series([
-    self.startConnections,
+    this.startConnections,
-    self.startDevices,
+    this.startDevices,
     begin
   ], function(err) {
     if (!!err) {

lib/utils.js

@@ -121,8 +121,7 @@ var Utils = module.exports = {
 
     var fn = function(method) {
       return base[method] = function() {
-        var args = arguments.length >= 1 ? [].slice.call(arguments, 0) : [];
+        return target[method].apply(target, arguments);
-        return target[method].apply(target, args);
       };
     };
 
@@ -161,26 +160,59 @@ var Utils = module.exports = {
     return base;
   },
 
-  // Public: Binds an argument to a caller
+  // Public: Analogue to Ruby's Hash#fetch method for looking up object
+  // properties.
   //
-  // fn - function to bind
+  // obj - object to do property lookup on
-  // me - value for 'this' scope inside the function
+  // property - string property name to attempt to look up
+  // fallback - either:
+  //    - a fallback value to return if `property` can't be found
+  //    - a function to be executed if `property` can't be found. The function
+  //    will be passed `property` as an argument.
   //
   //  Examples
   //
-  //    var me = { hello: "Hello World" };
+  //    var object = { property: "hello world" };
-  //    var proxy = { boundMethod: function() { return this.hello; } };
+  //    fetch(object, "property");
+  //    //=> "hello world"
   //
-  //    proxy.boundMethod = bind(proxy.boundMethod, me);
+  //    fetch(object, "notaproperty", "default value");
-  //    proxy.boundMethod();
+  //    //=> "default value"
   //
-  //    //=> "Hello World"
+  //    var notFound = function(prop) { return prop + " not found!" };
+  //    fetch(object, "notaproperty", notFound)
+  //    // "notaproperty not found!"
   //
-  // Returns a function wrapper
+  //    var badFallback = function(prop) { prop + " not found!" };
-  bind: function bind(fn, me) {
+  //    fetch(object, "notaproperty", badFallback)
-    return function() {
+  //    // Error: no return value from provided callback function
-      return fn.apply(me, arguments);
+  //
-    };
+  //    fetch(object, "notaproperty");
+  //    // Error: key not found: "notaproperty"
+  //
+  // Returns the value of obj[property], a fallback value, or the results of
+  // running 'fallback'. If the property isn't found, and 'fallback' hasn't been
+  // provided, will throw an error.
+  fetch: function(obj, property, fallback) {
+    if (obj.hasOwnProperty(property)) {
+      return obj[property];
+    }
+
+    if (fallback === void 0) {
+      throw new Error('key not found: "' + property + '"');
+    }
+
+    if (typeof(fallback) === 'function') {
+      var value = fallback(property);
+
+      if (value === void 0) {
+        throw new Error('no return value from provided fallback function');
+      }
+
+      return value;
+    }
+
+    return fallback;
   },
 
   // Public: Adds necessary utils to global namespace, along with base class

test/specs/adaptor.spec.js

@@ -11,10 +11,6 @@ describe("Adaptor", function() {
   var adaptor = new Adaptor({ name: 'adaptor', connection: connection });
 
   describe("#constructor", function() {
-    it("sets @self as a reference to the adaptor", function() {
-      expect(adaptor.self).to.be.eql(adaptor);
-    });
-
     it("sets @name to the provided name", function() {
       expect(adaptor.name).to.be.eql('adaptor');
     });

test/specs/api.spec.js

@@ -2,77 +2,182 @@
 
 var express = require('express'),
     https = require('https'),
-    fs = require('fs');
+    fs = require('fs'),
+    path = require('path');
 
 var API = source('api'),
-    Utils = source('utils');
+    Utils = source('utils'),
+    Logger = source('logger');
+
+var MockRequest = require('../support/mock_request'),
+    MockResponse = require('../support/mock_response');
 
 describe("API", function() {
-  var api, opts;
+  var api;
+
+  beforeEach(function() {
+    api = new API();
+  });
 
   describe("constructor", function() {
+    var mod;
 
     beforeEach(function() {
-      stub(https, 'createServer').returns({ listen: spy() });
+      mod = new API({
-
+        host: "0.0.0.0",
-      api = new API(opts);
+        port: "1234"
+      });
     });
 
-    afterEach(function() {
+    it("sets @express to an Express instance", function() {
-      https.createServer.restore();
+      expect(api.express.listen).to.be.a('function');
+    })
+
+    it("sets default values", function() {
+      var sslPath = path.normalize(__dirname + "/../../lib/api/ssl/");
+
+      expect(api.host).to.be.eql('127.0.0.1');
+      expect(api.port).to.be.eql('3000');
     });
 
-    it("sets @opts to the passed opts object", function() {
+    it("overrides default values if passed to constructor", function() {
-      expect(api.opts).to.be.eql(opts);
+      expect(mod.host).to.be.eql('0.0.0.0');
+      expect(mod.port).to.be.eql('1234');
     });
 
-    it("sets @host to 127.0.0.1 by default", function() {
+    it("sets the server title", function() {
-      expect(api.host).to.be.eql("127.0.0.1")
-    });
-
-    it("sets @post to 3000 by default", function() {
-      expect(api.port).to.be.eql("3000")
-    });
-
-    it("sets @express to an Express server instance", function() {
-      expect(api.express).to.be.a('function');
-
-      var methods = ['get', 'post', 'put', 'delete'];
-
-      for (var i = 0; i < methods.length; i++) {
-        expect(api.express[methods[i]]).to.be.a('function');
-      }
-    });
-
-    it("sets @server.https to a https.createServer", function() {
-      expect(https.createServer).to.be.calledWith();
-    });
-
-    it("sets the server's title", function() {
       var title = api.express.get('title');
-      expect(title).to.be.eql("Cylon API Server");
+      expect(title).to.be.eql('Cylon API Server');
+    });
   });
 
+  describe("default", function() {
+    var d = API.prototype.defaults;
+
+    it("host", function() {
+      expect(d.host).to.be.eql('127.0.0.1');
     });
 
+    it("port", function() {
+      expect(d.port).to.be.eql('3000');
+    });
 
-  describe("ssl disabled", function () {
+    it("auth", function() {
+      expect(d.auth).to.be.eql(false);
+    });
 
+    it("CORS", function() {
+      expect(d.CORS).to.be.eql('');
+    });
+
+    it("ssl", function() {
+      var sslDir = path.normalize(__dirname + "/../../lib/api/ssl/");
+      expect(d.ssl.key).to.be.eql(sslDir + "server.key");
+      expect(d.ssl.cert).to.be.eql(sslDir + "server.crt");
+    });
+  });
+
+  describe("#createServer", function() {
+    it("sets @express to an express server", function() {
+      api.express = null;
+      api.createServer();
+      expect(api.express).to.be.a('function');
+    });
+
+    context("if SSL is configured", function() {
+      it("sets @server to a https.Server instance", function() {
+        api.createServer();
+        expect(api.server).to.be.an.instanceOf(https.Server);
+      });
+    });
+
+    context("if SSL is not configured", function() {
       beforeEach(function() {
-      stub(https, 'createServer').returns({ listen: spy() });
+        api.ssl = false;
-
+        stub(Logger, 'warn');
-      opts = { ssl: false }
+        api.createServer();
-
-      api = new API(opts);
       });
 
       afterEach(function() {
-      https.createServer.restore();
+        Logger.warn.restore();
       });
 
-    it("doesn't create https server", function() {
+      it("logs that the API is insecure", function() {
-      expect(https.createServer).not.to.be.calledWith();
+        expect(Logger.warn).to.be.calledWithMatch("insecure connection")
       });
 
+      it("sets @server to @express", function() {
+        expect(api.server).to.be.eql(api.express);
+      });
+    });
+  });
+
+  describe("#setupAuth", function() {
+    context("when auth.type is basic", function() {
+      beforeEach(function() {
+        api.auth = { type: 'basic', user: 'user', pass: 'pass' }
+      });
+
+      it('returns a basic auth middleware function', function() {
+        var fn = api.setupAuth(),
+            req = new MockRequest(),
+            res = new MockResponse(),
+            next = spy();
+
+        var auth = 'Basic ' + new Buffer('user:pass').toString('base64')
+
+        req.headers.authorization = auth;
+
+        fn(req, res, next);
+        expect(next).to.be.called;
+      });
+    });
+
+    context("when auth is null", function() {
+      beforeEach(function() {
+        api.auth = null;
+      });
+
+      it("returns a pass-through middleware function", function() {
+        var fn = api.setupAuth(),
+            next = spy();
+
+        fn(null, null, next);
+        expect(next).to.be.called;
+      });
+    });
+  });
+
+  describe("#listen", function() {
+    beforeEach(function() {
+      // we create a plain HTTP server to avoid a log message from Node
+      api.ssl = false;
+      api.createServer();
+      api.express.set('title', 'Cylon API Server');
+
+      stub(api.server, 'listen').yields();
+      stub(Logger, 'info');
+
+      api.listen();
+    });
+
+    afterEach(function() {
+      api.server.listen.restore();
+      Logger.info.restore();
+    });
+
+    it("listens on the configured host and port", function() {
+      expect(api.server.listen).to.be.calledWith('3000', '127.0.0.1');
+    });
+
+    context("when the server is running", function() {
+      it("logs that it's online and listening", function() {
+        var online = "Cylon API Server is now online.",
+            listening = "Listening at http://127.0.0.1:3000";
+
+        expect(Logger.info).to.be.calledWith(online);
+        expect(Logger.info).to.be.calledWith(listening);
+      });
+    });
   });
 });

test/specs/connection.spec.js

@@ -13,10 +13,6 @@ describe("Connection", function() {
   var connection = robot.connections.loopback;
 
   describe("#constructor", function() {
-    it("sets @self as a circular reference", function() {
-      expect(connection.self).to.be.eql(connection);
-    });
-
     it("sets @robot to the passed robot", function() {
       expect(connection.robot).to.be.eql(robot);
     });

test/specs/driver.spec.js

@@ -18,10 +18,6 @@ describe("Driver", function() {
   });
 
   describe("#constructor", function() {
-    it("sets @self as a reference to the driver", function() {
-      expect(driver.self).to.be.eql(driver);
-    });
-
     it("sets @name to the provided name", function() {
       expect(driver.name).to.be.eql('driver');
     });

test/specs/utils.spec.js

@@ -164,9 +164,8 @@ describe("Utils", function() {
 
     var TestClass = (function() {
       function TestClass() {
-        this.self = this;
         this.testInstance = new ProxyClass;
-        utils.proxyFunctionsToObject(methods, this.testInstance, this.self, true);
+        utils.proxyFunctionsToObject(methods, this.testInstance, this, true);
       }
 
       return TestClass;
@@ -204,22 +203,51 @@ describe("Utils", function() {
     });
   });
 
-  describe("#bind", function() {
+  describe("#fetch", function() {
-    var me = { hello: "Hello World" },
+    var fetch = utils.fetch,
-        proxy = { boundMethod: function() { return this.hello; } };
+        obj = { property: 'hello world', 'false': false, 'null': null };
 
-    it("binds the 'this' scope for the method", function() {
+    context("if the property exists on the object", function() {
-      proxy.boundMethod = function() { return this.hello; };
+      it("returns the value", function() {
-      proxy.boundMethod = utils.bind(proxy.boundMethod, me);
+        expect(fetch(obj, 'property')).to.be.eql('hello world');
-
+        expect(fetch(obj, 'false')).to.be.eql(false);
-      expect(proxy.boundMethod()).to.eql("Hello World");
+        expect(fetch(obj, 'null')).to.be.eql(null);
-    });
+      });
-
+    });
-    it("passes arguments along to bound functions", function() {
+
-      proxy.boundMethod = function(hello, world) { return [hello, world]; };
+    context("if the property doesn't exist on the object", function() {
-      proxy.boundMethod = utils.bind(proxy.boundMethod, me);
+      context("and no fallback value has been provided", function() {
-
+        it("throws an Error", function() {
-      expect(proxy.boundMethod("Hello", "World")).to.eql(["Hello", "World"]);
+          var fn = function() { return fetch(obj, "notaproperty"); };
-    })
+          expect(fn).to.throw(Error, 'key not found: "notaproperty"');
+        });
+      });
+
+      context("and a fallback value has been provided", function() {
+        it('returns the fallback value', function() {
+          expect(fetch(obj, 'notakey', 'fallback')).to.be.eql('fallback');
+        });
+      });
+
+      context("and a fallback function has been provided", function() {
+        context("if the function has no return value", function() {
+          it("throws an Error", function() {
+            var fn = function() { fetch(obj, 'notakey', function() {}); },
+                str = 'no return value from provided fallback function';
+
+            expect(fn).to.throw(Error, str);
+          });
+        });
+
+        context("if the function returns a value", function() {
+          it("returns the value returned by the fallback function", function() {
+            var fn = function(key) { return "Couldn't find " + key },
+                value = "Couldn't find notakey";
+
+            expect(fetch(obj, 'notakey', fn)).to.be.eql(value);
+          });
+        });
+      });
+    });
   });
 });