diff --git a/src/diagram/drawio.ts b/src/diagram/drawio.ts index 663d40d..ce76d4c 100644 --- a/src/diagram/drawio.ts +++ b/src/diagram/drawio.ts @@ -26,10 +26,15 @@ * that silently disappears is worse than a plain one that does not. */ -import type { Diagram, DiagramMachine } from "./model.ts"; +import type { Diagram, DiagramMachine, DiagramSegment } from "./model.ts"; const LANE_MIN_HEIGHT = 170; const LANE_EMPTY_HEIGHT = 62; +/** Depth becomes indentation, which is how "behind" is shown without drawing a line. */ +const INDENT = 40; +/** A gap holding nothing needs no room; one between top-level groups needs a little. */ +const TIGHT_GAP = 24; +const GROUP_GAP = 50; // Wide enough that a gateway placed on a boundary sits BETWEEN the lanes rather than on // top of one — an earlier value let a router's box cover the lane's own name and ranges. const LANE_GAP = 110; @@ -158,10 +163,41 @@ export function toDrawio(diagram: Diagram): string { const cells: Cell[] = []; const edges: { id: string; source: string; target: string }[] = []; - // Lanes, ordered by depth: public first, then each level of private network below it. - const lanes = [...diagram.segments].sort( - (a, b) => a.depth - b.depth || a.name.localeCompare(b.name), - ); + /** + * Order: each public network, then everything behind it, depth first. + * + * A single stack sorted by depth put a private network far from the public one it sits + * behind, so a gateway's link to the outside ran the height of the picture and crossed + * networks it had nothing to do with — two such links overlapped and read as one wire. + * Grouping makes every gateway adjacent to both lanes it joins, and every link short. + */ + const byName = new Map(diagram.segments.map((s) => [s.name, s])); + const childrenOf = new Map(); + for (const segment of diagram.segments) { + if (!segment.behind) continue; + childrenOf.set(segment.behind, [...(childrenOf.get(segment.behind) ?? []), segment.name]); + } + const lanes: DiagramSegment[] = []; + const placed = new Set(); + const walk = (name: string): void => { + const segment = byName.get(name); + if (!segment || placed.has(name)) return; + placed.add(name); + lanes.push(segment); + for (const child of [...(childrenOf.get(name) ?? [])].sort()) walk(child); + }; + // By name, not by the order the source happened to yield them. The hypervisor cannot know + // declaration order, so ordering by it would give the two pictures different shapes and + // make the comparison they exist for impossible to read. + const byNameOrder = [...diagram.segments].sort((a, b) => a.name.localeCompare(b.name)); + for (const segment of byNameOrder) if (segment.kind === "public") walk(segment.name); + for (const segment of byNameOrder) walk(segment.name); // isolated ones + + const laneX = (segment: DiagramSegment) => LANE_X + segment.depth * INDENT; + const laneW = (segment: DiagramSegment) => LANE_WIDTH - segment.depth * INDENT; + const slotsIn = (segment: DiagramSegment) => + Math.max(1, Math.floor((laneW(segment) - 60) / SLOT_WIDTH)); + // A lane is sized to what it holds. Fixed heights meant a lane with enough machines to // wrap onto a second row drew that row outside the box it was supposed to be inside. const occupants = new Map(); @@ -173,8 +209,8 @@ export function toDrawio(diagram: Diagram): string { const only = machine.attachments[0]?.segment; if (only) occupants.set(only, (occupants.get(only) ?? 0) + 1); } - const laneHeight = (name: string): number => { - const rows = Math.ceil((occupants.get(name) ?? 0) / SLOTS_PER_ROW); + const laneHeight = (segment: DiagramSegment): number => { + const rows = Math.ceil((occupants.get(segment.name) ?? 0) / slotsIn(segment)); // A segment whose only occupants are the gateways in the gaps beside it holds nothing // itself, so it collapses to its own name and ranges. Left at full height it padded a // layered scenario with empty boxes and pushed the interesting rows apart. @@ -182,16 +218,46 @@ export function toDrawio(diagram: Diagram): string { return Math.max(LANE_MIN_HEIGHT, 45 + rows * (NODE_HEIGHT + 45)); }; + /** + * Which lane each gateway is drawn above — decided ONCE, by position in the lane order, + * and used both to size the gap and to place the box. + * + * Two rules deciding this separately is what put a gateway inside an unrelated network: + * the gap was reserved above one sibling while the box was drawn above the other, so it + * overflowed into the lane above and landed on top of a machine. + */ + const laneIndex = new Map(lanes.map((segment, index) => [segment.name, index])); + const servedTop = new Map(); + for (const machine of diagram.machines) { + if (machine.kind === "transit" || machine.attachments.length < 2) continue; + const served = machine.attachments + .slice(1) + .map((a) => a.segment) + .filter((n) => laneIndex.has(n)); + if (served.length === 0) continue; + servedTop.set( + machine, + served.reduce((best, n) => ((laneIndex.get(n) ?? 0) < (laneIndex.get(best) ?? 0) ? n : best), served[0]!), + ); + } + const gatewayAbove = new Set(servedTop.values()); + + const transit = diagram.machines.find((m) => m.kind === "transit"); const laneY = new Map(); const laneH = new Map(); - let cursor = 80; + const laneOf = new Map(); + let cursor = transit ? 80 + NODE_HEIGHT + GROUP_GAP : 80; - lanes.forEach((segment) => { + lanes.forEach((segment, index) => { const y = cursor; - const h = laneHeight(segment.name); - cursor = y + h + LANE_GAP; + const h = laneHeight(segment); laneY.set(segment.name, y); laneH.set(segment.name, h); + laneOf.set(segment.name, segment); + const next = lanes[index + 1]; + cursor = + y + h + + (!next ? 0 : gatewayAbove.has(next.name) ? LANE_GAP : next.depth === 0 ? GROUP_GAP : TIGHT_GAP); const facts = [ segment.cidr.join(" "), segment.mtu ? `MTU ${segment.mtu}` : "", @@ -201,9 +267,9 @@ export function toDrawio(diagram: Diagram): string { id: `lane-${segment.name}`, value: `${segment.name}
${facts.join(" · ")}`, style: segment.kind === "public" ? STYLE.publicLane : STYLE.privateLane, - x: LANE_X, + x: laneX(segment), y, - w: LANE_WIDTH, + w: laneW(segment), h, }); }); @@ -226,43 +292,34 @@ export function toDrawio(diagram: Diagram): string { let x: number; let y = 80; - if (machine.attachments.length === 0) { + if (machine.kind === "transit") { + // Transit is not on a boundary — it reaches every public network at once, so a line + // to each would cross everything between. Stated once, at the top, where it applies. + x = LANE_X; + y = 80; + } else if (machine.attachments.length === 0) { // Detached: parked to the side, because it genuinely is nowhere. x = LANE_X + LANE_WIDTH + 60; y = 80 + detachedSlot * (NODE_HEIGHT + 50); detachedSlot++; + } else if (machine.attachments.filter((a) => laneY.has(a.segment)).length > 1) { + // A gateway sits in the gap directly above the lane it SERVES, indented to that lane, + // so it reads as the door into it rather than a box floating nearby. Its first + // attachment is the segment it faces outward on; the rest are the ones it serves. + const top = servedTop.get(machine) ?? machine.attachments[1]?.segment ?? ""; + const segment = laneOf.get(top); + x = (segment ? laneX(segment) : LANE_X) + 40; + y = (laneY.get(top) ?? 80) - LANE_GAP / 2 - NODE_HEIGHT / 2; } else { - const first = machine.attachments[0]!; - const spans = - machine.attachments.filter((a) => laneY.has(a.segment)).length > 1; - - const slot = perLane.get(first.segment) ?? 0; - perLane.set(first.segment, slot + 1); + const only = machine.attachments[0]!.segment; + const segment = laneOf.get(only); + const per = segment ? slotsIn(segment) : SLOTS_PER_ROW; + const slot = perLane.get(only) ?? 0; + perLane.set(only, slot + 1); // Wrap onto a second row rather than running off the end of the lane. An earlier // version placed slot 5 outside the box it was supposed to be inside. - const column = slot % SLOTS_PER_ROW; - const row = Math.floor(slot / SLOTS_PER_ROW); - x = LANE_X + 40 + column * SLOT_WIDTH; - if (spans) { - // A gateway straddles a boundary, and WHICH boundary matters. Its first attachment - // is the segment it faces outward on; the rest are the ones it serves. Placing it - // below the outward lane put a gateway serving `home` and `devices` above an - // unrelated `cafe`, with its connection crossing a network it has nothing to do - // with. It belongs immediately above the topmost lane it actually serves. - const drawn = machine.attachments.map((a) => a.segment).filter((name) => laneY.has(name)); - const served = drawn.slice(1); - const highest = (names: string[]) => - names.reduce((best, name) => ((laneY.get(name) ?? 0) < (laneY.get(best) ?? 0) ? name : best)); - if (served.length > 0) { - y = (laneY.get(highest(served)) ?? 80) - LANE_GAP / 2 - NODE_HEIGHT / 2; - } else { - // Transit faces every lane and serves none, so it sits below the topmost. - const top = highest(drawn); - y = (laneY.get(top) ?? 80) + (laneH.get(top) ?? LANE_MIN_HEIGHT) + LANE_GAP / 2 - NODE_HEIGHT / 2; - } - } else { - y = (laneY.get(first.segment) ?? 80) + 45 + row * (NODE_HEIGHT + 45); - } + x = (segment ? laneX(segment) : LANE_X) + 40 + (slot % per) * SLOT_WIDTH; + y = (laneY.get(only) ?? 80) + 45 + Math.floor(slot / per) * (NODE_HEIGHT + 45); } const addresses = machine.attachments @@ -314,8 +371,18 @@ export function toDrawio(diagram: Diagram): string { }); } + // A link is drawn only where it is short. With the grouping above a gateway is always + // adjacent to both lanes it joins; anything else would be a line crossing networks it + // does not touch, and a lane already says "behind X" in words. + if (machine.kind === "transit") continue; for (const attachment of machine.attachments) { - if (!laneY.has(attachment.segment)) continue; + const ly = laneY.get(attachment.segment); + const lh = laneH.get(attachment.segment); + if (ly === undefined || lh === undefined) continue; + const above = ly + lh <= y && y - (ly + lh) <= LANE_GAP; + const below = y + NODE_HEIGHT <= ly && ly - (y + NODE_HEIGHT) <= LANE_GAP; + const within = y >= ly && y + NODE_HEIGHT <= ly + lh; + if (!above && !below && !within) continue; edges.push({ id: `${id}-e-${attachment.segment}`, source: id, target: `lane-${attachment.segment}` }); } } diff --git a/test/diagram.test.ts b/test/diagram.test.ts index 34de070..64c922a 100644 --- a/test/diagram.test.ts +++ b/test/diagram.test.ts @@ -147,3 +147,161 @@ test("a resource's shape is fixed by kind, and never varies with its metadata", diagram.machines.filter((m) => m.kind === "machine").length, ); }); + +test("no link crosses a network it does not touch", () => { + // The layout fault that made the first drawings unreadable: a gateway placed below the + // lane it FACES, with its link to the outside running the height of the picture through + // three networks it has nothing to do with — and overlapping another such link, so the + // two read as one wire. Every link must now be short and local. + const geometry = new Map(); + for (const match of xml.matchAll( + /<(?:mxCell|object)[^>]*id="([^"]*)"[\s\S]{0,400}? id.startsWith("lane-")) + .map(([id, g]) => ({ name: id.slice(5), top: g.y, bottom: g.y + g.h })); + assert.ok(lanes.length > 0, "no lanes found"); + + for (const edge of xml.matchAll(/]*edge="1"[^>]*source="([^"]*)" target="([^"]*)"/g)) { + const [, , source, target] = edge; + const from = geometry.get(source as string); + const lane = lanes.find((l) => `lane-${l.name}` === target); + if (!from || !lane) continue; + + const span = { top: Math.min(from.y, lane.top), bottom: Math.max(from.y + from.h, lane.bottom) }; + const attached = new Set([target, source]); + for (const other of lanes) { + if (attached.has(`lane-${other.name}`)) continue; + const crosses = other.top >= span.top && other.bottom <= span.bottom; + assert.ok(!crosses, `link ${source}→${target} crosses '${other.name}'`); + } + } +}); + +test("a network behind another is drawn inside it, not merely below it", () => { + // "Behind" is shown by indentation. Without it the reader has only a wire to follow, and + // in a layered scenario that wire is exactly what became unreadable. + const laneX = new Map(); + for (const match of xml.matchAll( + / parent, `${segment.name} is not indented inside ${segment.behind}`); + } +}); + +test("a gateway sits immediately above the network it serves", () => { + // What the grouping guarantees, and the whole reason a gateway reads as the door into a + // network rather than a box floating near one. + // + // It does NOT guarantee adjacency to the network the gateway FACES: a public network with + // two private networks behind it can only put one of them next to it. That case is carried + // by the lane's own "behind …" and by both children being indented to the same depth — + // and the link that would otherwise cross the sibling is suppressed, which is what the + // crossing test above asserts. + const diagram = diagramFromDeclaration(scenario); + const order = [...xml.matchAll(/ m[1] as string); + let checked = 0; + for (const machine of diagram.machines) { + if (machine.kind !== "router") continue; + const served = machine.attachments.slice(1).map((a) => a.segment); + const top = served.reduce((b, n) => (order.indexOf(n) < order.indexOf(b) ? n : b), served[0] ?? ""); + assert.ok(order.includes(top), `${machine.name} serves '${top}', which is not drawn`); + checked++; + } + assert.ok(checked > 0, "no gateways to check"); +}); + +test("a public network is followed by everything behind it, before the next public one", () => { + // The ordering rule itself. Sorting by depth alone put a private network far from the + // public one it sits behind, which is what made the links long in the first place. + const diagram = diagramFromDeclaration(scenario); + const order = [...xml.matchAll(/ m[1] as string); + const rootOf = (name: string): string => { + let current = name; + const seen = new Set(); + for (;;) { + const segment = diagram.segments.find((s) => s.name === current); + if (!segment?.behind || seen.has(current)) return current; + seen.add(current); + current = segment.behind; + } + }; + // Reading down the page, the root never returns to one already left behind. + const finished = new Set(); + let previous = ""; + for (const name of order) { + const root = rootOf(name); + if (root !== previous) { + assert.ok(!finished.has(root), `'${root}' is split apart by another group`); + if (previous) finished.add(previous); + previous = root; + } + } +}); + +test("both sources lay the same topology out identically", () => { + // The comparison is the feature. The hypervisor cannot know declaration order, so laying + // out by it would give the two pictures different shapes and nothing could be read off + // the difference. Proved by shuffling the segments and checking the layout does not move. + const shuffled = { + ...diagramFromDeclaration(scenario), + segments: [...diagramFromDeclaration(scenario).segments].reverse(), + }; + const laneOrder = (out: string) => [...out.matchAll(/ m[1]); + assert.deepEqual(laneOrder(toDrawio(shuffled)), laneOrder(xml)); +}); + +const ALL_SCENARIOS = [ + "bootstrap-single", + "two-on-a-segment", + "behind-nat", + "segmented-and-unforwardable", + "the-ordinary-shape", +]; + +for (const name of ALL_SCENARIOS) { + test(`${name}: no box is drawn inside a network it is not on`, () => { + // A gateway landed on top of a machine in an unrelated network, because the gap was + // reserved above one sibling while the box was placed above the other. Two rules deciding + // the same thing separately; both now read one map. + const each = loadScenario(`scenarios/${name}.yml`); + const out = toDrawio(diagramFromDeclaration(each)); + const boxes: { id: string; y: number; h: number; lanes: string[] }[] = []; + const lanes: { name: string; top: number; bottom: number }[] = []; + for (const match of out.matchAll( + /id="([^"]*)"[\s\S]{0,400}? { + const box = boxes.find((b) => b.id === `m${index}`); + if (box) box.lanes = machine.attachments.map((a) => a.segment); + }); + + for (const box of boxes) { + for (const lane of lanes) { + if (box.lanes.includes(lane.name)) continue; + const overlaps = box.y < lane.bottom && box.y + box.h > lane.top; + assert.ok(!overlaps, `${box.id} is drawn inside '${lane.name}', which it is not on`); + } + } + }); +}