Keep a trimmed stack dirty at -1 through save, load, and restore [major]

UndoRedo.Test/SerializationTests.cs

@@ -18,7 +18,7 @@
 		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 @@
 		];
 
 		// 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 @@
 		];
 
 		// 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 @@
 		// 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 @@
 		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 @@
 		Assert.AreEqual(-1, newStack.CurrentPosition);
 	}
 
+	/// <summary>
+	/// 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.
+	/// </summary>
+	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 @@
 		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 @@
 		};
 
 		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;

UndoRedo/Contracts/ISaveBoundaryManager.cs

@@ -12,6 +12,28 @@ public interface ISaveBoundaryManager
 	/// </summary>
 	public IReadOnlyList<SaveBoundary> SaveBoundaries { get; }
 
+	/// <summary>
+	/// Gets whether position -1 still holds the clean initial state, which needs no save boundary to
+	/// count as saved
+	/// </summary>
+	/// <remarks>
+	/// This becomes <see langword="false"/> 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.
+	/// </remarks>
+	public bool InitialStateIsClean { get; }
+
+	/// <summary>
+	/// Sets whether position -1 holds the clean initial state
+	/// </summary>
+	/// <remarks>
+	/// Used when restoring saved stack state, since <see cref="Clear"/> resets it to
+	/// <see langword="true"/>. Creating a save boundary afterwards still sets it to
+	/// <see langword="false"/>.
+	/// </remarks>
+	/// <param name="isClean">Whether position -1 holds the clean initial state</param>
+	public void SetInitialStateClean(bool isClean);
+
 	/// <summary>
 	/// Gets whether there are unsaved changes since the last save boundary
 	/// </summary>

UndoRedo/Contracts/IUndoRedoSerializer.cs

@@ -15,12 +15,18 @@ public interface IUndoRedoSerializer
 	/// <param name="commands">The commands in the stack</param>
 	/// <param name="currentPosition">The current position in the stack</param>
 	/// <param name="saveBoundaries">The save boundaries</param>
+	/// <param name="initialStateIsClean">
+	/// Whether position -1 holds the clean initial state, as <see cref="ISaveBoundaryManager.InitialStateIsClean"/>
+	/// reports it. It must round-trip into <see cref="UndoRedoStackState.InitialStateIsClean"/>, so a
+	/// reloaded stack whose oldest commands were trimmed still reports unsaved changes at -1.
+	/// </param>
 	/// <param name="cancellationToken">Cancellation token</param>
 	/// <returns>Serialized stack state</returns>
 	public Task<byte[]> SerializeAsync(
 		IReadOnlyList<ICommand> commands,
 		int currentPosition,
 		IReadOnlyList<SaveBoundary> saveBoundaries,
+		bool initialStateIsClean,
 		CancellationToken cancellationToken = default);
 
 	/// <summary>

UndoRedo/Models/UndoRedoStackState.cs

@@ -24,6 +24,17 @@ DateTime Timestamp
 	/// </summary>
 	private const int EmptyPosition = -1;
 
+	/// <summary>
+	/// Gets whether position -1 holds the clean initial state, which needs no save boundary to count
+	/// as saved
+	/// </summary>
+	/// <remarks>
+	/// Defaults to <see langword="true"/>, which is what state saved before this was recorded meant.
+	/// It is <see langword="false"/> once the stack has been saved, and once trimming the oldest
+	/// commands made -1 the state after them.
+	/// </remarks>
+	public bool InitialStateIsClean { get; init; } = true;
+
 	/// <summary>
 	/// Creates an empty stack state
 	/// </summary>

UndoRedo/Services/JsonUndoRedoSerializer.cs

@@ -41,6 +41,7 @@ public async Task<byte[]> SerializeAsync(
 		IReadOnlyList<ICommand> commands,
 		int currentPosition,
 		IReadOnlyList<SaveBoundary> saveBoundaries,
+		bool initialStateIsClean,
 		CancellationToken cancellationToken = default)
 	{
 		List<SerializableCommand> serializableCommands = [.. commands.Select(ConvertToSerializableCommand)];
@@ -49,6 +50,7 @@ public async Task<byte[]> SerializeAsync(
 			Commands = serializableCommands,
 			CurrentPosition = currentPosition,
 			SaveBoundaries = [.. saveBoundaries],
+			InitialStateIsClean = initialStateIsClean,
 			FormatVersion = FormatVersion,
 			Timestamp = DateTime.UtcNow
 		};
@@ -80,7 +82,10 @@ public async Task<UndoRedoStackState> DeserializeAsync(
 			serializableState.CurrentPosition,
 			serializableState.SaveBoundaries,
 			serializableState.FormatVersion,
-			serializableState.Timestamp);
+			serializableState.Timestamp)
+		{
+			InitialStateIsClean = serializableState.InitialStateIsClean,
+		};
 	}
 
 	/// <summary>
@@ -239,6 +244,9 @@ private sealed class SerializableStackState
 		public List<SerializableCommand> Commands { get; set; } = [];
 		public int CurrentPosition { get; set; }
 		public List<SaveBoundary> 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; }
 	}

UndoRedo/Services/SaveBoundaryManager.cs

@@ -11,18 +11,22 @@ public sealed class SaveBoundaryManager : ISaveBoundaryManager
 {
 	private readonly List<SaveBoundary> _saveBoundaries = [];
 
+	/// <inheritdoc />
+	public IReadOnlyList<SaveBoundary> 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;
+	/// <inheritdoc />
+	public bool InitialStateIsClean { get; private set; } = true;
 
 	/// <inheritdoc />
-	public IReadOnlyList<SaveBoundary> SaveBoundaries => _saveBoundaries.AsReadOnly();
+	public void SetInitialStateClean(bool isClean) => InitialStateIsClean = isClean;
 
 	/// <inheritdoc />
 	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;
 	}
 
 	/// <inheritdoc />
@@ -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<ICommand> GetCommandsToUndo(SaveBoundary saveBoundary, int cu
 	public void Clear()
 	{
 		_saveBoundaries.Clear();
-		_initialStateIsClean = true;
+		InitialStateIsClean = true;
 	}
 }

UndoRedo/Services/UndoRedoService.cs

@@ -307,7 +307,7 @@
 				cts.CancelAfter(timeout);
 			}
 
 			await _navigationProvider.NavigateToAsync(navigationContext!, cts.Token).ConfigureAwait(false);
 		}
 #pragma warning disable CA1031 // Do not catch general exception types
 		catch (Exception)
@@ -364,6 +364,7 @@
 			_stackManager.Commands,
 			_stackManager.CurrentPosition,
 			_saveBoundaryManager.SaveBoundaries,
+			_saveBoundaryManager.InitialStateIsClean,
 			cancellationToken).ConfigureAwait(false);
 	}
 
@@ -393,7 +394,10 @@
 		[.. _saveBoundaryManager.SaveBoundaries],
 		"1.0", // Format version
 		DateTime.UtcNow
-	);
+	)
+	{
+		InitialStateIsClean = _saveBoundaryManager.InitialStateIsClean,
+	};
 
 	/// <inheritdoc />
 	public bool RestoreFromState(UndoRedoStackState state)
@@ -427,6 +431,9 @@
 				_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.