From f18d3adbfc1f08976579af1b669c3ed0f1642c2f Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 06:28:00 -0400 Subject: [PATCH] fix(dcl): forget an OPC UA condition's last state once it returns to normal (review 2026-10-21 02/L3) --- .../Component-DataConnectionLayer.md | 2 +- .../Adapters/RealOpcUaClient.cs | 14 ++- .../Adapters/RealOpcUaAlarmLastStateTests.cs | 89 +++++++++++++++++++ 3 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/RealOpcUaAlarmLastStateTests.cs diff --git a/docs/requirements/Component-DataConnectionLayer.md b/docs/requirements/Component-DataConnectionLayer.md index 036d064c..857cffa6 100644 --- a/docs/requirements/Component-DataConnectionLayer.md +++ b/docs/requirements/Component-DataConnectionLayer.md @@ -468,4 +468,4 @@ The DCL reports the following metrics to the Health Monitoring component via the OPC UA connection and endpoint verification select exactly the configured security mode. If a protected mode is unavailable, or discovery fails while protection was requested, the operation fails with the requested mode in its diagnostic; it never falls back to an unsecured session. -DCL retains each subscribed alarm source's current standing state and its in-progress snapshot. A new subscriber sharing a feed receives a scoped replay, or the already-received prefix of an in-progress frame followed by the remaining frame. Cache state is discarded on reconnect and last-owner unsubscribe. A new Mx source reopens the shared stream even when the existing prefix covers it, obtaining a fresh standing-state snapshot; callbacks from the canceled stream are ignored. Mx refresh boundaries remain connection-wide but share a distinct per-stream epoch. Both replay cache and runtime mirror ignore stale-epoch rows and apply newer live raises/clears over a concurrent refresh before committing it. +DCL retains each subscribed alarm source's current standing state and its in-progress snapshot. A new subscriber sharing a feed receives a scoped replay, or the already-received prefix of an in-progress frame followed by the remaining frame. Cache state is discarded on reconnect and last-owner unsubscribe. A new Mx source reopens the shared stream even when the existing prefix covers it, obtaining a fresh standing-state snapshot; callbacks from the canceled stream are ignored. Mx refresh boundaries remain connection-wide but share a distinct per-stream epoch. Both replay cache and runtime mirror ignore stale-epoch rows and apply newer live raises/clears over a concurrent refresh before committing it. The OPC UA client's per-condition `(active, acked)` memory used to derive transition kinds follows the same removal rule as the replay cache — a condition that returns to inactive + acknowledged (the default prior state) is forgotten — so it stays bounded on servers whose condition names carry a per-occurrence component (review 2026-10-21). diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs index 6a7b4dd4..edd12df9 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/RealOpcUaClient.cs @@ -1236,8 +1236,18 @@ public class RealOpcUaClient : IOpcUaClient var kind = inRefresh ? AlarmTransitionKind.Snapshot : OpcUaAlarmMapper.DeriveKind(prevAcked, acked, prevActive, active); - lastState?.TryAdd(sourceRef, (active, acked)); - if (lastState != null) lastState[sourceRef] = (active, acked); + if (lastState != null) + { + // Inactive + acknowledged IS the default prior state above, so forgetting it changes + // no derived kind — and keeps the map bounded on servers whose condition names carry + // a per-occurrence component (batch, lot, counter), where every occurrence is a new + // key for the life of the session (review 2026-10-21 02/L3; mirrors the actor's + // CacheAlarmTransition removal rule). + if (!active && acked) + lastState.Remove(sourceRef); + else + lastState[sourceRef] = (active, acked); + } onTransition(new NativeAlarmTransition( SourceReference: sourceRef, diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/RealOpcUaAlarmLastStateTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/RealOpcUaAlarmLastStateTests.cs new file mode 100644 index 00000000..cc0aa8c0 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/RealOpcUaAlarmLastStateTests.cs @@ -0,0 +1,89 @@ +using System.Collections.Concurrent; +using System.Reflection; +using Opc.Ua; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Alarms; +using ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Adapters; + +namespace ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests.Adapters; + +/// +/// Review 2026-10-21 02/L3: the per-condition (active, acked) memory behind +/// DeriveKind must not grow for the life of a session. A condition that returns to +/// inactive + acknowledged is indistinguishable from one never seen (the default prior state), so +/// its key is dropped — otherwise a server whose condition names carry a per-occurrence component +/// (batch, lot, counter) adds a key per occurrence until the next reconnect. +/// +public class RealOpcUaAlarmLastStateTests +{ + private const string Handle = "h1"; + + private static readonly MethodInfo Handler = + typeof(RealOpcUaClient).GetMethod("HandleAlarmEvent", BindingFlags.Instance | BindingFlags.NonPublic)!; + + private static Dictionary SeedLastState(RealOpcUaClient client) + { + var field = typeof(RealOpcUaClient).GetField("_alarmLastState", BindingFlags.Instance | BindingFlags.NonPublic)!; + var map = (ConcurrentDictionary>)field.GetValue(client)!; + var perHandle = new Dictionary(StringComparer.Ordinal); + map[Handle] = perHandle; + return perHandle; + } + + private static void Emit(RealOpcUaClient client, string condition, bool active, bool acked, List sink) + { + var fields = new VariantCollection + { + new(ObjectTypeIds.AlarmConditionType), // 0 EventType + new(new NodeId("Tank01", 2)), // 1 SourceNode + new("Tank01"), // 2 SourceName + new(DateTime.UtcNow), // 3 Time + new(new LocalizedText("msg")), // 4 Message + new((ushort)500), // 5 Severity + new(active), // 6 ActiveState/Id + new(acked), // 7 AckedState/Id + Variant.Null, // 8 ConfirmedState/Id + new(false), // 9 SuppressedOrShelved + Variant.Null, // 10 ShelvingState + new(condition), // 11 ConditionName + }; + Handler.Invoke(client, [Handle, "ns=2;s=Tank01", new EventFieldList { EventFields = fields }, + (Action)sink.Add]); + } + + /// A per-occurrence condition that clears and is acknowledged leaves no key behind. + [Fact] + public void ReturnToNormal_DropsTheConditionKey() + { + var client = new RealOpcUaClient(); + var lastState = SeedLastState(client); + var sink = new List(); + + for (var lot = 1; lot <= 50; lot++) + { + Emit(client, $"Hi.Lot{lot}", active: true, acked: false, sink); + Emit(client, $"Hi.Lot{lot}", active: false, acked: true, sink); + } + + Assert.Empty(lastState); + Assert.Equal(100, sink.Count); + } + + /// + /// Dropping the key does not change the transition kinds derived for a condition that comes + /// back — the dropped state equals the default prior state. + /// + [Fact] + public void ReRaiseAfterReturnToNormal_StillDerivesARaise() + { + var client = new RealOpcUaClient(); + var lastState = SeedLastState(client); + var sink = new List(); + + Emit(client, "Hi", active: true, acked: false, sink); + Emit(client, "Hi", active: false, acked: true, sink); + Emit(client, "Hi", active: true, acked: false, sink); + + Assert.Equal(sink[0].Kind, sink[2].Kind); + Assert.Single(lastState); // the re-raised condition is tracked again + } +}