From e7d5ebe956df2be24259d2d9aa36219dc3456170 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 26 Jun 2026 12:57:08 -0400 Subject: [PATCH] fix(otopcua): cancel pending rediscover timer on TriggerRediscovery + test hardening (follow-up C) --- .../Drivers/DriverInstanceActor.cs | 17 ++++-- .../DriverInstanceActorDiscoveryTests.cs | 60 ++++++++++++------- 2 files changed, 53 insertions(+), 24 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs index b257c82d..37fc5ea3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs @@ -408,10 +408,19 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers PublishHealthSnapshot(); }); ReceiveAsync(HandleRediscoverAsync); - // The host asks for a fresh discovery pass after rebinding the driver to a new equipment. Re-kick the - // bounded loop via StartDiscovery (honours RediscoverPolicy + the ITagDiscovery guard, tagged with the - // current _initGeneration). Only handled here in Connected — non-Connected states no-op it below. - Receive(_ => StartDiscovery()); + // The host asks for a fresh discovery pass after rebinding the driver to a new equipment. Cancel any + // pending rediscover tick FIRST — mirroring ForceReconnect/DisconnectObserved — so a stale tick left + // over from the prior loop can't fire alongside the freshly-kicked one, then re-kick the bounded loop + // via StartDiscovery (honours RediscoverPolicy + the ITagDiscovery guard, tagged with the current + // _initGeneration). Only handled here in Connected — non-Connected states no-op it below. A stale tick + // that still slips through (one already mid-async-handler) is benign: the parent dedups + // DiscoveredNodesReady and node injection is idempotent — the Cancel just avoids the avoidable double + // pass in the common case. + Receive(_ => + { + Timers.Cancel("rediscover"); + StartDiscovery(); + }); ReceiveAsync(HandleWriteAsync); ReceiveAsync(HandleAcknowledgeAsync); ReceiveAsync(HandleSubscribeAsync); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorDiscoveryTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorDiscoveryTests.cs index e106bc92..7973c920 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorDiscoveryTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorDiscoveryTests.cs @@ -302,28 +302,31 @@ public sealed class DriverInstanceActorDiscoveryTests : RuntimeActorTestBase [Fact] public void TriggerRediscovery_when_Connected_reruns_discovery() { - var driver = new DiscoverableStubDriver(); + // Once-policy growing stub: exactly ONE pass per (re)kick, so each StartDiscovery publishes precisely + // one DiscoveredNodesReady — the trigger's effect is asserted with a single ExpectMsg + ExpectNoMsg + // (no second settling pass to drain, and no stale-tick double pass alongside the fresh one). + var driver = new GrowingDiscoverableStubDriver(DiscoveryRediscoverPolicy.Once); var parent = CreateTestProbe(); var actor = parent.ChildActorOf(DriverInstanceActor.Props( driver, rediscoverInterval: TimeSpan.FromMilliseconds(20))); actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); - // Let the initial post-connect loop settle (passes 0,0,3,3) and confirm it stopped. - for (var i = 0; i < 4; i++) - parent.ExpectMsg(TimeSpan.FromSeconds(2)); + // Initial connect: Once ⇒ exactly one pass (growing set → 1 node), then it settles. + parent.ExpectMsg(TimeSpan.FromSeconds(2)).Nodes.Count.ShouldBe(1); parent.ExpectNoMsg(TimeSpan.FromMilliseconds(200)); - var passesBeforeTrigger = driver.DiscoverCount; // 4 + var passesBeforeTrigger = driver.DiscoverCount; // 1 - // Re-kick discovery via the new message — the cache is warm, so the fresh pass sees the 3-node set. + // Re-kick discovery via the new message — Once ⇒ exactly one fresh pass (growing set → 2 nodes). actor.Tell(new DriverInstanceActor.TriggerRediscovery()); var afterTrigger = parent.ExpectMsg(TimeSpan.FromSeconds(2)); - afterTrigger.Nodes.Count.ShouldBe(3); + afterTrigger.Nodes.Count.ShouldBe(2); afterTrigger.DriverInstanceId.ShouldBe(driver.DriverInstanceId); - // A fresh pass genuinely ran — DiscoverCount advanced past the settled count. - driver.DiscoverCount.ShouldBeGreaterThan(passesBeforeTrigger); + // Exactly one fresh pass ran — DiscoverCount advanced by one and no extra pass arrived. + parent.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + driver.DiscoverCount.ShouldBe(passesBeforeTrigger + 1); } /// @@ -351,32 +354,49 @@ public sealed class DriverInstanceActorDiscoveryTests : RuntimeActorTestBase } /// - /// received while NOT Connected (still Connecting, - /// before init completes) is a clean silent no-op: no discovery pass runs, nothing is published, and - /// the actor neither crashes nor dies (the driver's eventual reconnect re-discovers anyway). A - /// follow-up connect then discovers normally, proving the actor is unharmed. + /// received while NOT Connected is a clean silent + /// no-op in EVERY non-Connected state: no discovery pass runs, nothing is published, and the actor + /// neither crashes nor dies (its eventual (re)connect re-discovers anyway). Covers both Connecting + /// (before init completes) and Reconnecting (after a , + /// parked there by a long reconnect interval), with an intervening connect proving the actor is unharmed. /// [Fact] public void TriggerRediscovery_when_not_Connected_is_a_silent_noop() { - var driver = new DiscoverableStubDriver(); + // Once-growing stub so a successful connect publishes exactly one pass (clean confirmation of state); + // a long reconnect interval so the actor parks in Reconnecting deterministically within the test window. + var driver = new GrowingDiscoverableStubDriver(DiscoveryRediscoverPolicy.Once); var parent = CreateTestProbe(); var actor = parent.ChildActorOf(DriverInstanceActor.Props( - driver, rediscoverInterval: TimeSpan.FromMilliseconds(20))); + driver, + reconnectInterval: TimeSpan.FromSeconds(30), + rediscoverInterval: TimeSpan.FromMilliseconds(20))); Watch(actor); - // The actor boots into Connecting; send the trigger BEFORE InitializeRequested so it is handled - // in a non-Connected state. + // (1) Connecting: the actor boots into Connecting; send the trigger BEFORE InitializeRequested so it + // is handled in that non-Connected state. actor.Tell(new DriverInstanceActor.TriggerRediscovery()); // No discovery resulted, and the actor is unharmed (no Terminated arrives at the watching test actor). - parent.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + parent.ExpectNoMsg(TimeSpan.FromMilliseconds(200)); ExpectNoMsg(TimeSpan.FromMilliseconds(100)); driver.DiscoverCount.ShouldBe(0); - // Sanity: the actor still works — driving it to Connected discovers normally afterwards. + // Drive to Connected (proves the Connecting-state trigger left the actor working); Once ⇒ one pass. actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); - parent.ExpectMsg(TimeSpan.FromSeconds(2)); + parent.ExpectMsg(TimeSpan.FromSeconds(2)).Nodes.Count.ShouldBe(1); + parent.ExpectNoMsg(TimeSpan.FromMilliseconds(200)); + var passesAfterConnect = driver.DiscoverCount; // 1 + + // (2) Reconnecting: ForceReconnect parks the actor in Reconnecting (30s retry interval ⇒ no auto + // reconnect within the window). A TriggerRediscovery here must ALSO be a clean silent no-op. Both + // messages are processed in order, so the trigger is handled while Reconnecting. + actor.Tell(new DriverInstanceActor.ForceReconnect()); + actor.Tell(new DriverInstanceActor.TriggerRediscovery()); + + parent.ExpectNoMsg(TimeSpan.FromMilliseconds(300)); + ExpectNoMsg(TimeSpan.FromMilliseconds(100)); // still alive — no Terminated + driver.DiscoverCount.ShouldBe(passesAfterConnect); // no fresh pass while Reconnecting } ///