Merge pull request #4081 from jeff-phillips-18/aggregate-edges

Bug 1796465: Fix for aggregate edge selection

frontend/packages/dev-console/src/components/topology/TopologyEdgePanel.tsx

@@ -2,7 +2,7 @@ import * as React from 'react';
 import { connect } from 'react-redux';
 import * as classNames from 'classnames';
 import * as _ from 'lodash';
-import { BaseEdge } from '@console/topology';
+import { Edge } from '@console/topology';
 import { RootState } from '@console/internal/redux';
 import { referenceFor, K8sResourceKind } from '@console/internal/module/k8s';
 import {
@@ -27,7 +27,7 @@ type StateProps = {
 };
 
 export type TopologyEdgePanelProps = {
-  edge: BaseEdge;
+  edge: Edge;
   data: TopologyDataModel;
 } & StateProps;
 

frontend/packages/dev-console/src/components/topology/actions/edgeActions.ts

@@ -1,7 +1,7 @@
 import { KebabOption } from '@console/internal/components/utils/kebab';
 import { modelFor, referenceFor } from '@console/internal/module/k8s';
 import { asAccessReview } from '@console/internal/components/utils';
-import { BaseEdge, Node } from '@console/topology';
+import { Edge, Node } from '@console/topology';
 import { getTopologyResourceObject } from '../topology-utils';
 import { removeConnection } from '../components/removeConnection';
 import {
@@ -16,7 +16,7 @@ import {
 } from '../const';
 import { moveConnectionModal } from '../components/MoveConnectionModal';
 
-const moveConnection = (edge: BaseEdge, availableTargets: Node[]) => {
+const moveConnection = (edge: Edge, availableTargets: Node[]) => {
   const resourceObj = getTopologyResourceObject(edge.getSource().getData());
   const resourceModel = modelFor(referenceFor(resourceObj));
 
@@ -29,7 +29,7 @@ const moveConnection = (edge: BaseEdge, availableTargets: Node[]) => {
   };
 };
 
-const deleteConnection = (edge: BaseEdge) => {
+const deleteConnection = (edge: Edge) => {
   const resourceObj = getTopologyResourceObject(edge.getSource().getData());
   const resourceModel = modelFor(referenceFor(resourceObj));
   return {
@@ -41,7 +41,7 @@ const deleteConnection = (edge: BaseEdge) => {
   };
 };
 
-export const edgeActions = (edge: BaseEdge, nodes: Node[]): KebabOption[] => {
+export const edgeActions = (edge: Edge, nodes: Node[]): KebabOption[] => {
   const actions: KebabOption[] = [];
 
   const availableTargets = nodes

frontend/packages/dev-console/src/components/topology/components/MoveConnectionModal.tsx

@@ -11,7 +11,7 @@ import {
   ModalBody,
   ModalSubmitFooter,
 } from '@console/internal/components/factory/modal';
-import { BaseEdge, Node } from '@console/topology';
+import { Edge, Node } from '@console/topology';
 import FormSection from '../../import/section/FormSection';
 import { RootState } from '@console/internal/redux';
 import { ALLOW_SERVICE_BINDING } from '../../../const';
@@ -22,7 +22,7 @@ interface StateProps {
   serviceBinding: boolean;
 }
 type MoveConnectionModalProps = {
-  edge: BaseEdge;
+  edge: Edge;
   availableTargets: Node[];
   cancel?: () => void;
   close?: () => void;

frontend/packages/dev-console/src/components/topology/components/edges/AggregateEdge.tsx

@@ -12,6 +12,7 @@ const AggregateEdge: React.FC<AggregateEdgeProps> = ({ element }) => {
   const [hover, hoverRef] = useHover();
   const startPoint = element.getStartPoint();
   const endPoint = element.getEndPoint();
+  const { bidirectional } = element.getData();
 
   return (
     <Layer id={hover ? 'top' : undefined}>
@@ -37,9 +38,10 @@ const AggregateEdge: React.FC<AggregateEdgeProps> = ({ element }) => {
           x2={endPoint.x}
           y2={endPoint.y}
         />
-        {(!element.getSource().isCollapsed() || !element.getTarget().isCollapsed()) && (
-          <EdgeConnectorArrow edge={element} />
-        )}
+        {!bidirectional &&
+          (!element.getSource().isCollapsed() || !element.getTarget().isCollapsed()) && (
+            <EdgeConnectorArrow edge={element} />
+          )}
       </g>
     </Layer>
   );

frontend/packages/dev-console/src/components/topology/topology-utils.ts

@@ -55,6 +55,8 @@ import {
   TYPE_OPERATOR_WORKLOAD,
   TYPE_TRAFFIC_CONNECTOR,
   TYPE_WORKLOAD,
+  TYPE_CONNECTS_TO,
+  TYPE_SERVICE_BINDING,
 } from './const';
 
 export const allowedResources = ['deployments', 'deploymentConfigs', 'daemonSets', 'statefulSets'];
@@ -246,7 +248,7 @@ export const getTopologyNodeItem = (
   }
   return {
     id: uid,
-    type: type || 'workload',
+    type: type || TYPE_WORKLOAD,
     name: label || name,
     ...(children && children.length && { children }),
   };
@@ -286,7 +288,7 @@ export const getTopologyEdgeItems = (
     if (targetNode) {
       edges.push({
         id: `${uid}_${targetNode}`,
-        type: 'connects-to',
+        type: TYPE_CONNECTS_TO,
         source: uid,
         target: targetNode,
       });
@@ -315,7 +317,7 @@ export const getTopologyEdgeItems = (
     if (targetNode) {
       edges.push({
         id: `${uid}_${targetNode}`,
-        type: 'service-binding',
+        type: TYPE_SERVICE_BINDING,
         source: uid,
         target: targetNode,
         data: { sbr },

frontend/packages/topology/src/components/ElementWrapper.tsx

@@ -7,22 +7,13 @@ type ElementWrapperProps = {
   element: GraphElement;
 };
 
-// in a separate component so that changes to interaction handlers do not re-render children
+// in a separate component so that changes to behaviors do not re-render children
 const ElementComponent: React.FC<ElementWrapperProps> = observer(({ element }) => {
   const Component = React.useMemo(
     () => element.getController().getComponent(element.getKind(), element.getType()),
     [element],
   );
-  if (!element.isVisible()) {
-    return null;
-  }
-  if (isEdge(element)) {
-    const source = element.getSource();
-    const target = element.getTarget();
-    if ((source && !source.isVisible()) || (target && !target.isVisible())) {
-      return null;
-    }
-  }
+
   return (
     <ElementContext.Provider value={element}>
       <Component {...element.getState()} element={element} />
@@ -50,6 +41,17 @@ const ElementChildren: React.FC<ElementWrapperProps> = observer(({ element }) =>
 });
 
 const ElementWrapper: React.FC<ElementWrapperProps> = observer(({ element }) => {
+  if (!element.isVisible()) {
+    return null;
+  }
+
+  if (isEdge(element)) {
+    const source = element.getSourceAnchorNode();
+    const target = element.getTargetAnchorNode();
+    if ((source && !source.isVisible()) || (target && !target.isVisible())) {
+      return null;
+    }
+  }
   const commonProps = {
     [`data-id`]: element.getId(),
     [`data-kind`]: element.getKind(),

frontend/packages/topology/src/elements/BaseEdge.ts

@@ -1,6 +1,7 @@
 import { observable, computed } from 'mobx';
 import Point from '../geom/Point';
 import { Edge, Node, EdgeModel, ModelKind, AnchorEnd, Anchor } from '../types';
+import { getTopCollapsedParent } from '../utils';
 import BaseElement from './BaseElement';
 
 export default class BaseEdge<E extends EdgeModel = EdgeModel, D = any> extends BaseElement<E, D>
@@ -22,12 +23,12 @@ export default class BaseEdge<E extends EdgeModel = EdgeModel, D = any> extends
 
   @computed
   private get sourceAnchor(): Anchor {
-    return this.getSource().getAnchor(AnchorEnd.source, this.getType());
+    return this.getSourceAnchorNode().getAnchor(AnchorEnd.source, this.getType());
   }
 
   @computed
   private get targetAnchor(): Anchor {
-    return this.getTarget().getAnchor(AnchorEnd.target, this.getType());
+    return this.getTargetAnchorNode().getAnchor(AnchorEnd.target, this.getType());
   }
 
   getKind(): ModelKind {
@@ -56,6 +57,20 @@ export default class BaseEdge<E extends EdgeModel = EdgeModel, D = any> extends
     this.target = target;
   }
 
+  getSourceAnchorNode(): Node {
+    if (!this.source) {
+      throw new Error(`Edge with ID '${this.getId()}' has no source.`);
+    }
+    return getTopCollapsedParent(this.source);
+  }
+
+  getTargetAnchorNode(): Node {
+    if (!this.target) {
+      throw new Error(`Edge with ID '${this.getId()}' has no target.`);
+    }
+    return getTopCollapsedParent(this.target);
+  }
+
   getBendpoints(): Point[] {
     return this.bendpoints || [];
   }

frontend/packages/topology/src/elements/BaseElement.ts

@@ -5,6 +5,7 @@ import {
   Graph,
   GraphElement,
   isGraph,
+  isNode,
   Controller,
   ModelKind,
   ADD_CHILD_EVENT,
@@ -127,7 +128,11 @@ export default abstract class BaseElement<E extends ElementModel = ElementModel,
   }
 
   isVisible(): boolean {
-    return this.visible;
+    return (
+      this.visible &&
+      (!this.parent ||
+        (this.parent.isVisible() && (!isNode(this.parent) || !this.parent.isCollapsed())))
+    );
   }
 
   getData(): D | undefined {

frontend/packages/topology/src/layouts/BaseLayout.ts

@@ -12,7 +12,11 @@ import {
   NODE_COLLAPSE_CHANGE_EVENT,
   NodeCollapseChangeEventListener,
 } from '../types';
-import { leafNodeElements, groupNodeElements, getTopCollapsedParent } from '../utils/element-utils';
+import {
+  leafNodeElements,
+  groupNodeElements,
+  getClosestVisibleParent,
+} from '../utils/element-utils';
 import {
   DRAG_MOVE_OPERATION,
   DRAG_NODE_END_EVENT,
@@ -386,7 +390,7 @@ class BaseLayout implements Layout {
     );
   };
 
-  protected getLayoutNode(nodes: LayoutNode[], node: Node): LayoutNode | undefined {
+  protected getLayoutNode(nodes: LayoutNode[], node: Node | null): LayoutNode | undefined {
     if (!node) {
       return undefined;
     }
@@ -396,7 +400,7 @@ class BaseLayout implements Layout {
       colaNode = _.find(nodes, { id: node.getChildren()[0].getId() });
     }
     if (!colaNode) {
-      colaNode = this.getLayoutNode(nodes, getTopCollapsedParent(node));
+      colaNode = this.getLayoutNode(nodes, getClosestVisibleParent(node));
     }
 
     return colaNode;

frontend/packages/topology/src/types.ts

@@ -130,6 +130,8 @@ export interface Edge<E extends EdgeModel = EdgeModel, D = any> extends GraphEle
   setSource(source: Node): void;
   getTarget(): Node;
   setTarget(target: Node): void;
+  getSourceAnchorNode(): Node;
+  getTargetAnchorNode(): Node;
   getStartPoint(): Point;
   setStartPoint(x?: number, y?: number): void;
   getEndPoint(): Point;

frontend/packages/topology/src/utils/createAggregateEdges.ts

@@ -29,39 +29,69 @@ const createAggregateEdges = (
   edges: EdgeModel[] | undefined,
   nodes: NodeModel[] | undefined,
 ): EdgeModel[] => {
+  const aggregateEdges: EdgeModel[] = [];
+
   return _.reduce(
     edges,
     (newEdges: EdgeModel[], edge: EdgeModel) => {
       const source = getDisplayedNodeForNode(edge.source, nodes);
       const target = getDisplayedNodeForNode(edge.target, nodes);
+
+      // Make sure visible is defined so that changes override what could already be in the element
+      edge.visible = 'visible' in edge ? edge.visible : true;
+
       if (source !== edge.source || target !== edge.target) {
-        edge.visible = false;
         if (source !== target) {
-          const existing = newEdges.find(
+          const existing = aggregateEdges.find(
             (e) =>
               (e.source === source || e.source === target) &&
               (e.target === target || e.target === source),
           );
+
           if (existing) {
-            existing.type = aggregateEdgeType;
+            // At least one other edge, add this edge and add the aggregate edge to the edges
+
+            // Add this edge to the aggregate and set it not visible
             existing.children && existing.children.push(edge.id);
-            if (existing.source !== edge.source) {
-              existing.data.bidirectional = true;
+            edge.visible = false;
+
+            // Hide edges that are depicted by this aggregate edge
+            _.forEach(existing.children, (existingChild) => {
+              const updateEdge = newEdges.find((newEdge) => newEdge.id === existingChild);
+              if (updateEdge) {
+                updateEdge.visible = false;
+              }
+            });
+
+            // Update the aggregate edges bidirectional flag
+            existing.data.bidirectional =
+              existing.data.bidirectional || existing.source !== edge.source;
+
+            // Check if this edge has already been added
+            if (
+              !newEdges.find(
+                (e) =>
+                  (e.source === source || e.source === target) &&
+                  (e.target === target || e.target === source),
+              )
+            ) {
+              newEdges.push(existing);
             }
           } else {
             const newEdge: EdgeModel = {
               data: { bidirectional: false },
               children: [edge.id],
               source,
               target,
-              id: `${source}_${target}`,
-              type: edge.type,
+              id: `aggregate_${source}_${target}`,
+              type: aggregateEdgeType,
             };
-            newEdges.push(newEdge);
+            aggregateEdges.push(newEdge);
           }
+        } else {
+          // Hide edges that connect to a non-visible node to its ancestor
+          edge.visible = false;
         }
-      } else {
-        edge.visible = true;
       }
       newEdges.push(edge);
       return newEdges;

frontend/packages/topology/src/utils/element-utils.ts

@@ -61,6 +61,29 @@ const getTopCollapsedParent = (node: Node): Node => {
   return returnNode;
 };
 
+const getClosestVisibleParent = (node: Node): Node | null => {
+  if (!node) {
+    return null;
+  }
+
+  let returnNode: Node | null = node.isVisible() ? node : null;
+  try {
+    let parent = node.getParent();
+    while (parent) {
+      if (!parent.isVisible()) {
+        // parent isn't visible so no descendant could be visible
+        returnNode = null;
+      } else if ((parent as Node).isCollapsed() || !returnNode) {
+        // parent is collapsed, no descendant is visible, but parent is
+        returnNode = parent as Node;
+      }
+      parent = parent.getParent();
+    }
+    // eslint-disable-next-line no-empty
+  } catch (e) {}
+  return returnNode;
+};
+
 const getElementPadding = (element: GraphElement): number => {
   const stylePadding = element.getStyle<GroupStyle>().padding;
   if (!stylePadding) {
@@ -95,6 +118,7 @@ export {
   groupNodeElements,
   leafNodeElements,
   getTopCollapsedParent,
+  getClosestVisibleParent,
   getElementPadding,
   getGroupPadding,
 };