From 2aafb72676f64992705586baa1e084ff945f9774 Mon Sep 17 00:00:00 2001 From: oliver Date: Wed, 5 Aug 2026 21:48:08 +0800 Subject: [PATCH] Fix topology truncate false positives and reduce graph/discover load. Batch ManagedNE lookups, correct project-neighbors truncation/frozen signals, page discover polls, and delete both nodes and edges when multi-selected. Co-authored-by: Cursor --- netx_api/topology_router.py | 9 ++- netx_api/topology_views_graph.py | 111 ++++++++++++++++++++++++++----- tests/test_topology.py | 8 +++ web/src/i18n/en.ts | 4 ++ web/src/i18n/zh.ts | 3 + web/src/pages/TopologyPage.tsx | 63 ++++++++++++++++-- web/src/services/api.ts | 13 +++- 7 files changed, 186 insertions(+), 25 deletions(-) diff --git a/netx_api/topology_router.py b/netx_api/topology_router.py index 8b70610..704c72c 100644 --- a/netx_api/topology_router.py +++ b/netx_api/topology_router.py @@ -224,8 +224,13 @@ def api_fabric_reconcile_links(db: Session = Depends(get_db)) -> dict[str, Any]: @router.get("/fabric/discover/{job_id}") -def api_fabric_discover_job(job_id: str, db: Session = Depends(get_db)) -> dict[str, Any]: - return get_discover_job(db, job_id).model_dump() +def api_fabric_discover_job( + job_id: str, + page: int = Query(default=1, ge=1), + page_size: int = Query(default=20, ge=1, le=100), + db: Session = Depends(get_db), +) -> dict[str, Any]: + return get_discover_job(db, job_id, page=page, page_size=page_size).model_dump() # --- Tree / folders --------------------------------------------------------- diff --git a/netx_api/topology_views_graph.py b/netx_api/topology_views_graph.py index bc04bb9..c1a5f32 100644 --- a/netx_api/topology_views_graph.py +++ b/netx_api/topology_views_graph.py @@ -24,6 +24,8 @@ from .topology_common import ( ROOT_FOLDER_NAME, VIEW_GRAPH_EDGE_HARD_CAP, VIEW_GRAPH_NODE_HARD_CAP, + _EDGE_STATUS_MISSING, + _EDGE_STATUS_MISSING_COMPAT, _LEGACY_UNASSIGNED_NAME, _normalize_edge_status, _utcnow, @@ -108,6 +110,39 @@ def _managed_source_for_node(db: Session, n: TopoFabricNode) -> str: return "" +def _batch_ne_lookups( + db: Session, fabric_nodes: dict[str, TopoFabricNode] +) -> tuple[dict[str, ManagedNE], dict[str, UmeInventoryNE]]: + mids = sorted( + { + str(fn.managed_ne_id or "").strip() + for fn in fabric_nodes.values() + if str(fn.managed_ne_id or "").strip() + } + ) + uids = sorted( + { + str(fn.ume_ne_id or "").strip() + for fn in fabric_nodes.values() + if str(fn.ume_ne_id or "").strip() + } + ) + managed = ( + {str(n.id): n for n in db.query(ManagedNE).filter(ManagedNE.id.in_(mids)).all()} + if mids + else {} + ) + ume = ( + { + str(u.ne_id): u + for u in db.query(UmeInventoryNE).filter(UmeInventoryNE.ne_id.in_(uids)).all() + } + if uids + else {} + ) + return managed, ume + + def get_view_graph(db: Session, view_id: str) -> TopologyViewGraphOut: view = _get_view_or_404(db, view_id) vnodes = db.query(TopoViewNode).filter(TopoViewNode.view_id == view.id).all() @@ -121,6 +156,7 @@ def get_view_graph(db: Session, view_id: str) -> TopologyViewGraphOut: fabric_nodes = { n.id: n for n in db.query(TopoFabricNode).filter(TopoFabricNode.id.in_(fids)).all() } if fids else {} + managed_by_id, ume_by_id = _batch_ne_lookups(db, fabric_nodes) filt = dict(view.filter or {}) layer = str(filt.get("layer") or "physical").strip() or "physical" status = str(filt.get("status") or "").strip().lower() @@ -130,6 +166,17 @@ def get_view_graph(db: Session, view_id: str) -> TopologyViewGraphOut: label = (vn.label or "").strip() if not label and fn is not None: label = (fn.name or fn.ip or vn.fabric_node_id)[:256] + connect_status = "" + managed_source = "" + if fn is not None: + mid = str(fn.managed_ne_id or "").strip() + uid = str(fn.ume_ne_id or "").strip() + if mid and mid in managed_by_id: + mne = managed_by_id[mid] + connect_status = mne.connect_status or "" + managed_source = str(mne.source or "").strip() + elif uid and uid in ume_by_id: + connect_status = ume_by_id[uid].connection_status or "" nodes_out.append( ViewNodeOut( fabric_node_id=vn.fabric_node_id, @@ -143,8 +190,8 @@ def get_view_graph(db: Session, view_id: str) -> TopologyViewGraphOut: ip=(fn.ip if fn else "") or "", vendor=(fn.vendor if fn else "") or "", device_type=(fn.device_type if fn else "") or "", - connect_status=_connect_status_for_node(db, fn) if fn else "", - managed_source=_managed_source_for_node(db, fn) if fn else "", + connect_status=connect_status, + managed_source=managed_source, ) ) edges_out: list[ViewEdgeOut] = [] @@ -925,7 +972,10 @@ def project_fabric_neighbors_to_view( view = _get_view_or_404(db, view_id) mem = _membership_for_view(view) if bool(mem.get("frozen")): - return get_view_graph(db, view.id) + g = get_view_graph(db, view.id) + g.truncated = True + g.truncate_reason = "membership_frozen" + return g max_nodes = int(mem.get("max_nodes") or 300) hops = int(mem.get("expand_hops") or 1) @@ -934,14 +984,34 @@ def project_fabric_neighbors_to_view( req = body or ViewProjectNeighborsRequest() vnodes = db.query(TopoViewNode).filter(TopoViewNode.view_id == view.id).all() + fids_on_view = [vn.fabric_node_id for vn in vnodes] + fabric_on_view = ( + { + n.id: n + for n in db.query(TopoFabricNode).filter(TopoFabricNode.id.in_(fids_on_view)).all() + } + if fids_on_view + else {} + ) # Drop placements pointing at missing fabric rows only (keep LLDP placeholders). - orphan_vns = [vn for vn in vnodes if db.get(TopoFabricNode, vn.fabric_node_id) is None] + orphan_vns = [vn for vn in vnodes if vn.fabric_node_id not in fabric_on_view] if orphan_vns: for vn in orphan_vns: db.delete(vn) view.updated_at = _utcnow() db.commit() vnodes = db.query(TopoViewNode).filter(TopoViewNode.view_id == view.id).all() + fids_on_view = [vn.fabric_node_id for vn in vnodes] + fabric_on_view = ( + { + n.id: n + for n in db.query(TopoFabricNode) + .filter(TopoFabricNode.id.in_(fids_on_view)) + .all() + } + if fids_on_view + else {} + ) existing = {vn.fabric_node_id for vn in vnodes} if not existing: @@ -955,13 +1025,12 @@ def project_fabric_neighbors_to_view( seed_ids: set[str] = { str(x).strip() for x in (req.seed_fabric_node_ids or []) if str(x).strip() } - for mid in req.managed_ne_ids or []: - mid_s = str(mid or "").strip() - if not mid_s: - continue - for fid in existing: - fn = db.get(TopoFabricNode, fid) - if fn is not None and str(fn.managed_ne_id or "").strip() == mid_s: + want_mids = { + str(mid or "").strip() for mid in (req.managed_ne_ids or []) if str(mid or "").strip() + } + if want_mids: + for fid, fn in fabric_on_view.items(): + if str(fn.managed_ne_id or "").strip() in want_mids: seed_ids.add(fid) if seed_ids: seed_ids &= existing @@ -971,22 +1040,30 @@ def project_fabric_neighbors_to_view( seed_ids = set(existing) peer_ids = _neighbor_ids(db, seed_ids=seed_ids, layer=layer, hops=hops) - to_add: list[str] = [] + peer_rows = ( + { + n.id: n + for n in db.query(TopoFabricNode).filter(TopoFabricNode.id.in_(list(peer_ids))).all() + } + if peer_ids + else {} + ) + eligible: list[str] = [] for peer in sorted(peer_ids): if peer in existing: continue - fn = db.get(TopoFabricNode, peer) + fn = peer_rows.get(peer) if fn is None or not _is_inventory_node(fn): continue if _fabric_match_score(db, fn) < 2: continue if not _fabric_in_hard_scope(db, fn, mem): continue - to_add.append(peer) - if len(existing) + len(to_add) >= max_nodes: - break + eligible.append(peer) - truncated = len(peer_ids) > len(to_add) + room = max(0, max_nodes - len(existing)) + to_add = eligible[:room] + truncated = len(eligible) > len(to_add) if to_add: _place_fabric_ids_on_view( db, view, to_add, existing=existing, near_fabric_ids=seed_ids diff --git a/tests/test_topology.py b/tests/test_topology.py index 242370b..38e7bee 100644 --- a/tests/test_topology.py +++ b/tests/test_topology.py @@ -705,6 +705,14 @@ class FabricTopologyTests(unittest.TestCase): self.assertIn(nodes[2].id, ids2) self.assertNotIn(nodes[3].id, ids2) + # Peers already on canvas must not raise a false truncated banner. + g3 = svc.project_fabric_neighbors_to_view( + self.db, + view2.id, + ViewProjectNeighborsRequest(seed_fabric_node_ids=[nodes[0].id]), + ) + self.assertFalse(g3.truncated) + pop = svc.populate_view( self.db, view.id, diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index 12049b1..4e8a4b3 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -1413,9 +1413,13 @@ const en = { edgeDeleted: "Deleted {{count}} link(s)", discoverCancelled: "Discovery cancelled", truncatedMembership: "Membership cap reached; some neighbors were not placed. Use classify/slices or a new map.", + truncatedFrozen: "This map membership is frozen; neighbors cannot be projected. Unfreeze in classify/slices or create a new map.", truncatedNodes: "Too many view nodes; display truncated. Narrow membership or create another map.", truncatedEdges: "Too many edges; display truncated. Filter status or create another map.", truncatedGeneric: "Graph data truncated. Narrow scope or create another map.", + deleteSelectionConfirm: + "Remove {{nodes}} NE(s) from this map and delete {{edges}} Fabric link(s)?", + selectionDeleted: "Removed {{nodes}} NE(s) and deleted {{edges}} link(s)", newMap: "New", newMapName: "New topology", rename: "Rename", diff --git a/web/src/i18n/zh.ts b/web/src/i18n/zh.ts index cf8327d..1b7cef3 100644 --- a/web/src/i18n/zh.ts +++ b/web/src/i18n/zh.ts @@ -1404,9 +1404,12 @@ const zh = { edgeDeleted: "已删除 {{count}} 条链路", discoverCancelled: "发现已取消", truncatedMembership: "视图已达成员上限，部分邻居未上图。可去分类/切片或新建视图。", + truncatedFrozen: "当前视图已冻结成员，无法投影邻居。请在分类/切片中解冻或新建视图。", truncatedNodes: "画布节点过多已截断显示。建议新建视图或收紧成员范围。", truncatedEdges: "画布链路过多已截断显示。建议筛选状态或新建视图。", truncatedGeneric: "图数据已截断显示。建议收紧范围或新建视图。", + deleteSelectionConfirm: "删除选中的 {{nodes}} 个网元（移出画布）和 {{edges}} 条链路（从 Fabric 删除）？", + selectionDeleted: "已移除 {{nodes}} 个网元、删除 {{edges}} 条链路", newMap: "新建", newMapName: "新拓扑图", rename: "重命名", diff --git a/web/src/pages/TopologyPage.tsx b/web/src/pages/TopologyPage.tsx index e48eacc..018d78c 100644 --- a/web/src/pages/TopologyPage.tsx +++ b/web/src/pages/TopologyPage.tsx @@ -1580,7 +1580,7 @@ export function TopologyPage() { cancelled = true; break; } - job = await fetchLldpDiscoverJob(jobStart.id); + job = await fetchLldpDiscoverJob(jobStart.id, { page: 1, pageSize: 5 }); setDiscoverProgress((p) => ({ ...p, index: job.done, @@ -1619,12 +1619,21 @@ export function TopologyPage() { if (job.status === "failed") { throw new Error(job.error || "discover_failed"); } + // Final page: pull a fuller item slice for the summary panel. + try { + job = await fetchLldpDiscoverJob(jobStart.id, { page: 1, pageSize: 100 }); + } catch { + /* keep last polled job */ + } const projected = discoverProjectNeighbors ? await projectTopologyNeighbors(mapId, { seed_fabric_node_ids: scoped.map((n) => n.id), }) : await fetchTopologyGraph(mapId); queryClient.setQueryData(queryKeys.topologyGraph(mapId), projected); + if (discoverProjectNeighbors && projected.truncate_reason === "membership_frozen") { + showError(t("topology.truncatedFrozen")); + } appliedMapIdRef.current = mapId; let { rfNodes, rfEdges } = graphToFlow(projected.nodes, projected.edges, edgeDefaults); // Keep existing node positions when we did not auto-layout. @@ -2021,10 +2030,15 @@ export function TopologyPage() { }, [setNodes, setEdges]); const persistDeleteEdges = useCallback( - async (edgeIds: string[], opts?: { confirmKey?: string; okKey?: string }) => { + async ( + edgeIds: string[], + opts?: { confirmKey?: string; okKey?: string; skipConfirm?: boolean }, + ) => { if (!mapId || !edgeIds.length) return false; - const confirmMsg = t(opts?.confirmKey || "topology.deleteEdgeConfirm"); - if (!window.confirm(confirmMsg)) return false; + if (!opts?.skipConfirm) { + const confirmMsg = t(opts?.confirmKey || "topology.deleteEdgeConfirm"); + if (!window.confirm(confirmMsg)) return false; + } pushHistory(); try { await deleteFabricEdges(edgeIds); @@ -2061,10 +2075,44 @@ export function TopologyPage() { if (e.selected) edgeIds.add(e.id); } if (!nodeIds.length && !edgeIds.size) return; + + if (nodeIds.length && edgeIds.size) { + const msg = t("topology.deleteSelectionConfirm") + .replace("{{nodes}}", String(nodeIds.length)) + .replace("{{edges}}", String(edgeIds.size)); + if (!window.confirm(msg)) return; + pushHistory(); + try { + if (dirtyRef.current) { + await patchTopologyPositions(mapId, flowToPositions(nodes)); + clearDirty(); + } + await deleteFabricEdges([...edgeIds]); + const localPos = new Map(nodes.map((n) => [n.id, n.position])); + const graph = await removeTopologyViewNodes(mapId, nodeIds); + queryClient.setQueryData(queryKeys.topologyGraph(mapId), graph); + appliedMapIdRef.current = mapId; + historyLockRef.current = true; + applyViewGraph(graph, edgeDefaults, setNodes, setEdges, localPos); + historyLockRef.current = false; + clearDirty(); + setSelectedEdgeId(null); + showOk( + t("topology.selectionDeleted") + .replace("{{nodes}}", String(nodeIds.length)) + .replace("{{edges}}", String(edgeIds.size)), + ); + } catch (err) { + showError(String(err)); + } + return; + } + if (!nodeIds.length && edgeIds.size) { await persistDeleteEdges([...edgeIds]); return; } + pushHistory(); try { if (dirtyRef.current) { @@ -2095,7 +2143,9 @@ export function TopologyPage() { edgeDefaults, clearDirty, showError, + showOk, persistDeleteEdges, + t, ]); const removeEdgeById = (edgeId: string) => { @@ -2135,6 +2185,10 @@ export function TopologyPage() { applyViewGraph(projected, edgeDefaults, setNodes, setEdges, localPos); historyLockRef.current = false; clearDirty(); + if (projected.truncate_reason === "membership_frozen") { + showError(t("topology.truncatedFrozen")); + return; + } const added = Math.max(0, projected.nodes.length - before); showOk(t("topology.projectedNeighbors").replace("{{count}}", String(added))); if (added > 0) { @@ -2668,6 +2722,7 @@ export function TopologyPage() { const truncateBannerText = useMemo(() => { if (!graphTruncated) return ""; if (truncateReason === "membership_cap") return t("topology.truncatedMembership"); + if (truncateReason === "membership_frozen") return t("topology.truncatedFrozen"); if (truncateReason === "too_many_view_nodes") return t("topology.truncatedNodes"); if (truncateReason === "too_many_edges") return t("topology.truncatedEdges"); return t("topology.truncatedGeneric"); diff --git a/web/src/services/api.ts b/web/src/services/api.ts index 6570a4a..5881103 100644 --- a/web/src/services/api.ts +++ b/web/src/services/api.ts @@ -1225,8 +1225,17 @@ export const startLldpDiscover = (body?: { trigger_mode?: "manual" | "schedule" | "topology"; }) => apiPost("/v1/topology/fabric/discover", body || {}); -export const fetchLldpDiscoverJob = (jobId: string) => - apiGet(`/v1/topology/fabric/discover/${encodeURIComponent(jobId)}`); +export const fetchLldpDiscoverJob = ( + jobId: string, + params?: { page?: number; pageSize?: number }, +) => { + const p = new URLSearchParams(); + p.set("page", String(Math.max(1, Number(params?.page || 1)))); + p.set("page_size", String(Math.max(1, Math.min(100, Number(params?.pageSize || 20))))); + return apiGet( + `/v1/topology/fabric/discover/${encodeURIComponent(jobId)}?${p.toString()}`, + ); +}; export const fetchLldpCollectDashboard = () => apiGet("/v1/topology/lldp-collect/dashboard");