From 8eb4be8d4f239f3aa2a3feacc3e6fe762196e247 Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Tue, 17 Nov 2015 22:30:53 +0100 Subject: [PATCH] Added tests for single nodes layout --- .../charts/__tests__/node-layout-test.js | 54 +++++++++ .../charts/__tests__/topology-utils-test.js | 109 ++++++++++++++++++ client/app/scripts/charts/nodes-chart.js | 2 - client/app/scripts/charts/nodes-layout.js | 45 ++++---- client/app/scripts/charts/topology-utils.js | 19 ++- 5 files changed, 195 insertions(+), 34 deletions(-) create mode 100644 client/app/scripts/charts/__tests__/topology-utils-test.js diff --git a/client/app/scripts/charts/__tests__/node-layout-test.js b/client/app/scripts/charts/__tests__/node-layout-test.js index 3a66950f6..c370c6e45 100644 --- a/client/app/scripts/charts/__tests__/node-layout-test.js +++ b/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); + }); }); diff --git a/client/app/scripts/charts/__tests__/topology-utils-test.js b/client/app/scripts/charts/__tests__/topology-utils-test.js new file mode 100644 index 000000000..7686eee92 --- /dev/null +++ b/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); + }); +}); diff --git a/client/app/scripts/charts/nodes-chart.js b/client/app/scripts/charts/nodes-chart.js index 79cd8e3e4..4c5507854 100644 --- a/client/app/scripts/charts/nodes-chart.js +++ b/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 }); diff --git a/client/app/scripts/charts/nodes-layout.js b/client/app/scripts/charts/nodes-layout.js index c52869f18..5fc5bf68f 100644 --- a/client/app/scripts/charts/nodes-layout.js +++ b/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); } diff --git a/client/app/scripts/charts/topology-utils.js b/client/app/scripts/charts/topology-utils.js index c37fb1188..ab5c57b4f 100644 --- a/client/app/scripts/charts/topology-utils.js +++ b/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; }