feat(otopcua): re-trigger discovery on config-unchanged rebind (follow-up C)

2026-06-26 14:06:50 -04:00
parent adcd7b57c1
commit 533671487e
2 changed files with 162 additions and 8 deletions
@@ -1313,34 +1313,43 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers
            var candidates = fromNodes.Concat(fromTags).ToHashSet(StringComparer.Ordinal);

            var plansByEquipment = _discoveredByDriver[driverId];
+            // Track whether ANY entry was dropped (no-longer-candidate or rebind) so we can re-trigger this
+            // driver's discovery exactly ONCE after the inner map is processed (see the post-loop block).
+            var droppedAny = false;
            foreach (var equipmentId in plansByEquipment.Keys.ToList())   // snapshot — we mutate the inner dict
            {
                var plan = plansByEquipment[equipmentId];
                if (!candidates.Contains(equipmentId))
                {
                    plansByEquipment.Remove(equipmentId);
+                    droppedAny = true;
                    _log.Debug("DriverHost {Node}: dropped cached discovered nodes for {Driver}/{Equipment} — equipment no longer resolves", _localNode, driverId, equipmentId);
                    continue;
                }
                // If the equipment was rebound (the cached plan's NodeIds are scoped to the OLD equipment), drop +
                // let re-discovery rebuild against the new equipment. The plan's NodeIds are "{equipmentId}/...".
-                // KNOWN LIMITATION (follow-up, alongside the multi-device-per-driver limitation): a
-                // CONFIG-UNCHANGED rebind (the driver's DriverConfig is identical, only its authored tag's
-                // EquipmentId moved) drops the cached plan here but does NOT itself re-trigger discovery —
-                // ReconcileDrivers only restarts a child on a DriverConfig change, so a config-unchanged child is
-                // never stopped/reconnected. The FixedTree subtree therefore stays ABSENT under the new equipment
-                // until the driver's next reconnect/restart re-discovers it. We deliberately do NOT add re-trigger
-                // logic here (it would couple the subscription pass to driver-lifecycle control); the drop is the
-                // safe, correct fail-state (a stale EQ-1-scoped graft under EQ-2 would be worse).
                var planEquipmentConsistent = plan.Variables.Count > 0
                    && plan.Variables[0].NodeId.StartsWith(equipmentId + "/", StringComparison.Ordinal);
                if (!planEquipmentConsistent)
                {
                    plansByEquipment.Remove(equipmentId);
+                    droppedAny = true;
                    _log.Debug("DriverHost {Node}: dropped cached discovered nodes for {Driver}/{Equipment} — equipment rebound", _localNode, driverId, equipmentId);
                }
            }

+            // Re-trigger discovery when ANY entry was dropped (no-longer-candidate or rebind). A CONFIG-UNCHANGED
+            // rebind (the driver's DriverConfig is identical, only its authored tag's EquipmentId moved) is NOT
+            // restarted by ReconcileDrivers — the child stays Connected — so without this nudge the FixedTree
+            // subtree would stay ABSENT under the new equipment until the driver's next natural reconnect. We now
+            // ask the child to re-run discovery so it re-grafts promptly: the next pass resolves against the new
+            // _lastComposition (the now-bound equipment). This is a DISCOVERY action, not lifecycle control — no
+            // stop/restart; it is idempotent, and the child no-ops it if not Connected (handled in
+            // DriverInstanceActor). Sent at most ONCE per driver per re-inject pass (here, after the inner map is
+            // processed — so even when the inner map empties below), guarded on the child still existing.
+            if (droppedAny && _children.TryGetValue(driverId, out var rediscoverEntry))
+                rediscoverEntry.Actor.Tell(new DriverInstanceActor.TriggerRediscovery());
+
            if (plansByEquipment.Count == 0)
            {
                _discoveredByDriver.Remove(driverId);
@@ -460,6 +460,68 @@ public sealed class DriverHostActorDiscoveryTests : RuntimeActorTestBase
        publish.ExpectNoMsg(TimeSpan.FromMilliseconds(500));
    }

+    /// <summary>Follow-up C, part 2: a CONFIG-UNCHANGED rebind must RE-TRIGGER discovery on the driver's child
+    /// so the dropped FixedTree re-grafts under the NEW equipment on the next pass (rather than staying absent
+    /// until the driver's next natural reconnect). Wires a REAL <see cref="ITagDiscovery"/> child (policy
+    /// <see cref="DiscoveryRediscoverPolicy.Once"/>, ever-growing set) so the connect-time pass populates the
+    /// per-equipment cache under EQ-1, then redeploys to rebind d1 → EQ-2 with the SAME (unchanged) DriverConfig
+    /// (so <c>ReconcileDrivers</c> does NOT restart the child — exactly the config-unchanged rebind). The
+    /// re-inject tail drops the stale EQ-1 entry and must send <see cref="DriverInstanceActor.TriggerRediscovery"/>
+    /// to d1's child. The trigger reaching the child is observed at the host level (the only faithful seam — there
+    /// is no probe-as-driver-child seam): the child re-runs discovery (its <c>DiscoverCount</c> advances past the
+    /// single Once pass — impossible without the trigger, since the child stays Connected so nothing else re-kicks
+    /// discovery) AND a fresh <see cref="OpcUaPublishActor.MaterialiseDiscoveredNodes"/> re-grafts the FixedTree
+    /// under the new equipment EQ-2.</summary>
+    [Fact]
+    public void Config_unchanged_rebind_re_triggers_discovery_on_the_child()
+    {
+        var db = NewInMemoryDbFactory();
+        // A REAL ITagDiscovery child (Once policy) — so the connect-time pass populates the cache and a
+        // TriggerRediscovery re-runs discovery, making the trigger observable at the host level.
+        var factory = new DiscoverableSubscribingDriverFactory("Modbus");
+        var deploymentId = SeedDeploymentWithEquipmentTags(db, RevA,
+            (Equip: "EQ-1", Driver: "d1", FullName: "40001", Folder: (string?)null, Name: "speed"));
+
+        var coordinator = CreateTestProbe();
+        var publish = CreateTestProbe();
+        var vtHost = CreateTestProbe();
+
+        var actor = Sys.ActorOf(DriverHostActor.Props(
+            db, TestNode, coordinator.Ref,
+            driverFactory: factory,
+            localRoles: new HashSet<string> { "driver" },
+            opcUaPublishActor: publish.Ref,
+            virtualTagHostOverride: vtHost.Ref));
+
+        actor.Tell(new DispatchDeployment(deploymentId, RevA, CorrelationId.NewId()));
+        coordinator.ExpectMsg<ApplyAck>(Timeout).Outcome.ShouldBe(ApplyAckOutcome.Applied);
+        publish.ExpectMsg<OpcUaPublishActor.RebuildAddressSpace>(Timeout);
+
+        // The child connects and runs its single (Once) post-connect discovery pass, which the host grafts
+        // under EQ-1 — this POPULATES the per-equipment cache (_discoveredByDriver[d1] = { EQ-1: plan }).
+        publish.FishForMessage<OpcUaPublishActor.MaterialiseDiscoveredNodes>(
+            m => m.EquipmentRootNodeId == "EQ-1", Timeout);
+        AwaitAssert(() => factory.DiscoverCount.ShouldBe(1), duration: Timeout);
+
+        // Redeploy: REBIND d1 from EQ-1 → EQ-2 (same FullName; DriverConfig "{}" unchanged so ReconcileDrivers
+        // does NOT restart the child). The re-inject tail drops the stale EQ-1-scoped cache entry.
+        var deploymentId2 = SeedDeploymentWithEquipmentTags(db, RevB,
+            (Equip: "EQ-2", Driver: "d1", FullName: "40001", Folder: (string?)null, Name: "speed"));
+        actor.Tell(new DispatchDeployment(deploymentId2, RevB, CorrelationId.NewId()));
+        coordinator.ExpectMsg<ApplyAck>(Timeout).Outcome.ShouldBe(ApplyAckOutcome.Applied);
+
+        // The drop must SEND DriverInstanceActor.TriggerRediscovery to d1's (still-Connected) child, which
+        // re-runs discovery: DiscoverCount advances past the single connect pass — the observable proof the
+        // trigger reached the child. (Pre-task: no trigger ⇒ Once already settled, child never reconnects ⇒
+        // DiscoverCount stays 1 ⇒ this fails.)
+        AwaitAssert(() => factory.DiscoverCount.ShouldBeGreaterThan(1), duration: TimeSpan.FromSeconds(10));
+
+        // ... and the re-triggered pass re-grafts the FixedTree under the NEW equipment EQ-2 (the host resolves
+        // d1 → EQ-2 against the new _lastComposition).
+        publish.FishForMessage<OpcUaPublishActor.MaterialiseDiscoveredNodes>(
+            m => m.EquipmentRootNodeId == "EQ-2", Timeout);
+    }
+
    /// <summary>Spawns the host with the subscribing driver factory + a publish probe, dispatches the
    /// deployment, and waits for the Applied ACK so the apply (and thus <c>_lastComposition</c> + the live
    /// child + the initial SubscribeBulk pass) has completed before the test injects discovered nodes. A
@@ -628,4 +690,87 @@ public sealed class DriverHostActorDiscoveryTests : RuntimeActorTestBase
        /// <inheritdoc />
        public IReadOnlyCollection<string> SupportedTypes => new[] { _supportedType };
    }
+
+    /// <summary>Factory producing a single shared <see cref="DiscoverableSubscribingDriver"/> for the supported
+    /// type — a real (non-stubbed) <see cref="DriverInstanceActor"/> child that exposes <see cref="ITagDiscovery"/>,
+    /// so the host's post-connect discovery loop populates the discovered-node cache AND a
+    /// <see cref="DriverInstanceActor.TriggerRediscovery"/> re-runs discovery (the seam the rebind re-trigger
+    /// test asserts through). Exposes the driver's pass count so a test can observe the trigger landing.</summary>
+    private sealed class DiscoverableSubscribingDriverFactory : IDriverFactory
+    {
+        private readonly string _supportedType;
+        private DiscoverableSubscribingDriver? _driver;
+        public DiscoverableSubscribingDriverFactory(string supportedType) { _supportedType = supportedType; }
+
+        /// <summary>Number of <c>DiscoverAsync</c> passes the child has driven (advances on every
+        /// connect-time pass and every <see cref="DriverInstanceActor.TriggerRediscovery"/>-driven pass).</summary>
+        public int DiscoverCount => _driver?.DiscoverCount ?? 0;
+
+        /// <inheritdoc />
+        public IDriver? TryCreate(string driverType, string driverInstanceId, string driverConfigJson) =>
+            string.Equals(driverType, _supportedType, StringComparison.Ordinal)
+                ? _driver ??= new DiscoverableSubscribingDriver(driverInstanceId)
+                : null;
+
+        /// <inheritdoc />
+        public IReadOnlyCollection<string> SupportedTypes => new[] { _supportedType };
+    }
+
+    /// <summary>A <see cref="StubDriver"/> that is BOTH <see cref="ISubscribable"/> (so the host's subscribe
+    /// path is exercised) and <see cref="ITagDiscovery"/> with policy <see cref="DiscoveryRediscoverPolicy.Once"/>
+    /// — exactly one post-connect pass, re-runnable by <see cref="DriverInstanceActor.TriggerRediscovery"/>. Each
+    /// pass streams an ever-growing FixedTree (pass N → N nodes, refs "ft-ref-1".."ft-ref-N", none shadowing the
+    /// authored "40001"), so a re-triggered pass yields a grown set the host re-applies — and the public
+    /// <see cref="DiscoverCount"/> makes the trigger observable at the host level. Re-implements
+    /// <see cref="IDriver.DriverInstanceId"/> (the base <see cref="StubDriver"/> hardcodes "stub-driver-1") so the
+    /// spawned child reports the SPEC's id — otherwise its auto-sent <see cref="DriverInstanceActor.DiscoveredNodesReady"/>
+    /// would carry the wrong driver id and resolve no equipment.</summary>
+    private sealed class DiscoverableSubscribingDriver : StubDriver, IDriver, ISubscribable, ITagDiscovery
+    {
+        private int _passCount;
+        private readonly string _driverInstanceId;
+        private readonly StubHandle _handle = new();
+
+        public DiscoverableSubscribingDriver(string driverInstanceId) => _driverInstanceId = driverInstanceId;
+
+        /// <summary>The spec's driver instance id (re-mapped from the base "stub-driver-1").</summary>
+        public new string DriverInstanceId => _driverInstanceId;
+
+        /// <summary>Single post-connect pass per (re)kick — re-runnable by TriggerRediscovery.</summary>
+        public DiscoveryRediscoverPolicy RediscoverPolicy => DiscoveryRediscoverPolicy.Once;
+
+        /// <summary>Number of <c>DiscoverAsync</c> passes driven so far.</summary>
+        public int DiscoverCount => Volatile.Read(ref _passCount);
+
+        /// <summary>Never raised (the test asserts on discovery + materialise, not data changes); explicit
+        /// empty accessors satisfy the interface without a never-used backing field (no CS0067).</summary>
+        public event EventHandler<DataChangeEventArgs>? OnDataChange { add { } remove { } }
+
+        /// <inheritdoc />
+        public Task<ISubscriptionHandle> SubscribeAsync(
+            IReadOnlyList<string> fullReferences, TimeSpan publishingInterval, CancellationToken cancellationToken)
+            => Task.FromResult<ISubscriptionHandle>(_handle);
+
+        /// <inheritdoc />
+        public Task UnsubscribeAsync(ISubscriptionHandle handle, CancellationToken cancellationToken)
+            => Task.CompletedTask;
+
+        /// <summary>Streams an ever-growing FixedTree (pass N → N nodes).</summary>
+        public Task DiscoverAsync(IAddressSpaceBuilder builder, CancellationToken cancellationToken)
+        {
+            var pass = Interlocked.Increment(ref _passCount);
+            var fixedTree = builder.Folder("FixedTree", "FixedTree");
+            for (var i = 0; i < pass; i++)
+            {
+                fixedTree.Variable($"v{i}", $"v{i}", new DriverAttributeInfo(
+                    FullName: $"ft-ref-{i + 1}",
+                    DriverDataType: DriverDataType.Float64,
+                    IsArray: false,
+                    ArrayDim: null,
+                    SecurityClass: SecurityClassification.ViewOnly,
+                    IsHistorized: false));
+            }
+            return Task.CompletedTask;
+        }
+    }
 }