fix(address-space): announce surgically-added raw nodes; equipment-with-references removal stays surgical (#599)
- AnnounceAddedNodes raises NodeAdded in the Raw realm for the existing parents of added raw containers/tags (ComputeRawAddAnnouncements); Uns side unchanged. - ApplyPureRemove removes UNS reference variables BEFORE the equipment subtree, so a reference the subtree's id-prefix scope reaches no longer ratchets to a full rebuild (03/P5). A NotSubsumed filter was not used: in production a reference NodeId is Area/Line/Equipment/Name, outside the subtree scope, so filtering would leak it. The matching RemoveEquipmentSubtree severing (03/S20) is in the node-manager commit. - UnsReferenceBackingParityTests: reflection guard discovering the classifier's surgical raw-tag whitelist and requiring a same-typed mirror field (03/U11). Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
+12
-3
@@ -99,9 +99,18 @@ mirror **in place** (its MonitoredItems survive); a re-point or effective-name c
|
||||
removes and recreates **that one mirror node**. See [`Uns.md`](Uns.md) and
|
||||
[`plans/2026-08-15-surgical-deploys.md`](plans/2026-08-15-surgical-deploys.md).
|
||||
|
||||
⚠️ A surgically added raw container/tag is materialised but **not** announced —
|
||||
`AnnounceAddedNodes` is UNS-only, so model-aware clients see new raw nodes on their next
|
||||
browse. This mattered less when every add forced a rebuild (and therefore a re-browse).
|
||||
A surgically added raw container/tag is **announced** (Gitea #599): `AnnounceAddedNodes`
|
||||
raises a `GeneralModelChangeEvent(NodeAdded)` in the **Raw** realm for each distinct
|
||||
existing parent of the added nodes (a parent that is itself new in the same deploy is
|
||||
skipped — its own parent is announced, and a re-browse there finds the whole new subtree;
|
||||
a new top-level folder announces itself). UNS-side adds announce in the Uns realm as
|
||||
before. Until #599 raw adds were materialised but never announced, so model-aware clients
|
||||
only saw them on their next browse — invisible while every add still forced a rebuild.
|
||||
|
||||
Removing an equipment that holds UNS references is also surgical: the references are
|
||||
removed (terminal Bad, then `RemoveVariableNode`, which severs the backing raw node's
|
||||
inverse `Organizes` edge) **before** the equipment subtree, and the subtree removal severs
|
||||
the same edge for anything it reaches itself (archreview 03/P5 + 03/S20).
|
||||
|
||||
## The `Calculation` driver
|
||||
|
||||
|
||||
@@ -397,29 +397,40 @@ public sealed class AddressSpaceApplier
|
||||
if (!SafeRemoveAlarmCondition(surgical, alarm.ScriptedAlarmId, AddressSpaceRealm.Uns)) 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, AddressSpaceRealm.Uns)) return false;
|
||||
}
|
||||
|
||||
// v3 Batch 4 — removed UNS reference variables (Uns realm, plain value nodes): terminal Bad then
|
||||
// in-place remove.
|
||||
//
|
||||
// WHAT REACHES THIS LOOP (the "→ rebuild" this comment used to claim is long dead — re-points are
|
||||
// surgical now): only genuine DEREFERENCES, i.e. the equipment dropped the reference row, so the node
|
||||
// is gone for good. A CHANGED reference never lands in RemovedUnsReferenceVariables at all — the
|
||||
// planner reports it as a UnsReferenceDelta, which Apply routes above either to an in-place
|
||||
// UpdateTagAttributes (identity unchanged) or to its own remove-the-previous + recreate-the-current
|
||||
// pair (identity changed). That second shape does remove a node, but it removes it from Apply's delta
|
||||
// loop, not from here.
|
||||
// is gone for good — INCLUDING every reference of a removed equipment, which the planner lists here
|
||||
// too. A CHANGED reference never lands in RemovedUnsReferenceVariables at all — the planner reports it
|
||||
// as a UnsReferenceDelta, which Apply routes above either to an in-place UpdateTagAttributes (identity
|
||||
// unchanged) or to its own remove-the-previous + recreate-the-current pair (identity changed). That
|
||||
// second shape does remove a node, but it removes it from Apply's delta loop, not from here.
|
||||
//
|
||||
// Gitea #599 (archreview 03/P5): this runs BEFORE the equipment-subtree removal below, deliberately,
|
||||
// instead of skipping a removed equipment's references with the NotSubsumed filter the loops above
|
||||
// use. Whether the subtree removal reaches a reference depends on its NodeId, not on its parent: the
|
||||
// node manager scopes a subtree by id prefix, and a reference's NodeId is Area/Line/Equipment/Name
|
||||
// while the equipment folder's is its EquipmentId — so in production the subtree does NOT reach it,
|
||||
// and a filter would leak the node. Where an id does sit under the equipment id, the old order had the
|
||||
// subtree take the node first, this loop then reported it unknown, and the deploy ratcheted to a full
|
||||
// rebuild — severing every subscription on the server to remove one equipment. Removing references
|
||||
// first is correct for both shapes: each is still present, gets its terminal Bad, and goes through
|
||||
// RemoveVariableNode, which also severs its inverse Organizes edge on the surviving raw node.
|
||||
foreach (var v in plan.RemovedUnsReferenceVariables)
|
||||
{
|
||||
SafeWriteValue(v.NodeId, null, OpcUaStatus.Bad, ts, AddressSpaceRealm.Uns);
|
||||
if (!SafeRemoveVariable(surgical, v.NodeId, AddressSpaceRealm.Uns)) 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, AddressSpaceRealm.Uns)) return false;
|
||||
}
|
||||
|
||||
// v3 Batch 4 — removed RAW tags (Raw realm): an alarm-bearing raw tag is a condition node (terminal
|
||||
// RemovedConditionState + RemoveAlarmConditionNode); a plain raw value tag is a variable (terminal Bad
|
||||
// + RemoveVariableNode).
|
||||
@@ -522,10 +533,8 @@ public sealed class AddressSpaceApplier
|
||||
foreach (var a in plan.AddedAlarms) ids.Add(a.EquipmentId);
|
||||
// v3 Batch 4 / review I5 — an added UNS reference variable is materialised directly under its
|
||||
// equipment folder (MaterialiseUnsReferences passes EquipmentId as the parent), and the equipment
|
||||
// folder is a UNS-realm node — the realm AnnounceAddedNodes hardcodes. So it announces correctly with
|
||||
// no realm plumbing. (Added RAW containers/tags are still NOT announced: their parents are Raw-realm
|
||||
// nodes, which needs a realm-aware RaiseNodesAddedModelChange call — see the gap note in
|
||||
// OpcUaPublishActor.HandleRebuild.)
|
||||
// folder is a UNS-realm node, announced in the Uns realm like every id this method returns. Added RAW
|
||||
// containers/tags have Raw-realm parents and are computed separately (ComputeRawAddAnnouncements).
|
||||
foreach (var v in plan.AddedUnsReferenceVariables) ids.Add(v.EquipmentId);
|
||||
foreach (var eq in plan.AddedEquipment)
|
||||
ids.Add(string.IsNullOrWhiteSpace(eq.UnsLineId) ? eq.EquipmentId : eq.UnsLineId);
|
||||
@@ -535,6 +544,42 @@ public sealed class AddressSpaceApplier
|
||||
return list;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Gitea #599 (archreview 01/U-8 = 03/P6) — the Raw-realm counterpart of
|
||||
/// <see cref="ComputeAddAnnouncements"/>: the deduplicated, ordinal-sorted RAW parent node ids to announce
|
||||
/// after a non-rebuild apply materialised added raw containers (Folder/Driver/Device/TagGroup) and raw
|
||||
/// tags. Each added node announces its <c>ParentNodeId</c> — the exact parent
|
||||
/// <c>MaterialiseRawSubtree</c> hangs it under. A parent that is ITSELF added in the same plan is skipped:
|
||||
/// its own parent is announced, and a client re-browsing there discovers the whole new subtree, so the
|
||||
/// inner announcements would only be noise. A top-level container (no parent — it hangs under the shared
|
||||
/// root folder, which has no realm-qualified id to announce) announces itself, the same convention an
|
||||
/// added equipment with no line uses.
|
||||
/// </summary>
|
||||
/// <param name="plan">The plan whose added raw nodes' parents to announce.</param>
|
||||
/// <returns>The distinct affected Raw-realm parent node ids, ordinal-sorted.</returns>
|
||||
public IReadOnlyList<string> ComputeRawAddAnnouncements(AddressSpacePlan plan)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(plan);
|
||||
|
||||
var added = new HashSet<string>(StringComparer.Ordinal);
|
||||
foreach (var c in plan.AddedRawContainers) added.Add(c.NodeId);
|
||||
foreach (var t in plan.AddedRawTags) added.Add(t.NodeId);
|
||||
|
||||
var ids = new HashSet<string>(StringComparer.Ordinal);
|
||||
void Announce(string nodeId, string? parentNodeId)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(parentNodeId)) ids.Add(nodeId);
|
||||
else if (!added.Contains(parentNodeId)) ids.Add(parentNodeId);
|
||||
}
|
||||
|
||||
foreach (var c in plan.AddedRawContainers) Announce(c.NodeId, c.ParentNodeId);
|
||||
foreach (var t in plan.AddedRawTags) Announce(t.NodeId, t.ParentNodeId);
|
||||
|
||||
var list = ids.ToList();
|
||||
list.Sort(StringComparer.Ordinal);
|
||||
return list;
|
||||
}
|
||||
|
||||
/// <summary>The materialise-parent node id a tag/vtag's variable (or native-alarm condition) hangs
|
||||
/// under: the equipment folder when <paramref name="folderPath"/> is null/empty, else the FolderPath
|
||||
/// sub-folder. Mirrors the parent derivation the Materialise passes use.</summary>
|
||||
@@ -561,21 +606,29 @@ public sealed class AddressSpaceApplier
|
||||
|
||||
/// <summary>
|
||||
/// Announce a Part 3 <c>GeneralModelChangeEvent(NodeAdded)</c> per distinct affected parent from
|
||||
/// <see cref="ComputeAddAnnouncements"/>, so model-aware clients re-browse the parents the new nodes
|
||||
/// were added under. Called by <c>OpcUaPublishActor</c> AFTER the Materialise passes (the nodes exist
|
||||
/// by then) and ONLY on a non-rebuild apply — after a full rebuild the announcement is moot
|
||||
/// (subscriptions are dead anyway). Uses the existing, guard-covered
|
||||
/// <see cref="ComputeAddAnnouncements"/> (Uns realm) and <see cref="ComputeRawAddAnnouncements"/>
|
||||
/// (Raw realm), so model-aware clients re-browse the parents the new nodes were added under. Called by
|
||||
/// <c>OpcUaPublishActor</c> AFTER the Materialise passes (the nodes exist by then) and ONLY on a
|
||||
/// non-rebuild apply — after a full rebuild the announcement is moot (subscriptions are dead anyway).
|
||||
/// Uses the existing, guard-covered, realm-aware
|
||||
/// <see cref="IOpcUaAddressSpaceSink.RaiseNodesAddedModelChange"/> — no new sink member. Each call is
|
||||
/// Safe-wrapped: a faulting announcement can never break a deploy (the nodes already stand).
|
||||
/// <para>Gitea #599: until the raw half existed, surgically-added raw containers/tags were materialised
|
||||
/// but never announced, so a connected model-aware client could not discover them until it happened to
|
||||
/// re-browse. (Before surgical deploys every raw add rebuilt the address space, which hid the gap.)</para>
|
||||
/// </summary>
|
||||
/// <param name="plan">The plan whose added nodes to announce.</param>
|
||||
public void AnnounceAddedNodes(AddressSpacePlan plan)
|
||||
{
|
||||
foreach (var id in ComputeAddAnnouncements(plan))
|
||||
{
|
||||
try { _sink.RaiseNodesAddedModelChange(id, AddressSpaceRealm.Uns); }
|
||||
catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: RaiseNodesAddedModelChange threw for {Node}", id); }
|
||||
foreach (var id in ComputeAddAnnouncements(plan)) SafeAnnounce(id, AddressSpaceRealm.Uns);
|
||||
foreach (var id in ComputeRawAddAnnouncements(plan)) SafeAnnounce(id, AddressSpaceRealm.Raw);
|
||||
}
|
||||
|
||||
/// <summary>Raise one NodeAdded model change, swallowing (and Warning-logging) any sink fault.</summary>
|
||||
private void SafeAnnounce(string nodeId, AddressSpaceRealm realm)
|
||||
{
|
||||
try { _sink.RaiseNodesAddedModelChange(nodeId, realm); }
|
||||
catch (Exception ex) { _logger.LogWarning(ex, "AddressSpaceApplier: RaiseNodesAddedModelChange threw for {Node} ({Realm})", nodeId, realm); }
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -592,15 +592,11 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers
|
||||
// (subscriptions are dead anyway) — the guard skips it. AnnounceAddedNodes is Safe-wrapped
|
||||
// internally, so a faulting announce can never break the deploy.
|
||||
//
|
||||
// KNOWN GAP, and NOT a neutral pre-existing one — the delta work made it matter. Before this
|
||||
// branch, a plan carrying raw adds went through a full RebuildAddressSpace, so every client had to
|
||||
// re-browse anyway and the missing announcement changed nothing observable. Now those adds are
|
||||
// materialised surgically while existing subscriptions stay alive, so an unannounced add is
|
||||
// genuinely INVISIBLE until a client happens to browse again.
|
||||
// Added UNS reference variables ARE announced (their equipment folder is the parent, which is the
|
||||
// realm AnnounceAddedNodes hardcodes). Added RAW containers/tags are still not: the announce is
|
||||
// Uns-only, and a raw parent needs a realm-aware RaiseNodesAddedModelChange call. That is a real
|
||||
// follow-up, not an accepted limitation.
|
||||
// The announce is realm-aware (Gitea #599): UNS parents in the Uns realm, and the parents of
|
||||
// added RAW containers/tags in the Raw realm. It matters because of the delta work — before it, a
|
||||
// plan carrying raw adds went through a full RebuildAddressSpace, so every client re-browsed
|
||||
// anyway; now those adds are materialised surgically while existing subscriptions stay alive, and
|
||||
// an unannounced add would stay invisible until a client happened to browse again.
|
||||
if (!outcome.RebuildCalled) _applier.AnnounceAddedNodes(plan);
|
||||
|
||||
OtOpcUaTelemetry.OpcUaSinkWrite.Add(1, new KeyValuePair<string, object?>("kind", "rebuild"));
|
||||
|
||||
@@ -1459,6 +1459,62 @@ public sealed class AddressSpaceApplierTests
|
||||
new[] { "eq-1", V3NodeIds.Uns("eq-1", "Diag"), "line-7" }.OrderBy(x => x));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Gitea #599 (archreview 01/U-8 = 03/P6) — surgically-added RAW nodes are announced in the RAW realm.
|
||||
/// A new tag under an existing device announces that device; a new TagGroup carrying a new tag under a
|
||||
/// new Device announces only the Device's existing parent — the inner parents are themselves new, and a
|
||||
/// client re-browsing the outer one discovers the whole new subtree. Before the fix only Uns-realm
|
||||
/// parents were announced, so these adds were invisible to connected model-aware clients.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void AnnounceAddedNodes_announces_added_raw_nodes_in_the_raw_realm()
|
||||
{
|
||||
var sink = new RecordingSink();
|
||||
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
|
||||
|
||||
var plan = EmptyPlan with
|
||||
{
|
||||
AddedRawContainers = new[]
|
||||
{
|
||||
new RawContainerNode("Plant/Modbus/dev2", "Plant/Modbus", "dev2", RawNodeKind.Device),
|
||||
new RawContainerNode("Plant/Modbus/dev2/grp", "Plant/Modbus/dev2", "grp", RawNodeKind.TagGroup),
|
||||
},
|
||||
AddedRawTags = new[]
|
||||
{
|
||||
RawTag("Plant/Modbus/dev2/grp/speed") with { ParentNodeId = "Plant/Modbus/dev2/grp" },
|
||||
RawTag("Plant/Modbus/dev1/temp"), // parent Plant/Modbus/dev1 already exists
|
||||
},
|
||||
// A UNS add in the same plan still announces in the Uns realm.
|
||||
AddedUnsReferenceVariables = new[] { UnsRef("filling/line-1/station-1/speed", "speed") },
|
||||
};
|
||||
|
||||
applier.AnnounceAddedNodes(plan);
|
||||
|
||||
sink.ModelChangeRealmQueue.OrderBy(x => x.NodeId, StringComparer.Ordinal).ToArray().ShouldBe(new[]
|
||||
{
|
||||
("Plant/Modbus", AddressSpaceRealm.Raw),
|
||||
("Plant/Modbus/dev1", AddressSpaceRealm.Raw),
|
||||
("eq-1", AddressSpaceRealm.Uns),
|
||||
});
|
||||
}
|
||||
|
||||
/// <summary>A top-level raw container (no parent: it hangs under the shared root, which has no
|
||||
/// realm-qualified id) announces itself; duplicates collapse; the Uns-side computation is unaffected.</summary>
|
||||
[Fact]
|
||||
public void ComputeRawAddAnnouncements_top_level_container_announces_itself_and_dedups()
|
||||
{
|
||||
var applier = new AddressSpaceApplier(new RecordingSink(), NullLogger<AddressSpaceApplier>.Instance);
|
||||
|
||||
var plan = EmptyPlan with
|
||||
{
|
||||
AddedRawContainers = new[] { new RawContainerNode("Plant2", null, "Plant2", RawNodeKind.Folder) },
|
||||
AddedRawTags = new[] { RawTag("Plant/Modbus/dev1/a"), RawTag("Plant/Modbus/dev1/b") },
|
||||
};
|
||||
|
||||
applier.ComputeRawAddAnnouncements(plan).ShouldBe(new[] { "Plant/Modbus/dev1", "Plant2" });
|
||||
applier.ComputeAddAnnouncements(plan).ShouldBeEmpty();
|
||||
}
|
||||
|
||||
// ----- 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
|
||||
@@ -1963,7 +2019,13 @@ public sealed class AddressSpaceApplierTests
|
||||
public List<string> ModelChangeCalls => ModelChangeQueue.ToList();
|
||||
/// <summary>Records a NodeAdded model-change announcement.</summary>
|
||||
/// <param name="affectedNodeId">The node under which discovered nodes were added.</param>
|
||||
public void RaiseNodesAddedModelChange(string affectedNodeId, AddressSpaceRealm realm = AddressSpaceRealm.Uns) => ModelChangeQueue.Enqueue(affectedNodeId);
|
||||
public void RaiseNodesAddedModelChange(string affectedNodeId, AddressSpaceRealm realm = AddressSpaceRealm.Uns)
|
||||
{
|
||||
ModelChangeQueue.Enqueue(affectedNodeId);
|
||||
ModelChangeRealmQueue.Enqueue((affectedNodeId, realm));
|
||||
}
|
||||
/// <summary>Gets the NodeAdded announcements WITH the realm each was raised in (Gitea #599).</summary>
|
||||
public ConcurrentQueue<(string NodeId, AddressSpaceRealm Realm)> ModelChangeRealmQueue { get; } = new();
|
||||
public void WireAlarmNotifiers(string alarmNodeId, AddressSpaceRealm alarmRealm, IReadOnlyList<string> notifierFolderNodeIds, AddressSpaceRealm notifierFolderRealm) { }
|
||||
public void AddReference(string sourceNodeId, AddressSpaceRealm sourceRealm, string targetNodeId, AddressSpaceRealm targetRealm, string referenceType = "Organizes") { }
|
||||
}
|
||||
|
||||
@@ -182,6 +182,104 @@ public sealed class NodeManagerSurgicalRemoveTests : IDisposable
|
||||
await host.DisposeAsync();
|
||||
}
|
||||
|
||||
// ---------- Gitea #599 (archreview 03/P5 + 03/S20): equipment-with-references removal ----------
|
||||
|
||||
/// <summary>
|
||||
/// S20 — the subtree removal severs the inverse half of every cross-tree <c>Organizes</c> edge a removed
|
||||
/// variable holds. The raw node survives a UNS-scoped removal, so without this it keeps a browsable,
|
||||
/// unresolvable reference to a node that no longer exists — the hazard <c>RemoveVariableNode</c> was
|
||||
/// already fixed for (2026-08-15 review I3), left open on the subtree path.
|
||||
/// </summary>
|
||||
[Trait("Category", "Unit")]
|
||||
[Fact]
|
||||
public async Task RemoveEquipmentSubtree_severs_the_inverse_Organizes_edge_on_the_surviving_raw_node()
|
||||
{
|
||||
var (host, server) = await BootAsync();
|
||||
try
|
||||
{
|
||||
var nm = server.NodeManager!;
|
||||
nm.EnsureVariable("Plant/Modbus/dev1/temp", null, "temp", "Float", writable: false, realm: AddressSpaceRealm.Raw);
|
||||
nm.EnsureFolder("eq-1", parentNodeId: null, displayName: "Equipment 1", realm: AddressSpaceRealm.Uns);
|
||||
nm.EnsureVariable("eq-1/Temp", "eq-1", "Temp", "Float", writable: false, realm: AddressSpaceRealm.Uns);
|
||||
nm.AddReference("eq-1/Temp", AddressSpaceRealm.Uns, "Plant/Modbus/dev1/temp", AddressSpaceRealm.Raw);
|
||||
var mirror = nm.TryGetVariable("eq-1/Temp", AddressSpaceRealm.Uns)!.NodeId;
|
||||
InverseOrganizesTo(nm, mirror).ShouldBeTrue(); // precondition: the edge really is there
|
||||
|
||||
nm.RemoveEquipmentSubtree("eq-1", AddressSpaceRealm.Uns).ShouldBeTrue();
|
||||
|
||||
nm.TryGetVariable("Plant/Modbus/dev1/temp", AddressSpaceRealm.Raw).ShouldNotBeNull();
|
||||
InverseOrganizesTo(nm, mirror).ShouldBeFalse();
|
||||
}
|
||||
finally
|
||||
{
|
||||
await host.DisposeAsync();
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// P5 — removing an equipment that holds UNS references is a scoped in-place removal, NOT a full rebuild,
|
||||
/// and leaves the surviving raw node with no dangling inverse edge. Run for both id shapes: a reference
|
||||
/// whose NodeId sits under the equipment id (so the subtree removal reaches it — before the fix the
|
||||
/// reference loop then found it gone and ratcheted to a rebuild, severing every subscription on the
|
||||
/// server), and the production shape, an <c>Area/Line/Equipment/Name</c> NodeId parented to an
|
||||
/// id-named equipment folder (which the subtree's id-prefix scope does not reach).
|
||||
/// </summary>
|
||||
[Trait("Category", "Unit")]
|
||||
[Theory]
|
||||
[InlineData("eq-1/Temp")]
|
||||
[InlineData("Area/Line/Eq/Temp")]
|
||||
public async Task Removing_equipment_with_references_is_surgical_and_leaves_no_dangling_edge(string mirrorId)
|
||||
{
|
||||
var (host, server) = await BootAsync();
|
||||
try
|
||||
{
|
||||
var nm = server.NodeManager!;
|
||||
nm.EnsureVariable("Plant/Modbus/dev1/temp", null, "temp", "Float", writable: false, realm: AddressSpaceRealm.Raw);
|
||||
nm.EnsureFolder("eq-1", parentNodeId: null, displayName: "Equipment 1", realm: AddressSpaceRealm.Uns);
|
||||
nm.EnsureVariable(mirrorId, "eq-1", "Temp", "Float", writable: false, realm: AddressSpaceRealm.Uns);
|
||||
nm.AddReference(mirrorId, AddressSpaceRealm.Uns, "Plant/Modbus/dev1/temp", AddressSpaceRealm.Raw);
|
||||
// A bystander whose value must survive (a rebuild would drop it).
|
||||
nm.EnsureFolder("eq-2", parentNodeId: null, displayName: "Equipment 2", realm: AddressSpaceRealm.Uns);
|
||||
nm.EnsureVariable("eq-2/S", "eq-2", "S", "Float", writable: false, realm: AddressSpaceRealm.Uns);
|
||||
var mirror = nm.TryGetVariable(mirrorId, AddressSpaceRealm.Uns)!.NodeId;
|
||||
|
||||
var plan = new AddressSpacePlan(
|
||||
AddedEquipment: Array.Empty<EquipmentNode>(),
|
||||
RemovedEquipment: new[] { new EquipmentNode("eq-1", "Equipment 1", "") },
|
||||
ChangedEquipment: Array.Empty<AddressSpacePlan.EquipmentDelta>(),
|
||||
AddedDrivers: Array.Empty<DriverInstancePlan>(),
|
||||
RemovedDrivers: Array.Empty<DriverInstancePlan>(),
|
||||
ChangedDrivers: Array.Empty<AddressSpacePlan.DriverDelta>(),
|
||||
AddedAlarms: Array.Empty<ScriptedAlarmPlan>(),
|
||||
RemovedAlarms: Array.Empty<ScriptedAlarmPlan>(),
|
||||
ChangedAlarms: Array.Empty<AddressSpacePlan.AlarmDelta>())
|
||||
{
|
||||
RemovedUnsReferenceVariables = new[]
|
||||
{
|
||||
new UnsReferenceVariable("ref-1", "eq-1", mirrorId, "Temp", "Plant/Modbus/dev1/temp", "Float", Writable: false),
|
||||
},
|
||||
};
|
||||
|
||||
var outcome = new AddressSpaceApplier(new SdkAddressSpaceSink(nm),
|
||||
Microsoft.Extensions.Logging.Abstractions.NullLogger<AddressSpaceApplier>.Instance).Apply(plan);
|
||||
|
||||
outcome.RebuildCalled.ShouldBeFalse();
|
||||
nm.TryGetVariable(mirrorId, AddressSpaceRealm.Uns).ShouldBeNull();
|
||||
nm.TryGetFolder("eq-1").ShouldBeNull();
|
||||
nm.TryGetVariable("eq-2/S", AddressSpaceRealm.Uns).ShouldNotBeNull(); // positive control: no rebuild
|
||||
nm.TryGetVariable("Plant/Modbus/dev1/temp", AddressSpaceRealm.Raw).ShouldNotBeNull();
|
||||
InverseOrganizesTo(nm, mirror).ShouldBeFalse();
|
||||
}
|
||||
finally
|
||||
{
|
||||
await host.DisposeAsync();
|
||||
}
|
||||
}
|
||||
|
||||
private static bool InverseOrganizesTo(OtOpcUaNodeManager nm, NodeId mirror) =>
|
||||
nm.GetNodeReferences("Plant/Modbus/dev1/temp", AddressSpaceRealm.Raw)
|
||||
.Any(r => r.ReferenceTypeId == ReferenceTypeIds.Organizes && r.IsInverse && r.TargetId == mirror);
|
||||
|
||||
private async Task<(OpcUaApplicationHost Host, OtOpcUaSdkServer Server)> BootAsync()
|
||||
{
|
||||
var host = new OpcUaApplicationHost(
|
||||
|
||||
@@ -0,0 +1,158 @@
|
||||
using System.Reflection;
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Gitea #599 / archreview 03/U11 — a mechanical guard on the <c>Backing*</c>-field parity invariant.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// A non-alarm raw-tag delta over the classifier's surgical whitelist is applied IN PLACE on the raw
|
||||
/// node, and every referencing UNS mirror is repaired only because the same edit makes its
|
||||
/// <see cref="UnsReferenceVariable"/> record unequal — the planner then emits a coincident
|
||||
/// <c>UnsReferenceDelta</c>. That works only while every whitelisted raw-tag field the mirror inherits
|
||||
/// has a counterpart on <see cref="UnsReferenceVariable"/> (same name, or <c>Backing</c> + name).
|
||||
/// Omitting one is silent: the raw node updates, the mirror stays stale, and nothing fails. Until this
|
||||
/// test the invariant was enforced by comments alone.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// The whitelist is DISCOVERED, not restated: each <see cref="RawTagPlan"/> property is perturbed in
|
||||
/// isolation and asked of <see cref="AddressSpaceChangeClassifier.RawTagDeltaIsSurgicalEligible"/>. So
|
||||
/// widening the classifier whitelist without mirroring the field fails here, and so does adding a
|
||||
/// <c>Backing*</c> field that names no raw-tag field.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public sealed class UnsReferenceBackingParityTests
|
||||
{
|
||||
/// <summary>
|
||||
/// Surgically-eligible raw-tag fields that a UNS mirror deliberately does NOT inherit, each with the
|
||||
/// reason. Anything else the classifier lets through surgically must be mirrored.
|
||||
/// </summary>
|
||||
private static readonly IReadOnlyDictionary<string, string> NotInheritedByMirror = new Dictionary<string, string>
|
||||
{
|
||||
// Node-inert for a value tag: it only drives native-alarm notifier wiring, and an alarm-bearing delta
|
||||
// never takes the surgical path. The mirror is the referencing side, so it has nothing to inherit.
|
||||
[nameof(RawTagPlan.ReferencingEquipmentPaths)] = "notifier wiring only; the mirror is the referencing side",
|
||||
};
|
||||
|
||||
private static RawTagPlan Baseline() => new(
|
||||
TagId: "tag-1",
|
||||
NodeId: "MAIN/modbus/dev/grp/temp",
|
||||
ParentNodeId: "MAIN/modbus/dev/grp",
|
||||
DriverInstanceId: "drv-1",
|
||||
Name: "temp",
|
||||
DataType: "Float",
|
||||
Writable: false,
|
||||
Alarm: null,
|
||||
ReferencingEquipmentPaths: new[] { "Area/Line/Eq" },
|
||||
IsHistorized: false,
|
||||
HistorianTagname: null,
|
||||
IsArray: false,
|
||||
ArrayLength: null,
|
||||
Realm: AddressSpaceRealm.Raw);
|
||||
|
||||
/// <summary>The raw-tag fields the classifier lets through surgically, found by perturbing each one.</summary>
|
||||
private static IReadOnlyList<PropertyInfo> SurgicallyEligibleRawTagFields()
|
||||
{
|
||||
var baseline = Baseline();
|
||||
var eligible = new List<PropertyInfo>();
|
||||
foreach (var p in RecordProperties(typeof(RawTagPlan)))
|
||||
{
|
||||
var current = Perturb(baseline, p);
|
||||
if (AddressSpaceChangeClassifier.RawTagDeltaIsSurgicalEligible(new AddressSpacePlan.RawTagDelta(baseline, current)))
|
||||
eligible.Add(p);
|
||||
}
|
||||
return eligible;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Every raw-tag field the classifier applies surgically is mirrored on the UNS reference record (or is
|
||||
/// explicitly listed as not inherited) — with the same type, so the planner's record equality sees the
|
||||
/// edit.
|
||||
/// </summary>
|
||||
[Trait("Category", "Unit")]
|
||||
[Fact]
|
||||
public void Every_surgically_eligible_raw_tag_field_is_mirrored_on_the_UNS_reference_record()
|
||||
{
|
||||
var eligible = SurgicallyEligibleRawTagFields();
|
||||
|
||||
// Positive control: the perturbation machinery really does find the whitelist. If it found nothing,
|
||||
// the loop below would pass vacuously.
|
||||
eligible.Select(p => p.Name).ShouldBe(
|
||||
new[]
|
||||
{
|
||||
nameof(RawTagPlan.DataType), nameof(RawTagPlan.Writable), nameof(RawTagPlan.ReferencingEquipmentPaths),
|
||||
nameof(RawTagPlan.IsHistorized), nameof(RawTagPlan.HistorianTagname), nameof(RawTagPlan.IsArray),
|
||||
nameof(RawTagPlan.ArrayLength),
|
||||
},
|
||||
ignoreOrder: true);
|
||||
|
||||
var mirror = RecordProperties(typeof(UnsReferenceVariable)).ToDictionary(p => p.Name, StringComparer.Ordinal);
|
||||
foreach (var p in eligible)
|
||||
{
|
||||
if (NotInheritedByMirror.ContainsKey(p.Name)) continue;
|
||||
|
||||
var counterpart = mirror.GetValueOrDefault(p.Name) ?? mirror.GetValueOrDefault("Backing" + p.Name);
|
||||
counterpart.ShouldNotBeNull(
|
||||
$"RawTagPlan.{p.Name} is applied surgically on the raw node, but UnsReferenceVariable has neither " +
|
||||
$"'{p.Name}' nor 'Backing{p.Name}' — a raw-side edit of it would leave every referencing UNS mirror stale.");
|
||||
counterpart.PropertyType.ShouldBe(p.PropertyType, $"UnsReferenceVariable.{counterpart.Name} vs RawTagPlan.{p.Name}");
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Every <c>Backing*</c> field on the UNS reference record names a real raw-tag field of the same
|
||||
/// type — a misspelt or orphaned mirror field repairs nothing.</summary>
|
||||
[Trait("Category", "Unit")]
|
||||
[Fact]
|
||||
public void Every_Backing_field_names_a_raw_tag_field_of_the_same_type()
|
||||
{
|
||||
var raw = RecordProperties(typeof(RawTagPlan)).ToDictionary(p => p.Name, StringComparer.Ordinal);
|
||||
var backing = RecordProperties(typeof(UnsReferenceVariable))
|
||||
.Where(p => p.Name.StartsWith("Backing", StringComparison.Ordinal) && p.Name != nameof(UnsReferenceVariable.BackingRawPath))
|
||||
.ToList();
|
||||
|
||||
backing.ShouldNotBeEmpty();
|
||||
foreach (var b in backing)
|
||||
{
|
||||
var source = raw.GetValueOrDefault(b.Name["Backing".Length..]);
|
||||
source.ShouldNotBeNull($"UnsReferenceVariable.{b.Name} names no RawTagPlan field");
|
||||
source.PropertyType.ShouldBe(b.PropertyType, $"UnsReferenceVariable.{b.Name} vs RawTagPlan.{source.Name}");
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>The allow-list stays honest: every entry must still be a surgically-eligible raw-tag field.</summary>
|
||||
[Trait("Category", "Unit")]
|
||||
[Fact]
|
||||
public void Not_inherited_allow_list_names_only_surgically_eligible_fields()
|
||||
{
|
||||
var eligible = SurgicallyEligibleRawTagFields().Select(p => p.Name).ToHashSet(StringComparer.Ordinal);
|
||||
foreach (var name in NotInheritedByMirror.Keys) eligible.ShouldContain(name);
|
||||
}
|
||||
|
||||
private static IEnumerable<PropertyInfo> RecordProperties(Type t) =>
|
||||
t.GetProperties(BindingFlags.Public | BindingFlags.Instance)
|
||||
.Where(p => p.CanWrite && p.Name != "EqualityContract");
|
||||
|
||||
/// <summary>A copy of <paramref name="source"/> differing ONLY in <paramref name="property"/>.</summary>
|
||||
private static RawTagPlan Perturb(RawTagPlan source, PropertyInfo property)
|
||||
{
|
||||
var clone = (RawTagPlan)typeof(RawTagPlan).GetMethod("<Clone>$")!.Invoke(source, null)!;
|
||||
var value = property.GetValue(source);
|
||||
object? changed = property.PropertyType switch
|
||||
{
|
||||
var t when t == typeof(bool) => !(bool)value!,
|
||||
var t when t == typeof(string) => (string?)value + "-changed",
|
||||
var t when t == typeof(uint?) => ((uint?)value ?? 0u) + 1u,
|
||||
var t when t == typeof(IReadOnlyList<string>) => ((IReadOnlyList<string>)value!).Append("Other/Eq").ToArray(),
|
||||
var t when t == typeof(AddressSpaceRealm) => (AddressSpaceRealm)value! == AddressSpaceRealm.Raw ? AddressSpaceRealm.Uns : AddressSpaceRealm.Raw,
|
||||
var t when t == typeof(EquipmentTagAlarmInfo) => new EquipmentTagAlarmInfo("OffNormalAlarm", 500),
|
||||
var t => throw new InvalidOperationException(
|
||||
$"RawTagPlan.{property.Name} has type {t.Name}, which this guard cannot perturb — add a case so the field is checked."),
|
||||
};
|
||||
property.SetValue(clone, changed);
|
||||
return clone;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user