From a537d29f4450cb04241937359a2788963140a53c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:48:30 -0400 Subject: [PATCH] refactor(r2-04): internal IMxWriteOps seam under GatewayGalaxyDataWriter (sealed SDK session untestable) --- ...2-04-failure-visibility-plan.md.tasks.json | 2 +- .../Runtime/GatewayGalaxyDataWriter.cs | 120 +++++++++++++++--- .../GatewayGalaxyDataWriterFailClosedTests.cs | 75 +++++++++++ 3 files changed, 180 insertions(+), 17 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyDataWriterFailClosedTests.cs diff --git a/archreview/plans/R2-04-failure-visibility-plan.md.tasks.json b/archreview/plans/R2-04-failure-visibility-plan.md.tasks.json index 6acb1ceb..8ff30a3e 100644 --- a/archreview/plans/R2-04-failure-visibility-plan.md.tasks.json +++ b/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"] }, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GatewayGalaxyDataWriter.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GatewayGalaxyDataWriter.cs index 1389b51c..67bb23e3 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GatewayGalaxyDataWriter.cs +++ b/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; } + /// Test seam: drive the real single-write pipeline against a fake + /// so the fail-closed / unconfirmed behaviour (archreview 06/S-1) is covered offline. Behaviourally + /// identical to the per-entry path runs. + /// The (fake) MX write operations. + /// The write request. + /// The tag's security classification. + /// The cancellation token. + /// The translated write result. + internal Task WriteOneForTestAsync( + IMxWriteOps ops, WriteRequest request, SecurityClassification classification, CancellationToken ct) + => WriteOneAsync(ops, request, classification, ct); + private async Task 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 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. /// 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. /// private static Task 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); } /// @@ -301,3 +317,75 @@ public sealed class GatewayGalaxyDataWriter : IGalaxyDataWriter return new WriteResult(StatusCodeMap.Good); } } + +/// +/// 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 MxGatewaySession +/// types are sealed with internal ctors and cannot be faked. Production binds +/// over the real (MxGatewaySession, serverHandle); tests inject a +/// fake that records calls and returns canned replies. +/// +internal interface IMxWriteOps +{ + /// The gateway session id stamped onto every . + string SessionId { get; } + + /// The MXAccess server handle carried in each item/advise/write command. + int ServerHandle { get; } + + /// Add an MXAccess item and return its item handle. + /// The dotted tag full reference. + /// The cancellation token. + /// The MXAccess item handle. + Task AddItemAsync(string fullRef, CancellationToken ct); + + /// Invoke a raw (AdviseSupervisory / WriteSecured) and return the reply. + /// The command request. + /// The cancellation token. + /// The gateway reply. + Task InvokeAsync(MxCommandRequest request, CancellationToken ct); + + /// Issue a raw (non-secured) Write for the given item handle. + /// The MXAccess item handle to write. + /// The encoded MX value. + /// The write user id (typically 0 — the supervisory path commits it). + /// The cancellation token. + /// The gateway reply. + Task WriteRawAsync(int itemHandle, MxValue value, int userId, CancellationToken ct); +} + +/// Production — a thin adapter over a live +/// and its server handle. Behaviour is bit-identical to the previous +/// inline (session, serverHandle) calls. +internal sealed class SessionMxWriteOps : IMxWriteOps +{ + private readonly MxGatewaySession _session; + private readonly int _serverHandle; + + /// Initializes the adapter over a connected session + server handle. + /// The connected gateway session. + /// The MXAccess server handle for this session. + public SessionMxWriteOps(MxGatewaySession session, int serverHandle) + { + _session = session; + _serverHandle = serverHandle; + } + + /// + public string SessionId => _session.SessionId; + + /// + public int ServerHandle => _serverHandle; + + /// + public Task AddItemAsync(string fullRef, CancellationToken ct) + => _session.AddItemAsync(_serverHandle, fullRef, ct); + + /// + public Task InvokeAsync(MxCommandRequest request, CancellationToken ct) + => _session.InvokeAsync(request, ct); + + /// + public Task WriteRawAsync(int itemHandle, MxValue value, int userId, CancellationToken ct) + => _session.WriteRawAsync(_serverHandle, itemHandle, value, userId, ct); +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyDataWriterFailClosedTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyDataWriterFailClosedTests.cs new file mode 100644 index 00000000..ec3c375f --- /dev/null +++ b/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; + +/// +/// archreview 06/S-1: Galaxy writes must fail CLOSED. When AdviseSupervisory 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 BadCommunicationError so the #5 node-revert fires instead +/// of leaving a phantom-Good node. These tests drive the real write pipeline through the internal +/// seam (the SDK session types are sealed + internal-ctor and cannot be faked). +/// +public sealed class GatewayGalaxyDataWriterFailClosedTests +{ + private static GatewayGalaxyDataWriter NewWriter() + => new(new GalaxyMxSession(new Config.GalaxyMxAccessOptions(ClientName: "OtOpcUa-Test")), writeUserId: 0); + + // ---------------- T5: seam ---------------- + + /// A secured-write classification routes through WriteSecured and never advises (regression via the seam). + [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 ---------------- + + /// A fake that records every call and returns canned replies. + private sealed class FakeMxWriteOps : IMxWriteOps + { + public string SessionId => "sess-1"; + public int ServerHandle => 100; + public int NextHandle { get; init; } = 7; + + public List AddItemRefs { get; } = new(); + public List Invokes { get; } = new(); + public List RawWriteHandles { get; } = new(); + + /// Responder for ; null ⇒ an OK (empty) reply. + public Func? InvokeResponder { get; init; } + /// Responder for ; null ⇒ an empty (Good) reply. + public Func? RawWriteResponder { get; init; } + + public Task AddItemAsync(string fullRef, CancellationToken ct) + { + AddItemRefs.Add(fullRef); + return Task.FromResult(NextHandle); + } + + public Task InvokeAsync(MxCommandRequest request, CancellationToken ct) + { + Invokes.Add(request); + return Task.FromResult(InvokeResponder?.Invoke(request) ?? new MxCommandReply()); + } + + public Task WriteRawAsync(int itemHandle, MxValue value, int userId, CancellationToken ct) + { + RawWriteHandles.Add(itemHandle); + return Task.FromResult(RawWriteResponder?.Invoke() ?? new MxCommandReply()); + } + } +}