From cfcabd4fe402f6f1de292342c99f0adb176af81b Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Fri, 22 Nov 2024 13:41:13 +0700 Subject: [PATCH 01/11] write failing test --- .../FwLiteProjectSync.Tests/EntrySyncTests.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs index d1b264839..d7a631759 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs @@ -105,4 +105,27 @@ public async Task CanChangeComplexFormTypeViaSync() actual.Should().NotBeNull(); actual.Should().BeEquivalentTo(after, options => options); } + + [Fact] + public async Task CanCreateAComplexFormAndItsComponentInOneSync() + { + //ensure they are synced so a real sync will happen when we want it to + await _fixture.SyncService.Sync(_fixture.CrdtApi, _fixture.FwDataApi); + + var complexFormEntry = await _fixture.CrdtApi.CreateEntry(new() { LexemeForm = { { "en", "complexForm" } } }); + var componentEntry = await _fixture.CrdtApi.CreateEntry(new() + { + LexemeForm = { { "en", "component" } }, + ComplexForms = + [ + new ComplexFormComponent() + { + ComplexFormEntryId = complexFormEntry.Id, ComponentEntryId = Guid.Empty + } + ] + }); + + //one of the entries will be created first, it will try to create the reference to the other but it won't exist yet + await _fixture.SyncService.Sync(_fixture.CrdtApi, _fixture.FwDataApi); + } } From 429b74ba219f1d98fa9af45f31bb0d8d461df0c8 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Fri, 22 Nov 2024 14:05:55 +0700 Subject: [PATCH 02/11] change Entry.Sync to create entries first without complex forms, then in a second pass update those entries so they contain those complex forms --- .../Api/UpdateProxy/UpdateEntryProxy.cs | 2 +- backend/FwLite/MiniLcm/Models/Entry.cs | 7 +- .../MiniLcm/SyncHelpers/DiffCollection.cs | 68 +++++++++++++++++++ .../FwLite/MiniLcm/SyncHelpers/EntrySync.cs | 11 +-- 4 files changed, 82 insertions(+), 6 deletions(-) diff --git a/backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateEntryProxy.cs b/backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateEntryProxy.cs index 503222fca..d0defe8ad 100644 --- a/backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateEntryProxy.cs +++ b/backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateEntryProxy.cs @@ -4,7 +4,7 @@ namespace FwDataMiniLcmBridge.Api.UpdateProxy; -public class UpdateEntryProxy : Entry +public record UpdateEntryProxy : Entry { private readonly ILexEntry _lcmEntry; private readonly FwDataMiniLcmApi _lexboxLcmApi; diff --git a/backend/FwLite/MiniLcm/Models/Entry.cs b/backend/FwLite/MiniLcm/Models/Entry.cs index 191db2753..7e7239ca5 100644 --- a/backend/FwLite/MiniLcm/Models/Entry.cs +++ b/backend/FwLite/MiniLcm/Models/Entry.cs @@ -1,6 +1,6 @@ namespace MiniLcm.Models; -public class Entry : IObjectWithId +public record Entry : IObjectWithId { public Guid Id { get; set; } public DateTimeOffset? DeletedAt { get; set; } @@ -68,6 +68,11 @@ public Guid[] GetReferences() public void RemoveReference(Guid id, DateTimeOffset time) { } + + public Entry WithoutEntryRefs() + { + return this with { Components = [], ComplexForms = [] }; + } } public class Variants diff --git a/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs b/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs index 2cc251f2b..33bda1e46 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs @@ -4,6 +4,73 @@ namespace MiniLcm.SyncHelpers; 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 + { + var changes = 0; + var afterEntriesDict = after.ToDictionary(identity); + + foreach (var beforeEntry in before) + { + if (afterEntriesDict.TryGetValue(identity(beforeEntry), out var afterEntry)) + { + changes += await replace(api, beforeEntry, afterEntry); + } + else + { + changes += await remove(api, beforeEntry); + } + + afterEntriesDict.Remove(identity(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)); + } + foreach ((T createdItem, T 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); + } + + return changes; + } + + /// + /// + /// + /// + /// + /// + /// + /// + /// + /// api, before, after is the parameter order + /// + /// + /// public static async Task Diff( IMiniLcmApi api, IList before, @@ -36,6 +103,7 @@ public static async Task Diff( return changes; } + public static async Task Diff( IMiniLcmApi api, IList before, diff --git a/backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs b/backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs index 50fe78892..a61a7fc58 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs @@ -10,10 +10,13 @@ public static async Task Sync(Entry[] afterEntries, Entry[] beforeEntries, IMiniLcmApi api) { - Func> add = static async (api, afterEntry) => + Func> add = static async (api, afterEntry) => { - await api.CreateEntry(afterEntry); - return 1; + //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) => { @@ -21,7 +24,7 @@ public static async Task Sync(Entry[] afterEntries, return 1; }; Func> replace = static async (api, beforeEntry, afterEntry) => await Sync(afterEntry, beforeEntry, api); - return await DiffCollection.Diff(api, beforeEntries, afterEntries, add, remove, replace); + return await DiffCollection.DiffAddThenUpdate(api, beforeEntries, afterEntries, entry => entry.Id, add, remove, replace); } public static async Task Sync(Entry afterEntry, Entry beforeEntry, IMiniLcmApi api) From ff499196c2e99de0ab0497b4e6ab18df7dbf22e5 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Mon, 25 Nov 2024 12:12:22 +0700 Subject: [PATCH 03/11] prevent duplicating complex forms which are the same/already exist --- ...ModelSnapshotTests.VerifyDbModel.verified.txt | 9 ++++++++- backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs | 5 +++++ backend/FwLite/LcmCrdt/LcmCrdtKernel.cs | 15 +++++++++++++++ .../ComplexFormComponentTestsBase.cs | 16 ++++++++++++++++ 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyDbModel.verified.txt b/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyDbModel.verified.txt index a7b2cc8c5..129ee865c 100644 --- a/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyDbModel.verified.txt +++ b/backend/FwLite/LcmCrdt.Tests/DataModelSnapshotTests.VerifyDbModel.verified.txt @@ -24,6 +24,8 @@ ComponentEntryId (Guid) Required FK Index ComponentHeadword (string) ComponentSenseId (Guid?) FK Index + Annotations: + Relational:ColumnName: ComponentSenseId DeletedAt (DateTimeOffset?) SnapshotId (no field, Guid?) Shadow FK Index Keys: @@ -34,10 +36,15 @@ ComplexFormComponent {'ComponentSenseId'} -> Sense {'Id'} Cascade ComplexFormComponent {'SnapshotId'} -> ObjectSnapshot {'Id'} Unique SetNull Indexes: - ComplexFormEntryId ComponentEntryId ComponentSenseId SnapshotId Unique + ComplexFormEntryId, ComponentEntryId Unique + Annotations: + Relational:Filter: ComponentSenseId IS NULL + ComplexFormEntryId, ComponentEntryId, ComponentSenseId Unique + Annotations: + Relational:Filter: ComponentSenseId IS NOT NULL Annotations: DiscriminatorProperty: Relational:FunctionName: diff --git a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs index 241c8daad..c39cb33d1 100644 --- a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs +++ b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs @@ -151,6 +151,11 @@ public async Task CreateComplexFormType(ComplexFormType complex public async Task CreateComplexFormComponent(ComplexFormComponent complexFormComponent) { + var existing = await ComplexFormComponents.SingleOrDefaultAsync(c => + c.ComplexFormEntryId == complexFormComponent.ComplexFormEntryId + && c.ComponentEntryId == complexFormComponent.ComponentEntryId + && c.ComponentSenseId == complexFormComponent.ComponentSenseId); + if (existing is not null) return existing; var addEntryComponentChange = new AddEntryComponentChange(complexFormComponent); await dataModel.AddChange(ClientId, addEntryComponentChange); return (await ComplexFormComponents.SingleOrDefaultAsync(c => c.Id == addEntryComponentChange.EntityId)) ?? throw NotFoundException.ForType(); diff --git a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs index 9fd13ce3e..74562c440 100644 --- a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs +++ b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs @@ -134,7 +134,22 @@ public static void ConfigureCrdt(CrdtConfig config) .Add() .Add(builder => { + const string componentSenseId = "ComponentSenseId"; builder.ToTable("ComplexFormComponents"); + builder.Property(c => c.ComponentSenseId).HasColumnName(componentSenseId); + //these indexes are used to ensure that we don't create duplicate complex form components + //we need the filter otherwise 2 components which are the same and have a null sense id can be created because 2 rows with the same null are not considered duplicates + builder.HasIndex(component => new + { + component.ComplexFormEntryId, + component.ComponentEntryId, + component.ComponentSenseId + }).IsUnique().HasFilter($"{componentSenseId} IS NOT NULL"); + builder.HasIndex(component => new + { + component.ComplexFormEntryId, + component.ComponentEntryId + }).IsUnique().HasFilter($"{componentSenseId} IS NULL"); }); config.ChangeTypeListBuilder.Add>() diff --git a/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs b/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs index 4f51ca275..820b660b1 100644 --- a/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs +++ b/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs @@ -50,6 +50,22 @@ public async Task CreateComplexFormComponent_Works() component.ComponentHeadword.Should().Be("component"); } + [Fact] + public async Task CreateComplexFormComponent_UsingTheSameComponentWithNullSenseDoesNothing() + { + var component1 = await Api.CreateComplexFormComponent(ComplexFormComponent.FromEntries(_complexFormEntry, _componentEntry)); + var component2 = await Api.CreateComplexFormComponent(ComplexFormComponent.FromEntries(_complexFormEntry, _componentEntry)); + component2.Should().BeEquivalentTo(component1); + } + + [Fact] + public async Task CreateComplexFormComponent_UsingTheSameComponentWithSenseDoesNothing() + { + var component1 = await Api.CreateComplexFormComponent(ComplexFormComponent.FromEntries(_complexFormEntry, _componentEntry, _componentSenseId1)); + var component2 = await Api.CreateComplexFormComponent(ComplexFormComponent.FromEntries(_complexFormEntry, _componentEntry, _componentSenseId1)); + component2.Should().BeEquivalentTo(component1); + } + [Fact] public async Task CreateComplexFormComponent_WorksWithSense() { From eba81b9e49e3279e36cf1c015187771b1a20b44e Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Mon, 25 Nov 2024 12:12:50 +0700 Subject: [PATCH 04/11] catch and throw some exceptions with additional context to help debugging --- .../Api/FwDataMiniLcmApi.cs | 75 +++++++++++-------- .../Exceptions/CreateObjectException.cs | 12 +++ .../MiniLcm/Exceptions/SyncObjectException.cs | 12 +++ .../FwLite/MiniLcm/SyncHelpers/EntrySync.cs | 21 ++++-- 4 files changed, 80 insertions(+), 40 deletions(-) create mode 100644 backend/FwLite/MiniLcm/Exceptions/CreateObjectException.cs create mode 100644 backend/FwLite/MiniLcm/Exceptions/SyncObjectException.cs diff --git a/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs b/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs index 8e132b2e6..a258593ed 100644 --- a/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs +++ b/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs @@ -6,6 +6,7 @@ using FwDataMiniLcmBridge.LcmUtils; using Microsoft.Extensions.Logging; using MiniLcm; +using MiniLcm.Exceptions; using MiniLcm.Models; using MiniLcm.SyncHelpers; using SIL.LCModel; @@ -559,40 +560,48 @@ public IAsyncEnumerable SearchEntries(string query, QueryOptions? options public async Task CreateEntry(Entry entry) { entry.Id = entry.Id == default ? Guid.NewGuid() : entry.Id; - UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Create Entry", - "Remove entry", - Cache.ServiceLocator.ActionHandler, - () => - { - var lexEntry = LexEntryFactory.Create(entry.Id, Cache.ServiceLocator.GetInstance().Singleton.LexDbOA); - lexEntry.LexemeFormOA = Cache.ServiceLocator.GetInstance().Create(); - UpdateLcmMultiString(lexEntry.LexemeFormOA.Form, entry.LexemeForm); - UpdateLcmMultiString(lexEntry.CitationForm, entry.CitationForm); - UpdateLcmMultiString(lexEntry.LiteralMeaning, entry.LiteralMeaning); - UpdateLcmMultiString(lexEntry.Comment, entry.Note); - - foreach (var sense in entry.Senses) - { - CreateSense(lexEntry, sense); - } - - //form types should be created before components, otherwise the form type "unspecified" will be added - foreach (var complexFormType in entry.ComplexFormTypes) - { - AddComplexFormType(lexEntry, complexFormType.Id); - } - - foreach (var component in entry.Components) - { - AddComplexFormComponent(lexEntry, component); - } - - foreach (var complexForm in entry.ComplexForms) + try + { + UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Create Entry", + "Remove entry", + Cache.ServiceLocator.ActionHandler, + () => { - var complexLexEntry = EntriesRepository.GetObject(complexForm.ComplexFormEntryId); - AddComplexFormComponent(complexLexEntry, complexForm); - } - }); + var lexEntry = LexEntryFactory.Create(entry.Id, + Cache.ServiceLocator.GetInstance().Singleton.LexDbOA); + lexEntry.LexemeFormOA = Cache.ServiceLocator.GetInstance().Create(); + UpdateLcmMultiString(lexEntry.LexemeFormOA.Form, entry.LexemeForm); + UpdateLcmMultiString(lexEntry.CitationForm, entry.CitationForm); + UpdateLcmMultiString(lexEntry.LiteralMeaning, entry.LiteralMeaning); + UpdateLcmMultiString(lexEntry.Comment, entry.Note); + + foreach (var sense in entry.Senses) + { + CreateSense(lexEntry, sense); + } + + //form types should be created before components, otherwise the form type "unspecified" will be added + foreach (var complexFormType in entry.ComplexFormTypes) + { + AddComplexFormType(lexEntry, complexFormType.Id); + } + + foreach (var component in entry.Components) + { + AddComplexFormComponent(lexEntry, component); + } + + foreach (var complexForm in entry.ComplexForms) + { + var complexLexEntry = EntriesRepository.GetObject(complexForm.ComplexFormEntryId); + AddComplexFormComponent(complexLexEntry, complexForm); + } + }); + } + catch (Exception e) + { + throw new CreateObjectException($"Failed to create entry {entry}", e); + } return await GetEntry(entry.Id) ?? throw new InvalidOperationException("Entry was not created"); } diff --git a/backend/FwLite/MiniLcm/Exceptions/CreateObjectException.cs b/backend/FwLite/MiniLcm/Exceptions/CreateObjectException.cs new file mode 100644 index 000000000..128cb0515 --- /dev/null +++ b/backend/FwLite/MiniLcm/Exceptions/CreateObjectException.cs @@ -0,0 +1,12 @@ +namespace MiniLcm.Exceptions; + +public class CreateObjectException: Exception +{ + public CreateObjectException(string? message) : base(message) + { + } + + public CreateObjectException(string? message, Exception? innerException) : base(message, innerException) + { + } +} diff --git a/backend/FwLite/MiniLcm/Exceptions/SyncObjectException.cs b/backend/FwLite/MiniLcm/Exceptions/SyncObjectException.cs new file mode 100644 index 000000000..e1f284cde --- /dev/null +++ b/backend/FwLite/MiniLcm/Exceptions/SyncObjectException.cs @@ -0,0 +1,12 @@ +namespace MiniLcm.Exceptions; + +public class SyncObjectException: Exception +{ + public SyncObjectException(string? message) : base(message) + { + } + + public SyncObjectException(string? message, Exception? innerException) : base(message, innerException) + { + } +} diff --git a/backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs b/backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs index a61a7fc58..c8f2db5a3 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs @@ -29,14 +29,21 @@ public static async Task Sync(Entry[] afterEntries, public static async Task Sync(Entry afterEntry, Entry beforeEntry, IMiniLcmApi api) { - var updateObjectInput = EntryDiffToUpdate(beforeEntry, afterEntry); - if (updateObjectInput is not null) await api.UpdateEntry(afterEntry.Id, updateObjectInput); - var changes = await SensesSync(afterEntry.Id, afterEntry.Senses, beforeEntry.Senses, api); + try + { + var updateObjectInput = EntryDiffToUpdate(beforeEntry, afterEntry); + if (updateObjectInput is not null) await api.UpdateEntry(afterEntry.Id, updateObjectInput); + var changes = await SensesSync(afterEntry.Id, afterEntry.Senses, beforeEntry.Senses, api); - changes += await Sync(afterEntry.Components, beforeEntry.Components, api); - changes += await Sync(afterEntry.ComplexForms, beforeEntry.ComplexForms, api); - changes += await Sync(afterEntry.Id, afterEntry.ComplexFormTypes, beforeEntry.ComplexFormTypes, api); - return changes + (updateObjectInput is null ? 0 : 1); + changes += await Sync(afterEntry.Components, beforeEntry.Components, api); + changes += await Sync(afterEntry.ComplexForms, beforeEntry.ComplexForms, api); + changes += await Sync(afterEntry.Id, afterEntry.ComplexFormTypes, beforeEntry.ComplexFormTypes, api); + return changes + (updateObjectInput is null ? 0 : 1); + } + catch (Exception e) + { + throw new SyncObjectException($"Failed to sync entry {afterEntry}", e); + } } private static async Task Sync(Guid entryId, From 4e6dacda3385ea4f181769fcc2e846b731ea3333 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Mon, 25 Nov 2024 12:14:53 +0700 Subject: [PATCH 05/11] add method to SyncService to expose snapshot path for testing --- .../Fixtures/SyncFixture.cs | 6 +++++ .../CrdtFwdataProjectSyncService.cs | 24 +++++++++++-------- .../FwLiteProjectSync.csproj | 4 ++++ 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs b/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs index 0b0443e0d..cb82ef31b 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs @@ -69,6 +69,12 @@ public async Task DisposeAsync() public CrdtMiniLcmApi CrdtApi { get; set; } = null!; public FwDataMiniLcmApi FwDataApi { get; set; } = null!; + + public void DeleteSyncSnapshot() + { + var snapshotPath = CrdtFwdataProjectSyncService.SnapshotPath(FwDataApi.Project); + if (File.Exists(snapshotPath)) File.Delete(snapshotPath); + } } public class MockProjectContext(CrdtProject? project) : ProjectContext diff --git a/backend/FwLite/FwLiteProjectSync/CrdtFwdataProjectSyncService.cs b/backend/FwLite/FwLiteProjectSync/CrdtFwdataProjectSyncService.cs index 12258dcee..dc50cd4ad 100644 --- a/backend/FwLite/FwLiteProjectSync/CrdtFwdataProjectSyncService.cs +++ b/backend/FwLite/FwLiteProjectSync/CrdtFwdataProjectSyncService.cs @@ -1,8 +1,8 @@ using System.Text.Json; +using FwDataMiniLcmBridge; using FwDataMiniLcmBridge.Api; using LcmCrdt; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; using MiniLcm; using MiniLcm.Models; using MiniLcm.SyncHelpers; @@ -11,7 +11,7 @@ namespace FwLiteProjectSync; -public class CrdtFwdataProjectSyncService(IOptions lcmCrdtConfig, MiniLcmImport miniLcmImport, ILogger logger) +public class CrdtFwdataProjectSyncService(MiniLcmImport miniLcmImport, ILogger logger) { public record SyncResult(int CrdtChanges, int FwdataChanges); @@ -21,13 +21,13 @@ public async Task Sync(IMiniLcmApi crdtApi, FwDataMiniLcmApi fwdataA { throw new InvalidOperationException($"Project id mismatch, CRDT Id: {crdt.ProjectData.FwProjectId}, FWData Id: {fwdataApi.ProjectId}"); } - var projectSnapshot = await GetProjectSnapshot(fwdataApi.Project.Name, fwdataApi.Project.ProjectsPath); + var projectSnapshot = await GetProjectSnapshot(fwdataApi.Project); SyncResult result = await Sync(crdtApi, fwdataApi, dryRun, fwdataApi.EntryCount, projectSnapshot); fwdataApi.Save(); if (!dryRun) { - await SaveProjectSnapshot(fwdataApi.Project.Name, fwdataApi.Project.ProjectsPath, + await SaveProjectSnapshot(fwdataApi.Project, new ProjectSnapshot( await fwdataApi.GetEntries().ToArrayAsync(), await fwdataApi.GetPartsOfSpeech().ToArrayAsync(), @@ -86,21 +86,25 @@ private void LogDryRun(IMiniLcmApi api, string type) public record ProjectSnapshot(Entry[] Entries, PartOfSpeech[] PartsOfSpeech, SemanticDomain[] SemanticDomains); - private async Task GetProjectSnapshot(string projectName, string? projectPath) + private async Task GetProjectSnapshot(FwDataProject project) { - projectPath ??= lcmCrdtConfig.Value.ProjectPath; - var snapshotPath = Path.Combine(projectPath, $"{projectName}_snapshot.json"); + var snapshotPath = SnapshotPath(project); if (!File.Exists(snapshotPath)) return null; await using var file = File.OpenRead(snapshotPath); return await JsonSerializer.DeserializeAsync(file); } - private async Task SaveProjectSnapshot(string projectName, string? projectPath, ProjectSnapshot projectSnapshot) + private async Task SaveProjectSnapshot(FwDataProject project, ProjectSnapshot projectSnapshot) { - projectPath ??= lcmCrdtConfig.Value.ProjectPath; - var snapshotPath = Path.Combine(projectPath, $"{projectName}_snapshot.json"); + var snapshotPath = SnapshotPath(project); await using var file = File.Create(snapshotPath); await JsonSerializer.SerializeAsync(file, projectSnapshot); } + internal static string SnapshotPath(FwDataProject project) + { + var projectPath = project.ProjectsPath; + var snapshotPath = Path.Combine(projectPath, $"{project.Name}_snapshot.json"); + return snapshotPath; + } } diff --git a/backend/FwLite/FwLiteProjectSync/FwLiteProjectSync.csproj b/backend/FwLite/FwLiteProjectSync/FwLiteProjectSync.csproj index 7e4c26e04..2d0c452f0 100644 --- a/backend/FwLite/FwLiteProjectSync/FwLiteProjectSync.csproj +++ b/backend/FwLite/FwLiteProjectSync/FwLiteProjectSync.csproj @@ -19,4 +19,8 @@ + + + + From 98f38d4069b17e9518ba4ce0fbe646d6623d9c68 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Mon, 25 Nov 2024 12:16:00 +0700 Subject: [PATCH 06/11] move CanCreateAComplexFormAndItsComponentInOneSync out of EntrySyncTests and into SyncTests --- .../FwLiteProjectSync.Tests/EntrySyncTests.cs | 23 ------------------- .../FwLiteProjectSync.Tests/SyncTests.cs | 20 ++++++++++++++++ 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs index d7a631759..d1b264839 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs @@ -105,27 +105,4 @@ public async Task CanChangeComplexFormTypeViaSync() actual.Should().NotBeNull(); actual.Should().BeEquivalentTo(after, options => options); } - - [Fact] - public async Task CanCreateAComplexFormAndItsComponentInOneSync() - { - //ensure they are synced so a real sync will happen when we want it to - await _fixture.SyncService.Sync(_fixture.CrdtApi, _fixture.FwDataApi); - - var complexFormEntry = await _fixture.CrdtApi.CreateEntry(new() { LexemeForm = { { "en", "complexForm" } } }); - var componentEntry = await _fixture.CrdtApi.CreateEntry(new() - { - LexemeForm = { { "en", "component" } }, - ComplexForms = - [ - new ComplexFormComponent() - { - ComplexFormEntryId = complexFormEntry.Id, ComponentEntryId = Guid.Empty - } - ] - }); - - //one of the entries will be created first, it will try to create the reference to the other but it won't exist yet - await _fixture.SyncService.Sync(_fixture.CrdtApi, _fixture.FwDataApi); - } } diff --git a/backend/FwLite/FwLiteProjectSync.Tests/SyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/SyncTests.cs index 752d77fa2..07fc8123e 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/SyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/SyncTests.cs @@ -383,4 +383,24 @@ public async Task AddingASenseToAnEntryInEachProjectSyncsAcrossBoth() options => options.For(e => e.Components).Exclude(c => c.Id) .For(e => e.ComplexForms).Exclude(c => c.Id)); } + + [Fact] + public async Task CanCreateAComplexFormAndItsComponentInOneSync() + { + //ensure they are synced so a real sync will happen when we want it to + await _fixture.SyncService.Sync(_fixture.CrdtApi, _fixture.FwDataApi); + + var complexFormEntry = await _fixture.CrdtApi.CreateEntry(new() { LexemeForm = { { "en", "complexForm" } } }); + var componentEntry = await _fixture.CrdtApi.CreateEntry(new() + { + LexemeForm = { { "en", "component" } }, + ComplexForms = + [ + new ComplexFormComponent() { ComplexFormEntryId = complexFormEntry.Id, ComponentEntryId = Guid.Empty } + ] + }); + + //one of the entries will be created first, it will try to create the reference to the other but it won't exist yet + await _fixture.SyncService.Sync(_fixture.CrdtApi, _fixture.FwDataApi); + } } From 1e56414141fcb74d54a1a6cf010a036b85ca95a7 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Mon, 25 Nov 2024 13:42:22 +0700 Subject: [PATCH 07/11] expose a dry run sync method which returns the dry run records for inspection --- .../CrdtFwdataProjectSyncService.cs | 17 ++++++++++++++++- .../FwLiteProjectSync/DryRunMiniLcmApi.cs | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/backend/FwLite/FwLiteProjectSync/CrdtFwdataProjectSyncService.cs b/backend/FwLite/FwLiteProjectSync/CrdtFwdataProjectSyncService.cs index dc50cd4ad..a4122e8ed 100644 --- a/backend/FwLite/FwLiteProjectSync/CrdtFwdataProjectSyncService.cs +++ b/backend/FwLite/FwLiteProjectSync/CrdtFwdataProjectSyncService.cs @@ -15,6 +15,16 @@ public class CrdtFwdataProjectSyncService(MiniLcmImport miniLcmImport, ILogger CrdtDryRunRecords, + List FwDataDryRunRecords) : SyncResult(CrdtChanges, FwdataChanges); + + public async Task SyncDryRun(IMiniLcmApi crdtApi, FwDataMiniLcmApi fwdataApi) + { + return (DryRunSyncResult) await Sync(crdtApi, fwdataApi, true); + } public async Task Sync(IMiniLcmApi crdtApi, FwDataMiniLcmApi fwdataApi, bool dryRun = false) { if (crdtApi is CrdtMiniLcmApi crdt && crdt.ProjectData.FwProjectId != fwdataApi.ProjectId) @@ -69,7 +79,7 @@ private async Task Sync(IMiniLcmApi crdtApi, IMiniLcmApi fwdataApi, LogDryRun(fwdataApi, "fwdata"); //todo push crdt changes to lexbox - + if (dryRun) return new DryRunSyncResult(crdtChanges, fwdataChanges, GetDryRunRecords(crdtApi), GetDryRunRecords(fwdataApi)); return new SyncResult(crdtChanges, fwdataChanges); } @@ -84,6 +94,11 @@ private void LogDryRun(IMiniLcmApi api, string type) logger.LogInformation($"Dry run {type} changes: {dryRunApi.DryRunRecords.Count}"); } + private List GetDryRunRecords(IMiniLcmApi api) + { + return ((DryRunMiniLcmApi)api).DryRunRecords; + } + public record ProjectSnapshot(Entry[] Entries, PartOfSpeech[] PartsOfSpeech, SemanticDomain[] SemanticDomains); private async Task GetProjectSnapshot(FwDataProject project) diff --git a/backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs b/backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs index 25c317670..15c17a387 100644 --- a/backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs +++ b/backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs @@ -216,7 +216,7 @@ public Task CreateComplexFormComponent(ComplexFormComponen public Task DeleteComplexFormComponent(ComplexFormComponent complexFormComponent) { - DryRunRecords.Add(new DryRunRecord(nameof(DeleteComplexFormComponent), $"Delete complex form component complex entry: {complexFormComponent.ComplexFormHeadword}, component entry: {complexFormComponent.ComponentHeadword}")); + DryRunRecords.Add(new DryRunRecord(nameof(DeleteComplexFormComponent), $"Delete complex form component: {complexFormComponent}")); return Task.CompletedTask; } From 373a5ec43151531225ec8f1d2e7486bd14c38cf5 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Mon, 25 Nov 2024 14:00:29 +0700 Subject: [PATCH 08/11] dont track EF queries in miniLcm, should fix weird sync test issues which only show up when running all tests at once --- backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs index c39cb33d1..e6e83fb7b 100644 --- a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs +++ b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs @@ -18,14 +18,15 @@ public class CrdtMiniLcmApi(DataModel dataModel, CurrentProjectService projectSe private Guid ClientId { get; } = projectService.ProjectData.ClientId; public ProjectData ProjectData => projectService.ProjectData; - private IQueryable Entries => dataModel.QueryLatest(); - private IQueryable ComplexFormComponents => dataModel.QueryLatest(); - private IQueryable ComplexFormTypes => dataModel.QueryLatest(); - private IQueryable Senses => dataModel.QueryLatest(); - private IQueryable ExampleSentences => dataModel.QueryLatest(); - private IQueryable WritingSystems => dataModel.QueryLatest(); - private IQueryable SemanticDomains => dataModel.QueryLatest(); - private IQueryable PartsOfSpeech => dataModel.QueryLatest(); + private IQueryable Entries => dataModel.QueryLatest().AsTracking(false); + private IQueryable ComplexFormComponents => dataModel.QueryLatest() + .AsTracking(false); + private IQueryable ComplexFormTypes => dataModel.QueryLatest().AsTracking(false); + private IQueryable Senses => dataModel.QueryLatest().AsTracking(false); + private IQueryable ExampleSentences => dataModel.QueryLatest().AsTracking(false); + private IQueryable WritingSystems => dataModel.QueryLatest().AsTracking(false); + private IQueryable SemanticDomains => dataModel.QueryLatest().AsTracking(false); + private IQueryable PartsOfSpeech => dataModel.QueryLatest().AsTracking(false); public async Task GetWritingSystems() { From aab136637f2bdd42f570b58fb44566fd0f1883ae Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Mon, 25 Nov 2024 14:04:06 +0700 Subject: [PATCH 09/11] don't seed crdt db when creating to avoid sync issues on second sync --- .../Fixtures/SyncFixture.cs | 2 +- .../FwLiteProjectSync.Tests/SyncTests.cs | 23 +++++++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs b/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs index cb82ef31b..c2120c669 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs @@ -58,7 +58,7 @@ public async Task InitializeAsync() if (Path.Exists(crdtProjectsFolder)) Directory.Delete(crdtProjectsFolder, true); Directory.CreateDirectory(crdtProjectsFolder); var crdtProject = await _services.ServiceProvider.GetRequiredService() - .CreateProject(new(_projectName, FwProjectId: FwDataApi.ProjectId, SeedNewProjectData: true)); + .CreateProject(new(_projectName, FwProjectId: FwDataApi.ProjectId, SeedNewProjectData: false)); CrdtApi = (CrdtMiniLcmApi) await _services.ServiceProvider.OpenCrdtProject(crdtProject); } diff --git a/backend/FwLite/FwLiteProjectSync.Tests/SyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/SyncTests.cs index 07fc8123e..d10451967 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/SyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/SyncTests.cs @@ -65,6 +65,8 @@ public async Task DisposeAsync() { await _fixture.CrdtApi.DeleteEntry(entry.Id); } + + _fixture.DeleteSyncSnapshot(); } public SyncTests(SyncFixture fixture) @@ -84,7 +86,18 @@ public async Task FirstSyncJustDoesAnImport() var fwdataEntries = await fwdataApi.GetEntries().ToArrayAsync(); crdtEntries.Should().BeEquivalentTo(fwdataEntries, 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)); + } + + [Fact] + public async Task SecondSyncDoesNothing() + { + var crdtApi = _fixture.CrdtApi; + var fwdataApi = _fixture.FwDataApi; + await _syncService.Sync(crdtApi, fwdataApi); + var secondSync = await _syncService.SyncDryRun(crdtApi, fwdataApi); + secondSync.CrdtChanges.Should().Be(0, $"changes were {string.Join(", ", secondSync.CrdtDryRunRecords)}"); + secondSync.FwdataChanges.Should().Be(0, $"changes were {string.Join(", ", secondSync.FwDataDryRunRecords)}"); } [Fact] @@ -273,7 +286,7 @@ await fwdataApi.CreateEntry(new Entry() LexemeForm = { { "en", "Pear" } }, Senses = [ - new Sense() { Gloss = { { "en", "Pear" } }, SemanticDomains = [ semdom3 ] } + new Sense() { Gloss = { { "en", "Pear" } }, SemanticDomains = [semdom3] } ] }); await crdtApi.CreateEntry(new Entry() @@ -281,7 +294,7 @@ await crdtApi.CreateEntry(new Entry() LexemeForm = { { "en", "Banana" } }, Senses = [ - new Sense() { Gloss = { { "en", "Banana" } }, SemanticDomains = [ semdom3 ] } + new Sense() { Gloss = { { "en", "Banana" } }, SemanticDomains = [semdom3] } ] }); await _syncService.Sync(crdtApi, fwdataApi); @@ -365,7 +378,7 @@ public async Task AddingASenseToAnEntryInEachProjectSyncsAcrossBoth() await _syncService.Sync(crdtApi, fwdataApi); await fwdataApi.CreateSense(_testEntry.Id, new Sense() - { + { Gloss = { { "en", "Fruit" } }, Definition = { { "en", "a round fruit, red or yellow" } }, }); @@ -373,7 +386,7 @@ public async Task AddingASenseToAnEntryInEachProjectSyncsAcrossBoth() { Gloss = { { "en", "Tree" } }, Definition = { { "en", "a tall, woody plant, which grows fruit" } }, - }); + }); await _syncService.Sync(crdtApi, fwdataApi); From 3bc17fcabd0cb40be4c9f81ac045002cfa61e5a2 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Mon, 25 Nov 2024 14:04:59 +0700 Subject: [PATCH 10/11] change test project vernacular ws and change how ComplexFormComponent headwords get set so that they match what we get from fieldworks --- .../Fixtures/SyncFixture.cs | 2 +- .../JsonPatchEntryRewriteTests.cs | 2 -- .../Entries/AddEntryComponentChange.cs | 19 ++++++++++--------- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs b/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs index c2120c669..67cdf0548 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs @@ -50,7 +50,7 @@ public async Task InitializeAsync() if (Path.Exists(projectsFolder)) Directory.Delete(projectsFolder, true); Directory.CreateDirectory(projectsFolder); _services.ServiceProvider.GetRequiredService() - .NewProject(new FwDataProject(_projectName, projectsFolder), "en", "fr"); + .NewProject(new FwDataProject(_projectName, projectsFolder), "en", "en"); FwDataApi = _services.ServiceProvider.GetRequiredService().GetFwDataMiniLcmApi(_projectName, false); var crdtProjectsFolder = diff --git a/backend/FwLite/LcmCrdt.Tests/JsonPatchEntryRewriteTests.cs b/backend/FwLite/LcmCrdt.Tests/JsonPatchEntryRewriteTests.cs index b2370c85e..13eb26933 100644 --- a/backend/FwLite/LcmCrdt.Tests/JsonPatchEntryRewriteTests.cs +++ b/backend/FwLite/LcmCrdt.Tests/JsonPatchEntryRewriteTests.cs @@ -20,7 +20,6 @@ public void ChangesFromJsonPatch_AddComponentMakesAddEntryComponentChange() changes.Should().ContainSingle().Which.Should().BeOfType().Subject; addEntryComponentChange.ComplexFormEntryId.Should().Be(_entry.Id); addEntryComponentChange.ComponentEntryId.Should().Be(componentEntry.Id); - addEntryComponentChange.ComponentHeadword.Should().Be(componentEntry.Headword()); } [Fact] @@ -84,7 +83,6 @@ public void ChangesFromJsonPatch_AddComplexFormMakesAddEntryComponentChange() changes.Should().ContainSingle().Which.Should().BeOfType().Subject; addEntryComponentChange.ComplexFormEntryId.Should().Be(_entry.Id); addEntryComponentChange.ComponentEntryId.Should().Be(componentEntry.Id); - addEntryComponentChange.ComponentHeadword.Should().Be(componentEntry.Headword()); } [Fact] diff --git a/backend/FwLite/LcmCrdt/Changes/Entries/AddEntryComponentChange.cs b/backend/FwLite/LcmCrdt/Changes/Entries/AddEntryComponentChange.cs index a51313198..f93630058 100644 --- a/backend/FwLite/LcmCrdt/Changes/Entries/AddEntryComponentChange.cs +++ b/backend/FwLite/LcmCrdt/Changes/Entries/AddEntryComponentChange.cs @@ -10,10 +10,8 @@ namespace LcmCrdt.Changes.Entries; public class AddEntryComponentChange : CreateChange, ISelfNamedType { public Guid ComplexFormEntryId { get; } - public string? ComplexFormHeadword { get; } public Guid ComponentEntryId { get; } public Guid? ComponentSenseId { get; } - public string? ComponentHeadword { get; } [JsonConstructor] public AddEntryComponentChange(Guid entityId, @@ -24,9 +22,7 @@ public AddEntryComponentChange(Guid entityId, Guid? componentSenseId = null) : base(entityId) { ComplexFormEntryId = complexFormEntryId; - ComplexFormHeadword = complexFormHeadword; ComponentEntryId = componentEntryId; - ComponentHeadword = componentHeadword; ComponentSenseId = componentSenseId; } @@ -41,17 +37,22 @@ public AddEntryComponentChange(Guid entityId, public override async ValueTask NewEntity(Commit commit, ChangeContext context) { + var complexFormEntry = await context.GetCurrent(ComplexFormEntryId); + var componentEntry = await context.GetCurrent(ComponentEntryId); + Sense? componentSense = null; + if (ComponentSenseId is not null) + componentSense = await context.GetCurrent(ComponentSenseId.Value); return new ComplexFormComponent { Id = EntityId, ComplexFormEntryId = ComplexFormEntryId, - ComplexFormHeadword = ComplexFormHeadword, + ComplexFormHeadword = complexFormEntry?.Headword(), ComponentEntryId = ComponentEntryId, - ComponentHeadword = ComponentHeadword, + ComponentHeadword = componentEntry?.Headword(), ComponentSenseId = ComponentSenseId, - DeletedAt = (await context.IsObjectDeleted(ComponentEntryId) || - await context.IsObjectDeleted(ComplexFormEntryId) || - ComponentSenseId.HasValue && await context.IsObjectDeleted(ComponentSenseId.Value)) + DeletedAt = (complexFormEntry?.DeletedAt is not null || + componentEntry?.DeletedAt is not null || + (ComponentSenseId.HasValue && componentSense?.DeletedAt is not null)) ? commit.DateTime : (DateTime?)null, }; From a253a8d70300bf300400790e143dd2f81659dd86 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Mon, 25 Nov 2024 14:12:20 +0700 Subject: [PATCH 11/11] fix test failing due to no complex form types existing --- backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs index d1b264839..50d9f56f8 100644 --- a/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs +++ b/backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs @@ -95,8 +95,8 @@ public async Task CanChangeComplexFormViaSync_ComplexForms() [Fact] public async Task CanChangeComplexFormTypeViaSync() { + var complexFormType = await _fixture.CrdtApi.CreateComplexFormType(new() { Name = new() { { "en", "complexFormType" } } }); var entry = await _fixture.CrdtApi.CreateEntry(new() { LexemeForm = { { "en", "complexForm1" } } }); - var complexFormType = await _fixture.CrdtApi.GetComplexFormTypes().FirstAsync(); var after = (Entry) entry.Copy(); after.ComplexFormTypes = [complexFormType]; await EntrySync.Sync(after, entry, _fixture.CrdtApi);