From 2b5d439d8f923c331d082d4465648f24c35dcb60 Mon Sep 17 00:00:00 2001 From: Robert McRackan Date: Wed, 16 Sep 2026 14:02:38 -0400 Subject: [PATCH] Address trash recovery and regression coverage review follow-ups --- .../Dialogs/TrashBinDialog.axaml.cs | 18 +---- .../Views/ProductsDisplay.axaml.cs | 14 +--- Source/LibationUiBase/LibraryOperation.cs | 25 +++++++ .../Dialogs/TrashBinDialog.cs | 32 ++++---- .../GridView/ProductsDisplay.cs | 13 +--- .../TrashOperationsTests.cs | 59 ++++++++++++++- .../LibraryOperationTests.cs | 73 +++++++++++++++++++ 7 files changed, 178 insertions(+), 56 deletions(-) create mode 100644 Source/LibationUiBase/LibraryOperation.cs create mode 100644 Source/_Tests/LibationUiBase.Tests/LibraryOperationTests.cs diff --git a/Source/LibationAvalonia/Dialogs/TrashBinDialog.axaml.cs b/Source/LibationAvalonia/Dialogs/TrashBinDialog.axaml.cs index 1bb63509..005d4db6 100644 --- a/Source/LibationAvalonia/Dialogs/TrashBinDialog.axaml.cs +++ b/Source/LibationAvalonia/Dialogs/TrashBinDialog.axaml.cs @@ -129,25 +129,15 @@ public class TrashBinViewModel : ViewModelBase public async Task PermanentlyDeleteCheckedAsync() => await ChangeCheckedBooksAsync(books => books.PermanentlyDeleteBooksAsync(), "Could not permanently delete books."); - private async Task ChangeCheckedBooksAsync(Func> operation, string message) - { - ControlsEnabled = false; - try + private Task ChangeCheckedBooksAsync(Func> operation, string message) + => LibationUiBase.LibraryOperation.RunAsync(async () => { var selection = GetCheckedBooks().ToArray(); var qtyChanges = await operation(selection); if (qtyChanges > 0) await ReloadAsync(); - } - catch (Exception ex) - { - await MessageBox.ShowAdminAlert(null, message, "Trash bin operation failed", ex); - } - finally - { - ControlsEnabled = true; - } - } + }, ex => MessageBox.ShowAdminAlert(null, message, "Trash bin operation failed", ex), + enabled => ControlsEnabled = enabled); private static List GetDeletedLibraryBooks() { diff --git a/Source/LibationAvalonia/Views/ProductsDisplay.axaml.cs b/Source/LibationAvalonia/Views/ProductsDisplay.axaml.cs index ddb3de16..1bbb3d2d 100644 --- a/Source/LibationAvalonia/Views/ProductsDisplay.axaml.cs +++ b/Source/LibationAvalonia/Views/ProductsDisplay.axaml.cs @@ -756,15 +756,7 @@ public partial class ProductsDisplay : UserControl } #endregion - private async Task RunLibraryOperationAsync(Func operation, string message) - { - try - { - await operation(); - } - catch (Exception ex) - { - await MessageBox.ShowAdminAlert(this.GetParentWindow() as Window, message, "Library operation failed", ex); - } - } + private Task RunLibraryOperationAsync(Func operation, string message) + => LibationUiBase.LibraryOperation.RunAsync(operation, + ex => MessageBox.ShowAdminAlert(this.GetParentWindow() as Window, message, "Library operation failed", ex)); } diff --git a/Source/LibationUiBase/LibraryOperation.cs b/Source/LibationUiBase/LibraryOperation.cs new file mode 100644 index 00000000..1d9cad03 --- /dev/null +++ b/Source/LibationUiBase/LibraryOperation.cs @@ -0,0 +1,25 @@ +using System; +using System.Threading.Tasks; + +namespace LibationUiBase; + +/// Await UI operations, report their failures, and always restore disabled controls. +public static class LibraryOperation +{ + public static async Task RunAsync(Func operation, Func reportError, Action? setControlsEnabled = null) + { + try + { + setControlsEnabled?.Invoke(false); + await operation(); + } + catch (Exception ex) + { + await reportError(ex); + } + finally + { + setControlsEnabled?.Invoke(true); + } + } +} diff --git a/Source/LibationWinForms/Dialogs/TrashBinDialog.cs b/Source/LibationWinForms/Dialogs/TrashBinDialog.cs index 523eaa4a..feee0883 100644 --- a/Source/LibationWinForms/Dialogs/TrashBinDialog.cs +++ b/Source/LibationWinForms/Dialogs/TrashBinDialog.cs @@ -78,29 +78,25 @@ public partial class TrashBinDialog : Form } private async void permanentlyDeleteBtn_Click(object sender, EventArgs e) - { - setControlsEnabled(false); - - var qtyChanges = await GetCheckedBooks().PermanentlyDeleteBooksAsync(); - if (qtyChanges > 0) - Reload(); - - setControlsEnabled(true); - } + => await ChangeCheckedBooksAsync(books => books.PermanentlyDeleteBooksAsync(), "Could not permanently delete books."); private async void restoreBtn_Click(object sender, EventArgs e) - { - setControlsEnabled(false); + => await ChangeCheckedBooksAsync(books => books.RestoreBooksAsync(), "Could not restore books."); - var qtyChanges = await GetCheckedBooks().RestoreBooksAsync(); - if (qtyChanges > 0) - Reload(); - - setControlsEnabled(true); - } + private Task ChangeCheckedBooksAsync(Func> operation, string message) + => LibationUiBase.LibraryOperation.RunAsync(async () => + { + var selection = GetCheckedBooks().ToArray(); + if (await operation(selection) > 0) + Reload(); + }, ex => + { + MessageBoxLib.ShowAdminAlert(this, message, "Trash bin operation failed", ex); + return Task.CompletedTask; + }, setControlsEnabled); private void setControlsEnabled(bool enabled) - => Invoke(() => productsGrid1.Enabled = restoreBtn.Enabled = permanentlyDeleteBtn.Enabled = everythingCb.Enabled = enabled); + => Invoke(() => productsGrid1.Enabled = restoreBtn.Enabled = permanentlyDeleteBtn.Enabled = everythingCb.Enabled = audiblePlusCb.Enabled = enabled); private void textBox1_KeyDown(object sender, KeyEventArgs e) { diff --git a/Source/LibationWinForms/GridView/ProductsDisplay.cs b/Source/LibationWinForms/GridView/ProductsDisplay.cs index 6eec016c..075dbb22 100644 --- a/Source/LibationWinForms/GridView/ProductsDisplay.cs +++ b/Source/LibationWinForms/GridView/ProductsDisplay.cs @@ -480,15 +480,10 @@ public partial class ProductsDisplay : UserControl { RemovableCountChanged?.Invoke(sender, productsGrid.GetAllBookEntries().Count(lbe => lbe.Remove is true)); } - private async Task RunLibraryOperationAsync(Func operation, string message) - { - try - { - await operation(); - } - catch (Exception ex) + private Task RunLibraryOperationAsync(Func operation, string message) + => LibationUiBase.LibraryOperation.RunAsync(operation, ex => { MessageBoxLib.ShowAdminAlert(this, message, "Library operation failed", ex); - } - } + return Task.CompletedTask; + }); } diff --git a/Source/_Tests/ApplicationServices.Tests/TrashOperationsTests.cs b/Source/_Tests/ApplicationServices.Tests/TrashOperationsTests.cs index 9618a6bb..ad550f57 100644 --- a/Source/_Tests/ApplicationServices.Tests/TrashOperationsTests.cs +++ b/Source/_Tests/ApplicationServices.Tests/TrashOperationsTests.cs @@ -28,6 +28,18 @@ public class TrashOperationsTests var book = new Book(new AudibleProductId("B0TRASHTEST"), "Trash test", "", "Description", 600, ContentType.Product, [new Contributor("Author")], [new Contributor("Narrator")], "us"); context.LibraryBooks.Add(new LibraryBook(book, DateTime.UtcNow, "original-account")); + foreach (var deleted in new[] { false, true }) + { + var id = deleted ? "B0UNSELECTEDTRASH" : "B0UNSELECTED"; + var untouched = new Book(new AudibleProductId(id), id, "", "Untouched description", 600, + ContentType.Product, [new Contributor(id + " author")], [new Contributor(id + " narrator")], "us"); + context.LibraryBooks.Add(new LibraryBook(untouched, new DateTime(2020, 1, 1), "untouched-account") + { + IsDeleted = deleted, + AbsentFromLastScan = true, + IsAudiblePlus = true + }); + } context.SaveChanges(); } @@ -47,6 +59,24 @@ public class TrashOperationsTests return context.GetLibraryBook_Flat_NoTracking("B0TRASHTEST")!; } + private static void AssertUnselectedUnchanged() + { + using var context = DbContexts.GetContext(); + foreach (var deleted in new[] { false, true }) + { + var id = deleted ? "B0UNSELECTEDTRASH" : "B0UNSELECTED"; + var row = context.LibraryBooks.Single(lb => lb.Book.AudibleProductId == id); + Assert.AreEqual(deleted, row.IsDeleted); + Assert.AreEqual("untouched-account", row.Account); + Assert.AreEqual(new DateTime(2020, 1, 1), row.DateAdded); + Assert.IsTrue(row.AbsentFromLastScan); + Assert.IsTrue(row.IsAudiblePlus); + var book = context.Books.Single(b => b.AudibleProductId == id); + Assert.AreEqual(id, book.Title); + Assert.AreEqual("Untouched description", book.Description); + } + } + [TestMethod] public async Task Remove_and_restore_duplicate_instances_preserve_newer_database_fields() { @@ -55,7 +85,7 @@ public class TrashOperationsTests Assert.AreNotSame(first, second); using (var context = DbContexts.GetContext()) { - var current = context.LibraryBooks.Single(); + var current = context.LibraryBooks.Single(lb => lb.Book.AudibleProductId == "B0TRASHTEST"); current.SetAccount("updated-account"); current.AbsentFromLastScan = true; context.SaveChanges(); @@ -63,16 +93,18 @@ public class TrashOperationsTests LibraryBook[] selection = [first, second]; Assert.IsTrue(await selection.RemoveBooksAsync() > 0); + AssertUnselectedUnchanged(); Assert.IsFalse(first.IsDeleted, "Do not mutate detached UI objects before refreshing the library."); using (var context = DbContexts.GetContext()) { - var current = context.LibraryBooks.Single(); + var current = context.LibraryBooks.Single(lb => lb.Book.AudibleProductId == "B0TRASHTEST"); Assert.IsTrue(current.IsDeleted); Assert.AreEqual("updated-account", current.Account); Assert.IsTrue(current.AbsentFromLastScan); } Assert.IsTrue(await selection.RestoreBooksAsync() > 0); + AssertUnselectedUnchanged(); var restored = Read(); Assert.IsFalse(restored.IsDeleted); Assert.AreEqual("updated-account", restored.Account); @@ -85,8 +117,27 @@ public class TrashOperationsTests LibraryBook[] selection = [Read(), Read()]; Assert.AreNotSame(selection[0], selection[1]); Assert.IsTrue(await selection.PermanentlyDeleteBooksAsync() > 0); + AssertUnselectedUnchanged(); using var context = DbContexts.GetContext(); - Assert.AreEqual(0, context.LibraryBooks.Count()); - Assert.AreEqual(0, context.Books.Count()); + Assert.AreEqual(2, context.LibraryBooks.Count()); + Assert.AreEqual(2, context.Books.Count()); + Assert.IsFalse(context.LibraryBooks.Any(lb => lb.Book.AudibleProductId == "B0TRASHTEST")); + Assert.IsFalse(context.Books.Any(b => b.AudibleProductId == "B0TRASHTEST")); + } + + [TestMethod] + public async Task Repeated_operations_and_missing_rows_are_no_ops() + { + LibraryBook[] selection = [Read(), Read()]; + Assert.AreEqual(0, await selection.RestoreBooksAsync()); + Assert.IsTrue(await selection.RemoveBooksAsync() > 0); + Assert.AreEqual(0, await selection.RemoveBooksAsync()); + Assert.IsTrue(await selection.RestoreBooksAsync() > 0); + Assert.AreEqual(0, await selection.RestoreBooksAsync()); + Assert.IsTrue(await selection.PermanentlyDeleteBooksAsync() > 0); + Assert.AreEqual(0, await selection.PermanentlyDeleteBooksAsync()); + Assert.AreEqual(0, await selection.RemoveBooksAsync()); + Assert.AreEqual(0, await selection.RestoreBooksAsync()); + AssertUnselectedUnchanged(); } } diff --git a/Source/_Tests/LibationUiBase.Tests/LibraryOperationTests.cs b/Source/_Tests/LibationUiBase.Tests/LibraryOperationTests.cs new file mode 100644 index 00000000..33dbe05a --- /dev/null +++ b/Source/_Tests/LibationUiBase.Tests/LibraryOperationTests.cs @@ -0,0 +1,73 @@ +namespace LibationUiBase.Tests; + +[TestClass] +public class LibraryOperationTests +{ + [TestMethod] + public async Task Delayed_failure_is_observed_and_controls_recover_after_alert_completes() + { + var operation = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var alertShown = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var dismissAlert = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var controls = new List(); + var failure = new InvalidOperationException("database failed"); + var pending = LibraryOperation.RunAsync(() => operation.Task, ex => + { + alertShown.SetResult(ex); + return dismissAlert.Task; + }, controls.Add); + + CollectionAssert.AreEqual(new[] { false }, controls); + Assert.IsFalse(pending.IsCompleted); + operation.SetException(failure); + Assert.AreSame(failure, await alertShown.Task.WaitAsync(TimeSpan.FromSeconds(5))); + Assert.IsFalse(pending.IsCompleted, "Keep the operation active until the user dismisses the alert."); + dismissAlert.SetResult(); + await pending.WaitAsync(TimeSpan.FromSeconds(5)); + CollectionAssert.AreEqual(new[] { false, true }, controls); + } + + [TestMethod] + public async Task Status_operation_failure_is_reported_without_faulting_the_returned_task() + { + var failure = new InvalidOperationException("status failed"); + Exception? reported = null; + await LibraryOperation.RunAsync(() => Task.FromException(failure), ex => + { + reported = ex; + return Task.CompletedTask; + }); + Assert.AreSame(failure, reported); + } + + [TestMethod] + public async Task Synchronous_failure_and_failed_alert_still_restore_controls() + { + var controls = new List(); + var alertFailure = new InvalidOperationException("alert failed"); + var result = LibraryOperation.RunAsync(() => throw new Exception("selection failed"), + _ => Task.FromException(alertFailure), controls.Add); + try + { + await result; + Assert.Fail("The alert failure must propagate."); + } + catch (InvalidOperationException ex) + { + Assert.AreSame(alertFailure, ex); + } + CollectionAssert.AreEqual(new[] { false, true }, controls); + } + + [TestMethod] + public async Task Successful_operation_restores_controls_without_alert() + { + var controls = new List(); + await LibraryOperation.RunAsync(() => Task.CompletedTask, _ => + { + Assert.Fail("No alert should be shown."); + return Task.CompletedTask; + }, controls.Add); + CollectionAssert.AreEqual(new[] { false, true }, controls); + } +}