fix(opcua): array equipment-tag nodes are read-only (array writes out of scope, review M-1)

This commit is contained in:
Joseph Doherty
2026-06-16 22:31:15 -04:00
parent 5f7a2acd27
commit 3bb2031d1d
2 changed files with 65 additions and 1 deletions
@@ -211,7 +211,11 @@ public sealed class Phase7Applier
string? historianTagname = tag.IsHistorized
? (string.IsNullOrWhiteSpace(tag.HistorianTagname) ? tag.FullName : tag.HistorianTagname)
: null;
SafeEnsureVariable(nodeId, parent, tag.Name, tag.DataType, tag.Writable, historianTagname, tag.IsArray, tag.ArrayLength);
// Array writes are out of scope (Phase 4c read-only surface): force array tags read-only
// even if authored ReadWrite, so a client write cannot reach the driver write path which
// does not handle arrays (e.g. S7 BoxValueForWrite would crash).
var writable = tag.Writable && !tag.IsArray;
SafeEnsureVariable(nodeId, parent, tag.Name, tag.DataType, writable, historianTagname, tag.IsArray, tag.ArrayLength);
}
}
@@ -378,6 +378,66 @@ public sealed class Phase7ApplierTests
call.ArrayLength.ShouldBeNull();
}
/// <summary>Review M-1 — an array equipment tag authored with <c>Writable: true</c> must be
/// materialised as READ-ONLY (<c>writable == false</c>) because array writes are out of scope
/// (Phase 4c read-only surface). The driver write path does not handle arrays and would crash
/// (e.g. S7 BoxValueForWrite). Guards against a future refactor that accidentally enables the
/// writable path for arrays.</summary>
[Fact]
public void MaterialiseEquipmentTags_array_writable_true_is_forced_read_only()
{
var sink = new RecordingSink();
var applier = new Phase7Applier(sink, NullLogger<Phase7Applier>.Instance);
var composition = new Phase7CompositionResult(
Array.Empty<EquipmentNode>(), Array.Empty<DriverInstancePlan>(), Array.Empty<ScriptedAlarmPlan>())
{
EquipmentTags = new[]
{
// Authored ReadWrite AND IsArray — the applier must clamp to read-only.
new EquipmentTagPlan("tag-arr-rw", "eq-1", "drv", FolderPath: "", Name: "Buffer", DataType: "Int16",
FullName: "40001", Writable: true, Alarm: null, IsArray: true, ArrayLength: 8u),
},
};
applier.MaterialiseEquipmentTags(composition);
// writable must be false (array writes out of scope), isArray must be true (forwarded verbatim).
var varCall = sink.VariableCalls.ShouldHaveSingleItem();
varCall.Writable.ShouldBeFalse(); // clamped to read-only despite Writable: true
var arrCall = sink.ArrayCalls.ShouldHaveSingleItem();
arrCall.IsArray.ShouldBeTrue();
arrCall.ArrayLength.ShouldBe(8u);
}
/// <summary>Review M-1 regression — a scalar tag authored with <c>Writable: true</c> must still
/// be materialised as read/write (<c>writable == true</c>). The array-clamp must NOT affect
/// scalar tags.</summary>
[Fact]
public void MaterialiseEquipmentTags_scalar_writable_true_stays_writable()
{
var sink = new RecordingSink();
var applier = new Phase7Applier(sink, NullLogger<Phase7Applier>.Instance);
var composition = new Phase7CompositionResult(
Array.Empty<EquipmentNode>(), Array.Empty<DriverInstancePlan>(), Array.Empty<ScriptedAlarmPlan>())
{
EquipmentTags = new[]
{
// Authored ReadWrite, scalar — must pass through writable: true unchanged.
new EquipmentTagPlan("tag-scalar-rw", "eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Float",
FullName: "40002", Writable: true, Alarm: null, IsArray: false, ArrayLength: null),
},
};
applier.MaterialiseEquipmentTags(composition);
var varCall = sink.VariableCalls.ShouldHaveSingleItem();
varCall.Writable.ShouldBeTrue(); // scalar: writable unchanged
var arrCall = sink.ArrayCalls.ShouldHaveSingleItem();
arrCall.IsArray.ShouldBeFalse();
}
/// <summary>Verifies MaterialiseEquipmentVirtualTags creates one Variable per VirtualTag directly
/// under its existing equipment folder, with a folder-scoped NodeId (EquipmentId/Name — NOT the
/// VirtualTagId or Expression), parent == EquipmentId, displayName == Name, and does NOT re-create