From 0f1f8ad8fac99894fad8af3cd8178fbef7c07561 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 18:21:46 -0400 Subject: [PATCH] fix(adminui): lane D-b review r0 fixes (I1, M1, M2, M5-M9, N1-N3, N5, H3) I1: /hosts eviction keyed off the AUTHORED DriverInstances, so a driver deleted in /raw but not yet deployed - still running, still publishing - vanished every tick. The stores' sweep is now EvictUnconfiguredStale(set, olderThanUtc): an entry goes only when unconfigured AND its PublishedUtc is older than 60 s. Tests cover unconfigured+fresh kept (positive control), unconfigured+stale evicted, configured+stale kept, and the deploy hand-off. M1: Test Connect falls back to the driver's documented ProbeTimeoutSeconds default (Galaxy 30, OpcUaClient 15, ...) when the key is absent. M2: the budget never undercuts the config's root timeoutMs (+1 s). M5: RawBrowseModal compares the open generation captured before the await, so a close-then-reopen mid-open closes the stale session. M6: /alarms-historian keeps a 30 s re-poll on 'not registered' (a driver node may still be starting) instead of stopping for good. M7: /scripts source fetch failure shows in the dialog, not the error boundary. M8: the export modal maps only its preview rows; the endpoint writes via Results.Text (no second byte[] copy). M9: /hosts config queries are ordered, so SequenceEqual change detection holds. N1 tooltip, N2 CRLF-safe regex, N3 throttled Warning for subscriber faults, N5 equipment-import help text now describes RFC 4180 quoting. H3: MonacoEditor documents that themes are page-global. Refs #745 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../Clients/HistorianSinkStatusReader.cs | 16 +++- .../Components/Pages/AlarmsHistorian.razor | 24 +++-- .../Components/Pages/Hosts.razor | 20 +++-- .../Components/Pages/Scripts.razor | 26 +++++- .../Drivers/DriverTestConnectButton.razor | 6 +- .../Shared/Drivers/TestConnectBudget.cs | 79 +++++++++++++---- .../Components/Shared/MonacoEditor.razor | 4 + .../Shared/Raw/RawBrowseModal.razor | 7 +- .../Shared/Raw/RawCsvExportModal.razor | 9 +- .../Shared/Uns/ImportEquipmentModal.razor | 2 +- .../Hubs/IDriverResilienceStatusStore.cs | 15 ++-- .../Hubs/IDriverStatusSnapshotStore.cs | 15 ++-- .../Hubs/IInProcessBroadcaster.cs | 5 +- .../InMemoryDriverResilienceStatusStore.cs | 11 ++- .../Hubs/InMemoryDriverStatusSnapshotStore.cs | 11 ++- .../Hubs/SafeMulticast.cs | 29 +++++++ .../Uns/RawTagCsvExport.cs | 22 +++-- .../AlarmsHistorianReaderTests.cs | 17 +++- .../Drivers/TestConnectBudgetTests.cs | 21 ++++- .../Hubs/StoreEvictionAndIsolationTests.cs | 87 ++++++++++++++----- .../Pickers/BrowseOpenGuardTests.cs | 2 +- .../Raw/RawBrowseRebirthRefreshTests.cs | 47 +++++++++- .../Raw/RawCsvRenderingTests.cs | 16 ++++ .../ScriptList/ScriptListQueriesTests.cs | 14 +++ .../Telemetry/TelemetryDialDeadlineTests.cs | 4 +- .../Telemetry/TelemetryDialSupervisorTests.cs | 4 +- 26 files changed, 411 insertions(+), 102 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Clients/HistorianSinkStatusReader.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Clients/HistorianSinkStatusReader.cs index 9e97cea1..eb741d83 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Clients/HistorianSinkStatusReader.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Clients/HistorianSinkStatusReader.cs @@ -10,8 +10,8 @@ public enum HistorianSinkAvailability /// The sink answered; is current. Available, - /// This node runs no historian adapter actor (an admin-only node, without the driver role). - /// Not transient: polling again cannot change it. + /// This node has no historian adapter actor registered — an admin-only node, or a driver node + /// that has not finished starting. Re-polled slowly (). NotOnThisRole, /// The actor exists but did not answer (timeout or failure); the last good status, if any, is stale. @@ -31,6 +31,18 @@ public sealed record HistorianSinkReading(HistorianSinkAvailability Availability /// public static class HistorianSinkStatusReader { + /// The normal poll cadence. + public static readonly TimeSpan PollInterval = TimeSpan.FromSeconds(5); + + /// The slow re-poll while the actor is not registered (admin-only node, or a driver node mid-startup). + public static readonly TimeSpan NotRegisteredPollInterval = TimeSpan.FromSeconds(30); + + /// The poll interval after a reading with . + /// The last reading's availability, or null before the first. + /// The interval until the next poll. + public static TimeSpan PollIntervalFor(HistorianSinkAvailability? availability) => + availability == HistorianSinkAvailability.NotOnThisRole ? NotRegisteredPollInterval : PollInterval; + /// The per-poll Ask timeout. public static readonly TimeSpan AskTimeout = TimeSpan.FromSeconds(2); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/AlarmsHistorian.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/AlarmsHistorian.razor index 453fc140..35bcdd04 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/AlarmsHistorian.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/AlarmsHistorian.razor @@ -17,11 +17,11 @@

Alarms historian sink

-
Polling every @PollSeconds s.
+
Polling every @((int)(_interval == TimeSpan.Zero ? PollSeconds : _interval.TotalSeconds)) s.
@if (_reading is { Availability: HistorianSinkAvailability.NotOnThisRole }) { -
@_reading.Message
+
@_reading.Message Checking again every @((int)HistorianSinkStatusReader.NotRegisteredPollInterval.TotalSeconds) s in case the node is still starting.
} else if (_status is null && _reading is { Availability: HistorianSinkAvailability.Unavailable }) { @@ -71,16 +71,17 @@ else private HistorianSinkReading? _reading; private Timer? _timer; private int _tickRunning; + private TimeSpan _interval; private readonly CancellationTokenSource _cts = new(); private volatile bool _disposed; protected override async Task OnInitializedAsync() { await RefreshAsync(); - // No historian on this role is permanent — don't poll for it. - if (_reading?.Availability == HistorianSinkAvailability.NotOnThisRole) return; - _timer = new Timer(_ => _ = InvokeAsync(RefreshAsync), null, - TimeSpan.FromSeconds(PollSeconds), TimeSpan.FromSeconds(PollSeconds)); + // "Not registered" is usually permanent (an admin-only node) but can also be a driver node still + // starting up, so it keeps a slow re-poll instead of stopping for good (review r0 M6). + _interval = HistorianSinkStatusReader.PollIntervalFor(_reading?.Availability); + _timer = new Timer(_ => _ = InvokeAsync(RefreshAsync), null, _interval, _interval); } private async Task RefreshAsync() @@ -105,7 +106,16 @@ else Volatile.Write(ref _tickRunning, 0); } - if (!_disposed) StateHasChanged(); + if (_disposed) return; + var interval = HistorianSinkStatusReader.PollIntervalFor(_reading?.Availability); + if (_timer is not null && interval != _interval) + { + _interval = interval; + try { _timer.Change(interval, interval); } + catch (ObjectDisposedException) { return; } + } + + StateHasChanged(); } private static string StateChipClass(HistorianDrainState state) => state switch diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor index 67c9be56..c2238207 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor @@ -329,6 +329,9 @@ else @code { private const int RefreshIntervalSeconds = 5; + // An unconfigured driver's snapshot must be this old before eviction (≈ 12 health publishes). + private static readonly TimeSpan EvictUnconfiguredAfter = TimeSpan.FromSeconds(60); + private List? _rows; private bool _refreshing; private DateTime? _lastRefreshUtc; @@ -434,22 +437,29 @@ else try { await using var db = await DbFactory.CreateDbContextAsync(_cts.Token); + // Ordered: the change test below compares with SequenceEqual (review r0 M9). _nodes = await db.ClusterNodes.AsNoTracking() + .OrderBy(n => n.ClusterId).ThenBy(n => n.NodeId) .Select(n => new HostsNodeInfo(n.ClusterId, n.NodeId, n.Host, n.OpcUaPort)) .ToListAsync(_cts.Token); _instances = await db.DriverInstances.AsNoTracking() + .OrderBy(d => d.DriverInstanceId) .Select(d => new HostsDriverInstanceInfo(d.DriverInstanceId, d.ClusterId, d.Name, d.DriverType, d.Enabled)) .ToListAsync(_cts.Token); // Evict snapshots of driver instances that are no longer configured (deleted, or re-created - // under a new id). The stores have no other removal path, so without this a removed driver - // stays on this page as a frozen row until the admin node restarts. Only a SUCCESSFUL load - // reaches here; the failure path below blanks _instances and must never sweep the stores. + // under a new id) AND have stopped publishing. The stores have no other removal path, so + // without this a removed driver stays on this page as a frozen row until the admin node + // restarts. DriverInstances is the AUTHORED table: a driver deleted in /raw keeps running (and + // publishing every few seconds) until the next deploy, so staleness is what proves it stopped. + // Only a SUCCESSFUL load reaches here; the failure path below blanks _instances and must + // never sweep the stores. var configured = _instances .Select(i => i.DriverInstanceId) .ToHashSet(StringComparer.OrdinalIgnoreCase); - var evicted = DriverStore.RetainOnly(configured); - ResilienceStore.RetainOnly(configured); + var staleBefore = DateTime.UtcNow - EvictUnconfiguredAfter; + var evicted = DriverStore.EvictUnconfiguredStale(configured, staleBefore); + ResilienceStore.EvictUnconfiguredStale(configured, staleBefore); return evicted > 0 || !_nodes.SequenceEqual(previousNodes) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Scripts.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Scripts.razor index 219b5eaa..38279f86 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Scripts.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Scripts.razor @@ -89,7 +89,11 @@ else
- @if (src is null || (_sourceLoaded && _sourceCode is null)) + @if (_sourceError is not null) + { +
@_sourceError
+ } + else if (src is null || (_sourceLoaded && _sourceCode is null)) {

This script is no longer loaded.

} @@ -123,6 +127,7 @@ else // operator opened another row, or closed the dialog, while the first one was in flight). private string? _sourceCode; private bool _sourceLoaded; + private string? _sourceError; private int _sourceGeneration; protected override async Task OnInitializedAsync() @@ -147,8 +152,22 @@ else _sourceCode = null; _sourceLoaded = false; - await using var db = await DbFactory.CreateDbContextAsync(); - var source = await ScriptListQueries.LoadSourceAsync(db, scriptId); + _sourceError = null; + string? source; + try + { + await using var db = await DbFactory.CreateDbContextAsync(); + source = await ScriptListQueries.LoadSourceAsync(db, scriptId); + } + catch (Exception ex) + { + // A ConfigDb hiccup must not escape the click handler into the page's error boundary (review r0 M7). + if (generation != _sourceGeneration) return; + _sourceError = $"The script source could not be loaded: {ex.Message}"; + _sourceLoaded = true; + return; + } + if (generation != _sourceGeneration) return; _sourceCode = source; _sourceLoaded = true; @@ -160,5 +179,6 @@ else _sourceScriptId = null; _sourceCode = null; _sourceLoaded = false; + _sourceError = null; } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverTestConnectButton.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverTestConnectButton.razor index d4db4d28..06ec04ec 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverTestConnectButton.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverTestConnectButton.razor @@ -5,7 +5,7 @@
@@ -36,7 +36,7 @@ /// /// Fallback probe budget, used only when the config JSON carries no positive - /// ProbeTimeoutSeconds. The configured value wins (it is every driver's documented "Test Connect + /// ProbeTimeoutSeconds and the driver has no documented default (TestConnectBudget.DriverDefaults). The configured value wins (it is every driver's documented "Test Connect /// probe timeout"), so it can lengthen the budget as well as shorten it. Clamped to [1, 60] s. /// [Parameter] public int TimeoutSeconds { get; set; } = TestConnectBudget.DefaultSeconds; @@ -62,7 +62,7 @@ try { var json = GetConfigJson() ?? "{}"; - var budget = TestConnectBudget.Resolve(json, TimeoutSeconds); + var budget = TestConnectBudget.Resolve(json, TimeoutSeconds, DriverType); _lastBudgetSeconds = budget; _result = await Probe.TestAsync(DriverType, json, budget); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/TestConnectBudget.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/TestConnectBudget.cs index b6c00635..192d314d 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/TestConnectBudget.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/TestConnectBudget.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers; @@ -24,34 +25,80 @@ public static class TestConnectBudget /// The config property that carries the budget (matched case-insensitively at the JSON root). public const string PropertyName = "ProbeTimeoutSeconds"; - /// Returns the configured budget, or when there is none. + /// The root per-request timeout some probes (Modbus) use as their own deadline, in ms. + public const string RequestTimeoutPropertyName = "timeoutMs"; + + /// + /// Each driver's documented ProbeTimeoutSeconds default (its options type's initializer), used + /// when the config carries no value — legacy rows and raw-JSON/seeded configs omit the key, and the + /// form hints promise these defaults (review r0 M1). Drivers not listed fall back to the caller's value. + /// + public static readonly IReadOnlyDictionary DriverDefaults = + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + [DriverTypeNames.Galaxy] = 30, + [DriverTypeNames.OpcUaClient] = 15, + [DriverTypeNames.TwinCAT] = 10, + [DriverTypeNames.FOCAS] = 10, + [DriverTypeNames.Modbus] = 5, + [DriverTypeNames.AbCip] = 5, + [DriverTypeNames.AbLegacy] = 5, + [DriverTypeNames.S7] = 5, + }; + + /// Returns the configured budget, else the driver's documented default, else . /// The (merged) config JSON the probe will run against. - /// The budget when the config has no positive ProbeTimeoutSeconds. + /// The budget when neither the config nor supplies one. + /// The driver type, selecting its documented default; null skips that step. /// The budget in seconds, within [, ]. - public static int Resolve(string? configJson, int fallbackSeconds = DefaultSeconds) + /// + /// A probe that runs under the config's own timeoutMs (Modbus, factory parity) is hard-cancelled + /// at this budget by the probe actor, so the budget is never set below ceil(timeoutMs / 1000) + 1 + /// (review r0 M2): a slow gateway that the deployed driver reaches within its 8 s request timeout must + /// not fail Test Connect at a 5 s budget. + /// + public static int Resolve(string? configJson, int fallbackSeconds = DefaultSeconds, string? driverType = null) { - var configured = TryReadConfigured(configJson); - var seconds = configured is > 0 ? configured.Value : fallbackSeconds; + var (configured, requestTimeoutMs) = TryRead(configJson); + var seconds = configured is > 0 + ? configured.Value + : driverType is not null && DriverDefaults.TryGetValue(driverType, out var d) ? d : fallbackSeconds; + + if (requestTimeoutMs is > 0) + { + var floor = (int)Math.Min(Math.Ceiling(requestTimeoutMs.Value / 1000.0) + 1, int.MaxValue); + seconds = Math.Max(seconds, floor); + } + return Math.Clamp(seconds, MinSeconds, MaxSeconds); } - private static int? TryReadConfigured(string? configJson) + private static (int? ProbeTimeoutSeconds, double? RequestTimeoutMs) TryRead(string? configJson) { - if (string.IsNullOrWhiteSpace(configJson)) return null; + if (string.IsNullOrWhiteSpace(configJson)) return (null, null); + int? probe = null; + double? request = null; try { using var doc = JsonDocument.Parse(configJson); - if (doc.RootElement.ValueKind != JsonValueKind.Object) return null; + if (doc.RootElement.ValueKind != JsonValueKind.Object) return (null, null); foreach (var prop in doc.RootElement.EnumerateObject()) { - if (!string.Equals(prop.Name, PropertyName, StringComparison.OrdinalIgnoreCase)) continue; - return prop.Value.ValueKind switch + if (string.Equals(prop.Name, PropertyName, StringComparison.OrdinalIgnoreCase)) { - JsonValueKind.Number when prop.Value.TryGetInt32(out var i) => i, - JsonValueKind.Number when prop.Value.TryGetDouble(out var d) => (int)Math.Min(Math.Ceiling(d), int.MaxValue), - JsonValueKind.String when int.TryParse(prop.Value.GetString(), out var s) => s, - _ => null, - }; + probe = prop.Value.ValueKind switch + { + JsonValueKind.Number when prop.Value.TryGetInt32(out var i) => i, + JsonValueKind.Number when prop.Value.TryGetDouble(out var dd) => (int)Math.Min(Math.Ceiling(dd), int.MaxValue), + JsonValueKind.String when int.TryParse(prop.Value.GetString(), out var s) => s, + _ => null, + }; + } + else if (string.Equals(prop.Name, RequestTimeoutPropertyName, StringComparison.OrdinalIgnoreCase) + && prop.Value.ValueKind == JsonValueKind.Number && prop.Value.TryGetDouble(out var ms)) + { + request = ms; + } } } catch (JsonException) @@ -59,6 +106,6 @@ public static class TestConnectBudget // An unparseable config is the probe's problem to report; the budget just falls back. } - return null; + return (probe, request); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/MonacoEditor.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/MonacoEditor.razor index 9c763b57..86716e0c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/MonacoEditor.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/MonacoEditor.razor @@ -225,6 +225,10 @@ await SafeInvokeAsync("MonacoBlazor.setEditorOption", "toggle minimap", _id, "minimap", new { enabled = _minimap }); } + // R04 H3 (accepted, cosmetic): Monaco themes are page-global — the JS side calls + // monaco.editor.setTheme, which re-themes EVERY editor on the page (e.g. a VirtualTagModal open over + // ScriptEdit), not just this one, and the other instance's _dark flag then no longer matches what it shows. + // Monaco has no per-editor theme; a fix would need a page-level theme service shared by all instances. private async Task ToggleTheme() { _dark = !_dark; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor index 433d5533..8ced0c54 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor @@ -457,12 +457,15 @@ { _opening = true; _openError = null; + // Captured BEFORE the await: a close-then-reopen while this open is in flight leaves Visible true + // again, so only the generation tells this open's result apart from the new modal's (review r0 M5). + var generation = _openGeneration; StateHasChanged(); try { var result = await BrowserService.OpenAsync(_driverType, _mergedConfig, default); - // Host disposed or modal closed mid-open: close the session nobody will (R04 L4). - if (BrowseOpenGuard.CloseIfOrphaned(result, _disposed || !Visible, BrowserService)) { return; } + // Host disposed, modal closed, or modal re-opened mid-open: close the session nobody will (R04 L4). + if (BrowseOpenGuard.CloseIfOrphaned(result, _disposed || IsStale(generation), BrowserService)) { return; } if (result.Ok) { _token = result.Token; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawCsvExportModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawCsvExportModal.razor index f9e080a7..d701b6a7 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawCsvExportModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawCsvExportModal.razor @@ -116,8 +116,10 @@ try { _groupFilter = null; - // The same builder the download endpoint runs, so the preview matches the file. - var export = await RawTagCsvExport.BuildAsync(ExportReader, DeviceId, TagGroupId, DriverType); + // The same builder the download endpoint runs, so the preview matches the file — but mapping only + // the preview's rows (the header takes the first line), never the whole export (review r0 M8). + var export = await RawTagCsvExport.BuildAsync(ExportReader, DeviceId, TagGroupId, DriverType, + maxRows: RawTagCsvExport.PreviewLines - 1); if (IsStale(generation)) { return; } if (export is null) { @@ -129,7 +131,8 @@ _rowCount = export.RowCount; _fileName = export.FileName; // Only the bounded preview is kept on the component; the full CSV is not held by the circuit. - (_preview, _truncated) = RawTagCsvExport.Preview(export.Csv); + (_preview, var cut) = RawTagCsvExport.Preview(export.Csv); + _truncated = cut || export.RowCount > RawTagCsvExport.PreviewLines - 1; _downloadUrl = RawTagCsvExport.DownloadUrl(DeviceId, TagGroupId, DriverType); } catch (Exception ex) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/ImportEquipmentModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/ImportEquipmentModal.razor index 86da0727..a7cff5d3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/ImportEquipmentModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/ImportEquipmentModal.razor @@ -31,7 +31,7 @@ -
Simple comma-separated values only — fields containing commas are not supported.
+
RFC 4180 CSV: wrap a field containing commas, quotes or line breaks in double quotes (a quote inside is doubled: ""). The opening quote must come straight after the comma — a, "b" is rejected.
@if (!string.IsNullOrWhiteSpace(_parseError)) { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverResilienceStatusStore.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverResilienceStatusStore.cs index 631b31be..f9889051 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverResilienceStatusStore.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverResilienceStatusStore.cs @@ -28,15 +28,20 @@ public interface IDriverResilienceStatusStore IReadOnlyCollection GetAll(); /// - /// Evicts every snapshot whose driver instance is not in . - /// The stores have no other removal path, so a deleted (or re-created under a new id) driver would - /// otherwise keep its last snapshot until the process restarts. The /hosts page calls this - /// after each successful config load; a failed load must never call it with an empty set. + /// Evicts every snapshot whose driver instance is not in + /// and whose last publish is older than . The stores have no other + /// removal path, so a deleted (or re-created under a new id) driver would otherwise keep its last + /// snapshot until the process restarts. Both conditions are required: the configured set comes from + /// the AUTHORED (draft) table, so a driver deleted in /raw but not yet deployed is still + /// running and still publishing — it must stay visible until a deploy actually stops it and its + /// snapshot goes stale. The /hosts page calls this after each successful config load; a failed + /// load must never call it. /// /// The configured driver-instance ids to keep. The set's own /// comparer decides matching. + /// Only an unconfigured entry published before this instant is evicted. /// The number of entries removed. - int RetainOnly(IReadOnlySet configuredInstanceIds); + int EvictUnconfiguredStale(IReadOnlySet configuredInstanceIds, DateTime olderThanUtc); /// /// Raised after every with the just-stored snapshot. Lets the Blazor Server diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverStatusSnapshotStore.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverStatusSnapshotStore.cs index 8cfbd2aa..120c91e4 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverStatusSnapshotStore.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IDriverStatusSnapshotStore.cs @@ -26,15 +26,20 @@ public interface IDriverStatusSnapshotStore IReadOnlyCollection GetAll(); /// - /// Evicts every snapshot whose driver instance is not in . - /// The stores have no other removal path, so a deleted (or re-created under a new id) driver would - /// otherwise keep its last snapshot until the process restarts. The /hosts page calls this - /// after each successful config load; a failed load must never call it with an empty set. + /// Evicts every snapshot whose driver instance is not in + /// and whose last publish is older than . The stores have no other + /// removal path, so a deleted (or re-created under a new id) driver would otherwise keep its last + /// snapshot until the process restarts. Both conditions are required: the configured set comes from + /// the AUTHORED (draft) table, so a driver deleted in /raw but not yet deployed is still + /// running and still publishing — it must stay visible until a deploy actually stops it and its + /// snapshot goes stale. The /hosts page calls this after each successful config load; a failed + /// load must never call it. /// /// The configured driver-instance ids to keep. The set's own /// comparer decides matching. + /// Only an unconfigured entry published before this instant is evicted. /// The number of entries removed. - int RetainOnly(IReadOnlySet configuredInstanceIds); + int EvictUnconfiguredStale(IReadOnlySet configuredInstanceIds, DateTime olderThanUtc); /// /// Raised after every with the just-stored snapshot. Lets in-process diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IInProcessBroadcaster.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IInProcessBroadcaster.cs index 3a29265a..f14554b4 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IInProcessBroadcaster.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/IInProcessBroadcaster.cs @@ -62,6 +62,7 @@ public sealed class InProcessBroadcaster : IInProcessBroadcaster private readonly object _connectionLock = new(); private bool _isConnected; private readonly ILogger>? _logger; + private readonly FailureLogThrottle _failureLog = new(); /// Creates the broadcaster. /// Optional logger for a subscriber that throws during a raise. @@ -77,7 +78,7 @@ public sealed class InProcessBroadcaster : IInProcessBroadcaster // Capture-then-invoke so a concurrent unsubscribe can't null the delegate mid-raise; one subscriber // at a time so a throwing one cannot starve the rest or escape onto the bridge actor. public void Publish(T item) => - SafeMulticast.Invoke(Received, item, ex => _logger?.LogDebug(ex, "In-process broadcast subscriber threw")); + SafeMulticast.Invoke(Received, item, ex => _logger?.Log(_failureLog.NextLevel(), ex, "In-process broadcast subscriber threw")); /// public bool IsConnected @@ -109,6 +110,6 @@ public sealed class InProcessBroadcaster : IInProcessBroadcaster } SafeMulticast.Invoke(handler, connected, - ex => _logger?.LogDebug(ex, "In-process connection-state subscriber threw")); + ex => _logger?.Log(_failureLog.NextLevel(), ex, "In-process connection-state subscriber threw")); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverResilienceStatusStore.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverResilienceStatusStore.cs index 36b64bec..5cbf847a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverResilienceStatusStore.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverResilienceStatusStore.cs @@ -12,6 +12,7 @@ public sealed class InMemoryDriverResilienceStatusStore : IDriverResilienceStatu { private readonly ConcurrentDictionary<(string Instance, string Host), DriverResilienceStatusChanged> _byPair = new(); private readonly ILogger? _logger; + private readonly FailureLogThrottle _failureLog = new(); /// Creates the store. /// Optional logger for a subscriber that throws during a raise. @@ -28,16 +29,18 @@ public sealed class InMemoryDriverResilienceStatusStore : IDriverResilienceStatu // Capture-then-invoke so a concurrent unsubscribe can't null the delegate mid-raise; one // subscriber at a time so a throwing one cannot starve the rest (or escape onto the bridge actor). SafeMulticast.Invoke(SnapshotChanged, snapshot, - ex => _logger?.LogDebug(ex, "Driver resilience subscriber threw for {DriverInstanceId}", snapshot.DriverInstanceId)); + ex => _logger?.Log(_failureLog.NextLevel(), ex, "Driver resilience subscriber threw for {DriverInstanceId}", snapshot.DriverInstanceId)); } /// - public int RetainOnly(IReadOnlySet configuredInstanceIds) + public int EvictUnconfiguredStale(IReadOnlySet configuredInstanceIds, DateTime olderThanUtc) { var removed = 0; - foreach (var key in _byPair.Keys) + foreach (var (key, snapshot) in _byPair) { - if (!configuredInstanceIds.Contains(key.Instance) && _byPair.TryRemove(key, out _)) removed++; + if (configuredInstanceIds.Contains(key.Instance) || snapshot.PublishedUtc >= olderThanUtc) continue; + // Remove only the exact snapshot inspected: a fresh publish that raced in is kept. + if (_byPair.TryRemove(new KeyValuePair<(string Instance, string Host), DriverResilienceStatusChanged>(key, snapshot))) removed++; } return removed; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverStatusSnapshotStore.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverStatusSnapshotStore.cs index 21089cef..395fa670 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverStatusSnapshotStore.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/InMemoryDriverStatusSnapshotStore.cs @@ -12,6 +12,7 @@ public sealed class InMemoryDriverStatusSnapshotStore : IDriverStatusSnapshotSto { private readonly ConcurrentDictionary _byInstance = new(); private readonly ILogger? _logger; + private readonly FailureLogThrottle _failureLog = new(); /// Creates the store. /// Optional logger for a subscriber that throws during a raise. @@ -28,16 +29,18 @@ public sealed class InMemoryDriverStatusSnapshotStore : IDriverStatusSnapshotSto // Capture-then-invoke so a concurrent unsubscribe can't null the delegate mid-raise; one // subscriber at a time so a throwing one cannot starve the rest (or escape onto the bridge actor). SafeMulticast.Invoke(SnapshotChanged, snapshot, - ex => _logger?.LogDebug(ex, "Driver status subscriber threw for {DriverInstanceId}", snapshot.DriverInstanceId)); + ex => _logger?.Log(_failureLog.NextLevel(), ex, "Driver status subscriber threw for {DriverInstanceId}", snapshot.DriverInstanceId)); } /// - public int RetainOnly(IReadOnlySet configuredInstanceIds) + public int EvictUnconfiguredStale(IReadOnlySet configuredInstanceIds, DateTime olderThanUtc) { var removed = 0; - foreach (var id in _byInstance.Keys) + foreach (var (id, snapshot) in _byInstance) { - if (!configuredInstanceIds.Contains(id) && _byInstance.TryRemove(id, out _)) removed++; + if (configuredInstanceIds.Contains(id) || snapshot.PublishedUtc >= olderThanUtc) continue; + // Remove only the exact snapshot inspected: a fresh publish that raced in is kept. + if (_byInstance.TryRemove(new KeyValuePair(id, snapshot))) removed++; } return removed; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/SafeMulticast.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/SafeMulticast.cs index 610f5d4c..e8b9f8e1 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/SafeMulticast.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Hubs/SafeMulticast.cs @@ -1,5 +1,34 @@ +using Microsoft.Extensions.Logging; + namespace ZB.MOM.WW.OtOpcUa.AdminUI.Hubs; +/// +/// Logs a subscriber failure at Warning at most once per , and at Debug otherwise, +/// so a subscriber that throws on every publish is visible at the default level without flooding it. +/// +internal sealed class FailureLogThrottle +{ + /// The minimum gap between Warning-level entries. + public static readonly TimeSpan Interval = TimeSpan.FromMinutes(1); + + private readonly Func _nowMs; + private long _lastWarnMs = long.MinValue; + + /// Creates the throttle. + /// Monotonic milliseconds; when null. + public FailureLogThrottle(Func? nowMs = null) => _nowMs = nowMs ?? (() => Environment.TickCount64); + + /// The level the next failure should log at; claims the Warning slot when it is free. + /// at most once per interval, else . + public LogLevel NextLevel() + { + var now = _nowMs(); + var last = Interlocked.Read(ref _lastWarnMs); + if (last != long.MinValue && now - last < (long)Interval.TotalMilliseconds) return LogLevel.Debug; + return Interlocked.CompareExchange(ref _lastWarnMs, now, last) == last ? LogLevel.Warning : LogLevel.Debug; + } +} + /// /// Raises a multicast delegate one subscriber at a time, so a subscriber that throws cannot stop /// delivery to the subscribers after it. The in-process stores and broadcasters raise their events on diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTagCsvExport.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTagCsvExport.cs index 794d9d5e..d2683f1b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTagCsvExport.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTagCsvExport.cs @@ -32,9 +32,13 @@ public static class RawTagCsvExport /// When set, only this group's subtree is exported. /// The driver type (selects the typed columns); resolved from the device when blank. /// Cancellation token. + /// When set, only the first tags are mapped into + /// (the modal's preview — review r0 M8: it used to map the whole + /// export only to keep 50 lines); is always the full count. /// The export, or null when the target device cannot be resolved. public static async Task BuildAsync( - RawTagCsvExportReader reader, string? deviceId, string? tagGroupId, string? driverType, CancellationToken ct = default) + RawTagCsvExportReader reader, string? deviceId, string? tagGroupId, string? driverType, + CancellationToken ct = default, int? maxRows = null) { string? groupPath = null; if (!string.IsNullOrEmpty(tagGroupId)) @@ -66,7 +70,8 @@ public static class RawTagCsvExport } var list = tags.ToList(); - return new RawTagCsvExportResult(RawTagCsvMapper.Export(driverType, list), BuildFileName(deviceId), list.Count, groupPath); + var mapped = maxRows is { } max && max >= 0 && max < list.Count ? list.Take(max) : list; + return new RawTagCsvExportResult(RawTagCsvMapper.Export(driverType, mapped), BuildFileName(deviceId), list.Count, groupPath); } /// The first lines of , without copying the rest. @@ -118,14 +123,17 @@ public static class RawTagCsvExport /// The route handler builder. public static RouteHandlerBuilder MapRawTagCsvExport(this IEndpointRouteBuilder app) => app.MapGet(DownloadPath, async ( - string? deviceId, string? tagGroupId, string? driverType, + HttpContext http, string? deviceId, string? tagGroupId, string? driverType, RawTagCsvExportReader reader, CancellationToken ct) => { var export = await BuildAsync(reader, deviceId, tagGroupId, driverType, ct); - // File(..., fileDownloadName) sends Content-Disposition: attachment, so the browser saves it. - return export is null - ? Results.NotFound() - : Results.File(System.Text.Encoding.UTF8.GetBytes(export.Csv), "text/csv; charset=utf-8", export.FileName); + if (export is null) return Results.NotFound(); + + // Attachment, so the browser saves rather than renders. Results.Text writes the string + // through the response encoder, so no second whole-file byte[] copy is held (review r0 M8). + http.Response.Headers.ContentDisposition = + new System.Net.Mime.ContentDisposition { FileName = export.FileName, DispositionType = "attachment" }.ToString(); + return Results.Text(export.Csv, "text/csv; charset=utf-8", System.Text.Encoding.UTF8); }) .RequireAuthorization(AdminUiPolicies.ConfigEditor) .WithName("RawTagCsvExport"); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AlarmsHistorianReaderTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AlarmsHistorianReaderTests.cs index c31c07cc..38846017 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AlarmsHistorianReaderTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AlarmsHistorianReaderTests.cs @@ -73,6 +73,21 @@ public sealed class AlarmsHistorianReaderTests : TestKit HistorianSinkStatusReader.ReadAsync(() => silent.Ref, TimeSpan.FromSeconds(30), cts.Token)); } + [Fact] + public void Not_registered_keeps_a_slow_repoll_instead_of_stopping() + { + // Review r0 M6: a driver node opened mid-startup is "not registered" for a moment — never permanent. + HistorianSinkStatusReader.PollIntervalFor(HistorianSinkAvailability.NotOnThisRole) + .ShouldBe(HistorianSinkStatusReader.NotRegisteredPollInterval); + HistorianSinkStatusReader.PollIntervalFor(HistorianSinkAvailability.Available) + .ShouldBe(HistorianSinkStatusReader.PollInterval); + HistorianSinkStatusReader.PollIntervalFor(null).ShouldBe(HistorianSinkStatusReader.PollInterval); + + var src = AdminUiSource.Read("Components", "Pages", "AlarmsHistorian.razor"); + src.ShouldContain("_timer.Change(interval, interval)"); + src.ShouldNotContain("if (_reading?.Availability == HistorianSinkAvailability.NotOnThisRole) return;"); + } + [Fact] public void Page_uses_the_reader_and_the_async_dispose_lifecycle() { @@ -80,6 +95,6 @@ public sealed class AlarmsHistorianReaderTests : TestKit src.ShouldContain("HistorianSinkStatusReader.ReadAsync("); src.ShouldContain("@implements IAsyncDisposable"); src.ShouldContain("HistorianSinkAvailability.NotOnThisRole"); - src.ShouldNotContain("catch\n {"); + System.Text.RegularExpressions.Regex.IsMatch(src, @"catch\s*\{").ShouldBeFalse("a bare catch {} swallows every failure"); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Drivers/TestConnectBudgetTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Drivers/TestConnectBudgetTests.cs index e0f6a271..dfdd2ea4 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Drivers/TestConnectBudgetTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Drivers/TestConnectBudgetTests.cs @@ -34,6 +34,25 @@ public sealed class TestConnectBudgetTests public void No_valid_configured_timeout_falls_back(string? json) => TestConnectBudget.Resolve(json, fallbackSeconds: 10).ShouldBe(10); + [Theory] + [InlineData("GalaxyMxGateway", 30)] + [InlineData("OpcUaClient", 15)] + [InlineData("Modbus", 5)] + [InlineData("Sql", 10)] // no documented default ⇒ the caller's fallback + public void An_absent_key_uses_the_drivers_documented_default(string driverType, int expected) + => TestConnectBudget.Resolve("{}", fallbackSeconds: 10, driverType).ShouldBe(expected); + + [Fact] + public void A_configured_value_beats_the_driver_default() + => TestConnectBudget.Resolve("{\"ProbeTimeoutSeconds\":7}", 10, "GalaxyMxGateway").ShouldBe(7); + + [Theory] + [InlineData("{\"ProbeTimeoutSeconds\":5,\"timeoutMs\":8000}", 9)] // M2: 8 s request + 1 s, never cut short + [InlineData("{\"ProbeTimeoutSeconds\":5,\"timeoutMs\":2000}", 5)] // positive control: a short request timeout changes nothing + [InlineData("{\"timeoutMs\":120000}", 60)] // still clamped + public void The_budget_never_undercuts_the_configs_request_timeout(string json, int expected) + => TestConnectBudget.Resolve(json, 10, "Modbus").ShouldBe(expected); + [Fact] public void The_fallback_is_clamped_too() { @@ -46,7 +65,7 @@ public sealed class TestConnectBudgetTests { var src = AdminUiSource.Read("Components", "Shared", "Drivers", "DriverTestConnectButton.razor"); - src.ShouldContain("TestConnectBudget.Resolve(json, TimeoutSeconds)"); + src.ShouldContain("TestConnectBudget.Resolve(json, TimeoutSeconds, DriverType)"); src.ShouldContain("@implements IAsyncDisposable"); src.ShouldContain("System.Threading.Timer"); src.ShouldNotContain("System.Timers.Timer"); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hubs/StoreEvictionAndIsolationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hubs/StoreEvictionAndIsolationTests.cs index 411eeb20..f45914f1 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hubs/StoreEvictionAndIsolationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hubs/StoreEvictionAndIsolationTests.cs @@ -12,53 +12,92 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Hubs; /// public sealed class StoreEvictionAndIsolationTests { - private static DriverHealthChanged Health(string instance) => - new("MAIN", instance, "Healthy", null, null, 0, new DateTime(2026, 10, 22, 0, 0, 0, DateTimeKind.Utc)); + private static readonly DateTime Now = new(2026, 10, 22, 12, 0, 0, DateTimeKind.Utc); + private static readonly DateTime Stale = Now.AddMinutes(-5); // stopped publishing + private static readonly DateTime Fresh = Now.AddSeconds(-3); // still publishing + private static readonly DateTime Cutoff = Now.AddSeconds(-60); // what /hosts passes - private static DriverResilienceStatusChanged Res(string instance, string host) => - new(instance, host, false, 0, 0, null, - new DateTime(2026, 10, 22, 0, 0, 0, DateTimeKind.Utc), - new DateTime(2026, 10, 22, 0, 0, 0, DateTimeKind.Utc)); + private static DriverHealthChanged Health(string instance, DateTime? publishedUtc = null) => + new("MAIN", instance, "Healthy", null, null, 0, publishedUtc ?? Now); + + private static DriverResilienceStatusChanged Res(string instance, string host, DateTime? publishedUtc = null) => + new(instance, host, false, 0, 0, null, publishedUtc ?? Now, publishedUtc ?? Now); + + private static HashSet Configured(params string[] ids) => new(ids, StringComparer.OrdinalIgnoreCase); [Fact] - public void HealthStore_RetainOnly_evicts_unconfigured_instances_and_keeps_configured_ones() + public void HealthStore_evicts_only_unconfigured_and_stale() { var store = new InMemoryDriverStatusSnapshotStore(); - store.Upsert(Health("drv-live")); - store.Upsert(Health("drv-deleted")); + store.Upsert(Health("drv-deleted-stopped", Stale)); // unconfigured + stale ⇒ evicted + store.Upsert(Health("drv-deleted-running", Fresh)); // unconfigured + fresh ⇒ kept (I1: deleted in /raw, not yet deployed) + store.Upsert(Health("drv-configured-quiet", Stale)); // configured + stale ⇒ kept + store.Upsert(Health("drv-configured-live", Fresh)); // configured + fresh ⇒ kept - var removed = store.RetainOnly(new HashSet(["drv-live"], StringComparer.OrdinalIgnoreCase)); + var removed = store.EvictUnconfiguredStale(Configured("drv-configured-quiet", "drv-configured-live"), Cutoff); removed.ShouldBe(1); - store.TryGet("drv-deleted", out _).ShouldBeFalse(); - // Positive control: the configured instance survives the sweep. - store.TryGet("drv-live", out _).ShouldBeTrue(); - store.GetAll().Select(s => s.DriverInstanceId).ShouldBe(["drv-live"]); + store.TryGet("drv-deleted-stopped", out _).ShouldBeFalse(); + // Positive controls: a still-publishing driver survives even though the draft no longer has it, + // and a configured driver survives whatever its age. + store.TryGet("drv-deleted-running", out _).ShouldBeTrue(); + store.TryGet("drv-configured-quiet", out _).ShouldBeTrue(); + store.TryGet("drv-configured-live", out _).ShouldBeTrue(); } [Fact] - public void HealthStore_RetainOnly_matches_ids_case_insensitively() + public void HealthStore_a_running_undeployed_delete_goes_once_it_stops_publishing() { var store = new InMemoryDriverStatusSnapshotStore(); - store.Upsert(Health("DRV-Live")); + store.Upsert(Health("drv-x", Fresh)); + store.EvictUnconfiguredStale(Configured(), Cutoff).ShouldBe(0); - store.RetainOnly(new HashSet(["drv-live"], StringComparer.OrdinalIgnoreCase)).ShouldBe(0); + // The deploy stops it: its last snapshot ages past the cutoff, and the next sweep evicts it. + store.Upsert(Health("drv-x", Stale)); + store.EvictUnconfiguredStale(Configured(), Cutoff).ShouldBe(1); + store.TryGet("drv-x", out _).ShouldBeFalse(); + } + + [Fact] + public void HealthStore_matches_ids_case_insensitively() + { + var store = new InMemoryDriverStatusSnapshotStore(); + store.Upsert(Health("DRV-Live", Stale)); + + store.EvictUnconfiguredStale(Configured("drv-live"), Cutoff).ShouldBe(0); store.TryGet("DRV-Live", out _).ShouldBeTrue(); } [Fact] - public void ResilienceStore_RetainOnly_evicts_every_host_of_an_unconfigured_instance() + public void ResilienceStore_evicts_only_unconfigured_and_stale_hosts() { var store = new InMemoryDriverResilienceStatusStore(); - store.Upsert(Res("drv-live", "h1")); - store.Upsert(Res("drv-gone", "h1")); - store.Upsert(Res("drv-gone", "h2")); + store.Upsert(Res("drv-live", "h1", Stale)); + store.Upsert(Res("drv-gone", "h1", Stale)); + store.Upsert(Res("drv-gone", "h2", Stale)); + store.Upsert(Res("drv-running", "h1", Fresh)); - var removed = store.RetainOnly(new HashSet(["drv-live"], StringComparer.OrdinalIgnoreCase)); + var removed = store.EvictUnconfiguredStale(Configured("drv-live"), Cutoff); removed.ShouldBe(2); store.GetForInstance("drv-gone").ShouldBeEmpty(); store.GetForInstance("drv-live").Count.ShouldBe(1); + store.GetForInstance("drv-running").Count.ShouldBe(1); + } + + [Fact] + public void Failure_log_throttle_warns_at_most_once_per_interval() + { + long now = 1_000; + var throttle = new FailureLogThrottle(() => now); + + throttle.NextLevel().ShouldBe(Microsoft.Extensions.Logging.LogLevel.Warning); + throttle.NextLevel().ShouldBe(Microsoft.Extensions.Logging.LogLevel.Debug); + now += (long)FailureLogThrottle.Interval.TotalMilliseconds - 1; + throttle.NextLevel().ShouldBe(Microsoft.Extensions.Logging.LogLevel.Debug); + // Positive control: once the interval has passed, the next failure is visible again. + now += 1; + throttle.NextLevel().ShouldBe(Microsoft.Extensions.Logging.LogLevel.Warning); } [Fact] @@ -133,7 +172,7 @@ public sealed class HostsEvictionWiringTests var firstCatch = src.IndexOf("catch (Exception ex) when (ex is OperationCanceledException", load, StringComparison.Ordinal); firstCatch.ShouldBeGreaterThan(load); - src.IndexOf("DriverStore.RetainOnly(", load, StringComparison.Ordinal).ShouldBeInRange(load, firstCatch); - src.IndexOf("ResilienceStore.RetainOnly(", load, StringComparison.Ordinal).ShouldBeInRange(load, firstCatch); + src.IndexOf("DriverStore.EvictUnconfiguredStale(", load, StringComparison.Ordinal).ShouldBeInRange(load, firstCatch); + src.IndexOf("ResilienceStore.EvictUnconfiguredStale(", load, StringComparison.Ordinal).ShouldBeInRange(load, firstCatch); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Pickers/BrowseOpenGuardTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Pickers/BrowseOpenGuardTests.cs index f744f815..94f5d2f0 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Pickers/BrowseOpenGuardTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Pickers/BrowseOpenGuardTests.cs @@ -101,7 +101,7 @@ public sealed class BrowseOpenGuardTests : IDisposable } var modal = AdminUiSource.Read("Components", "Shared", "Raw", "RawBrowseModal.razor"); - modal.ShouldContain("BrowseOpenGuard.CloseIfOrphaned(result, _disposed || !Visible, BrowserService)"); + modal.ShouldContain("BrowseOpenGuard.CloseIfOrphaned(result, _disposed || IsStale(generation), BrowserService)"); } private OpcUaClientAddressPickerBody NewPicker(IBrowserSessionService svc) diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawBrowseRebirthRefreshTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawBrowseRebirthRefreshTests.cs index 4b38fbbe..a470be6d 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawBrowseRebirthRefreshTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawBrowseRebirthRefreshTests.cs @@ -64,6 +64,41 @@ public sealed class RawBrowseRebirthRefreshTests : IDisposable svc.Scopes.ShouldBe(["Grp/EdgeA"]); } + [Fact] + public async Task An_open_superseded_by_a_close_and_reopen_closes_its_own_session() + { + // Review r0 M5: Visible is true again after a reopen, so only the generation identifies the stale open. + var gate = new TaskCompletionSource(); + var svc = new RebirthBrowserService { Result = Task.FromResult(0), Open = () => gate.Task }; + var modal = NewOpenModal(svc); + Set(modal, "_token", Guid.Empty); + + var open = (Task)typeof(RawBrowseModal).GetMethod("OpenBrowseAsync", Private)!.Invoke(modal, null)!; + Set(modal, "_openGeneration", Get(modal, "_openGeneration") + 1); // closed + reopened meanwhile + var stale = Guid.NewGuid(); + gate.SetResult(new BrowseOpenResult(true, null, stale)); + await open; + + await svc.Closed.Task.WaitAsync(TimeSpan.FromSeconds(10)); + svc.ClosedTokens.ShouldBe([stale]); + Get(modal, "_token").ShouldBe(Guid.Empty); + } + + [Fact] + public async Task A_current_open_adopts_its_session() + { + // Positive control for the test above. + var token = Guid.NewGuid(); + var svc = new RebirthBrowserService { Result = Task.FromResult(0), Open = () => Task.FromResult(new BrowseOpenResult(true, null, token)) }; + var modal = NewOpenModal(svc); + Set(modal, "_token", Guid.Empty); + + await (Task)typeof(RawBrowseModal).GetMethod("OpenBrowseAsync", Private)!.Invoke(modal, null)!; + + Get(modal, "_token").ShouldBe(token); + svc.ClosedTokens.ShouldBeEmpty(); + } + private RawBrowseModal NewOpenModal(IBrowserSessionService svc) { var modal = new RawBrowseModal(); @@ -98,15 +133,23 @@ public sealed class RawBrowseRebirthRefreshTests : IDisposable private sealed class RebirthBrowserService : IBrowserSessionService { public required Task Result { get; init; } + public Func> Open { get; init; } = () => throw new NotImplementedException(); public List Scopes { get; } = new(); + public List ClosedTokens { get; } = new(); + public TaskCompletionSource Closed { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); public bool CanBrowse(string driverType, string configJson) => true; - public Task OpenAsync(string driverType, string configJson, CancellationToken ct) => throw new NotImplementedException(); + public Task OpenAsync(string driverType, string configJson, CancellationToken ct) => Open(); public Task> RootAsync(Guid token, CancellationToken ct) => throw new NotImplementedException(); public Task> ExpandAsync(Guid token, string nodeId, CancellationToken ct) => throw new NotImplementedException(); public Task> AttributesAsync(Guid token, string nodeId, CancellationToken ct) => throw new NotImplementedException(); public bool CanRequestRebirth(Guid token) => true; - public Task CloseAsync(Guid token) => Task.CompletedTask; + public Task CloseAsync(Guid token) + { + lock (ClosedTokens) ClosedTokens.Add(token); + Closed.TrySetResult(); + return Task.CompletedTask; + } public Task RequestRebirthAsync(Guid token, string scope, CancellationToken ct) { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawCsvRenderingTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawCsvRenderingTests.cs index 7635ad14..36572353 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawCsvRenderingTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawCsvRenderingTests.cs @@ -61,6 +61,22 @@ public sealed class RawCsvRenderingTests all.ShouldBe("a\nb\n"); } + [Fact] + public async Task Preview_build_maps_only_the_first_rows_but_counts_all() + { + // Review r0 M8: the modal maps only its preview's rows; the full count still comes back. + var dbName = Uns.RawTreeTestDb.SeedFresh(); + var reader = new RawTagCsvExportReader(Uns.RawTreeTestDb.Factory(dbName)); + + var full = await RawTagCsvExport.BuildAsync(reader, Uns.RawTreeTestDb.FolderedDeviceId, null, "Modbus"); + var preview = await RawTagCsvExport.BuildAsync(reader, Uns.RawTreeTestDb.FolderedDeviceId, null, "Modbus", maxRows: 1); + + full!.RowCount.ShouldBe(2); + Commons.Csv.CsvParser.Parse(full.Csv).Count.ShouldBe(3); // header + 2 (positive control) + preview!.RowCount.ShouldBe(2); + Commons.Csv.CsvParser.Parse(preview.Csv).Count.ShouldBe(2); // header + 1 + } + [Fact] public void Download_url_escapes_its_query_values() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptList/ScriptListQueriesTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptList/ScriptListQueriesTests.cs index d97efb12..577ba579 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptList/ScriptListQueriesTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptList/ScriptListQueriesTests.cs @@ -56,4 +56,18 @@ public sealed class ScriptListQueriesTests src.ShouldContain("ScriptListQueries.LoadRowsAsync("); src.ShouldNotContain("List