Merge pull request #225 from hybridgroup/fix/name-collisions

Address namespace collisions for robots, devices, and connections

lib/cylon.js

@@ -47,6 +47,13 @@ var Cylon = module.exports = {
 //     work: (me) ->
 //       me.led.toggle()
 Cylon.robot = function robot(opts) {
+  // check if a robot with the same name exists already
+  if (opts.name && this.robots[opts.name]) {
+    var original = opts.name;
+    opts.name = Utils.makeUnique(original, Object.keys(this.robots));
+    Logger.warn("Robot names must be unique. Renaming '" + original + "' to '" + opts.name + "'");
+  }
+
   var robot = new Robot(opts);
   this.robots[robot.name] = robot;
   return robot;

lib/robot.js

@@ -180,6 +180,12 @@ Robot.prototype.initConnections = function(connections) {
   connections = [].concat(connections);
 
   connections.forEach(function(conn) {
+    if (this.connections[conn.name]) {
+      var original = conn.name;
+      conn.name = Utils.makeUnique(original, Object.keys(this.connections));
+      Logger.warn("Connection names must be unique. Renaming '" + original + "' to '" + conn.name + "'");
+    }
+
     Logger.info("Initializing connection '" + conn.name + "'.");
     conn['robot'] = this;
     this.connections[conn.name] = new Connection(conn);
@@ -203,6 +209,12 @@ Robot.prototype.initDevices = function(devices) {
   devices = [].concat(devices);
 
   devices.forEach(function(device) {
+    if (this.devices[device.name]) {
+      var original = device.name;
+      device.name = Utils.makeUnique(original, Object.keys(this.devices));
+      Logger.warn("Device names must be unique. Renaming '" + original + "' to '" + device.name + "'");
+    }
+
     Logger.info("Initializing device '" + device.name + "'.");
     device['robot'] = this;
     this.devices[device.name] = new Device(device);

lib/utils.js

@@ -192,6 +192,28 @@ var Utils = module.exports = {
     return fallback;
   },
 
+  // Public: Given a name, and an array of existing names, returns a unique
+  // name.
+  //
+  // name - name that's colliding with existing names
+  // arr - array of existing names
+  //
+  // Returns the new name as a string
+  makeUnique: function(name, arr) {
+    var newName;
+
+    if (!~arr.indexOf(name)) {
+      return name;
+    }
+
+    for (var n = 1; ; n++) {
+      newName = name + "-" + n;
+      if (!~arr.indexOf(newName)) {
+        return newName;
+      }
+    }
+  },
+
   // Public: Adds necessary utils to global namespace, along with base class
   // extensions.
   //

test/specs/cylon.spec.js

@@ -48,6 +48,14 @@ describe("Cylon", function() {
       expect(robot.toString()).to.be.eql("[Robot name='Ultron']")
       expect(Cylon.robots['Ultron']).to.be.eql(robot);
     });
+
+    it("avoids duplicating names", function() {
+      Cylon.robot({ name: "Ultron" })
+      Cylon.robot({ name: "Ultron" })
+
+      var bots = Object.keys(Cylon.robots);
+      expect(bots).to.be.eql(["Ultron", "Ultron-1"])
+    });
   });
 
   describe("#api", function() {

test/specs/robot.spec.js

@@ -205,54 +205,86 @@ describe("Robot", function() {
   });
 
   describe("initConnections", function() {
+    var bot;
+
+    beforeEach(function() {
+      bot = new Robot();
+    });
+
     context("when not passed anything", function() {
       it("returns immediately", function() {
-        expect(robot.initConnections()).to.be.eql(undefined);
+        expect(bot.initConnections()).to.be.eql(undefined);
       });
     });
 
     context("when passed a connection object", function() {
       it("instantiates a new connection with the provided object", function() {
         var connection = { name: 'loopback', adaptor: 'loopback' };
-        robot.initConnections(connection);
-        expect(robot.connections['loopback']).to.be.instanceOf(Connection);
+        bot.initConnections(connection);
+        expect(bot.connections['loopback']).to.be.instanceOf(Connection);
       });
     });
 
     context("when passed an array of connection objects", function() {
       it("instantiates a new connection with each of the provided objects", function() {
         var connections = [{ name: 'loopback', adaptor: 'loopback' }]
-        robot.initConnections(connections);
-        expect(robot.connections['loopback']).to.be.instanceOf(Connection);
+        bot.initConnections(connections);
+        expect(bot.connections['loopback']).to.be.instanceOf(Connection);
+      });
+
+      it("avoids name collisions collisions", function() {
+        bot.initConnections([
+          { name: 'loopback', adaptor: 'loopback' },
+          { name: 'loopback', adaptor: 'loopback' }
+        ]);
+
+        var keys = Object.keys(bot.connections);
+        expect(keys).to.be.eql(["loopback", "loopback-1"]);
       });
     });
   });
 
   describe("initDevices", function() {
+    var bot;
+
+    beforeEach(function() {
+      bot = new Robot();
+    });
+
     context("when not passed anything", function() {
       it("returns immediately", function() {
-        expect(robot.initDevices()).to.be.eql(undefined);
+        expect(bot.initDevices()).to.be.eql(undefined);
       });
     });
 
     context("when passed a connection object", function() {
-      afterEach(function() { robot.devices = {}; });
+      afterEach(function() { bot.devices = {}; });
 
       it("instantiates a new device with the provided object", function() {
         var device = { name: 'ping', driver: 'ping' };
-        robot.initDevices(device);
-        expect(robot.devices['ping']).to.be.instanceOf(Device);
+        bot.initDevices(device);
+        expect(bot.devices['ping']).to.be.instanceOf(Device);
       });
     });
 
     context("when passed an array of device objects", function() {
-      afterEach(function() { robot.devices = {}; });
+      afterEach(function() { bot.devices = {}; });
 
       it("instantiates a new device with each of the provided objects", function() {
         var devices = [{ name: 'ping', driver: 'ping' }]
-        robot.initDevices(devices);
+        bot.initDevices(devices);
 
-        expect(robot.devices['ping']).to.be.instanceOf(Device);
+        expect(bot.devices['ping']).to.be.instanceOf(Device);
+      });
+
+      it("avoids name collisions collisions", function() {
+        bot.initDevices([
+          { name: 'ping', driver: 'ping' },
+          { name: 'ping', driver: 'ping' }
+        ]);
+
+        var keys = Object.keys(bot.devices);
+        expect(keys).to.be.eql(["ping", "ping-1"]);
       });
     });
   });

test/specs/utils.spec.js

@@ -60,6 +60,23 @@ describe("Utils", function() {
     });
   });
 
+  describe("#makeUnique", function() {
+    it("returns the original name if it's not a conflict", function() {
+      var res = utils.makeUnique("hello", ["world"]);
+      expect(res).to.be.eql("hello");
+    });
+
+    it("generates a unique name if it does collide", function() {
+      var res = utils.makeUnique("hello", ["hello"]);
+      expect(res).to.be.eql("hello-1");
+    });
+
+    it("will ignore existing duplicates", function() {
+      var res = utils.makeUnique("hello", ["hello", "hello-1", "hello-2"]);
+      expect(res).to.be.eql("hello-3");
+    });
+  });
+
   describe("#every", function() {
     beforeEach(function() {
       this.clock = sinon.useFakeTimers();