From 2398c3c4beb327515822299a3bc559608f76bd48 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 19:12:23 +0000 Subject: [PATCH 1/3] Enforce required members when deserializing commits [SetsRequiredMembers] on the [JsonConstructor] ctors (added in e500757) made System.Text.Json skip required-member checks, so a ServerCommit without clientId or hybridDateTime deserialized with defaults. Fixes #104. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017iyZdteks1z5KpAsAAVMNt --- src/SIL.Harmony.Core/CommitBase.cs | 3 -- src/SIL.Harmony.Core/ServerCommit.cs | 2 - src/SIL.Harmony.Tests/ServerCommitTests.cs | 51 ++++++++++++++++++++++ src/SIL.Harmony/Commit.cs | 2 - 4 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 src/SIL.Harmony.Tests/ServerCommitTests.cs diff --git a/src/SIL.Harmony.Core/CommitBase.cs b/src/SIL.Harmony.Core/CommitBase.cs index 8ea08fe..c29deb9 100644 --- a/src/SIL.Harmony.Core/CommitBase.cs +++ b/src/SIL.Harmony.Core/CommitBase.cs @@ -1,4 +1,3 @@ -using System.Diagnostics.CodeAnalysis; using System.IO.Hashing; using System.Text.Json.Serialization; @@ -12,7 +11,6 @@ public abstract class CommitBase : IComparable { public const string NullParentHash = "0000"; [JsonConstructor] - [SetsRequiredMembers] protected internal CommitBase(Guid id, HybridDateTime hybridDateTime) { Id = id; @@ -71,7 +69,6 @@ public int CompareTo(CommitBase? other) /// public abstract class CommitBase : CommitBase { - [SetsRequiredMembers] internal CommitBase(Guid id, HybridDateTime hybridDateTime) : base(id, hybridDateTime) { } diff --git a/src/SIL.Harmony.Core/ServerCommit.cs b/src/SIL.Harmony.Core/ServerCommit.cs index 5c9fc2e..b87a5c0 100644 --- a/src/SIL.Harmony.Core/ServerCommit.cs +++ b/src/SIL.Harmony.Core/ServerCommit.cs @@ -1,4 +1,3 @@ -using System.Diagnostics.CodeAnalysis; using System.Runtime.Serialization; using System.Text.Json; using System.Text.Json.Serialization; @@ -8,7 +7,6 @@ namespace SIL.Harmony.Core; public class ServerCommit : CommitBase { [JsonConstructor] - [SetsRequiredMembers] protected ServerCommit(Guid id, HybridDateTime hybridDateTime) : base(id, hybridDateTime) { diff --git a/src/SIL.Harmony.Tests/ServerCommitTests.cs b/src/SIL.Harmony.Tests/ServerCommitTests.cs new file mode 100644 index 0000000..1265d2d --- /dev/null +++ b/src/SIL.Harmony.Tests/ServerCommitTests.cs @@ -0,0 +1,51 @@ +using System.Text.Json; +using SIL.Harmony.Core; + +namespace SIL.Harmony.Tests; + +public class ServerCommitTests +{ + // ASP.NET Core's defaults, which sync servers use to read uploaded commits + private static readonly JsonSerializerOptions WebOptions = new(JsonSerializerDefaults.Web); + + private const string ValidJson = """ + { + "id": "11111111-1111-1111-1111-111111111111", + "clientId": "22222222-2222-2222-2222-222222222222", + "hybridDateTime": { "dateTime": "2026-01-01T00:00:00+00:00", "counter": 3 }, + "changeEntities": [ + { + "index": 0, + "commitId": "11111111-1111-1111-1111-111111111111", + "entityId": "33333333-3333-3333-3333-333333333333", + "change": { "$type": "SomeChange", "text": "hello" } + } + ] + } + """; + + [Fact] + public void CanRoundTripThroughJson() + { + var commit = JsonSerializer.Deserialize(ValidJson, WebOptions)!; + commit.ClientId.Should().Be(Guid.Parse("22222222-2222-2222-2222-222222222222")); + commit.HybridDateTime.Counter.Should().Be(3); + + var json = JsonSerializer.Serialize(commit, WebOptions); + var roundTripped = JsonSerializer.Deserialize(json, WebOptions); + JsonSerializer.Serialize(roundTripped, WebOptions).Should().Be(json); + } + + [Theory] + [InlineData("clientId")] + [InlineData("hybridDateTime")] + public void RejectsMissingRequiredProperty(string property) + { + var json = JsonSerializer.Deserialize>(ValidJson)!; + json.Remove(property); + + var act = () => JsonSerializer.Deserialize(JsonSerializer.Serialize(json), WebOptions); + + act.Should().Throw().WithMessage($"*{property}*"); + } +} diff --git a/src/SIL.Harmony/Commit.cs b/src/SIL.Harmony/Commit.cs index 715648c..ce54a0a 100644 --- a/src/SIL.Harmony/Commit.cs +++ b/src/SIL.Harmony/Commit.cs @@ -1,4 +1,3 @@ -using System.Diagnostics.CodeAnalysis; using System.Text.Json.Serialization; using SIL.Harmony.Changes; using SIL.Harmony.Db; @@ -8,7 +7,6 @@ namespace SIL.Harmony; public class Commit : CommitBase { [JsonConstructor] - [SetsRequiredMembers] protected Commit(Guid id, string hash, string parentHash, HybridDateTime hybridDateTime) : base(id, hybridDateTime) { From dbb82247b10a91f01d361f90918674ff25d89688 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 14:21:35 +0000 Subject: [PATCH 2/3] Move ServerCommitTests next to the other Core type tests Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017iyZdteks1z5KpAsAAVMNt --- src/SIL.Harmony.Tests/{ => Core}/ServerCommitTests.cs | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/SIL.Harmony.Tests/{ => Core}/ServerCommitTests.cs (100%) diff --git a/src/SIL.Harmony.Tests/ServerCommitTests.cs b/src/SIL.Harmony.Tests/Core/ServerCommitTests.cs similarity index 100% rename from src/SIL.Harmony.Tests/ServerCommitTests.cs rename to src/SIL.Harmony.Tests/Core/ServerCommitTests.cs From 132be789882266fd02474457390dbe6e8285b81e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 14:21:56 +0000 Subject: [PATCH 3/3] Use the Core test namespace for ServerCommitTests Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017iyZdteks1z5KpAsAAVMNt --- src/SIL.Harmony.Tests/Core/ServerCommitTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SIL.Harmony.Tests/Core/ServerCommitTests.cs b/src/SIL.Harmony.Tests/Core/ServerCommitTests.cs index 1265d2d..eab7411 100644 --- a/src/SIL.Harmony.Tests/Core/ServerCommitTests.cs +++ b/src/SIL.Harmony.Tests/Core/ServerCommitTests.cs @@ -1,7 +1,7 @@ using System.Text.Json; using SIL.Harmony.Core; -namespace SIL.Harmony.Tests; +namespace SIL.Harmony.Tests.Core; public class ServerCommitTests {