ServerCommit no longer enforces required properties on JSON deserialization (regression rc.241 → rc.266)

[hahn-kev-bot]

Summary

Between SIL.Harmony.Core 0.2.1-rc.241 and 0.2.1-rc.266, ServerCommit stopped enforcing its required init properties during System.Text.Json deserialization. A JSON payload that omits clientId (and even hybridDateTime) now deserializes successfully, silently defaulting ClientId to Guid.Empty, where it previously threw a JsonException.

ClientId identifies the originating client of a CRDT commit, so accepting Guid.Empty is a data-integrity concern for any endpoint that deserializes commits from clients.

Impact

Downstream, lexbox's CRDT sync endpoint POST /api/crdt/{projectId}/add deserializes ServerCommit[] straight from the request body. With rc.266 a malformed or outdated client can upload commits with a missing/empty clientId and the server will accept them instead of rejecting the request.

Root cause

ClientId is unchanged — still public required Guid ClientId { get; init; } (non-nullable, RequiredMemberAttribute present) in both versions. The only relevant IL difference is that rc.266 adds a parameterless constructor to ServerCommit:

rc.241  ServerCommit ctors: [JsonConstructor] (Guid id, HybridDateTime hybridDateTime); (Guid id)
rc.266  ServerCommit ctors: [JsonConstructor] (Guid id, HybridDateTime hybridDateTime); (Guid id); ()   <-- new parameterless ctor

With a parameterless constructor available, STJ constructs the object and sets init properties afterward, and it stops reporting/enforcing the required init property ClientId. This shows up both in JsonSchemaExporter output and in actual deserialization.

Reproduction

Run STJ's schema exporter and a deserialization against each package version's SIL.Harmony.Core.dll:

JsonSchemaExporter.GetJsonSchemaAsNode(opts, typeof(ServerCommit))["required"]

• rc.241 → ["Id","HybridDateTime","ClientId"]
• rc.266 → ["Id","HybridDateTime"]

Deserializing a payload with clientId omitted:

{"id":"11111111-1111-1111-1111-111111111111","hybridDateTime":{...},"changeEntities":[]}

• rc.241 → throws JsonException: ... missing required properties including: 'ClientId'
• rc.266 → succeeds, ClientId == Guid.Empty

(Note: rc.266 also no longer enforces HybridDateTime, yet the exported schema still lists it as required — the schema and the enforcement have diverged.)

Expected

Deserializing a ServerCommit that omits a required property (e.g. clientId) should fail, as it did in rc.241. If the parameterless constructor is needed (e.g. for EF/materialization), it shouldn't come at the cost of required-property enforcement during JSON deserialization.