fix(centralui): Topology live tick behind PollGate, refresh no longer unmounts tree (review 2026-10-21 07/P-2)

2026-10-22 08:41:47 -04:00
parent c429fb374b
commit 6644cf917d
2 changed files with 57 additions and 20 deletions
@@ -141,6 +141,7 @@
    private readonly HashSet<int> _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<Area> _allAreas = new();
    private Dictionary<int, bool> _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.
    /// </summary>
-    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<Area>();
+                var instances = new List<Instance>();
+                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();
            }
@@ -199,4 +199,35 @@ public class TopologyLiveUpdatesTimerDisposeRaceTests : BunitContext
        Assert.Null(ex);
        Assert.Null(GetField<Timer?>(cut.Instance, "_liveUpdatesTimer"));
    }
+
+    private static void InvokeTick(object page) =>
+        page.GetType().GetMethod("OnLiveUpdatesTick", BindingFlags.Instance | BindingFlags.NonPublic)!
+            .Invoke(page, new object?[] { null });
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    [Fact]
+    public async Task LiveTick_WhilePreviousRefreshInFlight_IsSkipped_AndDoesNotUnmountTree()
+    {
+        var cut = Render<TopologyPage>();
+        _siteRepo.ClearReceivedCalls();
+        var release = new TaskCompletionSource<IReadOnlyList<Site>>();
+        _siteRepo.GetAllSitesAsync(Arg.Any<CancellationToken>()).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<CancellationToken>());
+        // The in-flight refresh must not have swapped the tree for the spinner.
+        Assert.False(GetField<bool>(cut.Instance, "_loading"));
+
+        release.SetResult(new List<Site>());
+        await Task.Delay(100);
+        cut.Instance.Dispose();
+    }
 }