From f0851af6b5bdb9bfeb4d15544881bc6eb4b5648e Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 20 Apr 2026 20:12:17 -0400 Subject: [PATCH] =?UTF-8?q?Phase=207=20Stream=20G=20follow-up=20=E2=80=94?= =?UTF-8?q?=20DriverNodeManager=20dispatch=20routing=20by=20NodeSourceKind?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Honors the ADR-002 discriminator at OPC UA Read/Write dispatch time. Virtual tag reads route to the VirtualTagEngine-backed IReadable; scripted alarm reads route to the ScriptedAlarmEngine-backed IReadable; driver reads continue to route to the driver's own IReadable (no regression for any existing driver test). ## Changes DriverNodeManager ctor gains optional `virtualReadable` + `scriptedAlarmReadable` parameters. When callers omit them (every existing driver test) the manager behaves exactly as before. SealedBootstrap wires the engines' IReadable adapters once the Phase 7 composition root is added. Per-variable NodeSourceKind tracked in `_sourceByFullRef` during Variable() registration alongside the existing `_writeIdempotentByFullRef` / `_securityByFullRef` maps. OnReadValue now picks the IReadable by source kind via the new internal SelectReadable helper. When the engine-backed IReadable isn't wired (virtual tag node but no engine provided), returns BadNotFound rather than silently falling back to the driver — surfaces a misconfiguration instead of masking it. OnWriteValue gates on IsWriteAllowedBySource which returns true only for Driver. Plan decision #6: virtual tags + scripted alarms reject direct OPC UA writes with BadUserAccessDenied. Scripts write virtual tags via `ctx.SetVirtualTag`; operators ack alarms via the Part 9 method nodes. ## Tests — 7/7 (internal helpers exposed via InternalsVisibleTo) DriverNodeManagerSourceDispatchTests covers: - Driver source routes to driver IReadable - Virtual source routes to virtual IReadable - ScriptedAlarm source routes to alarm IReadable - Virtual source with null virtual IReadable returns null (→ BadNotFound) - ScriptedAlarm source with null alarm IReadable returns null - Driver source with null driver IReadable returns null (preserves BadNotReadable) - IsWriteAllowedBySource: only Driver=true (Virtual=false, ScriptedAlarm=false) Full solution builds clean. Phase 7 test total now 197 green. --- .../OpcUa/DriverNodeManager.cs | 63 +++++++++++-- .../DriverNodeManagerSourceDispatchTests.cs | 89 +++++++++++++++++++ 2 files changed, 145 insertions(+), 7 deletions(-) create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Server.Tests/DriverNodeManagerSourceDispatchTests.cs diff --git a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs index 27cefe5..9046b4f 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs @@ -68,9 +68,18 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder private readonly AuthorizationGate? _authzGate; private readonly NodeScopeResolver? _scopeResolver; + // Phase 7 Stream G follow-up — per-variable NodeSourceKind so OnReadValue can dispatch + // to the VirtualTagEngine / ScriptedAlarmEngine instead of the driver's IReadable per + // ADR-002. Absent entries default to Driver so drivers registered before Phase 7 + // keep working unchanged. + private readonly Dictionary _sourceByFullRef = new(StringComparer.OrdinalIgnoreCase); + private readonly IReadable? _virtualReadable; + private readonly IReadable? _scriptedAlarmReadable; + public DriverNodeManager(IServerInternal server, ApplicationConfiguration configuration, IDriver driver, CapabilityInvoker invoker, ILogger logger, - AuthorizationGate? authzGate = null, NodeScopeResolver? scopeResolver = null) + AuthorizationGate? authzGate = null, NodeScopeResolver? scopeResolver = null, + IReadable? virtualReadable = null, IReadable? scriptedAlarmReadable = null) : base(server, configuration, namespaceUris: $"urn:OtOpcUa:{driver.DriverInstanceId}") { _driver = driver; @@ -80,6 +89,8 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder _invoker = invoker; _authzGate = authzGate; _scopeResolver = scopeResolver; + _virtualReadable = virtualReadable; + _scriptedAlarmReadable = scriptedAlarmReadable; _logger = logger; } @@ -185,6 +196,7 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder _variablesByFullRef[attributeInfo.FullName] = v; _securityByFullRef[attributeInfo.FullName] = attributeInfo.SecurityClass; _writeIdempotentByFullRef[attributeInfo.FullName] = attributeInfo.WriteIdempotent; + _sourceByFullRef[attributeInfo.FullName] = attributeInfo.Source; v.OnReadValue = OnReadValue; v.OnWriteValue = OnWriteValue; @@ -216,16 +228,18 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder private ServiceResult OnReadValue(ISystemContext context, NodeState node, NumericRange indexRange, QualifiedName dataEncoding, ref object? value, ref StatusCode statusCode, ref DateTime timestamp) { - if (_readable is null) + var fullRef = node.NodeId.Identifier as string ?? ""; + var source = _sourceByFullRef.TryGetValue(fullRef, out var s) ? s : NodeSourceKind.Driver; + var readable = SelectReadable(source, _readable, _virtualReadable, _scriptedAlarmReadable); + + if (readable is null) { - statusCode = StatusCodes.BadNotReadable; + statusCode = source == NodeSourceKind.Driver ? StatusCodes.BadNotReadable : StatusCodes.BadNotFound; return ServiceResult.Good; } try { - var fullRef = node.NodeId.Identifier as string ?? ""; - // Phase 6.2 Stream C — authorization gate. Runs ahead of the invoker so a denied // read never hits the driver. Returns true in lax mode when identity lacks LDAP // groups; strict mode denies those cases. See AuthorizationGate remarks. @@ -242,7 +256,7 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder var result = _invoker.ExecuteAsync( DriverCapability.Read, ResolveHostFor(fullRef), - async ct => (IReadOnlyList)await _readable.ReadAsync([fullRef], ct).ConfigureAwait(false), + async ct => (IReadOnlyList)await readable.ReadAsync([fullRef], ct).ConfigureAwait(false), CancellationToken.None).AsTask().GetAwaiter().GetResult(); if (result.Count == 0) { @@ -262,6 +276,32 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder return ServiceResult.Good; } + /// + /// Picks the the dispatch layer routes through based on the + /// node's Phase 7 source kind (ADR-002). Extracted as a pure function for unit test + /// coverage — the full dispatch requires the OPC UA server stack, but this kernel is + /// deterministic and small. + /// + internal static IReadable? SelectReadable( + NodeSourceKind source, + IReadable? driverReadable, + IReadable? virtualReadable, + IReadable? scriptedAlarmReadable) => source switch + { + NodeSourceKind.Virtual => virtualReadable, + NodeSourceKind.ScriptedAlarm => scriptedAlarmReadable, + _ => driverReadable, + }; + + /// + /// Plan decision #6 gate — returns true only when the write is allowed. Virtual tags + /// and scripted alarms reject OPC UA writes because the write path for virtual tags + /// is ctx.SetVirtualTag from within a script, and the write path for alarm + /// state is the Part 9 method nodes (Acknowledge / Confirm / Shelve). + /// + internal static bool IsWriteAllowedBySource(NodeSourceKind source) => + source == NodeSourceKind.Driver; + private static NodeId MapDataType(DriverDataType t) => t switch { DriverDataType.Boolean => DataTypeIds.Boolean, @@ -414,10 +454,19 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder private ServiceResult OnWriteValue(ISystemContext context, NodeState node, NumericRange indexRange, QualifiedName dataEncoding, ref object? value, ref StatusCode statusCode, ref DateTime timestamp) { - if (_writable is null) return StatusCodes.BadNotWritable; var fullRef = node.NodeId.Identifier as string; if (string.IsNullOrEmpty(fullRef)) return StatusCodes.BadNodeIdUnknown; + // Per Phase 7 plan decision #6 — virtual tags + scripted alarms reject direct + // OPC UA writes with BadUserAccessDenied. Scripts can write to virtual tags + // via ctx.SetVirtualTag; operators cannot. Operator alarm actions go through + // the Part 9 method nodes (Acknowledge / Confirm / Shelve), not through the + // variable-value write path. + if (_sourceByFullRef.TryGetValue(fullRef!, out var source) && !IsWriteAllowedBySource(source)) + return new ServiceResult(StatusCodes.BadUserAccessDenied); + + if (_writable is null) return StatusCodes.BadNotWritable; + // PR 26: server-layer write authorization. Look up the attribute's classification // (populated during Variable() in Discover) and check the session's roles against the // policy table. Drivers don't participate in this decision — IWritable.WriteAsync diff --git a/tests/ZB.MOM.WW.OtOpcUa.Server.Tests/DriverNodeManagerSourceDispatchTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Server.Tests/DriverNodeManagerSourceDispatchTests.cs new file mode 100644 index 0000000..136ca3f --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Server.Tests/DriverNodeManagerSourceDispatchTests.cs @@ -0,0 +1,89 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Server.OpcUa; + +namespace ZB.MOM.WW.OtOpcUa.Server.Tests; + +/// +/// Phase 7 Stream G follow-up — verifies the NodeSourceKind dispatch kernel that +/// DriverNodeManager's OnReadValue + OnWriteValue use to route per-node calls to +/// the right backend per ADR-002. Pure functions; no OPC UA stack required. +/// +[Trait("Category", "Unit")] +public sealed class DriverNodeManagerSourceDispatchTests +{ + private sealed class FakeReadable : IReadable + { + public string Name { get; init; } = ""; + public Task> ReadAsync( + IReadOnlyList fullReferences, CancellationToken cancellationToken) => + Task.FromResult>([]); + } + + [Fact] + public void Driver_source_routes_to_driver_readable() + { + var drv = new FakeReadable { Name = "drv" }; + var vt = new FakeReadable { Name = "vt" }; + var al = new FakeReadable { Name = "al" }; + + DriverNodeManager.SelectReadable(NodeSourceKind.Driver, drv, vt, al).ShouldBeSameAs(drv); + } + + [Fact] + public void Virtual_source_routes_to_virtual_readable() + { + var drv = new FakeReadable(); + var vt = new FakeReadable(); + var al = new FakeReadable(); + + DriverNodeManager.SelectReadable(NodeSourceKind.Virtual, drv, vt, al).ShouldBeSameAs(vt); + } + + [Fact] + public void ScriptedAlarm_source_routes_to_alarm_readable() + { + var drv = new FakeReadable(); + var vt = new FakeReadable(); + var al = new FakeReadable(); + + DriverNodeManager.SelectReadable(NodeSourceKind.ScriptedAlarm, drv, vt, al).ShouldBeSameAs(al); + } + + [Fact] + public void Virtual_source_without_virtual_readable_returns_null() + { + // Engine not wired → dispatch layer surfaces BadNotFound (the null propagates + // through to the OnReadValue null-check). + DriverNodeManager.SelectReadable( + NodeSourceKind.Virtual, driverReadable: new FakeReadable(), + virtualReadable: null, scriptedAlarmReadable: null).ShouldBeNull(); + } + + [Fact] + public void ScriptedAlarm_source_without_alarm_readable_returns_null() + { + DriverNodeManager.SelectReadable( + NodeSourceKind.ScriptedAlarm, driverReadable: new FakeReadable(), + virtualReadable: new FakeReadable(), scriptedAlarmReadable: null).ShouldBeNull(); + } + + [Fact] + public void Driver_source_without_driver_readable_returns_null() + { + // Pre-existing BadNotReadable behavior — unchanged by Phase 7 wiring. + DriverNodeManager.SelectReadable( + NodeSourceKind.Driver, driverReadable: null, + virtualReadable: new FakeReadable(), scriptedAlarmReadable: new FakeReadable()).ShouldBeNull(); + } + + [Fact] + public void IsWriteAllowedBySource_only_Driver_returns_true() + { + // Plan decision #6 — OPC UA writes to virtual tags / scripted alarms rejected. + DriverNodeManager.IsWriteAllowedBySource(NodeSourceKind.Driver).ShouldBeTrue(); + DriverNodeManager.IsWriteAllowedBySource(NodeSourceKind.Virtual).ShouldBeFalse(); + DriverNodeManager.IsWriteAllowedBySource(NodeSourceKind.ScriptedAlarm).ShouldBeFalse(); + } +}