fix(opcua): write-path robustness, injected clock, configurable quotas (B4)

Review 01 L3/H5/H4/P6/P4. - L3/H5 OnEquipmentTagWrite's dispatch is factored into DispatchDeviceWrite: a gateway that throws synchronously is refused inline with BadDeviceFailure (it used to escape the handler under the SDK Lock), and a faulted/cancelled dispatch task has its exception observed and logged and carries the real reason into the revert/audit event instead of surfacing as UnobservedTaskException at finalization. - H4 every timestamp the manager stamps (condition quality, timed-shelve expiry, method-transition events, carry-over, revert, revalidation) reads the injected Clock; RemainingShelveMs takes the clock reading. - P6 OpcUa:Quotas:* (OpcUaQuotaOptions on OpcUaApplicationHostOptions): Min/MaxRequestThreadCount, MaxQueuedRequestCount with the former hard-coded 5/100/200 as defaults, and the TransportQuotas members as nullable overrides over the SDK defaults. - P4 the SDK line exposes IHistoryReadAsyncNodeManager but CustomNodeManager2 offers no async virtual to override (adopting it means re-implementing the base dispatcher), so the lever is the admission cap: MaxConcurrentHistoryReadBatches defaults to clamp(2*ProcessorCount, 4, 16). The Host still overrides it from ServerHistorianOptions (default 16) — follow-up F3. Tests: NodeManagerWriteDispatchTests (sync throw refused; faulted task reverts; success leaves the value; clock-stamped revert), OpcUaApplicationHostQuotaTests. Refs #732 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
2026-10-22 10:22:29 -04:00
parent dca822dc59
commit 1578efd4a9
4 changed files with 402 additions and 31 deletions
@@ -0,0 +1,160 @@
+using Microsoft.Extensions.Logging.Abstractions;
+using Opc.Ua;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
+
+namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests;
+
+/// <summary>
+///     Review 01 L3 / H5 / H4 — the device-write dispatch behind <see cref="OtOpcUaNodeManager.OnEquipmentTagWrite"/>
+///     and the manager's clock: a gateway that throws synchronously is refused inline (the handler runs under
+///     the SDK <c>Lock</c>, and the throw used to escape it), a faulted dispatch task is observed and still
+///     reverts the optimistic value, and the timestamps the manager stamps come from the injected
+///     <see cref="OtOpcUaNodeManager.Clock"/>.
+/// </summary>
+public sealed class NodeManagerWriteDispatchTests : IDisposable
+{
+    private static CancellationToken Ct => TestContext.Current.CancellationToken;
+
+    private readonly string _pkiRoot = Path.Combine(Path.GetTempPath(), $"otopcua-dispatch-{Guid.NewGuid():N}");
+
+    /// <summary>H5 — a synchronous throw out of the gateway is a Bad result, never an escaped exception.</summary>
+    [Fact]
+    public async Task A_gateway_that_throws_synchronously_is_refused_inline()
+    {
+        var (host, server) = await BootAsync();
+        try
+        {
+            var nm = server.NodeManager!;
+            nm.EnsureVariable("eq-1/sp", null, "Sp", "Int32", writable: true, realm: AddressSpaceRealm.Uns);
+            nm.WriteValue("eq-1/sp", 7, OpcUaStatus.Good, DateTime.UtcNow, AddressSpaceRealm.Uns);
+
+            var result = nm.DispatchDeviceWrite(
+                new ThrowingGateway(), "ns=2;s=eq-1/sp", "eq-1/sp", AddressSpaceRealm.Uns,
+                optimisticValue: 42, priorValue: 7, priorStatus: StatusCodes.Good, clientUserId: "op");
+
+            ServiceResult.IsBad(result).ShouldBeTrue();
+            result.StatusCode.ShouldBe((StatusCode)StatusCodes.BadDeviceFailure);
+            result.ToString().ShouldContain("boom");
+            nm.TryGetVariable("eq-1/sp")!.Value.ShouldBe(7, "nothing was applied, so nothing needs reverting");
+        }
+        finally { await host.DisposeAsync(); }
+    }
+
+    /// <summary>L3 — a faulted dispatch task is observed and the optimistic value is reverted with its reason.</summary>
+    [Fact]
+    public async Task A_faulted_dispatch_is_observed_and_reverts_the_optimistic_value()
+    {
+        var (host, server) = await BootAsync();
+        try
+        {
+            var nm = server.NodeManager!;
+            nm.EnsureVariable("eq-1/sp", null, "Sp", "Int32", writable: true, realm: AddressSpaceRealm.Uns);
+            nm.WriteValue("eq-1/sp", 7, OpcUaStatus.Good, DateTime.UtcNow, AddressSpaceRealm.Uns);
+            var node = nm.TryGetVariable("eq-1/sp")!;
+
+            var result = nm.DispatchDeviceWrite(
+                new FaultingGateway(), "ns=2;s=eq-1/sp", "eq-1/sp", AddressSpaceRealm.Uns,
+                optimisticValue: 42, priorValue: 7, priorStatus: StatusCodes.Good, clientUserId: "op");
+            ServiceResult.IsGood(result).ShouldBeTrue("the dispatch itself started; the SDK applies the value optimistically");
+            node.Value = 42; // what the SDK does on Good
+
+            var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(5);
+            while (DateTime.UtcNow < deadline && !Equals(node.Value, 7)) await Task.Delay(20, Ct);
+            node.Value.ShouldBe(7, "the faulted dispatch must revert the optimistic value");
+            node.StatusCode.ShouldBe((StatusCode)StatusCodes.Good);
+        }
+        finally { await host.DisposeAsync(); }
+    }
+
+    /// <summary>Positive control for the two above: a gateway that succeeds leaves the optimistic value standing.</summary>
+    [Fact]
+    public async Task A_successful_dispatch_leaves_the_optimistic_value()
+    {
+        var (host, server) = await BootAsync();
+        try
+        {
+            var nm = server.NodeManager!;
+            nm.EnsureVariable("eq-1/sp", null, "Sp", "Int32", writable: true, realm: AddressSpaceRealm.Uns);
+            nm.WriteValue("eq-1/sp", 7, OpcUaStatus.Good, DateTime.UtcNow, AddressSpaceRealm.Uns);
+            var node = nm.TryGetVariable("eq-1/sp")!;
+
+            nm.DispatchDeviceWrite(
+                new SucceedingGateway(), "ns=2;s=eq-1/sp", "eq-1/sp", AddressSpaceRealm.Uns,
+                optimisticValue: 42, priorValue: 7, priorStatus: StatusCodes.Good, clientUserId: "op");
+            node.Value = 42;
+
+            await Task.Delay(200, Ct);
+            node.Value.ShouldBe(42);
+        }
+        finally { await host.DisposeAsync(); }
+    }
+
+    /// <summary>H4 — the revert stamps the manager's clock, not the wall clock.</summary>
+    [Fact]
+    public async Task The_revert_timestamps_come_from_the_injected_clock()
+    {
+        var (host, server) = await BootAsync();
+        try
+        {
+            var nm = server.NodeManager!;
+            var fixedNow = new DateTime(2030, 1, 2, 3, 4, 5, DateTimeKind.Utc);
+            nm.Clock = new FixedClock(fixedNow);
+            nm.EnsureVariable("eq-1/sp", null, "Sp", "Int32", writable: true, realm: AddressSpaceRealm.Uns);
+            nm.WriteValue("eq-1/sp", 7, OpcUaStatus.Good, new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc), AddressSpaceRealm.Uns);
+            var node = nm.TryGetVariable("eq-1/sp")!;
+            node.Value = 42;
+
+            nm.RevertOptimisticWriteIfNeeded(
+                "eq-1/sp", new NodeWriteOutcome(false, "rejected"), optimisticValue: 42, priorValue: 7,
+                priorStatus: StatusCodes.Good, clientUserId: "op", realm: AddressSpaceRealm.Uns);
+
+            node.Timestamp.ShouldBe(fixedNow);
+            OtOpcUaNodeManager.RemainingShelveMs(fixedNow.AddSeconds(30), fixedNow).ShouldBe(30_000);
+        }
+        finally { await host.DisposeAsync(); }
+    }
+
+    private async Task<(OpcUaApplicationHost Host, OtOpcUaSdkServer Server)> BootAsync() =>
+        await OpcUaHostStart.StartAsync(
+            port => new OpcUaApplicationHost(
+                new OpcUaApplicationHostOptions
+                {
+                    ApplicationName = "OtOpcUa.DispatchTest",
+                    ApplicationUri = $"urn:OtOpcUa.DispatchTest:{Guid.NewGuid():N}",
+                    OpcUaPort = port,
+                    PublicHostname = "localhost",
+                    PkiStoreRoot = _pkiRoot,
+                },
+                NullLogger<OpcUaApplicationHost>.Instance),
+            () => new OtOpcUaSdkServer(), Ct);
+
+    public void Dispose()
+    {
+        if (Directory.Exists(_pkiRoot)) Directory.Delete(_pkiRoot, recursive: true);
+    }
+
+    private sealed class ThrowingGateway : IOpcUaNodeWriteGateway
+    {
+        public Task<NodeWriteOutcome> WriteAsync(string nodeId, object? value, AddressSpaceRealm realm, CancellationToken ct) =>
+            throw new InvalidOperationException("boom");
+    }
+
+    private sealed class FaultingGateway : IOpcUaNodeWriteGateway
+    {
+        public Task<NodeWriteOutcome> WriteAsync(string nodeId, object? value, AddressSpaceRealm realm, CancellationToken ct) =>
+            Task.FromException<NodeWriteOutcome>(new TimeoutException("gateway ask timed out"));
+    }
+
+    private sealed class SucceedingGateway : IOpcUaNodeWriteGateway
+    {
+        public Task<NodeWriteOutcome> WriteAsync(string nodeId, object? value, AddressSpaceRealm realm, CancellationToken ct) =>
+            Task.FromResult(new NodeWriteOutcome(true, null));
+    }
+
+    private sealed class FixedClock(DateTime utcNow) : TimeProvider
+    {
+        public override DateTimeOffset GetUtcNow() => new(utcNow, TimeSpan.Zero);
+    }
+}
@@ -0,0 +1,74 @@
+using Opc.Ua;
+using Shouldly;
+using Xunit;
+
+namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests;
+
+/// <summary>Review 01 P6 — the SDK request and transport quotas are configurable (<c>OpcUa:Quotas:*</c>) and
+/// default to exactly what used to be hard-coded.</summary>
+public sealed class OpcUaApplicationHostQuotaTests
+{
+    [Fact]
+    public void Default_request_quotas_are_the_former_hard_coded_values()
+    {
+        var config = new ServerConfiguration();
+        OpcUaApplicationHost.ApplyRequestQuotas(config, new OpcUaQuotaOptions());
+
+        config.MinRequestThreadCount.ShouldBe(5);
+        config.MaxRequestThreadCount.ShouldBe(100);
+        config.MaxQueuedRequestCount.ShouldBe(200);
+    }
+
+    [Fact]
+    public void Configured_request_quotas_are_applied()
+    {
+        var config = new ServerConfiguration();
+        OpcUaApplicationHost.ApplyRequestQuotas(config, new OpcUaQuotaOptions
+        {
+            MinRequestThreadCount = 8, MaxRequestThreadCount = 250, MaxQueuedRequestCount = 1000,
+        });
+
+        config.MinRequestThreadCount.ShouldBe(8);
+        config.MaxRequestThreadCount.ShouldBe(250);
+        config.MaxQueuedRequestCount.ShouldBe(1000);
+    }
+
+    [Fact]
+    public void Unset_transport_quotas_keep_the_sdk_defaults()
+    {
+        var sdkDefault = new TransportQuotas();
+        var built = OpcUaApplicationHost.BuildTransportQuotas(new OpcUaQuotaOptions());
+
+        built.OperationTimeout.ShouldBe(sdkDefault.OperationTimeout);
+        built.MaxStringLength.ShouldBe(sdkDefault.MaxStringLength);
+        built.MaxByteStringLength.ShouldBe(sdkDefault.MaxByteStringLength);
+        built.MaxArrayLength.ShouldBe(sdkDefault.MaxArrayLength);
+        built.MaxMessageSize.ShouldBe(sdkDefault.MaxMessageSize);
+        built.MaxBufferSize.ShouldBe(sdkDefault.MaxBufferSize);
+        built.ChannelLifetime.ShouldBe(sdkDefault.ChannelLifetime);
+        built.SecurityTokenLifetime.ShouldBe(sdkDefault.SecurityTokenLifetime);
+    }
+
+    [Fact]
+    public void Configured_transport_quotas_replace_only_what_is_set()
+    {
+        var sdkDefault = new TransportQuotas();
+        var built = OpcUaApplicationHost.BuildTransportQuotas(new OpcUaQuotaOptions
+        {
+            MaxMessageSize = 8 * 1024 * 1024, OperationTimeoutMs = 45_000, MaxArrayLength = 1_000_000,
+        });
+
+        built.MaxMessageSize.ShouldBe(8 * 1024 * 1024);
+        built.OperationTimeout.ShouldBe(45_000);
+        built.MaxArrayLength.ShouldBe(1_000_000);
+        built.MaxStringLength.ShouldBe(sdkDefault.MaxStringLength);
+        built.ChannelLifetime.ShouldBe(sdkDefault.ChannelLifetime);
+    }
+
+    [Fact]
+    public void History_read_batch_cap_default_scales_with_the_machine()
+    {
+        OtOpcUaNodeManager.DefaultMaxConcurrentHistoryReadBatches
+            .ShouldBe(Math.Clamp(Environment.ProcessorCount * 2, 4, 16));
+    }
+}