Tolerate unknown IChange $type via PeekThenConcreteChangeConverter

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<JsonSerializerOptions>();
+
+    [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<IChange>(json, options);
+
+        roundTripped.Should().BeOfType<SetWordTextChange>()
+            .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<IChange>(json, options);
+
+        var opaque = change.Should().BeOfType<OpaqueChange>().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<IChange>(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<IChange>
+        {
+            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<Commit>(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<IChange>(json, options);
+        act.Should().Throw<JsonException>().WithMessage("*first property*");
+    }
+}

src/SIL.Harmony/Changes/Change.cs

@@ -3,7 +3,12 @@
 
 namespace SIL.Harmony.Changes;
 
-[JsonPolymorphic(TypeDiscriminatorPropertyName = CrdtConstants.ChangeDiscriminatorProperty)]
+/// <summary>
+/// Polymorphic JSON for <see cref="IChange"/> is owned by
+/// <c>PeekThenConcreteChangeConverter</c> (via <see cref="CrdtConfig"/>), not
+/// <see cref="JsonPolymorphicAttribute"/>. Unknown <c>$type</c> values become
+/// <see cref="OpaqueChange"/>.
+/// </summary>
 public interface IChange
 {
     [JsonIgnore]

src/SIL.Harmony/Changes/OpaqueChange.cs

@@ -0,0 +1,27 @@
+using System.Text.Json;
+
+namespace SIL.Harmony.Changes;
+
+/// <summary>
+/// An <see cref="IChange"/> whose <c>$type</c> was not registered on this client.
+/// Preserves the original JSON so it can round-trip and be applied once the type is known.
+/// </summary>
+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<IObjectBase> 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;
+}

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;
+
+/// <summary>
+/// Owns <see cref="IChange"/> discrimination. Requires <c>$type</c> as the first JSON property
+/// (matching synthetic write order from <see cref="CrdtConfig"/>).
+/// Known discriminators deserialize via cached concrete <see cref="JsonTypeInfo"/>;
+/// unknown → <see cref="OpaqueChange"/> preserving the raw payload.
+/// </summary>
+internal sealed class PeekThenConcreteChangeConverter : JsonConverter<IChange>
+{
+    private readonly KnownType[] _known;
+    private readonly byte[] _discriminatorPropertyUtf8;
+
+    public PeekThenConcreteChangeConverter(IReadOnlyDictionary<string, Type> 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(nameof(IChange.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;
+        }
+    }
+}

src/SIL.Harmony/CrdtConfig.cs

@@ -27,15 +27,49 @@ public class CrdtConfig
     public IEnumerable<Type> ObjectTypes => ObjectTypeListBuilder.AdapterProviders.SelectMany(p => p.GetRegistrations().Select(r => r.ObjectDbType));
     public JsonSerializerOptions JsonSerializerOptions => _lazyJsonSerializerOptions.Value;
     private readonly Lazy<JsonSerializerOptions> _lazyJsonSerializerOptions;
+    private readonly Lazy<ChangeDiscriminatorMaps> _lazyChangeDiscriminatorMaps;
 
     public CrdtConfig()
     {
-        _lazyJsonSerializerOptions = new Lazy<JsonSerializerOptions>(() => new JsonSerializerOptions(JsonSerializerDefaults.General)
+        _lazyChangeDiscriminatorMaps = new Lazy<ChangeDiscriminatorMaps>(BuildChangeDiscriminatorMaps);
+        _lazyJsonSerializerOptions = new Lazy<JsonSerializerOptions>(CreateJsonSerializerOptions);
+    }
+
+    private JsonSerializerOptions CreateJsonSerializerOptions()
+    {
+        var changeDiscriminators = _lazyChangeDiscriminatorMaps.Value;
+
+        var options = new JsonSerializerOptions(JsonSerializerDefaults.General)
         {
             TypeInfoResolver = MakeJsonTypeResolver()
-        });
+        };
+        options.Converters.Add(new PeekThenConcreteChangeConverter(changeDiscriminators.ByDiscriminator));
+        return options;
     }
 
+    private ChangeDiscriminatorMaps BuildChangeDiscriminatorMaps()
+    {
+        ChangeTypeListBuilder.Freeze();
+
+        var knownChanges = new Dictionary<string, Type>(ChangeTypeListBuilder.Types.Count);
+        var discriminators = new Dictionary<Type, string>(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);
+            discriminators.Add(derived.DerivedType, discriminator);
+        }
+
+        return new ChangeDiscriminatorMaps(knownChanges, discriminators);
+    }
+
+    private sealed record ChangeDiscriminatorMaps(
+        IReadOnlyDictionary<string, Type> ByDiscriminator,
+        IReadOnlyDictionary<Type, string> ByType);
+
     public Action<JsonTypeInfo> MakeJsonTypeModifier()
     {
         return JsonTypeModifier;
@@ -53,12 +87,13 @@ private void JsonTypeModifier(JsonTypeInfo typeInfo)
     {
         ChangeTypeListBuilder.Freeze();
         ObjectTypeListBuilder.Freeze();
-        if (typeInfo.Type == typeof(IChange))
+        var changeTypeDiscriminators = _lazyChangeDiscriminatorMaps.Value.ByType;
+
+        // IChange polymorphism is owned by PeekThenConcreteChangeConverter — do not set PolymorphismOptions.
+        if (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 +106,19 @@ private void JsonTypeModifier(JsonTypeInfo typeInfo)
         }
     }
 
+    /// <summary>
+    /// Serialize-only <c>$type</c> on concrete change types so write stays a plain concrete serialize
+    /// (converter Write does not inject the discriminator). Order forces <c>$type</c> first for the read path.
+    /// </summary>
+    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");

src/SIL.Harmony/SnapshotWorker.cs

@@ -74,6 +74,12 @@ private async ValueTask ApplyCommitChanges(SortedSet<Commit> commits)
 
                 if (prevSnapshot is null)
                 {
+                    if (commitChange.Change is OpaqueChange)
+                    {
+                        // 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);
                 }