Merge pull request #134 from jpinsonneau/293

NETOBSERV-293 Topology: Expand groups crash after refresh
netobserv · Apr 28, 2022 · 2c97fe8 · 2c97fe8
2 parents b387200 + 71dc9c9
commit 2c97fe8
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 92 deletions.
diff --git a/web/src/components/netflow-topology/__tests__/netflow-topology.spec.tsx b/web/src/components/netflow-topology/__tests__/netflow-topology.spec.tsx
@@ -1,7 +1,7 @@
 import { EmptyState, EmptyStateBody, Spinner } from '@patternfly/react-core';
 import { shallow } from 'enzyme';
 import * as React from 'react';
-import { MetricFunction, MetricType, Match } from '../../../model/flow-query';
+import { MetricFunction, MetricType } from '../../../model/flow-query';
 import { TopologyMetrics } from '../../../api/loki';
 import { DefaultOptions, LayoutName } from '../../../model/topology';
 import { defaultTimeRange } from '../../../utils/router';
@@ -14,8 +14,6 @@ describe('<NetflowTopology />', () => {
     range: defaultTimeRange,
     metricFunction: 'sum' as MetricFunction,
     metricType: 'bytes' as MetricType,
-    match: 'any' as Match,
-    limit: 100,
     metrics: [] as TopologyMetrics[],
     layout: LayoutName.Cola,
     options: DefaultOptions,

diff --git a/web/src/components/netflow-topology/netflow-topology.tsx b/web/src/components/netflow-topology/netflow-topology.tsx
@@ -16,6 +16,7 @@ import {
   defaultControlButtonsOptions,
   GraphElement,
   Model,
+  Node,
   SelectionEventListener,
   SELECTION_EVENT,
   TopologyControlBar,
@@ -33,7 +34,7 @@ import { saveSvgAsPng } from 'save-svg-as-png';
 import { findFilter } from '../../utils/filter-definitions';
 import { TopologyMetrics } from '../../api/loki';
 import { Filter, FilterDefinition } from '../../model/filters';
-import { Match, MetricFunction, MetricType } from '../../model/flow-query';
+import { MetricFunction, MetricType } from '../../model/flow-query';
 import { generateDataModel, LayoutName, TopologyOptions } from '../../model/topology';
 import { TimeRange } from '../../utils/datetime';
 import { usePrevious } from '../../utils/previous-hook';
@@ -55,8 +56,6 @@ const TopologyContent: React.FC<{
   range: number | TimeRange;
   metricFunction?: MetricFunction;
   metricType?: MetricType;
-  match: Match;
-  limit: number;
   metrics: TopologyMetrics[];
   options: TopologyOptions;
   layout: LayoutName;
@@ -69,8 +68,6 @@ const TopologyContent: React.FC<{
   range,
   metricFunction,
   metricType,
-  match,
-  limit,
   metrics,
   layout,
   options,
@@ -86,10 +83,7 @@ const TopologyContent: React.FC<{
   const prevLayout = usePrevious(layout);
   const prevMetricFunction = usePrevious(metricFunction);
   const prevMetricType = usePrevious(metricType);
-  const prevMatch = usePrevious(match);
-  const prevLimit = usePrevious(limit);
   const prevOptions = usePrevious(options);
-  const prevFilters = usePrevious(filters);
 
   const [selectedIds, setSelectedIds] = React.useState<string[]>([]);
   const [hoveredId, setHoveredId] = React.useState<string>('');
@@ -284,17 +278,32 @@ const TopologyContent: React.FC<{
       highlightedId = selectedIds[0];
     }
 
-    const currentModel = controller.toModel();
-    const mergedModel = generateDataModel(
-      metrics,
-      getOptions(),
-      searchValue,
-      highlightedId,
-      filters,
-      currentModel.nodes,
-      currentModel.edges
-    );
-    controller.fromModel(mergedModel);
+    const updatedModel = generateDataModel(metrics, getOptions(), searchValue, highlightedId, filters);
+    const allIds = [...(updatedModel.nodes || []), ...(updatedModel.edges || [])].map(item => item.id);
+    controller.getElements().forEach(e => {
+      if (e.getType() !== 'graph') {
+        if (allIds.includes(e.getId())) {
+          //keep previous data
+          switch (e.getType()) {
+            case 'node':
+              const updatedNode = updatedModel.nodes?.find(n => n.id === e.getId());
+              if (updatedNode) {
+                updatedNode.data = { ...e.getData(), ...updatedNode.data };
+              }
+              break;
+            case 'group':
+              const updatedGroup = updatedModel.nodes?.find(n => n.id === e.getId());
+              if (updatedGroup) {
+                updatedGroup.collapsed = (e as Node).isCollapsed();
+              }
+              break;
+          }
+        } else {
+          controller.removeElement(e);
+        }
+      }
+    });
+    controller.fromModel(updatedModel);
   }, [controller, hoveredId, selectedIds, metrics, getOptions, searchValue, filters]);
 
   //update model on layout / options / metrics / filters change
@@ -307,38 +316,19 @@ const TopologyContent: React.FC<{
     updateModel();
   }, [controller, metrics, filters, layout, options, prevLayout, prevOptions, resetGraph, updateModel]);
 
-  //clear existing elements on query change before getting new metrics
+  //clear existing edge tags on query change before getting new metrics
   React.useEffect(() => {
-    if (prevFilters !== filters || (prevMatch && prevMatch !== match) || (prevLimit && prevLimit > limit)) {
-      //remove all elements except graph
-      if (controller && controller.hasGraph()) {
+    if (controller && controller.hasGraph()) {
+      if (prevMetricFunction !== metricFunction || prevMetricType !== metricType) {
+        //remove edge tags on metrics change
         controller.getElements().forEach(e => {
-          if (e.getType() !== 'graph') {
-            controller.removeElement(e);
+          if (e.getType() === 'edge') {
+            e.setData({ ...e.getData(), tag: undefined });
           }
         });
       }
-    } else if (prevMetricFunction !== metricFunction || prevMetricType !== metricType) {
-      //remove edge tags on metrics change
-      controller.getElements().forEach(e => {
-        if (e.getType() === 'edge') {
-          e.setData({ ...e.getData(), tag: undefined });
-        }
-      });
     }
-  }, [
-    controller,
-    filters,
-    limit,
-    match,
-    metricFunction,
-    metricType,
-    prevFilters,
-    prevLimit,
-    prevMatch,
-    prevMetricFunction,
-    prevMetricType
-  ]);
+  }, [controller, metricFunction, metricType, prevMetricFunction, prevMetricType]);
 
   //refresh UI selected items
   React.useEffect(() => {
@@ -469,8 +459,6 @@ const NetflowTopology: React.FC<{
   range: number | TimeRange;
   metricFunction?: MetricFunction;
   metricType?: MetricType;
-  match: Match;
-  limit: number;
   metrics: TopologyMetrics[];
   options: TopologyOptions;
   layout: LayoutName;
@@ -485,8 +473,6 @@ const NetflowTopology: React.FC<{
   range,
   metricFunction,
   metricType,
-  match,
-  limit,
   metrics,
   layout,
   options,
@@ -531,8 +517,6 @@ const NetflowTopology: React.FC<{
           range={range}
           metricFunction={metricFunction}
           metricType={metricType}
-          match={match}
-          limit={limit}
           metrics={metrics}
           layout={layout}
           options={options}

diff --git a/web/src/components/netflow-traffic.tsx b/web/src/components/netflow-traffic.tsx
@@ -452,8 +452,6 @@ export const NetflowTraffic: React.FC<{
             range={range}
             metricFunction={metricFunction}
             metricType={metricType}
-            match={match}
-            limit={limit}
             metrics={metrics}
             layout={layout}
             options={topologyOptions}

diff --git a/web/src/model/topology.ts b/web/src/model/topology.ts
@@ -89,9 +89,7 @@ export const generateNode = (
   options: TopologyOptions,
   searchValue: string,
   highlightedId: string,
-  filters: Filter[],
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  previousDatas?: any
+  filters: Filter[]
 ): NodeModel => {
   const id = `${type}.${namespace}.${name}.${addr}`;
   const label = name ? name : addr;
@@ -110,8 +108,6 @@ export const generateNode = (
     status: NodeStatus.default,
     style: { padding: 20 },
     data: {
-      //keep previous datas between draws like isPinned, setPosition and point
-      ...previousDatas,
       namespace,
       type,
       name,
@@ -240,42 +236,14 @@ export const generateDataModel = (
   options: TopologyOptions,
   searchValue: string,
   highlightedId: string,
-  filters: Filter[],
-  nodes: NodeModel[] = [],
-  edges: EdgeModel[] = []
+  filters: Filter[]
 ): Model => {
+  let nodes: NodeModel[] = [];
+  const edges: EdgeModel[] = [];
   const opts = { ...DefaultOptions, ...options };
   //ensure each child to have single parent
   const childIds: string[] = [];
 
-  //refresh existing items
-  nodes = nodes.map(node =>
-    node.type === 'group'
-      ? //clear group children
-        { ...node, children: [] }
-      : {
-          ...node,
-          //update options and filter indicators
-          ...generateNode(
-            node.data.namespace,
-            node.data.type,
-            node.data.name,
-            node.data.addr,
-            node.data.host,
-            opts,
-            searchValue,
-            highlightedId,
-            filters,
-            node.data
-          )
-        }
-  );
-  edges = edges.map(edge => ({
-    ...edge,
-    //update options and reset counter
-    ...generateEdge(edge.source!, edge.target!, 0, opts, edge.data.shadowed, highlightedId)
-  }));
-
   function addGroup(name: string, type: string, parent?: NodeModel, secondaryLabelPadding = false) {
     let group = nodes.find(g => g.type === 'group' && g.data.type === type && g.data.name === name);
     if (!group) {