From ddd76782690015774da880a07b6a204a773ae37b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 17:02:12 -0400 Subject: [PATCH] fix(opcua): carry native alarm condition state across a full rebuild (#641) A full RebuildAddressSpace recreated every native Part 9 condition inactive + acknowledged, and when the alarm-tag set was unchanged (e.g. one UnsTagReference added to an alarm-bearing raw tag) nothing re-told the new node its state: the host does not re-subscribe for an unchanged set, and Galaxy / OpcUaClient have no snapshot to replay anyway. Active alarms read normal until their next device transition. OtOpcUaNodeManager now captures each NATIVE condition's live state (from the node, so a client-applied ack is kept) before the rebuild clears, restores it when the same id is re-materialised native in the same apply, and reports ONE condition event per restored off-normal condition at the new IOpcUaAddressSpaceSink.CompleteAddressSpaceApply (after notifier wiring). Unclaimed captures are discarded there, so a later re-add is fresh. Quality is carried silently for normal conditions too (it is only re-annotated on a connectivity edge). Scripted conditions are untouched (#487 re-asserts them). OpcUaPublishActor calls CompleteAddressSpaceApply at the end of every apply. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- .../OpcUa/DeferredAddressSpaceSink.cs | 3 + .../OpcUa/IOpcUaAddressSpaceSink.cs | 13 + .../OtOpcUaNodeManager.cs | 145 ++++++++++ .../SdkAddressSpaceSink.cs | 3 + .../OpcUa/OpcUaPublishActor.cs | 21 ++ .../DeferredSinkForwardingReflectionTests.cs | 8 +- .../NativeAlarmRebuildCarryOverWireTests.cs | 125 +++++++++ ...ManagerNativeAlarmRebuildCarryOverTests.cs | 255 ++++++++++++++++++ .../OpcUa/OpcUaPublishActorRebuildTests.cs | 41 +++ 9 files changed, 611 insertions(+), 3 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/NativeAlarmRebuildCarryOverWireTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerNativeAlarmRebuildCarryOverTests.cs 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 b53aa8b0..0c592bb1 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs @@ -69,6 +69,9 @@ public sealed class DeferredAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgical /// public void RebuildAddressSpace() => _inner.RebuildAddressSpace(); + /// + public void CompleteAddressSpaceApply() => _inner.CompleteAddressSpaceApply(); + /// public void RaiseNodesAddedModelChange(string affectedNodeId, AddressSpaceRealm realm) => _inner.RaiseNodesAddedModelChange(affectedNodeId, realm); 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 1573e05a..498cd59e 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs @@ -185,6 +185,19 @@ public interface IOpcUaAddressSpaceSink /// void RebuildAddressSpace(); + /// + /// Gitea #641 — mark the END of one address-space apply pass (after , if + /// one ran, and every materialise / notifier-wiring call that followed it). An SDK-backed sink carries each + /// native condition's live Part 9 state across a rebuild and restores it when the same condition is + /// re-materialised; this is where it reports the ONE condition event per restored condition (now that + /// every notifier folder is wired) and DISCARDS any captured state no materialise claimed, so a later + /// deploy that re-adds the same id gets a fresh node rather than a resurrection. OpcUaPublishActor + /// calls it once at the end of every apply, rebuild or not. Idempotent; the default is a no-op. + /// + void CompleteAddressSpaceApply() + { + } + /// /// Announce that nodes were added at runtime (discovered-node injection) under /// so subscribed clients refresh their browse diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 925bf37d..9132653a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -65,6 +65,16 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// a native re-materialise adds, and clears it alongside /// so a re-materialise as the other kind is correct. private readonly HashSet _nativeAlarmNodeIds = new(); + /// Gitea #641: every NATIVE condition's live state, captured by + /// just before it clears , waiting for + /// to re-create the same id as native in the same apply. Whatever is unclaimed when + /// runs is discarded. Guarded by Lock. + private readonly Dictionary _rebuildCarryOver = new(); + /// Gitea #641: conditions whose non-normal state a re-materialise restored, owed ONE condition + /// event at (deferred until then so every notifier folder the + /// apply wires receives it). Each entry pins the instance restored, so a condition replaced in between is + /// not reported. Guarded by Lock. + private readonly List<(RealmKey Key, AlarmConditionState Alarm, DateTime Time)> _restoredConditionReports = new(); /// Phase C: NodeId → resolved historian tagname for every variable materialised /// Historizing. Populated by when a historian tagname is supplied; the /// (later) HistoryRead override resolves a HistoryRead request's NodeId against this map. Cleared on @@ -1432,9 +1442,102 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // H6a: record native (driver-fed) conditions so a later task can route their inbound // Acknowledge to the driver rather than the scripted engine. if (isNative) _nativeAlarmNodeIds.Add(conditionKey); + + // Gitea #641: a native condition the rebuild in this apply discarded gets its live state back. Removed + // whatever the kind, so a native→scripted swap on the same id drops the capture instead of leaving it + // for a later re-add to resurrect. + if (_rebuildCarryOver.Remove(conditionKey, out var carried) && isNative) + RestoreCarryOver(alarm, conditionKey, carried); } } + /// + /// Gitea #641 — one native condition's live Part 9 state, carried across a full + /// . + /// + /// The gate-relevant fields, read off the node as does. + /// The condition's Time — the instant its current state came about. + /// When it last changed Active, if the SDK recorded it. + /// The source timestamp of its Quality annotation, if present. + private sealed record NativeConditionCarryOver( + AlarmConditionDelta State, DateTime Time, DateTime? ActiveTransitionTime, DateTime? QualityTimestamp); + + /// + /// Gitea #641 — read what a native condition node holds right now, so the rebuild can put it back. + /// + /// + /// + /// Why the NODE, not a host-side cache. The node is the only place that knows the true state: + /// a client's Acknowledge (and its comment) is applied to the node by the SDK and never passes through + /// the driver or NativeAlarmProjector. Replaying from any host-side record would un-acknowledge it. + /// + /// + /// Why it is needed at all. A rebuild recreates every condition inactive + acknowledged. When the + /// alarm-tag set is unchanged — the common case: adding a UnsTagReference to an alarm-bearing raw + /// tag is enough to force one — the host does not re-subscribe the driver's alarms, and even a + /// re-subscribe would not help a transition-only source (Galaxy, OpcUaClient) with nothing to replay. + /// Without this, every active alarm read normal until its next device transition. + /// + /// + /// The live native condition, about to be discarded. + /// Its carried state. + private static NativeConditionCarryOver CaptureCarryOver(AlarmConditionState condition) => new( + ReadConditionDelta(condition), + condition.Time?.Value ?? DateTime.UtcNow, + condition.ActiveState?.TransitionTime?.Value, + condition.Quality?.SourceTimestamp?.Value); + + /// + /// Gitea #641 — put a carried state back onto the freshly materialised native condition. MUST be called + /// under Lock, after the condition is registered. + /// + /// + /// + /// Quality always; the rest only off-normal. Source quality is annotated only on a driver's + /// connectivity edge, so a rebuild otherwise left every native condition — normal ones included — + /// reading BadWaitingForInitialData until the driver next flapped; it is restored silently. A + /// condition in the no-event position (inactive + acknowledged) otherwise keeps exactly what the + /// materialise built, message and authored severity included, and reports nothing — the same rule the + /// scripted re-assert follows (#487). + /// + /// + /// Reported later, once. The state is applied now, so a Read or ConditionRefresh in the window + /// already sees it, but the one condition event waits for : the + /// equipment folders the condition reports through are wired AFTER this call + /// (), and reporting now would miss them. Native conditions refuse + /// Confirm, shelving and Enable/Disable (#587), so Enabled and Shelving are not carried. + /// + /// + /// The new condition instance. + /// Its key. + /// What the discarded instance held. + private void RestoreCarryOver(AlarmConditionState alarm, RealmKey conditionKey, NativeConditionCarryOver carried) + { + var state = carried.State; + if (alarm.Quality is not null) + { + alarm.Quality.Value = state.Quality; + if (alarm.Quality.SourceTimestamp is not null && carried.QualityTimestamp is { } qualityAt) + alarm.Quality.SourceTimestamp.Value = qualityAt; + } + + var retained = state.Active || !state.Acknowledged; + if (!retained) return; + + alarm.SetActiveState(SystemContext, state.Active); + if (carried.ActiveTransitionTime is { } activeAt && alarm.ActiveState?.TransitionTime is { } transitionTime) + transitionTime.Value = activeAt; + alarm.SetAcknowledgedState(SystemContext, state.Acknowledged); + if (alarm.ConfirmedState is not null) alarm.SetConfirmedState(SystemContext, state.Confirmed); + alarm.SetSeverity(SystemContext, (EventSeverity)state.MappedSeverity); + alarm.Message.Value = new LocalizedText(state.Message); + alarm.Retain.Value = true; + alarm.Time.Value = carried.Time; + alarm.ReceiveTime.Value = carried.Time; + + _restoredConditionReports.Add((conditionKey, alarm, carried.Time)); + } + /// One materialised native-alarm condition together with the equipment folders wired as extra /// event-notifier roots for it. The list is mutated in place under Lock as /// notifiers are wired / torn down; is the concrete @@ -3207,6 +3310,16 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 _historizedTagnames.Clear(); _arrayOnScalarRefused.Clear(); + // Gitea #641: capture every NATIVE condition's live state before the conditions are discarded, so the + // re-materialise that follows in this same apply can put it back (see CaptureCarryOver). Scripted + // conditions are not captured — their engine re-asserts them after the reload (#487). + _rebuildCarryOver.Clear(); + _restoredConditionReports.Clear(); + foreach (var (key, alarm) in _alarmConditions) + { + if (_nativeAlarmNodeIds.Contains(key)) _rebuildCarryOver[key] = CaptureCarryOver(alarm); + } + foreach (var alarm in _alarmConditions.Values) { alarm.Parent?.RemoveChild(alarm); @@ -3251,6 +3364,38 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 } } + /// + /// + /// Gitea #641: reports ONE condition event for each native condition whose off-normal state the rebuild + /// carried over (stamped with the instant that state came about, not the deploy time — as the scripted + /// re-assert does), then discards whatever captured state no materialise claimed. A condition the apply + /// replaced again after restoring it is skipped: the instance reported must be the one restored. + /// + public void CompleteAddressSpaceApply() + { + int reported = 0, discarded; + lock (Lock) + { + foreach (var (key, alarm, time) in _restoredConditionReports) + { + if (!_alarmConditions.TryGetValue(key, out var live) || !ReferenceEquals(live, alarm)) continue; + ReportConditionEvent(alarm, time); + reported++; + } + + discarded = _rebuildCarryOver.Count; + _restoredConditionReports.Clear(); + _rebuildCarryOver.Clear(); + } + + if (reported > 0 || discarded > 0) + { + _logger.LogInformation( + "OtOpcUaNodeManager: rebuild carried {Reported} active/unacknowledged native condition(s) over; discarded {Discarded} captured condition(s) the apply did not re-create", + reported, discarded); + } + } + /// public bool RemoveVariableNode(string variableNodeId, AddressSpaceRealm realm) { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs index 3d4dda2c..32d37245 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs @@ -84,6 +84,9 @@ public sealed class SdkAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgicalAddre /// public void RebuildAddressSpace() => _nodeManager.RebuildAddressSpace(); + /// + public void CompleteAddressSpaceApply() => _nodeManager.CompleteAddressSpaceApply(); + /// public void RaiseNodesAddedModelChange(string affectedNodeId, AddressSpaceRealm realm) => _nodeManager.RaiseNodesAddedModelChange(affectedNodeId, realm); 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 27a321f7..883f8c6f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs @@ -437,6 +437,27 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers } private void HandleRebuild(RebuildAddressSpace msg) + { + try + { + HandleRebuildCore(msg); + } + finally + { + // Gitea #641 — close the apply on EVERY exit, early returns and a throwing pass included: the node + // manager reports here the native conditions a rebuild carried over (after the notifier wiring the + // passes did) and discards any captured state no materialise claimed. Skipping it on a throw would + // leave that state for a later surgical re-add of the same id to resurrect. No-op when nothing is + // pending; never throws out of the handler. + try { _sink.CompleteAddressSpaceApply(); } + catch (Exception ex) + { + _log.Error(ex, "OpcUaPublish: sink.CompleteAddressSpaceApply threw (correlation={Correlation})", msg.Correlation); + } + } + } + + private void HandleRebuildCore(RebuildAddressSpace msg) { using var span = OtOpcUaTelemetry.StartAddressSpaceRebuildSpan(); span?.SetTag("otopcua.correlation_id", msg.Correlation.ToString()); diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs index 7a455cec..6a53880e 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs @@ -80,11 +80,13 @@ public class DeferredSinkForwardingReflectionTests // v3 B4-WP2 dual-namespace invariant: a node id alone is no longer globally unique across the two // namespaces (Raw vs UNS), so every sink method that NAMES a node must carry an AddressSpaceRealm // discriminator — the sink resolves the namespace from the realm rather than parsing the id string. - // RebuildAddressSpace is the sole node-naming-free method (it clears BOTH realms). This guard locks in - // the realm surface so a future method can't be added that names a node without a realm. + // RebuildAddressSpace (it clears BOTH realms) and CompleteAddressSpaceApply (it closes an apply pass, + // Gitea #641) are the only node-naming-free methods. This guard locks in the realm surface so a future + // method can't be added that names a node without a realm. foreach (var method in ForwardingInterfaces.SelectMany(i => i.GetMethods())) { - if (method.Name == nameof(IOpcUaAddressSpaceSink.RebuildAddressSpace)) continue; + if (method.Name is nameof(IOpcUaAddressSpaceSink.RebuildAddressSpace) + or nameof(IOpcUaAddressSpaceSink.CompleteAddressSpaceApply)) continue; method.GetParameters().Any(p => CarriesRealm(p.ParameterType)).ShouldBeTrue( $"{method.DeclaringType!.Name}.{method.Name} names a node but has no AddressSpaceRealm parameter — " + diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/NativeAlarmRebuildCarryOverWireTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/NativeAlarmRebuildCarryOverWireTests.cs new file mode 100644 index 00000000..4a09b100 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/NativeAlarmRebuildCarryOverWireTests.cs @@ -0,0 +1,125 @@ +using Opc.Ua; +using Opc.Ua.Client; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using static ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests.ServerEventRig; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests; + +/// +/// Gitea #641 over the wire — a full address-space rebuild that leaves the native alarm set unchanged must not +/// leave an active condition reading inactive. A Server-object (i=2253) item, which outlives the rebuild +/// (#586), receives exactly ONE condition event carrying the restored state — and an acknowledgement a client +/// applied before the rebuild survives it. +/// +/// +/// The rebuild is driven exactly as OpcUaPublishActor drives one: RebuildAddressSpace, the +/// re-materialise + notifier wiring, then CompleteAddressSpaceApply. The count is read after a later +/// sentinel transition arrives on the same item, so a duplicate would already be queued ahead of it. +/// The native ack router echoes nothing back, like MTConnect and FOCAS: the acknowledgement lives only on the node. +/// +public sealed class NativeAlarmRebuildCarryOverWireTests +{ + private const NodePermissions All = + NodePermissions.Browse | NodePermissions.Read | NodePermissions.Subscribe | NodePermissions.AlarmRead + | NodePermissions.AlarmAcknowledge | NodePermissions.MethodCall; + + private static AlarmConditionSnapshot Fault(string message) => new( + Active: true, Acknowledged: false, Confirmed: true, Enabled: true, + Shelving: AlarmShelvingKind.Unshelved, Severity: 800, Message: message); + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_rebuild_with_an_unchanged_alarm_set_reports_the_restored_state_once(bool clientAcked) + { + var ct = TestContext.Current.CancellationToken; + var rig = await StartAsync($"Rebuild641{clientAcked}", encrypted: false, ct); + try + { + rig.Enforce(false, [Key()], ClusterGrant(KeyPrincipal, "acl-key", All)); + rig.Nm.NativeAlarmAckRouter = _ => { }; // a driver whose ack is a no-op: nothing echoes back + rig.MaterialiseNative(); + + using var session = await rig.ConnectAsync(anonymous: false, ct); + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(session, events, ct); + var serverItem = RecordedEvents.AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + + rig.Project(native: true, Fault("fault")); + await RecordedEvents.WaitUntilAsync(() => events.Has(serverItem, "fault")); + var expectedBefore = 1; + + if (clientAcked) + { + await session.CallAsync( + ConditionNodeId(session, native: true), MethodIds.AcknowledgeableConditionType_Acknowledge, ct, + events.LatestEventId(serverItem, "fault"), new LocalizedText("ack")); + await RecordedEvents.WaitUntilAsync(() => events.ConditionEvents(serverItem, "fault").Count == 2); + expectedBefore = 2; + } + + // The deploy: a rebuild whose composition carries the same alarm (e.g. one UnsTagReference added). + rig.Sink.RebuildAddressSpace(); + rig.MaterialiseNative(); + rig.Sink.CompleteAddressSpaceApply(); + + rig.Project(native: true, Fault("sentinel") with { Acknowledged = clientAcked }); + await RecordedEvents.WaitUntilAsync(() => events.Has(serverItem, "sentinel")); + + var faultEvents = events.ConditionEvents(serverItem, "fault"); + faultEvents.Count.ShouldBe(expectedBefore + 1, "the pre-rebuild event(s) plus exactly one restored event"); + var restored = faultEvents[^1]; + restored[RecordedEvents.ActiveField].Value.ShouldBe(true); + restored[RecordedEvents.AckedField].Value.ShouldBe(clientAcked); + restored[RecordedEvents.RetainField].Value.ShouldBe(true); + restored[RecordedEvents.SeverityField].Value.ShouldBe((ushort)800); + + // And a ConditionRefresh-style read of the live node agrees. + var active = await session.ReadValueAsync( + new NodeId($"{RawAlarmPath}/ActiveState/Id", ConditionNodeId(session, native: true).NamespaceIndex), ct); + active.Value.ShouldBe(true); + + await subscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } + + /// Falsification control: a NORMAL native condition produces no event across the rebuild. + [Fact] + public async Task A_rebuild_reports_nothing_for_a_normal_condition() + { + var ct = TestContext.Current.CancellationToken; + var rig = await StartAsync("Rebuild641Normal", encrypted: false, ct); + try + { + rig.Enforce(false, [Key()], ClusterGrant(KeyPrincipal, "acl-key", All)); + rig.MaterialiseNative(); + + using var session = await rig.ConnectAsync(anonymous: false, ct); + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(session, events, ct); + var serverItem = RecordedEvents.AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + + rig.Project(native: true, Fault("cleared") with { Active = false, Acknowledged = true }); + await RecordedEvents.WaitUntilAsync(() => events.Has(serverItem, "cleared")); + + rig.Sink.RebuildAddressSpace(); + rig.MaterialiseNative(); + rig.Sink.CompleteAddressSpaceApply(); + + rig.Project(native: true, Fault("sentinel")); + await RecordedEvents.WaitUntilAsync(() => events.Has(serverItem, "sentinel")); + + events.ConditionEvents(serverItem, "cleared").Count.ShouldBe(1); + events.ConditionEvents(serverItem, "temp_hi").ShouldBeEmpty("the fresh node is not reported"); + + await subscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerNativeAlarmRebuildCarryOverTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerNativeAlarmRebuildCarryOverTests.cs new file mode 100644 index 00000000..673fbef2 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerNativeAlarmRebuildCarryOverTests.cs @@ -0,0 +1,255 @@ +using Opc.Ua; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; + +/// +/// Gitea #641 — a full recreates every native condition +/// inactive, and when the alarm-tag set is unchanged nothing tells the new node its state again (the driver +/// does not re-subscribe, and Galaxy / OpcUaClient have nothing to replay anyway). The node manager therefore +/// carries each NATIVE condition's live state across the rebuild: captured before the clear, restored when +/// the same id is re-materialised native in the same apply, and reported as ONE condition event when the +/// apply completes (). +/// +/// +/// "One event" is observed here through the condition's EventId, which every report stamps fresh. +/// The exact count over the wire is pinned by NativeAlarmRebuildCarryOverWireTests. +/// +public sealed class NodeManagerNativeAlarmRebuildCarryOverTests : IDisposable +{ + private const string Device = "Plant/MTConnect/dev1"; + private const string Alarm = "Plant/MTConnect/dev1/Xtravel"; + + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + private readonly string _pkiRoot = Path.Combine(Path.GetTempPath(), $"otopcua-641-{Guid.NewGuid():N}"); + + private static AlarmConditionSnapshot Fault(bool acked) => new( + Active: true, Acknowledged: acked, Confirmed: true, Enabled: true, + Shelving: AlarmShelvingKind.Unshelved, Severity: 800, Message: "Xtravel FAULT 1001", + StatusCode: OpcUaStatus.Good); + + [Trait("Category", "Unit")] + [Fact] + public async Task An_active_unacked_native_condition_survives_a_rebuild_and_is_reported_once() + { + var (host, nm) = await BootAsync(); + var raisedAt = new DateTime(2026, 10, 8, 9, 0, 0, DateTimeKind.Utc); + Materialise(nm, native: true); + nm.WriteAlarmCondition(Alarm, Fault(acked: false), raisedAt, AddressSpaceRealm.Raw); + + nm.RebuildAddressSpace(); + Materialise(nm, native: true); + var restored = Condition(nm); + var idBeforeComplete = (byte[]?)restored.EventId.Value?.Clone(); + + // State is back as soon as the node exists — a Read or a ConditionRefresh in between already sees it. + restored.ActiveState.Id.Value.ShouldBeTrue(); + restored.AckedState.Id.Value.ShouldBeFalse(); + restored.Retain.Value.ShouldBeTrue(); + restored.Severity.Value.ShouldBe((ushort)800); + restored.Message.Value.Text.ShouldBe("Xtravel FAULT 1001"); + restored.Quality.Value.Code.ShouldBe(StatusCodes.Good); + + nm.CompleteAddressSpaceApply(); + + // The report happens at the END of the apply (after the notifier wiring), stamped with the instant the + // state came about, not the deploy time. + restored.EventId.Value.ShouldNotBe(idBeforeComplete); + restored.Time.Value.ShouldBe(raisedAt); + + // ...and only once: a second completion with nothing pending reports nothing. + var idAfterComplete = (byte[]?)restored.EventId.Value?.Clone(); + nm.CompleteAddressSpaceApply(); + restored.EventId.Value.ShouldBe(idAfterComplete); + + await host.DisposeAsync(); + } + + /// + /// The acknowledgement a CLIENT applied lives only on the node (the SDK applied it; no driver or projector + /// saw it) — the reason the carry-over reads the node rather than any host-side cache. + /// + [Trait("Category", "Unit")] + [Fact] + public async Task A_client_acknowledgement_survives_the_rebuild() + { + var (host, nm) = await BootAsync(); + Materialise(nm, native: true); + nm.WriteAlarmCondition(Alarm, Fault(acked: false), DateTime.UtcNow, AddressSpaceRealm.Raw); + // What the SDK's Acknowledge handler does to the node; nothing else hears of it. + Condition(nm).SetAcknowledgedState(nm.SystemContext, true); + + nm.RebuildAddressSpace(); + Materialise(nm, native: true); + nm.CompleteAddressSpaceApply(); + + var restored = Condition(nm); + restored.ActiveState.Id.Value.ShouldBeTrue(); + restored.AckedState.Id.Value.ShouldBeTrue(); + restored.Retain.Value.ShouldBeTrue(); + + await host.DisposeAsync(); + } + + /// Falsification control: a condition in the no-event position comes back exactly as a fresh + /// materialise builds it, and reports nothing. + [Trait("Category", "Unit")] + [Fact] + public async Task A_normal_native_condition_reports_nothing_and_keeps_the_fresh_state() + { + var (host, nm) = await BootAsync(); + Materialise(nm, native: true); + nm.WriteAlarmCondition(Alarm, Fault(acked: false) with { Active = false, Acknowledged = true, Message = "cleared" }, + DateTime.UtcNow, AddressSpaceRealm.Raw); + + nm.RebuildAddressSpace(); + Materialise(nm, native: true); + var fresh = Condition(nm); + var idBefore = (byte[]?)fresh.EventId.Value?.Clone(); + nm.CompleteAddressSpaceApply(); + + fresh.EventId.Value.ShouldBe(idBefore); + fresh.ActiveState.Id.Value.ShouldBeFalse(); + fresh.AckedState.Id.Value.ShouldBeTrue(); + fresh.Retain.Value.ShouldBeFalse(); + fresh.Message.Value.Text.ShouldBe("Xtravel", "a normal condition keeps the materialise's message"); + fresh.Severity.Value.ShouldBe((ushort)700, "and its authored severity"); + + await host.DisposeAsync(); + } + + /// + /// The source-data quality is carried for a NORMAL condition too, silently: quality is annotated only on a + /// driver's connectivity edge, so without the carry a rebuild left every normal native condition reading + /// BadWaitingForInitialData until the driver next flapped. + /// + [Trait("Category", "Unit")] + [Fact] + public async Task A_normal_condition_keeps_its_quality_without_an_event() + { + var (host, nm) = await BootAsync(); + Materialise(nm, native: true); + nm.WriteAlarmQuality(Alarm, OpcUaStatus.Good, DateTime.UtcNow, AddressSpaceRealm.Raw); + + nm.RebuildAddressSpace(); + Materialise(nm, native: true); + var fresh = Condition(nm); + var idBefore = (byte[]?)fresh.EventId.Value?.Clone(); + nm.CompleteAddressSpaceApply(); + + fresh.Quality.Value.Code.ShouldBe(StatusCodes.Good); + fresh.EventId.Value.ShouldBe(idBefore); + + await host.DisposeAsync(); + } + + /// A condition the rebuild did NOT re-create has its captured state discarded when the apply + /// completes, so a later deploy that adds the same RawPath back gets a fresh node, not a resurrection. + [Trait("Category", "Unit")] + [Fact] + public async Task A_condition_removed_by_the_rebuild_is_not_resurrected_by_a_later_add() + { + var (host, nm) = await BootAsync(); + Materialise(nm, native: true); + nm.WriteAlarmCondition(Alarm, Fault(acked: false), DateTime.UtcNow, AddressSpaceRealm.Raw); + + nm.RebuildAddressSpace(); + nm.CompleteAddressSpaceApply(); // the alarm was removed: nothing re-materialised it + + Materialise(nm, native: true); // a later, surgical re-add of the same RawPath + nm.CompleteAddressSpaceApply(); + + var readded = Condition(nm); + readded.ActiveState.Id.Value.ShouldBeFalse(); + readded.AckedState.Id.Value.ShouldBeTrue(); + + await host.DisposeAsync(); + } + + /// A native condition re-materialised as SCRIPTED under the same id is a different alarm: nothing + /// is restored onto it (and the scripted engine's own #487 re-assert owns scripted state). + [Trait("Category", "Unit")] + [Fact] + public async Task A_kind_swap_on_the_same_id_is_not_restored() + { + var (host, nm) = await BootAsync(); + Materialise(nm, native: true); + nm.WriteAlarmCondition(Alarm, Fault(acked: false), DateTime.UtcNow, AddressSpaceRealm.Raw); + + nm.RebuildAddressSpace(); + Materialise(nm, native: false); + nm.CompleteAddressSpaceApply(); + + Condition(nm).ActiveState.Id.Value.ShouldBeFalse(); + + await host.DisposeAsync(); + } + + /// Scripted conditions are not captured — ScriptedAlarmHostActor.ReassertConditionNodes + /// (#487) already re-projects them after a reload, and a second source would double-report. + [Trait("Category", "Unit")] + [Fact] + public async Task A_scripted_condition_is_not_carried() + { + var (host, nm) = await BootAsync(); + Materialise(nm, native: false); + nm.WriteAlarmCondition(Alarm, Fault(acked: false), DateTime.UtcNow, AddressSpaceRealm.Raw); + + nm.RebuildAddressSpace(); + Materialise(nm, native: false); + nm.CompleteAddressSpaceApply(); + + Condition(nm).ActiveState.Id.Value.ShouldBeFalse(); + + await host.DisposeAsync(); + } + + private static void Materialise(OtOpcUaNodeManager nm, bool native) + { + nm.EnsureFolder(Device, parentNodeId: null, displayName: "dev1", realm: AddressSpaceRealm.Raw); + nm.MaterialiseAlarmCondition(Alarm, Device, "Xtravel", "OffNormalAlarm", 700, AddressSpaceRealm.Raw, isNative: native); + } + + private static Opc.Ua.AlarmConditionState Condition(OtOpcUaNodeManager nm) => + nm.TryGetAlarmCondition(Alarm, AddressSpaceRealm.Raw).ShouldNotBeNull(); + + private async Task<(OpcUaApplicationHost Host, OtOpcUaNodeManager Nm)> BootAsync() + { + var host = new OpcUaApplicationHost( + new OpcUaApplicationHostOptions + { + ApplicationName = "OtOpcUa.Gitea641Test", + ApplicationUri = $"urn:OtOpcUa.Gitea641Test:{Guid.NewGuid():N}", + OpcUaPort = AllocateFreePort(), + PublicHostname = "localhost", + PkiStoreRoot = _pkiRoot, + }, + Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + + var server = new OtOpcUaSdkServer(); + await host.StartAsync(server, Ct); + return (host, server.NodeManager!); + } + + private static int AllocateFreePort() + { + using var listener = new System.Net.Sockets.TcpListener(System.Net.IPAddress.Loopback, 0); + listener.Start(); + var port = ((System.Net.IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } + + /// Cleans up the PKI root directory. + public void Dispose() + { + if (Directory.Exists(_pkiRoot)) + { + try { Directory.Delete(_pkiRoot, recursive: true); } + catch { /* best-effort cleanup */ } + } + } +} 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 94453d9a..93880e0c 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 @@ -61,6 +61,41 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase sink.RebuildCalls.ShouldBe(0); } + /// + /// Gitea #641 — every apply ends with CompleteAddressSpaceApply, AFTER the last materialise / announce + /// call: that is where the node manager reports the conditions a rebuild carried over (their notifier + /// folders are wired by then) and drops any captured state no materialise claimed. + /// + [Fact] + public void An_apply_completes_after_every_materialise_pass() + { + var db = NewInMemoryDbFactory(); + var sink = new RecordingSink(); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + SeedDeployment(db, equipmentIds: new[] { "eq-1", "eq-2" }, driverIds: new[] { "drv-1" }); + + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier)); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); + + AwaitAssert(() => sink.CompletedAfterCallCount.Count.ShouldBe(1), duration: PresenceBudget); + sink.CompletedAfterCallCount.Single().ShouldBe(sink.Calls.Count, "nothing may follow the completion"); + sink.Calls.ShouldContain("NA:line-1"); + } + + /// Gitea #641 — the raw-sink fallback rebuild (no applier) completes too, so the state it captured + /// is discarded rather than left for a later add to resurrect. + [Fact] + public void The_raw_sink_fallback_rebuild_completes_the_apply() + { + var sink = new RecordingSink(); + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink)); + + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); + + AwaitAssert(() => sink.CompletedAfterCallCount.Count.ShouldBe(1), duration: PresenceBudget); + sink.RebuildCalls.ShouldBe(1); + } + /// Tests that rebuild with no artifact is idempotent no-op. [Fact] public void Rebuild_with_no_artifact_is_idempotent_no_op() @@ -1053,6 +1088,12 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase } /// Records a rebuild address space call. public void RebuildAddressSpace() => Interlocked.Increment(ref RebuildCalls); + /// Gets, per call, how many had been + /// recorded at that moment — so a test can prove the completion came after every pass (Gitea #641). + /// Kept out of so existing call-count assertions are unaffected. + public ConcurrentQueue CompletedAfterCallCount { get; } = new(); + /// Records the end of an apply pass (Gitea #641). + public void CompleteAddressSpaceApply() => CompletedAfterCallCount.Enqueue(Calls.Count); /// Records a NodeAdded model-change announcement. /// The node under which discovered nodes were added. public void RaiseNodesAddedModelChange(string affectedNodeId, AddressSpaceRealm realm = AddressSpaceRealm.Uns) => Calls.Enqueue($"NA:{affectedNodeId}");