Skip to content

[claude] Bump SIL.Harmony to rc.228 and fix config-time freeze #2463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
myieye wants to merge 1 commit into from
Closed

[claude] Bump SIL.Harmony to rc.228 and fix config-time freeze #2463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions backend/Directory.Packages.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@
<PackageVersion Include="SIL.Chorus.LibChorus" Version="6.0.0-beta0064" />
<PackageVersion Include="SIL.Chorus.Mercurial" Version="6.5.1.43" />
<PackageVersion Include="SIL.ChorusPlugin.LfMergeBridge" Version="4.3.0-beta0048" />
<PackageVersion Include="SIL.Harmony" Version="0.2.1-rc.211" />
<PackageVersion Include="SIL.Harmony.Core" Version="0.2.1-rc.211" />
<PackageVersion Include="SIL.Harmony.Linq2db" Version="0.2.1-rc.211" />
<PackageVersion Include="SIL.Harmony" Version="0.2.1-rc.228" />
<PackageVersion Include="SIL.Harmony.Core" Version="0.2.1-rc.228" />
<PackageVersion Include="SIL.Harmony.Linq2db" Version="0.2.1-rc.228" />
<PackageVersion Include="SIL.Core" Version="17.0.0" />
<PackageVersion Include="SIL.LCModel" Version="11.0.0-beta0165" />
<PackageVersion Include="SIL.LCModel.FixData" Version="11.0.0-beta0165" />
Expand Down
20 changes: 19 additions & 1 deletion backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Runtime.CompilerServices;
using System.Text.Json;
using System.Text.Json.Serialization.Metadata;
using FluentAssertions.Execution;
Expand Down Expand Up @@ -74,7 +75,24 @@ private IEnumerable<JsonDerivedType> GetPolymorphicTypesFor(Type type)
[Trait("Category", "Verified")]
public async Task VerifyChangeModels()
{
await Verify(GetPolymorphicTypesFor(typeof(IChange)));
await Verify(GetChangePolymorphicTypes());
}

// IChange polymorphism is no longer expressed through PolymorphismOptions; each concrete change
// type instead carries a synthetic "$type" property (read by Harmony's change converter), so read
// the discriminator from there to snapshot the registered change set.
private IEnumerable<JsonDerivedType> GetChangePolymorphicTypes()
{
return LcmCrdtKernel.AllChangeTypes()
.Select(changeType =>
{
var discriminatorProperty = _jsonSerializerOptions.GetTypeInfo(changeType).Properties
.SingleOrDefault(p => p.Name == CrdtConstants.ChangeDiscriminatorProperty);
discriminatorProperty.Should().NotBeNull("change type {0} should declare a $type discriminator", changeType);
var discriminator = discriminatorProperty!.Get!(RuntimeHelpers.GetUninitializedObject(changeType));

@hahn-kev hahn-kev Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably make a helper function that does this, as I think we've done this in other places already and each one is a little different.

return new JsonDerivedType(changeType, (string)discriminator!);
})
.OrderBy(t => t.DerivedType.FullName);
}

[Fact]
Expand Down
1 change: 1 addition & 0 deletions backend/FwLite/LcmCrdt.Tests/TestJsonOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public static JsonSerializerOptions Harmony()
{
var config = new CrdtConfig();
LcmCrdtKernel.ConfigureCrdt(config);
LcmCrdtKernel.FinalizeJsonSerializerOptions(config);
return config.JsonSerializerOptions;
}
}
18 changes: 15 additions & 3 deletions backend/FwLite/LcmCrdt/LcmCrdtKernel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ public static IServiceCollection AddLcmCrdtClientCore(this IServiceCollection se
config => ConfigureCrdt(config, false)//don't add remote resources because they are added in AddCrdtRemoteResources
);
services.AddCrdtRemoteResources<LcmFileMetadata>();
// AddCrdtRemoteResources registers its change types in a Configure callback, so finalize the
// JSON options in a PostConfigure (which runs after every Configure) — see FinalizeJsonSerializerOptions.
services.AddOptions<CrdtConfig>().PostConfigure(FinalizeJsonSerializerOptions);
services.AddOptions<CrdtConfig>().PostConfigure((CrdtConfig crdtConfig, IOptions<LcmCrdtConfig> lcmConfig) =>
{
crdtConfig.LocalResourceCachePath = Path.Combine(lcmConfig.Value.ProjectPath, "localResourcesCache");
Expand Down Expand Up @@ -395,12 +398,21 @@ public static void ConfigureCrdt(CrdtConfig config, bool addRemoteResourceEntity
// you must add an instance of it to UseChangesTests.GetAllChanges()
;

if (addRemoteResourceEntity)
config.AddRemoteResourceEntity<LcmFileMetadata>();
}

// Adds the legacy ExampleSentence.Translation modifier to Harmony's JSON options. Reading
// config.JsonSerializerOptions builds the lazily-created options and freezes ChangeTypeListBuilder,
// so this must run only once every change type is registered. It's therefore kept out of
// ConfigureCrdt and run once the config is complete (a PostConfigure in DI, see AddLcmCrdtClientCore;
// right before reading the options for standalone callers, see TestJsonOptions.Harmony).
// Call once per config: it wraps the existing resolver, so a second call double-adds the modifier.
internal static void FinalizeJsonSerializerOptions(CrdtConfig config)
{
config.JsonSerializerOptions.TypeInfoResolver =
(config.JsonSerializerOptions.TypeInfoResolver ?? new DefaultJsonTypeInfoResolver())
.WithAddedModifier(Json.ExampleSentenceTranslationModifier);

if (addRemoteResourceEntity)
config.AddRemoteResourceEntity<LcmFileMetadata>();
}

public static IEnumerable<Type> AllChangeTypes()
Expand Down
Loading