Browse Source

Fix eviction not happening as part of RunSchedule

tznind 9 months ago
parent
commit
f84a58f301

+ 17 - 7
Terminal.Gui/ConsoleDrivers/AnsiResponseParser/AnsiRequestScheduler.cs

@@ -19,7 +19,7 @@ internal class AnsiRequestScheduler
     /// </summary>
     internal Func<DateTime> Now { get; set; }
 
-    private readonly List<Tuple<AnsiEscapeSequenceRequest, DateTime>> _queuedRequests = new ();
+    private readonly HashSet<Tuple<AnsiEscapeSequenceRequest, DateTime>> _queuedRequests = new ();
 
     internal IReadOnlyCollection<AnsiEscapeSequenceRequest> QueuedRequests => _queuedRequests.Select (r => r.Item1).ToList ();
 
@@ -68,6 +68,10 @@ internal class AnsiRequestScheduler
     /// <param name="request"></param>
     /// <returns><see langword="true"/> if request was sent immediately. <see langword="false"/> if it was queued.</returns>
     public bool SendOrSchedule (AnsiEscapeSequenceRequest request)
+    {
+        return SendOrSchedule (request, true);
+    }
+    private bool SendOrSchedule (AnsiEscapeSequenceRequest request,bool addToQueue)
     {
         if (CanSend (request, out ReasonCannotSend reason))
         {
@@ -91,7 +95,10 @@ internal class AnsiRequestScheduler
             }
         }
 
-        _queuedRequests.Add (Tuple.Create (request, Now ()));
+        if (addToQueue)
+        {
+            _queuedRequests.Add (Tuple.Create (request, Now ()));
+        }
 
         return false;
     }
@@ -137,14 +144,17 @@ internal class AnsiRequestScheduler
             return false;
         }
 
-        Tuple<AnsiEscapeSequenceRequest, DateTime>? opportunity = _queuedRequests.FirstOrDefault (r => CanSend (r.Item1, out _));
+        // Get oldest request
+        Tuple<AnsiEscapeSequenceRequest, DateTime>? opportunity = _queuedRequests.MinBy (r=>r.Item2);
 
         if (opportunity != null)
         {
-            _queuedRequests.Remove (opportunity);
-            Send (opportunity.Item1);
-
-            return true;
+            // Give it another go
+            if (SendOrSchedule (opportunity.Item1, false))
+            {
+                _queuedRequests.Remove (opportunity);
+                return true;
+            }
         }
 
         return false;

+ 47 - 0
UnitTests/ConsoleDrivers/AnsiRequestSchedulerTests.cs

@@ -144,6 +144,53 @@ public class AnsiRequestSchedulerTests
 
         _parserMock.Verify ();
     }
+
+    [Fact]
+    public void EvictStaleRequests_RemovesStaleRequest_AfterTimeout ()
+    {
+        // Arrange
+        var request1 = new AnsiEscapeSequenceRequest
+        {
+            Request = "\u001b[0c",
+            Terminator = "c",
+            ResponseReceived = r => { }
+        };
+
+        // Send
+        _parserMock.Setup (p => p.IsExpecting ("c")).Returns (false).Verifiable (Times.Once);
+        _parserMock.Setup (p => p.ExpectResponse ("c", It.IsAny<Action<string>> (), false)).Verifiable (Times.Exactly (2));
+
+        Assert.True (_scheduler.SendOrSchedule (request1));
+
+        // Parser already has an ongoing request for "c"
+        _parserMock.Setup (p => p.IsExpecting ("c")).Returns (true).Verifiable (Times.Exactly (2));
+
+        // Cannot send because there is already outstanding request
+        Assert.False(_scheduler.SendOrSchedule (request1));
+        Assert.Single (_scheduler.QueuedRequests);
+
+        // Simulate request going stale
+        SetTime (5001); // Exceeds stale timeout
+
+        // Parser should be told to give up on this one (evicted)
+        _parserMock.Setup (p => p.StopExpecting ("c", false))
+                   .Callback (() =>
+                    {
+                        // When we tell parser to evict - it should now tell us it is no longer expecting
+                        _parserMock.Setup (p => p.IsExpecting ("c")).Returns (false).Verifiable (Times.Once);
+                    }).Verifiable ();
+
+        // When we send again the evicted one should be
+        var evicted = _scheduler.RunSchedule ();
+
+        Assert.True (evicted); // Stale request should be evicted
+        Assert.Empty (_scheduler.QueuedRequests);
+
+        // Assert
+        _parserMock.Verify ();
+    }
+
+
     private void SetTime (int milliseconds)
     {
         // This simulates the passing of time by setting the Now function to return a specific time.