From 9e15d5dcebb5e0ea4286f7dc7fb99a2b8cf88039 Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Fri, 2 Oct 2026 20:22:24 -0400 Subject: [PATCH] fix(desktop): track only the latest database browse request A request is now marked superseded before the previous one is cancelled, so a query that completes synchronously on cancellation returns quietly instead of clearing the page and surfacing a stale cancellation error. NestBrowseSession.IsLoading follows only the latest request, and the dialog's Previous/Next and Loading text use it rather than a count that included superseded requests. Each request disposes its own cancellation source when it ends. --- OpenNest.Data/NestBrowseSession.cs | 29 +++++-- OpenNest.Tests/Data/NestBrowseSessionTests.cs | 83 ++++++++++++++++++- OpenNest/Forms/SavedNestsForm.cs | 16 ++-- 3 files changed, 112 insertions(+), 16 deletions(-) diff --git a/OpenNest.Data/NestBrowseSession.cs b/OpenNest.Data/NestBrowseSession.cs index 7c95056..ec65c03 100644 --- a/OpenNest.Data/NestBrowseSession.cs +++ b/OpenNest.Data/NestBrowseSession.cs @@ -40,6 +40,9 @@ public sealed class NestBrowseSession : IDisposable /// The latest applied page; null before the first result and after a failed request. public NestPage? Page { get; private set; } + /// True while the latest request is outstanding; superseded requests do not count. + public bool IsLoading { get; private set; } + public bool CanGoPrevious => Offset > 0; public bool CanGoNext => Page is { } page && page.Offset + page.Items.Count < page.Total; @@ -106,14 +109,17 @@ public sealed class NestBrowseSession : IDisposable private async Task RunAsync() { ObjectDisposedException.ThrowIf(_disposed, this); - // Superseded sources are cancelled but not disposed: their request may still observe the token. - _pending?.Cancel(); - var cancellation = new CancellationTokenSource(); + // Supersede before cancelling: a request that completes synchronously when cancelled + // must already see itself as superseded. Each source is released by its own request. + var previous = _pending; + using var cancellation = new CancellationTokenSource(); _pending = cancellation; var generation = ++_generation; + IsLoading = true; try { + previous?.Cancel(); var page = await QueryAsync(cancellation.Token); if (generation != _generation) return false; @@ -139,6 +145,13 @@ public sealed class NestBrowseSession : IDisposable Page = null; throw; } + finally + { + if (ReferenceEquals(_pending, cancellation)) + _pending = null; + if (generation == _generation) + IsLoading = false; + } } private Task QueryAsync(CancellationToken cancellationToken) => @@ -155,7 +168,10 @@ public sealed class NestBrowseSession : IDisposable private static string Count(int value) => value.ToString("N0", CultureInfo.CurrentCulture); - /// Cancels any request in flight; the repository is not owned and stays open. + /// + /// Cancels any request in flight (which releases its own source when it ends); + /// the repository is not owned and stays open. + /// public void Dispose() { if (_disposed) @@ -163,8 +179,9 @@ public sealed class NestBrowseSession : IDisposable _disposed = true; _generation++; - _pending?.Cancel(); - _pending?.Dispose(); + IsLoading = false; + var pending = _pending; _pending = null; + pending?.Cancel(); } } diff --git a/OpenNest.Tests/Data/NestBrowseSessionTests.cs b/OpenNest.Tests/Data/NestBrowseSessionTests.cs index ce4ea5f..44a88ca 100644 --- a/OpenNest.Tests/Data/NestBrowseSessionTests.cs +++ b/OpenNest.Tests/Data/NestBrowseSessionTests.cs @@ -145,6 +145,79 @@ public class NestBrowseSessionTests Assert.Equal("Showing 11-20 of 20 nests", session.Summary); } + [Fact] + public async Task SupersededRequest_CompletingSynchronouslyOnCancellation_IsNotReportedAsTheLatestFailure() + { + var repository = new ControlledRepository { CompleteOnCancellation = true }; + using var session = new NestBrowseSession(repository); + var older = session.SetSearchAsync("a"); + var newer = session.SetSearchAsync("ab"); + + Assert.False(await older); + repository.Requests[1].Reply.SetResult(Page("ab result")); + Assert.True(await newer); + + Assert.Equal("ab result", Assert.Single(session.Page!.Items).Name); + } + + [Fact] + public async Task IsLoading_FollowsOnlyTheLatestRequest() + { + var repository = new ControlledRepository(); + using var session = new NestBrowseSession(repository, pageSize: 1); + Assert.False(session.IsLoading); + + // A superseded request finishing first leaves the latest one loading. + var first = session.SetSearchAsync("a"); + var second = session.SetSearchAsync("ab"); + repository.Requests[0].Reply.SetResult(Page("a result")); + Assert.False(await first); + Assert.True(session.IsLoading); + repository.Requests[1].Reply.SetResult(Page("ab result")); + Assert.True(await second); + Assert.False(session.IsLoading); + + // The latest request finishing first ends loading although a superseded one is still out. + var third = session.SetSearchAsync("abc"); + var fourth = session.SetSearchAsync("abcd"); + var record = new NestRecord { Id = Guid.NewGuid(), Name = "abcd result" }; + repository.Requests[3].Reply.SetResult(new NestPage { Items = new[] { record }, Total = 3, Limit = 1 }); + Assert.True(await fourth); + Assert.False(session.IsLoading); + Assert.True(session.CanGoNext); + repository.Requests[2].Reply.SetResult(Page("abc result")); + Assert.False(await third); + Assert.False(session.IsLoading); + + var failing = session.RefreshAsync(); + Assert.True(session.IsLoading); + repository.Requests[4].Reply.SetException(new IOException("offline")); + await Assert.ThrowsAsync(() => failing); + Assert.False(session.IsLoading); + } + + [Fact] + public async Task EachRequestsCancellationSource_IsReleasedWhenThatRequestEnds() + { + var repository = new ControlledRepository(); + var session = new NestBrowseSession(repository); + var older = session.SetSearchAsync("a"); + var newer = session.SetSearchAsync("ab"); + + repository.Requests[1].Reply.SetResult(Page("ab")); + await newer; + Assert.Throws(() => repository.Requests[1].Token.WaitHandle); + repository.Requests[0].Reply.SetResult(Page("a")); + await older; + Assert.Throws(() => repository.Requests[0].Token.WaitHandle); + + var pending = session.RefreshAsync(); + session.Dispose(); + repository.Requests[2].Reply.SetResult(Page("late")); + await pending; + Assert.Throws(() => repository.Requests[2].Token.WaitHandle); + } + [Theory] [InlineData("", "No nests on the server.")] [InlineData("no such text", "No nests match the filter.")] @@ -216,14 +289,22 @@ public class NestBrowseSessionTests } } - /// Leaves every query pending until the test completes it. + /// + /// Leaves every query pending until the test completes it. With + /// , cancellation completes the query synchronously + /// inside the canceller's call, as some HTTP handlers do. + /// private sealed class ControlledRepository : RepositoryBase { + public bool CompleteOnCancellation { get; init; } + public List<(NestQuery Query, TaskCompletionSource Reply, CancellationToken Token)> Requests { get; } = new(); public override Task QueryAsync(NestQuery query, CancellationToken cancellationToken = default) { var reply = new TaskCompletionSource(); + if (CompleteOnCancellation) + cancellationToken.Register(() => reply.TrySetCanceled(cancellationToken)); Requests.Add((query, reply, cancellationToken)); return reply.Task; } diff --git a/OpenNest/Forms/SavedNestsForm.cs b/OpenNest/Forms/SavedNestsForm.cs index accc050..7bb507c 100644 --- a/OpenNest/Forms/SavedNestsForm.cs +++ b/OpenNest/Forms/SavedNestsForm.cs @@ -26,7 +26,6 @@ public sealed class SavedNestsForm : Form private readonly Button previousButton; private readonly Button nextButton; private readonly ToolStripStatusLabel statusLabel; - private int pendingRequests; /// Set to the chosen record's id when the dialog closes with OK. public Guid SelectedId { get; private set; } @@ -168,15 +167,16 @@ public sealed class SavedNestsForm : Form /// /// Sends one browse request and renders its page. A request superseded by a newer one /// renders nothing; a failure clears the rows and shows the error in the status line. + /// Navigation follows only the latest request (). /// private async Task RunAsync(Func> request) { - pendingRequests++; - UpdateNavigation(); statusLabel.Text = LoadingText; try { - if (await request() && !IsDisposed) + var started = request(); + UpdateNavigation(); + if (await started && !IsDisposed) Populate(session.Page); } catch (Exception ex) @@ -189,12 +189,11 @@ public sealed class SavedNestsForm : Form } finally { - pendingRequests--; if (!IsDisposed) { UpdateNavigation(); // A request that was declined or superseded renders nothing of its own. - if (pendingRequests == 0 && statusLabel.Text == LoadingText) + if (!session.IsLoading && statusLabel.Text == LoadingText) statusLabel.Text = session.Summary; } } @@ -202,9 +201,8 @@ public sealed class SavedNestsForm : Form private void UpdateNavigation() { - var idle = pendingRequests == 0; - previousButton.Enabled = idle && session.CanGoPrevious; - nextButton.Enabled = idle && session.CanGoNext; + previousButton.Enabled = !session.IsLoading && session.CanGoPrevious; + nextButton.Enabled = !session.IsLoading && session.CanGoNext; } private void Populate(NestPage page)