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 6effe9be..ee3c6b06 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 @@ -63,7 +63,7 @@ { "id": "T7", "subject": "Phase 2: add RemoveVariableNode/RemoveAlarmConditionNode/RemoveEquipmentSubtree to ISurgicalAddressSpaceSink + DeferredAddressSpaceSink forwarding \u2014 guard-first with observed negative control (high-risk)", - "status": "pending", + "status": "completed", "blockedBy": [ "T6" ] diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs index 701f8a55..f70d8be2 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs @@ -68,4 +68,21 @@ public sealed class DeferredAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgical public bool UpdateFolderDisplayName(string folderNodeId, string displayName) => _inner is ISurgicalAddressSpaceSink surgical && surgical.UpdateFolderDisplayName(folderNodeId, displayName); + + // R2-07 T7 — surgical remove forwards. Same capability-sniffing pattern as UpdateTagAttributes / + // UpdateFolderDisplayName: forward when the inner sink supports the surgical capability; return false + // otherwise (before the real SdkAddressSpaceSink is swapped in, or any non-surgical inner) so the caller + // (AddressSpaceApplier) falls back to a full rebuild. Without these forwards the surgical remove path is + // inert on every driver-role host — the F10b / PR#423 trap the reflection guard exists to catch. + /// + public bool RemoveVariableNode(string variableNodeId) + => _inner is ISurgicalAddressSpaceSink surgical && surgical.RemoveVariableNode(variableNodeId); + + /// + public bool RemoveAlarmConditionNode(string alarmNodeId) + => _inner is ISurgicalAddressSpaceSink surgical && surgical.RemoveAlarmConditionNode(alarmNodeId); + + /// + public bool RemoveEquipmentSubtree(string equipmentNodeId) + => _inner is ISurgicalAddressSpaceSink surgical && surgical.RemoveEquipmentSubtree(equipmentNodeId); } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs index 7dc685f6..f4033030 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs @@ -34,4 +34,34 @@ public interface ISurgicalAddressSpaceSink /// The new display name to apply. /// True when the in-place update was applied; false when the folder is missing (caller rebuilds). bool UpdateFolderDisplayName(string folderNodeId, string displayName); + + /// Remove ONE existing value-variable node IN PLACE (detach from its parent, drop it from the + /// predefined-node set, drop any historized-tagname registration), then raise a Part 3 + /// GeneralModelChangeEvent (verb=NodeDeleted) outside the lock — so subscribers of OTHER nodes are + /// untouched and their MonitoredItems survive. The caller has already written a final Bad quality to the + /// node so in-flight MonitoredItems on the removed node observe the removal; after the node is gone, + /// re-subscription attempts get BadNodeIdUnknown from the SDK. Returns false when the node id is unknown + /// (the node-manager maps drifted from the plan — caller falls back to a full rebuild). + /// The folder-scoped node id of the variable to remove in place. + /// True when the node was removed; false when the id is unknown (caller rebuilds). + bool RemoveVariableNode(string variableNodeId); + + /// Remove ONE existing Part 9 alarm-condition node IN PLACE (detach, drop from the + /// predefined-node set, clear the native-alarm flag), then raise NodeDeleted outside the lock. The + /// caller has already written the terminal RemovedConditionState (Retain=false) so a removed condition + /// stops replaying on ConditionRefresh. Returns false when the condition id is unknown (caller + /// rebuilds). + /// The alarm condition node id (== ScriptedAlarmId, or a native alarm tag's folder-scoped id). + /// True when the condition was removed; false when the id is unknown (caller rebuilds). + bool RemoveAlarmConditionNode(string alarmNodeId); + + /// Remove an equipment folder AND its entire descendant subtree (sub-folders, value variables, + /// condition nodes) IN PLACE: per-descendant map/registration cleanup (variables, historized tagnames, + /// conditions, native flags), notifier demotion (sever the Server↔folder root-notifier ref + + /// notifier-folder + event-notifier-source registrations for the equipment id), then one NodeDeleted for + /// the equipment folder outside the lock. Returns false when the folder id is unknown (caller + /// rebuilds). + /// The equipment folder node id whose entire subtree to remove. + /// True when the subtree was removed; false when the id is unknown (caller rebuilds). + bool RemoveEquipmentSubtree(string equipmentNodeId); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 1a480a9f..91b4acb4 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -1746,6 +1746,19 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 } } + // R2-07 Phase 2 (T7 placeholder) — surgical single-node / subtree removal. These return false (the + // rebuild-fallback contract) so the tree stays shippable at T7 while T8/T9/T10 implement the real + // in-place teardown (mirroring the per-node loops inside RebuildAddressSpace, scoped to the id/subtree, + // with a NodeDeleted model-change reported outside Lock). Implemented in T8/T9/T10. + /// + public bool RemoveVariableNode(string variableNodeId) => false; + + /// + public bool RemoveAlarmConditionNode(string alarmNodeId) => false; + + /// + public bool RemoveEquipmentSubtree(string equipmentNodeId) => false; + private FolderState ResolveParentFolder(string? parentNodeId) { EnsureAddressSpaceCreated(); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs index 9611c9fc..16350b02 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs @@ -48,6 +48,18 @@ public sealed class SdkAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgicalAddre public bool UpdateFolderDisplayName(string folderNodeId, string displayName) => _nodeManager.UpdateFolderDisplayName(folderNodeId, displayName); + /// + public bool RemoveVariableNode(string variableNodeId) + => _nodeManager.RemoveVariableNode(variableNodeId); + + /// + public bool RemoveAlarmConditionNode(string alarmNodeId) + => _nodeManager.RemoveAlarmConditionNode(alarmNodeId); + + /// + public bool RemoveEquipmentSubtree(string equipmentNodeId) + => _nodeManager.RemoveEquipmentSubtree(equipmentNodeId); + /// public void RebuildAddressSpace() => _nodeManager.RebuildAddressSpace(); diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredAddressSpaceSinkTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredAddressSpaceSinkTests.cs index fe0b9ea5..6b2e2ae8 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredAddressSpaceSinkTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredAddressSpaceSinkTests.cs @@ -114,6 +114,44 @@ public class DeferredAddressSpaceSinkTests surgical.FolderRenameCalled.ShouldBeTrue(); } + // ---------- R2-07 Phase 2: surgical remove forwarding ---------- + + [Fact] + public void Remove_members_return_false_for_non_surgical_inner() + { + var sink = new DeferredAddressSpaceSink(); + sink.SetSink(new SpySink()); + + sink.RemoveVariableNode("v-1").ShouldBeFalse(); + sink.RemoveAlarmConditionNode("a-1").ShouldBeFalse(); + sink.RemoveEquipmentSubtree("eq-1").ShouldBeFalse(); + } + + [Fact] + public void Remove_members_forward_to_surgical_inner() + { + var surgical = new SpySurgicalSink(); + var sink = new DeferredAddressSpaceSink(); + sink.SetSink(surgical); + + sink.RemoveVariableNode("v-1").ShouldBeTrue(); + sink.RemoveAlarmConditionNode("a-1").ShouldBeTrue(); + sink.RemoveEquipmentSubtree("eq-1").ShouldBeTrue(); + + surgical.RemoveVariableCalled.ShouldBeTrue(); + surgical.RemoveAlarmCalled.ShouldBeTrue(); + surgical.RemoveSubtreeCalled.ShouldBeTrue(); + } + + [Fact] + public void Before_SetSink_remove_members_return_false() + { + var sink = new DeferredAddressSpaceSink(); + sink.RemoveVariableNode("v-1").ShouldBeFalse(); + sink.RemoveAlarmConditionNode("a-1").ShouldBeFalse(); + sink.RemoveEquipmentSubtree("eq-1").ShouldBeFalse(); + } + // ---------- SetSink(null) reverts to null sink ---------- [Fact] @@ -183,5 +221,13 @@ public class DeferredAddressSpaceSinkTests FolderRenameCalled = true; return true; } + + public bool RemoveVariableCalled { get; private set; } + public bool RemoveAlarmCalled { get; private set; } + public bool RemoveSubtreeCalled { get; private set; } + + public bool RemoveVariableNode(string variableNodeId) { RemoveVariableCalled = true; return true; } + public bool RemoveAlarmConditionNode(string alarmNodeId) { RemoveAlarmCalled = true; return true; } + public bool RemoveEquipmentSubtree(string equipmentNodeId) { RemoveSubtreeCalled = true; return true; } } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs index d1bb299d..86ac2f37 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs @@ -2013,6 +2013,35 @@ public sealed class AddressSpaceApplierTests return FolderRenameReturns; } + /// Gets the queue of surgical remove calls (kind + node id) in call order (R2-07 Phase 2). + public ConcurrentQueue<(string Kind, string NodeId)> RemoveQueue { get; } = new(); + /// Gets the list of recorded surgical remove calls. + public List<(string Kind, string NodeId)> RemoveCalls => RemoveQueue.ToList(); + /// When false, the Remove* members report the node missing (return false), driving the + /// applier's rebuild fallback. Defaults to true (node present, removal succeeds). + public bool RemoveReturns { get; init; } = true; + + /// Records a surgical variable-node removal; returns . + public bool RemoveVariableNode(string variableNodeId) + { + RemoveQueue.Enqueue(("var", variableNodeId)); + return RemoveReturns; + } + + /// Records a surgical alarm-condition-node removal; returns . + public bool RemoveAlarmConditionNode(string alarmNodeId) + { + RemoveQueue.Enqueue(("alarm", alarmNodeId)); + return RemoveReturns; + } + + /// Records a surgical equipment-subtree removal; returns . + public bool RemoveEquipmentSubtree(string equipmentNodeId) + { + RemoveQueue.Enqueue(("equipment", equipmentNodeId)); + return RemoveReturns; + } + /// Gets the queue of alarm condition write calls. public ConcurrentQueue<(string NodeId, AlarmConditionSnapshot State)> AlarmQueue { get; } = new(); /// Gets the queue of folder creation calls. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs index 2fedad28..d10a9f58 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs @@ -174,6 +174,40 @@ public sealed class DeferredAddressSpaceSinkTests ((ISurgicalAddressSpaceSink)deferred).UpdateFolderDisplayName("area-1", "X").ShouldBeFalse(); } + /// R2-07 Phase 2 — the three surgical remove members forward to a surgical inner with + /// arg-fidelity (kind + node id), returning the inner's own result; false when the inner is not surgical + /// so the caller falls back to a full rebuild. + [Fact] + public void Remove_members_forward_to_a_surgical_inner_sink_with_arg_fidelity() + { + var deferred = new DeferredAddressSpaceSink(); + var inner = new SurgicalRecordingSink { Result = true }; + deferred.SetSink(inner); + + ((ISurgicalAddressSpaceSink)deferred).RemoveVariableNode("eq-1/A").ShouldBeTrue(); + ((ISurgicalAddressSpaceSink)deferred).RemoveAlarmConditionNode("alm-1").ShouldBeTrue(); + ((ISurgicalAddressSpaceSink)deferred).RemoveEquipmentSubtree("eq-1").ShouldBeTrue(); + + inner.RemoveCalls.ShouldBe(new[] { ("var", "eq-1/A"), ("alarm", "alm-1"), ("equipment", "eq-1") }); + } + + /// The remove forwards return the inner's own result (false ⇒ id unknown ⇒ caller rebuilds), + /// and return false when the inner is not surgical at all. + [Fact] + public void Remove_members_return_inner_result_and_false_when_not_surgical() + { + var deferred = new DeferredAddressSpaceSink(); + deferred.SetSink(new SurgicalRecordingSink { Result = false }); + ((ISurgicalAddressSpaceSink)deferred).RemoveVariableNode("v-1").ShouldBeFalse(); + ((ISurgicalAddressSpaceSink)deferred).RemoveAlarmConditionNode("a-1").ShouldBeFalse(); + ((ISurgicalAddressSpaceSink)deferred).RemoveEquipmentSubtree("eq-1").ShouldBeFalse(); + + deferred.SetSink(null); + ((ISurgicalAddressSpaceSink)deferred).RemoveVariableNode("v-1").ShouldBeFalse(); + ((ISurgicalAddressSpaceSink)deferred).RemoveAlarmConditionNode("a-1").ShouldBeFalse(); + ((ISurgicalAddressSpaceSink)deferred).RemoveEquipmentSubtree("eq-1").ShouldBeFalse(); + } + /// Builds a minimal for the forwarding tests (the /// inner sink only records the node id, so the exact state values don't matter here). private static AlarmConditionSnapshot Snapshot(bool active = false) => @@ -240,6 +274,16 @@ public sealed class DeferredAddressSpaceSinkTests return Result; } + /// Gets the recorded surgical remove calls (kind + node id), in call order (R2-07 Phase 2). + public List<(string Kind, string NodeId)> RemoveCalls { get; } = new(); + + /// + public bool RemoveVariableNode(string variableNodeId) { RemoveCalls.Add(("var", variableNodeId)); return Result; } + /// + public bool RemoveAlarmConditionNode(string alarmNodeId) { RemoveCalls.Add(("alarm", alarmNodeId)); return Result; } + /// + public bool RemoveEquipmentSubtree(string equipmentNodeId) { RemoveCalls.Add(("equipment", equipmentNodeId)); return Result; } + /// public void WriteValue(string nodeId, object? value, OpcUaQuality quality, DateTime sourceTimestampUtc) { } /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs index a34c325b..cc3b2fa1 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs @@ -487,5 +487,23 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase FolderRenameQueue.Enqueue((folderNodeId, displayName)); return true; } + /// Records a surgical variable-node removal (always succeeds in this recording sink). + public bool RemoveVariableNode(string variableNodeId) + { + Calls.Enqueue($"RV:{variableNodeId}"); + return true; + } + /// Records a surgical alarm-condition-node removal (always succeeds in this recording sink). + public bool RemoveAlarmConditionNode(string alarmNodeId) + { + Calls.Enqueue($"RA:{alarmNodeId}"); + return true; + } + /// Records a surgical equipment-subtree removal (always succeeds in this recording sink). + public bool RemoveEquipmentSubtree(string equipmentNodeId) + { + Calls.Enqueue($"RE:{equipmentNodeId}"); + return true; + } } }