diff --git a/client/app/scripts/selectors/nodes-chart-layout.js b/client/app/scripts/selectors/nodes-chart-layout.js index 4670fab28..995b58c39 100644 --- a/client/app/scripts/selectors/nodes-chart-layout.js +++ b/client/app/scripts/selectors/nodes-chart-layout.js @@ -3,41 +3,12 @@ import { createSelector, createStructuredSelector } from 'reselect'; import { Map as makeMap } from 'immutable'; import timely from 'timely'; -import { EDGE_ID_SEPARATOR } from '../constants/naming'; +import { initEdgesFromNodes, collapseMultiEdges } from '../utils/layouter-utils'; import { doLayout } from '../charts/nodes-layout'; const log = debug('scope:nodes-chart'); -function initEdgesFromNodes(nodes) { - let edges = makeMap(); - - nodes.forEach((node, nodeId) => { - const adjacency = node.get('adjacency'); - if (adjacency) { - adjacency.forEach((adjacent) => { - const edge = nodeId < adjacent ? [nodeId, adjacent] : [adjacent, nodeId]; - const edgeId = edge.join(EDGE_ID_SEPARATOR); - - if (!edges.has(edgeId)) { - const source = edge[0]; - const target = edge[1]; - if (nodes.has(source) && nodes.has(target)) { - edges = edges.set(edgeId, makeMap({ - id: edgeId, - value: 1, - source, - target - })); - } - } - }); - } - }); - - return edges; -} - // TODO: Make all the selectors below pure (so that they only depend on the global state). const layoutOptionsSelector = createStructuredSelector({ @@ -86,8 +57,9 @@ export const graphLayout = createSelector( log(`graph layout calculation took ${timedLayouter.time}ms`); return { + // NOTE: This might be a good place to add (some of) nodes/edges decorators. layoutNodes: graph.nodes, - layoutEdges: graph.edges, + layoutEdges: collapseMultiEdges(graph.edges), }; } ); diff --git a/client/app/scripts/utils/__tests__/layouter-utils-test.js b/client/app/scripts/utils/__tests__/layouter-utils-test.js new file mode 100644 index 000000000..467bfa57f --- /dev/null +++ b/client/app/scripts/utils/__tests__/layouter-utils-test.js @@ -0,0 +1,41 @@ +import { fromJS } from 'immutable'; + +import { + initEdgesFromNodes, + collapseMultiEdges, +} from '../layouter-utils'; + + +describe('LayouterUtils', () => { + describe('initEdgesFromNodes', () => { + it('should return map of edges', () => { + const input = fromJS({ + a: { adjacency: ['b', 'c'] }, + b: { adjacency: ['a', 'b'] }, + c: {} + }); + expect(initEdgesFromNodes(input).toJS()).toEqual({ + 'a-b': { id: 'a-b', source: 'a', target: 'b', value: 1 }, + 'a-c': { id: 'a-c', source: 'a', target: 'c', value: 1 }, + 'b-a': { id: 'b-a', source: 'b', target: 'a', value: 1 }, + 'b-b': { id: 'b-b', source: 'b', target: 'b', value: 1 }, + }); + }); + }); + + describe('collapseMultiEdges', () => { + it('should return collapsed multi-edges', () => { + const input = fromJS({ + 'a-b': { id: 'a-b', source: 'a', target: 'b' }, + 'a-c': { id: 'a-c', source: 'a', target: 'c' }, + 'b-a': { id: 'b-a', source: 'b', target: 'a' }, + 'b-b': { id: 'b-b', source: 'b', target: 'b' }, + }); + expect(collapseMultiEdges(input).toJS()).toEqual({ + 'a-b': { id: 'a-b', source: 'a', target: 'b', bidirectional: true }, + 'a-c': { id: 'a-c', source: 'a', target: 'c' }, + 'b-b': { id: 'b-b', source: 'b', target: 'b' }, + }); + }); + }); +}); diff --git a/client/app/scripts/utils/layouter-utils.js b/client/app/scripts/utils/layouter-utils.js new file mode 100644 index 000000000..c364b4280 --- /dev/null +++ b/client/app/scripts/utils/layouter-utils.js @@ -0,0 +1,55 @@ +import { Map as makeMap } from 'immutable'; + +import { EDGE_ID_SEPARATOR } from '../constants/naming'; + + +function constructEdgeId(source, target) { + return [source, target].join(EDGE_ID_SEPARATOR); +} + +// Constructs the edges for the layout engine from the nodes' adjacency table. +// We don't collapse edge pairs (A->B, B->A) here as we want to let the layout +// engine decide how to handle bidirectional edges. +export function initEdgesFromNodes(nodes) { + let edges = makeMap(); + + nodes.forEach((node, nodeId) => { + (node.get('adjacency') || []).forEach((adjacentId) => { + const source = nodeId; + const target = adjacentId; + + if (nodes.has(target)) { + // The direction source->target is important since dagre takes + // directionality into account when calculating the layout. + const edgeId = constructEdgeId(source, target); + const edge = makeMap({ id: edgeId, value: 1, source, target }); + edges = edges.set(edgeId, edge); + } + }); + }); + + return edges; +} + +// Replaces all pairs of edges (A->B, B->A) with a single A->B edge that is marked as +// bidirectional. We do this to prevent double rendering of edges between the same nodes. +export function collapseMultiEdges(directedEdges) { + let collapsedEdges = makeMap(); + + directedEdges.forEach((edge, edgeId) => { + const source = edge.get('source'); + const target = edge.get('target'); + const reversedEdgeId = constructEdgeId(target, source); + + if (collapsedEdges.has(reversedEdgeId)) { + // If the edge between the same nodes with the opposite direction already exists, + // mark it as bidirectional and don't add any other edges (making graph simple). + collapsedEdges = collapsedEdges.setIn([reversedEdgeId, 'bidirectional'], true); + } else { + // Otherwise just copy the edge. + collapsedEdges = collapsedEdges.set(edgeId, edge); + } + }); + + return collapsedEdges; +}