From ada01e1af8da49a5379a796fa57ea30db32036a7 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 15 Jun 2026 10:05:29 -0400 Subject: [PATCH] =?UTF-8?q?fix(vtags):=20respawn=20equipment=20virtualtag?= =?UTF-8?q?=20child=20on=20in-place=20plan=20change=20(H1b,=20stillpending?= =?UTF-8?q?=20=C2=A71)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../VirtualTags/VirtualTagHostActor.cs | 39 ++++++-- .../VirtualTags/VirtualTagHostActorTests.cs | 90 +++++++++++++++++++ 2 files changed, 122 insertions(+), 7 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs index fb92f253..ce34b2cc 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs @@ -39,6 +39,9 @@ public sealed class VirtualTagHostActor : ReceiveActor private readonly Dictionary _children = new(StringComparer.Ordinal); // vtagId -> folder-scoped OPC UA NodeId the materialiser placed the variable at. private readonly Dictionary _nodeIdByVtag = new(StringComparer.Ordinal); + // vtagId -> the plan its currently-spawned child was built from, so we can detect an in-place + // change (edited Expression/DependencyRefs) and stop+respawn the child to adopt it. + private readonly Dictionary _planByVtag = new(StringComparer.Ordinal); /// Factory method to create Props for a VirtualTagHostActor. /// The OPC UA publish actor that consumes @@ -78,6 +81,22 @@ public sealed class VirtualTagHostActor : ReceiveActor _children.Remove(vtagId); } + // Stop + forget children whose plan changed in place (edited Expression/DependencyRefs, or a + // toggled flag). EquipmentVirtualTagPlan has element-wise value equality, so an identical + // redeploy diffs equal and is left untouched. Forgetting the child here lets the spawn-new + // loop below recreate it from the new plan — children are auto-named (no explicit name), so a + // stop+respawn cannot hit the "actor name not unique" collision. + foreach (var p in msg.Plans) + { + if (_children.ContainsKey(p.VirtualTagId) + && _planByVtag.TryGetValue(p.VirtualTagId, out var prev) + && !prev.Equals(p)) + { + Context.Stop(_children[p.VirtualTagId]); // PostStop unregisters the old mux interest. + _children.Remove(p.VirtualTagId); + } + } + // Rebuild the NodeId map every apply so renames (Name/FolderPath/EquipmentId changes) are // picked up. The map only contains currently-desired vtags, so a result for a removed vtag // finds no entry and is dropped. @@ -87,15 +106,13 @@ public sealed class VirtualTagHostActor : ReceiveActor _nodeIdByVtag[p.VirtualTagId] = NodeIdFor(p); } - // Spawn children for new vtagIds only — existing children keep their mux subscriptions and - // last-value dedup state. Expression/dependency changes on an existing vtag are NOT - // re-applied here; the loader's vtags are stable, and a future enhancement can stop+respawn - // a child whose plan changed (the diff already identifies ChangedEquipmentVirtualTags). + // Spawn children for vtagIds that have no live child. This covers genuinely-new vtags AND any + // vtag whose child was just forgotten above because its plan changed in place — that child is + // recreated here from the new plan, adopting the edited Expression/DependencyRefs. Children + // whose plan was unchanged still have a live entry and are skipped, keeping their mux + // subscriptions and last-value dedup state. foreach (var p in msg.Plans) { - // TODO(equipment-virtualtags): when a plan's Expression/DependencyRefs change in place - // (ChangedEquipmentVirtualTags), stop+respawn the child here; today only spawn-new/stop-removed - // is handled (loader vtags are stable). if (_children.ContainsKey(p.VirtualTagId)) continue; // Auto-name the child: vtagIds can contain characters illegal in actor names, so let Akka @@ -113,6 +130,14 @@ public sealed class VirtualTagHostActor : ReceiveActor _log.Debug("VirtualTagHost: spawned child for vtag {VirtualTagId}", p.VirtualTagId); } + // Refresh the plan map to exactly the desired set so the next apply can detect in-place + // changes against what each live child was actually built from. + _planByVtag.Clear(); + foreach (var p in msg.Plans) + { + _planByVtag[p.VirtualTagId] = p; + } + _log.Debug("VirtualTagHost: applied (desired={Desired}, children={Children})", desired.Count, _children.Count); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs index 75771c82..c7cd3642 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs @@ -153,6 +153,96 @@ public sealed class VirtualTagHostActorTests : RuntimeActorTestBase secondChild.ShouldNotBe(firstChild); } + /// A plan with an explicit Expression + DependencyRefs (the H1b in-place-change case). + private static EquipmentVirtualTagPlan PlanWithRefs( + string vtagId, string equipmentId, string name, string expression, params string[] refs) => + new( + VirtualTagId: vtagId, + EquipmentId: equipmentId, + FolderPath: "", + Name: name, + DataType: "Double", + Expression: expression, + DependencyRefs: refs); + + /// + /// H1b respawn-on-change: re-applying the SAME vtagId with a changed Expression/DependencyRefs + /// must stop the old child (PostStop ⇒ UnregisterInterest on the old refs) and spawn a fresh one + /// (PreStart ⇒ RegisterInterest on the new refs). Proof: after the second apply the mux probe sees + /// an UnregisterInterest and a RegisterInterest carrying the NEW ref "B". + /// + [Fact] + public void ApplyVirtualTags_respawns_child_when_plan_changes_in_place() + { + var publish = CreateTestProbe(); + var mux = CreateTestProbe(); + var host = Sys.ActorOf(VirtualTagHostActor.Props(publish.Ref, mux.Ref, new StubEvaluator())); + + // First apply — child self-registers interest in "A". + host.Tell(new VirtualTagHostActor.ApplyVirtualTags( + new[] { PlanWithRefs("vt-1", "eq-1", "speed-rpm", "ctx.GetTag(\"A\")", "A") })); + var reg1 = mux.ExpectMsg(); + reg1.TagRefs.ShouldContain("A"); + var firstChild = mux.LastSender; + + // Re-apply the SAME vtagId with a changed Expression + DependencyRefs. + host.Tell(new VirtualTagHostActor.ApplyVirtualTags( + new[] { PlanWithRefs("vt-1", "eq-1", "speed-rpm", "ctx.GetTag(\"B\")", "B") })); + + // The old child is stopped (PostStop ⇒ UnregisterInterest) and a new one spawned + // (PreStart ⇒ RegisterInterest on "B"). Both messages arrive at the mux probe; order between + // the dying child's PostStop and the new child's PreStart is not guaranteed, so accept either. + DependencyMuxActor.RegisterInterest? reg2 = null; + var sawUnregister = false; + for (var i = 0; i < 2; i++) + { + var msg = mux.ReceiveOne(TimeSpan.FromSeconds(5)); + switch (msg) + { + case DependencyMuxActor.RegisterInterest r: + reg2 = r; + break; + case DependencyMuxActor.UnregisterInterest: + sawUnregister = true; + break; + } + } + + sawUnregister.ShouldBeTrue("old child's PostStop should have unregistered its interest"); + reg2.ShouldNotBeNull(); + reg2!.TagRefs.ShouldContain("B"); + reg2.TagRefs.ShouldNotContain("A"); + + // The replacement is a different actor ref than the original (auto-named, so no collision). + mux.LastSender.ShouldNotBe(firstChild); + } + + /// + /// H1b no-churn: re-applying an IDENTICAL plan must NOT respawn the child — the plan's value + /// equality diffs empty, so no second Unregister/Register pair hits the mux. + /// + [Fact] + public void ApplyVirtualTags_does_not_respawn_child_when_plan_unchanged() + { + var publish = CreateTestProbe(); + var mux = CreateTestProbe(); + var host = Sys.ActorOf(VirtualTagHostActor.Props(publish.Ref, mux.Ref, new StubEvaluator())); + + var plan = new[] { PlanWithRefs("vt-1", "eq-1", "speed-rpm", "ctx.GetTag(\"A\")", "A") }; + + // First apply — exactly one RegisterInterest. + host.Tell(new VirtualTagHostActor.ApplyVirtualTags(plan)); + var reg = mux.ExpectMsg(); + reg.TagRefs.ShouldContain("A"); + + // Re-apply an identical plan (fresh list instances, but value-equal) — no churn expected. + host.Tell(new VirtualTagHostActor.ApplyVirtualTags( + new[] { PlanWithRefs("vt-1", "eq-1", "speed-rpm", "ctx.GetTag(\"A\")", "A") })); + + // No second Register and no Unregister: the child was left in place. + mux.ExpectNoMsg(TimeSpan.FromMilliseconds(400)); + } + /// Deterministic no-op evaluator: keeps spawned children inert so tests drive the host's /// OnResult path directly via synthetic EvaluationResults. private sealed class StubEvaluator : IVirtualTagEvaluator