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 7874c46a..9f60b95c 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 @@ -17,9 +17,12 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Browse; /// Folder per gobject; variables placed inside their owner folder. /// Variable's full reference = tag_name.attribute_name — the format MXAccess /// expects for read/write addressing (translated from the contained-name browse path). -/// Hierarchy is rendered flat (one folder per gobject under the driver root). -/// Nesting under parent_gobject_id for a true tree shape is not implemented; -/// the flat layout is the current shipping behaviour. +/// Hierarchy is nested by parent_gobject_id: each gobject's folder is created +/// under the folder of the gobject named by its parent_gobject_id (resolved +/// 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. /// /// public sealed class GalaxyDiscoverer @@ -44,12 +47,30 @@ public sealed class GalaxyDiscoverer ArgumentNullException.ThrowIfNull(builder); var objects = await _source.GetHierarchyAsync(cancellationToken).ConfigureAwait(false); + // Pass 1 — collect gobjects with a usable browse identity (order preserved), and index + // them by gobject_id so pass 2 can resolve a parent the source returned AFTER the child + // (order-independent). Only the FIRST gobject per non-zero id is indexed as a parent + // target; the index excludes id 0 so identity-less / un-keyed objects can't collide. + var usable = new List(); + var byId = new Dictionary(); foreach (var obj in objects) { var browseName = string.IsNullOrEmpty(obj.ContainedName) ? obj.TagName : obj.ContainedName; if (string.IsNullOrEmpty(browseName)) continue; // skip objects with no usable identity + usable.Add(obj); + if (obj.GobjectId != 0) + { + byId.TryAdd(obj.GobjectId, obj); + } + } - var folder = builder.Folder(browseName, browseName); + // Pass 2 — create each gobject's folder under its parent (memoised), then its variables. + // Iterating the ordered list (not the index) keeps the flat case stable and emits a + // folder even for objects whose gobject_id is 0 or duplicated. + var folders = new Dictionary(); + foreach (var obj in usable) + { + var folder = EnsureFolder(obj, builder, byId, folders); foreach (var attr in obj.Attributes) { @@ -82,6 +103,42 @@ 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). + private static IAddressSpaceBuilder EnsureFolder( + GalaxyObject obj, + IAddressSpaceBuilder root, + IReadOnlyDictionary byId, + Dictionary folders, + HashSet? building = null) + { + 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)) + { + building.Add(obj.GobjectId); + parentBuilder = EnsureFolder(parentObj, root, byId, folders, building); + building.Remove(obj.GobjectId); + } + + var folder = parentBuilder.Folder(browseName, browseName); + if (obj.GobjectId != 0) + { + folders[obj.GobjectId] = folder; + } + return folder; + } + // 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 11b771f7..d2d72a61 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 @@ -23,7 +23,9 @@ public sealed class GalaxyDiscovererTests => Task.FromResult(objects); } - private sealed record FolderCall(string BrowseName, string DisplayName); + /// ParentBrowseName is the browse-name of the folder this folder was created under + /// (null = created at the driver root). + private sealed record FolderCall(string BrowseName, string DisplayName, string? ParentBrowseName); private sealed record VariableCall(string FolderBrowseName, string AttributeName, DriverAttributeInfo Info); private sealed class FakeBuilder : IAddressSpaceBuilder @@ -47,7 +49,8 @@ public sealed class GalaxyDiscovererTests /// An IAddressSpaceBuilder scoped to the new folder. public IAddressSpaceBuilder Folder(string browseName, string displayName) { - Folders.Add(new FolderCall(browseName, displayName)); + // Root-level folder — no parent. + Folders.Add(new FolderCall(browseName, displayName, ParentBrowseName: null)); // Return a child scoped to this folder; nested folders inherit the parent reference. return new ChildBuilder(this, browseName); } @@ -79,7 +82,8 @@ public sealed class GalaxyDiscovererTests /// An IAddressSpaceBuilder scoped to the new child folder. public IAddressSpaceBuilder Folder(string browseName, string displayName) { - parent.Folders.Add(new FolderCall(browseName, displayName)); + // Nested folder — record the folder it was created under. + parent.Folders.Add(new FolderCall(browseName, displayName, ParentBrowseName: folderBrowseName)); return new ChildBuilder(parent, browseName); } @@ -155,6 +159,22 @@ public sealed class GalaxyDiscovererTests return o; } + /// Builds a gobject with explicit id + parent id so nesting-by-parent can be asserted. + private static GalaxyObject Node( + int gobjectId, int parentGobjectId, string tagName, + string? containedName = null, params GalaxyAttribute[] attributes) + { + var o = new GalaxyObject + { + GobjectId = gobjectId, + ParentGobjectId = parentGobjectId, + TagName = tagName, + ContainedName = containedName ?? tagName, + }; + o.Attributes.AddRange(attributes); + return o; + } + /// Verifies that discovery creates one folder per object and one variable per attribute. [Fact] public async Task DiscoverAsync_BuildsOneFolderPerObject_AndOneVariablePerAttribute() @@ -333,6 +353,101 @@ public sealed class GalaxyDiscovererTests builder.Variables.Count.ShouldBe(1); } + /// Child gobject's folder is created UNDER its parent's folder, not at the root. + [Fact] + public async Task Nests_child_folder_under_its_parent() + { + var src = new FakeHierarchySource([ + Node(1, 0, "Parent", "parent", Attr("PV")), + Node(2, 1, "Child", "child", Attr("PV")), + ]); + var builder = new FakeBuilder(); + + await new GalaxyDiscoverer(src).DiscoverAsync(builder, CancellationToken.None); + + var parent = builder.Folders.Single(f => f.BrowseName == "parent"); + parent.ParentBrowseName.ShouldBeNull(); + var child = builder.Folders.Single(f => f.BrowseName == "child"); + child.ParentBrowseName.ShouldBe("parent"); + } + + /// Nesting is order-independent: child returned before its parent still nests correctly. + [Fact] + public async Task Is_order_independent_child_before_parent() + { + var src = new FakeHierarchySource([ + Node(2, 1, "Child", "child", Attr("PV")), // child FIRST + Node(1, 0, "Parent", "parent", Attr("PV")), + ]); + var builder = new FakeBuilder(); + + await new GalaxyDiscoverer(src).DiscoverAsync(builder, CancellationToken.None); + + var child = builder.Folders.Single(f => f.BrowseName == "child"); + child.ParentBrowseName.ShouldBe("parent"); + } + + /// Both gobjects with parent=0 land flat at the driver root. + [Fact] + public async Task Degrades_to_flat_when_parent_is_zero() + { + var src = new FakeHierarchySource([ + Node(1, 0, "A", "a", Attr("PV")), + Node(2, 0, "B", "b", Attr("PV")), + ]); + 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 whose parent_gobject_id is not in the returned set degrades to the root. + [Fact] + public async Task Degrades_to_flat_when_parent_not_in_set() + { + var src = new FakeHierarchySource([ + Node(2, 99, "Child", "child", Attr("PV")), // parent 99 absent + ]); + var builder = new FakeBuilder(); + + await new GalaxyDiscoverer(src).DiscoverAsync(builder, CancellationToken.None); + + builder.Folders.Single(f => f.BrowseName == "child").ParentBrowseName.ShouldBeNull(); + } + + /// A self-referential parent (id == parent id) attaches to the root, never to itself. + [Fact] + public async Task Self_parent_attaches_to_root() + { + var src = new FakeHierarchySource([ + Node(5, 5, "Selfie", "selfie", Attr("PV")), + ]); + var builder = new FakeBuilder(); + + await new GalaxyDiscoverer(src).DiscoverAsync(builder, CancellationToken.None); + + builder.Folders.Single(f => f.BrowseName == "selfie").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() + { + var src = new FakeHierarchySource([ + Node(1, 0, "Parent", "parent", Attr("ParentPV")), + Node(2, 1, "Child", "child", Attr("ChildPV")), + ]); + var builder = new FakeBuilder(); + + await new GalaxyDiscoverer(src).DiscoverAsync(builder, CancellationToken.None); + + builder.Variables.ShouldContain(v => v.FolderBrowseName == "parent" && v.AttributeName == "ParentPV"); + builder.Variables.ShouldContain(v => v.FolderBrowseName == "child" && v.AttributeName == "ChildPV"); + builder.Variables.ShouldNotContain(v => v.FolderBrowseName == "parent" && v.AttributeName == "ChildPV"); + } + /// Helper that exercises the internal ctor (test seam) without exposing it publicly. private sealed class GalaxyDriverHelper {