refactor(r2-04): internal IMxWriteOps seam under GatewayGalaxyDataWriter (sealed SDK session untestable)

archreview/plans/R2-04-failure-visibility-plan.md.tasks.json

@@ -6,7 +6,7 @@
     { "id": "T2", "subject": "Safe* helpers return bool; Apply tallies removal-pass failures into FailedNodes", "status": "completed", "blockedBy": ["T1"] },
     { "id": "T3", "subject": "Materialise* passes return swallowed-failure counts (void -> int, five methods)", "status": "completed", "blockedBy": ["T2"] },
     { "id": "T4", "subject": "OpcUaApplyFailed counter + OpcUaPublishActor Error/meter surfacing of degraded applies", "status": "completed", "blockedBy": ["T3"] },
-    { "id": "T5", "subject": "Galaxy internal IMxWriteOps seam under GatewayGalaxyDataWriter (sealed SDK session untestable) (06/S-1)", "status": "pending", "blockedBy": [] },
+    { "id": "T5", "subject": "Galaxy internal IMxWriteOps seam under GatewayGalaxyDataWriter (sealed SDK session untestable) (06/S-1)", "status": "completed", "blockedBy": [] },
     { "id": "T6", "subject": "Galaxy fail-closed write on AdviseSupervisory failure -> BadCommunicationError, no raw write", "status": "pending", "blockedBy": ["T5"] },
     { "id": "T7", "subject": "Galaxy counters galaxy.writes.advise_failed + galaxy.writes.unconfirmed (empty-statuses Good metered)", "status": "pending", "blockedBy": ["T6"] },
     { "id": "T8", "subject": "Galaxy skip-gated live smoke: normal advised write still returns Good post-change", "status": "pending", "blockedBy": ["T6"] },

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GatewayGalaxyDataWriter.cs

@@ -129,23 +129,39 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter
                 "GalaxyMxSession is not connected. Call ConnectAsync before issuing writes.");
         var serverHandle = _session.ServerHandle;
 
+        // Bind the per-batch MX write operations behind the internal IMxWriteOps seam (see below) so the
+        // whole write pipeline is unit-testable without the sealed SDK session (archreview 06/S-1).
+        var ops = new SessionMxWriteOps(session, serverHandle);
+
         var results = new WriteResult[writes.Count];
         for (var i = 0; i < writes.Count; i++)
         {
-            results[i] = await WriteOneAsync(session, serverHandle, writes[i],
+            results[i] = await WriteOneAsync(ops, writes[i],
                 securityResolver(writes[i].FullReference), cancellationToken)
                 .ConfigureAwait(false);
         }
         return results;
     }
 
+    /// <summary>Test seam: drive the real single-write pipeline against a fake <see cref="IMxWriteOps"/>
+    /// so the fail-closed / unconfirmed behaviour (archreview 06/S-1) is covered offline. Behaviourally
+    /// identical to the per-entry path <see cref="WriteAsync"/> runs.</summary>
+    /// <param name="ops">The (fake) MX write operations.</param>
+    /// <param name="request">The write request.</param>
+    /// <param name="classification">The tag's security classification.</param>
+    /// <param name="ct">The cancellation token.</param>
+    /// <returns>The translated write result.</returns>
+    internal Task<WriteResult> WriteOneForTestAsync(
+        IMxWriteOps ops, WriteRequest request, SecurityClassification classification, CancellationToken ct)
+        => WriteOneAsync(ops, request, classification, ct);
+
     private async Task<WriteResult> WriteOneAsync(
-        MxGatewaySession session, int serverHandle, WriteRequest request,
+        IMxWriteOps ops, WriteRequest request,
         SecurityClassification classification, CancellationToken ct)
     {
         try
         {
-            var itemHandle = await EnsureItemHandleAsync(session, serverHandle, request.FullReference, ct)
+            var itemHandle = await EnsureItemHandleAsync(ops, request.FullReference, ct)
                 .ConfigureAwait(false);
             var mxValue = MxValueEncoder.Encode(request.Value);
 
@@ -154,7 +170,7 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter
             {
                 // SecuredWrite/VerifiedWrite tags carry their own ArchestrA user identity
                 // (current/verifier user), so they don't use the supervisory path.
-                reply = await InvokeWriteSecuredAsync(session, serverHandle, itemHandle, mxValue, ct)
+                reply = await InvokeWriteSecuredAsync(ops, itemHandle, mxValue, ct)
                     .ConfigureAwait(false);
             }
             else
@@ -163,8 +179,8 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter
                 // only COMMITS the value when the item is advised in SUPERVISORY mode. Without it
                 // the gateway's Write call doesn't throw (reply looks OK) but the value never
                 // reaches the galaxy. AdviseSupervisory once per handle, then Write.
-                await EnsureSupervisoryAdvisedAsync(session, serverHandle, itemHandle, ct).ConfigureAwait(false);
-                reply = await session.WriteRawAsync(serverHandle, itemHandle, mxValue, _writeUserId, ct)
+                await EnsureSupervisoryAdvisedAsync(ops, itemHandle, ct).ConfigureAwait(false);
+                reply = await ops.WriteRawAsync(itemHandle, mxValue, _writeUserId, ct)
                     .ConfigureAwait(false);
             }
 
@@ -189,10 +205,10 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter
         classification is SecurityClassification.SecuredWrite or SecurityClassification.VerifiedWrite;
 
     private async Task<int> EnsureItemHandleAsync(
-        MxGatewaySession session, int serverHandle, string fullRef, CancellationToken ct)
+        IMxWriteOps ops, string fullRef, CancellationToken ct)
     {
         if (TryResolveCachedOrBorrowed(fullRef) is int resolved) return resolved;
-        var handle = await session.AddItemAsync(serverHandle, fullRef, ct).ConfigureAwait(false);
+        var handle = await ops.AddItemAsync(fullRef, ct).ConfigureAwait(false);
         Interlocked.Increment(ref _addItemCallCount);
         _itemHandles[fullRef] = handle;
         return handle;
@@ -211,26 +227,26 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter
     ///     re-advises — the supervisory state never outlives the handle it was taken against.
     /// </summary>
     private async Task EnsureSupervisoryAdvisedAsync(
-        MxGatewaySession session, int serverHandle, int itemHandle, CancellationToken ct)
+        IMxWriteOps ops, int itemHandle, CancellationToken ct)
     {
         if (!_supervisedHandles.TryAdd(itemHandle, 0)) return;
 
         var request = new MxCommandRequest
         {
-            SessionId = session.SessionId,
+            SessionId = ops.SessionId,
             ClientCorrelationId = Guid.NewGuid().ToString("N"),
             Command = new MxCommand
             {
                 Kind = MxCommandKind.AdviseSupervisory,
                 AdviseSupervisory = new AdviseSupervisoryCommand
                 {
-                    ServerHandle = serverHandle,
+                    ServerHandle = ops.ServerHandle,
                     ItemHandle = itemHandle,
                 },
             },
         };
 
-        var reply = await session.InvokeAsync(request, ct).ConfigureAwait(false);
+        var reply = await ops.InvokeAsync(request, ct).ConfigureAwait(false);
         if (reply.ProtocolStatus is { } proto && proto.Code != ProtocolStatusCode.Ok)
         {
             // Supervisory advise failed — forget it so the next write retries, and let the
@@ -250,14 +266,14 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter
     ///     interprets the underlying MXAccess command kind.
     /// </summary>
     private static Task<MxCommandReply> InvokeWriteSecuredAsync(
-        MxGatewaySession session, int serverHandle, int itemHandle, MxValue value, CancellationToken ct)
+        IMxWriteOps ops, int itemHandle, MxValue value, CancellationToken ct)
     {
         var command = new MxCommand
         {
             Kind = MxCommandKind.WriteSecured,
             WriteSecured = new WriteSecuredCommand
             {
-                ServerHandle = serverHandle,
+                ServerHandle = ops.ServerHandle,
                 ItemHandle = itemHandle,
                 Value = value,
                 CurrentUserId = 0,
@@ -266,11 +282,11 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter
         };
         var request = new MxCommandRequest
         {
-            SessionId = session.SessionId,
+            SessionId = ops.SessionId,
             ClientCorrelationId = Guid.NewGuid().ToString("N"),
             Command = command,
         };
-        return session.InvokeAsync(request, ct);
+        return ops.InvokeAsync(request, ct);
     }
 
     /// <summary>
@@ -301,3 +317,75 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter
         return new WriteResult(StatusCodeMap.Good);
     }
 }
+
+/// <summary>
+///     The per-write MXAccess gateway operations the writer needs, extracted behind an internal seam so the
+///     fail-closed write pipeline (archreview 06/S-1) is unit-testable — the SDK <c>MxGatewaySession</c>
+///     types are sealed with internal ctors and cannot be faked. Production binds
+///     <see cref="SessionMxWriteOps"/> over the real <c>(MxGatewaySession, serverHandle)</c>; tests inject a
+///     fake that records calls and returns canned replies.
+/// </summary>
+internal interface IMxWriteOps
+{
+    /// <summary>The gateway session id stamped onto every <see cref="MxCommandRequest"/>.</summary>
+    string SessionId { get; }
+
+    /// <summary>The MXAccess server handle carried in each item/advise/write command.</summary>
+    int ServerHandle { get; }
+
+    /// <summary>Add an MXAccess item and return its item handle.</summary>
+    /// <param name="fullRef">The dotted tag full reference.</param>
+    /// <param name="ct">The cancellation token.</param>
+    /// <returns>The MXAccess item handle.</returns>
+    Task<int> AddItemAsync(string fullRef, CancellationToken ct);
+
+    /// <summary>Invoke a raw <see cref="MxCommand"/> (AdviseSupervisory / WriteSecured) and return the reply.</summary>
+    /// <param name="request">The command request.</param>
+    /// <param name="ct">The cancellation token.</param>
+    /// <returns>The gateway reply.</returns>
+    Task<MxCommandReply> InvokeAsync(MxCommandRequest request, CancellationToken ct);
+
+    /// <summary>Issue a raw (non-secured) Write for the given item handle.</summary>
+    /// <param name="itemHandle">The MXAccess item handle to write.</param>
+    /// <param name="value">The encoded MX value.</param>
+    /// <param name="userId">The write user id (typically 0 — the supervisory path commits it).</param>
+    /// <param name="ct">The cancellation token.</param>
+    /// <returns>The gateway reply.</returns>
+    Task<MxCommandReply> WriteRawAsync(int itemHandle, MxValue value, int userId, CancellationToken ct);
+}
+
+/// <summary>Production <see cref="IMxWriteOps"/> — a thin adapter over a live
+/// <see cref="MxGatewaySession"/> and its server handle. Behaviour is bit-identical to the previous
+/// inline <c>(session, serverHandle)</c> calls.</summary>
+internal sealed class SessionMxWriteOps : IMxWriteOps
+{
+    private readonly MxGatewaySession _session;
+    private readonly int _serverHandle;
+
+    /// <summary>Initializes the adapter over a connected session + server handle.</summary>
+    /// <param name="session">The connected gateway session.</param>
+    /// <param name="serverHandle">The MXAccess server handle for this session.</param>
+    public SessionMxWriteOps(MxGatewaySession session, int serverHandle)
+    {
+        _session = session;
+        _serverHandle = serverHandle;
+    }
+
+    /// <inheritdoc />
+    public string SessionId => _session.SessionId;
+
+    /// <inheritdoc />
+    public int ServerHandle => _serverHandle;
+
+    /// <inheritdoc />
+    public Task<int> AddItemAsync(string fullRef, CancellationToken ct)
+        => _session.AddItemAsync(_serverHandle, fullRef, ct);
+
+    /// <inheritdoc />
+    public Task<MxCommandReply> InvokeAsync(MxCommandRequest request, CancellationToken ct)
+        => _session.InvokeAsync(request, ct);
+
+    /// <inheritdoc />
+    public Task<MxCommandReply> WriteRawAsync(int itemHandle, MxValue value, int userId, CancellationToken ct)
+        => _session.WriteRawAsync(_serverHandle, itemHandle, value, userId, ct);
+}

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyDataWriterFailClosedTests.cs

@@ -0,0 +1,75 @@
+using ZB.MOM.WW.MxGateway.Contracts.Proto;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Runtime;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests.Runtime;
+
+/// <summary>
+///     archreview 06/S-1: Galaxy writes must fail CLOSED. When <c>AdviseSupervisory</c> fails, the raw
+///     Write (which the file comment notes "doesn't throw but the value never reaches the galaxy") must
+///     NOT be issued — the writer returns <c>BadCommunicationError</c> so the #5 node-revert fires instead
+///     of leaving a phantom-Good node. These tests drive the real write pipeline through the internal
+///     <see cref="IMxWriteOps"/> seam (the SDK session types are sealed + internal-ctor and cannot be faked).
+/// </summary>
+public sealed class GatewayGalaxyDataWriterFailClosedTests
+{
+    private static GatewayGalaxyDataWriter NewWriter()
+        => new(new GalaxyMxSession(new Config.GalaxyMxAccessOptions(ClientName: "OtOpcUa-Test")), writeUserId: 0);
+
+    // ---------------- T5: seam ----------------
+
+    /// <summary>A secured-write classification routes through WriteSecured and never advises (regression via the seam).</summary>
+    [Fact]
+    public async Task WriteOne_SecuredClassification_RoutesThroughWriteSecured_NoAdvise()
+    {
+        var ops = new FakeMxWriteOps();
+        var writer = NewWriter();
+
+        var result = await writer.WriteOneForTestAsync(
+            ops, new WriteRequest("Tag.Sec", true), SecurityClassification.SecuredWrite, CancellationToken.None);
+
+        result.StatusCode.ShouldBe(StatusCodeMap.Good);
+        ops.Invokes.Count(r => r.Command.Kind == MxCommandKind.WriteSecured).ShouldBe(1);
+        ops.Invokes.Any(r => r.Command.Kind == MxCommandKind.AdviseSupervisory).ShouldBeFalse();
+        ops.RawWriteHandles.ShouldBeEmpty();
+    }
+
+    // ---------------- fakes ----------------
+
+    /// <summary>A fake <see cref="IMxWriteOps"/> that records every call and returns canned replies.</summary>
+    private sealed class FakeMxWriteOps : IMxWriteOps
+    {
+        public string SessionId => "sess-1";
+        public int ServerHandle => 100;
+        public int NextHandle { get; init; } = 7;
+
+        public List<string> AddItemRefs { get; } = new();
+        public List<MxCommandRequest> Invokes { get; } = new();
+        public List<int> RawWriteHandles { get; } = new();
+
+        /// <summary>Responder for <see cref="InvokeAsync"/>; null ⇒ an OK (empty) reply.</summary>
+        public Func<MxCommandRequest, MxCommandReply>? InvokeResponder { get; init; }
+        /// <summary>Responder for <see cref="WriteRawAsync"/>; null ⇒ an empty (Good) reply.</summary>
+        public Func<MxCommandReply>? RawWriteResponder { get; init; }
+
+        public Task<int> AddItemAsync(string fullRef, CancellationToken ct)
+        {
+            AddItemRefs.Add(fullRef);
+            return Task.FromResult(NextHandle);
+        }
+
+        public Task<MxCommandReply> InvokeAsync(MxCommandRequest request, CancellationToken ct)
+        {
+            Invokes.Add(request);
+            return Task.FromResult(InvokeResponder?.Invoke(request) ?? new MxCommandReply());
+        }
+
+        public Task<MxCommandReply> WriteRawAsync(int itemHandle, MxValue value, int userId, CancellationToken ct)
+        {
+            RawWriteHandles.Add(itemHandle);
+            return Task.FromResult(RawWriteResponder?.Invoke() ?? new MxCommandReply());
+        }
+    }
+}