diff --git a/src/SIL.Harmony.Tests/ChangeConverterTests.cs b/src/SIL.Harmony.Tests/ChangeConverterTests.cs index 039252b..9f413e8 100644 --- a/src/SIL.Harmony.Tests/ChangeConverterTests.cs +++ b/src/SIL.Harmony.Tests/ChangeConverterTests.cs @@ -1,6 +1,7 @@ using System.Text.Json; using Microsoft.Extensions.DependencyInjection; using SIL.Harmony.Changes; +using SIL.Harmony.Config; using SIL.Harmony.Sample; using SIL.Harmony.Sample.Changes; @@ -8,9 +9,10 @@ namespace SIL.Harmony.Tests; public class ChangeConverterTests { - private static JsonSerializerOptions SampleOptions() => + private static JsonSerializerOptions SampleOptions(UnknownChangeHandling handling = UnknownChangeHandling.Fallback) => new ServiceCollection() .AddCrdtDataSample(":memory:") + .Configure(c => c.UnknownChangeHandling = handling) .BuildServiceProvider() .GetRequiredService(); @@ -45,6 +47,20 @@ public void Unknown_type_deserializes_to_OpaqueChange() opaque.RawJson.GetProperty("Priority").GetInt32().Should().Be(7); opaque.SupportsNewEntity().Should().BeFalse(); opaque.SupportsApplyChange().Should().BeFalse(); + opaque.EntityType.Should().BeNull(); + } + + [Fact] + public void Unknown_type_throws_when_fallback_disabled() + { + var options = SampleOptions(UnknownChangeHandling.Throw); + var json = """ + {"$type":"SetWordPriorityChange","EntityId":"aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee","Priority":7} + """; + + var act = () => JsonSerializer.Deserialize(json, options); + + act.Should().Throw().WithMessage("*SetWordPriorityChange*"); } [Fact] diff --git a/src/SIL.Harmony.Tests/DataModelPerformanceTests.cs b/src/SIL.Harmony.Tests/DataModelPerformanceTests.cs index 5eca65c..8a76489 100644 --- a/src/SIL.Harmony.Tests/DataModelPerformanceTests.cs +++ b/src/SIL.Harmony.Tests/DataModelPerformanceTests.cs @@ -17,12 +17,13 @@ namespace SIL.Harmony.Tests; [Trait("Category", "Performance")] public class DataModelPerformanceTests(ITestOutputHelper output) { - [Fact] - public void AddingChangePerformance() - { + [Fact( #if DEBUG - Assert.Fail("This test is disabled in debug builds, not reliable"); + Skip = "This test is disabled in debug builds, not reliable" #endif + )] + public void AddingChangePerformance() + { var summary = BenchmarkRunner.Run( ManualConfig.CreateEmpty() diff --git a/src/SIL.Harmony/Changes/Change.cs b/src/SIL.Harmony/Changes/Change.cs index c816403..68ecc5b 100644 --- a/src/SIL.Harmony/Changes/Change.cs +++ b/src/SIL.Harmony/Changes/Change.cs @@ -15,7 +15,7 @@ public interface IChange Guid EntityId { get; set; } [JsonIgnore] - Type EntityType { get; } + Type? EntityType { get; } ValueTask ApplyChange(IObjectBase entity, IChangeContext context); ValueTask NewEntity(Commit commit, IChangeContext context); diff --git a/src/SIL.Harmony/Changes/OpaqueChange.cs b/src/SIL.Harmony/Changes/OpaqueChange.cs index 2a33314..c09a44b 100644 --- a/src/SIL.Harmony/Changes/OpaqueChange.cs +++ b/src/SIL.Harmony/Changes/OpaqueChange.cs @@ -13,8 +13,7 @@ public sealed class OpaqueChange : IChange public Guid EntityId { get; set; } - public Type EntityType => - throw new NotSupportedException($"Opaque change '{TypeName}' has no known entity type."); + public Type? EntityType => null; public ValueTask ApplyChange(IObjectBase entity, IChangeContext context) => default; diff --git a/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs b/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs index b129b07..0c13bed 100644 --- a/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs +++ b/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs @@ -2,6 +2,7 @@ using System.Text.Json; using System.Text.Json.Serialization; using System.Text.Json.Serialization.Metadata; +using SIL.Harmony.Config; namespace SIL.Harmony.Changes; @@ -9,17 +10,20 @@ 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. +/// an unknown discriminator either throws or falls back to +/// (preserving the raw payload), depending on . /// internal sealed class PeekThenConcreteChangeConverter : JsonConverter { private readonly KnownType[] _known; private readonly byte[] _discriminatorPropertyUtf8; + private readonly UnknownChangeHandling _unknownChangeHandling; - public PeekThenConcreteChangeConverter(IReadOnlyDictionary known) + public PeekThenConcreteChangeConverter(IReadOnlyDictionary known, UnknownChangeHandling unknownChangeHandling) { _discriminatorPropertyUtf8 = Encoding.UTF8.GetBytes(CrdtConstants.ChangeDiscriminatorProperty); _known = known.Select(kv => new KnownType(Encoding.UTF8.GetBytes(kv.Key), kv.Value)).ToArray(); + _unknownChangeHandling = unknownChangeHandling; } public override IChange Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) @@ -42,6 +46,10 @@ public override IChange Read(ref Utf8JsonReader reader, Type typeToConvert, Json if (!TryFindKnown(ref reader, out var knownIndex, out var unknownTypeName)) { + if (_unknownChangeHandling != UnknownChangeHandling.Fallback) + throw new JsonException( + $"Unknown IChange \"{CrdtConstants.ChangeDiscriminatorProperty}\" discriminator '{unknownTypeName}'"); + reader = checkpoint; return ReadOpaque(ref reader, unknownTypeName!); } diff --git a/src/SIL.Harmony/Config/HarmonyConfig.cs b/src/SIL.Harmony/Config/HarmonyConfig.cs index 65556aa..1e34850 100644 --- a/src/SIL.Harmony/Config/HarmonyConfig.cs +++ b/src/SIL.Harmony/Config/HarmonyConfig.cs @@ -21,6 +21,12 @@ public class HarmonyConfig /// after adding any commit validate the commit history, not great for performance but good for testing. /// public bool AlwaysValidateCommits { get; set; } = true; + /// + /// Controls how an unknown $type is handled during deserialization. + /// Defaults to ; set to + /// to preserve unknown changes as . + /// + public UnknownChangeHandling UnknownChangeHandling { get; set; } = UnknownChangeHandling.Throw; public ChangeTypeListBuilder ChangeTypeListBuilder { get; } = new(); public IReadOnlyList ChangeTypes => ChangeTypeListBuilder.Types; public ObjectTypeListBuilder ObjectTypeListBuilder { get; } = new(); @@ -38,17 +44,25 @@ public HarmonyConfig() private JsonSerializerOptions CreateJsonSerializerOptions() { - var changeDiscriminators = _lazyChangeDiscriminatorMaps.Value; - var options = new JsonSerializerOptions(JsonSerializerDefaults.General) - { - TypeInfoResolver = MakeJsonTypeResolver() - }; - options.Converters.Add(new PeekThenConcreteChangeConverter(changeDiscriminators.ByDiscriminator)); - _jsonOptionsBuilder.ApplyTo(options); + var options = new JsonSerializerOptions(JsonSerializerDefaults.General); + ConfigureExternalJsonOptions(options); return options; } + /// + /// Configures for Harmony's serialization of and types. + /// Also applies any callbacks registered via . + /// + public void ConfigureExternalJsonOptions(JsonSerializerOptions options) + { + var changeDiscriminators = _lazyChangeDiscriminatorMaps.Value; + options.TypeInfoResolver = options.TypeInfoResolver?.WithAddedModifier(MakeJsonTypeModifier()) + ?? MakeJsonTypeResolver(); + options.Converters.Add(new PeekThenConcreteChangeConverter(changeDiscriminators.ByDiscriminator, UnknownChangeHandling)); + _jsonOptionsBuilder.ApplyTo(options); + } + /// /// Registers a callback to customize before they are frozen. /// Callbacks run after Harmony's type resolver and change converter are configured. diff --git a/src/SIL.Harmony/Config/UnknownChangeHandling.cs b/src/SIL.Harmony/Config/UnknownChangeHandling.cs new file mode 100644 index 0000000..925349c --- /dev/null +++ b/src/SIL.Harmony/Config/UnknownChangeHandling.cs @@ -0,0 +1,13 @@ +namespace SIL.Harmony.Config; + +/// +/// Controls how PeekThenConcreteChangeConverter handles an +/// whose $type is not registered on this client. +/// +public enum UnknownChangeHandling +{ + /// Throw a on an unknown $type (default). + Throw, + /// Fall back to , preserving the raw JSON so it round-trips. + Fallback, +}