fix(dashboard): enforce/document the advised-set cap honestly; cover handle-0 and oversize-read paths

2026-08-15 12:31:36 -04:00
parent 7c1ea12331
commit 44ca7c8623
3 changed files with 163 additions and 13 deletions
@@ -46,12 +46,14 @@ public sealed class DashboardLiveDataServiceTests
        await using FakeSessionManager sessionManager = new(worker);
        await using DashboardLiveDataService service = CreateService(sessionManager);

+        // Within one read the last tag counts as most recently read, so filler[0] is
+        // the tail of the recency list.
        string[] filler = CreateTagAddresses(MaxSubscribedTags);
        await service.ReadAsync(filler, CancellationToken.None);

-        // Re-read the tag that is currently least recent, making it most recent: the
-        // tag read before it is now the eviction candidate.
-        await service.ReadAsync([filler[^1]], CancellationToken.None);
+        // Re-read the tail, making it most recent: the tag read before it is now the
+        // eviction candidate.
+        await service.ReadAsync([filler[0]], CancellationToken.None);
        Assert.Equal(MaxSubscribedTags, worker.SubscribedTags.Count);

        DashboardLiveReadResult overflow = await service.ReadAsync(
@@ -59,16 +61,104 @@ public sealed class DashboardLiveDataServiceTests
            CancellationToken.None);

        Assert.Null(overflow.Error);
-        Assert.Equal([worker.HandleFor(filler[^2])], worker.UnsubscribedHandles);
+        Assert.Equal([worker.HandleFor(filler[1])], worker.UnsubscribedHandles);
        Assert.Equal("Overflow.PV", worker.SubscribedTags[^1]);
        Assert.Equal(MaxSubscribedTags + 1, worker.SubscribedTags.Count);

        // The evicted tag is no longer tracked and re-subscribes; the re-read one does not.
-        await service.ReadAsync([filler[^1], filler[^2]], CancellationToken.None);
-        Assert.Equal(filler[^2], worker.SubscribedTags[^1]);
+        await service.ReadAsync([filler[0], filler[1]], CancellationToken.None);
+        Assert.Equal(filler[1], worker.SubscribedTags[^1]);
        Assert.Equal(MaxSubscribedTags + 2, worker.SubscribedTags.Count);
    }

+    /// <summary>
+    /// Verifies the cap is per-read, not absolute: a single read of more distinct tags
+    /// than the cap keeps them all (a read never evicts a tag it is about to return),
+    /// and the next read that subscribes anything squeezes the overshoot back out.
+    /// </summary>
+    [Fact]
+    public async Task ReadAsync_WithMoreDistinctTagsThanCap_KeepsThemAllThenSelfCorrects()
+    {
+        RecordingWorkerClient worker = new();
+        await using FakeSessionManager sessionManager = new(worker);
+        await using DashboardLiveDataService service = CreateService(sessionManager);
+
+        string[] oversize = CreateTagAddresses(300);
+        DashboardLiveReadResult oversizeResult = await service.ReadAsync(oversize, CancellationToken.None);
+
+        Assert.Null(oversizeResult.Error);
+        Assert.Equal(300, oversizeResult.Values.Count);
+        Assert.Equal(300, worker.SubscribedTags.Count);
+        Assert.Empty(worker.UnsubscribedHandles);
+
+        // 300 + 1 - 256 = 45 evicted in one pass, landing the set back on the cap.
+        await service.ReadAsync(["Overflow.PV"], CancellationToken.None);
+        Assert.Equal(oversize[..45].Select(worker.HandleFor), worker.UnsubscribedHandles);
+
+        // Exactly at the cap now: one more new tag evicts exactly one.
+        worker.UnsubscribedHandles.Clear();
+        await service.ReadAsync(["Overflow2.PV"], CancellationToken.None);
+        Assert.Equal([worker.HandleFor(oversize[45])], worker.UnsubscribedHandles);
+    }
+
+    /// <summary>
+    /// Verifies tags read in the same call are never evicted for each other: a read that
+    /// touches nearly the whole advise set evicts only the untouched remainder, ends over
+    /// the cap, and the following read trims it back.
+    /// </summary>
+    [Fact]
+    public async Task ReadAsync_WhenTouchedTagsFillTheCap_EvictsOnlyUntouchedTags()
+    {
+        RecordingWorkerClient worker = new();
+        await using FakeSessionManager sessionManager = new(worker);
+        await using DashboardLiveDataService service = CreateService(sessionManager);
+
+        string[] filler = CreateTagAddresses(MaxSubscribedTags);
+        await service.ReadAsync(filler, CancellationToken.None);
+
+        // 250 already-advised tags + 10 new ones: only the 6 untouched tags are
+        // evictable, so the set ends at 260.
+        string[] fresh = CreateTagAddresses(10, "Fresh");
+        await service.ReadAsync([.. filler[..250], .. fresh], CancellationToken.None);
+
+        Assert.Equal(filler[250..].Select(worker.HandleFor), worker.UnsubscribedHandles);
+
+        // 260 + 1 - 256 = 5 evicted on the next read that subscribes anything.
+        worker.UnsubscribedHandles.Clear();
+        await service.ReadAsync(["Overflow.PV"], CancellationToken.None);
+        Assert.Equal(5, worker.UnsubscribedHandles.Count);
+    }
+
+    /// <summary>
+    /// Verifies a tag the worker failed to advise still occupies a slot but is evicted
+    /// without any unsubscribe command — there is no item handle to unadvise.
+    /// </summary>
+    [Fact]
+    public async Task ReadAsync_WhenAdviseFailed_EvictsTagWithoutUnsubscribing()
+    {
+        RecordingWorkerClient worker = new();
+        worker.FailSubscribeFor.Add("Bad.PV");
+        await using FakeSessionManager sessionManager = new(worker);
+        await using DashboardLiveDataService service = CreateService(sessionManager);
+
+        // Bad.PV is read first, so it is the least recently read of the batch and the
+        // first tag evicted.
+        string[] filler = CreateTagAddresses(MaxSubscribedTags - 1);
+        await service.ReadAsync(["Bad.PV", .. filler], CancellationToken.None);
+
+        DashboardLiveReadResult overflow = await service.ReadAsync(
+            ["Overflow.PV"],
+            CancellationToken.None);
+
+        Assert.Null(overflow.Error);
+        Assert.Empty(worker.UnsubscribedHandles);
+        Assert.Equal(0, worker.UnsubscribeCommandCount);
+
+        // It was dropped from tracking all the same, so reading it again re-advises it.
+        await service.ReadAsync(["Bad.PV"], CancellationToken.None);
+        Assert.Equal("Bad.PV", worker.SubscribedTags[^1]);
+    }
+
    /// <summary>
    /// Verifies a failed unadvise of an evicted tag does not fail the read, and the
    /// evicted tag is dropped from tracking anyway.
@@ -92,8 +182,8 @@ public sealed class DashboardLiveDataServiceTests
        Assert.Equal(1, sessionManager.OpenCount);

        // The evicted tag was dropped from tracking despite the failed unadvise.
-        await service.ReadAsync([filler[^1]], CancellationToken.None);
-        Assert.Equal(filler[^1], worker.SubscribedTags[^1]);
+        await service.ReadAsync([filler[0]], CancellationToken.None);
+        Assert.Equal(filler[0], worker.SubscribedTags[^1]);
    }

    private static DashboardLiveDataService CreateService(ISessionManager sessionManager)
@@ -104,12 +194,12 @@ public sealed class DashboardLiveDataServiceTests
            NullLogger<DashboardLiveDataService>.Instance);
    }

-    private static string[] CreateTagAddresses(int count)
+    private static string[] CreateTagAddresses(int count, string prefix = "Tank")
    {
        string[] addresses = new string[count];
        for (int i = 0; i < count; i++)
        {
-            addresses[i] = $"Tank_{i:D4}.PV";
+            addresses[i] = $"{prefix}_{i:D4}.PV";
        }

        return addresses;
@@ -227,6 +317,12 @@ public sealed class DashboardLiveDataServiceTests
        /// <summary>Gets the item handles the dashboard unsubscribed, in order.</summary>
        public List<int> UnsubscribedHandles { get; } = [];

+        /// <summary>Gets the number of unsubscribe commands the dashboard sent.</summary>
+        public int UnsubscribeCommandCount { get; private set; }
+
+        /// <summary>Gets the tag addresses the worker refuses to advise.</summary>
+        public HashSet<string> FailSubscribeFor { get; } = new(StringComparer.OrdinalIgnoreCase);
+
        /// <summary>Gets or sets a value indicating whether unsubscribe commands throw.</summary>
        public bool FailUnsubscribe { get; set; }

@@ -262,6 +358,7 @@ public sealed class DashboardLiveDataServiceTests
                    reply.SubscribeBulk = Subscribe(mxCommand.SubscribeBulk.TagAddresses);
                    break;
                case MxCommandKind.UnsubscribeBulk:
+                    UnsubscribeCommandCount++;
                    if (FailUnsubscribe)
                    {
                        throw new InvalidOperationException("Simulated worker unsubscribe failure.");
@@ -305,6 +402,19 @@ public sealed class DashboardLiveDataServiceTests
            foreach (string tagAddress in tagAddresses)
            {
                SubscribedTags.Add(tagAddress);
+                if (FailSubscribeFor.Contains(tagAddress))
+                {
+                    subscribeReply.Results.Add(new SubscribeResult
+                    {
+                        ServerHandle = RegisteredServerHandle,
+                        TagAddress = tagAddress,
+                        ItemHandle = 0,
+                        WasSuccessful = false,
+                        ErrorMessage = "Simulated advise failure.",
+                    });
+                    continue;
+                }
+
                if (!_itemHandles.TryGetValue(tagAddress, out int itemHandle))
                {
                    itemHandle = _nextItemHandle++;