fix(galaxy): degrade all parent-cycle members to root (review)

This commit is contained in:
Joseph Doherty
2026-06-16 19:51:20 -04:00
parent 21c7645da8
commit bec37848d5
2 changed files with 50 additions and 12 deletions
@@ -22,7 +22,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Browse;
/// order-independently and memoised, so the parent can appear before or after the child).
/// A gobject degrades to the driver root when its <c>parent_gobject_id</c> is <c>0</c>,
/// self-referential, or names a gobject not present in the returned set — so a model
/// that carries no parentage still renders flat.</item>
/// that carries no parentage still renders flat. If a parent-reference cycle exists
/// (e.g. A→B→A), EVERY member of the cycle degrades to the root — none nests under
/// another cycle member — so materialisation stays finite and flat.</item>
/// </list>
/// </remarks>
public sealed class GalaxyDiscoverer
@@ -105,30 +107,29 @@ public sealed class GalaxyDiscoverer
// Resolve (and memoise) the folder for a gobject, recursively creating its parent's folder
// first so the result nests under it. Falls back to the driver root when the parent_gobject_id
// is 0, self-referential, absent from the returned set, or already mid-resolution (cycle guard).
// Memoisation is keyed by gobject_id and only applies to non-zero ids — an id-0 object always
// gets its own freshly-created folder (it can be neither a parent target nor shared).
// is 0, self-referential, or absent from the returned set. Before nesting, the gobject's parent
// chain is walked once (IsRootedTo) to ensure it terminates at a real root — if the chain ever
// revisits a gobject (a parent-reference cycle), EVERY member of the cycle degrades to the root
// rather than the outermost member nesting under another cycle member. Memoisation is keyed by
// gobject_id and only applies to non-zero ids — an id-0 object always gets its own freshly-created
// folder (it can be neither a parent target nor shared).
private static IAddressSpaceBuilder EnsureFolder(
GalaxyObject obj,
IAddressSpaceBuilder root,
IReadOnlyDictionary<int, GalaxyObject> byId,
Dictionary<int, IAddressSpaceBuilder> folders,
HashSet<int>? building = null)
Dictionary<int, IAddressSpaceBuilder> folders)
{
if (obj.GobjectId != 0 && folders.TryGetValue(obj.GobjectId, out var existing)) return existing;
var browseName = string.IsNullOrEmpty(obj.ContainedName) ? obj.TagName : obj.ContainedName;
building ??= [];
IAddressSpaceBuilder parentBuilder = root;
if (obj.ParentGobjectId != 0
&& obj.ParentGobjectId != obj.GobjectId
&& !building.Contains(obj.ParentGobjectId)
&& byId.TryGetValue(obj.ParentGobjectId, out var parentObj))
&& byId.TryGetValue(obj.ParentGobjectId, out var parentObj)
&& IsRootedTo(obj, byId))
{
building.Add(obj.GobjectId);
parentBuilder = EnsureFolder(parentObj, root, byId, folders, building);
building.Remove(obj.GobjectId);
parentBuilder = EnsureFolder(parentObj, root, byId, folders);
}
var folder = parentBuilder.Folder(browseName, browseName);
@@ -139,6 +140,26 @@ public sealed class GalaxyDiscoverer
return folder;
}
// Walk a gobject's parent chain to decide whether it can safely nest. Returns true when the chain
// terminates at a real root (parent == 0, self-parent, or a parent absent from the returned set);
// returns false when the walk revisits a gobject already on the chain — i.e. the gobject sits on a
// parent-reference cycle, so it (and every other member) must degrade to the driver root instead of
// nesting under another cycle member. The visited-set is seeded with the starting gobject so a
// mutual cycle (A→B→A) is detected the moment the walk loops back to it.
private static bool IsRootedTo(GalaxyObject obj, IReadOnlyDictionary<int, GalaxyObject> byId)
{
var visited = new HashSet<int> { obj.GobjectId };
var current = obj;
while (current.ParentGobjectId != 0
&& current.ParentGobjectId != current.GobjectId
&& byId.TryGetValue(current.ParentGobjectId, out var parent))
{
if (!visited.Add(current.ParentGobjectId)) return false; // revisited → cycle
current = parent;
}
return true; // reached a terminal root (parent 0 / self / absent)
}
// PR 5.W workaround for mxaccessgw GalaxyRepository.cs:173-175 — the gateway's
// SQL appends `[]` to array-typed `full_tag_reference` values, but MxAccess COM
// `IInstance.AddItem` doesn't accept `[]`-suffixed addresses (so any downstream
@@ -431,6 +431,23 @@ public sealed class GalaxyDiscovererTests
builder.Folders.Single(f => f.BrowseName == "selfie").ParentBrowseName.ShouldBeNull();
}
/// <summary>A mutual parent cycle (A→B, B→A) degrades EVERY member to the root — neither
/// folder nests under the other, and discovery stays finite (no infinite recursion / crash).</summary>
[Fact]
public async Task Mutual_cycle_degrades_both_to_root()
{
var src = new FakeHierarchySource([
Node(1, 2, "a", "a", Attr("PV")), // A's parent is B
Node(2, 1, "b", "b", Attr("PV")), // B's parent is A
]);
var builder = new FakeBuilder();
await new GalaxyDiscoverer(src).DiscoverAsync(builder, CancellationToken.None);
builder.Folders.Single(f => f.BrowseName == "a").ParentBrowseName.ShouldBeNull();
builder.Folders.Single(f => f.BrowseName == "b").ParentBrowseName.ShouldBeNull();
}
/// <summary>A child object's attributes are added into the child's own folder, not the parent's.</summary>
[Fact]
public async Task Variables_land_in_their_owner_folder()