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

fix(v3-batch4-wp4): alarm fan-out hardening — event-delivery test, resolution-failure meter, teardown guards (Wave C review M1/M2/M3/L1/L3)

Browse Source 
M1 — over-the-wire event-delivery proof (new NativeAlarmMultiNotifierEventDeliveryTests
in OpcUaServer.IntegrationTests): boots the real server, wires one condition to two
equipment folders, fires ONE transition, and asserts a Server-object subscriber gets
EXACTLY ONE event (the shared-InstanceStateSnapshot queue dedup), plus overlapping
Server + equipment-folder subscribers each get exactly one copy. Captures via the
subscription FastEventCallback keyed by ClientHandle (the per-item Notification/DequeueEvents
path delivers nothing for these conditions in the SDK client — the working capture is the
fast callback).

M2 — resolution-failure signal + path-agreement tripwire (AddressSpaceApplier): when a
native alarm HAS ReferencingEquipmentPaths but NONE resolve to an equipment folder, count
it as a failed node (degraded-apply meter) + LogWarning instead of a silent skip; documented
the DisplayName==Name coupling as a binding invariant. Tests:
Composer_referencing_equipment_paths_resolve_back_to_equipment_ids_in_the_applier (real
composer output → applier inversion) + ..._unresolvable_referencing_equipment_counts_as_failed.

M3 — WireAlarmNotifiers is now a RECONCILE (unwires a folder dropped from the supplied set,
bidirectionally) so a future surgical ChangedRawTags path can't leave a de-referenced
equipment receiving the alarm; binding guard comment added on the classifier's
ChangedRawTags→Rebuild branch. Test: WireAlarmNotifiers_reconciles_a_dropped_folder_out_of_the_notifier_set.

L1 — MaterialiseAlarmCondition's kind-swap drop-and-recreate now calls
UnwireAlarmNotifiers(conditionKey) before discarding the old instance (teardown symmetry).

L3 — BuildEquipmentIdByFolderPath LogWarnings on a duplicate Area/Line/Name collision
(defense-in-depth; UNS uniqueness prevents it).

Claude-Session: https://claude.ai/code/session_01LVneM3eh1UtJxEisFXgmox
This commit is contained in:
Joseph Doherty
2026-07-16 12:52:34 -04:00
parent 90e52a4415
commit 3cf3576c75
6 changed files with 465 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs
+37 -8
View File
@@ -716,8 +716,21 @@ public sealed class AddressSpaceApplier
.Select(id => id!)
.Distinct(StringComparer.Ordinal)
.ToList();
if (equipFolderNodeIds.Count > 0 &&
!SafeWireAlarmNotifiers(t.NodeId, AddressSpaceRealm.Raw, equipFolderNodeIds, AddressSpaceRealm.Uns))
if (equipFolderNodeIds.Count == 0)
{
// Wave C review M2: the alarm HAS referencing equipment but NONE of its paths resolved
// to an EquipmentId folder — the native fan-out to those equipment is silently absent.
// Surface it (Warning + a failed-node tally so the degraded-apply meter fires) instead
// of swallowing zero operator signal. Today this only happens on a broken UNS ancestry /
// an invalid segment (the composer would have dropped the equipment too); the latent
// risk is a future editable Area/Line display-name feature breaking the path↔id coupling
// (see BuildEquipmentIdByFolderPath + AddressSpaceComposerPathParityTests).
_logger.LogWarning(
"AddressSpaceApplier: native alarm {Node} has {Count} referencing equipment path(s) but NONE resolved to an equipment folder — the alarm's multi-notifier fan-out to those equipment is absent (paths={Paths})",
t.NodeId, t.ReferencingEquipmentPaths.Count, string.Join(", ", t.ReferencingEquipmentPaths));
failed++;
}
else if (!SafeWireAlarmNotifiers(t.NodeId, AddressSpaceRealm.Raw, equipFolderNodeIds, AddressSpaceRealm.Uns))
{
failed++;
}
@@ -1082,13 +1095,17 @@ public sealed class AddressSpaceApplier
/// as name paths (<c>V3NodeIds.Uns(areaName, lineName, equipName)</c>), but the equipment folders were
/// materialised under their logical <c>EquipmentId</c> by <see cref="MaterialiseHierarchy"/>, so the
/// multi-notifier wiring must translate path → id. This inverts the composer's own equipment-folder-path
/// construction EXACTLY (area/line/equipment DisplayName == the UNS level Name, so the paths match
/// byte-for-byte); an invalid segment throws in <see cref="V3NodeIds.Uns"/> and is skipped (the same
/// drop the composer applies), so an entry the composer produced always resolves here.
/// construction EXACTLY — <b>path↔id coupling invariant:</b> the composer builds the path from the
/// Area/Line/Equipment <c>Name</c>; this inverts it via the projected <c>DisplayName</c>, which the
/// composer sets to that same <c>Name</c> at every level (verified by
/// <c>AddressSpaceComposerPathParityTests</c>). A future editable Area/Line display-name feature that
/// lets <c>DisplayName != Name</c> would silently break this inversion — that test is the tripwire.
/// An invalid segment throws in <see cref="V3NodeIds.Uns"/> and is skipped (the same drop the composer
/// applies), so an entry the composer produced always resolves here.
/// </summary>
/// <param name="composition">The composition carrying the UNS topology + equipment nodes.</param>
/// <returns>A map from each resolvable equipment folder path to its EquipmentId.</returns>
private static IReadOnlyDictionary<string, string> BuildEquipmentIdByFolderPath(AddressSpaceComposition composition)
private IReadOnlyDictionary<string, string> BuildEquipmentIdByFolderPath(AddressSpaceComposition composition)
{
var areaName = composition.UnsAreas
.GroupBy(a => a.UnsAreaId, StringComparer.Ordinal)
@@ -1103,8 +1120,20 @@ public sealed class AddressSpaceApplier
if (string.IsNullOrWhiteSpace(e.UnsLineId)) continue;
if (!lineByid.TryGetValue(e.UnsLineId, out var line)) continue;
if (!areaName.TryGetValue(line.UnsAreaId, out var aName)) continue;
try { map[V3NodeIds.Uns(aName, line.DisplayName, e.DisplayName)] = e.EquipmentId; }
catch (ArgumentException) { /* invalid segment — dropped, mirroring the composer */ }
string path;
try { path = V3NodeIds.Uns(aName, line.DisplayName, e.DisplayName); }
catch (ArgumentException) { continue; /* invalid segment — dropped, mirroring the composer */ }
// Wave C review L3: two equipment sharing an identical Area/Line/Name collapse to one id
// (last-write-wins). UNS uniqueness prevents this, so it is defense-in-depth: warn (mirroring the
// composer's own last-write-wins spot) rather than silently pick one — a collision here would send
// the alarm's notifier to the wrong equipment folder.
if (map.TryGetValue(path, out var existing) && !string.Equals(existing, e.EquipmentId, StringComparison.Ordinal))
{
_logger.LogWarning(
"AddressSpaceApplier: two equipment share the UNS folder path {Path} ({Existing} vs {New}) — the native-alarm notifier map collapses them (last-write-wins); UNS uniqueness should prevent this",
path, existing, e.EquipmentId);
}
map[path] = e.EquipmentId;
}
return map;
}
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs
+8
View File
@@ -58,6 +58,14 @@ public static class AddressSpaceChangeClassifier
// display-name-override) route to Rebuild as the safe default — WP3's applier owns any surgical
// in-place refinement for them. Evaluated BEFORE the add/remove split so a non-surgical change
// alongside pure adds still rebuilds.
//
// WP4 BINDING GUARD (Wave C review M3): ChangedRawTags → Rebuild is what keeps native-alarm notifier
// wiring correct today. A native alarm's set of referencing equipment lives in
// RawTagPlan.ReferencingEquipmentPaths (part of its record equality), so ADDING/DROPPING a reference
// makes the raw tag a ChangedRawTag → full rebuild → OtOpcUaNodeManager clears + re-wires the notifiers
// cleanly. ANY future surgical (non-rebuild) ChangedRawTags path MUST also re-wire/unwire the alarm
// notifiers (WireAlarmNotifiers is a reconcile — pass the tag's COMPLETE current referencing set — plus
// an explicit unwire on removal), else a de-referenced equipment keeps receiving the alarm's events.
if (plan.ChangedEquipment.Count > 0 ||
plan.ChangedAlarms.Count > 0 ||
plan.ChangedRawContainers.Count > 0 ||
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs
+32
View File
@@ -762,6 +762,12 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
// type/severity on redeploy) reflects cleanly instead of leaking the old node.
if (_alarmConditions.TryRemove(conditionKey, out var existing))
{
// Wave C review L1: unwire the OLD instance's notifier pairs BEFORE discarding it, so no
// referencing equipment folder keeps a dangling inverse-notifier entry to the dropped
// AlarmConditionState. Unreachable today (native=Raw/RawPath, scripted=Uns/ScriptedAlarmId — no
// same-key kind-swap ever carries wired notifiers), but this is the one asymmetric teardown
// path; keep it symmetric with RemoveAlarmConditionNode / RebuildAddressSpace.
UnwireAlarmNotifiers(conditionKey);
existing.Parent?.RemoveChild(existing);
PredefinedNodes?.Remove(existing.NodeId);
}
@@ -892,6 +898,16 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
private sealed record AlarmNotifierWiring(AlarmConditionState Alarm, List<FolderState> Folders);
/// <inheritdoc cref="IOpcUaAddressSpaceSink.WireAlarmNotifiers"/>
/// <remarks>
/// Wave C review M3 — this is a RECONCILE, not additive-only: it wires the supplied set AND unwires any
/// previously-tracked folder whose id is no longer in <paramref name="notifierFolderNodeIds"/>
/// (bidirectional), so a de-referenced equipment stops receiving the alarm. Today the applier always
/// passes the COMPLETE referencing-equipment set for the condition, and a de-reference arrives as a
/// <c>ChangedRawTags</c> full rebuild (which clears the wiring), so the unwire leg is a no-op on the
/// current paths — but it makes this method correct for a FUTURE surgical <c>ChangedRawTags</c> path
/// (see <c>AddressSpaceChangeClassifier</c>'s <c>ChangedRawTags → Rebuild</c> branch, which carries the
/// matching binding guard).
/// </remarks>
public void WireAlarmNotifiers(string alarmNodeId, AddressSpaceRealm alarmRealm,
IReadOnlyList<string> notifierFolderNodeIds, AddressSpaceRealm notifierFolderRealm)
{
@@ -925,6 +941,22 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
_alarmNotifierWiring[alarmKey] = wiring;
}
// The DESIRED notifier-folder keys (ns-qualified) for this condition — the full set the caller
// supplied, regardless of whether each is currently materialised. Reconcile against it below.
var desiredKeys = notifierFolderNodeIds
.Select(id => MapKey(notifierFolderRealm, id))
.ToHashSet(StringComparer.Ordinal);
// Reconcile (M3): unwire any previously-wired folder whose id is NO LONGER in the supplied set — a
// genuine de-reference (the equipment dropped its reference to the alarm's raw tag). Matched by the
// folder's ns-qualified NodeId string so a transiently-unmaterialised-but-still-referenced folder
// (id still present) is NOT unwired. Bidirectional so the folder's inverse entry is removed too.
foreach (var wired in wiring.Folders.Where(w => !desiredKeys.Contains(MapKey(w.NodeId))).ToList())
{
wiring.Alarm.RemoveNotifier(SystemContext, wired, bidirectional: true);
wiring.Folders.Remove(wired);
}
foreach (var folderNodeId in notifierFolderNodeIds)
{
if (!_folders.TryGetValue(MapKey(notifierFolderRealm, folderNodeId), out var folder))