From 2a73b38ec6b9d8ebd7621dec1ccfb617fb98b774 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 21 Jul 2026 10:11:00 +0200 Subject: [PATCH] Defer change-type freeze until JsonSerializerOptions is first used PeekThenConcreteChangeConverter took the discriminator map eagerly, so building the options froze ChangeTypeListBuilder. Consumers that read config.JsonSerializerOptions mid-configuration (to add their own TypeInfoResolver modifier) and then register more change types hit "ChangeTypeListBuilder is frozen". Pass the map as a factory so the converter resolves it on first use, restoring the pre-#80 freeze timing. Co-Authored-By: Claude Opus 4.8 --- src/SIL.Harmony.Tests/ConfigTests.cs | 23 +++++++++++++++++++ .../PeekThenConcreteChangeConverter.cs | 14 ++++++----- src/SIL.Harmony/CrdtConfig.cs | 7 +++--- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/SIL.Harmony.Tests/ConfigTests.cs b/src/SIL.Harmony.Tests/ConfigTests.cs index e8ca40d..3fdc2c5 100644 --- a/src/SIL.Harmony.Tests/ConfigTests.cs +++ b/src/SIL.Harmony.Tests/ConfigTests.cs @@ -1,3 +1,5 @@ +using System.Text.Json; +using SIL.Harmony.Changes; using SIL.Harmony.Sample.Changes; using SIL.Harmony.Sample.Models; using SIL.Harmony.Tests.Adapter; @@ -29,4 +31,25 @@ public void CanGetChangeTypes() var types = config.ChangeTypes.ToArray(); types.Should().BeEquivalentTo([typeof(NewDefinitionChange), typeof(SetWordTextChange)]); } + + [Fact] + public void CanAddChangeTypesAfterReadingJsonSerializerOptions() + { + // Mirrors a consumer that reads JsonSerializerOptions mid-configuration (to layer its own + // TypeInfoResolver modifier onto ours) and then keeps registering change types. + var config = new CrdtConfig(); + config.ChangeTypeListBuilder.Add(); + + _ = config.JsonSerializerOptions; + + config.ChangeTypeListBuilder.Add(); + + var options = config.JsonSerializerOptions; + var entityId = Guid.Parse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"); + var json = JsonSerializer.Serialize(new SetWordTextChange(entityId, "hello"), options); + + var roundTripped = JsonSerializer.Deserialize(json, options); + roundTripped.Should().BeOfType() + .Which.Text.Should().Be("hello"); + } } diff --git a/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs b/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs index 765df42..473b7f2 100644 --- a/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs +++ b/src/SIL.Harmony/Changes/PeekThenConcreteChangeConverter.cs @@ -13,13 +13,14 @@ namespace SIL.Harmony.Changes; /// internal sealed class PeekThenConcreteChangeConverter : JsonConverter { - private readonly KnownType[] _known; + private readonly Lazy _known; private readonly byte[] _discriminatorPropertyUtf8; - public PeekThenConcreteChangeConverter(IReadOnlyDictionary known) + public PeekThenConcreteChangeConverter(Func> knownFactory) { _discriminatorPropertyUtf8 = Encoding.UTF8.GetBytes(CrdtConstants.ChangeDiscriminatorProperty); - _known = known.Select(kv => new KnownType(Encoding.UTF8.GetBytes(kv.Key), kv.Value)).ToArray(); + _known = new Lazy(() => + knownFactory().Select(kv => new KnownType(Encoding.UTF8.GetBytes(kv.Key), kv.Value)).ToArray()); } public override IChange Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) @@ -46,7 +47,7 @@ public override IChange Read(ref Utf8JsonReader reader, Type typeToConvert, Json return ReadOpaque(ref reader, unknownTypeName!); } - ref var known = ref _known[knownIndex]; + ref var known = ref _known.Value[knownIndex]; var typeInfo = known.EnsureTypeInfo(options); // Real change types use parameterized constructors / get-only props — let STJ materialize. @@ -83,9 +84,10 @@ private static OpaqueChange ReadOpaque(ref Utf8JsonReader reader, string typeNam private bool TryFindKnown(ref Utf8JsonReader reader, out int index, out string? unknownTypeName) { - for (var i = 0; i < _known.Length; i++) + var known = _known.Value; + for (var i = 0; i < known.Length; i++) { - if (reader.ValueTextEquals(_known[i].Utf8Discriminator)) + if (reader.ValueTextEquals(known[i].Utf8Discriminator)) { index = i; unknownTypeName = null; diff --git a/src/SIL.Harmony/CrdtConfig.cs b/src/SIL.Harmony/CrdtConfig.cs index 01d9d13..9788019 100644 --- a/src/SIL.Harmony/CrdtConfig.cs +++ b/src/SIL.Harmony/CrdtConfig.cs @@ -39,13 +39,14 @@ public CrdtConfig() private JsonSerializerOptions CreateJsonSerializerOptions() { - var changeDiscriminators = _lazyChangeDiscriminatorMaps.Value; - var options = new JsonSerializerOptions(JsonSerializerDefaults.General) { TypeInfoResolver = MakeJsonTypeResolver() }; - options.Converters.Add(new PeekThenConcreteChangeConverter(changeDiscriminators.ByDiscriminator)); + // A factory, not .Value: building the map freezes ChangeTypeListBuilder, but consumers read these + // options mid-configuration (to add their own TypeInfoResolver modifier) and keep registering + // change types. The converter pulls the map on first use, once registration is done. + options.Converters.Add(new PeekThenConcreteChangeConverter(() => _lazyChangeDiscriminatorMaps.Value.ByDiscriminator)); return options; }