Report the abort on the book the user answered for

With several books in flight, the book whose dialog was answered showed
"Cancelled" while some unrelated book showed "Error, Abort". Tearing the queue
down is claimed by whichever book finishes first, and the books genuinely are
interchangeable for that. The status left on a row is not: it is read afterwards
by someone who remembers which book they were asked about.

The two jobs are now separate. ShowRetryDialogAsync records the answering book as
BadBookSessionContext.AbortOriginator, written before Override so that a book
reading Override and racing ahead to the queue loop cannot find the originator
still unset and conclude there was none. The loop gives the abort to that book and
Cancelled to every book that inherited the answer. ClaimAbort is unchanged and
still decides the teardown.

With no dialog in play - Bad Book set to Abort in settings - nobody answered
anything, there is no originator, and the book that claimed the teardown keeps the
abort as before.
This commit is contained in:
Allamagoosa committed 2026-08-24 10:02:46 -07:00
1 parent 5e1ac3f9cc
commit 76e036fb42
4 files changed
+82 -6

No files matched your search

@@ -14,5 +14,22 @@ public class BadBookSessionContext
/// </summary>
internal SemaphoreSlim DialogGate { get; } = new(1, 1);
public void Reset() => Override = null;
/// <summary>
/// The book the user actually answered "Abort" for, when one was asked. Null when the abort came
/// from the Bad Book setting rather than a dialog, and null when nothing has aborted.
/// </summary>
/// <remarks>
/// Recorded because tearing the queue down and reporting the abort are different jobs. The
/// teardown is claimed by whichever book finishes first and the books are interchangeable for
/// it; the status left on a row is read afterwards by a person who remembers which book they
/// were asked about, and "Cancelled" on that row with "Error, Abort" on some other book's is
/// the wrong way round.
/// </remarks>
public ProcessBookViewModel? AbortOriginator { get; set; }
public void Reset()
{
Override = null;
AbortOriginator = null;
}
}
@@ -533,6 +533,14 @@ public class ProcessBookViewModel : ReactiveObject
{
var result = await BadBookActionDialogBase.Show(skipDialogText, "Skip this book?");
// This is the book the user was looking at when they said stop, so this is the book whose
// row should say so afterwards. Every other book in flight inherits the answer through
// Override below and was cancelled by it, which is a different thing and should read as
// one. Written before Override, not after: a book reading Override and racing ahead to
// the queue loop must not find the originator still unset and conclude there wasn't one.
if (result.Action is DialogResult.Abort && _badBookSession is not null)
_badBookSession.AbortOriginator = this;
// Abort is a statement about the run, not about this one book, so it becomes the session
// answer whether or not "apply to all" was checked. Without this a user who aborts with
// several books in flight is asked the same question again by every one of them, and the
@@ -29,6 +29,13 @@ public class ProcessQueueViewModel : ReactiveObject
public ObservableCollection<LogEntry> LogEntries { get; } = new();
public TrackedQueue<ProcessBookViewModel> Queue { get; } = new();
private readonly BadBookSessionContext _badBookSession = new();
/// <summary>
/// Internal rather than private so a test can stand in for a user having answered the bad book
/// dialog, which the dispatch tests otherwise never reach - their books finish on command instead
/// of failing their way into it.
/// </summary>
internal BadBookSessionContext BadBookSession => _badBookSession;
public Task? QueueRunner { get; private set; }
public bool Running => !QueueRunner?.IsCompleted ?? false;
@@ -756,10 +763,21 @@ public class ProcessQueueViewModel : ReactiveObject
{
if (tearsDownTheQueue)
await CancelAllAsync(book);
else
// Tearing the queue down and reporting the abort are separate jobs, and only the
// first one is indifferent to which book does it. The status left on a row is read
// afterwards by someone who remembers which book they were asked about, so the
// abort belongs to the book they answered for - not to whichever book happened to
// reach the teardown first, which put "Cancelled" on the row they aborted and
// "Error, Abort" on an unrelated one. With no dialog in play (Bad Book set to
// Abort in settings) nobody answered anything, and the book that claimed the
// teardown keeps the abort as before.
var originator = _badBookSession.AbortOriginator;
var reportsTheAbort = originator is null ? tearsDownTheQueue : ReferenceEquals(originator, book);
if (!reportsTheAbort)
{
// Inherited the abort rather than triggering the teardown. It was cancelled by
// the book that did, and that is what it should report.
// Cancelled by the abort rather than the cause of it.
book.Result = ProcessBookResult.Cancelled;
book.Status = ProcessBookStatus.Cancelled;
}
@@ -264,8 +264,11 @@ public class ProcessQueueDispatchTests
queue.AddToQueue([a, b, c]);
await books.WaitForStarted(3);
// All three inherit the abort, which is what a session-wide Abort answer produces. Only the
// first through gets to tear the queue down; the others were cancelled by it.
// No dialog was answered here - this is Bad Book set to Abort in settings, where every book
// that fails aborts on its own account. Nobody is the one the user was asked about, so the
// first book through tears the queue down and keeps the abort; the others were cancelled by it.
Assert.IsNull(queue.BadBookSession.AbortOriginator);
books.Finish("A", ProcessBookResult.FailedAbort);
books.Finish("B", ProcessBookResult.FailedAbort);
books.Finish("C", ProcessBookResult.FailedAbort);
@@ -278,6 +281,36 @@ public class ProcessQueueDispatchTests
Assert.AreEqual(2, cancelled, "Books that inherited the abort should report as cancelled.");
}
[TestMethod]
public async Task the_book_the_user_aborted_reports_the_abort_whichever_book_tears_the_queue_down()
{
var (queue, books) = NewQueue(atOnce: 3);
var a = Book("A");
var b = Book("B");
var c = Book("C");
queue.AddToQueue([a, b, c]);
await books.WaitForStarted(3);
// C is the book the user was looking at when they answered Abort. A and B inherit that answer
// through the session override, which is what puts all three here reporting FailedAbort.
queue.BadBookSession.AbortOriginator = c;
// A finishes first and so claims the teardown. Nothing below depends on it winning - that race
// is what made the old status arbitrary - but this is the ordering that used to leave the row
// the user actually aborted saying "Cancelled" while A's said "Error, Abort".
books.Finish("A", ProcessBookResult.FailedAbort);
await Task.Delay(50);
books.Finish("B", ProcessBookResult.FailedAbort);
books.Finish("C", ProcessBookResult.FailedAbort);
await RunToCompletion(queue);
Assert.AreEqual(ProcessBookResult.FailedAbort, c.Result, "The book the user aborted did not report the abort.");
Assert.AreEqual(ProcessBookResult.Cancelled, a.Result, "A book that inherited the abort reported it as its own.");
Assert.AreEqual(ProcessBookResult.Cancelled, b.Result, "A book that inherited the abort reported it as its own.");
}
[TestMethod]
public async Task a_book_that_throws_is_logged_and_the_rest_of_the_queue_still_finishes()
{