From bec37848d51122d0662378b4cf47aab3cc1f67da Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 16 Jun 2026 19:51:20 -0400 Subject: [PATCH] fix(galaxy): degrade all parent-cycle members to root (review) --- .../Browse/GalaxyDiscoverer.cs | 45 ++++++++++++++----- .../Browse/GalaxyDiscovererTests.cs | 17 +++++++ 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Browse/GalaxyDiscoverer.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Browse/GalaxyDiscoverer.cs index 9f60b95c..f1febd84 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Browse/GalaxyDiscoverer.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Browse/GalaxyDiscoverer.cs @@ -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 parent_gobject_id is 0, /// self-referential, or names a gobject not present in the returned set — so a model -/// that carries no parentage still renders flat. +/// 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. /// /// 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 byId, - Dictionary folders, - HashSet? building = null) + Dictionary 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 byId) + { + var visited = new HashSet { 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 diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Browse/GalaxyDiscovererTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Browse/GalaxyDiscovererTests.cs index d2d72a61..62a3af03 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Browse/GalaxyDiscovererTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Browse/GalaxyDiscovererTests.cs @@ -431,6 +431,23 @@ public sealed class GalaxyDiscovererTests builder.Folders.Single(f => f.BrowseName == "selfie").ParentBrowseName.ShouldBeNull(); } + /// 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). + [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(); + } + /// A child object's attributes are added into the child's own folder, not the parent's. [Fact] public async Task Variables_land_in_their_owner_folder()