From aba65747c9c204ac40bc12741502c6f347019e43 Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Tue, 26 Mar 2024 13:43:43 +0100 Subject: [PATCH] Nodegraph: Fix issue with rendering single node (#84930) Fix for single node graph case --- jest.config.js | 1 + .../panel/nodeGraph/createLayoutWorker.ts | 2 +- ...{layout.worker.utils.js => forceLayout.js} | 0 ...layoutMsagl.worker.js => layeredLayout.js} | 53 ++++++------------- .../panel/nodeGraph/layeredLayout.test.ts | 10 ++++ .../plugins/panel/nodeGraph/layout.worker.js | 3 +- .../panel/nodeGraph/layoutLayered.worker.js | 8 +++ public/test/mocks/workers.ts | 2 +- 8 files changed, 38 insertions(+), 41 deletions(-) rename public/app/plugins/panel/nodeGraph/{layout.worker.utils.js => forceLayout.js} (100%) rename public/app/plugins/panel/nodeGraph/{layoutMsagl.worker.js => layeredLayout.js} (83%) create mode 100644 public/app/plugins/panel/nodeGraph/layeredLayout.test.ts create mode 100644 public/app/plugins/panel/nodeGraph/layoutLayered.worker.js diff --git a/jest.config.js b/jest.config.js index 604abe7a00e..766d9e2e825 100644 --- a/jest.config.js +++ b/jest.config.js @@ -18,6 +18,7 @@ const esModules = [ '@kusto/monaco-kusto', 'monaco-editor', 'lodash-es', + '@msagl', ].join('|'); module.exports = { diff --git a/public/app/plugins/panel/nodeGraph/createLayoutWorker.ts b/public/app/plugins/panel/nodeGraph/createLayoutWorker.ts index c113773768d..d0fef044dac 100644 --- a/public/app/plugins/panel/nodeGraph/createLayoutWorker.ts +++ b/public/app/plugins/panel/nodeGraph/createLayoutWorker.ts @@ -1,4 +1,4 @@ import { CorsWorker as Worker } from 'app/core/utils/CorsWorker'; export const createWorker = () => new Worker(new URL('./layout.worker.js', import.meta.url)); -export const createMsaglWorker = () => new Worker(new URL('./layoutMsagl.worker.js', import.meta.url)); +export const createMsaglWorker = () => new Worker(new URL('./layoutLayered.worker.js', import.meta.url)); diff --git a/public/app/plugins/panel/nodeGraph/layout.worker.utils.js b/public/app/plugins/panel/nodeGraph/forceLayout.js similarity index 100% rename from public/app/plugins/panel/nodeGraph/layout.worker.utils.js rename to public/app/plugins/panel/nodeGraph/forceLayout.js diff --git a/public/app/plugins/panel/nodeGraph/layoutMsagl.worker.js b/public/app/plugins/panel/nodeGraph/layeredLayout.js similarity index 83% rename from public/app/plugins/panel/nodeGraph/layoutMsagl.worker.js rename to public/app/plugins/panel/nodeGraph/layeredLayout.js index f088b22f3fc..2a3b411bab3 100644 --- a/public/app/plugins/panel/nodeGraph/layoutMsagl.worker.js +++ b/public/app/plugins/panel/nodeGraph/layeredLayout.js @@ -10,12 +10,6 @@ import { } from '@msagl/core'; import { parseDot } from '@msagl/parser'; -addEventListener('message', async (event) => { - const { nodes, edges, config } = event.data; - const [newNodes, newEdges] = layout(nodes, edges, config); - postMessage({ nodes: newNodes, edges: newEdges }); -}); - /** * Use d3 force layout to lay the nodes in a sensible way. This function modifies the nodes adding the x,y positions * and also fills in node references in edges instead of node ids. @@ -23,7 +17,7 @@ addEventListener('message', async (event) => { export function layout(nodes, edges) { const { mappedEdges, DOTToIdMap } = createMappings(nodes, edges); - const dot = edgesToDOT(mappedEdges); + const dot = graphToDOT(mappedEdges, DOTToIdMap); const graph = parseDot(dot); const geomGraph = new GeomGraph(graph); for (const e of graph.deepEdges) { @@ -142,58 +136,41 @@ function createMappings(nodes, edges) { // Key is an iteration index and value is actual ID of the node const DOTToIdMap = {}; - // Crate the maps both ways let index = 0; - for (const edge of edges) { - if (!idToDOTMap[edge.source]) { - idToDOTMap[edge.source] = index.toString(10); - DOTToIdMap[index.toString(10)] = edge.source; - index++; - } + for (const node of nodes) { + idToDOTMap[node.id] = index.toString(10); + DOTToIdMap[index.toString(10)] = node.id; + index++; + } - if (!idToDOTMap[edge.target]) { - idToDOTMap[edge.target] = index.toString(10); - DOTToIdMap[index.toString(10)] = edge.target; - index++; - } + for (const edge of edges) { mappedEdges.push({ source: idToDOTMap[edge.source], target: idToDOTMap[edge.target] }); } return { mappedEdges, DOTToIdMap, + idToDOTMap, }; } -function toDOT(edges, graphAttr = '', edgeAttr = '') { +function graphToDOT(edges, nodeIDsMap) { let dot = ` digraph G { - ${graphAttr} + rankdir="LR"; TBbalance="min" `; for (const edge of edges) { - dot += edge.source + '->' + edge.target + ' ' + edgeAttr + '\n'; + dot += edge.source + '->' + edge.target + ' ' + '[ minlen=3 ]\n'; } - dot += nodesDOT(edges); + dot += nodesDOT(nodeIDsMap); dot += '}'; return dot; } -function edgesToDOT(edges) { - return toDOT(edges, 'rankdir="LR"; TBbalance="min"', '[ minlen=3 ]'); -} - -function nodesDOT(edges) { +function nodesDOT(nodeIdsMap) { let dot = ''; - const visitedNodes = new Set(); - // TODO: height/width for default sizing but nodes can have variable size now - const attr = '[fixedsize=true, width=1.2, height=1.7] \n'; - for (const edge of edges) { - if (!visitedNodes.has(edge.source)) { - dot += edge.source + attr; - } - if (!visitedNodes.has(edge.target)) { - dot += edge.target + attr; - } + for (const node of Object.keys(nodeIdsMap)) { + dot += node + ' [fixedsize=true, width=1.2, height=1.7] \n'; } return dot; } diff --git a/public/app/plugins/panel/nodeGraph/layeredLayout.test.ts b/public/app/plugins/panel/nodeGraph/layeredLayout.test.ts new file mode 100644 index 00000000000..aec26b9b4eb --- /dev/null +++ b/public/app/plugins/panel/nodeGraph/layeredLayout.test.ts @@ -0,0 +1,10 @@ +import { layout } from './layeredLayout'; + +describe('layout', () => { + it('can render single node', () => { + const nodes = [{ id: 'A', incoming: 0 }]; + const edges: unknown[] = []; + const graph = layout(nodes, edges); + expect(graph).toEqual([[{ id: 'A', incoming: 0, x: 0, y: 0 }], []]); + }); +}); diff --git a/public/app/plugins/panel/nodeGraph/layout.worker.js b/public/app/plugins/panel/nodeGraph/layout.worker.js index d0fafa0c54f..ff5c3936109 100644 --- a/public/app/plugins/panel/nodeGraph/layout.worker.js +++ b/public/app/plugins/panel/nodeGraph/layout.worker.js @@ -1,5 +1,6 @@ -import { layout } from './layout.worker.utils'; +import { layout } from './forceLayout'; +// Separate from main implementation so it does not trip out tests addEventListener('message', (event) => { const { nodes, edges, config } = event.data; layout(nodes, edges, config); diff --git a/public/app/plugins/panel/nodeGraph/layoutLayered.worker.js b/public/app/plugins/panel/nodeGraph/layoutLayered.worker.js new file mode 100644 index 00000000000..7fa2caec2c9 --- /dev/null +++ b/public/app/plugins/panel/nodeGraph/layoutLayered.worker.js @@ -0,0 +1,8 @@ +import { layout } from './layeredLayout'; + +// Separate from main implementation so it does not trip out tests +addEventListener('message', async (event) => { + const { nodes, edges, config } = event.data; + const [newNodes, newEdges] = layout(nodes, edges, config); + postMessage({ nodes: newNodes, edges: newEdges }); +}); diff --git a/public/test/mocks/workers.ts b/public/test/mocks/workers.ts index 34ae57a5864..70ec661c12c 100644 --- a/public/test/mocks/workers.ts +++ b/public/test/mocks/workers.ts @@ -1,7 +1,7 @@ import { Config } from 'app/plugins/panel/nodeGraph/layout'; import { EdgeDatum, NodeDatum } from 'app/plugins/panel/nodeGraph/types'; -const { layout } = jest.requireActual('../../app/plugins/panel/nodeGraph/layout.worker.utils.js'); +const { layout } = jest.requireActual('../../app/plugins/panel/nodeGraph/forceLayout.js'); class LayoutMockWorker { timeout: number | undefined;