worker(alarms): UnAdvise only advised handles in LmxSubtagAlarmSource teardown

B3: track advised handles separately from added handles so Dispose only UnAdvises
items that were actually advised — a write-only subtag (e.g. ack-comment added by
Write, never advised) is removed but not unadvised. Add Dispose tests covering the
advised/write-only split, full removal, single Unregister, and double-dispose
idempotency.
This commit is contained in:
Joseph Doherty
2026-06-14 02:35:59 -04:00
parent a3752799de
commit 986dcee14a
2 changed files with 79 additions and 10 deletions
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using ZB.MOM.WW.MxGateway.Worker.MxAccess;
namespace ZB.MOM.WW.MxGateway.Worker.Tests.MxAccess;
@@ -133,6 +134,60 @@ public sealed class LmxSubtagAlarmSourceTests
Assert.Single(server.Writes);
}
/// <summary>
/// Verifies <see cref="LmxSubtagAlarmSource.Dispose"/> UnAdvises only the
/// handles that were actually advised — a write-only item (added by
/// <see cref="LmxSubtagAlarmSource.Write"/> but never advised) is removed
/// but not unadvised — and unregisters the server exactly once.
/// </summary>
[Fact]
public void Dispose_UnAdvisesOnlyAdvisedHandles_RemovesAll_AndUnregistersOnce()
{
var server = new RecordingMxAccessServer();
var source = new LmxSubtagAlarmSource(server, FakeServerHandle);
source.Advise(new[] { "Tank1.Alarm.Subtag", "Tank2.Alarm.Subtag" });
// A write-only subtag: added by Write, never advised.
source.Write("Tank1.Alarm.AckComment", "acknowledged");
int advised1 = server.LastItemHandleFor("Tank1.Alarm.Subtag");
int advised2 = server.LastItemHandleFor("Tank2.Alarm.Subtag");
int writeOnly = server.LastItemHandleFor("Tank1.Alarm.AckComment");
source.Dispose();
// Only the two advised handles are unadvised — never the write-only one.
Assert.Equal(new[] { advised1, advised2 }, server.UnAdvisedItemHandles);
Assert.DoesNotContain(writeOnly, server.UnAdvisedItemHandles);
// Every added item (advised + write-only) is removed.
Assert.Equal(
new[] { advised1, advised2, writeOnly }.OrderBy(h => h),
server.RemovedItemHandles.OrderBy(h => h));
Assert.Equal(1, server.UnregisterCount);
}
/// <summary>
/// Verifies <see cref="LmxSubtagAlarmSource.Dispose"/> is idempotent: a
/// second call performs no further teardown.
/// </summary>
[Fact]
public void Dispose_IsIdempotent()
{
var server = new RecordingMxAccessServer();
var source = new LmxSubtagAlarmSource(server, FakeServerHandle);
source.Advise(new[] { "Tank1.Alarm.Subtag" });
source.Dispose();
int unadviseAfterFirst = server.UnAdvisedItemHandles.Count;
int unregisterAfterFirst = server.UnregisterCount;
source.Dispose();
Assert.Equal(unadviseAfterFirst, server.UnAdvisedItemHandles.Count);
Assert.Equal(unregisterAfterFirst, server.UnregisterCount);
}
/// <summary>
/// Recording <see cref="IMxAccessServer"/> test double that captures the
/// AddItem/Advise/Write/UnAdvise/RemoveItem/Unregister calls
@@ -152,13 +207,17 @@ public sealed class LmxSubtagAlarmSourceTests
public List<WriteRecord> Writes { get; } = new();
public List<int> UnAdvisedItemHandles { get; } = new();
public List<int> RemovedItemHandles { get; } = new();
public int UnregisterCount { get; private set; }
public int LastItemHandleFor(string itemAddress) => handlesByAddress[itemAddress];
public int Register(string clientName) => FakeServerHandle;
public void Unregister(int serverHandle)
{
}
public void Unregister(int serverHandle) => UnregisterCount++;
public int AddItem(int serverHandle, string itemDefinition)
{
@@ -171,9 +230,7 @@ public sealed class LmxSubtagAlarmSourceTests
public int AddItem2(int serverHandle, string itemDefinition, string itemContext)
=> AddItem(serverHandle, itemDefinition);
public void RemoveItem(int serverHandle, int itemHandle)
{
}
public void RemoveItem(int serverHandle, int itemHandle) => RemovedItemHandles.Add(itemHandle);
public void Advise(int serverHandle, int itemHandle)
{
@@ -181,9 +238,7 @@ public sealed class LmxSubtagAlarmSourceTests
AdvisedServerHandles.Add(serverHandle);
}
public void UnAdvise(int serverHandle, int itemHandle)
{
}
public void UnAdvise(int serverHandle, int itemHandle) => UnAdvisedItemHandles.Add(itemHandle);
public void AdviseSupervisory(int serverHandle, int itemHandle)
{
@@ -46,6 +46,12 @@ public sealed class LmxSubtagAlarmSource : ISubtagAlarmSource
private readonly Dictionary<int, string> addressesByItemHandle =
new Dictionary<int, string>();
// Handles that were actually Advise()d, tracked separately from the added
// set so Dispose only UnAdvises advised items. Write() can AddItem a
// write-only subtag (e.g. an ack-comment that was never advised); calling
// UnAdvise on such a handle would be an unbalanced teardown.
private readonly HashSet<int> advisedItemHandles = new HashSet<int>();
private object? mxAccessComObject;
private IMxAccessServer? server;
private LMXProxyServerClass? comEventSource;
@@ -134,6 +140,7 @@ public sealed class LmxSubtagAlarmSource : ISubtagAlarmSource
mxServer.Advise(serverHandle, itemHandle);
itemHandlesByAddress[itemAddress!] = itemHandle;
addressesByItemHandle[itemHandle] = itemAddress!;
advisedItemHandles.Add(itemHandle);
}
}
@@ -225,7 +232,13 @@ public sealed class LmxSubtagAlarmSource : ISubtagAlarmSource
{
foreach (KeyValuePair<int, string> entry in addressesByItemHandle)
{
try { mxServer.UnAdvise(serverHandle, entry.Key); } catch { /* swallow — best effort */ }
// Only UnAdvise handles that were actually advised; a write-only
// item (added by Write but never Advise'd) was never advised.
if (advisedItemHandles.Contains(entry.Key))
{
try { mxServer.UnAdvise(serverHandle, entry.Key); } catch { /* swallow — best effort */ }
}
try { mxServer.RemoveItem(serverHandle, entry.Key); } catch { /* swallow — best effort */ }
}
@@ -234,6 +247,7 @@ public sealed class LmxSubtagAlarmSource : ISubtagAlarmSource
itemHandlesByAddress.Clear();
addressesByItemHandle.Clear();
advisedItemHandles.Clear();
object? comToRelease = mxAccessComObject;
mxAccessComObject = null;