diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 11de032c..45248cde 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -142,11 +142,24 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// Build the internal map key for an already-resolved SDK (the SDK-facing /// seams). Equal to for the same node. The SDK-facing /// seams only ever pass nodes this manager minted (string s= identifiers); a non-string - /// identifier falls back to its string form rather than throwing. + /// identifier falls back to its string form rather than throwing — and says so loudly, because such a + /// key cannot match anything the manager stored, so the lookup would silently miss. /// The resolved SDK node id. /// The realm-qualified map key. - private RealmKey MapKey(NodeId nodeId) => - new(RealmOf(nodeId), nodeId.Identifier as string ?? nodeId.Identifier?.ToString() ?? string.Empty); + private RealmKey MapKey(NodeId nodeId) + { + if (nodeId.Identifier is string s) return new RealmKey(RealmOf(nodeId), s); + + // Unreachable by construction — every call site passes a node this manager minted with a string + // `s=` identifier. If it ever fires, the derived key resolves to nothing and the caller sees an + // unexplained miss, so name the seam rather than collapsing quietly. +#pragma warning disable CS0618 // Utils.LogWarning is [Obsolete] in favour of an ITelemetryContext this manager doesn't carry. + Utils.LogWarning( + "OtOpcUaNodeManager.MapKey: non-string NodeId identifier reached the map-key seam (node {0}, identifier type {1}) — the derived key will not resolve", + nodeId, nodeId.Identifier?.GetType().Name ?? "null"); +#pragma warning restore CS0618 + return new RealmKey(RealmOf(nodeId), nodeId.Identifier?.ToString() ?? string.Empty); + } /// Gets the count of variable nodes currently managed. public int VariableCount => _variables.Count; @@ -1262,7 +1275,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // ReportEvent catch below for the same pattern). Utils.LogInfo is [Obsolete] in 1.5.378 in // favour of an ITelemetryContext this manager doesn't wire — suppress the deprecation. #pragma warning disable CS0618 // Type or member is obsolete - Utils.LogInfo("OtOpcUaNodeManager.WireAlarmNotifiers: condition {0} not materialised — skipping notifier wiring", alarmKey); + // Interpolate the key's PARTS, not the RealmKey struct — a struct dump would render a shape + // no log-grep pattern in this file matches. + Utils.LogInfo("OtOpcUaNodeManager.WireAlarmNotifiers: condition {0} (realm {1}) not materialised — skipping notifier wiring", + alarmKey.Id, alarmKey.Realm); #pragma warning restore CS0618 return; } @@ -1297,8 +1313,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 { // Referencing-equipment folder not (yet) materialised — skip this one (logged, never thrown). #pragma warning disable CS0618 // Type or member is obsolete - Utils.LogInfo("OtOpcUaNodeManager.WireAlarmNotifiers: notifier folder {0} for condition {1} not present — skipped", - folderNodeId, alarmKey); + Utils.LogInfo("OtOpcUaNodeManager.WireAlarmNotifiers: notifier folder {0} for condition {1} (realm {2}) not present — skipped", + folderNodeId, alarmKey.Id, alarmKey.Realm); #pragma warning restore CS0618 continue; } @@ -1941,8 +1957,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // A historian is wired: advertise event history on this notifier and register it as a source. // The equipment-folder NodeId identifier IS the equipment id IS the ReadEventsAsync sourceName. folder.EventNotifier = (byte)(folder.EventNotifier | EventNotifiers.HistoryRead); - // Key by the full (namespace-qualified) NodeId string so a Raw and a UNS folder that share the - // same bare id are distinct sources; the VALUE stays the bare id — that is the historian + // Key by the realm-qualified RealmKey built from the NodeId so a Raw and a UNS folder that share + // the same bare id are distinct sources; the VALUE stays the bare id — that is the historian // ReadEventsAsync sourceName (== the equipment id). var sourceName = folder.NodeId.Identifier?.ToString() ?? string.Empty; _eventNotifierSources[MapKey(folder.NodeId)] = sourceName; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs index 78bceace..890a0282 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs @@ -507,7 +507,7 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers failedNodes += _applier.MaterialiseScriptedAlarms(composition); // Equipment-namespace tags get their own pass: ensures each signal's Variable (and any // FolderPath sub-folder) exists under its already-materialised equipment folder so - // clients can browse them. Live values are pushed by DriverHostActor.ForwardToMux after + // clients can browse them. Live values are pushed by DriverHostActor.ForwardBatchToMux after // each subscription cycle; variables show BadWaitingForInitialData only until the first // publish interval fires. failedNodes += _applier.MaterialiseEquipmentTags(composition); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/CalculationDependencyFlowTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/CalculationDependencyFlowTests.cs index a76c00a5..b70d82a5 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/CalculationDependencyFlowTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/CalculationDependencyFlowTests.cs @@ -17,7 +17,7 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; /// path (not merely that exists). /// A source value published to the mux is fed into the calc driver, which computes and publishes a /// value; and a calc tag reading ANOTHER calc tag's output (calc-of-calc) recomputes when the first -/// tag's output re-enters the mux — the exact re-entry the host's ForwardToMux performs, mirrored +/// tag's output re-enters the mux — the exact re-entry the host's ForwardBatchToMux performs, mirrored /// here by looping the driver's back into the mux. /// public sealed class CalculationDependencyFlowTests : RuntimeActorTestBase @@ -58,7 +58,7 @@ public sealed class CalculationDependencyFlowTests : RuntimeActorTestBase var published = new ConcurrentQueue(); var mux = Sys.ActorOf(DependencyMuxActor.Props(), "dep-mux"); - // Loop the driver's outputs back into the mux exactly as DriverHostActor.ForwardToMux does — this + // Loop the driver's outputs back into the mux exactly as DriverHostActor.ForwardBatchToMux does — this // is what makes calc-of-calc chains work. driver.OnDataChange += (_, e) => { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorBatchTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorBatchTests.cs index 92444a92..08052bb7 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorBatchTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorBatchTests.cs @@ -51,7 +51,12 @@ public sealed class OpcUaPublishActorBatchTests : RuntimeActorTestBase } /// A sink that throws out of WriteValues is swallowed and logged — the actor survives and keeps - /// processing (proved by a follow-up message landing), rather than restarting and dropping traffic. + /// processing, rather than restarting and dropping traffic. + /// The survival assertion alone does NOT prove the catch exists: Akka's default supervision + /// RESTARTS a throwing actor and the restart preserves the mailbox, so a follow-up message lands either + /// way. The load-bearing assertions are therefore the two event filters — the exception must never reach + /// supervision (no Error carrying it is ever logged), and the handler's own Warning must be logged so the + /// fault is swallowed but not silent. Deleting the catch turns BOTH red. [Fact] public void Sink_throw_is_swallowed_and_logged() { @@ -59,16 +64,25 @@ public sealed class OpcUaPublishActorBatchTests : RuntimeActorTestBase var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink)); var t0 = new DateTime(2026, 8, 15, 10, 0, 0, DateTimeKind.Utc); - actor.Tell(new OpcUaPublishActor.AttributeValueBatchUpdate(new[] + + // Expect(0, …) counts matches at the END of the action, so everything that must have happened by + // then goes INSIDE it — otherwise the absence assertion races the actor and passes vacuously. + EventFilter.Exception().Expect(0, () => { - new ValueWrite("Plant/Modbus/dev1/Speed", 1, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw), - })); + // The handler caught it AND said so: its own Warning, not supervision's Error. + EventFilter.Warning(contains: "sink.WriteValues threw").ExpectOne(() => + actor.Tell(new OpcUaPublishActor.AttributeValueBatchUpdate(new[] + { + new ValueWrite("Plant/Modbus/dev1/Speed", 1, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw), + }))); - // Follow-up on the untouched per-item path: it lands only if the actor is still alive and pumping. - actor.Tell(new OpcUaPublishActor.AttributeValueUpdate( - "Plant/Modbus/dev1/Temp", 2, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw)); + // Survival: a follow-up on the untouched per-item path lands only if the actor is still pumping. + actor.Tell(new OpcUaPublishActor.AttributeValueUpdate( + "Plant/Modbus/dev1/Temp", 2, OpcUaStatus.Good, t0, AddressSpaceRealm.Raw)); + + AwaitAssert(() => sink.SingleWrites.Count.ShouldBe(1), PresenceBudget); + }); - AwaitAssert(() => sink.SingleWrites.Count.ShouldBe(1), PresenceBudget); sink.Threw.ShouldBeTrue(); }