Follow-up from the test-suite review (batches merged as #92–#96). Existing JsonSyncable tests assert only file existence / line count and use a single client id, so the multi-client format behavior and error tolerance are untested.
Gaps
Per-client file fan-out — commits are grouped by ClientId into separate client_<id>.jsonl files, and GetSyncState aggregates heads across them. Every test uses one ClientId, so this is unexercised.
|
public async Task<SyncState> GetSyncState() |
|
{ |
|
var heads = new ConcurrentDictionary<Guid, long>(); |
|
await Parallel.ForEachAsync(AllClientFiles(), async (file, ct) => |
|
{ |
|
var ts = await GetHeadTimestampAsync(file, ct); |
|
if (ts is not null) |
|
heads[ClientIdForFile(file)] = ts.Value.ToUnixTimeMilliseconds(); |
|
}); |
|
return new SyncState(new Dictionary<Guid, long>(heads)); |
|
} |
|
|
|
public async Task<ChangesResult<Commit>> GetChanges(SyncState otherHeads) |
|
{ |
|
var heads = new ConcurrentDictionary<Guid, long>(); |
|
var allCommits = new ConcurrentBag<Commit>(); |
|
var files = AllClientFiles().ToArray(); |
|
await Parallel.ForEachAsync(files, async (file, ct) => |
|
{ |
|
DateTimeOffset? latest = null; |
|
await foreach (var commit in ReadAllCommitsAsync(file, ct)) |
|
{ |
|
if (latest is null || commit.HybridDateTime.DateTime > latest) |
|
latest = commit.HybridDateTime.DateTime; |
|
allCommits.Add(commit); |
|
} |
|
if (latest is not null) |
|
heads[ClientIdForFile(file)] = latest.Value.ToUnixTimeMilliseconds(); |
|
}); |
Malformed / blank / truncated files — empty file yields nothing, blank lines are skipped, null deserialize is skipped. A crash mid-write leaves a truncated trailing line; no test confirms it is tolerated rather than making the whole client file unreadable.
|
private async IAsyncEnumerable<Commit> ReadAllCommitsAsync(FileInfo file, [EnumeratorCancellation] CancellationToken cancellationToken) |
|
{ |
|
if (!file.Exists || file.Length == 0) |
|
yield break; |
|
|
|
await using var stream = file.OpenRead(); |
|
using var reader = new StreamReader(stream); |
|
while (await reader.ReadLineAsync(cancellationToken) is { } line) |
|
{ |
|
cancellationToken.ThrowIfCancellationRequested(); |
|
if (string.IsNullOrWhiteSpace(line)) |
|
continue; |
|
var commit = JsonSerializer.Deserialize<Commit>(line, _serializerOptions); |
|
if (commit is not null) |
|
yield return commit; |
|
} |
|
} |
|
|
ClientIdForFile on a non-GUID filename — a stray client_*.jsonl with an unparseable id would throw from Guid.Parse inside the parallel loop; behavior is undefined by tests.
|
private static Guid ClientIdForFile(FileInfo clientIdFile) |
|
{ |
|
var id = clientIdFile.Name[FilenamePrefix.Length..^FilenameExtension.Length]; |
|
return Guid.Parse(id); |
|
} |
Content round-trip — no test reads a commit back and verifies ChangeEntities / HybridDateTime / Metadata survived; a field-drop regression passes today.
Suggested tests
AddRange_MultipleClients_WritesSeparateFilesAndAggregatesSyncState
AddRangeThenGetChanges_RoundTripsCommitContent
ReadAllCommits_SkipsBlankLinesAndTruncatedTrailingLine
GetSyncState_IgnoresOrHandlesUnparseableClientFile
Why it matters
JSONL files are the on-disk sync format. Real projects have many clients (fan-out is load-bearing) and interrupted writes are expected in practice; today a serialization or truncation regression would go undetected.
Follow-up from the test-suite review (batches merged as #92–#96). Existing
JsonSyncabletests assert only file existence / line count and use a single client id, so the multi-client format behavior and error tolerance are untested.Gaps
Per-client file fan-out — commits are grouped by
ClientIdinto separateclient_<id>.jsonlfiles, andGetSyncStateaggregates heads across them. Every test uses oneClientId, so this is unexercised.harmony/src/SIL.Harmony/JsonSyncable.cs
Lines 50 to 78 in 03f609f
Malformed / blank / truncated files — empty file yields nothing, blank lines are skipped, null deserialize is skipped. A crash mid-write leaves a truncated trailing line; no test confirms it is tolerated rather than making the whole client file unreadable.
harmony/src/SIL.Harmony/JsonSyncable.cs
Lines 116 to 133 in 03f609f
ClientIdForFileon a non-GUID filename — a strayclient_*.jsonlwith an unparseable id would throw fromGuid.Parseinside the parallel loop; behavior is undefined by tests.harmony/src/SIL.Harmony/JsonSyncable.cs
Lines 110 to 114 in 03f609f
Content round-trip — no test reads a commit back and verifies
ChangeEntities/HybridDateTime/Metadatasurvived; a field-drop regression passes today.Suggested tests
AddRange_MultipleClients_WritesSeparateFilesAndAggregatesSyncStateAddRangeThenGetChanges_RoundTripsCommitContentReadAllCommits_SkipsBlankLinesAndTruncatedTrailingLineGetSyncState_IgnoresOrHandlesUnparseableClientFileWhy it matters
JSONL files are the on-disk sync format. Real projects have many clients (fan-out is load-bearing) and interrupted writes are expected in practice; today a serialization or truncation regression would go undetected.