diff --git a/Navigation.Test/NavigationStackTests.cs b/Navigation.Test/NavigationStackTests.cs index 8dd3a65..0cc3cc6 100644 --- a/Navigation.Test/NavigationStackTests.cs +++ b/Navigation.Test/NavigationStackTests.cs @@ -479,6 +479,66 @@ public void Redo_WithAHandlerThatRedirects_KeepsTheRestOfTheRedoHistory() Assert.IsFalse(_undoRedoProvider.CanRedo); } + [TestMethod] + public void Undo_WhenAHandlerThrows_LeavesTheStackAndTheHistoryInAgreement() + { + // Arrange: a handler that throws once, on the first navigation after the history is built + NavigateToABC(); + bool throwOnce = true; + _navigation!.NavigationChanged += (_, _) => + { + if (throwOnce) + { + throwOnce = false; + throw new InvalidOperationException("handler failed"); + } + }; + + // Act + Assert.ThrowsExactly(() => _undoRedoProvider!.Undo()); + + // Assert: the undo did not happen, so the stack is still at C with C's navigation still to undo + AssertStack("A,B,C", "C"); + Assert.HasCount(3, _undoRedoProvider!.GetUndoStack()); + Assert.IsFalse(_undoRedoProvider.CanRedo); + + // And the next undo goes back to B, as the first one should have + Assert.IsTrue(_undoRedoProvider.Undo()); + AssertStack("A,B", "B"); + Assert.IsTrue(_undoRedoProvider.CanRedo); + } + + [TestMethod] + public void Redo_WhenAHandlerThrows_LeavesTheStackAndTheHistoryInAgreement() + { + // Arrange: undo to B, then a handler that throws once on the redo + NavigateToABC(); + Assert.IsTrue(_undoRedoProvider!.Undo()); + AssertStack("A,B", "B"); + bool throwOnce = true; + _navigation!.NavigationChanged += (_, _) => + { + if (throwOnce) + { + throwOnce = false; + throw new InvalidOperationException("handler failed"); + } + }; + + // Act + Assert.ThrowsExactly(() => _undoRedoProvider.Redo()); + + // Assert: the redo did not happen, so the stack is still at B with C still to redo + AssertStack("A,B", "B"); + Assert.HasCount(2, _undoRedoProvider.GetUndoStack()); + Assert.IsTrue(_undoRedoProvider.CanRedo); + + // And the next redo goes forward to C, as the first one should have + Assert.IsTrue(_undoRedoProvider.Redo()); + AssertStack("A,B,C", "C"); + Assert.IsFalse(_undoRedoProvider.CanRedo); + } + [TestMethod] public void NavigateTo_FromAHandlerOutsideUndo_IsStillRecorded() { diff --git a/Navigation/Services/NavigationStack.cs b/Navigation/Services/NavigationStack.cs index 9725090..8a77b87 100644 --- a/Navigation/Services/NavigationStack.cs +++ b/Navigation/Services/NavigationStack.cs @@ -314,10 +314,17 @@ private static bool IsValidState(List items, int currentIndex) => /// /// The items to restore /// The current index to restore + /// + /// If a handler throws, the stack goes back to the state it had before the + /// restore and the exception propagates. The undo/redo provider treats an action that throws as one that + /// changed nothing, so leaving the restored state in place would put the stack and its history out of step. + /// internal void RestoreState(IEnumerable items, int currentIndex) { T? previousItem; T? currentItem; + List beforeItems; + int beforeIndex; lock (_sync) { List restored = [.. items]; @@ -332,6 +339,8 @@ internal void RestoreState(IEnumerable items, int currentIndex) } previousItem = CurrentUnlocked; + beforeItems = [.. _items]; + beforeIndex = _currentIndex; ReplaceItems(restored, currentIndex); currentItem = CurrentUnlocked; } @@ -348,6 +357,15 @@ internal void RestoreState(IEnumerable items, int currentIndex) { OnNavigationChanged(NavigationType.NavigateTo, previousItem, currentItem); } + catch + { + lock (_sync) + { + ReplaceItems(beforeItems, beforeIndex); + } + + throw; + } finally { lock (_sync)