From b61f06ce7efc0014e7e68cd9408ec2aa4878bec3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 08:27:52 +0000 Subject: [PATCH] Keep a trimmed stack dirty at -1 through save, load, and restore [major] The "initial state is clean" flag on SaveBoundaryManager was not part of the saved state, and RestoreFromState's Clear() reset it to true. A stack whose oldest commands were trimmed, with no save boundary, reported no unsaved changes at -1 after every LoadStateAsync or RestoreFromState, which is #76 again. Breaking changes, per the maintainer's decision on #83: - ISaveBoundaryManager gains InitialStateIsClean and SetInitialStateClean(bool) - IUndoRedoSerializer.SerializeAsync takes an initialStateIsClean parameter - UndoRedoStackState gains an InitialStateIsClean init property, default true JsonUndoRedoSerializer writes the flag and reads a missing one as true, so data saved before this change still loads with its old meaning. Fixes #83 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XEFMRMk4z5RaHqHVp4rTtj --- UndoRedo.Test/SerializationTests.cs | 148 +++++++++++++++++++- UndoRedo/Contracts/ISaveBoundaryManager.cs | 22 +++ UndoRedo/Contracts/IUndoRedoSerializer.cs | 6 + UndoRedo/Models/UndoRedoStackState.cs | 11 ++ UndoRedo/Services/JsonUndoRedoSerializer.cs | 10 +- UndoRedo/Services/SaveBoundaryManager.cs | 18 ++- UndoRedo/Services/UndoRedoService.cs | 9 +- docs/serialization.md | 16 ++- 8 files changed, 222 insertions(+), 18 deletions(-) diff --git a/UndoRedo.Test/SerializationTests.cs b/UndoRedo.Test/SerializationTests.cs index 5ddee22..261061d 100644 --- a/UndoRedo.Test/SerializationTests.cs +++ b/UndoRedo.Test/SerializationTests.cs @@ -18,7 +18,7 @@ public async Task JsonSerializer_SerializeEmpty_ReturnsValidData() JsonUndoRedoSerializer serializer = new(); // Act - byte[] data = await serializer.SerializeAsync([], 0, []).ConfigureAwait(false); + byte[] data = await serializer.SerializeAsync([], 0, [], true).ConfigureAwait(false); // Assert Assert.IsNotNull(data); @@ -50,7 +50,7 @@ public async Task JsonSerializer_SerializeDeserialize_PreservesBasicData() ]; // Act - byte[] data = await serializer.SerializeAsync(commands, 1, boundaries).ConfigureAwait(false); + byte[] data = await serializer.SerializeAsync(commands, 1, boundaries, false).ConfigureAwait(false); UndoRedoStackState state = await serializer.DeserializeAsync(data).ConfigureAwait(false); // Assert @@ -301,7 +301,7 @@ public async Task SerializableCommand_SerializesCorrectly() ]; // Act - byte[] data = await serializer.SerializeAsync(commands, 1, []).ConfigureAwait(false); + byte[] data = await serializer.SerializeAsync(commands, 1, [], true).ConfigureAwait(false); UndoRedoStackState state = await serializer.DeserializeAsync(data).ConfigureAwait(false); // Assert @@ -381,7 +381,7 @@ public async Task JsonSerializer_DeserializeCommandHasNoParameterlessConstructor // Arrange JsonUndoRedoSerializer serializer = new(); ConstructorOnlySerializableCommand command = new("saved"); - byte[] data = await serializer.SerializeAsync([command], 0, []).ConfigureAwait(false); + byte[] data = await serializer.SerializeAsync([command], 0, [], true).ConfigureAwait(false); // Act & Assert: the failure is reported as part of the deserialization contract, not as the // raw reflection error @@ -528,7 +528,8 @@ public async Task UndoRedoService_LoadStateAsyncInvalidBoundaryPosition_ReturnsF byte[] data = await serializer.SerializeAsync( [new TestSerializableCommand("X")], 0, - [new SaveBoundary(-7), new SaveBoundary(42)]).ConfigureAwait(false); + [new SaveBoundary(-7), new SaveBoundary(42)], + false).ConfigureAwait(false); UndoRedoService stack = CreateService(); stack.SetSerializer(new JsonUndoRedoSerializer()); @@ -568,6 +569,138 @@ public async Task UndoRedoService_SaveLoadState_ReconstructsCommandWithEmptyData Assert.AreEqual(-1, newStack.CurrentPosition); } + /// + /// Builds the #83 repro: with room for two commands, three are executed, so the first is trimmed, + /// then both remaining are undone. Position -1 now holds the first command's never-saved result. + /// + private static UndoRedoService CreateDirtyAtStartAfterTrimming() + { + UndoRedoService stack = new(new StackManager(), new SaveBoundaryManager(), new CommandMerger(), UndoRedoOptions.Create(maxStackSize: 2)); + stack.SetSerializer(new JsonUndoRedoSerializer()); + stack.Execute(new TestSerializableCommand("1")); + stack.Execute(new TestSerializableCommand("2")); + stack.Execute(new TestSerializableCommand("3")); + stack.Undo(); + stack.Undo(); + + Assert.AreEqual(-1, stack.CurrentPosition); + Assert.IsEmpty(stack.SaveBoundaries); + Assert.IsTrue(stack.HasUnsavedChanges, "The trimmed command's result at -1 was never saved"); + return stack; + } + + [TestMethod] + public void UndoRedoService_RestoreFromState_KeepsUnsavedChangesAtStartAfterTrimming() + { + // Arrange + UndoRedoService stack = CreateDirtyAtStartAfterTrimming(); + UndoRedoStackState state = stack.GetCurrentState(); + + // Act + UndoRedoService restored = CreateService(); + bool success = restored.RestoreFromState(state); + + // Assert + Assert.IsTrue(success); + Assert.IsFalse(state.InitialStateIsClean); + Assert.AreEqual(-1, restored.CurrentPosition); + Assert.IsTrue(restored.HasUnsavedChanges, "Restoring must not make the dirty state at -1 look saved"); + } + + [TestMethod] + public void UndoRedoService_RestoreFromOwnState_KeepsUnsavedChangesAtStartAfterTrimming() + { + // Arrange + UndoRedoService stack = CreateDirtyAtStartAfterTrimming(); + + // Act + bool success = stack.RestoreFromState(stack.GetCurrentState()); + + // Assert + Assert.IsTrue(success); + Assert.IsTrue(stack.HasUnsavedChanges, "Restoring its own state must not make the dirty state at -1 look saved"); + } + + [TestMethod] + public async Task UndoRedoService_SaveLoadState_KeepsUnsavedChangesAtStartAfterTrimming() + { + // Arrange + UndoRedoService stack = CreateDirtyAtStartAfterTrimming(); + byte[] data = await stack.SaveStateAsync().ConfigureAwait(false); + + UndoRedoService reloaded = CreateService(); + reloaded.SetSerializer(new JsonUndoRedoSerializer()); + + // Act + bool success = await reloaded.LoadStateAsync(data).ConfigureAwait(false); + + // Assert + Assert.IsTrue(success); + Assert.AreEqual(-1, reloaded.CurrentPosition); + Assert.IsTrue(reloaded.HasUnsavedChanges, "A JSON round trip must not make the dirty state at -1 look saved"); + } + + [TestMethod] + public async Task UndoRedoService_SaveLoadState_KeepsCleanInitialState() + { + // Arrange: nothing trimmed or saved, so -1 is still the clean initial state + UndoRedoService stack = CreateService(); + stack.SetSerializer(new JsonUndoRedoSerializer()); + stack.Execute(new TestSerializableCommand("1")); + await stack.UndoAsync().ConfigureAwait(false); + Assert.IsFalse(stack.HasUnsavedChanges); + byte[] data = await stack.SaveStateAsync().ConfigureAwait(false); + + UndoRedoService reloaded = CreateService(); + reloaded.SetSerializer(new JsonUndoRedoSerializer()); + + // Act + bool success = await reloaded.LoadStateAsync(data).ConfigureAwait(false); + + // Assert + Assert.IsTrue(success); + Assert.IsFalse(reloaded.HasUnsavedChanges); + } + + [TestMethod] + public async Task UndoRedoService_LoadStateSavedBeforeInitialStateFlag_TreatsInitialStateAsClean() + { + // Arrange: data written before the flag existed has no initialStateIsClean field + UndoRedoService stack = CreateDirtyAtStartAfterTrimming(); + byte[] data = await stack.SaveStateAsync().ConfigureAwait(false); + System.Text.Json.Nodes.JsonObject root = System.Text.Json.Nodes.JsonNode.Parse(data)!.AsObject(); + Assert.IsTrue(root.Remove("initialStateIsClean"), "The flag should be written as initialStateIsClean"); + data = System.Text.Encoding.UTF8.GetBytes(root.ToJsonString()); + + UndoRedoService reloaded = CreateService(); + reloaded.SetSerializer(new JsonUndoRedoSerializer()); + + // Act + bool success = await reloaded.LoadStateAsync(data).ConfigureAwait(false); + + // Assert: old data still loads, with the meaning it had when it was written + Assert.IsTrue(success); + Assert.AreEqual(-1, reloaded.CurrentPosition); + Assert.IsFalse(reloaded.HasUnsavedChanges); + } + + [TestMethod] + public async Task JsonSerializer_SerializeDeserialize_PreservesInitialStateIsClean() + { + // Arrange + JsonUndoRedoSerializer serializer = new(); + + // Act + UndoRedoStackState dirty = await serializer.DeserializeAsync( + await serializer.SerializeAsync([new TestSerializableCommand("X")], -1, [], false).ConfigureAwait(false)).ConfigureAwait(false); + UndoRedoStackState clean = await serializer.DeserializeAsync( + await serializer.SerializeAsync([new TestSerializableCommand("X")], -1, [], true).ConfigureAwait(false)).ConfigureAwait(false); + + // Assert + Assert.IsFalse(dirty.InitialStateIsClean); + Assert.IsTrue(clean.InitialStateIsClean); + } + private static readonly DateTimeOffset SavedAt = new(2020, 1, 2, 3, 4, 5, TimeSpan.FromHours(10)); [TestMethod] @@ -578,7 +711,8 @@ public async Task JsonSerializer_SerializeDeserialize_PreservesSaveBoundaryTimes byte[] data = await serializer.SerializeAsync( [new TestSerializableCommand("X")], 0, - [new SaveBoundary(0, "Saved", SavedAt)]).ConfigureAwait(false); + [new SaveBoundary(0, "Saved", SavedAt)], + false).ConfigureAwait(false); // Act UndoRedoStackState state = await serializer.DeserializeAsync(data).ConfigureAwait(false); @@ -677,7 +811,7 @@ private static async Task SerializeWithCommandTypeAsync(string caseName) }; JsonUndoRedoSerializer serializer = new(); - byte[] data = await serializer.SerializeAsync([new TestSerializableCommand("saved")], 0, []).ConfigureAwait(false); + byte[] data = await serializer.SerializeAsync([new TestSerializableCommand("saved")], 0, [], true).ConfigureAwait(false); System.Text.Json.Nodes.JsonNode root = System.Text.Json.Nodes.JsonNode.Parse(data)!; System.Text.Json.Nodes.JsonObject command = root["commands"]![0]!.AsObject(); string typeKey = command.Single(p => p.Key.Equals("type", StringComparison.OrdinalIgnoreCase)).Key; diff --git a/UndoRedo/Contracts/ISaveBoundaryManager.cs b/UndoRedo/Contracts/ISaveBoundaryManager.cs index 07bd66a..4810d00 100644 --- a/UndoRedo/Contracts/ISaveBoundaryManager.cs +++ b/UndoRedo/Contracts/ISaveBoundaryManager.cs @@ -12,6 +12,28 @@ public interface ISaveBoundaryManager /// public IReadOnlyList SaveBoundaries { get; } + /// + /// Gets whether position -1 still holds the clean initial state, which needs no save boundary to + /// count as saved + /// + /// + /// This becomes once a save boundary is created, and once trimming the + /// oldest commands makes -1 the state after them. Persisted stack state carries it, so a reloaded + /// stack reports unsaved changes at -1 exactly as the original did. + /// + public bool InitialStateIsClean { get; } + + /// + /// Sets whether position -1 holds the clean initial state + /// + /// + /// Used when restoring saved stack state, since resets it to + /// . Creating a save boundary afterwards still sets it to + /// . + /// + /// Whether position -1 holds the clean initial state + public void SetInitialStateClean(bool isClean); + /// /// Gets whether there are unsaved changes since the last save boundary /// diff --git a/UndoRedo/Contracts/IUndoRedoSerializer.cs b/UndoRedo/Contracts/IUndoRedoSerializer.cs index 78c6846..d7fd291 100644 --- a/UndoRedo/Contracts/IUndoRedoSerializer.cs +++ b/UndoRedo/Contracts/IUndoRedoSerializer.cs @@ -15,12 +15,18 @@ public interface IUndoRedoSerializer /// The commands in the stack /// The current position in the stack /// The save boundaries + /// + /// Whether position -1 holds the clean initial state, as + /// reports it. It must round-trip into , so a + /// reloaded stack whose oldest commands were trimmed still reports unsaved changes at -1. + /// /// Cancellation token /// Serialized stack state public Task SerializeAsync( IReadOnlyList commands, int currentPosition, IReadOnlyList saveBoundaries, + bool initialStateIsClean, CancellationToken cancellationToken = default); /// diff --git a/UndoRedo/Models/UndoRedoStackState.cs b/UndoRedo/Models/UndoRedoStackState.cs index 26e07e2..4a2c5a1 100644 --- a/UndoRedo/Models/UndoRedoStackState.cs +++ b/UndoRedo/Models/UndoRedoStackState.cs @@ -24,6 +24,17 @@ DateTime Timestamp /// private const int EmptyPosition = -1; + /// + /// Gets whether position -1 holds the clean initial state, which needs no save boundary to count + /// as saved + /// + /// + /// Defaults to , which is what state saved before this was recorded meant. + /// It is once the stack has been saved, and once trimming the oldest + /// commands made -1 the state after them. + /// + public bool InitialStateIsClean { get; init; } = true; + /// /// Creates an empty stack state /// diff --git a/UndoRedo/Services/JsonUndoRedoSerializer.cs b/UndoRedo/Services/JsonUndoRedoSerializer.cs index 89b39bf..57f13a4 100644 --- a/UndoRedo/Services/JsonUndoRedoSerializer.cs +++ b/UndoRedo/Services/JsonUndoRedoSerializer.cs @@ -41,6 +41,7 @@ public async Task SerializeAsync( IReadOnlyList commands, int currentPosition, IReadOnlyList saveBoundaries, + bool initialStateIsClean, CancellationToken cancellationToken = default) { List serializableCommands = [.. commands.Select(ConvertToSerializableCommand)]; @@ -49,6 +50,7 @@ public async Task SerializeAsync( Commands = serializableCommands, CurrentPosition = currentPosition, SaveBoundaries = [.. saveBoundaries], + InitialStateIsClean = initialStateIsClean, FormatVersion = FormatVersion, Timestamp = DateTime.UtcNow }; @@ -80,7 +82,10 @@ public async Task DeserializeAsync( serializableState.CurrentPosition, serializableState.SaveBoundaries, serializableState.FormatVersion, - serializableState.Timestamp); + serializableState.Timestamp) + { + InitialStateIsClean = serializableState.InitialStateIsClean, + }; } /// @@ -239,6 +244,9 @@ private sealed class SerializableStackState public List Commands { get; set; } = []; public int CurrentPosition { get; set; } public List SaveBoundaries { get; set; } = []; + + // Data saved before this field existed has no value for it, and meant a clean initial state + public bool InitialStateIsClean { get; set; } = true; public string FormatVersion { get; set; } = string.Empty; public DateTime Timestamp { get; set; } } diff --git a/UndoRedo/Services/SaveBoundaryManager.cs b/UndoRedo/Services/SaveBoundaryManager.cs index 729babf..ddf735d 100644 --- a/UndoRedo/Services/SaveBoundaryManager.cs +++ b/UndoRedo/Services/SaveBoundaryManager.cs @@ -11,18 +11,22 @@ public sealed class SaveBoundaryManager : ISaveBoundaryManager { private readonly List _saveBoundaries = []; + /// + public IReadOnlyList SaveBoundaries => _saveBoundaries.AsReadOnly(); + // Whether position -1 still holds the untouched initial state, which is clean without a boundary. // It stops being true once anything is saved, since the saved state replaces it, and once trimming // shifts later commands' results down to -1. - private bool _initialStateIsClean = true; + /// + public bool InitialStateIsClean { get; private set; } = true; /// - public IReadOnlyList SaveBoundaries => _saveBoundaries.AsReadOnly(); + public void SetInitialStateClean(bool isClean) => InitialStateIsClean = isClean; /// public bool HasUnsavedChanges(int currentPosition) { - if (currentPosition == -1 && _initialStateIsClean) + if (currentPosition == -1 && InitialStateIsClean) { return false; } @@ -36,7 +40,7 @@ public SaveBoundary CreateSaveBoundary(int position, string? description = null) { SaveBoundary saveBoundary = new(position, description); _saveBoundaries.Add(saveBoundary); - _initialStateIsClean = false; + InitialStateIsClean = false; return saveBoundary; } @@ -46,7 +50,7 @@ public SaveBoundary CreateSaveBoundary(int position, string? description = null) internal void RestoreSaveBoundary(SaveBoundary saveBoundary) { _saveBoundaries.Add(new SaveBoundary(saveBoundary.Position, saveBoundary.Description, saveBoundary.Timestamp)); - _initialStateIsClean = false; + InitialStateIsClean = false; } /// @@ -75,7 +79,7 @@ public void AdjustPositions(int adjustment) if (adjustment < 0) { // Commands were trimmed from the bottom, so -1 is now the state after them, not the initial one - _initialStateIsClean = false; + InitialStateIsClean = false; } for (int i = _saveBoundaries.Count - 1; i >= 0; i--) @@ -116,6 +120,6 @@ public IEnumerable GetCommandsToUndo(SaveBoundary saveBoundary, int cu public void Clear() { _saveBoundaries.Clear(); - _initialStateIsClean = true; + InitialStateIsClean = true; } } diff --git a/UndoRedo/Services/UndoRedoService.cs b/UndoRedo/Services/UndoRedoService.cs index 53878ab..4f137fd 100644 --- a/UndoRedo/Services/UndoRedoService.cs +++ b/UndoRedo/Services/UndoRedoService.cs @@ -364,6 +364,7 @@ public async Task SaveStateAsync(CancellationToken cancellationToken = d _stackManager.Commands, _stackManager.CurrentPosition, _saveBoundaryManager.SaveBoundaries, + _saveBoundaryManager.InitialStateIsClean, cancellationToken).ConfigureAwait(false); } @@ -393,7 +394,10 @@ public async Task LoadStateAsync(byte[] data, CancellationToken cancellati [.. _saveBoundaryManager.SaveBoundaries], "1.0", // Format version DateTime.UtcNow - ); + ) + { + InitialStateIsClean = _saveBoundaryManager.InitialStateIsClean, + }; /// public bool RestoreFromState(UndoRedoStackState state) @@ -427,6 +431,9 @@ public bool RestoreFromState(UndoRedoStackState state) _stackManager.MoveNext(); } + // Clear() reset this to true. Restore it before the boundaries, which set it to false. + _saveBoundaryManager.SetInitialStateClean(state.InitialStateIsClean); + // Recreate save boundaries at the stored positions. The built-in manager keeps each one's // original timestamp; ISaveBoundaryManager has no member for that, so a custom manager // creates them afresh. diff --git a/docs/serialization.md b/docs/serialization.md index d306ee0..3956011 100644 --- a/docs/serialization.md +++ b/docs/serialization.md @@ -32,6 +32,7 @@ public interface IUndoRedoSerializer IReadOnlyList commands, int currentPosition, IReadOnlyList saveBoundaries, + bool initialStateIsClean, CancellationToken cancellationToken = default); Task DeserializeAsync( @@ -50,9 +51,18 @@ public record UndoRedoStackState( int CurrentPosition, IReadOnlyList SaveBoundaries, string FormatVersion, - DateTime Timestamp); + DateTime Timestamp) +{ + // Whether position -1 is the clean initial state. False once the stack has been saved, or once + // trimming the oldest commands made -1 the state after them. Defaults to true, so data saved + // before this was recorded still loads. + public bool InitialStateIsClean { get; init; } = true; +} ``` +A serializer must round-trip `initialStateIsClean` into `UndoRedoStackState.InitialStateIsClean`. +Otherwise a reloaded stack whose oldest commands were trimmed reports no unsaved changes at -1. + ## Basic Usage ### Setting Up Serialization @@ -111,6 +121,7 @@ public class BinaryUndoRedoSerializer : IUndoRedoSerializer IReadOnlyList commands, int currentPosition, IReadOnlyList saveBoundaries, + bool initialStateIsClean, CancellationToken cancellationToken = default) { using var stream = new MemoryStream(); @@ -445,9 +456,10 @@ public class CompressedJsonSerializer : IUndoRedoSerializer IReadOnlyList commands, int currentPosition, IReadOnlyList saveBoundaries, + bool initialStateIsClean, CancellationToken cancellationToken = default) { - var jsonData = await _jsonSerializer.SerializeAsync(commands, currentPosition, saveBoundaries, cancellationToken); + var jsonData = await _jsonSerializer.SerializeAsync(commands, currentPosition, saveBoundaries, initialStateIsClean, cancellationToken); using var output = new MemoryStream(); using var gzip = new GZipStream(output, CompressionLevel.Optimal);