Skip to content

Commit

Permalink
Fix eviction not happening as part of RunSchedule
Browse files Browse the repository at this point in the history
  • Loading branch information
tznind committed Oct 31, 2024
1 parent 43aef42 commit f84a58f
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 ();

Expand Down Expand Up @@ -68,6 +68,10 @@ public AnsiRequestScheduler (IAnsiResponseParser parser, Func<DateTime>? now = n
/// <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))
{
Expand All @@ -91,7 +95,10 @@ public bool SendOrSchedule (AnsiEscapeSequenceRequest request)
}
}

_queuedRequests.Add (Tuple.Create (request, Now ()));
if (addToQueue)
{
_queuedRequests.Add (Tuple.Create (request, Now ()));
}

return false;
}
Expand Down Expand Up @@ -137,14 +144,17 @@ public bool RunSchedule (bool force = false)
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;
Expand Down
47 changes: 47 additions & 0 deletions UnitTests/ConsoleDrivers/AnsiRequestSchedulerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,53 @@ public void RunSchedule_ThrottleExceeded_QueueRequest ()

_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.
Expand Down

0 comments on commit f84a58f

Please sign in to comment.