diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/AlarmAckWriterTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/AlarmAckWriterTests.cs index ce400c4..fc1630b 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/AlarmAckWriterTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/AlarmAckWriterTests.cs @@ -93,6 +93,68 @@ public sealed class AlarmAckWriterTests Assert.Equal(1, server.Operations.Count(op => op == "Advise")); } + [Fact] + public void Acknowledge_WhenAdviseThrows_RejectsCleansUpAndReAddsNextTime() + { + RecordingAckServer server = new RecordingAckServer { AuthenticateResult = 5, AdviseException = new COMException("x", unchecked((int)0x80004005)) }; + using AlarmAckWriter writer = new AlarmAckWriter(server, new MxAccessWriteCompletionCache(), () => { }, TimeSpan.FromMilliseconds(10), "MxGateway"); + + AlarmAckResult first = writer.Acknowledge("T.A.AckMsg", "one", Alice()); + server.AdviseException = null; + writer.Acknowledge("T.A.AckMsg", "two", Alice()); + + Assert.Equal(AlarmAckOutcome.WriteRejected, first.Outcome); + Assert.Equal(unchecked((int)0x80004005), first.NativeStatus); + Assert.DoesNotContain("pw-secret", first.Diagnostic, StringComparison.Ordinal); + Assert.Contains("RemoveItem", server.Operations); + Assert.Equal(2, server.Operations.Count(op => op.StartsWith("AddItem", StringComparison.Ordinal))); + } + + [Fact] + public void Acknowledge_WhenCachedWriteThrows_RejectsAndReAddsNextTime() + { + RecordingAckServer server = new RecordingAckServer { AuthenticateResult = 5 }; + using AlarmAckWriter writer = new AlarmAckWriter(server, new MxAccessWriteCompletionCache(), () => { }, TimeSpan.FromMilliseconds(10), "MxGateway"); + writer.Acknowledge("T.A.AckMsg", "one", Alice()); + server.WriteException = new COMException("x", unchecked((int)0x80004005)); + + AlarmAckResult failed = writer.Acknowledge("T.A.AckMsg", "two", Alice()); + server.WriteException = null; + writer.Acknowledge("T.A.AckMsg", "three", Alice()); + + Assert.Equal(AlarmAckOutcome.WriteRejected, failed.Outcome); + Assert.Equal(2, server.Operations.Count(op => op.StartsWith("AddItem", StringComparison.Ordinal))); + } + + [Fact] + public void Acknowledge_WhenWriteDisconnected_TearsDownServer() + { + // The test seam has no COM factory, so re-registration itself is not + // exercised here; this verifies the teardown that makes it possible. + RecordingAckServer server = new RecordingAckServer { AuthenticateResult = 5, WriteException = new COMException("x", unchecked((int)0x80010108)) }; + using AlarmAckWriter writer = new AlarmAckWriter(server, new MxAccessWriteCompletionCache(), () => { }, TimeSpan.FromMilliseconds(10), "MxGateway"); + + AlarmAckResult result = writer.Acknowledge("T.A.AckMsg", "one", Alice()); + + Assert.Equal(AlarmAckOutcome.WriteRejected, result.Outcome); + Assert.Equal(unchecked((int)0x80010108), result.NativeStatus); + Assert.Contains("Unregister", server.Operations); + } + + [Fact] + public void Acknowledge_WhenNonOkCompletionHasNoDetail_ReturnsMinusOne() + { + RecordingAckServer server = new RecordingAckServer { AuthenticateResult = 5 }; + MxAccessWriteCompletionCache cache = new MxAccessWriteCompletionCache(); + server.OnWrite = (handle, item) => cache.Record(handle, item, Rows(MxStatusCategory.SecurityError, 0)); + using AlarmAckWriter writer = new AlarmAckWriter(server, cache, () => { }, TimeSpan.FromSeconds(1), "MxGateway"); + + AlarmAckResult result = writer.Acknowledge("T.A.AckMsg", "checked", Alice()); + + Assert.Equal(AlarmAckOutcome.WriteRejected, result.Outcome); + Assert.Equal(-1, result.NativeStatus); + } + [Fact] public void AlarmAckOperator_ToString_OmitsPassword() { @@ -110,6 +172,10 @@ internal sealed class RecordingAckServer : IMxAccessServer public Exception? AuthenticateException { get; set; } + public Exception? AdviseException { get; set; } + + public Exception? WriteException { get; set; } + public string? LastPassword { get; private set; } public Action? OnWrite { get; set; } @@ -132,7 +198,14 @@ internal sealed class RecordingAckServer : IMxAccessServer public void RemoveItem(int serverHandle, int itemHandle) => Operations.Add("RemoveItem"); - public void Advise(int serverHandle, int itemHandle) => Operations.Add("Advise"); + public void Advise(int serverHandle, int itemHandle) + { + Operations.Add("Advise"); + if (AdviseException is not null) + { + throw AdviseException; + } + } public void UnAdvise(int serverHandle, int itemHandle) => Operations.Add("UnAdvise"); @@ -143,6 +216,11 @@ internal sealed class RecordingAckServer : IMxAccessServer public void Write(int serverHandle, int itemHandle, object? value, int userId) { Operations.Add("Write:" + value + ":" + userId); + if (WriteException is not null) + { + throw WriteException; + } + OnWrite?.Invoke(serverHandle, itemHandle); } diff --git a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/AlarmAckWriter.cs b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/AlarmAckWriter.cs index 6f61402..ce9bb39 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/AlarmAckWriter.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/AlarmAckWriter.cs @@ -111,17 +111,48 @@ public sealed class AlarmAckWriter : IAlarmAckWriter $"Operator authentication failed for '{ackOperator.VerifyUser}' (no user id returned)."); } - if (!itemHandles.TryGetValue(itemAddress, out int itemHandle)) + ulong baselineVersion = 0; + int itemHandle = 0; + bool cached = itemHandles.TryGetValue(itemAddress, out itemHandle); + string step = "AddItem"; + try { - itemHandle = mx.AddItem(serverHandle, itemAddress); - // MXAccess rejects writes to added-but-unadvised items with E_INVALIDARG. - mx.Advise(serverHandle, itemHandle); - itemHandles[itemAddress] = itemHandle; + if (!cached) + { + itemHandle = mx.AddItem(serverHandle, itemAddress); + itemHandles[itemAddress] = itemHandle; + step = "Advise"; + // MXAccess rejects writes to added-but-unadvised items with E_INVALIDARG. + mx.Advise(serverHandle, itemHandle); + } + + pumpStep(); + step = "Write"; + baselineVersion = completionCache.CurrentVersion(serverHandle, itemHandle); + mx.Write(serverHandle, itemHandle, comment ?? string.Empty, userId); + } + catch (COMException ex) + { + int hresult = ex.HResult; + if (itemHandles.Remove(itemAddress)) + { + try { mx.UnAdvise(serverHandle, itemHandle); } catch { /* best effort */ } + try { mx.RemoveItem(serverHandle, itemHandle); } catch { /* best effort */ } + } + + if (IsServerGone(hresult)) + { + TearDownServer(); + } + + // Diagnostic names the step and HRESULT only: exception text can echo the item or operator. + return AlarmAckResult.Create( + AlarmAckOutcome.WriteRejected, + hresult, + $"AckMsg {step} failed (HRESULT 0x{hresult:X8})."); } - pumpStep(); - ulong baseline = completionCache.CurrentVersion(serverHandle, itemHandle); - mx.Write(serverHandle, itemHandle, comment ?? string.Empty, userId); + ulong baseline = baselineVersion; if (!completionCache.TryWaitForCompletion( serverHandle, @@ -158,6 +189,17 @@ public sealed class AlarmAckWriter : IAlarmAckWriter if (disposed) return; disposed = true; + TearDownServer(); + } + + private static bool IsServerGone(int hresult) => + hresult == unchecked((int)0x80010108) // RPC_E_DISCONNECTED + || hresult == unchecked((int)0x800706BA) // RPC_S_SERVER_UNAVAILABLE + || hresult == unchecked((int)0x80010105); // RPC_E_SERVERFAULT + + /// Releases the MXAccess registration so the next call re-registers (also used by Dispose). + private void TearDownServer() + { if (comEventSource is not null) { try { comEventSource.OnWriteComplete -= OnWriteComplete; } catch { /* best effort */ } @@ -190,7 +232,12 @@ public sealed class AlarmAckWriter : IAlarmAckWriter { if (server is not null) return server; - object created = factory!.Create() ?? throw new InvalidOperationException("MXAccess COM factory returned null."); + if (factory is null) + { + throw new InvalidOperationException("MXAccess server is gone and no COM factory is available to re-register."); + } + + object created = factory.Create() ?? throw new InvalidOperationException("MXAccess COM factory returned null."); try { comObject = created;