fix(opcuaserver): degraded applies re-diff instead of losing nodes; in-place UNS mirror updates; sever inverse edges
C1 — a non-rebuild apply reporting failedNodes > 0 no longer advances _lastApplied. Pre-delta-materialisation the full passes re-ensured every node on every apply, which silently repaired any prior swallowed sink fault. The delta path removed that accidental self-healing: a failed node sat inside _lastApplied, never appeared in a diff set again, and EnsureVariable early-returns on the ones that landed — so it was lost until an unrelated change forced a rebuild. The advance also moved to AFTER the materialise passes, so a throwing pass leaves _lastApplied at the previous composition too. A rebuild still advances even with failures (the ratchet is its resync path). I2 — a UnsReferenceDelta whose identity is unchanged (same NodeId, EquipmentId and BackingRawPath) is now applied in place via UpdateTagAttributes(Uns) instead of remove+recreate, so the mirror's MonitoredItems survive an operator historizing or reshaping the raw tag behind them. Remove+recreate stays for identity-changing deltas (re-point, effective-name edit). I3 — RemoveVariableNode severs the inverse half of every outgoing Organizes edge, the variable-node counterpart of UnwireAlarmNotifiers. Rebuilds used to sweep these up; surgical applies made rebuilds rare, so they accumulated on the raw node for the life of the process. I5 — added UNS reference variables are announced (their equipment folder is the parent, already the Uns realm). Raw adds still are not; the gap comment now says so honestly rather than calling it neutral and pre-existing. I4 + M1 — corrected two comments that no longer described the code: the "re-point → rebuild" claim in ApplyPureRemove, and the "N lock acquisitions" rationale for the delta passes (Ensure* early-return on a dictionary probe before the lock).
This commit is contained in:
Joseph Doherty
@@ -244,25 +244,59 @@ public sealed class AddressSpaceApplier
|
||||
}
|
||||
if (!ok) { allApplied = false; break; }
|
||||
}
|
||||
// v3 UNS reference variables — REMOVE the previous node (terminal Bad first, so an in-flight
|
||||
// MonitoredItem observes the removal), and let the CURRENT node be (re)created by the
|
||||
// materialise pass that runs after Apply returns.
|
||||
// v3 UNS reference variables — TWO shapes, split on whether the delta moves the node's
|
||||
// IDENTITY (2026-08-15 review I2).
|
||||
//
|
||||
// WHY remove+recreate rather than an in-place update: a UnsReferenceDelta is either a re-point
|
||||
// (BackingRawPath moved — the node's Organizes edge must move with it, and the surgical sink
|
||||
// surface has no RemoveReference) or a display-name-override edit (the effective name is part of
|
||||
// the UNS NodeId, so the identity itself changes — an OPC UA NodeId is immutable). Remove+recreate
|
||||
// covers both shapes with one mechanism. The cost is that ONE node's monitored items, against
|
||||
// today's cost of a full rebuild killing every subscription server-wide.
|
||||
// (a) IDENTITY UNCHANGED (same NodeId, same EquipmentId, same BackingRawPath): only the
|
||||
// inherited Backing* / DataType / Writable attributes moved — the 875ec9fe repair shape,
|
||||
// produced whenever a raw tag's historian or array shape is edited under a reference that
|
||||
// was not itself touched. Nothing about the node's identity or its Organizes edge changes,
|
||||
// so an in-place UpdateTagAttributes in the Uns realm says everything that needs saying
|
||||
// AND the mirror's client MonitoredItems survive. Remove+recreate here would kill the
|
||||
// subscriptions of every client watching a UNS signal because someone historized the raw
|
||||
// tag behind it.
|
||||
//
|
||||
// WHERE the recreate happens: the publish actor's materialise passes, which run after Apply
|
||||
// returns. On a NON-rebuild apply that pass is now narrowed to the plan's added references
|
||||
// PLUS the CURRENT side of every delta removed here (surgical-deploys Task 6) — dropping the
|
||||
// currents from that subset would leave a re-pointed / renamed reference removed and never
|
||||
// recreated. Either way the ordering holds (remove here, recreate strictly after), and the
|
||||
// recreated node inherits its shape / historian tagname from the UnsReferenceVariable record.
|
||||
// (b) IDENTITY CHANGED — a re-point (BackingRawPath moved: the node's Organizes edge must move
|
||||
// with it, and the surgical sink surface has no RemoveReference) or an effective-name edit
|
||||
// (the effective name is part of the UNS NodeId, and an OPC UA NodeId is immutable). Remove
|
||||
// the PREVIOUS node here (terminal Bad first, so an in-flight MonitoredItem observes the
|
||||
// removal) and let the CURRENT node be created by the materialise pass after Apply returns.
|
||||
// The cost is that ONE node's monitored items, against today's cost of a full rebuild
|
||||
// killing every subscription server-wide.
|
||||
//
|
||||
// BOTH shapes leave the delta's Current in the publish actor's materialise subset, and that is
|
||||
// deliberate: for (b) the subset IS the recreate (dropping it would leave the reference removed
|
||||
// and never brought back), and for (a) it is a cheap idempotent no-op — EnsureVariable
|
||||
// early-returns on the already-present node and AddReference is idempotent on the unchanged
|
||||
// edge. Making the subset shape-aware would buy nothing and add a way for the two sides to
|
||||
// disagree about which references were repaired.
|
||||
foreach (var d in allApplied ? unsRefDeltas : Enumerable.Empty<AddressSpacePlan.UnsReferenceDelta>())
|
||||
{
|
||||
if (IsIdentityPreservingUnsRefDelta(d))
|
||||
{
|
||||
// Resolve writable + historian EXACTLY as MaterialiseUnsReferences does when it creates
|
||||
// the node: arrays are forced read-only, and a blank override falls back to the backing
|
||||
// RawPath (which IS the backing raw tag's NodeId).
|
||||
var writable = d.Current.Writable && !d.Current.BackingIsArray;
|
||||
var historian = d.Current.BackingIsHistorized
|
||||
? (string.IsNullOrWhiteSpace(d.Current.BackingHistorianTagname) ? d.Current.BackingRawPath : d.Current.BackingHistorianTagname)
|
||||
: null;
|
||||
bool updated;
|
||||
try
|
||||
{
|
||||
updated = surgical.UpdateTagAttributes(
|
||||
d.Current.NodeId, writable, historian, d.Current.DataType,
|
||||
d.Current.BackingIsArray, d.Current.BackingArrayLength, AddressSpaceRealm.Uns);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.LogError(ex, "AddressSpaceApplier: surgical UpdateTagAttributes threw for UNS reference {Node}", d.Current.NodeId);
|
||||
updated = false;
|
||||
}
|
||||
if (!updated) { allApplied = false; break; }
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!SafeWriteValue(d.Previous.NodeId, null, OpcUaStatus.Bad, ts, AddressSpaceRealm.Uns)) failedNodes++;
|
||||
if (!SafeRemoveVariable(surgical, d.Previous.NodeId, AddressSpaceRealm.Uns)) { allApplied = false; break; }
|
||||
}
|
||||
@@ -371,8 +405,15 @@ public sealed class AddressSpaceApplier
|
||||
}
|
||||
|
||||
// v3 Batch 4 — removed UNS reference variables (Uns realm, plain value nodes): terminal Bad then
|
||||
// in-place remove. A backing-raw rename is a re-point (Changed, → rebuild), NOT a removal, so a UNS
|
||||
// ref only reaches here on a genuine dereference (the equipment dropped the reference).
|
||||
// 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.
|
||||
foreach (var v in plan.RemovedUnsReferenceVariables)
|
||||
{
|
||||
SafeWriteValue(v.NodeId, null, OpcUaStatus.Bad, ts, AddressSpaceRealm.Uns);
|
||||
@@ -404,6 +445,22 @@ public sealed class AddressSpaceApplier
|
||||
return true;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 2026-08-15 review I2 — true when a <see cref="AddressSpacePlan.UnsReferenceDelta"/> can be applied
|
||||
/// as an IN-PLACE attribute update rather than a remove+recreate: the node's identity is untouched.
|
||||
/// Three fields make up that identity — the <c>NodeId</c> (an OPC UA NodeId is immutable, so a moved
|
||||
/// effective name is a different node), the <c>EquipmentId</c> (the node's PARENT, which
|
||||
/// <c>UpdateTagAttributes</c> cannot re-hang), and the <c>BackingRawPath</c> (the target of the node's
|
||||
/// <c>Organizes</c> edge, which the surgical sink surface has no way to move — there is no
|
||||
/// RemoveReference). Everything else on the record is a plain attribute the in-place update carries.
|
||||
/// </summary>
|
||||
/// <param name="d">The UNS reference delta to classify.</param>
|
||||
/// <returns><c>true</c> when the delta is attribute-only and may be applied in place.</returns>
|
||||
private static bool IsIdentityPreservingUnsRefDelta(AddressSpacePlan.UnsReferenceDelta d) =>
|
||||
string.Equals(d.Previous.NodeId, d.Current.NodeId, StringComparison.Ordinal) &&
|
||||
string.Equals(d.Previous.EquipmentId, d.Current.EquipmentId, StringComparison.Ordinal) &&
|
||||
string.Equals(d.Previous.BackingRawPath, d.Current.BackingRawPath, StringComparison.Ordinal);
|
||||
|
||||
/// <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, AddressSpaceRealm realm)
|
||||
@@ -463,6 +520,13 @@ public sealed class AddressSpaceApplier
|
||||
foreach (var t in plan.AddedEquipmentTags) ids.Add(MaterialiseParent(t.EquipmentId, t.FolderPath));
|
||||
foreach (var v in plan.AddedEquipmentVirtualTags) ids.Add(MaterialiseParent(v.EquipmentId, v.FolderPath));
|
||||
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.)
|
||||
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);
|
||||
|
||||
|
||||
@@ -2618,6 +2618,18 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
// caller (AddressSpaceApplier) falls back to a full rebuild (resync). Mirrors the per-node
|
||||
// teardown inside RebuildAddressSpace, scoped to this one id.
|
||||
if (!_variables.TryRemove(key, out var variable)) return false;
|
||||
// 2026-08-15 review I3: sever the INVERSE half of every cross-tree Organizes edge this node holds
|
||||
// BEFORE detaching it — the same hazard (and the same fix shape) as UnwireAlarmNotifiers on a
|
||||
// condition removal. AddReference wires the edge bidirectionally: forward on the UNS reference
|
||||
// variable, inverse (OrganizedBy) on the backing RAW node. The raw node SURVIVES this scoped
|
||||
// remove, so without this it keeps a dangling inverse reference to a node that no longer exists —
|
||||
// browsable, unresolvable, and permanent.
|
||||
//
|
||||
// Why it matters more now than it used to: a full RebuildAddressSpace clears every node and every
|
||||
// reference with it, so these edges were swept up on the next structural deploy. Surgical applies
|
||||
// made rebuilds rare, so the dangling edges now ACCUMULATE on the raw node for the life of the
|
||||
// process — one per re-point or effective-name edit of any reference that ever pointed at it.
|
||||
SeverOutgoingReferences(variable);
|
||||
variable.Parent?.RemoveChild(variable);
|
||||
PredefinedNodes?.Remove(variable.NodeId);
|
||||
// Drop the historized-tagname registration alongside the variable it maps (Phase C parity).
|
||||
@@ -2628,6 +2640,33 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
return true;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 2026-08-15 review I3 — remove the INVERSE half, held on the far node, of every FORWARD reference
|
||||
/// <paramref name="node"/> carries to another node this manager owns. The exact reverse of the
|
||||
/// bidirectional wiring in <see cref="AddReference"/>, and the variable-node counterpart of
|
||||
/// <see cref="UnwireAlarmNotifiers"/>. MUST be called under <c>Lock</c>.
|
||||
/// <para>Only FORWARD references are walked. A node's link to its own parent manifests as an INVERSE
|
||||
/// reference (the parent owns the forward half through <c>AddChild</c>), and the parent's side of that
|
||||
/// is severed by <c>Parent.RemoveChild</c> — walking inverse references too would try to unwire the
|
||||
/// parent link a second time. A forward reference whose target is not one of this manager's nodes is
|
||||
/// skipped: there is nothing here to mutate, and the target's own owner is responsible for it.</para>
|
||||
/// </summary>
|
||||
/// <param name="node">The node being removed, whose outgoing edges' inverse halves must be severed.</param>
|
||||
private void SeverOutgoingReferences(NodeState node)
|
||||
{
|
||||
var references = new List<IReference>();
|
||||
node.GetReferences(SystemContext, references);
|
||||
foreach (var r in references)
|
||||
{
|
||||
if (r.IsInverse || r.TargetId is null || r.TargetId.IsAbsolute) continue;
|
||||
var targetNodeId = (NodeId)r.TargetId;
|
||||
if (NodeId.IsNull(targetNodeId)) continue;
|
||||
if (ResolveNodeState(MapKey(targetNodeId)) is not { } target) continue;
|
||||
if (!target.RemoveReference(r.ReferenceTypeId, isInverse: true, node.NodeId)) continue;
|
||||
target.ClearChangeMasks(SystemContext, includeChildren: false);
|
||||
}
|
||||
}
|
||||
|
||||
/// <inheritdoc cref="ISurgicalAddressSpaceSink.RemoveAlarmConditionNode"/>
|
||||
public bool RemoveAlarmConditionNode(string alarmNodeId, AddressSpaceRealm realm)
|
||||
{
|
||||
|
||||
@@ -496,8 +496,6 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers
|
||||
}
|
||||
|
||||
var outcome = _applier.Apply(plan);
|
||||
_lastApplied = composition;
|
||||
_hasAppliedComposition = true;
|
||||
|
||||
// Sum swallowed per-node materialise failures across every pass together with Apply's own
|
||||
// removal-pass tally (archreview 01/S-1). A degraded apply used to vanish into per-node
|
||||
@@ -510,6 +508,41 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers
|
||||
failedNodes += _applier.MaterialiseHierarchy(composition);
|
||||
failedNodes += MaterialiseNodes(plan, composition, fullComposition: outcome.RebuildCalled);
|
||||
|
||||
// 2026-08-15 surgical-deploys review C1 — record the composition as applied ONLY when the apply was
|
||||
// clean, or when a rebuild ran. Two reasons this sits HERE (after the materialise passes) rather
|
||||
// than immediately after Apply:
|
||||
//
|
||||
// • A pass that THROWS outright leaves _lastApplied at the previous composition (the outer catch
|
||||
// below takes over), so the next deploy re-diffs against what is actually materialised.
|
||||
// • A DEGRADED non-rebuild apply (failedNodes > 0, no rebuild) must not advance either. Before the
|
||||
// delta materialisation the full passes re-ensured every node on every apply, which silently
|
||||
// repaired any prior swallowed sink fault — an accidental self-healing mechanism. The delta path
|
||||
// removed it: a node whose EnsureVariable failed is inside _lastApplied, never appears in a diff
|
||||
// set again, and EnsureVariable early-returns on the ones that DID land — so the failed node is
|
||||
// lost until some unrelated change happens to force a rebuild. Holding the composition back is
|
||||
// the replacement: the next deploy re-diffs the same composition and re-ensures what failed.
|
||||
//
|
||||
// What the re-diff costs, stated honestly: it also re-emits the previous apply's REMOVES. A
|
||||
// re-remove of an already-removed node returns false from the surgical sink, which trips the
|
||||
// applier's one-way ratchet into a full rebuild — the correct resync for an apply that has already
|
||||
// been logged at Error as degraded, and a bounded one (it happens once, then the address space is
|
||||
// known-good again).
|
||||
//
|
||||
// A rebuild DOES advance even with failures: it already re-materialised the whole composition, and
|
||||
// the ratchet is the resync path for anything it could not place. Holding it back would make every
|
||||
// subsequent deploy re-diff the entire address space as added.
|
||||
if (outcome.RebuildCalled || failedNodes == 0)
|
||||
{
|
||||
_lastApplied = composition;
|
||||
_hasAppliedComposition = true;
|
||||
}
|
||||
else
|
||||
{
|
||||
_log.Error(
|
||||
"OpcUaPublish: degraded non-rebuild apply (correlation={Correlation}, failedNodes={FailedNodes}) — NOT recording this composition as applied, so the next deploy re-diffs it and repairs the missing node(s)",
|
||||
msg.Correlation, failedNodes);
|
||||
}
|
||||
|
||||
// R2-07 T4b — on a NON-rebuild apply (PureAdd / AttributeOnly), the Materialise passes above
|
||||
// just created exactly the added nodes WITHOUT tearing anything down, so announce them with a
|
||||
// Part 3 GeneralModelChangeEvent(NodeAdded) per affected parent — model-aware clients then
|
||||
@@ -519,10 +552,15 @@ 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, PRE-EXISTING GAP (unchanged by the delta-materialise work above): ComputeAddAnnouncements
|
||||
// only walks the equipment-namespace add sets, so added RAW containers/tags and added UNS
|
||||
// reference variables are materialised but never announced. Model-aware clients pick them up on
|
||||
// their next unprompted browse.
|
||||
// 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.
|
||||
if (!outcome.RebuildCalled) _applier.AnnounceAddedNodes(plan);
|
||||
|
||||
OtOpcUaTelemetry.OpcUaSinkWrite.Add(1, new KeyValuePair<string, object?>("kind", "rebuild"));
|
||||
@@ -554,13 +592,20 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers
|
||||
/// the caller always runs it in full, because parents must exist before any add.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para><b>Why the delta path exists.</b> The passes are idempotent, but idempotent is not free:
|
||||
/// every <c>EnsureVariable</c> / <c>EnsureFolder</c> / <c>MaterialiseAlarmCondition</c> is a sink call
|
||||
/// that takes the node manager's lock and re-derives the node's identity before discovering there is
|
||||
/// nothing to do. Running them over the whole composition made the cost of ANY apply proportional to
|
||||
/// the size of the served address space rather than to the size of the change: adding one tag to a
|
||||
/// 50k-node server was ~50k sink calls, on the actor's pinned publish thread, with every client's
|
||||
/// publish blocked behind it.</para>
|
||||
/// <para><b>Why the delta path exists.</b> The passes are idempotent, but idempotent is not free.
|
||||
/// <c>EnsureFolder</c> / <c>EnsureVariable</c> early-return on a <c>ConcurrentDictionary</c> hit
|
||||
/// BEFORE taking the node manager's lock, so an already-standing node costs a dictionary probe rather
|
||||
/// than a lock acquisition — but <c>MaterialiseAlarmCondition</c> DOES take the lock every time, and
|
||||
/// the probes are still O(address space) per apply plus the composition walk that produces them.
|
||||
/// Running the passes over the whole composition made the cost of ANY apply proportional to the size
|
||||
/// of the served address space rather than to the size of the change: adding one tag to a 50k-node
|
||||
/// server was ~50k sink calls on the actor's pinned publish thread, with every client's publish
|
||||
/// blocked behind it. The delta path makes it O(change).</para>
|
||||
/// <para><b>What the full pass was silently doing, and its replacement.</b> Re-ensuring every node on
|
||||
/// every apply also REPAIRED any node a previous apply had swallowed a sink fault on — accidental
|
||||
/// self-healing nobody designed. The caller's degraded-apply no-advance rule (a non-rebuild apply
|
||||
/// reporting <c>failedNodes > 0</c> does not record its composition in <c>_lastApplied</c>, so the
|
||||
/// next deploy re-diffs and re-ensures it) is the deliberate replacement for it.</para>
|
||||
/// <para><b>Why <see cref="AddressSpaceApplyOutcome.RebuildCalled"/> is the branch key</b> rather than
|
||||
/// the classifier's kind: a surgical apply that hits a <c>false</c>/throw from the surgical sink falls
|
||||
/// back to a full <c>RebuildAddressSpace()</c> INSIDE <c>Apply</c>, which wipes every node. That plan
|
||||
|
||||
@@ -1401,6 +1401,31 @@ public sealed class AddressSpaceApplierTests
|
||||
applier.ComputeAddAnnouncements(noLine).ShouldBe(new[] { "eq-2" });
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 2026-08-15 review I5 — an added UNS reference variable announces its EQUIPMENT folder, which is the
|
||||
/// parent <c>MaterialiseUnsReferences</c> hangs it under and a UNS-realm node (the realm
|
||||
/// <c>AnnounceAddedNodes</c> hardcodes). Before the surgical-deploy branch a plan carrying adds went
|
||||
/// through a full rebuild, so every client re-browsed regardless and the missing announcement was
|
||||
/// invisible; now the add is surgical and an unannounced node stays unseen until some later browse.
|
||||
/// Two references under one equipment dedup to a single announcement.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void ComputeAddAnnouncements_added_uns_reference_announces_its_equipment()
|
||||
{
|
||||
var applier = new AddressSpaceApplier(new RecordingSink(), NullLogger<AddressSpaceApplier>.Instance);
|
||||
|
||||
var plan = EmptyPlan with
|
||||
{
|
||||
AddedUnsReferenceVariables = new[]
|
||||
{
|
||||
UnsRef("filling/line-1/station-1/speed", "speed"),
|
||||
UnsRef("filling/line-1/station-1/temp", "temp"),
|
||||
},
|
||||
};
|
||||
|
||||
applier.ComputeAddAnnouncements(plan).ShouldBe(new[] { "eq-1" });
|
||||
}
|
||||
|
||||
/// <summary>A plan with no additions yields no announcements.</summary>
|
||||
[Fact]
|
||||
public void ComputeAddAnnouncements_empty_add_plan_yields_none()
|
||||
@@ -1557,12 +1582,17 @@ public sealed class AddressSpaceApplierTests
|
||||
/// 2026-08-15 review — the combined plan a backing-tag HISTORIAN edit produces: a surgical raw-tag
|
||||
/// delta AND the coincident UNS-reference delta (the reference record carries the backing historian
|
||||
/// intent, so it goes unequal too). Both are non-rebuild kinds, so ONE apply must update the raw node
|
||||
/// in place AND tear the stale UNS mirror down for the materialise pass to recreate — with NO rebuild.
|
||||
/// Without the coincident delta the UNS mirror kept the OLD historian tagname and HistoryRead through
|
||||
/// the UNS NodeId served a different series than through the raw NodeId.
|
||||
/// in place AND repair the stale UNS mirror — with NO rebuild. Without the coincident delta the UNS
|
||||
/// mirror kept the OLD historian tagname and HistoryRead through the UNS NodeId served a different
|
||||
/// series than through the raw NodeId.
|
||||
/// <para><b>The mirror is repaired IN PLACE (review I2), not by remove+recreate.</b> This delta's
|
||||
/// IDENTITY is unchanged (same NodeId, same EquipmentId, same BackingRawPath) — only the inherited
|
||||
/// Backing* attributes moved — so <c>UpdateTagAttributes</c> in the Uns realm says everything that
|
||||
/// needs saying and the mirror's client MonitoredItems survive. Remove+recreate stays for the deltas
|
||||
/// that genuinely change identity (a re-point or an effective-name edit).</para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Backing_historian_edit_updates_raw_in_place_and_removes_the_stale_uns_mirror_without_rebuilding()
|
||||
public void Backing_historian_edit_updates_both_the_raw_node_and_the_uns_mirror_in_place()
|
||||
{
|
||||
var sink = new RecordingSink();
|
||||
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
|
||||
@@ -1590,21 +1620,26 @@ public sealed class AddressSpaceApplierTests
|
||||
sink.RebuildCalls.ShouldBe(0);
|
||||
|
||||
// Raw node: updated in place with the NEW tagname, in the Raw realm.
|
||||
var call = sink.SurgicalCalls.ShouldHaveSingleItem();
|
||||
call.NodeId.ShouldBe("Plant/Modbus/dev1/speed");
|
||||
call.Historian.ShouldBe("WW.New");
|
||||
call.Realm.ShouldBe(AddressSpaceRealm.Raw);
|
||||
var raw = sink.SurgicalCalls.Single(c => c.Realm == AddressSpaceRealm.Raw);
|
||||
raw.NodeId.ShouldBe("Plant/Modbus/dev1/speed");
|
||||
raw.Historian.ShouldBe("WW.New");
|
||||
|
||||
// UNS mirror: terminal Bad + in-place remove of the PREVIOUS node (the recreate rides the materialise
|
||||
// pass after Apply returns, which reads the CURRENT record's Backing* values).
|
||||
sink.ValueWrites.ShouldContain((unsBefore.NodeId, OpcUaStatus.Bad, AddressSpaceRealm.Uns));
|
||||
sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", unsBefore.NodeId, AddressSpaceRealm.Uns));
|
||||
// UNS mirror: the SAME in-place update, in the Uns realm, carrying the new inherited tagname.
|
||||
var uns = sink.SurgicalCalls.Single(c => c.Realm == AddressSpaceRealm.Uns);
|
||||
uns.NodeId.ShouldBe(unsAfter.NodeId);
|
||||
uns.Historian.ShouldBe("WW.New");
|
||||
|
||||
// …and NOT torn down: no remove, and no terminal Bad on the mirror's subscribers.
|
||||
sink.RemoveCalls.ShouldBeEmpty();
|
||||
sink.ValueWrites.ShouldNotContain((unsBefore.NodeId, OpcUaStatus.Bad, AddressSpaceRealm.Uns));
|
||||
}
|
||||
|
||||
/// <summary>The same combined-plan repair for a scalar→ARRAY flip on the backing tag: without it the UNS
|
||||
/// mirror stays scalar AND writable, bypassing the array read-only write gate through the UNS NodeId.</summary>
|
||||
/// mirror stays scalar AND writable, bypassing the array read-only write gate through the UNS NodeId.
|
||||
/// Identity is unchanged, so the mirror is repaired in place too (review I2) — and the in-place call must
|
||||
/// carry the array shape AND the forced read-only, exactly as a recreate would have.</summary>
|
||||
[Fact]
|
||||
public void Backing_shape_flip_removes_the_stale_uns_mirror_without_rebuilding()
|
||||
public void Backing_shape_flip_updates_the_uns_mirror_in_place_without_rebuilding()
|
||||
{
|
||||
var sink = new RecordingSink();
|
||||
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
|
||||
@@ -1624,8 +1659,16 @@ public sealed class AddressSpaceApplierTests
|
||||
|
||||
outcome.RebuildCalled.ShouldBeFalse();
|
||||
sink.RebuildCalls.ShouldBe(0);
|
||||
sink.SurgicalCalls.ShouldHaveSingleItem().IsArray.ShouldBeTrue();
|
||||
sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", unsBefore.NodeId, AddressSpaceRealm.Uns));
|
||||
sink.SurgicalCalls.Single(c => c.Realm == AddressSpaceRealm.Raw).IsArray.ShouldBeTrue();
|
||||
|
||||
var uns = sink.SurgicalCalls.Single(c => c.Realm == AddressSpaceRealm.Uns);
|
||||
uns.NodeId.ShouldBe(unsAfter.NodeId);
|
||||
uns.IsArray.ShouldBeTrue();
|
||||
uns.ArrayLength.ShouldBe(8u);
|
||||
// The reference row is authored Writable, but an array node is forced read-only — the same
|
||||
// `Writable && !BackingIsArray` rule MaterialiseUnsReferences applies when it creates the node.
|
||||
uns.Writable.ShouldBeFalse();
|
||||
sink.RemoveCalls.ShouldBeEmpty();
|
||||
}
|
||||
|
||||
/// <summary>The one-way rebuild ratchet holds for the new raw-tag path: a surgical update that reports the
|
||||
|
||||
@@ -190,6 +190,51 @@ public sealed class SdkAddressSpaceSinkTests : IDisposable
|
||||
await host.DisposeAsync();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 2026-08-15 review I3 — <c>RemoveVariableNode</c> must sever the INVERSE half of the cross-tree
|
||||
/// <c>Organizes</c> edge it holds, not just drop its own node. <c>AddReference</c> wires the edge
|
||||
/// bidirectionally (forward on the UNS reference variable, inverse on the backing RAW node), and the
|
||||
/// raw node SURVIVES a scoped remove — so without this the raw node keeps a browsable, unresolvable
|
||||
/// reference to a node that no longer exists. Rebuilds used to sweep these up; surgical applies made
|
||||
/// rebuilds rare, so they now accumulate one per re-point for the life of the process.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task RemoveVariableNode_severs_the_inverse_Organizes_edge_on_the_raw_target()
|
||||
{
|
||||
var (host, server) = await BootAsync();
|
||||
try
|
||||
{
|
||||
var nm = server.NodeManager!;
|
||||
var sink = new SdkAddressSpaceSink(nm);
|
||||
|
||||
sink.EnsureVariable("MAIN/modbus/dev/grp/temp", parentFolderNodeId: null, displayName: "Temp",
|
||||
dataType: "Float", writable: false, realm: AddressSpaceRealm.Raw);
|
||||
sink.EnsureVariable("Area/Line/Eq/Temperature", parentFolderNodeId: null, displayName: "Temperature",
|
||||
dataType: "Float", writable: false, realm: AddressSpaceRealm.Uns);
|
||||
sink.AddReference("Area/Line/Eq/Temperature", AddressSpaceRealm.Uns,
|
||||
"MAIN/modbus/dev/grp/temp", AddressSpaceRealm.Raw);
|
||||
|
||||
var unsVar = nm.TryGetVariable("Area/Line/Eq/Temperature", AddressSpaceRealm.Uns);
|
||||
unsVar.ShouldNotBeNull();
|
||||
// Precondition: the raw node really does hold the inverse half.
|
||||
nm.GetNodeReferences("MAIN/modbus/dev/grp/temp", AddressSpaceRealm.Raw)
|
||||
.Any(r => r.ReferenceTypeId == ReferenceTypeIds.Organizes && r.IsInverse &&
|
||||
r.TargetId == unsVar.NodeId).ShouldBeTrue();
|
||||
|
||||
nm.RemoveVariableNode("Area/Line/Eq/Temperature", AddressSpaceRealm.Uns).ShouldBeTrue();
|
||||
|
||||
// The raw node survives…
|
||||
nm.TryGetVariable("MAIN/modbus/dev/grp/temp", AddressSpaceRealm.Raw).ShouldNotBeNull();
|
||||
// …carrying NO reference back to the node that just went away.
|
||||
nm.GetNodeReferences("MAIN/modbus/dev/grp/temp", AddressSpaceRealm.Raw)
|
||||
.Any(r => r.TargetId == unsVar.NodeId).ShouldBeFalse();
|
||||
}
|
||||
finally
|
||||
{
|
||||
await host.DisposeAsync();
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Verifies that RebuildAddressSpace clears all registered variables.</summary>
|
||||
[Fact]
|
||||
public async Task RebuildAddressSpace_clears_all_registered_variables()
|
||||
|
||||
+113
-11
@@ -609,10 +609,13 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase
|
||||
|
||||
// ---------------------------------------------------------------------------------------------------
|
||||
// 2026-08-15 surgical-deploys (Task 6) — a NON-rebuild apply materialises the plan's DELTA, not the whole
|
||||
// composition. The passes are idempotent, but idempotent is not free: every EnsureVariable / EnsureFolder
|
||||
// is a sink call that takes the node manager's lock, so re-walking the composition made the cost of ANY
|
||||
// apply proportional to the size of the served address space (a one-tag add on a 50k-node server was
|
||||
// ~50k sink calls on the publish thread) instead of to the size of the change.
|
||||
// composition. The passes are idempotent, but idempotent is not free. EnsureVariable / EnsureFolder
|
||||
// early-return on a ConcurrentDictionary hit BEFORE taking the node manager's lock, so an already-standing
|
||||
// node costs a dictionary probe, not a lock acquisition — but MaterialiseAlarmCondition DOES take the lock
|
||||
// every time, and the probes are still O(address space) per apply plus the composition walk producing them.
|
||||
// So re-walking the composition made the cost of ANY apply proportional to the size of the served address
|
||||
// space (a one-tag add on a 50k-node server was ~50k sink calls on the publish thread) instead of to the
|
||||
// size of the change.
|
||||
// ---------------------------------------------------------------------------------------------------
|
||||
|
||||
/// <summary>THE headline test. Deploy three raw tags, then deploy the same three PLUS one more: the
|
||||
@@ -716,11 +719,17 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase
|
||||
/// <summary>
|
||||
/// The same repair for the shape the 2026-08-15 review found (commit 875ec9fe): the reference row is
|
||||
/// untouched, but its BACKING raw tag becomes historized. The coincident <c>UnsReferenceDelta</c> must
|
||||
/// still reach the materialise pass, or the UNS mirror keeps serving no history while the raw node
|
||||
/// serves it — the exact split the <c>Backing*</c> fields exist to close.
|
||||
/// still be applied, or the UNS mirror keeps serving no history while the raw node serves it — the
|
||||
/// exact split the <c>Backing*</c> fields exist to close.
|
||||
/// <para><b>Re-pointed by review I2</b>, which changed HOW this shape is repaired: the delta's identity
|
||||
/// is unchanged (same NodeId, same equipment, same backing RawPath — only the inherited attributes
|
||||
/// moved), so it is now an IN-PLACE <c>UpdateTagAttributes</c> in the Uns realm rather than a
|
||||
/// remove+recreate. The client MonitoredItems on the UNS mirror survive an operator historizing the raw
|
||||
/// tag behind them. The historian-value assertion is unchanged — that is the actual repair being
|
||||
/// pinned; only the mechanism that delivers it moved.</para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Changed_uns_reference_is_recreated_when_the_backing_tag_flips_historian()
|
||||
public void Changed_uns_reference_is_updated_in_place_when_the_backing_tag_flips_historian()
|
||||
{
|
||||
var db = NewInMemoryDbFactory();
|
||||
var sink = new RecordingSink();
|
||||
@@ -736,15 +745,18 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase
|
||||
historianTagnameForA: "WW.A");
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2)));
|
||||
|
||||
// The mirror now carries the backing tag's historian tagname — the repair itself, unchanged by I2.
|
||||
AwaitAssert(
|
||||
() => sink.VariableDetails.ShouldContain(
|
||||
v => v.NodeId == "filling/line1/station1/a" && v.HistorianTagname == "WW.A"),
|
||||
() => sink.UpdateTagDetails.ShouldContain(
|
||||
u => u.NodeId == "filling/line1/station1/a" && u.HistorianTagname == "WW.A" &&
|
||||
u.Realm == AddressSpaceRealm.Uns),
|
||||
duration: PresenceBudget);
|
||||
Thread.Sleep(200); // let any (unwanted) remove+recreate land before asserting its absence
|
||||
|
||||
var second = sink.Calls.Skip(afterFirst).ToList();
|
||||
sink.RebuildCalls.ShouldBe(0);
|
||||
second.ShouldContain("RV:filling/line1/station1/a"); // removed…
|
||||
second.ShouldContain("EV:filling/line1/station1/a"); // …and re-ensured, now carrying the tagname
|
||||
second.ShouldContain("UT:filling/line1/station1/a"); // repaired in place…
|
||||
second.ShouldNotContain("RV:filling/line1/station1/a"); // …NOT torn down (subscriptions survive)
|
||||
}
|
||||
|
||||
/// <summary>An AttributeOnly apply (here a raw tag gaining a historian override, with no UNS reference to
|
||||
@@ -775,6 +787,85 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase
|
||||
second.ShouldContain("UT:Plant/Modbus/dev1/A"); // the in-place attribute update DID happen
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 2026-08-15 surgical-deploys review C1 — THE headline degraded-apply test. Before the delta
|
||||
/// materialisation the full passes re-ensured EVERY node on EVERY apply, so a swallowed
|
||||
/// <c>EnsureVariable</c> fault silently repaired itself on the next deploy. The delta path removed that
|
||||
/// accidental self-healing: once <c>_lastApplied</c> records a composition, the failed node is never in
|
||||
/// a diff set again and <c>EnsureVariable</c>'s existing-node early-return means it is never re-created
|
||||
/// — the node is lost until a rebuild happens to fire. So a non-rebuild apply that reports
|
||||
/// <c>failedNodes > 0</c> must NOT advance <c>_lastApplied</c>: the next deploy re-diffs the same
|
||||
/// composition and re-ensures what failed.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Degraded_non_rebuild_apply_does_not_advance_last_applied()
|
||||
{
|
||||
var db = NewInMemoryDbFactory();
|
||||
// The sink faults on exactly one of the two added variables — a PureAdd apply that lands B but
|
||||
// loses A, with no rebuild anywhere in sight.
|
||||
var sink = new RecordingSink { FailEnsureVariableFor = "Plant/Modbus/dev1/A" };
|
||||
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
|
||||
var dep1 = SeedRawTagDeployment(db, new[] { "A", "B" });
|
||||
|
||||
var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier));
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1)));
|
||||
AwaitAssert(() => sink.Calls.ShouldContain("EV:Plant/Modbus/dev1/B"), duration: PresenceBudget);
|
||||
Thread.Sleep(200);
|
||||
sink.RebuildCalls.ShouldBe(0); // genuinely a non-rebuild (PureAdd) apply
|
||||
var afterFirst = sink.Calls.Count;
|
||||
|
||||
// The fault clears (a transient sink fault, an SDK hiccup). Re-deploy the SAME composition: because
|
||||
// the degraded apply did not record it as applied, the planner re-diffs A (and B) as ADDED and the
|
||||
// materialise pass re-ensures them. With _lastApplied advanced this second apply would diff to an
|
||||
// EMPTY plan and A would be gone from the served address space forever.
|
||||
sink.FailEnsureVariableFor = null;
|
||||
var dep2 = SeedRawTagDeployment(db, new[] { "A", "B" });
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2)));
|
||||
|
||||
AwaitAssert(
|
||||
() => sink.Calls.Skip(afterFirst).ShouldContain("EV:Plant/Modbus/dev1/A"),
|
||||
duration: PresenceBudget);
|
||||
sink.RebuildCalls.ShouldBe(0);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The other half of C1's rule: a REBUILD that also reports failed nodes DOES advance
|
||||
/// <c>_lastApplied</c>. A rebuild already re-materialised the whole composition, and the applier's
|
||||
/// one-way rebuild ratchet is the resync mechanism for anything it could not place — holding the
|
||||
/// composition back here would make every subsequent deploy re-diff the entire address space as added.
|
||||
/// Proven by the third apply of the same composition being an empty-plan no-op.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Rebuild_with_failures_still_advances_last_applied()
|
||||
{
|
||||
var db = NewInMemoryDbFactory();
|
||||
var sink = new RecordingSink();
|
||||
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
|
||||
var dep1 = SeedUnsRawDeployment(db, equipmentName: "station1", referencedTag: "a");
|
||||
|
||||
var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier));
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1)));
|
||||
AwaitAssert(() => sink.Calls.ShouldContain("EV:Plant/Modbus/dev1/b"), duration: PresenceBudget);
|
||||
|
||||
// Renaming the equipment is ChangedEquipment ⇒ the classifier's default-closed Rebuild; the sink
|
||||
// faults on one variable of the full re-materialise that follows.
|
||||
sink.FailEnsureVariableFor = "Plant/Modbus/dev1/b";
|
||||
var dep2 = SeedUnsRawDeployment(db, equipmentName: "station1-renamed", referencedTag: "a");
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2)));
|
||||
AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: PresenceBudget);
|
||||
Thread.Sleep(200);
|
||||
var afterSecond = sink.Calls.Count;
|
||||
|
||||
// Re-deploy the SAME composition. _lastApplied advanced despite the failure, so this diffs to an
|
||||
// empty plan and adds no sink calls at all.
|
||||
sink.FailEnsureVariableFor = null;
|
||||
var dep3 = SeedUnsRawDeployment(db, equipmentName: "station1-renamed", referencedTag: "a");
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep3)));
|
||||
Thread.Sleep(300);
|
||||
|
||||
sink.Calls.Count.ShouldBe(afterSecond);
|
||||
}
|
||||
|
||||
/// <summary>Seal a deployment whose raw tree is <c>Plant/Modbus/dev1</c> carrying one tag per name in
|
||||
/// <paramref name="tagNames"/> (RawPath <c>Plant/Modbus/dev1/<name></c>). There is no UNS topology, so
|
||||
/// the composition is raw-only. <paramref name="historizedTag"/> names one tag that carries an
|
||||
@@ -894,6 +985,14 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase
|
||||
public ConcurrentQueue<(string Source, string Target)> ReferenceEdges { get; } = new();
|
||||
/// <summary>Gets or sets the count of rebuild address space calls.</summary>
|
||||
public int RebuildCalls;
|
||||
/// <summary>Gets or sets the node id whose <see cref="EnsureVariable"/> throws (null ⇒ none). The
|
||||
/// applier's <c>SafeEnsureVariable</c> swallows the throw into its failed-node tally, which is exactly
|
||||
/// the degraded-apply shape the C1 no-advance rule exists for. Settable mid-test so a transient fault
|
||||
/// can clear between deploys.</summary>
|
||||
public volatile string? FailEnsureVariableFor;
|
||||
/// <summary>Gets every surgical in-place tag-attribute update with the attributes the applier resolved
|
||||
/// for it (the ordered <see cref="Calls"/> queue only carries node ids).</summary>
|
||||
public ConcurrentQueue<(string NodeId, bool Writable, string? HistorianTagname, AddressSpaceRealm Realm)> UpdateTagDetails { get; } = new();
|
||||
/// <summary>Gets the queue of surgical in-place folder display-name update calls (OpcUaServer-001).</summary>
|
||||
public ConcurrentQueue<(string FolderNodeId, string DisplayName)> FolderRenameQueue { get; } = new();
|
||||
/// <summary>Gets the list of recorded surgical folder display-name update calls.</summary>
|
||||
@@ -935,6 +1034,8 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase
|
||||
/// <param name="historianTagname">The resolved historian tagname (null ⇒ not historized).</param>
|
||||
public void EnsureVariable(string variableNodeId, string? parentFolderNodeId, string displayName, string dataType, bool writable, AddressSpaceRealm realm, string? historianTagname = null, bool isArray = false, uint? arrayLength = null)
|
||||
{
|
||||
if (string.Equals(FailEnsureVariableFor, variableNodeId, StringComparison.Ordinal))
|
||||
throw new InvalidOperationException($"simulated EnsureVariable fault for {variableNodeId}");
|
||||
Calls.Enqueue($"EV:{variableNodeId}");
|
||||
VariableDetails.Enqueue((variableNodeId, parentFolderNodeId, writable, historianTagname, realm));
|
||||
}
|
||||
@@ -950,6 +1051,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase
|
||||
public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname, string dataType, bool isArray, uint? arrayLength, AddressSpaceRealm realm = AddressSpaceRealm.Uns)
|
||||
{
|
||||
Calls.Enqueue($"UT:{variableNodeId}");
|
||||
UpdateTagDetails.Enqueue((variableNodeId, writable, historianTagname, realm));
|
||||
return true;
|
||||
}
|
||||
/// <summary>Records a surgical in-place folder display-name update (always succeeds in this recording sink).</summary>
|
||||
|
||||
Reference in New Issue
Block a user