diff --git a/UndoRedo.Test/UndoRedoStackTests.cs b/UndoRedo.Test/UndoRedoStackTests.cs index ce23036..e70b1bc 100644 --- a/UndoRedo.Test/UndoRedoStackTests.cs +++ b/UndoRedo.Test/UndoRedoStackTests.cs @@ -538,6 +538,66 @@ public async Task NavigationProvider_CancellationToken_HandlesCorrectly() Assert.IsTrue(navigationProvider.WasCancelled, "Navigation should have been cancelled due to timeout"); } + [TestMethod] + public async Task UndoAsync_NavigationProviderThrows_ReturnsTrueWithUndoApplied() + { + // Arrange + UndoRedoService stack = CreateService(); + stack.SetNavigationProvider(new ThrowingNavigationProvider()); + int value = 0; + stack.Execute(new DelegateCommand("Increment", () => value++, () => value--, navigationContext: "editor")); + + // Act + bool result = await stack.UndoAsync().ConfigureAwait(false); + + // Assert + Assert.IsTrue(result, "UndoAsync should report the undo, which was applied before navigation failed"); + Assert.AreEqual(0, value); + Assert.AreEqual(-1, stack.CurrentPosition); + } + + [TestMethod] + public async Task RedoAsync_NavigationProviderThrows_ReturnsTrueWithRedoApplied() + { + // Arrange + UndoRedoService stack = CreateService(); + stack.SetNavigationProvider(new ThrowingNavigationProvider()); + int value = 0; + stack.Execute(new DelegateCommand("Increment", () => value++, () => value--, navigationContext: "editor")); + await stack.UndoAsync(navigateToChange: false).ConfigureAwait(false); + + // Act + bool result = await stack.RedoAsync().ConfigureAwait(false); + + // Assert + Assert.IsTrue(result, "RedoAsync should report the redo, which was applied before navigation failed"); + Assert.AreEqual(1, value); + Assert.AreEqual(0, stack.CurrentPosition); + } + + [TestMethod] + public async Task UndoToSaveBoundaryAsync_NavigationProviderThrows_ReturnsTrueAtBoundary() + { + // Arrange + UndoRedoService stack = CreateService(); + stack.SetNavigationProvider(new ThrowingNavigationProvider()); + int value = 0; + stack.Execute(new DelegateCommand("Increment", () => value++, () => value--, navigationContext: "editor")); + stack.MarkAsSaved(); + stack.Execute(new DelegateCommand("Increment", () => value++, () => value--, navigationContext: "editor")); + stack.Execute(new DelegateCommand("Increment", () => value++, () => value--, navigationContext: "editor")); + SaveBoundary boundary = stack.SaveBoundaries[0]; + + // Act + bool result = await stack.UndoToSaveBoundaryAsync(boundary).ConfigureAwait(false); + + // Assert + Assert.IsTrue(result, "UndoToSaveBoundaryAsync should report the undo, which was applied before navigation failed"); + Assert.AreEqual(1, value); + Assert.AreEqual(0, stack.CurrentPosition); + Assert.IsFalse(stack.HasUnsavedChanges, "The stack should be back at the save boundary"); + } + [TestMethod] public void Execute_CommandThrowsException_DoesNotCorruptStack() { @@ -1049,6 +1109,14 @@ public override ICommand MergeWith(ICommand other) } } + private sealed class ThrowingNavigationProvider : INavigationProvider + { + public Task NavigateToAsync(string context, CancellationToken cancellationToken = default) => + throw new InvalidOperationException("editor closed"); + + public bool IsValidContext(string context) => true; + } + private sealed class SlowNavigationProvider : INavigationProvider { public bool WasCancelled { get; private set; } diff --git a/UndoRedo/Services/UndoRedoService.cs b/UndoRedo/Services/UndoRedoService.cs index 99b0f7a..152b1c2 100644 --- a/UndoRedo/Services/UndoRedoService.cs +++ b/UndoRedo/Services/UndoRedoService.cs @@ -165,19 +165,9 @@ public async Task UndoAsync(bool navigateToChange = true, CancellationToke _stackManager.MovePrevious(); CommandUndone?.Invoke(this, new CommandUndoneEventArgs(command, _stackManager.CurrentPosition)); - if (navigateToChange && _options.EnableNavigation && _navigationProvider != null && !string.IsNullOrEmpty(command.NavigationContext)) + if (navigateToChange) { - using CancellationTokenSource cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - cts.CancelAfter(_options.EffectiveNavigationTimeout); - - try - { - await _navigationProvider.NavigateToAsync(command.NavigationContext!, cts.Token).ConfigureAwait(false); - } - catch (OperationCanceledException) - { - // Navigation timeout or cancellation - not critical - } + await NavigateSafelyAsync(command.NavigationContext, cancellationToken).ConfigureAwait(false); } return true; @@ -201,19 +191,9 @@ public async Task RedoAsync(bool navigateToChange = true, CancellationToke _stackManager.MoveNext(); CommandRedone?.Invoke(this, new CommandRedoneEventArgs(command, _stackManager.CurrentPosition)); - if (navigateToChange && _options.EnableNavigation && _navigationProvider != null && !string.IsNullOrEmpty(command.NavigationContext)) + if (navigateToChange) { - using CancellationTokenSource cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - cts.CancelAfter(_options.EffectiveNavigationTimeout); - - try - { - await _navigationProvider.NavigateToAsync(command.NavigationContext!, cts.Token).ConfigureAwait(false); - } - catch (OperationCanceledException) - { - // Navigation timeout or cancellation - not critical - } + await NavigateSafelyAsync(command.NavigationContext, cancellationToken).ConfigureAwait(false); } return true; @@ -266,25 +246,41 @@ public async Task UndoToSaveBoundaryAsync(SaveBoundary saveBoundary, bool CommandUndone?.Invoke(this, new CommandUndoneEventArgs(command, _stackManager.CurrentPosition)); } - if (navigateToLastChange && lastCommand != null && _options.EnableNavigation && _navigationProvider != null && - !string.IsNullOrEmpty(lastCommand.NavigationContext)) + if (navigateToLastChange && lastCommand != null) { - using CancellationTokenSource cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - cts.CancelAfter(_options.EffectiveNavigationTimeout); - - try - { - await _navigationProvider.NavigateToAsync(lastCommand.NavigationContext!, cts.Token).ConfigureAwait(false); - } - catch (OperationCanceledException) - { - // Navigation timeout or cancellation - not critical - } + await NavigateSafelyAsync(lastCommand.NavigationContext, cancellationToken).ConfigureAwait(false); } return true; } + /// + /// Navigates to where a change was made, after the undo or redo has already been applied. + /// Navigation is best effort: any failure is swallowed, because an exception here would tell the + /// caller the undo or redo did not happen when it did, and a retry would then undo a second command. + /// + private async Task NavigateSafelyAsync(string? navigationContext, CancellationToken cancellationToken) + { + if (!_options.EnableNavigation || _navigationProvider == null || string.IsNullOrEmpty(navigationContext)) + { + return; + } + + using CancellationTokenSource cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + cts.CancelAfter(_options.EffectiveNavigationTimeout); + + try + { + await _navigationProvider.NavigateToAsync(navigationContext!, cts.Token).ConfigureAwait(false); + } +#pragma warning disable CA1031 // Do not catch general exception types + catch (Exception) + { + // Navigation timeout, cancellation or provider failure - not critical + } +#pragma warning restore CA1031 // Do not catch general exception types + } + /// public IEnumerable GetChangeVisualizations(int maxItems = 50) {