From 55b12dc2170c1cdc02df1e15ceb6cb0fdc44390c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 10 Aug 2026 06:53:41 -0400 Subject: [PATCH] fix(historian-gateway): map alarm severity numerically; dispose provisioner's gRPC channel Driver.Historian.Gateway-001 (High): AlarmEventMapper wrote the alarm's Severity wire property as the .NET enum name (e.g. "High"), but EventMapper's read side int.TryParse()s that same property and silently falls back to the minimum severity on any unparseable value - so every alarm event's severity round-tripped through this module's own mappers collapsed to 1 (Low), contradicting AlarmHistorianEvent.Severity's own documented contract ("mapped to Historian's numeric priority"). Now mapped through a numeric priority mirroring the existing NativeAlarmProjector.MapSeverity convention (200/500/700/900). Driver.Historian.Gateway-002 (Low): GatewayTagProvisioner never disposed its IHistorianGatewayClient, unlike its GatewayHistorianDataSource / GatewayAlarmHistorianWriter siblings. Now implements IAsyncDisposable so the DI-owned singleton's gRPC channel closes gracefully at shutdown. Both TDD: new/corrected tests confirmed RED against the pre-fix code. --- .../GatewayTagProvisioner.cs | 11 ++++++- .../Mapping/AlarmEventMapper.cs | 22 ++++++++++++- .../GatewayTagProvisionerTests.cs | 21 +++++++++++++ .../Mapping/AlarmEventMapperTests.cs | 31 ++++++++++++++++++- 4 files changed, 82 insertions(+), 3 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayTagProvisioner.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayTagProvisioner.cs index 986f82ab..b160f04f 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayTagProvisioner.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayTagProvisioner.cs @@ -17,7 +17,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway; /// as and returns. The method never throws and never /// logs tag values, hostnames, or credentials. /// -public sealed class GatewayTagProvisioner : IHistorianProvisioning +public sealed class GatewayTagProvisioner : IHistorianProvisioning, IAsyncDisposable { /// /// Storage rate (ms) stamped on every provisioned analog tag. The gateway's EnsureTags path @@ -104,4 +104,13 @@ public sealed class GatewayTagProvisioner : IHistorianProvisioning return new HistorianProvisionResult(requests.Count, Ensured: 0, Skipped: skipped, Failed: definitions.Count); } } + + /// + /// Disposes the underlying gateway client (and its gRPC channel). The DI container owns this + /// provisioner as a singleton (AddHistorianProvisioning), so this fires once at host + /// shutdown — closing the channel gracefully, matching the read () + /// and alarm-write () paths' dispose ownership. + /// + /// A task that represents the asynchronous operation. + public ValueTask DisposeAsync() => _client.DisposeAsync(); } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Mapping/AlarmEventMapper.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Mapping/AlarmEventMapper.cs index a0112dcd..bdda3fc5 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Mapping/AlarmEventMapper.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Mapping/AlarmEventMapper.cs @@ -1,5 +1,6 @@ using Google.Protobuf.WellKnownTypes; using ZB.MOM.WW.HistorianGateway.Contracts.Grpc; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian; namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Mapping; @@ -10,6 +11,25 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Mapping; /// internal static class AlarmEventMapper { + /// + /// Maps to the numeric OPC UA Part 9 priority (1-1000) carried in the + /// wire event's Severity property, per the + /// contract ("Mapped to Historian's numeric priority on the sink side"). The read side + /// () int.TryParses this same property, so it + /// must stay a numeric string — the enum's .ToString() name (e.g. "High") is NOT + /// parseable and used to silently collapse every read-back severity to the minimum (1). The + /// values mirror the existing native-alarm convention + /// (ZB.MOM.WW.OtOpcUa.Runtime.Drivers.NativeAlarmProjector.MapSeverity). + /// + private static string ToNumericSeverity(AlarmSeverity severity) => severity switch + { + AlarmSeverity.Low => "200", + AlarmSeverity.Medium => "500", + AlarmSeverity.High => "700", + AlarmSeverity.Critical => "900", + _ => "500", + }; + /// Maps an alarm historian event to a gateway event. /// The driver-agnostic alarm event. /// The gateway wire event ready for SendEvent. @@ -36,7 +56,7 @@ internal static class AlarmEventMapper historianEvent.Properties["AlarmId"] = alarm.AlarmId; historianEvent.Properties["AlarmName"] = alarm.AlarmName; historianEvent.Properties["EventKind"] = alarm.EventKind; - historianEvent.Properties["Severity"] = alarm.Severity.ToString(); + historianEvent.Properties["Severity"] = ToNumericSeverity(alarm.Severity); historianEvent.Properties["User"] = alarm.User; historianEvent.Properties["Message"] = alarm.Message; if (alarm.Comment is not null) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayTagProvisionerTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayTagProvisionerTests.cs index b03a9af9..798a0dca 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayTagProvisionerTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayTagProvisionerTests.cs @@ -149,6 +149,27 @@ public sealed class GatewayTagProvisionerTests Assert.Equal(1, result.Failed); // non-blocking: no throw } + /// + /// Driver.Historian.Gateway-002: unlike its siblings (GatewayHistorianDataSource, + /// GatewayAlarmHistorianWriter), had no dispose path at + /// all — the gRPC channel GatewayHistorian.CreateProvisioner opens was never closed. The + /// provisioner is registered as a process-lifetime DI singleton + /// (AddHistorianProvisioning), and the built-in container disposes any singleton instance + /// it creates that implements / — regardless + /// of the registered service-interface type — so implementing it here is sufficient for a clean + /// shutdown with no other wiring change. + /// + [Fact] + public async Task DisposeAsync_disposes_underlying_client() + { + var fake = new FakeHistorianGatewayClient(); + var p = Provisioner(fake); + + await ((IAsyncDisposable)p).DisposeAsync(); + + Assert.Equal(1, fake.DisposeCallCount); + } + [Fact] public async Task Ensured_count_reflects_successful_results() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/AlarmEventMapperTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/AlarmEventMapperTests.cs index f26784ec..e0b489b1 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/AlarmEventMapperTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/AlarmEventMapperTests.cs @@ -23,12 +23,41 @@ public sealed class AlarmEventMapperTests Assert.Equal("A1", e.Properties["AlarmId"]); Assert.Equal("HiHi", e.Properties["AlarmName"]); Assert.Equal("Activated", e.Properties["EventKind"]); - Assert.Equal("High", e.Properties["Severity"]); + // Severity is the AlarmHistorianEvent.Severity XML doc contract: "Mapped to Historian's + // numeric priority on the sink side" — NOT the .NET enum name. EventMapper.ParseSeverity on + // the read side int.TryParse()s this same property, so a non-numeric string (the old bug: + // alarm.Severity.ToString() == "High") silently always read back as the minimum severity (1). + Assert.Equal("700", e.Properties["Severity"]); Assert.Equal("operator1", e.Properties["User"]); Assert.Equal("ack note", e.Properties["Comment"]); Assert.Equal("Temp high", e.Properties["Message"]); } + /// + /// Driver.Historian.Gateway-001 regression: the write side () and + /// the read side () must agree on the wire representation of severity — + /// a numeric OPC UA Part 9 priority string, matching the convention already used elsewhere in the + /// codebase for the same enum (NativeAlarmProjector.MapSeverity: Low=200/Medium=500/ + /// High=700/Critical=900). Round-tripping through both mappers must reproduce the numeric severity, + /// not collapse to the minimum every time. + /// + [Theory] + [InlineData(AlarmSeverity.Low, (ushort)200)] + [InlineData(AlarmSeverity.Medium, (ushort)500)] + [InlineData(AlarmSeverity.High, (ushort)700)] + [InlineData(AlarmSeverity.Critical, (ushort)900)] + public void Severity_round_trips_through_EventMapper_as_numeric_priority(AlarmSeverity severity, ushort expected) + { + var a = new AlarmHistorianEvent("A1", "Area/Line/Pump1", "HiHi", "LimitAlarm", + severity, "Activated", "m", "operator1", null, + new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc)); + + var wire = AlarmEventMapper.ToHistorianEvent(a); + var readBack = EventMapper.ToHistoricalEvent(wire); + + Assert.Equal(expected, readBack.Severity); + } + [Fact] public void Null_comment_is_omitted_not_null() {