From df9f33ee35dccaaa30e155d36d6b2e970d0b457b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 03:28:46 +0000 Subject: [PATCH] Keep save-boundary timestamps through load and restore [minor] SaveBoundary.Timestamp was get-only and not a constructor parameter, so System.Text.Json discarded the saved value and stamped every loaded boundary with the load time, and RestoreFromState re-created each boundary through CreateSaveBoundary, which stamped it again. Add a public SaveBoundary(position, description, timestamp) constructor marked [JsonConstructor]; a missing timestamp still means now, so older data loads as before. RestoreFromState adds boundaries with their original timestamp when the manager is the built-in SaveBoundaryManager; ISaveBoundaryManager is unchanged, so custom managers keep working and create boundaries afresh. Trimming already carried the timestamp over; a test now guards that too. Correct the SaveBoundary shape in the API reference and the binary serializer example. Fixes #91 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TXZMdofSRgS8oXS9RgTRhM --- UndoRedo.Test/SerializationTests.cs | 88 ++++++++++++++++++++++++ UndoRedo.Test/UndoRedoStackTests.cs | 18 +++++ UndoRedo/Models/SaveBoundary.cs | 21 ++++++ UndoRedo/Services/SaveBoundaryManager.cs | 9 +++ UndoRedo/Services/UndoRedoService.cs | 13 +++- docs/api-reference.md | 15 ++-- docs/serialization.md | 6 +- 7 files changed, 160 insertions(+), 10 deletions(-) diff --git a/UndoRedo.Test/SerializationTests.cs b/UndoRedo.Test/SerializationTests.cs index 7e08b3f..b96711a 100644 --- a/UndoRedo.Test/SerializationTests.cs +++ b/UndoRedo.Test/SerializationTests.cs @@ -568,6 +568,94 @@ public async Task UndoRedoService_SaveLoadState_ReconstructsCommandWithEmptyData Assert.AreEqual(-1, newStack.CurrentPosition); } + private static readonly DateTimeOffset SavedAt = new(2020, 1, 2, 3, 4, 5, TimeSpan.FromHours(10)); + + [TestMethod] + public async Task JsonSerializer_SerializeDeserialize_PreservesSaveBoundaryTimestamp() + { + // Arrange + JsonUndoRedoSerializer serializer = new(); + byte[] data = await serializer.SerializeAsync( + [new TestSerializableCommand("X")], + 0, + [new SaveBoundary(0, "Saved", SavedAt)]).ConfigureAwait(false); + + // Act + UndoRedoStackState state = await serializer.DeserializeAsync(data).ConfigureAwait(false); + + // Assert + Assert.AreEqual(SavedAt, state.SaveBoundaries[0].Timestamp, "The timestamp must be when the save was made, not when it was loaded"); + } + + [TestMethod] + public async Task UndoRedoService_SaveLoadState_PreservesSaveBoundaryTimestamp() + { + // Arrange + UndoRedoService stack = CreateService(); + stack.SetSerializer(new JsonUndoRedoSerializer()); + stack.Execute(new TestSerializableCommand("X")); + stack.MarkAsSaved("Saved"); + DateTimeOffset savedAt = stack.SaveBoundaries[0].Timestamp; + byte[] data = await stack.SaveStateAsync().ConfigureAwait(false); + + // Rewrite the saved timestamp to a fixed past time, so a load that restamps it cannot match by chance + System.Text.Json.Nodes.JsonNode root = System.Text.Json.Nodes.JsonNode.Parse(data)!; + System.Text.Json.Nodes.JsonObject boundary = root["saveBoundaries"]![0]!.AsObject(); + Assert.AreEqual(savedAt, boundary["timestamp"]!.GetValue(), "The save boundary timestamp should be written"); + boundary["timestamp"] = SavedAt; + 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 + Assert.IsTrue(success); + Assert.AreEqual(SavedAt, reloaded.SaveBoundaries[0].Timestamp); + Assert.IsFalse(reloaded.HasUnsavedChanges, "The restored boundary should still mark the saved position"); + } + + [TestMethod] + public async Task JsonSerializer_DeserializeSaveBoundaryWithoutTimestamp_UsesLoadTime() + { + // Arrange: data written before save boundary timestamps were read back may not carry one + JsonUndoRedoSerializer serializer = new(); + byte[] data = System.Text.Encoding.UTF8.GetBytes( + """{"commands":[],"currentPosition":-1,"saveBoundaries":[{"position":-1,"description":"d"}],"formatVersion":"json-v1.0"}"""); + DateTimeOffset before = DateTimeOffset.Now; + + // Act + UndoRedoStackState state = await serializer.DeserializeAsync(data).ConfigureAwait(false); + + // Assert + Assert.AreEqual("d", state.SaveBoundaries[0].Description); + Assert.IsGreaterThanOrEqualTo(before, state.SaveBoundaries[0].Timestamp); + } + + [TestMethod] + public void UndoRedoService_RestoreFromState_PreservesSaveBoundaryTimestamp() + { + // Arrange + UndoRedoStackState state = new( + [new TestSerializableCommand("X")], + 0, + [new SaveBoundary(0, "Saved", SavedAt)], + "1.0", + DateTime.UtcNow); + UndoRedoService stack = CreateService(); + + // Act + bool success = stack.RestoreFromState(state); + + // Assert + Assert.IsTrue(success); + Assert.AreEqual(SavedAt, stack.SaveBoundaries[0].Timestamp); + Assert.AreEqual("Saved", stack.SaveBoundaries[0].Description); + Assert.IsFalse(stack.HasUnsavedChanges); + } + private const string MalformedAssemblyName = "malformed assembly name"; private const string InvalidVersion = "invalid assembly version"; private const string NotACommand = "serializable type that is not a command"; diff --git a/UndoRedo.Test/UndoRedoStackTests.cs b/UndoRedo.Test/UndoRedoStackTests.cs index fd42f6e..96e8934 100644 --- a/UndoRedo.Test/UndoRedoStackTests.cs +++ b/UndoRedo.Test/UndoRedoStackTests.cs @@ -856,6 +856,24 @@ public async Task UndoToSaveBoundary_BoundaryHeldAcrossTrim_UndoesToSavedState() Assert.IsFalse(stack.HasUnsavedChanges, "The stack should be at the save point"); } + [TestMethod] + public void SaveBoundary_StackTrimmed_KeepsTimestamp() + { + // Arrange + UndoRedoService stack = new(new StackManager(), new SaveBoundaryManager(), new CommandMerger(), UndoRedoOptions.Create(maxStackSize: 2)); + stack.Execute(new DelegateCommand("A", () => { }, () => { })); + stack.Execute(new DelegateCommand("B", () => { }, () => { })); + stack.MarkAsSaved(); + DateTimeOffset savedAt = stack.SaveBoundaries[0].Timestamp; + + // Act + stack.Execute(new DelegateCommand("C", () => { }, () => { })); // Trims A, shifting the save point down + + // Assert + Assert.AreEqual(0, stack.SaveBoundaries[0].Position); + Assert.AreEqual(savedAt, stack.SaveBoundaries[0].Timestamp, "Trimming must not restamp the save point"); + } + [TestMethod] public async Task UndoToSaveBoundary_BoundaryRemovedByBranching_ReturnsFalse() { diff --git a/UndoRedo/Models/SaveBoundary.cs b/UndoRedo/Models/SaveBoundary.cs index 4fd566a..96564b3 100644 --- a/UndoRedo/Models/SaveBoundary.cs +++ b/UndoRedo/Models/SaveBoundary.cs @@ -2,6 +2,8 @@ namespace ktsu.UndoRedo; +using System.Text.Json.Serialization; + /// /// Represents a save boundary in the undo/redo stack /// @@ -9,6 +11,25 @@ namespace ktsu.UndoRedo; /// Optional description public sealed class SaveBoundary(int position, string? description = null) { + /// + /// Recreates a save boundary that was created earlier, keeping when it was created + /// + /// The position in the stack + /// Optional description + /// + /// When the save boundary was created, or to use the current time, which is + /// also what saved data without a timestamp deserializes to + /// + [JsonConstructor] + public SaveBoundary(int position, string? description, DateTimeOffset timestamp) + : this(position, description) + { + if (timestamp != default) + { + Timestamp = timestamp; + } + } + /// /// Creates a copy of at a new position that is still the same save point, /// so a caller holding the original can have it resolved to where the save point is now diff --git a/UndoRedo/Services/SaveBoundaryManager.cs b/UndoRedo/Services/SaveBoundaryManager.cs index 3d92029..c825f4f 100644 --- a/UndoRedo/Services/SaveBoundaryManager.cs +++ b/UndoRedo/Services/SaveBoundaryManager.cs @@ -40,6 +40,15 @@ public SaveBoundary CreateSaveBoundary(int position, string? description = null) return saveBoundary; } + /// + /// Adds a save boundary recreated from saved state, keeping the time it was originally created + /// + internal void RestoreSaveBoundary(SaveBoundary saveBoundary) + { + _saveBoundaries.Add(new SaveBoundary(saveBoundary.Position, saveBoundary.Description, saveBoundary.Timestamp)); + _initialStateIsClean = false; + } + /// public int CleanupInvalidBoundaries(int maxValidPosition) { diff --git a/UndoRedo/Services/UndoRedoService.cs b/UndoRedo/Services/UndoRedoService.cs index 660ff30..bab51e1 100644 --- a/UndoRedo/Services/UndoRedoService.cs +++ b/UndoRedo/Services/UndoRedoService.cs @@ -415,10 +415,19 @@ public bool RestoreFromState(UndoRedoStackState state) _stackManager.MoveNext(); } - // Recreate save boundaries by creating them at the stored positions + // 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. foreach (SaveBoundary boundary in state.SaveBoundaries) { - _saveBoundaryManager.CreateSaveBoundary(boundary.Position, boundary.Description); + if (_saveBoundaryManager is SaveBoundaryManager builtInManager) + { + builtInManager.RestoreSaveBoundary(boundary); + } + else + { + _saveBoundaryManager.CreateSaveBoundary(boundary.Position, boundary.Description); + } } return true; diff --git a/docs/api-reference.md b/docs/api-reference.md index f88c1c8..faf02c9 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -689,15 +689,20 @@ public record ChangeMetadata( Represents a save point in the command history. ```csharp -public record SaveBoundary( - int Position, - DateTime Timestamp, - string? Description = null); +public sealed class SaveBoundary(int position, string? description = null) +{ + // Recreates an earlier save boundary, keeping when it was created + public SaveBoundary(int position, string? description, DateTimeOffset timestamp); + + public int Position { get; } + public DateTimeOffset Timestamp { get; } + public string? Description { get; } +} ``` **Properties:** - `Position`: Position in the command stack -- `Timestamp`: When the save boundary was created +- `Timestamp`: When the save boundary was created. It is kept through a save and load, `RestoreFromState` with the built-in `SaveBoundaryManager`, and stack trimming - `Description`: Optional description ### ChangeVisualization diff --git a/docs/serialization.md b/docs/serialization.md index bb1182d..d306ee0 100644 --- a/docs/serialization.md +++ b/docs/serialization.md @@ -134,7 +134,7 @@ public class BinaryUndoRedoSerializer : IUndoRedoSerializer foreach (var boundary in saveBoundaries) { writer.Write(boundary.Position); - writer.Write(boundary.Timestamp.ToBinary()); + writer.Write(boundary.Timestamp.UtcTicks); writer.Write(boundary.Description ?? string.Empty); } @@ -173,9 +173,9 @@ public class BinaryUndoRedoSerializer : IUndoRedoSerializer for (int i = 0; i < boundaryCount; i++) { int position = reader.ReadInt32(); - DateTime boundaryTime = DateTime.FromBinary(reader.ReadInt64()); + DateTimeOffset boundaryTime = new(reader.ReadInt64(), TimeSpan.Zero); string description = reader.ReadString(); - boundaries.Add(new SaveBoundary(position, boundaryTime, description)); + boundaries.Add(new SaveBoundary(position, description, boundaryTime)); } return new UndoRedoStackState(commands, currentPosition, boundaries, version, timestamp);