Fix default crdt WS doesn't respect order (#2008)

myieye · web-flow · commit 4c03705cf4b7 · 2025-10-02T12:18:15.000+02:00
* Fix and standardize ws ordering

* Fix MoveWritingSystem_Works
diff --git a/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs b/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs
@@ -86,28 +86,25 @@ internal int GetWritingSystemHandle(WritingSystemId ws, WritingSystemType? type
 
     public Task<WritingSystems> GetWritingSystems()
     {
-        var currentVernacularWs = WritingSystemContainer
-            .CurrentVernacularWritingSystems
-            .Select(ws => ws.Id).ToHashSet();
-        var currentAnalysisWs = WritingSystemContainer
-            .CurrentAnalysisWritingSystems
-            .Select(ws => ws.Id).ToHashSet();
         var writingSystems = new WritingSystems
         {
             Vernacular = WritingSystemContainer.CurrentVernacularWritingSystems.Select((definition, index) =>
-                FromLcmWritingSystem(definition, index, WritingSystemType.Vernacular)).ToArray(),
+                FromLcmWritingSystem(definition, WritingSystemType.Vernacular, index)).ToArray(),
             Analysis = WritingSystemContainer.CurrentAnalysisWritingSystems.Select((definition, index) =>
-                FromLcmWritingSystem(definition, index, WritingSystemType.Analysis)).ToArray()
+                FromLcmWritingSystem(definition, WritingSystemType.Analysis, index)).ToArray()
         };
-        CompleteExemplars(writingSystems);
+        // Not used and not implemented in CRDT (also not done in GetWritingSystem())
+        // CompleteExemplars(writingSystems);
         return Task.FromResult(writingSystems);
     }
 
-    private WritingSystem FromLcmWritingSystem(CoreWritingSystemDefinition ws, int index, WritingSystemType type)
+    private WritingSystem FromLcmWritingSystem(CoreWritingSystemDefinition ws, WritingSystemType type, int index = default)
     {
         return new WritingSystem
         {
             Id = Guid.Empty,
+            // todo: Order probably shouldn't be relied on in fwdata, because it's implicit,
+            // so it probably shouldn't be used or set at all
             Order = index,
             Type = type,
             //todo determine current and create a property for that.
@@ -119,15 +116,12 @@ private WritingSystem FromLcmWritingSystem(CoreWritingSystemDefinition ws, int i
         };
     }
 
-    public async Task<WritingSystem?> GetWritingSystem(WritingSystemId id, WritingSystemType type)
+    public Task<WritingSystem?> GetWritingSystem(WritingSystemId id, WritingSystemType type)
     {
-        var writingSystems = await GetWritingSystems();
-        return type switch
-        {
-            WritingSystemType.Vernacular => writingSystems.Vernacular.FirstOrDefault(ws => ws.WsId == id),
-            WritingSystemType.Analysis => writingSystems.Analysis.FirstOrDefault(ws => ws.WsId == id),
-            _ => throw new ArgumentOutOfRangeException(nameof(type), type, null)
-        };
+        var lcmWs = Cache.TryGetCoreWritingSystem(id, type);
+        if (lcmWs is null) return Task.FromResult<WritingSystem?>(null);
+        var ws = FromLcmWritingSystem(lcmWs, type);
+        return Task.FromResult<WritingSystem?>(ws);
     }
 
     internal void CompleteExemplars(WritingSystems writingSystems)
@@ -188,7 +182,7 @@ await Cache.DoUsingNewOrCurrentUOW("Create Writing System",
             WritingSystemType.Vernacular => WritingSystemContainer.CurrentVernacularWritingSystems.Count,
             _ => throw new ArgumentOutOfRangeException(nameof(type), type, null)
         } - 1;
-        return FromLcmWritingSystem(ws, index, type);
+        return FromLcmWritingSystem(ws, type, index);
     }
 
     public async Task<WritingSystem> UpdateWritingSystem(WritingSystemId id, WritingSystemType type, UpdateObjectInput<WritingSystem> update)
@@ -225,10 +219,10 @@ await Cache.DoUsingNewOrCurrentUOW("Update WritingSystem",
 
     public async Task MoveWritingSystem(WritingSystemId id, WritingSystemType type, BetweenPosition<WritingSystemId?> between)
     {
-        var wsToUpdate = GetLexWritingSystem(id, type);
+        var wsToUpdate = GetNonDefaultLexWritingSystem(id, type);
         if (wsToUpdate is null) throw new NullReferenceException($"unable to find writing system with id {id}");
-        var previousWs = between.Previous is null ? null : GetLexWritingSystem(between.Previous.Value, type);
-        var nextWs = between.Next is null ? null : GetLexWritingSystem(between.Next.Value, type);
+        var previousWs = between.Previous is null ? null : GetNonDefaultLexWritingSystem(between.Previous.Value, type);
+        var nextWs = between.Next is null ? null : GetNonDefaultLexWritingSystem(between.Next.Value, type);
         if (nextWs is null && previousWs is null) throw new NullReferenceException($"unable to find writing system with id {between.Previous} or {between.Next}");
         await Cache.DoUsingNewOrCurrentUOW("Move WritingSystem",
             "Revert Move WritingSystem",
@@ -270,12 +264,10 @@ void MoveWs(CoreWritingSystemDefinition ws,
             });
     }
 
-    private CoreWritingSystemDefinition? GetLexWritingSystem(WritingSystemId id, WritingSystemType type)
+    private CoreWritingSystemDefinition? GetNonDefaultLexWritingSystem(WritingSystemId id, WritingSystemType type)
     {
-        var exitingWs = type == WritingSystemType.Analysis
-            ? WritingSystemContainer.AnalysisWritingSystems
-            : WritingSystemContainer.VernacularWritingSystems;
-        return exitingWs.FirstOrDefault(ws => ws.Id == id);
+        if (id == default) throw new ArgumentException("Cannot use default writing system ID", nameof(id));
+        return Cache.TryGetCoreWritingSystem(id, type);
     }
 
     public IAsyncEnumerable<PartOfSpeech> GetPartsOfSpeech()
diff --git a/backend/FwLite/FwDataMiniLcmBridge/Api/LcmHelpers.cs b/backend/FwLite/FwDataMiniLcmBridge/Api/LcmHelpers.cs
@@ -166,42 +166,44 @@ internal static WritingSystemId GetWritingSystemId(this LcmCache cache, int ws)
         return cache.ServiceLocator.WritingSystemManager.Get(ws).Id;
     }
 
-    internal static int GetWritingSystemHandle(this LcmCache cache, WritingSystemId ws, WritingSystemType? type = null)
+    internal static CoreWritingSystemDefinition? TryGetCoreWritingSystem(this LcmCache cache, WritingSystemId wsId, WritingSystemType type)
     {
         var wsContainer = cache.ServiceLocator.WritingSystems;
-        if (ws == "default")
+        var writingSystemsOfType = type switch
         {
-            return type switch
-            {
-                WritingSystemType.Analysis => wsContainer.DefaultAnalysisWritingSystem.Handle,
-                WritingSystemType.Vernacular => wsContainer.DefaultVernacularWritingSystem.Handle,
-                _ => throw new ArgumentOutOfRangeException(nameof(type), type, null)
-            };
-        }
+            WritingSystemType.Analysis => wsContainer.AnalysisWritingSystems,
+            WritingSystemType.Vernacular => wsContainer.VernacularWritingSystems,
+            _ => throw new ArgumentOutOfRangeException(nameof(type), type, null)
+        };
+        if (wsId == default) return writingSystemsOfType.FirstOrDefault();
+        return writingSystemsOfType.FirstOrDefault(ws => ws.Id == wsId);
+    }
 
-        if (!cache.ServiceLocator.WritingSystemManager.TryGet(ws.Code, out var lcmWs))
+    internal static CoreWritingSystemDefinition GetCoreWritingSystem(this LcmCache cache, WritingSystemId wsId, WritingSystemType? type = null)
+    {
+        if (type is not null)
         {
-            throw new NullReferenceException($"unable to find writing system with id '{ws.Code}'");
+            return TryGetCoreWritingSystem(cache, wsId, type.Value)
+                ?? throw new NullReferenceException($"unable to find writing system with id '{wsId.Code}' of type {type}");
         }
-        if (lcmWs is not null && type is not null)
+
+        if (wsId == default)
         {
-            var validWs = type switch
-            {
-                WritingSystemType.Analysis => wsContainer.AnalysisWritingSystems,
-                WritingSystemType.Vernacular => wsContainer.VernacularWritingSystems,
-                _ => throw new ArgumentOutOfRangeException(nameof(type), type, null)
-            };
-            if (!validWs.Contains(lcmWs))
-            {
-                throw new InvalidOperationException($"Writing system {ws} is not of the requested type: {type}.");
-            }
+            throw new ArgumentException("Cannot look up default writing system ID when type is not specified", nameof(wsId));
         }
-        if (lcmWs is null)
+
+        if (!cache.ServiceLocator.WritingSystemManager.TryGet(wsId.Code, out var lcmWs))
         {
-            throw new NullReferenceException($"unable to find writing system with id {ws}");
+            throw new NullReferenceException($"unable to find writing system with id '{wsId.Code}'");
         }
 
-        return lcmWs.Handle;
+        return lcmWs ?? throw new NullReferenceException($"unable to find writing system with id {wsId}");
+    }
+
+    internal static int GetWritingSystemHandle(this LcmCache cache, WritingSystemId wsId, WritingSystemType? type = null)
+    {
+        var ws = GetCoreWritingSystem(cache, wsId, type);
+        return ws.Handle;
     }
 
     internal static string PickText(this ICmObject obj, ITsMultiString multiString, string ws)
diff --git a/backend/FwLite/FwLiteProjectSync.Tests/WritingSystemSyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/WritingSystemSyncTests.cs
@@ -51,32 +51,25 @@ public async Task DisposeAsync()
     [Fact]
     public async Task SyncWs_UpdatesOrder()
     {
-        var en = await _fixture.FwDataApi.GetWritingSystem("en", WritingSystemType.Vernacular);
+        var fwVernacularWSs = (await _fixture.FwDataApi.GetWritingSystems())!.Vernacular;
+        var crdtVernacularWSs = (await _fixture.CrdtApi.GetWritingSystems())!.Vernacular;
+        fwVernacularWSs.Should().HaveCount(2);
+        crdtVernacularWSs.Should().HaveCount(2);
+        fwVernacularWSs.Should().BeEquivalentTo(crdtVernacularWSs, options => options.WithStrictOrdering().Excluding(ws => ws.Order));
+        var en = await _fixture.CrdtApi.GetWritingSystem("en", WritingSystemType.Vernacular);
+        var fr = await _fixture.CrdtApi.GetWritingSystem("fr", WritingSystemType.Vernacular);
         en.Should().NotBeNull();
-        en.Order.Should().Be(0); // 1st - fw order starts at 0
-        var fr = await _fixture.FwDataApi.GetWritingSystem("fr", WritingSystemType.Vernacular);
         fr.Should().NotBeNull();
-        fr.Order.Should().Be(1);
-        var crdtEn = await _fixture.CrdtApi.GetWritingSystem("en", WritingSystemType.Vernacular);
-        crdtEn.Should().NotBeNull();
-        crdtEn.Order.Should().Be(1); // 1st - crdt order starts at 1
-        var crdtFr = await _fixture.CrdtApi.GetWritingSystem("fr", WritingSystemType.Vernacular);
-        crdtFr.Should().NotBeNull();
-        crdtFr.Order.Should().Be(2);
-
+        fwVernacularWSs.Should().BeEquivalentTo([en, fr], options => options.WithStrictOrdering().Excluding(ws => ws.Order));
 
         // act - move fr before en
         await _fixture.FwDataApi.MoveWritingSystem("fr", WritingSystemType.Vernacular, new(null, "en"));
-        fr = await _fixture.FwDataApi.GetWritingSystem("fr", WritingSystemType.Vernacular);
-        fr.Should().NotBeNull();
-        fr.Order.Should().Be(0);
+        var updatedFwVernacularWSs = (await _fixture.FwDataApi.GetWritingSystems())!.Vernacular;
+        updatedFwVernacularWSs.Should().BeEquivalentTo([fr, en], options => options.WithStrictOrdering().Excluding(ws => ws.Order));
         await _syncService.Sync(_fixture.CrdtApi, _fixture.FwDataApi);
 
         // assert
-        var updatedCrdtEn = await _fixture.CrdtApi.GetWritingSystem("en", WritingSystemType.Vernacular);
-        updatedCrdtEn.Should().NotBeNull();
-        var updatedCrdtFr = await _fixture.CrdtApi.GetWritingSystem("fr", WritingSystemType.Vernacular);
-        updatedCrdtFr.Should().NotBeNull();
-        updatedCrdtFr.Order.Should().BeLessThan(updatedCrdtEn.Order);
+        var updatedCrdtVernacularWSs = (await _fixture.CrdtApi.GetWritingSystems())!.Vernacular;
+        updatedCrdtVernacularWSs.Should().BeEquivalentTo(updatedFwVernacularWSs, options => options.WithStrictOrdering().Excluding(ws => ws.Order));
     }
 }
diff --git a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs
@@ -67,10 +67,7 @@ private void AssertWritable()
     public async Task<WritingSystems> GetWritingSystems()
     {
         await using var repo = await repoFactory.CreateRepoAsync();
-        var systems = await repo.WritingSystems
-            .OrderBy(ws => ws.Order)
-            .ThenBy(ws => ws.WsId)
-            .ToArrayAsync();
+        var systems = await repo.WritingSystemsOrdered.ToArrayAsync();
         return new WritingSystems
         {
             Analysis = [.. systems.Where(ws => ws.Type == WritingSystemType.Analysis)],
diff --git a/backend/FwLite/LcmCrdt/Data/MiniLcmRepository.cs b/backend/FwLite/LcmCrdt/Data/MiniLcmRepository.cs
@@ -14,7 +14,7 @@
 namespace LcmCrdt.Data;
 
 public class MiniLcmRepositoryFactory(
-    Microsoft.EntityFrameworkCore.IDbContextFactory<LcmCrdtDbContext> dbContextFactory,
+    IDbContextFactory<LcmCrdtDbContext> dbContextFactory,
     IServiceProvider serviceProvider,
     EntrySearchServiceFactory? entrySearchServiceFactory = null)
 {
@@ -67,6 +67,7 @@ public void Dispose()
     public IQueryable<Sense> Senses => dbContext.Senses;
     public IQueryable<ExampleSentence> ExampleSentences => dbContext.ExampleSentences;
     public IQueryable<WritingSystem> WritingSystems => dbContext.WritingSystems;
+    public IQueryable<WritingSystem> WritingSystemsOrdered => dbContext.WritingSystemsOrdered;
     public IQueryable<SemanticDomain> SemanticDomains => dbContext.SemanticDomains;
     public IQueryable<PartOfSpeech> PartsOfSpeech => dbContext.PartsOfSpeech;
     public IQueryable<Publication> Publications => dbContext.Publications;
@@ -81,14 +82,14 @@ public void Dispose()
             return type switch
             {
                 WritingSystemType.Analysis => _defaultAnalysisWs ??=
-                    await AsyncExtensions.FirstOrDefaultAsync(dbContext.WritingSystems, ws => ws.Type == type),
+                    await AsyncExtensions.FirstOrDefaultAsync(WritingSystemsOrdered, ws => ws.Type == type),
                 WritingSystemType.Vernacular => _defaultVernacularWs ??=
-                    await AsyncExtensions.FirstOrDefaultAsync(dbContext.WritingSystems, ws => ws.Type == type),
+                    await AsyncExtensions.FirstOrDefaultAsync(WritingSystemsOrdered, ws => ws.Type == type),
                 _ => throw new ArgumentOutOfRangeException(nameof(type), type, null)
             } ?? throw new NullReferenceException($"Unable to find a default writing system of type {type}");
         }
 
-        return await AsyncExtensions.FirstOrDefaultAsync(dbContext.WritingSystems, ws => ws.WsId == id && ws.Type == type);
+        return await AsyncExtensions.FirstOrDefaultAsync(WritingSystemsOrdered, ws => ws.WsId == id && ws.Type == type);
     }
 
     public async Task<AddEntryComponentChange> CreateComplexFormComponentChange(
diff --git a/backend/FwLite/LcmCrdt/FullTextSearch/EntrySearchService.cs b/backend/FwLite/LcmCrdt/FullTextSearch/EntrySearchService.cs
@@ -1,5 +1,3 @@
-using System.Globalization;
-using System.Linq.Expressions;
 using System.Text;
 using LcmCrdt.Data;
 using LinqToDB;
@@ -139,7 +137,7 @@ public async Task UpdateEntrySearchTable(Guid entryId)
 
     public async Task UpdateEntrySearchTable(Entry entry)
     {
-        var writingSystems = await dbContext.WritingSystems.OrderBy(ws => ws.Order).ToArrayAsync();
+        var writingSystems = await dbContext.WritingSystemsOrdered.ToArrayAsync();
         var record = ToEntrySearchRecord(entry, writingSystems);
         await InsertOrUpdateEntrySearchRecord(record, EntrySearchRecordsTable);
     }
@@ -181,7 +179,12 @@ public static async Task UpdateEntrySearchTable(IEnumerable<Entry> entries,
             ..dbContext.WritingSystems,
             ..newWritingSystems
         ];
-        Array.Sort(writingSystems, (ws1, ws2) => ws1.Order.CompareTo(ws2.Order));
+        Array.Sort(writingSystems, (ws1, ws2) =>
+        {
+            var orderComparison = ws1.Order.CompareTo(ws2.Order);
+            if (orderComparison != 0) return orderComparison;
+            return ws1.Id.CompareTo(ws2.Id);
+        });
         var entrySearchRecordsTable = dbContext.GetTable<EntrySearchRecord>();
         var searchRecords = entries.Select(entry => ToEntrySearchRecord(entry, writingSystems));
         foreach (var entrySearchRecord in searchRecords)
@@ -200,7 +203,7 @@ public async Task RegenerateEntrySearchTable()
         await using var transaction = await dbContext.Database.BeginTransactionAsync();
         await EntrySearchRecordsTable.TruncateAsync();
 
-        var writingSystems = await dbContext.WritingSystems.OrderBy(ws => ws.Order).ToArrayAsync();
+        var writingSystems = await dbContext.WritingSystemsOrdered.ToArrayAsync();
         await EntrySearchRecordsTable
             .BulkCopyAsync(dbContext.Set<Entry>()
                 .LoadWith(e => e.Senses)
diff --git a/backend/FwLite/LcmCrdt/LcmCrdtDbContext.cs b/backend/FwLite/LcmCrdt/LcmCrdtDbContext.cs
@@ -1,5 +1,4 @@
 using System.Text.Json;
-using LcmCrdt.Data;
 using LcmCrdt.FullTextSearch;
 using SIL.Harmony;
 using SIL.Harmony.Db;
@@ -17,6 +16,8 @@ IOptions<CrdtConfig> options
 {
     public DbSet<ProjectData> ProjectData => Set<ProjectData>();
     public IQueryable<WritingSystem> WritingSystems => Set<WritingSystem>().AsNoTracking();
+    public IQueryable<WritingSystem> WritingSystemsOrdered => Set<WritingSystem>().AsNoTracking()
+        .OrderBy(ws => ws.Order).ThenBy(ws => ws.Id);
     public IQueryable<Entry> Entries => Set<Entry>().AsNoTracking();
     public IQueryable<ComplexFormComponent> ComplexFormComponents => Set<ComplexFormComponent>().AsNoTracking();
     public IQueryable<ComplexFormType> ComplexFormTypes => Set<ComplexFormType>().AsNoTracking();
diff --git a/backend/FwLite/MiniLcm.Tests/WritingSystemTestsBase.cs b/backend/FwLite/MiniLcm.Tests/WritingSystemTestsBase.cs

Original file line number	Diff line number	Diff line change
`@@ -67,10 +67,7 @@ private void AssertWritable()`
`67`	`67`	`public async Task<WritingSystems> GetWritingSystems()`
`68`	`68`	`{`
`69`	`69`	`await using var repo = await repoFactory.CreateRepoAsync();`
`70`		`- var systems = await repo.WritingSystems`
`71`		`- .OrderBy(ws => ws.Order)`
`72`		`- .ThenBy(ws => ws.WsId)`
`73`		`- .ToArrayAsync();`
	`70`	`+ var systems = await repo.WritingSystemsOrdered.ToArrayAsync();`
`74`	`71`	`return new WritingSystems`
`75`	`72`	`{`
`76`	`73`	`Analysis = [.. systems.Where(ws => ws.Type == WritingSystemType.Analysis)],`