perf(opcua): skip address-space rebuild for vtag expression/deps/historize-only edits (F10b)

src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Applier.cs

@@ -84,16 +84,23 @@ public sealed class Phase7Applier
         // Any add / remove / in-place CHANGE of Equipment, ScriptedAlarm, Equipment tag, or Equipment
         // VirtualTag topology requires a real address-space rebuild — the materialise passes re-derive
         // every node from the composition, so a changed-only deploy (e.g. a renamed equipment, a
-        // re-severitied alarm, a flipped tag dataType/Writable, or an edited VirtualTag expression) must
+        // re-severitied alarm, a flipped tag dataType/Writable) must still rebuild or the running server
-        // still rebuild or the running server keeps the stale node.
+        // keeps the stale node.
         // ChangedDrivers is deliberately EXCLUDED: a driver-instance config change doesn't touch the
         // address-space topology — it routes through DriverHostActor's spawn-plan in Runtime, which
         // re-spawns the affected driver actor without re-materialising any nodes.
+        // F10b: a CHANGED VirtualTag whose ONLY differences are Expression/DependencyRefs/Historize is
+        // node-IRRELEVANT (see VtagDeltaIsNodeIrrelevant) — its materialised node is byte-identical and
+        // the vtag engine adopts those edits via VirtualTagHostActor's INDEPENDENT respawn
+        // (DriverHostActor → ApplyVirtualTags), so it skips the rebuild and PRESERVES every client's
+        // server-wide subscriptions. Any structural / node-affecting vtag change (Name/FolderPath/
+        // DataType) — or any non-vtag change anywhere — still forces a full rebuild (safe default).
         var needsRebuild =
             plan.AddedEquipment.Count > 0 || plan.RemovedEquipment.Count > 0 || plan.ChangedEquipment.Count > 0 ||
             plan.AddedAlarms.Count > 0 || plan.RemovedAlarms.Count > 0 || plan.ChangedAlarms.Count > 0 ||
             plan.AddedEquipmentTags.Count > 0 || plan.RemovedEquipmentTags.Count > 0 || plan.ChangedEquipmentTags.Count > 0 ||
-            plan.AddedEquipmentVirtualTags.Count > 0 || plan.RemovedEquipmentVirtualTags.Count > 0 || plan.ChangedEquipmentVirtualTags.Count > 0;
+            plan.AddedEquipmentVirtualTags.Count > 0 || plan.RemovedEquipmentVirtualTags.Count > 0 ||
+            plan.ChangedEquipmentVirtualTags.Any(d => !VtagDeltaIsNodeIrrelevant(d));
 
         if (needsRebuild)
         {
@@ -319,6 +326,20 @@ public sealed class Phase7Applier
         catch (Exception ex) { _logger.LogWarning(ex, "Phase7Applier: EnsureVariable threw for {Node}", nodeId); }
     }
 
+    // A VirtualTag's materialised OPC UA node (MaterialiseEquipmentVirtualTags) is derived ONLY from
+    // {EquipmentId, FolderPath, Name, DataType}. Expression/DependencyRefs/Historize are engine/write-side
+    // only and are adopted by VirtualTagHostActor's INDEPENDENT respawn (DriverHostActor → ApplyVirtualTags),
+    // so a delta changing ONLY those three leaves a byte-identical node and needs no address-space rebuild.
+    // Whitelist-of-may-differ via `with` + the record's custom Equals: any OTHER field difference (current
+    // or future) makes the override unequal → falls back to a full rebuild (safe default).
+    private static bool VtagDeltaIsNodeIrrelevant(Phase7Plan.EquipmentVirtualTagDelta d) =>
+        (d.Previous with
+        {
+            Expression = d.Current.Expression,
+            DependencyRefs = d.Current.DependencyRefs,
+            Historize = d.Current.Historize,
+        }).Equals(d.Current);
+
     /// <summary>The "no-event" condition state written to a removed equipment / alarm node before the
     /// rebuild tears it down: inactive, acked, confirmed, enabled, unshelved, severity 0, empty message.
     /// Drives Retain to false so a removed condition stops replaying on ConditionRefresh.</summary>

tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7ApplierTests.cs

@@ -648,11 +648,15 @@ public sealed class Phase7ApplierTests
         sink.RebuildCalls.ShouldBe(1);
     }
 
-    /// <summary>H1a — a deploy that ONLY edits an existing VirtualTag's expression must rebuild the
+    /// <summary>F10b (backlog #11) — a deploy that ONLY edits an existing VirtualTag's <c>Expression</c>
-    /// address space. The planner diffs it into <c>ChangedEquipmentVirtualTags</c> alone; the applier
+    /// must SKIP the address-space rebuild. The vtag's materialised node is derived only from
-    /// must drive exactly one rebuild.</summary>
+    /// {EquipmentId, FolderPath, Name, DataType}, so an Expression-only edit leaves a byte-identical node;
+    /// the vtag engine adopts the new expression via <c>VirtualTagHostActor</c>'s independent respawn, not
+    /// the address-space path. Skipping the rebuild preserves every client's server-wide subscriptions.
+    /// The edit STILL counts as a change (ChangedNodes == 1) — it just no longer forces a rebuild.
+    /// (Supersedes the former H1a "expression edit ⇒ rebuild" behavior.)</summary>
     [Fact]
-    public void Changed_equipment_virtual_tags_only_trigger_rebuild()
+    public void Changed_virtual_tag_expression_only_skips_rebuild()
     {
         var sink = new RecordingSink();
         var applier = new Phase7Applier(sink, NullLogger<Phase7Applier>.Instance);
@@ -674,8 +678,186 @@ public sealed class Phase7ApplierTests
 
         var outcome = applier.Apply(plan);
 
-        outcome.RebuildCalled.ShouldBeTrue();
+        outcome.RebuildCalled.ShouldBeFalse();
+        sink.RebuildCalls.ShouldBe(0);   // NO RebuildAddressSpace — subscriptions preserved
+        outcome.ChangedNodes.ShouldBe(1); // the edit is still tallied as a change
+    }
+
+    /// <summary>F10b — a vtag delta differing ONLY in <c>DependencyRefs</c> (e.g. a new <c>ctx.GetTag</c>
+    /// literal in the script) is node-irrelevant and SKIPS the rebuild; the engine respawn picks up the
+    /// new dependency set.</summary>
+    [Fact]
+    public void Changed_virtual_tag_dependency_refs_only_skips_rebuild()
+    {
+        var sink = new RecordingSink();
+        var applier = new Phase7Applier(sink, NullLogger<Phase7Applier>.Instance);
+
+        var previous = CompositionWithVirtualTags(
+            new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "Efficiency", DataType: "Float64",
+                Expression: "ctx.GetTag(\"a\")", DependencyRefs: new[] { "a" }));
+        // Same node-relevant fields; only the dependency set differs.
+        var next = CompositionWithVirtualTags(
+            new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "Efficiency", DataType: "Float64",
+                Expression: "ctx.GetTag(\"a\")", DependencyRefs: new[] { "a", "b" }));
+
+        var plan = Phase7Planner.Compute(previous, next);
+        plan.ChangedEquipmentVirtualTags.Count.ShouldBe(1);
+
+        var outcome = applier.Apply(plan);
+
+        outcome.RebuildCalled.ShouldBeFalse();
+        sink.RebuildCalls.ShouldBe(0);
         outcome.ChangedNodes.ShouldBe(1);
+    }
+
+    /// <summary>F10b — a vtag delta differing ONLY in <c>Historize</c> (write-side flag, not materialised
+    /// on the node) is node-irrelevant and SKIPS the rebuild.</summary>
+    [Fact]
+    public void Changed_virtual_tag_historize_only_skips_rebuild()
+    {
+        var sink = new RecordingSink();
+        var applier = new Phase7Applier(sink, NullLogger<Phase7Applier>.Instance);
+
+        var previous = CompositionWithVirtualTags(
+            new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "Efficiency", DataType: "Float64",
+                Expression: "ctx.GetTag(\"a\")", DependencyRefs: new[] { "a" }, Historize: false));
+        // Same node-relevant fields; only the Historize flag flips.
+        var next = CompositionWithVirtualTags(
+            new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "Efficiency", DataType: "Float64",
+                Expression: "ctx.GetTag(\"a\")", DependencyRefs: new[] { "a" }, Historize: true));
+
+        var plan = Phase7Planner.Compute(previous, next);
+        plan.ChangedEquipmentVirtualTags.Count.ShouldBe(1);
+
+        var outcome = applier.Apply(plan);
+
+        outcome.RebuildCalled.ShouldBeFalse();
+        sink.RebuildCalls.ShouldBe(0);
+        outcome.ChangedNodes.ShouldBe(1);
+    }
+
+    /// <summary>F10b safe-default — a vtag delta where the node-affecting <c>DataType</c> ALSO changed is
+    /// NOT node-irrelevant: the materialised node would differ, so the applier must still rebuild. Pins
+    /// the whitelist (Expression/DependencyRefs/Historize) against accidentally swallowing a DataType
+    /// edit.</summary>
+    [Fact]
+    public void Changed_virtual_tag_data_type_change_still_rebuilds()
+    {
+        var sink = new RecordingSink();
+        var applier = new Phase7Applier(sink, NullLogger<Phase7Applier>.Instance);
+
+        var previous = CompositionWithVirtualTags(
+            new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "Efficiency", DataType: "Float64",
+                Expression: "ctx.GetTag(\"a\") * 120", DependencyRefs: new[] { "a" }));
+        // DataType flips AND the expression changes — DataType is node-affecting, so this must rebuild.
+        var next = CompositionWithVirtualTags(
+            new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "Efficiency", DataType: "Int32",
+                Expression: "ctx.GetTag(\"a\") * 60", DependencyRefs: new[] { "a" }));
+
+        var plan = Phase7Planner.Compute(previous, next);
+        plan.ChangedEquipmentVirtualTags.Count.ShouldBe(1);
+
+        var outcome = applier.Apply(plan);
+
+        outcome.RebuildCalled.ShouldBeTrue();
+        sink.RebuildCalls.ShouldBe(1);
+    }
+
+    /// <summary>F10b safe-default — a vtag delta where the node-affecting <c>Name</c> changed must
+    /// rebuild (the node's BrowseName/NodeId derive from Name).</summary>
+    [Fact]
+    public void Changed_virtual_tag_name_change_still_rebuilds()
+    {
+        var sink = new RecordingSink();
+        var applier = new Phase7Applier(sink, NullLogger<Phase7Applier>.Instance);
+
+        var previous = CompositionWithVirtualTags(
+            new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "Efficiency", DataType: "Float64",
+                Expression: "ctx.GetTag(\"a\")", DependencyRefs: new[] { "a" }));
+        var next = CompositionWithVirtualTags(
+            new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "EfficiencyPct", DataType: "Float64",
+                Expression: "ctx.GetTag(\"a\")", DependencyRefs: new[] { "a" }));
+
+        var plan = Phase7Planner.Compute(previous, next);
+        plan.ChangedEquipmentVirtualTags.Count.ShouldBe(1);
+
+        var outcome = applier.Apply(plan);
+
+        outcome.RebuildCalled.ShouldBeTrue();
+        sink.RebuildCalls.ShouldBe(1);
+    }
+
+    /// <summary>F10b safe-default — a vtag delta where the node-affecting <c>FolderPath</c> changed must
+    /// rebuild (the folder-scoped NodeId derives from FolderPath).</summary>
+    [Fact]
+    public void Changed_virtual_tag_folder_path_change_still_rebuilds()
+    {
+        var sink = new RecordingSink();
+        var applier = new Phase7Applier(sink, NullLogger<Phase7Applier>.Instance);
+
+        var previous = CompositionWithVirtualTags(
+            new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "Efficiency", DataType: "Float64",
+                Expression: "ctx.GetTag(\"a\")", DependencyRefs: new[] { "a" }));
+        var next = CompositionWithVirtualTags(
+            new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "Calc", Name: "Efficiency", DataType: "Float64",
+                Expression: "ctx.GetTag(\"a\")", DependencyRefs: new[] { "a" }));
+
+        var plan = Phase7Planner.Compute(previous, next);
+        plan.ChangedEquipmentVirtualTags.Count.ShouldBe(1);
+
+        var outcome = applier.Apply(plan);
+
+        outcome.RebuildCalled.ShouldBeTrue();
+        sink.RebuildCalls.ShouldBe(1);
+    }
+
+    /// <summary>F10b — the skip is ONLY for a node-irrelevant vtag edit that is the SOLE change. A
+    /// node-irrelevant Expression-only vtag edit MIXED with any other change (here a changed equipment
+    /// tag) must still rebuild — the rebuild is forced by the OTHER change, and the running server gets
+    /// its single rebuild as before.</summary>
+    [Fact]
+    public void Node_irrelevant_vtag_edit_mixed_with_another_change_still_rebuilds()
+    {
+        var sink = new RecordingSink();
+        var applier = new Phase7Applier(sink, NullLogger<Phase7Applier>.Instance);
+
+        var previous = new Phase7CompositionResult(
+            Array.Empty<EquipmentNode>(), Array.Empty<DriverInstancePlan>(), Array.Empty<ScriptedAlarmPlan>())
+        {
+            EquipmentTags = new[]
+            {
+                new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001", Writable: false, Alarm: null),
+            },
+            EquipmentVirtualTags = new[]
+            {
+                new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "Efficiency", DataType: "Float64",
+                    Expression: "ctx.GetTag(\"a\") * 60", DependencyRefs: new[] { "a" }),
+            },
+        };
+        // Expression-only vtag edit (node-irrelevant) AND a node-affecting tag DataType flip.
+        var next = new Phase7CompositionResult(
+            Array.Empty<EquipmentNode>(), Array.Empty<DriverInstancePlan>(), Array.Empty<ScriptedAlarmPlan>())
+        {
+            EquipmentTags = new[]
+            {
+                new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Int32", FullName: "40001", Writable: false, Alarm: null),
+            },
+            EquipmentVirtualTags = new[]
+            {
+                new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "Efficiency", DataType: "Float64",
+                    Expression: "ctx.GetTag(\"a\") * 120", DependencyRefs: new[] { "a" }),
+            },
+        };
+
+        var plan = Phase7Planner.Compute(previous, next);
+
+        // Both a node-irrelevant vtag change AND a node-affecting tag change are present.
+        plan.ChangedEquipmentVirtualTags.Count.ShouldBe(1);
+        plan.ChangedEquipmentTags.Count.ShouldBe(1);
+
+        var outcome = applier.Apply(plan);
+
+        outcome.RebuildCalled.ShouldBeTrue();
         sink.RebuildCalls.ShouldBe(1);
     }