diff --git a/archreview/plans/R2-07-surgical-pure-adds-plan.md.tasks.json b/archreview/plans/R2-07-surgical-pure-adds-plan.md.tasks.json index 27f9622c..56c629d6 100644 --- a/archreview/plans/R2-07-surgical-pure-adds-plan.md.tasks.json +++ b/archreview/plans/R2-07-surgical-pure-adds-plan.md.tasks.json @@ -5,92 +5,123 @@ { "id": "T1", "subject": "Phase 1: AddressSpaceChangeClassifier + table-driven tests (move TagDeltaIsSurgicalEligible/VtagDeltaIsNodeIrrelevant)", - "status": "pending", + "status": "completed", "blockedBy": [] }, { "id": "T2", - "subject": "Phase 1: route AddressSpaceApplier.Apply through the classifier — PureAdd skips rebuild (high-risk)", + "subject": "Phase 1: route AddressSpaceApplier.Apply through the classifier \u2014 PureAdd skips rebuild (high-risk)", "status": "pending", - "blockedBy": ["T1"] + "blockedBy": [ + "T1" + ] }, { "id": "T3", "subject": "Phase 1: ComputeAddAnnouncements + AnnounceAddedNodes on the applier (dedup parent NodeAdded announces)", "status": "pending", - "blockedBy": ["T1"] + "blockedBy": [ + "T1" + ] }, { "id": "T4a", "subject": "Phase 1: idempotent MaterialiseAlarmCondition (skip-if-present-and-same-kind) (high-risk)", "status": "pending", - "blockedBy": ["T1"] + "blockedBy": [ + "T1" + ] }, { "id": "T4b", "subject": "Phase 1: OpcUaPublishActor calls AnnounceAddedNodes after the Materialise passes on non-rebuild applies (high-risk)", "status": "pending", - "blockedBy": ["T2", "T3", "T4a"] + "blockedBy": [ + "T2", + "T3", + "T4a" + ] }, { "id": "T5", - "subject": "Phase 1: over-the-wire SubscriptionSurvivalTests — subscribe, pure-add, item survives + new node browsable", + "subject": "Phase 1: over-the-wire SubscriptionSurvivalTests \u2014 subscribe, pure-add, item survives + new node browsable", "status": "pending", - "blockedBy": ["T4b"] + "blockedBy": [ + "T4b" + ] }, { "id": "T6", - "subject": "Phase 1: MANDATORY live-/run gate on docker-dev (rebuild BOTH centrals; Client.CLI subscribe survives +1-tag deploy; kind=PureAdd rebuild=False) — Phase 1 shippable boundary", + "subject": "Phase 1: MANDATORY live-/run gate on docker-dev (rebuild BOTH centrals; Client.CLI subscribe survives +1-tag deploy; kind=PureAdd rebuild=False) \u2014 Phase 1 shippable boundary", "status": "pending", - "blockedBy": ["T5"] + "blockedBy": [ + "T5" + ] }, { "id": "T7", - "subject": "Phase 2: add RemoveVariableNode/RemoveAlarmConditionNode/RemoveEquipmentSubtree to ISurgicalAddressSpaceSink + DeferredAddressSpaceSink forwarding — guard-first with observed negative control (high-risk)", + "subject": "Phase 2: add RemoveVariableNode/RemoveAlarmConditionNode/RemoveEquipmentSubtree to ISurgicalAddressSpaceSink + DeferredAddressSpaceSink forwarding \u2014 guard-first with observed negative control (high-risk)", "status": "pending", - "blockedBy": ["T6"] + "blockedBy": [ + "T6" + ] }, { "id": "T8", "subject": "Phase 2: OtOpcUaNodeManager.RemoveVariableNode + BuildNodesRemovedModelChange (NodeDeleted) (high-risk)", "status": "pending", - "blockedBy": ["T7"] + "blockedBy": [ + "T7" + ] }, { "id": "T9", "subject": "Phase 2: OtOpcUaNodeManager.RemoveAlarmConditionNode (+ native-flag cleanup) (high-risk)", "status": "pending", - "blockedBy": ["T7"] + "blockedBy": [ + "T7" + ] }, { "id": "T10", "subject": "Phase 2: OtOpcUaNodeManager.RemoveEquipmentSubtree (descendants + notifier demotion) + SdkAddressSpaceSink forwards (high-risk)", "status": "pending", - "blockedBy": ["T8", "T9"] + "blockedBy": [ + "T8", + "T9" + ] }, { "id": "T11", - "subject": "Phase 2: applier PureRemove arm — terminal Bad/RemovedConditionState writes, equipment subsumption, false-to-rebuild ratchet (high-risk)", + "subject": "Phase 2: applier PureRemove arm \u2014 terminal Bad/RemovedConditionState writes, equipment subsumption, false-to-rebuild ratchet (high-risk)", "status": "pending", - "blockedBy": ["T10"] + "blockedBy": [ + "T10" + ] }, { "id": "T12", - "subject": "Phase 2: remove-side subscription-survival integration test + live-/run remove gate (kind=PureRemove rebuild=False) — Phase 2 shippable boundary", + "subject": "Phase 2: remove-side subscription-survival integration test + live-/run remove gate (kind=PureRemove rebuild=False) \u2014 Phase 2 shippable boundary", "status": "pending", - "blockedBy": ["T11"] + "blockedBy": [ + "T11" + ] }, { "id": "T13", - "subject": "Phase 3: AddRemoveMix routing — removes-then-adds surgically, same-NodeId remove-then-recreate ordering test (high-risk)", + "subject": "Phase 3: AddRemoveMix routing \u2014 removes-then-adds surgically, same-NodeId remove-then-recreate ordering test (high-risk)", "status": "pending", - "blockedBy": ["T12"] + "blockedBy": [ + "T12" + ] }, { "id": "T14", - "subject": "Phase 3: mixed-deploy integration test + live-/run mixed gate + STATUS.md/P1 close-out — plan complete", + "subject": "Phase 3: mixed-deploy integration test + live-/run mixed gate + STATUS.md/P1 close-out \u2014 plan complete", "status": "pending", - "blockedBy": ["T13"] + "blockedBy": [ + "T13" + ] } ] } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs index 5d63a6c0..6a8345b8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs @@ -138,11 +138,11 @@ public sealed class AddressSpaceApplier 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.Any(d => !TagDeltaIsSurgicalEligible(d)) || + plan.ChangedEquipmentTags.Any(d => !AddressSpaceChangeClassifier.TagDeltaIsSurgicalEligible(d)) || plan.AddedEquipmentVirtualTags.Count > 0 || plan.RemovedEquipmentVirtualTags.Count > 0 || - plan.ChangedEquipmentVirtualTags.Any(d => !VtagDeltaIsNodeIrrelevant(d)); + plan.ChangedEquipmentVirtualTags.Any(d => !AddressSpaceChangeClassifier.VtagDeltaIsNodeIrrelevant(d)); - var surgicalTagDeltas = plan.ChangedEquipmentTags.Where(TagDeltaIsSurgicalEligible).ToList(); + var surgicalTagDeltas = plan.ChangedEquipmentTags.Where(AddressSpaceChangeClassifier.TagDeltaIsSurgicalEligible).ToList(); // UNS Area / Line renames are surgically applicable (in-place DisplayName swap) // when no structural rebuild fires. When a rebuild DOES fire (any add/remove/structural change), // MaterialiseHierarchy re-creates every folder with the new display names, so the renames are @@ -663,46 +663,10 @@ public sealed class AddressSpaceApplier catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: EnsureVariable threw for {Node}", nodeId); return false; } } - // 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(AddressSpacePlan.EquipmentVirtualTagDelta d) => - (d.Previous with - { - Expression = d.Current.Expression, - DependencyRefs = d.Current.DependencyRefs, - Historize = d.Current.Historize, - }).Equals(d.Current); - - // F10b: a CHANGED equipment tag whose ONLY differences are Writable / IsHistorized / - // HistorianTagname / DataType / IsArray / ArrayLength (a plain value variable — no alarm condition node) - // can be updated IN PLACE on the existing node via ISurgicalAddressSpaceSink.UpdateTagAttributes, - // avoiding a full rebuild (preserving subscriptions). The presentation-shape fields (DataType / IsArray / - // ArrayLength) join the whitelist now that the surgical sink swaps DataType + ValueRank + ArrayDimensions - // in place (and raises a GeneralModelChangeEvent). FullName / DriverInstanceId / Name / identity / alarm - // differences still fall through to a rebuild — FullName/DriverInstanceId re-route the node to a different - // driver point, Name re-derives the NodeId, and an alarm flip turns the node into a Part 9 condition. The - // override-unequal default also covers any future field. - // REACH (live-verified): the shape path only fires for drivers whose TagConfig carries a stable - // top-level "FullName" (Galaxy = tag_name.AttributeName; OpcUaClient = the node id) — there a DataType/array - // edit leaves FullName untouched ⇒ surgical. For structured-TagConfig protocol drivers (Modbus/S7/AbCip/…) - // TagConfigIntent.Parse falls back to the RAW TagConfig blob as FullName, so a DataType/ - // array edit also mutates that blob ⇒ FullName differs ⇒ this returns false ⇒ full rebuild. That is the - // correct safe default (a protocol driver's subscription needs re-spawning for a new shape anyway). - private static bool TagDeltaIsSurgicalEligible(AddressSpacePlan.EquipmentTagDelta 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, - }).Equals(d.Current); + // The tag/vtag surgical-eligibility predicates moved to AddressSpaceChangeClassifier (R2-07 T1) so the + // whole delta classification is one pure, table-testable unit; the applier references them there (and the + // classifier's Classify names the routing). The `with`-expression whitelist technique is preserved there + // byte-for-byte. /// 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. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs new file mode 100644 index 00000000..695ef436 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs @@ -0,0 +1,126 @@ +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer; + +/// How must mutate the address space for a given +/// . A pure policy classification OVER the planner's diff (the planner stays +/// a pure diff; the kind is the applier's routing policy over that diff — see R2-07 03/P1). +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. + AttributeOnly, + + /// Only additions (any mix of Equipment/Alarms/Tags/VirtualTags) on top of at-most-AttributeOnly + /// changes — no removals, no node-affecting changes. Phase 1: the idempotent Materialise passes create + /// exactly the added nodes with no teardown, so every client subscription survives. + PureAdd, + + /// Only removals on top of at-most-AttributeOnly changes — no additions, no node-affecting + /// changes. Phase 2: scoped in-place teardown of only the removed subtree. + PureRemove, + + /// Additions AND removals, still no node-affecting changes. Phase 3 applies removes-then-adds + /// 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. + Rebuild, +} + +/// +/// Pure static classifier over an — names the delta class the applier +/// routes each plan through, so a pure-add deploy no longer tears down and recreates every node on the +/// server (03/P1). First-match-wins per the plan's classification table; the default-closed +/// catches every node-affecting change (including any future +/// plan field that makes a changed record unequal), so the worst outcome of any misclassification is +/// today's full-rebuild behaviour. +/// +public static class AddressSpaceChangeClassifier +{ + /// Classify into the delta class the applier routes it through. + /// The plan to classify. + /// The for the plan. + public static AddressSpaceChangeKind Classify(AddressSpacePlan plan) + { + ArgumentNullException.ThrowIfNull(plan); + + // 1 — nothing to do. + 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. Evaluated + // BEFORE the add/remove split so a non-surgical change alongside pure adds still rebuilds. + if (plan.ChangedEquipment.Count > 0 || + plan.ChangedAlarms.Count > 0 || + plan.ChangedEquipmentTags.Any(d => !TagDeltaIsSurgicalEligible(d)) || + plan.ChangedEquipmentVirtualTags.Any(d => !VtagDeltaIsNodeIrrelevant(d))) + { + return AddressSpaceChangeKind.Rebuild; + } + + // Driver deltas (Added/Removed/ChangedDrivers) are node-INERT — they never touch the address-space + // topology (they route to DriverHostActor's spawn plan in Runtime), so they are excluded from the + // add/remove split below and leave a driver-only plan classified AttributeOnly. + var hasAdds = + plan.AddedEquipment.Count + plan.AddedAlarms.Count + + plan.AddedEquipmentTags.Count + plan.AddedEquipmentVirtualTags.Count > 0; + var hasRemoves = + plan.RemovedEquipment.Count + plan.RemovedAlarms.Count + + plan.RemovedEquipmentTags.Count + plan.RemovedEquipmentVirtualTags.Count > 0; + + // 3 — additions AND removals. + if (hasAdds && hasRemoves) return AddressSpaceChangeKind.AddRemoveMix; + // 4 — additions only. + if (hasAdds) return AddressSpaceChangeKind.PureAdd; + // 5 — removals only. + if (hasRemoves) return AddressSpaceChangeKind.PureRemove; + // 6 — only surgical tag deltas / node-irrelevant vtag deltas / folder renames / driver deltas. + return AddressSpaceChangeKind.AttributeOnly; + } + + // 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). + internal static bool VtagDeltaIsNodeIrrelevant(AddressSpacePlan.EquipmentVirtualTagDelta d) => + (d.Previous with + { + Expression = d.Current.Expression, + DependencyRefs = d.Current.DependencyRefs, + Historize = d.Current.Historize, + }).Equals(d.Current); + + // F10b: a CHANGED equipment tag whose ONLY differences are Writable / IsHistorized / + // HistorianTagname / DataType / IsArray / ArrayLength (a plain value variable — no alarm condition node) + // can be updated IN PLACE on the existing node via ISurgicalAddressSpaceSink.UpdateTagAttributes, + // avoiding a full rebuild (preserving subscriptions). The presentation-shape fields (DataType / IsArray / + // ArrayLength) join the whitelist now that the surgical sink swaps DataType + ValueRank + ArrayDimensions + // in place (and raises a GeneralModelChangeEvent). FullName / DriverInstanceId / Name / identity / alarm + // differences still fall through to a rebuild — FullName/DriverInstanceId re-route the node to a different + // driver point, Name re-derives the NodeId, and an alarm flip turns the node into a Part 9 condition. The + // override-unequal default also covers any future field. + // REACH (live-verified): the shape path only fires for drivers whose TagConfig carries a stable + // top-level "FullName" (Galaxy = tag_name.AttributeName; OpcUaClient = the node id) — there a DataType/array + // edit leaves FullName untouched ⇒ surgical. For structured-TagConfig protocol drivers (Modbus/S7/AbCip/…) + // TagConfigIntent.Parse falls back to the RAW TagConfig blob as FullName, so a DataType/ + // array edit also mutates that blob ⇒ FullName differs ⇒ this returns false ⇒ full rebuild. That is the + // correct safe default (a protocol driver's subscription needs re-spawning for a new shape anyway). + internal static bool TagDeltaIsSurgicalEligible(AddressSpacePlan.EquipmentTagDelta 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, + }).Equals(d.Current); +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierTests.cs new file mode 100644 index 00000000..24507eb6 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierTests.cs @@ -0,0 +1,285 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; + +/// +/// R2-07 T1 — table-driven tests over . The +/// classifier is a pure function over an : it names the delta class the +/// applier routes each plan through (Empty / AttributeOnly / PureAdd / PureRemove / AddRemoveMix / +/// Rebuild). First-match-wins per the plan's classification table, with the default-closed +/// Rebuild safety valve catching every node-affecting change. +/// +public sealed class AddressSpaceChangeClassifierTests +{ + [Fact] + public void Empty_plan_classifies_as_Empty() + { + AddressSpaceChangeClassifier.Classify(Plan()).ShouldBe(AddressSpaceChangeKind.Empty); + } + + // ----- single add class ⇒ PureAdd ----- + + [Fact] + public void Added_equipment_only_is_PureAdd() + { + var plan = Plan(addedEquipment: new[] { Eq("eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureAdd); + } + + [Fact] + public void Added_alarm_only_is_PureAdd() + { + var plan = Plan(addedAlarms: new[] { Alarm("alm-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureAdd); + } + + [Fact] + public void Added_equipment_tag_only_is_PureAdd() + { + var plan = Plan(addedTags: new[] { Tag("tag-1", "eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureAdd); + } + + [Fact] + public void Added_virtual_tag_only_is_PureAdd() + { + var plan = Plan(addedVtags: new[] { Vtag("vt-1", "eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureAdd); + } + + // ----- single remove class ⇒ PureRemove ----- + + [Fact] + public void Removed_equipment_only_is_PureRemove() + { + var plan = Plan(removedEquipment: new[] { Eq("eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureRemove); + } + + [Fact] + public void Removed_alarm_only_is_PureRemove() + { + var plan = Plan(removedAlarms: new[] { Alarm("alm-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureRemove); + } + + [Fact] + public void Removed_equipment_tag_only_is_PureRemove() + { + var plan = Plan(removedTags: new[] { Tag("tag-1", "eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureRemove); + } + + [Fact] + public void Removed_virtual_tag_only_is_PureRemove() + { + var plan = Plan(removedVtags: new[] { Vtag("vt-1", "eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureRemove); + } + + // ----- add AND remove ⇒ AddRemoveMix ----- + + [Fact] + public void Add_and_remove_is_AddRemoveMix() + { + var plan = Plan(addedTags: new[] { Tag("tag-1", "eq-1") }, removedTags: new[] { Tag("tag-2", "eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AddRemoveMix); + } + + [Fact] + public void Add_equipment_and_remove_vtag_is_AddRemoveMix() + { + var plan = Plan(addedEquipment: new[] { Eq("eq-2") }, removedVtags: new[] { Vtag("vt-1", "eq-1") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AddRemoveMix); + } + + // ----- node-affecting change ⇒ Rebuild ----- + + [Fact] + public void Changed_equipment_is_Rebuild() + { + var plan = Plan(changedEquipment: new[] + { + new AddressSpacePlan.EquipmentDelta(Eq("eq-1"), Eq("eq-1") with { DisplayName = "New" }), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + [Fact] + public void Changed_alarm_is_Rebuild() + { + var plan = Plan(changedAlarms: new[] + { + new AddressSpacePlan.AlarmDelta(Alarm("alm-1"), Alarm("alm-1") with { MessageTemplate = "changed" }), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + [Fact] + public void Non_surgical_changed_tag_is_Rebuild() + { + // FullName re-routes the node to a different driver point ⇒ NOT surgical-eligible ⇒ Rebuild. + var plan = Plan(changedTags: new[] + { + new AddressSpacePlan.EquipmentTagDelta( + Tag("tag-1", "eq-1", fullName: "40001"), + Tag("tag-1", "eq-1", fullName: "40002")), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + [Fact] + public void Node_relevant_changed_vtag_is_Rebuild() + { + // Name change re-derives the NodeId ⇒ node-relevant ⇒ Rebuild. + var plan = Plan(changedVtags: new[] + { + new AddressSpacePlan.EquipmentVirtualTagDelta( + Vtag("vt-1", "eq-1", name: "A"), + Vtag("vt-1", "eq-1", name: "B")), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + // ----- surgical-only / rename-only / driver-only ⇒ AttributeOnly ----- + + [Fact] + public void Surgical_eligible_changed_tag_only_is_AttributeOnly() + { + // Only Writable flips ⇒ surgical-eligible ⇒ AttributeOnly (no adds, no removes). + var plan = Plan(changedTags: new[] + { + new AddressSpacePlan.EquipmentTagDelta( + Tag("tag-1", "eq-1", writable: false), + Tag("tag-1", "eq-1", writable: true)), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + [Fact] + public void Node_irrelevant_changed_vtag_only_is_AttributeOnly() + { + // Only Expression differs ⇒ node-irrelevant ⇒ AttributeOnly. + var plan = Plan(changedVtags: new[] + { + new AddressSpacePlan.EquipmentVirtualTagDelta( + Vtag("vt-1", "eq-1", expression: "a"), + Vtag("vt-1", "eq-1", expression: "a * 2")), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + [Fact] + public void Folder_renames_only_is_AttributeOnly() + { + var plan = Plan(renamedFolders: new[] { new AddressSpacePlan.FolderRename("area-1", "New Area") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + [Fact] + public void Driver_only_deltas_are_AttributeOnly_node_inert() + { + // Driver adds/removes/changes never touch the address space — excluded from hasAdds/hasRemoves. + var plan = Plan( + addedDrivers: new[] { new DriverInstancePlan("d-1", "Modbus", "{}") }, + removedDrivers: new[] { new DriverInstancePlan("d-2", "Modbus", "{}") }, + changedDrivers: new[] + { + new AddressSpacePlan.DriverDelta( + new DriverInstancePlan("d-3", "Modbus", "{\"v\":1}"), + new DriverInstancePlan("d-3", "Modbus", "{\"v\":2}")), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.AttributeOnly); + } + + // ----- mixed-with-surgical: adds still win over coincident surgical changes ----- + + [Fact] + public void Adds_with_coincident_surgical_change_is_PureAdd() + { + var plan = Plan( + addedTags: new[] { Tag("tag-new", "eq-1") }, + changedTags: new[] + { + new AddressSpacePlan.EquipmentTagDelta( + Tag("tag-1", "eq-1", writable: false), + Tag("tag-1", "eq-1", writable: true)), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureAdd); + } + + [Fact] + public void Adds_with_coincident_non_surgical_change_is_Rebuild() + { + // Rule 2 (node-affecting change) is evaluated BEFORE the add/remove split, so a non-surgical + // change forces Rebuild even alongside pure adds. + var plan = Plan( + addedTags: new[] { Tag("tag-new", "eq-1") }, + changedTags: new[] + { + new AddressSpacePlan.EquipmentTagDelta( + Tag("tag-1", "eq-1", fullName: "40001"), + Tag("tag-1", "eq-1", fullName: "40002")), + }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.Rebuild); + } + + [Fact] + public void Adds_with_coincident_folder_rename_is_PureAdd() + { + var plan = Plan( + addedTags: new[] { Tag("tag-new", "eq-1") }, + renamedFolders: new[] { new AddressSpacePlan.FolderRename("area-1", "New Area") }); + AddressSpaceChangeClassifier.Classify(plan).ShouldBe(AddressSpaceChangeKind.PureAdd); + } + + // ----- construction helpers ----- + + private static EquipmentNode Eq(string id) => new(id, id, "line-1"); + + private static ScriptedAlarmPlan Alarm(string id) => new(id, "eq-1", "scr-1", "msg"); + + private static EquipmentTagPlan Tag(string id, string equipmentId, string fullName = "40001", bool writable = false) => + new(id, equipmentId, "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: fullName, Writable: writable, Alarm: null); + + private static EquipmentVirtualTagPlan Vtag(string id, string equipmentId, string name = "Efficiency", string expression = "ctx.GetTag(\"a\")") => + new(id, equipmentId, FolderPath: "", Name: name, DataType: "Float64", Expression: expression, DependencyRefs: new[] { "a" }); + + private static AddressSpacePlan Plan( + IReadOnlyList? addedEquipment = null, + IReadOnlyList? removedEquipment = null, + IReadOnlyList? changedEquipment = null, + IReadOnlyList? addedDrivers = null, + IReadOnlyList? removedDrivers = null, + IReadOnlyList? changedDrivers = null, + IReadOnlyList? addedAlarms = null, + IReadOnlyList? removedAlarms = null, + IReadOnlyList? changedAlarms = null, + IReadOnlyList? addedTags = null, + IReadOnlyList? removedTags = null, + IReadOnlyList? changedTags = null, + IReadOnlyList? addedVtags = null, + IReadOnlyList? removedVtags = null, + IReadOnlyList? changedVtags = null, + IReadOnlyList? renamedFolders = null) => + new( + addedEquipment ?? Array.Empty(), + removedEquipment ?? Array.Empty(), + changedEquipment ?? Array.Empty(), + addedDrivers ?? Array.Empty(), + removedDrivers ?? Array.Empty(), + changedDrivers ?? Array.Empty(), + addedAlarms ?? Array.Empty(), + removedAlarms ?? Array.Empty(), + changedAlarms ?? Array.Empty()) + { + AddedEquipmentTags = addedTags ?? Array.Empty(), + RemovedEquipmentTags = removedTags ?? Array.Empty(), + ChangedEquipmentTags = changedTags ?? Array.Empty(), + AddedEquipmentVirtualTags = addedVtags ?? Array.Empty(), + RemovedEquipmentVirtualTags = removedVtags ?? Array.Empty(), + ChangedEquipmentVirtualTags = changedVtags ?? Array.Empty(), + RenamedFolders = renamedFolders ?? Array.Empty(), + }; +}