fix(central-ui): InstanceTreeBuilder — orphaned instances surface at root; read-only Children; edge tests
Review follow-ups on f12ba2f9:
- An instance whose AreaId points at an area outside the supplied list was
silently dropped; it now surfaces at the root, mirroring the orphaned-area
rule, and stays filterable.
- InstanceTreeNode.Children is an IReadOnlyList over a private backing list,
composed by the builder through an internal AddChild — the record no longer
hands callers a mutable list.
- Extra tests: unknown-area instance (root + filterable), 3-level nesting,
a ParentAreaId cycle that must neither loop nor throw, and a filter that
matches instance names only (an area name match alone does not keep it).
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
This commit is contained in:
@@ -14,9 +14,9 @@ public static class InstanceTreeBuilder
|
||||
/// instances beneath it, mirroring the Topology page's ordering: at every level the
|
||||
/// child areas come first (sorted by name, ordinal-ignore-case), then that level's
|
||||
/// instances (sorted by unique name, ordinal-ignore-case). Instances with no area sit
|
||||
/// at the root after the root areas. An area whose parent id points at an area that is
|
||||
/// not in <paramref name="areas"/> is treated as a root, so a stale row can never hide
|
||||
/// a subtree.
|
||||
/// at the root after the root areas. A row whose parent points at an area that is not in
|
||||
/// <paramref name="areas"/> is treated as a root — both an orphaned area and an instance
|
||||
/// with an unknown area id — so a stale row can never hide a subtree or an instance.
|
||||
/// </summary>
|
||||
/// <param name="areas">The site's areas (flat, linked by <see cref="Area.ParentAreaId"/>); null is treated as empty.</param>
|
||||
/// <param name="instances">The site's instances; null is treated as empty.</param>
|
||||
@@ -47,7 +47,7 @@ public static class InstanceTreeBuilder
|
||||
var instancesByArea = instances
|
||||
.Where(i => !hasFilter || i.UniqueName.Contains(needle!, StringComparison.OrdinalIgnoreCase))
|
||||
.OrderBy(i => i.UniqueName, StringComparer.OrdinalIgnoreCase)
|
||||
.ToLookup(i => i.AreaId);
|
||||
.ToLookup(i => i.AreaId is int a && knownAreaIds.Contains(a) ? a : (int?)null);
|
||||
|
||||
return BuildLevel(parentId: null, areasByParent, instancesByArea, hasFilter, ancestors: []);
|
||||
}
|
||||
@@ -67,7 +67,11 @@ public static class InstanceTreeBuilder
|
||||
if (!ancestors.Add(area.Id)) continue;
|
||||
|
||||
var node = new InstanceTreeNode($"a:{area.Id}", area.Name, area, Instance: null);
|
||||
node.Children.AddRange(BuildLevel(area.Id, areasByParent, instancesByArea, hasFilter, ancestors));
|
||||
foreach (var child in BuildLevel(area.Id, areasByParent, instancesByArea, hasFilter, ancestors))
|
||||
{
|
||||
node.AddChild(child);
|
||||
}
|
||||
|
||||
ancestors.Remove(area.Id);
|
||||
|
||||
// With a filter an area survives only if something matched beneath it.
|
||||
|
||||
@@ -5,6 +5,7 @@ namespace ZB.MOM.WW.ScadaBridge.CentralUI.Components.Shared;
|
||||
/// <summary>
|
||||
/// One row of the Instance Picker tree: an area branch or an instance leaf.
|
||||
/// Keys are <c>a:{areaId}</c> for areas and <c>i:{instanceId}</c> for instances.
|
||||
/// Children are populated by <see cref="InstanceTreeBuilder"/> and read-only to callers.
|
||||
/// </summary>
|
||||
/// <param name="Key">Stable tree key — <c>a:{areaId}</c> or <c>i:{instanceId}</c>.</param>
|
||||
/// <param name="Label">Text rendered for this row.</param>
|
||||
@@ -12,12 +13,18 @@ namespace ZB.MOM.WW.ScadaBridge.CentralUI.Components.Shared;
|
||||
/// <param name="Instance">The instance this node represents, or null for an area branch.</param>
|
||||
public sealed record InstanceTreeNode(string Key, string Label, Area? Area, Instance? Instance)
|
||||
{
|
||||
private readonly List<InstanceTreeNode> _children = new();
|
||||
|
||||
/// <summary>Gets the child rows nested under this node (child areas first, then instances).</summary>
|
||||
public List<InstanceTreeNode> Children { get; } = new();
|
||||
public IReadOnlyList<InstanceTreeNode> Children => _children;
|
||||
|
||||
/// <summary>Gets a value indicating whether this node is a selectable instance leaf.</summary>
|
||||
public bool IsInstance => Instance is not null;
|
||||
|
||||
/// <summary>Gets a value indicating whether this node has any child rows.</summary>
|
||||
public bool HasChildren => Children.Count > 0;
|
||||
public bool HasChildren => _children.Count > 0;
|
||||
|
||||
/// <summary>Appends a child row. Only the builder composes the tree.</summary>
|
||||
/// <param name="child">The child row to append.</param>
|
||||
internal void AddChild(InstanceTreeNode child) => _children.Add(child);
|
||||
}
|
||||
|
||||
@@ -82,6 +82,84 @@ public class InstanceTreeBuilderTests
|
||||
Assert.Equal("i:10", leaf.Key);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Build_InstanceWithUnknownArea_SurfacesAtRoot()
|
||||
{
|
||||
// AreaId points at an area that is not in the supplied list — the instance must not
|
||||
// vanish; it is listed at the root like an instance with no area at all.
|
||||
var line1 = MakeArea(1, "Line1");
|
||||
var stranded = MakeInstance(10, "Stranded", areaId: 99);
|
||||
|
||||
var roots = InstanceTreeBuilder.Build([line1], [stranded], filter: null);
|
||||
|
||||
Assert.Equal(new[] { "a:1", "i:10" }, roots.Select(n => n.Key));
|
||||
Assert.Same(stranded, roots[1].Instance);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Build_InstanceWithUnknownArea_StaysFilterable()
|
||||
{
|
||||
var line1 = MakeArea(1, "Line1");
|
||||
var stranded = MakeInstance(10, "Stranded", areaId: 99);
|
||||
|
||||
var matched = InstanceTreeBuilder.Build([line1], [stranded], filter: "rand");
|
||||
|
||||
Assert.Equal("i:10", Assert.Single(matched).Key);
|
||||
Assert.Empty(InstanceTreeBuilder.Build([line1], [stranded], filter: "nomatch"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Build_DeepNesting_WiresChildrenAtEveryLevel()
|
||||
{
|
||||
var plant = MakeArea(1, "Plant");
|
||||
var line = MakeArea(2, "Line", parentAreaId: 1);
|
||||
var cell = MakeArea(3, "Cell", parentAreaId: 2);
|
||||
var deep = MakeInstance(10, "Deep", areaId: 3);
|
||||
|
||||
var roots = InstanceTreeBuilder.Build([plant, line, cell], [deep], filter: null);
|
||||
|
||||
var plantNode = Assert.Single(roots);
|
||||
Assert.Equal("a:1", plantNode.Key);
|
||||
var lineNode = Assert.Single(plantNode.Children);
|
||||
Assert.Equal("a:2", lineNode.Key);
|
||||
var cellNode = Assert.Single(lineNode.Children);
|
||||
Assert.Equal("a:3", cellNode.Key);
|
||||
var leaf = Assert.Single(cellNode.Children);
|
||||
Assert.Equal("i:10", leaf.Key);
|
||||
Assert.False(leaf.HasChildren);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Build_CyclicParentAreaIds_DoNotLoopOrThrow()
|
||||
{
|
||||
// A bad row pair: each area names the other as its parent. Neither is reachable
|
||||
// from a root, so the forest is simply empty — and building it must terminate.
|
||||
var a = MakeArea(1, "A", parentAreaId: 2);
|
||||
var b = MakeArea(2, "B", parentAreaId: 1);
|
||||
var inA = MakeInstance(10, "InA", areaId: 1);
|
||||
var inB = MakeInstance(11, "InB", areaId: 2);
|
||||
|
||||
var roots = InstanceTreeBuilder.Build([a, b], [inA, inB], filter: null);
|
||||
|
||||
Assert.Empty(roots);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Build_Filter_MatchesInstanceNamesOnly_AreaNameMatchIsNotEnough()
|
||||
{
|
||||
// "Line1" contains the needle but holds no matching instance — it is dropped.
|
||||
var line1 = MakeArea(1, "Line1");
|
||||
var line2 = MakeArea(2, "Line2");
|
||||
var other = MakeInstance(10, "Pump", areaId: 1);
|
||||
var hit = MakeInstance(11, "Line1Feeder", areaId: 2);
|
||||
|
||||
var roots = InstanceTreeBuilder.Build([line1, line2], [other, hit], filter: "line1");
|
||||
|
||||
var root = Assert.Single(roots);
|
||||
Assert.Equal("a:2", root.Key);
|
||||
Assert.Equal("i:11", Assert.Single(root.Children).Key);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Build_Filter_KeepsMatchingInstancesWithAncestors_DropsEmptyBranches()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user