diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a5788aa..b676735a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Fixed + +- `DocxSnapshotStore.CompareAsync` and `DocxVersionHistory.CompareVersionsAsync` now copy the + caller's `DocxDiffSettings` before awaiting host blob storage. Previously the settings object + was read only after the asynchronous snapshot reads (and lazily again by the returned + comparison), so a caller that reused and changed it mid-flight got a comparison stamped with + the changed values, e.g. the wrong revision author. + ## [12.6.2] - 2026-09-16 ### Fixed diff --git a/Docxodus.Tests/DocxSnapshotStoreTests.cs b/Docxodus.Tests/DocxSnapshotStoreTests.cs index 7a523252..d66b3546 100644 --- a/Docxodus.Tests/DocxSnapshotStoreTests.cs +++ b/Docxodus.Tests/DocxSnapshotStoreTests.cs @@ -1,8 +1,6 @@ // Copyright (c) John Scrudato IV. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -#nullable enable - using System.IO.Compression; using System.Text.Json; using Docxodus.History; @@ -115,14 +113,40 @@ public async Task ComparisonReusesExistingSemanticAndNativeRedlineProducts() Assert.Equal(after, await store.ExportAsync(right)); } + [Fact] + public async Task ComparisonOwnsSettingsBeforeAwaitingHostStorage() + { + var before = DocxSession.CreateBlankDocxBytes(); + using var session = new DocxSession(before); + Assert.True(session.ReplaceText(session.Project().AnchorIndex.Keys.First(), "version two").Success); + var blobs = new RecordingStore(); + var store = new DocxSnapshotStore(blobs); + var left = await store.CaptureAsync(before); + var right = await store.CaptureAsync(session.Save()); + var settings = new DocxDiffSettings { AuthorForRevisions = "History comparison" }; + blobs.PauseReads = true; + var pending = store.CompareAsync(left, right, settings).AsTask(); + await blobs.ReadStarted.Task; + // A caller reusing its settings object while storage is slow must not retarget this comparison. + settings.AuthorForRevisions = "Changed during storage read"; + blobs.ReleaseRead.SetResult(); + var comparison = await pending; + var revisions = comparison.GetRevisions(); + Assert.NotEmpty(revisions); + Assert.All(revisions, revision => Assert.Equal("History comparison", revision.Author)); + } + private sealed class RecordingStore : IHistoryBlobStore { private readonly MemoryHistoryBlobStore _inner = new(); internal bool PauseWrites { get; init; } + internal bool PauseReads { get; set; } internal int Writes { get; private set; } internal int Reads { get; private set; } internal TaskCompletionSource WriteStarted { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); internal TaskCompletionSource ReleaseWrite { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + internal TaskCompletionSource ReadStarted { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + internal TaskCompletionSource ReleaseRead { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); public async ValueTask PutAsync(HistoryBlobReference reference, Stream content, CancellationToken cancellationToken) { @@ -132,10 +156,12 @@ public async ValueTask PutAsync(HistoryBlobReference reference, Stream content, await _inner.PutAsync(reference, content, cancellationToken); } - public ValueTask OpenReadAsync(HistoryBlobReference reference, CancellationToken cancellationToken) + public async ValueTask OpenReadAsync(HistoryBlobReference reference, CancellationToken cancellationToken) { Reads++; - return _inner.OpenReadAsync(reference, cancellationToken); + ReadStarted.TrySetResult(); + if (PauseReads) await ReleaseRead.Task.WaitAsync(cancellationToken); + return await _inner.OpenReadAsync(reference, cancellationToken); } } } diff --git a/Docxodus.Tests/DocxVersionHistoryTests.cs b/Docxodus.Tests/DocxVersionHistoryTests.cs index 1f608504..121d0ea6 100644 --- a/Docxodus.Tests/DocxVersionHistoryTests.cs +++ b/Docxodus.Tests/DocxVersionHistoryTests.cs @@ -194,6 +194,25 @@ public async Task CancellationAfterSuccessfulCASDoesNotReportACommittedVersionAs Assert.Equal(saved.Head, (await history.ReadAsync("doc"))!.Head); } + [Fact] + public async Task VersionComparisonOwnsSettingsBeforeAwaitingHostStorage() + { + var blobs = new PausingBlobs(); + var history = new DocxVersionHistory(blobs, new MemoryHistoryHeadStore()); + var (before, after) = Documents(); + var first = await history.CreateVersionAsync("doc", null, before, Metadata("first")); + var edited = await history.CreateVersionAsync("doc", first.Head, after, Metadata("edited")); + var settings = new DocxDiffSettings { AuthorForRevisions = "History comparison" }; + blobs.Pause = true; + var pending = history.CompareVersionsAsync("doc", first.Version.Id, edited.Version.Id, settings).AsTask(); + await blobs.Started.Task; + settings.AuthorForRevisions = "Changed during storage read"; + blobs.Release.SetResult(); + var revisions = (await pending).GetRevisions(); + Assert.NotEmpty(revisions); + Assert.All(revisions, revision => Assert.Equal("History comparison", revision.Author)); + } + private static DocxVersionMetadata Metadata(string label) => new() { Author = "Host actor", CreatedAt = DateTimeOffset.UnixEpoch, Label = label, Message = "saved", @@ -232,6 +251,24 @@ private sealed class FaultingHeads : IHistoryHeadStore return result; } } + private sealed class PausingBlobs : IHistoryBlobStore + { + private readonly MemoryHistoryBlobStore _inner = new(); + internal bool Pause { get; set; } + internal TaskCompletionSource Started { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + internal TaskCompletionSource Release { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + public ValueTask PutAsync(HistoryBlobReference reference, Stream content, CancellationToken cancellationToken) => + _inner.PutAsync(reference, content, cancellationToken); + public async ValueTask OpenReadAsync(HistoryBlobReference reference, CancellationToken cancellationToken) + { + if (Pause) + { + Started.TrySetResult(); + await Release.Task.WaitAsync(cancellationToken); + } + return await _inner.OpenReadAsync(reference, cancellationToken); + } + } private sealed class PausingHeads : IHistoryHeadStore { private readonly MemoryHistoryHeadStore _inner = new(); diff --git a/Docxodus/History/DocxSnapshotStore.cs b/Docxodus/History/DocxSnapshotStore.cs index f4260a2f..a0061373 100644 --- a/Docxodus/History/DocxSnapshotStore.cs +++ b/Docxodus/History/DocxSnapshotStore.cs @@ -1,8 +1,6 @@ // Copyright (c) John Scrudato IV. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -#nullable enable - using Docxodus.Verification; namespace Docxodus.History; @@ -68,12 +66,17 @@ public async ValueTask ExportAsync(DocxSnapshotReference snapshot, } /// - /// Verify both snapshots and return the existing lazy semantic/redline comparison. DocxDiff - /// owns compatibility policy and limitations, including pre-existing tracked revisions. + /// Verify both snapshots and return the raw-engine lazy semantic/redline comparison. This is + /// semantics, not the front door: no input + /// revisions are pre-accepted unless asks for it. Settings are + /// copied before the first await, so a caller reusing its object cannot retarget the comparison. /// public async ValueTask CompareAsync(DocxSnapshotReference before, DocxSnapshotReference after, DocxDiffSettings? settings = null, CancellationToken cancellationToken = default) { + ArgumentNullException.ThrowIfNull(before); + ArgumentNullException.ThrowIfNull(after); + settings = settings?.Clone(); var left = await ExportAsync(before, cancellationToken).ConfigureAwait(false); var right = await ExportAsync(after, cancellationToken).ConfigureAwait(false); cancellationToken.ThrowIfCancellationRequested(); diff --git a/Docxodus/History/DocxVersionHistory.cs b/Docxodus/History/DocxVersionHistory.cs index a23d5b81..956fe0e7 100644 --- a/Docxodus/History/DocxVersionHistory.cs +++ b/Docxodus/History/DocxVersionHistory.cs @@ -228,6 +228,8 @@ await _snapshots.ExportAsync((await GetVersionAsync(documentId, versionId, cance public async ValueTask CompareVersionsAsync(string documentId, HistoryBlobReference before, HistoryBlobReference after, DocxDiffSettings? settings = null, CancellationToken cancellationToken = default) { + // Own the caller's settings before the version reads yield to host storage. + settings = settings?.Clone(); var left = await GetVersionAsync(documentId, before, cancellationToken).ConfigureAwait(false); var right = await GetVersionAsync(documentId, after, cancellationToken).ConfigureAwait(false); return await _snapshots.CompareAsync(left.Record.Snapshot, right.Record.Snapshot, settings, cancellationToken).ConfigureAwait(false); diff --git a/docs/architecture/collaboration_and_version_history.md b/docs/architecture/collaboration_and_version_history.md index a0aadb23..ec9b5a25 100644 --- a/docs/architecture/collaboration_and_version_history.md +++ b/docs/architecture/collaboration_and_version_history.md @@ -12,7 +12,7 @@ The next layer adds `PackageChangeSetCodec.SaveAsync`/`LoadAsync` and `IHistoryB Reference payload adapters are `MemoryHistoryBlobStore` (thread-safe, process-local) and `FileHistoryBlobStore` (independent instances may share a host-owned directory). Both enforce a configurable per-blob byte limit and verify incoming length/SHA-256 before publication. Filesystem writes stream through a private temporary file, flush its bytes, and atomically publish without overwriting existing content; an existing blob is verified before accepting an idempotent write. Reopen works without process-local state. Readers still verify returned bytes through the codec, since externally modified disk contents are not trusted. The host owns directory permissions, aggregate quotas, retention, and cleanup of abandoned `.tmp` files; power-loss durability depends on its filesystem's rename/directory guarantees. These adapters store blobs, not document heads or version metadata. -`DocxSnapshotStore.CaptureAsync` now retains exact caller-supplied DOCX bytes after bounded WordprocessingML inspection, returning both a raw-byte blob reference and the existing ordered OPC content digest. `ExportAsync` returns those bytes unchanged after verification; repacked equivalents share a content digest but can have different exact snapshot identities. Capture owns its bytes before waiting for host storage. `CompareAsync` verifies both snapshots and delegates lazy semantic/native-redline products to `DocxDiff`, retaining its compatibility policy and tracked-revision limitations. Version IDs, metadata, head publication, and restore are separate next layers; capture alone does not publish a version or edit a live session. +`DocxSnapshotStore.CaptureAsync` now retains exact caller-supplied DOCX bytes after bounded WordprocessingML inspection, returning both a raw-byte blob reference and the existing ordered OPC content digest. `ExportAsync` returns those bytes unchanged after verification; repacked equivalents share a content digest but can have different exact snapshot identities. Capture owns its bytes before waiting for host storage. `CompareAsync` verifies both snapshots and delegates lazy semantic/native-redline products to the raw `DocxDiff` engine, with its opt-in input-revision defaults rather than the `DocxCompare` front-door pre-accept (the client-facing `CompareVersionsToDocxAsync` goes through `DocxCompare`). It copies the caller's settings before its first await. Version IDs, metadata, head publication, and restore are separate next layers; capture alone does not publish a version or edit a live session. `IHistoryHeadStore` supplies the atomic publication primitive: read a `(revision, stateManifestBlob)` head, then `TryAdvanceAsync` with that exact expectation after persisting all referenced blobs. One immutable state manifest can bind log/checkpoint/version/deduplication tips together. Every successful publication increments its revision, even for identical state, preventing ABA; this revision is separate from content-log sequence. The memory adapter is process-local; the filesystem adapter uses persistent per-document exclusive-share lock files and atomic head replacement across cooperating local processes. It rejects corrupt/oversized head metadata and supports cancellation while contended. Since [.NET's Unix file sharing is best-effort and can be disabled](https://github.com/dotnet/runtime/blob/v10.0.0/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs), the adapter probes actual exclusion and fails closed if a second exclusive open succeeds. Protect its directory, never delete active `.lock` files, and do not use it as a network-filesystem/distributed coordinator. Authorization, state-graph validation, and storage durability remain host responsibilities.