From 8fed6ac2c69f1bcf76838478aac0de5286d61b0a Mon Sep 17 00:00:00 2001 From: Valerio Bartolini <79913332+bartoval@users.noreply.github.com> Date: Mon, 21 Nov 2022 15:15:52 +0100 Subject: [PATCH] perf(Graph): removed useless update calls (#42) --- package.json | 2 +- src/assets/router.svg | 250 ------------------ src/assets/service.svg | 2 +- src/assets/site.svg | 2 +- src/core/components/Graph/Graph.enum.ts | 2 + src/core/components/Graph/Graph.interfaces.ts | 2 +- src/core/components/Graph/Graph.ts | 71 +++-- src/pages/Topology/Topology.enum.ts | 1 + src/pages/Topology/Topology.interfaces.ts | 11 + .../Topology/components/TopologyPanel.tsx | 247 ++++++++++------- .../components/TopologyProcessGroups.tsx | 18 +- .../Topology/components/TopologyProcesses.tsx | 25 +- .../Topology/components/TopologySite.tsx | 19 +- src/pages/Topology/services/index.ts | 36 +-- 14 files changed, 244 insertions(+), 444 deletions(-) delete mode 100644 src/assets/router.svg diff --git a/package.json b/package.json index 64ccee50..ecc13b31 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ "engines": { "npm": "please-use-yarn", "yarn": ">= 1.19.1" - }, + }, "scripts": { "start": "webpack serve --config config/webpack.dev.js", "build": "webpack --config config/webpack.prod.js", diff --git a/src/assets/router.svg b/src/assets/router.svg deleted file mode 100644 index 890a107a..00000000 --- a/src/assets/router.svg +++ /dev/null @@ -1,250 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - image/svg+xml - - - - - Openclipart - - - Router - 2006-12-07T10:59:07 - A simple router for network diagrams - https://openclipart.org/detail/1918/router-by-juanjo - - - juanjo - - - - - computer - diagram - network - router - - - - - - - - - - - diff --git a/src/assets/service.svg b/src/assets/service.svg index b480e759..5af60dba 100644 --- a/src/assets/service.svg +++ b/src/assets/service.svg @@ -1 +1 @@ - \ No newline at end of file + diff --git a/src/assets/site.svg b/src/assets/site.svg index a324c849..1bf235a5 100644 --- a/src/assets/site.svg +++ b/src/assets/site.svg @@ -1 +1 @@ - \ No newline at end of file + diff --git a/src/core/components/Graph/Graph.enum.ts b/src/core/components/Graph/Graph.enum.ts index f65e6476..ee9a5d75 100644 --- a/src/core/components/Graph/Graph.enum.ts +++ b/src/core/components/Graph/Graph.enum.ts @@ -1,4 +1,6 @@ export enum GraphEvents { IsGraphLoaded = 'graph:loaded', + IsDraggingNodeEnd = 'node:drag-end', + IsDraggingNodesEnd = 'nodes:drag-end', NodeClick = 'node:click', } diff --git a/src/core/components/Graph/Graph.interfaces.ts b/src/core/components/Graph/Graph.interfaces.ts index fbfff8fc..cce1afdc 100644 --- a/src/core/components/Graph/Graph.interfaces.ts +++ b/src/core/components/Graph/Graph.interfaces.ts @@ -6,7 +6,7 @@ export interface GraphNode { color: string; groupName: string; group: number; - img?: XMLDocument; + img?: string; fx: number | null; fy: number | null; groupFx?: number; diff --git a/src/core/components/Graph/Graph.ts b/src/core/components/Graph/Graph.ts index 7c2720cd..75cf64d6 100644 --- a/src/core/components/Graph/Graph.ts +++ b/src/core/components/Graph/Graph.ts @@ -16,7 +16,7 @@ import { interpolate } from 'd3-interpolate'; import { polygonCentroid, polygonHull } from 'd3-polygon'; import { scaleOrdinal } from 'd3-scale'; import { select, Selection } from 'd3-selection'; -import { curveCatmullRomClosed, Line, line } from 'd3-shape'; +import { Line, line, curveCardinalClosed } from 'd3-shape'; import { zoom, zoomTransform, zoomIdentity, ZoomBehavior } from 'd3-zoom'; import EventEmitter from '@core/components/Graph/EventEmitter'; @@ -46,11 +46,12 @@ export default class Graph { selectedNode: null | string; EventEmitter: EventEmitter; options: { showGroup?: boolean | undefined } | undefined; + isGraphLoaded: boolean; constructor( $node: HTMLElement, nodes: GraphNode[], - edges: GraphEdge[] | GraphEdgeModifiedByForce[], + edges: GraphEdge[], boxWidth: number, boxHeight: number, options?: { showGroup?: boolean }, @@ -65,7 +66,9 @@ export default class Graph { this.EventEmitter = new EventEmitter(); this.isDraggingNode = false; - this.force = this.initForce(nodes); + this.isGraphLoaded = false; + + this.force = this.initForce(this.nodes); this.svgContainer = this.createSvgContainer(); this.svgContainerGroupNodes = this.svgContainer @@ -80,7 +83,7 @@ export default class Graph { .y(function (d) { return d[1]; }) - .curve(curveCatmullRomClosed)), + .curve(curveCardinalClosed)), (this.groupIds = []); this.handleZoom = zoom() @@ -90,6 +93,7 @@ export default class Graph { }); this.svgContainer.call(this.handleZoom); + this.redrawTopology(this.nodes, this.links); } private createSvgContainer() { @@ -130,8 +134,8 @@ export default class Graph { this.force.stop(); } - localStorage.setItem(node.id, JSON.stringify({ fx: node.x, fy: node.y })); this.isDraggingNode = false; + this.EventEmitter.emit(GraphEvents.IsDraggingNodeEnd, [node]); }; private groupDragStarted = ( @@ -171,14 +175,10 @@ export default class Graph { this.force.stop(); } - this.force - .nodes() - .filter(({ group }) => group === Number(groupId)) - .forEach((node) => { - localStorage.setItem(node.id, JSON.stringify({ fx: node.x, fy: node.y })); - }); - this.isDraggingNode = false; + this.EventEmitter.emit(GraphEvents.IsDraggingNodesEnd, [ + this.nodes.filter(({ group }) => group === Number(groupId)), + ]); }; private ticked = () => { @@ -329,16 +329,12 @@ export default class Graph { }), ) .on('end', () => { - nodes.forEach((node) => { - if (!localStorage.getItem(node.id)) { - node.fx = node.x; - node.fy = node.y; - - localStorage.setItem(node.id, JSON.stringify({ fx: node.fx, fy: node.fy })); - } - }); + if (!this.isGraphLoaded) { + this.EventEmitter.emit(GraphEvents.IsGraphLoaded, [this.nodes]); + this.isGraphLoaded = true; + } - this.EventEmitter.emit(GraphEvents.IsGraphLoaded); + this.force.stop(); }); } @@ -358,7 +354,6 @@ export default class Graph { .append('svg:path') .style('fill', 'gray') .attr('d', 'M0,-5L10,0L0,5'); - // links services const svgEdgesData = this.svgContainerGroupNodes.selectAll('.serviceLink').data(edges); const svgEdges = svgEdgesData.enter(); @@ -443,7 +438,8 @@ export default class Graph { .style('fill', ({ color }) => color); enterSelection - .append(({ img }) => img?.documentElement.cloneNode(true) as HTMLElement) + .append('image') + .attr('xlink:href', ({ img }) => img || null) .attr('class', 'node-img') .attr('width', NODE_SIZE / 2) .attr('x', -NODE_SIZE / 4) @@ -560,23 +556,26 @@ export default class Graph { }); } - updateTopology = (nodes: GraphNode[], links: GraphEdge[] | GraphEdgeModifiedByForce[]) => { - if (!this.isDraggingNode) { - this.svgContainerGroupNodes.selectAll('*').remove(); + updateTopology = (nodes: GraphNode[], links: GraphEdge[]) => { + if (this.isGraphLoaded && !this.isDraggingNode) { + this.redrawTopology(nodes, links); + } + }; - this.force.nodes(nodes).on('tick', this.ticked); + private redrawTopology = (nodes: GraphNode[], links: GraphEdge[]) => { + this.svgContainerGroupNodes.selectAll('*').remove(); + this.links = JSON.parse(JSON.stringify(links)); + this.nodes = JSON.parse(JSON.stringify(nodes)); - this.force - .force>('link') - ?.links(links); + this.force.nodes(this.nodes).on('tick', this.ticked); + this.force + .force>('link') + ?.links(this.links); - this.force.restart(); - this.links = links as GraphEdgeModifiedByForce[]; - this.nodes = nodes; + this.force.restart(); - this.updateEdges(); - this.redrawNodes(); - } + this.updateEdges(); + this.redrawNodes(); }; // exposed events diff --git a/src/pages/Topology/Topology.enum.ts b/src/pages/Topology/Topology.enum.ts index 26af526d..0863aee8 100644 --- a/src/pages/Topology/Topology.enum.ts +++ b/src/pages/Topology/Topology.enum.ts @@ -13,6 +13,7 @@ export enum TopologyViews { export enum Labels { Topology = 'Network', LegendGroupsItems = 'Sites', + ShowAll = 'Show all addresses', } export enum ConnectionsLabels { diff --git a/src/pages/Topology/Topology.interfaces.ts b/src/pages/Topology/Topology.interfaces.ts index 79096564..2ef33b15 100644 --- a/src/pages/Topology/Topology.interfaces.ts +++ b/src/pages/Topology/Topology.interfaces.ts @@ -1,3 +1,6 @@ +import { ReactNode } from 'react'; + +import { GraphEdge, GraphNode } from '@core/components/Graph/Graph.interfaces'; import { FlowAggregatesResponse } from 'API/REST.interfaces'; export interface TopologyDetailsProps { @@ -7,6 +10,14 @@ export interface TopologyDetailsProps { tcpConnectionsInEntries: FlowAggregatesResponse[]; } +export interface TopologyPanelProps { + nodes: GraphNode[]; + links: GraphEdge[]; + options?: { showGroup: boolean }; + onGetSelectedNode?: Function; + children: ReactNode; +} + export interface TrafficData { identity: string; targetIdentity: string; diff --git a/src/pages/Topology/components/TopologyPanel.tsx b/src/pages/Topology/components/TopologyPanel.tsx index 57c8a261..f343b78b 100644 --- a/src/pages/Topology/components/TopologyPanel.tsx +++ b/src/pages/Topology/components/TopologyPanel.tsx @@ -1,4 +1,11 @@ -import React, { forwardRef, useCallback, useEffect, useImperativeHandle, useState } from 'react'; +import React, { + forwardRef, + useCallback, + useEffect, + useImperativeHandle, + useRef, + useState, +} from 'react'; import { Drawer, @@ -18,109 +25,149 @@ import { } from '@patternfly/react-topology'; import { GraphEvents } from '@core/components/Graph/Graph.enum'; -import { GraphEdge, GraphNode } from '@core/components/Graph/Graph.interfaces'; +import { GraphNode } from '@core/components/Graph/Graph.interfaces'; import Graph from '../../../core/components/Graph/Graph'; +import { TopologyPanelProps } from '../Topology.interfaces'; -const TopologyPanel = forwardRef< - { deselectAll: () => void }, - { - nodes: GraphNode[]; - links: GraphEdge[]; - options?: { showGroup: boolean }; - onGetSelectedNode?: Function; - children: React.ReactNode; - } ->(({ nodes, links, onGetSelectedNode, children, options }, ref) => { - const [topologyGraphInstance, setTopologyGraphInstance] = useState(); - const [areDetailsExpanded, setIsExpandedDetails] = useState(false); - - const handleExpandDetails = useCallback( - ({ data: { id } }: { data: GraphNode }) => { - setIsExpandedDetails(!!id); - - if (onGetSelectedNode) { - onGetSelectedNode(id); - } - }, - [onGetSelectedNode], - ); - - function handleCloseDetails() { - topologyGraphInstance?.deselectAll(); - setIsExpandedDetails(false); - } - - // Create Graph - const graphRef = useCallback( - ($node: HTMLDivElement | null) => { - if ($node && nodes.length && links.length && !topologyGraphInstance) { - $node.replaceChildren(); - - const topologyGraph = new Graph( - $node, - nodes, - links, - $node.getBoundingClientRect().width, - $node.getBoundingClientRect().height, - options, - ); - - topologyGraph.EventEmitter.on(GraphEvents.NodeClick, handleExpandDetails); - topologyGraph.updateTopology(nodes, links); - - setTopologyGraphInstance(topologyGraph); +const TopologyPanel = forwardRef<{ deselectAll: () => void }, TopologyPanelProps>( + ({ nodes, links, onGetSelectedNode, children, options }, ref) => { + const [topologyGraphInstance, setTopologyGraphInstance] = useState(); + const [areDetailsExpanded, setIsExpandedDetails] = useState(false); + + const prevNodesRef = useRef(); + + const handleExpandDetails = useCallback( + ({ data: { id } }: { data: GraphNode }) => { + setIsExpandedDetails(!!id); + + if (onGetSelectedNode) { + onGetSelectedNode(id); + } + }, + [onGetSelectedNode], + ); + + function handleCloseDetails() { + topologyGraphInstance?.deselectAll(); + setIsExpandedDetails(false); + } + + function handleIsGraphLoaded(topologyNodes: GraphNode[]) { + topologyNodes.forEach((node) => { + if (!localStorage.getItem(node.id)) { + handleSaveNodePosition(node); + } + }); + } + + function handleSaveNodesPositions(topologyNodes: GraphNode[]) { + topologyNodes.forEach((node) => { + handleSaveNodePosition(node); + }); + } + + function handleSaveNodePosition(node: GraphNode) { + if (node.x && node.y) { + localStorage.setItem(node.id, JSON.stringify({ fx: node.x, fy: node.y })); } - }, - [handleExpandDetails, links, nodes, options, topologyGraphInstance], - ); - - useImperativeHandle(ref, () => ({ - deselectAll() { - handleCloseDetails(); - }, - })); - - // Update topology - useEffect(() => { - if (topologyGraphInstance && links && nodes) { - topologyGraphInstance.updateTopology(nodes, links); } - }, [links, nodes, topologyGraphInstance]); - - const ControlButtons = createTopologyControlButtons({ - ...defaultControlButtonsOptions, - zoomInCallback: () => topologyGraphInstance?.zoomIn(), - zoomOutCallback: () => topologyGraphInstance?.zoomOut(), - resetViewCallback: () => topologyGraphInstance?.zoomReset(), - fitToScreenHidden: true, - legendHidden: true, - }); - - const Details = ( - - - {children} - - - - - - ); - - return ( - - - - } - > - - - - - - ); -}); + + // Create Graph + const graphRef = useCallback( + ($node: HTMLDivElement | null) => { + if ($node && nodes.length && links.length && !topologyGraphInstance) { + $node.replaceChildren(); + + const topologyGraph = new Graph( + $node, + nodes, + links, + $node.getBoundingClientRect().width, + $node.getBoundingClientRect().height, + options, + ); + + topologyGraph.EventEmitter.on(GraphEvents.NodeClick, handleExpandDetails); + topologyGraph.EventEmitter.on(GraphEvents.IsGraphLoaded, handleIsGraphLoaded); + topologyGraph.EventEmitter.on( + GraphEvents.IsDraggingNodesEnd, + handleSaveNodesPositions, + ); + topologyGraph.EventEmitter.on( + GraphEvents.IsDraggingNodeEnd, + handleSaveNodePosition, + ); + + topologyGraph.updateTopology(nodes, links); + + setTopologyGraphInstance(topologyGraph); + } + }, + [ + handleExpandDetails, + handleIsGraphLoaded, + handleSaveNodesPositions, + links, + nodes, + options, + topologyGraphInstance, + ], + ); + + useImperativeHandle(ref, () => ({ + deselectAll() { + handleCloseDetails(); + }, + })); + + // Update topology + useEffect(() => { + if ( + topologyGraphInstance && + links && + nodes && + JSON.stringify(prevNodesRef.current) !== JSON.stringify(nodes) + ) { + topologyGraphInstance.updateTopology(nodes, links); + prevNodesRef.current = nodes; + } + }, [nodes, links, topologyGraphInstance]); + + const ControlButtons = createTopologyControlButtons({ + ...defaultControlButtonsOptions, + zoomInCallback: () => topologyGraphInstance?.zoomIn(), + zoomOutCallback: () => topologyGraphInstance?.zoomOut(), + resetViewCallback: () => topologyGraphInstance?.zoomReset(), + fitToScreenHidden: true, + legendHidden: true, + }); + + const Details = ( + + + {children} + + + + + + ); + + return ( + + + + } + > + + + + + + ); + }, +); export default TopologyPanel; diff --git a/src/pages/Topology/components/TopologyProcessGroups.tsx b/src/pages/Topology/components/TopologyProcessGroups.tsx index 9abcf05e..5454702f 100644 --- a/src/pages/Topology/components/TopologyProcessGroups.tsx +++ b/src/pages/Topology/components/TopologyProcessGroups.tsx @@ -48,18 +48,20 @@ const TopologyProcessGroups = function () { navigate(route); } - function handleGetSelectedNode(id: string) { - if (id !== nodeSelected) { - setNodeSelected(id); - } - } + const handleGetSelectedNode = useCallback( + (id: string) => { + if (id !== nodeSelected) { + setNodeSelected(id); + } + }, + [nodeSelected], + ); // Refresh topology data const updateTopologyData = useCallback(async () => { if (processGroups && processGroupsLinks) { - const processGroupsNodes = await TopologyController.getNodesFromSitesOrProcessGroups( - processGroups, - ); + const processGroupsNodes = + TopologyController.getNodesFromSitesOrProcessGroups(processGroups); setNodes(processGroupsNodes); setLinks(TopologyController.getEdgesFromLinks(processGroupsLinks)); diff --git a/src/pages/Topology/components/TopologyProcesses.tsx b/src/pages/Topology/components/TopologyProcesses.tsx index 3d44d9ec..47afae6c 100644 --- a/src/pages/Topology/components/TopologyProcesses.tsx +++ b/src/pages/Topology/components/TopologyProcesses.tsx @@ -37,7 +37,7 @@ import TopologyPanel from './TopologyPanel'; const TopologyProcesses = function () { const navigate = useNavigate(); - const topologyRef = useRef(); + const topologyRef = useRef<{ deselectAll: () => void } | null>(null); const [refetchInterval, setRefetchInterval] = useState(UPDATE_INTERVAL); const [nodes, setNodes] = useState([]); @@ -100,11 +100,14 @@ const TopologyProcesses = function () { navigate(route); } - function handleGetSelectedNode(id: string) { - if (id !== nodeSelected) { - setNodeSelected(id); - } - } + const handleGetSelectedNode = useCallback( + (id: string) => { + if (id !== nodeSelected) { + setNodeSelected(id); + } + }, + [nodeSelected], + ); function handleToggle(isSelectAddressOpen: boolean) { setIsOpen(isSelectAddressOpen); @@ -130,12 +133,8 @@ const TopologyProcesses = function () { processesLinks && !(isLoadingProcessLinksByAddress && addressIdSelected) ) { - const siteNodes = await TopologyController.getNodesFromSitesOrProcessGroups(sites); - - const processesNodes = await TopologyController.getNodesFromProcesses( - processes, - siteNodes, - ); + const siteNodes = TopologyController.getNodesFromSitesOrProcessGroups(sites); + const processesNodes = TopologyController.getNodesFromProcesses(processes, siteNodes); const processesSourcesIds = processesLinksByAddress?.map((p) => p.source) || []; const processesTargetIds = processesLinksByAddress?.map((p) => p.target) || []; const processesAddress = [...processesSourcesIds, ...processesTargetIds]; @@ -187,7 +186,7 @@ const TopologyProcesses = function () { )); const optionsWithDefault = [ - , + , ...(options || []), ]; diff --git a/src/pages/Topology/components/TopologySite.tsx b/src/pages/Topology/components/TopologySite.tsx index 578de529..7906965d 100644 --- a/src/pages/Topology/components/TopologySite.tsx +++ b/src/pages/Topology/components/TopologySite.tsx @@ -19,7 +19,7 @@ const TopologySite = function () { const [nodes, setNodes] = useState([]); const [links, setLinks] = useState([]); - const [nodeIdSelected, setNodeIdSelected] = useState(null); + const [nodeSelected, setNodeSelected] = useState(null); const { data: sites, isLoading: isLoading } = useQuery( [QueriesTopology.GetSitesWithLinksCreated], @@ -39,16 +39,19 @@ const TopologySite = function () { navigate(route); } - function handleGetSelectedNode(id: string) { - if (id !== nodeIdSelected) { - setNodeIdSelected(id); - } - } + const handleGetSelectedNode = useCallback( + (id: string) => { + if (id !== nodeSelected) { + setNodeSelected(id); + } + }, + [nodeSelected], + ); // Refresh topology data const updateTopologyData = useCallback(async () => { if (sites) { - const siteNodes = await TopologyController.getNodesFromSitesOrProcessGroups(sites); + const siteNodes = TopologyController.getNodesFromSitesOrProcessGroups(sites); setNodes(siteNodes); setLinks(TopologyController.getEdgesFromSitesConnected(sites)); @@ -65,7 +68,7 @@ const TopologySite = function () { return ( - {nodeIdSelected && } + {nodeSelected && } ); }; diff --git a/src/pages/Topology/services/index.ts b/src/pages/Topology/services/index.ts index 4a67e78d..c9f2fbb5 100644 --- a/src/pages/Topology/services/index.ts +++ b/src/pages/Topology/services/index.ts @@ -1,5 +1,3 @@ -import { xml } from 'd3-fetch'; - import processSVG from '@assets/service.svg'; import siteSVG from '@assets/site.svg'; import skupperProcessSVG from '@assets/skupper.svg'; @@ -176,14 +174,10 @@ export const TopologyController = { }; }, - getNodesFromSitesOrProcessGroups: async ( + getNodesFromSitesOrProcessGroups: ( entities: SiteResponse[] | ProcessGroupResponse[], - ): Promise => { - const skupperProcessGroupXML = await xml(skupperProcessSVG); - const processGroupXML = await xml(processSVG); - const siteXML = await xml(siteSVG); - - return entities + ): GraphNode[] => + entities ?.sort((a, b) => a.identity.localeCompare(b.identity)) .map((node, index) => { const positions = localStorage.getItem(node.identity); @@ -202,22 +196,15 @@ export const TopologyController = { color: getColor(node.type === 'skupper' ? 16 : index), img: node.type === 'skupper' - ? skupperProcessGroupXML + ? skupperProcessSVG : node.recType === 'SITE' - ? siteXML - : processGroupXML, + ? siteSVG + : processSVG, }; - }); - }, + }), - getNodesFromProcesses: async ( - processes: ProcessResponse[], - siteNodes: GraphNode[], - ): Promise => { - const skupperProcessGroupXML = await xml(skupperProcessSVG); - const processGroupXML = await xml(processSVG); - - return processes + getNodesFromProcesses: (processes: ProcessResponse[], siteNodes: GraphNode[]): GraphNode[] => + processes ?.map(({ name, identity, parent, type }) => { const site = siteNodes?.find(({ id }) => id === parent); const groupIndex = site?.group || 0; @@ -236,11 +223,10 @@ export const TopologyController = { groupName: site?.name || '', group: groupIndex, color: getColor(type === 'skupper' ? 16 : groupIndex), - img: type === 'skupper' ? skupperProcessGroupXML : processGroupXML, + img: type === 'skupper' ? skupperProcessSVG : processSVG, }; }) - .sort((a, b) => a.group - b.group); - }, + .sort((a, b) => a.group - b.group), getEdgesFromLinks: (links: LinkTopology[]): GraphEdge[] => links?.map(({ source, target }) => ({