fix(opcuaclient): scope the #670 F2 late-replay drop to the post-recovery window

The global 'never older than the last Good source time' rule froze a node
whenever an upstream clock stepped backwards (NTP correction, PLC clock
reset) until the clock caught up. The check is now a replay guard armed
only by a same-session recovery (keep-alive bad->good): per ref it holds
the last Good upstream source time (raised by the recovery read), a ref
disarms on its first in-order notification, and the whole guard expires
after max(30 s, 2x the longest subscription keep-alive window). Outside
it every notification is accepted. The steady-state tracker records the
LAST accepted time, not a high-water mark.
This commit is contained in:
Joseph Doherty
2026-10-11 09:15:51 -04:00
parent 99c13e4459
commit 87e8aff786
4 changed files with 157 additions and 27 deletions
@@ -496,13 +496,60 @@ public sealed class OpcUaClientSessionLossTests
bad.ServerTimestampUtc.ShouldBe(clock.Now.UtcDateTime);
}
private static void SetupReadAt(Mock<ISession> session, object value, DateTime sourceTime) =>
session.Setup(s => s.ReadAsync(
It.IsAny<RequestHeader>(), It.IsAny<double>(), It.IsAny<TimestampsToReturn>(),
It.IsAny<ReadValueIdCollection>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(new ReadResponse
{
ResponseHeader = new ResponseHeader(),
Results = [new DataValue(new Variant(value), StatusCodes.Good, sourceTime, sourceTime)],
DiagnosticInfos = [],
});
/// <summary>Drives a same-session outage + recovery (bad → good keep-alive) and waits for the recovery read.</summary>
private static async Task RecoverInPlaceAsync(OpcUaClientDriver drv)
{
drv.RaiseKeepAliveForTest(StatusCodes.BadNoCommunication);
drv.RaiseKeepAliveForTest(StatusCodes.Good);
await drv.RecoveryTaskForTest;
}
/// <summary>
/// #670 follow-up F2 (live gate): after a same-session recovery the SDK delivered one notification late with
/// an OLDER source time, and the node regressed to it. A notification older than the last Good source time
/// published for its ref is dropped; an equal one (a status-only change) and a newer one pass.
/// an OLDER source time, and the node regressed to it. Inside the post-recovery replay window an older
/// notification is dropped; an equal one (a status-only change) passes and disarms the ref, after which the
/// ref is back to accepting anything.
/// </summary>
[Fact]
public void A_notification_older_than_the_last_good_source_time_is_dropped()
public async Task After_a_recovery_a_late_older_notification_is_dropped_until_the_ref_is_back_in_order()
{
var (drv, session, clock) = NewConnectedDriver();
using var _ = drv;
var handle = drv.AddSubscriptionForTest([SpeedRef]);
var t0 = clock.Now.UtcDateTime;
drv.DeliverNotificationForTest(handle, SpeedRef, Good(668, t0.AddSeconds(2)));
SetupReadAt(session, 668, t0.AddSeconds(2));
await RecoverInPlaceAsync(drv);
var seen = new List<DataChangeEventArgs>();
drv.OnDataChange += (_, e) => seen.Add(e);
drv.DeliverNotificationForTest(handle, SpeedRef, Good(667, t0.AddSeconds(1))); // late replay
drv.DeliverNotificationForTest(handle, SpeedRef,
new DataValue(new Variant(668), StatusCodes.UncertainLastUsableValue, t0.AddSeconds(2), t0.AddSeconds(2))); // same ts, status-only
drv.DeliverNotificationForTest(handle, SpeedRef, Good(669, t0.AddSeconds(3)));
drv.DeliverNotificationForTest(handle, SpeedRef, Good(665, t0)); // disarmed: accepted
seen.Select(e => e.Snapshot.Value).ShouldBe(new object[] { 668, 669, 665 });
seen[0].Snapshot.StatusCode.ShouldBe((uint)StatusCodes.UncertainLastUsableValue);
}
/// <summary>
/// Coordinator review of F2: with no recovery in play, an upstream clock that steps BACKWARDS (NTP correction,
/// a PLC with a reset clock) must not freeze the node — every notification is published, whatever its time.
/// </summary>
[Fact]
public void In_steady_state_a_clock_step_backwards_still_publishes()
{
var (drv, _, clock) = NewConnectedDriver();
using var _ = drv;
@@ -511,14 +558,35 @@ public sealed class OpcUaClientSessionLossTests
var seen = new List<DataChangeEventArgs>();
drv.OnDataChange += (_, e) => seen.Add(e);
drv.DeliverNotificationForTest(handle, SpeedRef, Good(668, t0.AddSeconds(2)));
drv.DeliverNotificationForTest(handle, SpeedRef, Good(667, t0.AddSeconds(1))); // late replay
drv.DeliverNotificationForTest(handle, SpeedRef,
new DataValue(new Variant(668), StatusCodes.UncertainLastUsableValue, t0.AddSeconds(2), t0.AddSeconds(2))); // same ts, status-only
drv.DeliverNotificationForTest(handle, SpeedRef, Good(669, t0.AddSeconds(3)));
drv.DeliverNotificationForTest(handle, SpeedRef, Good(1, t0.AddHours(1)));
drv.DeliverNotificationForTest(handle, SpeedRef, Good(2, t0)); // clock stepped back an hour
drv.DeliverNotificationForTest(handle, SpeedRef, Good(3, t0.AddSeconds(1)));
seen.Select(e => e.Snapshot.Value).ShouldBe(new object[] { 668, 668, 669 });
seen[1].Snapshot.StatusCode.ShouldBe((uint)StatusCodes.UncertainLastUsableValue);
seen.Select(e => e.Snapshot.Value).ShouldBe(new object[] { 1, 2, 3 });
}
/// <summary>The replay guard is bounded in time: once its window has passed, an older notification is accepted
/// even for a ref that never saw an in-order one.</summary>
[Fact]
public async Task The_replay_guard_expires()
{
var (drv, session, clock) = NewConnectedDriver();
using var _ = drv;
var handle = drv.AddSubscriptionForTest([SpeedRef]);
var t0 = clock.Now.UtcDateTime;
drv.DeliverNotificationForTest(handle, SpeedRef, Good(10, t0.AddSeconds(5)));
SetupReadAt(session, 10, t0.AddSeconds(5));
await RecoverInPlaceAsync(drv);
var seen = new List<DataChangeEventArgs>();
drv.OnDataChange += (_, e) => seen.Add(e);
clock.Advance(TimeSpan.FromSeconds(29));
drv.DeliverNotificationForTest(handle, SpeedRef, Good(9, t0));
seen.ShouldBeEmpty("still inside the 30 s window");
clock.Advance(TimeSpan.FromSeconds(2));
drv.DeliverNotificationForTest(handle, SpeedRef, Good(8, t0));
seen.ShouldHaveSingleItem().Snapshot.Value.ShouldBe(8);
}
[Fact]