From 82778518af3ee67333f6137bd6e92ddf709b2cf4 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Fri, 10 Jul 2026 14:19:20 +0700 Subject: [PATCH 1/7] Add AGENTS.md and gitignore local agent skill files. Co-authored-by: Cursor --- .gitignore | 4 ++++ AGENTS.md | 13 +++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 AGENTS.md diff --git a/.gitignore b/.gitignore index 7361c97..e7c353e 100644 --- a/.gitignore +++ b/.gitignore @@ -402,3 +402,7 @@ FodyWeavers.xsd *.received.* *.lscache + +# Agent skills (local-only) +.scratch/ +docs/agents/ diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..6ec05a8 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,13 @@ +## Agent skills + +### Issue tracker + +Issues live as local markdown files under `.scratch/` (gitignored). See `docs/agents/issue-tracker.md`. + +### Triage labels + +Default triage role strings (`needs-triage`, `needs-info`, `ready-for-agent`, `ready-for-human`, `wontfix`). See `docs/agents/triage-labels.md`. + +### Domain docs + +Single-context layout — `CONTEXT.md` and `docs/adr/` at the repo root. See `docs/agents/domain.md`. From 1e7c02f22d9d1f4c49ed60edd69c476de75a254f Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Wed, 15 Jul 2026 13:10:39 +0700 Subject: [PATCH 2/7] Add OpaqueChange and drop JsonPolymorphic from IChange. Unknown $type values will be held as OpaqueChange once the converter owns discrimination. Co-authored-by: Cursor --- src/SIL.Harmony/Changes/Change.cs | 7 ++++++- src/SIL.Harmony/Changes/OpaqueChange.cs | 27 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 src/SIL.Harmony/Changes/OpaqueChange.cs diff --git a/src/SIL.Harmony/Changes/Change.cs b/src/SIL.Harmony/Changes/Change.cs index aad8a0b..8367b89 100644 --- a/src/SIL.Harmony/Changes/Change.cs +++ b/src/SIL.Harmony/Changes/Change.cs @@ -3,7 +3,12 @@ namespace SIL.Harmony.Changes; -[JsonPolymorphic(TypeDiscriminatorPropertyName = CrdtConstants.ChangeDiscriminatorProperty)] +/// +/// Polymorphic JSON for is owned by +/// PeekThenConcreteChangeConverter (via ), not +/// . Unknown $type values become +/// . +/// public interface IChange { [JsonIgnore] diff --git a/src/SIL.Harmony/Changes/OpaqueChange.cs b/src/SIL.Harmony/Changes/OpaqueChange.cs new file mode 100644 index 0000000..2a33314 --- /dev/null +++ b/src/SIL.Harmony/Changes/OpaqueChange.cs @@ -0,0 +1,27 @@ +using System.Text.Json; + +namespace SIL.Harmony.Changes; + +/// +/// An whose $type was not registered on this client. +/// Preserves the original JSON so it can round-trip and be applied once the type is known. +/// +public sealed class OpaqueChange : IChange +{ + public required string TypeName { get; init; } + public required JsonElement RawJson { get; init; } + + public Guid EntityId { get; set; } + + public Type EntityType => + throw new NotSupportedException($"Opaque change '{TypeName}' has no known entity type."); + + public ValueTask ApplyChange(IObjectBase entity, IChangeContext context) => default; + + public ValueTask NewEntity(Commit commit, IChangeContext context) => + throw new NotSupportedException( + $"Opaque change '{TypeName}' cannot create entities on this client. CommitId: {commit.Id}, EntityId: {EntityId}"); + + public bool SupportsApplyChange() => false; + public bool SupportsNewEntity() => false; +} From 2d008f7ab4a9bb655dc3dbec2f9c7f322df05e37 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Wed, 15 Jul 2026 13:13:04 +0700 Subject: [PATCH 3/7] Wire PeekThenConcreteChangeConverter for unknown $type tolerance. CrdtConfig owns IChange discrimination with synthetic $type on write; SnapshotWorker skips opaque creates. Co-authored-by: Cursor --- src/SIL.Harmony.Tests/ChangeConverterTests.cs | 109 ++++++++++++++++ .../PeekThenConcreteChangeConverter.cs | 122 ++++++++++++++++++ src/SIL.Harmony/CrdtConfig.cs | 59 ++++++++- src/SIL.Harmony/SnapshotWorker.cs | 7 +- 4 files changed, 289 insertions(+), 8 deletions(-) create mode 100644 src/SIL.Harmony.Tests/ChangeConverterTests.cs create mode 100644 src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs diff --git a/src/SIL.Harmony.Tests/ChangeConverterTests.cs b/src/SIL.Harmony.Tests/ChangeConverterTests.cs new file mode 100644 index 0000000..039252b --- /dev/null +++ b/src/SIL.Harmony.Tests/ChangeConverterTests.cs @@ -0,0 +1,109 @@ +using System.Text.Json; +using Microsoft.Extensions.DependencyInjection; +using SIL.Harmony.Changes; +using SIL.Harmony.Sample; +using SIL.Harmony.Sample.Changes; + +namespace SIL.Harmony.Tests; + +public class ChangeConverterTests +{ + private static JsonSerializerOptions SampleOptions() => + new ServiceCollection() + .AddCrdtDataSample(":memory:") + .BuildServiceProvider() + .GetRequiredService(); + + [Fact] + public void Happy_path_deserializes_to_concrete_change() + { + var options = SampleOptions(); + var entityId = Guid.Parse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"); + IChange change = new SetWordTextChange(entityId, "hello"); + + var json = JsonSerializer.Serialize(change, options); + var roundTripped = JsonSerializer.Deserialize(json, options); + + roundTripped.Should().BeOfType() + .Which.Text.Should().Be("hello"); + roundTripped!.EntityId.Should().Be(entityId); + json.Should().StartWith("{\"$type\":\"SetWordTextChange\""); + } + + [Fact] + public void Unknown_type_deserializes_to_OpaqueChange() + { + var options = SampleOptions(); + var json = """ + {"$type":"SetWordPriorityChange","EntityId":"aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee","Priority":7} + """; + + var change = JsonSerializer.Deserialize(json, options); + + var opaque = change.Should().BeOfType().Subject; + opaque.TypeName.Should().Be("SetWordPriorityChange"); + opaque.RawJson.GetProperty("Priority").GetInt32().Should().Be(7); + opaque.SupportsNewEntity().Should().BeFalse(); + opaque.SupportsApplyChange().Should().BeFalse(); + } + + [Fact] + public void OpaqueChange_round_trips_original_discriminator() + { + var options = SampleOptions(); + var json = """ + {"$type":"SetWordPriorityChange","EntityId":"aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee","Priority":7} + """; + + var change = JsonSerializer.Deserialize(json, options)!; + var rewritten = JsonSerializer.Serialize(change, options); + + rewritten.Should().Contain("\"$type\":\"SetWordPriorityChange\""); + rewritten.Should().Contain("\"Priority\":7"); + rewritten.Should().NotContain("OpaqueChange"); + } + + [Fact] + public void Mixed_commit_round_trips_known_and_opaque_changes() + { + var options = SampleOptions(); + var entityId = Guid.Parse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"); + var commit = new Commit + { + ClientId = Guid.NewGuid(), + HybridDateTime = new HybridDateTime(DateTimeOffset.UtcNow, 0), + }; + commit.ChangeEntities.Add(new ChangeEntity + { + Index = 0, + CommitId = commit.Id, + EntityId = entityId, + Change = new SetWordTextChange(entityId, "hello") + }); + + var json = JsonSerializer.Serialize(commit, options); + // Inject an unknown change as if from a newer client. + json = json.Replace( + "\"ChangeEntities\":[", + """ + "ChangeEntities":[{"Index":1,"CommitId":"00000000-0000-0000-0000-000000000000","EntityId":"aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee","Change":{"$type":"SetWordPriorityChange","Priority":3}}, + """); + + var roundTripped = JsonSerializer.Deserialize(json, options)!; + roundTripped.ChangeEntities.Should().HaveCount(2); + roundTripped.ChangeEntities.Select(c => c.Change.GetType()) + .Should().BeEquivalentTo([typeof(OpaqueChange), typeof(SetWordTextChange)]); + } + + [Fact] + public void Requires_type_as_first_property() + { + var options = SampleOptions(); + var json = """ + {"Text":"hello","EntityId":"aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee","$type":"SetWordTextChange"} + """; + + var act = () => JsonSerializer.Deserialize(json, options); + act.Should().Throw().WithMessage("*first property*"); + } +} diff --git a/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs b/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs new file mode 100644 index 0000000..2cfd76b --- /dev/null +++ b/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs @@ -0,0 +1,122 @@ +using System.Text; +using System.Text.Json; +using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; + +namespace SIL.Harmony.Changes; + +/// +/// Owns discrimination. Requires $type as the first JSON property +/// (matching synthetic write order from ). +/// Known discriminators deserialize via cached concrete ; +/// unknown → preserving the raw payload. +/// +internal sealed class PeekThenConcreteChangeConverter : JsonConverter +{ + private readonly KnownType[] _known; + private readonly byte[] _discriminatorPropertyUtf8; + + public PeekThenConcreteChangeConverter(IReadOnlyDictionary known) + { + _discriminatorPropertyUtf8 = Encoding.UTF8.GetBytes(CrdtConstants.ChangeDiscriminatorProperty); + _known = known.Select(kv => new KnownType(Encoding.UTF8.GetBytes(kv.Key), kv.Value)).ToArray(); + } + + public override IChange Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType != JsonTokenType.StartObject) + throw new JsonException("Expected StartObject"); + + // Checkpoint: restore for full-object deserialize / opaque capture after peeking $type. + var checkpoint = reader; + + if (!reader.Read() || reader.TokenType != JsonTokenType.PropertyName) + throw new JsonException("Expected property name"); + + if (!reader.ValueTextEquals(_discriminatorPropertyUtf8)) + throw new JsonException( + $"IChange requires \"{CrdtConstants.ChangeDiscriminatorProperty}\" as the first property"); + + if (!reader.Read() || reader.TokenType != JsonTokenType.String) + throw new JsonException($"Expected string {CrdtConstants.ChangeDiscriminatorProperty} discriminator"); + + if (!TryFindKnown(ref reader, out var knownIndex, out var unknownTypeName)) + { + reader = checkpoint; + return ReadOpaque(ref reader, unknownTypeName!); + } + + ref var known = ref _known[knownIndex]; + var typeInfo = known.EnsureTypeInfo(options); + + // Real change types use parameterized constructors / get-only props — let STJ materialize. + reader = checkpoint; + return (IChange)(JsonSerializer.Deserialize(ref reader, typeInfo) + ?? throw new JsonException($"null {known.ClrType.Name}")); + } + + public override void Write(Utf8JsonWriter writer, IChange value, JsonSerializerOptions options) + { + if (value is OpaqueChange opaque) + { + opaque.RawJson.WriteTo(writer); + return; + } + + // Concrete runtime type: synthetic $type comes from JsonTypeInfo modifier. + JsonSerializer.Serialize(writer, value, value.GetType(), options); + } + + private static OpaqueChange ReadOpaque(ref Utf8JsonReader reader, string typeName) + { + using var doc = JsonDocument.ParseValue(ref reader); + var element = doc.RootElement.Clone(); + return new OpaqueChange + { + TypeName = typeName, + EntityId = element.TryGetProperty("EntityId", out var id) && id.ValueKind == JsonValueKind.String + ? id.GetGuid() + : default, + RawJson = element + }; + } + + private bool TryFindKnown(ref Utf8JsonReader reader, out int index, out string? unknownTypeName) + { + for (var i = 0; i < _known.Length; i++) + { + if (reader.ValueTextEquals(_known[i].Utf8Discriminator)) + { + index = i; + unknownTypeName = null; + return true; + } + } + + index = -1; + unknownTypeName = reader.GetString(); + return false; + } + + private struct KnownType + { + public KnownType(byte[] utf8Discriminator, Type clrType) + { + Utf8Discriminator = utf8Discriminator; + ClrType = clrType; + } + + public byte[] Utf8Discriminator { get; } + public Type ClrType { get; } + private JsonTypeInfo? _typeInfo; + + public JsonTypeInfo EnsureTypeInfo(JsonSerializerOptions options) + { + if (_typeInfo is not null && ReferenceEquals(_typeInfo.Options, options)) + return _typeInfo; + + _typeInfo = options.GetTypeInfo(ClrType); + return _typeInfo; + } + } +} diff --git a/src/SIL.Harmony/CrdtConfig.cs b/src/SIL.Harmony/CrdtConfig.cs index 2ae0895..c6aabe9 100644 --- a/src/SIL.Harmony/CrdtConfig.cs +++ b/src/SIL.Harmony/CrdtConfig.cs @@ -27,13 +27,42 @@ public class CrdtConfig public IEnumerable ObjectTypes => ObjectTypeListBuilder.AdapterProviders.SelectMany(p => p.GetRegistrations().Select(r => r.ObjectDbType)); public JsonSerializerOptions JsonSerializerOptions => _lazyJsonSerializerOptions.Value; private readonly Lazy _lazyJsonSerializerOptions; + private Dictionary? _changeTypeDiscriminators; public CrdtConfig() { - _lazyJsonSerializerOptions = new Lazy(() => new JsonSerializerOptions(JsonSerializerDefaults.General) + _lazyJsonSerializerOptions = new Lazy(CreateJsonSerializerOptions); + } + + private JsonSerializerOptions CreateJsonSerializerOptions() + { + var knownChanges = BuildChangeDiscriminatorMaps(); + + var options = new JsonSerializerOptions(JsonSerializerDefaults.General) { TypeInfoResolver = MakeJsonTypeResolver() - }); + }; + options.Converters.Add(new PeekThenConcreteChangeConverter(knownChanges)); + return options; + } + + private Dictionary BuildChangeDiscriminatorMaps() + { + ChangeTypeListBuilder.Freeze(); + + var knownChanges = new Dictionary(ChangeTypeListBuilder.Types.Count); + _changeTypeDiscriminators = new Dictionary(ChangeTypeListBuilder.Types.Count); + foreach (var derived in ChangeTypeListBuilder.Types) + { + if (derived.TypeDiscriminator is not string discriminator) + throw new InvalidOperationException( + $"Change type {derived.DerivedType} must use a string $type discriminator"); + + knownChanges.Add(discriminator, derived.DerivedType); + _changeTypeDiscriminators.Add(derived.DerivedType, discriminator); + } + + return knownChanges; } public Action MakeJsonTypeModifier() @@ -53,12 +82,15 @@ private void JsonTypeModifier(JsonTypeInfo typeInfo) { ChangeTypeListBuilder.Freeze(); ObjectTypeListBuilder.Freeze(); - if (typeInfo.Type == typeof(IChange)) + if (_changeTypeDiscriminators is null) + BuildChangeDiscriminatorMaps(); + + // IChange polymorphism is owned by PeekThenConcreteChangeConverter — do not set PolymorphismOptions. + if (_changeTypeDiscriminators is not null + && typeInfo.Kind == JsonTypeInfoKind.Object + && _changeTypeDiscriminators.TryGetValue(typeInfo.Type, out var discriminator)) { - foreach (var type in ChangeTypeListBuilder.Types) - { - typeInfo.PolymorphismOptions!.DerivedTypes.Add(type); - } + AddSyntheticTypeDiscriminator(typeInfo, discriminator); } if (ObjectTypeListBuilder.JsonTypes?.TryGetValue(typeInfo.Type, out var types) == true) @@ -71,6 +103,19 @@ private void JsonTypeModifier(JsonTypeInfo typeInfo) } } + /// + /// Serialize-only $type on concrete change types so write stays a plain concrete serialize + /// (converter Write does not inject the discriminator). Order forces $type first for the read path. + /// + private static void AddSyntheticTypeDiscriminator(JsonTypeInfo typeInfo, string discriminator) + { + var typeName = discriminator; + var prop = typeInfo.CreateJsonPropertyInfo(typeof(string), CrdtConstants.ChangeDiscriminatorProperty); + prop.Get = _ => typeName; + prop.Order = int.MinValue; + typeInfo.Properties.Add(prop); + } + public bool RemoteResourcesEnabled { get; private set; } public string LocalResourceCachePath { get; set; } = Path.GetFullPath("./localResourceCache"); public string FailedSyncOutputPath { get; set; } = Path.GetFullPath("./failedSyncs"); diff --git a/src/SIL.Harmony/SnapshotWorker.cs b/src/SIL.Harmony/SnapshotWorker.cs index c4fe70f..3d58012 100644 --- a/src/SIL.Harmony/SnapshotWorker.cs +++ b/src/SIL.Harmony/SnapshotWorker.cs @@ -74,7 +74,12 @@ private async ValueTask ApplyCommitChanges(SortedSet commits) if (prevSnapshot is null) { - // create brand new entity - this will (and should) throw if the change doesn't support NewEntity + if (!commitChange.Change.SupportsNewEntity()) + { + // e.g. OpaqueChange / unknown create — keep the change in history, skip apply + continue; + } + entity = await commitChange.Change.NewEntity(commit, changeContext); } else if (prevSnapshot.EntityIsDeleted && commitChange.Change.SupportsNewEntity()) From 1503b336091537d3f979ff90160751a1c5b7671e Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 10:22:31 +0700 Subject: [PATCH 4/7] Build change discriminator map fully before publishing field. _changeTypeDiscriminators was assigned before its loop finished populating it, so a concurrent JsonTypeModifier callback (invoked lazily by STJ on arbitrary threads) could observe a partially-built dictionary. Build into a local and assign the field once, so readers only ever see null or a complete map. Co-Authored-By: Claude Opus 4.8 --- src/SIL.Harmony/CrdtConfig.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/SIL.Harmony/CrdtConfig.cs b/src/SIL.Harmony/CrdtConfig.cs index c6aabe9..1bfdd91 100644 --- a/src/SIL.Harmony/CrdtConfig.cs +++ b/src/SIL.Harmony/CrdtConfig.cs @@ -51,7 +51,7 @@ private Dictionary BuildChangeDiscriminatorMaps() ChangeTypeListBuilder.Freeze(); var knownChanges = new Dictionary(ChangeTypeListBuilder.Types.Count); - _changeTypeDiscriminators = new Dictionary(ChangeTypeListBuilder.Types.Count); + var discriminators = new Dictionary(ChangeTypeListBuilder.Types.Count); foreach (var derived in ChangeTypeListBuilder.Types) { if (derived.TypeDiscriminator is not string discriminator) @@ -59,9 +59,12 @@ private Dictionary BuildChangeDiscriminatorMaps() $"Change type {derived.DerivedType} must use a string $type discriminator"); knownChanges.Add(discriminator, derived.DerivedType); - _changeTypeDiscriminators.Add(derived.DerivedType, discriminator); + discriminators.Add(derived.DerivedType, discriminator); } + // Publish the field only once fully built: JsonTypeModifier reads it from arbitrary + // threads, so a partially-populated dictionary must never be observable. + _changeTypeDiscriminators = discriminators; return knownChanges; } From 77f711078d5d5f41a804b3a396e993c32ebbdd45 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 10:36:37 +0700 Subject: [PATCH 5/7] Revert "Add AGENTS.md and gitignore local agent skill files." This reverts commit 82778518af3ee67333f6137bd6e92ddf709b2cf4. --- .gitignore | 4 ---- AGENTS.md | 13 ------------- 2 files changed, 17 deletions(-) delete mode 100644 AGENTS.md diff --git a/.gitignore b/.gitignore index e7c353e..7361c97 100644 --- a/.gitignore +++ b/.gitignore @@ -402,7 +402,3 @@ FodyWeavers.xsd *.received.* *.lscache - -# Agent skills (local-only) -.scratch/ -docs/agents/ diff --git a/AGENTS.md b/AGENTS.md deleted file mode 100644 index 6ec05a8..0000000 --- a/AGENTS.md +++ /dev/null @@ -1,13 +0,0 @@ -## Agent skills - -### Issue tracker - -Issues live as local markdown files under `.scratch/` (gitignored). See `docs/agents/issue-tracker.md`. - -### Triage labels - -Default triage role strings (`needs-triage`, `needs-info`, `ready-for-agent`, `ready-for-human`, `wontfix`). See `docs/agents/triage-labels.md`. - -### Domain docs - -Single-context layout — `CONTEXT.md` and `docs/adr/` at the repo root. See `docs/agents/domain.md`. From 1b398e8ea3cafe70c312cd2b9c7a96e2fae641ce Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Fri, 17 Jul 2026 08:58:40 +0700 Subject: [PATCH 6/7] use name of instead of hard coding the string name Co-authored-by: Tim Haasdyk --- src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs b/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs index 2cfd76b..765df42 100644 --- a/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs +++ b/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs @@ -74,7 +74,7 @@ private static OpaqueChange ReadOpaque(ref Utf8JsonReader reader, string typeNam return new OpaqueChange { TypeName = typeName, - EntityId = element.TryGetProperty("EntityId", out var id) && id.ValueKind == JsonValueKind.String + EntityId = element.TryGetProperty(nameof(IChange.EntityId), out var id) && id.ValueKind == JsonValueKind.String ? id.GetGuid() : default, RawJson = element From 5f0a86993eb53f6de26404167858eb0a38234fe1 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Fri, 17 Jul 2026 09:11:33 +0700 Subject: [PATCH 7/7] Refine unknown change handling and initialization. Co-authored-by: Cursor --- src/SIL.Harmony/CrdtConfig.cs | 26 +++++++++++++------------- src/SIL.Harmony/SnapshotWorker.cs | 5 +++-- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/SIL.Harmony/CrdtConfig.cs b/src/SIL.Harmony/CrdtConfig.cs index 1bfdd91..087c9e3 100644 --- a/src/SIL.Harmony/CrdtConfig.cs +++ b/src/SIL.Harmony/CrdtConfig.cs @@ -27,26 +27,27 @@ public class CrdtConfig public IEnumerable ObjectTypes => ObjectTypeListBuilder.AdapterProviders.SelectMany(p => p.GetRegistrations().Select(r => r.ObjectDbType)); public JsonSerializerOptions JsonSerializerOptions => _lazyJsonSerializerOptions.Value; private readonly Lazy _lazyJsonSerializerOptions; - private Dictionary? _changeTypeDiscriminators; + private readonly Lazy _lazyChangeDiscriminatorMaps; public CrdtConfig() { + _lazyChangeDiscriminatorMaps = new Lazy(BuildChangeDiscriminatorMaps); _lazyJsonSerializerOptions = new Lazy(CreateJsonSerializerOptions); } private JsonSerializerOptions CreateJsonSerializerOptions() { - var knownChanges = BuildChangeDiscriminatorMaps(); + var changeDiscriminators = _lazyChangeDiscriminatorMaps.Value; var options = new JsonSerializerOptions(JsonSerializerDefaults.General) { TypeInfoResolver = MakeJsonTypeResolver() }; - options.Converters.Add(new PeekThenConcreteChangeConverter(knownChanges)); + options.Converters.Add(new PeekThenConcreteChangeConverter(changeDiscriminators.ByDiscriminator)); return options; } - private Dictionary BuildChangeDiscriminatorMaps() + private ChangeDiscriminatorMaps BuildChangeDiscriminatorMaps() { ChangeTypeListBuilder.Freeze(); @@ -62,12 +63,13 @@ private Dictionary BuildChangeDiscriminatorMaps() discriminators.Add(derived.DerivedType, discriminator); } - // Publish the field only once fully built: JsonTypeModifier reads it from arbitrary - // threads, so a partially-populated dictionary must never be observable. - _changeTypeDiscriminators = discriminators; - return knownChanges; + return new ChangeDiscriminatorMaps(knownChanges, discriminators); } + private sealed record ChangeDiscriminatorMaps( + IReadOnlyDictionary ByDiscriminator, + IReadOnlyDictionary ByType); + public Action MakeJsonTypeModifier() { return JsonTypeModifier; @@ -85,13 +87,11 @@ private void JsonTypeModifier(JsonTypeInfo typeInfo) { ChangeTypeListBuilder.Freeze(); ObjectTypeListBuilder.Freeze(); - if (_changeTypeDiscriminators is null) - BuildChangeDiscriminatorMaps(); + var changeTypeDiscriminators = _lazyChangeDiscriminatorMaps.Value.ByType; // IChange polymorphism is owned by PeekThenConcreteChangeConverter — do not set PolymorphismOptions. - if (_changeTypeDiscriminators is not null - && typeInfo.Kind == JsonTypeInfoKind.Object - && _changeTypeDiscriminators.TryGetValue(typeInfo.Type, out var discriminator)) + if (typeInfo.Kind == JsonTypeInfoKind.Object + && changeTypeDiscriminators.TryGetValue(typeInfo.Type, out var discriminator)) { AddSyntheticTypeDiscriminator(typeInfo, discriminator); } diff --git a/src/SIL.Harmony/SnapshotWorker.cs b/src/SIL.Harmony/SnapshotWorker.cs index 3d58012..c79e66d 100644 --- a/src/SIL.Harmony/SnapshotWorker.cs +++ b/src/SIL.Harmony/SnapshotWorker.cs @@ -74,12 +74,13 @@ private async ValueTask ApplyCommitChanges(SortedSet commits) if (prevSnapshot is null) { - if (!commitChange.Change.SupportsNewEntity()) + if (commitChange.Change is OpaqueChange) { - // e.g. OpaqueChange / unknown create — keep the change in history, skip apply + // Keep unknown changes in history until this client understands how to apply them. continue; } + // create brand new entity - this will (and should) throw if the change doesn't support NewEntity entity = await commitChange.Change.NewEntity(commit, changeContext); } else if (prevSnapshot.EntityIsDeleted && commitChange.Change.SupportsNewEntity())