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/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; +} diff --git a/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs b/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs new file mode 100644 index 0000000..765df42 --- /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(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; + } + } +} diff --git a/src/SIL.Harmony/CrdtConfig.cs b/src/SIL.Harmony/CrdtConfig.cs index 2ae0895..087c9e3 100644 --- a/src/SIL.Harmony/CrdtConfig.cs +++ b/src/SIL.Harmony/CrdtConfig.cs @@ -27,15 +27,49 @@ 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 readonly Lazy _lazyChangeDiscriminatorMaps; public CrdtConfig() { - _lazyJsonSerializerOptions = new Lazy(() => new JsonSerializerOptions(JsonSerializerDefaults.General) + _lazyChangeDiscriminatorMaps = new Lazy(BuildChangeDiscriminatorMaps); + _lazyJsonSerializerOptions = new Lazy(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(ChangeTypeListBuilder.Types.Count); + var discriminators = 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); + discriminators.Add(derived.DerivedType, discriminator); + } + + return new ChangeDiscriminatorMaps(knownChanges, discriminators); + } + + private sealed record ChangeDiscriminatorMaps( + IReadOnlyDictionary ByDiscriminator, + IReadOnlyDictionary ByType); + public Action 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) } } + /// + /// 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..c79e66d 100644 --- a/src/SIL.Harmony/SnapshotWorker.cs +++ b/src/SIL.Harmony/SnapshotWorker.cs @@ -74,6 +74,12 @@ private async ValueTask ApplyCommitChanges(SortedSet 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); }