dohertj2/lmxopcua
1
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

feat(opcua): surgical pure-remove deploys — scoped teardown, no full rebuild (R2-07 T11)

Browse Source
This commit is contained in:
Joseph Doherty
2026-07-13 12:18:08 -04:00
parent 4504ed930c
commit 502d7650d0
3 changed files with 257 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
archreview/plans/R2-07-surgical-pure-adds-plan.md.tasks.json
+1 -1
View File
@@ -96,7 +96,7 @@
{
"id": "T11",
"subject": "Phase 2: applier PureRemove arm \u2014 terminal Bad/RemovedConditionState writes, equipment subsumption, false-to-rebuild ratchet (high-risk)",
"status": "pending",
"status": "completed",
"blockedBy": [
"T10"
]
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs
+116 -4
View File
@@ -128,8 +128,12 @@ public sealed class AddressSpaceApplier
// every client subscription server-wide survives; coincident surgical tag deltas + folder
// renames are applied IN PLACE via ISurgicalAddressSpaceSink below (any false/throw there falls
// back to a full rebuild — the F10b contract).
// • PureRemove / AddRemoveMix ⇒ full rebuild UNTIL their phases ship (R2-07 T11 / T13); mapping
// them here keeps each phase independently shippable with today's behaviour as the floor.
// • PureRemove ⇒ NO full rebuild. The removed nodes are torn down IN PLACE, scoped to the affected
// subtree, via the ISurgicalAddressSpaceSink remove members below (each preceded by a terminal
// Bad / RemovedConditionState write so in-flight MonitoredItems observe the removal); subscribers
// of OTHER nodes are untouched. Any false/throw from a remove ⇒ full-rebuild fallback (ratchet).
// • AddRemoveMix ⇒ full rebuild UNTIL Phase 3 ships (R2-07 T13); mapping it here keeps each phase
// independently shippable with today's behaviour as the floor.
// • Rebuild ⇒ the default-closed safety valve for any node-affecting change (ChangedEquipment /
// ChangedAlarms / non-surgical ChangedEquipmentTags / node-relevant ChangedEquipmentVirtualTags)
// — the classifier's rule 2, which also catches any future plan field that makes a changed
@@ -138,7 +142,6 @@ public sealed class AddressSpaceApplier
// a driver-only plan AttributeOnly, so it never rebuilds here.
var kind = AddressSpaceChangeClassifier.Classify(plan);
var mustRebuild = kind is AddressSpaceChangeKind.Rebuild
or AddressSpaceChangeKind.PureRemove
or AddressSpaceChangeKind.AddRemoveMix;
var surgicalTagDeltas = plan.ChangedEquipmentTags.Where(AddressSpaceChangeClassifier.TagDeltaIsSurgicalEligible).ToList();
@@ -155,7 +158,7 @@ public sealed class AddressSpaceApplier
rebuildFailed = !SafeRebuild();
rebuilt = true;
}
else if (surgicalTagDeltas.Count > 0 || renamedFolders.Count > 0)
else if (surgicalTagDeltas.Count > 0 || renamedFolders.Count > 0 || kind == AddressSpaceChangeKind.PureRemove)
{
if (_sink is ISurgicalAddressSpaceSink surgical)
{
@@ -191,6 +194,13 @@ public sealed class AddressSpaceApplier
}
if (!ok) { allApplied = false; break; }
}
// R2-07 Phase 2 — PureRemove scoped teardown (runs after any coincident surgical
// attribute updates / renames succeeded). Terminal Bad / RemovedConditionState writes then
// in-place removes; any false/throw flips allApplied → the rebuild ratchet below.
if (allApplied && kind == AddressSpaceChangeKind.PureRemove)
{
allApplied = ApplyPureRemove(surgical, plan, ts, ref failedNodes);
}
if (!allApplied) { rebuildFailed = !SafeRebuild(); rebuilt = true; }
}
else
@@ -218,6 +228,108 @@ public sealed class AddressSpaceApplier
return new AddressSpaceApplyOutcome(removedCount, addedCount, changedCount, rebuilt, rebuildFailed, failedNodes);
}
/// <summary>
/// R2-07 Phase 2 — apply a PureRemove plan by tearing down ONLY the affected nodes IN PLACE (no full
/// rebuild). Removed equipment "own" their child tags/vtags/alarms, so any removed child whose
/// <c>EquipmentId</c> is itself in <see cref="AddressSpacePlan.RemovedEquipment"/> is SUBSUMED by the
/// subtree removal and skipped individually. For surviving-equipment removals: a removed value
/// variable gets a terminal Bad <c>WriteValue</c> then <see cref="ISurgicalAddressSpaceSink.RemoveVariableNode"/>;
/// a removed alarm-bearing tag (the pre-R2-07 today-gap: it got no terminal condition write) and a
/// removed scripted alarm get a terminal <see cref="RemovedConditionState"/> write then
/// <see cref="ISurgicalAddressSpaceSink.RemoveAlarmConditionNode"/>; each removed equipment gets one
/// <see cref="ISurgicalAddressSpaceSink.RemoveEquipmentSubtree"/>. The terminal write is what an
/// in-flight MonitoredItem observes (a final Bad); after the node is gone, re-subscription gets
/// <c>BadNodeIdUnknown</c>. Returns false on the FIRST remove that reports the node unknown / throws
/// (the caller's one-way rebuild ratchet takes over), leaving no further surgical work attempted.
/// </summary>
/// <param name="surgical">The surgical sink to route removes through.</param>
/// <param name="plan">The PureRemove plan.</param>
/// <param name="ts">The timestamp for the terminal writes.</param>
/// <param name="failedNodes">Accumulator for swallowed terminal-write failures (archreview 01/S-1).</param>
/// <returns>True when every removal succeeded; false on the first failure (caller rebuilds).</returns>
private bool ApplyPureRemove(ISurgicalAddressSpaceSink surgical, AddressSpacePlan plan, DateTime ts, ref int failedNodes)
{
var removedEquipmentIds = new HashSet<string>(
plan.RemovedEquipment.Select(e => e.EquipmentId), StringComparer.Ordinal);
bool NotSubsumed(string equipmentId) => !removedEquipmentIds.Contains(equipmentId);
// Removed equipment tags — value variables OR alarm-bearing conditions — for SURVIVING equipment.
foreach (var tag in plan.RemovedEquipmentTags.Where(t => NotSubsumed(t.EquipmentId)))
{
var nodeId = EquipmentNodeIds.Variable(tag.EquipmentId, tag.FolderPath, tag.Name);
if (tag.Alarm is not null)
{
// Alarm-bearing tag → a Part 9 condition node. Terminal RemovedConditionState (today-gap
// closed), then remove the condition in place.
if (!SafeWriteAlarmCondition(nodeId, RemovedConditionState, ts)) failedNodes++;
if (!SafeRemoveAlarmCondition(surgical, nodeId)) return false;
}
else
{
// Plain value variable → terminal Bad so an in-flight MonitoredItem sees the removal.
SafeWriteValue(nodeId, null, OpcUaQuality.Bad, ts);
if (!SafeRemoveVariable(surgical, nodeId)) return false;
}
}
// Removed VirtualTags (always plain value variables) for surviving equipment.
foreach (var v in plan.RemovedEquipmentVirtualTags.Where(t => NotSubsumed(t.EquipmentId)))
{
var nodeId = EquipmentNodeIds.Variable(v.EquipmentId, v.FolderPath, v.Name);
SafeWriteValue(nodeId, null, OpcUaQuality.Bad, ts);
if (!SafeRemoveVariable(surgical, nodeId)) return false;
}
// Removed scripted alarms for surviving equipment — the terminal RemovedConditionState was already
// written by the top-of-Apply removal block; here we tear the condition node down in place.
foreach (var alarm in plan.RemovedAlarms.Where(a => NotSubsumed(a.EquipmentId)))
{
if (!SafeRemoveAlarmCondition(surgical, alarm.ScriptedAlarmId)) return false;
}
// Removed equipment — one subtree teardown each (subsumes their child tags/vtags/alarms). The
// top-of-Apply block already wrote the equipment id's terminal condition state.
foreach (var eq in plan.RemovedEquipment)
{
if (!SafeRemoveEquipmentSubtree(surgical, eq.EquipmentId)) return false;
}
return true;
}
/// <summary>Remove a value-variable node in place, treating a throw as a false (⇒ rebuild fallback).</summary>
/// <returns><c>true</c> when the node was removed; <c>false</c> when unknown or the sink threw.</returns>
private bool SafeRemoveVariable(ISurgicalAddressSpaceSink surgical, string nodeId)
{
try { return surgical.RemoveVariableNode(nodeId); }
catch (Exception ex) { _logger.LogError(ex, "AddressSpaceApplier: surgical RemoveVariableNode threw for {Node}", nodeId); return false; }
}
/// <summary>Remove an alarm-condition node in place, treating a throw as a false (⇒ rebuild fallback).</summary>
/// <returns><c>true</c> when the node was removed; <c>false</c> when unknown or the sink threw.</returns>
private bool SafeRemoveAlarmCondition(ISurgicalAddressSpaceSink surgical, string nodeId)
{
try { return surgical.RemoveAlarmConditionNode(nodeId); }
catch (Exception ex) { _logger.LogError(ex, "AddressSpaceApplier: surgical RemoveAlarmConditionNode threw for {Node}", nodeId); return false; }
}
/// <summary>Remove an equipment subtree in place, treating a throw as a false (⇒ rebuild fallback).</summary>
/// <returns><c>true</c> when the subtree was removed; <c>false</c> when unknown or the sink threw.</returns>
private bool SafeRemoveEquipmentSubtree(ISurgicalAddressSpaceSink surgical, string nodeId)
{
try { return surgical.RemoveEquipmentSubtree(nodeId); }
catch (Exception ex) { _logger.LogError(ex, "AddressSpaceApplier: surgical RemoveEquipmentSubtree threw for {Node}", nodeId); return false; }
}
/// <summary>Write a value, swallowing (and Warning-logging) any sink fault — used for the terminal Bad
/// on a removed variable so an in-flight MonitoredItem observes the removal.</summary>
/// <returns><c>true</c> when the write landed; <c>false</c> when the sink threw.</returns>
private bool SafeWriteValue(string nodeId, object? value, OpcUaQuality quality, DateTime ts)
{
try { _sink.WriteValue(nodeId, value, quality, ts); return true; }
catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: WriteValue threw for {Node}", nodeId); return false; }
}
/// <summary>
/// Compute the deduplicated, deterministically-ordered set of affected PARENT node ids to announce
/// (Part 3 <c>NodeAdded</c>) after a pure-add apply's Materialise passes have created the new nodes.
tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs
+140 -13
View File
@@ -25,9 +25,12 @@ public sealed class AddressSpaceApplierTests
sink.AlarmWrites.ShouldBeEmpty();
}
/// <summary>Verifies that removed equipment writes inactive alarm state and triggers rebuild.</summary>
/// <summary>R2-07 T11 — removed equipment is a PureRemove: the applier writes each id's terminal
/// "no-event" condition state (top-of-Apply block) then tears down each equipment SUBTREE in place via
/// RemoveEquipmentSubtree — NO full rebuild (other clients' subscriptions survive). (Supersedes the
/// pre-R2-07 "removed equipment ⇒ rebuild" pin.)</summary>
[Fact]
public void Removed_equipment_writes_inactive_alarm_state_per_id_and_triggers_rebuild()
public void Removed_equipment_writes_terminal_state_and_removes_subtree_without_rebuild()
{
var sink = new RecordingSink();
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
@@ -36,11 +39,13 @@ public sealed class AddressSpaceApplierTests
var outcome = applier.Apply(plan);
outcome.RemovedNodes.ShouldBe(2);
outcome.RebuildCalled.ShouldBeTrue();
outcome.RebuildCalled.ShouldBeFalse();
sink.RebuildCalls.ShouldBe(0);
// Terminal "no-event" condition state written per id (inactive + acked + confirmed).
sink.AlarmWrites.Select(a => a.NodeId).OrderBy(x => x).ShouldBe(new[] { "eq-1", "eq-2" });
// Removed nodes are reset to the "no-event" state: inactive + acked + confirmed + enabled.
sink.AlarmWrites.All(a => !a.State.Active && a.State.Acknowledged && a.State.Confirmed).ShouldBeTrue();
sink.RebuildCalls.ShouldBe(1);
// Each equipment torn down as a subtree.
sink.RemoveCalls.OrderBy(x => x.NodeId).ShouldBe(new[] { ("equipment", "eq-1"), ("equipment", "eq-2") });
}
/// <summary>R2-07 T2 — added equipment is a PureAdd: the applier SKIPS the rebuild (the idempotent
@@ -789,10 +794,11 @@ public sealed class AddressSpaceApplierTests
sink.SurgicalCalls.ShouldHaveSingleItem(); // the surgical update was attempted first
}
/// <summary>R2-07 T2 phase pin — a remove-only plan still triggers a full rebuild until Phase 2
/// (PureRemove) ships. Flipped in T11.</summary>
/// <summary>R2-07 T11 — a remove-one-tag plan is a PureRemove: the applier writes a terminal Bad to the
/// removed variable then removes it in place via RemoveVariableNode — NO rebuild. (Supersedes the T2
/// phase pin.)</summary>
[Fact]
public void Removed_equipment_tags_only_still_rebuild_phase_pin()
public void Removed_equipment_tag_writes_terminal_bad_and_removes_variable_without_rebuild()
{
var sink = new RecordingSink();
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
@@ -807,6 +813,116 @@ public sealed class AddressSpaceApplierTests
var outcome = applier.Apply(plan);
outcome.RebuildCalled.ShouldBeFalse();
sink.RebuildCalls.ShouldBe(0);
var nodeId = EquipmentNodeIds.Variable("eq-1", "", "Speed");
// Terminal Bad written to the removed variable BEFORE the removal.
sink.ValueWrites.ShouldContain((nodeId, OpcUaQuality.Bad));
sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", nodeId));
}
/// <summary>R2-07 T11 — a removed alarm-bearing tag writes the terminal RemovedConditionState (closing
/// the pre-R2-07 today-gap where a removed alarm tag got no condition write) then removes the CONDITION
/// node in place (not a value variable).</summary>
[Fact]
public void Removed_alarm_bearing_tag_writes_removed_condition_state_and_removes_condition()
{
var sink = new RecordingSink();
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
var plan = EmptyPlan with
{
RemovedEquipmentTags = new[]
{
new EquipmentTagPlan("tag-alm", "eq-1", "drv", FolderPath: "", Name: "OverTemp", DataType: "Boolean", FullName: "00001", Writable: false,
Alarm: new EquipmentTagAlarmInfo("OffNormalAlarm", 700)),
},
};
var outcome = applier.Apply(plan);
outcome.RebuildCalled.ShouldBeFalse();
var nodeId = EquipmentNodeIds.Variable("eq-1", "", "OverTemp");
// Terminal RemovedConditionState (inactive/acked/confirmed) written to the condition node.
var write = sink.AlarmWrites.ShouldHaveSingleItem();
write.NodeId.ShouldBe(nodeId);
(!write.State.Active && write.State.Acknowledged && write.State.Confirmed).ShouldBeTrue();
// Removed as a condition node, not a value variable.
sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("alarm", nodeId));
sink.ValueWrites.ShouldNotContain(w => w.NodeId == nodeId);
}
/// <summary>R2-07 T11 — a removed child (tag/vtag/alarm) whose equipment is ALSO removed is SUBSUMED by
/// the equipment-subtree removal: only RemoveEquipmentSubtree fires, no individual child remove.</summary>
[Fact]
public void Removed_child_under_removed_equipment_is_subsumed_by_subtree_removal()
{
var sink = new RecordingSink();
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
var plan = EmptyPlan with
{
RemovedEquipment = new[] { new EquipmentNode("eq-1", "One", "line-1") },
RemovedEquipmentTags = new[]
{
new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001", Writable: false, Alarm: null),
},
RemovedEquipmentVirtualTags = new[]
{
new EquipmentVirtualTagPlan("vt-1", "eq-1", FolderPath: "", Name: "Eff", DataType: "Float", Expression: "a", DependencyRefs: new[] { "a" }),
},
};
var outcome = applier.Apply(plan);
outcome.RebuildCalled.ShouldBeFalse();
// ONLY the subtree removal fires — no individual var removal for the subsumed child tag/vtag.
sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("equipment", "eq-1"));
}
/// <summary>R2-07 T11 — if a surgical remove reports the node unknown (RemoveReturns=false), the applier
/// falls back to a full rebuild (the one-way ratchet) and attempts no further surgical removes.</summary>
[Fact]
public void Pure_remove_with_remove_returns_false_falls_back_to_rebuild()
{
var sink = new RecordingSink { RemoveReturns = false };
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
var plan = EmptyPlan with
{
RemovedEquipmentTags = new[]
{
new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Float", FullName: "1", Writable: false, Alarm: null),
new EquipmentTagPlan("tag-2", "eq-1", "drv", FolderPath: "", Name: "B", DataType: "Float", FullName: "2", Writable: false, Alarm: null),
},
};
var outcome = applier.Apply(plan);
outcome.RebuildCalled.ShouldBeTrue();
sink.RebuildCalls.ShouldBe(1);
// Ratchet: stopped at the FIRST failed remove — did not attempt the second.
sink.RemoveCalls.Count.ShouldBe(1);
}
/// <summary>R2-07 T11 — a PureRemove on a sink lacking the surgical capability falls back to a full
/// rebuild (safe default), same as the F10b attribute path.</summary>
[Fact]
public void Pure_remove_on_non_surgical_sink_rebuilds()
{
var sink = new PlainRecordingSink();
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
var plan = EmptyPlan with
{
RemovedEquipmentTags = new[]
{
new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Float", FullName: "1", Writable: false, Alarm: null),
},
};
var outcome = applier.Apply(plan);
outcome.RebuildCalled.ShouldBeTrue();
sink.RebuildCalls.ShouldBe(1);
}
@@ -1276,9 +1392,14 @@ public sealed class AddressSpaceApplierTests
var outcome = applier.Apply(plan);
outcome.RebuildCalled.ShouldBeTrue();
sink.RebuildCalls.ShouldBe(1);
outcome.RemovedNodes.ShouldBe(2); // both removals counted (was 0 before the fix)
// R2-07 T11 — a removed-tag + removed-vtag plan is a PureRemove: both variables are torn down in
// place (RemoveVariableNode ×2), NO full rebuild; both removals are still counted.
outcome.RebuildCalled.ShouldBeFalse();
sink.RebuildCalls.ShouldBe(0);
outcome.RemovedNodes.ShouldBe(2); // both removals counted
sink.RemoveCalls.Count.ShouldBe(2);
sink.RemoveCalls.ShouldContain(("var", EquipmentNodeIds.Variable("eq-1", "", "Speed")));
sink.RemoveCalls.ShouldContain(("var", EquipmentNodeIds.Variable("eq-1", "", "Efficiency")));
}
// ----- F10b: surgical in-place tag-attribute writes (Writable / IsHistorized / HistorianTagname) -----
@@ -2072,12 +2193,18 @@ public sealed class AddressSpaceApplierTests
/// <summary>Gets the list of recorded alarm-condition materialise calls.</summary>
public List<(string AlarmNodeId, string EquipmentNodeId, string DisplayName, string AlarmType, int Severity, bool IsNative)> AlarmConditionCalls => AlarmConditionQueue.ToList();
/// <summary>Records a value write (no-op in this recording sink).</summary>
/// <summary>Gets the queue of value writes (NodeId, quality) — used to assert the PureRemove terminal Bad.</summary>
public ConcurrentQueue<(string NodeId, OpcUaQuality Quality)> ValueWriteQueue { get; } = new();
/// <summary>Gets the list of recorded value writes.</summary>
public List<(string NodeId, OpcUaQuality Quality)> ValueWrites => ValueWriteQueue.ToList();
/// <summary>Records a value write (NodeId + quality).</summary>
/// <param name="nodeId">The node ID.</param>
/// <param name="value">The value to write.</param>
/// <param name="quality">The OPC UA quality.</param>
/// <param name="sourceTimestampUtc">The source timestamp in UTC.</param>
public void WriteValue(string nodeId, object? value, OpcUaQuality quality, DateTime sourceTimestampUtc) { }
public void WriteValue(string nodeId, object? value, OpcUaQuality quality, DateTime sourceTimestampUtc)
=> ValueWriteQueue.Enqueue((nodeId, quality));
/// <summary>Records an alarm condition write call.</summary>
/// <param name="alarmNodeId">The alarm node ID.</param>
/// <param name="state">The full condition state snapshot.</param>