From 9818d0cba8e6087e65b18b2f308534eb604362b9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 7 Jun 2026 05:19:47 -0400 Subject: [PATCH] fix(opcua): structural equality for EquipmentVirtualTagPlan so no-op redeploys diff empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IReadOnlyList DependencyRefs compared by reference in the auto-generated record equality, causing every VirtualTag with dependencies to be flagged "Changed" on every parse (fresh list instances from composer and artifact-decoder). Add Equals/GetHashCode overrides with element-wise ordinal comparison so Phase7Plan.IsEmpty short-circuits a no-op redeploy. Add regression test Identical_virtualtag_snapshots_diff_to_empty_plan (separate list instances, same contents → IsEmpty true). Add TODO comment in Phase7Applier near needsRebuild predicate. --- .../Phase7Applier.cs | 1 + .../Phase7Composer.cs | 26 ++++++++++++- .../Phase7PlannerTests.cs | 38 +++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Applier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Applier.cs index 4f3ca2e6..f66b90f3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Applier.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Applier.cs @@ -81,6 +81,7 @@ public sealed class Phase7Applier // VirtualTag topology requires a real address-space rebuild. Driver-instance changes don't // touch the address-space topology directly — they go through DriverHostActor's spawn-plan // in Runtime. + // TODO(equipment-virtualtags): when MaterialiseEquipmentVirtualTags drives per-delta sink work, revisit whether ChangedEquipmentVirtualTags should also force needsRebuild. var needsRebuild = plan.AddedEquipment.Count > 0 || plan.RemovedEquipment.Count > 0 || plan.AddedAlarms.Count > 0 || plan.RemovedAlarms.Count > 0 || diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Composer.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Composer.cs index 3517608c..fe9cfb1c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Composer.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Composer.cs @@ -122,7 +122,31 @@ public sealed record EquipmentVirtualTagPlan( string Name, string DataType, string Expression, - IReadOnlyList DependencyRefs); + IReadOnlyList DependencyRefs) +{ + /// Structural equality: the auto-generated record equality would compare + /// (an interface-typed list) BY REFERENCE, flagging every + /// VirtualTag as "changed" on every parse (fresh list instances). Compare it element-wise + /// so a no-op redeploy diffs empty. + public bool Equals(EquipmentVirtualTagPlan? other) => + other is not null && + VirtualTagId == other.VirtualTagId && + EquipmentId == other.EquipmentId && + FolderPath == other.FolderPath && + Name == other.Name && + DataType == other.DataType && + Expression == other.Expression && + DependencyRefs.SequenceEqual(other.DependencyRefs, StringComparer.Ordinal); + + public override int GetHashCode() + { + var hash = new HashCode(); + hash.Add(VirtualTagId); hash.Add(EquipmentId); hash.Add(FolderPath); + hash.Add(Name); hash.Add(DataType); hash.Add(Expression); + foreach (var r in DependencyRefs) hash.Add(r, StringComparer.Ordinal); + return hash.ToHashCode(); + } +} /// /// Pure composer that flattens the live-edit DB tables into the address-space build plan a diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7PlannerTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7PlannerTests.cs index 8b399b0b..886a7de1 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7PlannerTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7PlannerTests.cs @@ -139,6 +139,44 @@ public sealed class Phase7PlannerTests plan.RemovedEquipmentVirtualTags.ShouldBeEmpty(); } + /// Regression guard for structural equality on : + /// two snapshots containing the SAME VirtualTag built from SEPARATE list instances must diff to an empty plan + /// (IReadOnlyList equality is BY REFERENCE without the custom Equals override, so every VirtualTag with + /// dependencies would be wrongly flagged "Changed" on every parse, preventing IsEmpty short-circuits). + [Fact] + public void Identical_virtualtag_snapshots_diff_to_empty_plan() + { + // Two separate list instances with identical contents — proves structural (not reference) equality. + var refsA = new[] { "EQ1.Speed", "EQ1.Torque" }; + var refsB = new[] { "EQ1.Speed", "EQ1.Torque" }; + + var prev = new Phase7CompositionResult( + Array.Empty(), Array.Empty(), Array.Empty()) + { + EquipmentVirtualTags = new[] + { + new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "Efficiency", DataType: "Float", + Expression: "ctx.GetTag(\"EQ1.Speed\") / ctx.GetTag(\"EQ1.Torque\")", DependencyRefs: refsA), + }, + }; + var next = new Phase7CompositionResult( + Array.Empty(), Array.Empty(), Array.Empty()) + { + EquipmentVirtualTags = new[] + { + new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "Efficiency", DataType: "Float", + Expression: "ctx.GetTag(\"EQ1.Speed\") / ctx.GetTag(\"EQ1.Torque\")", DependencyRefs: refsB), + }, + }; + + var plan = Phase7Planner.Compute(prev, next); + + plan.IsEmpty.ShouldBeTrue(); + plan.ChangedEquipmentVirtualTags.ShouldBeEmpty(); + plan.AddedEquipmentVirtualTags.ShouldBeEmpty(); + plan.RemovedEquipmentVirtualTags.ShouldBeEmpty(); + } + /// Verifies that new equipment goes to the AddedEquipment list. [Fact] public void New_equipment_goes_to_AddedEquipment()