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

feat(opcuaserver): applier applies raw-tag + UNS-reference deltas surgically

Browse Source 
Task 4 (9f68f272) taught AddressSpaceChangeClassifier that a non-alarm
RawTagDelta and any UnsReferenceDelta are surgical-eligible, so such plans
started classifying AttributeOnly/PureAdd instead of Rebuild. The applier's
surgical branch did not know about either delta set, so those deploys fell
through as SILENT NO-OPS (or, on a non-surgical sink, applied nothing at all
where a rebuild was the safe answer). This closes that gap:

  - Raw-tag loop: an eligible ChangedRawTags delta is applied IN PLACE via
    ISurgicalAddressSpaceSink.UpdateTagAttributes in the RAW realm, computing
    writable (`Writable && !IsArray`) and the historian tagname (blank override
    ⇒ the RawPath) exactly as MaterialiseRawSubtree does. A delta whose ONLY
    difference is ReferencingEquipmentPaths is node-inert for a value tag and
    is SKIPPED entirely — that is the shape a single UnsTagReference add/drop
    produces, and it used to rebuild the world.
  - UNS-reference loop: a ChangedUnsReferenceVariables delta removes the
    PREVIOUS node in place (terminal Bad, then RemoveVariableNode, Uns realm).
    Remove+recreate rather than an in-place update because a re-point must move
    the Organizes edge (the sink has no RemoveReference) and a display-name
    edit changes the NodeId itself; the cost is that one node's monitored
    items, versus every subscription server-wide today.
  - The branch trigger now fires for both sets, so a non-surgical sink falls
    back to a full rebuild instead of applying nothing.

Every new operation is inside the existing one-way rebuild ratchet: any false
or throw ⇒ SafeRebuild(). The eligibility filter is defense-in-depth — when
kind != Rebuild the classifier already guarantees every delta is eligible.

The recreate of a changed UNS reference is NOT done here. Until Task 6 the
publish actor still materialises the FULL composition after Apply returns, so
the existing MaterialiseUnsReferences pass recreates it and the system is
correct end to end after this commit; the remove-then-recreate ordering holds
because Apply returns before the materialise passes run. Task 6 narrows those
passes to the plan's delta and keeps the same ordering.

Tests: 7 new applier tests (in-place raw update incl. array/override parity,
reference-only skip, UNS-ref remove, both ratchet fallbacks, non-surgical-sink
rebuild). RecordingSink now captures the realm on surgical / remove / value-write
calls, so the existing UNS-realm assertions are explicit too.
This commit is contained in:
Joseph Doherty
2026-08-15 13:25:33 -04:00
parent 59202017cd
commit 9c37330a4c
2 changed files with 285 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs
+217 -19
View File
@@ -147,8 +147,8 @@ public sealed class AddressSpaceApplierTests
sink.RebuildCalls.ShouldBe(0);
var nodeId = V3NodeIds.Uns("eq-1", "Speed");
// Terminal Bad written to the removed variable BEFORE the removal.
sink.ValueWrites.ShouldContain((nodeId, OpcUaStatus.Bad));
sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", nodeId));
sink.ValueWrites.ShouldContain((nodeId, OpcUaStatus.Bad, AddressSpaceRealm.Uns));
sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", nodeId, AddressSpaceRealm.Uns));
}
/// <summary>R2-07 T11 — a removed alarm-bearing tag writes the terminal RemovedConditionState (closing
@@ -178,7 +178,7 @@ public sealed class AddressSpaceApplierTests
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.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("alarm", nodeId, AddressSpaceRealm.Uns));
sink.ValueWrites.ShouldNotContain(w => w.NodeId == nodeId);
}
@@ -207,7 +207,7 @@ public sealed class AddressSpaceApplierTests
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"));
sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("equipment", "eq-1", AddressSpaceRealm.Uns));
}
/// <summary>R2-07 T11 — if a surgical remove reports the node unknown (RemoveReturns=false), the applier
@@ -283,7 +283,7 @@ public sealed class AddressSpaceApplierTests
outcome.RebuildCalled.ShouldBeFalse();
sink.RebuildCalls.ShouldBe(0);
// The removed node is torn down in place; the add is left to the publish actor's materialise passes.
sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", V3NodeIds.Uns("eq-1", "Old")));
sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", V3NodeIds.Uns("eq-1", "Old"), AddressSpaceRealm.Uns));
outcome.AddedNodes.ShouldBe(1);
outcome.RemovedNodes.ShouldBe(1);
}
@@ -314,7 +314,7 @@ public sealed class AddressSpaceApplierTests
var outcome = applier.Apply(plan);
outcome.RebuildCalled.ShouldBeFalse();
sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", nodeId));
sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", nodeId, AddressSpaceRealm.Uns));
}
/// <summary>R2-07 T13 — a remove failure inside an AddRemoveMix still trips the rebuild ratchet.</summary>
@@ -787,8 +787,8 @@ public sealed class AddressSpaceApplierTests
sink.RebuildCalls.ShouldBe(0);
outcome.RemovedNodes.ShouldBe(2); // both removals counted
sink.RemoveCalls.Count.ShouldBe(2);
sink.RemoveCalls.ShouldContain(("var", V3NodeIds.Uns("eq-1", "Speed")));
sink.RemoveCalls.ShouldContain(("var", V3NodeIds.Uns("eq-1", "Efficiency")));
sink.RemoveCalls.ShouldContain(("var", V3NodeIds.Uns("eq-1", "Speed"), AddressSpaceRealm.Uns));
sink.RemoveCalls.ShouldContain(("var", V3NodeIds.Uns("eq-1", "Efficiency"), AddressSpaceRealm.Uns));
}
// ----- F10b: surgical in-place tag-attribute writes (Writable / IsHistorized / HistorianTagname) -----
@@ -1434,6 +1434,204 @@ public sealed class AddressSpaceApplierTests
new[] { "eq-1", V3NodeIds.Uns("eq-1", "Diag"), "line-7" }.OrderBy(x => x));
}
// ----- Surgical v3 deltas: raw-tag attribute edits (Raw realm) + UNS-reference remove -----
// Task 4 taught AddressSpaceChangeClassifier that a non-alarm RawTagDelta and any UnsReferenceDelta are
// surgical-eligible; these pin the applier actually APPLYING them (before this, such a plan classified
// AttributeOnly and fell through the surgical branch as a silent no-op).
/// <summary>A non-alarm raw-tag delta (historize flipped on) is applied IN PLACE on the raw node via
/// <c>ISurgicalAddressSpaceSink.UpdateTagAttributes</c> in the <b>Raw</b> realm — no rebuild, so every
/// client subscription survives. The arguments match <see cref="AddressSpaceApplier.MaterialiseRawSubtree"/>
/// exactly: a blank historian override resolves to the RawPath (== NodeId).</summary>
[Fact]
public void Surgical_raw_tag_delta_updates_in_place_raw_realm()
{
var sink = new RecordingSink();
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
var previous = RawTag("Plant/Modbus/dev1/speed");
var current = previous with { IsHistorized = true };
var plan = AddressSpacePlanner.Compute(CompositionWithRawTags(previous), CompositionWithRawTags(current));
plan.ChangedRawTags.Count.ShouldBe(1);
plan.AddedRawTags.ShouldBeEmpty();
plan.RemovedRawTags.ShouldBeEmpty();
var outcome = applier.Apply(plan);
outcome.RebuildCalled.ShouldBeFalse();
sink.RebuildCalls.ShouldBe(0);
var call = sink.SurgicalCalls.ShouldHaveSingleItem();
call.NodeId.ShouldBe("Plant/Modbus/dev1/speed");
call.Writable.ShouldBeTrue(); // Writable && !IsArray
call.Historian.ShouldBe("Plant/Modbus/dev1/speed"); // blank override ⇒ the RawPath
call.DataType.ShouldBe("Float");
call.IsArray.ShouldBeFalse();
call.ArrayLength.ShouldBeNull();
call.Realm.ShouldBe(AddressSpaceRealm.Raw); // the raw node, NOT a UNS projection
outcome.ChangedNodes.ShouldBe(1);
}
/// <summary>MaterialiseRawSubtree parity for the two computed arguments: an ARRAY tag is forced read-only
/// (the driver write path can't handle arrays) and a non-blank <c>HistorianTagname</c> override wins over
/// the RawPath default.</summary>
[Fact]
public void Surgical_raw_tag_delta_forces_array_read_only_and_honours_historian_override()
{
var sink = new RecordingSink();
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
var previous = RawTag("Plant/Modbus/dev1/block") with
{
Writable = true, IsArray = true, ArrayLength = 8, IsHistorized = true, HistorianTagname = "WW.Old",
};
var current = previous with { HistorianTagname = "WW.New", ArrayLength = 16 };
var plan = AddressSpacePlanner.Compute(CompositionWithRawTags(previous), CompositionWithRawTags(current));
plan.ChangedRawTags.Count.ShouldBe(1);
applier.Apply(plan);
sink.RebuildCalls.ShouldBe(0);
var call = sink.SurgicalCalls.ShouldHaveSingleItem();
call.Writable.ShouldBeFalse(); // Writable && !IsArray ⇒ an array tag is read-only
call.Historian.ShouldBe("WW.New"); // override verbatim
call.IsArray.ShouldBeTrue();
call.ArrayLength.ShouldBe(16u);
call.Realm.ShouldBe(AddressSpaceRealm.Raw);
}
/// <summary>The headline UNS-reference case: adding/dropping ONE <c>UnsTagReference</c> flips its backing
/// raw tag to Changed purely through <c>ReferencingEquipmentPaths</c>, which is node-INERT for a value tag.
/// The applier must recognise that and make NO sink call at all — not a rebuild, and not even an
/// idempotent in-place update.</summary>
[Fact]
public void Reference_only_raw_tag_delta_skips_the_sink_call()
{
var sink = new RecordingSink();
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
var previous = RawTag("Plant/Modbus/dev1/speed");
var current = previous with { ReferencingEquipmentPaths = new[] { "filling/line-1/station-1" } };
var plan = AddressSpacePlanner.Compute(CompositionWithRawTags(previous), CompositionWithRawTags(current));
plan.ChangedRawTags.Count.ShouldBe(1);
var outcome = applier.Apply(plan);
outcome.RebuildCalled.ShouldBeFalse();
sink.RebuildCalls.ShouldBe(0);
sink.SurgicalCalls.ShouldBeEmpty(); // node-inert ⇒ nothing to say to the sink
}
/// <summary>A UNS-reference delta (here a display-name-override edit, which moves the NodeId) tears the
/// PREVIOUS node down in place: a terminal Bad so an in-flight MonitoredItem observes the removal, then
/// RemoveVariableNode — both in the Uns realm, no rebuild. The RECREATE of the current node is NOT the
/// applier's job (it rides the materialise pass after Apply returns).</summary>
[Fact]
public void Uns_reference_delta_removes_previous_node()
{
var sink = new RecordingSink();
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
var previous = UnsRef("filling/line-1/station-1/speed", "speed");
var current = previous with { NodeId = "filling/line-1/station-1/Line Speed", EffectiveName = "Line Speed" };
var plan = AddressSpacePlanner.Compute(CompositionWithUnsRefs(previous), CompositionWithUnsRefs(current));
plan.ChangedUnsReferenceVariables.Count.ShouldBe(1);
plan.AddedUnsReferenceVariables.ShouldBeEmpty();
plan.RemovedUnsReferenceVariables.ShouldBeEmpty();
var outcome = applier.Apply(plan);
outcome.RebuildCalled.ShouldBeFalse();
sink.RebuildCalls.ShouldBe(0);
sink.ValueWrites.ShouldContain((previous.NodeId, OpcUaStatus.Bad, AddressSpaceRealm.Uns));
sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", previous.NodeId, AddressSpaceRealm.Uns));
// The applier never recreates — no EnsureVariable for the new NodeId here.
sink.VariableCalls.ShouldBeEmpty();
outcome.ChangedNodes.ShouldBe(1);
}
/// <summary>The one-way rebuild ratchet holds for the new raw-tag path: a surgical update that reports the
/// node missing (false) falls back to a full rebuild.</summary>
[Fact]
public void Surgical_raw_tag_false_falls_back_to_rebuild()
{
var sink = new RecordingSink { SurgicalReturns = false };
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
var previous = RawTag("Plant/Modbus/dev1/speed");
var current = previous with { IsHistorized = true };
var outcome = applier.Apply(
AddressSpacePlanner.Compute(CompositionWithRawTags(previous), CompositionWithRawTags(current)));
sink.SurgicalCalls.ShouldHaveSingleItem(); // attempted in place first
outcome.RebuildCalled.ShouldBeTrue();
sink.RebuildCalls.ShouldBe(1);
}
/// <summary>The same ratchet for the UNS-reference remove: a remove reporting the node missing (false)
/// falls back to a full rebuild.</summary>
[Fact]
public void Uns_reference_remove_false_falls_back_to_rebuild()
{
var sink = new RecordingSink { RemoveReturns = false };
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
var previous = UnsRef("filling/line-1/station-1/speed", "speed");
var current = previous with { BackingRawPath = "Plant/Modbus/dev1/speed2" };
var outcome = applier.Apply(
AddressSpacePlanner.Compute(CompositionWithUnsRefs(previous), CompositionWithUnsRefs(current)));
sink.RemoveCalls.ShouldHaveSingleItem(); // attempted in place first
outcome.RebuildCalled.ShouldBeTrue();
sink.RebuildCalls.ShouldBe(1);
}
/// <summary>A sink lacking the surgical capability cannot apply a raw-tag delta in place, so it must
/// REBUILD. This is what closes the Task-4 gap for the fallback case: before the branch trigger learned
/// about raw-tag deltas, such a plan classified AttributeOnly and applied nothing at all.</summary>
[Fact]
public void Raw_tag_delta_with_non_surgical_sink_rebuilds()
{
var sink = new PlainRecordingSink();
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
var previous = RawTag("Plant/Modbus/dev1/speed");
var current = previous with { IsHistorized = true };
var outcome = applier.Apply(
AddressSpacePlanner.Compute(CompositionWithRawTags(previous), CompositionWithRawTags(current)));
outcome.RebuildCalled.ShouldBeTrue();
sink.RebuildCalls.ShouldBe(1);
}
private static RawTagPlan RawTag(string rawPath) =>
new("tag-" + rawPath, rawPath, ParentNodeId: "Plant/Modbus/dev1", DriverInstanceId: "drv-1",
Name: rawPath[(rawPath.LastIndexOf('/') + 1)..], DataType: "Float", Writable: true, Alarm: null,
ReferencingEquipmentPaths: Array.Empty<string>());
private static UnsReferenceVariable UnsRef(string nodeId, string effectiveName) =>
new("ref-1", "eq-1", nodeId, effectiveName, "Plant/Modbus/dev1/speed", "Float", Writable: true);
private static AddressSpaceComposition CompositionWithRawTags(params RawTagPlan[] tags) =>
new(
Array.Empty<EquipmentNode>(), Array.Empty<DriverInstancePlan>(), Array.Empty<ScriptedAlarmPlan>())
{
RawTags = tags,
};
private static AddressSpaceComposition CompositionWithUnsRefs(params UnsReferenceVariable[] refs) =>
new(
Array.Empty<EquipmentNode>(), Array.Empty<DriverInstancePlan>(), Array.Empty<ScriptedAlarmPlan>())
{
UnsReferenceVariables = refs,
};
private static AddressSpaceComposition CompositionWithAreas(params UnsAreaProjection[] areas) =>
new(
areas, Array.Empty<UnsLineProjection>(), Array.Empty<EquipmentNode>(),
@@ -1486,9 +1684,9 @@ public sealed class AddressSpaceApplierTests
private sealed class RecordingSink : IOpcUaAddressSpaceSink, ISurgicalAddressSpaceSink
{
/// <summary>Gets the queue of surgical in-place tag-attribute update calls (F10b + FB-7).</summary>
public ConcurrentQueue<(string NodeId, bool Writable, string? Historian, string DataType, bool IsArray, uint? ArrayLength)> SurgicalQueue { get; } = new();
public ConcurrentQueue<(string NodeId, bool Writable, string? Historian, string DataType, bool IsArray, uint? ArrayLength, AddressSpaceRealm Realm)> SurgicalQueue { get; } = new();
/// <summary>Gets the list of recorded surgical in-place tag-attribute update calls.</summary>
public List<(string NodeId, bool Writable, string? Historian, string DataType, bool IsArray, uint? ArrayLength)> SurgicalCalls => SurgicalQueue.ToList();
public List<(string NodeId, bool Writable, string? Historian, string DataType, bool IsArray, uint? ArrayLength, AddressSpaceRealm Realm)> SurgicalCalls => SurgicalQueue.ToList();
/// <summary>When false, <see cref="UpdateTagAttributes"/> reports the node missing (returns false),
/// driving the applier's rebuild fallback. Defaults to true (node present, update succeeds).</summary>
public bool SurgicalReturns { get; init; } = true;
@@ -1502,7 +1700,7 @@ public sealed class AddressSpaceApplierTests
/// <param name="arrayLength">The new 1-D array length when <paramref name="isArray"/> is true.</param>
public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname, string dataType, bool isArray, uint? arrayLength, AddressSpaceRealm realm = AddressSpaceRealm.Uns)
{
SurgicalQueue.Enqueue((variableNodeId, writable, historianTagname, dataType, isArray, arrayLength));
SurgicalQueue.Enqueue((variableNodeId, writable, historianTagname, dataType, isArray, arrayLength, realm));
return SurgicalReturns;
}
@@ -1524,9 +1722,9 @@ public sealed class AddressSpaceApplierTests
}
/// <summary>Gets the queue of surgical remove calls (kind + node id) in call order (R2-07 Phase 2).</summary>
public ConcurrentQueue<(string Kind, string NodeId)> RemoveQueue { get; } = new();
public ConcurrentQueue<(string Kind, string NodeId, AddressSpaceRealm Realm)> RemoveQueue { get; } = new();
/// <summary>Gets the list of recorded surgical remove calls.</summary>
public List<(string Kind, string NodeId)> RemoveCalls => RemoveQueue.ToList();
public List<(string Kind, string NodeId, AddressSpaceRealm Realm)> RemoveCalls => RemoveQueue.ToList();
/// <summary>When false, the Remove* members report the node missing (return false), driving the
/// applier's rebuild fallback. Defaults to true (node present, removal succeeds).</summary>
public bool RemoveReturns { get; init; } = true;
@@ -1534,21 +1732,21 @@ public sealed class AddressSpaceApplierTests
/// <summary>Records a surgical variable-node removal; returns <see cref="RemoveReturns"/>.</summary>
public bool RemoveVariableNode(string variableNodeId, AddressSpaceRealm realm = AddressSpaceRealm.Uns)
{
RemoveQueue.Enqueue(("var", variableNodeId));
RemoveQueue.Enqueue(("var", variableNodeId, realm));
return RemoveReturns;
}
/// <summary>Records a surgical alarm-condition-node removal; returns <see cref="RemoveReturns"/>.</summary>
public bool RemoveAlarmConditionNode(string alarmNodeId, AddressSpaceRealm realm = AddressSpaceRealm.Uns)
{
RemoveQueue.Enqueue(("alarm", alarmNodeId));
RemoveQueue.Enqueue(("alarm", alarmNodeId, realm));
return RemoveReturns;
}
/// <summary>Records a surgical equipment-subtree removal; returns <see cref="RemoveReturns"/>.</summary>
public bool RemoveEquipmentSubtree(string equipmentNodeId, AddressSpaceRealm realm = AddressSpaceRealm.Uns)
{
RemoveQueue.Enqueue(("equipment", equipmentNodeId));
RemoveQueue.Enqueue(("equipment", equipmentNodeId, realm));
return RemoveReturns;
}
@@ -1583,9 +1781,9 @@ public sealed class AddressSpaceApplierTests
public List<(string AlarmNodeId, string EquipmentNodeId, string DisplayName, string AlarmType, int Severity, bool IsNative)> AlarmConditionCalls => AlarmConditionQueue.ToList();
/// <summary>Gets the queue of value writes (NodeId, quality) — used to assert the PureRemove terminal Bad.</summary>
public ConcurrentQueue<(string NodeId, uint Quality)> ValueWriteQueue { get; } = new();
public ConcurrentQueue<(string NodeId, uint Quality, AddressSpaceRealm Realm)> ValueWriteQueue { get; } = new();
/// <summary>Gets the list of recorded value writes.</summary>
public List<(string NodeId, uint Quality)> ValueWrites => ValueWriteQueue.ToList();
public List<(string NodeId, uint Quality, AddressSpaceRealm Realm)> ValueWrites => ValueWriteQueue.ToList();
/// <summary>Records a value write (NodeId + quality).</summary>
/// <param name="nodeId">The node ID.</param>
@@ -1593,7 +1791,7 @@ public sealed class AddressSpaceApplierTests
/// <param name="statusCode">The OPC UA status code, sub-code included.</param>
/// <param name="sourceTimestampUtc">The source timestamp in UTC.</param>
public void WriteValue(string nodeId, object? value, uint statusCode, DateTime sourceTimestampUtc, AddressSpaceRealm realm = AddressSpaceRealm.Uns)
=> ValueWriteQueue.Enqueue((nodeId, statusCode));
=> ValueWriteQueue.Enqueue((nodeId, statusCode, realm));
/// <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>