diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs index 52082009..c554b629 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs @@ -8,8 +8,9 @@ public enum AddressSpaceChangeKind /// — nothing to do. Empty, - /// Only surgical-eligible changed tags, node-irrelevant vtag changes, folder renames, and/or - /// node-inert driver deltas — the existing F10b in-place path. No adds, no removes. + /// Only surgical-eligible changed equipment/raw tags, surgical UNS-reference deltas, + /// node-irrelevant vtag changes, folder renames, and/or node-inert driver deltas — the existing F10b + /// in-place path. No adds, no removes. AttributeOnly, /// Only additions (any mix of Equipment/Alarms/Tags/VirtualTags) on top of at-most-AttributeOnly @@ -25,9 +26,9 @@ public enum AddressSpaceChangeKind /// surgically; until Phase 3 it maps to a full rebuild. AddRemoveMix, - /// Anything with a node-affecting change (ChangedEquipment, ChangedAlarms, non-surgical - /// ChangedEquipmentTags, node-relevant ChangedEquipmentVirtualTags) — the unclassifiable-safe default. - /// Full rebuild. + /// Anything with a node-affecting change (ChangedEquipment, ChangedAlarms, ChangedRawContainers, + /// non-surgical ChangedEquipmentTags, alarm-bearing/identity-changing ChangedRawTags, node-relevant + /// ChangedEquipmentVirtualTags) — the unclassifiable-safe default. Full rebuild. Rebuild, } @@ -52,25 +53,34 @@ public static class AddressSpaceChangeClassifier if (plan.IsEmpty) return AddressSpaceChangeKind.Empty; // 2 — any node-affecting CHANGE forces a full rebuild (default-closed). ChangedEquipment / - // ChangedAlarms are always node-affecting; a changed tag is node-affecting UNLESS it is - // surgical-eligible, and a changed vtag is node-affecting UNLESS it is node-irrelevant. The v3 - // Batch-4 Raw/UNS Changed sets (container re-shape, raw-tag attribute/alarm edit, UNS re-point / - // display-name-override) route to Rebuild as the safe default — WP3's applier owns any surgical - // in-place refinement for them. Evaluated BEFORE the add/remove split so a non-surgical change - // alongside pure adds still rebuilds. + // ChangedAlarms / ChangedRawContainers are always node-affecting; a changed equipment tag is + // node-affecting UNLESS it is surgical-eligible, a changed raw tag UNLESS it is + // RawTagDeltaIsSurgicalEligible, and a changed vtag UNLESS it is node-irrelevant. Evaluated BEFORE the + // add/remove split so a non-surgical change alongside pure adds still rebuilds. // - // WP4 BINDING GUARD (Wave C review M3): ChangedRawTags → Rebuild is what keeps native-alarm notifier - // wiring correct today. A native alarm's set of referencing equipment lives in - // RawTagPlan.ReferencingEquipmentPaths (part of its record equality), so ADDING/DROPPING a reference - // makes the raw tag a ChangedRawTag → full rebuild → OtOpcUaNodeManager clears + re-wires the notifiers - // cleanly. ANY future surgical (non-rebuild) ChangedRawTags path MUST also re-wire/unwire the alarm - // notifiers (WireAlarmNotifiers is a reconcile — pass the tag's COMPLETE current referencing set — plus - // an explicit unwire on removal), else a de-referenced equipment keeps receiving the alarm's events. + // WP4 BINDING GUARD (Wave C review M3), as narrowed by the 2026-08-15 surgical-deploys work. The + // guard's subject is NATIVE-ALARM NOTIFIER WIRING: a native alarm's set of referencing equipment lives + // in RawTagPlan.ReferencingEquipmentPaths (part of its record equality), and only a rebuild clears + + // re-wires those notifiers, so an ALARM-BEARING raw tag (Alarm != null on EITHER side) still routes to + // Rebuild. What changed is the blast radius: ReferencingEquipmentPaths is node-INERT for a plain VALUE + // tag, yet adding/dropping ONE UnsTagReference flips its backing raw tag to Changed — so the blanket + // "ChangedRawTags ⇒ Rebuild" tore down every node on the server (killing every client subscription) for + // an edit that creates a single UNS variable. The new contract: + // • alarm-bearing raw-tag delta ⇒ Rebuild (notifier wiring is owned by the rebuild — unchanged); + // • non-alarm raw-tag delta over the in-place-updatable attributes and/or ReferencingEquipmentPaths + // ⇒ surgical (the applier's ISurgicalAddressSpaceSink.UpdateTagAttributes path); + // • any other raw-tag field difference ⇒ Rebuild, via RawTagDeltaIsSurgicalEligible's + // with-equality default-closed whitelist (which also closes over any FUTURE field); + // • ChangedUnsReferenceVariables ⇒ surgical remove-previous + recreate-current, applier-owned (a UNS + // reference variable is a mirror node binding no driver, so it carries no notifier wiring of its + // own — the alarm fan-out it participates in is re-derived from the raw tag's referencing set). + // Anything a future surgical raw-tag path lets through that DOES touch alarm notifiers must still + // re-wire/unwire them (WireAlarmNotifiers is a reconcile — pass the tag's COMPLETE current referencing + // set — plus an explicit unwire on removal), else a de-referenced equipment keeps receiving the events. if (plan.ChangedEquipment.Count > 0 || plan.ChangedAlarms.Count > 0 || plan.ChangedRawContainers.Count > 0 || - plan.ChangedRawTags.Count > 0 || - plan.ChangedUnsReferenceVariables.Count > 0 || + plan.ChangedRawTags.Any(d => !RawTagDeltaIsSurgicalEligible(d)) || plan.ChangedEquipmentTags.Any(d => !TagDeltaIsSurgicalEligible(d)) || plan.ChangedEquipmentVirtualTags.Any(d => !VtagDeltaIsNodeIrrelevant(d))) { @@ -141,4 +151,23 @@ public static class AddressSpaceChangeClassifier IsArray = d.Current.IsArray, ArrayLength = d.Current.ArrayLength, }).Equals(d.Current); + + // v3 surgical raw-tag eligibility: a NON-alarm raw tag whose only differences are the in-place-updatable + // attributes (Writable / IsHistorized / HistorianTagname / DataType / IsArray / ArrayLength) and/or its + // ReferencingEquipmentPaths. The referencing set is node-inert for a VALUE tag — it only drives native-alarm + // notifier wiring (the WP4 guard), which is why Alarm != null on EITHER side disqualifies. Identity fields + // (NodeId is the diff key; TagId / DriverInstanceId / Name / ParentNodeId / Realm) still force a rebuild via + // the with-equality default-closed pattern. + internal static bool RawTagDeltaIsSurgicalEligible(AddressSpacePlan.RawTagDelta d) => + d.Previous.Alarm is null && d.Current.Alarm is null && + (d.Previous with + { + Writable = d.Current.Writable, + IsHistorized = d.Current.IsHistorized, + HistorianTagname = d.Current.HistorianTagname, + DataType = d.Current.DataType, + IsArray = d.Current.IsArray, + ArrayLength = d.Current.ArrayLength, + ReferencingEquipmentPaths = d.Current.ReferencingEquipmentPaths, + }).Equals(d.Current); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 45248cde..5d7faab8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -1249,11 +1249,14 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// Wave C review M3 — this is a RECONCILE, not additive-only: it wires the supplied set AND unwires any /// previously-tracked folder whose id is no longer in /// (bidirectional), so a de-referenced equipment stops receiving the alarm. Today the applier always - /// passes the COMPLETE referencing-equipment set for the condition, and a de-reference arrives as a - /// ChangedRawTags full rebuild (which clears the wiring), so the unwire leg is a no-op on the - /// current paths — but it makes this method correct for a FUTURE surgical ChangedRawTags path - /// (see AddressSpaceChangeClassifier's ChangedRawTags → Rebuild branch, which carries the - /// matching binding guard). + /// passes the COMPLETE referencing-equipment set for the condition, and a de-reference on an + /// ALARM-BEARING raw tag still arrives as a full rebuild (which clears the wiring), so the unwire leg is + /// a no-op on the current paths — but it makes this method correct for a FUTURE surgical path that + /// reaches an alarm-bearing tag. The 2026-08-15 surgical-deploys work narrowed the classifier's blanket + /// ChangedRawTags → Rebuild branch to alarm-bearing / identity-changing raw-tag deltas + /// only (see AddressSpaceChangeClassifier.RawTagDeltaIsSurgicalEligible, which carries the + /// matching binding guard): a NON-alarm raw tag carries no notifier wiring, so its + /// ReferencingEquipmentPaths edit is node-inert and routes surgically. /// public void WireAlarmNotifiers(string alarmNodeId, AddressSpaceRealm alarmRealm, IReadOnlyList notifierFolderNodeIds, AddressSpaceRealm notifierFolderRealm) diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs index 28f758df..779c572b 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierDualNamespaceTests.cs @@ -6,8 +6,11 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; /// /// v3 Batch 4 (B4-WP1) — the classifier folds the Raw + UNS subtree diff sets into its routing policy: -/// raw/uns adds count as adds (PureAdd), removes as removes (PureRemove), and any Changed raw/uns delta -/// routes to Rebuild (the safe default — WP3's applier owns surgical refinement). +/// raw/uns adds count as adds (PureAdd), removes as removes (PureRemove). +/// Surgical deploys (2026-08-15 Task 4) narrowed the Changed sets: a NON-alarm raw-tag delta whose +/// only differences are the in-place-updatable attributes and/or its ReferencingEquipmentPaths is +/// surgical (NOT a rebuild), and every UNS-reference delta is applied remove+recreate. Alarm-bearing and +/// identity-field raw-tag deltas still route to Rebuild — the default-closed WP4 notifier guard. /// public sealed class AddressSpaceChangeClassifierDualNamespaceTests { @@ -71,8 +74,12 @@ public sealed class AddressSpaceChangeClassifierDualNamespaceTests AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AddRemoveMix); } + /// The headline surgical case: adding/dropping a UnsTagReference flips the BACKING raw tag + /// to Changed (ReferencingEquipmentPaths is part of RawTagPlan's equality). For a non-alarm VALUE tag that + /// set is node-inert — it only drives native-alarm notifier wiring — so it must NOT tear down the world. + /// (Was Changed_raw_tag_is_Rebuild; deliberately re-pointed by 2026-08-15 Task 4.) [Fact] - public void Changed_raw_tag_is_Rebuild() + public void Reference_only_raw_tag_delta_is_not_a_rebuild() { var plan = Empty() with { @@ -80,14 +87,88 @@ public sealed class AddressSpaceChangeClassifierDualNamespaceTests { new AddressSpacePlan.RawTagDelta( RawTag("Plant/Modbus1/PLC-A/Speed"), - RawTag("Plant/Modbus1/PLC-A/Speed") with { IsHistorized = true }), + RawTag("Plant/Modbus1/PLC-A/Speed") with + { + ReferencingEquipmentPaths = new[] { "filling/line-1/station-1" }, + }), }, }; + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + /// The in-place-updatable attribute whitelist (Writable / IsHistorized / HistorianTagname / + /// DataType / IsArray / ArrayLength) on a non-alarm raw tag is surgical, not a rebuild. + [Theory] + [InlineData("writable")] + [InlineData("historized")] + [InlineData("historianTagname")] + [InlineData("dataType")] + [InlineData("array")] + public void Attribute_raw_tag_delta_is_not_a_rebuild(string field) + { + var prev = RawTag("Plant/Modbus1/PLC-A/Speed"); + var cur = field switch + { + "writable" => prev with { Writable = true }, + "historized" => prev with { IsHistorized = true }, + "historianTagname" => prev with { IsHistorized = true, HistorianTagname = "Plant.PLC_A.Speed" }, + "dataType" => prev with { DataType = "Double" }, + "array" => prev with { IsArray = true, ArrayLength = 8 }, + _ => throw new ArgumentOutOfRangeException(nameof(field)), + }; + + var plan = Empty() with { ChangedRawTags = new[] { new AddressSpacePlan.RawTagDelta(prev, cur) } }; + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + /// WP4 binding guard: a raw tag carrying a native alarm on EITHER side still rebuilds — the + /// rebuild is what clears + re-wires the per-equipment alarm notifiers. + [Theory] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(true, true)] + public void Alarm_bearing_raw_tag_delta_still_rebuilds(bool alarmBefore, bool alarmAfter) + { + var alarm = new EquipmentTagAlarmInfo("OffNormalAlarm", 700); + var prev = RawTag("Plant/Modbus1/PLC-A/Speed") with { Alarm = alarmBefore ? alarm : null }; + var cur = RawTag("Plant/Modbus1/PLC-A/Speed") with + { + Alarm = alarmAfter ? alarm : null, + ReferencingEquipmentPaths = new[] { "filling/line-1/station-1" }, + }; + + var plan = Empty() with { ChangedRawTags = new[] { new AddressSpacePlan.RawTagDelta(prev, cur) } }; AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); } + /// Default-closed: an identity field outside the whitelist (DriverInstanceId re-routes the node to + /// another driver point; Name/TagId/ParentNodeId re-shape identity) still forces a full rebuild. + [Theory] + [InlineData("driver")] + [InlineData("name")] + [InlineData("tagId")] + [InlineData("parent")] + public void Identity_field_raw_tag_delta_still_rebuilds(string field) + { + var prev = RawTag("Plant/Modbus1/PLC-A/Speed"); + var cur = field switch + { + "driver" => prev with { DriverInstanceId = "drv-2" }, + "name" => prev with { Name = "Velocity" }, + "tagId" => prev with { TagId = "t-2" }, + "parent" => prev with { ParentNodeId = "Plant/Modbus1/PLC-B" }, + _ => throw new ArgumentOutOfRangeException(nameof(field)), + }; + + var plan = Empty() with { ChangedRawTags = new[] { new AddressSpacePlan.RawTagDelta(prev, cur) } }; + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + /// A UNS-reference re-point (BackingRawPath moved) is applied surgically (remove-previous + + /// recreate-current), NOT as a rebuild. (Was Uns_reference_repoint_is_Rebuild; deliberately + /// re-pointed by 2026-08-15 Task 4.) [Fact] - public void Uns_reference_repoint_is_Rebuild() + public void Uns_reference_repoint_delta_is_not_a_rebuild() { var plan = Empty() with { @@ -98,6 +179,40 @@ public sealed class AddressSpaceChangeClassifierDualNamespaceTests UnsRef("filling/line-1/station-1/Speed") with { BackingRawPath = "Plant/Modbus1/PLC-A/Velocity" }), }, }; - AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + /// The other UnsReferenceDelta variant — a display-name override edit keeps the row id while the + /// NodeId + EffectiveName change. Also surgical (remove-previous + recreate-current). + [Fact] + public void Uns_reference_display_name_delta_is_not_a_rebuild() + { + var plan = Empty() with + { + ChangedUnsReferenceVariables = new[] + { + new AddressSpacePlan.UnsReferenceDelta( + UnsRef("filling/line-1/station-1/Speed"), + UnsRef("filling/line-1/station-1/LineSpeed") with { EffectiveName = "LineSpeed" }), + }, + }; + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + /// The headline end-to-end shape: adding ONE UNS reference produces an added UNS variable PLUS the + /// induced reference-only delta on the backing raw tag. That must classify PureAdd — one node created, + /// zero teardown, every client subscription server-wide survives. + [Fact] + public void Uns_reference_add_with_induced_reference_only_raw_delta_is_pure_add() + { + var prev = RawTag("Plant/Modbus1/PLC-A/Speed"); + var cur = prev with { ReferencingEquipmentPaths = new[] { "filling/line-1/station-1" } }; + + var plan = Empty() with + { + AddedUnsReferenceVariables = new[] { UnsRef("filling/line-1/station-1/Speed") }, + ChangedRawTags = new[] { new AddressSpacePlan.RawTagDelta(prev, cur) }, + }; + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureAdd); } }