From 0df02ca0bdbcb1043866256e199de52c01bb873f Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 28 Oct 2025 13:13:59 +0100 Subject: [PATCH 01/14] Don't generate redundant multi-string values --- .../AutoFakerHelpers/MultiStringOverride.cs | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/MultiStringOverride.cs b/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/MultiStringOverride.cs index 469fe09be..bfb65bd32 100644 --- a/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/MultiStringOverride.cs +++ b/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/MultiStringOverride.cs @@ -1,4 +1,3 @@ -using MiniLcm.Models; using Soenneker.Utils.AutoBogus.Context; using Soenneker.Utils.AutoBogus.Override; @@ -9,15 +8,18 @@ public class MultiStringOverride(string[]? validWs = null): AutoFakerOverride false; public override void Generate(AutoFakerOverrideContext context) { - var target = context.Instance as MultiString; - if (target is null) + if (context.Instance is not MultiString target) { - context.Instance = target = new MultiString(); + context.Instance = target = []; } - var wordsArray = context.Faker.Random.WordsArray(1, 4); - foreach (var word in wordsArray) + var validWritingSystems = validWs ?? WritingSystemCodes.ValidTwoLetterCodes; + if (validWritingSystems.Length == 0) throw new ArgumentException("validWs cannot be an empty array"); + + var count = context.Faker.Random.Int(1, Math.Min(4, validWritingSystems.Length)); + var writingSystems = context.Faker.Random.ListItems(validWritingSystems, count); + foreach (var writingSystemId in writingSystems) { - var writingSystemId = context.Faker.Random.ArrayElement(validWs ?? WritingSystemCodes.ValidTwoLetterCodes); + var word = context.Faker.Random.Word(); target[writingSystemId] = word; } } @@ -29,14 +31,18 @@ public class RichMultiStringOverride(string[]? validWs = null): AutoFakerOverrid public override void Generate(AutoFakerOverrideContext context) { - var target = context.Instance as RichMultiString; - if (target is null) + if (context.Instance is not RichMultiString target) { - context.Instance = target = new RichMultiString(); + context.Instance = target = []; } - for (int i = 0; i < context.Faker.Random.Int(1, 4); i++) + + var validWritingSystems = validWs ?? WritingSystemCodes.ValidTwoLetterCodes; + if (validWritingSystems.Length == 0) throw new ArgumentException("validWs cannot be an empty array"); + + var count = context.Faker.Random.Int(1, Math.Min(4, validWritingSystems.Length)); + var writingSystems = context.Faker.Random.ListItems(validWritingSystems, count); + foreach (var writingSystemId in writingSystems) { - var writingSystemId = context.Faker.Random.ArrayElement(validWs ?? WritingSystemCodes.ValidTwoLetterCodes); var wordsArray = context.Faker.Random.WordsArray(1, 4); var spans = new List(); foreach (var word in wordsArray) From 329696f8ab727ae2589206f21b8c41324e49a8de Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 28 Oct 2025 13:16:36 +0100 Subject: [PATCH 02/14] Stablize fwdata complex-form and component headwords --- backend/FwLite/FwDataMiniLcmBridge/Api/LcmHelpers.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/FwLite/FwDataMiniLcmBridge/Api/LcmHelpers.cs b/backend/FwLite/FwDataMiniLcmBridge/Api/LcmHelpers.cs index b50e2bbdb..e3c77375b 100644 --- a/backend/FwLite/FwDataMiniLcmBridge/Api/LcmHelpers.cs +++ b/backend/FwLite/FwDataMiniLcmBridge/Api/LcmHelpers.cs @@ -15,7 +15,7 @@ internal static class LcmHelpers { var citationFormTs = ws.HasValue ? entry.CitationForm.get_String(ws.Value) - : entry.CitationForm.StringCount > 0 ? entry.CitationForm.GetStringFromIndex(0, out var _) + : entry.CitationForm.StringCount > 0 ? entry.CitationForm.BestVernacularAlternative : null; var citationForm = citationFormTs?.Text?.Trim(WhitespaceChars); @@ -23,14 +23,14 @@ internal static class LcmHelpers var lexemeFormTs = ws.HasValue ? entry.LexemeFormOA?.Form.get_String(ws.Value) - : entry.LexemeFormOA?.Form.StringCount > 0 ? entry.LexemeFormOA?.Form.GetStringFromIndex(0, out var _) + : entry.LexemeFormOA?.Form.StringCount > 0 ? entry.LexemeFormOA?.Form.BestVernacularAlternative : null; var lexemeForm = lexemeFormTs?.Text?.Trim(WhitespaceChars); return lexemeForm; } - internal static string? LexEntryHeadwordOrUnknown(this ILexEntry entry, int? ws = null) + internal static string LexEntryHeadwordOrUnknown(this ILexEntry entry, int? ws = null) { var headword = entry.LexEntryHeadword(ws); return string.IsNullOrEmpty(headword) ? Entry.UnknownHeadword : headword; From 99f7da75d51aa54be72b0c42b13e81d123e84fc3 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 28 Oct 2025 13:24:32 +0100 Subject: [PATCH 03/14] Make CanSyncRandomEntries robuster - Runs against FwApi as well - Include moves and positional creates (components, example, complex-forms) - Round-trip data through each api so it's realistic --- .../FwLiteProjectSync.Tests/EntrySyncTests.cs | 202 +++++++++++++++--- .../Fixtures/SyncFixture.cs | 67 +++--- .../AutoFakerHelpers/EntryFakerHelper.cs | 133 +++++++----- 3 files changed, 286 insertions(+), 116 deletions(-) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs index fc45494c1..f4d2ef9d0 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs @@ -1,5 +1,7 @@ using System.Text; +using FwDataMiniLcmBridge.Api; using FwLiteProjectSync.Tests.Fixtures; +using LcmCrdt; using MiniLcm; using MiniLcm.Models; using MiniLcm.SyncHelpers; @@ -8,52 +10,48 @@ namespace FwLiteProjectSync.Tests; -public class CrdtEntrySyncTests(SyncFixture fixture) : EntrySyncTestsBase(fixture) +public class CrdtEntrySyncTests(ExtraWritingSystemsSyncFixture fixture) : EntrySyncTestsBase(fixture) { - private static readonly AutoFaker AutoFaker = new(AutoFakerDefault.Config); - protected override IMiniLcmApi GetApi(SyncFixture fixture) { return fixture.CrdtApi; } - - [Fact] - public async Task CanSyncRandomEntries() - { - var createdEntry = await Api.CreateEntry(await AutoFaker.EntryReadyForCreation(Api)); - var after = await AutoFaker.EntryReadyForCreation(Api, entryId: createdEntry.Id); - - after.Senses = [.. AutoFaker.Faker.Random.Shuffle([ - // copy some senses over, so moves happen - ..AutoFaker.Faker.Random.ListItems(createdEntry.Senses), - ..after.Senses - ])]; - - await EntrySync.SyncFull(createdEntry, after, Api); - var actual = await Api.GetEntry(after.Id); - actual.Should().NotBeNull(); - actual.Should().BeEquivalentTo(after, options => options - .For(e => e.Senses).Exclude(s => s.Order) - .For(e => e.Components).Exclude(c => c.Order) - .For(e => e.ComplexForms).Exclude(c => c.Order) - .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(e => e.Order) - ); - } } -public class FwDataEntrySyncTests(SyncFixture fixture) : EntrySyncTestsBase(fixture) +public class FwDataEntrySyncTests(ExtraWritingSystemsSyncFixture fixture) : EntrySyncTestsBase(fixture) { protected override IMiniLcmApi GetApi(SyncFixture fixture) { return fixture.FwDataApi; } + + // this will notify us when we start syncing MorphType (if that ever happens) + [Fact] + public async Task FwDataApiDoesNotUpdateMorphType() + { + // arrange + var entry = await Api.CreateEntry(new() + { + LexemeForm = { { "en", "morph-type-test" } }, + MorphType = MorphType.BoundStem + }); + + // act + var updatedEntry = entry.Copy(); + updatedEntry.MorphType = MorphType.Suffix; + await EntrySync.SyncFull(entry, updatedEntry, Api); + + // assert + var actual = await Api.GetEntry(entry.Id); + actual.Should().NotBeNull(); + actual.MorphType.Should().Be(MorphType.BoundStem); + } } -public abstract class EntrySyncTestsBase(SyncFixture fixture) : IClassFixture, IAsyncLifetime +public abstract class EntrySyncTestsBase(ExtraWritingSystemsSyncFixture fixture) : IClassFixture, IAsyncLifetime { public async Task InitializeAsync() { - await _fixture.EnsureDefaultVernacularWritingSystemExistsInCrdt(); Api = GetApi(_fixture); } @@ -67,6 +65,152 @@ public Task DisposeAsync() private readonly SyncFixture _fixture = fixture; protected IMiniLcmApi Api = null!; + private static readonly AutoFaker AutoFaker = new(AutoFakerDefault.MakeConfig( + ExtraWritingSystemsSyncFixture.VernacularWritingSystems)); + + public enum ApiType + { + Crdt, + FwData + } + + // Not all of these cases are realistic, but they should all work + // An especially critical case is syncing data to crdt that has been round-tripped through fwdata first. + [Theory] + [InlineData(ApiType.Crdt)] + [InlineData(ApiType.FwData)] + [InlineData(null)] + public async Task CanSyncRandomEntries(ApiType? roundTripApiType) + { + // arrange + var currentApiType = Api switch + { + FwDataMiniLcmApi => ApiType.FwData, + CrdtMiniLcmApi => ApiType.Crdt, + _ => throw new InvalidOperationException("Unknown API type") + }; + + IMiniLcmApi? roundTripApi = roundTripApiType switch + { + ApiType.Crdt => _fixture.CrdtApi, + ApiType.FwData => _fixture.FwDataApi, + _ => null + }; + + var before = AutoFaker.Generate(); + var after = AutoFaker.Generate(); + after.Id = before.Id; + + await Api.PrepareToCreateEntry(before); + await Api.PrepareToCreateEntry(after); + + if (roundTripApi is not null && currentApiType != roundTripApiType) + { + // we have to prepare before we start mixing parts of before into after + // otherwise "PrepareToCreateEntry" would fail due to trying to create duplicate related entities + await roundTripApi.PrepareToCreateEntry(before); + await roundTripApi.PrepareToCreateEntry(after); + } + + after.Senses = [.. + // shuffle to cause moves + AutoFaker.Faker.Random.Shuffle([.. + // keep some, remove others + AutoFaker.Faker.Random.ListItems(before.Senses).Select(createdSense => + { + var copy = createdSense.Copy(); + copy.ExampleSentences = [.. + // shuffle to cause moves + AutoFaker.Faker.Random.Shuffle([.. + // keep some, remove others + AutoFaker.Faker.Random.ListItems(copy.ExampleSentences), + // add new + AutoFaker.ExampleSentence(copy), + AutoFaker.ExampleSentence(copy), + ])]; + return copy; + }), + // keep new + ..after.Senses + ])]; + + after.ComplexForms = [.. + // shuffle to cause moves + AutoFaker.Faker.Random.Shuffle([.. + // keep some, remove others + AutoFaker.Faker.Random.ListItems(before.ComplexForms).Select(createdCfc => + { + var copy = createdCfc.Copy(); + copy.ComponentHeadword = after.Headword(); + return copy; + }), + // keep new + ..after.ComplexForms + ])]; + + after.Components = [.. + // shuffle to cause moves + AutoFaker.Faker.Random.Shuffle([.. + // keep some, remove others + AutoFaker.Faker.Random.ListItems(before.Components).Select(createdCfc => + { + var copy = createdCfc.Copy(); + copy.ComplexFormHeadword = after.Headword(); + return copy; + }), + // keep new + ..after.Components + ])]; + + // expected should not be round-tripped, because an api might manipulate it somehow. + // We expect final result to be equivalent to this "raw"/untouched, requested state. + var expected = after; + + if (roundTripApi is not null) + { + // round-tripping ensures we're dealing with realistic data + // (e.g. in fwdata ComplexFormComponents do not have an Id) + before = await roundTripApi.CreateEntry(before); + await roundTripApi.DeleteEntry(before.Id); + after = await roundTripApi.CreateEntry(after); + await roundTripApi.DeleteEntry(after.Id); + } + + // before should not be round-tripped here. That's handled above. + await Api.CreateEntry(before); + + // act + await EntrySync.SyncFull(before, after, Api); + var actual = await Api.GetEntry(after.Id); + + // assert + actual.Should().NotBeNull(); + actual.Should().BeEquivalentTo(after, options => + { + options = options + .WithStrictOrdering() + .WithoutStrictOrderingFor(e => e.ComplexForms) // sorted alphabetically + .WithoutStrictOrderingFor(e => e.Path.EndsWith($".{nameof(Sense.SemanticDomains)}")) // not sorted + .For(e => e.Senses).Exclude(s => s.Order) + .For(e => e.Components).Exclude(c => c.Order) + .For(e => e.ComplexForms).Exclude(c => c.Order) + .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(e => e.Order); + if (currentApiType == ApiType.Crdt) + { + // does not yet update Headwords + options = options + .For(e => e.Components).Exclude(c => c.ComplexFormHeadword) + .For(e => e.ComplexForms).Exclude(c => c.ComponentHeadword); + } + if (currentApiType == ApiType.FwData) + { + // does not support changing MorphType yet (see UpdateEntryProxy.MorphType) + options = options.Excluding(e => e.MorphType); + } + return options; + }); + } + [Fact] public async Task NormalizesStringsToNFD() { diff --git a/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs b/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs index ca61b83ab..1a5e3b939 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs @@ -4,12 +4,45 @@ using FwDataMiniLcmBridge.LcmUtils; using LcmCrdt; using LexCore.Utils; -using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using MiniLcm.Models; namespace FwLiteProjectSync.Tests.Fixtures; +public class ExtraWritingSystemsSyncFixture : SyncFixture +{ + private static readonly string[] ExtraVernacularWritingSystems = ["es", "fr"]; + public static readonly string[] VernacularWritingSystems = [ + DefaultVernacularWritingSystem, + .. ExtraVernacularWritingSystems, + ]; + + public override async Task InitializeAsync() + { + await base.InitializeAsync(); + + foreach (var ws in ExtraVernacularWritingSystems) + { + await FwDataApi.CreateWritingSystem(new WritingSystem + { + Id = Guid.NewGuid(), + WsId = ws, + Name = ws, + Abbreviation = ws, + Font = "Arial", + Type = WritingSystemType.Vernacular + }); + } + + // Crdt data doesn't strictly require writing-systems to exist in order for them to be used. + // However, a (default) vernacular writing system is required in order to query for entries. + // This is not part of SyncFixture, because our core sync integration tests benefit from having a CRDT project that's as empty as possible. + var firstVernacularWs = (await FwDataApi.GetWritingSystems()).Vernacular.First(); + await CrdtApi.CreateWritingSystem(firstVernacularWs); + } +} + public class SyncFixture : IAsyncLifetime { private readonly AsyncServiceScope _services; @@ -17,6 +50,7 @@ public class SyncFixture : IAsyncLifetime public CrdtFwdataProjectSyncService SyncService => _services.ServiceProvider.GetRequiredService(); public IServiceProvider Services => _services.ServiceProvider; + protected static readonly string DefaultVernacularWritingSystem = "en"; private readonly string _projectName; private readonly string _projectFolder; private readonly IDisposable _cleanup; @@ -40,7 +74,7 @@ public SyncFixture() : this("sena-3_" + Guid.NewGuid().ToString().Split("-")[0], { } - public async Task InitializeAsync() + public virtual async Task InitializeAsync() { lock (_preCleanupLock) { @@ -59,7 +93,7 @@ public async Task InitializeAsync() Directory.CreateDirectory(projectsFolder); var fwDataProject = new FwDataProject(_projectName, projectsFolder); _services.ServiceProvider.GetRequiredService() - .NewProject(fwDataProject, "en", "en"); + .NewProject(fwDataProject, "en", DefaultVernacularWritingSystem); FwDataApi = _services.ServiceProvider.GetRequiredService().GetFwDataMiniLcmApi(fwDataProject, false); var crdtProjectsFolder = @@ -68,7 +102,6 @@ public async Task InitializeAsync() var crdtProject = await _services.ServiceProvider.GetRequiredService() .CreateProject(new(_projectName, _projectName, FwProjectId: FwDataApi.ProjectId, SeedNewProjectData: false)); CrdtApi = (CrdtMiniLcmApi)await _services.ServiceProvider.OpenCrdtProject(crdtProject); - } public async Task DisposeAsync() @@ -85,30 +118,4 @@ public void DeleteSyncSnapshot() var snapshotPath = CrdtFwdataProjectSyncService.SnapshotPath(FwDataApi.Project); if (File.Exists(snapshotPath)) File.Delete(snapshotPath); } - - private readonly SemaphoreSlim _vernacularSemaphore = new(1, 1); - - // a vernacular writing system is required in order to query for entries - // this is optional setup, because our core sync integration tests benefit from having a CRDT project that's as empty as possible - public async Task EnsureDefaultVernacularWritingSystemExistsInCrdt() - { - // This is optionally called from tests that consume this fixture, so it could get called multiple times in parallel - if (!await _vernacularSemaphore.WaitAsync(100)) - { - throw new InvalidOperationException("Timeout waiting for vernacular semaphore"); - } - - try - { - if ((await CrdtApi.GetWritingSystems()).Vernacular.Length == 0) - { - var firstVernacularWs = (await FwDataApi.GetWritingSystems()).Vernacular.First(); - await CrdtApi.CreateWritingSystem(firstVernacularWs); - } - } - finally - { - _vernacularSemaphore.Release(); - } - } } diff --git a/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/EntryFakerHelper.cs b/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/EntryFakerHelper.cs index 94fc6f396..c6ae31cbe 100644 --- a/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/EntryFakerHelper.cs +++ b/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/EntryFakerHelper.cs @@ -1,4 +1,3 @@ -using MiniLcm.Models; using Soenneker.Utils.AutoBogus; namespace MiniLcm.Tests.AutoFakerHelpers; @@ -15,6 +14,18 @@ public static async Task EntryReadyForCreation(this AutoFaker autoFaker, { var entry = autoFaker.Generate(); if (entryId.HasValue) entry.Id = entryId.Value; + await PrepareToCreateEntry(api, entry, createComplexForms, createComplexFormTypes, createComponents, createPublications); + return entry; + } + + public static async Task PrepareToCreateEntry( + this IMiniLcmApi api, + Entry entry, + bool createComplexForms = true, + bool createComplexFormTypes = true, + bool createComponents = true, + bool createPublications = true) + { if (createComponents) await CreateComplexFormComponentEntry(entry, true, entry.Components, api); if (createComplexForms) await CreateComplexFormComponentEntry(entry, false, entry.ComplexForms, api); if (createComplexFormTypes) await CreateComplexFormTypes(entry.ComplexFormTypes, api); @@ -47,75 +58,83 @@ public static async Task EntryReadyForCreation(this AutoFaker autoFaker, exampleSentence.SenseId = sense.Id; } } - return entry; - static async Task CreateComplexFormComponentEntry(Entry entry, + } + + public static ExampleSentence ExampleSentence(this AutoFaker autoFaker, Sense sense) + { + var exampleSentence = autoFaker.Generate(); + exampleSentence.SenseId = sense.Id; + return exampleSentence; + } + + private static async Task CreateComplexFormComponentEntry(Entry entry, bool isComponent, IList complexFormComponents, IMiniLcmApi api) + { + int i = 1; + foreach (var complexFormComponent in complexFormComponents) { - int i = 1; - foreach (var complexFormComponent in complexFormComponents) + //generated entries won't have the expected ids, so fix them up here + if (isComponent) { - //generated entries won't have the expected ids, so fix them up here - if (isComponent) - { - complexFormComponent.ComplexFormEntryId = entry.Id; - } - else - { - complexFormComponent.ComponentEntryId = entry.Id; - complexFormComponent.ComponentSenseId = null; - } + complexFormComponent.ComplexFormEntryId = entry.Id; + } + else + { + complexFormComponent.ComponentEntryId = entry.Id; + complexFormComponent.ComponentSenseId = null; + } - var name = $"test {(isComponent ? "component" : "complex form")} {i}"; - var createdEntry = await api.CreateEntry(new() - { - Id = isComponent - ? complexFormComponent.ComponentEntryId - : complexFormComponent.ComplexFormEntryId, - LexemeForm = { { "en", name } }, - Senses = - [ - ..complexFormComponent.ComponentSenseId.HasValue && - isComponent - ? - [ - new Sense - { - Id = complexFormComponent.ComponentSenseId.Value, Gloss = { { "en", name } } - } - ] - : (ReadOnlySpan) [] - ] - }); - if (isComponent) - { - complexFormComponent.ComponentHeadword = createdEntry.Headword(); - complexFormComponent.ComplexFormHeadword = entry.Headword(); - complexFormComponent.Order = i++; - } else - { - complexFormComponent.ComplexFormHeadword = createdEntry.Headword(); - complexFormComponent.ComponentHeadword = entry.Headword(); - complexFormComponent.Order = 1; - } + var name = $"test {(isComponent ? "component" : "complex form")} {i}"; + var createdEntry = await api.CreateEntry(new() + { + Id = isComponent + ? complexFormComponent.ComponentEntryId + : complexFormComponent.ComplexFormEntryId, + LexemeForm = { { "en", name } }, + Senses = + [ + ..complexFormComponent.ComponentSenseId.HasValue && + isComponent + ? + [ + new Sense + { + Id = complexFormComponent.ComponentSenseId.Value, Gloss = { { "en", name } } + } + ] + : (ReadOnlySpan) [] + ] + }); + if (isComponent) + { + complexFormComponent.ComponentHeadword = createdEntry.Headword(); + complexFormComponent.ComplexFormHeadword = entry.Headword(); + complexFormComponent.Order = i++; + } + else + { + complexFormComponent.ComplexFormHeadword = createdEntry.Headword(); + complexFormComponent.ComponentHeadword = entry.Headword(); + complexFormComponent.Order = 1; } } + } - static async Task CreateComplexFormTypes(IList complexFormTypes, IMiniLcmApi api) + private static async Task CreateComplexFormTypes(IList complexFormTypes, IMiniLcmApi api) + { + foreach (var complexFormType in complexFormTypes) { - foreach (var complexFormType in complexFormTypes) - { - await api.CreateComplexFormType(complexFormType); - } + await api.CreateComplexFormType(complexFormType); } + } - static async Task CreatePublications(IList publications, IMiniLcmApi api) + private static async Task CreatePublications(IList publications, IMiniLcmApi api) + { + foreach (var publication in publications) { - foreach (var publication in publications) - { - await api.CreatePublication(publication); - } + await api.CreatePublication(publication); } } } From 910316afe0dd3ce5539aab3157f24385dc26fceb Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 28 Oct 2025 13:34:53 +0100 Subject: [PATCH 04/14] Add GitHub link to headword todo --- backend/FwLite/MiniLcm/Models/Entry.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/FwLite/MiniLcm/Models/Entry.cs b/backend/FwLite/MiniLcm/Models/Entry.cs index b3bf75fe0..41a0d8601 100644 --- a/backend/FwLite/MiniLcm/Models/Entry.cs +++ b/backend/FwLite/MiniLcm/Models/Entry.cs @@ -35,6 +35,7 @@ public string Headword() { //order by code to ensure the headword is stable //todo choose ws by preference based on ws order/default + //https://github.com/sillsdev/languageforge-lexbox/issues/1284 var word = CitationForm.Values.OrderBy(kvp => kvp.Key.Code).FirstOrDefault().Value; if (string.IsNullOrEmpty(word)) word = LexemeForm.Values.OrderBy(kvp => kvp.Key.Code).FirstOrDefault().Value; return word?.Trim() ?? UnknownHeadword; From ac0b5f3fafe95aeda26fe647a19900c1c9a5b875 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 28 Oct 2025 13:35:59 +0100 Subject: [PATCH 05/14] Expand comments --- .../FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs index f4d2ef9d0..2a7c49caf 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs @@ -87,6 +87,8 @@ public async Task CanSyncRandomEntries(ApiType? roundTripApiType) { FwDataMiniLcmApi => ApiType.FwData, CrdtMiniLcmApi => ApiType.Crdt, + // This works now, because we're not currently wrapping Api, + // but if we ever do, then we want this to throw, so we know we need to detect the api differently. _ => throw new InvalidOperationException("Unknown API type") }; @@ -106,8 +108,10 @@ public async Task CanSyncRandomEntries(ApiType? roundTripApiType) if (roundTripApi is not null && currentApiType != roundTripApiType) { - // we have to prepare before we start mixing parts of before into after - // otherwise "PrepareToCreateEntry" would fail due to trying to create duplicate related entities + // We have to prepare while before and after have no overlap (i.e. before we start mixing parts of before into after), + // otherwise "PrepareToCreateEntry" would fail due to trying to create duplicate related entities. + // After this we can't ADD anything to after that has dependencies + // (e.g. ExampleSentences are fine, because they're created as part of an entry, but Parts of speech aren't) await roundTripApi.PrepareToCreateEntry(before); await roundTripApi.PrepareToCreateEntry(after); } @@ -197,7 +201,7 @@ public async Task CanSyncRandomEntries(ApiType? roundTripApiType) .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(e => e.Order); if (currentApiType == ApiType.Crdt) { - // does not yet update Headwords + // does not yet update Headwords 😕 options = options .For(e => e.Components).Exclude(c => c.ComplexFormHeadword) .For(e => e.ComplexForms).Exclude(c => c.ComponentHeadword); From f14514a85f72c102ba6ed552553fcb6cf86d00ef Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 28 Oct 2025 13:44:48 +0100 Subject: [PATCH 06/14] Remove todo --- backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs index a11ece856..6e817abf2 100644 --- a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs +++ b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs @@ -337,7 +337,6 @@ public async Task MoveComplexFormComponent(ComplexFormComponent component, Betwe public async Task DeleteComplexFormComponent(ComplexFormComponent complexFormComponent) { - // todo test missing ID (i.e. from LibLCM) await using var repo = await repoFactory.CreateRepoAsync(); var existing = await repo.FindComplexFormComponent(complexFormComponent); if (existing is null) return; From 63e35dc885dd0a5d8694fcd4724e54b278f822ec Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 28 Oct 2025 15:15:21 +0100 Subject: [PATCH 07/14] Add another comment mentioning poor headword handling --- backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs b/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs index 1a5e3b939..a89c4a3d2 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs @@ -13,6 +13,9 @@ namespace FwLiteProjectSync.Tests.Fixtures; public class ExtraWritingSystemsSyncFixture : SyncFixture { private static readonly string[] ExtraVernacularWritingSystems = ["es", "fr"]; + // "en", "es", "fr" = sorted alphabetically. + // Otherwise, headwords would differ between fwdata and crdt. + // See: https://github.com/sillsdev/languageforge-lexbox/issues/1284 public static readonly string[] VernacularWritingSystems = [ DefaultVernacularWritingSystem, .. ExtraVernacularWritingSystems, From 5d507574f9066a3d3a63c27167bee43d5ffd9fb1 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 28 Oct 2025 17:32:19 +0100 Subject: [PATCH 08/14] Fix async syntax warning --- backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs index 2a7c49caf..bf7a202ca 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs @@ -50,9 +50,10 @@ public async Task FwDataApiDoesNotUpdateMorphType() public abstract class EntrySyncTestsBase(ExtraWritingSystemsSyncFixture fixture) : IClassFixture, IAsyncLifetime { - public async Task InitializeAsync() + public Task InitializeAsync() { Api = GetApi(_fixture); + return Task.CompletedTask; } public Task DisposeAsync() From a5a14e8d89dd4274ef26f1ae3e396d1a06db71b3 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Wed, 29 Oct 2025 10:49:54 +0100 Subject: [PATCH 09/14] Make expected a clone to ensure it's stable. --- backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs index bf7a202ca..816d92b5f 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs @@ -168,8 +168,8 @@ public async Task CanSyncRandomEntries(ApiType? roundTripApiType) ])]; // expected should not be round-tripped, because an api might manipulate it somehow. - // We expect final result to be equivalent to this "raw"/untouched, requested state. - var expected = after; + // We expect the final result to be equivalent to this "raw"/untouched, requested state. + var expected = after.Copy(); if (roundTripApi is not null) { From 3e6b2e2f7b571ea60fca6cd9863f026a8f7e78d7 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Wed, 29 Oct 2025 11:04:20 +0100 Subject: [PATCH 10/14] Move comment --- backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs index 816d92b5f..7e698e426 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs @@ -104,15 +104,15 @@ public async Task CanSyncRandomEntries(ApiType? roundTripApiType) var after = AutoFaker.Generate(); after.Id = before.Id; + // We have to "prepare" while before and after have no overlap (i.e. before we start mixing parts of before into after), + // otherwise "PrepareToCreateEntry" would fail due to trying to create duplicate related entities. + // After this we can't ADD anything to after that has dependencies + // (e.g. ExampleSentences are fine, because they're created as part of an entry, but Parts of speech aren't) await Api.PrepareToCreateEntry(before); await Api.PrepareToCreateEntry(after); if (roundTripApi is not null && currentApiType != roundTripApiType) { - // We have to prepare while before and after have no overlap (i.e. before we start mixing parts of before into after), - // otherwise "PrepareToCreateEntry" would fail due to trying to create duplicate related entities. - // After this we can't ADD anything to after that has dependencies - // (e.g. ExampleSentences are fine, because they're created as part of an entry, but Parts of speech aren't) await roundTripApi.PrepareToCreateEntry(before); await roundTripApi.PrepareToCreateEntry(after); } From 6c4aae2d2f082a346d6968064637e5bce8b33a2d Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Wed, 29 Oct 2025 11:04:47 +0100 Subject: [PATCH 11/14] Thoroughly explain the motivation for the test --- .../FwLiteProjectSync.Tests/EntrySyncTests.cs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs index 7e698e426..bf7d6fe7a 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs @@ -75,8 +75,22 @@ public enum ApiType FwData } - // Not all of these cases are realistic, but they should all work - // An especially critical case is syncing data to crdt that has been round-tripped through fwdata first. + // The round-tripping api is not what is under test here. It's purely for preprocessing. + // It's so that that the data under test is being read from a real API (i.e. fwdata or crdt) + // and thus reflects whatever nuances that API may have. + // + // Not all of the test cases are realistic, but they should all work and they reflect the idea + // that "any MiniLcmApi implementation should be compatible with any other implementation". + // Even the unrealistic test cases could potentially expose unexpected, undesirable nuances in API behaviour. + // They also reflect the diversity of pipelines real entries might go through. + // For example, a currently real scenario is that "after" is read from fwdata and "before" is read from crdt + // and then round-tripped through a json file. + // That case is not explicitly covered here. + // + // The most critical test cases are: + // Api == CrdtApi and RoundTripApi == FwDataApi + // Api == FwDataApi and RoundTripApi == CrdtApi + // (though, as noted above, this case doesn't perfectly reflect real usage) [Theory] [InlineData(ApiType.Crdt)] [InlineData(ApiType.FwData)] From f2d6b115a9c657cba64681712ece3eb7adc4edae Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Mon, 3 Nov 2025 15:17:07 +0100 Subject: [PATCH 12/14] PR feedback: comments and indentation --- .../FwLiteProjectSync.Tests/EntrySyncTests.cs | 101 ++++++++++-------- .../AutoFakerHelpers/EntryFakerHelper.cs | 3 + 2 files changed, 57 insertions(+), 47 deletions(-) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs index bf7d6fe7a..f50f31338 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs @@ -121,7 +121,8 @@ public async Task CanSyncRandomEntries(ApiType? roundTripApiType) // We have to "prepare" while before and after have no overlap (i.e. before we start mixing parts of before into after), // otherwise "PrepareToCreateEntry" would fail due to trying to create duplicate related entities. // After this we can't ADD anything to after that has dependencies - // (e.g. ExampleSentences are fine, because they're created as part of an entry, but Parts of speech aren't) + // e.g. ExampleSentences are fine, because they're owned/part of an entry. + // Parts of speech, on the other hand, are not owned by an entry. await Api.PrepareToCreateEntry(before); await Api.PrepareToCreateEntry(after); @@ -131,55 +132,61 @@ public async Task CanSyncRandomEntries(ApiType? roundTripApiType) await roundTripApi.PrepareToCreateEntry(after); } - after.Senses = [.. + after.Senses = [ // shuffle to cause moves - AutoFaker.Faker.Random.Shuffle([.. - // keep some, remove others - AutoFaker.Faker.Random.ListItems(before.Senses).Select(createdSense => - { - var copy = createdSense.Copy(); - copy.ExampleSentences = [.. - // shuffle to cause moves - AutoFaker.Faker.Random.Shuffle([.. - // keep some, remove others - AutoFaker.Faker.Random.ListItems(copy.ExampleSentences), - // add new - AutoFaker.ExampleSentence(copy), - AutoFaker.ExampleSentence(copy), - ])]; - return copy; - }), - // keep new - ..after.Senses - ])]; - - after.ComplexForms = [.. + ..AutoFaker.Faker.Random.Shuffle([ + // keep some, remove others + ..AutoFaker.Faker.Random.ListItems(before.Senses) + .Select(createdSense => + { + var copy = createdSense.Copy(); + copy.ExampleSentences = [.. + // shuffle to cause moves + AutoFaker.Faker.Random.Shuffle([.. + // keep some, remove others + AutoFaker.Faker.Random.ListItems(copy.ExampleSentences), + // add new + AutoFaker.ExampleSentence(copy), + AutoFaker.ExampleSentence(copy), + ])]; + return copy; + }), + // keep new + ..after.Senses + ]), + ]; + + after.ComplexForms = [ // shuffle to cause moves - AutoFaker.Faker.Random.Shuffle([.. - // keep some, remove others - AutoFaker.Faker.Random.ListItems(before.ComplexForms).Select(createdCfc => - { - var copy = createdCfc.Copy(); - copy.ComponentHeadword = after.Headword(); - return copy; - }), - // keep new - ..after.ComplexForms - ])]; - - after.Components = [.. + ..AutoFaker.Faker.Random.Shuffle([ + // keep some, remove others + ..AutoFaker.Faker.Random.ListItems(before.ComplexForms) + .Select(createdCfc => + { + var copy = createdCfc.Copy(); + copy.ComponentHeadword = after.Headword(); + return copy; + }), + // keep new + ..after.ComplexForms + ]), + ]; + + after.Components = [ // shuffle to cause moves - AutoFaker.Faker.Random.Shuffle([.. - // keep some, remove others - AutoFaker.Faker.Random.ListItems(before.Components).Select(createdCfc => - { - var copy = createdCfc.Copy(); - copy.ComplexFormHeadword = after.Headword(); - return copy; - }), - // keep new - ..after.Components - ])]; + ..AutoFaker.Faker.Random.Shuffle([ + // keep some, remove others + ..AutoFaker.Faker.Random.ListItems(before.Components) + .Select(createdCfc => + { + var copy = createdCfc.Copy(); + copy.ComplexFormHeadword = after.Headword(); + return copy; + }), + // keep new + ..after.Components + ]), + ]; // expected should not be round-tripped, because an api might manipulate it somehow. // We expect the final result to be equivalent to this "raw"/untouched, requested state. diff --git a/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/EntryFakerHelper.cs b/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/EntryFakerHelper.cs index c6ae31cbe..949635083 100644 --- a/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/EntryFakerHelper.cs +++ b/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/EntryFakerHelper.cs @@ -18,6 +18,9 @@ public static async Task EntryReadyForCreation(this AutoFaker autoFaker, return entry; } + /// + /// Makes the entry consistent/valid and creates any necessary dependencies using the provided API + /// public static async Task PrepareToCreateEntry( this IMiniLcmApi api, Entry entry, From 805cf970fda95eab106c7f9ef6cdb0f924fa9dfb Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Mon, 3 Nov 2025 16:29:02 +0100 Subject: [PATCH 13/14] fixup! PR feedback: comments and indentation --- .../FwLiteProjectSync.Tests/EntrySyncTests.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs index f50f31338..5d60a02b4 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs @@ -140,15 +140,16 @@ public async Task CanSyncRandomEntries(ApiType? roundTripApiType) .Select(createdSense => { var copy = createdSense.Copy(); - copy.ExampleSentences = [.. + copy.ExampleSentences = [ // shuffle to cause moves - AutoFaker.Faker.Random.Shuffle([.. - // keep some, remove others - AutoFaker.Faker.Random.ListItems(copy.ExampleSentences), - // add new - AutoFaker.ExampleSentence(copy), - AutoFaker.ExampleSentence(copy), - ])]; + ..AutoFaker.Faker.Random.Shuffle([ + // keep some, remove others + ..AutoFaker.Faker.Random.ListItems(copy.ExampleSentences), + // add new + AutoFaker.ExampleSentence(copy), + AutoFaker.ExampleSentence(copy), + ]), + ]; return copy; }), // keep new From 21d0695ba914f3236d2f74722d98907b80e69ea3 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Mon, 3 Nov 2025 16:50:51 +0100 Subject: [PATCH 14/14] Refactor deep nesting --- .../FwLiteProjectSync.Tests/EntrySyncTests.cs | 42 ++++++++----------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs index 5d60a02b4..5daebecce 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs @@ -132,30 +132,24 @@ public async Task CanSyncRandomEntries(ApiType? roundTripApiType) await roundTripApi.PrepareToCreateEntry(after); } - after.Senses = [ - // shuffle to cause moves - ..AutoFaker.Faker.Random.Shuffle([ - // keep some, remove others - ..AutoFaker.Faker.Random.ListItems(before.Senses) - .Select(createdSense => - { - var copy = createdSense.Copy(); - copy.ExampleSentences = [ - // shuffle to cause moves - ..AutoFaker.Faker.Random.Shuffle([ - // keep some, remove others - ..AutoFaker.Faker.Random.ListItems(copy.ExampleSentences), - // add new - AutoFaker.ExampleSentence(copy), - AutoFaker.ExampleSentence(copy), - ]), - ]; - return copy; - }), - // keep new - ..after.Senses - ]), - ]; + // keep some old senses, remove others + var someRandomBeforeSenses = AutoFaker.Faker.Random.ListItems(before.Senses).Select(createdSense => + { + var copy = createdSense.Copy(); + copy.ExampleSentences = [ + // shuffle to cause moves + ..AutoFaker.Faker.Random.Shuffle([ + // keep some, remove others + ..AutoFaker.Faker.Random.ListItems(copy.ExampleSentences), + // add new + AutoFaker.ExampleSentence(copy), + AutoFaker.ExampleSentence(copy), + ]), + ]; + return copy; + }); + // keep new, and shuffle to cause moves + after.Senses = [.. AutoFaker.Faker.Random.Shuffle([.. someRandomBeforeSenses, .. after.Senses])]; after.ComplexForms = [ // shuffle to cause moves