fix(opcua): replay kept scripted conditions after a rebuild; fit window values onto re-shaped nodes (Lane B fix round 3)

N3 (critical): the node manager carries only NATIVE conditions' state over a
full rebuild; scripted conditions come back fresh and rely on the engine's
re-assert, which since the off-thread plan is handled in the window and lands
on the discarded instance. On a rebuild the condition replay scope is now
AddedConditions plus every enabled scripted condition in the new composition
(native kept conditions stay carried-over, not replayed). The test sink again
models the real rebuild: it clears non-native condition state only.

N4: a surgical in-place shape edit (DataType / IsArray / ArrayLength) resets the
raw node and its UNS mirror to BadWaitingForInitialData without ensuring them,
so the window's value was not replayed and the two NodeIds disagreed. The
applier records re-shaped nodes (MaterialisationRecording.Reshaped); their
deferred value replays only once fitted to the new shape (VariableShape.TryFit:
as is, or converted invariantly; never an old-typed value - WriteValues assigns
the value verbatim with no type check), otherwise it is dropped at Debug and the
node keeps waiting. Status-only entries are not replayed onto a re-shaped node.

Minors: m1/m2 documented on _replayScope (degraded-path only); m3 corrected the
false carry-over premise in the scope doc (the ScriptedAlarmHostActor ordering
remark is outside this lane - F8); m4 the replay Debug log reports alarm-state
and alarm-quality counts again.

Refs #732
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 15:46:29 -04:00
parent 9c8e70c5b7
commit ff4e210a27
4 changed files with 457 additions and 46 deletions
@@ -0,0 +1,64 @@
using Shouldly;
using Xunit;
namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests;
/// <summary>
/// Lane B review N4 — <see cref="VariableShape.TryFit"/> decides whether a value deferred during a plan window may
/// be replayed onto a node an in-place edit re-shaped: never an old-typed value the new DataType contradicts.
/// </summary>
public sealed class VariableShapeTests
{
[Fact]
public void A_value_of_the_new_type_fits_unchanged()
{
new VariableShape("Int32", false, null).TryFit(7, out var fitted).ShouldBeTrue();
fitted.ShouldBe(7);
}
[Fact]
public void A_convertible_old_typed_value_is_converted_to_the_new_type()
{
new VariableShape("Int32", false, null).TryFit(5.0f, out var fitted).ShouldBeTrue();
fitted.ShouldBeOfType<int>().ShouldBe(5);
new VariableShape("Double", false, null).TryFit((short)3, out var d).ShouldBeTrue();
d.ShouldBeOfType<double>().ShouldBe(3d);
}
[Theory]
[InlineData("Int32", "fast")] // not a number
[InlineData("Byte", 300)] // overflow
[InlineData("DateTime", 12)] // no conversion
public void A_value_the_new_type_cannot_hold_does_not_fit(string dataType, object value)
{
new VariableShape(dataType, false, null).TryFit(value, out var fitted).ShouldBeFalse();
fitted.ShouldBeNull();
}
[Fact]
public void Scalar_and_array_never_cross()
{
new VariableShape("Int32", false, null).TryFit(new[] { 1, 2 }, out _).ShouldBeFalse();
new VariableShape("Int32", true, 2).TryFit(1, out _).ShouldBeFalse();
new VariableShape("String", true, null).TryFit("ab", out _).ShouldBeFalse("a string is not an array of strings");
}
[Fact]
public void An_array_is_converted_element_wise_and_its_fixed_length_enforced()
{
new VariableShape("Int16", true, 3).TryFit(new[] { 1, 2, 3 }, out var fitted).ShouldBeTrue();
fitted.ShouldBeOfType<short[]>().ShouldBe(new short[] { 1, 2, 3 });
new VariableShape("Int16", true, 4).TryFit(new[] { 1, 2, 3 }, out _).ShouldBeFalse("wrong length");
new VariableShape("Int16", true, null).TryFit(new[] { 1, 2, 3 }, out _).ShouldBeTrue("no fixed length");
new VariableShape("Byte", true, null).TryFit(new[] { 1, 999 }, out _).ShouldBeFalse("an element overflows");
}
[Fact]
public void Null_and_unknown_types_always_fit()
{
new VariableShape("Int32", false, null).TryFit(null, out var n).ShouldBeTrue();
n.ShouldBeNull();
new VariableShape("Structure", false, null).TryFit("anything", out var any).ShouldBeTrue();
any.ShouldBe("anything");
}
}
@@ -277,13 +277,11 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
/// <summary>
/// N2 — a condition the plan KEEPS took the live write already; replaying it would roll back an operator's
/// Acknowledge applied in the window (the SDK mutates the node directly) and fire spurious events. Driven over
/// both apply shapes: a surgical pure-add, and a full rebuild (which carries the condition's state over).
/// Acknowledge applied in the window (the SDK mutates the node directly) and fire spurious events. The surgical
/// apply shape; the rebuild shapes are the two tests below (scripted: replayed; native: carried over).
/// </summary>
[Theory]
[InlineData(false)]
[InlineData(true)]
public void An_out_of_band_ack_on_a_kept_condition_is_not_reverted_by_the_replay(bool rebuild)
[Fact]
public void An_out_of_band_ack_on_a_kept_condition_is_not_reverted_by_the_replay()
{
var sink = new ExistenceSink();
using var gate = new ManualResetEventSlim(true);
@@ -293,8 +291,8 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
sink.ConditionExists("al-1").ShouldBeTrue();
gate.Reset();
// A surgical pure-add (new raw tags) or a full rebuild (an equipment rename) that keeps al-1.
actor.Tell(Rebuild(rebuild ? AlarmArtifact(equipmentName: "Eq-renamed") : AlarmArtifact(rawTags: 2)));
// A surgical pure-add (new raw tags) that keeps al-1.
actor.Tell(Rebuild(AlarmArtifact(rawTags: 2)));
actor.Tell(new OpcUaPublishActor.AlarmStateUpdate("al-1", ActiveUnacked(OpcUaStatus.Good), DateTime.UtcNow, AddressSpaceRealm.Uns));
AwaitAssert(() => sink.AlarmWritesTo("al-1").ShouldBe(1), PlanBudget); // the live write landed
sink.AcknowledgeOutOfBand("al-1"); // an operator acks in the window
@@ -302,7 +300,7 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
gate.Set();
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget).Degraded.ShouldBeFalse();
sink.SawRebuild.ShouldBe(rebuild, "non-vacuity: the intended apply shape ran");
sink.SawRebuild.ShouldBeFalse("non-vacuity: a surgical apply");
sink.AlarmStateOf("al-1")!.Acknowledged.ShouldBeTrue("the operator's ack must survive the apply");
sink.AlarmWritesTo("al-1").ShouldBe(1, "no replayed write (and so no extra Part 9 event) on a kept condition");
}
@@ -422,6 +420,149 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
second.Degraded.ShouldBeTrue();
}
/// <summary>
/// N3 (i) — a full rebuild recreates every KEPT SCRIPTED condition fresh (only native conditions are carried
/// over), so the engine's re-assert handled in the window — landing on the discarded instance — must be
/// replayed onto the new one, or the active alarm reads Normal until its next transition (#487).
/// </summary>
[Fact]
public void A_rebuild_replays_the_window_state_onto_every_kept_scripted_condition()
{
var sink = new ExistenceSink();
using var gate = new ManualResetEventSlim(true);
var actor = Spawn(sink, gate);
actor.Tell(Rebuild(AlarmArtifact()));
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget).Degraded.ShouldBeFalse();
gate.Reset();
actor.Tell(Rebuild(AlarmArtifact(equipmentName: "Eq-renamed"))); // ChangedEquipment ⇒ full rebuild, al-1 kept
actor.Tell(new OpcUaPublishActor.AlarmStateUpdate("al-1", ActiveUnacked(OpcUaStatus.Good), DateTime.UtcNow, AddressSpaceRealm.Uns));
AwaitAssert(() => sink.AlarmWritesTo("al-1").ShouldBe(1), PlanBudget); // landed live on the old instance
gate.Set();
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget).Degraded.ShouldBeFalse();
sink.SawRebuild.ShouldBeTrue("non-vacuity: a full rebuild ran");
sink.AlarmStateOf("al-1").ShouldNotBeNull("the re-assert must reach the recreated condition")
.Active.ShouldBeTrue();
}
/// <summary>
/// N3 (ii) — the native counterpart: a kept NATIVE condition is restored from the node manager's carry-over
/// (which already holds the live write and the operator's ack), so it is not replayed — the ack survives and no
/// extra write lands.
/// </summary>
[Fact]
public void A_rebuild_does_not_replay_onto_a_kept_native_condition()
{
var sink = new ExistenceSink();
using var gate = new ManualResetEventSlim(true);
var actor = Spawn(sink, gate);
actor.Tell(Rebuild(AlarmArtifact(nativeAlarm: true)));
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget).Degraded.ShouldBeFalse();
sink.ConditionExists(NativeAlarm).ShouldBeTrue();
gate.Reset();
actor.Tell(Rebuild(AlarmArtifact(equipmentName: "Eq-renamed", nativeAlarm: true)));
actor.Tell(new OpcUaPublishActor.AlarmStateUpdate(NativeAlarm, ActiveUnacked(OpcUaStatus.Good), DateTime.UtcNow, AddressSpaceRealm.Raw));
AwaitAssert(() => sink.AlarmWritesTo(NativeAlarm).ShouldBe(1), PlanBudget);
sink.AcknowledgeOutOfBand(NativeAlarm);
gate.Set();
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget).Degraded.ShouldBeFalse();
sink.SawRebuild.ShouldBeTrue("non-vacuity: a full rebuild ran");
sink.AlarmStateOf(NativeAlarm)!.Acknowledged.ShouldBeTrue("the carried-over ack survives");
sink.AlarmWritesTo(NativeAlarm).ShouldBe(1, "no replayed write on a carried-over native condition");
}
/// <summary>
/// N4 — a surgical in-place DataType edit resets the raw node AND its UNS mirror (the node manager drops an
/// old-typed value), and neither is ensured by the apply. The window's value is replayed onto both, fitted to
/// the new type, so the two NodeIds of one value agree.
/// </summary>
[Fact]
public void A_surgical_shape_edit_replays_the_window_value_fitted_onto_the_raw_node_and_its_mirror()
{
var sink = new ExistenceSink();
using var gate = new ManualResetEventSlim(true);
var actor = Spawn(sink, gate);
actor.Tell(Rebuild(ShapeArtifact("Float")));
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget).Degraded.ShouldBeFalse();
sink.Exists(RawSpeed).ShouldBeTrue();
sink.Exists(UnsSpeed).ShouldBeTrue("non-vacuity: the mirror exists");
gate.Reset();
actor.Tell(Rebuild(ShapeArtifact("Int32")));
var at = DateTime.UtcNow;
actor.Tell(new OpcUaPublishActor.AttributeValueBatchUpdate(
[
new ValueWrite(RawSpeed, 5.0f, OpcUaStatus.Good, at, AddressSpaceRealm.Raw),
new ValueWrite(UnsSpeed, 5.0f, OpcUaStatus.Good, at, AddressSpaceRealm.Uns),
]));
AwaitAssert(() => sink.ValueOf(RawSpeed)!.Value.Value.ShouldBe(5.0f), PlanBudget);
gate.Set();
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget).Degraded.ShouldBeFalse();
sink.SawRebuild.ShouldBeFalse("non-vacuity: a surgical in-place edit");
sink.ReshapedNodes.ShouldContain(RawSpeed, "non-vacuity: the raw node was reset");
sink.ReshapedNodes.ShouldContain(UnsSpeed, "non-vacuity: the mirror was reset");
var raw = sink.ValueOf(RawSpeed).ShouldNotBeNull();
raw.Value.ShouldBeOfType<int>().ShouldBe(5);
sink.ValueOf(UnsSpeed).ShouldBe(raw, "the raw node and its UNS mirror agree");
}
/// <summary>N4 — a window value that cannot be fitted to the new shape is never written: both NodeIds stay
/// value-less (BadWaitingForInitialData) until the driver's next value.</summary>
[Fact]
public void A_surgical_shape_edit_never_replays_a_value_that_does_not_fit_the_new_shape()
{
var sink = new ExistenceSink();
using var gate = new ManualResetEventSlim(true);
var actor = Spawn(sink, gate);
actor.Tell(Rebuild(ShapeArtifact("String")));
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget);
gate.Reset();
actor.Tell(Rebuild(ShapeArtifact("Int32")));
var at = DateTime.UtcNow;
actor.Tell(new OpcUaPublishActor.AttributeValueBatchUpdate(
[
new ValueWrite(RawSpeed, "fast", OpcUaStatus.Good, at, AddressSpaceRealm.Raw),
new ValueWrite(UnsSpeed, "fast", OpcUaStatus.Good, at, AddressSpaceRealm.Uns),
]));
AwaitAssert(() => sink.ValueOf(RawSpeed)!.Value.Value.ShouldBe("fast"), PlanBudget);
gate.Set();
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget);
sink.ReshapedNodes.ShouldContain(RawSpeed, "non-vacuity: the raw node was reset");
sink.ValueOf(RawSpeed).ShouldBeNull("\"fast\" is not an Int32 — the node keeps waiting");
sink.ValueOf(UnsSpeed).ShouldBeNull();
}
private const string NativeAlarm = "Plant/Modbus/dev1/Hi";
private const string RawSpeed = "Plant/Modbus/dev1/Speed";
private const string UnsSpeed = "Area/Line/Eq/Speed";
/// <summary>One raw tag (Speed) of <paramref name="dataType"/> and one UNS reference to it under Area/Line/Eq.</summary>
private static byte[] ShapeArtifact(string dataType) =>
JsonSerializer.SerializeToUtf8Bytes(new
{
UnsAreas = new[] { new { UnsAreaId = "area-1", ClusterId = "c1", Name = "Area" } },
UnsLines = new[] { new { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "Line" } },
Equipment = new[] { new { EquipmentId = "eq-1", DriverInstanceId = (string?)null, UnsLineId = "line-1", Name = "Eq", MachineCode = "EQ-1" } },
RawFolders = new[] { new { RawFolderId = "rf-1", ParentRawFolderId = (string?)null, Name = "Plant", ClusterId = "c1" } },
DriverInstances = new[] { new { DriverInstanceId = "drv-1", DriverType = "Modbus", DriverConfig = "{}", Name = "Modbus", RawFolderId = "rf-1", ClusterId = "c1" } },
Devices = new[] { new { DeviceId = "dev-1", DriverInstanceId = "drv-1", Name = "dev1", DeviceConfig = "{}" } },
TagGroups = Array.Empty<object>(),
Tags = new[] { new { TagId = "t-speed", DeviceId = "dev-1", TagGroupId = (string?)null, Name = "Speed", DataType = dataType, AccessLevel = 0, TagConfig = "{}" } },
UnsTagReferences = new[] { new { UnsTagReferenceId = "ref-1", EquipmentId = "eq-1", TagId = "t-speed", DisplayNameOverride = (string?)null } },
ScriptedAlarms = Array.Empty<object>(),
});
private static AlarmConditionSnapshot ActiveUnacked(uint quality) =>
new(Active: true, Acknowledged: false, Confirmed: true, Enabled: true, AlarmShelvingKind.Unshelved, 500, "high", quality);
@@ -454,7 +595,7 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
ScriptedAlarms = Array.Empty<object>(),
});
private static byte[] AlarmArtifact(string equipmentName = "Eq", int rawTags = 0) =>
private static byte[] AlarmArtifact(string equipmentName = "Eq", int rawTags = 0, bool nativeAlarm = false) =>
JsonSerializer.SerializeToUtf8Bytes(new
{
UnsAreas = new[] { new { UnsAreaId = "area-1", ClusterId = "c1", Name = "Area" } },
@@ -463,13 +604,13 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
{
new { EquipmentId = "eq-1", DriverInstanceId = (string?)null, UnsLineId = "line-1", Name = equipmentName, MachineCode = "EQ-1" },
},
RawFolders = rawTags == 0
RawFolders = rawTags == 0 && !nativeAlarm
? Array.Empty<object>()
: new object[] { new { RawFolderId = "rf-1", ParentRawFolderId = (string?)null, Name = "Plant", ClusterId = "c1" } },
DriverInstances = rawTags == 0
DriverInstances = rawTags == 0 && !nativeAlarm
? Array.Empty<object>()
: new object[] { new { DriverInstanceId = "drv-1", DriverType = "Modbus", DriverConfig = "{}", Name = "Modbus", RawFolderId = "rf-1", ClusterId = "c1" } },
Devices = rawTags == 0
Devices = rawTags == 0 && !nativeAlarm
? Array.Empty<object>()
: new object[] { new { DeviceId = "dev-1", DriverInstanceId = "drv-1", Name = "dev1", DeviceConfig = "{}" } },
TagGroups = Array.Empty<object>(),
@@ -477,7 +618,16 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
{
TagId = $"t-{i}", DeviceId = "dev-1", TagGroupId = (string?)null, Name = $"T{i}",
DataType = "Float", AccessLevel = 0, TagConfig = "{}",
}).ToArray(),
}).Concat(nativeAlarm
? new object[]
{
new
{
TagId = "t-hi", DeviceId = "dev-1", TagGroupId = (string?)null, Name = "Hi", DataType = "Boolean",
AccessLevel = 0, TagConfig = "{\"alarm\":{\"alarmType\":\"OffNormalAlarm\",\"severity\":700}}",
},
}
: Array.Empty<object>()).ToArray(),
ScriptedAlarms = new[]
{
new
@@ -615,9 +765,18 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
public void MaterialiseAlarmCondition(string alarmNodeId, string equipmentNodeId, string displayName, string alarmType, int severity, AddressSpaceRealm realm, bool isNative = false)
{
lock (_gate) { _applyClosed = false; _conditions.Add(alarmNodeId); }
lock (_gate)
{
_applyClosed = false;
_conditions.Add(alarmNodeId);
if (isNative) _nativeConditions.Add(alarmNodeId); else _nativeConditions.Remove(alarmNodeId);
}
}
private readonly HashSet<string> _nativeConditions = new();
private readonly Dictionary<string, (string DataType, bool IsArray, uint? ArrayLength)> _shapes = new();
public ConcurrentQueue<string> ReshapedNodes { get; } = new();
public void EnsureFolder(string folderNodeId, string? parentNodeId, string displayName, AddressSpaceRealm realm)
{
lock (_gate) _applyClosed = false;
@@ -625,20 +784,28 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
public void EnsureVariable(string variableNodeId, string? parentFolderNodeId, string displayName, string dataType, bool writable, AddressSpaceRealm realm, string? historianTagname = null, bool isArray = false, uint? arrayLength = null, bool refuseArrayOnScalar = false)
{
lock (_gate) { _applyClosed = false; _variables.Add(variableNodeId); }
lock (_gate)
{
_applyClosed = false;
if (_variables.Add(variableNodeId)) _shapes[variableNodeId] = (dataType, isArray, arrayLength);
}
}
public void RebuildAddressSpace()
{
lock (_gate)
{
// Variables are recreated empty; a condition's state is captured and carried over to its
// re-materialised instance (the node manager's rebuild carry-over).
// OtOpcUaNodeManager.RebuildAddressSpace: variables are recreated empty; only NATIVE conditions'
// state is captured and carried over to the re-materialised instance — a scripted condition comes
// back fresh (its engine re-asserts it).
_applyClosed = false;
_sawRebuild = true;
_variables.Clear();
_shapes.Clear();
_conditions.Clear();
_values.Clear();
foreach (var id in _alarmStates.Keys.Where(k => !_nativeConditions.Contains(k)).ToList()) _alarmStates.Remove(id);
foreach (var id in _alarmQualities.Keys.Where(k => !_nativeConditions.Contains(k)).ToList()) _alarmQualities.Remove(id);
}
}
@@ -652,7 +819,23 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
public void RaiseNodesAddedModelChange(string affectedNodeId, AddressSpaceRealm realm) { }
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") { }
public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname, string dataType, bool isArray, uint? arrayLength, AddressSpaceRealm realm) => true;
/// <summary>OtOpcUaNodeManager.UpdateTagAttributes: a SHAPE change (DataType / array-ness / length) drops the
/// node's value (BadWaitingForInitialData); any other attribute edit leaves it.</summary>
public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname, string dataType, bool isArray, uint? arrayLength, AddressSpaceRealm realm)
{
lock (_gate)
{
if (!_variables.Contains(variableNodeId)) return false;
if (_shapes.GetValueOrDefault(variableNodeId) != (dataType, isArray, arrayLength))
{
_shapes[variableNodeId] = (dataType, isArray, arrayLength);
_values.Remove(variableNodeId);
ReshapedNodes.Enqueue(variableNodeId);
}
return true;
}
}
public bool UpdateFolderDisplayName(string folderNodeId, string displayName, AddressSpaceRealm realm) => true;
public bool RemoveVariableNode(string variableNodeId, AddressSpaceRealm realm) { lock (_gate) return _variables.Remove(variableNodeId); }
public bool RemoveAlarmConditionNode(string alarmNodeId, AddressSpaceRealm realm) { lock (_gate) return _conditions.Remove(alarmNodeId); }