From 6644cf917d294a6e8cf8675cafaa0aca80b688a1 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 08:41:47 -0400 Subject: [PATCH] fix(centralui): Topology live tick behind PollGate, refresh no longer unmounts tree (review 2026-10-21 07/P-2) --- .../Pages/Deployment/Topology.razor | 46 +++++++++++-------- ...opologyLiveUpdatesTimerDisposeRaceTests.cs | 31 +++++++++++++ 2 files changed, 57 insertions(+), 20 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/Topology.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/Topology.razor index de93e18e..ef8156b8 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/Topology.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/Topology.razor @@ -141,6 +141,7 @@ private readonly HashSet _loadedSites = new(); private bool _branchLoading; private readonly SemaphoreSlim _loadGate = new(1, 1); + private readonly ZB.MOM.WW.ScadaBridge.CentralUI.Services.PollGate _pollGate = new(); private List _allAreas = new(); private Dictionary _stalenessMap = new(); @@ -242,25 +243,23 @@ private void OnLiveUpdatesTick(object? state) { if (_disposed) return; - InvokeAsync(async () => - { - if (_disposed || !_liveUpdates) return; - try + // PollGate (arch-review S3): skip the tick while the previous refresh is still in + // flight, so a slow configuration DB cannot stack queries and renderer continuations. + _ = _pollGate.RunAsync( + () => InvokeAsync(async () => { + if (_disposed || !_liveUpdates) return; // Staleness is deliberately OFF the live loop: it only changes when a // template/instance is edited or redeployed, and recomputing it (a // full re-flatten per deployed instance) every 15s is the page's // dominant cost. The poll refreshes only the cheap deployed state. - await LoadDeployedStateAsync(); + await LoadDeployedStateAsync(isRefresh: true); if (_disposed) return; StateHasChanged(); - } - catch (ObjectDisposedException) - { - // Component disposed between the guard and the dispatch — ignore - // (mirrors DebugView's SafeInvokeAsync / CentralUI-009). - } - }); + }), + // Disposed between the guard and the dispatch (mirrors DebugView's + // SafeInvokeAsync / CentralUI-009); LoadDeployedStateAsync reports its own failures. + _ => { }); } private void OnLiveUpdatesToggled(ChangeEventArgs e) @@ -339,12 +338,15 @@ /// This is the live-update timer path; it does not recompute staleness. Internal so the live-update regression /// test can drive the timer's code path directly. /// - internal async Task LoadDeployedStateAsync() + internal async Task LoadDeployedStateAsync(bool isRefresh = false) { await _loadGate.WaitAsync(); try { - _loading = true; + // Only the initial/explicit load unmounts the tree behind the spinner; the 15s + // live tick (isRefresh) must not, or any render landing mid-tick swaps the whole + // tree for the spinner and back (review 2026-10-21 07/P-2). + if (!isRefresh) _loading = true; _errorMessage = null; try { @@ -355,12 +357,16 @@ _loadedSites.IntersectWith(permittedSiteIds); foreach (var key in _loadedPages.Keys.Where(k => !permittedSiteIds.Contains(k.Site)).ToArray()) _loadedPages.Remove(key); - _allInstances.Clear(); - _allAreas.Clear(); - foreach (var siteId in _loadedSites) - _allAreas.AddRange(await TemplateEngineRepository.GetAreasBySiteIdAsync(siteId)); - foreach (var (key, offset) in _loadedPages) - _allInstances.AddRange(await TemplateEngineRepository.GetTopologyInstancesAsync(key.Site, key.Area, offset, InstancePageSize)); + // Gather into locals and swap, so a render landing mid-fetch never sees + // the cleared lists. + var areas = new List(); + var instances = new List(); + foreach (var siteId in _loadedSites.ToArray()) + areas.AddRange(await TemplateEngineRepository.GetAreasBySiteIdAsync(siteId)); + foreach (var (key, offset) in _loadedPages.ToArray()) + instances.AddRange(await TemplateEngineRepository.GetTopologyInstancesAsync(key.Site, key.Area, offset, InstancePageSize)); + _allAreas = areas; + _allInstances = instances; BuildTree(); } diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/TopologyLiveUpdatesTimerDisposeRaceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/TopologyLiveUpdatesTimerDisposeRaceTests.cs index a98ba5fe..c13bdf1a 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/TopologyLiveUpdatesTimerDisposeRaceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/TopologyLiveUpdatesTimerDisposeRaceTests.cs @@ -199,4 +199,35 @@ public class TopologyLiveUpdatesTimerDisposeRaceTests : BunitContext Assert.Null(ex); Assert.Null(GetField(cut.Instance, "_liveUpdatesTimer")); } + + private static void InvokeTick(object page) => + page.GetType().GetMethod("OnLiveUpdatesTick", BindingFlags.Instance | BindingFlags.NonPublic)! + .Invoke(page, new object?[] { null }); + + /// + /// Review 2026-10-21 07/P-2: a tick arriving while the previous refresh is still in + /// flight is skipped (PollGate), and a refresh never flips the page back to the spinner. + /// + [Fact] + public async Task LiveTick_WhilePreviousRefreshInFlight_IsSkipped_AndDoesNotUnmountTree() + { + var cut = Render(); + _siteRepo.ClearReceivedCalls(); + var release = new TaskCompletionSource>(); + _siteRepo.GetAllSitesAsync(Arg.Any()).Returns(release.Task); + + InvokeTick(cut.Instance); + InvokeTick(cut.Instance); + InvokeTick(cut.Instance); + await Task.Delay(100); + + // Three ticks, one query: the gate dropped the overlapping two. + await _siteRepo.Received(1).GetAllSitesAsync(Arg.Any()); + // The in-flight refresh must not have swapped the tree for the spinner. + Assert.False(GetField(cut.Instance, "_loading")); + + release.SetResult(new List()); + await Task.Delay(100); + cut.Instance.Dispose(); + } }