diff --git a/backend/FwLite/FwDataMiniLcmBridge.Tests/MiniLcmTests/UpdateEntryTests.cs b/backend/FwLite/FwDataMiniLcmBridge.Tests/MiniLcmTests/UpdateEntryTests.cs index a3efbc833..70d1a88a4 100644 --- a/backend/FwLite/FwDataMiniLcmBridge.Tests/MiniLcmTests/UpdateEntryTests.cs +++ b/backend/FwLite/FwDataMiniLcmBridge.Tests/MiniLcmTests/UpdateEntryTests.cs @@ -9,4 +9,6 @@ protected override Task NewApi() { return Task.FromResult(fixture.NewProjectApi("update-entry-test", "en", "en")); } + + protected override bool ApiUsesImplicitOrdering => true; } diff --git a/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs b/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs index c5ffc2974..0a60d09a9 100644 --- a/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs +++ b/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs @@ -1,6 +1,5 @@ using System.Collections.Frozen; using System.Globalization; -using System.Reflection; using System.Text; using FluentValidation; using FwDataMiniLcmBridge.Api.UpdateProxy; @@ -649,7 +648,7 @@ public IAsyncEnumerable SearchEntries(string query, QueryOptions? options var entries = GetEntries(e => e.CitationForm.SearchValue(query) || e.LexemeFormOA.Form.SearchValue(query) || - e.SensesOS.Any(s => s.Gloss.SearchValue(query)), options); + e.AllSenses.Any(s => s.Gloss.SearchValue(query)), options); return entries; } @@ -874,9 +873,10 @@ public Task DeleteEntry(Guid id) return Task.CompletedTask; } - internal void CreateSense(ILexEntry lexEntry, Sense sense) + internal void CreateSense(ILexEntry lexEntry, Sense sense, BetweenPosition? between = null) { - var lexSense = LexSenseFactory.Create(sense.Id, lexEntry); + var lexSense = LexSenseFactory.Create(sense.Id); + InsertSense(lexEntry, lexSense, between); var msa = new SandboxGenericMSA() { MsaType = lexSense.GetDesiredMsaType() }; if (sense.PartOfSpeechId.HasValue && PartOfSpeechRepository.TryGetObject(sense.PartOfSpeechId.Value, out var pos)) { @@ -886,6 +886,46 @@ internal void CreateSense(ILexEntry lexEntry, Sense sense) ApplySenseToLexSense(sense, lexSense); } + internal void InsertSense(ILexEntry lexEntry, ILexSense lexSense, BetweenPosition? between = null) + { + var previousSenseId = between?.Previous; + var nextSenseId = between?.Next; + + var previousSense = previousSenseId.HasValue ? lexEntry.AllSenses.Find(s => s.Guid == previousSenseId) : null; + if (previousSense is not null) + { + if (previousSense.SensesOS.Count > 0) + { + // if the sense has sub-senses, our sense will only come directly after it if it is the first sub-sense + previousSense.SensesOS.Insert(0, lexSense); + } + else + { + // todo the user might have wanted it to be a subsense of previousSense + var allSiblings = previousSense.Owner == lexEntry ? lexEntry.SensesOS + : previousSense.Owner is ILexSense parentSense ? parentSense.SensesOS + : throw new InvalidOperationException("Sense parent is not a sense or the expected entry"); + var insertI = allSiblings.IndexOf(previousSense) + 1; + lexEntry.SensesOS.Insert(insertI, lexSense); + } + return; + } + + var nextSense = nextSenseId.HasValue ? lexEntry.AllSenses.Find(s => s.Guid == nextSenseId) : null; + if (nextSense is not null) + { + // todo the user might have wanted it to be a subsense of whatever is before nextSense + var allSiblings = nextSense.Owner == lexEntry ? lexEntry.SensesOS + : nextSense.Owner is ILexSense parentSense ? parentSense.SensesOS + : throw new InvalidOperationException("Sense parent is not a sense or the expected entry"); + var insertI = allSiblings.IndexOf(nextSense); + lexEntry.SensesOS.Insert(insertI, lexSense); + return; + } + + lexEntry.SensesOS.Add(lexSense); + } + private void ApplySenseToLexSense(Sense sense, ILexSense lexSense) { if (lexSense.MorphoSyntaxAnalysisRA.GetPartOfSpeech()?.Guid != sense.PartOfSpeechId) @@ -917,7 +957,7 @@ private void ApplySenseToLexSense(Sense sense, ILexSense lexSense) return Task.FromResult(lcmSense is null ? null : FromLexSense(lcmSense)); } - public Task CreateSense(Guid entryId, Sense sense) + public Task CreateSense(Guid entryId, Sense sense, BetweenPosition? between = null) { if (sense.Id == default) sense.Id = Guid.NewGuid(); if (!EntriesRepository.TryGetObject(entryId, out var lexEntry)) @@ -925,7 +965,7 @@ public Task CreateSense(Guid entryId, Sense sense) UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Create Sense", "Remove sense", Cache.ServiceLocator.ActionHandler, - () => CreateSense(lexEntry, sense)); + () => CreateSense(lexEntry, sense, between)); return Task.FromResult(FromLexSense(SenseRepository.GetObject(sense.Id))); } @@ -955,6 +995,24 @@ await Cache.DoUsingNewOrCurrentUOW("Update Sense", return await GetSense(entryId, after.Id) ?? throw new NullReferenceException("unable to find sense with id " + after.Id); } + public Task MoveSense(Guid entryId, Guid senseId, BetweenPosition between) + { + if (!EntriesRepository.TryGetObject(entryId, out var lexEntry)) + throw new InvalidOperationException("Entry not found"); + if (!SenseRepository.TryGetObject(senseId, out var lexSense)) + throw new InvalidOperationException("Sense not found"); + + UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Move Sense", + "Move Sense back", + Cache.ServiceLocator.ActionHandler, + () => + { + // LibLCM treats an insert as a move if the sense is already in the entry + InsertSense(lexEntry, lexSense, between); + }); + return Task.CompletedTask; + } + public Task AddSemanticDomainToSense(Guid senseId, SemanticDomain semanticDomain) { UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Add Semantic Domain to Sense", diff --git a/backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateEntryProxy.cs b/backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateEntryProxy.cs index d0defe8ad..548c1d1f5 100644 --- a/backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateEntryProxy.cs +++ b/backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateEntryProxy.cs @@ -45,15 +45,9 @@ public override MultiString LiteralMeaning set => throw new NotImplementedException(); } - public override IList Senses + public override List Senses { - get => - new UpdateListProxy( - sense => _lexboxLcmApi.CreateSense(_lcmEntry, sense), - sense => _lexboxLcmApi.DeleteSense(Id, sense.Id), - i => new UpdateSenseProxy(_lcmEntry.SensesOS[i], _lexboxLcmApi), - _lcmEntry.SensesOS.Count - ); + get => throw new NotImplementedException(); set => throw new NotImplementedException(); } diff --git a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs index b2377ae5c..83dfa17a3 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs @@ -1,15 +1,25 @@ using FwLiteProjectSync.Tests.Fixtures; using MiniLcm.Models; using MiniLcm.SyncHelpers; -using MiniLcm.Tests; using MiniLcm.Tests.AutoFakerHelpers; using Soenneker.Utils.AutoBogus; +using Soenneker.Utils.AutoBogus.Config; namespace FwLiteProjectSync.Tests; public class EntrySyncTests : IClassFixture { - private static readonly AutoFaker AutoFaker = new(builder => builder.WithOverride(new MultiStringOverride()).WithOverride(new ObjectWithIdOverride())); + private static readonly AutoFaker AutoFaker = new(new AutoFakerConfig() + { + RepeatCount = 5, + Overrides = + [ + new MultiStringOverride(), + new ObjectWithIdOverride(), + new OrderableOverride(), + ] + }); + public EntrySyncTests(SyncFixture fixture) { _fixture = fixture; @@ -22,10 +32,18 @@ public async Task CanSyncRandomEntries() { var createdEntry = await _fixture.CrdtApi.CreateEntry(await AutoFaker.EntryReadyForCreation(_fixture.CrdtApi)); var after = await AutoFaker.EntryReadyForCreation(_fixture.CrdtApi, entryId: createdEntry.Id); + + after.Senses = AutoFaker.Faker.Random.Shuffle([ + // copy some senses over, so moves happen + ..AutoFaker.Faker.Random.ListItems(createdEntry.Senses), + ..(after.Senses) + ]).ToList(); + await EntrySync.Sync(after, createdEntry, _fixture.CrdtApi); var actual = await _fixture.CrdtApi.GetEntry(after.Id); actual.Should().NotBeNull(); - actual.Should().BeEquivalentTo(after, options => options); + actual.Should().BeEquivalentTo(after, options => options + .For(e => e.Senses).Exclude(s => s.Order)); } [Fact] diff --git a/backend/FwLite/FwLiteProjectSync.Tests/Sena3SyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/Sena3SyncTests.cs index 54e991041..80b2e8079 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/Sena3SyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/Sena3SyncTests.cs @@ -51,7 +51,8 @@ private void ShouldAllBeEquivalentTo(Dictionary crdtEntries, Dictio crdtEntry.Should().BeEquivalentTo(fwdataEntry, options => options .For(e => e.Components).Exclude(c => c.Id) - .For(e => e.ComplexForms).Exclude(c => c.Id), + .For(e => e.ComplexForms).Exclude(c => c.Id) + .For(e => e.Senses).Exclude(s => s.Order), $"CRDT entry {crdtEntry.Id} was synced with FwData"); } } diff --git a/backend/FwLite/FwLiteProjectSync.Tests/SyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/SyncTests.cs index 49a9fd860..a6f822d99 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/SyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/SyncTests.cs @@ -83,7 +83,9 @@ public async Task FirstSyncJustDoesAnImport() var crdtEntries = await crdtApi.GetAllEntries().ToArrayAsync(); var fwdataEntries = await fwdataApi.GetAllEntries().ToArrayAsync(); crdtEntries.Should().BeEquivalentTo(fwdataEntries, - options => options.For(e => e.Components).Exclude(c => c.Id) + options => options + .For(e => e.Senses).Exclude(s => s.Order) + .For(e => e.Components).Exclude(c => c.Id) .For(e => e.ComplexForms).Exclude(c => c.Id)); } @@ -145,7 +147,9 @@ await crdtApi.CreateEntry(new Entry() var crdtEntries = await crdtApi.GetAllEntries().ToArrayAsync(); var fwdataEntries = await fwdataApi.GetAllEntries().ToArrayAsync(); crdtEntries.Should().BeEquivalentTo(fwdataEntries, - options => options.For(e => e.Components).Exclude(c => c.Id) + options => options + .For(e => e.Senses).Exclude(s => s.Order) + .For(e => e.Components).Exclude(c => c.Id) .For(e => e.ComplexForms).Exclude(c => c.Id)); } @@ -223,7 +227,9 @@ public async Task CreatingAComplexEntryInFwDataSyncsWithoutIssue() var crdtEntries = await crdtApi.GetAllEntries().ToArrayAsync(); var fwdataEntries = await fwdataApi.GetAllEntries().ToArrayAsync(); crdtEntries.Should().BeEquivalentTo(fwdataEntries, - options => options.For(e => e.Components).Exclude(c => c.Id) + options => options + .For(e => e.Senses).Exclude(s => s.Order) + .For(e => e.Components).Exclude(c => c.Id) .For(e => e.ComplexForms).Exclude(c => c.Id)); // Sync again, ensure no problems or changes @@ -305,7 +311,9 @@ await crdtApi.CreateEntry(new Entry() var crdtEntries = await crdtApi.GetAllEntries().ToArrayAsync(); var fwdataEntries = await fwdataApi.GetAllEntries().ToArrayAsync(); crdtEntries.Should().BeEquivalentTo(fwdataEntries, - options => options.For(e => e.Components).Exclude(c => c.Id) + options => options + .For(e => e.Senses).Exclude(s => s.Order) + .For(e => e.Components).Exclude(c => c.Id) .For(e => e.ComplexForms).Exclude(c => c.Id)); } @@ -384,7 +392,9 @@ await crdtApi.CreateEntry(new Entry() var crdtEntries = await crdtApi.GetAllEntries().ToArrayAsync(); var fwdataEntries = await fwdataApi.GetAllEntries().ToArrayAsync(); crdtEntries.Should().BeEquivalentTo(fwdataEntries, - options => options.For(e => e.Components).Exclude(c => c.Id) + options => options + .For(e => e.Senses).Exclude(s => s.Order) + .For(e => e.Components).Exclude(c => c.Id) .For(e => e.ComplexForms).Exclude(c => c.Id)); } @@ -408,6 +418,7 @@ public async Task UpdatingAnEntryInEachProjectSyncsAcrossBoth() var fwdataEntries = await fwdataApi.GetAllEntries().ToArrayAsync(); crdtEntries.Should().BeEquivalentTo(fwdataEntries, options => options + .For(e => e.Senses).Exclude(s => s.Order) .For(e => e.Components).Exclude(c => c.Id) //todo the headword should be changed .For(e => e.Components).Exclude(c => c.ComponentHeadword) @@ -475,7 +486,9 @@ public async Task AddingASenseToAnEntryInEachProjectSyncsAcrossBoth() var crdtEntries = await crdtApi.GetAllEntries().ToArrayAsync(); var fwdataEntries = await fwdataApi.GetAllEntries().ToArrayAsync(); crdtEntries.Should().BeEquivalentTo(fwdataEntries, - options => options.For(e => e.Components).Exclude(c => c.Id) + options => options + .For(e => e.Senses).Exclude(s => s.Order) + .For(e => e.Components).Exclude(c => c.Id) .For(e => e.ComplexForms).Exclude(c => c.Id)); } diff --git a/backend/FwLite/FwLiteProjectSync.Tests/UpdateDiffTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/UpdateDiffTests.cs index 37d4b7026..1f3523981 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/UpdateDiffTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/UpdateDiffTests.cs @@ -1,5 +1,4 @@ -using FwLiteProjectSync.Tests.Fixtures; -using MiniLcm.Models; +using MiniLcm.Models; using MiniLcm.SyncHelpers; using MiniLcm.Tests.AutoFakerHelpers; using Soenneker.Utils.AutoBogus; @@ -11,7 +10,13 @@ public class UpdateDiffTests { private static readonly AutoFaker AutoFaker = new(new AutoFakerConfig() { - Overrides = [new MultiStringOverride(), new WritingSystemIdOverride()] + RepeatCount = 5, + Overrides = + [ + new MultiStringOverride(), + new WritingSystemIdOverride(), + new OrderableOverride(), + ] }); [Fact] diff --git a/backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs b/backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs index 82d5190ca..bdda169aa 100644 --- a/backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs +++ b/backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs @@ -1,5 +1,6 @@ using MiniLcm; using MiniLcm.Models; +using MiniLcm.SyncHelpers; namespace FwLiteProjectSync; @@ -195,9 +196,9 @@ public async Task RemoveComplexFormType(Guid entryId, Guid complexFormTypeId) return api.GetSense(entryId, id); } - public Task CreateSense(Guid entryId, Sense sense) + public Task CreateSense(Guid entryId, Sense sense, BetweenPosition? position = null) { - DryRunRecords.Add(new DryRunRecord(nameof(CreateSense), $"Create sense {sense.Gloss}")); + DryRunRecords.Add(new DryRunRecord(nameof(CreateSense), $"Create sense {sense.Gloss} between {position?.Previous} and {position?.Next}")); return Task.FromResult(sense); } @@ -218,6 +219,12 @@ public async Task UpdateSense(Guid entryId, Sense before, Sense after) return await GetSense(entryId, after.Id) ?? throw new NullReferenceException($"unable to find sense with id {after.Id}"); } + public Task MoveSense(Guid entryId, Guid senseId, BetweenPosition between) + { + DryRunRecords.Add(new DryRunRecord(nameof(MoveSense), $"Move sense {senseId} between {between.Previous} and {between.Next}")); + return Task.CompletedTask; + } + public Task DeleteSense(Guid entryId, Guid senseId) { DryRunRecords.Add(new DryRunRecord(nameof(DeleteSense), $"Delete sense {senseId}")); diff --git a/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyChangeModels.verified.txt b/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyChangeModels.verified.txt index 685df4f41..578529de3 100644 --- a/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyChangeModels.verified.txt +++ b/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyChangeModels.verified.txt @@ -119,6 +119,10 @@ { DerivedType: CreateComplexFormType, TypeDiscriminator: CreateComplexFormType + }, + { + DerivedType: SetOrderChange, + TypeDiscriminator: SetOrderChange:Sense } ], IgnoreUnrecognizedTypeDiscriminators: false, diff --git a/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyDbModel.verified.txt b/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyDbModel.verified.txt index b1ae8d40a..e33723359 100644 --- a/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyDbModel.verified.txt +++ b/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyDbModel.verified.txt @@ -98,7 +98,7 @@ Navigations: ComplexForms (IList) Collection ToDependent ComplexFormComponent Components (IList) Collection ToDependent ComplexFormComponent - Senses (IList) Collection ToDependent Sense + Senses (List) Collection ToDependent Sense Keys: Id PK Foreign keys: @@ -200,6 +200,7 @@ Gloss (MultiString) Required Annotations: Relational:ColumnType: jsonb + Order (double) Required PartOfSpeech (string) Required PartOfSpeechId (Guid?) SemanticDomains (IList) Required diff --git a/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.cs b/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.cs index 3173e290d..8b6403340 100644 --- a/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.cs +++ b/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.cs @@ -17,9 +17,15 @@ namespace LcmCrdt.Tests; public class DataModelSnapshotTests : IAsyncLifetime { - private static readonly AutoFaker Faker = new AutoFaker(new AutoFakerConfig() + private static readonly AutoFaker Faker = new(new AutoFakerConfig() { - Overrides = [new MultiStringOverride(), new WritingSystemIdOverride()] + RepeatCount = 5, + Overrides = + [ + new MultiStringOverride(), + new WritingSystemIdOverride(), + new OrderableOverride(), + ] }); protected readonly AsyncServiceScope _services; diff --git a/backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs b/backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs index 10387a6ef..cc60a393b 100644 --- a/backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs +++ b/backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs @@ -1,7 +1,4 @@ -using SIL.Harmony.Entities; -using LcmCrdt.Objects; -using MiniLcm.Tests.AutoFakerHelpers; -using SIL.Harmony; +using MiniLcm.Tests.AutoFakerHelpers; using Soenneker.Utils.AutoBogus; using Soenneker.Utils.AutoBogus.Config; @@ -11,7 +8,13 @@ public class EntityCopyMethodTests { private static readonly AutoFaker AutoFaker = new(new AutoFakerConfig() { - Overrides = [new MultiStringOverride(), new WritingSystemIdOverride()] + RepeatCount = 5, + Overrides = + [ + new MultiStringOverride(), + new WritingSystemIdOverride(), + new OrderableOverride(), + ], }); public static IEnumerable GetEntityTypes() diff --git a/backend/FwLite/LcmCrdt/Changes/CreateSenseChange.cs b/backend/FwLite/LcmCrdt/Changes/CreateSenseChange.cs index 4682ae30d..dd034e2ed 100644 --- a/backend/FwLite/LcmCrdt/Changes/CreateSenseChange.cs +++ b/backend/FwLite/LcmCrdt/Changes/CreateSenseChange.cs @@ -11,6 +11,7 @@ public CreateSenseChange(Sense sense, Guid entryId) : base(sense.Id == Guid.Empt { sense.Id = EntityId; EntryId = entryId; + Order = sense.Order; Definition = sense.Definition; SemanticDomains = sense.SemanticDomains; Gloss = sense.Gloss; @@ -25,6 +26,7 @@ private CreateSenseChange(Guid entityId, Guid entryId) : base(entityId) } public Guid EntryId { get; set; } + public double Order { get; set; } public MultiString? Definition { get; set; } public MultiString? Gloss { get; set; } public string? PartOfSpeech { get; set; } @@ -37,6 +39,7 @@ public override async ValueTask NewEntity(Commit commit, ChangeContext co { Id = EntityId, EntryId = EntryId, + Order = Order, Definition = Definition ?? new MultiString(), Gloss = Gloss ?? new MultiString(), PartOfSpeech = PartOfSpeech ?? string.Empty, diff --git a/backend/FwLite/LcmCrdt/Changes/SetOrderChange.cs b/backend/FwLite/LcmCrdt/Changes/SetOrderChange.cs new file mode 100644 index 000000000..8b56f923b --- /dev/null +++ b/backend/FwLite/LcmCrdt/Changes/SetOrderChange.cs @@ -0,0 +1,18 @@ +using SIL.Harmony.Changes; +using SIL.Harmony.Entities; + +namespace LcmCrdt.Changes; + +public class SetOrderChange(Guid entityId, double order) : EditChange(entityId), IPolyType + where T : class, IOrderable +{ + public static string TypeName => $"{nameof(SetOrderChange)}:" + typeof(T).Name; + + public double Order { get; init; } = order; + + public override ValueTask ApplyChange(T entity, ChangeContext context) + { + entity.Order = Order; + return ValueTask.CompletedTask; + } +} diff --git a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs index 3955ab957..c13168e3c 100644 --- a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs +++ b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs @@ -267,7 +267,8 @@ private async IAsyncEnumerable GetEntries( if (sortWs is null) throw new NullReferenceException($"sort writing system {options.Order.WritingSystem} not found"); queryable = queryable - .LoadWith(e => e.Senses).ThenLoad(s => s.ExampleSentences) + .LoadWith(e => e.Senses) + .ThenLoad(s => s.ExampleSentences) .LoadWith(e => e.ComplexForms) .LoadWith(e => e.Components) .AsQueryable() @@ -277,6 +278,7 @@ private async IAsyncEnumerable GetEntries( var entries = queryable.AsAsyncEnumerable(); await foreach (var entry in entries) { + entry.ApplySortOrder(); yield return entry; } } @@ -290,6 +292,7 @@ private async IAsyncEnumerable GetEntries( .LoadWith(e => e.Components) .AsQueryable() .SingleOrDefaultAsync(e => e.Id == id); + entry?.ApplySortOrder(); return entry; } @@ -333,6 +336,7 @@ private IEnumerable CreateEntryChanges(Entry entry, Dictionary CreateEntryChanges(Entry entry, Dictionary CreateSenseChanges(entry.Id, s)) + .SelectMany((s, i) => + { + if (s.Order != default) // we don't anticipate this being necessary, so we'll be strict for now + throw new InvalidOperationException("Order should not be provided when creating a sense"); + s.Order = i + 1; + return CreateSenseChanges(entry.Id, s); + }) .ToArrayAsync(), ..await ToComplexFormComponents(entry.Components).ToArrayAsync(), ..await ToComplexFormComponents(entry.ComplexForms).ToArrayAsync(), @@ -474,8 +487,12 @@ private async IAsyncEnumerable CreateSenseChanges(Guid entryId, Sense s return entry?.Senses.FirstOrDefault(s => s.Id == id); } - public async Task CreateSense(Guid entryId, Sense sense) + public async Task CreateSense(Guid entryId, Sense sense, BetweenPosition? between = null) { + if (sense.Order != default) // we don't anticipate this being necessary, so we'll be strict for now + throw new InvalidOperationException("Order should not be provided when creating a sense"); + + sense.Order = await OrderPicker.PickOrder(Senses.Where(s => s.EntryId == entryId), between); await dataModel.AddChanges(ClientId, await CreateSenseChanges(entryId, sense).ToArrayAsync()); return await dataModel.GetLatest(sense.Id) ?? throw new NullReferenceException(); } @@ -496,6 +513,12 @@ public async Task UpdateSense(Guid entryId, Sense before, Sense after) return await GetSense(entryId, after.Id) ?? throw new NullReferenceException("unable to find sense with id " + after.Id); } + public async Task MoveSense(Guid entryId, Guid senseId, BetweenPosition between) + { + var order = await OrderPicker.PickOrder(Senses.Where(s => s.EntryId == entryId), between); + await dataModel.AddChange(ClientId, new Changes.SetOrderChange(senseId, order)); + } + public async Task DeleteSense(Guid entryId, Guid senseId) { await dataModel.AddChange(ClientId, new DeleteChange(senseId)); @@ -551,5 +574,4 @@ public async Task DeleteExampleSentence(Guid entryId, Guid senseId, Guid example { await dataModel.AddChange(ClientId, new DeleteChange(exampleSentenceId)); } - } diff --git a/backend/FwLite/LcmCrdt/Json.cs b/backend/FwLite/LcmCrdt/Json.cs index 694d8c0f1..e12f0c14b 100644 --- a/backend/FwLite/LcmCrdt/Json.cs +++ b/backend/FwLite/LcmCrdt/Json.cs @@ -1,8 +1,10 @@ using System.Linq.Expressions; using System.Reflection; +using System.Text.Json.Serialization.Metadata; using LinqToDB; using LinqToDB.Common; using LinqToDB.SqlQuery; +using SIL.Harmony; namespace LcmCrdt; @@ -109,4 +111,11 @@ public static bool IsIndexerPropertyMethod(MethodInfo method) { return valueAccess(prop); } + + public static IJsonTypeInfoResolver MakeLcmCrdtExternalJsonTypeResolver(this CrdtConfig config) + { + var resolver = config.MakeJsonTypeResolver(); + resolver = resolver.AddExternalMiniLcmModifiers(); + return resolver; + } } diff --git a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs index 9b1495579..cb540f7df 100644 --- a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs +++ b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs @@ -1,4 +1,3 @@ -using System.Linq.Expressions; using System.Reflection; using System.Text.Json; using System.Text.Json.Serialization.Metadata; @@ -21,7 +20,6 @@ using Microsoft.Extensions.Options; using MiniLcm.Validators; using Refit; -using SIL.Harmony.Db; namespace LcmCrdt; @@ -46,12 +44,12 @@ public static IServiceCollection AddLcmCrdtClient(this IServiceCollection servic services.AddSingleton(); services.AddHttpClient(); - services.AddSingleton(provider => new RefitSettings + services.AddSingleton(provider => new RefitSettings { ContentSerializer = new SystemTextJsonContentSerializer(new(JsonSerializerDefaults.Web) { TypeInfoResolver = provider.GetRequiredService>().Value - .MakeJsonTypeResolver() + .MakeLcmCrdtExternalJsonTypeResolver() }) }); services.AddSingleton(); @@ -190,7 +188,8 @@ public static void ConfigureCrdt(CrdtConfig config) .Add() .Add() .Add() - .Add(); + .Add() + .Add>(); } public static Type[] AllChangeTypes() diff --git a/backend/FwLite/LcmCrdt/OrderPicker.cs b/backend/FwLite/LcmCrdt/OrderPicker.cs new file mode 100644 index 000000000..e044ee110 --- /dev/null +++ b/backend/FwLite/LcmCrdt/OrderPicker.cs @@ -0,0 +1,38 @@ +using Microsoft.EntityFrameworkCore; +using MiniLcm.SyncHelpers; + +namespace LcmCrdt; + +public static class OrderPicker +{ + public static async Task PickOrder(IQueryable siblings, BetweenPosition? between = null) where T : class, IOrderable + { + // a common case that we can optimize by not querying whole objects + if (between is null or { Previous: null, Next: null }) + { + var currMaxOrder = await siblings.Select(s => s.Order).DefaultIfEmpty().MaxAsync(); + return currMaxOrder + 1; + } + + var items = await siblings.ToListAsync(); + var previousId = between?.Previous; + var nextId = between?.Next; + var previous = previousId is not null ? items.Find(item => item.Id == previousId) : null; + var next = nextId is not null ? items.Find(item => item.Id == nextId) : null; + + // There are various things we chould check for such as whether + // previous.Order + 1 actually puts it after previous (i.e. there isn't an item at previous.Order + <1) + // but even if that were the case, there's about a 50/50 chance that that's what actually should happen. + // So, overthinking it is probably not valuable. + return (previous, next) switch + { + // another user deleted items in the meantime? + (null, null) => items.Select(s => s.Order).DefaultIfEmpty().Max() + 1, + (_, null) => previous.Order + 1, + (null, _) => next.Order - 1, + // If the next item has been shifted previous the previous item, then between is likely not the actual intent, + // so we revert to only using previous + _ => previous.Order < next.Order ? (previous.Order + next.Order) / 2 : previous.Order + 1, + }; + } +} diff --git a/backend/FwLite/LcmCrdt/QueryHelpers.cs b/backend/FwLite/LcmCrdt/QueryHelpers.cs new file mode 100644 index 000000000..b2564ba58 --- /dev/null +++ b/backend/FwLite/LcmCrdt/QueryHelpers.cs @@ -0,0 +1,22 @@ +namespace LcmCrdt; + +public static class QueryHelpers +{ + public static void ApplySortOrder(this Entry entry) + { + entry.Senses.ApplySortOrder(); + } + + public static void ApplySortOrder(this List items) where T : IOrderable + { + items.Sort((x, y) => + { + var result = x.Order.CompareTo(y.Order); + if (result == 0) + { + result = x.Id.CompareTo(y.Id); + } + return result; + }); + } +} diff --git a/backend/FwLite/LocalWebApp/LocalAppKernel.cs b/backend/FwLite/LocalWebApp/LocalAppKernel.cs index 4b375f243..5903f4f35 100644 --- a/backend/FwLite/LocalWebApp/LocalAppKernel.cs +++ b/backend/FwLite/LocalWebApp/LocalAppKernel.cs @@ -1,16 +1,11 @@ -using System.Text.Json; -using SIL.Harmony; -using FwLiteProjectSync; -using FwDataMiniLcmBridge; +using SIL.Harmony; using FwLiteShared; using FwLiteShared.Auth; -using FwLiteShared.Sync; using LcmCrdt; using LocalWebApp.Services; using Microsoft.AspNetCore.Http.Json; using Microsoft.AspNetCore.SignalR; using Microsoft.Extensions.Options; -using Refit; namespace LocalWebApp; @@ -27,13 +22,13 @@ public static IServiceCollection AddLocalAppServices(this IServiceCollection ser services.AddOptions().PostConfigure>((jsonOptions, crdtConfig) => { - jsonOptions.SerializerOptions.TypeInfoResolver = crdtConfig.Value.MakeJsonTypeResolver(); + jsonOptions.SerializerOptions.TypeInfoResolver = crdtConfig.Value.MakeLcmCrdtExternalJsonTypeResolver(); }); services.AddOptions().PostConfigure>( (jsonOptions, crdtConfig) => { - jsonOptions.PayloadSerializerOptions.TypeInfoResolver = crdtConfig.Value.MakeJsonTypeResolver(); + jsonOptions.PayloadSerializerOptions.TypeInfoResolver = crdtConfig.Value.MakeLcmCrdtExternalJsonTypeResolver(); }); return services; } diff --git a/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/OrderableOverride.cs b/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/OrderableOverride.cs new file mode 100644 index 000000000..34e02e836 --- /dev/null +++ b/backend/FwLite/MiniLcm.Tests/AutoFakerHelpers/OrderableOverride.cs @@ -0,0 +1,21 @@ +using Soenneker.Utils.AutoBogus.Context; +using Soenneker.Utils.AutoBogus.Generators; + +namespace MiniLcm.Tests.AutoFakerHelpers; + +public class OrderableOverride : AutoFakerGeneratorOverride +{ + public override bool CanOverride(AutoFakerContext context) + { + return context.GenerateType.IsAssignableTo(typeof(IOrderable)); + } + + public override void Generate(AutoFakerOverrideContext context) + { + if (context.Instance is IOrderable obj) + { + // Order should never be predefined + obj.Order = 0; + } + } +} diff --git a/backend/FwLite/MiniLcm.Tests/DiffCollectionTests.cs b/backend/FwLite/MiniLcm.Tests/DiffCollectionTests.cs new file mode 100644 index 000000000..1a1e9dfb0 --- /dev/null +++ b/backend/FwLite/MiniLcm.Tests/DiffCollectionTests.cs @@ -0,0 +1,202 @@ +using MiniLcm.SyncHelpers; + +namespace MiniLcm.Tests; + +public class DiffCollectionTests +{ + [Fact] + public async Task MatchingCollections_NoChangesAreGenerated() + { + // arrange + var value1 = new TestOrderable(1, Guid.NewGuid()); + var value2 = new TestOrderable(2, Guid.NewGuid()); + + // act + var (changeCount, diffOperations, replacements) = await Diff([value1, value2], [value1, value2]); + + // assert + changeCount.Should().Be(0); + diffOperations.Should().BeEmpty(); + replacements.Should().BeEquivalentTo([(value1, value1), (value2, value2)]); + } + + [Fact] + public async Task AddedValues() + { + // arrange + var value1 = new TestOrderable(1, Guid.NewGuid()); + var value2 = new TestOrderable(2, Guid.NewGuid()); + var value3 = new TestOrderable(3, Guid.NewGuid()); + + // act + var (changeCount, diffOperations, replacements) = await Diff([value1], [value2, value1, value3]); + + // assert + changeCount.Should().Be(2); + replacements.Should().BeEquivalentTo([(value1, value1)]); + diffOperations.Should().BeEquivalentTo([ + Add(value2, Between(null, value1)), + Add(value3, Between(value1, null)), + ], options => options.WithStrictOrdering()); + } + + [Fact] + public async Task RemovedValues() + { + // arrange + var value1 = new TestOrderable(1, Guid.NewGuid()); + var value2 = new TestOrderable(2, Guid.NewGuid()); + var value3 = new TestOrderable(3, Guid.NewGuid()); + + // act + var (changeCount, diffOperations, replacements) = await Diff([value2, value1, value3], [value1]); + + // assert + changeCount.Should().Be(2); + replacements.Should().BeEquivalentTo([(value1, value1)]); + diffOperations.Should().BeEquivalentTo([ + Remove(value3), + Remove(value2), + ], options => options.WithStrictOrdering()); + } + + [Fact] + public async Task SwappedValues() + { + // arrange + var value1 = new TestOrderable(1, Guid.NewGuid()); + var value2 = new TestOrderable(2, Guid.NewGuid()); + + // act + var (changeCount, diffOperations, replacements) = await Diff([value1, value2], [value2, value1]); + + // assert + changeCount.Should().Be(1); + replacements.Should().BeEquivalentTo([(value1, value1), (value2, value2)]); + diffOperations.Should().BeEquivalentTo([ + Move(value1, Between(value2, null)), + ]); + } + + public static IEnumerable CollectionDiffTestCaseData() + { + var _1 = new TestOrderable(1, Guid.NewGuid()); + var _2 = new TestOrderable(2, Guid.NewGuid()); + var _3 = new TestOrderable(3, Guid.NewGuid()); + var _4 = new TestOrderable(4, Guid.NewGuid()); + yield return [new CollectionDiffTestCase + { + OldValues = [_1, _2, _3], + NewValues = [_3, _2, _1], + ExpectedOperations = [ + Move(_2, Between(_3, null)), + Move(_1, Between(_2, null)), + ], + }]; + yield return [new CollectionDiffTestCase + { + OldValues = [_1, _2, _3, _4], + NewValues = [_1, _4, _2, _3], + ExpectedOperations = [ + Move(_4, Between(_1, _2)), + ], + }]; + yield return [new CollectionDiffTestCase + { + OldValues = [_1, _2, _3, _4], + NewValues = [_2, _1, _4, _3], + ExpectedOperations = [ // When only 4, moving the 2 outsides to middle is represented slightly differently: + Move(_1, Between(_2, _4)), + Move(_3, Between(_4, null)), + ], + }]; + + var _5 = new TestOrderable(5, Guid.NewGuid()); + var _6 = new TestOrderable(6, Guid.NewGuid()); + yield return [new CollectionDiffTestCase + { + OldValues = [_1, _2, _3, _4, _5, _6], + NewValues = [_2, _3, _1, _6, _4, _5], + ExpectedOperations = [ // When 6+, moving the 2 outsides to middle is represented as such: + Move(_1, Between(_3, _4)), + Move(_6, Between(_1, _4)), + ], + }]; + + var _7 = new TestOrderable(7, Guid.NewGuid()); + var _8 = new TestOrderable(8, Guid.NewGuid()); + yield return [new CollectionDiffTestCase + { + OldValues = [_1, _2, _3, _4, _5], + NewValues = [_6, _8, _4, _2, _7], + ExpectedOperations = [ + Remove(_5), + Remove(_3), + Remove(_1), + Add(_6, Between(null, _4)), // (not next: _8, because _8 is not "stable") + Add(_8, Between(_6, _4)), + Move(_2, Between(_4, null)), + Add(_7, Between(_2, null)), + ], + }]; + } + + [Theory] + [MemberData(nameof(CollectionDiffTestCaseData))] + public async Task DiffTests(CollectionDiffTestCase testCase) + { + // act + var (changeCount, diffOperations, replacements) = await Diff(testCase.OldValues, testCase.NewValues); + + // assert + changeCount.Should().Be(testCase.ExpectedOperations.Count); + diffOperations.Should().BeEquivalentTo(testCase.ExpectedOperations, options => options.WithStrictOrdering()); + } + + private static async Task<( + int changeCount, + List DiffOperations, + List<(TestOrderable before, TestOrderable after)> Replacements + )> Diff(TestOrderable[] oldValues, TestOrderable[] newValues) + { + var diffApi = new TestOrderableDiffApi(oldValues); + var changeCount = await DiffCollection.DiffOrderable(oldValues, newValues, diffApi); + // verify that the operations did in fact transform the old collection into the new collection + diffApi.Current.Should().BeEquivalentTo(newValues, options => options.WithStrictOrdering()); + var expectedReplacements = oldValues.Join(newValues, o => o.Id, o => o.Id, (o, n) => (o, n)).ToList(); + diffApi.Replacements.Should().BeEquivalentTo(expectedReplacements, options => options.WithStrictOrdering()); + return (changeCount, diffApi.DiffOperations, diffApi.Replacements); + } + + private static BetweenPosition Between(TestOrderable? previous, TestOrderable? next) + { + return Between(previous?.Id, next?.Id); + } + + private static BetweenPosition Between(Guid? previous = null, Guid? next = null) + { + return new BetweenPosition(previous, next); + } + + private static CollectionDiffOperation Move(TestOrderable value, BetweenPosition between) + { + return new CollectionDiffOperation(value, PositionDiffKind.Move, between); + } + + private static CollectionDiffOperation Add(TestOrderable value, BetweenPosition between) + { + return new CollectionDiffOperation(value, PositionDiffKind.Add, between); + } + + private static CollectionDiffOperation Remove(TestOrderable value) + { + return new CollectionDiffOperation(value, PositionDiffKind.Remove); + } +} + +public class CollectionDiffTestCase +{ + public required TestOrderable[] OldValues { get; init; } + public required TestOrderable[] NewValues { get; init; } + public required List ExpectedOperations { get; init; } +} diff --git a/backend/FwLite/MiniLcm.Tests/MiniLcm.Tests.csproj b/backend/FwLite/MiniLcm.Tests/MiniLcm.Tests.csproj index 1e5de3a28..9c6ac6602 100644 --- a/backend/FwLite/MiniLcm.Tests/MiniLcm.Tests.csproj +++ b/backend/FwLite/MiniLcm.Tests/MiniLcm.Tests.csproj @@ -21,6 +21,7 @@ + diff --git a/backend/FwLite/MiniLcm.Tests/MiniLcmTestBase.cs b/backend/FwLite/MiniLcm.Tests/MiniLcmTestBase.cs index 1b5601be2..b71c57a53 100644 --- a/backend/FwLite/MiniLcm.Tests/MiniLcmTestBase.cs +++ b/backend/FwLite/MiniLcm.Tests/MiniLcmTestBase.cs @@ -1,15 +1,21 @@ using MiniLcm.Tests.AutoFakerHelpers; using Soenneker.Utils.AutoBogus; +using Soenneker.Utils.AutoBogus.Config; namespace MiniLcm.Tests; public abstract class MiniLcmTestBase : IAsyncLifetime { - - protected static readonly AutoFaker AutoFaker = new(builder => - builder.WithOverride(new MultiStringOverride(["en"])) - .WithOverride(new ObjectWithIdOverride()) - ); + protected static readonly AutoFaker AutoFaker = new(new AutoFakerConfig() + { + RepeatCount = 5, + Overrides = + [ + new MultiStringOverride(["en"]), + new ObjectWithIdOverride(), + new OrderableOverride(), + ] + }); protected IMiniLcmApi Api = null!; protected abstract Task NewApi(); diff --git a/backend/FwLite/MiniLcm.Tests/TestOrderableDiffApi.cs b/backend/FwLite/MiniLcm.Tests/TestOrderableDiffApi.cs new file mode 100644 index 000000000..230ec70a5 --- /dev/null +++ b/backend/FwLite/MiniLcm.Tests/TestOrderableDiffApi.cs @@ -0,0 +1,160 @@ +using MiniLcm.SyncHelpers; + +namespace MiniLcm.Tests; + +public class TestOrderableDiffApiTests +{ + [Fact] + public async Task Add() + { + // arrange + var value1 = new TestOrderable(1, Guid.NewGuid()); + var value2 = new TestOrderable(2, Guid.NewGuid()); + + // act + var diffApi = new TestOrderableDiffApi([value1]); + var position = new BetweenPosition(value1.Id, null); + await diffApi.Add(value2, position); + + // assert + diffApi.DiffOperations.Should().BeEquivalentTo([ + new CollectionDiffOperation(value2, PositionDiffKind.Add, position) + ]); + diffApi.Replacements.Should().BeEquivalentTo(Array.Empty<(TestOrderable, TestOrderable)>()); + diffApi.Current.Should().BeEquivalentTo([value1, value2]); + } + + [Fact] + public async Task Remove() + { + // arrange + var value1 = new TestOrderable(1, Guid.NewGuid()); + var value2 = new TestOrderable(2, Guid.NewGuid()); + + // act + var diffApi = new TestOrderableDiffApi([value1, value2]); + await diffApi.Remove(value1); + + // assert + diffApi.DiffOperations.Should().BeEquivalentTo([ + new CollectionDiffOperation(value1, PositionDiffKind.Remove) + ]); + diffApi.Replacements.Should().BeEquivalentTo(Array.Empty<(TestOrderable, TestOrderable)>()); + diffApi.Current.Should().BeEquivalentTo([value2]); + } + + [Fact] + public async Task Move() + { + // arrange + var value1 = new TestOrderable(1, Guid.NewGuid()); + var value2 = new TestOrderable(2, Guid.NewGuid()); + var value3 = new TestOrderable(3, Guid.NewGuid()); + + // act + var diffApi = new TestOrderableDiffApi([value1, value2, value3]); + var position = new BetweenPosition(value1.Id, value2.Id); + await diffApi.Move(value3, position); + + // assert + diffApi.DiffOperations.Should().BeEquivalentTo([ + new CollectionDiffOperation(value3, PositionDiffKind.Move, position) + ]); + diffApi.Replacements.Should().BeEquivalentTo(Array.Empty<(TestOrderable, TestOrderable)>()); + diffApi.Current.Should().BeEquivalentTo([value1, value3, value2]); + } + + [Fact] + public async Task Replace() + { + // arrange + var oldValue = new TestOrderable(1, Guid.NewGuid()); + var newValue = new TestOrderable(2, oldValue.Id); + + // act + var diffApi = new TestOrderableDiffApi([oldValue]); + await diffApi.Replace(oldValue, newValue); + + // assert + diffApi.DiffOperations.Should().BeEquivalentTo(Array.Empty()); + diffApi.Replacements.Should().BeEquivalentTo([(oldValue, newValue)]); + diffApi.Current.Should().BeEquivalentTo([newValue]); + } +} + +public class TestOrderableDiffApi(TestOrderable[] before) : IOrderableCollectionDiffApi +{ + public List Current { get; } = [.. before]; + public List DiffOperations = []; + public List<(TestOrderable before, TestOrderable after)> Replacements = []; + + public Task Add(TestOrderable value, BetweenPosition between) + { + DiffOperations.Add(new CollectionDiffOperation(value, PositionDiffKind.Add, between)); + return AddInternal(value, between); + } + + private Task AddInternal(TestOrderable value, BetweenPosition between) + { + if (between.Previous is not null) + { + var previousIndex = Current.FindIndex(v => v.Id == between.Previous); + Current.Insert(previousIndex + 1, value); + } + else if (between.Next is not null) + { + var nextIndex = Current.FindIndex(v => v.Id == between.Next); + Current.Insert(nextIndex, value); + } + else + { + Current.Add(value); + } + return Task.FromResult(1); + } + + public Task Remove(TestOrderable value) + { + DiffOperations.Add(new CollectionDiffOperation(value, PositionDiffKind.Remove)); + return RemoveInternal(value); + } + + public Task RemoveInternal(TestOrderable value) + { + var removeCount = Current.RemoveAll(v => v.Id == value.Id); + removeCount.Should().Be(1); + return Task.FromResult(1); + } + + public async Task Move(TestOrderable value, BetweenPosition between) + { + DiffOperations.Add(new CollectionDiffOperation(value, PositionDiffKind.Move, between)); + await RemoveInternal(value); + await AddInternal(value, between); + return 1; + } + + public Task Replace(TestOrderable before, TestOrderable after) + { + Replacements.Add((before, after)); + before.Id.Should().Be(after.Id); + Current[Current.FindIndex(v => v.Id == before.Id)] = after; + try + { + before.Should().BeEquivalentTo(after); + return Task.FromResult(0); + } + catch + { + return Task.FromResult(1); + } + } +} + +public record CollectionDiffOperation(TestOrderable Value, PositionDiffKind Kind, BetweenPosition? Between = null); + +public class TestOrderable(double order, Guid id) : IOrderable +{ + public Guid Id { get; set; } = id; + public double Order { get; set; } = order; +} diff --git a/backend/FwLite/MiniLcm.Tests/UpdateEntryTestsBase.cs b/backend/FwLite/MiniLcm.Tests/UpdateEntryTestsBase.cs index 73e978afd..1c7e21886 100644 --- a/backend/FwLite/MiniLcm.Tests/UpdateEntryTestsBase.cs +++ b/backend/FwLite/MiniLcm.Tests/UpdateEntryTestsBase.cs @@ -1,10 +1,14 @@ -namespace MiniLcm.Tests; +using System.Globalization; + +namespace MiniLcm.Tests; public abstract class UpdateEntryTestsBase : MiniLcmTestBase { protected readonly Guid Entry1Id = new Guid("a3f5aa5a-578f-4181-8f38-eaaf27f01f1c"); protected readonly Guid Entry2Id = new Guid("2de6c334-58fa-4844-b0fd-0bc2ce4ef835"); + protected virtual bool ApiUsesImplicitOrdering => false; + public override async Task InitializeAsync() { await base.InitializeAsync(); @@ -102,4 +106,56 @@ public async Task UpdateEntry_CanUseSameVersionMultipleTimes() updatedEntry2.Should().BeEquivalentTo(update2, options => options.Excluding(e => e.LexemeForm)); } + + [Theory] + [InlineData("a,b", "a,b,c,d", "1,2,3,4")] // append + [InlineData("a,2", "c,a,b", "0,1,2")] // single prepend + [InlineData("a,b", "d,c,a,b", "0,0.5,1,2")] // multi prepend + [InlineData("a,b,c,d", "d,a,b,c", "0,1,2,3")] // move to back + [InlineData("a,b,c,d", "b,c,d,a", "2,3,4,5")] // move to front + [InlineData("a,b,c,d,e", "a,b,e,c,d", "1,2,2.5,3,4")] // move to middle + [InlineData("a,b,c", "c,b,a", "3,4,5")] // reverse + [InlineData("a,b,c,d", "d,b,c,a", "1,2,3,4")] // swap + public async Task UpdateEntry_CanReorderSenses(string before, string after, string expectedOrderValues) + { + // arrange + var entryId = Guid.NewGuid(); + var senseIds = before.Split(',').Concat(after.Split(',')).Distinct() + .ToDictionary(i => i, _ => Guid.NewGuid()); + var beforeSenses = before.Split(',').Select(i => new Sense() { Id = senseIds[i], EntryId = entryId, Gloss = { { "en", i } } }).ToList(); + var afterSenses = after.Split(',').Select(i => new Sense() { Id = senseIds[i], EntryId = entryId, Gloss = { { "en", i } } }).ToList(); + + var beforeEntry = await Api.CreateEntry(new() + { + Id = entryId, + LexemeForm = { { "en", "order" } }, + Senses = beforeSenses, + }); + + var afterEntry = beforeEntry!.Copy(); + afterEntry.Senses = afterSenses; + + // sanity checks + beforeEntry.Senses.Should().BeEquivalentTo(beforeSenses, options => options.WithStrictOrdering()); + if (!ApiUsesImplicitOrdering) + { + beforeEntry.Senses.Select(s => s.Order).Should() + .BeEquivalentTo(Enumerable.Range(1, beforeSenses.Count), options => options.WithStrictOrdering()); + } + + // act + await Api.UpdateEntry(beforeEntry, afterEntry); + var actual = await Api.GetEntry(afterEntry.Id); + + // assert + actual.Should().NotBeNull(); + actual.Senses.Should().BeEquivalentTo(afterEntry.Senses, + options => options.WithStrictOrdering().Excluding(s => s.Order)); + + if (!ApiUsesImplicitOrdering) + { + var actualOrderValues = string.Join(',', actual.Senses.Select(s => s.Order.ToString(CultureInfo.GetCultureInfo("en-US")))); + actualOrderValues.Should().Be(expectedOrderValues); + } + } } diff --git a/backend/FwLite/MiniLcm/Attributes/MiniLcmInternalAttribute.cs b/backend/FwLite/MiniLcm/Attributes/MiniLcmInternalAttribute.cs new file mode 100644 index 000000000..e577ac38f --- /dev/null +++ b/backend/FwLite/MiniLcm/Attributes/MiniLcmInternalAttribute.cs @@ -0,0 +1,8 @@ +namespace MiniLcm.Attributes; +/// +/// For now this just controls whether the property should be serialized or not when leaving dotnet +/// +[AttributeUsage(AttributeTargets.Property | AttributeTargets.Field, Inherited = true)] +public class MiniLcmInternalAttribute : Attribute +{ +} diff --git a/backend/FwLite/MiniLcm/IMiniLcmApi.cs b/backend/FwLite/MiniLcm/IMiniLcmApi.cs index 6beb6db07..7c6179282 100644 --- a/backend/FwLite/MiniLcm/IMiniLcmApi.cs +++ b/backend/FwLite/MiniLcm/IMiniLcmApi.cs @@ -1,3 +1,7 @@ -namespace MiniLcm; +using MiniLcm.Models; -public interface IMiniLcmApi: IMiniLcmReadApi, IMiniLcmWriteApi; +namespace MiniLcm; + +public interface IMiniLcmApi : IMiniLcmReadApi, IMiniLcmWriteApi +{ +} diff --git a/backend/FwLite/MiniLcm/IMiniLcmWriteApi.cs b/backend/FwLite/MiniLcm/IMiniLcmWriteApi.cs index b41040cb1..c9e74442b 100644 --- a/backend/FwLite/MiniLcm/IMiniLcmWriteApi.cs +++ b/backend/FwLite/MiniLcm/IMiniLcmWriteApi.cs @@ -1,5 +1,6 @@ using System.Linq.Expressions; using MiniLcm.Models; +using MiniLcm.SyncHelpers; using SystemTextJsonPatch; namespace MiniLcm; @@ -46,9 +47,10 @@ Task UpdateWritingSystem(WritingSystemId id, #endregion #region Sense - Task CreateSense(Guid entryId, Sense sense); + Task CreateSense(Guid entryId, Sense sense, BetweenPosition? position = null); Task UpdateSense(Guid entryId, Guid senseId, UpdateObjectInput update); Task UpdateSense(Guid entryId, Sense before, Sense after); + Task MoveSense(Guid entryId, Guid senseId, BetweenPosition position); Task DeleteSense(Guid entryId, Guid senseId); Task AddSemanticDomainToSense(Guid senseId, SemanticDomain semanticDomain); Task RemoveSemanticDomainFromSense(Guid senseId, Guid semanticDomainId); diff --git a/backend/FwLite/MiniLcm/MiniLcm.csproj b/backend/FwLite/MiniLcm/MiniLcm.csproj index 3a89c7e9e..9da5fbb1d 100644 --- a/backend/FwLite/MiniLcm/MiniLcm.csproj +++ b/backend/FwLite/MiniLcm/MiniLcm.csproj @@ -9,6 +9,7 @@ + diff --git a/backend/FwLite/MiniLcm/MiniLcmJson.cs b/backend/FwLite/MiniLcm/MiniLcmJson.cs new file mode 100644 index 000000000..64fe276be --- /dev/null +++ b/backend/FwLite/MiniLcm/MiniLcmJson.cs @@ -0,0 +1,24 @@ +using System.Text.Json.Serialization.Metadata; +using MiniLcm.Attributes; + +namespace MiniLcm; + +public static class MiniLcmJson +{ + public static IJsonTypeInfoResolver AddExternalMiniLcmModifiers(this IJsonTypeInfoResolver resolver) + { + resolver = resolver.WithAddedModifier(IgnoreInternalMiniLcmProperties); + return resolver; + } + + private static void IgnoreInternalMiniLcmProperties(JsonTypeInfo typeInfo) + { + foreach (var prop in typeInfo.Properties) + { + if (prop.AttributeProvider?.IsDefined(typeof(MiniLcmInternalAttribute), inherit: true) ?? false) + { + prop.ShouldSerialize = (_, _) => false; + } + } + } +} diff --git a/backend/FwLite/MiniLcm/Models/Entry.cs b/backend/FwLite/MiniLcm/Models/Entry.cs index cabb8c9bb..8123b0c72 100644 --- a/backend/FwLite/MiniLcm/Models/Entry.cs +++ b/backend/FwLite/MiniLcm/Models/Entry.cs @@ -10,7 +10,7 @@ public record Entry : IObjectWithId public virtual MultiString CitationForm { get; set; } = new(); public virtual MultiString LiteralMeaning { get; set; } = new(); - public virtual IList Senses { get; set; } = []; + public virtual List Senses { get; set; } = []; public virtual MultiString Note { get; set; } = new(); diff --git a/backend/FwLite/MiniLcm/Models/IOrderable.cs b/backend/FwLite/MiniLcm/Models/IOrderable.cs new file mode 100644 index 000000000..994e2e31c --- /dev/null +++ b/backend/FwLite/MiniLcm/Models/IOrderable.cs @@ -0,0 +1,7 @@ +namespace MiniLcm.Models; + +public interface IOrderable +{ + Guid Id { get; } + double Order { get; set; } +} diff --git a/backend/FwLite/MiniLcm/Models/Sense.cs b/backend/FwLite/MiniLcm/Models/Sense.cs index 2117e2738..d774a0fae 100644 --- a/backend/FwLite/MiniLcm/Models/Sense.cs +++ b/backend/FwLite/MiniLcm/Models/Sense.cs @@ -1,8 +1,12 @@ -namespace MiniLcm.Models; +using MiniLcm.Attributes; -public class Sense : IObjectWithId +namespace MiniLcm.Models; + +public class Sense : IObjectWithId, IOrderable { public virtual Guid Id { get; set; } + [MiniLcmInternal] + public double Order { get; set; } public DateTimeOffset? DeletedAt { get; set; } public Guid EntryId { get; set; } public virtual MultiString Definition { get; set; } = new(); @@ -33,6 +37,7 @@ public IObjectWithId Copy() { Id = Id, EntryId = EntryId, + Order = Order, DeletedAt = DeletedAt, Definition = Definition.Copy(), Gloss = Gloss.Copy(), diff --git a/backend/FwLite/MiniLcm/SyncHelpers/ComplexFormTypeSync.cs b/backend/FwLite/MiniLcm/SyncHelpers/ComplexFormTypeSync.cs index e7ae771cf..05a92442f 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/ComplexFormTypeSync.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/ComplexFormTypeSync.cs @@ -9,21 +9,10 @@ public static async Task Sync(ComplexFormType[] afterComplexFormTypes, ComplexFormType[] beforeComplexFormTypes, IMiniLcmApi api) { - return await DiffCollection.Diff(api, + return await DiffCollection.Diff( beforeComplexFormTypes, afterComplexFormTypes, - complexFormType => complexFormType.Id, - static async (api, afterComplexFormType) => - { - await api.CreateComplexFormType(afterComplexFormType); - return 1; - }, - static async (api, beforeComplexFormType) => - { - await api.DeleteComplexFormType(beforeComplexFormType.Id); - return 1; - }, - static (api, beforeComplexFormType, afterComplexFormType) => Sync(beforeComplexFormType, afterComplexFormType, api)); + new ComplexFormTypesDiffApi(api)); } public static async Task Sync(ComplexFormType before, @@ -44,4 +33,24 @@ public static async Task Sync(ComplexFormType before, if (patchDocument.Operations.Count == 0) return null; return new UpdateObjectInput(patchDocument); } + + private class ComplexFormTypesDiffApi(IMiniLcmApi api) : ObjectWithIdCollectionDiffApi + { + public override async Task Add(ComplexFormType afterComplexFormType) + { + await api.CreateComplexFormType(afterComplexFormType); + return 1; + } + + public override async Task Remove(ComplexFormType beforeComplexFormType) + { + await api.DeleteComplexFormType(beforeComplexFormType.Id); + return 1; + } + + public override Task Replace(ComplexFormType beforeComplexFormType, ComplexFormType afterComplexFormType) + { + return Sync(beforeComplexFormType, afterComplexFormType, api); + } + } } diff --git a/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs b/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs index 33bda1e46..23d70249f 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs @@ -1,141 +1,240 @@ -using MiniLcm.Models; +using System.Collections.Immutable; +using System.Text.Json.JsonDiffPatch; +using System.Text.Json.JsonDiffPatch.Diffs; +using System.Text.Json.JsonDiffPatch.Diffs.Formatters; +using System.Text.Json.Nodes; +using MiniLcm.Models; namespace MiniLcm.SyncHelpers; +public abstract class CollectionDiffApi where TId : notnull +{ + public abstract Task Add(T value); + public virtual async Task<(int, T)> AddAndGet(T value) + { + var changes = await Add(value); + return (changes, value); + } + public abstract Task Remove(T value); + public abstract Task Replace(T before, T after); + public abstract TId GetId(T value); +} + +public abstract class ObjectWithIdCollectionDiffApi : CollectionDiffApi where T : IObjectWithId +{ + public override Guid GetId(T value) + { + return value.Id; + } +} + +public interface IOrderableCollectionDiffApi where T : IOrderable +{ + Task Add(T value, BetweenPosition between); + Task Remove(T value); + Task Move(T value, BetweenPosition between); + Task Replace(T before, T after); +} + public static class DiffCollection { /// /// Diffs a list, for new items calls add, it will then call update for the item returned from the add, using that as the before item for the replace call /// - /// - /// - /// - /// - /// api, value, return value to be used as the before item for the replace call - /// - /// api, before, after is the parameter order - /// - /// - /// public static async Task DiffAddThenUpdate( - IMiniLcmApi api, IList before, IList after, - Func identity, - Func> add, - Func> remove, - Func> replace) where TId : notnull + CollectionDiffApi diffApi) where TId : notnull { var changes = 0; - var afterEntriesDict = after.ToDictionary(identity); + var afterEntriesDict = after.ToDictionary(diffApi.GetId); foreach (var beforeEntry in before) { - if (afterEntriesDict.TryGetValue(identity(beforeEntry), out var afterEntry)) + if (afterEntriesDict.TryGetValue(diffApi.GetId(beforeEntry), out var afterEntry)) { - changes += await replace(api, beforeEntry, afterEntry); + changes += await diffApi.Replace(beforeEntry, afterEntry); } else { - changes += await remove(api, beforeEntry); + changes += await diffApi.Remove(beforeEntry); } - afterEntriesDict.Remove(identity(beforeEntry)); + afterEntriesDict.Remove(diffApi.GetId(beforeEntry)); } var postAddUpdates = new List<(T created, T after)>(afterEntriesDict.Values.Count); foreach (var value in afterEntriesDict.Values) { - changes++; - postAddUpdates.Add((await add(api, value), value)); + var (change, created) = await diffApi.AddAndGet(value); + changes += change; + postAddUpdates.Add((created, value)); } - foreach ((T createdItem, T afterItem) in postAddUpdates) + foreach ((var createdItem, var afterItem) in postAddUpdates) { //todo this may do a lot more work than it needs to, eg sense will be created during add, but they will be checked again here when we know they didn't change - await replace(api, createdItem, afterItem); + await diffApi.Replace(createdItem, afterItem); } return changes; } - /// - /// - /// - /// - /// - /// - /// - /// - /// - /// api, before, after is the parameter order - /// - /// - /// public static async Task Diff( - IMiniLcmApi api, IList before, IList after, - Func identity, - Func> add, - Func> remove, - Func> replace) where TId : notnull + CollectionDiffApi diffApi) where TId : notnull { var changes = 0; - var afterEntriesDict = after.ToDictionary(identity); + var afterEntriesDict = after.ToDictionary(diffApi.GetId); foreach (var beforeEntry in before) { - if (afterEntriesDict.TryGetValue(identity(beforeEntry), out var afterEntry)) + if (afterEntriesDict.TryGetValue(diffApi.GetId(beforeEntry), out var afterEntry)) { - changes += await replace(api, beforeEntry, afterEntry); + changes += await diffApi.Replace(beforeEntry, afterEntry); } else { - changes += await remove(api, beforeEntry); + changes += await diffApi.Remove(beforeEntry); } - afterEntriesDict.Remove(identity(beforeEntry)); + afterEntriesDict.Remove(diffApi.GetId(beforeEntry)); } foreach (var value in afterEntriesDict.Values) { - changes += await add(api, value); + changes += await diffApi.Add(value); } return changes; } - public static async Task Diff( - IMiniLcmApi api, + public static async Task DiffOrderable( IList before, IList after, - Func> add, - Func> remove, - Func> replace) where T : IObjectWithId + IOrderableCollectionDiffApi diffApi) where T : IOrderable + { + var changes = 0; + + var positionDiffs = DiffPositions(before, after); + if (positionDiffs is not null) + { + // The positive keys in positionDiffs are the indexes of added or moved items. I.e. they're the unstable ones. + // Deleted items are given a negative index. So, they aren't picked up here. They also don't exist in the after list, so they're not relevant. + var stableIds = after.Where((_, i) => !positionDiffs.ContainsKey(i)) + .Select(item => item.Id) + .ToHashSet(); + + foreach (var (_, diff) in positionDiffs) + { + switch (diff.Kind) + { + case PositionDiffKind.Move: + var movedEntry = after[diff.Index]; + var between = GetStableBetween(diff.Index, after, stableIds); + changes += await diffApi.Move(movedEntry, between); + stableIds.Add(movedEntry.Id); + break; + case PositionDiffKind.Remove: + changes += await diffApi.Remove(before[diff.Index]); + break; + case PositionDiffKind.Add: + var addedEntry = after[diff.Index]; + between = GetStableBetween(diff.Index, after, stableIds); + changes += await diffApi.Add(addedEntry, between); + stableIds.Add(addedEntry.Id); + break; + default: + throw new InvalidOperationException($"Unsupported position diff kind {diff.Kind}"); + } + } + } + + var afterEntriesDict = after.ToDictionary(entry => entry.Id); + foreach (var beforeEntry in before) + { + if (afterEntriesDict.TryGetValue(beforeEntry.Id, out var afterEntry)) + { + changes += await diffApi.Replace(beforeEntry, afterEntry); + } + } + + return changes; + } + + private static BetweenPosition GetStableBetween(int i, IList current, HashSet stable) where T : IOrderable { - return await Diff(api, before, after, t => t.Id, add, remove, replace); + T? beforeEntity = default; + T? afterEntity = default; + for (var j = i - 1; j >= 0; j--) + { + if (stable.Contains(current[j].Id)) + { + beforeEntity = current[j]; + break; + } + } + for (var j = i + 1; j < current.Count; j++) + { + if (stable.Contains(current[j].Id)) + { + afterEntity = current[j]; + break; + } + } + return new BetweenPosition(beforeEntity?.Id, afterEntity?.Id); } - public static async Task Diff( - IMiniLcmApi api, + private static ImmutableSortedDictionary? DiffPositions( IList before, - IList after, - Func add, - Func remove, - Func> replace) where T : IObjectWithId + IList after) where T : IOrderable { - return await Diff(api, - before, - after, - async (api, entry) => - { - await add(api, entry); - return 1; - }, - async (api, entry) => + var beforeJson = new JsonArray([.. before.Select(item => JsonValue.Create(item.Id))]); + var afterJson = new JsonArray([.. after.Select(item => JsonValue.Create(item.Id))]); + return JsonDiffPatcher.Diff(beforeJson, afterJson, DiffFormatter.Instance); + } + +#pragma warning disable IDE0072 // Add missing cases + /// + /// Formats Json array diffs into a list sorted by: + /// - deletes first (with arbitrary negative indexes) + /// - added and moved in order of new/current index (using current index) + /// + private class DiffFormatter : IJsonDiffDeltaFormatter> + { + public static readonly DiffFormatter Instance = new(); + + private DiffFormatter() { } + + public ImmutableSortedDictionary? Format(ref JsonDiffDelta delta, JsonNode? left) + { + if (delta.Kind == DeltaKind.None) return null; + + return delta.GetArrayChangeEnumerable().Select(change => { - await remove(api, entry); - return 1; - }, - async (api, beforeEntry, afterEntry) => await replace(api, beforeEntry, afterEntry)); + return change.Diff.Kind switch + { + DeltaKind.ArrayMove => new PositionDiff(change.Diff.GetNewIndex(), PositionDiffKind.Move), + DeltaKind.Added => new PositionDiff(change.Index, PositionDiffKind.Add), + DeltaKind.Deleted => new PositionDiff(change.Index, PositionDiffKind.Remove), + _ => throw new InvalidOperationException("Only array diff results are supported"), + }; + }).ToImmutableSortedDictionary(diff => diff.SortIndex, diff => diff); + } } +#pragma warning restore IDE0072 // Add missing cases } + +public enum PositionDiffKind { Add, Remove, Move } +public record PositionDiff(int Index, PositionDiffKind Kind) +{ + // Indexes for add and move operations represent final positions. + // I.e. the order of the diffs doesn't really have meaning, but rather the caller is expected to make sure that's where the item ends up. + // Also, final position indexes might not yet exist in the current list. + + // So, the easiest way to make sure the caller will be able to apply the diffs sequentially is to order them so that: + // - Deletes happens first + // - Adds and moves are then ordered by the new index (i.e. we work from front to back) + public int SortIndex => Kind == PositionDiffKind.Remove ? -Index - 1 : Index; +} + +public record BetweenPosition(Guid? Previous, Guid? Next); diff --git a/backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs b/backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs index c8f2db5a3..15dd6463a 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs @@ -10,21 +10,7 @@ public static async Task Sync(Entry[] afterEntries, Entry[] beforeEntries, IMiniLcmApi api) { - Func> add = static async (api, afterEntry) => - { - //create each entry without components. - //After each entry is created, then replace will be called to create those components - var entryWithoutEntryRefs = afterEntry.WithoutEntryRefs(); - await api.CreateEntry(entryWithoutEntryRefs); - return entryWithoutEntryRefs; - }; - Func> remove = static async (api, beforeEntry) => - { - await api.DeleteEntry(beforeEntry.Id); - return 1; - }; - Func> replace = static async (api, beforeEntry, afterEntry) => await Sync(afterEntry, beforeEntry, api); - return await DiffCollection.DiffAddThenUpdate(api, beforeEntries, afterEntries, entry => entry.Id, add, remove, replace); + return await DiffCollection.DiffAddThenUpdate(beforeEntries, afterEntries, new EntriesDiffApi(api)); } public static async Task Sync(Entry afterEntry, Entry beforeEntry, IMiniLcmApi api) @@ -51,61 +37,18 @@ private static async Task Sync(Guid entryId, IList beforeComplexFormTypes, IMiniLcmApi api) { - return await DiffCollection.Diff(api, + return await DiffCollection.Diff( beforeComplexFormTypes, afterComplexFormTypes, - complexFormType => complexFormType.Id, - async (api, afterComplexFormType) => - { - await api.AddComplexFormType(entryId, afterComplexFormType.Id); - return 1; - }, - async (api, beforeComplexFormType) => - { - await api.RemoveComplexFormType(entryId, beforeComplexFormType.Id); - return 1; - }, - //do nothing, complex form types are not editable, ignore any changes to them here - static (api, beforeComplexFormType, afterComplexFormType) => Task.FromResult(0)); + new ComplexFormTypesDiffApi(api, entryId)); } private static async Task Sync(IList afterComponents, IList beforeComponents, IMiniLcmApi api) { - return await DiffCollection.Diff(api, + return await DiffCollection.Diff( beforeComponents, afterComponents, - //we can't use the ID as there's none defined by Fw so it won't work as a sync key - component => (component.ComplexFormEntryId, component.ComponentEntryId, component.ComponentSenseId), - static async (api, afterComponent) => - { - //change id, since we're not using the id as the key for this collection - //the id may be the same, which is not what we want here - afterComponent.Id = Guid.NewGuid(); - try - { - await api.CreateComplexFormComponent(afterComponent); - } - catch (NotFoundException) - { - //this can happen if the entry was deleted, so we can just ignore it - } - return 1; - }, - static async (api, beforeComponent) => - { - await api.DeleteComplexFormComponent(beforeComponent); - return 1; - }, - static (api, beforeComponent, afterComponent) => - { - if (beforeComponent.ComplexFormEntryId == afterComponent.ComplexFormEntryId && - beforeComponent.ComponentEntryId == afterComponent.ComponentEntryId && - beforeComponent.ComponentSenseId == afterComponent.ComponentSenseId) - { - return Task.FromResult(0); - } - throw new InvalidOperationException($"changing complex form components is not supported, they should just be deleted and recreated"); - } + new ComplexFormComponentsDiffApi(api) ); } @@ -114,18 +57,7 @@ private static async Task SensesSync(Guid entryId, IList beforeSenses, IMiniLcmApi api) { - Func> add = async (api, afterSense) => - { - await api.CreateSense(entryId, afterSense); - return 1; - }; - Func> remove = async (api, beforeSense) => - { - await api.DeleteSense(entryId, beforeSense.Id); - return 1; - }; - Func> replace = async (api, beforeSense, afterSense) => await SenseSync.Sync(entryId, afterSense, beforeSense, api); - return await DiffCollection.Diff(api, beforeSenses, afterSenses, add, remove, replace); + return await DiffCollection.DiffOrderable(beforeSenses, afterSenses, new SensesDiffApi(api, entryId)); } public static UpdateObjectInput? EntryDiffToUpdate(Entry beforeEntry, Entry afterEntry) @@ -138,4 +70,121 @@ private static async Task SensesSync(Guid entryId, if (patchDocument.Operations.Count == 0) return null; return new UpdateObjectInput(patchDocument); } + + private class EntriesDiffApi(IMiniLcmApi api) : ObjectWithIdCollectionDiffApi + { + public override async Task<(int, Entry)> AddAndGet(Entry afterEntry) + { + //create each entry without components. + //After each entry is created, then replace will be called to create those components + var entryWithoutEntryRefs = afterEntry.WithoutEntryRefs(); + var changes = await Add(entryWithoutEntryRefs); + return (changes, entryWithoutEntryRefs); + } + + public override async Task Add(Entry afterEntry) + { + await api.CreateEntry(afterEntry); + return 1; + } + + public override async Task Remove(Entry entry) + { + await api.DeleteEntry(entry.Id); + return 1; + } + + public override Task Replace(Entry before, Entry after) + { + return Sync(after, before, api); + } + } + + private class ComplexFormTypesDiffApi(IMiniLcmApi api, Guid entryId) : ObjectWithIdCollectionDiffApi + { + public override async Task Add(ComplexFormType afterComplexFormType) + { + await api.AddComplexFormType(entryId, afterComplexFormType.Id); + return 1; + } + + public override async Task Remove(ComplexFormType beforeComplexFormType) + { + await api.RemoveComplexFormType(entryId, beforeComplexFormType.Id); + return 1; + } + + public override Task Replace(ComplexFormType before, ComplexFormType after) + { + return Task.FromResult(0); + } + } + + private class ComplexFormComponentsDiffApi(IMiniLcmApi api) : CollectionDiffApi + { + public override (Guid, Guid, Guid?) GetId(ComplexFormComponent component) + { + //we can't use the ID as there's none defined by Fw so it won't work as a sync key + return (component.ComplexFormEntryId, component.ComponentEntryId, component.ComponentSenseId); + } + + public override async Task Add(ComplexFormComponent afterComplexFormType) + { + //change id, since we're not using the id as the key for this collection + //the id may be the same, which is not what we want here + afterComplexFormType.Id = Guid.NewGuid(); + try + { + await api.CreateComplexFormComponent(afterComplexFormType); + } + catch (NotFoundException) + { + //this can happen if the entry was deleted, so we can just ignore it + } + return 1; + } + + public override async Task Remove(ComplexFormComponent beforeComplexFormType) + { + await api.DeleteComplexFormComponent(beforeComplexFormType); + return 1; + } + + public override Task Replace(ComplexFormComponent beforeComponent, ComplexFormComponent afterComponent) + { + if (beforeComponent.ComplexFormEntryId == afterComponent.ComplexFormEntryId && + beforeComponent.ComponentEntryId == afterComponent.ComponentEntryId && + beforeComponent.ComponentSenseId == afterComponent.ComponentSenseId) + { + return Task.FromResult(0); + } + throw new InvalidOperationException($"changing complex form components is not supported, they should just be deleted and recreated"); + } + } + + private class SensesDiffApi(IMiniLcmApi api, Guid entryId) : IOrderableCollectionDiffApi + { + public async Task Add(Sense sense, BetweenPosition between) + { + await api.CreateSense(entryId, sense, between); + return 1; + } + + public async Task Move(Sense sense, BetweenPosition between) + { + await api.MoveSense(entryId, sense.Id, between); + return 1; + } + + public async Task Remove(Sense sense) + { + await api.DeleteSense(entryId, sense.Id); + return 1; + } + + public Task Replace(Sense before, Sense after) + { + return SenseSync.Sync(entryId, after, before, api); + } + } } diff --git a/backend/FwLite/MiniLcm/SyncHelpers/ExampleSentenceSync.cs b/backend/FwLite/MiniLcm/SyncHelpers/ExampleSentenceSync.cs index ef2e6cf1a..f685e9650 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/ExampleSentenceSync.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/ExampleSentenceSync.cs @@ -11,25 +11,10 @@ public static async Task Sync(Guid entryId, IList beforeExampleSentences, IMiniLcmApi api) { - Func> add = async (api, afterExampleSentence) => - { - await api.CreateExampleSentence(entryId, senseId, afterExampleSentence); - return 1; - }; - Func> remove = async (api, beforeExampleSentence) => - { - await api.DeleteExampleSentence(entryId, senseId, beforeExampleSentence.Id); - return 1; - }; - Func> replace = - (api, beforeExampleSentence, afterExampleSentence) => - Sync(entryId, senseId, afterExampleSentence, beforeExampleSentence, api); - return await DiffCollection.Diff(api, + return await DiffCollection.Diff( beforeExampleSentences, afterExampleSentences, - add, - remove, - replace); + new ExampleSentencesDiffApi(api, entryId, senseId)); } public static async Task Sync(Guid entryId, @@ -64,4 +49,24 @@ public static async Task Sync(Guid entryId, if (patchDocument.Operations.Count == 0) return null; return new UpdateObjectInput(patchDocument); } + + private class ExampleSentencesDiffApi(IMiniLcmApi api, Guid entryId, Guid senseId) : ObjectWithIdCollectionDiffApi + { + public override async Task Add(ExampleSentence afterExampleSentence) + { + await api.CreateExampleSentence(entryId, senseId, afterExampleSentence); + return 1; + } + + public override async Task Remove(ExampleSentence beforeExampleSentence) + { + await api.DeleteExampleSentence(entryId, senseId, beforeExampleSentence.Id); + return 1; + } + + public override Task Replace(ExampleSentence beforeExampleSentence, ExampleSentence afterExampleSentence) + { + return Sync(entryId, senseId, afterExampleSentence, beforeExampleSentence, api); + } + } } diff --git a/backend/FwLite/MiniLcm/SyncHelpers/PartOfSpeechSync.cs b/backend/FwLite/MiniLcm/SyncHelpers/PartOfSpeechSync.cs index 9a7cf0bba..be6eff5a1 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/PartOfSpeechSync.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/PartOfSpeechSync.cs @@ -9,21 +9,10 @@ public static async Task Sync(PartOfSpeech[] currentPartsOfSpeech, PartOfSpeech[] previousPartsOfSpeech, IMiniLcmApi api) { - return await DiffCollection.Diff(api, + return await DiffCollection.Diff( previousPartsOfSpeech, currentPartsOfSpeech, - pos => pos.Id, - async (api, currentPos) => - { - await api.CreatePartOfSpeech(currentPos); - return 1; - }, - async (api, previousPos) => - { - await api.DeletePartOfSpeech(previousPos.Id); - return 1; - }, - (api, previousPos, currentPos) => Sync(previousPos, currentPos, api)); + new PartsOfSpeechDiffApi(api)); } public static async Task Sync(PartOfSpeech before, @@ -48,4 +37,24 @@ public static async Task Sync(PartOfSpeech before, if (patchDocument.Operations.Count == 0) return null; return new UpdateObjectInput(patchDocument); } + + private class PartsOfSpeechDiffApi(IMiniLcmApi api) : ObjectWithIdCollectionDiffApi + { + public override async Task Add(PartOfSpeech currentPos) + { + await api.CreatePartOfSpeech(currentPos); + return 1; + } + + public override async Task Remove(PartOfSpeech previousPos) + { + await api.DeletePartOfSpeech(previousPos.Id); + return 1; + } + + public override Task Replace(PartOfSpeech previousPos, PartOfSpeech currentPos) + { + return Sync(previousPos, currentPos, api); + } + } } diff --git a/backend/FwLite/MiniLcm/SyncHelpers/SemanticDomainSync.cs b/backend/FwLite/MiniLcm/SyncHelpers/SemanticDomainSync.cs index 3892e8ed1..b6bc793fc 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/SemanticDomainSync.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/SemanticDomainSync.cs @@ -9,21 +9,10 @@ public static async Task Sync(SemanticDomain[] currentSemanticDomains, SemanticDomain[] previousSemanticDomains, IMiniLcmApi api) { - return await DiffCollection.Diff(api, + return await DiffCollection.Diff( previousSemanticDomains, currentSemanticDomains, - pos => pos.Id, - async (api, currentPos) => - { - await api.CreateSemanticDomain(currentPos); - return 1; - }, - async (api, previousPos) => - { - await api.DeleteSemanticDomain(previousPos.Id); - return 1; - }, - (api, previousSemdom, currentSemdom) => Sync(previousSemdom, currentSemdom, api)); + new SemanticDomainsDiffApi(api)); } public static async Task Sync(SemanticDomain before, @@ -48,4 +37,24 @@ public static async Task Sync(SemanticDomain before, if (patchDocument.Operations.Count == 0) return null; return new UpdateObjectInput(patchDocument); } + + private class SemanticDomainsDiffApi(IMiniLcmApi api) : ObjectWithIdCollectionDiffApi + { + public override async Task Add(SemanticDomain currentSemDom) + { + await api.CreateSemanticDomain(currentSemDom); + return 1; + } + + public override async Task Remove(SemanticDomain previousSemDom) + { + await api.DeleteSemanticDomain(previousSemDom.Id); + return 1; + } + + public override Task Replace(SemanticDomain previousSemDom, SemanticDomain currentSemDom) + { + return Sync(previousSemDom, currentSemDom, api); + } + } } diff --git a/backend/FwLite/MiniLcm/SyncHelpers/SenseSync.cs b/backend/FwLite/MiniLcm/SyncHelpers/SenseSync.cs index 181107105..6c028cd56 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/SenseSync.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/SenseSync.cs @@ -17,24 +17,10 @@ public static async Task Sync(Guid entryId, afterSense.ExampleSentences, beforeSense.ExampleSentences, api); - changes += await DiffCollection.Diff(api, + changes += await DiffCollection.Diff( beforeSense.SemanticDomains, afterSense.SemanticDomains, - async (api, domain) => - { - await api.AddSemanticDomainToSense(beforeSense.Id, domain); - return 1; - }, - async (api, beforeDomain) => - { - await api.RemoveSemanticDomainFromSense(beforeSense.Id, beforeDomain.Id); - return 1; - }, - (_, beforeDomain, afterDomain) => - { - //do nothing, semantic domains are not editable here - return Task.FromResult(0); - }); + new SenseSemanticDomainsDiffApi(api, beforeSense.Id)); return changes + (updateObjectInput is null ? 0 : 1); } @@ -59,4 +45,24 @@ public static async Task Sync(Guid entryId, if (patchDocument.Operations.Count == 0) return null; return new UpdateObjectInput(patchDocument); } + + private class SenseSemanticDomainsDiffApi(IMiniLcmApi api, Guid senseId) : ObjectWithIdCollectionDiffApi + { + public override async Task Add(SemanticDomain domain) + { + await api.AddSemanticDomainToSense(senseId, domain); + return 1; + } + + public override async Task Remove(SemanticDomain beforeDomain) + { + await api.RemoveSemanticDomainFromSense(senseId, beforeDomain.Id); + return 1; + } + + public override Task Replace(SemanticDomain previousSemDom, SemanticDomain currentSemDom) + { + return Task.FromResult(0); + } + } } diff --git a/backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs b/backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs index 142ae97e7..548edd0e9 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs @@ -16,25 +16,10 @@ public static async Task Sync(WritingSystem[] currentWritingSystems, WritingSystem[] previousWritingSystems, IMiniLcmApi api) { - return await DiffCollection.Diff(api, + return await DiffCollection.Diff( previousWritingSystems, currentWritingSystems, - ws => (ws.WsId, ws.Type), - async (api, currentWs) => - { - await api.CreateWritingSystem(currentWs.Type, currentWs); - return 1; - }, - async (api, previousWs) => - { - // await api.DeleteWritingSystem(previousWs.Id); // Deleting writing systems is dangerous as it causes cascading data deletion. Needs careful thought. - // TODO: should we throw an exception? - return 0; - }, - async (api, previousWs, currentWs) => - { - return await Sync(currentWs, previousWs, api); - }); + new WritingSystemsDiffApi(api)); } public static async Task Sync(WritingSystem afterWs, WritingSystem beforeWs, IMiniLcmApi api) @@ -65,4 +50,30 @@ public static async Task Sync(WritingSystem afterWs, WritingSystem beforeWs if (patchDocument.Operations.Count == 0) return null; return new UpdateObjectInput(patchDocument); } + + private class WritingSystemsDiffApi(IMiniLcmApi api) : CollectionDiffApi + { + public override (WritingSystemId, WritingSystemType) GetId(WritingSystem value) + { + return (value.WsId, value.Type); + } + + public override async Task Add(WritingSystem currentWs) + { + await api.CreateWritingSystem(currentWs.Type, currentWs); + return 1; + } + + public override Task Remove(WritingSystem beforeDomain) + { + // await api.DeleteWritingSystem(previousWs.Id); // Deleting writing systems is dangerous as it causes cascading data deletion. Needs careful thought. + // TODO: should we throw an exception? + return Task.FromResult(0); + } + + public override Task Replace(WritingSystem previousWs, WritingSystem currentWs) + { + return Sync(currentWs, previousWs, api); + } + } } diff --git a/frontend/viewer/src/lib/Editor.svelte b/frontend/viewer/src/lib/Editor.svelte index bb56788bc..9e26781c6 100644 --- a/frontend/viewer/src/lib/Editor.svelte +++ b/frontend/viewer/src/lib/Editor.svelte @@ -48,6 +48,7 @@ async function updateEntry(updatedEntry: IEntry) { if (entry.id != updatedEntry.id) throw new Error('Entry id mismatch'); + console.debug('Updating entry', updatedEntry); await saveHandler(() => lexboxApi.UpdateEntry(initialEntry, updatedEntry)); } diff --git a/frontend/viewer/src/lib/services/service-provider-signalr.ts b/frontend/viewer/src/lib/services/service-provider-signalr.ts index 08d31e703..049245904 100644 --- a/frontend/viewer/src/lib/services/service-provider-signalr.ts +++ b/frontend/viewer/src/lib/services/service-provider-signalr.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/naming-convention */ -import { getHubProxyFactory, getReceiverRegister } from '../generated-signalr-client/TypedSignalR.Client'; +import {getHubProxyFactory, getReceiverRegister} from '../generated-signalr-client/TypedSignalR.Client'; import { type HubConnection, @@ -7,7 +7,7 @@ import { HubConnectionState, type IHttpConnectionOptions } from '@microsoft/signalr'; -import type { LexboxApiClient, LexboxApiFeatures, LexboxApiMetadata } from './lexbox-api'; +import type {LexboxApiClient, LexboxApiFeatures, LexboxApiMetadata} from './lexbox-api'; import {LexboxService} from './service-provider'; import {onDestroy} from 'svelte'; import {type Readable, type Writable, writable} from 'svelte/store'; @@ -67,6 +67,10 @@ function setupConnection(url: string, options: IHttpConnectionOptions, onError: .withUrl(url, options) .withAutomaticReconnect() .build(); + + if (import.meta.env.DEV) connection.serverTimeoutInMilliseconds = 60_000 * 10; + console.debug('SignalR connection timeout', connection.serverTimeoutInMilliseconds); + onDestroy(() => connection.stop()); connection.onclose((error) => { connected.set(false);