Merge pull request #515 from anna2130/nr-cli-enhancements

No-op and return success on enabling/disabling node by type name

red/api/nodes.js

@@ -142,10 +142,8 @@ module.exports = {
             var info;
             if (!node) {
                 res.send(404);
-            } else if (!node.err && node.enabled === body.enabled) {
-                res.json(node);
             } else {
-                res.json(putNode(id, body.enabled));
+                res.json(putNode(node, body.enabled));
             }
         } catch(err) {
             res.send(400,err.toString());
@@ -169,7 +167,7 @@ module.exports = {
                 var matching = getMatchingNodes(mod);
                 if (matching.length === 1) {
                     // One match, assume correct
-                    res.json(putNode(matching[0].id, body.enabled));
+                    res.json(putNode(matching[0], body.enabled));
                     return;
                 } else if (matching.length > 1) {
                     // Multiple matches, need clarification
@@ -229,24 +227,28 @@ function getMatchingNodes(node) {
     return matching;
 }
 
-function putNode(id, enabled) {
+function putNode(node, enabled) {
     var info;
 
-    if (enabled) {
+    if (!node.err && node.enabled === enabled) {
-        info = redNodes.enableNode(id);
+        info = node;
     } else {
-        info = redNodes.disableNode(id);
+        if (enabled) {
-    }
+            info = redNodes.enableNode(node.id);
-
+        } else {
-    if (info.enabled === enabled && !info.err) {
+            info = redNodes.disableNode(node.id);
-        comms.publish("node/"+(enabled?"enabled":"disabled"),info,false);
+        }
-        util.log("[red] "+(enabled?"Enabled":"Disabled")+" node types:");
+
-        for (var i=0;i<info.types.length;i++) {
+        if (info.enabled === enabled && !info.err) {
-            util.log("[red] - "+info.types[i]);
+            comms.publish("node/"+(enabled?"enabled":"disabled"),info,false);
+            util.log("[red] "+(enabled?"Enabled":"Disabled")+" node types:");
+            for (var i=0;i<info.types.length;i++) {
+                util.log("[red] - "+info.types[i]);
+            }
+        } else if (enabled && info.err) {
+            util.log("[red] Failed to enable node:");
+            util.log("[red] - "+info.name+" : "+info.err);
         }
-    } else if (enabled && info.err) {
-        util.log("[red] Failed to enable node:");
-        util.log("[red] - "+info.name+" : "+info.err);
     }
 
     return info;

test/red/api/nodes_spec.js

@@ -126,6 +126,7 @@ describe("nodes api", function() {
                 .expect(200)
                 .end(function(err,res) {
                     getNodeInfo.restore();
+                    getModuleVersion.restore();
                     if (err) {
                         throw err;
                     }
@@ -812,6 +813,63 @@ describe("nodes api", function() {
             });
         });
 
+        describe('no-ops if a node specified by type name already in the right state', function() {
+            function run(state,done) {
+                var node = {id:"123",enabled:state,types:['a']};
+                var settingsAvailable = sinon.stub(settings,'available', function() {
+                    return true;
+                });
+                var getModuleInfo = sinon.stub(redNodes,'getModuleInfo',function(id) {
+                    return null;
+                });
+                var enableNode = sinon.stub(redNodes,'enableNode',function(id) {
+                    node.enabled = true;
+                    return node;
+                });
+                var disableNode = sinon.stub(redNodes,'disableNode',function(id) {
+                    node.enabled = false;
+                    return node;
+                });
+                var getNodeList = sinon.stub(redNodes,'getNodeList', function() {
+                    return [{name: 'foo', enabled: state}, {name: 'bar'}];
+                });
+                var getModuleVersion = sinon.stub(redNodes,'getModuleVersion', function() {
+                    return '0.0.1';
+                });
+
+                request(app)
+                    .put('/nodes/foo')
+                    .send({enabled:state})
+                    .expect(200)
+                    .end(function(err,res) {
+                        settingsAvailable.restore();
+                        getModuleInfo.restore();
+                        var enableNodeCalled = enableNode.called;
+                        var disableNodeCalled = disableNode.called;
+                        enableNode.restore();
+                        disableNode.restore();
+                        getNodeList.restore();
+                        getModuleVersion.restore();
+                        if (err) {
+                            throw err;
+                        }
+                        enableNodeCalled.should.be.false;
+                        disableNodeCalled.should.be.false;
+                        res.body.should.have.property("name","foo");
+                        res.body.should.have.property("version");
+                        res.body.should.have.property("enabled",state);
+
+                        done();
+                    });
+            }
+            it('already enabled', function(done) {
+                run(true,done);
+            });
+            it('already disabled', function(done) {
+                run(false,done);
+            });
+        });
+
         describe('does not no-op if err on a node in module', function() {
             function run(state,done) {
                 var node = {id:"123",enabled:state,types:['a'],err:"foo"};