Repository: basho/riak-nodejs-client
Pull request #97 (2015-09-15 14:50:48)

Some code refactoring and optimizations
See original PR #94 

Tested on Ubuntu 14 trusty.
Updated README with correct git command for submodule initialization.
Allow run a specific unit-test suite i.e. $ make unit-test SUITE=core
Added more tests for RiakCluster.removeNode
Refactored RiakCluster.removeNode for separation of concerns and better readability.
Pass jshint checks.


Patch:
diff --git a/Gruntfile.js b/Gruntfile.js
index 707fd06..f4ad51a 100644
--- a/Gruntfile.js
+++ b/Gruntfile.js
@@ -75,7 +75,13 @@ module.exports = function(grunt) {
   grunt.loadNpmTasks('grunt-benchmark');
 
   grunt.registerTask('lint', 'jshint');
-  grunt.registerTask('unit', ['jshint', 'mochaTest:unit']);
+  grunt.registerTask('unit', function (testSuite) {
+    testSuite = testSuite.trim();
+    if (testSuite !== '') {
+      grunt.config.set('mochaTest.unit.src', ['test/unit/' + testSuite + '/*.js']);
+    }
+    grunt.task.run(['jshint', 'mochaTest:unit']);
+  });
   grunt.registerTask('integration', ['jshint', 'mochaTest:integration']);
   grunt.registerTask('security', ['jshint', 'mochaTest:security']);
   grunt.registerTask('default', ['jshint', 'mochaTest:unit', 'mochaTest:integration']);
diff --git a/README.md b/README.md
index a3b0b69..36291a3 100644
--- a/README.md
+++ b/README.md
@@ -32,7 +32,7 @@ git clone --recursive https://github.com/basho/riak-nodejs-client
 # OR:
 
 git clone https://github.com/basho/riak-nodejs-client
-git submodule init --update
+git submodule update --init --recursive
 ```
 
 This repository's maintainers are engineers at Basho and we welcome your contribution to the project! Review the details in [CONTRIBUTING.md](CONTRIBUTING.md) in order to give back to this project.
diff --git a/build/common.mk b/build/common.mk
index a885d15..d851466 100644
--- a/build/common.mk
+++ b/build/common.mk
@@ -12,7 +12,7 @@ lint:
 	$(GRUNT) lint
 
 unit-test:
-	$(GRUNT) unit
+	$(GRUNT) unit:$(SUITE)
 
 integration-test:
 	$(GRUNT) integration
@@ -29,13 +29,13 @@ publish:
 help:
 	@echo ''
 	@echo ' Targets:'
-	@echo '--------------------------------------------------'
-	@echo ' all              - Run everything                '
-	@echo ' lint             - Run jshint                    '
-	@echo ' install-deps     - Install required dependencies '
-	@echo ' test             - Run unit & integration tests  '
-	@echo ' unit-test        - Run unit tests                '
-	@echo ' integration-test - Run integration tests         '
-	@echo ' security-test    - Run security tests            '
-	@echo '--------------------------------------------------'
+	@echo '--------------------------------------------------------------'
+	@echo ' all                          - Run everything                '
+	@echo ' lint                         - Run jshint                    '
+	@echo ' install-deps                 - Install required dependencies '
+	@echo ' test                         - Run unit & integration tests  '
+	@echo ' unit-test [SUITE=suite_name] - Run unit tests                '
+	@echo ' integration-test             - Run integration tests         '
+	@echo ' security-test                - Run security tests            '
+	@echo '--------------------------------------------------------------'
 	@echo ''
diff --git a/build/install-deps b/build/install-deps
index 93d9151..08b86cc 100755
--- a/build/install-deps
+++ b/build/install-deps
@@ -20,9 +20,9 @@ fi
 if [[ -f /etc/lsb-release ]]
 then
     source /etc/lsb-release
-    if [[ $DISTRIB_CODENAME != 'precise' ]]
+    if [[ $DISTRIB_CODENAME != 'precise' ]] && [[ $DISTRIB_CODENAME != 'trusty' ]]
     then
-        echo '[error] Only tested on Ubuntu 12 precise' 2>&1
+        echo '[error] Only tested on Ubuntu 12 precise and Ubuntu 14 trusty' 2>&1
         exit 1
     fi
 else
diff --git a/lib/core/riakcluster.js b/lib/core/riakcluster.js
index 1b15e6f..d634169 100644
--- a/lib/core/riakcluster.js
+++ b/lib/core/riakcluster.js
@@ -253,6 +253,32 @@ RiakCluster.prototype.addNode = function(node) {
     this.nodes.push(node);
 };
 
+/**
+* Get a RiakNode index from this cluster.
+* @method getNodeIndex
+* @param {RiakNode|String} node - the node for getting the index. May be supplied as a RiakNode instance or IP|hostname[:port]
+* @return {RiakNode} - The node index in this cluster. Returns -1 if the node does not exist.
+*/
+RiakCluster.prototype.getNodeIndex = function(node) {
+    if (node instanceof RiakNode) {
+        return this.nodes.indexOf(node);
+    }
+
+    // Hopefully it's a string "addr[:port]"
+    var split = node.split(':');
+    var addr = split[0];
+    var port = (split[1])? Number(split[1]) : undefined;
+    for (var i = 0; i < this.nodes.length; i++) {
+        var n = this.nodes[i];
+        if (n.remoteAddress === addr) {
+            if (port === undefined || n.remotePort === port) {
+                return i;
+            }
+        }
+    }
+    return -1;
+};
+
 /**
  * Remove a RiakNode from this cluster.
  * The node being removed will also be stopped.
@@ -261,36 +287,14 @@ RiakCluster.prototype.addNode = function(node) {
  * @return {Boolean} - true if the node was removed.
  */
 RiakCluster.prototype.removeNode = function(node) {
-
-    for (var i = 0; i < this.nodes.length; i++) {
-        if (node instanceof RiakNode) {
-            if (node === this.nodes[i]) {
-                this.nodes.splice(i,1);
-                node.stop();
-                return true;
-            }
-        } else {
-            // Hopefully it's a string "addr[:port]"
-            var split = node.split(':');
-            if (this.nodes[i].remoteAddress === split[0]) {
-                var removed;
-                if (split.length === 2) {
-                    if (this.nodes[i].remotePort === Number(split[1])) {
-                        removed = this.nodes.splice(i,1);
-                        removed[0].stop();
-                        return true;
-                    }
-                } else {
-                    removed = this.nodes.splice(i,1);
-                    removed[0].stop();
-                    return true;
-                }
-            }
-        }
+    var index = this.getNodeIndex(node);
+    var n = this.nodes[index];
+    if (n) {
+        this.nodes.splice(index, 1);
+        n.stop();
+        return true;
     }
-
     return false;
-
 };
 
 RiakCluster.prototype._onNodeStateChange = function(node, state) {


Test patch:
diff --git a/test/unit/core/riakcluster.js b/test/unit/core/riakcluster.js
index 7b18fe4..04e07c4 100644
--- a/test/unit/core/riakcluster.js
+++ b/test/unit/core/riakcluster.js
@@ -44,7 +44,22 @@ describe('RiakCluster', function() {
             done();
         });
     });
-    describe('Test removing node while executing command', function() {
+    describe('Test getting a node index', function () {
+        it('getting some nodes index', function (done) {
+            var nodeAddrs = ['192.168.1.1:8087', '192.168.1.2:8087', '192.168.1.3:8087'];
+            var arrayOfNodes = RiakNode.buildNodes(nodeAddrs);
+            var myCluster = new RiakCluster.Builder().withRiakNodes(arrayOfNodes).build();
+            var node = arrayOfNodes[0];
+            assert.equal(myCluster.getNodeIndex(node),0);
+            assert.equal(myCluster.getNodeIndex('192.168.1.2'),1);
+            assert.equal(myCluster.getNodeIndex('192.168.1.3:8087'),2);
+            assert.equal(myCluster.getNodeIndex('192.168.1.3:1234'),-1);
+            assert.equal(myCluster.getNodeIndex('192.168.1.11'),-1);
+            done();
+        });
+    });
+
+    describe('Test removing node', function() {
         it('removing node while executing command should not throw exception', function(done) {
             var nodeTemplate = new RiakNode.Builder().withMinConnections(10);
             var nodeAddys = ['192.168.1.1:8087', '192.168.1.2:8087', '192.168.1.3:8087', '192.168.1.4:8087'];
@@ -80,5 +95,68 @@ describe('RiakCluster', function() {
             assert.equal(myCluster.nodeManager._nodeIndex, 1);
             done();
         });
+        it('removing a node passing an existing RiakNode instance should return true', function (done) {
+            var nodeAddrs = ['192.168.1.1:8087', '192.168.1.2:8087', '192.168.1.3:8087'];
+            var arrayOfNodes = RiakNode.buildNodes(nodeAddrs);
+            var myCluster = new RiakCluster.Builder().withRiakNodes(arrayOfNodes).build();
+            myCluster.nodes.forEach(function (n) {
+                n.state = RiakNode.State.RUNNING;
+                n.execute = function() {
+                    return true;
+                };
+            });
+            var node = arrayOfNodes[0];
+            assert(myCluster.removeNode(node));
+            assert.equal(node.state, RiakNode.State.SHUTDOWN);
+            assert.equal(myCluster.nodes.length, 2);
+            assert.equal(myCluster.nodes[0].remoteAddress, '192.168.1.2');
+            assert.equal(myCluster.nodes[0].remotePort, 8087);
+            assert.equal(myCluster.nodes[1].remoteAddress, '192.168.1.3');
+            assert.equal(myCluster.nodes[1].remotePort, 8087);
+            done();
+        });
+        it ('removing a node passing an existing addr:[host] string should return true', function (done) {
+            var nodeAddrs = ['192.168.1.1:8087', '192.168.1.2:8087', '192.168.1.3:8087'];
+            var arrayOfNodes = RiakNode.buildNodes(nodeAddrs);
+            var myCluster = new RiakCluster.Builder().withRiakNodes(arrayOfNodes).build();
+            myCluster.nodes.forEach(function (n) {
+                n.state = RiakNode.State.RUNNING;
+                n.execute = function() {
+                    return true;
+                };
+            });
+            assert(myCluster.removeNode('192.168.1.2'));
+            assert.equal(arrayOfNodes[1].state, RiakNode.State.SHUTDOWN);
+            assert.equal(myCluster.nodes.length, 2);
+            assert(myCluster.removeNode('192.168.1.1:8087'));
+            assert.equal(arrayOfNodes[0].state, RiakNode.State.SHUTDOWN);
+            assert.equal(myCluster.nodes.length, 1);
+            assert.equal(myCluster.nodes[0].remoteAddress, '192.168.1.3');
+            assert.equal(myCluster.nodes[0].remotePort, 8087);
+            assert.equal(arrayOfNodes[2].state, RiakNode.State.RUNNING);
+            done();
+        });
+        it ('removing a node passing a non-existing addr:[host] should return false', function (done) {
+            var nodeAddrs = ['192.168.1.1:8087', '192.168.1.2:8087'];
+            var arrayOfNodes = RiakNode.buildNodes(nodeAddrs);
+            var myCluster = new RiakCluster.Builder().withRiakNodes(arrayOfNodes).build();
+            myCluster.nodes.forEach(function (n) {
+                n.state = RiakNode.State.RUNNING;
+                n.execute = function() {
+                    return true;
+                };
+            });
+            assert(!myCluster.removeNode('192.168.1.11'));
+            assert.equal(myCluster.nodes.length, 2);
+            assert(!myCluster.removeNode('192.168.1.1:1234'));
+            assert.equal(myCluster.nodes.length, 2);
+            assert.equal(myCluster.nodes[0].remoteAddress, '192.168.1.1');
+            assert.equal(myCluster.nodes[0].remotePort, 8087);
+            assert.equal(myCluster.nodes[0].state, RiakNode.State.RUNNING);
+            assert.equal(myCluster.nodes[1].remoteAddress, '192.168.1.2');
+            assert.equal(myCluster.nodes[1].remotePort, 8087);
+            assert.equal(myCluster.nodes[1].state, RiakNode.State.RUNNING);
+            done();
+        });
     });
 });