fix(opcua): DeleteRawModified with an unspecified bound is BadInvalidArgument; HistoryUpdate node-manager tests

- A DateTime.MinValue start or end on DeleteRawModified is refused
  BadInvalidArgument before the backend.
- Tests: anonymous without the HistoryUpdate role denied (with it allowed),
  UpdateTagAttributes enabling historization sets HistoryWrite, a throwing
  updater is BadInternalError, the MinValue bounds case; corrected the
  test helper's comment on how the SDK base seeds unadmitted slots.
- GatewayHistorianValueWriter remarks describe the recorder's real status
  capture (and that it does not persist on the gateway).

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-09 14:27:33 -04:00
parent c6d4134795
commit f8ad74dc19
3 changed files with 129 additions and 8 deletions
@@ -28,12 +28,18 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Recorder;
/// <b>Quality is a documented-dead input</b> on the <c>WriteLiveValues</c> SQL path
/// (HistorianGateway 0.2.0 Contracts C-002): <c>HistorianLiveValue.Quality</c> /
/// <c>QualityDetail</c> are server-managed columns — any value sent is accepted on the wire and
/// then silently discarded. We populate <c>Quality</c> anyway (see the mapping below): proto3
/// transmits <c>0</c> when a field is left unset, and <c>0</c> is "Bad" in OPC-DA terms, so
/// <em>not</em> setting it would send a <em>wrong</em> quality to any future gateway version that
/// starts honoring the field — whereas the recorder's <c>192</c> ("Good") is the correct value.
/// <b>Do NOT extend the recorder to capture real node quality expecting it to persist</b> — it
/// will not, until the gateway honors the field.
/// then silently discarded. We populate <c>Quality</c> anyway: proto3 transmits <c>0</c> when a
/// field is left unset, and <c>0</c> is "Bad" in OPC-DA terms, so <em>not</em> setting it would send a
/// <em>wrong</em> quality to any future gateway version that starts honoring the field. The recorder
/// now carries each sample's real OPC UA status, which <see cref="Mapping.GatewayQualityMapper.ToDaQuality"/>
/// maps to the DA byte (Good ⇒ <c>192</c>, as before). <b>That status does not persist on this
/// backend</b>: the gateway discards the field, so AVEVA still stores every sample Good — use the
/// SqlServer backend where the stored status must be the real one.
/// </para>
/// <para>
/// <b>Numeric only.</b> Values whose kind has no double form (String, DateTime, Empty) are skipped,
/// never thrown; a batch with none left is acked without calling the gateway. The recorder already drops
/// those kinds at capture for this backend, so the skip is a backstop.
/// </para>
/// </remarks>
public sealed class GatewayHistorianValueWriter : IHistorianValueWriter, IAsyncDisposable
@@ -5295,6 +5295,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
if (details.IsDeleteModified) return null;
var start = details.StartTime;
var end = details.EndTime;
// Part 11 requires both bounds of a raw delete; DateTime.MinValue is the wire's "unspecified". Refuse
// it here rather than let a backend read MinValue as "from the beginning of time".
if (start == DateTime.MinValue || end == DateTime.MinValue)
return (_, _, _) => Task.FromResult(new HistoryUpdateResult { StatusCode = StatusCodes.BadInvalidArgument });
return async (updater, tagname, ct) =>
{
var code = await updater.DeleteRawAsync(tagname, start, end, ct).ConfigureAwait(false);
@@ -166,6 +166,35 @@ public sealed class HistoryUpdateTests : IDisposable
finally { await host.DisposeAsync(); }
}
/// <summary>
/// ACL off: Anonymous is an ordinary principal, not a bypass — an anonymous session WITHOUT the HistoryUpdate
/// role (even one carrying WriteOperate) is denied, while one whose grants confer the role is admitted. The
/// allow half keeps the deny from passing against a build that denies anonymous unconditionally.
/// </summary>
[Fact]
public async Task Acl_off_anonymous_without_the_role_is_denied_and_with_it_is_allowed()
{
var (host, server) = await BootAsync();
try
{
var nm = server.NodeManager!;
var updater = new FakeUpdater();
nm.HistorianDataUpdater = updater;
WireAcl(nm, enabled: false, NodePermissions.HistoryUpdate);
var nodeId = Historized(nm, AddressSpaceRealm.Raw, RawPath);
var denied = Insert(nm, AnonymousIdentity(OpcUaDataPlaneRoles.WriteOperate), nodeId, Value(T1, 1d));
denied.Errors[0].StatusCode.Code.ShouldBe(StatusCodes.BadUserAccessDenied);
denied.Results[0].StatusCode.Code.ShouldBe(StatusCodes.BadUserAccessDenied);
updater.UpdateCalls.ShouldBeEmpty();
var allowed = Insert(nm, AnonymousIdentity(OpcUaDataPlaneRoles.HistoryUpdate), nodeId, Value(T2, 2d));
allowed.Errors[0].StatusCode.Code.ShouldBe(StatusCodes.Good);
updater.UpdateCalls.Count.ShouldBe(1);
}
finally { await host.DisposeAsync(); }
}
// ───────────────────────────── availability + AccessLevel ─────────────────────────────
/// <summary>
@@ -241,6 +270,82 @@ public sealed class HistoryUpdateTests : IDisposable
finally { await host.DisposeAsync(); }
}
/// <summary>
/// A surgical deploy that turns historization ON for an existing node (<c>UpdateTagAttributes</c>) grants the
/// HistoryWrite bit when an updater is wired — the same bit <c>EnsureVariable</c> sets at materialise — so
/// the newly historized node is updatable without a rebuild.
/// </summary>
[Fact]
public async Task Update_tag_attributes_turning_historization_on_sets_history_write()
{
var (host, server) = await BootAsync();
try
{
var nm = server.NodeManager!;
nm.HistorianDataUpdater = new FakeUpdater();
nm.EnsureVariable(RawPath, parentFolderNodeId: null, displayName: RawPath, dataType: "Float",
writable: false, historianTagname: null, realm: AddressSpaceRealm.Raw);
(nm.TryGetVariable(RawPath, AddressSpaceRealm.Raw)!.AccessLevel & AccessLevels.HistoryWrite).ShouldBe((byte)0);
nm.UpdateTagAttributes(RawPath, writable: false, historianTagname: Tagname, dataType: "Float",
isArray: false, arrayLength: null, realm: AddressSpaceRealm.Raw).ShouldBeTrue();
var access = nm.TryGetVariable(RawPath, AddressSpaceRealm.Raw)!.AccessLevel;
(access & AccessLevels.HistoryWrite).ShouldNotBe((byte)0);
(access & AccessLevels.HistoryRead).ShouldNotBe((byte)0);
}
finally { await host.DisposeAsync(); }
}
/// <summary>An updater that throws is <c>BadInternalError</c> for that node, never an exception out of the service.</summary>
[Fact]
public async Task An_updater_that_throws_is_bad_internal_error()
{
var (host, server) = await BootAsync();
try
{
var nm = server.NodeManager!;
nm.HistorianDataUpdater = new FakeUpdater { Throw = new InvalidOperationException("backend down") };
WireAcl(nm, enabled: false, NodePermissions.HistoryUpdate);
var nodeId = Historized(nm, AddressSpaceRealm.Raw, RawPath);
var (results, errors) = Insert(nm, KeyIdentity(OpcUaDataPlaneRoles.HistoryUpdate), nodeId, Value(T1, 1d));
errors[0].StatusCode.Code.ShouldBe(StatusCodes.BadInternalError);
results[0].StatusCode.Code.ShouldBe(StatusCodes.BadInternalError);
}
finally { await host.DisposeAsync(); }
}
/// <summary>
/// DeleteRawModified needs both bounds (Part 11): a <c>DateTime.MinValue</c> ("unspecified") start or end is
/// <c>BadInvalidArgument</c> and never reaches the backend.
/// </summary>
[Fact]
public async Task Delete_raw_with_an_unspecified_bound_is_bad_invalid_argument()
{
var (host, server) = await BootAsync();
try
{
var nm = server.NodeManager!;
var updater = new FakeUpdater();
nm.HistorianDataUpdater = updater;
WireAcl(nm, enabled: false, NodePermissions.HistoryUpdate);
var nodeId = Historized(nm, AddressSpaceRealm.Raw, RawPath);
var identity = KeyIdentity(OpcUaDataPlaneRoles.HistoryUpdate);
foreach (var (start, end) in new[] { (DateTime.MinValue, DateTime.MinValue), (DateTime.MinValue, T2), (T1, DateTime.MinValue) })
{
var (results, _) = Call(nm, identity, typeof(DeleteRawModifiedDetails),
new DeleteRawModifiedDetails { NodeId = nodeId, StartTime = start, EndTime = end, IsDeleteModified = false });
results[0].StatusCode.Code.ShouldBe(StatusCodes.BadInvalidArgument);
}
updater.DeleteRawCalls.ShouldBeEmpty();
}
finally { await host.DisposeAsync(); }
}
// ───────────────────────────── semantics ─────────────────────────────
/// <summary>
@@ -441,8 +546,9 @@ public sealed class HistoryUpdateTests : IDisposable
nm.HistoryUpdate(context, detailsType, nodesToUpdate, results, errors);
// Unadmitted (no HistoryWrite bit) slots are left untouched by the base and later filled by the
// master node manager; report them the way the client would see them.
// The base seeds every node THIS manager owns — with or without the HistoryWrite bit — with
// BadHistoryOperationUnsupported. A slot is only still null when GetManagerHandle found no node here
// (the master node manager would answer it); report that the way the client would see it.
if (results[0] is null) results[0] = new HistoryUpdateResult { StatusCode = StatusCodes.BadHistoryOperationUnsupported };
errors[0] ??= new ServiceResult(StatusCodes.BadHistoryOperationUnsupported);
return (results, errors);
@@ -451,6 +557,9 @@ public sealed class HistoryUpdateTests : IDisposable
private static RoleCarryingUserIdentity KeyIdentity(params string[] roles) =>
new(new UserNameIdentityToken { UserName = "apikey" }, roles, Principal, KeyId);
private static RoleCarryingUserIdentity AnonymousIdentity(params string[] roles) =>
new(new AnonymousIdentityToken(), roles, OpcUaPrincipals.Anonymous, apiKeyId: null);
private static void WireAcl(
OtOpcUaNodeManager nm, bool enabled, NodePermissions flags, TimeProvider? clock = null,
DateTime? expiresUtc = null)
@@ -489,11 +598,13 @@ public sealed class HistoryUpdateTests : IDisposable
public uint[]? UpdateCodes { get; init; }
public uint DeleteRawCode { get; init; } = StatusCodes.Good;
public uint[]? DeleteAtTimeCodes { get; init; }
public Exception? Throw { get; init; }
public async Task<IReadOnlyList<uint>> UpdateDataAsync(
string tag, HistoryUpdateMode mode, IReadOnlyList<HistorizationValue> values, CancellationToken ct)
{
await Task.Delay(1, ct).ConfigureAwait(false);
if (Throw is not null) throw Throw;
lock (UpdateCalls) UpdateCalls.Add((tag, mode, values.ToList()));
return UpdateCodes ?? Enumerable.Repeat(StatusCodes.GoodEntryInserted, values.Count).ToArray();
}