From 598cdfad5a2267e61188d40fb536aed00311bc4a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 26 Jun 2026 07:16:36 -0400 Subject: [PATCH] feat(otopcua): applier pass to materialise discovered nodes idempotently --- .../OpcUa/DeferredAddressSpaceSink.cs | 4 + .../OpcUa/IOpcUaAddressSpaceSink.cs | 7 + .../AddressSpaceApplier.cs | 39 ++++++ .../SdkAddressSpaceSink.cs | 4 + .../OpcUa/DeferredAddressSpaceSinkTests.cs | 2 + .../AddressSpaceApplierHierarchyTests.cs | 2 + .../AddressSpaceApplierTests.cs | 120 ++++++++++++++++++ .../DeferredAddressSpaceSinkTests.cs | 4 + .../OtOpcUaTelemetryHookTests.cs | 2 + .../OpcUa/OpcUaPublishActorRebuildTests.cs | 3 + .../OpcUa/OpcUaPublishActorTests.cs | 4 + 11 files changed, 191 insertions(+) diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs index d3b15f01..9d5664e9 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs @@ -70,6 +70,10 @@ public sealed class DeferredAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgical /// Rebuilds the address space through the inner sink. public void RebuildAddressSpace() => _inner.RebuildAddressSpace(); + /// Announces a runtime NodeAdded model-change (discovered-node injection) through the inner sink. + /// The node under which discovered nodes were added. + public void RaiseNodesAddedModelChange(string affectedNodeId) => _inner.RaiseNodesAddedModelChange(affectedNodeId); + /// Forwards an in-place tag-attribute update (F10b) to the inner sink when it supports the /// surgical capability. Returns false otherwise — before the real SdkAddressSpaceSink is /// swapped in (inner is still the null sink), or any inner sink that isn't surgical — so the caller diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs index a7a1dd9a..0f74db9b 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs @@ -84,6 +84,10 @@ public interface IOpcUaAddressSpaceSink /// successful deployment apply so the node manager reflects the new config. Idempotent. /// void RebuildAddressSpace(); + + /// Announce that nodes were added at runtime (discovered-node injection) under + /// so subscribed clients refresh their browse (Part 3 GeneralModelChangeEvent, verb NodeAdded). + void RaiseNodesAddedModelChange(string affectedNodeId); } /// OPC UA status code projection — Good / Uncertain / Bad. Real SDK has finer-grained @@ -114,4 +118,7 @@ public sealed class NullOpcUaAddressSpaceSink : IOpcUaAddressSpaceSink /// public void RebuildAddressSpace() { } + + /// + public void RaiseNodesAddedModelChange(string affectedNodeId) { } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs index f8a22fd8..1dfac048 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs @@ -303,6 +303,45 @@ public sealed class AddressSpaceApplier composition.EquipmentTags.Select(t => t.EquipmentId).Distinct(StringComparer.Ordinal).Count()); } + /// + /// Materialise driver-discovered nodes (FixedTree) under an equipment at runtime. Idempotent: + /// re-applies are cheap (the sink's EnsureFolder/EnsureVariable early-return on existing nodes), so + /// this is safely re-run after every address-space rebuild. Folders are ensured parent-first. + /// Emits a NodeAdded model-change so connected clients can refresh. Discovered nodes are read-only + /// value nodes; array discovered nodes (rare) are forced read-only like the equipment-tag pass. + /// + /// The equipment root node the discovered nodes hang under; the + /// NodeAdded model-change is announced under this node. + /// The discovered folders to ensure (parent-first by depth). + /// The discovered variables to ensure (read-only value nodes). + public void MaterialiseDiscoveredNodes( + string equipmentRootNodeId, + IReadOnlyList folders, + IReadOnlyList variables) + { + ArgumentNullException.ThrowIfNull(folders); + ArgumentNullException.ThrowIfNull(variables); + if (folders.Count == 0 && variables.Count == 0) return; + + // Parent-first: a child folder's parent must exist before it. Ordering by '/' count == depth. + foreach (var f in folders.OrderBy(f => f.NodeId.Count(c => c == '/'))) + SafeEnsureFolder(f.NodeId, f.ParentNodeId, f.DisplayName); + + foreach (var v in variables) + { + // Mirror MaterialiseEquipmentTags: arrays forced read-only (the driver write path can't handle arrays). + var writable = v.Writable && !v.IsArray; + SafeEnsureVariable(v.NodeId, v.ParentNodeId, v.DisplayName, v.DataType, writable, + historianTagname: null, isArray: v.IsArray, arrayLength: v.ArrayLength); + } + + _sink.RaiseNodesAddedModelChange(equipmentRootNodeId); + + _logger.LogInformation( + "AddressSpaceApplier: discovered nodes materialised under {Equipment} (folders={Folders}, vars={Vars})", + equipmentRootNodeId, folders.Count, variables.Count); + } + /// /// Materialise Equipment-namespace VirtualTags from a composition snapshot — the VirtualTag /// analogue of . For each , diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs index 0bf157bf..995b0269 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs @@ -88,4 +88,8 @@ public sealed class SdkAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgicalAddre /// Rebuilds the entire OPC UA address space. public void RebuildAddressSpace() => _nodeManager.RebuildAddressSpace(); + + /// Announces a runtime NodeAdded model-change (discovered-node injection) to subscribed clients. + /// The node under which discovered nodes were added. + public void RaiseNodesAddedModelChange(string affectedNodeId) => _nodeManager.RaiseNodesAddedModelChange(affectedNodeId); } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredAddressSpaceSinkTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredAddressSpaceSinkTests.cs index 1780263d..fe0b9ea5 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredAddressSpaceSinkTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredAddressSpaceSinkTests.cs @@ -156,6 +156,7 @@ public class DeferredAddressSpaceSinkTests public void EnsureFolder(string folderNodeId, string? parentNodeId, string displayName) { } public void EnsureVariable(string variableNodeId, string? parentFolderNodeId, string displayName, string dataType, bool writable, string? historianTagname = null, bool isArray = false, uint? arrayLength = null) { } public void RebuildAddressSpace() => RebuildCalled = true; + public void RaiseNodesAddedModelChange(string affectedNodeId) { } } private sealed class SpySurgicalSink : IOpcUaAddressSpaceSink, ISurgicalAddressSpaceSink @@ -169,6 +170,7 @@ public class DeferredAddressSpaceSinkTests public void EnsureFolder(string folderNodeId, string? parentNodeId, string displayName) { } public void EnsureVariable(string variableNodeId, string? parentFolderNodeId, string displayName, string dataType, bool writable, string? historianTagname = null, bool isArray = false, uint? arrayLength = null) { } public void RebuildAddressSpace() { } + public void RaiseNodesAddedModelChange(string affectedNodeId) { } public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname, string dataType, bool isArray, uint? arrayLength) { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs index 17ba1248..6215eea9 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs @@ -333,5 +333,7 @@ public sealed class AddressSpaceApplierHierarchyTests : IDisposable public void EnsureVariable(string variableNodeId, string? parentFolderNodeId, string displayName, string dataType, bool writable, string? historianTagname = null, bool isArray = false, uint? arrayLength = null) { } /// Rebuilds the address space (stub implementation for testing). public void RebuildAddressSpace() { } + /// Announces a NodeAdded model-change (stub implementation for testing). + public void RaiseNodesAddedModelChange(string affectedNodeId) { } } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs index f15c2795..96433384 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs @@ -567,6 +567,114 @@ public sealed class AddressSpaceApplierTests .ShouldBe(("alm-1", "eq-1", "HighTemp", "OffNormalAlarm", 700, false)); } + /// Task 4 — MaterialiseDiscoveredNodes ensures the discovered folders PARENT-FIRST (ordered by + /// depth = '/' count) and the discovered variables at their folder-scoped NodeIds/parents, with variables + /// created READ-ONLY (writable == false), then raises EXACTLY ONE NodeAdded model-change under the + /// equipment root. Folders are passed in REVERSE (child-first) to prove the applier re-orders them + /// parent-first before ensuring (a child folder's parent must exist first). + [Fact] + public void MaterialiseDiscoveredNodes_ensures_folders_parent_first_read_only_variables_and_raises_model_change_once() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + // Child folder listed BEFORE its parent — the applier must re-order parent-first. + var folders = new[] + { + new DiscoveredFolder("EQ-1/FOCAS/Identity", "EQ-1/FOCAS", "Identity"), + new DiscoveredFolder("EQ-1/FOCAS", "EQ-1", "FOCAS"), + }; + var variables = new[] + { + new DiscoveredVariable("EQ-1/FOCAS/Identity/SeriesNumber", "EQ-1/FOCAS/Identity", "SeriesNumber", + "String", Writable: false, IsArray: false, ArrayLength: null), + }; + + applier.MaterialiseDiscoveredNodes("EQ-1", folders, variables); + + // Folders ensured parent-first regardless of input order (shallowest depth first). + sink.FolderCalls.Select(f => f.NodeId).ShouldBe(new[] { "EQ-1/FOCAS", "EQ-1/FOCAS/Identity" }); + sink.FolderCalls.ShouldContain(("EQ-1/FOCAS", "EQ-1", "FOCAS")); + sink.FolderCalls.ShouldContain(("EQ-1/FOCAS/Identity", "EQ-1/FOCAS", "Identity")); + + // Variable ensured at its folder-scoped NodeId, parented to its sub-folder, READ-ONLY. + sink.VariableCalls.ShouldHaveSingleItem() + .ShouldBe(("EQ-1/FOCAS/Identity/SeriesNumber", "EQ-1/FOCAS/Identity", "SeriesNumber", "String", false)); + + // Exactly one NodeAdded model-change, announced under the equipment root. + sink.ModelChangeCalls.ShouldHaveSingleItem().ShouldBe("EQ-1"); + } + + /// Task 4 — a discovered array variable (rare) authored Writable: true is forced + /// READ-ONLY (mirrors MaterialiseEquipmentTags: the driver write path can't handle arrays), while the + /// IsArray / ArrayLength flags are forwarded verbatim to the sink. + [Fact] + public void MaterialiseDiscoveredNodes_array_variable_is_forced_read_only() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var variables = new[] + { + new DiscoveredVariable("EQ-1/FOCAS/Buffer", "EQ-1", "Buffer", "Int16", + Writable: true, IsArray: true, ArrayLength: 8u), + }; + + applier.MaterialiseDiscoveredNodes("EQ-1", Array.Empty(), variables); + + var varCall = sink.VariableCalls.ShouldHaveSingleItem(); + varCall.Writable.ShouldBeFalse(); // clamped to read-only despite Writable: true + var arrCall = sink.ArrayCalls.ShouldHaveSingleItem(); + arrCall.IsArray.ShouldBeTrue(); + arrCall.ArrayLength.ShouldBe(8u); + } + + /// Task 4 — re-applying the SAME discovered plan is idempotent-SAFE: it does not throw, the + /// distinct folder/variable set the applier issues per pass is stable (the real sink early-returns on + /// existing nodes), and a model-change is raised once PER call (twice across two calls). + [Fact] + public void MaterialiseDiscoveredNodes_is_idempotent_safe_on_repeated_application() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var folders = new[] + { + new DiscoveredFolder("EQ-1/FOCAS", "EQ-1", "FOCAS"), + new DiscoveredFolder("EQ-1/FOCAS/Identity", "EQ-1/FOCAS", "Identity"), + }; + var variables = new[] + { + new DiscoveredVariable("EQ-1/FOCAS/Identity/SeriesNumber", "EQ-1/FOCAS/Identity", "SeriesNumber", + "String", Writable: false, IsArray: false, ArrayLength: null), + }; + + applier.MaterialiseDiscoveredNodes("EQ-1", folders, variables); + Should.NotThrow(() => applier.MaterialiseDiscoveredNodes("EQ-1", folders, variables)); + + // Each pass re-issues the same parent-first ensures (the real sink dedups via early-return); the + // DISTINCT set the applier produces is stable across re-applies. + sink.FolderCalls.Select(f => f.NodeId).Distinct().ShouldBe(new[] { "EQ-1/FOCAS", "EQ-1/FOCAS/Identity" }); + sink.VariableCalls.Select(v => v.NodeId).Distinct().ShouldBe(new[] { "EQ-1/FOCAS/Identity/SeriesNumber" }); + // One model-change per call ⇒ two across two calls. + sink.ModelChangeCalls.ShouldBe(new[] { "EQ-1", "EQ-1" }); + } + + /// Task 4 — empty input (no folders, no variables) returns WITHOUT touching the sink: no + /// EnsureFolder/EnsureVariable and, crucially, NO NodeAdded model-change. + [Fact] + public void MaterialiseDiscoveredNodes_empty_input_does_not_touch_sink() + { + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + applier.MaterialiseDiscoveredNodes("EQ-1", Array.Empty(), Array.Empty()); + + sink.FolderCalls.ShouldBeEmpty(); + sink.VariableCalls.ShouldBeEmpty(); + sink.ModelChangeCalls.ShouldBeEmpty(); + } + /// Verifies that added equipment tags in an otherwise-empty plan trigger an /// address-space rebuild (the planner now diffs equipment tags, so a tags-only deploy is no /// longer a silent no-op). @@ -1761,6 +1869,14 @@ public sealed class AddressSpaceApplierTests } /// Records a rebuild address space call. public void RebuildAddressSpace() => Interlocked.Increment(ref RebuildCalls); + + /// Gets the queue of NodeAdded model-change announcements (discovered-node injection). + public ConcurrentQueue ModelChangeQueue { get; } = new(); + /// Gets the list of recorded NodeAdded model-change announcements (discovered-node injection). + public List ModelChangeCalls => ModelChangeQueue.ToList(); + /// Records a NodeAdded model-change announcement. + /// The node under which discovered nodes were added. + public void RaiseNodesAddedModelChange(string affectedNodeId) => ModelChangeQueue.Enqueue(affectedNodeId); } /// A recording sink that does NOT implement — used to @@ -1783,6 +1899,8 @@ public sealed class AddressSpaceApplierTests public void EnsureVariable(string variableNodeId, string? parentFolderNodeId, string displayName, string dataType, bool writable, string? historianTagname = null, bool isArray = false, uint? arrayLength = null) { } /// Records a rebuild address space call. public void RebuildAddressSpace() => Interlocked.Increment(ref RebuildCalls); + /// No-op NodeAdded model-change announcement. + public void RaiseNodesAddedModelChange(string affectedNodeId) { } } private sealed class ThrowingSink : IOpcUaAddressSpaceSink @@ -1829,5 +1947,7 @@ public sealed class AddressSpaceApplierTests public void EnsureVariable(string variableNodeId, string? parentFolderNodeId, string displayName, string dataType, bool writable, string? historianTagname = null, bool isArray = false, uint? arrayLength = null) { } /// No-op rebuild address space call. public void RebuildAddressSpace() { } + /// No-op NodeAdded model-change announcement. + public void RaiseNodesAddedModelChange(string affectedNodeId) { } } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs index 9d6f28f4..7ebbb2a9 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs @@ -212,6 +212,8 @@ public sealed class DeferredAddressSpaceSinkTests } /// public void RebuildAddressSpace() => CallQueue.Enqueue("RB"); + /// + public void RaiseNodesAddedModelChange(string affectedNodeId) => CallQueue.Enqueue($"NA:{affectedNodeId}"); } private sealed class SurgicalRecordingSink : IOpcUaAddressSpaceSink, ISurgicalAddressSpaceSink @@ -249,5 +251,7 @@ public sealed class DeferredAddressSpaceSinkTests public void EnsureVariable(string variableNodeId, string? parentFolderNodeId, string displayName, string dataType, bool writable, string? historianTagname = null, bool isArray = false, uint? arrayLength = null) { } /// public void RebuildAddressSpace() { } + /// + public void RaiseNodesAddedModelChange(string affectedNodeId) { } } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Observability/OtOpcUaTelemetryHookTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Observability/OtOpcUaTelemetryHookTests.cs index 06b5de9f..78063069 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Observability/OtOpcUaTelemetryHookTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Observability/OtOpcUaTelemetryHookTests.cs @@ -222,5 +222,7 @@ public sealed class OtOpcUaTelemetryHookTests : RuntimeActorTestBase public void EnsureVariable(string variableNodeId, string? parentFolderNodeId, string displayName, string dataType, bool writable, string? historianTagname = null, bool isArray = false, uint? arrayLength = null) { } /// Rebuilds address space (recorded via span). public void RebuildAddressSpace() { /* recorded via span */ } + /// Announces a NodeAdded model-change (stub implementation). + public void RaiseNodesAddedModelChange(string affectedNodeId) { } } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs index b9afe52b..ed596971 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs @@ -361,6 +361,9 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase => Calls.Enqueue($"EV:{variableNodeId}"); /// Records a rebuild address space call. public void RebuildAddressSpace() => Interlocked.Increment(ref RebuildCalls); + /// Records a NodeAdded model-change announcement. + /// The node under which discovered nodes were added. + public void RaiseNodesAddedModelChange(string affectedNodeId) => Calls.Enqueue($"NA:{affectedNodeId}"); /// Records a surgical in-place tag-attribute update (always succeeds in this recording sink). public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname, string dataType, bool isArray, uint? arrayLength) { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorTests.cs index 77696819..26215dd3 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorTests.cs @@ -596,6 +596,10 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase /// Records a rebuild call. public void RebuildAddressSpace() => Interlocked.Increment(ref RebuildCalls); + + /// Announces a NodeAdded model-change (no-op in test). + /// The node under which discovered nodes were added. + public void RaiseNodesAddedModelChange(string affectedNodeId) { } } /// Test implementation of IServiceLevelPublisher that records publishes.