dohertj2/lmxopcua
1
0
Fork 0
Code Issues 43 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(adminui): lane D-b review r0 fixes (I1, M1, M2, M5-M9, N1-N3, N5, H3)

Browse Source 
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
This commit is contained in:
Joseph Doherty
2026-10-22 18:21:46 -04:00
parent 01f7e3d62e
commit 0f1f8ad8fa
26 changed files with 411 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AlarmsHistorianReaderTests.cs
+16 -1
View File
@@ -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");
}
}
tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Drivers/TestConnectBudgetTests.cs
+20 -1
View File
@@ -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");
tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Hubs/StoreEvictionAndIsolationTests.cs
+63 -24
View File
@@ -12,53 +12,92 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Hubs;
/// </summary>
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<string> 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<string>(["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<string>(["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<string>(["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);
}
}
tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Pickers/BrowseOpenGuardTests.cs
+1 -1
View File
@@ -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)
tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawBrowseRebirthRefreshTests.cs
+45 -2
View File
@@ -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<BrowseOpenResult>();
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<int>(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<Guid>(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<Guid>(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<int> Result { get; init; }
public Func<Task<BrowseOpenResult>> Open { get; init; } = () => throw new NotImplementedException();
public List<string> Scopes { get; } = new();
public List<Guid> ClosedTokens { get; } = new();
public TaskCompletionSource Closed { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
public bool CanBrowse(string driverType, string configJson) => true;
public Task<BrowseOpenResult> OpenAsync(string driverType, string configJson, CancellationToken ct) => throw new NotImplementedException();
public Task<BrowseOpenResult> OpenAsync(string driverType, string configJson, CancellationToken ct) => Open();
public Task<IReadOnlyList<BrowseNode>> RootAsync(Guid token, CancellationToken ct) => throw new NotImplementedException();
public Task<IReadOnlyList<BrowseNode>> ExpandAsync(Guid token, string nodeId, CancellationToken ct) => throw new NotImplementedException();
public Task<IReadOnlyList<AttributeInfo>> 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<int> RequestRebirthAsync(Guid token, string scope, CancellationToken ct)
{
tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Raw/RawCsvRenderingTests.cs
+16
View File
@@ -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()
{
tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptList/ScriptListQueriesTests.cs
+14
View File
@@ -56,4 +56,18 @@ public sealed class ScriptListQueriesTests
src.ShouldContain("ScriptListQueries.LoadRowsAsync(");
src.ShouldNotContain("List<Script>");
}
[Fact]
public void Source_fetch_failure_is_shown_in_the_dialog_not_thrown()
{
// Review r0 M7 source pin: the fetch sits inside a try whose catch sets _sourceError.
var src = AdminUiSource.Read("Components", "Pages", "Scripts.razor");
var fetch = src.IndexOf("ScriptListQueries.LoadSourceAsync(", StringComparison.Ordinal);
var tryAt = src.LastIndexOf("try", fetch, StringComparison.Ordinal);
var catchAt = src.IndexOf("catch (Exception ex)", fetch, StringComparison.Ordinal);
tryAt.ShouldBeGreaterThan(src.IndexOf("private async Task ShowSource", StringComparison.Ordinal));
catchAt.ShouldBeGreaterThan(fetch);
src.IndexOf("_sourceError = ", catchAt, StringComparison.Ordinal).ShouldBeGreaterThan(catchAt);
src.ShouldContain("@_sourceError");
}
}
tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Telemetry/TelemetryDialDeadlineTests.cs
+2 -2
View File
@@ -141,7 +141,7 @@ public sealed class TelemetryDialDeadlineTests : TestKit
public void Upsert(DriverHealthChanged snapshot) { Upserts.Enqueue(snapshot); SnapshotChanged?.Invoke(snapshot); }
public bool TryGet(string driverInstanceId, out DriverHealthChanged snapshot) { snapshot = null!; return false; }
public IReadOnlyCollection<DriverHealthChanged> GetAll() => Upserts.ToArray();
public int RetainOnly(IReadOnlySet<string> configuredInstanceIds) => 0;
public int EvictUnconfiguredStale(IReadOnlySet<string> configuredInstanceIds, DateTime olderThanUtc) => 0;
}
private sealed class NullResilienceStore : IDriverResilienceStatusStore
@@ -150,6 +150,6 @@ public sealed class TelemetryDialDeadlineTests : TestKit
public void Upsert(DriverResilienceStatusChanged snapshot) => SnapshotChanged?.Invoke(snapshot);
public IReadOnlyList<DriverResilienceStatusChanged> GetForInstance(string driverInstanceId) => Array.Empty<DriverResilienceStatusChanged>();
public IReadOnlyCollection<DriverResilienceStatusChanged> GetAll() => Array.Empty<DriverResilienceStatusChanged>();
public int RetainOnly(IReadOnlySet<string> configuredInstanceIds) => 0;
public int EvictUnconfiguredStale(IReadOnlySet<string> configuredInstanceIds, DateTime olderThanUtc) => 0;
}
}
tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Telemetry/TelemetryDialSupervisorTests.cs
+2 -2
View File
@@ -438,7 +438,7 @@ public sealed class TelemetryDialSupervisorTests : TestKit
}
public IReadOnlyCollection<DriverHealthChanged> GetAll() => Upserts.ToArray();
public int RetainOnly(IReadOnlySet<string> configuredInstanceIds) => 0;
public int EvictUnconfiguredStale(IReadOnlySet<string> configuredInstanceIds, DateTime olderThanUtc) => 0;
}
private sealed class RecordingResilienceStore : IDriverResilienceStatusStore
@@ -457,7 +457,7 @@ public sealed class TelemetryDialSupervisorTests : TestKit
Upserts.ToArray();
public IReadOnlyCollection<DriverResilienceStatusChanged> GetAll() => Upserts.ToArray();
public int RetainOnly(IReadOnlySet<string> configuredInstanceIds) => 0;
public int EvictUnconfiguredStale(IReadOnlySet<string> configuredInstanceIds, DateTime olderThanUtc) => 0;
}
private sealed class InMemoryFactory(string name) : IDbContextFactory<OtOpcUaConfigDbContext>