Added tests for single nodes layout

client/app/scripts/charts/__tests__/node-layout-test.js

@@ -1,4 +1,5 @@
 jest.dontMock('../nodes-layout');
+jest.dontMock('../topology-utils');
 jest.dontMock('../../constants/naming'); // edge naming: 'source-target'
 
 import { fromJS, Map } from 'immutable';
@@ -69,6 +70,26 @@ describe('NodesLayout', () => {
       edges: fromJS({
         'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
       })
+    },
+    single3: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n2: {id: 'n2'},
+        n3: {id: 'n3'}
+      }),
+      edges: fromJS({})
+    },
+    singlePortrait: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n2: {id: 'n2'},
+        n3: {id: 'n3'},
+        n4: {id: 'n4'},
+        n5: {id: 'n5'}
+      }),
+      edges: fromJS({
+        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+      })
     }
   };
 
@@ -232,4 +253,37 @@ describe('NodesLayout', () => {
     expect(resultCoords.slice(0, 2)).toEqual(coords.slice(0, 2));
     expect(resultCoords.slice(2, 6)).toEqual(coords.slice(4, 8));
   });
+
+  it('renders single nodes in a square', () => {
+    const result = NodesLayout.doLayout(
+      nodeSets.single3.nodes,
+      nodeSets.single3.edges);
+
+    nodes = result.nodes.toJS();
+
+    expect(nodes.n1.x).toEqual(nodes.n3.x);
+    expect(nodes.n1.y).toEqual(nodes.n2.y);
+    expect(nodes.n1.x).toBeLessThan(nodes.n2.x);
+    expect(nodes.n1.y).toBeLessThan(nodes.n3.y);
+  });
+
+  it('renders single nodes next to portrait graph', () => {
+    const result = NodesLayout.doLayout(
+      nodeSets.singlePortrait.nodes,
+      nodeSets.singlePortrait.edges);
+
+    nodes = result.nodes.toJS();
+
+    // first square row on same level as top-most other node
+    expect(nodes.n1.y).toEqual(nodes.n2.y);
+    expect(nodes.n1.y).toEqual(nodes.n3.y);
+    expect(nodes.n4.y).toEqual(nodes.n5.y);
+
+    // all singles right to other nodes
+    expect(nodes.n1.x).toEqual(nodes.n4.x);
+    expect(nodes.n1.x).toBeLessThan(nodes.n2.x);
+    expect(nodes.n1.x).toBeLessThan(nodes.n3.x);
+    expect(nodes.n1.x).toBeLessThan(nodes.n5.x);
+    expect(nodes.n2.x).toEqual(nodes.n5.x);
+  });
 });

client/app/scripts/charts/__tests__/topology-utils-test.js

@@ -0,0 +1,109 @@
+jest.dontMock('../topology-utils');
+jest.dontMock('../../constants/naming'); // edge naming: 'source-target'
+
+import { fromJS } from 'immutable';
+
+describe('TopologyUtils', () => {
+  let TopologyUtils;
+  let nodes;
+
+  const nodeSets = {
+    initial4: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n2: {id: 'n2'},
+        n3: {id: 'n3'},
+        n4: {id: 'n4'}
+      }),
+      edges: fromJS({
+        'n1-n3': {id: 'n1-n3', source: 'n1', target: 'n3'},
+        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'},
+        'n2-n4': {id: 'n2-n4', source: 'n2', target: 'n4'}
+      })
+    },
+    removeEdge24: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n2: {id: 'n2'},
+        n3: {id: 'n3'},
+        n4: {id: 'n4'}
+      }),
+      edges: fromJS({
+        'n1-n3': {id: 'n1-n3', source: 'n1', target: 'n3'},
+        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+      })
+    },
+    removeNode2: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n3: {id: 'n3'},
+        n4: {id: 'n4'}
+      }),
+      edges: fromJS({
+        'n1-n3': {id: 'n1-n3', source: 'n1', target: 'n3'},
+        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+      })
+    },
+    removeNode23: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n4: {id: 'n4'}
+      }),
+      edges: fromJS({
+        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+      })
+    },
+    single3: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n2: {id: 'n2'},
+        n3: {id: 'n3'}
+      }),
+      edges: fromJS({})
+    },
+    singlePortrait: {
+      nodes: fromJS({
+        n1: {id: 'n1'},
+        n2: {id: 'n2'},
+        n3: {id: 'n3'},
+        n4: {id: 'n4'},
+        n5: {id: 'n5'}
+      }),
+      edges: fromJS({
+        'n1-n4': {id: 'n1-n4', source: 'n1', target: 'n4'}
+      })
+    }
+  };
+
+  beforeEach(() => {
+    TopologyUtils = require('../topology-utils');
+  });
+
+  it('sets node degrees', () => {
+    nodes = TopologyUtils.updateNodeDegrees(
+      nodeSets.initial4.nodes,
+      nodeSets.initial4.edges).toJS();
+
+    expect(nodes.n1.degree).toEqual(2);
+    expect(nodes.n2.degree).toEqual(1);
+    expect(nodes.n3.degree).toEqual(1);
+    expect(nodes.n4.degree).toEqual(2);
+
+    nodes = TopologyUtils.updateNodeDegrees(
+      nodeSets.removeEdge24.nodes,
+      nodeSets.removeEdge24.edges).toJS();
+
+    expect(nodes.n1.degree).toEqual(2);
+    expect(nodes.n2.degree).toEqual(0);
+    expect(nodes.n3.degree).toEqual(1);
+    expect(nodes.n4.degree).toEqual(1);
+
+    nodes = TopologyUtils.updateNodeDegrees(
+      nodeSets.single3.nodes,
+      nodeSets.single3.edges).toJS();
+
+    expect(nodes.n1.degree).toEqual(0);
+    expect(nodes.n2.degree).toEqual(0);
+    expect(nodes.n3.degree).toEqual(0);
+  });
+});

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

@@ -12,7 +12,6 @@ const Naming = require('../constants/naming');
 const NodesLayout = require('./nodes-layout');
 const Node = require('./node');
 const NodesError = require('./nodes-error');
-const TopologyUtils = require('./topology-utils');
 
 const MARGINS = {
   top: 130,
@@ -236,7 +235,6 @@ const NodesChart = React.createClass({
         pseudo: node.get('pseudo'),
         subLabel: node.get('label_minor'),
         rank: node.get('rank'),
-        degree: TopologyUtils.getDegreeForNodeId(topology, id),
         x: 0,
         y: 0
       });

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

@@ -2,7 +2,9 @@ const dagre = require('dagre');
 const debug = require('debug')('scope:nodes-layout');
 const makeMap = require('immutable').Map;
 const ImmSet = require('immutable').Set;
+
 const Naming = require('../constants/naming');
+const TopologyUtils = require('./topology-utils');
 
 const MAX_NODES = 100;
 const topologyCaches = {};
@@ -141,22 +143,24 @@ function layoutSingleNodes(layout, opts) {
   const singleNodes = nodes.filter(node => node.get('degree') === 0);
 
   if (singleNodes.size) {
-    const nonSingleNodes = nodes.filter(node => node.get('degree') !== 0);
     let offsetX;
     let offsetY;
-    if (aspectRatio < 1) {
-      debug('laying out single nodes to the right', aspectRatio);
-      offsetX = nonSingleNodes.maxBy(node => node.get('x')).get('x');
-      offsetY = nonSingleNodes.minBy(node => node.get('y')).get('y');
-      if (offsetX) {
-        offsetX += nodeWidth + nodesep;
-      }
-    } else {
-      debug('laying out single nodes below', aspectRatio);
-      offsetX = nonSingleNodes.minBy(node => node.get('x')).get('x');
-      offsetY = nonSingleNodes.maxBy(node => node.get('y')).get('y');
-      if (offsetY) {
-        offsetY += nodeHeight + ranksep;
+    const nonSingleNodes = nodes.filter(node => node.get('degree') !== 0);
+    if (nonSingleNodes.size > 0) {
+      if (aspectRatio < 1) {
+        debug('laying out single nodes to the right', aspectRatio);
+        offsetX = nonSingleNodes.maxBy(node => node.get('x')).get('x');
+        offsetY = nonSingleNodes.minBy(node => node.get('y')).get('y');
+        if (offsetX) {
+          offsetX += nodeWidth + nodesep;
+        }
+      } else {
+        debug('laying out single nodes below', aspectRatio);
+        offsetX = nonSingleNodes.minBy(node => node.get('x')).get('x');
+        offsetY = nonSingleNodes.maxBy(node => node.get('y')).get('y');
+        if (offsetY) {
+          offsetY += nodeHeight + ranksep;
+        }
       }
     }
 
@@ -320,12 +324,12 @@ function copyLayoutProperties(layout, nodeCache, edgeCache) {
  * Layout of nodes and edges
  * If a previous layout was given and not too much changed, the previous layout
  * is changed and returned. Otherwise does a new layout engine run.
- * @param  {Map} nodes All nodes
- * @param  {Map} edges All edges
+ * @param  {Map} immNodes All nodes
+ * @param  {Map} immEdges All edges
  * @param  {object} opts  width, height, margins, etc...
  * @return {object} graph object with nodes, edges, dimensions
  */
-export function doLayout(nodes, edges, opts) {
+export function doLayout(immNodes, immEdges, opts) {
   const options = opts || {};
   const topologyId = options.topologyId || 'noId';
 
@@ -345,14 +349,15 @@ export function doLayout(nodes, edges, opts) {
   let layout;
 
   ++layoutRuns;
-  if (cachedLayout && nodeCache && edgeCache && !hasUnseenNodes(nodes, nodeCache)) {
+  if (cachedLayout && nodeCache && edgeCache && !hasUnseenNodes(immNodes, nodeCache)) {
     debug('skip layout, trivial adjustment', ++layoutRunsTrivial, layoutRuns);
-    layout = cloneLayout(cachedLayout, nodes, edges);
+    layout = cloneLayout(cachedLayout, immNodes, immEdges);
     // copy old properties, works also if nodes get re-added
     layout = copyLayoutProperties(layout, nodeCache, edgeCache);
   } else {
     const graph = cache.graph;
-    layout = runLayoutEngine(graph, nodes, edges, opts);
+    const nodesWithDegrees = TopologyUtils.updateNodeDegrees(immNodes, immEdges);
+    layout = runLayoutEngine(graph, nodesWithDegrees, immEdges, opts);
     layout = layoutSingleNodes(layout, opts);
     layout = shiftLayoutToCenter(layout, opts);
   }

client/app/scripts/charts/topology-utils.js

@@ -1,15 +1,10 @@
 
-export function getDegreeForNodeId(topology, nodeId) {
-  let degree = 0;
-  topology.forEach(node => {
-    if (node.get('id') === nodeId) {
-      if (node.get('adjacency')) {
-        degree += node.get('adjacency').size;
-      }
-    } else if (node.get('adjacency') && node.get('adjacency').includes(nodeId)) {
-      // FIXME this can still count edges double if both directions exist
-      degree++;
-    }
+export function updateNodeDegrees(nodes, edges) {
+  return nodes.map(node => {
+    const nodeId = node.get('id');
+    const degree = edges.count(edge => {
+      return edge.get('source') === nodeId || edge.get('target') === nodeId;
+    });
+    return node.set('degree', degree);
   });
-  return degree;
 }