Merge pull request #410 from weaveworks/immutable

Use immutable objects for storing nodes in browser

client/app/scripts/charts/nodes-chart.js

@@ -43,14 +43,13 @@ const NodesChart = React.createClass({
   },
 
   componentWillReceiveProps: function(nextProps) {
-    if (this.getTopologyFingerprint(nextProps.nodes) !== this.getTopologyFingerprint(this.props.nodes)) {
+    if (nextProps.nodes !== this.props.nodes) {
       this.setState({
         nodes: {},
         edges: {}
       });
+      this.updateGraphState(nextProps);
     }
-
-    this.updateGraphState(nextProps);
   },
 
   componentWillUnmount: function() {
@@ -130,7 +129,7 @@ const NodesChart = React.createClass({
     const centerY = this.props.height / 2;
     const nodes = {};
 
-    _.each(topology, function(node, id) {
+    topology.forEach(function(node, id) {
       nodes[id] = prevNodes[id] || {};
 
       // use cached positions if available
@@ -141,14 +140,13 @@ const NodesChart = React.createClass({
 
       // copy relevant fields to state nodes
       _.assign(nodes[id], {
-        adjacency: node.adjacency,
         id: id,
-        label: node.label_major,
-        pseudo: node.pseudo,
-        subLabel: node.label_minor,
-        degree: _.size(node.adjacency)
+        label: node.get('label_major'),
+        pseudo: node.get('pseudo'),
+        subLabel: node.get('label_minor'),
+        rank: node.get('rank')
       });
-    }, this);
+    });
 
     return nodes;
   },
@@ -156,34 +154,37 @@ const NodesChart = React.createClass({
   initEdges: function(topology, nodes) {
     const edges = {};
 
-    _.each(topology, function(node) {
-      _.each(node.adjacency, function(adjacent) {
-        const edge = [node.id, adjacent];
-        const edgeId = edge.join(Naming.EDGE_ID_SEPARATOR);
+    topology.forEach(function(node, nodeId) {
+      const adjacency = node.get('adjacency');
+      if (adjacency) {
+        adjacency.forEach(function(adjacent) {
+          const edge = [nodeId, adjacent];
+          const edgeId = edge.join(Naming.EDGE_ID_SEPARATOR);
 
-        if (!edges[edgeId]) {
-          const source = nodes[edge[0]];
-          const target = nodes[edge[1]];
+          if (!edges[edgeId]) {
+            const source = nodes[edge[0]];
+            const target = nodes[edge[1]];
 
-          if (!source || !target) {
-            debug('Missing edge node', edge[0], source, edge[1], target);
+            if (!source || !target) {
+              debug('Missing edge node', edge[0], source, edge[1], target);
+            }
+
+            edges[edgeId] = {
+              id: edgeId,
+              value: 1,
+              source: source,
+              target: target
+            };
           }
-
-          edges[edgeId] = {
-            id: edgeId,
-            value: 1,
-            source: source,
-            target: target
-          };
-        }
-      });
-    }, this);
+        });
+      }
+    });
 
     return edges;
   },
 
   updateGraphState: function(props) {
-    const n = _.size(props.nodes);
+    const n = props.nodes.size;
 
     if (n === 0) {
       return;

client/app/scripts/components/__tests__/node-details-test.js

@@ -0,0 +1,31 @@
+describe('NodeDetails', () => {
+  let NodeDetails;
+  let nodes;
+  let nodeId;
+  let details;
+  const React = require('react');
+  const Immutable = require('immutable');
+  const TestUtils = require('react/lib/ReactTestUtils');
+
+  beforeEach(() => {
+    NodeDetails = require('../node-details.js');
+    nodes = Immutable.OrderedMap();
+    nodeId = 'n1';
+  });
+
+  it('shows n/a when node was not found', () => {
+    const c = TestUtils.renderIntoDocument(<NodeDetails nodes={nodes} nodeId={nodeId} />);
+    const notFound = TestUtils.findRenderedDOMComponentWithClass(c, 'node-details-header-notavailable');
+    expect(notFound).toBeDefined();
+  });
+
+  it('show label of node with title', () => {
+    nodes = nodes.set(nodeId, Immutable.fromJS({id: nodeId}));
+    details = {label_major: 'Node 1', tables: []};
+    const c = TestUtils.renderIntoDocument(<NodeDetails nodes={nodes} nodeId={nodeId} details={details} />);
+
+    const title = TestUtils.findRenderedDOMComponentWithClass(c, 'node-details-header-label');
+    expect(title.getDOMNode().textContent).toBe('Node 1');
+  });
+
+});

client/app/scripts/components/node-details.js

@@ -47,7 +47,7 @@ const NodeDetails = React.createClass({
 
   render: function() {
     const details = this.props.details;
-    const nodeExists = this.props.nodes[this.props.nodeId];
+    const nodeExists = this.props.nodes && this.props.nodes.has(this.props.nodeId);
 
     if (!nodeExists) {
       return this.renderNotAvailable();
@@ -58,7 +58,7 @@ const NodeDetails = React.createClass({
     }
 
     const style = {
-      'background-color': this.getNodeColorDark(details.label_major)
+      'backgroundColor': this.getNodeColorDark(details.label_major)
     };
 
     return (

client/app/scripts/stores/__tests__/app-store-test.js

@@ -7,7 +7,24 @@ describe('AppStore', function() {
 
   // fixtures
 
-  const NODE_SET = {n1: {id: 'n1'}, n2: {id: 'n2'}};
+  const NODE_SET = {
+    n1: {
+      id: 'n1',
+      rank: undefined,
+      adjacency: undefined,
+      pseudo: undefined,
+      label_major: undefined,
+      label_minor: undefined
+    },
+    n2: {
+      id: 'n2',
+      rank: undefined,
+      adjacency: undefined,
+      pseudo: undefined,
+      label_major: undefined,
+      label_minor: undefined
+    }
+  };
 
   // actions
 
@@ -113,7 +130,7 @@ describe('AppStore', function() {
 
   it('shows nodes that were received', function() {
     registeredCallback(ReceiveNodesDeltaAction);
-    expect(AppStore.getNodes()).toEqual(NODE_SET);
+    expect(AppStore.getNodes().toJS()).toEqual(NODE_SET);
   });
 
   it('gets selected node after click', function() {
@@ -121,11 +138,11 @@ describe('AppStore', function() {
 
     registeredCallback(ClickNodeAction);
     expect(AppStore.getSelectedNodeId()).toBe('n1');
-    expect(AppStore.getNodes()).toEqual(NODE_SET);
+    expect(AppStore.getNodes().toJS()).toEqual(NODE_SET);
 
     registeredCallback(HitEscAction)
     expect(AppStore.getSelectedNodeId()).toBe(null);
-    expect(AppStore.getNodes()).toEqual(NODE_SET);
+    expect(AppStore.getNodes().toJS()).toEqual(NODE_SET);
   });
 
   it('keeps showing nodes on navigating back after node click', function() {
@@ -144,7 +161,7 @@ describe('AppStore', function() {
     RouteAction.state = {"topologyId":"topo1","selectedNodeId": null};
     registeredCallback(RouteAction);
     expect(AppStore.getSelectedNodeId()).toBe(null);
-    expect(AppStore.getNodes()).toEqual(NODE_SET);
+    expect(AppStore.getNodes().toJS()).toEqual(NODE_SET);
   });
 
   it('closes details when changing topologies', function() {
@@ -168,17 +185,17 @@ describe('AppStore', function() {
 
   it('resets topology on websocket reconnect', function() {
     registeredCallback(ReceiveNodesDeltaAction);
-    expect(AppStore.getNodes()).toEqual(NODE_SET);
+    expect(AppStore.getNodes().toJS()).toEqual(NODE_SET);
 
     registeredCallback(CloseWebsocketAction);
     expect(AppStore.isWebsocketClosed()).toBeTruthy();
-    expect(AppStore.getNodes()).toEqual(NODE_SET);
+    expect(AppStore.getNodes().toJS()).toEqual(NODE_SET);
 
     registeredCallback(OpenWebsocketAction);
     expect(AppStore.isWebsocketClosed()).toBeFalsy();
 
     registeredCallback(ReceiveEmptyNodesDeltaAction);
-    expect(AppStore.getNodes()).toEqual({});
+    expect(AppStore.getNodes().toJS()).toEqual({});
   });
 
 

client/app/scripts/stores/app-store.js

@@ -2,11 +2,14 @@ const EventEmitter = require('events').EventEmitter;
 const _ = require('lodash');
 const assign = require('object-assign');
 const debug = require('debug')('scope:app-store');
+const Immutable = require('immutable');
 
 const AppDispatcher = require('../dispatcher/app-dispatcher');
 const ActionTypes = require('../constants/action-types');
 const Naming = require('../constants/naming');
 
+const makeOrderedMap = Immutable.OrderedMap;
+
 // Helpers
 
 function findCurrentTopology(subTree, topologyId) {
@@ -27,6 +30,17 @@ function findCurrentTopology(subTree, topologyId) {
   return foundTopology;
 }
 
+function makeNode(node) {
+  return {
+    id: node.id,
+    label_major: node.label_major,
+    label_minor: node.label_minor,
+    rank: node.rank,
+    pseudo: node.pseudo,
+    adjacency: node.adjacency
+  };
+}
+
 // Initial values
 
 let currentTopologyId = 'containers';
@@ -34,7 +48,7 @@ let errorUrl = null;
 let version = '';
 let mouseOverEdgeId = null;
 let mouseOverNodeId = null;
-let nodes = {};
+let nodes = makeOrderedMap();
 let nodeDetails = null;
 let selectedNodeId = null;
 let topologies = [];
@@ -76,15 +90,17 @@ const AppStore = assign({}, EventEmitter.prototype, {
   getHighlightedEdgeIds: function() {
     if (mouseOverNodeId) {
       // all neighbour combinations because we dont know which direction exists
-      const node = nodes[mouseOverNodeId];
-      return _.flatten(
-        _.map(node.adjacency, function(nodeId) {
-          return [
-            [nodeId, mouseOverNodeId].join(Naming.EDGE_ID_SEPARATOR),
-            [mouseOverNodeId, nodeId].join(Naming.EDGE_ID_SEPARATOR)
-          ];
-        })
-      );
+      const adjacency = nodes.get(mouseOverNodeId).get('adjacency');
+      if (adjacency) {
+        return _.flatten(
+          adjacency.forEach(function(nodeId) {
+            return [
+              [nodeId, mouseOverNodeId].join(Naming.EDGE_ID_SEPARATOR),
+              [mouseOverNodeId, nodeId].join(Naming.EDGE_ID_SEPARATOR)
+            ];
+          })
+        );
+      }
     }
     if (mouseOverEdgeId) {
       return mouseOverEdgeId;
@@ -94,8 +110,10 @@ const AppStore = assign({}, EventEmitter.prototype, {
 
   getHighlightedNodeIds: function() {
     if (mouseOverNodeId) {
-      const node = nodes[mouseOverNodeId];
-      return _.union(node.adjacency, [mouseOverNodeId]);
+      const adjacency = nodes.get(mouseOverNodeId).get('adjacency');
+      if (adjacency) {
+        return _.union(adjacency.toJS(), [mouseOverNodeId]);
+      }
     }
     if (mouseOverEdgeId) {
       return mouseOverEdgeId.split(Naming.EDGE_ID_SEPARATOR);
@@ -152,12 +170,13 @@ AppStore.registeredCallback = function(payload) {
       selectedNodeId = null;
       if (payload.topologyId !== currentTopologyId) {
         currentTopologyId = payload.topologyId;
-        nodes = {};
+        nodes = nodes.clear();
       }
       AppStore.emit(AppStore.CHANGE_EVENT);
       break;
 
     case ActionTypes.CLOSE_WEBSOCKET:
+      nodes = nodes.clear();
       websocketClosed = true;
       AppStore.emit(AppStore.CHANGE_EVENT);
       break;
@@ -221,20 +240,20 @@ AppStore.registeredCallback = function(payload) {
         if (mouseOverNodeId === nodeId) {
           mouseOverNodeId = null;
         }
-        if (nodes[nodeId] && _.contains(mouseOverEdgeId, nodeId)) {
+        if (nodes.has(nodeId) && _.contains(mouseOverEdgeId, nodeId)) {
           mouseOverEdgeId = null;
         }
-        delete nodes[nodeId];
+        nodes = nodes.delete(nodeId);
       });
 
       // update existing nodes
       _.each(payload.delta.update, function(node) {
-        nodes[node.id] = node;
+        nodes = nodes.set(node.id, nodes.get(node.id).mergeDeep(makeNode(node)));
       });
 
       // add new nodes
       _.each(payload.delta.add, function(node) {
-        nodes[node.id] = node;
+        nodes = nodes.set(node.id, Immutable.fromJS(makeNode(node)));
       });
 
       AppStore.emit(AppStore.CHANGE_EVENT);
@@ -254,7 +273,7 @@ AppStore.registeredCallback = function(payload) {
 
     case ActionTypes.ROUTE_TOPOLOGY:
       if (currentTopologyId !== payload.state.topologyId) {
-        nodes = {};
+        nodes = nodes.clear();
       }
       currentTopologyId = payload.state.topologyId;
       selectedNodeId = payload.state.selectedNodeId;

client/package.json

@@ -12,6 +12,7 @@
     "flux": "^2.0.3",
     "font-awesome": "^4.3.0",
     "font-awesome-webpack": "0.0.3",
+    "immutable": "^3.7.4",
     "keymirror": "^0.1.1",
     "lodash": "~3.9.3",
     "material-ui": "~0.7.5",

client/test/karma.conf.js

@@ -4,6 +4,7 @@ module.exports = function(config) {
       'PhantomJS'
     ],
     files: [
+      './polyfill.js',
       {
         pattern: 'tests.webpack.js',
         watched: false

client/test/polyfill.js

@@ -0,0 +1,21 @@
+/**
+ * Function.prototype.bind polyfill used by PhantomJS
+ */
+if (typeof Function.prototype.bind != 'function') {
+  Function.prototype.bind = function bind(obj) {
+    var args = Array.prototype.slice.call(arguments, 1),
+      self = this,
+      nop = function() {
+      },
+      bound = function() {
+        return self.apply(
+          this instanceof nop ? this : (obj || {}), args.concat(
+            Array.prototype.slice.call(arguments)
+          )
+        );
+      };
+    nop.prototype = this.prototype || {};
+    bound.prototype = new nop();
+    return bound;
+  };
+}